[LLVM-COV] Fix an issue: a statement after calling 'assert()' function is wrongly marked as 'not executed'

[MaggieYi]

Summary

In the current coverage mapping implementation, we terminate the current region and start a zero region when we hit a nonreturn function. However, for logical OR, the second operand is not executed if the first operand evaluates to true. If the nonreturn function is called in the right side of logical OR and the left side of logical OR is TRUE, we should not start a zero GapRegionCounter. This will also apply to VisitAbstractConditionalOperator.

The following issues are fixed by this patch:

1. https://github.com/llvm/llvm-project/issues/59030
2. https://github.com/llvm/llvm-project/issues/57388
3. https://github.com/llvm/llvm-project/issues/57481
4. https://github.com/llvm/llvm-project/issues/60701

[MaggieYi]

Gentle ping ...

[zequanwu]

Thanks for fixing it.

Just one comment.

clang/test/CoverageMapping/terminate-statements.cpp

335

For completeness, can you add following two tests:

( true ? abort() : void (0) );
( false ? abort() : void (0) );

[MaggieYi]

Thanks Zequan. I have added two tests following your suggestion.

Kind regards,
Maggie

[MaggieYi]

clang/test/CoverageMapping/terminate-statements.cpp

335

Thanks, two tests have been added.

[zequanwu]

LGTM.

clang/lib/CodeGen/CoverageMappingGen.cpp

struct CounterCoverageMappingBuilder
 
   void VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
     extendRegion(E);
 
     Counter ParentCount = getRegion().getCounter();
     Counter TrueCount = getRegionCounter(E);
 
     propagateCounts(ParentCount, E->getCond());
+    Counter OutCount;
 
     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);
 
       extendRegion(E->getTrueExpr());
-      propagateCounts(TrueCount, E->getTrueExpr());
+      OutCount = propagateCounts(TrueCount, E->getTrueExpr());
     }
 
     extendRegion(E->getFalseExpr());
-    propagateCounts(subtractCounters(ParentCount, TrueCount),
-                    E->getFalseExpr());
+    OutCount = addCounters(
+        OutCount, propagateCounts(subtractCounters(ParentCount, TrueCount),
+                                  E->getFalseExpr()));
+
+    if (OutCount != ParentCount) {
+      pushRegion(OutCount);
+      GapRegionCounter = OutCount;
+    }
 
     // Create Branch Region around condition.
     createBranchRegion(E->getCond(), TrueCount,
                        subtractCounters(ParentCount, TrueCount));
   }
 
   void VisitBinLAnd(const BinaryOperator *E) {
     extendRegion(E->getLHS());
void VisitBinLAnd(const BinaryOperator *E) {
     createBranchRegion(E->getLHS(), RHSExecCnt,
                        subtractCounters(ParentCnt, RHSExecCnt));
 
     // Create Branch Region around RHS condition.
     createBranchRegion(E->getRHS(), RHSTrueCnt,
                        subtractCounters(RHSExecCnt, RHSTrueCnt));
   }
 
+  // Determine whether the right side of OR operation need to be visited.
+  bool shouldVisitRHS(const Expr *LHS) {
+    bool LHSIsTrue = false;
+    bool LHSIsConst = false;
+    if (!LHS->isValueDependent())
+      LHSIsConst = LHS->EvaluateAsBooleanCondition(
+          LHSIsTrue, CVM.getCodeGenModule().getContext());
+    return !LHSIsConst || (LHSIsConst && !LHSIsTrue);
+  }
+
   void VisitBinLOr(const BinaryOperator *E) {
     extendRegion(E->getLHS());
-    propagateCounts(getRegion().getCounter(), E->getLHS());
+    Counter OutCount = propagateCounts(getRegion().getCounter(), E->getLHS());
     handleFileExit(getEnd(E->getLHS()));
 
     // Counter tracks the right hand side of a logical or operator.
     extendRegion(E->getRHS());
     propagateCounts(getRegionCounter(E), E->getRHS());
 
     // Extract the RHS's Execution Counter.
     Counter RHSExecCnt = getRegionCounter(E);
 
     // Extract the RHS's "False" Instance Counter.
     Counter RHSFalseCnt = getRegionCounter(E->getRHS());
 
+    if (!shouldVisitRHS(E->getLHS())) {
+      GapRegionCounter = OutCount;
+    }
+
     // Extract the Parent Region Counter.
     Counter ParentCnt = getRegion().getCounter();
 
     // Create Branch Region around LHS condition.
     createBranchRegion(E->getLHS(), subtractCounters(ParentCnt, RHSExecCnt),
                        RHSExecCnt);
 
     // Create Branch Region around RHS condition.

clang/test/CoverageMapping/terminate-statements.cpp

 // CHECK:  Gap,File 0, 2:13 -> 3:5 = #1
 // CHECK:  Gap,File 0, 3:12 -> 4:3 = (#0 - #1)
 
 // CHECK-LABEL: _Z7includev:
 void include() {
   included_func();
 }
 
+// CHECK-LABEL: _Z7ornoretv:
+void abort() __attribute__((noreturn));
+
+int ornoret(void) {
+  ( true || (abort(), 0) );  // CHECK: Gap,File 0, [[@LINE]]:28 -> [[@LINE+1]]:3 = #0
+  ( false || (abort(), 0) ); // CHECK: Gap,File 0, [[@LINE]]:29 -> [[@LINE+1]]:3 = 0
+  return 0;
+}
+
+// CHECK-LABEL: _Z17abstractcondnoretv:
+int abstractcondnoret(void) {
+  ( true ? void (0) : abort() );  // CHECK: Gap,File 0, [[@LINE]]:33 -> [[@LINE+1]]:3 = #1
+  ( false ? void (0) : abort() ); // CHECK: Gap,File 0, [[@LINE]]:34 -> [[@LINE+1]]:3 = #2
+  ( true ? abort() : void (0) );  // CHECK: Gap,File 0, [[@LINE]]:33 -> [[@LINE+1]]:3 = (#2 - #3)
+  ( false ? abort() : void (0) ); // CHECK: Gap,File 0, [[@LINE]]:34 -> [[@LINE+1]]:3 = ((#2 - #3) - #4)
+  return 0;
+}
+
+// CHECK-LABEL: _Z13elsecondnoretv:
+int elsecondnoret(void) {
+  if (true) {} else {
+    true ? void (0) : abort();
+  } // CHECK: Gap,File 0, [[@LINE]]:4 -> [[@LINE+1]]:3 = (#1 + #2)
+  return 0;
+}
+
 int main() {
   foo(0);
   foo(1);
   fooo(0);
   fooo(1);
   maaz();
   maaaz();
   baz();
   bar(0);
   bar(1);
   quux(0);
   quux(1);
   quux(2);
   quux(3);
   weird_if();
   for_loop();
   while_loop();
   gotos();
   include();
+  ornoret();
+  abstractcondnoret();
+  elsecondnoret();
   return 0;
 }