Re: [RFC] Add support for the "retain" attribute utilizing SHF_GNU_RETAIN
On 10/26/20 2:43 PM, Jozef Lawrynowicz wrote:
> On Mon, Oct 26, 2020 at 07:08:06PM +, Pedro Alves via Gcc-patches wrote:
>> On 10/6/20 12:10 PM, Jozef Lawrynowicz wrote:
>>
>>> Should "used" apply SHF_GNU_RETAIN?
>>> ===
>>> Another talking point is whether the existing "used" attribute should
>>> apply the SHF_GNU_RETAIN flag to the containing section.
>>>
>>> It seems unlikely that a user applies the "used" attribute to a
>>> declaration, and means for it to be saved from only compiler
>>> optimization, but *not* linker optimization. So perhaps it would be
>>> beneficial for "used" to apply SHF_GNU_RETAIN in some way.
>>>
>>> If "used" did apply SHF_GNU_RETAIN, we would also have to
>>> consider the above options for how to apply SHF_GNU_RETAIN to the
>>> section. Since the "used" attribute has been around for a while
>>> it might not be appropriate for its behavior to be changed to place the
>>> associated declaration in its own, unique section, as in option (2).
>>>
>> To me, if I use attribute((used)), and the linker still garbage
>> collects the symbol, then the toolchain has a bug. Is there any
>> use case that would suggest otherwise?
>>
>> Thanks,
>> Pedro Alves
>>
> I agree that "used" should imply SHF_GNU_RETAIN on whatever section
> contains the declaration that the attribute is applied to. However, I
> think that apart from the section flag being applied to the section, the
> behaviour of "used" shouldn't be modified i.e. the declaration shouldn't
> be put in a unique section.
>
> I originally justified the addition of a "retain" attribute, alongside
> "used" implying SHF_GNU_RETAIN, as indicating that the declaration
> should be placed in it's own section. In hindsight, this is unnecessary;
> if the user wants to increase the granularity of the portions of their
> program being retained, they should build with
> -f{function,data}-sections, or manually put the declaration in its own
> section with the "section" attribute.
>
> So we could shelve the "retain" attribute, and just modify the "used"
> attribute to imply SHF_GNU_RETAIN. If we get consensus on that, I could
> go ahead an implement it, but I never got any specific feedback on the
> GCC behavior from anyone apart from you. I don't know whether to
> interpret that lack of feedback, whilst the other aspects of the
> implementation were commented on, as support for the "retain" attribute.
>
> (I appreciate you giving that feedback in the Binutils discussions, and
> should have engaged in those discussions more at the time. There was
> just a lot of opinions flying about on many aspects of it, which is
> attention for this proposal I now miss...)
>
> Since I'm not proposing to modify the behavior of "used" apart from
> applying SHF_GNU_RETAIN to its section, I'm hoping the GCC side of
> things won't be too controversial.
>
> However, the assembler will have to support mis-matched section
> declarations, i.e.:
> .section .text,"ax",%progbits
> ...
> .section .text,"axR",%progbits
> ...
>
> The Binutils patch that supported this would create two separate .text
> sections in the assembled object file, one with SHF_GNU_RETAIN and one
> without.
> Perhaps they should be merged into a single .text section, with
> SHF_GNU_RETAIN applied to that merged section, so as to truly not
> interfere with "used" attribute behavior.
>
> There was an opinion that allowing these separate .section directives
> with the same name but different flags was undesirable.
>
> Personally, I don't see it as a problem, this exception is beneficial
> and makes sense, if the assembler merges the sections it is as if they
> all had the flag applied anyway.
So at a high level I agree with Pedro. As a developer, if I marked
something as used and the linker removed it, I'd be awful annoyed.
What I think it still debatable is the implementation. I think we
probably want a directive we pass to the assembler that says to keep the
symbol (ie, the .retain directive from the other thread).
The question then becomes how is .retain implemented. The easy way is
to have it change the section flags under the hood to ensure the section
is never garbage collected away. The other way would be to do deeper
surgery by making .retain mark the symbol and teach the linker about the
sematics of that new symbol flag.
Jeff
Re: [RFC] Add support for the "retain" attribute utilizing SHF_GNU_RETAIN
On Mon, Oct 26, 2020 at 07:08:06PM +, Pedro Alves via Gcc-patches wrote:
> On 10/6/20 12:10 PM, Jozef Lawrynowicz wrote:
>
> > Should "used" apply SHF_GNU_RETAIN?
> > ===
> > Another talking point is whether the existing "used" attribute should
> > apply the SHF_GNU_RETAIN flag to the containing section.
> >
> > It seems unlikely that a user applies the "used" attribute to a
> > declaration, and means for it to be saved from only compiler
> > optimization, but *not* linker optimization. So perhaps it would be
> > beneficial for "used" to apply SHF_GNU_RETAIN in some way.
> >
> > If "used" did apply SHF_GNU_RETAIN, we would also have to
> > consider the above options for how to apply SHF_GNU_RETAIN to the
> > section. Since the "used" attribute has been around for a while
> > it might not be appropriate for its behavior to be changed to place the
> > associated declaration in its own, unique section, as in option (2).
> >
>
> To me, if I use attribute((used)), and the linker still garbage
> collects the symbol, then the toolchain has a bug. Is there any
> use case that would suggest otherwise?
I revised the implementation so the "used" attribute will save the
symbol from garbage collection.
By implementing the TARGET_MARK_DECL_PRESERVED macro, a
".retain " directive will be emitted for decls that had the
"used" attribute applied.
GAS will set the SHF_GNU_RETAIN flag on sections containing symbols
referenced in ".retain" directives.
GAS still supports setting the "R" flag in .section directives, but GCC
won't emit these.
LD will save SHF_GNU_RETAIN sections from garbage collection.
For reference, I've attached the Binutils and GCC patches that implement
this. The results from a bootstrap and regtest for x86_64-pc-linux-gnu
and light testing for arm-eabi are looking good, but I need to add more
tests and clean the patches up before final submission.
Thanks!
Jozef
>
> Thanks,
> Pedro Alves
>
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 140a98594d..ffb75f7919 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1897,14 +1897,15 @@ struct output_elf_obj_tdata
bfd_boolean flags_init;
};
-/* Indicate if the bfd contains SHF_GNU_MBIND sections or symbols that
- have the STT_GNU_IFUNC symbol type or STB_GNU_UNIQUE binding. Used
- to set the osabi field in the ELF header structure. */
+/* Indicate if the bfd contains SHF_GNU_MBIND/SHF_GNU_RETAIN sections or
+ symbols that have the STT_GNU_IFUNC symbol type or STB_GNU_UNIQUE
+ binding. Used to set the osabi field in the ELF header structure. */
enum elf_gnu_osabi
{
elf_gnu_osabi_mbind = 1 << 0,
elf_gnu_osabi_ifunc = 1 << 1,
elf_gnu_osabi_unique = 1 << 2,
+ elf_gnu_osabi_retain = 1 << 3,
};
typedef struct elf_section_list
@@ -2034,7 +2035,7 @@ struct elf_obj_tdata
ENUM_BITFIELD (dynamic_lib_link_class) dyn_lib_class : 4;
/* Whether the bfd uses OS specific bits that require ELFOSABI_GNU. */
- ENUM_BITFIELD (elf_gnu_osabi) has_gnu_osabi : 3;
+ ENUM_BITFIELD (elf_gnu_osabi) has_gnu_osabi : 4;
/* Whether if the bfd contains the GNU_PROPERTY_NO_COPY_ON_PROTECTED
property. */
diff --git a/bfd/elf.c b/bfd/elf.c
index 9d7cbd52e0..8ec21d7705 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -1066,9 +1066,12 @@ _bfd_elf_make_section_from_shdr (bfd *abfd,
/* FIXME: We should not recognize SHF_GNU_MBIND for ELFOSABI_NONE,
but binutils as of 2019-07-23 did not set the EI_OSABI header
byte. */
-case ELFOSABI_NONE:
case ELFOSABI_GNU:
case ELFOSABI_FREEBSD:
+ if ((hdr->sh_flags & SHF_GNU_RETAIN) != 0)
+ elf_tdata (abfd)->has_gnu_osabi |= elf_gnu_osabi_retain;
+ /* Fall through */
+case ELFOSABI_NONE:
if ((hdr->sh_flags & SHF_GNU_MBIND) != 0)
elf_tdata (abfd)->has_gnu_osabi |= elf_gnu_osabi_mbind;
break;
@@ -12454,8 +12457,8 @@ _bfd_elf_final_write_processing (bfd *abfd)
i_ehdrp->e_ident[EI_OSABI] = get_elf_backend_data (abfd)->elf_osabi;
/* Set the osabi field to ELFOSABI_GNU if the binary contains
- SHF_GNU_MBIND sections or symbols of STT_GNU_IFUNC type or
- STB_GNU_UNIQUE binding. */
+ SHF_GNU_MBIND or SHF_GNU_RETAIN sections or symbols of STT_GNU_IFUNC type
+ or STB_GNU_UNIQUE binding. */
if (elf_tdata (abfd)->has_gnu_osabi != 0)
{
if (i_ehdrp->e_ident[EI_OSABI] == ELFOSABI_NONE)
@@ -12464,11 +12467,17 @@ _bfd_elf_final_write_processing (bfd *abfd)
&& i_ehdrp->e_ident[EI_OSABI] != ELFOSABI_FREEBSD)
{
if (elf_tdata (abfd)->has_gnu_osabi & elf_gnu_osabi_mbind)
- _bfd_error_handler (_("GNU_MBIND section is unsupported"));
+ _bfd_error_handler (_("GNU_MBIND section is supported only by GNU "
+ "and FreeBSD targets"));
if (elf_tdata (abfd)->has_gnu_osabi & elf_gnu_osabi_ifunc)
- _bfd_error_handler (_("symbol type STT_GNU_IFUNC is unsupported"));
+
Re: [RFC] Add support for the "retain" attribute utilizing SHF_GNU_RETAIN
On Mon, Oct 26, 2020 at 07:08:06PM +, Pedro Alves via Gcc-patches wrote:
> On 10/6/20 12:10 PM, Jozef Lawrynowicz wrote:
>
> > Should "used" apply SHF_GNU_RETAIN?
> > ===
> > Another talking point is whether the existing "used" attribute should
> > apply the SHF_GNU_RETAIN flag to the containing section.
> >
> > It seems unlikely that a user applies the "used" attribute to a
> > declaration, and means for it to be saved from only compiler
> > optimization, but *not* linker optimization. So perhaps it would be
> > beneficial for "used" to apply SHF_GNU_RETAIN in some way.
> >
> > If "used" did apply SHF_GNU_RETAIN, we would also have to
> > consider the above options for how to apply SHF_GNU_RETAIN to the
> > section. Since the "used" attribute has been around for a while
> > it might not be appropriate for its behavior to be changed to place the
> > associated declaration in its own, unique section, as in option (2).
> >
>
> To me, if I use attribute((used)), and the linker still garbage
> collects the symbol, then the toolchain has a bug. Is there any
> use case that would suggest otherwise?
>
> Thanks,
> Pedro Alves
>
I agree that "used" should imply SHF_GNU_RETAIN on whatever section
contains the declaration that the attribute is applied to. However, I
think that apart from the section flag being applied to the section, the
behaviour of "used" shouldn't be modified i.e. the declaration shouldn't
be put in a unique section.
I originally justified the addition of a "retain" attribute, alongside
"used" implying SHF_GNU_RETAIN, as indicating that the declaration
should be placed in it's own section. In hindsight, this is unnecessary;
if the user wants to increase the granularity of the portions of their
program being retained, they should build with
-f{function,data}-sections, or manually put the declaration in its own
section with the "section" attribute.
So we could shelve the "retain" attribute, and just modify the "used"
attribute to imply SHF_GNU_RETAIN. If we get consensus on that, I could
go ahead an implement it, but I never got any specific feedback on the
GCC behavior from anyone apart from you. I don't know whether to
interpret that lack of feedback, whilst the other aspects of the
implementation were commented on, as support for the "retain" attribute.
(I appreciate you giving that feedback in the Binutils discussions, and
should have engaged in those discussions more at the time. There was
just a lot of opinions flying about on many aspects of it, which is
attention for this proposal I now miss...)
Since I'm not proposing to modify the behavior of "used" apart from
applying SHF_GNU_RETAIN to its section, I'm hoping the GCC side of
things won't be too controversial.
However, the assembler will have to support mis-matched section
declarations, i.e.:
.section .text,"ax",%progbits
...
.section .text,"axR",%progbits
...
The Binutils patch that supported this would create two separate .text
sections in the assembled object file, one with SHF_GNU_RETAIN and one
without.
Perhaps they should be merged into a single .text section, with
SHF_GNU_RETAIN applied to that merged section, so as to truly not
interfere with "used" attribute behavior.
There was an opinion that allowing these separate .section directives
with the same name but different flags was undesirable.
Personally, I don't see it as a problem, this exception is beneficial
and makes sense, if the assembler merges the sections it is as if they
all had the flag applied anyway.
On Mon, Oct 26, 2020 at 07:12:45PM +, Pedro Alves via Gcc-patches wrote:
> On 10/6/20 12:10 PM, Jozef Lawrynowicz wrote:
> > The changes would also only affect targets
> > that support the GNU ELF OSABI, which would lead to inconsistent
> > behavior between non-GNU OS's.
>
> Well, a separate __attribute__((retain)) will necessarily only work
> on GNU ELF targets, so that just shifts the "inconsistent" behavior
> elsewhere.
True, a note in the documentation would cover this. For example:
"As a GNU ELF extension, the used attribute will also prevent the
linker from garbage collecting the section containing the symbol"
Thanks,
Jozef
Re: [RFC] Add support for the "retain" attribute utilizing SHF_GNU_RETAIN
On 10/6/20 12:10 PM, Jozef Lawrynowicz wrote: > The changes would also only affect targets > that support the GNU ELF OSABI, which would lead to inconsistent > behavior between non-GNU OS's. Well, a separate __attribute__((retain)) will necessarily only work on GNU ELF targets, so that just shifts the "inconsistent" behavior elsewhere. Pedro Alves
Re: [RFC] Add support for the "retain" attribute utilizing SHF_GNU_RETAIN
On 10/6/20 12:10 PM, Jozef Lawrynowicz wrote: > Should "used" apply SHF_GNU_RETAIN? > === > Another talking point is whether the existing "used" attribute should > apply the SHF_GNU_RETAIN flag to the containing section. > > It seems unlikely that a user applies the "used" attribute to a > declaration, and means for it to be saved from only compiler > optimization, but *not* linker optimization. So perhaps it would be > beneficial for "used" to apply SHF_GNU_RETAIN in some way. > > If "used" did apply SHF_GNU_RETAIN, we would also have to > consider the above options for how to apply SHF_GNU_RETAIN to the > section. Since the "used" attribute has been around for a while > it might not be appropriate for its behavior to be changed to place the > associated declaration in its own, unique section, as in option (2). > To me, if I use attribute((used)), and the linker still garbage collects the symbol, then the toolchain has a bug. Is there any use case that would suggest otherwise? Thanks, Pedro Alves
[RFC] Add support for the "retain" attribute utilizing SHF_GNU_RETAIN
Hi, I'd like to propose a new "retain" attribute, which can be applied to function and variable declarations. The attribute is used to protect the function or variable declaration it is applied to from linker garbage collection, by applying the SHF_GNU_RETAIN section flag to the section containing it. This flag is a GNU OSABI ELF extension. The SHF_GNU_RETAIN flag was discussed on the GNU gABI mailing list here: https://sourceware.org/pipermail/gnu-gabi/2020q3/000429.html The Binutils patch for SHF_GNU_RETAIN was discussed in the following threads: https://sourceware.org/pipermail/binutils/2020-September/113406.html https://sourceware.org/pipermail/binutils/2020-September/113499.html The Binutils patch is still being iterated on, and I'd like to get some feedback on one particular aspect of the GCC functionality before finalizing the Binutils side of things. When the "retain" attribute is applied to a declaration, there are three ways to apply the SHF_GNU_RETAIN flag to the section containing the declaration: (1) Mark the entire section containing the declaration with the SHF_GNU_RETAIN flag (2) Place the declaration in a new, uniquely named section with SHF_GNU_RETAIN set. (3) Place the declaration in a new section with its default name, and SHF_GNU_RETAIN set. I think that (2) is the best option, as it most closely corresponds to the behavior the user wants to apply by using the "retain" attribute. That is, only the declaration itself needs to be retained. Option (3) has the same advantage, however it requires some non-standard behavior in the assembler to support. The assembler would normally emit an error if two input sections have the same name, but different flags set. At the moment, SHF_GNU_RETAIN is an exception to this, but there is no fundamental reason that this exception is required, as the associated behavior can be fully supported by just giving the section a unique name. As far as I'm aware, option (1) would be tricky to support in GCC. We'd have to examine all the declarations within a section before the first assembler directive to create a section is created, which isn't really compatible with the current, linear nature of the assembly output stream. I guess there's probably something we could do in the middle-end to set a flag somewhere to catch this without it getting too complicated. However, it would also lead to large portions of the program being unnecessarily retained in the linked file, when only one declaration was required. If anyone has any strong opinions that option (2) isn't the best choice for the "retain" attribute, please let me know. I plan on finalizing the Binutils patch in the coming days, removing the added support for unique input sections with the same name, but different states for the SHF_GNU_RETAIN flag, which is required for option (3). Should "used" apply SHF_GNU_RETAIN? === Another talking point is whether the existing "used" attribute should apply the SHF_GNU_RETAIN flag to the containing section. It seems unlikely that a user applies the "used" attribute to a declaration, and means for it to be saved from only compiler optimization, but *not* linker optimization. So perhaps it would be beneficial for "used" to apply SHF_GNU_RETAIN in some way. If "used" did apply SHF_GNU_RETAIN, we would also have to consider the above options for how to apply SHF_GNU_RETAIN to the section. Since the "used" attribute has been around for a while it might not be appropriate for its behavior to be changed to place the associated declaration in its own, unique section, as in option (2). However, I tested this "used" attribute modification on x86_64-pc-linux-gnu, and there was only a small number of regressions (27 PASS->FAIL, from 6 tests) across the GCC and G++ testsuites. I briefly investigated these, and some failures are just due to a change in the expected output of tests, but also some real errors from issues with hot/cold function partitioning. I believe those would just require some additional functional changes, and there isn't anything fundamentally broken. So nothing that can't be worked around, but I am more concerned about the wider impact of changing the attribute, which is not represented by this small subset of testing. The changes would also only affect targets that support the GNU ELF OSABI, which would lead to inconsistent behavior between non-GNU OS's. Perhaps this isn't an issue since we can just document it in the description for the "used" attribute: As a GNU ELF extension, the declaration the "used" attribute is applied to will be placed in a new, uniquely named section with the SHF_GNU_RETAIN flag applied. I think that unless "used" creates a new, uniquely named SHF_GNU_RETAIN section for a declaration, there is merit to having a separate "retain" attribute that has this behavior. To summarize the talking points: - Any downsides to the new "retain" attribute creating a new, uniquely
