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

Fri Nov 12 11:43:29 PST 2021

jrtc27 added a comment.

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

> In D96914#3127982 <https://reviews.llvm.org/D96914#3127982>, @tstellar wrote:
>
>> @MaskRay To be clear, I'm talking about reverting 6d2d3bd0a61f5fc7fd9f61f48bc30e9ca77cc619 <https://reviews.llvm.org/rG6d2d3bd0a61f5fc7fd9f61f48bc30e9ca77cc619> not this patch.  Again, I understand your motivations for keeping the default as-is, but we don't have consensus on this change and the policy for this project is to revert until a consensus can be reached.  Also, the fact the this patch was approved on the condition that the default would stay the same, but then the default was changed without discussion is a pretty strong reason to revert.  Can we please revert 6d2d3bd0a61f5fc7fd9f61f48bc30e9ca77cc619 <https://reviews.llvm.org/rG6d2d3bd0a61f5fc7fd9f61f48bc30e9ca77cc619> in trunk and continue the discussions in this thread?
>>
>> @jrtc27 @hvdijk  Do you have a suggested timeline for transitioning to the new default?
>
> @tstellar I know you are talking about 6d2d3bd0a61f5fc7fd9f61f48bc30e9ca77cc619 <https://reviews.llvm.org/rG6d2d3bd0a61f5fc7fd9f61f48bc30e9ca77cc619>, but I am afraid you may have missed some important discussions, e.g. https://bugs.llvm.org/show_bug.cgi?id=52384#c1
> https://reviews.llvm.org/D96914#3115988 and https://reviews.llvm.org/D96914#3117455 .
> I'd appreciate if others can read them first.
> In particular, I have mentioned that this default switch been discussed with several groups, in the absence of jrtc27's agreement (who isn't a regular reviewer for lld/ELF code).

I'm a regular contributor, pretty familiar with the code base, knowledgeable about ELF and reviewed this specific patch. There is no need to attempt to undermine me like that.

> I don't think I necessarily agree with "revert until a consensus can be reached" if beyond reasonableness. The LLVM 17 release schedule proposed by @jrtc27 would just cause more trouble to FreeBSD if more newer software relies on the unfortunate GNU ld>=2015-10 behavior,

So spit out a warning that the default is going to change if we detect --gc-sections + __start_/__stop_ symbols + no explicit -z (no)start-stop-gc? That's probably a good idea to do regardless.

> even if it might make few LLVM IR code generation software (ldc which has a pending path out by its maintainer: https://github.com/ldc-developers/ldc/issues/3861) happy.

Again, even if it newer versions of LDC get "fixed" (which they will anyway by virtue of using LLVM 13 for CodeGen), this does not "fix" older versions of LDC that exist in the wild.

> I have repeatedly mentioned that delaying the switch will just cause more problems for distro like FreeBSD. I hope you can seek advice from @emaste and @dim for FreeBSD matter.
> Let me emphasize again that distro-default `--gc-sections` isn't a common thing.

We don't have a distro-default --gc-sections. The software affected is software that uses --gc-sections, or static libraries that are linked into binaries from a different source that use it.

> On one hand, I am glad that LLD is so widely used; on the other hand, a decision may not be possible to make every group happy and we need to make right call.
> Please don't overly emphasize a very rare thing just to make all other important use cases suffer.

I would argue both are currently important, and that not breaking existing commonplace software is more important than optimising for a newer use case.

> I can accept @hvdijk's "enabling -z start-stop-gc behavior for LLD 14" timeline, which only means a revert on release/13.x, without touching main.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96914