[clang] Allow const variables with weak attribute to be overridden
authorAnders Waldenborg <anders@0x63.nu>
Tue, 24 May 2022 19:46:32 +0000 (21:46 +0200)
committerAnders Waldenborg <anders@0x63.nu>
Fri, 3 Jun 2022 21:44:15 +0000 (23:44 +0200)
A variable with `weak` attribute signifies that it can be replaced with
a "strong" symbol link time. Therefore it must not emitted with
"weak_odr" linkage, as that allows the backend to use its value in
optimizations.

The frontend already considers weak const variables as
non-constant (note_constexpr_var_init_weak diagnostic) so this change
makes frontend and backend consistent.

This commit reverses the
  f49573d1 weak globals that are const should get weak_odr linkage.
commit from 2009-08-05 which introduced this behavior. Unfortunately
that commit doesn't provide any details on why the change was made.

This was discussed in
https://discourse.llvm.org/t/weak-attribute-semantics-on-const-variables/62311

Differential Revision: https://reviews.llvm.org/D126324

clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/global-init.c
clang/test/CodeGen/weak_constant.c

index bcd7ec1..c377738 100644 (file)
@@ -336,6 +336,9 @@ Attribute Changes in Clang
   builtins (corresponding to the specific names listed in the attribute) in the
   body of the function the attribute is on.
 
+- When the ``weak`` attribute is applied to a const qualified variable clang no longer
+  tells the backend it is allowed to optimize based on initializer value.
+
 Windows Support
 ---------------
 
index 4644323..fe6295f 100644 (file)
@@ -2959,7 +2959,7 @@ def WarnUnusedResult : InheritableAttr {
 def Weak : InheritableAttr {
   let Spellings = [GCC<"weak">];
   let Subjects = SubjectList<[Var, Function, CXXRecord]>;
-  let Documentation = [Undocumented];
+  let Documentation = [WeakDocs];
   let SimpleHandler = 1;
 }
 
index aedf11a..872eb8a 100644 (file)
@@ -6477,3 +6477,69 @@ is only supported in compute shaders.
 The full documentation is available here: https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/sv-groupindex
   }];
 }
+
+def WeakDocs : Documentation {
+  let Category = DocCatDecl;
+  let Content = [{
+
+In supported output formats the ``weak`` attribute can be used to
+specify that a variable or function should be emitted as a symbol with
+``weak`` (if a definition) or ``extern_weak`` (if a declaration of an
+external symbol) `linkage
+<https://llvm.org/docs/LangRef.html#linkage-types>`_.
+
+If there is a non-weak definition of the symbol the linker will select
+that over the weak. They must have same type and alignment (variables
+must also have the same size), but may have a different value.
+
+If there are multiple weak definitions of same symbol, but no non-weak
+definition, they should have same type, size, alignment and value, the
+linker will select one of them (see also selectany_ attribute).
+
+If the ``weak`` attribute is applied to a ``const`` qualified variable
+definition that variable is no longer consider a compiletime constant
+as its value can change during linking (or dynamic linking). This
+means that it can e.g no longer be part of an initializer expression.
+
+.. code-block:: c
+
+  const int ANSWER __attribute__ ((weak)) = 42;
+
+  /* This function may be replaced link-time */
+  __attribute__ ((weak)) void debug_log(const char *msg)
+  {
+      fprintf(stderr, "DEBUG: %s\n", msg);
+  }
+
+  int main(int argc, const char **argv)
+  {
+      debug_log ("Starting up...");
+
+      /* This may print something else than "6 * 7 = 42",
+         if there is a non-weak definition of "ANSWER" in
+        an object linked in */
+      printf("6 * 7 = %d\n", ANSWER);
+
+      return 0;
+   }
+
+If an external declaration is marked weak and that symbol does not
+exist during linking (possibly dynamic) the address of the symbol will
+evaluate to NULL.
+
+.. code-block:: c
+
+  void may_not_exist(void) __attribute__ ((weak));
+
+  int main(int argc, const char **argv)
+  {
+      if (may_not_exist) {
+          may_not_exist();
+      } else {
+          printf("Function did not exist\n");
+      }
+      return 0;
+  }
+
+  }];
+}
index b4ae266..758887a 100644 (file)
@@ -4899,12 +4899,8 @@ llvm::GlobalValue::LinkageTypes CodeGenModule::getLLVMLinkageForDeclarator(
   if (Linkage == GVA_Internal)
     return llvm::Function::InternalLinkage;
 
-  if (D->hasAttr<WeakAttr>()) {
-    if (IsConstantVariable)
-      return llvm::GlobalVariable::WeakODRLinkage;
-    else
-      return llvm::GlobalVariable::WeakAnyLinkage;
-  }
+  if (D->hasAttr<WeakAttr>())
+    return llvm::GlobalVariable::WeakAnyLinkage;
 
   if (const auto *FD = D->getAsFunction())
     if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally)
index 11e214a..a69f028 100644 (file)
@@ -9,14 +9,20 @@ int a = 242;
 
 // This should get normal weak linkage.
 int c __attribute__((weak))= 0;
-// CHECK: @c = weak{{.*}} global i32 0
+// CHECK: @c = weak global i32 0
 
-
-// Since this is marked const, it should get weak_odr linkage, since all
-// definitions have to be the same.
-// CHECK: @d = weak_odr constant i32 0
+// Even though is marked const, it should get still get "weak"
+// linkage, not "weak_odr" as the weak attribute makes it possible
+// that there is a strong definition that changes the value linktime,
+// so the value must not be considered constant.
+// CHECK: @d = weak constant i32 0
 const int d __attribute__((weak))= 0;
 
+// However, "selectany" is similar to "weak", but isn't interposable
+// by a strong definition, and should appear as weak_odr.
+// CHECK: @e = weak_odr constant i32 17
+const int e __attribute__((selectany)) = 17;
+
 // PR6168 "too many undefs"
 struct ManyFields {
   int a;
index 726d2ef..67bdd91 100644 (file)
@@ -1,13 +1,31 @@
 // RUN: %clang_cc1 -w -emit-llvm %s -O1 -o - | FileCheck %s
-// Check for bug compatibility with gcc.
+// This used to "check for bug compatibility with gcc".
+// Now it checks that that the "weak" declaration makes the value
+// fully interposable whereas a "selectany" one is handled as constant
+// and propagated.
 
+// CHECK: @x = weak {{.*}}constant i32 123
 const int x __attribute((weak)) = 123;
 
+// CHECK: @y = weak_odr {{.*}}constant i32 234
+const int y __attribute((selectany)) = 234;
+
 int* f(void) {
   return &x;
 }
 
 int g(void) {
-  // CHECK: ret i32 123
+  // CHECK: load i32, ptr @x
+  // CHECK-NOT: ret i32 123
   return *f();
 }
+
+int *k(void) {
+  return &y;
+}
+
+int l(void) {
+  // CHECK-NOT: load i32, ptr @y
+  // CHECK: ret i32 234
+  return *k();
+}