[PATCH] D38239: [ELF] - Define linkerscript symbols early.

[Rafael Avila de Espindola via llvm-commits]

George Rimar <grimar at accesssoftek.com> writes:

> If we try to link them as is using regular LTO (I am assuming it has correct behavior),
> link will fail: 
>
> llvm-as first.ll -o first.o
> llvm-mc -filetype=obj -triple=x86_64-pc-linux second.s -o second.o
> ld.lld first.o second.o -o out
> ld.lld: error: duplicate symbol: bar
>
> It looks reasonable and valid behavior, right ?

Yes.

> Now if we use -defsym instead:
> ld.lld first.o -defsym=bar=11 -o out
> It links fine and result output contains ABS bar with value of 0xb.
>
> And it works only because defsym logic uses replaceSymbol<Defined>, so it replaces
> already existent Defined symbol bar (from BC) instead of reporting multiple declaration.
> If logic would use addDefined, it would fail.
> That probably means that -defsym still does not work as "this makes the linkerscript defined symbol
> just like .o defined symbols". And it is actually unclear for me why we should not just report error here.

True, the linker script symbols are still somewhat special. We have to
at least handling

foo = 41
foo += 1

The ability to replace existing symbols seems intentional and was
implemented in r291459 / D27276.


> Part II. "ThinLTO fix for defsym.ll"
> (patch for below is in attachment)
>
> Currently LTO sets WeakAnyLinkage for all symbols that are Prevailing and have LinkerRedefined flag set.
> I am not sure I understand why it is not done just for all LinkerRedefined symbols. 
> According to comment there "we want to switch the linkage to `weak` to prevent IPOs from happening",
> so dont we need to do that for all linker redefined symbols, like symbols used in -defsym ?
> Changing that here:
> (https://github.com/llvm-mirror/llvm/blob/master/lib/LTO/LTO.cpp#L717)
>
> stops `bar2` from being exported:
> https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/IPO/FunctionImport.cpp#L181
>
> and then LTO sets `bar2` linkage to internal here:
> https://github.com/llvm-mirror/llvm/blob/master/lib/LTO/LTO.cpp#L329
>
> So `bar2` never shows up and we have no multiple defenition error anymore.

Thanks a lot. I will take a look at the combined patch.

Cheers,
Rafael