[PATCH] D42977: Don't set dso_local flag in LTO resolutions for absolute symbols defined in ELF objects

[Rafael Avila de Espindola via llvm-commits]

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