https://github.com/comex created https://github.com/llvm/llvm-project/pull/154034
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. Fix the side effect by pushing an `ExpressionEvaluationContext`, like we do for real overload resolution. >From cb74071969e6642317e05505a16fbdc85cef55b4 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. --- clang/lib/Sema/SemaExpr.cpp | 48 +++++++++++-------- .../SemaCXX/builtin-overload-resolution.cpp | 8 ++++ 2 files changed, 35 insertions(+), 21 deletions(-) create mode 100644 clang/test/SemaCXX/builtin-overload-resolution.cpp diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 237c068f59283..c00de2d4ed17b 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -6312,30 +6312,36 @@ static FunctionDecl *rewriteBuiltinFunctionDecl(Sema *Sema, ASTContext &Context, unsigned i = 0; SmallVector<QualType, 8> OverloadParams; - for (QualType ParamType : FT->param_types()) { + { + // Push an evaluation context since the lvalue conversions in this loop + // are only for type resolution and don't actually occur. + EnterExpressionEvaluationContext Unevaluated( + *Sema, Sema::ExpressionEvaluationContext::Unevaluated); + for (QualType ParamType : FT->param_types()) { - // 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; - } + // 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; + } - QualType PointeeType = ParamType->getPointeeType(); - NeedsNewDecl = true; - LangAS AS = ArgType->getPointeeType().getAddressSpace(); + QualType PointeeType = ParamType->getPointeeType(); + NeedsNewDecl = true; + LangAS AS = ArgType->getPointeeType().getAddressSpace(); - PointeeType = Context.getAddrSpaceQualType(PointeeType, AS); - OverloadParams.push_back(Context.getPointerType(PointeeType)); + PointeeType = Context.getAddrSpaceQualType(PointeeType, AS); + OverloadParams.push_back(Context.getPointerType(PointeeType)); + } } if (!NeedsNewDecl) 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); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
