Re: Another MIPS multigot patch

[Daniel Jacobowitz <drow at mvista dot com>]

Still looking at this bug... just very slowly.  More below.

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:
> > Daniel Jacobowitz <drow@mvista.com> writes:
> > > We're in mips_elf_global_got_index, looking at this symbol.  It has
> > > h->dynindx of around 3500, and global_got_dynindx is 5.  g->local_gotno
> > > is about 5000.  So we have an overflow; that's more than 8K 8-byte
> > > entries.  Are you saying that the fact that there are R_MIPS_GOT_PAGE
> > > entries against flag_dump_unnumbered using the primary GOT means it
> > > should have a lower dynindx?
> > 
> > Well, there are two ways we can handle the relocation:
> > 
> >     1. Decay to R_MIPS_GOT_DISP.  In that case, yes, the index
> >        should be lower.
> > 
> >     2. Use a local page entry.
> > 
> > (2) should be possible in this case, and looking at the code,
> > it does seem like we try to support it.
> > 
> > It's difficult to be sure without access to the test case, so this is
> > probably completely wrong.  But I'm guessing the problem is:
> 
> I can package up the testcase if you want.  It's a bit large.  In the
> mean time, I'll look at this more tomorrow or Monday but:
> 
> >     (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.  */

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.  The attached patch seems plausible to me, and
generates a correct reference in the cc1 binary which failed to link
before.  N64 glibc and GCC both work with the new linker, and
-fdump-unnumbered (the affected variable) also works.

OK?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2004-01-29  Daniel Jacobowitz  <drow@mvista.com>

	* elfxx-mips.c (mips_elf_calculate_relocation): Allow
	SYMBOL_REFERENCES_LOCAL to decay GOT_PAGE/GOT_OFST references.

--- binutils-2.14/bfd/elfxx-mips.c.orig	2004-01-28 16:16:06.000000000 -0500
+++ binutils-2.14/bfd/elfxx-mips.c	2004-01-28 17:52:08.000000000 -0500
@@ -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.  */