[PATCH] D156611: [llvm-cov] Fix a bug about using `convert-for-testing` on multi-source object files

[Yuhao Gu via Phabricator via llvm-commits]

AtomicGu created this revision.
AtomicGu added reviewers: phosek, gulfem, davidxl.
Herald added a subscriber: hiraditya.
Herald added a project: All.
AtomicGu requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

`llvm-cov convert-for-testing` is used to build the .covmapping files used in its regression tests.
However the current implementation only works when there's only one source file in the mapping information data.
If there are more than 1 source files, `llvm-cov convert-for-testing` can still produce a .covmapping file,
but when read it back, `llvm-cov` will report:

  error: Failed to load coverage: 'main.covmapping': Malformed coverage data

This is because the output .covmapping file doesn't have any mark to indicate the boundary between file records
and function records, and current implementation jsut assume there's only one file record in the .covmapping file.

Changes to the code:

- Make `llvm-cov convert-for-testing` output a LEB128 number before file records to indicate its size.
- Change the testing format parsing code correspondingly.
- Update existing .covmapping files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156611

Files:
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/test/tools/llvm-cov/Inputs/combine_expansions.covmapping
  llvm/test/tools/llvm-cov/Inputs/compilation_dir.covmapping
  llvm/test/tools/llvm-cov/Inputs/coverage_prefix_map/main.covmapping
  llvm/test/tools/llvm-cov/Inputs/deferred-regions.covmapping
  llvm/test/tools/llvm-cov/Inputs/dir-with-filtering.covmapping
  llvm/test/tools/llvm-cov/Inputs/double_dots.covmapping
  llvm/test/tools/llvm-cov/Inputs/highlightedRanges.covmapping
  llvm/test/tools/llvm-cov/Inputs/ifdef.covmapping
  llvm/test/tools/llvm-cov/Inputs/lineExecutionCounts.covmapping
  llvm/test/tools/llvm-cov/Inputs/malformedRegions.covmapping
  llvm/test/tools/llvm-cov/Inputs/multiple-files.covmapping
  llvm/test/tools/llvm-cov/Inputs/multiple-files2.covmapping
  llvm/test/tools/llvm-cov/Inputs/multiple_objects/use_1.covmapping
  llvm/test/tools/llvm-cov/Inputs/multiple_objects/use_2.covmapping
  llvm/test/tools/llvm-cov/Inputs/multithreaded_report/main.covmapping
  llvm/test/tools/llvm-cov/Inputs/name_allowlist.covmapping
  llvm/test/tools/llvm-cov/Inputs/name_whitelist.covmapping
  llvm/test/tools/llvm-cov/Inputs/native_separators.covmapping
  llvm/test/tools/llvm-cov/Inputs/path_equivalence.covmapping
  llvm/test/tools/llvm-cov/Inputs/prefer_used_to_unused.covmapping
  llvm/test/tools/llvm-cov/Inputs/prevent_false_instantiations.covmapping
  llvm/test/tools/llvm-cov/Inputs/regionMarkers.covmapping
  llvm/test/tools/llvm-cov/Inputs/relative_dir/main.covmapping
  llvm/test/tools/llvm-cov/Inputs/report.covmapping
  llvm/test/tools/llvm-cov/Inputs/showExpansions.covmapping
  llvm/test/tools/llvm-cov/Inputs/showProjectSummary.covmapping
  llvm/test/tools/llvm-cov/Inputs/showTabsHTML.covmapping
  llvm/test/tools/llvm-cov/Inputs/sources_specified/main.covmapping
  llvm/test/tools/llvm-cov/Inputs/templateInstantiations.covmapping
  llvm/test/tools/llvm-cov/Inputs/zeroFunctionFile.covmapping
  llvm/tools/llvm-cov/TestingSupport.cpp


Index: llvm/tools/llvm-cov/TestingSupport.cpp
===================================================================
--- llvm/tools/llvm-cov/TestingSupport.cpp
+++ llvm/tools/llvm-cov/TestingSupport.cpp
@@ -111,6 +111,7 @@
   encodeULEB128(ProfileNamesData.size(), OS);
   encodeULEB128(ProfileNamesAddress, OS);
   OS << ProfileNamesData;
+  encodeULEB128(CoverageMappingData.size(), OS);
   // Coverage mapping data is expected to have an alignment of 8.
   for (unsigned Pad = offsetToAlignment(OS.tell(), Align(8)); Pad; --Pad)
     OS.write(uint8_t(0));
Index: llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
===================================================================
--- llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -886,41 +886,31 @@
   if (Error E = ProfileNames.create(Data.substr(0, ProfileNamesSize), Address))
     return std::move(E);
   Data = Data.substr(ProfileNamesSize);
+
+  N = 0;
+  uint64_t CoverageMappingSize = decodeULEB128(Data.bytes_begin(), &N);
+  if (N > Data.size())
+    return make_error<CoverageMapError>(coveragemap_error::malformed);
+  Data = Data.substr(N);
+
   // Skip the padding bytes because coverage map data has an alignment of 8.
   size_t Pad = offsetToAlignedAddr(Data.data(), Align(8));
   if (Data.size() < Pad)
     return make_error<CoverageMapError>(coveragemap_error::malformed);
   Data = Data.substr(Pad);
-  if (Data.size() < sizeof(CovMapHeader))
+  if (Data.size() < CoverageMappingSize)
     return make_error<CoverageMapError>(coveragemap_error::malformed);
-  auto const *CovHeader = reinterpret_cast<const CovMapHeader *>(
-      Data.substr(0, sizeof(CovMapHeader)).data());
-  CovMapVersion Version =
-      (CovMapVersion)CovHeader->getVersion<support::endianness::little>();
-  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>();
-    uint32_t CoverageMappingSize = sizeof(CovMapHeader) + FilenamesSize;
-    CoverageMapping = Data.substr(0, CoverageMappingSize);
-    if (CoverageMapping.empty())
-      return make_error<CoverageMapError>(coveragemap_error::truncated);
-    Data = Data.substr(CoverageMappingSize);
-    // Skip the padding bytes because coverage records data has an alignment
-    // of 8.
-    Pad = offsetToAlignedAddr(Data.data(), Align(8));
-    if (Data.size() < Pad)
-      return make_error<CoverageMapError>(coveragemap_error::malformed);
-    CoverageRecords = MemoryBuffer::getMemBuffer(Data.substr(Pad));
-    if (CoverageRecords->getBufferSize() == 0)
-      return make_error<CoverageMapError>(coveragemap_error::truncated);
-  }
+  StringRef CoverageMapping = Data.substr(0, CoverageMappingSize);
+  Data = Data.substr(CoverageMappingSize);
+
+  // Skip the padding bytes because coverage records data has an alignment of 8.
+  Pad = offsetToAlignedAddr(Data.data(), Align(8));
+  if (Data.size() < Pad)
+    return make_error<CoverageMapError>(coveragemap_error::malformed);
+  Data = Data.substr(Pad);
+  BinaryCoverageReader::FuncRecordsStorage CoverageRecords =
+      MemoryBuffer::getMemBuffer(Data);
+
   return BinaryCoverageReader::createCoverageReaderFromBuffer(
       CoverageMapping, std::move(CoverageRecords), std::move(ProfileNames),
       BytesInAddress, Endian, CompilationDir);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D156611.545428.patch
Type: text/x-patch
Size: 3670 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230730/bd027594/attachment.bin>