[PATCH] D118491: [lld][ELF] support READONLY in linker script section

Sat Jan 29 08:20:34 PST 2022

bluca added a comment.

In D118491#3281374 <https://reviews.llvm.org/D118491#3281374>, @MaskRay wrote:

> In D118491#3281099 <https://reviews.llvm.org/D118491#3281099>, @bluca wrote:
>
>>> The main request is whether `READONLY` is necessary.
>>>
>>> I can send a patch to add `SHF_ALLOC` for an empty output section but I'd like to observe GNU ld's behavior more closely first.
>>
>> It is necessary because currently lld aborts with an error if it finds it in a script. This patch fixes the problem.
>
> `READONLY` papers over the problem and works for your case, but I don't think that is appropriate place to change.
>
> The problem is that with `INSERT [BEFORE|AFTER]`, the section order is fuzzy in `LinkerScript::adjustSectionsBeforeSorting` `if (isEmpty)`.
> I'll need to think how to support the case more appropriately.
>
> The high-level problem is whether `READONLY` should be added.
> In https://sourceware.org/pipermail/binutils/2021-May/116579.html I mentioned that it is not needed. 
> I keep my opinion that this is not needed to achieve your goal, even with the linker script I tend to grumble about.

It is not needed to make the section read-only in lld (it is needed for source-level compatibility of scripts, which is the reason I sent this patch, as lld curently aborts with an error when specifying a script that is valid for bfd), but is is needed in bfd. Without it the section is writable:

[ 3] .note.package     NOTE             00000000000002e8  000002e8

  0000000000000030  0000000000000000  WA       0     0     4

> In addition, `READONLY` should be orthogonal to SHF_ALLOC. I don't think `READONLY` should make a section `SHF_ALLOC`.

Sure, one doesn't imply the other, and my patches for lld don't do that, so it should be all good.

> If you maintain the relevant systemd package, I'd hope that you re-check whether it is needed. If it is needed, it more likely exposes a GNU ld issue which should be reported instead.

I do maintain them, and yes, it is needed. The build-id note gets the read-only flag as expected before/after the changes.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118491