[ELF] Only scan executables for shlib undefined symbols

[smeenai]

Summary

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.

[ruiu]

LGTM, but please also fix scanShlibUndefined so that it doesn't automatically make symbols visible if they are referenced by DSOs. That seems like a root cause of the issue.

[smeenai]

In D41524#963992, @ruiu wrote:

LGTM, but please also fix scanShlibUndefined so that it doesn't automatically make symbols visible if they are referenced by DSOs. That seems like a root cause of the issue.

As @rafael said, that's the purpose of scanShlibUndefined. Do you want behavior like bfd's, where the linker gives an error instead of automatically marking symbols visible?

I experimented a bit, and bfd's current behavior is:

• Shared libraries are only scanned when building an executable (not when building another shared library).
• If a shared library undefined symbol is not present in the executable's dynamic list, the symbol is automatically added to the dynamic symbol table.
• If a shared library undefined symbol is explicitly marked local in the executable's version script, the linker gives an error.
• If a shared library undefined symbol is implicitly marked local in the exectuable's version script as a result of a local: *, the linker gives an error.

LLD's current behavior (from memory, but I believe this to be accurate):

• Shared libraries are scanned both when building executables and shared libraries.
• If a shared library undefined symbol is not present in the executable's dynamic list, the symbol is automatically added to the dynamic symbol table.
• If a shared library undefined symbol is explicitly marked local in the executable's version script, the symbol is not added to the dynamic symbol table and the linker gives no error.
• If a shared library undefined symbol is implicitly marked local in the exectuable's version script as a result of a local: *, the symbol is silently added to the dynamic symbol table and the linker gives no error.

This patch fixes the first bullet point. I do want to address the other differences too, but I'd like to do so in a follow-up, since it requires a bit more work (in particular fixing the discrepancy between explicitly marking a symbol local and implicitly marking it local via local: * will require moving the call to scanShlibUndefined later).

Do you think it's worth emulating bfd's behavior difference for dynamic lists vs. version scripts? I believe LLD currently treats the two as essentially equivalent for executables, so this would be a bit of a departure from that.

lld/trunk/ELF/Driver.cpp

template <class ELFT> void LinkerDriver::link(opt::InputArgList &Args) {
   // few linker-synthesized ones will be added to the symbol table.
   Symtab->fetchIfLazy<ELFT>(Config->Entry);
 
   // Return if there were name resolution errors.
   if (errorCount())
     return;
 
   // Handle undefined symbols in DSOs.
+  if (!Config->Shared)
     Symtab->scanShlibUndefined<ELFT>();
 
   // Handle the -exclude-libs option.
   if (Args.hasArg(OPT_exclude_libs))
     excludeLibs<ELFT>(Args, Files);
 
   // Create ElfHeader early. We need a dummy section in
   // addReservedSymbols to mark the created symbols as not absolute.
   Out::ElfHeader = make<OutputSection>("", 0, SHF_ALLOC);

lld/trunk/test/ELF/Inputs/shlib-undefined-ref.s

+.globl f
+f:
+  call  should_not_be_exported@PLT
+  ret

lld/trunk/test/ELF/shlib-undefined-shared.s

+# 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