[Coverage] Propogate counter to condition of conditional operator

[zequanwu]

Summary

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.

[vsk]

Thanks, lgtm.

[rnk]

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?

[zequanwu]

I tested it locally. They are all passed.

[vsk]

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.

[zequanwu]

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 <stdlib.h>
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.

clang/lib/CodeGen/CoverageMappingGen.cpp

struct CounterCoverageMappingBuilder
   }
 
   void VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
     extendRegion(E);
 
     Counter ParentCount = getRegion().getCounter();
     Counter TrueCount = getRegionCounter(E);
 
-    Visit(E->getCond());
+    propagateCounts(ParentCount, E->getCond());
 
     if (!isa<BinaryConditionalOperator>(E)) {
       // The 'then' count applies to the area immediately after the condition.
       auto Gap =
           findGapAreaBetween(E->getQuestionLoc(), getStart(E->getTrueExpr()));
       if (Gap)
         fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), TrueCount);
 

clang/test/CoverageMapping/if.cpp

 // RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -std=c++1z -triple %itanium_abi_triple -main-file-name if.cpp %s | FileCheck %s
 
 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
                    : nop();     // CHECK-NEXT: [[@LINE]]:22 -> [[@LINE]]:27 = (#0 - #2)
       j)                        // CHECK-NEXT: [[@LINE]]:7 -> [[@LINE]]:8 = #0
     ++j;                        // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:8 = #1, (#0 - #1)
 }                               // CHECK-NEXT: [[@LINE-2]]:9 -> [[@LINE-1]]:5 = #1
                                 // CHECK-NEXT: [[@LINE-2]]:5 -> [[@LINE-2]]:8 = #1
i = 2;                      // CHECK-NEXT: File 0, [[@LINE-1]]:14 -> [[@LINE+1]]:4 = #3
                                 // CHECK-NEXT: File 0, [[@LINE+2]]:6 -> [[@LINE+2]]:12 = #0
                                 // CHECK-NEXT: Branch,File 0, [[@LINE+1]]:6 -> [[@LINE+1]]:12 = #4, (#0 - #4)
   if(i != 0) {                  // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE]]:14 = #4
     i = 1;                      // CHECK-NEXT: File 0, [[@LINE-1]]:14 -> [[@LINE+1]]:4 = #4
   } else {                      // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE]]:10 = (#0 - #4)
     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
   i = i == 0?i + 12:i + 10;     // CHECK-NEXT: File 0, [[@LINE]]:21 -> [[@LINE]]:27 = (#0 - #6)
 
   return 0;
 }
 
 #define FOO true
 
 // CHECK-LABEL: _Z7ternaryv:
 void ternary() {
   true ? FOO : FOO; // CHECK-NOT: Gap,{{.*}}, [[@LINE]]:8 ->
 }

clang/test/CoverageMapping/macro-expressions.cpp

void foo(int i) {
   // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:21 = (#0 + #6)
   do {} while (NEXPR(i));
   // CHECK-NEXT: Expansion,File 0, [[@LINE+4]]:8 -> [[@LINE+4]]:12 = #0
   // CHECK-NEXT: Branch,File 0, [[@LINE+3]]:21 -> [[@LINE+3]]:22 = #7, #0
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:23 -> [[@LINE+2]]:26 = #0
   // 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);
 }
 
 // CHECK-NEXT: File {{[0-9]+}}, 3:17 -> 3:20 = #0
 // CHECK-NEXT: Branch,File {{[0-9]+}}, 3:17 -> 3:20 = #2, (#0 - #2)
 // CHECK-NEXT: File {{[0-9]+}}, 4:18 -> 4:22 = (#0 + #3)
 // CHECK-NEXT: Branch,File {{[0-9]+}}, 4:18 -> 4:22 = #3, #0