search for: weak_odr

...t 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...

...fdef 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 cra...

...#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, resu...

> I see. Using two comdats would still cause the same problem for us, > no? So the solution in the end is to emit: > > TU1: > -------------------------------- > @_ZN1UI1SE1kE = weak_odr constant i32 42, align 4, comdat _ZN1UI1SE1kE > @_ZGVN1UI1SE1kE = weak_odr global i64 1, comdat _ZN1UI1SE1kE > -------------------------------- > > TU2: > ----------------------------------- > @_ZN1UI1SE1kE = weak_odr global i32 0, align 4, comdat _ZN1UI1SE1kE > @_ZGVN1UI1SE1kE...

...or you put them in a single comdat > named for the variable.  It also doesn't actually help unless we disable > inlining. I see. Using two comdats would still cause the same problem for us, no? So the solution in the end is to emit: TU1: -------------------------------- @_ZN1UI1SE1kE = weak_odr constant i32 42, align 4, comdat _ZN1UI1SE1kE @_ZGVN1UI1SE1kE = weak_odr global i64 1, comdat _ZN1UI1SE1kE -------------------------------- TU2: ----------------------------------- @_ZN1UI1SE1kE = weak_odr global i32 0, align 4, comdat _ZN1UI1SE1kE @_ZGVN1UI1SE1kE = weak_odr global i64 0, comdat _...

...nded. So far we just create trivial COMDATs in codegen > for weak objects. > > We also need the IL linker itself needs to work on COMDATs too > otherwise this bug would still exist when doing LTO. > > In the "extended" example we would output > > @_ZN1UI1SE1kE = weak_odr constant i32 42, align 4, comdat _ZN1UI1SE1kE > > for TU1 and > > @_ZN1UI1SE1kE = weak_odr global i32 0, align 4, comdat _ZN1UI1SE1kE > ... > define internal void @_GLOBAL__I_a() nounwind section ".text.startup" > comdat _ZN1UI1SE1kE { > .... > } > > f...

...int f() { return U<S>::k; } #endif and it crashes with gcc too :-( The original testcase works because gcc folds the value into main even at -O0. If we are going to support it for real, I think Richard patch is going on the right direction, but we should completely drop the idea of a "weak_odr constant". If we cannot use it for instructing the linker to put it an o ro section, the "constant" gives us nothing that the "odr" doesn't. We could always convert a "weak_odr global" into a plain constant in LTO if the linker tells us we have all the parts....

...or COMDATs, but yes, it needs to be extended. So far we just create trivial COMDATs in codegen for weak objects. We also need the IL linker itself needs to work on COMDATs too otherwise this bug would still exist when doing LTO. In the "extended" example we would output @_ZN1UI1SE1kE = weak_odr constant i32 42, align 4, comdat _ZN1UI1SE1kE for TU1 and @_ZN1UI1SE1kE = weak_odr global i32 0, align 4, comdat _ZN1UI1SE1kE ... define internal void @_GLOBAL__I_a() nounwind section ".text.startup" comdat _ZN1UI1SE1kE { .... } for TU2. > Mach-O doesn't support anything like...

There should be no semantic difference between linkonce_odr and weak_odr, except that weak_odr is non-discardable. Why doesn't the autohide optimization work just as well on weak_odr + unnamed_addr as linkonce_odr + unnamed_addr? On Tue, Feb 6, 2018 at 5:35 PM, Steven Wu via llvm-dev < llvm-dev at lists.llvm.org> wrote: > Hi, > > I recently found th...

...; named for the variable. It also doesn't actually help unless we disable >> inlining. > > I see. Using two comdats would still cause the same problem for us, > no? So the solution in the end is to emit: > > TU1: > -------------------------------- > @_ZN1UI1SE1kE = weak_odr constant i32 42, align 4, comdat _ZN1UI1SE1kE > @_ZGVN1UI1SE1kE = weak_odr global i64 1, comdat _ZN1UI1SE1kE > -------------------------------- > > TU2: > ----------------------------------- > @_ZN1UI1SE1kE = weak_odr global i32 0, align 4, comdat _ZN1UI1SE1kE > @_ZGVN1UI1SE1k...

...original testcase works because gcc >> folds the value into main even at -O0. Indeed, I filed gcc bug#50968 for this last week. >> If we are going to support it for real, I think Richard patch is going >> on the right direction, but we should completely drop the idea of a "weak_odr >> constant". If we cannot use it for instructing the linker to put it an o ro >> section, the "constant" gives us nothing that the "odr" doesn't. Well, a weak_odr constant's value can be propagated in opt, which is an optimization we should strive to a...

On Nov 7, 2011, at 9:47 AM, Richard Smith wrote: >> In cases where the C++ standard requires static initialization, >> introducing a write violates the guarantees of the C++ standard for static >> initialization. Therefore, I'm not sure the whole "make the constant >> writable" approach is actually viable. > > There is another problem which afflicts

...; #endif > > and it crashes with gcc too :-( The original testcase works because > gcc folds the value into main even at -O0. > > If we are going to support it for real, I think Richard patch is going > on the right direction, but we should completely drop the idea of a > "weak_odr constant". If we cannot use it for instructing the linker to > put it an o ro section, the "constant" gives us nothing that the "odr" > doesn't. In cases where the C++ standard requires static initialization, introducing a write violates the guarantees of the C++...

Hi Richard, >>> and it crashes with gcc too :-( The original testcase works because gcc >>> folds the value into main even at -O0. > > Indeed, I filed gcc bug#50968 for this last week. are you sure this is the right gcc bug number? Ciao, Duncan.

...M, we tagged a global linkonce_odr and unnamed_addr to indicate to the linker can hide them from symbol table if they were picked (aka, linkonce_odr_auto_hide linkage). It is very commonly used for some type of Tables for c++ code in clang for example. However, thinLTO is promoting these symbols to weak_odr + unnamed_addr, which lose the property. As a result, it introduces unnecessary weak external symbols and weak external are not good for performance on darwin platforms. I have few proposed solutions for this issue but I don't know which one works the best for none macho platforms and other LT...

That is a good question and I don't know. The optimization is defined include/llvm/Analysis/ObjectUtils.h. If I enable that for weak_odr + unnamed_addr, no tests are failing so I guess it is a safe optimization? :) It is probably because the autohide optimization is targeted at c++ templates and inline functions and we know they have linkonce_odr linkage, which suggests whoever uses this symbol should have their own copy. Because t...

...he platforms. It's a much better out of the box experience anyway. On Wed, Feb 7, 2018 at 11:11 AM, Steven Wu <stevenwu at apple.com> wrote: > That is a good question and I don't know. The optimization is > defined include/llvm/Analysis/ObjectUtils.h. If I enable that for > weak_odr + unnamed_addr, no tests are failing so I guess it is a safe > optimization? :) > > It is probably because the autohide optimization is targeted at c++ > templates and inline functions and we know they have linkonce_odr linkage, > which suggests whoever uses this symbol should have t...

...much better out of the box experience anyway. > > On Wed, Feb 7, 2018 at 11:11 AM, Steven Wu <stevenwu at apple.com> wrote: > >> That is a good question and I don't know. The optimization is >> defined include/llvm/Analysis/ObjectUtils.h. If I enable that for >> weak_odr + unnamed_addr, no tests are failing so I guess it is a safe >> optimization? :) >> >> It is probably because the autohide optimization is targeted at c++ >> templates and inline functions and we know they have linkonce_odr linkage, >> which suggests whoever uses this...

Hi, I've had problem with a program using LLVM that tried to dynamic_cast objects created in the JIT execution engine, from the native part of the program (for the curious, the program is PinaVM http://gitorious.org/pinavm/pages/Home). I've narrowed down the issue to the linkage of weak_odr and linkonce_odr symbols, used for the vtables, and that _must_ be unique for dynamic_cast to work. Attached are two patches: the first adds a (failing) testcase, the second fixes the issue. I'm not familiar with patch submission on this list, let me know if there's a better way to submit...

...is a constant or a function (which means that the program cannot >> observe that the unique-address rule has been broken by writing to the >> global) >> >> >> When promoting from a linkonce_odr, it seems safe to me to *keep* the local_unnamed_addr >> on the weak_odr since we know the symbol was linkonce_odr in the first >> place. >> >> >> I don't think there is such guarantees. The description doesn't prevent >> local_unnamed_addr on other linkage types. If your assumption is correct, >> then the description can simp...