This is the mail archive of the
mailing list for the binutils project.
Re: Another MIPS multigot patch
On Thu, Jan 29, 2004 at 05:39:56PM +0000, Richard Sandiford wrote:
> Daniel Jacobowitz <firstname.lastname@example.org> 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.
Indeed, we agree.
> >> - 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?
Well, at the basic level, for a symbol with dynindx > 0 and dynindx <
mips_elf_get_global_gotsym SYMBOL_REFERENCES_LOCAL will return false
> 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?
It's the other way around, _bfd_elf_symbol_refs_local_p will (may)
return false for something that we would previously have accessed using
GOT_PAGE. Iff we are linking a shared library. Now, on most targets
the symbol could be overridden at this point. So maybe the existing
test of dynindx is downright incorrect.
> >> - 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.
Except it's the sort of thing I'd go around and blindly clean up later.
I try not to introduce things that I would then clean up :) Perhaps a
> >> - 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.
I think the whole section is broken. Let's start at the top:
while (hmips->root.root.type == bfd_link_hash_indirect
|| hmips->root.root.type == bfd_link_hash_warning)
hmips = (struct mips_elf_link_hash_entry *)
if ((hmips->root.root.type == bfd_link_hash_defined
|| hmips->root.root.type == bfd_link_hash_defweak)
If the symbol has been defined, OK. But we're in check_relocs. Here's
some sample code:
extern int foo_func(void);
extern int foo;
Try "mips64-linux-ld -o main main.o func.o" and "mips64-linux-ld -o
main func.o main.o". In one of them, root.root.type is
bfd_link_hash_undefined. In the other, it's bfd_link_hash_common. So,
things are already wrong, since we don't check for common. If I
initialize it, then in one case it becomes bfd_link_hash_defined.
Now, in one link order mips_elf_record_global_got_symbol gets called,
and in the other it doesn't.
I'd rather fix the one thing I know how to fix than dig neck-deep into
this mess I don't understand :) But I suspect this entire segment
should just be ripped out, since it demonstrably doesn't do whatever it
is that it's supposed to do.
MontaVista Software Debian GNU/Linux Developer