Fix LTO unreferenced symbol bug due to alias renaming

[Yin Ma via llvm-commits]

Hello,

 

Any update on this review?

 

Thanks,

 

Yin 

 

 

From: mehdi.amini at apple.com [mailto:mehdi.amini at apple.com] 
Sent: Friday, January 08, 2016 8:30 AM
To: Yin Ma
Cc: llvm-commits
Subject: Re: Fix LTO unreferenced symbol bug due to alias renaming

 

Hi,

 

Let’s wait Monday, Rafael should be back, it may be easier for him to come up with a simpler test (or he may just take the patch without any tests, we’ll see).

 

— 

Mehdi

 

On Jan 7, 2016, at 5:20 PM, Yin Ma <yinma at codeaurora.org <mailto:yinma at codeaurora.org> > wrote:

 

Hi Mehdi,

 

It is not straight forward to reduce it. If it is just 

Extern foo from 1.ll

Define foo from 2.ll

Extern foo from 3.ll

It wouldn’t show the problem. The definition of foo have to be recorded by

scanning other codes before the definition is resolved. 

 

The current test case is about 1000 lines with a lot of 

c++ setup. It has to be

in three files like

llvm-link 1.ll 2.ll 3.ll –o lto.o

llvm-nm lto.o

it shows 

               U _ZN7cOfooD2Ev

-------- T _ZN7cOfooD2Ev.1

Which later causes _ZN7cOfooD2Ev unreferenced. Because the definition of 

_ZN7cOfooD2Ev becomes ZN7cOfooD2Ev.1

 

_ZN7cOfooD2Ev is renamed in copyGlobalValueProto(SGV, ShouldLink);

Because there is already a value with the same name when 1.ll is processed.

 

I hope you could come up a test case based on my description if you are

familiar with what IRMover.cpp is doing. Or you have an automatic tool to 

have me to reduce it further.

 

Thanks,

 

Yin 

 

 

From: mehdi.amini at apple.com <mailto:mehdi.amini at apple.com>  [mailto:mehdi.amini at apple.com] 
Sent: Thursday, January 07, 2016 4:39 PM
To: Yin Ma
Cc: llvm-commits
Subject: Re: Fix LTO unreferenced symbol bug due to alias renaming

 

Hi,

 

I’d rather have a test case with it. 

Is it hard to “anonymize” the test case? I don’t expect it to be large considering your description of the problem, you should be able to discard almost all the body of all function, strip the metadata, strip the function parameter, strip the custom types if any, and rename the few remaining function to foo and bar?

 

— 

Mehdi

 

 

On Jan 7, 2016, at 4:29 PM, Yin Ma < <mailto:yinma at codeaurora.org> yinma at codeaurora.org> wrote:

 

Hi Mehdi,

 

I have developed a test case for llvm-link. However, I just figure out

The test case was developed from licensed code. I cannot released

to the public. Could you review my change without test case.

 

Thanks,

 

Yin 

 

 

From:  <mailto:mehdi.amini at apple.com> mehdi.amini at apple.com [ <mailto:mehdi.amini at apple.com> mailto:mehdi.amini at apple.com] 
Sent: Thursday, January 07, 2016 1:34 PM
To: Yin Ma
Cc: llvm-commits
Subject: Re: Fix LTO unreferenced symbol bug due to alias renaming

 

Hi Yin,

 

Do you have a test-case for that?

 

Thanks,

 

Mehdi

 

On Jan 7, 2016, at 12:28 PM, Yin Ma via llvm-commits < <mailto:llvm-commits at lists.llvm.org> llvm-commits at lists.llvm.org> wrote:

 

Hi, 

 

After 18f201b4db339145af8f340196943ef57b9c8d9f from Rafael was merged in, we are experiencing 

unreferenced symbol in several LTO programs. I analyzed the problem and find because 18f201 patch 

tries to discard the aliasee by renaming it. However, it will create a problem

If the renaming happens for external functions, where non byte code may reference to the original

name and cause unreferenced symbol at the end.

 

So, we have to perform forceRenaming like before without 18f201 for those symbols in ShouldLink.

Like this to force those global symbols which may expose to other module to be the original name.

 

diff --git a/lib/Linker/IRMover.cpp b/lib/Linker/IRMover.cpp

index 16de0ec..68c3346 100644

--- a/lib/Linker/IRMover.cpp

+++ b/lib/Linker/IRMover.cpp

@@ -1077,7 +1077,7 @@ Constant *IRLinker::linkGlobalValueProto(GlobalValue *SGV, bool ForAlias) {

       return nullptr;

     NewGV = copyGlobalValueProto(SGV, ShouldLink);

-    if (!ForAlias)

+    if (ShouldLink || !ForAlias)

       forceRenaming(NewGV, SGV->getName());

   }

   if (ShouldLink || ForAlias) {

 

Please review.

 

Thanks,

 

Yin 

 

 

<i.diff>_______________________________________________
llvm-commits mailing list
 <mailto:llvm-commits at lists.llvm.org> llvm-commits at lists.llvm.org
 <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160119/669c7d97/attachment.html>