[rfc][gold plugin] Fix pr19901

[Nick Lewycky]

Rafael Espíndola wrote:
>>> I am going on vacations tonight, so I don't think I will have time to
>>> finish this until I get back (20th), but I wanted to send out a
>>> preview of the patch so that others can take a look.
>>>
>>> The fundamental issue with that bug is the api mismatch between gold
>>> and our own LTO api. The gold api talks about "symbol foo in file
>>> bar". Our api talks about "symbol foo in the merged module".
>>>
>>> In that particular testcase, what happens is that gold asks us to keep
>>> a particular linkonce_odr symbol, but the IR linker doesn't copy it to
>>> the merged module and we never have a chance to ask lib/LTO to keep
>>> it.
>>
>>
>> Why is that not the bug? The linker can't assume that all callers are
>> visible, just that it's merging two modules. A flag for the linker that
>> tells it that it can see the whole program and therefore may drop linkonce
>> symbols sounds like a nice optimization, but not correct in general.
>>
>
> I would call it a missing feature. It is not as bad as assuming a
> whole program. What the the current design of lib/Linker assumes is
> that each module in "independent" given the IR semantics. By
> independent I mean things like "no undefined reference to a
> linkonce_odr", which follows from the rule than an unused linkonce_odr
> can be dropped.

*sigh*

The LangRef doesn't describe what it means by "can be dropped if not 
used". Can we drop them in codegen if they aren't used? What if we split 
up the module into per-function pieces (say, for parallel codegen) then 
try to native link the resulting .o files? Now splitting up a module is 
not a safe operation, which in turn means that of course joining them 
isn't either?

If it were going to be "only used within this TU" then it's internal, 
right? So then linkonce_odr means "only used within this whole program"? 
Or "only used within this shared object"? Or what? Where's the point at 
which we can drop it? Or is it "only used within this TU, but multiple 
definitions aren't supposed to be an error"? That can't be right because 
then we wouldn't have references coming out of native ELF, right?


If we already drop unused linkonce_odr functions in codegen, then I'll 
accept that your approach is correct. Otherwise I'm sure it should be a 
change in the linker.

> What causes problems is the the plugin API expects us to be more
> flexible than that. It expects to be able to tell us to keep a
> linkonce_odr and use it from a native ELF file, effectively creating
> an undefined reference to a linkonce_odr. I think the best way to
> think about it is that during the linking process the modules can
> transit through invalid states (like having a linkonce_odr that must
> be kept) but the linker (gold+plugin, not lib/Linker) knows how to
> patch things up in the end.
>
> What this patch does is a small dance to make each module look
> individually valid. That is why it sometimes replaces linkonce_odr
> with weak_odr or an alias with a variable declaration. This is a bit
> of a hack, but is the least invasive change that fixes the bug and
> gets the test passing.
>
> With that in I then hope to add support for COMDATs and then make the
> lib/Linker interface more flexible. It should be possible to, for
> example, directly say that a linkonce_odr must be copied and that a
> given alias must not.
>
> Cheers,
> Rafael
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>