Re: Fix undefined weak hidden syms for MIPS shared libraries

[Richard Sandiford <rdsandiford at googlemail dot com>]

Daniel Jacobowitz <drow@false.org> writes:
> This patch goes along with the testcase in my previous message.  We
> were always emitting a dynamic relocation for even undefined weak
> symbols, but nothing marked them as dynamic, so the relocation got
> symndx 0xffffff.  The consensus seems to be that relocations should be
> emitted in shared libraries if default visibility, and clearly no
> relocation is necessary for non-default visibility, so that's what
> I've implemented.
>
> Like other ports, MIPS now needs an allocate_dynrelocs routine.
> adjust_dynamic_symbol is not good enough, since the symbol might not
> be dynamic.

I'm probably being dense, but why might it not be dynamic?  Are you
thinking about the post-non-PIC code or the current code?

In the current code, I'd have expected mips_elf_create_got_section
to be called by this point.  We can't go adding new dynamic
GOT-referenced symbols in size_dynamic_sections, so even if the
new bfd_elf_link_record_dynamic_symbol call is currently dead,
it looks like dangerous dead code.  I don't see why the first
two hunks aren't currently enough.

As far as the implementation goes:

> +  /* VxWorks executables are handled elsewhere; we only need to
> +     allocate relocations in shared objects.  */
> +  if (htab->is_vxworks && !info->shared)
> +    return TRUE;

Where's "elsewhere" in this case?

> +      if (h->root.type == bfd_link_hash_undefweak)
> +	{
> +	  /* Do not copy relocations for undefined weak symbols with
> +	     non-default visibility.  */
> +	  if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT)
> +	    do_copy = FALSE;
> +
> +	  /* Make sure undefined weak symbols are output as a dynamic
> +	     symbol in PIEs.  */
> +	  else if (h->dynindx == -1 && !h->forced_local)
> +	    {
> +	      if (! bfd_elf_link_record_dynamic_symbol (info, h))
> +		return FALSE;
> +	    }

This implies that "h->forced_local && ELF_ST_VISIBILITY (h->other)
== STV_DEFAULT" is a valid combination.  Is it one we can really
create?  If so, why do want do_copy to be true in that case?

Richard