[cfe-dev] weak_odr constant versus weak_odr global

[Richard Smith]

On Wed, November 2, 2011 00:58, Eli Friedman wrote:
> On Tue, Nov 1, 2011 at 5:19 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>> [Adding cfe-dev to widen the net]
>>
>>
>> On Tue, November 1, 2011 23:19, Richard Smith wrote:
>>
>>> We recently narrowed down a clang regression to the following testcase:
>>>
>>>
>>> struct S { static const int x; }; template<typename T> struct U { static
>>> const int k; }; template<typename T> const int U<T>::k = T::x;
>>>
>>> #ifdef TU1
>>> extern int f(); const int S::x = 42; int main() { return f() + U<S>::k; }
>>> #endif
>>>
>>>
>>> #ifdef TU2
>>> int f() { return U<S>::k; } #endif
>>>
>>>
>>> /* Steps to repro:
>>>
>>>
>>> clang repro.cpp -DTU1 -c -o tu1.o clang repro.cpp -DTU2 -c -o tu2.o clang
>>> tu1.o tu2.o ./a.out
>>>
>>>
>>> */
>>>
>>>
>>> This segfaults, because... in TU1, we get:
>>>
>>>
>>> @_ZN1UI1SE1kE = weak_odr constant i32 42, align 4
>>>
>>>
>>> and in TU2, we get:
>>>
>>> @_ZN1UI1SE1kE = weak_odr global i32 0, align 4
>>>
>>>
>>> plus a global constructor which writes to @_ZN1UI1SE1kE. The linker then
>>> selects the symbol from TU1, which ends up in a read-only section,
>>> resulting in the binary crashing when TU2 tries to write to it.
>>>
>>> Is this a clang bug (should we be generating a weak_odr global, and
>>> losing optimization opportunities in TU1), or is this an llvm bug (should
>>> weak_odr constants be banned from read-only sections, since another module
>>> might write to them)?
>>
>> I think this is best fixed in llvm, since that gives maximum optimization
>> opportunities: it's fine to propagate weak_odr constant values, even if some
>>  other module has the same object as a non-constant. Plus, with LTO, we'd
>> want to merge the weak_odr constant and weak_odr global symbols into a
>> weak_odr constant (and simultaneously remove any writes into the global).
>
> Another solution you haven't mentioned is that we could make clang
> generate a guard variable initialized to 1 for all relevant static data members
> of class templates.  The solutions you mention will require that we can't put
> static data members of templates into a read-only section unless we can prove
> every translation unit will statically initialize the definition.

I think that's an excellent idea. However, we do not seem to be able to
provide a guarantee that the object and its guard variable will be taken from
the same translation unit, so I'm not sure that it can be made to work
reliably.

Richard