[PATCH] D42977: Don't set dso_local flag in LTO resolutions for absolute symbols defined in ELF objects
via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 6 15:47:36 PST 2018
I addressed the comments and updated the diff.
> On Feb 6, 2018, at 3:24 PM, Rafael Avila de Espindola <rafael.espindola at gmail.com> wrote:
>
> Dmitry Mikulin via Phabricator <reviews at reviews.llvm.org> writes:
>
>> Index: lld/test/ELF/lto/abs-resol.ll
>> ===================================================================
>> --- /dev/null
>> +++ lld/test/ELF/lto/abs-resol.ll
>> @@ -0,0 +1,15 @@
>> +; REQUIRES: x86
>> +
>> +; RUN: llvm-as %s -o %t.o
>> +; RUN: llvm-mc -triple=x86_64-pc-linux %p/Inputs/absolute.s -o %t2.o -filetype=obj
>> +; RUN: ld.lld %t.o %t2.o -o %t.out -pie
>> +
>> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
>> +target triple = "x86_64-unknown-linux-gnu"
>> +
>> + at blah = external global i8, align 1
>> +
>> +; Function Attrs: noinline nounwind optnone uwtable
>
> You can remove the above comment.
>
>> Index: lld/test/ELF/lto/Inputs/absolute.s
>> ===================================================================
>> --- /dev/null
>> +++ lld/test/ELF/lto/Inputs/absolute.s
>> @@ -0,0 +1,3 @@
>> +.globl blah
>> +.equ blah, 0xdeadbeef
>> +.type blah, at notype
>
> notype is the default I think. Most other tests use = instead of .equ.
>
>> Index: lld/ELF/LTO.cpp
>> ===================================================================
>> --- lld/ELF/LTO.cpp
>> +++ lld/ELF/LTO.cpp
>> @@ -154,8 +154,12 @@
>> R.VisibleToRegularObj = Config->Relocatable || Sym->IsUsedInRegularObj ||
>> (R.Prevailing && Sym->includeInDynsym()) ||
>> UsedStartStop.count(ObjSym.getSectionName());
>> + const auto *DR = dyn_cast<Defined>(Sym);
>> R.FinalDefinitionInLinkageUnit =
>> - Sym->isDefined() && (IsExecutable || Sym->Visibility != STV_DEFAULT);
>> + (IsExecutable || Sym->Visibility != STV_DEFAULT) && DR &&
>> + // Skip absolute symbols from ELF objects, otherwise PC-rel relocations
>> + // will be generated by for them, triggering linker errors.
>> + !(Sym->File && Sym->File->isElf() && DR->Section == nullptr);
>
> In the last part of the if it is not immediately clear why you need to
> check isElf. Experimenting with the patch it turns out the reason is that
> bitcode files always have a null section.
>
> Please update the comment to say that.
>
> LGTM with the above changes.
>
> Thanks,
> Rafael
>
More information about the llvm-commits
mailing list