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

Petr Hosek via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Aug 5 10:57:18 PDT 2023


phosek added inline comments.


================
Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp:908-910
-    uint32_t FilenamesSize =
-        CovHeader->getFilenamesSize<support::endianness::little>();
-    uint32_t CoverageMappingSize = sizeof(CovMapHeader) + FilenamesSize;
----------------
AtomicGu wrote:
> phosek wrote:
> > It looks like in the current implementation, we're using `getFilenameSize` to compute the `CoverageMappingSize`, but `CovMapHeader` also contains `CoverageMappingSize`, accessible through `getCoverageSize()`. Shouldn't we be using that instead? Isn't that the issue with the current implementation?
> I have inspected this variable. The value is not right. It is always zero, as the documentation says:
> 
> > The length of the string in the third field of __llvm_coverage_mapping that contains any encoded coverage mapping data affixed to the coverage header. Always 0, but present for backwards compatibility.
If it's not being used, could we reuse it rather than introducing a new field?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156611/new/

https://reviews.llvm.org/D156611



More information about the llvm-commits mailing list