public inbox for gnu-gabi@sourceware.org
 help / color / mirror / Atom feed
* Re: [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag
       [not found]   ` <20200929100442.o3e66jbmqpv5cajh@jozef-acer-manjaro>
@ 2020-09-29 19:38     ` Fangrui Song
  2020-09-29 19:54       ` H.J. Lu
  2020-09-29 21:37       ` Jozef Lawrynowicz
  0 siblings, 2 replies; 10+ messages in thread
From: Fangrui Song @ 2020-09-29 19:38 UTC (permalink / raw)
  To: binutils, gnu-gabi

On 2020-09-29, Jozef Lawrynowicz wrote:
>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.

Hi Jozef, apologies if I did not respond to your previous replies in
time or if I acted as if I "blocked" your change. I maintain LLVM's
linker LLD and am also a contributor to its binary utilities
(llvm-readelf, llvm-objdump, ...). If GCC makes use of this flag for __attribute__((used)),
LLD and LLVM's binary utilities do need to support it.
So I want to participate in the design before we regret :)

I have studied linker GC quite a lot (contributed patches in LLD
and coordinated on the feature parity among the 3 linkers
(e.g. https://sourceware.org/pipermail/binutils/2020-August/112732.html)

LLD and GNU ld, LLVM's integrated assembler and GNU as have achieved consensus on many aspects
on SHF_LINK_ORDER and 'unique' linkage. Credits to H.J. for all the binutils side work.
)

I have asked a couple of toolchain folks. Many do share the same feeling
with me: an additional GC feature is useful but whether it should be
this exact form of a new flag we don't know.

Your patch also deprives an OS-specific flag from ELFOSABI_FREEBSD:)
I am not a committer of FreeBSD and I am surely not authoritative on
whether this is acceptable. But, FreeBSD does use the aforementioned
LLVM tools and I don't want to dirty the code base by adding
if (osabi == ELFOSABI_FREEBSD) do something. 

We have 8 bits in SHF_MASKOS: not too few, but also not too many.  We
should be prudent with new flags, especially if we already have a
facility for section retaining. There is a difference ".reloc requires a
known live section" which might cause friction but it has the advantage that it is
time-tested and has good toolchain support.


Opinions from others:

* Peter Smith mentioned that Arm's proprietary toolchain does something
   similar to SHF_GNU_RETAIN
   (https://www.keil.com/support/man/docs/armcc/armcc_chr1359124983230.htm)
   "I put __attribute__((used)) on my function and yet the tools removed
   it anyway."

   I have personally used a .reloc to retain
   __attribute__((used, section(".data.PERSIST_SIG"))) static char SigAfl[] = "##SIG_AFL_PERSISTENT##";

* James Henderson mentioned whether we can
   "create a new section type listing section indexes that should be
   retained"
   As a variant, we could use a SHT_GROUP with a new GRP_* bit.
   Anchor: [group] (see below)

> Do we want to make life easier for ourselves, or easier for our users?
>
> I get that ABI changes can be a bit disruptive, but this new flag in particular really isn't complicated anyway.

On 2020-09-29, Jozef Lawrynowicz wrote:
>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	.bss,"aw"
>    ...
>      .section	.bss,"awR",%nobits
>
>I already gave a detailed response to your reloc proposal:
>https://sourceware.org/pipermail/binutils/2020-September/113450.html

Letting the following assembly creates two .bss sections adds an exception to
"gas: error for section type, attr, or entsize change"
(https://sourceware.org/pipermail/binutils/2020-February/109945.html)
for which I have achieved consensus with Alan Modra and some LLVM
toolchain folks. That is why I hope we could re-consider this design
choice.

.section	.bss,"aw"
.section	.bss,"awR",%nobits

See above for [group]: it is actually nice to use a group here, because
the quadruple
(section name, group name if "G", linked-to section if "o", unique id of
"unique") is the section key. The following creates a section different
from the default .bss:

   .section	.bss,"awG",%nobits,gc_root

>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.

I hope my previous paragraphs have answered this point.

>> .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.
>
>Thanks,
>Jozef

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag
  2020-09-29 19:38     ` [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag Fangrui Song
@ 2020-09-29 19:54       ` H.J. Lu
  2020-09-29 21:37       ` Jozef Lawrynowicz
  1 sibling, 0 replies; 10+ messages in thread
From: H.J. Lu @ 2020-09-29 19:54 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Binutils, GNU gABI gnu-gabi

On Tue, Sep 29, 2020 at 12:38 PM Fangrui Song <i@maskray.me> wrote:
>

> >>
> >>   // 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 .bss,"aw"
> >    ...
> >      .section .bss,"awR",%nobits
> >
> >I already gave a detailed response to your reloc proposal:
> >https://sourceware.org/pipermail/binutils/2020-September/113450.html
>
> Letting the following assembly creates two .bss sections adds an exception to
> "gas: error for section type, attr, or entsize change"
> (https://sourceware.org/pipermail/binutils/2020-February/109945.html)
> for which I have achieved consensus with Alan Modra and some LLVM
> toolchain folks. That is why I hope we could re-consider this design
> choice.
>
> .section        .bss,"aw"
> .section        .bss,"awR",%nobits
>

FWIW, GNU assembler has been able to create multiple sections with
the same section name for a while.  For SHF_GNU_RETAIN, we added

+/* Create unique input sections for sections with the same name, but different
+   values for the flags in this mask.  */
+#define SEC_ASSEMBLER_SHF_MASK SHF_GNU_RETAIN

to control when to create a new section with the same section name.
For other cases, no new section is created.

-- 
H.J.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag
  2020-09-29 19:38     ` [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag Fangrui Song
  2020-09-29 19:54       ` H.J. Lu
@ 2020-09-29 21:37       ` Jozef Lawrynowicz
  2020-09-30  0:10         ` Roland McGrath
  1 sibling, 1 reply; 10+ messages in thread
From: Jozef Lawrynowicz @ 2020-09-29 21:37 UTC (permalink / raw)
  To: Fangrui Song; +Cc: binutils, gnu-gabi

On Tue, Sep 29, 2020 at 12:38:06PM -0700, Fangrui Song wrote:
> On 2020-09-29, Jozef Lawrynowicz wrote:
> > 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.
> 
> Hi Jozef, apologies if I did not respond to your previous replies in
> time or if I acted as if I "blocked" your change. I maintain LLVM's
> linker LLD and am also a contributor to its binary utilities
> (llvm-readelf, llvm-objdump, ...). If GCC makes use of this flag for __attribute__((used)),
> LLD and LLVM's binary utilities do need to support it.
> So I want to participate in the design before we regret :)
> 
> I have studied linker GC quite a lot (contributed patches in LLD
> and coordinated on the feature parity among the 3 linkers
> (e.g. https://sourceware.org/pipermail/binutils/2020-August/112732.html)
> 
> LLD and GNU ld, LLVM's integrated assembler and GNU as have achieved consensus on many aspects
> on SHF_LINK_ORDER and 'unique' linkage. Credits to H.J. for all the binutils side work.
> )
> 
> I have asked a couple of toolchain folks. Many do share the same feeling
> with me: an additional GC feature is useful but whether it should be
> this exact form of a new flag we don't know.

Hi Fangrui,

Thank you for providing some more information to backup your points, I
will address those below.

> 
> Your patch also deprives an OS-specific flag from ELFOSABI_FREEBSD:)
> I am not a committer of FreeBSD and I am surely not authoritative on
> whether this is acceptable. But, FreeBSD does use the aforementioned
> LLVM tools and I don't want to dirty the code base by adding
> if (osabi == ELFOSABI_FREEBSD) do something.

I don't have any insight into the ELFOSABI_FREEBSD change, H.J. added that
part, I assume it is very closely aligned with the "pure" GNU OSABI, so
any changes to one should be kept in parity with the other. However,
that is only my assumption.

> 
> We have 8 bits in SHF_MASKOS: not too few, but also not too many.  We
> should be prudent with new flags, especially if we already have a
> facility for section retaining. There is a difference ".reloc requires a
> known live section" which might cause friction but it has the advantage that it is
> time-tested and has good toolchain support.

Yes, this is something I considered, but there are 6 bits remaining to
be used, with SHF_GNU_RETAIN there will be 5 bits remaining.
Given how little movement there has been within the SHF_MAKOS bits for
the past 20 years, I don't think we need to be too protective of the
bits. They are there to be used after all, and IMO SHF_GNU_RETAIN is a
very well-defined use case to use one of those bits.

For the .reloc mechanism, in every object file with a retained section
you will need to have an empty input section which is guaranteed to be
kept by the linker. How is the compiler to know what that section is?

> 
> 
> Opinions from others:
> 
> * Peter Smith mentioned that Arm's proprietary toolchain does something
>   similar to SHF_GNU_RETAIN
>   (https://www.keil.com/support/man/docs/armcc/armcc_chr1359124983230.htm)
>   "I put __attribute__((used)) on my function and yet the tools removed
>   it anyway."
> 
> * James Henderson mentioned whether we can
>   "create a new section type listing section indexes that should be
>   retained"
>   As a variant, we could use a SHT_GROUP with a new GRP_* bit.
>   Anchor: [group] (see below)

Using groups was discussed on the GNU gABI thread, but I don't really
see the benefit of the added complication. Doesn't handling new section
group types still require significant coordination between all the
different compilers, assemblers and linkers?

Any attempt to reference a section which does not exist in the object
file, and is not guaranteed to be present in the linked output file, is
a non-starter in my opinion. We give users the freedom to do what they
want with linker scripts as ELF doesn't mandate any section must be
linked into the final executable. I think it's better to consolidate
the requirement to retain within the section that requires retaining,
instead of spreading the requirement to retain between ELF constructs
which have unrelated purposes, and need to be aligned between different
stages of the build process.

> 
> > Do we want to make life easier for ourselves, or easier for our users?
> > 
> > I get that ABI changes can be a bit disruptive, but this new flag in particular really isn't complicated anyway.
> 
> On 2020-09-29, Jozef Lawrynowicz wrote:
> > 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	.bss,"aw"
> >    ...
> >      .section	.bss,"awR",%nobits
> > 
> > I already gave a detailed response to your reloc proposal:
> > https://sourceware.org/pipermail/binutils/2020-September/113450.html
> 
> Letting the following assembly creates two .bss sections adds an exception to
> "gas: error for section type, attr, or entsize change"
> (https://sourceware.org/pipermail/binutils/2020-February/109945.html)
> for which I have achieved consensus with Alan Modra and some LLVM
> toolchain folks. That is why I hope we could re-consider this design
> choice.
> 
> .section	.bss,"aw"
> .section	.bss,"awR",%nobits

I agree that this could be considered slightly inelegant, but much less
so than having .reloc directives in arbitrary empty sections.
Once someone understands what the "R" flag does, it should be clear why
we want to create a separate input section for it.

If we really want, we can force the compiler to create unique section
names for declarations which have the "retain" attribute applied. I
assume most of the time, users will want this fine section
granularity. I would prefer not to go that route and leave it to the
users to specify unique sections if they want.

> 
> See above for [group]: it is actually nice to use a group here, because
> the quadruple
> (section name, group name if "G", linked-to section if "o", unique id of
> "unique") is the section key. The following creates a section different
> from the default .bss:
> 
>   .section	.bss,"awG",%nobits,gc_root

I don't think it is really very important whether input sections with
the same name are unique or not within an object file. ELF doesn't care
about this. You can have sections with the same name and same flags in
different object files and the linker can just deal with it. It's the
same situation if you have two sections from the same object file which
have the same name.

Within an object file, you'll end up with all the non-SHF_GNU_RETAIN
sections with the same name being merged together by the assembler, and
all those with SHF_GNU_RETAIN being merged. I don't think there is any
benefit to keep SHF_GNU_RETAIN sections with the same name as separate
input sections in the object file.

Thanks,
Jozef

> 
> > 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.
> 
> I hope my previous paragraphs have answered this point.
> 
> > > .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.
> > 
> > Thanks,
> > Jozef

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag
  2020-09-29 21:37       ` Jozef Lawrynowicz
@ 2020-09-30  0:10         ` Roland McGrath
  2020-09-30 10:18           ` Jozef Lawrynowicz
  2020-09-30 14:13           ` Michael Matz
  0 siblings, 2 replies; 10+ messages in thread
From: Roland McGrath @ 2020-09-30  0:10 UTC (permalink / raw)
  To: Fangrui Song, Binutils Development, gnu-gabi

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.

On Tue, Sep 29, 2020 at 2:37 PM Jozef Lawrynowicz <jozef.l@mittosystems.com>
wrote:

> On Tue, Sep 29, 2020 at 12:38:06PM -0700, Fangrui Song wrote:
> > On 2020-09-29, Jozef Lawrynowicz wrote:
> > > 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.
> >
> > Hi Jozef, apologies if I did not respond to your previous replies in
> > time or if I acted as if I "blocked" your change. I maintain LLVM's
> > linker LLD and am also a contributor to its binary utilities
> > (llvm-readelf, llvm-objdump, ...). If GCC makes use of this flag for
> __attribute__((used)),
> > LLD and LLVM's binary utilities do need to support it.
> > So I want to participate in the design before we regret :)
> >
> > I have studied linker GC quite a lot (contributed patches in LLD
> > and coordinated on the feature parity among the 3 linkers
> > (e.g. https://sourceware.org/pipermail/binutils/2020-August/112732.html)
> >
> > LLD and GNU ld, LLVM's integrated assembler and GNU as have achieved
> consensus on many aspects
> > on SHF_LINK_ORDER and 'unique' linkage. Credits to H.J. for all the
> binutils side work.
> > )
> >
> > I have asked a couple of toolchain folks. Many do share the same feeling
> > with me: an additional GC feature is useful but whether it should be
> > this exact form of a new flag we don't know.
>
> Hi Fangrui,
>
> Thank you for providing some more information to backup your points, I
> will address those below.
>
> >
> > Your patch also deprives an OS-specific flag from ELFOSABI_FREEBSD:)
> > I am not a committer of FreeBSD and I am surely not authoritative on
> > whether this is acceptable. But, FreeBSD does use the aforementioned
> > LLVM tools and I don't want to dirty the code base by adding
> > if (osabi == ELFOSABI_FREEBSD) do something.
>
> I don't have any insight into the ELFOSABI_FREEBSD change, H.J. added that
> part, I assume it is very closely aligned with the "pure" GNU OSABI, so
> any changes to one should be kept in parity with the other. However,
> that is only my assumption.
>
> >
> > We have 8 bits in SHF_MASKOS: not too few, but also not too many.  We
> > should be prudent with new flags, especially if we already have a
> > facility for section retaining. There is a difference ".reloc requires a
> > known live section" which might cause friction but it has the advantage
> that it is
> > time-tested and has good toolchain support.
>
> Yes, this is something I considered, but there are 6 bits remaining to
> be used, with SHF_GNU_RETAIN there will be 5 bits remaining.
> Given how little movement there has been within the SHF_MAKOS bits for
> the past 20 years, I don't think we need to be too protective of the
> bits. They are there to be used after all, and IMO SHF_GNU_RETAIN is a
> very well-defined use case to use one of those bits.
>
> For the .reloc mechanism, in every object file with a retained section
> you will need to have an empty input section which is guaranteed to be
> kept by the linker. How is the compiler to know what that section is?
>
> >
> >
> > Opinions from others:
> >
> > * Peter Smith mentioned that Arm's proprietary toolchain does something
> >   similar to SHF_GNU_RETAIN
> >   (
> https://www.keil.com/support/man/docs/armcc/armcc_chr1359124983230.htm)
> >   "I put __attribute__((used)) on my function and yet the tools removed
> >   it anyway."
> >
> > * James Henderson mentioned whether we can
> >   "create a new section type listing section indexes that should be
> >   retained"
> >   As a variant, we could use a SHT_GROUP with a new GRP_* bit.
> >   Anchor: [group] (see below)
>
> Using groups was discussed on the GNU gABI thread, but I don't really
> see the benefit of the added complication. Doesn't handling new section
> group types still require significant coordination between all the
> different compilers, assemblers and linkers?
>
> Any attempt to reference a section which does not exist in the object
> file, and is not guaranteed to be present in the linked output file, is
> a non-starter in my opinion. We give users the freedom to do what they
> want with linker scripts as ELF doesn't mandate any section must be
> linked into the final executable. I think it's better to consolidate
> the requirement to retain within the section that requires retaining,
> instead of spreading the requirement to retain between ELF constructs
> which have unrelated purposes, and need to be aligned between different
> stages of the build process.
>
> >
> > > Do we want to make life easier for ourselves, or easier for our users?
> > >
> > > I get that ABI changes can be a bit disruptive, but this new flag in
> particular really isn't complicated anyway.
> >
> > On 2020-09-29, Jozef Lawrynowicz wrote:
> > > 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       .bss,"aw"
> > >    ...
> > >      .section       .bss,"awR",%nobits
> > >
> > > I already gave a detailed response to your reloc proposal:
> > > https://sourceware.org/pipermail/binutils/2020-September/113450.html
> >
> > Letting the following assembly creates two .bss sections adds an
> exception to
> > "gas: error for section type, attr, or entsize change"
> > (https://sourceware.org/pipermail/binutils/2020-February/109945.html)
> > for which I have achieved consensus with Alan Modra and some LLVM
> > toolchain folks. That is why I hope we could re-consider this design
> > choice.
> >
> > .section      .bss,"aw"
> > .section      .bss,"awR",%nobits
>
> I agree that this could be considered slightly inelegant, but much less
> so than having .reloc directives in arbitrary empty sections.
> Once someone understands what the "R" flag does, it should be clear why
> we want to create a separate input section for it.
>
> If we really want, we can force the compiler to create unique section
> names for declarations which have the "retain" attribute applied. I
> assume most of the time, users will want this fine section
> granularity. I would prefer not to go that route and leave it to the
> users to specify unique sections if they want.
>
> >
> > See above for [group]: it is actually nice to use a group here, because
> > the quadruple
> > (section name, group name if "G", linked-to section if "o", unique id of
> > "unique") is the section key. The following creates a section different
> > from the default .bss:
> >
> >   .section    .bss,"awG",%nobits,gc_root
>
> I don't think it is really very important whether input sections with
> the same name are unique or not within an object file. ELF doesn't care
> about this. You can have sections with the same name and same flags in
> different object files and the linker can just deal with it. It's the
> same situation if you have two sections from the same object file which
> have the same name.
>
> Within an object file, you'll end up with all the non-SHF_GNU_RETAIN
> sections with the same name being merged together by the assembler, and
> all those with SHF_GNU_RETAIN being merged. I don't think there is any
> benefit to keep SHF_GNU_RETAIN sections with the same name as separate
> input sections in the object file.
>
> Thanks,
> Jozef
>
> >
> > > 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.
> >
> > I hope my previous paragraphs have answered this point.
> >
> > > > .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.
> > >
> > > Thanks,
> > > Jozef
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag
  2020-09-30  0:10         ` Roland McGrath
@ 2020-09-30 10:18           ` Jozef Lawrynowicz
  2020-09-30 14:01             ` H.J. Lu
  2020-10-01 19:22             ` Fangrui Song
  2020-09-30 14:13           ` Michael Matz
  1 sibling, 2 replies; 10+ messages in thread
From: Jozef Lawrynowicz @ 2020-09-30 10:18 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Fangrui Song, Binutils Development, gnu-gabi

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

> On Tue, Sep 29, 2020 at 2:37 PM Jozef Lawrynowicz <jozef.l@mittosystems.com>
> wrote:
> 
> > On Tue, Sep 29, 2020 at 12:38:06PM -0700, Fangrui Song wrote:
> > > On 2020-09-29, Jozef Lawrynowicz wrote:
> > > > 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.
> > >
> > > Hi Jozef, apologies if I did not respond to your previous replies in
> > > time or if I acted as if I "blocked" your change. I maintain LLVM's
> > > linker LLD and am also a contributor to its binary utilities
> > > (llvm-readelf, llvm-objdump, ...). If GCC makes use of this flag for
> > __attribute__((used)),
> > > LLD and LLVM's binary utilities do need to support it.
> > > So I want to participate in the design before we regret :)
> > >
> > > I have studied linker GC quite a lot (contributed patches in LLD
> > > and coordinated on the feature parity among the 3 linkers
> > > (e.g. https://sourceware.org/pipermail/binutils/2020-August/112732.html)
> > >
> > > LLD and GNU ld, LLVM's integrated assembler and GNU as have achieved
> > consensus on many aspects
> > > on SHF_LINK_ORDER and 'unique' linkage. Credits to H.J. for all the
> > binutils side work.
> > > )
> > >
> > > I have asked a couple of toolchain folks. Many do share the same feeling
> > > with me: an additional GC feature is useful but whether it should be
> > > this exact form of a new flag we don't know.
> >
> > Hi Fangrui,
> >
> > Thank you for providing some more information to backup your points, I
> > will address those below.
> >
> > >
> > > Your patch also deprives an OS-specific flag from ELFOSABI_FREEBSD:)
> > > I am not a committer of FreeBSD and I am surely not authoritative on
> > > whether this is acceptable. But, FreeBSD does use the aforementioned
> > > LLVM tools and I don't want to dirty the code base by adding
> > > if (osabi == ELFOSABI_FREEBSD) do something.
> >
> > I don't have any insight into the ELFOSABI_FREEBSD change, H.J. added that
> > part, I assume it is very closely aligned with the "pure" GNU OSABI, so
> > any changes to one should be kept in parity with the other. However,
> > that is only my assumption.
> >
> > >
> > > We have 8 bits in SHF_MASKOS: not too few, but also not too many.  We
> > > should be prudent with new flags, especially if we already have a
> > > facility for section retaining. There is a difference ".reloc requires a
> > > known live section" which might cause friction but it has the advantage
> > that it is
> > > time-tested and has good toolchain support.
> >
> > Yes, this is something I considered, but there are 6 bits remaining to
> > be used, with SHF_GNU_RETAIN there will be 5 bits remaining.
> > Given how little movement there has been within the SHF_MAKOS bits for
> > the past 20 years, I don't think we need to be too protective of the
> > bits. They are there to be used after all, and IMO SHF_GNU_RETAIN is a
> > very well-defined use case to use one of those bits.
> >
> > For the .reloc mechanism, in every object file with a retained section
> > you will need to have an empty input section which is guaranteed to be
> > kept by the linker. How is the compiler to know what that section is?
> >
> > >
> > >
> > > Opinions from others:
> > >
> > > * Peter Smith mentioned that Arm's proprietary toolchain does something
> > >   similar to SHF_GNU_RETAIN
> > >   (
> > https://www.keil.com/support/man/docs/armcc/armcc_chr1359124983230.htm)
> > >   "I put __attribute__((used)) on my function and yet the tools removed
> > >   it anyway."
> > >
> > > * James Henderson mentioned whether we can
> > >   "create a new section type listing section indexes that should be
> > >   retained"
> > >   As a variant, we could use a SHT_GROUP with a new GRP_* bit.
> > >   Anchor: [group] (see below)
> >
> > Using groups was discussed on the GNU gABI thread, but I don't really
> > see the benefit of the added complication. Doesn't handling new section
> > group types still require significant coordination between all the
> > different compilers, assemblers and linkers?
> >
> > Any attempt to reference a section which does not exist in the object
> > file, and is not guaranteed to be present in the linked output file, is
> > a non-starter in my opinion. We give users the freedom to do what they
> > want with linker scripts as ELF doesn't mandate any section must be
> > linked into the final executable. I think it's better to consolidate
> > the requirement to retain within the section that requires retaining,
> > instead of spreading the requirement to retain between ELF constructs
> > which have unrelated purposes, and need to be aligned between different
> > stages of the build process.
> >
> > >
> > > > Do we want to make life easier for ourselves, or easier for our users?
> > > >
> > > > I get that ABI changes can be a bit disruptive, but this new flag in
> > particular really isn't complicated anyway.
> > >
> > > On 2020-09-29, Jozef Lawrynowicz wrote:
> > > > 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       .bss,"aw"
> > > >    ...
> > > >      .section       .bss,"awR",%nobits
> > > >
> > > > I already gave a detailed response to your reloc proposal:
> > > > https://sourceware.org/pipermail/binutils/2020-September/113450.html
> > >
> > > Letting the following assembly creates two .bss sections adds an
> > exception to
> > > "gas: error for section type, attr, or entsize change"
> > > (https://sourceware.org/pipermail/binutils/2020-February/109945.html)
> > > for which I have achieved consensus with Alan Modra and some LLVM
> > > toolchain folks. That is why I hope we could re-consider this design
> > > choice.
> > >
> > > .section      .bss,"aw"
> > > .section      .bss,"awR",%nobits
> >
> > I agree that this could be considered slightly inelegant, but much less
> > so than having .reloc directives in arbitrary empty sections.
> > Once someone understands what the "R" flag does, it should be clear why
> > we want to create a separate input section for it.
> >
> > If we really want, we can force the compiler to create unique section
> > names for declarations which have the "retain" attribute applied. I
> > assume most of the time, users will want this fine section
> > granularity. I would prefer not to go that route and leave it to the
> > users to specify unique sections if they want.
> >
> > >
> > > See above for [group]: it is actually nice to use a group here, because
> > > the quadruple
> > > (section name, group name if "G", linked-to section if "o", unique id of
> > > "unique") is the section key. The following creates a section different
> > > from the default .bss:
> > >
> > >   .section    .bss,"awG",%nobits,gc_root
> >
> > I don't think it is really very important whether input sections with
> > the same name are unique or not within an object file. ELF doesn't care
> > about this. You can have sections with the same name and same flags in
> > different object files and the linker can just deal with it. It's the
> > same situation if you have two sections from the same object file which
> > have the same name.
> >
> > Within an object file, you'll end up with all the non-SHF_GNU_RETAIN
> > sections with the same name being merged together by the assembler, and
> > all those with SHF_GNU_RETAIN being merged. I don't think there is any
> > benefit to keep SHF_GNU_RETAIN sections with the same name as separate
> > input sections in the object file.
> >
> > Thanks,
> > Jozef
> >
> > >
> > > > 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.
> > >
> > > I hope my previous paragraphs have answered this point.
> > >
> > > > > .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.
> > > >
> > > > Thanks,
> > > > Jozef
> >
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag
  2020-09-30 10:18           ` Jozef Lawrynowicz
@ 2020-09-30 14:01             ` H.J. Lu
  2020-10-01 19:22             ` Fangrui Song
  1 sibling, 0 replies; 10+ messages in thread
From: H.J. Lu @ 2020-09-30 14:01 UTC (permalink / raw)
  To: Roland McGrath, Fangrui Song, Binutils Development, GNU gABI gnu-gabi

On Wed, Sep 30, 2020 at 3:19 AM Jozef Lawrynowicz
<jozef.l@mittosystems.com> 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.
>

I like SHF_GNU_RETAIN which is complementary to SHF_EXCLUDE.

-- 
H.J.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag
  2020-09-30  0:10         ` Roland McGrath
  2020-09-30 10:18           ` Jozef Lawrynowicz
@ 2020-09-30 14:13           ` Michael Matz
  1 sibling, 0 replies; 10+ messages in thread
From: Michael Matz @ 2020-09-30 14:13 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Fangrui Song, Binutils Development, gnu-gabi

Hello,

On Tue, 29 Sep 2020, 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.

pseudo-code for retaining a section in producer.  SHF_GNU_RETAIN:

  retain_section(S):
    S.flag |= SHF_GNU_RETAIN

GRP_RETAIN:

  retain_section(S):
    if !retain_group:
      retain_group = new_group(GRP_RETAIN)
    retain_group.add (S)

pseudo-code for interpreting this in the consumer (which presumably is 
section based as basic building block, as groups are optional).  
SHF_GNU_RETAIN:

  in the 'output_section_p' predicate, add " || S.flag & SHF_GNU_RETAIN".

GRP_RETAIN:

  in preparatory per-object parsing, in the existing group parser:
  
    if G.type == GRP_RETAIN:
      foreach S in G:
        S.needed = true

  in the 'output_section_p' predicate, add " || S.needed"

That's the additional structure you mention.  While it's not much 
down-side, what is the up-side?  There should be one, not just only small 
down-sides.


Ciao,
Michael.

 > 
> On Tue, Sep 29, 2020 at 2:37 PM Jozef Lawrynowicz <jozef.l@mittosystems.com>
> wrote:
> 
> > On Tue, Sep 29, 2020 at 12:38:06PM -0700, Fangrui Song wrote:
> > > On 2020-09-29, Jozef Lawrynowicz wrote:
> > > > 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.
> > >
> > > Hi Jozef, apologies if I did not respond to your previous replies in
> > > time or if I acted as if I "blocked" your change. I maintain LLVM's
> > > linker LLD and am also a contributor to its binary utilities
> > > (llvm-readelf, llvm-objdump, ...). If GCC makes use of this flag for
> > __attribute__((used)),
> > > LLD and LLVM's binary utilities do need to support it.
> > > So I want to participate in the design before we regret :)
> > >
> > > I have studied linker GC quite a lot (contributed patches in LLD
> > > and coordinated on the feature parity among the 3 linkers
> > > (e.g. https://sourceware.org/pipermail/binutils/2020-August/112732.html)
> > >
> > > LLD and GNU ld, LLVM's integrated assembler and GNU as have achieved
> > consensus on many aspects
> > > on SHF_LINK_ORDER and 'unique' linkage. Credits to H.J. for all the
> > binutils side work.
> > > )
> > >
> > > I have asked a couple of toolchain folks. Many do share the same feeling
> > > with me: an additional GC feature is useful but whether it should be
> > > this exact form of a new flag we don't know.
> >
> > Hi Fangrui,
> >
> > Thank you for providing some more information to backup your points, I
> > will address those below.
> >
> > >
> > > Your patch also deprives an OS-specific flag from ELFOSABI_FREEBSD:)
> > > I am not a committer of FreeBSD and I am surely not authoritative on
> > > whether this is acceptable. But, FreeBSD does use the aforementioned
> > > LLVM tools and I don't want to dirty the code base by adding
> > > if (osabi == ELFOSABI_FREEBSD) do something.
> >
> > I don't have any insight into the ELFOSABI_FREEBSD change, H.J. added that
> > part, I assume it is very closely aligned with the "pure" GNU OSABI, so
> > any changes to one should be kept in parity with the other. However,
> > that is only my assumption.
> >
> > >
> > > We have 8 bits in SHF_MASKOS: not too few, but also not too many.  We
> > > should be prudent with new flags, especially if we already have a
> > > facility for section retaining. There is a difference ".reloc requires a
> > > known live section" which might cause friction but it has the advantage
> > that it is
> > > time-tested and has good toolchain support.
> >
> > Yes, this is something I considered, but there are 6 bits remaining to
> > be used, with SHF_GNU_RETAIN there will be 5 bits remaining.
> > Given how little movement there has been within the SHF_MAKOS bits for
> > the past 20 years, I don't think we need to be too protective of the
> > bits. They are there to be used after all, and IMO SHF_GNU_RETAIN is a
> > very well-defined use case to use one of those bits.
> >
> > For the .reloc mechanism, in every object file with a retained section
> > you will need to have an empty input section which is guaranteed to be
> > kept by the linker. How is the compiler to know what that section is?
> >
> > >
> > >
> > > Opinions from others:
> > >
> > > * Peter Smith mentioned that Arm's proprietary toolchain does something
> > >   similar to SHF_GNU_RETAIN
> > >   (
> > https://www.keil.com/support/man/docs/armcc/armcc_chr1359124983230.htm)
> > >   "I put __attribute__((used)) on my function and yet the tools removed
> > >   it anyway."
> > >
> > > * James Henderson mentioned whether we can
> > >   "create a new section type listing section indexes that should be
> > >   retained"
> > >   As a variant, we could use a SHT_GROUP with a new GRP_* bit.
> > >   Anchor: [group] (see below)
> >
> > Using groups was discussed on the GNU gABI thread, but I don't really
> > see the benefit of the added complication. Doesn't handling new section
> > group types still require significant coordination between all the
> > different compilers, assemblers and linkers?
> >
> > Any attempt to reference a section which does not exist in the object
> > file, and is not guaranteed to be present in the linked output file, is
> > a non-starter in my opinion. We give users the freedom to do what they
> > want with linker scripts as ELF doesn't mandate any section must be
> > linked into the final executable. I think it's better to consolidate
> > the requirement to retain within the section that requires retaining,
> > instead of spreading the requirement to retain between ELF constructs
> > which have unrelated purposes, and need to be aligned between different
> > stages of the build process.
> >
> > >
> > > > Do we want to make life easier for ourselves, or easier for our users?
> > > >
> > > > I get that ABI changes can be a bit disruptive, but this new flag in
> > particular really isn't complicated anyway.
> > >
> > > On 2020-09-29, Jozef Lawrynowicz wrote:
> > > > 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       .bss,"aw"
> > > >    ...
> > > >      .section       .bss,"awR",%nobits
> > > >
> > > > I already gave a detailed response to your reloc proposal:
> > > > https://sourceware.org/pipermail/binutils/2020-September/113450.html
> > >
> > > Letting the following assembly creates two .bss sections adds an
> > exception to
> > > "gas: error for section type, attr, or entsize change"
> > > (https://sourceware.org/pipermail/binutils/2020-February/109945.html)
> > > for which I have achieved consensus with Alan Modra and some LLVM
> > > toolchain folks. That is why I hope we could re-consider this design
> > > choice.
> > >
> > > .section      .bss,"aw"
> > > .section      .bss,"awR",%nobits
> >
> > I agree that this could be considered slightly inelegant, but much less
> > so than having .reloc directives in arbitrary empty sections.
> > Once someone understands what the "R" flag does, it should be clear why
> > we want to create a separate input section for it.
> >
> > If we really want, we can force the compiler to create unique section
> > names for declarations which have the "retain" attribute applied. I
> > assume most of the time, users will want this fine section
> > granularity. I would prefer not to go that route and leave it to the
> > users to specify unique sections if they want.
> >
> > >
> > > See above for [group]: it is actually nice to use a group here, because
> > > the quadruple
> > > (section name, group name if "G", linked-to section if "o", unique id of
> > > "unique") is the section key. The following creates a section different
> > > from the default .bss:
> > >
> > >   .section    .bss,"awG",%nobits,gc_root
> >
> > I don't think it is really very important whether input sections with
> > the same name are unique or not within an object file. ELF doesn't care
> > about this. You can have sections with the same name and same flags in
> > different object files and the linker can just deal with it. It's the
> > same situation if you have two sections from the same object file which
> > have the same name.
> >
> > Within an object file, you'll end up with all the non-SHF_GNU_RETAIN
> > sections with the same name being merged together by the assembler, and
> > all those with SHF_GNU_RETAIN being merged. I don't think there is any
> > benefit to keep SHF_GNU_RETAIN sections with the same name as separate
> > input sections in the object file.
> >
> > Thanks,
> > Jozef
> >
> > >
> > > > 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.
> > >
> > > I hope my previous paragraphs have answered this point.
> > >
> > > > > .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.
> > > >
> > > > Thanks,
> > > > Jozef
> >
> >
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag
  2020-09-30 10:18           ` Jozef Lawrynowicz
  2020-09-30 14:01             ` H.J. Lu
@ 2020-10-01 19:22             ` Fangrui Song
  2020-10-01 19:53               ` H.J. Lu
  2020-10-02 12:44               ` Jozef Lawrynowicz
  1 sibling, 2 replies; 10+ messages in thread
From: Fangrui Song @ 2020-10-01 19:22 UTC (permalink / raw)
  To: Roland McGrath, H.J. Lu, Binutils Development, gnu-gabi,
	Peter Smith, George Rimar, James Henderson, James Y Knight

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)


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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag
  2020-10-01 19:22             ` Fangrui Song
@ 2020-10-01 19:53               ` H.J. Lu
  2020-10-02 12:44               ` Jozef Lawrynowicz
  1 sibling, 0 replies; 10+ messages in thread
From: H.J. Lu @ 2020-10-01 19:53 UTC (permalink / raw)
  To: Fangrui Song
  Cc: Roland McGrath, Binutils Development, GNU gABI gnu-gabi,
	Peter Smith, George Rimar, James Henderson, James Y Knight

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag
  2020-10-01 19:22             ` Fangrui Song
  2020-10-01 19:53               ` H.J. Lu
@ 2020-10-02 12:44               ` Jozef Lawrynowicz
  1 sibling, 0 replies; 10+ messages in thread
From: Jozef Lawrynowicz @ 2020-10-02 12:44 UTC (permalink / raw)
  To: Fangrui Song
  Cc: Roland McGrath, H.J. Lu, Binutils Development, gnu-gabi,
	Peter Smith, George Rimar, James Henderson, James Y Knight

On Thu, Oct 01, 2020 at 12:22:11PM -0700, Fangrui Song 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)
> 
> 
> 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.

Hi Fangrui,

My revised implementation for the "retain" attribute in GCC is for
declarations with this attribute to always be placed in their own,
uniquely named section. This will leverage the same mechanism that the
-f{function,data}-sections options use. The "unique" option to the
.section directive will not be required.

I don't think there is any downside to doing this, compared to using the
default section name, and it solves the issue of having two sections
with the same name but different flags.

I therefore no longer have any specific need for
>   .section retain,"a",@progbits
>   .section retain,"aR",@progbits  # error for section flags change
to be supported by GAS, and could remove that support from my next
iteration of the Binutils patch.

Thanks,
Jozef

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-10-02 12:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200928132613.btkqaoomv4fdnupn@jozef-acer-manjaro>
     [not found] ` <20200929044353.vx26ypc2oioqbmfb@gmail.com>
     [not found]   ` <20200929100442.o3e66jbmqpv5cajh@jozef-acer-manjaro>
2020-09-29 19:38     ` [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag Fangrui Song
2020-09-29 19:54       ` H.J. Lu
2020-09-29 21:37       ` Jozef Lawrynowicz
2020-09-30  0:10         ` Roland McGrath
2020-09-30 10:18           ` Jozef Lawrynowicz
2020-09-30 14:01             ` H.J. Lu
2020-10-01 19:22             ` Fangrui Song
2020-10-01 19:53               ` H.J. Lu
2020-10-02 12:44               ` Jozef Lawrynowicz
2020-09-30 14:13           ` Michael Matz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).