Skip to content
This repository was archived by the owner on Mar 28, 2020. It is now read-only.

Commit 90e92d9

Browse files
Dor1svedantk
authored andcommitted
[Coverage] Take filenames into account when loading function records.
Summary: Don't skip functions with the same name but from different files. That change makes it possible to generate code coverage reports from different binaries compiled from different sources even if there are functions with non-unique names. Without that change, code coverage for such functions is missing except of the first function processed. Reviewers: vsk, morehouse Reviewed By: vsk Subscribers: llvm-commits, kcc Differential Revision: https://reviews.llvm.org/D46478 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@331801 91177308-0d34-0410-b5e6-96231b3b80d8 (cherry picked from commit c97ab8f)
1 parent da1c9a3 commit 90e92d9

File tree

4 files changed

+29
-4
lines changed

4 files changed

+29
-4
lines changed

include/llvm/ProfileData/Coverage/CoverageMapping.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#include "llvm/ADT/ArrayRef.h"
1919
#include "llvm/ADT/DenseMap.h"
20+
#include "llvm/ADT/DenseSet.h"
2021
#include "llvm/ADT/Hashing.h"
2122
#include "llvm/ADT/None.h"
2223
#include "llvm/ADT/StringRef.h"
@@ -506,7 +507,7 @@ class CoverageData {
506507
/// This is the main interface to get coverage information, using a profile to
507508
/// fill out execution counts.
508509
class CoverageMapping {
509-
StringSet<> FunctionNames;
510+
DenseMap<size_t, DenseSet<size_t>> RecordProvenance;
510511
std::vector<FunctionRecord> Functions;
511512
std::vector<std::pair<std::string, uint64_t>> FuncHashMismatches;
512513
std::vector<std::pair<std::string, uint64_t>> FuncCounterMismatches;

lib/ProfileData/Coverage/CoverageMapping.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,10 @@ Error CoverageMapping::loadFunctionRecord(
207207
else
208208
OrigFuncName = getFuncNameWithoutPrefix(OrigFuncName, Record.Filenames[0]);
209209

210-
// Don't load records for functions we've already seen.
211-
if (!FunctionNames.insert(OrigFuncName).second)
210+
// Don't load records for (filenames, function) pairs we've already seen.
211+
auto FilenamesHash = hash_combine_range(Record.Filenames.begin(),
212+
Record.Filenames.end());
213+
if (!RecordProvenance[FilenamesHash].insert(hash_value(OrigFuncName)).second)
212214
return Error::success();
213215

214216
CounterMappingContext Ctx(Record.Expressions);

test/tools/llvm-cov/multiple-objects.test

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ REPORT: Filename{{ +}}Regions{{ +}}Missed Regions{{ +}}Cover
66
REPORT-NEXT: ---
77
REPORT-NEXT: header.h{{ +}}25{{ +}}14{{ +}}44.00%
88

9+
# Make sure that both use_1.cc and use_2.cc have coverage reported.
10+
# Before https://reviews.llvm.org/D46478, only one of them used to be reported.
11+
REPORT-NEXT: use_1.cc{{ +}}1{{ +}}0{{ +}}100.00%
12+
REPORT-NEXT: use_2.cc{{ +}}2{{ +}}0{{ +}}100.00%
13+
914
Instructions for regenerating the test:
1015

1116
clang -std=c++11 -mllvm -enable-name-compression=false -fprofile-instr-generate -fcoverage-mapping use_1.cc -o use_1

unittests/ProfileData/CoverageMappingTest.cpp

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -859,17 +859,34 @@ TEST_P(CoverageMappingTest, load_coverage_for_expanded_file) {
859859
TEST_P(CoverageMappingTest, skip_duplicate_function_record) {
860860
ProfileWriter.addRecord({"func", 0x1234, {1}}, Err);
861861

862+
// This record should be loaded.
862863
startFunction("func", 0x1234);
863864
addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
864865

866+
// This record should be loaded.
865867
startFunction("func", 0x1234);
866868
addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
869+
addCMR(Counter::getCounter(0), "file2", 1, 1, 9, 9);
870+
871+
// This record should be skipped.
872+
startFunction("func", 0x1234);
873+
addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
874+
875+
// This record should be loaded.
876+
startFunction("func", 0x1234);
877+
addCMR(Counter::getCounter(0), "file2", 1, 1, 9, 9);
878+
addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
879+
880+
// This record should be skipped.
881+
startFunction("func", 0x1234);
882+
addCMR(Counter::getCounter(0), "file1", 1, 1, 9, 9);
883+
addCMR(Counter::getCounter(0), "file2", 1, 1, 9, 9);
867884

868885
EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
869886

870887
auto Funcs = LoadedCoverage->getCoveredFunctions();
871888
unsigned NumFuncs = std::distance(Funcs.begin(), Funcs.end());
872-
ASSERT_EQ(1U, NumFuncs);
889+
ASSERT_EQ(3U, NumFuncs);
873890
}
874891

875892
// FIXME: Use ::testing::Combine() when llvm updates its copy of googletest.

0 commit comments

Comments
 (0)