[clang] [Clang][Smea] Accept weak reference after declaration (PR #228809)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: aokblast

<details>
<summary>Changes</summary>

GCC accepts a weak reference even after there is a declaration. It is fine for us to just append a weak attribute in the previous declaration directly.

Fixes: #<!-- -->56760 

---
Full diff: https://github.com/llvm/llvm-project/pull/228809.diff


4 Files Affected:

- (modified) clang/docs/ReleaseNotes.md (+3) 
- (modified) clang/lib/Sema/SemaDeclAttr.cpp (+25) 
- (modified) clang/test/CodeGen/pragma-weak.c (+37) 
- (modified) clang/test/Sema/pragma-weak.c (+17) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index b71758e4b9647..88537fd63e481 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -580,6 +580,9 @@ features cannot lower the translation-unit ABI level;
 
 #### Bug Fixes to Attribute Support
 
+- `#pragma weak alias = target` no longer makes every later use of `alias`
+  ambiguous when `alias` was already declared: the alias is now attached to the
+  existing declaration, and a definition of `alias` is diagnosed. (#GH35478), (#GH56760)
 - Fixed crash (assertion) when the `alloc_align` attribute was applied to a declaration whose type has a `FunctionProtoType` but which is not itself a `FunctionDecl`, such as a function-pointer variable. (#GH122058)
 
 - Fixed a crash on `bool` vectors declared with `ext_vector_type` and more than
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index eb4a8c2ab9ae0..0d9f2eb2029dd 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -8804,6 +8804,31 @@ NamedDecl *Sema::DeclClonePragmaWeak(NamedDecl *ND, const IdentifierInfo *II,
 void Sema::DeclApplyPragmaWeak(Scope *S, NamedDecl *ND, const WeakInfo &W) {
   if (W.getAlias()) { // clone decl, impersonate __attribute(weak,alias(...))
     IdentifierInfo *NDId = ND->getIdentifier();
+
+    // Append attribute on previous declaration only.
+    NamedDecl *Prev = LookupSingleName(TUScope, W.getAlias(), W.getLocation(),
+                                       LookupOrdinaryName);
+    if (Prev && (isa<FunctionDecl>(Prev) || isa<VarDecl>(Prev)) &&
+        Prev->getDeclContext()->getRedeclContext()->isTranslationUnit()) {
+      if (Prev->hasAttr<AliasAttr>())
+        return;
+      bool IsDefinition = false;
+      if (auto *FD = dyn_cast<FunctionDecl>(Prev))
+        IsDefinition = FD->isDefined();
+      else
+        IsDefinition =
+            cast<VarDecl>(Prev)->hasDefinition() != VarDecl::DeclarationOnly;
+      if (IsDefinition) {
+        Diag(W.getLocation(), diag::err_alias_is_definition) << Prev << 0;
+        return;
+      }
+      Prev->addAttr(
+          AliasAttr::CreateImplicit(Context, NDId->getName(), W.getLocation()));
+      Prev->addAttr(WeakAttr::CreateImplicit(Context, W.getLocation()));
+      WeakTopLevelDecl.push_back(Prev);
+      return;
+    }
+
     NamedDecl *NewD = DeclClonePragmaWeak(ND, W.getAlias(), W.getLocation());
     NewD->addAttr(
         AliasAttr::CreateImplicit(Context, NDId->getName(), W.getLocation()));
diff --git a/clang/test/CodeGen/pragma-weak.c b/clang/test/CodeGen/pragma-weak.c
index ea508a485ef61..9d32e1f177294 100644
--- a/clang/test/CodeGen/pragma-weak.c
+++ b/clang/test/CodeGen/pragma-weak.c
@@ -21,6 +21,10 @@
 // CHECK-DAG: @undecfunc_alias2 = weak{{.*}} alias void (), ptr @undecfunc
 // CHECK-DAG: @undecfunc_alias3 = weak{{.*}} alias void (), ptr @undecfunc
 // CHECK-DAG: @undecfunc_alias4 = weak{{.*}} alias void (), ptr @undecfunc
+// CHECK-DAG: @predecl_alias = weak{{.*}} alias i32 (i32), ptr @__predecl_alias
+// CHECK-DAG: @predecl_noproto_alias = weak{{.*}} alias i32 (...), ptr @__predecl_noproto_alias
+// CHECK-DAG: @predecl_alias2 = weak{{.*}} alias i32 (i32), ptr @__predecl_alias2
+// CHECK-DAG: @predecl_var_alias = weak{{.*}} alias i32, ptr @__predecl_var_alias
 
 
 
@@ -149,6 +153,39 @@ __attribute((noinline,const)) int __xxx(void) { return 0; }
 void undecfunc_alias2(void);
 void undecfunc(void) { }
 
+///////////// GH35478, GH56760: the weak name is declared before the pragma. The
+// pragma must apply to that declaration rather than introduce a second one, so
+// uses of the name stay unambiguous.
+int predecl_alias(int), __predecl_alias(int);
+#pragma weak predecl_alias = __predecl_alias
+int __predecl_alias(int i) { return 0; }
+int use_predecl_alias(void) { return predecl_alias(0); }
+// CHECK-LABEL: define{{.*}} i32 @use_predecl_alias()
+// CHECK: call i32 @predecl_alias(i32 noundef 0)
+
+// Same, without a prototype (GH35478).
+extern int predecl_noproto_alias();
+#pragma weak predecl_noproto_alias = __predecl_noproto_alias
+int __predecl_noproto_alias() { return 0; }
+int use_predecl_noproto_alias(void) { return predecl_noproto_alias(); }
+// CHECK-LABEL: define{{.*}} i32 @use_predecl_noproto_alias()
+// CHECK: call i32 (...) @predecl_noproto_alias()
+
+// Same, but with the pragma preceding both declarations.
+#pragma weak predecl_alias2 = __predecl_alias2
+int predecl_alias2(int), __predecl_alias2(int);
+int __predecl_alias2(int i) { return 0; }
+int use_predecl_alias2(void) { return predecl_alias2(0); }
+// CHECK-LABEL: define{{.*}} i32 @use_predecl_alias2()
+// CHECK: call i32 @predecl_alias2(i32 noundef 0)
+
+extern int predecl_var_alias;
+int __predecl_var_alias = 3;
+#pragma weak predecl_var_alias = __predecl_var_alias
+int use_predecl_var_alias(void) { return predecl_var_alias; }
+// CHECK-LABEL: define{{.*}} i32 @use_predecl_var_alias()
+// CHECK: load i32, ptr @predecl_var_alias
+
 ///////////// PR10878: Make sure we can call a weak alias
 void SHA512Pad(void *context) {}
 #pragma weak SHA384Pad = SHA512Pad
diff --git a/clang/test/Sema/pragma-weak.c b/clang/test/Sema/pragma-weak.c
index c14125eac9f7c..3b9991cc82052 100644
--- a/clang/test/Sema/pragma-weak.c
+++ b/clang/test/Sema/pragma-weak.c
@@ -9,3 +9,20 @@ void __a3(void) __attribute((noinline));
 #pragma weak a3 = __a3 // expected-note {{previous definition}}
 void a3(void) __attribute((alias("__a3"))); // expected-error {{redefinition of 'a3'}}
 void __a3(void) {}
+
+// The weak name is already defined: a definition cannot also be an alias.
+void __defined_fn(void);
+void defined_fn(void) {}
+#pragma weak defined_fn = __defined_fn // expected-error {{definition 'defined_fn' cannot also be an alias}}
+void __defined_fn(void) {}
+
+int __defined_var = 1;
+int defined_var;
+#pragma weak defined_var = __defined_var // expected-error {{definition 'defined_var' cannot also be an alias}}
+
+// GH35478, GH56760: a pre-existing declaration of the weak name must not make
+// uses ambiguous.
+int predecl(int), __predecl(int);
+#pragma weak predecl = __predecl
+int __predecl(int i) { return 0; }
+int use_predecl(void) { return predecl(0); }

``````````

</details>


https://github.com/llvm/llvm-project/pull/228809