PATCH: PR ld/14170: ld: assertion fail bfd/linker.c:641

[H.J. Lu]

On Sun, May 27, 2012 at 4:06 AM, Alan Modra <amodra@gmail.com> wrote:
> On Sat, May 26, 2012 at 07:59:37AM -0700, H.J. Lu wrote:
>> @@ -1217,7 +1217,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
>>       }
>>
>>        if ((h->root.u.undef.next || info->hash->undefs_tail == &h->root)
>> -       && bfd_is_und_section (sec))
>> +       && !newdef)
>
> This is the same logic as the patch I added to your bugzilla, stating
> that it wasn't a complete fix.
>
> As best I can see, this is the correct fix.  Please review.
>
>        PR ld/14170
>        * elflink.c (_bfd_elf_merge_symbol): When a symbol defined in
>        a dynamic library finds a new instance with non-default
>        visibility in a regular object, correctly handle symbols
>        already on the undefs list and undo dynamic symbol state when
>        the new symbol is hidden or internal.
>
> Index: bfd/elflink.c
> ===================================================================
> RCS file: /cvs/src/src/bfd/elflink.c,v
> retrieving revision 1.445
> diff -u -p -r1.445 elflink.c
> --- bfd/elflink.c       25 May 2012 01:12:19 -0000      1.445
> +++ bfd/elflink.c       27 May 2012 10:42:33 -0000
> @@ -1216,15 +1216,15 @@ _bfd_elf_merge_symbol (bfd *abfd,
>            h = *sym_hash;
>        }
>
> -      if ((h->root.u.undef.next || info->hash->undefs_tail == &h->root)
> -         && bfd_is_und_section (sec))
> +      /* If the old symbol was undefined before, then it will still be
> +        on the undefs list.  If the new symbol is undefined or
> +        common, we can't make it bfd_link_hash_new here, because new
> +        undefined or common symbols will be added to the undefs list
> +        by _bfd_generic_link_add_one_symbol.  Symbols may not be
> +        added twice to the undefs list.  Also, if the new symbol is
> +        undefweak then we don't want to lose the strong undef.  */
> +      if (h->root.u.undef.next || info->hash->undefs_tail == &h->root)
>        {
> -         /* If the new symbol is undefined and the old symbol was
> -            also undefined before, we need to make sure
> -            _bfd_generic_link_add_one_symbol doesn't mess
> -            up the linker hash table undefs list.  Since the old
> -            definition came from a dynamic object, it is still on the
> -            undefs list.  */
>          h->root.type = bfd_link_hash_undefined;
>          h->root.u.undef.abfd = abfd;
>        }
> @@ -1234,11 +1234,21 @@ _bfd_elf_merge_symbol (bfd *abfd,
>          h->root.u.undef.abfd = NULL;
>        }
>
> -      if (h->def_dynamic)
> +      if (ELF_ST_VISIBILITY (sym->st_other) != STV_PROTECTED)
> +       {
> +         /* If the new symbol is hidden or internal, completely undo
> +            any dynamic link state.  */
> +         (*bed->elf_backend_hide_symbol) (info, h, TRUE);
> +         h->forced_local = 0;
> +         h->def_dynamic = 0;
> +         h->ref_dynamic = 0;
> +       }
> +      else if (h->def_dynamic)

It looks OK, except for " if (h->def_dynamic)".  h->def_dynamic is
always true when we get it.

>        {
>          h->def_dynamic = 0;
>          h->ref_dynamic = 1;
>        }
> +      h->dynamic_def = 0;
>       /* FIXME: Should we check type and size for protected symbol?  */
>       h->size = 0;
>       h->type = 0;
>

Thanks.


-- 
H.J.