public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Another MIPS multigot patch
@ 2003-11-21 22:17 Daniel Jacobowitz
  2003-11-22 16:30 ` Richard Sandiford
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Jacobowitz @ 2003-11-21 22:17 UTC (permalink / raw)
  To: binutils

Currently I can't build an n64 GCC (with my other patches applied to the
linker).  The error is an R_MIPS_GOT_PAGE overflow for a common variable
(flag_dump_unnumbered?).  Here's the problem:

  if (1)
    {
      gg->assigned_gotno = gg->global_gotno - g->global_gotno;
      g->global_gotno = gg->global_gotno;
      set_got_offset_arg.value = 2;
    }

With this, global_gotno increases by a substantial amount.  If the primary
GOT was full, then the odds are good that there is now a 16-bit relocation
in the primary GOT that can no longer be resolved.  Essentially, we're
accessing two GOTs (the "master" GOT and the "primary" GOT) using the same
$gp value.

The patch below implements the obvious fix if my understanding is correct:
consider the number of global symbols in the primary GOT to be the total
number of global symbols.  If it goes over the limit of GOT size, we'll
create a dummy primary GOT with no local symbols and no input BFD using it,
to satisfy the dynamic linker.  So everything should work out OK.

Tested on mips64el-linux-gnu, lightly.  I'll do more thorough testing after
I have a chance to revise my previous patch to Richard's comment.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-11-21  Daniel Jacobowitz  <drow@mvista.com>

	* elfxx-mips.c (struct mips_elf_got_per_bfd_arg): Add global_count.
	(mips_elf_merge_gots): Use arg->global_count instead of the primary
	GOT's global_gotno.
	(mips_elf_multi_got): Set got_per_bfd_arg.global_count.

--- src/bfd/elfxx-mips.c.orig	2003-11-21 15:13:04.000000000 -0500
+++ src/bfd/elfxx-mips.c	2003-11-21 15:50:25.000000000 -0500
@@ -121,7 +121,11 @@ struct mips_elf_got_per_bfd_arg
   /* The maximum number of got entries that can be addressed with a
      16-bit offset.  */
   unsigned int max_count;
-  /* The number of local and global entries in the primary got.  */
+  /* The number of global entries in the master GOT (i.e. including
+     entries otherwise unreferenced in the primary GOT).  */
+  unsigned int global_count;
+  /* The number of local and global entries in the primary and master
+     GOTs.  */
   unsigned int primary_count;
   /* The number of local and global entries in the current got.  */
   unsigned int current_count;
@@ -2271,16 +2275,15 @@ mips_elf_merge_gots (bfd2got_, p)
   
   /* If we don't have a primary GOT and this is not too big, use it as
      a starting point for the primary GOT.  */
-  if (! arg->primary && lcount + gcount <= maxcnt)
+  if (! arg->primary && lcount + arg->global_count <= maxcnt)
     {
       arg->primary = bfd2got->g;
       arg->primary_count = lcount + gcount;
     }
   /* If it looks like we can merge this bfd's entries with those of
-     the primary, merge them.  The heuristics is conservative, but we
+     the primary, merge them.  The heuristic is conservative, but we
      don't have to squeeze it too hard.  */
-  else if (arg->primary
-	   && (arg->primary_count + lcount + gcount) <= maxcnt)
+  else if (arg->primary && (arg->primary_count + lcount) <= maxcnt)
     {
       struct mips_got_info *g = bfd2got->g;
       int old_lcount = arg->primary->local_gotno;
@@ -2302,8 +2305,7 @@ mips_elf_merge_gots (bfd2got_, p)
       BFD_ASSERT (old_lcount + lcount >= arg->primary->local_gotno);
       BFD_ASSERT (old_gcount + gcount >= arg->primary->global_gotno);
 
-      arg->primary_count = arg->primary->local_gotno
-	+ arg->primary->global_gotno;
+      arg->primary_count = arg->primary->local_gotno + arg->global_count;
     }
   /* If we can merge with the last-created got, do it.  */
   else if (arg->current
@@ -2544,6 +2546,12 @@ mips_elf_multi_got (abfd, info, g, got, 
 				/ MIPS_ELF_GOT_SIZE (abfd))
 			       - MIPS_RESERVED_GOTNO - pages);
 
+  /* The primary GOT will be expanded to include all global symbols, so
+     pass the total number so that we don't merge too many other GOT's
+     local symbols in.  See below for why IRIX needs this and GNU/Linux
+     has adoped the requirement.  */
+  got_per_bfd_arg.global_count = g->global_gotno;
+
   /* Try to merge the GOTs of input bfds together, as long as they
      don't seem to exceed the maximum GOT size, choosing one of them
      to be the primary GOT.  */

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2003-11-21 22:17 Another MIPS multigot patch Daniel Jacobowitz
@ 2003-11-22 16:30 ` Richard Sandiford
  2003-11-22 17:40   ` Daniel Jacobowitz
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Sandiford @ 2003-11-22 16:30 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: binutils

Daniel Jacobowitz <drow@mvista.com> writes:
> Currently I can't build an n64 GCC (with my other patches applied to the
> linker).  The error is an R_MIPS_GOT_PAGE overflow for a common variable
> (flag_dump_unnumbered?).

Ouch.  An R_MIPS_GOT_PAGE against a global symbol?  OK, so it's supposed
to work, but gcc shouldn't really be generating that.  Can you send me
the .i file?

> Here's the problem:
> 
>   if (1)
>     {
>       gg->assigned_gotno = gg->global_gotno - g->global_gotno;
>       g->global_gotno = gg->global_gotno;
>       set_got_offset_arg.value = 2;
>     }
> 
> With this, global_gotno increases by a substantial amount.  If the primary
> GOT was full, then the odds are good that there is now a 16-bit relocation
> in the primary GOT that can no longer be resolved.

Not sure I understand.  We're supposed to order the master GOT so that
the symbols in the primary GOT come first.  Is that not happening for
some reason?

Richard

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2003-11-22 16:30 ` Richard Sandiford
@ 2003-11-22 17:40   ` Daniel Jacobowitz
  2003-11-22 19:13     ` Richard Sandiford
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Jacobowitz @ 2003-11-22 17:40 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: binutils

On Sat, Nov 22, 2003 at 04:31:10PM +0000, Richard Sandiford wrote:
> Daniel Jacobowitz <drow@mvista.com> writes:
> > Currently I can't build an n64 GCC (with my other patches applied to the
> > linker).  The error is an R_MIPS_GOT_PAGE overflow for a common variable
> > (flag_dump_unnumbered?).
> 
> Ouch.  An R_MIPS_GOT_PAGE against a global symbol?  OK, so it's supposed
> to work, but gcc shouldn't really be generating that.  Can you send me
> the .i file?

I'm using GCC 3.3.1, FYI.  So it may be fixed in HEAD which I'm not
really set up to try at the moment.  The relocation comes from:
        lw      $2,flag_dump_unnumbered

Let me know if you want the .i anyway.

> > Here's the problem:
> > 
> >   if (1)
> >     {
> >       gg->assigned_gotno = gg->global_gotno - g->global_gotno;
> >       g->global_gotno = gg->global_gotno;
> >       set_got_offset_arg.value = 2;
> >     }
> > 
> > With this, global_gotno increases by a substantial amount.  If the primary
> > GOT was full, then the odds are good that there is now a 16-bit relocation
> > in the primary GOT that can no longer be resolved.
> 
> Not sure I understand.  We're supposed to order the master GOT so that
> the symbols in the primary GOT come first.  Is that not happening for
> some reason?

I'm not sure I understand this code either - the fact that the *_gotno
fields change meaning halfway through compilation is really confusing,
since the comments do not clearly explain both meanings.  Let me sketch
what goes wrong.

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?  I see that dynindx's are set in
mips_elf_sort_hash_table_f, which is called after the GOT is formed.

Hmm, and I see that at the time of the relocation overflow,
h->root.got.offset is 2, suggesting that it is unreferenced.  That's
peculiar and suggests I had the wrong problem - why isn't it marked?
print-rtl.o is definitely mapped to the primary GOT according to
bfd2got.

It looks like the problem is one of common symbols. 
flag_dump_unnumbered occurs in toplev.o and print-rtl.o.  These two
functions end up in different GOTs.  The hash table for the master got
(gg) contains a GOT entry for this symbol, but it's pointing to
toplev.o.  The primary got is used for print-rtl.o.  And the symbol for
flag_dump_unnumbered ends up being associated with print-rtl.o in its
link hash (h->root.u.def.sec.owner).

It's hard to grub through the hash table to find out but it seems that
print-rtl.o's got_entries hash has nothing about flag_dump_unnumbered.
So I guess that's the real problem.  I'm guessing the two references
were somehow coalesced earlier?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2003-11-22 17:40   ` Daniel Jacobowitz
@ 2003-11-22 19:13     ` Richard Sandiford
  2003-11-23  1:24       ` Daniel Jacobowitz
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Sandiford @ 2003-11-22 19:13 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: binutils

Daniel Jacobowitz <drow@mvista.com> writes:
> I'm using GCC 3.3.1, FYI.  So it may be fixed in HEAD which I'm not
> really set up to try at the moment.  The relocation comes from:
>         lw      $2,flag_dump_unnumbered

Ah, never mind then. ;)  I assumed this was gcc explicit-reloc output.

> 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:

    (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.

    (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.

Richard

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2003-11-22 19:13     ` Richard Sandiford
@ 2003-11-23  1:24       ` Daniel Jacobowitz
  2004-01-29 14:46         ` Daniel Jacobowitz
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Jacobowitz @ 2003-11-23  1:24 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: binutils

On Sat, Nov 22, 2003 at 07:14:06PM +0000, Richard Sandiford wrote:
> Daniel Jacobowitz <drow@mvista.com> writes:
> > I'm using GCC 3.3.1, FYI.  So it may be fixed in HEAD which I'm not
> > really set up to try at the moment.  The relocation comes from:
> >         lw      $2,flag_dump_unnumbered
> 
> Ah, never mind then. ;)  I assumed this was gcc explicit-reloc output.
> 
> > 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
condition really ought to be.  This:
    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.  */
      local_p = local_p || ! h
        || (h->root.dynindx
            < mips_elf_get_global_gotsym_index (elf_hash_table (info)
needs to become... I'm not quite sure what.  Is it reasonable to use
SYMBOL_REFERENCES_LOCAL here?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2003-11-23  1:24       ` Daniel Jacobowitz
@ 2004-01-29 14:46         ` Daniel Jacobowitz
  2004-01-29 15:55           ` Richard Sandiford
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Jacobowitz @ 2004-01-29 14:46 UTC (permalink / raw)
  To: Richard Sandiford, binutils

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.  */

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2004-01-29 14:46         ` Daniel Jacobowitz
@ 2004-01-29 15:55           ` Richard Sandiford
  2004-01-29 16:19             ` Daniel Jacobowitz
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Sandiford @ 2004-01-29 15:55 UTC (permalink / raw)
  To: binutils

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2004-01-29 15:55           ` Richard Sandiford
@ 2004-01-29 16:19             ` Daniel Jacobowitz
  2004-01-29 17:39               ` Richard Sandiford
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Jacobowitz @ 2004-01-29 16:19 UTC (permalink / raw)
  To: binutils

On Thu, Jan 29, 2004 at 03:55:45PM +0000, Richard Sandiford wrote:
> 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?

Yes, hmips->root.dynindx == -1 does apply here.

> > 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

Yes, that's what happened.

> 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.  At this point all objects have
not been loaded, so we can not make any 100% certain decisions based on
the type or visibility of a symbol.  It's not that we can't decide not
to decay on a per-GOT basis, it's that we can't decide in check_relocs
not to do so.

Later, we can say "for all GOTs, PAGE references to this symbol can
remain".  And then, if the particular GOT has no decayed entry for this
symbol, it so happens that all references for that GOT will be handled
as PAGE.

> 
> > @@ -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.)

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.

> - 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.

  /* Function pointer equality tests may require that STV_PROTECTED
     symbols be treated as dynamic symbols, even when we know that the
     dynamic linker will resolve them locally.  */
  return local_protected;

I guess that doesn't apply here, so SYMBOL_CALLS_LOCAL should be used.

> - 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.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2004-01-29 16:19             ` Daniel Jacobowitz
@ 2004-01-29 17:39               ` Richard Sandiford
  2004-01-29 18:03                 ` Daniel Jacobowitz
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Sandiford @ 2004-01-29 17:39 UTC (permalink / raw)
  To: binutils

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2004-01-29 17:39               ` Richard Sandiford
@ 2004-01-29 18:03                 ` Daniel Jacobowitz
  2004-01-31 21:13                   ` Richard Sandiford
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Jacobowitz @ 2004-01-29 18:03 UTC (permalink / raw)
  To: binutils

On Thu, Jan 29, 2004 at 05:39:56PM +0000, Richard Sandiford wrote:
> 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.

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
(if info->shared).

> 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
macro MIPS_REFERENCES_LOCAL?

> >> - 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 *)
                  hmips->root.root.u.i.link;

              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:

main.c:

int foo;
extern int foo_func(void);

int
main(void)
{
  return foo_func();
}

func.c:

extern int foo;

int
foo_func(void)
{
  return 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.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2004-01-29 18:03                 ` Daniel Jacobowitz
@ 2004-01-31 21:13                   ` Richard Sandiford
  2004-02-10 22:59                     ` Daniel Jacobowitz
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Sandiford @ 2004-01-31 21:13 UTC (permalink / raw)
  To: binutils

Daniel Jacobowitz <drow@mvista.com> writes:
>> >> - 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
> (if info->shared).

That shouldn't happen.  mips_elf_get_global_gotsym_index is the value
associated with DT_MIPS_GOTSYM, i.e., the index of the first global GOT
entry.  Anything in the range [0,mips_elf_get_global_gotsym_index] is
supposed to bind locally.

>> 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.

I don't see what you're getting at.  Your patch changed the condition
in calculate_relocations() to:

      local_p = local_p || ! h
	|| (h->root.dynindx
	    < mips_elf_get_global_gotsym_index (elf_hash_table (info)
						->dynobj))
	|| SYMBOL_REFERENCES_LOCAL (info, h);

and my point was that:

      local_p = local_p || SYMBOL_REFERENCES_LOCAL (info, h);

ought to be just as safe.  But you said that the "h->root.dynindx < ..."
part was still needed, which presumably means that there are cases in
which _bfd_elf_symbol_refs_local_p will return FALSE and local_p should
nevertheless be set to TRUE.  That seems unlikely, since setting local_p
to TRUE will prevent symbol preemption.  Hence the question above.

>> >> - 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
> macro MIPS_REFERENCES_LOCAL?

Well, I still think it's cleaner to call the function directly.  OK, so
we have two convenience macros for it, but I don't see that _all_ calls
to the function have to be through convenience macros.

Mind you, I can easily get carried away over minor stuff like this ;)

> main.c:
>
> int foo;
> extern int foo_func(void);
>
> int
> main(void)
> {
>   return foo_func();
> }
>
> func.c:
>
> extern int foo;
>
> int
> foo_func(void)
> {
>   return 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.

But I don't see why that's a problem.  If main.o is linked first, then
check_relocs() knows that foo binds locally when processing both input
files.  There's no point adding a global GOT entry for it.

If func.o is linked first, then check_relocs() _doesn't_ know for certain
whether foo is local or global when processing the relocations in func.o.
It has to add the global entry, just to be on the safe side.

It sounds from your message like you think that check_relocs() is doing
the wrong thing if it's sensitive to link order.  But I don't think it is.
The GOT_PAGE code in check_relocs() is purely an optimisation.  It's trying
to avoid adding GOT entries when it knows that calculate_relocation() won't
need them.  The optimisation will work better for some link orders than
others, but that's just the way of things.

FWIW, here's how I understand the current code.  Bear in mind
that I didn't write any of it, so this could well be wrong. ;)

  (1) check_relocs(): Decide whether the referenced symbol is already
      known to bind locally.  If so, do nothing, otherwise add a
      global GOT entry just in case it _doesn't_ bind locally.

  (2) calculate_relocation(): Be aggressive about decaying PAGE/OFST
      to DISP/addend.  If a symbol has a global GOT entry, use it and
      decay the relocation, even if a PAGE/OFST combination would have
      been correct.  The check for whether the symbol had a global GOT
      entry is _the inverse of_:

	   (h->root.dynindx
	    < mips_elf_get_global_gotsym_index (elf_hash_table (info)
						->dynobj))

This doesn't work because a symbol might have global entries in some
GOTs but not others.  The existing code only checks whether the "master"
GOT has a global entry.

On the scale of possible fixes, it seems like there are two extremes:

  (a) Change the condition in (2) so that it checks whether the input
      bfd's GOT has a global entry.  We'd still be as aggressive as
      possible about decaying PAGE/OFST to DISP/addend.

  (b) Use the opposite approach to decaying in (2).  Never decay a
      PAGE/OFST to DISP/addend if the symbol binds locally, even if
      the symbol has an in-range global entry.

Your patch did (b), which I agree is the better choice.  It's simpler,
it's less likely to lead to overflow, and it might just give better cache
locality, since we'll be reusing the same entries wherever possible.

With either fix, I think the hmips->root.dynindx == -1 in check_relocs()
can go.

Richard

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2004-01-31 21:13                   ` Richard Sandiford
@ 2004-02-10 22:59                     ` Daniel Jacobowitz
  2004-02-11  8:19                       ` Richard Sandiford
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Jacobowitz @ 2004-02-10 22:59 UTC (permalink / raw)
  To: binutils

On Sat, Jan 31, 2004 at 09:13:57PM +0000, Richard Sandiford wrote:
> Daniel Jacobowitz <drow@mvista.com> writes:
> >> >> - 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
> > (if info->shared).
> 
> That shouldn't happen.  mips_elf_get_global_gotsym_index is the value
> associated with DT_MIPS_GOTSYM, i.e., the index of the first global GOT
> entry.  Anything in the range [0,mips_elf_get_global_gotsym_index] is
> supposed to bind locally.

Considering that the relatively recent _bfd_elf_symbol_refs_local_p is
not used from the MIPS code, I would have been amazed if it agreed on
all counts.  Indeed it does not.  I did this:

+      /* 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).  */
+      if (h && (h->root.dynindx
+               < mips_elf_get_global_gotsym_index (elf_hash_table (info)
+                                                   ->dynobj)))
+       BFD_ASSERT (_bfd_elf_symbol_refs_local_p (h, info, 1));
+      local_p = local_p || _bfd_elf_symbol_refs_local_p (h, info, 1);

and built world; the assertion failed a number of times.  The related
assertion in mips_elf_global_got_index did also:

  BFD_ASSERT (h->dynindx >= global_got_dynindx);

The first example I checked was while building ranlib.  The symbol was
_xexit_cleanup, a global function pointer.  No visibility tricks
involved, although it resolves from the shared libbfd rather than the
included static libiberty.  It failed both assertions:

(gdb) p h->dynindx
$4 = 8
(gdb) p global_got_dynindx 
$5 = 14
(gdb) p _bfd_elf_symbol_refs_local_p (h, info, 1)
$6 = 0

I would prefer to investigate this problem separately from the original
patch, which was needed for large applications to link on n64.  But if
you prefer then we can keep digging - I don't have time to figure out
what the problem with _xexit_cleanup is right now.  I have long
suspected that MIPS's crazy unique GOT handling did not honor the
symbol pre-emption rules properly.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2004-02-10 22:59                     ` Daniel Jacobowitz
@ 2004-02-11  8:19                       ` Richard Sandiford
  2004-02-15 13:46                         ` Richard Sandiford
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Sandiford @ 2004-02-11  8:19 UTC (permalink / raw)
  To: binutils

Daniel Jacobowitz <drow@mvista.com> writes:
> Considering that the relatively recent _bfd_elf_symbol_refs_local_p is
> not used from the MIPS code, I would have been amazed if it agreed on
> all counts.  Indeed it does not.  I did this:
>
> +      /* 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).  */
> +      if (h && (h->root.dynindx
> +               < mips_elf_get_global_gotsym_index (elf_hash_table (info)
> +                                                   ->dynobj)))
> +       BFD_ASSERT (_bfd_elf_symbol_refs_local_p (h, info, 1));
> +      local_p = local_p || _bfd_elf_symbol_refs_local_p (h, info, 1);
>
> and built world; the assertion failed a number of times.

Interesting.  I'll see if I can reproduce.

> I would prefer to investigate this problem separately from the original
> patch, which was needed for large applications to link on n64.  But if
> you prefer then we can keep digging - I don't have time to figure out
> what the problem with _xexit_cleanup is right now.  I have long
> suspected that MIPS's crazy unique GOT handling did not honor the
> symbol pre-emption rules properly.

Well, I'm not a maintainer of course, but I'd prefer it we keep them
together.  If we bolt a short-term fix onto the local_p condition, while
leaving stuff that _ought_ to be dead, then that's just going to increase
confusion in the long run.  It's the last thing this particular bit of
code needs...

Richard

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2004-02-11  8:19                       ` Richard Sandiford
@ 2004-02-15 13:46                         ` Richard Sandiford
  2004-02-15 18:42                           ` Daniel Jacobowitz
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Sandiford @ 2004-02-15 13:46 UTC (permalink / raw)
  To: binutils

Richard Sandiford <rsandifo@redhat.com> writes:
> Daniel Jacobowitz <drow@mvista.com> writes:
>> Considering that the relatively recent _bfd_elf_symbol_refs_local_p is
>> not used from the MIPS code, I would have been amazed if it agreed on
>> all counts.  Indeed it does not.  I did this:
>>
>> +      /* 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).  */
>> +      if (h && (h->root.dynindx
>> +               < mips_elf_get_global_gotsym_index (elf_hash_table (info)
>> +                                                   ->dynobj)))
>> +       BFD_ASSERT (_bfd_elf_symbol_refs_local_p (h, info, 1));
>> +      local_p = local_p || _bfd_elf_symbol_refs_local_p (h, info, 1);
>>
>> and built world; the assertion failed a number of times.
>
> Interesting.  I'll see if I can reproduce.

I couldn't.  The condition didn't trigger in my own build world (done
with CVS binutils & gcc).  It also didn't trigger in an IRIX bootstrap,
although there's admittedly not much opportunity for it to.  FWIW, I've
attached the patch I was using belong.

Could you send me the ranlib testcase?  Or make it available for
ftp if it's big?

Thanks,
Richard

Index: bfd/elfxx-mips.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-mips.c,v
retrieving revision 1.89
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.89 elfxx-mips.c
--- bfd/elfxx-mips.c	9 Feb 2004 08:04:00 -0000	1.89
+++ bfd/elfxx-mips.c	15 Feb 2004 10:32:05 -0000
@@ -3261,12 +3261,13 @@ mips_elf_calculate_relocation (bfd *abfd
     {
     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.  */
-      local_p = local_p || ! h
-	|| (h->root.dynindx
-	    < mips_elf_get_global_gotsym_index (elf_hash_table (info)
-						->dynobj));
+      if (h
+	  && (h->root.dynindx
+	      < mips_elf_get_global_gotsym_index (elf_hash_table (info)
+						  ->dynobj))
+	  && !_bfd_elf_symbol_refs_local_p (&h->root, info, 1))
+	abort ();
+      local_p = local_p || _bfd_elf_symbol_refs_local_p (&h->root, info, 1);
       if (local_p || r_type == R_MIPS_GOT_OFST)
 	break;
       /* Fall through.  */
@@ -5389,20 +5390,7 @@ _bfd_mips_elf_check_relocs (bfd *abfd, s
 		  && hmips->root.root.u.def.section
 		  && ! (info->shared && ! info->symbolic
 			&& ! (hmips->root.elf_link_hash_flags
-			      & ELF_LINK_FORCED_LOCAL))
-		  /* 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.  */
-		  && hmips->root.dynindx == -1)
+			      & ELF_LINK_FORCED_LOCAL)))
 		break;
 	    }
 	  /* Fall through.  */

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2004-02-15 13:46                         ` Richard Sandiford
@ 2004-02-15 18:42                           ` Daniel Jacobowitz
  2004-02-15 22:00                             ` Richard Sandiford
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Jacobowitz @ 2004-02-15 18:42 UTC (permalink / raw)
  To: binutils

On Sun, Feb 15, 2004 at 01:47:28PM +0000, Richard Sandiford wrote:
> Richard Sandiford <rsandifo@redhat.com> writes:
> > Daniel Jacobowitz <drow@mvista.com> writes:
> >> Considering that the relatively recent _bfd_elf_symbol_refs_local_p is
> >> not used from the MIPS code, I would have been amazed if it agreed on
> >> all counts.  Indeed it does not.  I did this:
> >>
> >> +      /* 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).  */
> >> +      if (h && (h->root.dynindx
> >> +               < mips_elf_get_global_gotsym_index (elf_hash_table (info)
> >> +                                                   ->dynobj)))
> >> +       BFD_ASSERT (_bfd_elf_symbol_refs_local_p (h, info, 1));
> >> +      local_p = local_p || _bfd_elf_symbol_refs_local_p (h, info, 1);
> >>
> >> and built world; the assertion failed a number of times.
> >
> > Interesting.  I'll see if I can reproduce.
> 
> I couldn't.  The condition didn't trigger in my own build world (done
> with CVS binutils & gcc).  It also didn't trigger in an IRIX bootstrap,
> although there's admittedly not much opportunity for it to.  FWIW, I've
> attached the patch I was using belong.
> 
> Could you send me the ranlib testcase?  Or make it available for
> ftp if it's big?

I have put it at:
  http://www.false.org/~drow/binutils-mips-got.tgz

It only takes two files to reproduce.  Examples:

% mips64_fp_be-ld libbfd-2.14.so xatexit.o
mips64_fp_be-ld: warning: cannot find entry symbol __start; defaulting to 00000000100003e0
mips64_fp_be-ld: BFD 2.14 20030612 (MontaVista 2.14-18.0.0.nevyn 2004-02-10) assertion fail ../../bfd/elfxx-mips.c:3259
mips64_fp_be-ld: BFD 2.14 20030612 (MontaVista 2.14-18.0.0.nevyn 2004-02-10) assertion fail ../../bfd/elfxx-mips.c:1797
mips64_fp_be-ld: BFD 2.14 20030612 (MontaVista 2.14-18.0.0.nevyn 2004-02-10) assertion fail ../../bfd/elfxx-mips.c:3259
mips64_fp_be-ld: BFD 2.14 20030612 (MontaVista 2.14-18.0.0.nevyn 2004-02-10) assertion fail ../../bfd/elfxx-mips.c:3259
mips64_fp_be-ld: BFD 2.14 20030612 (MontaVista 2.14-18.0.0.nevyn 2004-02-10) assertion fail ../../bfd/elfxx-mips.c:1797
mips64_fp_be-ld: BFD 2.14 20030612 (MontaVista 2.14-18.0.0.nevyn 2004-02-10) assertion fail ../../bfd/elfxx-mips.c:3259

% mips64_fp_be-ld xatexit.o libbfd-2.14.so
mips64_fp_be-ld: warning: cannot find entry symbol __start; defaulting to 0000000010000400

%

My patch is the same as the one you attached.  This is not CVS HEAD
binutils; it's possible that there's something else wrong in my local
tree, but at this point elfxx-mips.c is pretty much the same as it is
in HEAD.  I can try to reproduce in HEAD next week.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2004-02-15 18:42                           ` Daniel Jacobowitz
@ 2004-02-15 22:00                             ` Richard Sandiford
  2004-02-16  3:17                               ` Daniel Jacobowitz
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Sandiford @ 2004-02-15 22:00 UTC (permalink / raw)
  To: binutils

[-- Attachment #1: Type: text/plain, Size: 3334 bytes --]

Daniel Jacobowitz <drow@false.org> writes:
> I have put it at:
>   http://www.false.org/~drow/binutils-mips-got.tgz

Thanks.  In hindsight, I really should have been able to work it
out from your description, sorry about that.

I think the problem is simply that check_relocs() was only checking
whether the symbol was defined, not whether the symbol was defined
by a regular or shared object.

The patch below seems to do the trick.  I'll run a bootstrap on irix,
but since you seem to trip over the problem more often, it'd be great
if you could give it a spin.

I've attached a shell archive (to be run from the top level of the
build directory).  Before the patch, a.x needlessly had a global GOT
entry for gs.  After the patch, both a.x and b.x only have a global
GOT entry for "us".

Richard


	* elfxx-mips.c (mips_elf_calculate_relocation): Use
	_bfd_elf_symbol_refs_local_p to decide whether to decay
	a GOT_PAGE/GOT_OFST pair to GOT_DISP/addend.
	(_bfd_mips_elf_check_relocs): Add a global GOT entry for GOT_PAGE
	relocs if the symbol wasn't defined by a regular object file.
	Don't check the symbol's dynindx.

Index: bfd/elfxx-mips.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-mips.c,v
retrieving revision 1.89
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.89 elfxx-mips.c
--- bfd/elfxx-mips.c	9 Feb 2004 08:04:00 -0000	1.89
+++ bfd/elfxx-mips.c	15 Feb 2004 21:24:41 -0000
@@ -3261,12 +3261,9 @@ mips_elf_calculate_relocation (bfd *abfd
     {
     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.  */
-      local_p = local_p || ! h
-	|| (h->root.dynindx
-	    < mips_elf_get_global_gotsym_index (elf_hash_table (info)
-						->dynobj));
+      /* We need to decay to GOT_DISP/addend if the symbol doesn't
+	 bind locally.  */
+      local_p = local_p || _bfd_elf_symbol_refs_local_p (&h->root, info, 1);
       if (local_p || r_type == R_MIPS_GOT_OFST)
 	break;
       /* Fall through.  */
@@ -5384,25 +5381,10 @@ _bfd_mips_elf_check_relocs (bfd *abfd, s
 		hmips = (struct mips_elf_link_hash_entry *)
 		  hmips->root.root.u.i.link;
 
-	      if ((hmips->root.root.type == bfd_link_hash_defined
-		   || hmips->root.root.type == bfd_link_hash_defweak)
-		  && hmips->root.root.u.def.section
+	      if ((hmips->root.elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR)
 		  && ! (info->shared && ! info->symbolic
 			&& ! (hmips->root.elf_link_hash_flags
-			      & ELF_LINK_FORCED_LOCAL))
-		  /* 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.  */
-		  && hmips->root.dynindx == -1)
+			      & ELF_LINK_FORCED_LOCAL)))
 		break;
 	    }
 	  /* Fall through.  */

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: foo.sh --]
[-- Type: text/x-sh, Size: 597 bytes --]

cat <<EOF > a.s
	.globl	__start
	.globl	gs
__start:
gs:
ls:
	lw	\$4,%got_page(us)(\$gp)
	addiu	\$4,\$4,%got_ofst(us)
	lw	\$4,%got_page(gs)(\$gp)
	addiu	\$4,\$4,%got_ofst(gs)
	lw	\$4,%got_page(ls)(\$gp)
	addiu	\$4,\$4,%got_ofst(ls)
EOF
cat <<EOF > b.s
	.globl	us
	.globl	gs
us:
gs:
ls:
	lw	\$4,%got_page(us)(\$gp)
	addiu	\$4,\$4,%got_ofst(us)
	lw	\$4,%got_page(gs)(\$gp)
	addiu	\$4,\$4,%got_ofst(gs)
	lw	\$4,%got_page(ls)(\$gp)
	addiu	\$4,\$4,%got_ofst(ls)
EOF
./gas/as-new a.s -o a.o
./gas/as-new b.s -o b.o
./ld/ld-new b.o -o b.so -shared
./ld/ld-new b.so a.o -o a.x
./ld/ld-new a.o b.so -o b.x


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2004-02-15 22:00                             ` Richard Sandiford
@ 2004-02-16  3:17                               ` Daniel Jacobowitz
  2004-02-16  8:06                                 ` Richard Sandiford
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Jacobowitz @ 2004-02-16  3:17 UTC (permalink / raw)
  To: binutils

On Sun, Feb 15, 2004 at 10:01:25PM +0000, Richard Sandiford wrote:
> Daniel Jacobowitz <drow@false.org> writes:
> > I have put it at:
> >   http://www.false.org/~drow/binutils-mips-got.tgz
> 
> Thanks.  In hindsight, I really should have been able to work it
> out from your description, sorry about that.
> 
> I think the problem is simply that check_relocs() was only checking
> whether the symbol was defined, not whether the symbol was defined
> by a regular or shared object.
> 
> The patch below seems to do the trick.  I'll run a bootstrap on irix,
> but since you seem to trip over the problem more often, it'd be great
> if you could give it a spin.

I will do this and let you know if the build works.  Do you expect the
proposed assertion to succeed now?

> I've attached a shell archive (to be run from the top level of the
> build directory).  Before the patch, a.x needlessly had a global GOT
> entry for gs.  After the patch, both a.x and b.x only have a global
> GOT entry for "us".

Interested in adding this to the testsuite? :)


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2004-02-16  3:17                               ` Daniel Jacobowitz
@ 2004-02-16  8:06                                 ` Richard Sandiford
  2004-02-16 18:37                                   ` Daniel Jacobowitz
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Sandiford @ 2004-02-16  8:06 UTC (permalink / raw)
  To: binutils

Daniel Jacobowitz <drow@false.org> writes:
> On Sun, Feb 15, 2004 at 10:01:25PM +0000, Richard Sandiford wrote:
>> I think the problem is simply that check_relocs() was only checking
>> whether the symbol was defined, not whether the symbol was defined
>> by a regular or shared object.
>> 
> I will do this and let you know if the build works.

Thanks.

> Do you expect the proposed assertion to succeed now?

Yeah, I just wasn't sure whether it was worth keeping in the final version.
If you think it is, I suggest we move it outside the switch statement.
It isn't really specific to GOT_PAGE relocations.

>> I've attached a shell archive (to be run from the top level of the
>> build directory).  Before the patch, a.x needlessly had a global GOT
>> entry for gs.  After the patch, both a.x and b.x only have a global
>> GOT entry for "us".
>
> Interested in adding this to the testsuite? :)

Maybe...

Richard

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Another MIPS multigot patch
  2004-02-16  8:06                                 ` Richard Sandiford
@ 2004-02-16 18:37                                   ` Daniel Jacobowitz
  2004-02-17  9:17                                     ` RFA: " Richard Sandiford
  0 siblings, 1 reply; 23+ messages in thread
From: Daniel Jacobowitz @ 2004-02-16 18:37 UTC (permalink / raw)
  To: binutils

On Mon, Feb 16, 2004 at 08:07:33AM +0000, Richard Sandiford wrote:
> Daniel Jacobowitz <drow@false.org> writes:
> > On Sun, Feb 15, 2004 at 10:01:25PM +0000, Richard Sandiford wrote:
> >> I think the problem is simply that check_relocs() was only checking
> >> whether the symbol was defined, not whether the symbol was defined
> >> by a regular or shared object.
> >> 
> > I will do this and let you know if the build works.
> 
> Thanks.
> 
> > Do you expect the proposed assertion to succeed now?
> 
> Yeah, I just wasn't sure whether it was worth keeping in the final version.
> If you think it is, I suggest we move it outside the switch statement.
> It isn't really specific to GOT_PAGE relocations.

I would prefer that.

I've just finished a build.  Everything came out OK, and the assertion
never triggered.  Will you commit the patch?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RFA: Another MIPS multigot patch
  2004-02-16 18:37                                   ` Daniel Jacobowitz
@ 2004-02-17  9:17                                     ` Richard Sandiford
  2004-02-17  9:27                                       ` Richard Sandiford
  2004-02-17 10:07                                       ` Eric Christopher
  0 siblings, 2 replies; 23+ messages in thread
From: Richard Sandiford @ 2004-02-17  9:17 UTC (permalink / raw)
  To: binutils

[-- Attachment #1: Type: text/plain, Size: 28989 bytes --]

Daniel Jacobowitz <drow@false.org> writes:
>> > Do you expect the proposed assertion to succeed now?
>> 
>> Yeah, I just wasn't sure whether it was worth keeping in the final version.
>> If you think it is, I suggest we move it outside the switch statement.
>> It isn't really specific to GOT_PAGE relocations.
>
> I would prefer that.

Been looking further into this assertion thing, and I think it's
already covered by:

    static bfd_vma
    mips_elf_global_got_index [...]
    {
      [...]
      BFD_ASSERT (h->dynindx >= global_got_dynindx);
      [...]
    }

If you remove the dynindx check from check_relocs() without applying
the rest of the patch, this assertion triggers for your test case and
for the reduced version below.

I don't think it's worth adding the attachment to dejagnu since (a) the
current sources pass it anyway and (b) we can use the existing elf-rel-got*
tests to make sure that locally-binding symbols use page entries.  As below.

> I've just finished a build.  Everything came out OK, and the assertion
> never triggered.  Will you commit the patch?

Can do if it's approved.  As expected, some changes are needed to the
ld testsuite.  In elf-rel-got-*.d:

  - We now use page entries to access dg1.
  - The GOT entries are in a different (but legitimate) order.
  - The large-offset bits need updating after Maciej's macro changes.
  - We need a "#pass" at the end since the object contains other
    sections too, e.g. .pdr.

The last two are also needed for *-xgot.  I didn't update the SGI
versions of these tests since there are lots of other problems with them.
Another day...

OK to install?

Richard



2004-02-17  Daniel Jacobowitz  <drow@mvista.com>
            Richard Sandiford  <rsandifo@redhat.com>

bfd/
	* elfxx-mips.c (mips_elf_calculate_relocation): Use
	_bfd_elf_symbol_refs_local_p to decide whether to decay
	a GOT_PAGE/GOT_OFST pair to GOT_DISP/addend.
	(_bfd_mips_elf_check_relocs): Add a global GOT entry for GOT_PAGE
	relocs if the symbol wasn't defined by a regular object file.
	Don't check the symbol's dynindx.

ld/testsuite/
	* ld-mips/elf/elf-rel-xgot-{n32,n64-linux}.d: Update after 2004-02-02
	changes to the way large constants are added.
	* ld-mips/elf/elf-rel-got-{n32,n64-linux}.d: Likewise.  Adjust order
	of GOT entries after today's change to the handling of GOT_PAGE
	relocations.

Index: bfd/elfxx-mips.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-mips.c,v
retrieving revision 1.89
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.89 elfxx-mips.c
--- bfd/elfxx-mips.c	9 Feb 2004 08:04:00 -0000	1.89
+++ bfd/elfxx-mips.c	17 Feb 2004 08:56:06 -0000
@@ -3261,12 +3261,9 @@ mips_elf_calculate_relocation (bfd *abfd
     {
     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.  */
-      local_p = local_p || ! h
-	|| (h->root.dynindx
-	    < mips_elf_get_global_gotsym_index (elf_hash_table (info)
-						->dynobj));
+      /* We need to decay to GOT_DISP/addend if the symbol doesn't
+	 bind locally.  */
+      local_p = local_p || _bfd_elf_symbol_refs_local_p (&h->root, info, 1);
       if (local_p || r_type == R_MIPS_GOT_OFST)
 	break;
       /* Fall through.  */
@@ -5384,25 +5381,10 @@ _bfd_mips_elf_check_relocs (bfd *abfd, s
 		hmips = (struct mips_elf_link_hash_entry *)
 		  hmips->root.root.u.i.link;
 
-	      if ((hmips->root.root.type == bfd_link_hash_defined
-		   || hmips->root.root.type == bfd_link_hash_defweak)
-		  && hmips->root.root.u.def.section
+	      if ((hmips->root.elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR)
 		  && ! (info->shared && ! info->symbolic
 			&& ! (hmips->root.elf_link_hash_flags
-			      & ELF_LINK_FORCED_LOCAL))
-		  /* 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.  */
-		  && hmips->root.dynindx == -1)
+			      & ELF_LINK_FORCED_LOCAL)))
 		break;
 	    }
 	  /* Fall through.  */
Index: ld/testsuite/ld-mips-elf/elf-rel-got-n32.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-mips-elf/elf-rel-got-n32.d,v
retrieving revision 1.2
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.2 elf-rel-got-n32.d
--- ld/testsuite/ld-mips-elf/elf-rel-got-n32.d	21 Jun 2003 21:38:04 -0000	1.2
+++ ld/testsuite/ld-mips-elf/elf-rel-got-n32.d	17 Feb 2004 08:56:06 -0000
@@ -13,8 +13,8 @@ 100000a0:	8f858064 	lw	a1,-32668\(gp\)
 100000a4:	8f858064 	lw	a1,-32668\(gp\)
 100000a8:	24a5000c 	addiu	a1,a1,12
 100000ac:	8f858064 	lw	a1,-32668\(gp\)
-100000b0:	3c010002 	lui	at,0x2
-100000b4:	2421e240 	addiu	at,at,-7616
+100000b0:	3c010001 	lui	at,0x1
+100000b4:	3421e240 	ori	at,at,0xe240
 100000b8:	00a12821 	addu	a1,a1,at
 100000bc:	8f858064 	lw	a1,-32668\(gp\)
 100000c0:	00b12821 	addu	a1,a1,s1
@@ -22,26 +22,26 @@ 100000c4:	8f858064 	lw	a1,-32668\(gp\)
 100000c8:	24a5000c 	addiu	a1,a1,12
 100000cc:	00b12821 	addu	a1,a1,s1
 100000d0:	8f858064 	lw	a1,-32668\(gp\)
-100000d4:	3c010002 	lui	at,0x2
-100000d8:	2421e240 	addiu	at,at,-7616
+100000d4:	3c010001 	lui	at,0x1
+100000d8:	3421e240 	ori	at,at,0xe240
 100000dc:	00a12821 	addu	a1,a1,at
 100000e0:	00b12821 	addu	a1,a1,s1
-100000e4:	8f858064 	lw	a1,-32668\(gp\)
-100000e8:	8ca50000 	lw	a1,0\(a1\)
-100000ec:	8f858064 	lw	a1,-32668\(gp\)
-100000f0:	8ca5000c 	lw	a1,12\(a1\)
-100000f4:	8f858064 	lw	a1,-32668\(gp\)
+100000e4:	8f858018 	lw	a1,-32744\(gp\)
+100000e8:	8ca5050c 	lw	a1,1292\(a1\)
+100000ec:	8f858018 	lw	a1,-32744\(gp\)
+100000f0:	8ca50518 	lw	a1,1304\(a1\)
+100000f4:	8f858018 	lw	a1,-32744\(gp\)
 100000f8:	00b12821 	addu	a1,a1,s1
-100000fc:	8ca50000 	lw	a1,0\(a1\)
-10000100:	8f858064 	lw	a1,-32668\(gp\)
+100000fc:	8ca5050c 	lw	a1,1292\(a1\)
+10000100:	8f858018 	lw	a1,-32744\(gp\)
 10000104:	00b12821 	addu	a1,a1,s1
-10000108:	8ca5000c 	lw	a1,12\(a1\)
-1000010c:	8f818064 	lw	at,-32668\(gp\)
+10000108:	8ca50518 	lw	a1,1304\(a1\)
+1000010c:	8f818018 	lw	at,-32744\(gp\)
 10000110:	00250821 	addu	at,at,a1
-10000114:	8c250022 	lw	a1,34\(at\)
-10000118:	8f818064 	lw	at,-32668\(gp\)
+10000114:	8c25052e 	lw	a1,1326\(at\)
+10000118:	8f818018 	lw	at,-32744\(gp\)
 1000011c:	00250821 	addu	at,at,a1
-10000120:	ac250038 	sw	a1,56\(at\)
+10000120:	ac250544 	sw	a1,1348\(at\)
 10000124:	8f818064 	lw	at,-32668\(gp\)
 10000128:	88250000 	lwl	a1,0\(at\)
 1000012c:	98250003 	lwr	a1,3\(at\)
@@ -68,42 +68,42 @@ 1000017c:	24210038 	addiu	at,at,56
 10000180:	00250821 	addu	at,at,a1
 10000184:	a8250000 	swl	a1,0\(at\)
 10000188:	b8250003 	swr	a1,3\(at\)
-1000018c:	8f858018 	lw	a1,-32744\(gp\)
-10000190:	8f85801c 	lw	a1,-32740\(gp\)
-10000194:	8f858020 	lw	a1,-32736\(gp\)
-10000198:	8f858018 	lw	a1,-32744\(gp\)
+1000018c:	8f85801c 	lw	a1,-32740\(gp\)
+10000190:	8f858020 	lw	a1,-32736\(gp\)
+10000194:	8f858024 	lw	a1,-32732\(gp\)
+10000198:	8f85801c 	lw	a1,-32740\(gp\)
 1000019c:	00b12821 	addu	a1,a1,s1
-100001a0:	8f85801c 	lw	a1,-32740\(gp\)
+100001a0:	8f858020 	lw	a1,-32736\(gp\)
 100001a4:	00b12821 	addu	a1,a1,s1
-100001a8:	8f858020 	lw	a1,-32736\(gp\)
+100001a8:	8f858024 	lw	a1,-32732\(gp\)
 100001ac:	00b12821 	addu	a1,a1,s1
-100001b0:	8f858024 	lw	a1,-32732\(gp\)
+100001b0:	8f858018 	lw	a1,-32744\(gp\)
 100001b4:	8ca5050c 	lw	a1,1292\(a1\)
-100001b8:	8f858024 	lw	a1,-32732\(gp\)
+100001b8:	8f858018 	lw	a1,-32744\(gp\)
 100001bc:	8ca50518 	lw	a1,1304\(a1\)
-100001c0:	8f858024 	lw	a1,-32732\(gp\)
+100001c0:	8f858018 	lw	a1,-32744\(gp\)
 100001c4:	00b12821 	addu	a1,a1,s1
 100001c8:	8ca5050c 	lw	a1,1292\(a1\)
-100001cc:	8f858024 	lw	a1,-32732\(gp\)
+100001cc:	8f858018 	lw	a1,-32744\(gp\)
 100001d0:	00b12821 	addu	a1,a1,s1
 100001d4:	8ca50518 	lw	a1,1304\(a1\)
-100001d8:	8f818024 	lw	at,-32732\(gp\)
+100001d8:	8f818018 	lw	at,-32744\(gp\)
 100001dc:	00250821 	addu	at,at,a1
 100001e0:	8c25052e 	lw	a1,1326\(at\)
-100001e4:	8f818024 	lw	at,-32732\(gp\)
+100001e4:	8f818018 	lw	at,-32744\(gp\)
 100001e8:	00250821 	addu	at,at,a1
 100001ec:	ac250544 	sw	a1,1348\(at\)
-100001f0:	8f818018 	lw	at,-32744\(gp\)
+100001f0:	8f81801c 	lw	at,-32740\(gp\)
 100001f4:	88250000 	lwl	a1,0\(at\)
 100001f8:	98250003 	lwr	a1,3\(at\)
-100001fc:	8f81801c 	lw	at,-32740\(gp\)
+100001fc:	8f818020 	lw	at,-32736\(gp\)
 10000200:	88250000 	lwl	a1,0\(at\)
 10000204:	98250003 	lwr	a1,3\(at\)
-10000208:	8f818018 	lw	at,-32744\(gp\)
+10000208:	8f81801c 	lw	at,-32740\(gp\)
 1000020c:	00310821 	addu	at,at,s1
 10000210:	88250000 	lwl	a1,0\(at\)
 10000214:	98250003 	lwr	a1,3\(at\)
-10000218:	8f81801c 	lw	at,-32740\(gp\)
+10000218:	8f818020 	lw	at,-32736\(gp\)
 1000021c:	00310821 	addu	at,at,s1
 10000220:	88250000 	lwl	a1,0\(at\)
 10000224:	98250003 	lwr	a1,3\(at\)
@@ -129,8 +129,8 @@ 10000270:	8f858068 	lw	a1,-32664\(gp\)
 10000274:	8f858068 	lw	a1,-32664\(gp\)
 10000278:	24a5000c 	addiu	a1,a1,12
 1000027c:	8f858068 	lw	a1,-32664\(gp\)
-10000280:	3c010002 	lui	at,0x2
-10000284:	2421e240 	addiu	at,at,-7616
+10000280:	3c010001 	lui	at,0x1
+10000284:	3421e240 	ori	at,at,0xe240
 10000288:	00a12821 	addu	a1,a1,at
 1000028c:	8f858068 	lw	a1,-32664\(gp\)
 10000290:	00b12821 	addu	a1,a1,s1
@@ -138,26 +138,26 @@ 10000294:	8f858068 	lw	a1,-32664\(gp\)
 10000298:	24a5000c 	addiu	a1,a1,12
 1000029c:	00b12821 	addu	a1,a1,s1
 100002a0:	8f858068 	lw	a1,-32664\(gp\)
-100002a4:	3c010002 	lui	at,0x2
-100002a8:	2421e240 	addiu	at,at,-7616
+100002a4:	3c010001 	lui	at,0x1
+100002a8:	3421e240 	ori	at,at,0xe240
 100002ac:	00a12821 	addu	a1,a1,at
 100002b0:	00b12821 	addu	a1,a1,s1
-100002b4:	8f858068 	lw	a1,-32664\(gp\)
-100002b8:	8ca50000 	lw	a1,0\(a1\)
-100002bc:	8f858068 	lw	a1,-32664\(gp\)
-100002c0:	8ca5000c 	lw	a1,12\(a1\)
-100002c4:	8f858068 	lw	a1,-32664\(gp\)
+100002b4:	8f858018 	lw	a1,-32744\(gp\)
+100002b8:	8ca50584 	lw	a1,1412\(a1\)
+100002bc:	8f858018 	lw	a1,-32744\(gp\)
+100002c0:	8ca50590 	lw	a1,1424\(a1\)
+100002c4:	8f858018 	lw	a1,-32744\(gp\)
 100002c8:	00b12821 	addu	a1,a1,s1
-100002cc:	8ca50000 	lw	a1,0\(a1\)
-100002d0:	8f858068 	lw	a1,-32664\(gp\)
+100002cc:	8ca50584 	lw	a1,1412\(a1\)
+100002d0:	8f858018 	lw	a1,-32744\(gp\)
 100002d4:	00b12821 	addu	a1,a1,s1
-100002d8:	8ca5000c 	lw	a1,12\(a1\)
-100002dc:	8f818068 	lw	at,-32664\(gp\)
+100002d8:	8ca50590 	lw	a1,1424\(a1\)
+100002dc:	8f818018 	lw	at,-32744\(gp\)
 100002e0:	00250821 	addu	at,at,a1
-100002e4:	8c250022 	lw	a1,34\(at\)
-100002e8:	8f818068 	lw	at,-32664\(gp\)
+100002e4:	8c2505a6 	lw	a1,1446\(at\)
+100002e8:	8f818018 	lw	at,-32744\(gp\)
 100002ec:	00250821 	addu	at,at,a1
-100002f0:	ac250038 	sw	a1,56\(at\)
+100002f0:	ac2505bc 	sw	a1,1468\(at\)
 100002f4:	8f818068 	lw	at,-32664\(gp\)
 100002f8:	88250000 	lwl	a1,0\(at\)
 100002fc:	98250003 	lwr	a1,3\(at\)
@@ -193,20 +193,20 @@ 10000370:	8f858038 	lw	a1,-32712\(gp\)
 10000374:	00b12821 	addu	a1,a1,s1
 10000378:	8f85803c 	lw	a1,-32708\(gp\)
 1000037c:	00b12821 	addu	a1,a1,s1
-10000380:	8f858024 	lw	a1,-32732\(gp\)
+10000380:	8f858018 	lw	a1,-32744\(gp\)
 10000384:	8ca50584 	lw	a1,1412\(a1\)
-10000388:	8f858024 	lw	a1,-32732\(gp\)
+10000388:	8f858018 	lw	a1,-32744\(gp\)
 1000038c:	8ca50590 	lw	a1,1424\(a1\)
-10000390:	8f858024 	lw	a1,-32732\(gp\)
+10000390:	8f858018 	lw	a1,-32744\(gp\)
 10000394:	00b12821 	addu	a1,a1,s1
 10000398:	8ca50584 	lw	a1,1412\(a1\)
-1000039c:	8f858024 	lw	a1,-32732\(gp\)
+1000039c:	8f858018 	lw	a1,-32744\(gp\)
 100003a0:	00b12821 	addu	a1,a1,s1
 100003a4:	8ca50590 	lw	a1,1424\(a1\)
-100003a8:	8f818024 	lw	at,-32732\(gp\)
+100003a8:	8f818018 	lw	at,-32744\(gp\)
 100003ac:	00250821 	addu	at,at,a1
 100003b0:	8c2505a6 	lw	a1,1446\(at\)
-100003b4:	8f818024 	lw	at,-32732\(gp\)
+100003b4:	8f818018 	lw	at,-32744\(gp\)
 100003b8:	00250821 	addu	at,at,a1
 100003bc:	ac2505bc 	sw	a1,1468\(at\)
 100003c0:	8f818034 	lw	at,-32716\(gp\)
@@ -243,24 +243,24 @@ 10000438:	0320f809 	jalr	t9
 1000043c:	00000000 	nop
 10000440:	1000ff17 	b	100000a0 <fn>
 10000444:	8f858064 	lw	a1,-32668\(gp\)
-10000448:	8f858068 	lw	a1,-32664\(gp\)
+10000448:	8f858018 	lw	a1,-32744\(gp\)
 1000044c:	10000015 	b	100004a4 <fn2>
-10000450:	8ca50000 	lw	a1,0\(a1\)
+10000450:	8ca50584 	lw	a1,1412\(a1\)
 10000454:	1000ff12 	b	100000a0 <fn>
-10000458:	8f858018 	lw	a1,-32744\(gp\)
+10000458:	8f85801c 	lw	a1,-32740\(gp\)
 1000045c:	8f858038 	lw	a1,-32712\(gp\)
 10000460:	10000010 	b	100004a4 <fn2>
 10000464:	00000000 	nop
-10000468:	8f858020 	lw	a1,-32736\(gp\)
+10000468:	8f858024 	lw	a1,-32732\(gp\)
 1000046c:	1000ff0c 	b	100000a0 <fn>
 10000470:	00000000 	nop
-10000474:	8f858024 	lw	a1,-32732\(gp\)
+10000474:	8f858018 	lw	a1,-32744\(gp\)
 10000478:	1000000a 	b	100004a4 <fn2>
 1000047c:	8ca50584 	lw	a1,1412\(a1\)
-10000480:	8f858024 	lw	a1,-32732\(gp\)
+10000480:	8f858018 	lw	a1,-32744\(gp\)
 10000484:	1000ff06 	b	100000a0 <fn>
 10000488:	8ca50518 	lw	a1,1304\(a1\)
-1000048c:	8f818024 	lw	at,-32732\(gp\)
+1000048c:	8f818018 	lw	at,-32744\(gp\)
 10000490:	00250821 	addu	at,at,a1
 10000494:	10000003 	b	100004a4 <fn2>
 10000498:	8c2505a6 	lw	a1,1446\(at\)
@@ -292,10 +292,10 @@ Disassembly of section \.got:
 101005c0 <_GLOBAL_OFFSET_TABLE_>:
 101005c0:	00000000 	.*
 101005c4:	80000000 	.*
-101005c8:	1010050c 	.*
-101005cc:	10100518 	.*
-101005d0:	1011e74c 	.*
-101005d4:	10100000 	.*
+101005c8:	10100000 	.*
+101005cc:	1010050c 	.*
+101005d0:	10100518 	.*
+101005d4:	1011e74c 	.*
 101005d8:	1010052e 	.*
 101005dc:	10100544 	.*
 101005e0:	100000a0 	.*
@@ -311,3 +311,4 @@ 1010060c:	100000a0 	.*
 10100610:	100004a4 	.*
 10100614:	1010050c 	.*
 10100618:	10100584 	.*
+#pass
Index: ld/testsuite/ld-mips-elf/elf-rel-got-n64-linux.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-mips-elf/elf-rel-got-n64-linux.d,v
retrieving revision 1.1
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.1 elf-rel-got-n64-linux.d
--- ld/testsuite/ld-mips-elf/elf-rel-got-n64-linux.d	12 Jun 2003 17:07:14 -0000	1.1
+++ ld/testsuite/ld-mips-elf/elf-rel-got-n64-linux.d	17 Feb 2004 08:56:06 -0000
@@ -22,8 +22,8 @@ 00000001200000e0 <fn>:
    1200000e4:	df8580b8 	ld	a1,-32584\(gp\)
    1200000e8:	64a5000c 	daddiu	a1,a1,12
    1200000ec:	df8580b8 	ld	a1,-32584\(gp\)
-   1200000f0:	3c010002 	lui	at,0x2
-   1200000f4:	6421e240 	daddiu	at,at,-7616
+   1200000f0:	3c010001 	lui	at,0x1
+   1200000f4:	3421e240 	ori	at,at,0xe240
    1200000f8:	00a1282d 	daddu	a1,a1,at
    1200000fc:	df8580b8 	ld	a1,-32584\(gp\)
    120000100:	00b1282d 	daddu	a1,a1,s1
@@ -31,26 +31,26 @@ 00000001200000e0 <fn>:
    120000108:	64a5000c 	daddiu	a1,a1,12
    12000010c:	00b1282d 	daddu	a1,a1,s1
    120000110:	df8580b8 	ld	a1,-32584\(gp\)
-   120000114:	3c010002 	lui	at,0x2
-   120000118:	6421e240 	daddiu	at,at,-7616
+   120000114:	3c010001 	lui	at,0x1
+   120000118:	3421e240 	ori	at,at,0xe240
    12000011c:	00a1282d 	daddu	a1,a1,at
    120000120:	00b1282d 	daddu	a1,a1,s1
-   120000124:	df8580b8 	ld	a1,-32584\(gp\)
-   120000128:	dca50000 	ld	a1,0\(a1\)
-   12000012c:	df8580b8 	ld	a1,-32584\(gp\)
-   120000130:	dca5000c 	ld	a1,12\(a1\)
-   120000134:	df8580b8 	ld	a1,-32584\(gp\)
+   120000124:	df858020 	ld	a1,-32736\(gp\)
+   120000128:	dca5052c 	ld	a1,1324\(a1\)
+   12000012c:	df858020 	ld	a1,-32736\(gp\)
+   120000130:	dca50538 	ld	a1,1336\(a1\)
+   120000134:	df858020 	ld	a1,-32736\(gp\)
    120000138:	00b1282d 	daddu	a1,a1,s1
-   12000013c:	dca50000 	ld	a1,0\(a1\)
-   120000140:	df8580b8 	ld	a1,-32584\(gp\)
+   12000013c:	dca5052c 	ld	a1,1324\(a1\)
+   120000140:	df858020 	ld	a1,-32736\(gp\)
    120000144:	00b1282d 	daddu	a1,a1,s1
-   120000148:	dca5000c 	ld	a1,12\(a1\)
-   12000014c:	df8180b8 	ld	at,-32584\(gp\)
+   120000148:	dca50538 	ld	a1,1336\(a1\)
+   12000014c:	df818020 	ld	at,-32736\(gp\)
    120000150:	0025082d 	daddu	at,at,a1
-   120000154:	dc250022 	ld	a1,34\(at\)
-   120000158:	df8180b8 	ld	at,-32584\(gp\)
+   120000154:	dc25054e 	ld	a1,1358\(at\)
+   120000158:	df818020 	ld	at,-32736\(gp\)
    12000015c:	0025082d 	daddu	at,at,a1
-   120000160:	fc250038 	sd	a1,56\(at\)
+   120000160:	fc250564 	sd	a1,1380\(at\)
    120000164:	df8180b8 	ld	at,-32584\(gp\)
    120000168:	88250000 	lwl	a1,0\(at\)
    12000016c:	98250003 	lwr	a1,3\(at\)
@@ -77,42 +77,42 @@ 00000001200000e0 <fn>:
    1200001c0:	0025082d 	daddu	at,at,a1
    1200001c4:	a8250000 	swl	a1,0\(at\)
    1200001c8:	b8250003 	swr	a1,3\(at\)
-   1200001cc:	df858020 	ld	a1,-32736\(gp\)
-   1200001d0:	df858028 	ld	a1,-32728\(gp\)
-   1200001d4:	df858030 	ld	a1,-32720\(gp\)
-   1200001d8:	df858020 	ld	a1,-32736\(gp\)
+   1200001cc:	df858028 	ld	a1,-32728\(gp\)
+   1200001d0:	df858030 	ld	a1,-32720\(gp\)
+   1200001d4:	df858038 	ld	a1,-32712\(gp\)
+   1200001d8:	df858028 	ld	a1,-32728\(gp\)
    1200001dc:	00b1282d 	daddu	a1,a1,s1
-   1200001e0:	df858028 	ld	a1,-32728\(gp\)
+   1200001e0:	df858030 	ld	a1,-32720\(gp\)
    1200001e4:	00b1282d 	daddu	a1,a1,s1
-   1200001e8:	df858030 	ld	a1,-32720\(gp\)
+   1200001e8:	df858038 	ld	a1,-32712\(gp\)
    1200001ec:	00b1282d 	daddu	a1,a1,s1
-   1200001f0:	df858038 	ld	a1,-32712\(gp\)
+   1200001f0:	df858020 	ld	a1,-32736\(gp\)
    1200001f4:	dca5052c 	ld	a1,1324\(a1\)
-   1200001f8:	df858038 	ld	a1,-32712\(gp\)
+   1200001f8:	df858020 	ld	a1,-32736\(gp\)
    1200001fc:	dca50538 	ld	a1,1336\(a1\)
-   120000200:	df858038 	ld	a1,-32712\(gp\)
+   120000200:	df858020 	ld	a1,-32736\(gp\)
    120000204:	00b1282d 	daddu	a1,a1,s1
    120000208:	dca5052c 	ld	a1,1324\(a1\)
-   12000020c:	df858038 	ld	a1,-32712\(gp\)
+   12000020c:	df858020 	ld	a1,-32736\(gp\)
    120000210:	00b1282d 	daddu	a1,a1,s1
    120000214:	dca50538 	ld	a1,1336\(a1\)
-   120000218:	df818038 	ld	at,-32712\(gp\)
+   120000218:	df818020 	ld	at,-32736\(gp\)
    12000021c:	0025082d 	daddu	at,at,a1
    120000220:	dc25054e 	ld	a1,1358\(at\)
-   120000224:	df818038 	ld	at,-32712\(gp\)
+   120000224:	df818020 	ld	at,-32736\(gp\)
    120000228:	0025082d 	daddu	at,at,a1
    12000022c:	fc250564 	sd	a1,1380\(at\)
-   120000230:	df818020 	ld	at,-32736\(gp\)
+   120000230:	df818028 	ld	at,-32728\(gp\)
    120000234:	88250000 	lwl	a1,0\(at\)
    120000238:	98250003 	lwr	a1,3\(at\)
-   12000023c:	df818028 	ld	at,-32728\(gp\)
+   12000023c:	df818030 	ld	at,-32720\(gp\)
    120000240:	88250000 	lwl	a1,0\(at\)
    120000244:	98250003 	lwr	a1,3\(at\)
-   120000248:	df818020 	ld	at,-32736\(gp\)
+   120000248:	df818028 	ld	at,-32728\(gp\)
    12000024c:	0031082d 	daddu	at,at,s1
    120000250:	88250000 	lwl	a1,0\(at\)
    120000254:	98250003 	lwr	a1,3\(at\)
-   120000258:	df818028 	ld	at,-32728\(gp\)
+   120000258:	df818030 	ld	at,-32720\(gp\)
    12000025c:	0031082d 	daddu	at,at,s1
    120000260:	88250000 	lwl	a1,0\(at\)
    120000264:	98250003 	lwr	a1,3\(at\)
@@ -138,8 +138,8 @@ 00000001200000e0 <fn>:
    1200002b4:	df8580c0 	ld	a1,-32576\(gp\)
    1200002b8:	64a5000c 	daddiu	a1,a1,12
    1200002bc:	df8580c0 	ld	a1,-32576\(gp\)
-   1200002c0:	3c010002 	lui	at,0x2
-   1200002c4:	6421e240 	daddiu	at,at,-7616
+   1200002c0:	3c010001 	lui	at,0x1
+   1200002c4:	3421e240 	ori	at,at,0xe240
    1200002c8:	00a1282d 	daddu	a1,a1,at
    1200002cc:	df8580c0 	ld	a1,-32576\(gp\)
    1200002d0:	00b1282d 	daddu	a1,a1,s1
@@ -147,26 +147,26 @@ 00000001200000e0 <fn>:
    1200002d8:	64a5000c 	daddiu	a1,a1,12
    1200002dc:	00b1282d 	daddu	a1,a1,s1
    1200002e0:	df8580c0 	ld	a1,-32576\(gp\)
-   1200002e4:	3c010002 	lui	at,0x2
-   1200002e8:	6421e240 	daddiu	at,at,-7616
+   1200002e4:	3c010001 	lui	at,0x1
+   1200002e8:	3421e240 	ori	at,at,0xe240
    1200002ec:	00a1282d 	daddu	a1,a1,at
    1200002f0:	00b1282d 	daddu	a1,a1,s1
-   1200002f4:	df8580c0 	ld	a1,-32576\(gp\)
-   1200002f8:	dca50000 	ld	a1,0\(a1\)
-   1200002fc:	df8580c0 	ld	a1,-32576\(gp\)
-   120000300:	dca5000c 	ld	a1,12\(a1\)
-   120000304:	df8580c0 	ld	a1,-32576\(gp\)
+   1200002f4:	df858020 	ld	a1,-32736\(gp\)
+   1200002f8:	dca505a4 	ld	a1,1444\(a1\)
+   1200002fc:	df858020 	ld	a1,-32736\(gp\)
+   120000300:	dca505b0 	ld	a1,1456\(a1\)
+   120000304:	df858020 	ld	a1,-32736\(gp\)
    120000308:	00b1282d 	daddu	a1,a1,s1
-   12000030c:	dca50000 	ld	a1,0\(a1\)
-   120000310:	df8580c0 	ld	a1,-32576\(gp\)
+   12000030c:	dca505a4 	ld	a1,1444\(a1\)
+   120000310:	df858020 	ld	a1,-32736\(gp\)
    120000314:	00b1282d 	daddu	a1,a1,s1
-   120000318:	dca5000c 	ld	a1,12\(a1\)
-   12000031c:	df8180c0 	ld	at,-32576\(gp\)
+   120000318:	dca505b0 	ld	a1,1456\(a1\)
+   12000031c:	df818020 	ld	at,-32736\(gp\)
    120000320:	0025082d 	daddu	at,at,a1
-   120000324:	dc250022 	ld	a1,34\(at\)
-   120000328:	df8180c0 	ld	at,-32576\(gp\)
+   120000324:	dc2505c6 	ld	a1,1478\(at\)
+   120000328:	df818020 	ld	at,-32736\(gp\)
    12000032c:	0025082d 	daddu	at,at,a1
-   120000330:	fc250038 	sd	a1,56\(at\)
+   120000330:	fc2505dc 	sd	a1,1500\(at\)
    120000334:	df8180c0 	ld	at,-32576\(gp\)
    120000338:	88250000 	lwl	a1,0\(at\)
    12000033c:	98250003 	lwr	a1,3\(at\)
@@ -202,20 +202,20 @@ 00000001200000e0 <fn>:
    1200003b4:	00b1282d 	daddu	a1,a1,s1
    1200003b8:	df858068 	ld	a1,-32664\(gp\)
    1200003bc:	00b1282d 	daddu	a1,a1,s1
-   1200003c0:	df858038 	ld	a1,-32712\(gp\)
+   1200003c0:	df858020 	ld	a1,-32736\(gp\)
    1200003c4:	dca505a4 	ld	a1,1444\(a1\)
-   1200003c8:	df858038 	ld	a1,-32712\(gp\)
+   1200003c8:	df858020 	ld	a1,-32736\(gp\)
    1200003cc:	dca505b0 	ld	a1,1456\(a1\)
-   1200003d0:	df858038 	ld	a1,-32712\(gp\)
+   1200003d0:	df858020 	ld	a1,-32736\(gp\)
    1200003d4:	00b1282d 	daddu	a1,a1,s1
    1200003d8:	dca505a4 	ld	a1,1444\(a1\)
-   1200003dc:	df858038 	ld	a1,-32712\(gp\)
+   1200003dc:	df858020 	ld	a1,-32736\(gp\)
    1200003e0:	00b1282d 	daddu	a1,a1,s1
    1200003e4:	dca505b0 	ld	a1,1456\(a1\)
-   1200003e8:	df818038 	ld	at,-32712\(gp\)
+   1200003e8:	df818020 	ld	at,-32736\(gp\)
    1200003ec:	0025082d 	daddu	at,at,a1
    1200003f0:	dc2505c6 	ld	a1,1478\(at\)
-   1200003f4:	df818038 	ld	at,-32712\(gp\)
+   1200003f4:	df818020 	ld	at,-32736\(gp\)
    1200003f8:	0025082d 	daddu	at,at,a1
    1200003fc:	fc2505dc 	sd	a1,1500\(at\)
    120000400:	df818058 	ld	at,-32680\(gp\)
@@ -252,24 +252,24 @@ 00000001200000e0 <fn>:
    12000047c:	00000000 	nop
    120000480:	1000ff17 	b	1200000e0 <fn>
    120000484:	df8580b8 	ld	a1,-32584\(gp\)
-   120000488:	df8580c0 	ld	a1,-32576\(gp\)
+   120000488:	df858020 	ld	a1,-32736\(gp\)
    12000048c:	10000015 	b	1200004e4 <fn2>
-   120000490:	dca50000 	ld	a1,0\(a1\)
+   120000490:	dca505a4 	ld	a1,1444\(a1\)
    120000494:	1000ff12 	b	1200000e0 <fn>
-   120000498:	df858020 	ld	a1,-32736\(gp\)
+   120000498:	df858028 	ld	a1,-32728\(gp\)
    12000049c:	df858060 	ld	a1,-32672\(gp\)
    1200004a0:	10000010 	b	1200004e4 <fn2>
    1200004a4:	00000000 	nop
-   1200004a8:	df858030 	ld	a1,-32720\(gp\)
+   1200004a8:	df858038 	ld	a1,-32712\(gp\)
    1200004ac:	1000ff0c 	b	1200000e0 <fn>
    1200004b0:	00000000 	nop
-   1200004b4:	df858038 	ld	a1,-32712\(gp\)
+   1200004b4:	df858020 	ld	a1,-32736\(gp\)
    1200004b8:	1000000a 	b	1200004e4 <fn2>
    1200004bc:	dca505a4 	ld	a1,1444\(a1\)
-   1200004c0:	df858038 	ld	a1,-32712\(gp\)
+   1200004c0:	df858020 	ld	a1,-32736\(gp\)
    1200004c4:	1000ff06 	b	1200000e0 <fn>
    1200004c8:	dca50538 	ld	a1,1336\(a1\)
-   1200004cc:	df818038 	ld	at,-32712\(gp\)
+   1200004cc:	df818020 	ld	at,-32736\(gp\)
    1200004d0:	0025082d 	daddu	at,at,a1
    1200004d4:	10000003 	b	1200004e4 <fn2>
    1200004d8:	dc2505c6 	ld	a1,1478\(at\)
@@ -296,13 +296,13 @@ 00000001201005e0 <_GLOBAL_OFFSET_TABLE_>
 	\.\.\.
    1201005ec:	80000000 	.*
    1201005f0:	00000001 	.*
-   1201005f4:	2010052c 	.*
+   1201005f4:	20100000 	.*
    1201005f8:	00000001 	.*
-   1201005fc:	20100538 	.*
+   1201005fc:	2010052c 	.*
    120100600:	00000001 	.*
-   120100604:	2011e76c 	.*
+   120100604:	20100538 	.*
    120100608:	00000001 	.*
-   12010060c:	20100000 	.*
+   12010060c:	2011e76c 	.*
    120100610:	00000001 	.*
    120100614:	2010054e 	.*
    120100618:	00000001 	.*
@@ -330,3 +330,4 @@ 00000001201005e0 <_GLOBAL_OFFSET_TABLE_>
    12010068c:	2010052c 	.*
    120100690:	00000001 	.*
    120100694:	201005a4 	.*
+#pass
Index: ld/testsuite/ld-mips-elf/elf-rel-xgot-n32.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-mips-elf/elf-rel-xgot-n32.d,v
retrieving revision 1.3
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.3 elf-rel-xgot-n32.d
--- ld/testsuite/ld-mips-elf/elf-rel-xgot-n32.d	7 Dec 2003 21:49:09 -0000	1.3
+++ ld/testsuite/ld-mips-elf/elf-rel-xgot-n32.d	17 Feb 2004 08:56:06 -0000
@@ -19,8 +19,8 @@ 100000b8:	24a5000c 	addiu	a1,a1,12
 100000bc:	3c050000 	lui	a1,0x0
 100000c0:	00bc2821 	addu	a1,a1,gp
 100000c4:	8ca58034 	lw	a1,-32716\(a1\)
-100000c8:	3c010002 	lui	at,0x2
-100000cc:	2421e240 	addiu	at,at,-7616
+100000c8:	3c010001 	lui	at,0x1
+100000cc:	3421e240 	ori	at,at,0xe240
 100000d0:	00a12821 	addu	a1,a1,at
 100000d4:	3c050000 	lui	a1,0x0
 100000d8:	00bc2821 	addu	a1,a1,gp
@@ -34,8 +34,8 @@ 100000f4:	00b12821 	addu	a1,a1,s1
 100000f8:	3c050000 	lui	a1,0x0
 100000fc:	00bc2821 	addu	a1,a1,gp
 10000100:	8ca58034 	lw	a1,-32716\(a1\)
-10000104:	3c010002 	lui	at,0x2
-10000108:	2421e240 	addiu	at,at,-7616
+10000104:	3c010001 	lui	at,0x1
+10000108:	3421e240 	ori	at,at,0xe240
 1000010c:	00a12821 	addu	a1,a1,at
 10000110:	00b12821 	addu	a1,a1,s1
 10000114:	3c050000 	lui	a1,0x0
@@ -192,8 +192,8 @@ 1000036c:	24a5000c 	addiu	a1,a1,12
 10000370:	3c050000 	lui	a1,0x0
 10000374:	00bc2821 	addu	a1,a1,gp
 10000378:	8ca58038 	lw	a1,-32712\(a1\)
-1000037c:	3c010002 	lui	at,0x2
-10000380:	2421e240 	addiu	at,at,-7616
+1000037c:	3c010001 	lui	at,0x1
+10000380:	3421e240 	ori	at,at,0xe240
 10000384:	00a12821 	addu	a1,a1,at
 10000388:	3c050000 	lui	a1,0x0
 1000038c:	00bc2821 	addu	a1,a1,gp
@@ -207,8 +207,8 @@ 100003a8:	00b12821 	addu	a1,a1,s1
 100003ac:	3c050000 	lui	a1,0x0
 100003b0:	00bc2821 	addu	a1,a1,gp
 100003b4:	8ca58038 	lw	a1,-32712\(a1\)
-100003b8:	3c010002 	lui	at,0x2
-100003bc:	2421e240 	addiu	at,at,-7616
+100003b8:	3c010001 	lui	at,0x1
+100003bc:	3421e240 	ori	at,at,0xe240
 100003c0:	00a12821 	addu	a1,a1,at
 100003c4:	00b12821 	addu	a1,a1,s1
 100003c8:	3c050000 	lui	a1,0x0
@@ -425,3 +425,4 @@ 101007cc:	100000a0 	.*
 101007d0:	100006a0 	.*
 101007d4:	101006fc 	.*
 101007d8:	10100774 	.*
+#pass
Index: ld/testsuite/ld-mips-elf/elf-rel-xgot-n64-linux.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-mips-elf/elf-rel-xgot-n64-linux.d,v
retrieving revision 1.2
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.2 elf-rel-xgot-n64-linux.d
--- ld/testsuite/ld-mips-elf/elf-rel-xgot-n64-linux.d	7 Dec 2003 21:49:09 -0000	1.2
+++ ld/testsuite/ld-mips-elf/elf-rel-xgot-n64-linux.d	17 Feb 2004 08:56:06 -0000
@@ -28,8 +28,8 @@ 00000001200000e0 <fn>:
    1200000fc:	3c050000 	lui	a1,0x0
    120000100:	00bc282d 	daddu	a1,a1,gp
    120000104:	dca58058 	ld	a1,-32680\(a1\)
-   120000108:	3c010002 	lui	at,0x2
-   12000010c:	6421e240 	daddiu	at,at,-7616
+   120000108:	3c010001 	lui	at,0x1
+   12000010c:	3421e240 	ori	at,at,0xe240
    120000110:	00a1282d 	daddu	a1,a1,at
    120000114:	3c050000 	lui	a1,0x0
    120000118:	00bc282d 	daddu	a1,a1,gp
@@ -43,8 +43,8 @@ 00000001200000e0 <fn>:
    120000138:	3c050000 	lui	a1,0x0
    12000013c:	00bc282d 	daddu	a1,a1,gp
    120000140:	dca58058 	ld	a1,-32680\(a1\)
-   120000144:	3c010002 	lui	at,0x2
-   120000148:	6421e240 	daddiu	at,at,-7616
+   120000144:	3c010001 	lui	at,0x1
+   120000148:	3421e240 	ori	at,at,0xe240
    12000014c:	00a1282d 	daddu	a1,a1,at
    120000150:	00b1282d 	daddu	a1,a1,s1
    120000154:	3c050000 	lui	a1,0x0
@@ -201,8 +201,8 @@ 00000001200000e0 <fn>:
    1200003b0:	3c050000 	lui	a1,0x0
    1200003b4:	00bc282d 	daddu	a1,a1,gp
    1200003b8:	dca58060 	ld	a1,-32672\(a1\)
-   1200003bc:	3c010002 	lui	at,0x2
-   1200003c0:	6421e240 	daddiu	at,at,-7616
+   1200003bc:	3c010001 	lui	at,0x1
+   1200003c0:	3421e240 	ori	at,at,0xe240
    1200003c4:	00a1282d 	daddu	a1,a1,at
    1200003c8:	3c050000 	lui	a1,0x0
    1200003cc:	00bc282d 	daddu	a1,a1,gp
@@ -216,8 +216,8 @@ 00000001200000e0 <fn>:
    1200003ec:	3c050000 	lui	a1,0x0
    1200003f0:	00bc282d 	daddu	a1,a1,gp
    1200003f4:	dca58060 	ld	a1,-32672\(a1\)
-   1200003f8:	3c010002 	lui	at,0x2
-   1200003fc:	6421e240 	daddiu	at,at,-7616
+   1200003f8:	3c010001 	lui	at,0x1
+   1200003fc:	3421e240 	ori	at,at,0xe240
    120000400:	00a1282d 	daddu	a1,a1,at
    120000404:	00b1282d 	daddu	a1,a1,s1
    120000408:	3c050000 	lui	a1,0x0
@@ -434,3 +434,4 @@ 00000001201007d0 <_GLOBAL_OFFSET_TABLE_>
    12010081c:	2010071c 	.*
    120100820:	00000001 	.*
    120100824:	20100794 	.*
+#pass



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: foo.sh --]
[-- Type: text/x-sh, Size: 631 bytes --]

cat <<EOF > a.s
	.globl	__start
	.globl	gs
__start:
gs:
ls:
	lw	\$4,%got_page(gs)(\$gp)
	addiu	\$4,\$4,%got_ofst(gs)
	lw	\$4,%got_page(us)(\$gp)
	addiu	\$4,\$4,%got_ofst(us)
	lw	\$4,%got_page(ls)(\$gp)
	addiu	\$4,\$4,%got_ofst(ls)
	.data
	.word	us2
EOF
cat <<EOF > b.s
	.globl	us
	.globl	us2
	.globl	gs
us:
us2:
gs:
ls:
	lw	\$4,%got_page(gs)(\$gp)
	addiu	\$4,\$4,%got_ofst(gs)
	lw	\$4,%got_page(us)(\$gp)
	addiu	\$4,\$4,%got_ofst(us)
	lw	\$4,%got_page(ls)(\$gp)
	addiu	\$4,\$4,%got_ofst(ls)
EOF
./gas/as-new a.s -o a.o
./gas/as-new b.s -o b.o
./ld/ld-new b.o -o b.so -shared
./ld/ld-new b.so a.o -o a.x
./ld/ld-new a.o b.so -o b.x

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: RFA: Another MIPS multigot patch
  2004-02-17  9:17                                     ` RFA: " Richard Sandiford
@ 2004-02-17  9:27                                       ` Richard Sandiford
  2004-02-17 10:07                                       ` Eric Christopher
  1 sibling, 0 replies; 23+ messages in thread
From: Richard Sandiford @ 2004-02-17  9:27 UTC (permalink / raw)
  To: binutils

Richard Sandiford <rsandifo@redhat.com> writes:
> Been looking further into this assertion thing, and I think it's
> already covered by:
>
>     static bfd_vma
>     mips_elf_global_got_index [...]
>     {
>       [...]
>       BFD_ASSERT (h->dynindx >= global_got_dynindx);
>       [...]
>     }
>
> If you remove the dynindx check from check_relocs() without applying
> the rest of the patch, this assertion triggers for your test case and
> for the reduced version below.

Correction: if you apply all of the patch except for the
ELF_LINK_HASH_DEF_REGULAR bit.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: RFA: Another MIPS multigot patch
  2004-02-17  9:17                                     ` RFA: " Richard Sandiford
  2004-02-17  9:27                                       ` Richard Sandiford
@ 2004-02-17 10:07                                       ` Eric Christopher
  2004-02-17 15:17                                         ` Daniel Jacobowitz
  1 sibling, 1 reply; 23+ messages in thread
From: Eric Christopher @ 2004-02-17 10:07 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: binutils


> OK to install?

yes.

-eric

-- 
Eric Christopher <echristo@redhat.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: RFA: Another MIPS multigot patch
  2004-02-17 10:07                                       ` Eric Christopher
@ 2004-02-17 15:17                                         ` Daniel Jacobowitz
  0 siblings, 0 replies; 23+ messages in thread
From: Daniel Jacobowitz @ 2004-02-17 15:17 UTC (permalink / raw)
  To: binutils

On Tue, Feb 17, 2004 at 02:06:57AM -0800, Eric Christopher wrote:
> 
> > OK to install?
> 
> yes.

Thank you both!

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2004-02-17 15:17 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-11-21 22:17 Another MIPS multigot patch Daniel Jacobowitz
2003-11-22 16:30 ` Richard Sandiford
2003-11-22 17:40   ` Daniel Jacobowitz
2003-11-22 19:13     ` Richard Sandiford
2003-11-23  1:24       ` Daniel Jacobowitz
2004-01-29 14:46         ` Daniel Jacobowitz
2004-01-29 15:55           ` Richard Sandiford
2004-01-29 16:19             ` Daniel Jacobowitz
2004-01-29 17:39               ` Richard Sandiford
2004-01-29 18:03                 ` Daniel Jacobowitz
2004-01-31 21:13                   ` Richard Sandiford
2004-02-10 22:59                     ` Daniel Jacobowitz
2004-02-11  8:19                       ` Richard Sandiford
2004-02-15 13:46                         ` Richard Sandiford
2004-02-15 18:42                           ` Daniel Jacobowitz
2004-02-15 22:00                             ` Richard Sandiford
2004-02-16  3:17                               ` Daniel Jacobowitz
2004-02-16  8:06                                 ` Richard Sandiford
2004-02-16 18:37                                   ` Daniel Jacobowitz
2004-02-17  9:17                                     ` RFA: " Richard Sandiford
2004-02-17  9:27                                       ` Richard Sandiford
2004-02-17 10:07                                       ` Eric Christopher
2004-02-17 15:17                                         ` Daniel Jacobowitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).