[llvm] effc333 - [llvm-cov] Enforce alignment of function records

[via llvm-commits]

Author: serge-sans-paille
Date: 2021-06-25T10:56:06+02:00
New Revision: effc3339f6c7702357471efc849c4fc31b4b1aad

URL: https://github.com/llvm/llvm-project/commit/effc3339f6c7702357471efc849c4fc31b4b1aad
DIFF: https://github.com/llvm/llvm-project/commit/effc3339f6c7702357471efc849c4fc31b4b1aad.diff

LOG: [llvm-cov] Enforce alignment of function records

Function Records are required to be aligned on 8 bytes. This is enforced for each
records except the first, when one relies on the default alignment within an
std::string. There's no such guarantee, and indeed on 32 bits for some
implementation of std::string this is not enforced.

Provide a portable implementation based on llvm's MemoryBuffer.

Differential Revision: https://reviews.llvm.org/D104745

Added: 
    

Modified: 
    llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
    llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
index eb71bf6c8975c..242ffdd16e3f2 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
@@ -179,6 +179,8 @@ class BinaryCoverageReader : public CoverageMappingReader {
           FilenamesBegin(FilenamesBegin), FilenamesSize(FilenamesSize) {}
   };
 
+  using FuncRecordsStorage = std::unique_ptr<MemoryBuffer>;
+
 private:
   std::vector<std::string> Filenames;
   std::vector<ProfileMappingRecord> MappingRecords;
@@ -191,9 +193,9 @@ class BinaryCoverageReader : public CoverageMappingReader {
   // Used to tie the lifetimes of coverage function records to the lifetime of
   // this BinaryCoverageReader instance. Needed to support the format change in
   // D69471, which can split up function records into multiple sections on ELF.
-  std::string FuncRecords;
+  FuncRecordsStorage FuncRecords;
 
-  BinaryCoverageReader(std::string &&FuncRecords)
+  BinaryCoverageReader(FuncRecordsStorage &&FuncRecords)
       : FuncRecords(std::move(FuncRecords)) {}
 
 public:
@@ -206,7 +208,8 @@ class BinaryCoverageReader : public CoverageMappingReader {
          StringRef CompilationDir = "");
 
   static Expected<std::unique_ptr<BinaryCoverageReader>>
-  createCoverageReaderFromBuffer(StringRef Coverage, std::string &&FuncRecords,
+  createCoverageReaderFromBuffer(StringRef Coverage,
+                                 FuncRecordsStorage &&FuncRecords,
                                  InstrProfSymtab &&ProfileNames,
                                  uint8_t BytesInAddress,
                                  support::endianness Endian,

diff  --git a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
index 4fbcefd236cbe..9fffb249e72db 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -824,13 +824,13 @@ static const char *TestingFormatMagic = "llvmcovmtestdata";
 
 Expected<std::unique_ptr<BinaryCoverageReader>>
 BinaryCoverageReader::createCoverageReaderFromBuffer(
-    StringRef Coverage, std::string &&FuncRecords,
+    StringRef Coverage, FuncRecordsStorage &&FuncRecords,
     InstrProfSymtab &&ProfileNames, uint8_t BytesInAddress,
     support::endianness Endian, StringRef CompilationDir) {
   std::unique_ptr<BinaryCoverageReader> Reader(
       new BinaryCoverageReader(std::move(FuncRecords)));
   Reader->ProfileNames = std::move(ProfileNames);
-  StringRef FuncRecordsRef = Reader->FuncRecords;
+  StringRef FuncRecordsRef = Reader->FuncRecords->getBuffer();
   if (BytesInAddress == 4 && Endian == support::endianness::little) {
     if (Error E =
             readCoverageMappingData<uint32_t, support::endianness::little>(
@@ -895,11 +895,13 @@ loadTestingFormat(StringRef Data, StringRef CompilationDir) {
       Data.substr(0, sizeof(CovMapHeader)).data());
   CovMapVersion Version =
       (CovMapVersion)CovHeader->getVersion<support::endianness::little>();
-  StringRef CoverageMapping, CoverageRecords;
+  StringRef CoverageMapping;
+  BinaryCoverageReader::FuncRecordsStorage CoverageRecords;
   if (Version < CovMapVersion::Version4) {
     CoverageMapping = Data;
     if (CoverageMapping.empty())
       return make_error<CoverageMapError>(coveragemap_error::truncated);
+    CoverageRecords = MemoryBuffer::getMemBuffer("");
   } else {
     uint32_t FilenamesSize =
         CovHeader->getFilenamesSize<support::endianness::little>();
@@ -913,12 +915,12 @@ loadTestingFormat(StringRef Data, StringRef CompilationDir) {
     Pad = offsetToAlignedAddr(Data.data(), Align(8));
     if (Data.size() < Pad)
       return make_error<CoverageMapError>(coveragemap_error::malformed);
-    CoverageRecords = Data.substr(Pad);
-    if (CoverageRecords.empty())
+    CoverageRecords = MemoryBuffer::getMemBuffer(Data.substr(Pad));
+    if (CoverageRecords->getBufferSize() == 0)
       return make_error<CoverageMapError>(coveragemap_error::truncated);
   }
   return BinaryCoverageReader::createCoverageReaderFromBuffer(
-      CoverageMapping, CoverageRecords.str(), std::move(ProfileNames),
+      CoverageMapping, std::move(CoverageRecords), std::move(ProfileNames),
       BytesInAddress, Endian, CompilationDir);
 }
 
@@ -1003,21 +1005,49 @@ loadBinaryFormat(std::unique_ptr<Binary> Bin, StringRef Arch,
     return std::move(E);
 
   // Look for the coverage records section (Version4 only).
-  std::string FuncRecords;
   auto CoverageRecordsSections =
       lookupSections(*OF, getInstrProfSectionName(IPSK_covfun, ObjFormat,
                                                   /*AddSegmentInfo=*/false));
-  if (auto E = CoverageRecordsSections.takeError())
+
+  BinaryCoverageReader::FuncRecordsStorage FuncRecords;
+  if (auto E = CoverageRecordsSections.takeError()) {
     consumeError(std::move(E));
-  else {
+    FuncRecords = MemoryBuffer::getMemBuffer("");
+  } else {
+    // Compute the FuncRecordsBuffer of the buffer, taking into account the
+    // padding between each record, and making sure the first block is aligned
+    // in memory to maintain consistency between buffer address and size
+    // alignment.
+    const Align RecordAlignment(8);
+    uint64_t FuncRecordsSize = 0;
+    for (SectionRef Section : *CoverageRecordsSections) {
+      auto CoverageRecordsOrErr = Section.getContents();
+      if (!CoverageRecordsOrErr)
+        return CoverageRecordsOrErr.takeError();
+      FuncRecordsSize += alignTo(CoverageRecordsOrErr->size(), RecordAlignment);
+    }
+    auto WritableBuffer =
+        WritableMemoryBuffer::getNewUninitMemBuffer(FuncRecordsSize);
+    char *FuncRecordsBuffer = WritableBuffer->getBufferStart();
+    assert(isAddrAligned(RecordAlignment, FuncRecordsBuffer) &&
+           "Allocated memory is correctly aligned");
+
     for (SectionRef Section : *CoverageRecordsSections) {
       auto CoverageRecordsOrErr = Section.getContents();
       if (!CoverageRecordsOrErr)
         return CoverageRecordsOrErr.takeError();
-      FuncRecords += CoverageRecordsOrErr.get();
-      while (FuncRecords.size() % 8 != 0)
-        FuncRecords += '\0';
+      const auto &CoverageRecords = CoverageRecordsOrErr.get();
+      FuncRecordsBuffer = std::copy(CoverageRecords.begin(),
+                                    CoverageRecords.end(), FuncRecordsBuffer);
+      FuncRecordsBuffer =
+          std::fill_n(FuncRecordsBuffer,
+                      alignAddr(FuncRecordsBuffer, RecordAlignment) -
+                          (uintptr_t)FuncRecordsBuffer,
+                      '\0');
     }
+    assert(FuncRecordsBuffer == WritableBuffer->getBufferEnd() &&
+           "consistent init");
+    FuncRecords = std::move(WritableBuffer);
   }
 
   return BinaryCoverageReader::createCoverageReaderFromBuffer(