llvm/llvm-projectPublic

NotificationsYou must be signed in to change notification settings
Fork14.5k
Star33.6k

[Coverage] Add gap region between binary operator '&& and ||' and RHS#149085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

int-zjt wants to merge3 commits intollvm:main

base:main

Choose a base branch

fromint-zjt:add_gap_region_after_and_or

Open

[Coverage] Add gap region between binary operator '&& and ||' and RHS#149085

int-zjt wants to merge3 commits intollvm:mainfromint-zjt:add_gap_region_after_and_or

+59 −9

Conversation

Copy link

Contributor

int-zjt commentedJul 16, 2025

Issue Summary

We identified an inaccuracy in line coverage reporting when short-circuit evaluation occurs in multi-line conditional expressions. Specifically:

Un-executed conditions following line breaks may be incorrectly marked as covered
(e.g., conditionB in a non-executed && chain shows coverage)

    1|       |#include <iostream>    2|       |    3|      1|int main() {    4|      1|    bool conditionA = false;    5|      1|    bool conditionB = true;    6|      1|    if (conditionA &&    7|      1|        conditionB) {    8|      0|        std::cout << "IF-THEN" << std::endl;    9|      0|    }   10|      1|    return 0;   11|      1|}

Inconsistent coverage reporting across un-executed conditions
(adjacent un-executed conditions may show 1 vs 0 line coverage)

    1|       |#include <iostream>    2|       |    3|      1|int main() {    4|      1|    bool conditionA = false;    5|      1|    bool conditionB = true;    6|      1|    bool conditionC = true;    7|      1|    if (conditionA &&    8|      1|        (conditionB ||    9|      0|        conditionC)) {   10|      0|        std::cout << "IF-THEN" << std::endl;   11|      0|    }   12|      1|    return 0;   13|      1|}

Root Cause Analysis

The currentWrappedSegment mechanism may propagates coverage data from the last segment of the LHS line to subsequent RHS lines. Because the LHS line's coverage data depends on count of its segments andWrappedSegment's count, it may causes false positive coverage for un-executed RHS conditions.

Proposed Solution

The current implementation correctly handles single-statement if bodies through GapRegion insertion:

if (cond)       // Line 1    statement;  // Line 2

Processing workflow:

A GapRegion is created spanning from the end of the condition (Line 1) to the start of the body statement (Line 2)
This GapRegion inherits the execution counter from Line 2
During coverage calculation:

Line 2 uses this GapRegion as its WrappedSegment
Prevents propagation of Line 1's coverage data to Line 2

Generalize the GapRegion insertion strategy from if-statements to logical operators, placing RHS-synchronized segments at LHS-line-end to isolate coverage contexts during short-circuit evaluation.

[llvm-cov] Add gap region after binary operator && and ||

763293b

llvmbot added clang

Clang issues not falling into any other category

compiler-rt clang:codegen

IR generation bugs: mangling, exceptions, etc.

PGOProfile Guided Optimizations labels

Jul 16, 2025

Copy link

Member

llvmbot commentedJul 16, 2025•
edited
Loading

@llvm/pr-subscribers-pgo
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: None (int-zjt)

Changes

Issue Summary

We identified an inaccuracy in line coverage reporting when short-circuit evaluation occurs in multi-line conditional expressions. Specifically:

Un-executed conditions following line breaks may be incorrectly marked as covered
(e.g., conditionB in a non-executed && chain shows coverage)

    1|       |#include &lt;iostream&gt;    2|       |    3|      1|int main() {    4|      1|    bool conditionA = false;    5|      1|    bool conditionB = true;    6|      1|    if (conditionA &amp;&amp;    7|      1|        conditionB) {    8|      0|        std::cout &lt;&lt; "IF-THEN" &lt;&lt; std::endl;    9|      0|    }   10|      1|    return 0;   11|      1|}

Inconsistent coverage reporting across un-executed conditions
(adjacent un-executed conditions may show 1 vs 0 line coverage)

    1|       |#include &lt;iostream&gt;    2|       |    3|      1|int main() {    4|      1|    bool conditionA = false;    5|      1|    bool conditionB = true;    6|      1|    bool conditionC = true;    7|      1|    if (conditionA &amp;&amp;    8|      1|        (conditionB ||    9|      0|        conditionC)) {   10|      0|        std::cout &lt;&lt; "IF-THEN" &lt;&lt; std::endl;   11|      0|    }   12|      1|    return 0;   13|      1|}

Root Cause Analysis

Proposed Solution

The current implementation correctly handles single-statement if bodies through GapRegion insertion:

if (cond)       // Line 1    statement;  // Line 2

Processing workflow:

A GapRegion is created spanning from the end of the condition (Line 1) to the start of the body statement (Line 2)
This GapRegion inherits the execution counter from Line 2
During coverage calculation:

Line 2 uses this GapRegion as its WrappedSegment
Prevents propagation of Line 1's coverage data to Line 2

Generalize the GapRegion insertion strategy from if-statements to logical operators, placing RHS-synchronized segments at LHS-line-end to isolate coverage contexts during short-circuit evaluation.

Full diff:https://github.com/llvm/llvm-project/pull/149085.diff

2 Files Affected:

(modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+10)
(added) compiler-rt/test/profile/Linux/coverage_short_circuit.cpp (+36)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cppindex 4aafac349e3e9..f55290a5feee6 100644--- a/clang/lib/CodeGen/CoverageMappingGen.cpp+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp@@ -2269,6 +2269,11 @@ struct CounterCoverageMappingBuilder     // Track LHS True/False Decision.     const auto DecisionLHS = MCDCBuilder.pop();+    if (auto Gap =+            findGapAreaBetween(getEnd(E->getLHS()), getStart(E->getRHS()))) {+      fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), getRegionCounter(E));+    }+     // Counter tracks the right hand side of a logical and operator.     extendRegion(E->getRHS());     propagateCounts(getRegionCounter(E), E->getRHS());@@ -2330,6 +2335,11 @@ struct CounterCoverageMappingBuilder     // Track LHS True/False Decision.     const auto DecisionLHS = MCDCBuilder.pop();+    if (auto Gap =+            findGapAreaBetween(getEnd(E->getLHS()), getStart(E->getRHS()))) {+      fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), getRegionCounter(E));+    }+     // Counter tracks the right hand side of a logical or operator.     extendRegion(E->getRHS());     propagateCounts(getRegionCounter(E), E->getRHS());diff --git a/compiler-rt/test/profile/Linux/coverage_short_circuit.cpp b/compiler-rt/test/profile/Linux/coverage_short_circuit.cppnew file mode 100644index 0000000000000..cc4022bc3c286--- /dev/null+++ b/compiler-rt/test/profile/Linux/coverage_short_circuit.cpp@@ -0,0 +1,36 @@+// RUN: %clangxx_profgen -std=c++17 -fuse-ld=lld -fcoverage-mapping -o %t %s+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t+// RUN: llvm-profdata merge -o %t.profdata %t.profraw+// RUN: llvm-cov show %t -instr-profile=%t.profdata 2>&1 | FileCheck %s++void foo() {          // CHECK:       [[@LINE]]| 1|void foo() {+  bool cond1 = false; // CHECK-NEXT:  [[@LINE]]| 1|  bool cond1 = false;+  bool cond2 = true;  // CHECK-NEXT:  [[@LINE]]| 1|  bool cond2 = true;+  if (cond1 &&        // CHECK-NEXT:  [[@LINE]]| 1|  if (cond1 &&+      cond2) {        // CHECK-NEXT:  [[@LINE]]| 0|      cond2) {+  }                   // CHECK-NEXT:  [[@LINE]]| 0|  }+} // CHECK-NEXT:  [[@LINE]]| 1|}++void bar() {          // CHECK:       [[@LINE]]| 1|void bar() {+  bool cond1 = true;  // CHECK-NEXT:  [[@LINE]]| 1|  bool cond1 = true;+  bool cond2 = false; // CHECK-NEXT:  [[@LINE]]| 1|  bool cond2 = false;+  if (cond1 &&        // CHECK-NEXT:  [[@LINE]]| 1|  if (cond1 &&+      cond2) {        // CHECK-NEXT:  [[@LINE]]| 1|      cond2) {+  }                   // CHECK-NEXT:  [[@LINE]]| 0|  }+} // CHECK-NEXT:  [[@LINE]]| 1|}++void baz() {          // CHECK:       [[@LINE]]| 1|void baz() {+  bool cond1 = false; // CHECK-NEXT:  [[@LINE]]| 1|  bool cond1 = false;+  bool cond2 = true;  // CHECK-NEXT:  [[@LINE]]| 1|  bool cond2 = true;+  if (cond1           // CHECK-NEXT:  [[@LINE]]| 1|  if (cond1+      &&              // CHECK-NEXT:  [[@LINE]]| 0|      &&+      cond2) {        // CHECK-NEXT:  [[@LINE]]| 0|      cond2) {+  }                   // CHECK-NEXT:  [[@LINE]]| 0|  }+} // CHECK-NEXT:  [[@LINE]]| 1|}++int main() {+  foo();+  bar();+  baz();+  return 0;+}

Copy link

github-actionsbot commentedJul 16, 2025•
edited
Loading

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link

Contributor

evodius96 commentedJul 16, 2025

Thank you! I assume this is only a problem with line coverage? In other words, region coverage is still correct when visualized.

Copy link

ContributorAuthor

int-zjt commentedJul 17, 2025

Thank you! I assume this is only a problem with line coverage? In other words, region coverage is still correct when visualized.

Yeah, region coverage is still correct, just line coverage is wrong.

Fix format

5482444

Copy link

Contributor

Enna1 commentedJul 17, 2025

Clang :: CoverageMapping/logical.cpp test failed

Fix coverage mapping logical test

f7929d7

Enna1 requested review fromchapuni,MaskRay,ZequanWu andevodius96

July 17, 2025 08:24

int-zjt changed the title~~[llvm-cov] Add gap region between binary operator '&& and ||' and RHS~~[Coverage] Add gap region between binary operator '&& and ||' and RHS

Jul 17, 2025