[PATCH] D41524: [ELF] Only scan executables for shlib undefined symbols

[Rafael Avila de Espindola via llvm-commits]

LGTM and I agree that producing an error like bfd would be a nice
improvement.

Thanks,
Rafael

Shoaib Meenai via Phabricator <reviews at reviews.llvm.org> writes:

> smeenai created this revision.
> smeenai added reviewers: ruiu, rafael.
> Herald added a subscriber: emaste.
>
> If using a version script with a `local: *` in it, symbols in shared
> libraries will still get default visibility if another shared library on
> the link line has an undefined reference to the symbol. This is quite
> surprising. Neither bfd nor gold have this behavior when linking a
> shared library, and none of LLD's tests fail without this behavior, so
> it seems safe to limit scanShlibUndefined to executables.
>
> As far as executables are concerned, gold doesn't do any automatic
> default visibility marking, and bfd issues a link error about a shared
> library having a reference to a hidden symbol rather than silently
> giving that symbol default visibility. I think bfd's behavior here is
> preferable to LLD's, but that's something to be considered in a
> follow-up.
>
>
> Repository:
>   rLLD LLVM Linker
>
> https://reviews.llvm.org/D41524
>
> Files:
>   ELF/Driver.cpp
>   test/ELF/Inputs/shlib-undefined-ref.s
>   test/ELF/shlib-undefined-shared.s
>
>
> Index: test/ELF/shlib-undefined-shared.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/shlib-undefined-shared.s
> @@ -0,0 +1,15 @@
> +# REQUIRES: x86
> +
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-linux-gnu -o %t1.o %S/Inputs/shlib-undefined-ref.s
> +# RUN: ld.lld -shared -o %t1.so %t1.o
> +
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-linux-gnu -o %t2.o %s
> +# RUN: echo "{ local: *; };" > %t.script
> +# RUN: ld.lld -shared -version-script %t.script -o %t2.so %t2.o %t1.so
> +# RUN: llvm-nm -g %t2.so | FileCheck -allow-empty %s
> +
> +# CHECK-NOT: should_not_be_exported
> +
> +.globl should_not_be_exported
> +should_not_be_exported:
> +	ret
> Index: test/ELF/Inputs/shlib-undefined-ref.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/Inputs/shlib-undefined-ref.s
> @@ -0,0 +1,4 @@
> +.globl f
> +f:
> +	call	should_not_be_exported at PLT
> +	ret
> Index: ELF/Driver.cpp
> ===================================================================
> --- ELF/Driver.cpp
> +++ ELF/Driver.cpp
> @@ -1043,7 +1043,8 @@
>      return;
>  
>    // Handle undefined symbols in DSOs.
> -  Symtab->scanShlibUndefined<ELFT>();
> +  if (!Config->Shared)
> +    Symtab->scanShlibUndefined<ELFT>();
>  
>    // Handle the -exclude-libs option.
>    if (Args.hasArg(OPT_exclude_libs))
>
>
> Index: test/ELF/shlib-undefined-shared.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/shlib-undefined-shared.s
> @@ -0,0 +1,15 @@
> +# REQUIRES: x86
> +
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-linux-gnu -o %t1.o %S/Inputs/shlib-undefined-ref.s
> +# RUN: ld.lld -shared -o %t1.so %t1.o
> +
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-linux-gnu -o %t2.o %s
> +# RUN: echo "{ local: *; };" > %t.script
> +# RUN: ld.lld -shared -version-script %t.script -o %t2.so %t2.o %t1.so
> +# RUN: llvm-nm -g %t2.so | FileCheck -allow-empty %s
> +
> +# CHECK-NOT: should_not_be_exported
> +
> +.globl should_not_be_exported
> +should_not_be_exported:
> +	ret
> Index: test/ELF/Inputs/shlib-undefined-ref.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/Inputs/shlib-undefined-ref.s
> @@ -0,0 +1,4 @@
> +.globl f
> +f:
> +	call	should_not_be_exported at PLT
> +	ret
> Index: ELF/Driver.cpp
> ===================================================================
> --- ELF/Driver.cpp
> +++ ELF/Driver.cpp
> @@ -1043,7 +1043,8 @@
>      return;
>  
>    // Handle undefined symbols in DSOs.
> -  Symtab->scanShlibUndefined<ELFT>();
> +  if (!Config->Shared)
> +    Symtab->scanShlibUndefined<ELFT>();
>  
>    // Handle the -exclude-libs option.
>    if (Args.hasArg(OPT_exclude_libs))