[clang] [SemaCXX] Make __builtin_addressof more like std::addressof (PR #78035)

via cfe-commits cfe-commits at lists.llvm.org
Sat Jan 13 06:03:19 PST 2024


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


More information about the cfe-commits mailing list