Re: [PATCH] PR ld/19579/21306 Properly turn common symbol into definition

[Alan Modra <amodra at gmail dot com>]

On Thu, Apr 06, 2017 at 07:38:54AM -0700, H.J. Lu wrote:
> On Thu, Apr 6, 2017 at 3:43 AM, Alan Modra <amodra@gmail.com> wrote:
> > On Wed, Apr 05, 2017 at 09:46:38AM -0700, H.J. Lu wrote:
> >> +bfd_boolean
> >> +_bfd_elf_define_common_symbol (bfd *output_bfd,
> >> +                            struct bfd_link_info *info,
> >> +                            struct bfd_link_hash_entry *h)
> >> +{
> >> +  bfd_boolean ret
> >> +    = bfd_generic_define_common_symbol (output_bfd, info, h);
> >> +  if (ret)
> >> +    {
> >> +      /* Since def_regular may not be set if it is overridden by a
> >> +      dynamic definition, we need to set def_regular when it is
> >> +      converted to a defined symbol.  */
> >> +      struct elf_link_hash_entry *eh
> >> +     = (struct elf_link_hash_entry *) h;
> >> +      eh->def_regular = 1;
> >> +    }
> >> +  return TRUE;
> >> +}
> >
> > This fails for ELF targets using the generic linker, like d30v-elf.
> 
> Does such target support shared object, which is the issue here?

No, the issue is that the above code uses a cast that is invalid.  The
generic linker does not use a hash table with elf_link_hash_entry (or
superclass) elements.

> > Isn't the real bug that somewhere we have code lacking an
> > ELF_COMMON_DEF_P test?
> 
> ELF targets check def_regular for definition in relocatable object.
> It should be set for all definitions in relocatable objects

Please read the ELF_COMMON_DEF_P comment.  I disagree that it is
*necessary* to set def_regular when a common symbol becomes a
definition.  It may be desirable to set the flag, to simplify other
code, if there is no place where we want to distinguish symbols that
actually are definitions in relocatable object files from symbols that
*become* definitions.  I'd need to be convinced of that before
accepting a patch that sets def_regular for commons.

-- 
Alan Modra
Australia Development Lab, IBM