[PATCH] D96914: [ELF] Add -z start-stop-gc to let __start_/__stop_ not retain C identifier name sections

Jessica Clarke via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 4 14:44:28 PDT 2021


jrtc27 added a comment.

In D96914#3105209 <https://reviews.llvm.org/D96914#3105209>, @MaskRay wrote:

> In D96914#3104999 <https://reviews.llvm.org/D96914#3104999>, @jrtc27 wrote:
>
>> In D96914#3104978 <https://reviews.llvm.org/D96914#3104978>, @MaskRay wrote:
>>
>>>> Not if you're in code that doesn't know what linker, let alone the version of it, is being used. If you don't use it for new LLD you break because your sections are bogusly GC'ed. If you use it for old LLD or BFD you break because the option doesn't exist and gives an error. Even Clang's driver can't know if the option is supported, so how do you expect other projects using similar code (like LDC) to do so? Just because you can detect the linker at toolchain build time doesn't mean you know what linker will be used at toolchain run time.
>>>
>>> I don't understand why this is a problem.
>>> The code doesn't need to dispatch on different behaviors.
>>> It just needs to be written in a way portable to pre-2015-10 GNU ld and current LLD.
>>>
>>> If it needs time for transition, `-Wl,-z,nostart-stop-gc` in a configure time detection.
>>
>> Which breaks if I build with LLD 12 around and then update to LLD 13. This is the exact same reason why Clang doesn't do configure-time detection for the exact linker version that will be used at run time (I forget if it even does any, but if so it's just taken to be a baseline, which is of no use here if it's pre-13).
>
> It's a pain either way. For metadata section users, for Linux, we probably could not flip the default as the default needs to work with very old GNU ld (typically 5+ years old).
> Once choice might to special case `-fuse-ld=lld`. Some groups even default to `ld` for lld and may not enjoy the size saving.
> The inconsistency between `-fuse-ld=lld` and others also has a cost.
>
>>>> Sure you can. If you can add SHF_GNU_RETAIN, you can add SHF_GNU_IT_IS_SAFE_TO_GC_ME. Or you can do a proper transition (see below).
>>>
>>> Since there is a section flag with the positive semantics, there is zero chance the flag with the negative semantics would be accepted.
>>> The name also looks irony, rather than a real proposal.
>>
>> Yes, the name is obviously not what you'd use. It was deliberately chosen to be extremely clear what it meant. Though I don't see what the problem with the semantics would be, force-yes/force-no/default is a pretty standard tri-state all over the place, having both flags would just encode that (albeit wasting the fourth possible state, but don't see how you can avoid that unless there's some other useful option).
>>
>>>> That is irrelevant. This is not about what libLLVM version it links against. This is about what the system linker is, which could be BFD, gold or LLD, and has zero connection to the libLLVM version used.
>>>
>>> My point is that every time they upgrade llvm-project, they already need to deal with changes.
>>> As experienced toolchain developers they are in a better position detecting and fixing the issues.
>>> If you find a https://github.com/ldc-developers/ldc issue, the more productive way is to open an issue there.
>>
>> Re-read what I said. "Every time they upgrade llvm-project" is irrelevant. It could be built against libLLVM13 or libLLVM3, it doesn't matter, that has zero bearing on what version /usr/bin/ld.lld is.
>
> Well, my point persists. ldc developers are in a better position fixing the problem. They even contacted me for investigating some LTO problems so they know whether to get help if needed.
>
>>>> This is not how you do a transition. You do a transition by adding the option, then _several years later_ flipping the default so that it can just be assumed to exist. You _can't_ add the option and flip the default _in the same release cycle_, that leads to a mess.
>>>
>>> Not for this case. For this case fixing early can make sure the whole ecosystem takes the least pain. Waiting larger would create a larger disconnection between metadata section users and packages potentially relying on the unfortunate GNU ld behavior.
>>
>> If it's so painful, why has metadata been using the scheme it does all these years? Surely it should've just picked a better design that was more efficient back then, and more efficient even today with GNU ld that still has the traditional behaviour? This really feels to me like solving the problem from the wrong end; you can't just come in and declare an old thing broken just so your new thing that you chose to work this way rather than some other way works better, you're supposed to design things appropriately based on the constraints that exist.
>
> Well, the encapsulation symbol design is quite the good. The only unfortunate thing was that GNU ld somehow broke it.
> I think the current model can serve us for many years from now on.

There's nothing fundamentally wrong with either model. The issue is when you change what the model is without providing a long enough transition period.

>>> FWIW I still don't see why it is a mess. It is an easy change dispatching on the availability of the option, (slightly worse) dispatching on LLD version.
>>
>> If it's so easy then please provide a robust patch for LDC that's acceptable to upstream and doesn't rely on brittle configure-time detection of LLD that ties it to the exact version of LLD in the environment it was built.
>
> I opened https://github.com/ldc-developers/ldc/issues/3861
> If you think there is a issue, please comment there.
>
> It seems that ldc places `__minfo` global variables in `llvm.used`, so what's the problem? Using llvm-project<13 library with ld.lld>=13.0.0?

Yes. The version of libLLVM used by LDC, and the version of LLD, are completely independent; I wouldn't even be surprised if they're different major versions more often than they're the same major version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96914



More information about the llvm-commits mailing list