Re: Another MIPS multigot patch

[Richard Sandiford <rsandifo at redhat dot com>]

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