https://github.com/comex updated https://github.com/llvm/llvm-project/pull/154034
>From 5a0549b35cd5b3b4ecb8a25bcde1560cf85c094d Mon Sep 17 00:00:00 2001 From: comex <com...@gmail.com> Date: Sun, 17 Aug 2025 12:41:44 -0700 Subject: [PATCH] [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@-1{{possible target for call}} void test_function(double); // expected-note{{possible target for call}} - // expected-note@-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@-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); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
