Skip to content

Commit dd2362a

Browse files
committed
[clang] Allow const variables with weak attribute to be overridden
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 f49573d 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
1 parent 1257fe1 commit dd2362a

File tree

6 files changed

+103
-14
lines changed

6 files changed

+103
-14
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,9 @@ Attribute Changes in Clang
336336
builtins (corresponding to the specific names listed in the attribute) in the
337337
body of the function the attribute is on.
338338

339+
- When the ``weak`` attribute is applied to a const qualified variable clang no longer
340+
tells the backend it is allowed to optimize based on initializer value.
341+
339342
Windows Support
340343
---------------
341344

clang/include/clang/Basic/Attr.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2959,7 +2959,7 @@ def WarnUnusedResult : InheritableAttr {
29592959
def Weak : InheritableAttr {
29602960
let Spellings = [GCC<"weak">];
29612961
let Subjects = SubjectList<[Var, Function, CXXRecord]>;
2962-
let Documentation = [Undocumented];
2962+
let Documentation = [WeakDocs];
29632963
let SimpleHandler = 1;
29642964
}
29652965

clang/include/clang/Basic/AttrDocs.td

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6477,3 +6477,69 @@ is only supported in compute shaders.
64776477
The full documentation is available here: https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/sv-groupindex
64786478
}];
64796479
}
6480+
6481+
def WeakDocs : Documentation {
6482+
let Category = DocCatDecl;
6483+
let Content = [{
6484+
6485+
In supported output formats the ``weak`` attribute can be used to
6486+
specify that a variable or function should be emitted as a symbol with
6487+
``weak`` (if a definition) or ``extern_weak`` (if a declaration of an
6488+
external symbol) `linkage
6489+
<https://llvm.org/docs/LangRef.html#linkage-types>`_.
6490+
6491+
If there is a non-weak definition of the symbol the linker will select
6492+
that over the weak. They must have same type and alignment (variables
6493+
must also have the same size), but may have a different value.
6494+
6495+
If there are multiple weak definitions of same symbol, but no non-weak
6496+
definition, they should have same type, size, alignment and value, the
6497+
linker will select one of them (see also selectany_ attribute).
6498+
6499+
If the ``weak`` attribute is applied to a ``const`` qualified variable
6500+
definition that variable is no longer consider a compiletime constant
6501+
as its value can change during linking (or dynamic linking). This
6502+
means that it can e.g no longer be part of an initializer expression.
6503+
6504+
.. code-block:: c
6505+
6506+
const int ANSWER __attribute__ ((weak)) = 42;
6507+
6508+
/* This function may be replaced link-time */
6509+
__attribute__ ((weak)) void debug_log(const char *msg)
6510+
{
6511+
fprintf(stderr, "DEBUG: %s\n", msg);
6512+
}
6513+
6514+
int main(int argc, const char **argv)
6515+
{
6516+
debug_log ("Starting up...");
6517+
6518+
/* This may print something else than "6 * 7 = 42",
6519+
if there is a non-weak definition of "ANSWER" in
6520+
an object linked in */
6521+
printf("6 * 7 = %d\n", ANSWER);
6522+
6523+
return 0;
6524+
}
6525+
6526+
If an external declaration is marked weak and that symbol does not
6527+
exist during linking (possibly dynamic) the address of the symbol will
6528+
evaluate to NULL.
6529+
6530+
.. code-block:: c
6531+
6532+
void may_not_exist(void) __attribute__ ((weak));
6533+
6534+
int main(int argc, const char **argv)
6535+
{
6536+
if (may_not_exist) {
6537+
may_not_exist();
6538+
} else {
6539+
printf("Function did not exist\n");
6540+
}
6541+
return 0;
6542+
}
6543+
6544+
}];
6545+
}

clang/lib/CodeGen/CodeGenModule.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4899,12 +4899,8 @@ llvm::GlobalValue::LinkageTypes CodeGenModule::getLLVMLinkageForDeclarator(
48994899
if (Linkage == GVA_Internal)
49004900
return llvm::Function::InternalLinkage;
49014901

4902-
if (D->hasAttr<WeakAttr>()) {
4903-
if (IsConstantVariable)
4904-
return llvm::GlobalVariable::WeakODRLinkage;
4905-
else
4906-
return llvm::GlobalVariable::WeakAnyLinkage;
4907-
}
4902+
if (D->hasAttr<WeakAttr>())
4903+
return llvm::GlobalVariable::WeakAnyLinkage;
49084904

49094905
if (const auto *FD = D->getAsFunction())
49104906
if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally)

clang/test/CodeGen/global-init.c

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,20 @@ int a = 242;
99

1010
// This should get normal weak linkage.
1111
int c __attribute__((weak))= 0;
12-
// CHECK: @c = weak{{.*}} global i32 0
12+
// CHECK: @c = weak global i32 0
1313

14-
15-
// Since this is marked const, it should get weak_odr linkage, since all
16-
// definitions have to be the same.
17-
// CHECK: @d = weak_odr constant i32 0
14+
// Even though is marked const, it should get still get "weak"
15+
// linkage, not "weak_odr" as the weak attribute makes it possible
16+
// that there is a strong definition that changes the value linktime,
17+
// so the value must not be considered constant.
18+
// CHECK: @d = weak constant i32 0
1819
const int d __attribute__((weak))= 0;
1920

21+
// However, "selectany" is similar to "weak", but isn't interposable
22+
// by a strong definition, and should appear as weak_odr.
23+
// CHECK: @e = weak_odr constant i32 17
24+
const int e __attribute__((selectany)) = 17;
25+
2026
// PR6168 "too many undefs"
2127
struct ManyFields {
2228
int a;

clang/test/CodeGen/weak_constant.c

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,31 @@
11
// RUN: %clang_cc1 -w -emit-llvm %s -O1 -o - | FileCheck %s
2-
// Check for bug compatibility with gcc.
2+
// This used to "check for bug compatibility with gcc".
3+
// Now it checks that that the "weak" declaration makes the value
4+
// fully interposable whereas a "selectany" one is handled as constant
5+
// and propagated.
36

7+
// CHECK: @x = weak {{.*}}constant i32 123
48
const int x __attribute((weak)) = 123;
59

10+
// CHECK: @y = weak_odr {{.*}}constant i32 234
11+
const int y __attribute((selectany)) = 234;
12+
613
int* f(void) {
714
return &x;
815
}
916

1017
int g(void) {
11-
// CHECK: ret i32 123
18+
// CHECK: load i32, ptr @x
19+
// CHECK-NOT: ret i32 123
1220
return *f();
1321
}
22+
23+
int *k(void) {
24+
return &y;
25+
}
26+
27+
int l(void) {
28+
// CHECK-NOT: load i32, ptr @y
29+
// CHECK: ret i32 234
30+
return *k();
31+
}

0 commit comments

Comments
 (0)