[PATCH] D95918: [Coverage] Propogate counter to condition of conditional operator

2021-02-03 Thread Zequan Wu via Phabricator via cfe-commits 
zequanwu added a comment.

In D95918#2540367 , @vsk wrote:

> How was the issue spotted? If there was a crash or an incorrect coverage bug 
> that's not triggered by one of the existing frontend tests, it'd be worth 
> adding a regression test. If the problem can't be replicated without 
> involving llvm-cov, this can be an integration test.

The issue was spotted by @pirama. But this doesn't completely solve the 
problem. When I was investigating the issue, I noticed condition expression of 
conditional operator was not given a counter. So, I sent this patch.

For the issue reported by @pirama, it is that llvm-cov shows following:

  1|   |#include 
  2|   |
  3|  1|int main() {
  4|  1|  return getenv(
  5|  0|  "TEST") ? 1
  6|  1|  : 0;
  7|  1|}

Here are the debug dump.

  Combined regions:
3:12 -> 7:2 (count=1)
4:10 -> 5:14 (count=1)
5:15 -> 5:17 (count=0)
5:17 -> 5:18 (count=0)
6:17 -> 6:18 (count=1)
  Segment at 3:12 (count = 1), RegionEntry
  Segment at 4:10 (count = 1), RegionEntry
  Segment at 5:14 (count = 1)
  Segment at 5:15 (count = 0), Gap
  Segment at 5:17 (count = 0), RegionEntry
  Segment at 5:18 (count = 1)
  Segment at 6:17 (count = 1), RegionEntry
  Segment at 6:18 (count = 1)
  Segment at 7:2 (count = 0), Skipped

I am working on this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95918/new/

https://reviews.llvm.org/D95918

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

[PATCH] D95918: [Coverage] Propogate counter to condition of conditional operator

2021-02-03 Thread Vedant Kumar via Phabricator via cfe-commits 
vsk added a comment.

How was the issue spotted? If there was a crash or an incorrect coverage bug 
that's not triggered by one of the existing frontend tests, it'd be worth 
adding a regression test. If the problem can't be replicated without involving 
llvm-cov, this can be an integration test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95918/new/

https://reviews.llvm.org/D95918

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

[PATCH] D95918: [Coverage] Propogate counter to condition of conditional operator

2021-02-03 Thread Zequan Wu via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4dc08cc3aa41: [Coverage] Propogate counter to condition of 
conditional operator (authored by zequanwu).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95918/new/

https://reviews.llvm.org/D95918

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/if.cpp
  clang/test/CoverageMapping/macro-expressions.cpp


Index: clang/test/CoverageMapping/macro-expressions.cpp
===
--- clang/test/CoverageMapping/macro-expressions.cpp
+++ clang/test/CoverageMapping/macro-expressions.cpp
@@ -83,20 +83,23 @@
   // CHECK: File 0, [[@LINE+1]]:42 -> [[@LINE+1]]:44 = #7
   for (DECL(int, j) : ARR(int, 1, 2, 3)) {}
 
+  // CHECK-NEXT: File 0, [[@LINE+4]]:10 -> [[@LINE+4]]:11 = #0
   // CHECK-NEXT: Branch,File 0, [[@LINE+3]]:10 -> [[@LINE+3]]:11 = #8, (#0 - 
#8)
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:14 -> [[@LINE+2]]:20 = #0
   // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:23 -> [[@LINE+1]]:29 = #0
   (void)(i ? PRIo64 : PRIu64);
 
+  // CHECK-NEXT: File 0, [[@LINE+6]]:10 -> [[@LINE+6]]:11 = #0
   // CHECK: File 0, [[@LINE+5]]:14 -> [[@LINE+5]]:15 = #9
-  // CHECK-NEXT: Expansion,File 0, [[@LINE+4]]:18 -> [[@LINE+4]]:22 = (#0 - #9)
-  // CHECK-NEXT: File 0, [[@LINE+3]]:22 -> [[@LINE+3]]:33 = (#0 - #9)
+  // CHECK-NEXT: File 0, [[@LINE+4]]:18 -> [[@LINE+4]]:33 = (#0 - #9)
+  // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:18 -> [[@LINE+3]]:22 = (#0 - #9)
   // CHECK: File 0, [[@LINE+2]]:28 -> [[@LINE+2]]:29 = #10
   // CHECK-NEXT: File 0, [[@LINE+1]]:32 -> [[@LINE+1]]:33 = ((#0 - #9) - #10)
   (void)(i ? i : EXPR(i) ? i : 0);
+  // CHECK-NEXT: File 0, [[@LINE+5]]:10 -> [[@LINE+5]]:11 = #0
   // CHECK-NEXT: Branch,File 0, [[@LINE+4]]:10 -> [[@LINE+4]]:11 = #11, (#0 - 
#11)
-  // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:15 -> [[@LINE+3]]:19 = (#0 - 
#11)
-  // CHECK-NEXT: File 0, [[@LINE+2]]:19 -> [[@LINE+2]]:27 = (#0 - #11)
+  // CHECK-NEXT: File 0, [[@LINE+3]]:15 -> [[@LINE+3]]:27 = (#0 - #11)
+  // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:15 -> [[@LINE+2]]:19 = (#0 - 
#11)
   // CHECK-NEXT: File 0, [[@LINE+1]]:26 -> [[@LINE+1]]:27 = ((#0 - #11) - #12)
   (void)(i ?: EXPR(i) ?: 0);
 }
Index: clang/test/CoverageMapping/if.cpp
===
--- clang/test/CoverageMapping/if.cpp
+++ clang/test/CoverageMapping/if.cpp
@@ -3,7 +3,8 @@
 int nop() { return 0; }
 
 // CHECK-LABEL: _Z3foov:
-// CHECK-NEXT: [[@LINE+2]]:12 -> [[@LINE+7]]:2 
= #0
+// CHECK-NEXT: [[@LINE+3]]:12 -> [[@LINE+8]]:2 
= #0
+// CHECK-NEXT: [[@LINE+3]]:15 -> 
[[@LINE+3]]:19 = #0
 // CHECK-NEXT: Branch,File 0, [[@LINE+2]]:15 
-> [[@LINE+2]]:19 = 0, 0
 void foo() {// CHECK-NEXT: Gap,File 0, [[@LINE+1]]:20 -> 
[[@LINE+1]]:22 = #2
   if (int j = true ? nop()  // CHECK-NEXT: [[@LINE]]:22 -> [[@LINE]]:27 = 
#2
@@ -38,11 +39,13 @@
 i = 3;  // CHECK-NEXT: File 0, [[@LINE-1]]:10 -> 
[[@LINE+1]]:4 = (#0 - #4)
   }
 
+// CHECK-NEXT: File 0, [[@LINE+2]]:7 -> 
[[@LINE+2]]:13 = #0
 // CHECK-NEXT: Branch,File 0, [[@LINE+1]]:7 -> 
[[@LINE+1]]:13 = #5, (#0 - #5)
   i = i == 0?   // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> 
[[@LINE+1]]:9 = #5
 i + 1 : // CHECK-NEXT: File 0, [[@LINE]]:9 -> 
[[@LINE]]:14 = #5
 i + 2;  // CHECK-NEXT: File 0, [[@LINE]]:9 -> 
[[@LINE]]:14 = (#0 - #5)
 
+// CHECK-NEXT: File 0, [[@LINE+4]]:7 -> 
[[@LINE+4]]:13 = #0
 // CHECK-NEXT: Branch,File 0, [[@LINE+3]]:7 -> 
[[@LINE+3]]:13 = #6, (#0 - #6)
 // CHECK-NEXT: Gap,File 0, [[@LINE+2]]:13 -> 
[[@LINE+2]]:14 = #6
 // CHECK-NEXT: File 0, [[@LINE+1]]:14 -> 
[[@LINE+1]]:20 = #6
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1462,7 +1462,7 @@
 Counter ParentCount = getRegion().getCounter();
 Counter TrueCount = getRegionCounter(E);
 
-Visit(E->getCond());
+propagateCounts(ParentCount, E->getCond());
 
 if (!isa(E)) {
   // The 'then' count applies to the area immediately after the condition.


Index: clang/test/CoverageMapping/macro-expressions.cpp
===
--- clang/test/CoverageMapping/macro-expressions.cpp
+++ clang/test/CoverageMapping/macro-expressions.cpp
@@ -83,20 +83,23 @@
   // CHECK: File 0,

[PATCH] D95918: [Coverage] Propogate counter to condition of conditional operator

2021-02-03 Thread Zequan Wu via Phabricator via cfe-commits 
zequanwu added a comment.

I tested it locally. They are all passed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95918/new/

https://reviews.llvm.org/D95918

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

[PATCH] D95918: [Coverage] Propogate counter to condition of conditional operator

2021-02-03 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk added a comment.

Nice. I believe there are some integration tests in compiler-rt/test/profile. 
First, please run them and make sure they pass. Second, @vsk, do you think it's 
worth adding an integration test for this, seeing as the bug is really only 
observable when you put all the coverage tools together?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95918/new/

https://reviews.llvm.org/D95918

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

[PATCH] D95918: [Coverage] Propogate counter to condition of conditional operator

2021-02-03 Thread Vedant Kumar via Phabricator via cfe-commits 
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

Thanks, lgtm.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95918/new/

https://reviews.llvm.org/D95918

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

[PATCH] D95918: [Coverage] Propogate counter to condition of conditional operator

2021-02-02 Thread Zequan Wu via Phabricator via cfe-commits 
zequanwu created this revision.
zequanwu added reviewers: vsk, rnk.
zequanwu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Clang usually propagates counter mapping region for conditions of `if`, `while`,
`for`, etc from parent counter. We should do the same for condition of 
conditional operator.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95918

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/if.cpp
  clang/test/CoverageMapping/macro-expressions.cpp


Index: clang/test/CoverageMapping/macro-expressions.cpp
===
--- clang/test/CoverageMapping/macro-expressions.cpp
+++ clang/test/CoverageMapping/macro-expressions.cpp
@@ -83,20 +83,23 @@
   // CHECK: File 0, [[@LINE+1]]:42 -> [[@LINE+1]]:44 = #7
   for (DECL(int, j) : ARR(int, 1, 2, 3)) {}
 
+  // CHECK-NEXT: File 0, [[@LINE+4]]:10 -> [[@LINE+4]]:11 = #0
   // CHECK-NEXT: Branch,File 0, [[@LINE+3]]:10 -> [[@LINE+3]]:11 = #8, (#0 - 
#8)
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:14 -> [[@LINE+2]]:20 = #0
   // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:23 -> [[@LINE+1]]:29 = #0
   (void)(i ? PRIo64 : PRIu64);
 
+  // CHECK-NEXT: File 0, [[@LINE+6]]:10 -> [[@LINE+6]]:11 = #0
   // CHECK: File 0, [[@LINE+5]]:14 -> [[@LINE+5]]:15 = #9
-  // CHECK-NEXT: Expansion,File 0, [[@LINE+4]]:18 -> [[@LINE+4]]:22 = (#0 - #9)
-  // CHECK-NEXT: File 0, [[@LINE+3]]:22 -> [[@LINE+3]]:33 = (#0 - #9)
+  // CHECK-NEXT: File 0, [[@LINE+4]]:18 -> [[@LINE+4]]:33 = (#0 - #9)
+  // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:18 -> [[@LINE+3]]:22 = (#0 - #9)
   // CHECK: File 0, [[@LINE+2]]:28 -> [[@LINE+2]]:29 = #10
   // CHECK-NEXT: File 0, [[@LINE+1]]:32 -> [[@LINE+1]]:33 = ((#0 - #9) - #10)
   (void)(i ? i : EXPR(i) ? i : 0);
+  // CHECK-NEXT: File 0, [[@LINE+5]]:10 -> [[@LINE+5]]:11 = #0
   // CHECK-NEXT: Branch,File 0, [[@LINE+4]]:10 -> [[@LINE+4]]:11 = #11, (#0 - 
#11)
-  // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:15 -> [[@LINE+3]]:19 = (#0 - 
#11)
-  // CHECK-NEXT: File 0, [[@LINE+2]]:19 -> [[@LINE+2]]:27 = (#0 - #11)
+  // CHECK-NEXT: File 0, [[@LINE+3]]:15 -> [[@LINE+3]]:27 = (#0 - #11)
+  // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:15 -> [[@LINE+2]]:19 = (#0 - 
#11)
   // CHECK-NEXT: File 0, [[@LINE+1]]:26 -> [[@LINE+1]]:27 = ((#0 - #11) - #12)
   (void)(i ?: EXPR(i) ?: 0);
 }
Index: clang/test/CoverageMapping/if.cpp
===
--- clang/test/CoverageMapping/if.cpp
+++ clang/test/CoverageMapping/if.cpp
@@ -3,7 +3,8 @@
 int nop() { return 0; }
 
 // CHECK-LABEL: _Z3foov:
-// CHECK-NEXT: [[@LINE+2]]:12 -> [[@LINE+7]]:2 
= #0
+// CHECK-NEXT: [[@LINE+3]]:12 -> [[@LINE+8]]:2 
= #0
+// CHECK-NEXT: [[@LINE+3]]:15 -> 
[[@LINE+3]]:19 = #0
 // CHECK-NEXT: Branch,File 0, [[@LINE+2]]:15 
-> [[@LINE+2]]:19 = 0, 0
 void foo() {// CHECK-NEXT: Gap,File 0, [[@LINE+1]]:20 -> 
[[@LINE+1]]:22 = #2
   if (int j = true ? nop()  // CHECK-NEXT: [[@LINE]]:22 -> [[@LINE]]:27 = 
#2
@@ -38,11 +39,13 @@
 i = 3;  // CHECK-NEXT: File 0, [[@LINE-1]]:10 -> 
[[@LINE+1]]:4 = (#0 - #4)
   }
 
+// CHECK-NEXT: File 0, [[@LINE+2]]:7 -> 
[[@LINE+2]]:13 = #0
 // CHECK-NEXT: Branch,File 0, [[@LINE+1]]:7 -> 
[[@LINE+1]]:13 = #5, (#0 - #5)
   i = i == 0?   // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> 
[[@LINE+1]]:9 = #5
 i + 1 : // CHECK-NEXT: File 0, [[@LINE]]:9 -> 
[[@LINE]]:14 = #5
 i + 2;  // CHECK-NEXT: File 0, [[@LINE]]:9 -> 
[[@LINE]]:14 = (#0 - #5)
 
+// CHECK-NEXT: File 0, [[@LINE+4]]:7 -> 
[[@LINE+4]]:13 = #0
 // CHECK-NEXT: Branch,File 0, [[@LINE+3]]:7 -> 
[[@LINE+3]]:13 = #6, (#0 - #6)
 // CHECK-NEXT: Gap,File 0, [[@LINE+2]]:13 -> 
[[@LINE+2]]:14 = #6
 // CHECK-NEXT: File 0, [[@LINE+1]]:14 -> 
[[@LINE+1]]:20 = #6
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1462,7 +1462,7 @@
 Counter ParentCount = getRegion().getCounter();
 Counter TrueCount = getRegionCounter(E);
 
-Visit(E->getCond());
+propagateCounts(ParentCount, E->getCond());
 
 if (!isa(E)) {
   // The 'then' count applies to the area immediately after the condition.


Index: clang/test/CoverageMapping/macro-expressions.cpp
===
--- clang/test/CoverageMapping/macro-expressions.cpp
+++ clang/test/CoverageMapping/macro-expressions.cpp
@@ -83,20