[clang] [Coverage][Expansion] handle nested macros in scratch space (PR #89869)

Wentao Zhang via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 24 14:29:05 PDT 2024


https://github.com/whentojump updated https://github.com/llvm/llvm-project/pull/89869

>From 968ef430dd09ee4545323426d0c5b550c6a0f690 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi <geek4civic at gmail.com>
Date: Fri, 19 Apr 2024 15:16:05 +0900
Subject: [PATCH 1/4] [MC/DC][Coverage] Workaround for `##` conditions

A synthesized identifier with `##` is emitted to `<scratch space>`.
`llvm-cov` cannot handle `<scratch space> since it doesn't have actual files.

As a workaround, peel `<scratch space>` to the actual definition if the definition is present.

This affects predefined built-in macros, like __LINE__.

Fixes #87000
---
 clang/lib/CodeGen/CoverageMappingGen.cpp      | 12 +++++-
 clang/test/CoverageMapping/builtinmacro.c     |  2 +-
 clang/test/CoverageMapping/macros.c           |  8 ++--
 .../test/CoverageMapping/mcdc-scratch-space.c | 39 +++++++++++++++++++
 4 files changed, 56 insertions(+), 5 deletions(-)
 create mode 100644 clang/test/CoverageMapping/mcdc-scratch-space.c

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 733686d4946b3c..f92706f579a7b9 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -339,8 +339,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;
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..1b8735cd27445a
--- /dev/null
+++ b/clang/test/CoverageMapping/mcdc-scratch-space.c
@@ -0,0 +1,39 @@
+// 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 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]
+}

>From ee6c8b668aac575f7dfb52319703f9ab3006921a Mon Sep 17 00:00:00 2001
From: Wentao Zhang <zhangwt1997 at gmail.com>
Date: Tue, 23 Apr 2024 22:46:54 -0500
Subject: [PATCH 2/4] [Coverage][Expansion] handle nested macros in scratch
 space

---
 clang/lib/CodeGen/CoverageMappingGen.cpp      | 19 +++++++++++++-
 .../test/CoverageMapping/mcdc-scratch-space.c | 26 +++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index f92706f579a7b9..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>";
@@ -527,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/mcdc-scratch-space.c b/clang/test/CoverageMapping/mcdc-scratch-space.c
index 1b8735cd27445a..2b5b12d9dcad65 100644
--- a/clang/test/CoverageMapping/mcdc-scratch-space.c
+++ b/clang/test/CoverageMapping/mcdc-scratch-space.c
@@ -26,6 +26,19 @@ int pre0(int pre_a, int b_post) {
   // 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:
@@ -37,3 +50,16 @@ int post0(int pre_a, int b_post) {
   // 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]
+}

>From 145c807a177db8e649f7f3343d640a000e65cebb Mon Sep 17 00:00:00 2001
From: Wentao Zhang <zhangwt1997 at gmail.com>
Date: Wed, 24 Apr 2024 14:05:45 -0500
Subject: [PATCH 3/4] address comments: one getIncludeOrExpansionLoc() function
 controlled by a flag

---
 clang/lib/CodeGen/CoverageMappingGen.cpp | 31 ++++++++++--------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 9268ac5b4c61a3..068f90846d89f1 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -292,25 +292,21 @@ class CoverageMappingBuilder {
     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 the current file is included or macro is expanded. If the
-  /// found expansion is a <scratch space>, keep looking.
-  SourceLocation getIncludeOrNonScratchExpansionLoc(SourceLocation Loc) {
+  /// 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()) {
       Loc = SM.getImmediateExpansionRange(Loc).getBegin();
-      while (Loc.isMacroID() &&
-             SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
-        auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
-        Loc = ExpansionRange.getBegin();
-      }
-    } else {
+      if (!AcceptScratch)
+        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;
   }
 
@@ -543,8 +539,7 @@ class CoverageMappingBuilder {
     SourceRegionFilter Filter;
     for (const auto &FM : FileIDMapping) {
       SourceLocation ExpandedLoc = FM.second.second;
-      SourceLocation ParentLoc =
-          getIncludeOrNonScratchExpansionLoc(ExpandedLoc);
+      SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc, false);
       if (ParentLoc.isInvalid())
         continue;
 

>From 93ace1a2b2317ed1f040e30ff7a4e2f01beb3a9b Mon Sep 17 00:00:00 2001
From: Wentao Zhang <zhangwt1997 at gmail.com>
Date: Wed, 24 Apr 2024 16:21:24 -0500
Subject: [PATCH 4/4] address comments: extract common code

---
 clang/lib/CodeGen/CoverageMappingGen.cpp | 38 ++++++++++++++++--------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 068f90846d89f1..be2fa16cee66c0 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -292,6 +292,25 @@ class CoverageMappingBuilder {
     return SM.getLocForEndOfFile(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>.
@@ -300,11 +319,7 @@ class CoverageMappingBuilder {
     if (Loc.isMacroID()) {
       Loc = SM.getImmediateExpansionRange(Loc).getBegin();
       if (!AcceptScratch)
-        while (Loc.isMacroID() &&
-               SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
-          auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
-          Loc = ExpansionRange.getBegin();
-        }
+        Loc = getNonScratchExpansionLoc(Loc).first;
     } else
       Loc = SM.getIncludeLoc(SM.getFileID(Loc));
     return Loc;
@@ -355,13 +370,12 @@ class CoverageMappingBuilder {
       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());
-      }
+      auto NonScratchExpansionLoc = getNonScratchExpansionLoc(Loc);
+      Loc = NonScratchExpansionLoc.first;
+      auto EndLoc = NonScratchExpansionLoc.second;
+      Region.setStartLoc(Loc);
+      Region.setEndLoc(EndLoc.has_value() ? EndLoc.value()
+                                          : Region.getEndLoc());
 
       FileID File = SM.getFileID(Loc);
       if (!Visited.insert(File).second)



More information about the cfe-commits mailing list