Re: Another MIPS multigot patch

[Richard Sandiford <rsandifo at redhat dot com>]

Daniel Jacobowitz <drow@mvista.com> writes:
>> So why isn't it possible to decay on a per-GOT basis?  Objects that
>> use the primary GOT (print-rtl) can use a page entry.  Objects that use
>> toplev's GOT can use the global entry in that GOT.  Not that there's any
>> benefit to doing so -- they both might as well use page entries if the
>> symbol binds locally -- but I don't see why "we can't".
>
> Remember, we're in check_relocs here.

Ah.  I wasn't ;)  I realise the comment you quoted was from check_relocs,
but check_relocs doesn't make the final decision about whether a GOT_PAGE
will decay, because it can't know in general.  I think we're in violent
agreement on that.

So I thought you were saying it was impossible for calculate_relocation()
to decide on a per-GOT basis.  Whereas, AFAICT, it can.  Sorry about that.

>> - Are the !h and h->root.dynindx checks still needed after this?
>>   (Not sure off-hand.)
>
> The !h test is no longer necessary.  The dynindx check still is.
> h->root.dynindx == -1 will be handled by SYMBOL_REFERENCES_LOCAL, but
> otherwise (if info->shared) a low dynindx and a high dynindx will be
> both return FALSE.

I don't know what you mean here.  Do you have an example?

As I understand it, the whole point of your change is that if a symbol
binds locally, we can use a normal page/ofst access for it, regardless
of whether we happened to allocate a global entry as well.  So in what
cases will _bfd_elf_symbol_refs_local_p return true for something that
doesn't bind locally?

>> - Will this fix the problem even for protected symbols?
>>   A direct call to _bfd_elf_symbol_refs_local_p (h, info, 1)
>>   seems more correct.
>
> That's SYMBOL_CALLS_LOCAL; not the clearest name for this circumstance.

Exactly, that's why I suggested calling the function directly. ;)
The two macros are set up for a distinction that doesn't really
apply on MIPS.

>> - Is the hmips->root.dynindx == -1 check above still needed after
>>   your patch?
>
> In check_relocs?  I'm not sure.  Certainly anything touching dynindx at
> this point is suspect - remember, again, that check_relocs gets called
> on a per-input-file basis, so this code is sensitive to the order of
> input files.

Well, OK, turn the question around then: under what circumstances do you
think this test is still needed?  If the rest of the condition is only
true for symbols that are known to bind locally (and I think that should
be the case, otherwise the condition is buggy), then with your change to
calculate_relocations(), the condition should be safe.

Richard