[clang] [clang] Fix side effects resolving overloads of builtin functions (#138651) (PR #154034)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 8 12:17:48 PDT 2025
https://github.com/comex updated https://github.com/llvm/llvm-project/pull/154034
>From 5a0549b35cd5b3b4ecb8a25bcde1560cf85c094d Mon Sep 17 00:00:00 2001
From: comex <comexk at gmail.com>
Date: Sun, 17 Aug 2025 12:41:44 -0700
Subject: [PATCH 1/2] [clang] Fix side effects resolving overloads of builtin
functions (#138651)
When parsing `__builtin_addressof(Value)`, where `Value` is a constexpr
variable of primitive type, we will run through
`rewriteBuiltinFunctionDecl`.
`rewriteBuiltinFunctionDecl` is meant to handle a special case which is
not applicable here. (It only applies when a builtin function's type
has a parameter with pointer type, which is not true for
`__builtin_addressof`, not even if the actual argument is a pointer.)
Therefore, `rewriteBuiltinFunctionDecl` returns `nullptr` and things go
on as usual.
But `rewriteBuiltinFunctionDecl` accidentally has a side effect. It
calls `DefaultFunctionArrayLvalueConversion` ->
`DefaultLvalueConversion` -> `CheckLValueToRValueConversionOperand` ->
`rebuildPotentialResultsAsNonOdrUsed` -> `MarkNotOdrUsed`, which removes
the expression from `S.MaybeODRUseExprs`.
This would be correct if `Value` were actually going through an
lvalue-to-rvalue conversion, because it's a constant expression:
https://eel.is/c++draft/basic.def.odr#5.2.2.2
But in this case the conversion is only hypothetical, as part of
`rewriteBuiltinFunctionDecl`'s pseudo-overload-resolution logic.
Avoid the side effect by pushing an `ExpressionEvaluationContext`, like
we do for real overload resolution.
Similarly, push a `SFINAETrap` to suppress diagnostics emitted during
`DefaultFunctionArrayLvalueConversion`. This fixes a false-positive
compile error when applying `__builtin_addressof` to certain volatile
union variables, and fixes a duplicated compile error when applying
`__builtin_get_vtable_pointer` to an overloaded function name.
---
clang/docs/ReleaseNotes.rst | 2 +
clang/lib/Sema/SemaExpr.cpp | 50 +++++++++++--------
.../SemaCXX/builtin-get-vtable-pointer.cpp | 3 --
.../SemaCXX/builtin-overload-resolution.cpp | 8 +++
clang/test/SemaObjC/non-trivial-c-union.m | 7 +++
5 files changed, 46 insertions(+), 24 deletions(-)
create mode 100644 clang/test/SemaCXX/builtin-overload-resolution.cpp
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d1ef91b7e7c14..ff217e9272c25 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -349,6 +349,8 @@ Bug Fixes to C++ Support
authentication enabled. (#GH152601)
- Fix the check for narrowing int-to-float conversions, so that they are detected in
cases where converting the float back to an integer is undefined behaviour (#GH157067).
+- Fix an assertion failure when a ``constexpr`` variable is only referenced through
+ ``__builtin_addressof``, and related issues with builtin arguments. (#GH154034)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 317b7caec6fb7..9d153f0627bb4 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6313,30 +6313,38 @@ static FunctionDecl *rewriteBuiltinFunctionDecl(Sema *Sema, ASTContext &Context,
unsigned i = 0;
SmallVector<QualType, 8> OverloadParams;
- for (QualType ParamType : FT->param_types()) {
+ {
+ // The lvalue conversions in this loop are only for type resolution and
+ // don't actually occur.
+ EnterExpressionEvaluationContext Unevaluated(
+ *Sema, Sema::ExpressionEvaluationContext::Unevaluated);
+ Sema::SFINAETrap Trap(*Sema);
- // Convert array arguments to pointer to simplify type lookup.
- ExprResult ArgRes =
- Sema->DefaultFunctionArrayLvalueConversion(ArgExprs[i++]);
- if (ArgRes.isInvalid())
- return nullptr;
- Expr *Arg = ArgRes.get();
- QualType ArgType = Arg->getType();
- if (!ParamType->isPointerType() ||
- ParamType->getPointeeType().hasAddressSpace() ||
- !ArgType->isPointerType() ||
- !ArgType->getPointeeType().hasAddressSpace() ||
- isPtrSizeAddressSpace(ArgType->getPointeeType().getAddressSpace())) {
- OverloadParams.push_back(ParamType);
- continue;
- }
+ for (QualType ParamType : FT->param_types()) {
- QualType PointeeType = ParamType->getPointeeType();
- NeedsNewDecl = true;
- LangAS AS = ArgType->getPointeeType().getAddressSpace();
+ // Convert array arguments to pointer to simplify type lookup.
+ ExprResult ArgRes =
+ Sema->DefaultFunctionArrayLvalueConversion(ArgExprs[i++]);
+ if (ArgRes.isInvalid())
+ return nullptr;
+ Expr *Arg = ArgRes.get();
+ QualType ArgType = Arg->getType();
+ if (!ParamType->isPointerType() ||
+ ParamType->getPointeeType().hasAddressSpace() ||
+ !ArgType->isPointerType() ||
+ !ArgType->getPointeeType().hasAddressSpace() ||
+ isPtrSizeAddressSpace(ArgType->getPointeeType().getAddressSpace())) {
+ OverloadParams.push_back(ParamType);
+ continue;
+ }
- PointeeType = Context.getAddrSpaceQualType(PointeeType, AS);
- OverloadParams.push_back(Context.getPointerType(PointeeType));
+ QualType PointeeType = ParamType->getPointeeType();
+ NeedsNewDecl = true;
+ LangAS AS = ArgType->getPointeeType().getAddressSpace();
+
+ PointeeType = Context.getAddrSpaceQualType(PointeeType, AS);
+ OverloadParams.push_back(Context.getPointerType(PointeeType));
+ }
}
if (!NeedsNewDecl)
diff --git a/clang/test/SemaCXX/builtin-get-vtable-pointer.cpp b/clang/test/SemaCXX/builtin-get-vtable-pointer.cpp
index b04b38d7996eb..b99bbf0e82030 100644
--- a/clang/test/SemaCXX/builtin-get-vtable-pointer.cpp
+++ b/clang/test/SemaCXX/builtin-get-vtable-pointer.cpp
@@ -66,9 +66,7 @@ struct PolymorphicTemplate {
};
void test_function(int); // expected-note{{possible target for call}}
- // expected-note at -1{{possible target for call}}
void test_function(double); // expected-note{{possible target for call}}
- // expected-note at -1{{possible target for call}}
void getVTablePointer() {
ForwardDeclaration *fd = nullptr;
@@ -89,7 +87,6 @@ void getVTablePointer() {
__builtin_get_vtable_pointer(np_array); // expected-error{{__builtin_get_vtable_pointer requires an argument of polymorphic class pointer type, but 'NonPolymorphic' has no virtual methods}}
__builtin_get_vtable_pointer(&np_array); // expected-error{{__builtin_get_vtable_pointer requires an argument of class pointer type, but 'NonPolymorphic (*)[1]' was provided}}
__builtin_get_vtable_pointer(test_function); // expected-error{{reference to overloaded function could not be resolved; did you mean to call it?}}
- // expected-error at -1{{reference to overloaded function could not be resolved; did you mean to call it?}}
Foo<double> Food;
Foo<int> Fooi;
__builtin_get_vtable_pointer(Food); // expected-error{{__builtin_get_vtable_pointer requires an argument of class pointer type, but 'Foo<double>' was provided}}
diff --git a/clang/test/SemaCXX/builtin-overload-resolution.cpp b/clang/test/SemaCXX/builtin-overload-resolution.cpp
new file mode 100644
index 0000000000000..81d3055a2f7b2
--- /dev/null
+++ b/clang/test/SemaCXX/builtin-overload-resolution.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -std=c++20 %s -emit-obj -o /dev/null
+
+const int* test_odr_used() {
+ // This previously crashed due to Value improperly being removed from
+ // MaybeODRUseExprs.
+ static constexpr int Value = 0;
+ return __builtin_addressof(Value);
+}
diff --git a/clang/test/SemaObjC/non-trivial-c-union.m b/clang/test/SemaObjC/non-trivial-c-union.m
index 34f1caad59df7..39fbe2d33818e 100644
--- a/clang/test/SemaObjC/non-trivial-c-union.m
+++ b/clang/test/SemaObjC/non-trivial-c-union.m
@@ -87,3 +87,10 @@ void testVolatileLValueToRValue(volatile U0 *a) {
void unionInSystemHeader0(U0_SystemHeader);
void unionInSystemHeader1(U1_SystemHeader); // expected-error {{cannot use type 'U1_SystemHeader' for a function/method parameter since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U1_SystemHeader' for a function/method parameter since it is a union that is non-trivial to copy}}
+
+void testAddressof(void) {
+ extern volatile U0 t0;
+ // These don't dereference so they shouldn't cause an error.
+ (void)&t0;
+ (void)__builtin_addressof(t0);
+}
>From d71f6e207573ee6f0a57505a769dc2d3e3287d7c Mon Sep 17 00:00:00 2001
From: comex <comexk at gmail.com>
Date: Mon, 8 Sep 2025 12:17:40 -0700
Subject: [PATCH 2/2] Set ForValidityCheck in SFINAETrap constructor
Co-authored-by: Corentin Jabot <corentinjabot at gmail.com>
---
clang/lib/Sema/SemaExpr.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 9d153f0627bb4..aba00dc8ff9b6 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6318,7 +6318,7 @@ static FunctionDecl *rewriteBuiltinFunctionDecl(Sema *Sema, ASTContext &Context,
// don't actually occur.
EnterExpressionEvaluationContext Unevaluated(
*Sema, Sema::ExpressionEvaluationContext::Unevaluated);
- Sema::SFINAETrap Trap(*Sema);
+ Sema::SFINAETrap Trap(*Sema, /*ForValidityCheck=*/true);
for (QualType ParamType : FT->param_types()) {
More information about the cfe-commits
mailing list