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

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

On Fri, Oct 02, 2020 at 11:41:10PM +0930, Alan Modra via Binutils wrote:
> On Fri, Oct 02, 2020 at 01:30:23PM +0100, Jozef Lawrynowicz wrote:
> > On Thu, Oct 01, 2020 at 09:09:07PM +0930, Alan Modra via Binutils wrote:
> > > On Thu, Oct 01, 2020 at 11:50:33AM +0100, Jozef Lawrynowicz wrote:
> > > > --- a/bfd/elflink.c
> > > > +++ b/bfd/elflink.c
> > > > @@ -14102,7 +14102,8 @@ bfd_elf_gc_sections (bfd *abfd, struct bfd_link_info *info)
> > > >  			|| (elf_section_data (o)->this_hdr.sh_type
> > > >  			    == SHT_FINI_ARRAY)))
> > > >  		|| (elf_section_data (o)->this_hdr.sh_type == SHT_NOTE
> > > > -		    && elf_next_in_group (o) == NULL )))
> > > > +		    && elf_next_in_group (o) == NULL)
> > > > +		|| (elf_section_flags (o) & SHF_GNU_RETAIN)))
> > > 
> > > Flag bits in SHF_MASKOS depend on OS, so this needs a test of OSABI.
> > > That can be done by checking elf_tdata (sub)->has_gnu_osabi for the
> > > appropriate bit.
> > > 
> > 
> > Fixed the two OSABI issues in the attached patch.
> 
> I didn't mention it, but readelf has two occurrences of decoding
> SHF_GNU_UNIQUE.  They both need fixing, not just the first one.

Well in fact, the part I added OSABI handling to already in readelf
wasn't correct anyway, those nested switch/if-else/switch statements
caught me out ;)
Cleaning that up is a task for some other time, however.

I started adding tests to validate the readelf output with/without "-N",
when the numeric values for SHF_GNU_{RETAIN,MBIND} are set, for targets
which neither error when these bits are set (ELFOSABI_STANDALONE) nor
handle them as their GNU OSABI values (ELFOSABI_{NONE,GNU,FREEBSD}).

However, hppa-unknown-elf is the only target I've been testing that
satisfies these constraints (ELFOSABI_HPUX), so I'm going to find some
others to validate the changes and finish this off next week.

> 
> > > > +* Add support for the SHF_GNU_RETAIN ELF section flag.
> > > > +  This flag specifies that the section should not be garbage collected by the
> > > > +  linker if it is unused.
> > > 
> > > I would drop "if it is unused".  The phrase doesn't really add
> > > anything to a user's understanding.
> > > 
> > 
> > I don't really have a strong opinion either way, but I thought "if it is
> > unused" might be helpful to add to the description since "garbage
> > collection" is not precisely defined in ELF.
> 
> The reason I don't particularly like "if it is unused" is that it
> sounds like something said by someone who doesn't really understand
> linker garbage collection.  "used" and "referenced by a relocation
> from a kept section" are only loosely related.  And indeed the whole
> purpose of SHF_GNU_RETAIN is surely to keep needed sections, ones that
> are used for some purpose.

Sounds fair to me. The precise ELF definition is targeting technical
users after all.

Thanks,
Jozef

> 
> -- 
> Alan Modra
> Australia Development Lab, IBM