Fix LTO unreferenced symbol bug due to alias renaming

Rafael Espíndola via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 20 14:10:38 PST 2016


I just approved http://reviews.llvm.org/D16330 which is the same fix
but has a testcase.

Sorry for taking so long to reply, I was on vacations at the start of
the year and I am still catching up.

Cheers,
Rafael


On 19 January 2016 at 13:46, Yin Ma <yinma at codeaurora.org> wrote:
> 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> 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]
> 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 <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: 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
> <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
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>


More information about the llvm-commits mailing list