[Clang][Sema] Give calls synthesized by __builtin_invoke a valid source range - #220593
Conversation
…ce range BuiltinInvoke wrapped the synthesized obj.*memptr expression in a ParenExpr with invalid source locations, so the member call built from it had an invalid begin location whenever there were no trailing arguments to fall back on. The constant evaluator stores each call expression's range as the frame's CallRange and asserts it is valid when building the "in call to" note chain, so diagnosing a failure inside such a call crashed. Use the builtin call's own locations for the ParenExpr so the synthesized call maps back to the __builtin_invoke call site. Fixes llvm#185241
|
@llvm/pr-subscribers-clang Author: Akash Manna (akash-manna-sky) ChangesFixes #185241 When The Full diff: https://github.com/llvm/llvm-project/pull/220593.diff 3 Files Affected:
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index 4c2bf55f6ebdd..0a7e628aa1880 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -492,6 +492,9 @@ features cannot lower the translation-unit ABI level;
such as when the call is used as an `auto` non-type template argument.
- Fixed a crash in ``__builtin_dump_struct`` when ``-Werror`` promotes
format warnings to errors. (#GH211943)
+- Fixed an assertion failure when diagnosing a constant evaluation failure
+ inside a member function call synthesized by ``__builtin_invoke``.
+ (#GH185241)
#### Bug Fixes to Attribute Support
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 7b4dca61f70dc..564369da4ddea 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3015,8 +3015,9 @@ static ExprResult BuiltinInvoke(Sema &S, CallExpr *TheCall) {
if (MPT->isMemberDataPointer())
return BinOp;
+ // Give the synthesized expression a valid source range for diagnostics.
auto *MemCall = new (S.Context)
- ParenExpr(SourceLocation(), SourceLocation(), BinOp.get());
+ ParenExpr(TheCall->getBeginLoc(), TheCall->getRParenLoc(), BinOp.get());
return S.ActOnCallExpr(S.getCurScope(), MemCall, TheCall->getBeginLoc(),
Args.drop_front(2), TheCall->getRParenLoc());
diff --git a/clang/test/SemaCXX/builtin-invoke.cpp b/clang/test/SemaCXX/builtin-invoke.cpp
index d7694d619c692..e92ccc4e62d8f 100644
--- a/clang/test/SemaCXX/builtin-invoke.cpp
+++ b/clang/test/SemaCXX/builtin-invoke.cpp
@@ -238,3 +238,17 @@ static_assert([]() {
return true;
}());
+
+namespace GH185241 {
+struct S {
+ constexpr int func() {} // expected-note {{control reached end of constexpr function}}
+};
+
+static_assert([]() { // expected-error {{static assertion expression is not an integral constant expression}} \
+ expected-note {{in call to '[]() {}.operator()()'}}
+ S s;
+ if (__builtin_invoke(&S::func, s)) // expected-note {{in call to 's.func()'}}
+ return false;
+ return true;
+}());
+} // namespace GH185241
|
|
@Sirraide, can you please take a look? |
shafik
left a comment
There was a problem hiding this comment.
This looks reasonable but I would like @philnik777 to chime in since he added BuiltinInvoke
| inside a member function call synthesized by ``__builtin_invoke``. | ||
| (#GH185241) |
There was a problem hiding this comment.
| inside a member function call synthesized by ``__builtin_invoke``. | |
| (#GH185241) | |
| inside a member function call synthesized by ``__builtin_invoke``. (#GH185241) |
+1 |
| - Fixed a crash in ``__builtin_dump_struct`` when ``-Werror`` promotes | ||
| format warnings to errors. (#GH211943) | ||
| - Fixed an assertion failure when diagnosing a constant evaluation failure | ||
| inside a member function call synthesized by ``__builtin_invoke``. (#GH185241) |
There was a problem hiding this comment.
(nit: double-backticks are valid markdown and this does work properly, but you technically only need single backticks)
|
Let's resolve the conflict |
…urce range (llvm#220593) Fixes llvm#185241 When `__builtin_invoke` is called with a pointer to member function, Sema rewrites it into `(obj.*memptr)(args)` and wraps the `.*` expression in a `ParenExpr` that was created without source locations. A call expression takes its begin location from its callee, with the first argument as fallback — so when there are no trailing arguments, the synthesized member call ends up with no valid location at all. The classic constant evaluator stores each call expression's range as the frame's `CallRange` and asserts it is valid while building the "in call to" note chain, so diagnosing any failure inside such a call (here, control flowing off the end of a `constexpr` function) crashed instead of producing the error. The bytecode interpreter only dodges this because it skips invalid ranges when picking a location for the note. The `ParenExpr` now carries the locations of the `__builtin_invoke` call itself, so the synthesized call maps back to its actual call site like an ordinary call would, and the "in call to" note points at the `__builtin_invoke(...)` expression. This also covers the `std::ref` and pointer object forms, which go through the same node, and the assertion stays as is since it guards a real invariant. I also added the reproducer to the existing `builtin-invoke` test.
Fixes #185241
When
__builtin_invokeis called with a pointer to member function, Sema rewrites it into(obj.*memptr)(args)and wraps the.*expression in aParenExprthat was created without source locations. A call expression takes its begin location from its callee, with the first argument as fallback — so when there are no trailing arguments, the synthesized member call ends up with no valid location at all. The classic constant evaluator stores each call expression's range as the frame'sCallRangeand asserts it is valid while building the "in call to" note chain, so diagnosing any failure inside such a call (here, control flowing off the end of aconstexprfunction) crashed instead of producing the error. The bytecode interpreter only dodges this because it skips invalid ranges when picking a location for the note.The
ParenExprnow carries the locations of the__builtin_invokecall itself, so the synthesized call maps back to its actual call site like an ordinary call would, and the "in call to" note points at the__builtin_invoke(...)expression. This also covers thestd::refand pointer object forms, which go through the same node, and the assertion stays as is since it guards a real invariant. I also added the reproducer to the existingbuiltin-invoketest.