[coverage] fix crash in code coverage and if constexpr with ExprWithCleanups

[hanickadot]

Fixes #80285

[llvmbot]

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Hana Dusíková (hanickadot)

Changes

Fixes #80285

---

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

2 Files Affected:

• (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+14-2)
• (modified) clang/test/CoverageMapping/if.cpp (+29)

diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 8b5e6c4ad8272..6640fe6f41fd1 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -1808,12 +1808,24 @@ struct CounterCoverageMappingBuilder } } +private: + static bool evaluateConstantCondition(const Expr *Condition) { + if (const auto *Expr = dyn_cast<ConstantExpr>(Condition)) + return Expr->getResultAsAPSInt().getExtValue(); + + if (const auto *Expr = dyn_cast<ExprWithCleanups>(Condition)) + return evaluateConstantCondition(Expr->getSubExpr()); // recursion + + assert(false && "Unexpected node in 'if constexpr' condition"); + return false; + } + +public: void coverIfConstexpr(const IfStmt *S) { assert(S->isConstexpr()); // evaluate constant condition... - const auto *E = cast<ConstantExpr>(S->getCond()); - const bool isTrue = E->getResultAsAPSInt().getExtValue(); + const bool isTrue = evaluateConstantCondition(S->getCond()); extendRegion(S); diff --git a/clang/test/CoverageMapping/if.cpp b/clang/test/CoverageMapping/if.cpp index 3045ffe43948c..4de1467aa7ee3 100644 --- a/clang/test/CoverageMapping/if.cpp +++ b/clang/test/CoverageMapping/if.cpp @@ -234,6 +234,35 @@ constexpr int check_macro_consteval_if_skipped(int i) { // CHECK-NEXT: [[@LINE return i; } +struct false_value { + constexpr operator bool() { + return false; + } +}; + +template <typename> struct dependable_false_value { + constexpr operator bool() { + return false; + } +}; + +// GH-80285 +void should_not_crash() { + if constexpr (false_value{}) { }; +} + +template <typename> void should_not_crash_dependable() { + if constexpr (dependable_false_value<int>{}) { }; +} + +void should_not_crash_with_template_instance() { + should_not_crash_dependable<int>(); +} + +void should_not_crash_with_requires_expr() { + if constexpr (requires {42;}) { }; +} + int instantiate_consteval(int i) { i *= check_consteval_with_else_discarded_then(i); i *= check_notconsteval_with_else_discarded_else(i); 

[AaronBallman]

Changes LGTM, but please add a release note to clang/docs/ReleaseNotes.rst so users know about the fix.

Thank you for the fix!

[hanickadot]

Changes LGTM, but please add a release note to clang/docs/ReleaseNotes.rst so users know about the fix.

Thank you for the fix!

Even if it's a fix for recent PR I did few days ago?

Also, how do I get this into release branch (the previous change is there)?

[cor3ntin]

Changes LGTM, but please add a release note to clang/docs/ReleaseNotes.rst so users know about the fix.

Thank you for the fix!

FYI this is a regression in 18 affecting users so we should backport

[AaronBallman]

Changes LGTM, but please add a release note to clang/docs/ReleaseNotes.rst so users know about the fix.
Thank you for the fix!

Even if it's a fix for recent PR I did few days ago?

Oh, I had missed that, sorry! Then no, a release note isn't needed.

[AaronBallman]

Changes LGTM, but please add a release note to clang/docs/ReleaseNotes.rst so users know about the fix.
Thank you for the fix!

FYI this is a regression in 18 affecting users so we should backport

Btw, instructions on how to do that can be found here: https://www.llvm.org/docs/GitHub.html#backporting-fixes-to-the-release-branches