[PATCH] D88877: [ELF] Drop --whole-archive before processing dependent libraries.

[ben via Phabricator via llvm-commits]

bd1976llvm added a comment.

In D88877#2331728 <https://reviews.llvm.org/D88877#2331728>, @ikudrin wrote:

> In D88877#2331416 <https://reviews.llvm.org/D88877#2331416>, @bd1976llvm wrote:
>
>> Firstly, is this a real problem?
>
> The problem is real. We have a report from our customer, so the issue is already observed in the wild.
>
>> In the RFC thread http://lists.llvm.org/pipermail/llvm-dev/2019-March/131105.html Rui said:
>>
>>> Then they can specify --no-whole-archive on the end of the command line, no?
>
> Unfortunately, sometimes that is not that simple as it sounds. In our case, the users who need autolinking are different from a unit that provides tools to adjust command lines. There is no option to add anything at the very end.

Thanks for supplying this detail. To me this sounds like you need to file a bug against the tools unit? I want to evaluate your proposal on it's own merits rather accepting it to work around problems caused by other tools.

>> I think we want to be sure that users can't simply adjust their linker command line before we complicate autolinking. Given that autolinking is a new feature on ELF it doesn't seem unfair that users should understand the feature and consider adjusting their linker command lines. Maybe we could add some diagnostics to help them.. e.g. lld could advise users that "--whole-archive was active" if a duplicate symbol error arises during auto-linking?
>
>
>
>> As to --whole-archive not being compatible with autolinking.. maybe the set of autolinked inputs should be deduplicated. This was discussed in the original RFC thread and would also solve https://bugs.llvm.org/show_bug.cgi?id=42460.
>
> Duplicate symbols are one part of the issue, the most noticeable because it prevents linking. Another, hidden part is that if the library is referenced only once, the linker fetches unneeded unreferenced members. It would be fair if the linker works in an expected way.
>
> As I understand the idea of the `--whole-archive` switch, it meant to instruct a linker which libraries should be included as a whole, not how you prefer the output to be linked. In the latter case that would be simple a global switch that would affects all input libraries.
>
> You know how they say, explicit is better than implicit. What can be less explicit than a switch in the command line which affects linking a library referenced far away from it, in a source file header? If there will ever be a use case for autolinking in a `--whole-archive` way, it will be much better to add an explicit option in the pragma itself or provide a distinct linker switch, something like `--whole-autolink-library`.

Currently, the mental model for auto-linking is very simple to reason about. LLD acts as if it didn't do auto-linking and its command line was "lld <options> <auto-linked inputs>". You want to change this to: "lld <options> --no-whole-archive <auto-linked inputs>". Whilst I agree that --whole-archive applying can be confusing I dislike having special cases. Looking at my notes from when I was thinking about auto-linking I identified these options...

L library path command line options
whole-archive
start-group, end-group
start-lib, end-lib
as-needed, https://gcc.gnu.org/ml/gcc-help/2010-12/msg00338.html and other shared object options.

...as needing special consideration w.r.t auto-linking (maybe there are others?). I wonder if it would be more consistent to reset all of these to their defaults before the auto-linked inputs are processed. Rather than adding special auto-link versions of command line options we could have one option --no-autolink-reset-cmdline (there must be a better name!) which reverts to the current behaviour (for powerusers that want to apply these options to auto-linking). OTOH perhaps order independent options e.g. your  --whole-autolink-library are better if there are build systems that are unable to ensure that options are appended to the "end" of the command line, are such build systems common?

Sorry, if this all sounds pedantic. A lot of effort went into making the current auto-linking feature ("dependent libraries") useful, minimal and simple to understand. I just want to make sure we have a good design before making changes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88877/new/

https://reviews.llvm.org/D88877