Page MenuHomePhabricator

[ELF] Only scan executables for shlib undefined symbols
ClosedPublic

Authored by smeenai on Dec 21 2017, 3:00 PM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

smeenai created this revision.Dec 21 2017, 3:00 PM
ruiu added a comment.Dec 25 2017, 11:27 PM

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.

ruiu accepted this revision.Dec 25 2017, 11:27 PM
This revision is now accepted and ready to land.Dec 25 2017, 11:27 PM
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.

This revision was automatically updated to reflect the committed changes.