[PATCH] D107747: [ELF] Don't emit SHF_GNU_RETAIN on Solaris

Tue Aug 10 05:40:17 PDT 2021

ro added a comment.

In D107747#2934864 <https://reviews.llvm.org/D107747#2934864>, @tmatheson wrote:

> According to D95749 <https://reviews.llvm.org/D95749>, the rationale for allowing `ELFOSABI_NONE` with `SHF_GNU_RETAIN` is to keep consistent behaviour with `STT_GNU_IFUNC` and `STB_GNU_UNIQUE`, which are permitted under `ELFOSABI_NONE`. @MaskRay Should we perhaps revisit that decision?
>
> More details at https://sourceware.org/bugzilla/show_bug.cgi?id=27282

Thanks for the background.

> This doesn't seem like the right fix. I would suggest either:
>
> 1. Add `SHF_SUNW_NODISCARD` to flags instead of `SHF_GNU_RETAIN` if `Retain` is set and the target is Solaris (if they do in fact have the same semantics)
> 2. `Retain` is set based on `llvm.compiler.used` (D97448 <https://reviews.llvm.org/D97448>). If this behaviour is not wanted then set `Retain = Used.count(...) && !Solaris`.
> 3. Add a support check function as you describe here:
>
>> For full generality, any code emitting extensions beyond the ELF gABI should check that all of the assembler, linker, and runtime linker used support it.

I'm quite wary to try 3), in particular at this point in time: the `release/13.x` branch is affected by the same issue, and I'd like to unbreak the release in time.  The larger/more complex a fix is, the less likely it's allowed to get in.  So my goal would be a minimal fix right now.

However, I believe either 2) (which is semantically equivalent to my current patch) or 1) (which may require a bit more legwork to make the new section flag known) are doable and acceptable for 13.x.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107747