Re: [PATCH] ld: Support customized output section type

[Fangrui Song <i@maskray.me>]

On 2022-02-03, Luca Boccassi wrote:
>On Wed, 2022-02-02 at 10:32 -0800, Fangrui Song wrote:
>> On 2022-02-02, Nick Clifton via Binutils wrote:
>> > Hi Fangrui,
>> >
>> > > The current output section type allows to set the ELF section type to
>> > > SHT_PROGBITS or SHT_NOLOAD. This patch allows an arbitrary section value
>> > > to be specified. Some ELF section type names are supported as well,
>> >
>> > Thanks for the patch submission.  I am regression testing it at the moment
>> > but in the meantime a couple of things stood out for me:
>> >
>> >
>> > > +@item TYPE = @var{type}
>> > > +Set the section type to the integer @var{type}. For the ELF output
>> > > +file, some type names (e.g. @code{SHT_NOTE}) are also allowed for
>> > > +@var{type}.
>> >
>> > Rather than having users guess, it would probably be best to list which
>> > type names are supported.
>> >
>> > Also it is probably worth documenting that it is the user's fault if they
>> > set the section type to something which has special semantics (eg SHT_GROUP)
>> > but then they do not also arrange for whatever necessary support that feature
>> > needs.  Something like: "it is the user's responsibility to ensure that
>> > any special requirements of the section type are met".
>>
>> Thanks for the quick review! Adopted the wording and the change below.
>> The new patch is in the attachment.
>>
>>
>> For SHT_GROUP, I think it is useful to support SHT_GROUP as well. I
>> actually did an experiment last night but SHT_GROUP led to an internal
>> error. There may be some issues that need to be fixed to use the
>> SHT_GROUP feature.
>>
>> >
>> >
>> > > +	    case type_section:
>> > > +	      if (os->sectype_value->type.node_class == etree_name
>> > > +		  && os->sectype_value->type.node_code == NAME)
>> > > +		{
>> > > +		  const char *name = os->sectype_value->name.name;
>> > > +		  if (strcmp (name, "SHT_PROGBITS") == 0)
>> > > +		    type = SHT_PROGBITS;
>> > > +		  else if (strcmp (name, "SHT_NOTE") == 0)
>> > > +		    type = SHT_NOTE;
>> > > +		  else if (strcmp (name, "SHT_NOBITS") == 0)
>> > > +		    type = SHT_NOBITS;
>> > > +		  else if (strcmp (name, "SHT_INIT_ARRAY") == 0)
>> > > +		    type = SHT_INIT_ARRAY;
>> > > +		  else if (strcmp (name, "SHT_FINI_ARRAY") == 0)
>> > > +		    type = SHT_FINI_ARRAY;
>> > > +		  else if (strcmp (name, "SHT_PREINIT_ARRAY") == 0)
>> > > +		    type = SHT_PREINIT_ARRAY;
>> > > +		  else
>> > > +		    einfo (_ ("%F%P: invalid type for output section `%s'\n"),
>> > > +			   os->name);
>> >
>> > It might be worth adding SHT_STRTAB to this list, as I can imagine some
>> > weird sceanario where someone would want it.
>>
>> Added.
>>
>> >
>> > Also - given that this is a new feature, there really ought to be an entry
>> > for it in the ld/NEWS file.
>>
>> Added. The NEW entry is for 2.39, but feel free to port it to 2.38 if
>> you think appropriate:)
>
>Hi,
>
>I tested this patch, it doesn't seem to allow combining multiple
>attributes. Tried both in the same brackets and separately, eg:
>
>.note.foo (READONLY) (TYPE=SHT_NOTE)
>
>.note.foo (READONLY TYPE=SHT_NOTE)
>
>Could you please send a new revision that fixes this (no opinion on the
>syntax), so that we can use it? Thanks!
>

It doesn't, as I am not sure the combination is useful.

---

I raised my concern twice when READONLY was added:

* https://sourceware.org/pipermail/binutils/2021-May/116579.html
* https://sourceware.org/pipermail/binutils/2021-July/117492.html

See also Michael Matz's https://sourceware.org/pipermail/binutils/2022-February/119598.html

"IOW: I think the introduction of READONLY was ill-advised :-/  But, ...  well :)"

In another place, you said

> As I have already shown, it is necessary. The note is read/write
> otherwise:
> 
> [ 3] .note.package NOTE 00000000000002e8 000002e8
> 
> 0000000000000030  0000000000000000  WA       0     0     4
> and that breaks processing the note from core files. It needs to be
> read/only, like the build-id, and hence the required change in bfd to
> make it possible to do so.

I believe none of Alan, Matz, and I can reproduce what you have seen.
(If Alan noticed it, he should have fixed it in https://sourceware.org/bugzilla/show_bug.cgi?id=26378#c11)
Can you give more instructions how your .note.package got the SHF_WRITE
flag? It looks like a separate ld bug, if your ld included the above change.