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

Tue Oct 20 14:46:41 PDT 2020

bd1976llvm added a comment.

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

> Hmm, that starts looking like a discussion for the mailing list, but I guess that all interested people are already here and the conversation will be better preserved for further references if being linked to the review.
>
> In D88877#2332692 <https://reviews.llvm.org/D88877#2332692>, @bd1976llvm wrote:
>
>>> 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.
>
> And for the tool unit folks that looks like an issue in the linker. Why should they work around problems caused by the linker? Why not fix them in the source? After all, build systems are many and reasoning for fixing them is vague. Closing "--whole-archive" range is more like tidiness, which was not that strictly required before (or without) the autolinking feature.

I think I agree that build systems might not easily be able to arrange for appending options to the end of the linker command line. Therefore, for the "dependent libraries" feature (I changed the name in https://reviews.llvm.org/D60274 because autolinking was too general) it would be reasonable to reset any temporal options that the build system would otherwise have to handle by appending an argument to the very end of the command line (e.g. --whole-archives). Where appropriate we can create new order-independent options to apply the options that are reset to the dependent libraries. This seems like an improvement over what we have now.

>> 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>".
>
> The problem with the model is that it looks natural for very few people in the entire world because it just describes how the feature is implemented in LLD. I guess a developer that just uses the pragma would usually expect a library to be included in the link at the place where it is mentioned, i.e. when the object file that references it is processed.
>
>> You want to change this to: "lld <options> --no-whole-archive <auto-linked inputs>".
>
> Not exactly. I want auto-linked inputs to be processed independently of any temporal options of the command line.
>
>> 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.
>
> My point exactly. I prefer `LinkerDriver::createFiles()` to restore the temporal options to their default state.
>
>> 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?
>
> I would prefer not to add any special options for that. If some libraries needed to be treated in a special way, that should be mentioned in the `#pragma` and stored along with the name of the library.

I understand why you would want those features; however, I don't think they fit with dependent libraries. If you read the RFC you can see that we were deliberate in avoiding these complications. I think that it might be worth looking into completing the linker options scheme discussed here: http://lists.llvm.org/pipermail/llvm-dev/2019-March/131360.html.

>> 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.
>
> BTW, I have one more thing to consider...
>
> In D88877#2331416 <https://reviews.llvm.org/D88877#2331416>, @bd1976llvm wrote:
>
>> I also want to be certain that the GNU linkers can implement whatever we decide as that was as important design consideration.
>
> As far as I can remember, the GNU linkers process input files right when they encounter them in the linking process. They are unable to postpone processing the dependent libraries until after all the command line is processed, at least because they cannot resolve back references. Thus, they will not be able to mimic the approach with the state of temporal options as-it-would-be-at-the-end-of-the-command-line. Rather than, they most probably would use the state of the options at the moment a particular object file is processed, which would diverge from LLD and, most importantly, would be very hard to reproduce at LLD's side. Thus, the simplest common way to process dependent libraries would be to ignore the temporal state of temporal options.

Rui detailed how GNU linkers could support dependent libraries here: http://lists.llvm.org/pipermail/llvm-dev/2019-March/131186.html.

LGTM to this change. I'm happy to tackle the other range options as problems are encountered, rather than doing them all at once now. I'm also happy to add position independent options to apply range options to the dependent libraries later (as they are requested, via bug reports etc).

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

https://reviews.llvm.org/D88877