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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D118491#3281099 <https://reviews.llvm.org/D118491#3281099>, @bluca wrote:

> In D118491#3281093 <https://reviews.llvm.org/D118491#3281093>, @MaskRay wrote:
>
>> In D118491#3281073 <https://reviews.llvm.org/D118491#3281073>, @bluca wrote:
>>
>>> In D118491#3281023 <https://reviews.llvm.org/D118491#3281023>, @MaskRay wrote:
>>>
>>>> In D118491#3280965 <https://reviews.llvm.org/D118491#3280965>, @bluca wrote:
>>>>
>>>>> In D118491#3280927 <https://reviews.llvm.org/D118491#3280927>, @MaskRay wrote:
>>>>>
>>>>>>> bfd recently added a READONLY attribute for sections, as they are created writable by default.
>>>>>>
>>>>>> I think saying it writable is incorrect (see https://sourceware.org/bugzilla/show_bug.cgi?id=26378). See https://sourceware.org/pipermail/binutils/2021-May/116579.html
>>>>>>
>>>>>> I mentioned that `READONLY` is not needed but it seems that my comment has been ignored.
>>>>>
>>>>> There might be cases/combination where that is not the case, but with the provided linker script, before that patch, the note section was definitely created as writable by bfd, and there was no way to mark it read-only from the script.
>>>>
>>>> Sorry, I do not follow. `.note.package : ALIGN(4) { ... }` does not produce a writable output section in GNU ld.
>>>> That's the bug fixed by https://sourceware.org/bugzilla/show_bug.cgi?id=26378 (2020-09), before `READONLY` was added.
>>>
>>> Without readonly:
>>>
>>>   [ 3] .note.package     NOTE             00000000000002e8  000002e8
>>>        0000000000000030  0000000000000000  WA       0     0     4
>>>
>>>> If we want to customize both `sh_type` and `sh_flags`, I think the proper way is to add syntax to specify both.
>>>>
>>>> (I know that ld.lld does not set any flag if you don't have a regular input section. There are quite a few cases in GNU ld. It's just overwhelming to support every one.)
>>>
>>> That's what this patch does, no? It adds support for the same option.
>>
>> 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)`.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118491