llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (comex) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/154034.diff 2 Files Affected: - (modified) clang/lib/Sema/SemaExpr.cpp (+27-21) - (added) clang/test/SemaCXX/builtin-overload-resolution.cpp (+8) ``````````diff 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); +} `````````` </details> https://github.com/llvm/llvm-project/pull/154034 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
