This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: Another MIPS multigot patch
- From: Richard Sandiford <rsandifo at redhat dot com>
- To: binutils at sources dot redhat dot com
- Date: Thu, 29 Jan 2004 15:55:45 +0000
- Subject: Re: Another MIPS multigot patch
- References: <20031121221758.GA2371@nevyn.them.org> <87he0we569.fsf@redhat.com><20031122173959.GA6247@nevyn.them.org> <874qwwusg1.fsf@redhat.com><20031123012406.GA11321@nevyn.them.org><20040129144621.GA18176@nevyn.them.org>
Daniel Jacobowitz <drow@mvista.com> writes:
> On Sat, Nov 22, 2003 at 08:24:06PM -0500, Daniel Jacobowitz wrote:
>> On Sat, Nov 22, 2003 at 07:14:06PM +0000, Richard Sandiford wrote:
>> > (a) _bfd_mips_elf_check_relocs assumes print-rtl.c can use a local
>> > page entry for GOT_PAGE/GOT_OFST accesses to flag_dump_unnumbered.
>> > f_d_u is therefore not entered into print-rtl's got_entries.
>> >
>> > (b) toplev uses GOT_DISP, so we end up needing a global GOT
>> > entry for f_d_u.
>>
>> 000000001028 00a900000012 R_MIPS_64 0000000000000000 flag_dump_unnumbered + 0
>> Type2: R_MIPS_NONE
>> Type3: R_MIPS_NONE
>>
>> So, close enough for our purposes.
>>
>> > (c) Because of (a), no bfd that uses the primary GOT seems to need
>> > an entry for f_d_u. We therefore give it a higher index than
>> > symbols that _do_ need an entry.
>> >
>> > (d) Because of (c), mips_elf_calculate_relocation assumes that f_d_u
>> > is !local_p, and it therefore decays the GOT_PAGE reloc into a
>> > GOT_DISP. So print-rtl uses f_d_u's GOT entry after all.
>> >
>> > If so, then IMO the bug is with (d). We can still use local pages
>> > for print-rtl, despite the entry in .dynsym.
>>
>> This all makes sense to me. I'll think about what the local_p
>
> Notice this in _bfd_mips_elf_check_relocs:
> /* If we've encountered any other relocation
> referencing the symbol, we'll have marked it as
> dynamic, and, even though we might be able to get
> rid of the GOT entry should we know for sure all
> previous relocations were GOT_PAGE ones, at this
> point we can't tell, so just keep using the
> symbol as dynamic. This is very important in the
> multi-got case, since we don't decide whether to
> decay GOT_PAGE to GOT_DISP on a per-GOT basis: if
> the symbol is dynamic, we'll need a GOT entry for
> every GOT in which the symbol is referenced with
> a GOT_PAGE relocation. */
I'm not really sure what your point is here. FWIW, this was the code I
was referring to in (a), on the assumption that the following condition:
&& hmips->root.dynindx == -1)
is true. Is that right?
> Fortunately, the comment is misleading. We can't decide to decay on a
> per-GOT basis, but we can decide not to decay even later, on a
> per-symbol basis.
I don't really follow this explanation. From what I can remember
of the test case, my assumption was:
- print-rtl uses the primary GOT
- toplev uses a secondary GOT
- toplev needs a global GOT entry for f_d_u, print-rtl doesn't
- nothing else uses f_d_u
- we put f_d_u after the global entries for the primary GOT
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".
> @@ -3247,12 +3248,16 @@
> {
> case R_MIPS_GOT_PAGE:
> case R_MIPS_GOT_OFST:
> - /* If this symbol got a global GOT entry, we have to decay
> - GOT_PAGE/GOT_OFST to GOT_DISP/addend. */
> + /* If this symbol got a global GOT entry, we might have to decay
> + GOT_PAGE/GOT_OFST to GOT_DISP/addend. We check whether we need
> + to decay, because if we don't need to it's possible that no GOT
> + entry was allocated in this input BFD's GOT for this reference
> + (it might be in some other GOT, and out of range). */
> local_p = local_p || ! h
> || (h->root.dynindx
> < mips_elf_get_global_gotsym_index (elf_hash_table (info)
> - ->dynobj));
> + ->dynobj))
> + || SYMBOL_REFERENCES_LOCAL (info, h);
> if (local_p || r_type == R_MIPS_GOT_OFST)
> break;
> /* Fall through. */
A few questions: (probably dumb ones ;)
- Are the !h and h->root.dynindx checks still needed after this?
(Not sure off-hand.)
- 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.
- Is the hmips->root.dynindx == -1 check above still needed after
your patch?
Richard