[clang] 94dd476 - [Coverage] Fix an issue: a statement after calling 'assert()' function is wrongly

[Ying Yi via cfe-commits]

Author: Ying Yi
Date: 2023-03-02T09:14:44Z
New Revision: 94dd4766a61bb18b263415e17e745dc2fa609162

URL: https://github.com/llvm/llvm-project/commit/94dd4766a61bb18b263415e17e745dc2fa609162
DIFF: https://github.com/llvm/llvm-project/commit/94dd4766a61bb18b263415e17e745dc2fa609162.diff

LOG: [Coverage] Fix an issue: a statement after calling 'assert()' function is wrongly
marked as 'not executed'.

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`.

Fixes https://github.com/llvm/llvm-project/issues/59030

Reviewed By: zequanwu

Differential Revision: https://reviews.llvm.org/D144371

Added: 
    

Modified: 
    clang/lib/CodeGen/CoverageMappingGen.cpp
    clang/test/CoverageMapping/terminate-statements.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 56ff36438098..68e6457419ab 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1466,6 +1466,7 @@ struct CounterCoverageMappingBuilder
     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.
@@ -1475,12 +1476,18 @@ struct CounterCoverageMappingBuilder
         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,
@@ -1514,9 +1521,19 @@ struct CounterCoverageMappingBuilder
                        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.
@@ -1529,6 +1546,10 @@ struct CounterCoverageMappingBuilder
     // 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();
 

diff  --git a/clang/test/CoverageMapping/terminate-statements.cpp b/clang/test/CoverageMapping/terminate-statements.cpp
index fa309b8efea2..0067185fee8e 100644
--- a/clang/test/CoverageMapping/terminate-statements.cpp
+++ b/clang/test/CoverageMapping/terminate-statements.cpp
@@ -320,6 +320,32 @@ 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);
@@ -339,5 +365,8 @@ int main() {
   while_loop();
   gotos();
   include();
+  ornoret();
+  abstractcondnoret();
+  elsecondnoret();
   return 0;
 }