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

[Fangrui Song <i@maskray.me>]

On 2022-02-03, Luca Boccassi wrote:
>On Thu, 2022-02-03 at 11:46 -0800, Fangrui Song wrote:
>> 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 understand you don't like it, but as I've already explained again,
>and again, and again, it is needed, and frankly I'm getting exhausted.
>If you don't want to update your patch that's fine, it's no problem at
>all, I'll send a follow-up myself if it gets merged.

What I repeatedly requested is how you got SHF_WRITE .note.package but I
never never get an example. You may understand how I got exausted after
my two questions in 2021 were ignored and the recent
https://sourceware.org/pipermail/binutils/2022-January/119537.html got a
reply "it is used" with no detail.