Author: Fangrui Song Date: 2026-05-10T10:38:05-07:00 New Revision: 10f941727ffc81c9237499d259e1246dd70263ab
URL: https://github.com/llvm/llvm-project/commit/10f941727ffc81c9237499d259e1246dd70263ab DIFF: https://github.com/llvm/llvm-project/commit/10f941727ffc81c9237499d259e1246dd70263ab.diff LOG: [Coverage] Fix assertion failure when a -isystem header invokes a user macro (#195427) ``` // a.cc static void foo(int x) { switch (x) { #define GENERIC(n) case n: #include "types.def" // -isystem header invokes a user macro break; } } // sys/types.def #define MID(name) GENERIC(name) MID(0) MID(1) MID(2) ``` ``` $ clang -fprofile-instr-generate -fcoverage-mapping -isystem sys -c a.cc Assertion `SystemHeadersCoverage || !SM.isInSystemHeader(SM.getSpellingLoc(Loc))' failed. ``` Commit 702a2b627ff4 ("[Coverage] Rework !SystemHeadersCoverage") replaced the system-header skip in gatherFileIDs with this assertion, which trips as `SM.isInSystemHeader(SM.getSpellingLoc(Loc))` is false. This patch adds back the pre-#91446 condition but folds it with the macro-token remap `if` statement. Fixes #179316/#195422. Clang Opus 4.7 identified clang/lib/Parse/ParseExpr.cpp, created a minimal reproduce with cvise, and wrote the initial version of this CodeGen patch. (An earlier session papered over the bug by patching llvm-cov instead, which I abandoned). Added: clang/test/CoverageMapping/system_macro_switch.cpp Modified: clang/lib/CodeGen/CoverageMappingGen.cpp Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 803037d1874b3..eadb6e3bb25a8 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -386,24 +386,29 @@ class CoverageMappingBuilder { 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); - auto EndLoc = SM.getSpellingLoc(Region.getEndLoc()); - if (SM.isWrittenInSameFile(BeginLoc, EndLoc)) { - Loc = SM.getFileLoc(Loc); - Region.setStartLoc(Loc); - Region.setEndLoc(SM.getFileLoc(Region.getEndLoc())); + // For regions whose spelling is in a system header, remap macro + // tokens to their user-code call site so coverage is attributed to + // the user expression. Drop anything still in a system header + // (e.g. a plain FileID into a -isystem .def file). + if (!SystemHeadersCoverage && + SM.isInSystemHeader(SM.getSpellingLoc(Loc))) { + if (Loc.isMacroID()) { + auto BeginLoc = SM.getSpellingLoc(Loc); + auto EndLoc = SM.getSpellingLoc(Region.getEndLoc()); + if (SM.isWrittenInSameFile(BeginLoc, EndLoc)) { + Loc = SM.getFileLoc(Loc); + Region.setStartLoc(Loc); + Region.setEndLoc(SM.getFileLoc(Region.getEndLoc())); + } } + if (SM.isInSystemHeader(SM.getSpellingLoc(Loc))) + continue; } FileID File = SM.getFileID(Loc); if (!Visited.insert(File).second) continue; - assert(SystemHeadersCoverage || - !SM.isInSystemHeader(SM.getSpellingLoc(Loc))); - unsigned Depth = 0; for (SourceLocation Parent = getIncludeOrExpansionLoc(Loc); Parent.isValid(); Parent = getIncludeOrExpansionLoc(Parent)) diff --git a/clang/test/CoverageMapping/system_macro_switch.cpp b/clang/test/CoverageMapping/system_macro_switch.cpp new file mode 100644 index 0000000000000..71f8ebfc2a777 --- /dev/null +++ b/clang/test/CoverageMapping/system_macro_switch.cpp @@ -0,0 +1,42 @@ +/// `case` labels generated by a user macro invoked from a -isystem header +/// must not crash the coverage mapping builder. + +// RUN: rm -rf %t && split-file %s %t && cd %t +// RUN: %clang_cc1 -triple %itanium_abi_triple -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -isystem sys a.cpp | FileCheck %s + +// CHECK-LABEL: main: +// CHECK-NEXT: File 0, 9:29 -> 9:43 = #0 +// CHECK-LABEL: _ZL3fooi: +// CHECK-NEXT: File 0, 1:24 -> 7:2 = #0 +// CHECK-NEXT: Branch,File 0, 2:11 -> 2:12 = ((#2 + #3) + #4), (((#0 - #2) - #3) - #4) +// CHECK-NEXT: Gap,File 0, 2:14 -> 5:10 = 0 +// CHECK-NEXT: File 0, 4:21 -> 5:10 = ((#2 + #3) + #4) +// CHECK-NEXT: File 0, 4:21 -> 5:10 = (#2 + #3) +// CHECK-NEXT: File 0, 4:21 -> 5:10 = #2 +// CHECK-NEXT: File 1, 3:20 -> 3:27 = #2 +// CHECK-NEXT: Branch,File 1, 3:20 -> 3:26 = #2, (#0 - #2) +// CHECK-NEXT: File 2, 3:20 -> 3:27 = (#2 + #3) +// CHECK-NEXT: Branch,File 2, 3:20 -> 3:26 = #3, (#0 - #3) +// CHECK-NEXT: File 3, 3:20 -> 3:27 = ((#2 + #3) + #4) +// CHECK-NEXT: Branch,File 3, 3:20 -> 3:26 = #4, (#0 - #4) + +/// types.def must not leak into the file mapping. +// CHECK-NOT: Expansion, +// CHECK-NOT: File + +//--- a.cpp +static void foo(int x) { + switch (x) { +#define GENERIC(n) case n: +#include "types.def" + break; + } +} + +int main(int argc, char **) { foo(argc); } + +//--- sys/types.def +#define MID(name) GENERIC(name) +MID(0) +MID(1) +MID(2) _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
