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

[Jozef Lawrynowicz <jozef.l@mittosystems.com>]

On Mon, Sep 28, 2020 at 09:43:53PM -0700, Fangrui Song wrote:
> On 2020-09-28, Jozef Lawrynowicz wrote:
> > The attached patch is version 2 of the SHF_GNU_RETAIN patch that was
> > previously discussed here:
> > https://sourceware.org/pipermail/binutils/2020-September/113406.html
> > 
> > The following changes have been made:
> > - Removed the .retain directive
> > - The assembler will create different input sections for sections with
> >  the same name but SHF_GNU_RETAIN set/unset (thanks to H.J. for that).
> >  This means the linker will be able to do a better job garbage
> >  collecting input sections, as the "retain" attribute applied to a
> >  symbol declaration in the source code will not cause other parts of
> >  the program that are not required, but are in the same section, to be
> >  unnecessarily retained.
> > - Added GNU OSABI handling (also thanks to H.J.).
> 
> My point from https://sourceware.org/pipermail/binutils/2020-September/113466.html stands.
> Section flags are a bit cumbersome. If the following
> 
>   // a.h
>   __attribute__((section("sec")))
>   inline void bar() { ... }
>   // a.c
>   #include "a.h"
>   __attribute__((section("sec"), retain))
>   void foo() {
>   }
> 
> compiles to
> 
>   .section sec,"a",@progbits
>   ...
>   .section sec,"aR",@progbits
>   ...
> 
> You will get a gas error for changing section flags
> (https://sourceware.org/pipermail/binutils/2020-February/109945.html)
> 

There is no error in this case.
The original patch handled it, and it has been further improved in v2.
There's even a testcase testing exactly the functionality you say is
broken.

  gas/testsuite/gas/elf/section22.s:
      .section	.text,"ax",%progbits
    ...
      .section	.data,"aw"
    ...
      .section	.bss,"aw"
    ...
      .section	.bss,"awR",%nobits
    ...
      .section	.data,"awR",%progbits
    ...
      .section	.text,"axR",%progbits

I already gave a detailed response to your reloc proposal:
https://sourceware.org/pipermail/binutils/2020-September/113450.html

Why don't you first answer my questions about why using relocs is a
really bad idea, is a hack, doesn't conceptually make any sense, and
will confuse users?

You never actually gave a reason that using relocs is better than
SHF_GNU_RETAIN, you seem to be pushing this alternative implementation just
because it is an alternative implementation. I don't count your
description of section flags as "cumbersome" as a reason we should
shelve this proposal.

> .reloc is really convenience in this case. You can add as many .reloc
> directives as you like, each contributing one R_*_NONE to the object
> file but zero cost to the linked image.

Are we really counting a section flag as adding "cost" to a linked
image?  Your method requires a "benign zero-sized section" (suggested in
your previous email), doesn't this also add cost? What about the cost of
having nonsensical relocs in object files?

If we are using the term "cost" loosely, and to further our own point,
then SHF_GNU_RETAIN actually has negative cost to the linked image. You
can have different input sections with the same name, but with different
states for SHF_GNU_RETAIN (set and unset), and so garbage collection
will operate with finer granularity, removing more unneeded parts of the
program compared to your reloc mechanism, which would keep the entire
section containing the symbol referenced by the reloc.

There are more logic holes in your other email that I didn't
respond to, but I've already spent a tonne of time trying to address
your points (most of which you ignored, so why did I bother?), so I
think I'm done debating this.
If other people back up your suggestion, and address my concerns, then
we can re-open this.

Thanks,
Jozef