[PATCH] elf: Drop GRP_COMDAT when localizing a global symbol

[Alan Modra]

On Tue, Dec 07, 2021 at 05:24:12AM -0800, H.J. Lu wrote:
> On Tue, Dec 7, 2021 at 5:11 AM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Tue, Dec 07, 2021 at 04:46:29AM -0800, H.J. Lu wrote:
> > > On Tue, Dec 7, 2021 at 12:00 AM Alan Modra <amodra@gmail.com> wrote:
> > > >
> > > > On Mon, Dec 06, 2021 at 08:54:42AM -0800, H.J. Lu via Binutils wrote:
> > > > > --- a/binutils/objcopy.c
> > > > > +++ b/binutils/objcopy.c
> > > > > @@ -1696,6 +1696,21 @@ filter_symbols (bfd *abfd, bfd *obfd, asymbol **osyms,
> > > > >                     && ! is_specified_symbol (name, keepglobal_specific_htab))
> > > > >                 || (localize_hidden && is_hidden_symbol (sym))))
> > > > >           {
> > > > > +           if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
> > > > > +             {
> > > > > +               sec_ptr s = sym->section;
> > > > > +               if (s != NULL
> > > > > +                   && elf_section_data (s) != NULL
> > > > > +                   && elf_group_name (s) != NULL)
> > > > > +                 {
> > > > > +                   /* When a global symbol in a COMDAT group was
> > > > > +                      changed to local, drop GRP_COMDAT on the
> > > > > +                      group.  */
> > > > > +                   s = elf_sec_group (s);
> > > > > +                   if (s != NULL && s->output_section != NULL)
> > > > > +                     s->output_section->flags &= ~SEC_LINK_ONCE;
> > > > > +                 }
> > > > > +             }
> > > > >             sym->flags &= ~ (BSF_GLOBAL | BSF_WEAK);
> > > > >             sym->flags |= BSF_LOCAL;
> > > > >           }
> > > >
> > > > I don't think this is quite correct.  Surely you only want to change
> > > > the group semantics if the group symbol changes?  This patch would
> > > > change group semantics if *any* symbol defined in the group is
> > > > localised.
> > >
> > > This is done on purpose.  When any global symbol in a comdat group
> > > is localized, this comdat group is no longer interchangeable with other
> > > comdat groups of the same group signature.
> >
> > The ELF gABI says of GRP_COMDAT that "duplication is defined as having
> > the same group signature".  There is no other requirement.  Things
> > that you might imagine are necessary to make groups work in practice
> 
> It is not my imagination:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59842
> 
> > are not relevant.  objcopy is a tool that can destroy object files.
> >
> > Besides that, someone may be using objcopy on multiple object files
> > localising one symbol for every object, leaving all the comdat groups
> > with a given signature the same.
> >
> > Please read https://groups.google.com/g/generic-abi/c/2X6mR-s2zoc
> > again.  The only symbol change that should affect the comdat status of
> > the group is the group symbol.
> 
> >From Roland in the URL above
> 
> A. In step #2, merge COMDAT groups so all the desired merging from the
> step #1 translation units is complete before step #3.
>     In step #3, have objcopy notice that it's localizing a symbol
> that's used as a COMDAT signature symbol and clear the GRP_COMDAT bit
> for that group.
>     Now step #4 will not be doing any merging of the #1->#2 inputs but
> will do all appropriate merging among the other inputs to step #4 (and
> any non-localized symbols left after step #3).
> 
> >From Ali:
> 
> To sum up, it sounds to me like objcopy failing to remove the
> GRP_COMDAT as part of step 3 in this process was the bug, and
> that your solution A is the right fix.
> 
> My patch drops GRP_COMDAT.

The patch is wrong.  Note that the signature symbol for a group does
not even need to be defined in sections that are part of the group.

-- 
Alan Modra
Australia Development Lab, IBM