[clang] [Coverage][Expansion] handle nested macros in scratch space (PR #89869)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 23 23:18:35 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-codegen
Author: Wentao Zhang (whentojump)
<details>
<summary>Changes</summary>
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 and #<!-- -->89573 together fix #<!-- -->87000.
---
Full diff: https://github.com/llvm/llvm-project/pull/89869.diff
4 Files Affected:
- (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+29-2)
- (modified) clang/test/CoverageMapping/builtinmacro.c (+1-1)
- (modified) clang/test/CoverageMapping/macros.c (+5-3)
- (added) clang/test/CoverageMapping/mcdc-scratch-space.c (+65)
``````````diff
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 733686d4946b3c..9268ac5b4c61a3 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -298,6 +298,22 @@ class CoverageMappingBuilder {
: SM.getIncludeLoc(SM.getFileID(Loc));
}
+ /// Find out where the current file is included or macro is expanded. If the
+ /// found expansion is a <scratch space>, keep looking.
+ SourceLocation getIncludeOrNonScratchExpansionLoc(SourceLocation Loc) {
+ if (Loc.isMacroID()) {
+ Loc = SM.getImmediateExpansionRange(Loc).getBegin();
+ while (Loc.isMacroID() &&
+ SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
+ auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
+ Loc = ExpansionRange.getBegin();
+ }
+ } else {
+ Loc = SM.getIncludeLoc(SM.getFileID(Loc));
+ }
+ return Loc;
+ }
+
/// Return true if \c Loc is a location in a built-in macro.
bool isInBuiltin(SourceLocation Loc) {
return SM.getBufferName(SM.getSpellingLoc(Loc)) == "<built-in>";
@@ -339,8 +355,18 @@ class CoverageMappingBuilder {
llvm::SmallSet<FileID, 8> Visited;
SmallVector<std::pair<SourceLocation, unsigned>, 8> FileLocs;
- for (const auto &Region : SourceRegions) {
+ for (auto &Region : SourceRegions) {
SourceLocation Loc = Region.getBeginLoc();
+
+ // Replace Region with its definition if it is in <scratch space>.
+ while (Loc.isMacroID() &&
+ SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
+ auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
+ Loc = ExpansionRange.getBegin();
+ Region.setStartLoc(Loc);
+ Region.setEndLoc(ExpansionRange.getEnd());
+ }
+
FileID File = SM.getFileID(Loc);
if (!Visited.insert(File).second)
continue;
@@ -517,7 +543,8 @@ class CoverageMappingBuilder {
SourceRegionFilter Filter;
for (const auto &FM : FileIDMapping) {
SourceLocation ExpandedLoc = FM.second.second;
- SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc);
+ SourceLocation ParentLoc =
+ getIncludeOrNonScratchExpansionLoc(ExpandedLoc);
if (ParentLoc.isInvalid())
continue;
diff --git a/clang/test/CoverageMapping/builtinmacro.c b/clang/test/CoverageMapping/builtinmacro.c
index abcdc191523a5d..5d5a176aa7d87e 100644
--- a/clang/test/CoverageMapping/builtinmacro.c
+++ b/clang/test/CoverageMapping/builtinmacro.c
@@ -4,7 +4,7 @@
// CHECK: filename
const char *filename (const char *name) { // CHECK-NEXT: File 0, [[@LINE]]:41 -> [[@LINE+3]]:2 = #0
- static const char this_file[] = __FILE__;
+ static const char this_file[] = __FILE__; // CHECK-NEXT: File 0, [[@LINE]]:35 -> [[@LINE]]:35 = #0
return this_file;
}
diff --git a/clang/test/CoverageMapping/macros.c b/clang/test/CoverageMapping/macros.c
index 6bd3be434139a2..fcf21170ef135c 100644
--- a/clang/test/CoverageMapping/macros.c
+++ b/clang/test/CoverageMapping/macros.c
@@ -80,12 +80,14 @@ void func7(void) { // CHECK-NEXT: File 0, [[@LINE]]:18 -> [[@LINE+6]]:2 = #0
int kk,ll; // CHECK-NEXT: File 0, [[@LINE+1]]:7 -> [[@LINE+1]]:8 = #0
if (k) // CHECK-NEXT: Branch,File 0, [[@LINE]]:7 -> [[@LINE]]:8 = #1
m(k); // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:9 -> [[@LINE]]:5 = #1
- else // CHECK-NEXT: Expansion,File 0, [[@LINE-1]]:5 -> [[@LINE-1]]:6 = #0
+ else // CHECK-NEXT: Expansion,File 0, [[@LINE-1]]:5 -> [[@LINE-1]]:6 = #1
l = m(l); // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:7 -> [[@LINE]]:5 = (#0 - #1)
} // CHECK-NEXT: File 0, [[@LINE-1]]:5 -> [[@LINE-1]]:10 = (#0 - #1)
// CHECK-NEXT: Expansion,File 0, [[@LINE-2]]:9 -> [[@LINE-2]]:10 = (#0 - #1)
- // CHECK-NEXT: File 1, [[@LINE-9]]:14 -> [[@LINE-9]]:18 = #0
- // CHECK-NEXT: File 2, [[@LINE-10]]:14 -> [[@LINE-10]]:15 = (#0 - #1)
+ // CHECK-NEXT: File 1, [[@LINE-9]]:14 -> [[@LINE-9]]:17 = #1
+ // CHECK-NEXT: File 1, [[@LINE-10]]:14 -> [[@LINE-10]]:18 = #0
+ // CHECK-NEXT: File 2, [[@LINE-11]]:14 -> [[@LINE-11]]:17 = (#0 - #1)
+ // CHECK-NEXT: File 2, [[@LINE-12]]:14 -> [[@LINE-12]]:15 = (#0 - #1)
int main(int argc, const char *argv[]) {
func();
diff --git a/clang/test/CoverageMapping/mcdc-scratch-space.c b/clang/test/CoverageMapping/mcdc-scratch-space.c
new file mode 100644
index 00000000000000..2b5b12d9dcad65
--- /dev/null
+++ b/clang/test/CoverageMapping/mcdc-scratch-space.c
@@ -0,0 +1,65 @@
+// 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
+
+// CHECK: builtin_macro0:
+int builtin_macro0(int a) {
+ // CHECK: Decision,File 0, [[@LINE+1]]:11 -> [[@LINE+2]]:15 = M:0, C:2
+ return (__LINE__ // CHECK: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:11 = 0, 0 [1,2,0]
+ && a); // CHECK: Branch,File 0, [[@LINE]]:14 -> [[@LINE]]:15 = #2, (#1 - #2) [2,0,0]
+}
+
+// CHECK: builtin_macro1:
+int builtin_macro1(int a) {
+ // CHECK: Decision,File 0, [[@LINE+1]]:11 -> [[@LINE+2]]:22 = M:0, C:2
+ return (a // CHECK: Branch,File 0, [[@LINE]]:11 -> [[@LINE]]:12 = (#0 - #1), #1 [1,0,2]
+ || __LINE__); // CHECK: Branch,File 0, [[@LINE]]:14 -> [[@LINE]]:14 = 0, 0 [2,0,0]
+}
+
+#define PRE(x) pre_##x
+
+// CHECK: pre0:
+int pre0(int pre_a, int b_post) {
+ // CHECK: Decision,File 0, [[@LINE+2]]:11 -> [[@LINE+3]]:20 = M:0, C:2
+ // CHECK: Expansion,File 0, [[@LINE+1]]:11 -> [[@LINE+1]]:14 = #0 (Expanded file = 1)
+ return (PRE(a)
+ && b_post);
+ // CHECK: Branch,File 0, [[@LINE-1]]:14 -> [[@LINE-1]]:20 = #2, (#1 - #2) [2,0,0]
+ // CHECK: Branch,File 1, [[@LINE-9]]:16 -> [[@LINE-9]]:22 = #1, (#0 - #1) [1,2,0]
+}
+
+#define pre_foo pre_a
+
+// CHECK: pre1:
+int pre1(int pre_a, int b_post) {
+ // CHECK: Decision,File 0, [[@LINE+3]]:11 -> [[@LINE+4]]:20 = M:0, C:2
+ // CHECK: Expansion,File 0, [[@LINE+2]]:11 -> [[@LINE+2]]:14 = #0 (Expanded file = 1)
+ // CHECK: Branch,File 0, [[@LINE+2]]:14 -> [[@LINE+2]]:20 = #2, (#1 - #2) [2,0,0]
+ return (PRE(foo)
+ && b_post);
+ // CHECK: Expansion,File 1, 17:16 -> 17:20 = #0 (Expanded file = 2)
+ // CHECK: Branch,File 2, 29:17 -> 29:22 = #1, (#0 - #1) [1,2,0]
+}
+
+#define POST(x) x##_post
+
+// CHECK: post0:
+int post0(int pre_a, int b_post) {
+ // CHECK: Decision,File 0, [[@LINE+2]]:11 -> [[@LINE+3]]:18 = M:0, C:2
+ // CHECK: Branch,File 0, [[@LINE+1]]:11 -> [[@LINE+1]]:16 = (#0 - #1), #1 [1,0,2]
+ return (pre_a
+ || POST(b));
+ // CHECK: Expansion,File 0, [[@LINE-1]]:14 -> [[@LINE-1]]:18 = #1 (Expanded file = 1)
+ // CHECK: Branch,File 1, [[@LINE-9]]:17 -> [[@LINE-9]]:20 = (#1 - #2), #2 [2,0,0]
+}
+
+#define bar_post b_post
+
+// CHECK: post1:
+int post1(int pre_a, int b_post) {
+ // CHECK: Decision,File 0, [[@LINE+3]]:11 -> [[@LINE+4]]:18 = M:0, C:2
+ // CHECK: Branch,File 0, [[@LINE+2]]:11 -> [[@LINE+2]]:16 = (#0 - #1), #1 [1,0,2]
+ // CHECK: Expansion,File 0, [[@LINE+2]]:14 -> [[@LINE+2]]:18 = 0 (Expanded file = 1)
+ return (pre_a
+ || POST(bar));
+ // CHECK: Expansion,File 1, 42:17 -> 42:18 = #1 (Expanded file = 2)
+ // CHECK: Branch,File 2, 54:18 -> 54:24 = (#1 - #2), #2 [2,0,0]
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/89869
More information about the cfe-commits
mailing list