[cfe-dev] weak_odr constant versus weak_odr global

[Eli Friedman]

On Tue, Nov 1, 2011 at 5:19 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> Hi
>
> [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.

-Eli