[llvm] r270194 - [Coverage] Fix an issue where improper coverage mapping data could be loaded for an inline function.
Igor Kudrin via llvm-commits
llvm-commits at lists.llvm.org
Fri May 20 02:14:25 PDT 2016
Author: ikudrin
Date: Fri May 20 04:14:24 2016
New Revision: 270194
URL: http://llvm.org/viewvc/llvm-project?rev=270194&view=rev
Log:
[Coverage] Fix an issue where improper coverage mapping data could be loaded for an inline function.
If an inline function is observed but unused in a translation unit, dummy
coverage mapping data with zero hash is stored for this function.
If such a coverage mapping section came earlier than real one, the latter
was ignored. As a result, llvm-cov was unable to show coverage information
for those functions.
Differential Revision: http://reviews.llvm.org/D20286
Added:
llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.covmapping
llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.cpp
llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.proftext
llvm/trunk/test/tools/llvm-cov/prefer_used_to_unused.h
Modified:
llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
llvm/trunk/lib/ProfileData/Coverage/CoverageMappingReader.cpp
Modified: llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMappingReader.h?rev=270194&r1=270193&r2=270194&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMappingReader.h (original)
+++ llvm/trunk/include/llvm/ProfileData/Coverage/CoverageMappingReader.h Fri May 20 04:14:24 2016
@@ -103,6 +103,16 @@ public:
Error read();
};
+/// \brief Checks if the given coverage mapping data is exported for
+/// an unused function.
+class RawCoverageMappingDummyChecker : public RawCoverageReader {
+public:
+ RawCoverageMappingDummyChecker(StringRef MappingData)
+ : RawCoverageReader(MappingData) {}
+
+ Expected<bool> isDummy();
+};
+
/// \brief Reader for the raw coverage mapping data.
class RawCoverageMappingReader : public RawCoverageReader {
ArrayRef<StringRef> TranslationUnitFilenames;
Modified: llvm/trunk/lib/ProfileData/Coverage/CoverageMappingReader.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/Coverage/CoverageMappingReader.cpp?rev=270194&r1=270193&r2=270194&view=diff
==============================================================================
--- llvm/trunk/lib/ProfileData/Coverage/CoverageMappingReader.cpp (original)
+++ llvm/trunk/lib/ProfileData/Coverage/CoverageMappingReader.cpp Fri May 20 04:14:24 2016
@@ -13,7 +13,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/ProfileData/Coverage/CoverageMappingReader.h"
-#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/DenseMap.h"
#include "llvm/Object/MachOUniversal.h"
#include "llvm/Object/ObjectFile.h"
#include "llvm/Support/Debug.h"
@@ -294,6 +294,36 @@ Error RawCoverageMappingReader::read() {
return Error::success();
}
+Expected<bool> RawCoverageMappingDummyChecker::isDummy() {
+ // A dummy coverage mapping data consists of just one region with zero count.
+ uint64_t NumFileMappings;
+ if (Error Err = readSize(NumFileMappings))
+ return std::move(Err);
+ if (NumFileMappings != 1)
+ return false;
+ // We don't expect any specific value for the filename index, just skip it.
+ uint64_t FilenameIndex;
+ if (Error Err =
+ readIntMax(FilenameIndex, std::numeric_limits<unsigned>::max()))
+ return std::move(Err);
+ uint64_t NumExpressions;
+ if (Error Err = readSize(NumExpressions))
+ return std::move(Err);
+ if (NumExpressions != 0)
+ return false;
+ uint64_t NumRegions;
+ if (Error Err = readSize(NumRegions))
+ return std::move(Err);
+ if (NumRegions != 1)
+ return false;
+ uint64_t EncodedCounterAndRegion;
+ if (Error Err = readIntMax(EncodedCounterAndRegion,
+ std::numeric_limits<unsigned>::max()))
+ return std::move(Err);
+ unsigned Tag = EncodedCounterAndRegion & Counter::EncodingTagMask;
+ return Tag == Counter::Zero;
+}
+
Error InstrProfSymtab::create(SectionRef &Section) {
if (auto EC = Section.getContents(Data))
return errorCodeToError(EC);
@@ -310,6 +340,14 @@ StringRef InstrProfSymtab::getFuncName(u
return Data.substr(Pointer - Address, Size);
}
+// Check if the mapping data is a dummy, i.e. is emitted for an unused function.
+static Expected<bool> isCoverageMappingDummy(uint64_t Hash, StringRef Mapping) {
+ // The hash value of dummy mapping records is always zero.
+ if (Hash)
+ return false;
+ return RawCoverageMappingDummyChecker(Mapping).isDummy();
+}
+
namespace {
struct CovMapFuncRecordReader {
// The interface to read coverage mapping function records for
@@ -334,11 +372,56 @@ class VersionedCovMapFuncRecordReader :
typedef typename coverage::CovMapTraits<Version, IntPtrT>::NameRefType
NameRefType;
- llvm::DenseSet<NameRefType> UniqueFunctionMappingData;
+ // Maps function's name references to the indexes of their records
+ // in \c Records.
+ llvm::DenseMap<NameRefType, size_t> FunctionRecords;
InstrProfSymtab &ProfileNames;
std::vector<StringRef> &Filenames;
std::vector<BinaryCoverageReader::ProfileMappingRecord> &Records;
+ // Add the record to the collection if we don't already have a record that
+ // points to the same function name. This is useful to ignore the redundant
+ // records for the functions with ODR linkage.
+ // In addition, prefer records with real coverage mapping data to dummy
+ // records, which were emitted for inline functions which were seen but
+ // not used in the corresponding translation unit.
+ Error insertFunctionRecordIfNeeded(const FuncRecordType *CFR,
+ StringRef Mapping, size_t FilenamesBegin) {
+ uint64_t FuncHash = CFR->template getFuncHash<Endian>();
+ NameRefType NameRef = CFR->template getFuncNameRef<Endian>();
+ auto InsertResult =
+ FunctionRecords.insert(std::make_pair(NameRef, Records.size()));
+ if (InsertResult.second) {
+ StringRef FuncName;
+ if (Error Err = CFR->template getFuncName<Endian>(ProfileNames, FuncName))
+ return Err;
+ Records.emplace_back(Version, FuncName, FuncHash, Mapping, FilenamesBegin,
+ Filenames.size() - FilenamesBegin);
+ return Error::success();
+ }
+ // Update the existing record if it's a dummy and the new record is real.
+ size_t OldRecordIndex = InsertResult.first->second;
+ BinaryCoverageReader::ProfileMappingRecord &OldRecord =
+ Records[OldRecordIndex];
+ Expected<bool> OldIsDummyExpected = isCoverageMappingDummy(
+ OldRecord.FunctionHash, OldRecord.CoverageMapping);
+ if (Error Err = OldIsDummyExpected.takeError())
+ return Err;
+ if (!*OldIsDummyExpected)
+ return Error::success();
+ Expected<bool> NewIsDummyExpected =
+ isCoverageMappingDummy(FuncHash, Mapping);
+ if (Error Err = NewIsDummyExpected.takeError())
+ return Err;
+ if (*NewIsDummyExpected)
+ return Error::success();
+ OldRecord.FunctionHash = FuncHash;
+ OldRecord.CoverageMapping = Mapping;
+ OldRecord.FilenamesBegin = FilenamesBegin;
+ OldRecord.FilenamesSize = Filenames.size() - FilenamesBegin;
+ return Error::success();
+ }
+
public:
VersionedCovMapFuncRecordReader(
InstrProfSymtab &P,
@@ -387,7 +470,6 @@ public:
while ((const char *)CFR < FunEnd) {
// Read the function information
uint32_t DataSize = CFR->template getDataSize<Endian>();
- uint64_t FuncHash = CFR->template getFuncHash<Endian>();
// Now use that to read the coverage data.
if (CovBuf + DataSize > CovEnd)
@@ -395,21 +477,9 @@ public:
auto Mapping = StringRef(CovBuf, DataSize);
CovBuf += DataSize;
- // Ignore this record if we already have a record that points to the same
- // function name. This is useful to ignore the redundant records for the
- // functions with ODR linkage.
- NameRefType NameRef = CFR->template getFuncNameRef<Endian>();
- if (!UniqueFunctionMappingData.insert(NameRef).second) {
- CFR++;
- continue;
- }
-
- StringRef FuncName;
- if (Error E = CFR->template getFuncName<Endian>(ProfileNames, FuncName))
- return E;
- Records.push_back(BinaryCoverageReader::ProfileMappingRecord(
- Version, FuncName, FuncHash, Mapping, FilenamesBegin,
- Filenames.size() - FilenamesBegin));
+ if (Error Err =
+ insertFunctionRecordIfNeeded(CFR, Mapping, FilenamesBegin))
+ return Err;
CFR++;
}
return Error::success();
Added: llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.covmapping
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.covmapping?rev=270194&view=auto
==============================================================================
Binary files llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.covmapping (added) and llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.covmapping Fri May 20 04:14:24 2016 differ
Added: llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.cpp?rev=270194&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.cpp (added)
+++ llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.cpp Fri May 20 04:14:24 2016
@@ -0,0 +1,5 @@
+#include "prefer_used_to_unused.h"
+
+int main() {
+ return sampleFunc(5) + simpleFunc(5);
+}
Added: llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.proftext
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.proftext?rev=270194&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.proftext (added)
+++ llvm/trunk/test/tools/llvm-cov/Inputs/prefer_used_to_unused.proftext Fri May 20 04:14:24 2016
@@ -0,0 +1,25 @@
+_Z10sampleFunci
+# Func Hash:
+10
+# Num Counters:
+2
+# Counter Values:
+1
+1
+
+main
+# Func Hash:
+0
+# Num Counters:
+1
+# Counter Values:
+1
+
+_Z10simpleFunci
+# Func Hash:
+0
+# Num Counters:
+1
+# Counter Values:
+1
+
Added: llvm/trunk/test/tools/llvm-cov/prefer_used_to_unused.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cov/prefer_used_to_unused.h?rev=270194&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-cov/prefer_used_to_unused.h (added)
+++ llvm/trunk/test/tools/llvm-cov/prefer_used_to_unused.h Fri May 20 04:14:24 2016
@@ -0,0 +1,24 @@
+// Check that llvm-cov loads real coverage mapping data for a function
+// even though dummy coverage data for that function comes first.
+// Dummy coverage data is exported if the definition of an inline function
+// is seen but the function is not used in the translation unit.
+
+// If you need to rebuild the 'covmapping' file for this test, please use
+// the following commands:
+// clang++ -fprofile-instr-generate -fcoverage-mapping -o tmp -x c++ prefer_used_to_unused.h prefer_used_to_unused.cpp
+// llvm-cov convert-for-testing -o prefer_used_to_unused.covmapping tmp
+
+// RUN: llvm-profdata merge %S/Inputs/prefer_used_to_unused.proftext -o %t.profdata
+// RUN: llvm-cov show %S/Inputs/prefer_used_to_unused.covmapping -instr-profile %t.profdata -filename-equivalence %s | FileCheck %s
+
+// Coverage data for this function has a non-zero hash value if it is used in the translation unit.
+inline int sampleFunc(int A) { // CHECK: 1| [[@LINE]]|inline int sampleFunc(int A) {
+ if (A > 0) // CHECK-NEXT: 1| [[@LINE]]| if (A > 0)
+ return A; // CHECK-NEXT: 1| [[@LINE]]| return A;
+ return 0; // CHECK-NEXT: 0| [[@LINE]]| return 0;
+} // CHECK-NEXT: 1| [[@LINE]]|}
+
+// The hash for this function is zero in both cases, either it is used in the translation unit or not.
+inline int simpleFunc(int A) { // CHECK: 1| [[@LINE]]|inline int simpleFunc(int A) {
+ return A; // CHECK-NEXT: 1| [[@LINE]]| return A;
+} // CHECK-NEXT: 1| [[@LINE]]|}
More information about the llvm-commits
mailing list