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

[H.J. Lu]

On Thu, Apr 6, 2017 at 8:18 AM, Alan Modra <amodra@gmail.com> wrote:
> 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.
>

Here is  a patch to use ELF_COMMON_DEF_P.



-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ELF-Check-ELF_COMMON_DEF_P-for-common-symbols.patch
Type: text/x-patch
Size: 3933 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20170406/9e882aad/attachment.bin>