llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Mital Ashok (MitalAshok) <details> <summary>Changes</summary> Fixes #<!-- -->77928 There are a few places which expect that, if for an expression `E->getType()->isBuiltinType(BuiltinType::Overload)`, then `E` must be an `OverloadExpr` or a `UnaryOperator` where `cast<UnaryOperator>(E)->getOpcode() == UO_AddrOf`. Because `__builtin_addressof` is implemented similarly to `&`: https://github.com/llvm/llvm-project/blob/8d817f6479a5df874028a8b40fd30aecd3479005/clang/lib/Sema/SemaExpr.cpp#L15041 It can result in something with `OverloadTy` that isn't an `OverloadExpr` or a `UnaryOperator` (it's a `CallExpr`). There are two things we can do to fix this: Either change those checks to accept both `&E` and `__builtin_addressof(E)`, or change it so `__builtin_addressof(E)` doesn't treat OverloadExprs differently. This takes the latter approach, disabling `__builtin_addressof(name-of-overloaded-function)`. It appears that GCC allows `__builtin_addressof(fn)` to be an overloaded function: <https://godbolt.org/z/G4xcbaffY>. The problem with allowing it in Clang is too much code generically deals with `CallExpr`s and you would have to special case `__builtin_addressof` to not try to resolve the arguments. And also it would be so rarely used in the first place (the fix being replacing `__builtin_addressof` with `&`) --- Full diff: https://github.com/llvm/llvm-project/pull/78035.diff 4 Files Affected: - (modified) clang/include/clang/Sema/Sema.h (+2-1) - (modified) clang/lib/Sema/SemaChecking.cpp (+1-1) - (modified) clang/lib/Sema/SemaExpr.cpp (+17-7) - (modified) clang/test/SemaCXX/builtins.cpp (+37-1) ``````````diff diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index cf2d4fbe6d3ba1..9941cbaadf780a 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5805,7 +5805,8 @@ class Sema final { const Expr *Op, const CXXMethodDecl *MD); - QualType CheckAddressOfOperand(ExprResult &Operand, SourceLocation OpLoc); + QualType CheckAddressOfOperand(ExprResult &Operand, SourceLocation OpLoc, + bool IsBuiltinAddressof = false); bool CheckTypeTraitArity(unsigned Arity, SourceLocation Loc, size_t N); diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 74f8f626fb1637..e96436fd75bb68 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -242,7 +242,7 @@ static bool SemaBuiltinAddressof(Sema &S, CallExpr *TheCall) { return true; ExprResult Arg(TheCall->getArg(0)); - QualType ResultType = S.CheckAddressOfOperand(Arg, TheCall->getBeginLoc()); + QualType ResultType = S.CheckAddressOfOperand(Arg, TheCall->getBeginLoc(), true); if (ResultType.isNull()) return true; diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 2f48ea237cdfa4..518c51e77c7a99 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -15038,9 +15038,15 @@ bool Sema::CheckUseOfCXXMethodAsAddressOfOperand(SourceLocation OpLoc, /// operator (C99 6.3.2.1p[2-4]), and its result is never an lvalue. /// In C++, the operand might be an overloaded function name, in which case /// we allow the '&' but retain the overloaded-function type. -QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc) { +/// If IsBuiltinAddressof is true, this is spelt __builtin_addressof(E) +/// instead of &E. +QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc, + bool IsBuiltinAddressof) { if (const BuiltinType *PTy = OrigOp.get()->getType()->getAsPlaceholderType()){ if (PTy->getKind() == BuiltinType::Overload) { + if (IsBuiltinAddressof) + return QualType(); + Expr *E = OrigOp.get()->IgnoreParens(); if (!isa<OverloadExpr>(E)) { assert(cast<UnaryOperator>(E)->getOpcode() == UO_AddrOf); @@ -15094,7 +15100,7 @@ QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc) { } } - if (getLangOpts().C99) { + if (!IsBuiltinAddressof && getLangOpts().C99) { // Implement C99-only parts of addressof rules. if (UnaryOperator* uOp = dyn_cast<UnaryOperator>(op)) { if (uOp->getOpcode() == UO_Deref) @@ -15116,11 +15122,11 @@ QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc) { unsigned AddressOfError = AO_No_Error; if (lval == Expr::LV_ClassTemporary || lval == Expr::LV_ArrayTemporary) { - bool sfinae = (bool)isSFINAEContext(); - Diag(OpLoc, isSFINAEContext() ? diag::err_typecheck_addrof_temporary - : diag::ext_typecheck_addrof_temporary) + bool AllowTemporaryAddress = !isSFINAEContext() && !IsBuiltinAddressof; + Diag(OpLoc, AllowTemporaryAddress ? diag::ext_typecheck_addrof_temporary + : diag::err_typecheck_addrof_temporary) << op->getType() << op->getSourceRange(); - if (sfinae) + if (!AllowTemporaryAddress) return QualType(); // Materialize the temporary as an lvalue so that we can take its address. OrigOp = op = @@ -15130,6 +15136,8 @@ QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc) { } else if (lval == Expr::LV_MemberFunction) { // If it's an instance method, make a member pointer. // The expression must have exactly the form &A::foo. + if (IsBuiltinAddressof) + return QualType(); // If the underlying expression isn't a decl ref, give up. if (!isa<DeclRefExpr>(op)) { @@ -15187,12 +15195,14 @@ QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc) { } else if (isa<MSPropertyDecl>(dcl)) { AddressOfError = AO_Property_Expansion; } else if (isa<FunctionTemplateDecl>(dcl)) { - return Context.OverloadTy; + return IsBuiltinAddressof ? QualType() : Context.OverloadTy; } else if (isa<FieldDecl>(dcl) || isa<IndirectFieldDecl>(dcl)) { // Okay: we can take the address of a field. // Could be a pointer to member, though, if there is an explicit // scope qualifier for the class. if (isa<DeclRefExpr>(op) && cast<DeclRefExpr>(op)->getQualifier()) { + if (IsBuiltinAddressof) + return QualType(); DeclContext *Ctx = dcl->getDeclContext(); if (Ctx && Ctx->isRecord()) { if (dcl->getType()->isReferenceType()) { diff --git a/clang/test/SemaCXX/builtins.cpp b/clang/test/SemaCXX/builtins.cpp index 567094c94c171b..a59eb36eef4451 100644 --- a/clang/test/SemaCXX/builtins.cpp +++ b/clang/test/SemaCXX/builtins.cpp @@ -39,7 +39,43 @@ namespace addressof { struct U { int n : 5; } u; int *pbf = __builtin_addressof(u.n); // expected-error {{address of bit-field requested}} - S *ptmp = __builtin_addressof(S{}); // expected-error {{taking the address of a temporary}} expected-warning {{temporary whose address is used as value of local variable 'ptmp' will be destroyed at the end of the full-expression}} + S *ptmp = __builtin_addressof(S{}); // expected-error {{taking the address of a temporary}} + int *memtmp = __builtin_addressof(T{}.n); // expected-error {{cannot take the address of an rvalue of type 'int'}} + S *pxval = __builtin_addressof(static_cast<S&&>(s)); // expected-error {{cannot take the address of an rvalue of type 'S'}} + + void a(int) {} // expected-note 3 {{possible target for call}} + void a(float) {} // expected-note 3 {{possible target for call}} + + int A = __builtin_addressof(a); // expected-error {{reference to overloaded function could not be resolved; did you mean to call it?}} + int B = &__builtin_addressof(a); // expected-error {{reference to overloaded function could not be resolved; did you mean to call it?}} + int C = __builtin_addressof(&a); // expected-error {{reference to overloaded function could not be resolved; did you mean to call it?}} + + template<typename T> + constexpr decltype(void(__builtin_addressof(T::f)), true) can_take_address_of_f(int) { return true; } + template<typename T> + constexpr bool can_take_address_of_f(...) { return false; } + + struct V1 { static void f(int); static void f(char); }; + struct V2 { static void f(int); }; + struct V3 { static int f; }; + struct V4 { + int f; + int* g() { + return __builtin_addressof(f); + } + int* h() { + return __builtin_addressof(V4::f); + } + }; + struct V5 { void f(int); }; + struct V6 { using f = int; }; + + static_assert(!can_take_address_of_f<V1>(0), ""); + static_assert( can_take_address_of_f<V2>(0), ""); + static_assert( can_take_address_of_f<V3>(0), ""); + static_assert(!can_take_address_of_f<V4>(0), ""); + static_assert(!can_take_address_of_f<V5>(0), ""); + static_assert(!can_take_address_of_f<V6>(0), ""); } namespace function_start { `````````` </details> https://github.com/llvm/llvm-project/pull/78035 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
