Skip to content

Commit f9e9e59

Browse files
whentojumpchapuni
andauthored
[Coverage][Expansion] handle nested macros in scratch space (#89869)
The problematic program is as follows: ```shell #define pre_a 0 #define PRE(x) pre_##x void f(void) { PRE(a) && 0; } int main(void) { return 0; } ``` in which after token concatenation (`##`), there's another nested macro `pre_a`. Currently only the outer expansion region will be produced. ([compiler explorer link](https://godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(filename:'1',fontScale:14,fontUsePx:'0',j:1,lang:___c,selection:(endColumn:29,endLineNumber:8,positionColumn:29,positionLineNumber:8,selectionStartColumn:29,selectionStartLineNumber:8,startColumn:29,startLineNumber:8),source:'%23define+pre_a+0%0A%23define+PRE(x)+pre_%23%23x%0A%0Avoid+f(void)+%7B%0A++++PRE(a)+%26%26+0%3B%0A%7D%0A%0Aint+main(void)+%7B+return+0%3B+%7D'),l:'5',n:'0',o:'C+source+%231',t:'0')),k:51.69491525423727,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((g:!((h:compiler,i:(compiler:cclang_assertions_trunk,filters:(b:'0',binary:'1',binaryObject:'1',commentOnly:'0',debugCalls:'1',demangle:'0',directives:'0',execute:'0',intel:'0',libraryCode:'1',trim:'1',verboseDemangling:'0'),flagsViewOpen:'1',fontScale:14,fontUsePx:'0',j:2,lang:___c,libs:!(),options:'-fprofile-instr-generate+-fcoverage-mapping+-fcoverage-mcdc+-Xclang+-dump-coverage-mapping+',overrides:!(),selection:(endColumn:1,endLineNumber:1,positionColumn:1,positionLineNumber:1,selectionStartColumn:1,selectionStartLineNumber:1,startColumn:1,startLineNumber:1),source:1),l:'5',n:'0',o:'+x86-64+clang+(assertions+trunk)+(Editor+%231)',t:'0')),k:34.5741843594503,l:'4',m:28.903654485049834,n:'0',o:'',s:0,t:'0'),(g:!((h:output,i:(compilerName:'x86-64+clang+(trunk)',editorid:1,fontScale:14,fontUsePx:'0',j:2,wrap:'1'),l:'5',n:'0',o:'Output+of+x86-64+clang+(assertions+trunk)+(Compiler+%232)',t:'0')),header:(),l:'4',m:71.09634551495017,n:'0',o:'',s:0,t:'0')),k:48.30508474576271,l:'3',n:'0',o:'',t:'0')),l:'2',m:100,n:'0',o:'',t:'0')),version:4)) ```text f: File 0, 4:14 -> 6:2 = #0 Decision,File 0, 5:5 -> 5:16 = M:0, C:2 Expansion,File 0, 5:5 -> 5:8 = #0 (Expanded file = 1) File 0, 5:15 -> 5:16 = #1 Branch,File 0, 5:15 -> 5:16 = 0, 0 [2,0,0] File 1, 2:16 -> 2:23 = #0 File 2, 1:15 -> 1:16 = #0 File 2, 1:15 -> 1:16 = #0 Branch,File 2, 1:15 -> 1:16 = 0, 0 [1,2,0] ``` The inner expansion region isn't produced because: 1. In the range-based for loop quoted below, each sloc is processed and possibly emit a corresponding expansion region. 2. For our sloc in question, its direct parent returned by `getIncludeOrExpansionLoc()` is a `<scratch space>`, because that's how `##` is processed. https://github.com/llvm/llvm-project/blob/88b6186af3908c55b357858eb348b5143f21c289/clang/lib/CodeGen/CoverageMappingGen.cpp#L518-L520 3. This `<scratch space>` cannot be found in the FileID mapping so `ParentFileID` will be assigned an `std::nullopt` https://github.com/llvm/llvm-project/blob/88b6186af3908c55b357858eb348b5143f21c289/clang/lib/CodeGen/CoverageMappingGen.cpp#L521-L526 4. As a result this iteration of for loop finishes early and no expansion region is added for the sloc. This problem gets worse with MC/DC: as the example shows, there's a branch from File 2 but File 2 itself is missing. This will trigger assertion failures. The fix is more or less a workaround and takes a similar approach as #89573. ~~Depends on #89573.~~ This includes #89573. Kudos to @chapuni! This and #89573 together fix #87000: I tested locally, both the reduced program and my original use case (fwiw, Linux kernel) can run successfully. --------- Co-authored-by: NAKAMURA Takumi <[email protected]>
1 parent 09c5525 commit f9e9e59

File tree

4 files changed

+91
-16
lines changed

4 files changed

+91
-16
lines changed

clang/lib/CodeGen/CoverageMappingGen.cpp

+40-5
Original file line numberDiff line numberDiff line change
@@ -294,10 +294,36 @@ class CoverageMappingBuilder {
294294
return SM.getLocForEndOfFile(SM.getFileID(Loc));
295295
}
296296

297-
/// Find out where the current file is included or macro is expanded.
298-
SourceLocation getIncludeOrExpansionLoc(SourceLocation Loc) {
299-
return Loc.isMacroID() ? SM.getImmediateExpansionRange(Loc).getBegin()
300-
: SM.getIncludeLoc(SM.getFileID(Loc));
297+
/// Find out where a macro is expanded. If the immediate result is a
298+
/// <scratch space>, keep looking until the result isn't. Return a pair of
299+
/// \c SourceLocation. The first object is always the begin sloc of found
300+
/// result. The second should be checked by the caller: if it has value, it's
301+
/// the end sloc of the found result. Otherwise the while loop didn't get
302+
/// executed, which means the location wasn't changed and the caller has to
303+
/// learn the end sloc from somewhere else.
304+
std::pair<SourceLocation, std::optional<SourceLocation>>
305+
getNonScratchExpansionLoc(SourceLocation Loc) {
306+
std::optional<SourceLocation> EndLoc = std::nullopt;
307+
while (Loc.isMacroID() &&
308+
SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
309+
auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
310+
Loc = ExpansionRange.getBegin();
311+
EndLoc = ExpansionRange.getEnd();
312+
}
313+
return std::make_pair(Loc, EndLoc);
314+
}
315+
316+
/// Find out where the current file is included or macro is expanded. If
317+
/// \c AcceptScratch is set to false, keep looking for expansions until the
318+
/// found sloc is not a <scratch space>.
319+
SourceLocation getIncludeOrExpansionLoc(SourceLocation Loc,
320+
bool AcceptScratch = true) {
321+
if (!Loc.isMacroID())
322+
return SM.getIncludeLoc(SM.getFileID(Loc));
323+
Loc = SM.getImmediateExpansionRange(Loc).getBegin();
324+
if (AcceptScratch)
325+
return Loc;
326+
return getNonScratchExpansionLoc(Loc).first;
301327
}
302328

303329
/// Return true if \c Loc is a location in a built-in macro.
@@ -344,6 +370,15 @@ class CoverageMappingBuilder {
344370
for (auto &Region : SourceRegions) {
345371
SourceLocation Loc = Region.getBeginLoc();
346372

373+
// Replace Region with its definition if it is in <scratch space>.
374+
auto NonScratchExpansionLoc = getNonScratchExpansionLoc(Loc);
375+
auto EndLoc = NonScratchExpansionLoc.second;
376+
if (EndLoc.has_value()) {
377+
Loc = NonScratchExpansionLoc.first;
378+
Region.setStartLoc(Loc);
379+
Region.setEndLoc(EndLoc.value());
380+
}
381+
347382
// Replace Loc with FileLoc if it is expanded with system headers.
348383
if (!SystemHeadersCoverage && SM.isInSystemMacro(Loc)) {
349384
auto BeginLoc = SM.getSpellingLoc(Loc);
@@ -538,7 +573,7 @@ class CoverageMappingBuilder {
538573
SourceRegionFilter Filter;
539574
for (const auto &FM : FileIDMapping) {
540575
SourceLocation ExpandedLoc = FM.second.second;
541-
SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc);
576+
SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc, false);
542577
if (ParentLoc.isInvalid())
543578
continue;
544579

clang/test/CoverageMapping/builtinmacro.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
// CHECK: filename
66
const char *filename (const char *name) { // CHECK-NEXT: File 0, [[@LINE]]:41 -> [[@LINE+3]]:2 = #0
7-
static const char this_file[] = __FILE__;
7+
static const char this_file[] = __FILE__; // CHECK-NEXT: File 0, [[@LINE]]:35 -> [[@LINE]]:35 = #0
88
return this_file;
99
}
1010

clang/test/CoverageMapping/macros.c

+5-3
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,14 @@ void func7(void) { // CHECK-NEXT: File 0, [[@LINE]]:18 -> [[@LINE+6]]:2 = #0
8080
int kk,ll; // CHECK-NEXT: File 0, [[@LINE+1]]:7 -> [[@LINE+1]]:8 = #0
8181
if (k) // CHECK-NEXT: Branch,File 0, [[@LINE]]:7 -> [[@LINE]]:8 = #1
8282
m(k); // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:9 -> [[@LINE]]:5 = #1
83-
else // CHECK-NEXT: Expansion,File 0, [[@LINE-1]]:5 -> [[@LINE-1]]:6 = #0
83+
else // CHECK-NEXT: Expansion,File 0, [[@LINE-1]]:5 -> [[@LINE-1]]:6 = #1
8484
l = m(l); // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:7 -> [[@LINE]]:5 = (#0 - #1)
8585
} // CHECK-NEXT: File 0, [[@LINE-1]]:5 -> [[@LINE-1]]:10 = (#0 - #1)
8686
// CHECK-NEXT: Expansion,File 0, [[@LINE-2]]:9 -> [[@LINE-2]]:10 = (#0 - #1)
87-
// CHECK-NEXT: File 1, [[@LINE-9]]:14 -> [[@LINE-9]]:18 = #0
88-
// CHECK-NEXT: File 2, [[@LINE-10]]:14 -> [[@LINE-10]]:15 = (#0 - #1)
87+
// CHECK-NEXT: File 1, [[@LINE-9]]:14 -> [[@LINE-9]]:17 = #1
88+
// CHECK-NEXT: File 1, [[@LINE-10]]:14 -> [[@LINE-10]]:18 = #0
89+
// CHECK-NEXT: File 2, [[@LINE-11]]:14 -> [[@LINE-11]]:17 = (#0 - #1)
90+
// CHECK-NEXT: File 2, [[@LINE-12]]:14 -> [[@LINE-12]]:15 = (#0 - #1)
8991

9092
int main(int argc, const char *argv[]) {
9193
func();
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,65 @@
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
1+
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c99 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s | FileCheck %s
42

3+
// CHECK: builtin_macro0:
54
int builtin_macro0(int a) {
6-
return (__LINE__
7-
&& a);
5+
// CHECK: Decision,File 0, [[@LINE+1]]:11 -> [[@LINE+2]]:15 = M:0, C:2
6+
return (__LINE__ // CHECK: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:11 = 0, 0 [1,2,0]
7+
&& a); // CHECK: Branch,File 0, [[@LINE]]:14 -> [[@LINE]]:15 = #2, (#1 - #2) [2,0,0]
88
}
99

10+
// CHECK: builtin_macro1:
1011
int builtin_macro1(int a) {
11-
return (a
12-
|| __LINE__);
12+
// CHECK: Decision,File 0, [[@LINE+1]]:11 -> [[@LINE+2]]:22 = M:0, C:2
13+
return (a // CHECK: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:12 = (#0 - #1), #1 [1,0,2]
14+
|| __LINE__); // CHECK: Branch,File 0, [[@LINE]]:14 -> [[@LINE]]:14 = 0, 0 [2,0,0]
1315
}
1416

1517
#define PRE(x) pre_##x
1618

19+
// CHECK: pre0:
1720
int pre0(int pre_a, int b_post) {
21+
// CHECK: Decision,File 0, [[@LINE+2]]:11 -> [[@LINE+3]]:20 = M:0, C:2
22+
// CHECK: Expansion,File 0, [[@LINE+1]]:11 -> [[@LINE+1]]:14 = #0 (Expanded file = 1)
1823
return (PRE(a)
1924
&& b_post);
25+
// CHECK: Branch,File 0, [[@LINE-1]]:14 -> [[@LINE-1]]:20 = #2, (#1 - #2) [2,0,0]
26+
// CHECK: Branch,File 1, [[@LINE-9]]:16 -> [[@LINE-9]]:22 = #1, (#0 - #1) [1,2,0]
27+
}
28+
29+
#define pre_foo pre_a
30+
31+
// CHECK: pre1:
32+
int pre1(int pre_a, int b_post) {
33+
// CHECK: Decision,File 0, [[@LINE+3]]:11 -> [[@LINE+4]]:20 = M:0, C:2
34+
// CHECK: Expansion,File 0, [[@LINE+2]]:11 -> [[@LINE+2]]:14 = #0 (Expanded file = 1)
35+
// CHECK: Branch,File 0, [[@LINE+2]]:14 -> [[@LINE+2]]:20 = #2, (#1 - #2) [2,0,0]
36+
return (PRE(foo)
37+
&& b_post);
38+
// CHECK: Expansion,File 1, 17:16 -> 17:20 = #0 (Expanded file = 2)
39+
// CHECK: Branch,File 2, 29:17 -> 29:22 = #1, (#0 - #1) [1,2,0]
2040
}
2141

2242
#define POST(x) x##_post
2343

44+
// CHECK: post0:
2445
int post0(int pre_a, int b_post) {
46+
// CHECK: Decision,File 0, [[@LINE+2]]:11 -> [[@LINE+3]]:18 = M:0, C:2
47+
// CHECK: Branch,File 0, [[@LINE+1]]:11 -> [[@LINE+1]]:16 = (#0 - #1), #1 [1,0,2]
2548
return (pre_a
2649
|| POST(b));
50+
// CHECK: Expansion,File 0, [[@LINE-1]]:14 -> [[@LINE-1]]:18 = #1 (Expanded file = 1)
51+
// CHECK: Branch,File 1, [[@LINE-9]]:17 -> [[@LINE-9]]:20 = (#1 - #2), #2 [2,0,0]
52+
}
53+
54+
#define bar_post b_post
55+
56+
// CHECK: post1:
57+
int post1(int pre_a, int b_post) {
58+
// CHECK: Decision,File 0, [[@LINE+3]]:11 -> [[@LINE+4]]:18 = M:0, C:2
59+
// CHECK: Branch,File 0, [[@LINE+2]]:11 -> [[@LINE+2]]:16 = (#0 - #1), #1 [1,0,2]
60+
// CHECK: Expansion,File 0, [[@LINE+2]]:14 -> [[@LINE+2]]:18 = 0 (Expanded file = 1)
61+
return (pre_a
62+
|| POST(bar));
63+
// CHECK: Expansion,File 1, 42:17 -> 42:18 = #1 (Expanded file = 2)
64+
// CHECK: Branch,File 2, 54:18 -> 54:24 = (#1 - #2), #2 [2,0,0]
2765
}

0 commit comments

Comments
 (0)