Re: Fix MIPS multi-GOT -Bsymbolic link failure

[Richard Sandiford <rdsandiford at googlemail dot com>]

"Joseph S. Myers" <joseph@codesourcery.com> writes:
> This patch fixes a link failure for MIPS multi-GOT with -Bsymbolic.
>
> The testcase, as added to the testsuite, is the existing multi-got-1
> testcase but with -Bsymbolic added to the linker options.  Symbols
> that are only binding locally because of -Bsymbolic get global rather
> than local GOT entries - the code is very complicated, but I must
> presume it is deliberate that not all symbols binding locally (as
> indicated by SYMBOL_REFERENCES_LOCAL or _bfd_elf_symbol_refs_local_p)
> get local entries, given that for example
> mips_elf_calculate_relocation sets local_p based on
> _bfd_elf_symbol_refs_local_p only for some types of relocations.

Hmm, that's probably a dangerous assumption.  A lot of this code
was written before SYMBOL_REFERENCES_LOCAL and does look suspicious...

There's also the problem that the choice of GOT entry type needs
to match the choice we made when laying out the GOT.  That used
to be difficult, because we'd create an initial GOT layout fairly
early on, then adjust it when symbols were forced local.  That's
probably why there are forced_local checks where you might expect
something like SYMBOL_REFERENCES_LOCAL.  The later GOT layout that
we now use should make it easier to get things right.

> _bfd_mips_elf_finish_dynamic_symbol iterates over global GOT entries,
> and ends up calling mips_elf_create_dynamic_relocation with a NULL sec
> value, while the code in mips_elf_create_dynamic_relocation checks
> SYMBOL_REFERENCES_LOCAL and on that basis reaches code requiring a
> non-NULL sec.  In turn the link fails with "final link failed: Bad
> value".  This patch arranges for NULL sec to go through the global
> rather than the local case.

I'm not sure this will correctly preserve the -Bsymbolic semantics.
We really don't want the relocation to be against a global symbol,
since the ABI requires any global symbols involved in relocations
to have a global GOT entry, which in turn means we'll end up using
the usual -Bdynamic value of the symbol.

I'll try to have a deeper look over the weekend, but I think you're
right that some checks of forced_local should really be testing
SYMBOL_REFERENCES_LOCAL instead.  The first setting of local_p in
mips_elf_calculate_relocation is fine, since it's determining the
correct interpretion of the input relocation, and that depends on
the original STB_* binding for some relocation types.  But when
laying out the GOT, and when using the second local_p to calculate
the new relocation, I think SYMBOL_REFERENCES_LOCAL should decide
whether we use local (non-page) or global GOT entries.  I'm also
not sure that SYMBOL_CALLS_LOCAL is used correctly, or at least
optimally, for traditional call stubs.

Richard