Re: [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag

["H.J. Lu" <hjl.tools@gmail.com>]

On Thu, Oct 1, 2020 at 12:22 PM Fangrui Song <i@maskray.me> wrote:
>
> On 2020-09-30, Jozef Lawrynowicz wrote:
> >On Tue, Sep 29, 2020 at 05:10:09PM -0700, Roland McGrath wrote:
> >> Since group semantics mean that if any section in a group is retained the
> >> whole group must be retained, it seems to me that making the feature a
> >> group flag rather than a section flag makes sense. I don't think it really
> >> has any downside. It's a little bit more structure to generate a group but
> >> everything already knows how to do it so that should be no problem.  While
> >> it is true that multiple sections with the same name and different
> >> attributes should already be fine, the place where multiple different
> >> sections of the same name are usually seen today is when each different
> >> section reusing a name is in a different group from the other sections with
> >> the same name.
> >>
> >
> >Let's assume:
> >  GRP_RETAIN
> >    Sections in a group with this flag set should always be included in
> >    the linked object, even if they appear unused.
> >
> >One issue I have with the group method is conceptual; groups are supposed
> >to be used to group related sections. They are related because of their
> >meaning from the application perspective, not because of their metadata
> >(i.e. sections to be retained are related by their metadata because they
> >all should be retained).
> >
> >Just because groups have a side effect that can be leveraged to
> >get a section to be retained in the final link, why does that mean we
> >need to leverage that property when it it's not actually the simplest or
> >most obvious way to implement the new behavior?
> >
> >SHF_GNU_RETAIN describes in the simplest possible way that the section
> >should be retained in the final link, without relying on other
> >constructs.
> >
> >Just because groups are discarded or retained as a group, doesn't mean
> >we need to leverage groups to try and implement retention or discarding
> >functionality.
> >
> >Why wasn't SHF_EXCLUDE implemented as a group flag? After all, groups
> >are included or excluded from the link together.
> >  GRP_EXCLUDE
> >    Sections in a group with GRP_EXCLUDE set should be discarded from
> >    the link.
> >
> >I mean, you could kind of use groups for anything when you decide
> >grouping sections by metadata is OK.
> >Why define SHT_NOBITS when you can create:
> >  GRP_NOBITS
> >    Sections in this group occupy no space in the file. They must have
> >    type SHT_PROGBITS.
> >
> >Retention of a section is a property of the section. We are misusing ELF
> >constructs by using groups to indicate an arbitrary section needs to be
> >retained.
> >
> >Another issue (if more is needed) is about how to name the groups.
> >* If it is mandated that GRP_RETAIN groups have the same name e.g.
> >  "grp_retain", that means you can't put a section you want to
> >  retain in a different logical group that makes sense from the
> >  application perspective. So the other sections you would want to put
> >  in a group with the retained section need to all be put in a bundle
> >  with all the other GRP_RETAIN sections.
> >* If GRP_RETAIN groups can have any name, so you can have multiple
> >  GRP_RETAIN groups, how does the compiler decide how to name the
> >  groups? It seems like it would be a mess. "grp_retain0", "grp_retain2"
> >  ... "grp_retain10" from one object file, "grp_retain0"...
> >  "grp_retain5" from another. Extra processing in the
> >  linker to clean up these group names and keep them unique would be
> >  required when performing a relocatable link.
> >
> >As a general point, what if I decide that there's enough pressure from
> >the anti-SHF_GNU_RETAIN side that I change the implementation to use
> >groups. But then those who already backed the flag prefer that method
> >over using groups think the implementation should not have been changed.
> >
> >I feel like we already got enough backing for SHF_GNU_RETAIN between the
> >GNU gABI and Binutils discussions. I understand, and welcome, more
> >feedback, but I haven't been convinced any other method is obviously
> >better than SHF_GNU_RETAIN, so why change it?
> >
> >I'm not saying it's possible to quantify which mechanism for "retain" is
> >best, but SHF_GNU_RETAIN is the simplest, most obvious, and easiest to
> >understand. Surely that gives it top marks?
> >
> >I'm also yet to hear one convincing reason why SHF_GNU_RETAIN is bad.
> >As far as I can tell, the main argument against it stems from the fact
> >that you can have two input sections with the same name but different
> >SHF_GNU_RETAIN flag states. Not only is this a non-issue, groups have
> >exactly the same problem!
> >
> >It would be great if a global maintainer to chime in on whether the
> >attached patch is acceptable, because otherwise we are going to go back
> >and forth forever.
> >
> >Thanks,
> >Jozef
>
> Hi Jozef,
>
> I have checked with a few folks on the LLVM side. James Henderson and James Y
> Knight seem to prefer a section flag to other mechanism. I prefer a
> section flag, too (I was on the fence and wanted the alternatives to be considered).
>
> About the section flag 'R' in assembly:
>
> I'd still prefer an error to not add another exception to https://sourceware.org/pipermail/binutils/2020-February/109945.html
> (a consensus GNU as and LLVM's integrated assembler have reached)
>
>    .section retain,"a",@progbits
>    .section retain,"aR",@progbits  # error for section flags change
>
> If a separate section is desired, I'd prefer an explicit ,unique,0 or ,unique,1 or ...
>
>    .section retain,"a",@progbits
>    .section retain,"aR",@progbits,unique,0   # 'unique' is a binutils 2.35 feature (available in LLVM for a while)

I don't think it is desirable since it requires tracking the unique number
which isn't an issue with the compiler.  But it is a hassle for handwritten
assembly codes with #include.

>
> About the use case in GCC (and probably Clang in the future):
>
>    // a.h
>    __attribute__((section("sec"))) inline void bar() { ... }
>
>    // a.c
>    #include "a.h"
>    __attribute__((section("sec"), retain))   // retain may be the pre-existing used.
>    void foo() { ... }
>
> Perhaps there needs a function attribute to lower to ,unique,1 in assembly.



-- 
H.J.