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

[Rafael Avila de Espindola via llvm-commits]

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

> smeenai added a comment.
>
> In https://reviews.llvm.org/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).

+1 for doing it in multiple patches.

> 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.

I would not aim for bug for bug compatibility. I think that giving an
error when the version script marks the symbol local is a more user
friendly behavior. The fact that bfd does it just makes it more likely
that we can do it without breaking compatibility.

Cheers,
Rafael