[PATCH] D41113: [LLVMgold] Don't set resolutions for undefined symbols to 'Prevailing'

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D41113#954049, @evgeny777 wrote:

> >   I meant that the asm_undefined2.ll test should cause this assert without the fix in this patch 
>
> Without modification to gold-plugin.cpp assertion fires.


Great, that's what I was expecting. Go ahead and add the assert to this patch.

> I've added these lines of code:
> 
>   if (Res.Prevailing) {
>     assert(GlobalRes.IRName.empty() || !Sym.getIRName().empty());

I wonder if this can just be "assert(GlobalRes.IRName.empty())" in fact. Can you give that a try?

>   GlobalRes.IRName = Sym.getIRName();
> 
> }
> 
>   > Not sure what you mean about marking via llvm-lto2
>    
>    I meant that this assertion may also fire (and actually does) when you set symbol resolutions manually, which is possible to do using llvm-lto and llvm-lto2.
>    This is the reason why it breaks one of llvm-lto test cases.

Ah got it. No need to add a new test, the fact that the existing test would fail this new assertion without your fix is good enough.

Hoping pcc has a chance to look today. I'd like to get his thoughts on what should be happening here in the prevailing asm def case.


https://reviews.llvm.org/D41113