[memprof] Align each rawprofile section to 8b.

[snehasish]

Summary

The first 8b of each raw profile section need to be aligned to 8b since
the first item in each section is a u64 count of the number of items in
the section.
Summary of changes:

• Assert alignment when reading counts.
• Update test to check alignment, relax some size checks to allow padding.
• Update raw binary inputs for llvm-profdata tests.

[tejohnson]

compiler-rt/lib/memprof/memprof_rawprofile.cpp

36

Should this be Size + 8 - Size%8?

[kda]

This does appear to fix the "fast" buildbot.
Thank you.

[vitalybuka]

compiler-rt/lib/memprof/memprof_rawprofile.cpp

36

RoundUpTo(size, 8)

BTW why 8? should this be alignof(T) ?

[snehasish]

Use RoundUpTo helper, add comments to clarify why 8b.

[snehasish]

PTAL, thanks!

compiler-rt/lib/memprof/memprof_rawprofile.cpp

36

Since each section in the profile starts with a u64, I hardcoded the value to 8. I added a comment to explain why.

[vitalybuka]

In D114826#3163083, @snehasish wrote:

PTAL, thanks!

It does not look like a trivial fix.
If you need to wait @davidxl review I recommend to revert original patch and squash the fix into that one to unbreak buildbots

BTW there is also https://lab.llvm.org/buildbot/#/builders/85/builds/7051/steps/7/logs/stdio

[davidxl]

lgtm

[snehasish]

In D114826#3163128, @vitalybuka wrote:

In D114826#3163083, @snehasish wrote:

PTAL, thanks!

It does not look like a trivial fix.
If you need to wait @davidxl review I recommend to revert original patch and squash the fix into that one to unbreak buildbots

BTW there is also https://lab.llvm.org/buildbot/#/builders/85/builds/7051/steps/7/logs/stdio

The breakage above should have been fixed by https://reviews.llvm.org/rGa2ce97cc3f99c8af80b2325acf03a4c171ffb48f.
I'll go ahead and pushed this since David lgtm'ed it and keep an eye on the bot. Thanks for taking a look and your helpful suggestions.

compiler-rt/lib/memprof/memprof_rawprofile.cpp

 #include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
 
 #include "memprof_meminfoblock.h"
 #include "memprof_rawprofile.h"
 #include "profile/MemProfData.inc"
 #include "sanitizer_common/sanitizer_allocator_internal.h"
+#include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_linux.h"
 #include "sanitizer_common/sanitizer_procmaps.h"
 #include "sanitizer_common/sanitizer_stackdepot.h"
 #include "sanitizer_common/sanitizer_stackdepotbase.h"
 #include "sanitizer_common/sanitizer_stacktrace.h"
 #include "sanitizer_common/sanitizer_vector.h"
 
 namespace __memprof {

 void RecordStackId(const uptr Key, UNUSED LockedMemInfoBlock *const &MIB,
                    void *Arg) {
   // No need to touch the MIB value here since we are only recording the key.
   auto *StackIds = reinterpret_cast<Vector<u64> *>(Arg);
   StackIds->PushBack(Key);
 }
 } // namespace
 
 u64 SegmentSizeBytes(MemoryMappingLayoutBase &Layout) {
   u64 NumSegmentsToRecord = 0;
   MemoryMappedSegment segment;
   for (Layout.Reset(); Layout.Next(&segment);)
     if (segment.IsReadable() && segment.IsExecutable())
       NumSegmentsToRecord++;
 
   return sizeof(u64) // A header which stores the number of records.
          + sizeof(SegmentEntry) * NumSegmentsToRecord;
if (segment.IsReadable() && segment.IsExecutable()) {
       memcpy(Ptr, &Entry, sizeof(SegmentEntry));
       Ptr += sizeof(SegmentEntry);
       NumSegmentsRecorded++;
     }
   }
 
   // Store the number of segments we recorded in the space we reserved.
   *((u64 *)Buffer) = NumSegmentsRecorded;
-  CHECK(ExpectedNumBytes == static_cast<u64>(Ptr - Buffer) &&
+  CHECK(ExpectedNumBytes >= static_cast<u64>(Ptr - Buffer) &&
         "Expected num bytes != actual bytes written");
 }
 
 u64 StackSizeBytes(const Vector<u64> &StackIds) {
   u64 NumBytesToWrite = sizeof(u64);
 
   const u64 NumIds = StackIds.Size();
   for (unsigned k = 0; k < NumIds; ++k) {
for (uptr i = 0; i < St.size && St.trace[i] != 0; i++) {
       uptr pc = StackTrace::GetPreviousInstructionPc(St.trace[i]);
       Ptr = WriteBytes(static_cast<u64>(pc), Ptr);
       ++Count;
     }
     // Store the count in the space we reserved earlier.
     *(u64 *)(Ptr - (Count + 1) * sizeof(u64)) = Count;
   }
 
-  CHECK(ExpectedNumBytes == static_cast<u64>(Ptr - Buffer) &&
+  CHECK(ExpectedNumBytes >= static_cast<u64>(Ptr - Buffer) &&
         "Expected num bytes != actual bytes written");
 }
 
 // The MIB section has the following format:
 // ---------- MIB Info
 // Num Entries
 // ---------- MIB Entry 0
 // Alloc Count
void SerializeMIBInfoToBuffer(MIBMapTy &MIBMap, const Vector<u64> &StackIds,
   for (u64 i = 0; i < NumEntries; i++) {
     const u64 Key = StackIds[i];
     MIBMapTy::Handle h(&MIBMap, Key, /*remove=*/true, /*create=*/false);
     CHECK(h.exists());
     Ptr = WriteBytes(Key, Ptr);
     Ptr = WriteBytes((*h)->mib, Ptr);
   }
 
-  CHECK(ExpectedNumBytes == static_cast<u64>(Ptr - Buffer) &&
+  CHECK(ExpectedNumBytes >= static_cast<u64>(Ptr - Buffer) &&
         "Expected num bytes != actual bytes written");
 }
 
 // Format
 // ---------- Header
 // Magic
 // Version
 // Total Size
 // Segment Offset
 // MIB Info Offset
 // Stack Offset
 // ---------- Segment Info
 // Num Entries
 // ---------- Segment Entry
 // Start
 // End
 // Offset
 // BuildID 32B
 // ----------
 // ...
+// ----------
+// Optional Padding Bytes
 // ---------- MIB Info
 // Num Entries
 // ---------- MIB Entry
 // Alloc Count
 // ...
+// ----------
+// Optional Padding Bytes
 // ---------- Stack Info
 // Num Entries
 // ---------- Stack Entry
 // Num Stacks
 // PC1
 // PC2
 // ...
 // ----------
+// Optional Padding Bytes
 // ...
 u64 SerializeToRawProfile(MIBMapTy &MIBMap, MemoryMappingLayoutBase &Layout,
                           char *&Buffer) {
-  const u64 NumSegmentBytes = SegmentSizeBytes(Layout);
+  // Each section size is rounded up to 8b since the first entry in each section
+  // is a u64 which holds the number of entries in the section by convention.
+  const u64 NumSegmentBytes = RoundUpTo(SegmentSizeBytes(Layout), 8);
 
   Vector<u64> StackIds;
   MIBMap.ForEach(RecordStackId, reinterpret_cast<void *>(&StackIds));
   // The first 8b are for the total number of MIB records. Each MIB record is
   // preceded by a 8b stack id which is associated with stack frames in the next
   // section.
-  const u64 NumMIBInfoBytes =
-      sizeof(u64) + StackIds.Size() * (sizeof(u64) + sizeof(MemInfoBlock));
+  const u64 NumMIBInfoBytes = RoundUpTo(
+      sizeof(u64) + StackIds.Size() * (sizeof(u64) + sizeof(MemInfoBlock)), 8);
 
-  const u64 NumStackBytes = StackSizeBytes(StackIds);
+  const u64 NumStackBytes = RoundUpTo(StackSizeBytes(StackIds), 8);
 
-  const u64 TotalSizeBytes =
-      sizeof(Header) + NumSegmentBytes + NumStackBytes + NumMIBInfoBytes;
+  // Ensure that the profile is 8b aligned. We allow for some optional padding
+  // at the end so that any subsequent profile serialized to the same file does
+  // not incur unaligned accesses.
+  const u64 TotalSizeBytes = RoundUpTo(
+      sizeof(Header) + NumSegmentBytes + NumStackBytes + NumMIBInfoBytes, 8);
 
   // Allocate the memory for the entire buffer incl. info blocks.
   Buffer = (char *)InternalAlloc(TotalSizeBytes);
   char *Ptr = Buffer;
 
   Header header{MEMPROF_RAW_MAGIC_64,
                 MEMPROF_RAW_VERSION,
                 static_cast<u64>(TotalSizeBytes),

compiler-rt/lib/memprof/tests/rawprofile.cpp

u64 PopulateFakeMap(const MemInfoBlock &FakeMIB, uptr StackPCBegin,
   u32 Id = StackDepotPut(St);
 
   InsertOrMerge(Id, FakeMIB, FakeMap);
   return Id;
 }
 
 template <class T = u64> T Read(char *&Buffer) {
   static_assert(std::is_pod<T>::value, "Must be a POD type.");
+  assert(reinterpret_cast<size_t>(Buffer) % sizeof(T) == 0 &&
+         "Unaligned read!");
   T t = *reinterpret_cast<T *>(Buffer);
   Buffer += sizeof(T);
   return t;
 }
 
 TEST(MemProf, Basic) {
   MockMemoryMappingLayout Layout;
   MemoryMappedSegment FakeSegment;
TEST(MemProf, Basic) {
   // Check the header.
   EXPECT_THAT(Read(Ptr), MEMPROF_RAW_MAGIC_64);
   EXPECT_THAT(Read(Ptr), MEMPROF_RAW_VERSION);
   const u64 TotalSize = Read(Ptr);
   const u64 SegmentOffset = Read(Ptr);
   const u64 MIBOffset = Read(Ptr);
   const u64 StackOffset = Read(Ptr);
 
-  // ============= Check sizes.
+  // ============= Check sizes and padding.
   EXPECT_EQ(TotalSize, NumBytes);
+  EXPECT_EQ(TotalSize % 8, 0ULL);
 
   // Should be equal to the size of the raw profile header.
   EXPECT_EQ(SegmentOffset, 48ULL);
 
   // We expect only 1 segment entry, 8b for the count and 56b for SegmentEntry
   // in memprof_rawprofile.cpp.
   EXPECT_EQ(MIBOffset - SegmentOffset, 64ULL);
 
   EXPECT_EQ(MIBOffset, 112ULL);
   // We expect 2 mib entry, 8b for the count and sizeof(u64) +
   // sizeof(MemInfoBlock) contains stack id + MeminfoBlock.
   EXPECT_EQ(StackOffset - MIBOffset, 8 + 2 * (8 + sizeof(MemInfoBlock)));
 
   EXPECT_EQ(StackOffset, 336ULL);
   // We expect 2 stack entries, with 5 frames - 8b for total count,
-  // 2 * (8b for id, 8b for frame count and 5*8b for fake frames)
-  EXPECT_EQ(TotalSize - StackOffset, 8ULL + 2 * (8 + 8 + 5 * 8));
+  // 2 * (8b for id, 8b for frame count and 5*8b for fake frames).
+  // Since this is the last section, there may be additional padding at the end
+  // to make the total profile size 8b aligned.
+  EXPECT_GE(TotalSize - StackOffset, 8ULL + 2 * (8 + 8 + 5 * 8));
 
   // ============= Check contents.
   unsigned char ExpectedSegmentBytes[64] = {
       0x01, 0,   0,   0,   0,   0,   0, 0, // Number of entries
       0x10, 0,   0,   0,   0,   0,   0, 0, // Start
       0x20, 0,   0,   0,   0,   0,   0, 0, // End
       0x10, 0,   0,   0,   0,   0,   0, 0, // Offset
       0x0C, 0x0, 0xF, 0xF, 0xE, 0xE,       // Uuid

llvm/lib/ProfileData/RawMemProfReader.cpp

 //===- RawMemProfReader.cpp - Instrumented memory profiling reader --------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file contains support for reading MemProf profiling data.
 //
 //===----------------------------------------------------------------------===//
 
 #include <cstdint>
+#include <type_traits>
 
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/ProfileData/MemProfData.inc"
 #include "llvm/ProfileData/RawMemProfReader.h"
 
 namespace llvm {
 namespace memprof {
 namespace {
 
 struct Summary {
   uint64_t Version;
   uint64_t TotalSizeBytes;
   uint64_t NumSegments;
   uint64_t NumMIBInfo;
   uint64_t NumStackOffsets;
 };
 
+template <class T = uint64_t> inline T alignedRead(const char *Ptr) {
+  static_assert(std::is_pod<T>::value, "Not a pod type.");
+  assert(reinterpret_cast<size_t>(Ptr) % sizeof(T) == 0 && "Unaligned Read");
+  return *reinterpret_cast<const T *>(Ptr);
+}
+
 Summary computeSummary(const char *Start) {
   auto *H = reinterpret_cast<const Header *>(Start);
 
+  // Check alignment while reading the number of items in each section.
   return Summary{
       H->Version,
       H->TotalSize,
-      *reinterpret_cast<const uint64_t *>(Start + H->SegmentOffset),
-      *reinterpret_cast<const uint64_t *>(Start + H->MIBOffset),
-      *reinterpret_cast<const uint64_t *>(Start + H->StackOffset),
+      alignedRead(Start + H->SegmentOffset),
+      alignedRead(Start + H->MIBOffset),
+      alignedRead(Start + H->StackOffset),
   };
 }
 
 } // namespace
 
 Expected<std::unique_ptr<RawMemProfReader>>
 RawMemProfReader::create(const Twine &Path) {
   auto BufferOr = MemoryBuffer::getFileOrSTDIN(Path, /*IsText=*/true);
RawMemProfReader::create(const Twine &Path) {
   }
 
   return std::make_unique<RawMemProfReader>(std::move(Buffer));
 }
 
 bool RawMemProfReader::hasFormat(const MemoryBuffer &Buffer) {
   if (Buffer.getBufferSize() < sizeof(uint64_t))
     return false;
-  uint64_t Magic = *reinterpret_cast<const uint64_t *>(Buffer.getBufferStart());
+  // Aligned read to sanity check that the buffer was allocated with at least 8b
+  // alignment.
+  const uint64_t Magic = alignedRead(Buffer.getBufferStart());
   return Magic == MEMPROF_RAW_MAGIC_64;
 }
 
 void RawMemProfReader::printSummaries(raw_ostream &OS) const {
   int Count = 0;
   const char *Next = DataBuffer->getBufferStart();
   while (Next < DataBuffer->getBufferEnd()) {
     auto Summary = computeSummary(Next);

llvm/test/tools/llvm-profdata/memprof-basic.test

 
 RUN: llvm-profdata show --memory %p/Inputs/basic.memprofraw -o - | FileCheck %s
 
 We expect 3 MIB entries, 1 each for the malloc calls in the program and one
 additional entry from a realloc in glibc/libio/vasprintf.c.
 
 CHECK: MemProf Profile 1
 CHECK:   Version: 1
-CHECK:   TotalSizeBytes: 1012
+CHECK:   TotalSizeBytes: 1016
 CHECK:   NumSegments: 9
 CHECK:   NumMIBInfo: 3
 CHECK:   NumStackOffsets: 3