You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Warn when unique objects might be duplicated in shared libraries (#117622)
When a hidden object is built into multiple shared libraries, each
instance of the library will get its own copy. If
the object was supposed to be globally unique (e.g. a global variable or
static data member), this can cause very subtle bugs.
An object might be incorrectly duplicated if it:
- Is defined in a header (so it might appear in multiple TUs), and
- Has external linkage (otherwise it's supposed to be duplicated), and
- Has hidden visibility (or else the dynamic linker will handle it)
The duplication is only a problem semantically if one of the following
is true:
1. The object is mutable (the copies won't be in sync), or
2. Its initialization has side effects (it may now run more than once),
or
3. The value of its address is used (different copies have different
addresses).
To detect this, we add a new -Wunique-object-duplication warning. It
warns on cases (1) and (2) above. To be conservative, we only warn in
case (2) if we are certain the initializer has side effects, and we
don't warn on `new` because the only side effect is some extra memory
usage.
We don't currently warn on case (3) because doing so is prone to false
positives: there are many reasons for taking the address which aren't
inherently problematic (e.g. passing to a function that expects a
pointer). We only run into problems if the code inspects the value of
the address.
The check is currently disabled for windows, which uses its own analogue
of visibility (declimport/declexport). The check is also disabled inside
templates, since it can give false positives if a template is never
instantiated.
### Resolving the warning
The warning can be fixed in several ways:
- If the object in question doesn't need to be mutable, it should be
made const. Note that the variable must be completely immutable, e.g.
we'll warn on `const int* p` because the pointer itself is mutable. To
silence the warning, it should instead be `const int* const p`.
- If the object must be mutable, it (or the enclosing function, in the
case of static local variables) should be made visible using
`__attribute((visibility("default")))`
- If the object is supposed to be duplicated, it should be be given
internal linkage.
### Testing
I've tested the warning by running it on clang itself, as well as on
chromium. Compiling clang resulted in [10 warnings across 6
files](https://github.com/user-attachments/files/17908069/clang-warnings.txt),
while Chromium resulted in [160 warnings across 85
files](https://github.com/user-attachments/files/17908072/chromium-warnings.txt),
mostly in third-party code. Almost all warnings were due to mutable
variables.
I evaluated the warnings by manual inspection. I believe all the
resulting warnings are true positives, i.e. they represent
potentially-problematic code where duplication might cause a problem.
For the clang warnings, I also validated them by either adding `const`
or visibility annotations as appropriate.
### Limitations
I am aware of four main limitations with the current warning:
1. We do not warn when the address of a duplicated object is taken,
since doing so is prone to false positives. I'm hopeful that we can
create a refined version in the future, however.
2. We only warn for side-effectful initialization if we are certain side
effects exist. Warning on potential side effects produced a huge number
of false positives; I don't expect there's much that can be done about
this in modern C++ code bases, since proving a lack of side effects is
difficult.
3. Windows uses a different system (declexport/import) instead of
visibility. From manual testing, it seems to behave analogously to the
visibility system for the purposes of this warning, but to keep things
simple the warning is disabled on windows for now.
4. We don't warn on code inside templates. This is unfortuate, since it
masks many real issues, e.g. a templated variable which is implicitly
instantiated the same way in multiple TUs should be globally unique, but
may accidentally be duplicated. Unfortunately, we found some potential
false positives during testing that caused us to disable the warning for
now.
staticint disallowedStatic1 = 0; // hidden-warning {{'disallowedStatic1' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
21
+
// Initialization might run more than once
22
+
staticconstdouble disallowedStatic2 = disallowedStatic1++; // hidden-warning {{initializeation of 'disallowedStatic2' may run twice when built into a shared library: it has hidden visibility and external linkage}}
23
+
24
+
// OK, because immutable and compile-time-initialized
staticint disallowedStatic1 = 3; // hidden-warning {{'disallowedStatic1' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
62
+
// expected-warning@-1 {{'disallowedStatic1' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
63
+
{
64
+
staticint disallowedStatic2 = 3; // hidden-warning {{'disallowedStatic2' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
65
+
// expected-warning@-1 {{'disallowedStatic2' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
66
+
}
67
+
68
+
auto lmb = []() {
69
+
staticint disallowedStatic3 = 3; // hidden-warning {{'disallowedStatic3' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
70
+
// expected-warning@-1 {{'disallowedStatic3' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
71
+
};
72
+
}
73
+
74
+
DEFAULT voidstatic_local_never_hidden() {
75
+
staticint allowedStatic1 = 3;
76
+
77
+
{
78
+
staticint allowedStatic2 = 3;
79
+
}
80
+
81
+
auto lmb = []() {
82
+
staticint allowedStatic3 = 3;
83
+
};
84
+
}
85
+
86
+
// Don't warn on this because it's not in a function
externint allowedAddressExtern; // Not a definition
91
+
}
92
+
93
+
inlinevoidhas_regular_local() {
94
+
int allowedAddressLocal = 0;
95
+
}
96
+
97
+
inlinevoidhas_thread_local() {
98
+
// thread_local variables are static by default
99
+
thread_localint disallowedThreadLocal = 0; // hidden-warning {{'disallowedThreadLocal' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
inlinefloat disallowedGlobal1 = 3.14; // hidden-warning {{'disallowedGlobal1' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
110
+
111
+
// Initialization might run more than once
112
+
inlineconstdouble disallowedGlobal5 = disallowedGlobal1++; // hidden-warning {{initializeation of 'disallowedGlobal5' may run twice when built into a shared library: it has hidden visibility and external linkage}}
113
+
114
+
// OK because internal linkage, so duplication is intended
// OK, because immutable and compile-time-initialized
121
+
constexprint allowedGlobal5 = 0;
122
+
constfloat allowedGlobal6 = 1;
123
+
constexprint allowedGlobal7 = init_constexpr(2);
124
+
constint allowedGlobal8 = init_constexpr(3);
125
+
126
+
// We don't warn on this because non-inline variables can't (legally) appear
127
+
// in more than one TU.
128
+
float allowedGlobal9 = 3.14;
129
+
130
+
// Pointers need to be double-const-qualified
131
+
inlinefloat& nonConstReference = disallowedGlobal1; // hidden-warning {{'nonConstReference' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
132
+
constinlineint& constReference = allowedGlobal5;
133
+
134
+
inlineint* nonConstPointerToNonConst = nullptr; // hidden-warning {{'nonConstPointerToNonConst' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
135
+
inlineintconst* nonConstPointerToConst = nullptr; // hidden-warning {{'nonConstPointerToConst' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
136
+
inlineint* const constPointerToNonConst = nullptr; // hidden-warning {{'constPointerToNonConst' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
inlineintconst * const ** const * const nestedNonConstPointer = nullptr; // hidden-warning {{'nestedNonConstPointer' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
143
+
144
+
structTest {
145
+
staticinlinefloat disallowedStaticMember1; // hidden-warning {{'disallowedStaticMember1' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
146
+
// Defined below, in the header file
147
+
staticfloat disallowedStaticMember2;
148
+
// Defined in the cpp file, so won't get duplicated
149
+
staticfloat allowedStaticMember1;
150
+
151
+
// Tests here are sparse because the AddrTest case below will define plenty
152
+
// more, which aren't problematic to define (because they're immutable), but
153
+
// may still cause problems if their address is taken.
154
+
};
155
+
156
+
inlinefloat Test::disallowedStaticMember2 = 2.3; // hidden-warning {{'disallowedStaticMember2' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
0 commit comments