This is an archive of the discontinued LLVM Phabricator instance.

[ELF] For relative paths in INPUT() and GROUP(), search the directory of the current linker script before searching other paths
ClosedPublic

Authored by MaskRay on Apr 8 2020, 11:23 PM.

Details

Summary

For a relative path in INPUT() or GROUP(), this patch changes the search order by adding the directory of the current linker script.
The new search order (consistent with GNU ld >= 2.35 regarding the new test test/ELF/input-relative.s):

  1. the directory of the current linker script (GNU ld from Binutils 2.35 onwards; https://sourceware.org/bugzilla/show_bug.cgi?id=25806)
  2. the current working directory
  3. library paths (-L)

This behavior makes it convenient to replace a .so or .a with a linker script with additional input. For example, glibc

% cat /usr/lib/x86_64-linux-gnu/libm.a
/* GNU ld script
*/
OUTPUT_FORMAT(elf64-x86-64)
GROUP ( /usr/lib/x86_64-linux-gnu/libm-2.29.a /usr/lib/x86_64-linux-gnu/libmvec.a )

could be simplified as GROUP(libm-2.29.a libmvec.a).

Another example is to make libc++.a a linker script:

INPUT(libc++.a.1 libc++abi.a)

Diff Detail

Event Timeline

MaskRay created this revision.Apr 8 2020, 11:23 PM

😷 -> work from 🏠

The feature seems reasonable to me. It is a potentially breaking change for someone that had two libraries called libfoo.a.1 one in CWD and one adjacent to libfoo.a (where libfoo.a is a linker script with INPUT(libfooa.a.1)). As I understand it we'd prefer the libfoo.a.1 from the same directory as libfoo over the version in CWD. I think that in this case it is very unlikely so a release note entry and documentation (LinkerScript.rst?) would be sufficient.

Will be worth making sure that if BFD follow a slightly different route then we synchronize with them rather than gold as I think BFD is more widely used in practice.

Given it is Easter this week and people may be on holiday, will be worth waiting a bit for further comments, if we don't get any more I'll be happy to approve next week.

lld/ELF/ScriptParser.cpp
291

Now that the rules are getting a bit more complicated. It would be good to have a comment like Gold that explains them.

MaskRay updated this revision to Diff 258631.EditedApr 19 2020, 3:55 PM
MaskRay marked an inline comment as done.

Add comments.

Will be worth making sure that if BFD follow a slightly different route then we synchronize with them rather than gold as I think BFD is more widely used in practice.

Thanks for the advice. I'll try to make the linkers align on this feature. I filed a feature request (https://sourceware.org/bugzilla/show_bug.cgi?id=25806) to get binutils maintainers' attention... Just created a GNU ld patch by myself..

MaskRay updated this revision to Diff 258666.Apr 19 2020, 11:44 PM

Improve tests

MaskRay retitled this revision from [ELF] Find files relative to the current linker script for INPUT() and GROUP() to [ELF] For relative paths in INPUT() and GROUP(), search the directory of the current linker script before searching other paths.Apr 22 2020, 9:39 AM

Add comments.

Will be worth making sure that if BFD follow a slightly different route then we synchronize with them rather than gold as I think BFD is more widely used in practice.

Thanks for the advice. I'll try to make the linkers align on this feature. I filed a feature request (https://sourceware.org/bugzilla/show_bug.cgi?id=25806) to get binutils maintainers' attention... Just created a GNU ld patch by myself..

WoW, On GNU ld side, Nick applied my patch (https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=161719466ac9ea5f186514312f6bce842181804f). My mouthful description was lost, though...

psmith accepted this revision.Apr 22 2020, 11:43 AM

LGTM. Thanks for the updates and congratulations on getting the binutils patch accepted.

This revision is now accepted and ready to land.Apr 22 2020, 11:43 AM
MaskRay edited the summary of this revision. (Show Details)Apr 22 2020, 12:27 PM
MaskRay edited the summary of this revision. (Show Details)Apr 22 2020, 12:32 PM
This revision was automatically updated to reflect the committed changes.