[PATCH] D37059: [ELF] - LTO: do not optimize away symbols accessed from linkerscript.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/LTO.cpp:170
       undefine(Sym);
     R.LinkerRedefined = Config->RenamedSymbols.count(Sym);
   }
----------------
pcc wrote:
> I think we should be setting `LinkerRedefined` to true for these symbols instead of `VisibleToRegularObj`, otherwise the optimizer may IPO past them.
I am not really familar with LTO logic, but if that is true, that sounds like a bug of LTO for me. 
May be I am missing something, but if linker says symbols should be visible (and that means used) from regular object,
I would expect LTO should not eliminate them out.

But actually I think it does not do that:
I looked how both `VisibleToRegularObj` and `LinkerRedefined` are used. They set symbol resolution to
GlobalResolution::External:
https://github.com/llvm-mirror/llvm/blob/master/lib/LTO/LTO.cpp#L426

And that prevents making them internal here:
https://github.com/llvm-mirror/llvm/blob/master/lib/LTO/LTO.cpp#L812

After all when parsing LTO objects and symbols:
https://github.com/llvm-mirror/lld/blob/master/ELF/SymbolTable.cpp#L125
there is no `bar` symbol at all (because `VisibleToRegularObj` was not set for it,
but `foo` is still there). That probably shows that logic is correct.
What am I missing ?



https://reviews.llvm.org/D37059