Skip to content

Commit

Permalink
[Clang] Fix crash in coverage of if consteval.
Browse files Browse the repository at this point in the history
Clang crashes when encountering an `if consteval` statement.
This is the minimum fix not to crash.
The fix is consistent with the current behavior of if constexpr,
which does generate coverage data for the discarded branches.
This is of course not correct and a better solution is
needed for both if constexpr and if consteval.
See #54419.

Fixes #57377

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D132723
  • Loading branch information
cor3ntin committed Aug 26, 2022
1 parent 98f6a56 commit 463e30f
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 14 deletions.
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ Bug Fixes
- ``-Wtautological-compare`` missed warnings for tautological comparisons
involving a negative integer literal. This fixes
`Issue 42918 <https://github.com/llvm/llvm-project/issues/42918>`_.
- Fix a crash when generating code coverage information for an
``if consteval`` statement. This fixes
`Issue 57377 <https://github.com/llvm/llvm-project/issues/57377>`_.


Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
32 changes: 19 additions & 13 deletions clang/lib/CodeGen/CoverageMappingGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1377,19 +1377,23 @@ struct CounterCoverageMappingBuilder

// Extend into the condition before we propagate through it below - this is
// needed to handle macros that generate the "if" but not the condition.
extendRegion(S->getCond());
if (!S->isConsteval())
extendRegion(S->getCond());

Counter ParentCount = getRegion().getCounter();
Counter ThenCount = getRegionCounter(S);

// Emitting a counter for the condition makes it easier to interpret the
// counter for the body when looking at the coverage.
propagateCounts(ParentCount, S->getCond());
if (!S->isConsteval()) {
// Emitting a counter for the condition makes it easier to interpret the
// counter for the body when looking at the coverage.
propagateCounts(ParentCount, S->getCond());

// The 'then' count applies to the area immediately after the condition.
auto Gap = findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
if (Gap)
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
// The 'then' count applies to the area immediately after the condition.
Optional<SourceRange> Gap =
findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
if (Gap)
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
}

extendRegion(S->getThen());
Counter OutCount = propagateCounts(ThenCount, S->getThen());
Expand All @@ -1398,9 +1402,9 @@ struct CounterCoverageMappingBuilder
if (const Stmt *Else = S->getElse()) {
bool ThenHasTerminateStmt = HasTerminateStmt;
HasTerminateStmt = false;

// The 'else' count applies to the area immediately after the 'then'.
Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
Optional<SourceRange> Gap =
findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
if (Gap)
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ElseCount);
extendRegion(Else);
Expand All @@ -1416,9 +1420,11 @@ struct CounterCoverageMappingBuilder
GapRegionCounter = OutCount;
}

// Create Branch Region around condition.
createBranchRegion(S->getCond(), ThenCount,
subtractCounters(ParentCount, ThenCount));
if (!S->isConsteval()) {
// Create Branch Region around condition.
createBranchRegion(S->getCond(), ThenCount,
subtractCounters(ParentCount, ThenCount));
}
}

void VisitCXXTryStmt(const CXXTryStmt *S) {
Expand Down
22 changes: 21 additions & 1 deletion clang/test/CoverageMapping/if.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// 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
// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -std=c++2b -triple %itanium_abi_triple -main-file-name if.cpp %s | FileCheck %s

int nop() { return 0; }

Expand All @@ -13,6 +13,22 @@ void foo() { // CHECK-NEXT: Gap,File 0, [[@LINE+1]]:21 -> [[@
++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

// FIXME: Do not generate coverage for discarded branches in if consteval and if constexpr statements
constexpr int check_consteval(int i) {
if consteval {
i++;
}
if !consteval {
i++;
}
if consteval {
return 42;
} else {
return i;
}
}

// CHECK-LABEL: main:
int main() { // CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 = #0
int i = 0;
Expand Down Expand Up @@ -50,6 +66,10 @@ int main() { // CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 =
// 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)

// GH-57377
constexpr int c_i = check_consteval(0);
check_consteval(i);

return 0;
}

Expand Down

0 comments on commit 463e30f

Please sign in to comment.