Skip to content

Commit 896bceb

Browse files
authored
[MC/DC][Coverage] Add assertions into emitSourceRegions() (#89572)
`emitSourceRegions()` has bugs to emit malformed MC/DC coverage mappings. They were detected in `llvm-cov` as the crash. Detect inconsistencies earlier in `clang` with assertions. * mcdc-scratch-space.c covers #87000.
1 parent beac910 commit 896bceb

File tree

2 files changed

+43
-3
lines changed

2 files changed

+43
-3
lines changed

clang/lib/CodeGen/CoverageMappingGen.cpp

+16-3
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,10 @@ class SourceMappingRegion {
191191

192192
bool isBranch() const { return FalseCount.has_value(); }
193193

194+
bool isMCDCBranch() const {
195+
return std::holds_alternative<mcdc::BranchParameters>(MCDCParams);
196+
}
197+
194198
bool isMCDCDecision() const {
195199
return std::holds_alternative<mcdc::DecisionParameters>(MCDCParams);
196200
}
@@ -472,13 +476,19 @@ class CoverageMappingBuilder {
472476
// Ignore regions from system headers unless collecting coverage from
473477
// system headers is explicitly enabled.
474478
if (!SystemHeadersCoverage &&
475-
SM.isInSystemHeader(SM.getSpellingLoc(LocStart)))
479+
SM.isInSystemHeader(SM.getSpellingLoc(LocStart))) {
480+
assert(!Region.isMCDCBranch() && !Region.isMCDCDecision() &&
481+
"Don't suppress the condition in system headers");
476482
continue;
483+
}
477484

478485
auto CovFileID = getCoverageFileID(LocStart);
479486
// Ignore regions that don't have a file, such as builtin macros.
480-
if (!CovFileID)
487+
if (!CovFileID) {
488+
assert(!Region.isMCDCBranch() && !Region.isMCDCDecision() &&
489+
"Don't suppress the condition in non-file regions");
481490
continue;
491+
}
482492

483493
SourceLocation LocEnd = Region.getEndLoc();
484494
assert(SM.isWrittenInSameFile(LocStart, LocEnd) &&
@@ -488,8 +498,11 @@ class CoverageMappingBuilder {
488498
// This not only suppresses redundant regions, but sometimes prevents
489499
// creating regions with wrong counters if, for example, a statement's
490500
// body ends at the end of a nested macro.
491-
if (Filter.count(std::make_pair(LocStart, LocEnd)))
501+
if (Filter.count(std::make_pair(LocStart, LocEnd))) {
502+
assert(!Region.isMCDCBranch() && !Region.isMCDCDecision() &&
503+
"Don't suppress the condition");
492504
continue;
505+
}
493506

494507
// Find the spelling locations for the mapping region.
495508
SpellingRegion SR{SM, LocStart, LocEnd};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c99 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s
2+
// XFAIL: *
3+
// REQUIRES: asserts
4+
5+
int builtin_macro0(int a) {
6+
return (__LINE__
7+
&& a);
8+
}
9+
10+
int builtin_macro1(int a) {
11+
return (a
12+
|| __LINE__);
13+
}
14+
15+
#define PRE(x) pre_##x
16+
17+
int pre0(int pre_a, int b_post) {
18+
return (PRE(a)
19+
&& b_post);
20+
}
21+
22+
#define POST(x) x##_post
23+
24+
int post0(int pre_a, int b_post) {
25+
return (pre_a
26+
|| POST(b));
27+
}

0 commit comments

Comments
 (0)