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

2024-05-23 Thread Wentao Zhang via cfe-commits 

whentojump wrote:

Thanks everyone!

I notice that #87000 has been tagged with release milestone. I can handle the 
backport there. Maybe in a few days? Or sooner if that's needed.

https://github.com/llvm/llvm-project/pull/89869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-05-23 Thread Wentao Zhang via cfe-commits 

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

>From 510e0f131ad11d7924f33fc4c4130dcf866bd8da Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi 
Date: Fri, 19 Apr 2024 15:16:05 +0900
Subject: [PATCH 1/9] [MC/DC][Coverage] Workaround for `##` conditions

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

As a workaround, peel `` to the actual definition if the 
definition is present.

This affects predefined built-in macros, like __LINE__.

Fixes #87000
---
 clang/lib/CodeGen/CoverageMappingGen.cpp  |  9 +++
 clang/test/CoverageMapping/builtinmacro.c |  2 +-
 clang/test/CoverageMapping/macros.c   |  8 +++---
 .../test/CoverageMapping/mcdc-scratch-space.c | 26 ++-
 4 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 993d7cc7e21fa..d6b0145d19fea 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -344,6 +344,15 @@ class CoverageMappingBuilder {
 for (auto  : SourceRegions) {
   SourceLocation Loc = Region.getBeginLoc();
 
+  // Replace Region with its definition if it is in .
+  while (Loc.isMacroID() &&
+ SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
+auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
+Loc = ExpansionRange.getBegin();
+Region.setStartLoc(Loc);
+Region.setEndLoc(ExpansionRange.getEnd());
+  }
+
   // Replace Loc with FileLoc if it is expanded with system headers.
   if (!SystemHeadersCoverage && SM.isInSystemMacro(Loc)) {
 auto BeginLoc = SM.getSpellingLoc(Loc);
diff --git a/clang/test/CoverageMapping/builtinmacro.c 
b/clang/test/CoverageMapping/builtinmacro.c
index abcdc191523a5..5d5a176aa7d87 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 6bd3be434139a..fcf21170ef135 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
index 962d10653a028..1b8735cd27445 100644
--- a/clang/test/CoverageMapping/mcdc-scratch-space.c
+++ b/clang/test/CoverageMapping/mcdc-scratch-space.c
@@ -1,27 +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
-// XFAIL: *
-// REQUIRES: asserts
+// 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) {
-  return (__LINE__
-  && 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

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

2024-05-23 Thread Wentao Zhang via cfe-commits 

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

>From 510e0f131ad11d7924f33fc4c4130dcf866bd8da Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi 
Date: Fri, 19 Apr 2024 15:16:05 +0900
Subject: [PATCH 1/8] [MC/DC][Coverage] Workaround for `##` conditions

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

As a workaround, peel `` to the actual definition if the 
definition is present.

This affects predefined built-in macros, like __LINE__.

Fixes #87000
---
 clang/lib/CodeGen/CoverageMappingGen.cpp  |  9 +++
 clang/test/CoverageMapping/builtinmacro.c |  2 +-
 clang/test/CoverageMapping/macros.c   |  8 +++---
 .../test/CoverageMapping/mcdc-scratch-space.c | 26 ++-
 4 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 993d7cc7e21fa..d6b0145d19fea 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -344,6 +344,15 @@ class CoverageMappingBuilder {
 for (auto  : SourceRegions) {
   SourceLocation Loc = Region.getBeginLoc();
 
+  // Replace Region with its definition if it is in .
+  while (Loc.isMacroID() &&
+ SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
+auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
+Loc = ExpansionRange.getBegin();
+Region.setStartLoc(Loc);
+Region.setEndLoc(ExpansionRange.getEnd());
+  }
+
   // Replace Loc with FileLoc if it is expanded with system headers.
   if (!SystemHeadersCoverage && SM.isInSystemMacro(Loc)) {
 auto BeginLoc = SM.getSpellingLoc(Loc);
diff --git a/clang/test/CoverageMapping/builtinmacro.c 
b/clang/test/CoverageMapping/builtinmacro.c
index abcdc191523a5..5d5a176aa7d87 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 6bd3be434139a..fcf21170ef135 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
index 962d10653a028..1b8735cd27445 100644
--- a/clang/test/CoverageMapping/mcdc-scratch-space.c
+++ b/clang/test/CoverageMapping/mcdc-scratch-space.c
@@ -1,27 +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
-// XFAIL: *
-// REQUIRES: asserts
+// 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) {
-  return (__LINE__
-  && 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

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

2024-05-22 Thread Wentao Zhang via cfe-commits 


@@ -517,7 +552,7 @@ class CoverageMappingBuilder {
 SourceRegionFilter Filter;
 for (const auto  : FileIDMapping) {
   SourceLocation ExpandedLoc = FM.second.second;
-  SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc);
+  SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc, false);

whentojump wrote:

Hi, in the initial version we were actually using a separate function (but with 
some duplicate code). See this commit 
https://github.com/llvm/llvm-project/pull/89869/commits/145c807a177db8e649f7f3343d640a000e65cebb

Do you suggest that way is better?

https://github.com/llvm/llvm-project/pull/89869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-05-22 Thread Wentao Zhang via cfe-commits 

whentojump wrote:

Sorry for getting back this late.

I reverted the latest commit (using `SourceRange`) and addressed the earlier 
comment about updating region sloc only when they are changed.

And yes if you have better ideas about `getNonScratchExpansion` implementation 
you can please post a new PR.

https://github.com/llvm/llvm-project/pull/89869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-05-22 Thread Wentao Zhang via cfe-commits 

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 
Date: Fri, 19 Apr 2024 15:16:05 +0900
Subject: [PATCH 1/8] [MC/DC][Coverage] Workaround for `##` conditions

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

As a workaround, peel `` 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 733686d4946b3..f92706f579a7b 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -339,8 +339,18 @@ class CoverageMappingBuilder {
 
 llvm::SmallSet Visited;
 SmallVector, 8> FileLocs;
-for (const auto  : SourceRegions) {
+for (auto  : SourceRegions) {
   SourceLocation Loc = Region.getBeginLoc();
+
+  // Replace Region with its definition if it is in .
+  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 abcdc191523a5..5d5a176aa7d87 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 6bd3be434139a..fcf21170ef135 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 0..1b8735cd27445
--- /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]
+

[clang] [clang][CoverageMapping] do not emit gap when either end is an `ImplicitValueInitExpr` (PR #89564)

2024-04-27 Thread Wentao Zhang via cfe-commits 

https://github.com/whentojump milestoned 
https://github.com/llvm/llvm-project/pull/89564
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][CoverageMapping] do not emit gap when either end is an `ImplicitValueInitExpr` (PR #89564)

2024-04-27 Thread Wentao Zhang via cfe-commits 

whentojump wrote:

/cherry-pick c1b6cca1214e7a9c14a30b81585dd8b81baeaa77


https://github.com/llvm/llvm-project/pull/89564
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-04-25 Thread Wentao Zhang via cfe-commits 


@@ -292,10 +292,36 @@ 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 a macro is expanded. If the immediate result is a
+  /// , keep looking until the result isn't. Return the source
+  /// range of the found result, or std::nullopt if the while loop didn't get
+  /// executed, which means the location wasn't changed.
+  std::optional getNonScratchExpansion(SourceLocation Loc) {
+std::optional EndLoc = std::nullopt;
+while (Loc.isMacroID() &&
+   SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
+  auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
+  Loc = ExpansionRange.getBegin();
+  EndLoc = ExpansionRange.getEnd();
+}
+if (EndLoc.has_value())
+  return SourceRange(Loc, EndLoc.value());

whentojump wrote:

> Actually returns CharSourceRange.

True. Ideally we can circulate around `CharSourceRange` without converting back 
and forth too much but I found it hard to make both two callers clean and 
straightforward. 

> You may rewind them 

I can do that, meanwhile I'll also seek if there's a better way



https://github.com/llvm/llvm-project/pull/89869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-04-24 Thread Wentao Zhang via cfe-commits 

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 
Date: Fri, 19 Apr 2024 15:16:05 +0900
Subject: [PATCH 1/6] [MC/DC][Coverage] Workaround for `##` conditions

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

As a workaround, peel `` 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 Visited;
 SmallVector, 8> FileLocs;
-for (const auto  : SourceRegions) {
+for (auto  : SourceRegions) {
   SourceLocation Loc = Region.getBeginLoc();
+
+  // Replace Region with its definition if it is in .
+  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 00..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]
+

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

2024-04-24 Thread Wentao Zhang via cfe-commits 

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 
Date: Fri, 19 Apr 2024 15:16:05 +0900
Subject: [PATCH 1/5] [MC/DC][Coverage] Workaround for `##` conditions

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

As a workaround, peel `` 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 Visited;
 SmallVector, 8> FileLocs;
-for (const auto  : SourceRegions) {
+for (auto  : SourceRegions) {
   SourceLocation Loc = Region.getBeginLoc();
+
+  // Replace Region with its definition if it is in .
+  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 00..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]
+

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

2024-04-24 Thread Wentao Zhang via cfe-commits 

https://github.com/whentojump edited 
https://github.com/llvm/llvm-project/pull/89869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-04-24 Thread Wentao Zhang via cfe-commits 


@@ -339,8 +355,18 @@ class CoverageMappingBuilder {
 
 llvm::SmallSet Visited;
 SmallVector, 8> FileLocs;
-for (const auto  : SourceRegions) {
+for (auto  : SourceRegions) {
   SourceLocation Loc = Region.getBeginLoc();
+
+  // Replace Region with its definition if it is in .
+  while (Loc.isMacroID() &&
+ SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc))) {
+auto ExpansionRange = SM.getImmediateExpansionRange(Loc);
+Loc = ExpansionRange.getBegin();
+Region.setStartLoc(Loc);
+Region.setEndLoc(ExpansionRange.getEnd());
+  }

whentojump wrote:

The common code is now being reused. But it looks a bit awkward. 
I'd like to hear your suggestions.

https://github.com/llvm/llvm-project/pull/89869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-04-24 Thread Wentao Zhang via cfe-commits 

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 
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 ``.
`llvm-cov` cannot handle ` since it doesn't have actual files.

As a workaround, peel `` 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 Visited;
 SmallVector, 8> FileLocs;
-for (const auto  : SourceRegions) {
+for (auto  : SourceRegions) {
   SourceLocation Loc = Region.getBeginLoc();
+
+  // Replace Region with its definition if it is in .
+  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 00..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]
+

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

2024-04-24 Thread Wentao Zhang via cfe-commits 

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 
Date: Fri, 19 Apr 2024 15:16:05 +0900
Subject: [PATCH 1/3] [MC/DC][Coverage] Workaround for `##` conditions

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

As a workaround, peel `` 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 Visited;
 SmallVector, 8> FileLocs;
-for (const auto  : SourceRegions) {
+for (auto  : SourceRegions) {
   SourceLocation Loc = Region.getBeginLoc();
+
+  // Replace Region with its definition if it is in .
+  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 00..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]
+

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

2024-04-24 Thread Wentao Zhang via cfe-commits 

https://github.com/whentojump edited 
https://github.com/llvm/llvm-project/pull/89869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [MC/DC][Coverage] Workaround for `##` conditions (PR #89573)

2024-04-24 Thread Wentao Zhang via cfe-commits 

whentojump wrote:

Yes I can do it. Thank you as well!!

https://github.com/llvm/llvm-project/pull/89573
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-04-24 Thread Wentao Zhang via cfe-commits 

https://github.com/whentojump edited 
https://github.com/llvm/llvm-project/pull/89869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-04-24 Thread Wentao Zhang via cfe-commits 

https://github.com/whentojump ready_for_review 
https://github.com/llvm/llvm-project/pull/89869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-04-24 Thread Wentao Zhang via cfe-commits 

https://github.com/whentojump edited 
https://github.com/llvm/llvm-project/pull/89869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-04-23 Thread Wentao Zhang via cfe-commits 

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 
Date: Fri, 19 Apr 2024 15:16:05 +0900
Subject: [PATCH 1/2] [MC/DC][Coverage] Workaround for `##` conditions

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

As a workaround, peel `` 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 733686d4946b3c1..f92706f579a7b9e 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -339,8 +339,18 @@ class CoverageMappingBuilder {
 
 llvm::SmallSet Visited;
 SmallVector, 8> FileLocs;
-for (const auto  : SourceRegions) {
+for (auto  : SourceRegions) {
   SourceLocation Loc = Region.getBeginLoc();
+
+  // Replace Region with its definition if it is in .
+  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 abcdc191523a5d5..5d5a176aa7d87e2 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 6bd3be434139a21..fcf21170ef135cf 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 000..1b8735cd27445a1
--- /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

[clang] [MC/DC][Coverage] Workaround for `##` conditions (PR #89573)

2024-04-23 Thread Wentao Zhang via cfe-commits 

whentojump wrote:

@chapuni Would you please take a look at #89869? Specifically, the extra one 
commit atop yours.

Essentially I did similar things to `getIncludeOrExpansionLoc()`. 


https://github.com/llvm/llvm-project/pull/89573
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-04-23 Thread Wentao Zhang via cfe-commits 

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

None

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

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

As a workaround, peel `` 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 733686d4946b3c1..f92706f579a7b9e 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -339,8 +339,18 @@ class CoverageMappingBuilder {
 
 llvm::SmallSet Visited;
 SmallVector, 8> FileLocs;
-for (const auto  : SourceRegions) {
+for (auto  : SourceRegions) {
   SourceLocation Loc = Region.getBeginLoc();
+
+  // Replace Region with its definition if it is in .
+  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 abcdc191523a5d5..5d5a176aa7d87e2 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 6bd3be434139a21..fcf21170ef135cf 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 000..1b8735cd27445a1
--- /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),

[clang] [MC/DC][Coverage] Workaround for `##` conditions (PR #89573)

2024-04-22 Thread Wentao Zhang via cfe-commits 

whentojump wrote:

Hi, thanks for the fix.

I can confirm it addressed your test case (https://godbolt.org/z/sMscqWeq7). 
But unfortunately it doesn't fix the one in my original post of #87000 
([compiler explorer

[clang] [clang][CoverageMapping] do not emit gap when either end is an `ImplicitValueInitExpr` (PR #89564)

2024-04-22 Thread Wentao Zhang via cfe-commits 

https://github.com/whentojump closed 
https://github.com/llvm/llvm-project/pull/89564
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][CoverageMapping] do not emit gap when either end is an `ImplicitValueInitExpr` (PR #89564)

2024-04-22 Thread Wentao Zhang via cfe-commits 

whentojump wrote:

Fwiw, I uncovered #86998 in the first place when I was compiling this file in 
Linux kernel: 
https://elixir.bootlin.com/linux/v6.8.1/source/fs/coredump.c#L545. I can 
confirm on my local side, with this patch, `fs/coredump.o` can be compiled and 
instrumented successfully.

I am going to squash merge this PR.

https://github.com/llvm/llvm-project/pull/89564
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][CoverageMapping] do not emit gap when either end is an `ImplicitValueInitExpr` (PR #89564)

2024-04-22 Thread Wentao Zhang via cfe-commits 

whentojump wrote:

Thanks for your suggestion! @ZequanWu 

Would you please take another look?

https://github.com/llvm/llvm-project/pull/89564
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][CoverageMapping] do not emit gap when either end is an `ImplicitValueInitExpr` (PR #89564)

2024-04-22 Thread Wentao Zhang via cfe-commits 

https://github.com/whentojump ready_for_review 
https://github.com/llvm/llvm-project/pull/89564
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][CoverageMapping] do not emit gap when either end is an `ImplicitValueInitExpr` (PR #89564)

2024-04-22 Thread Wentao Zhang via cfe-commits 

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

>From abbdb318d62bb2e5ab6f07e7d0fe11f4a06b5a11 Mon Sep 17 00:00:00 2001
From: Wentao Zhang 
Date: Sun, 21 Apr 2024 21:27:01 -0500
Subject: [PATCH 1/4] [clang][CoverageMapping] do not emit gap when either end
 is an ImplicitValueInitExpr

---
 clang/lib/CodeGen/CoverageMappingGen.cpp | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 64c39c5de351c7..dd8c0577d758ca 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1368,9 +1368,12 @@ struct CounterCoverageMappingBuilder
 for (const Stmt *Child : S->children())
   if (Child) {
 // If last statement contains terminate statements, add a gap area
-// between the two statements. Skipping attributed statements, because
-// they don't have valid start location.
-if (LastStmt && HasTerminateStmt && !isa(Child)) {
+// between the two statements. Skipping attributed statements and
+// implicit initializations, because they don't have valid source
+// location.
+if (LastStmt && HasTerminateStmt && !isa(Child) &&
+!isa(Child) &&
+!isa(LastStmt)) {
   auto Gap = findGapAreaBetween(getEnd(LastStmt), getStart(Child));
   if (Gap)
 fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(),

>From f2931efcc97ce418ab724e2cb052b47ec5c87986 Mon Sep 17 00:00:00 2001
From: Wentao Zhang 
Date: Mon, 22 Apr 2024 11:17:49 -0500
Subject: [PATCH 2/4] add tests

---
 .../CoverageMapping/statement-expression.c| 36 +++
 1 file changed, 36 insertions(+)
 create mode 100644 clang/test/CoverageMapping/statement-expression.c

diff --git a/clang/test/CoverageMapping/statement-expression.c 
b/clang/test/CoverageMapping/statement-expression.c
new file mode 100644
index 00..5f9ab5838af342
--- /dev/null
+++ b/clang/test/CoverageMapping/statement-expression.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false 
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
-emit-llvm-only -main-file-name statement-expression.c %s
+
+// No crash for the following examples, where GNU Statement Expression 
extension
+// could introduce region terminators (break, goto etc) before implicit
+// initializers in a struct or an array.
+// See https://github.com/llvm/llvm-project/pull/89564
+
+struct Foo {
+  int field1;
+  int field2;
+};
+
+void f1(void) {
+  struct Foo foo = {
+.field1 = ({
+  switch (0) {
+  case 0:
+break; // A region terminator
+  }
+  0;
+}),
+// ImplicitValueInitExpr introduced here for .field2
+  };
+}
+
+void f2(void) {
+  int arr[3] = {
+[0] = ({
+goto L0; // A region terminator
+L0:
+  0;
+}),
+// ImplicitValueInitExpr introduced here for subscript [1]
+[2] = 0,
+  };
+}

>From 6a00626a44b23815b99ad217b0ab9ed8567c19a2 Mon Sep 17 00:00:00 2001
From: Wentao Zhang 
Date: Mon, 22 Apr 2024 11:20:08 -0500
Subject: [PATCH 3/4] Revert "[clang][CoverageMapping] do not emit gap when
 either end is an ImplicitValueInitExpr"

This reverts commit abbdb318d62bb2e5ab6f07e7d0fe11f4a06b5a11.
---
 clang/lib/CodeGen/CoverageMappingGen.cpp | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index dd8c0577d758ca..64c39c5de351c7 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1368,12 +1368,9 @@ struct CounterCoverageMappingBuilder
 for (const Stmt *Child : S->children())
   if (Child) {
 // If last statement contains terminate statements, add a gap area
-// between the two statements. Skipping attributed statements and
-// implicit initializations, because they don't have valid source
-// location.
-if (LastStmt && HasTerminateStmt && !isa(Child) &&
-!isa(Child) &&
-!isa(LastStmt)) {
+// between the two statements. Skipping attributed statements, because
+// they don't have valid start location.
+if (LastStmt && HasTerminateStmt && !isa(Child)) {
   auto Gap = findGapAreaBetween(getEnd(LastStmt), getStart(Child));
   if (Gap)
 fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(),

>From c7359ddf8ee689f12fa75ccc3ccff1bfecb2027a Mon Sep 17 00:00:00 2001
From: Wentao Zhang 
Date: Mon, 22 Apr 2024 11:33:41 -0500
Subject: [PATCH 4/4] move into findGapAreaBetween

---
 clang/lib/CodeGen/CoverageMappingGen.cpp | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 64c39c5de351c7..733686d4946b3c 100644
---

[clang] [clang][CoverageMapping] do not emit gap when either end is an `ImplicitValueInitExpr` (PR #89564)

2024-04-22 Thread Wentao Zhang via cfe-commits 

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

>From abbdb318d62bb2e5ab6f07e7d0fe11f4a06b5a11 Mon Sep 17 00:00:00 2001
From: Wentao Zhang 
Date: Sun, 21 Apr 2024 21:27:01 -0500
Subject: [PATCH 1/2] [clang][CoverageMapping] do not emit gap when either end
 is an ImplicitValueInitExpr

---
 clang/lib/CodeGen/CoverageMappingGen.cpp | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 64c39c5de351c7..dd8c0577d758ca 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1368,9 +1368,12 @@ struct CounterCoverageMappingBuilder
 for (const Stmt *Child : S->children())
   if (Child) {
 // If last statement contains terminate statements, add a gap area
-// between the two statements. Skipping attributed statements, because
-// they don't have valid start location.
-if (LastStmt && HasTerminateStmt && !isa(Child)) {
+// between the two statements. Skipping attributed statements and
+// implicit initializations, because they don't have valid source
+// location.
+if (LastStmt && HasTerminateStmt && !isa(Child) &&
+!isa(Child) &&
+!isa(LastStmt)) {
   auto Gap = findGapAreaBetween(getEnd(LastStmt), getStart(Child));
   if (Gap)
 fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(),

>From cc42595ea34f4f4b5b9dda968f323ec95ba712e9 Mon Sep 17 00:00:00 2001
From: Wentao Zhang 
Date: Mon, 22 Apr 2024 11:17:49 -0500
Subject: [PATCH 2/2] add tests

---
 .../CoverageMapping/statement-expression.c| 36 +++
 1 file changed, 36 insertions(+)
 create mode 100644 clang/test/CoverageMapping/statement-expression.c

diff --git a/clang/test/CoverageMapping/statement-expression.c 
b/clang/test/CoverageMapping/statement-expression.c
new file mode 100644
index 00..6006e3646ee943
--- /dev/null
+++ b/clang/test/CoverageMapping/statement-expression.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false 
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
-emit-llvm-only -main-file-name statement-expression.c %s
+
+// No crash for the following examples, where GNU Statement Expression 
extension
+// could contain region terminators (break, goto etc) before implicit
+// initializers in a struct or an array.
+// See https://github.com/llvm/llvm-project/pull/89564
+
+struct Foo {
+  int field1;
+  int field2;
+};
+
+void test1(void) {
+  struct Foo foo = {
+.field1 = ({
+  switch (0) {
+  case 0:
+break; // A region terminator
+  }
+  0;
+}),
+// ImplicitValueInitExpr introduced here for .field2
+  };
+}
+
+void test2(void) {
+  int arr[3] = {
+[0] = ({
+goto L0; // A region terminator
+L0:
+  0;
+}),
+// ImplicitValueInitExpr introduced here for subscript [1]
+[2] = 0,
+  };
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][CoverageMapping] do not emit gap when either end is an `ImplicitValueInitExpr` (PR #89564)

2024-04-22 Thread Wentao Zhang via cfe-commits 

https://github.com/whentojump converted_to_draft 
https://github.com/llvm/llvm-project/pull/89564
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][CoverageMapping] do not emit gap when either end is an `ImplicitValueInitExpr` (PR #89564)

2024-04-21 Thread Wentao Zhang via cfe-commits 

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

>From abbdb318d62bb2e5ab6f07e7d0fe11f4a06b5a11 Mon Sep 17 00:00:00 2001
From: Wentao Zhang 
Date: Sun, 21 Apr 2024 21:27:01 -0500
Subject: [PATCH] [clang][CoverageMapping] do not emit gap when either end is
 an ImplicitValueInitExpr

---
 clang/lib/CodeGen/CoverageMappingGen.cpp | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 64c39c5de351c7..dd8c0577d758ca 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1368,9 +1368,12 @@ struct CounterCoverageMappingBuilder
 for (const Stmt *Child : S->children())
   if (Child) {
 // If last statement contains terminate statements, add a gap area
-// between the two statements. Skipping attributed statements, because
-// they don't have valid start location.
-if (LastStmt && HasTerminateStmt && !isa(Child)) {
+// between the two statements. Skipping attributed statements and
+// implicit initializations, because they don't have valid source
+// location.
+if (LastStmt && HasTerminateStmt && !isa(Child) &&
+!isa(Child) &&
+!isa(LastStmt)) {
   auto Gap = findGapAreaBetween(getEnd(LastStmt), getStart(Child));
   if (Gap)
 fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(),

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][CoverageMapping] do not emit gap when either end is an `ImplicitValueInitExpr` (PR #89564)

2024-04-21 Thread Wentao Zhang via cfe-commits 

https://github.com/whentojump created 
https://github.com/llvm/llvm-project/pull/89564

Fixes #86998 

Two compiler explorer examples: 
[1](https://godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(filename:'1',fontScale:14,fontUsePx:'0',j:1,lang:___c,selection:(endColumn:21,endLineNumber:8,positionColumn:21,positionLineNumber:8,selectionStartColumn:21,selectionStartLineNumber:8,startColumn:21,startLineNumber:8),source:'struct+Foo+%7B%0Aint+field1%3B%0Aint+field2%3B%0A%7D%3B%0A%0Aint+main(void)+%7B%0Astruct+Foo+foo+%3D+%7B%0A.field1+%3D+(%7B%0Aswitch+(0)+%7B%0Acase+0:%0Abreak%3B%0A+//+%5E%5E%5E%5E%5E+HasTerminateStmt+set%0A%7D%0A0%3B%0A%7D),%0A//+%3C--+ImplicitValueInitExpr+introduced+for+.field2%0A%7D%3B%0A%7D'),l:'5',n:'0',o:'C+source+%231',t:'0')),k:61.44745998608211,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',overrides:!(),selection:(endColumn:45,endLineNumber:3,positionColumn:45,positionLineNumber:3,selectionStartColumn:45,selectionStartLineNumber:3,startColumn:45,startLineNumber:3),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:38.55254001391788,l:'3',n:'0',o:'',t:'0')),l:'2',m:100,n:'0',o:'',t:'0')),version:4),
 
[2](https://godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(filename:'1',fontScale:14,fontUsePx:'0',j:1,lang:___c,selection:(endColumn:2,endLineNumber:12,positionColumn:2,positionLineNumber:12,selectionStartColumn:2,selectionStartLineNumber:12,startColumn:2,startLineNumber:12),source:'int+main(void)+%7B%0Aint+arr%5B3%5D+%3D+%7B%0A%5B0%5D+%3D+(%7B%0Agoto+L0%3B%0A+//+%5E%5E%5E%5E+HasTerminateStmt+set%0AL0:%0A0%3B%0A%7D),%0A//+%3C--+ImplicitValueInitExpr+introduced+for+subscript+%5B1%5D%0A%5B2%5D+%3D+0,%0A%7D%3B%0A%7D'),l:'5',n:'0',o:'C+source+%231',t:'0')),k:61.44745998608211,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',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:38.55254001391788,l:'3',n:'0',o:'',t:'0')),l:'2',m:100,n:'0',o:'',t:'0')),version:4)

Cause:

1. When visiting AST and generating mapping regions, a [region 
terminator](https://github.com/llvm/llvm-project/blob/llvmorg-18.1.4/clang/lib/CodeGen/CoverageMappingGen.cpp#L1197)
 (like `break` and `goto`) is likely followed by a stmt with `` 
(like `ImplicitValueInitExpr`).
2. Because a terminator is seen, the below branch will be executed when 
visiting the 2nd stmt:


https://github.com/llvm/llvm-project/blob/e6c3289804a67ea0bb6a86fadbe454dd93b8d855/clang/lib/CodeGen/CoverageMappingGen.cpp#L1375-L1376

3. However, the 2nd stmt doesn't have a valid source location and will fail 
some assertions in `findGapAreaBetween()`.







>From 73e29e5dc47bfdab0708476eb5961c70de59a6cd Mon Sep 17 00:00:00 2001
From: Wentao Zhang 
Date: Sun, 21 Apr 2024 21:27:01 -0500
Subject: [PATCH] [clang][CoverageMapping] do not emit gap when either end is
 an ImplicitValueInitExpr

---
 clang/lib/CodeGen/CoverageMappingGen.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 64c39c5de351c7..45296ff9cfb5e3 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1370,7 +1370,9 @@ struct CounterCoverageMappingBuilder
 // If last statement contains terminate statements, add a gap area

[clang] [clang][CodeGen] Keep processing the rest of AST after encountering unsupported MC/DC expressions (PR #82464)

2024-02-22 Thread Wentao Zhang via cfe-commits 

https://github.com/whentojump closed 
https://github.com/llvm/llvm-project/pull/82464
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][CodeGen] Keep processing the rest of AST after encountering unsupported MC/DC expressions (PR #82464)

2024-02-22 Thread Wentao Zhang via cfe-commits 


@@ -292,7 +295,7 @@ struct MapRegionCounters : public 
RecursiveASTVisitor {
 "contains an operation with a nested boolean expression. "
 "Expression will not be covered");
 Diag.Report(S->getBeginLoc(), DiagID);
-return false;
+return true;

whentojump wrote:

Actually my point is: returning `false` would exit early from the AST 
traversal, and I suppose both places *have to* return `true`. Otherwise we'd 
observe things in my original post.




https://github.com/llvm/llvm-project/pull/82464
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][CodeGen] Keep processing the rest of AST after encountering unsupported MC/DC expressions (PR #82464)

2024-02-20 Thread Wentao Zhang via cfe-commits 

https://github.com/whentojump created 
https://github.com/llvm/llvm-project/pull/82464

Currently, upon seeing [unsupported 
decisions](https://clang.llvm.org/docs/SourceBasedCodeCoverage.html#mc-dc-instrumentation)
 (more than 6 conditions, or split nesting), the post-visitor hook 
`dataTraverseStmtPost()` returns a `false`. As a result, in the rest of tree 
even supported decisions will be skipped as well. Like in the below code:

```c
{ // CompoundStmt
  a && b;   // 1: BinaryOperator (supported)
  a && foo(b && c); // 2: BinaryOperator (not yet supported due to split 
nesting)
  a && b;   // 3: BinaryOperator (supported)
}
```

Decision 3 will not be processed at all. And only one "Decision" region will be 
emitted. Compiler explorer example: https://godbolt.org/z/Px61sesoo

We hope to process such cases and emit two "Decision" regions (1 and 3) in the 
above example.

>From 67a2c10b044e0e9c2eceb6646300e546ee990e0b Mon Sep 17 00:00:00 2001
From: Wentao Zhang 
Date: Wed, 21 Feb 2024 01:48:11 -0500
Subject: [PATCH] [clang][CodeGen] Keep processing the rest of AST after
 encountering unsupported MC/DC expressions

---
 clang/lib/CodeGen/CodeGenPGO.cpp | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 48c5e68a3b7ba4..1ef7be3c72593d 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -239,9 +239,12 @@ struct MapRegionCounters : public 
RecursiveASTVisitor {
 if (MCDCMaxCond == 0)
   return true;
 
-/// At the top of the logical operator nest, reset the number of 
conditions.
-if (LogOpStack.empty())
+/// At the top of the logical operator nest, reset the number of 
conditions,
+/// also forget previously seen split nesting cases.
+if (LogOpStack.empty()) {
   NumCond = 0;
+  SplitNestedLogicalOp = false;
+}
 
 if (const Expr *E = dyn_cast(S)) {
   const BinaryOperator *BinOp = 
dyn_cast(E->IgnoreParens());
@@ -292,7 +295,7 @@ struct MapRegionCounters : public 
RecursiveASTVisitor {
 "contains an operation with a nested boolean expression. "
 "Expression will not be covered");
 Diag.Report(S->getBeginLoc(), DiagID);
-return false;
+return true;
   }
 
   /// Was the maximum number of conditions encountered?
@@ -303,7 +306,7 @@ struct MapRegionCounters : public 
RecursiveASTVisitor {
 "number of conditions (%0) exceeds max (%1). "
 "Expression will not be covered");
 Diag.Report(S->getBeginLoc(), DiagID) << NumCond << MCDCMaxCond;
-return false;
+return true;
   }
 
   // Otherwise, allocate the number of bytes required for the bitmap

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] `llvm-cov` assertion failure when handling MC/DC that involves macros (PR #80098)

2024-01-31 Thread Wentao Zhang via cfe-commits 


@@ -1973,6 +1981,8 @@ struct CounterCoverageMappingBuilder
   void VisitBinLAnd(const BinaryOperator *E) {
 bool IsRootNode = MCDCBuilder.isIdle();
 
+MCDCDebugCounter++;

whentojump wrote:

Hi thanks again for taking the look!
My intuition was to make the ID different whenever a new decision region is 
going to be added by calling `createDecisionRegion()`. There might be something 
obvious that I ignored.
And after reading some of the other posts (I'm still learning most of them, 
please bear with that!), I generally agree with the approaches whose 
modifications are minimized within `llvm-cov`, instead of touching the front 
end.

Regards

https://github.com/llvm/llvm-project/pull/80098
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] `llvm-cov` assertion failure when handling MC/DC that involves macros (PR #80098)

2024-01-30 Thread Wentao Zhang via cfe-commits 

whentojump wrote:

Hey @chapuni thanks for the comments!

This is indeed a draft. Since there're already reports regarding this issue, 
let me close this one. I will try those linked patches and report if I have 
findings.

Sorry for not searching for the ongoing effort and thanks for your work!


https://github.com/llvm/llvm-project/pull/80098
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] `llvm-cov` assertion failure when handling MC/DC that involves macros (PR #80098)

2024-01-30 Thread Wentao Zhang via cfe-commits 

https://github.com/whentojump closed 
https://github.com/llvm/llvm-project/pull/80098
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] `llvm-cov` assertion failure when handling MC/DC that involves macros (PR #80098)

2024-01-30 Thread Wentao Zhang via cfe-commits 

whentojump wrote:

Hi thanks for the prompt reply and the pointer! Will definitely check.

https://github.com/llvm/llvm-project/pull/80098
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] `llvm-cov` assertion failure when handling MC/DC that involves macros (PR #80098)

2024-01-30 Thread Wentao Zhang via cfe-commits 

https://github.com/whentojump created 
https://github.com/llvm/llvm-project/pull/80098

## Problem

The behavior is described with this tiny example: 
https://github.com/whentojump/llvm-mcdc-assertion-failure

Essentially, current MC/DC implementation cannot properly handle decisions that 
involve macros, like this example:

![Picture1](https://github.com/llvm/llvm-project/assets/35722712/e1750238-8523-4871-aed1-8439c873ec55)

## Root cause

Some terms I'll use:

1. Decision region: the composite logical expression
2. Branch region: smaller conditions or branches within a larger expression

In the full lifecycle of these regions, here are several important stages:

1. Compilation
1. Generated at front end: [source 
code](https://github.com/llvm/llvm-project/blob/a0b6747804e46665ecfd00295b60432bfe1775b6/clang/lib/CodeGen/CoverageMappingGen.cpp#L860-L903)
2. Written to the binary as coverage mapping sections: [source 
code](https://github.com/llvm/llvm-project/blob/a0b6747804e46665ecfd00295b60432bfe1775b6/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp#L240-L256)
2. Generate report
1. Read from the binary: [source 
code](https://github.com/llvm/llvm-project/blob/a0b6747804e46665ecfd00295b60432bfe1775b6/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp#L650-L703)

The assertion in question is in step 2.i and goes like this: 

1. `llvm-cov` walks all regions of the target program
2. It encounters a decision region, say **R3**. By reading **R3**'s metadata, 
it knows it has two branches
3. `llvm-cov` then asserts: the next two regions it's gonna see must be two 
branch regions, not otherwise

This assertion assumes the order of MCDC regions. However, this order doesn't 
always hold

In step 1.ii all regions are 
[sorted](https://github.com/llvm/llvm-project/blob/a0b6747804e46665ecfd00295b60432bfe1775b6/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp#L162-L171)
 first by their file IDs, then by locations within the file and finally by 
region types. Macros, however, are traced back to definitions, which can be far 
away from their invocations or even separated in different files. As a result, 
the MC/DC regions could be sorted in a way where they are separated from each 
other. In the shown example, the order could be: `R3 (many irrelevant regions) 
R1 R2` which apparently breaks the assertion at step 2.i.

## Solution

This can be solved in two ways

1. (This PR) Sort with MC/DC in consideration and group relevant decisions and 
branches together.

I honestly don't know if this's gonna break other things. But at least I 
can confirm it solves the problem mentioned. Again please see an example in 
this repo https://github.com/whentojump/llvm-mcdc-assertion-failure

2. In `CoverageMapping::loadFunctionRecord()`, use a cleverer way to correlate 
decisions and branches that are sorted far away from each other.

## Other to-do's

- Tests are not yet taken care of 
- A bit comments/docs

Last but not least, a huge thanks to @evodius96 et al for the great work 
regarding MC/DC :))


>From daf74be7f20d20bcc54a541d50fc891bff8ae388 Mon Sep 17 00:00:00 2001
From: Wentao Zhang 
Date: Tue, 30 Jan 2024 21:32:26 -0500
Subject: [PATCH 1/2] [Coverage][llvm-cov] Fix failures to handle MC/DC
 decisions involving macros (1/2)

This patch lets MC/DC code regions for the same composite logical
expression get sorted next to each other, so that `llvm-cov show` won't
hit assertion failures.

The problematic cases are mostly when macros are involved:

  (FOO(x) && BAR(y))

   <-R2-><-R3->
  <---R1--->

Let's call the full expression Region 1, `FOO(x)` Region 2 and `BAR(y)`
Region 3. If these macros are defined far away from their invocations,
the eventual order of all regions would look like: Region 1, (many other
irrelevant regions), Region 2, Region 3. This will break an assertion in
CoverageMapping::loadFunctionRecord() which assumes branch regions of an
MC/DC decision would immediately follow the decision region itself.

This patch adds a new field `GroupID` to `MCDCParameters` struct, sort
all regions based on this information, and place related MC/DC regions
together. The `GroupID` assignment is included in the other patch to
Clang CodeGen. This patch also disables 3 assertions that's based on the
old sorting criteria.
---
 .../llvm/ProfileData/Coverage/CoverageMapping.h|  1 +
 .../lib/ProfileData/Coverage/CoverageMappingReader.cpp |  8 ++--
 .../lib/ProfileData/Coverage/CoverageMappingWriter.cpp | 10 +++---
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h 
b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index 33fa17ba811fa..5ca5923cbd82e 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -260,6 +260,7 @@ struct CounterMappingRegion {
 /// IDs used to represent a branch region and other branch