aboutsummaryrefslogtreecommitdiff
path: root/clang/lib/CodeGen/CoverageMappingGen.cpp
diff options
context:
space:
mode:
authorWentao Zhang <35722712+whentojump@users.noreply.github.com>2024-05-23 22:06:43 -0500
committerGitHub <noreply@github.com>2024-05-24 12:06:43 +0900
commitf9e9e599c0138b2403bc89934ddace0c2a8d84d6 (patch)
tree68aa412a538c78791f02d389821dcb56e085aa44 /clang/lib/CodeGen/CoverageMappingGen.cpp
parent09c5525610579d71f45502c304146db1d7b38107 (diff)
downloadllvm-f9e9e599c0138b2403bc89934ddace0c2a8d84d6.zip
llvm-f9e9e599c0138b2403bc89934ddace0c2a8d84d6.tar.gz
llvm-f9e9e599c0138b2403bc89934ddace0c2a8d84d6.tar.bz2
[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 <geek4civic@gmail.com>
Diffstat (limited to 'clang/lib/CodeGen/CoverageMappingGen.cpp')
-rw-r--r--clang/lib/CodeGen/CoverageMappingGen.cpp45
1 files changed, 40 insertions, 5 deletions
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 993d7cc..05de50e 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -294,10 +294,36 @@ public:
return SM.getLocForEndOfFile(SM.getFileID(Loc));
}
- /// Find out where the current file is included or macro is expanded.
- SourceLocation getIncludeOrExpansionLoc(SourceLocation Loc) {
- return Loc.isMacroID() ? SM.getImmediateExpansionRange(Loc).getBegin()
- : SM.getIncludeLoc(SM.getFileID(Loc));
+ /// Find out where a macro is expanded. If the immediate result is a
+ /// <scratch space>, keep looking until the result isn't. Return a pair of
+ /// \c SourceLocation. The first object is always the begin sloc of found
+ /// result. The second should be checked by the caller: if it has value, it's
+ /// the end sloc of the found result. Otherwise the while loop didn't get
+ /// executed, which means the location wasn't changed and the caller has to
+ /// learn the end sloc from somewhere else.
+ std::pair<SourceLocation, std::optional<SourceLocation>>
+ getNonScratchExpansionLoc(SourceLocation Loc) {
+ std::optional<SourceLocation> EndLoc = std::nullopt;
+ while (Loc.isMacroID() &&
+ SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
+ auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
+ Loc = ExpansionRange.getBegin();
+ EndLoc = ExpansionRange.getEnd();
+ }
+ return std::make_pair(Loc, EndLoc);
+ }
+
+ /// Find out where the current file is included or macro is expanded. If
+ /// \c AcceptScratch is set to false, keep looking for expansions until the
+ /// found sloc is not a <scratch space>.
+ SourceLocation getIncludeOrExpansionLoc(SourceLocation Loc,
+ bool AcceptScratch = true) {
+ if (!Loc.isMacroID())
+ return SM.getIncludeLoc(SM.getFileID(Loc));
+ Loc = SM.getImmediateExpansionRange(Loc).getBegin();
+ if (AcceptScratch)
+ return Loc;
+ return getNonScratchExpansionLoc(Loc).first;
}
/// Return true if \c Loc is a location in a built-in macro.
@@ -344,6 +370,15 @@ public:
for (auto &Region : SourceRegions) {
SourceLocation Loc = Region.getBeginLoc();
+ // Replace Region with its definition if it is in <scratch space>.
+ auto NonScratchExpansionLoc = getNonScratchExpansionLoc(Loc);
+ auto EndLoc = NonScratchExpansionLoc.second;
+ if (EndLoc.has_value()) {
+ Loc = NonScratchExpansionLoc.first;
+ Region.setStartLoc(Loc);
+ Region.setEndLoc(EndLoc.value());
+ }
+
// Replace Loc with FileLoc if it is expanded with system headers.
if (!SystemHeadersCoverage && SM.isInSystemMacro(Loc)) {
auto BeginLoc = SM.getSpellingLoc(Loc);
@@ -538,7 +573,7 @@ public:
SourceRegionFilter Filter;
for (const auto &FM : FileIDMapping) {
SourceLocation ExpandedLoc = FM.second.second;
- SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc);
+ SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc, false);
if (ParentLoc.isInvalid())
continue;