[clang] [Coverage] Fix assertion failure when a -isystem header invokes a user macro (PR #195427)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Fangrui Song (MaskRay)

<details>
<summary>Changes</summary>

```
  // 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 #<!-- -->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).


---
Full diff: https://github.com/llvm/llvm-project/pull/195427.diff


2 Files Affected:

- (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+16-11) 
- (added) clang/test/CoverageMapping/system_macro_switch.cpp (+26) 


``````````diff
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..f6a43d8394d7e
--- /dev/null
+++ b/clang/test/CoverageMapping/system_macro_switch.cpp
@@ -0,0 +1,26 @@
+/// `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 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -isystem sys a.cpp | FileCheck %s
+
+// CHECK-LABEL: _ZL3fooi:
+// CHECK-NEXT: File 0, 1:24 -> 7:2 = #0
+// CHECK-NEXT: Branch,File 0, 2:11 -> 2:12
+
+//--- 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)

``````````

</details>


https://github.com/llvm/llvm-project/pull/195427