Skip to content

Commit e4274cf

Browse files
[CoverageMapping] Handle gaps in counter IDs for source-based coverage
For source-based coverage, the frontend sets the counter IDs and the constraints of counter IDs is not defined. For e.g., the Rust frontend until recently had a reserved counter #0 (rust-lang/rust#83774). Rust coverage instrumentation also creates counters on edges in addition to basic blocks. Some functions may have more counters than regions. This breaks an assumption in CoverageMapping.cpp where the number of counters in a function is assumed to be bounded by the number of regions: Counts.assign(Record.MappingRegions.size(), 0); This assumption causes CounterMappingContext::evaluate() to fail since there are not enough counter values created in the above call to `Counts.assign`. Consequently, some uncovered functions are not reported in coverage reports. This change walks a Function's CoverageMappingRecord to find the maximum counter ID, and uses it to initialize the counter array when instrprof records are missing for a function in sparse profiles. Differential Revision: https://reviews.llvm.org/D101780
1 parent 40fb4ee commit e4274cf

File tree

3 files changed

+57
-5
lines changed

3 files changed

+57
-5
lines changed

llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,8 @@ class CounterMappingContext {
334334
/// Return the number of times that a region of code associated with this
335335
/// counter was executed.
336336
Expected<int64_t> evaluate(const Counter &C) const;
337+
338+
unsigned getMaxCounterID(const Counter &C) const;
337339
};
338340

339341
/// Code coverage information for a single function.

llvm/lib/ProfileData/Coverage/CoverageMapping.cpp

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,22 @@ Expected<int64_t> CounterMappingContext::evaluate(const Counter &C) const {
186186
llvm_unreachable("Unhandled CounterKind");
187187
}
188188

189+
unsigned CounterMappingContext::getMaxCounterID(const Counter &C) const {
190+
switch (C.getKind()) {
191+
case Counter::Zero:
192+
return 0;
193+
case Counter::CounterValueReference:
194+
return C.getCounterID();
195+
case Counter::Expression: {
196+
if (C.getExpressionID() >= Expressions.size())
197+
return 0;
198+
const auto &E = Expressions[C.getExpressionID()];
199+
return std::max(getMaxCounterID(E.LHS), getMaxCounterID(E.RHS));
200+
}
201+
}
202+
llvm_unreachable("Unhandled CounterKind");
203+
}
204+
189205
void FunctionRecordIterator::skipOtherFiles() {
190206
while (Current != Records.end() && !Filename.empty() &&
191207
Filename != Current->Filenames[0])
@@ -203,6 +219,15 @@ ArrayRef<unsigned> CoverageMapping::getImpreciseRecordIndicesForFilename(
203219
return RecordIt->second;
204220
}
205221

222+
static unsigned getMaxCounterID(const CounterMappingContext &Ctx,
223+
const CoverageMappingRecord &Record) {
224+
unsigned MaxCounterID = 0;
225+
for (const auto &Region : Record.MappingRegions) {
226+
MaxCounterID = std::max(MaxCounterID, Ctx.getMaxCounterID(Region.Count));
227+
}
228+
return MaxCounterID;
229+
}
230+
206231
Error CoverageMapping::loadFunctionRecord(
207232
const CoverageMappingRecord &Record,
208233
IndexedInstrProfReader &ProfileReader) {
@@ -227,7 +252,7 @@ Error CoverageMapping::loadFunctionRecord(
227252
return Error::success();
228253
} else if (IPE != instrprof_error::unknown_function)
229254
return make_error<InstrProfError>(IPE);
230-
Counts.assign(Record.MappingRegions.size(), 0);
255+
Counts.assign(getMaxCounterID(Ctx, Record) + 1, 0);
231256
}
232257
Ctx.setCounts(Counts);
233258

llvm/unittests/ProfileData/CoverageMappingTest.cpp

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ struct OutputFunctionCoverageData {
6262
uint64_t Hash;
6363
std::vector<StringRef> Filenames;
6464
std::vector<CounterMappingRegion> Regions;
65+
std::vector<CounterExpression> Expressions;
6566

6667
OutputFunctionCoverageData() : Hash(0) {}
6768

@@ -78,7 +79,7 @@ struct OutputFunctionCoverageData {
7879
Record.FunctionName = Name;
7980
Record.FunctionHash = Hash;
8081
Record.Filenames = Filenames;
81-
Record.Expressions = {};
82+
Record.Expressions = Expressions;
8283
Record.MappingRegions = Regions;
8384
}
8485
};
@@ -111,6 +112,7 @@ struct InputFunctionCoverageData {
111112
std::string Name;
112113
uint64_t Hash;
113114
std::vector<CounterMappingRegion> Regions;
115+
std::vector<CounterExpression> Expressions;
114116

115117
InputFunctionCoverageData(std::string Name, uint64_t Hash)
116118
: Name(std::move(Name)), Hash(Hash) {}
@@ -189,13 +191,17 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> {
189191
LS, CS, LE, CE));
190192
}
191193

194+
void addExpression(CounterExpression CE) {
195+
InputFunctions.back().Expressions.push_back(CE);
196+
}
197+
192198
std::string writeCoverageRegions(InputFunctionCoverageData &Data) {
193199
SmallVector<unsigned, 8> FileIDs(Data.ReverseVirtualFileMapping.size());
194200
for (const auto &E : Data.ReverseVirtualFileMapping)
195201
FileIDs[E.second] = E.first;
196202
std::string Coverage;
197203
llvm::raw_string_ostream OS(Coverage);
198-
CoverageMappingWriter(FileIDs, None, Data.Regions).write(OS);
204+
CoverageMappingWriter(FileIDs, Data.Expressions, Data.Regions).write(OS);
199205
return OS.str();
200206
}
201207

@@ -207,10 +213,9 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> {
207213
Filenames.resize(Files.size() + 1);
208214
for (const auto &E : Files)
209215
Filenames[E.getValue()] = E.getKey().str();
210-
std::vector<CounterExpression> Expressions;
211216
ArrayRef<std::string> FilenameRefs = llvm::makeArrayRef(Filenames);
212217
RawCoverageMappingReader Reader(Coverage, FilenameRefs, Data.Filenames,
213-
Expressions, Data.Regions);
218+
Data.Expressions, Data.Regions);
214219
EXPECT_THAT_ERROR(Reader.read(), Succeeded());
215220
}
216221

@@ -796,6 +801,26 @@ TEST_P(CoverageMappingTest, combine_expansions) {
796801
EXPECT_EQ(CoverageSegment(5, 5, false), Segments[3]);
797802
}
798803

804+
// Test that counters not associated with any code regions are allowed.
805+
TEST_P(CoverageMappingTest, non_code_region_counters) {
806+
// No records in profdata
807+
808+
startFunction("func", 0x1234);
809+
addCMR(Counter::getCounter(0), "file", 1, 1, 5, 5);
810+
addCMR(Counter::getExpression(0), "file", 6, 1, 6, 5);
811+
addExpression(CounterExpression(
812+
CounterExpression::Add, Counter::getCounter(1), Counter::getCounter(2)));
813+
814+
EXPECT_THAT_ERROR(loadCoverageMapping(), Succeeded());
815+
816+
std::vector<std::string> Names;
817+
for (const auto &Func : LoadedCoverage->getCoveredFunctions()) {
818+
Names.push_back(Func.Name);
819+
ASSERT_EQ(2U, Func.CountedRegions.size());
820+
}
821+
ASSERT_EQ(1U, Names.size());
822+
}
823+
799824
TEST_P(CoverageMappingTest, strip_filename_prefix) {
800825
ProfileWriter.addRecord({"file1:func", 0x1234, {0}}, Err);
801826

0 commit comments

Comments
 (0)