Skip to content

Commit 3a08ad2

Browse files
cor3ntintru
authored andcommitted
[Clang] Fix crash in coverage of if consteval.
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 llvm/llvm-project#54419. Fixes #57377 Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D132723
1 parent 1c73596 commit 3a08ad2

File tree

3 files changed

+43
-14
lines changed

3 files changed

+43
-14
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,9 @@ Bug Fixes
206206
missing when used, or vice versa. This makes sure that Clang picks the
207207
correct one, where it previously would consider multiple ones as potentially
208208
acceptable (and erroneously use whichever one is tried first).
209+
- Fix a crash when generating code coverage information for an
210+
``if consteval`` statement. This fixes
211+
`Issue 57377 <https://github.com/llvm/llvm-project/issues/57377>`_.
209212

210213
Improvements to Clang's diagnostics
211214
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

clang/lib/CodeGen/CoverageMappingGen.cpp

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,19 +1377,23 @@ struct CounterCoverageMappingBuilder
13771377

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

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

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

1389-
// The 'then' count applies to the area immediately after the condition.
1390-
auto Gap = findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
1391-
if (Gap)
1392-
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
1391+
// The 'then' count applies to the area immediately after the condition.
1392+
Optional<SourceRange> Gap =
1393+
findGapAreaBetween(S->getRParenLoc(), getStart(S->getThen()));
1394+
if (Gap)
1395+
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
1396+
}
13931397

13941398
extendRegion(S->getThen());
13951399
Counter OutCount = propagateCounts(ThenCount, S->getThen());
@@ -1398,9 +1402,9 @@ struct CounterCoverageMappingBuilder
13981402
if (const Stmt *Else = S->getElse()) {
13991403
bool ThenHasTerminateStmt = HasTerminateStmt;
14001404
HasTerminateStmt = false;
1401-
14021405
// The 'else' count applies to the area immediately after the 'then'.
1403-
Gap = findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
1406+
Optional<SourceRange> Gap =
1407+
findGapAreaBetween(getEnd(S->getThen()), getStart(Else));
14041408
if (Gap)
14051409
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ElseCount);
14061410
extendRegion(Else);
@@ -1416,9 +1420,11 @@ struct CounterCoverageMappingBuilder
14161420
GapRegionCounter = OutCount;
14171421
}
14181422

1419-
// Create Branch Region around condition.
1420-
createBranchRegion(S->getCond(), ThenCount,
1421-
subtractCounters(ParentCount, ThenCount));
1423+
if (!S->isConsteval()) {
1424+
// Create Branch Region around condition.
1425+
createBranchRegion(S->getCond(), ThenCount,
1426+
subtractCounters(ParentCount, ThenCount));
1427+
}
14221428
}
14231429

14241430
void VisitCXXTryStmt(const CXXTryStmt *S) {

clang/test/CoverageMapping/if.cpp

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// 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
1+
// 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
22

33
int nop() { return 0; }
44

@@ -13,6 +13,22 @@ void foo() { // CHECK-NEXT: Gap,File 0, [[@LINE+1]]:21 -> [[@
1313
++j; // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:8 = #1, (#0 - #1)
1414
} // CHECK-NEXT: [[@LINE-2]]:9 -> [[@LINE-1]]:5 = #1
1515
// CHECK-NEXT: [[@LINE-2]]:5 -> [[@LINE-2]]:8 = #1
16+
17+
// FIXME: Do not generate coverage for discarded branches in if consteval and if constexpr statements
18+
constexpr int check_consteval(int i) {
19+
if consteval {
20+
i++;
21+
}
22+
if !consteval {
23+
i++;
24+
}
25+
if consteval {
26+
return 42;
27+
} else {
28+
return i;
29+
}
30+
}
31+
1632
// CHECK-LABEL: main:
1733
int main() { // CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 = #0
1834
int i = 0;
@@ -50,6 +66,10 @@ int main() { // CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 =
5066
// CHECK-NEXT: File 0, [[@LINE+1]]:14 -> [[@LINE+1]]:20 = #6
5167
i = i == 0?i + 12:i + 10; // CHECK-NEXT: File 0, [[@LINE]]:21 -> [[@LINE]]:27 = (#0 - #6)
5268

69+
// GH-57377
70+
constexpr int c_i = check_consteval(0);
71+
check_consteval(i);
72+
5373
return 0;
5474
}
5575

0 commit comments

Comments
 (0)