[PATCH] MIPS/BFD: Correctly report undefined relocations

["Maciej W. Rozycki" <macro at codesourcery dot com>]

Hello,

 The result of the (*undefined_symbol)() callback is interpreted 
incorrectly in mips_elf_calculate_relocation().  The callback is used to 
report a symbol which has already been determined to be undefined and 
returns TRUE under normal conditions or FALSE on a failure (which is 
meant not to happen).

 This change makes mips_elf_calculate_relocation() return 
bfd_reloc_undefined for undefined symbols handled correctly by 
(*undefined_symbol)() and bfd_reloc_notsupported for ones which triggered 
a failure in the callback, which in turn results in an appropriate 
diagnostic message ("internal error: unsupported relocation error") in the 
caller.

 Regression-tested successfully with the mips-sde-elf target.

2009-12-04  Maciej W. Rozycki  <macro@codesourcery.com>

	bfd/
	* elfxx-mips.c (mips_elf_calculate_relocation): Correct handling 
	of undefined symbols.

	ld/testsuite/
	* ld-mips-elf/undefined.d: New test.
	* ld-mips-elf/undefined.s: Source for the new test.
	* ld-mips-elf/mips-elf.exp: Run the new test.

 OK to apply?

  Maciej

binutils-2.20.51-mips-reloc-undef-3.patch
Index: ld/testsuite/ld-mips-elf/undefined.d
===================================================================
--- ld/testsuite/ld-mips-elf/undefined.d	(revision 0)
+++ ld/testsuite/ld-mips-elf/undefined.d	(revision 0)
@@ -0,0 +1,4 @@
+#name: MIPS undefined reference
+#source: undefined.s
+#ld: -e foo
+#error: \A[^\n]*\.o: In function `foo':\n\(\.text\+0x0\): undefined reference to `bar'\Z
Index: ld/testsuite/ld-mips-elf/undefined.s
===================================================================
--- ld/testsuite/ld-mips-elf/undefined.s	(revision 0)
+++ ld/testsuite/ld-mips-elf/undefined.s	(revision 0)
@@ -0,0 +1,24 @@
+# Relocations against undefined symbols would be incorrectly let through
+# by mips_elf_calculate_relocation() once the result of the
+# ->undefined_symbol() callback has been interpreted in the opposite
+# sense.  The link would fail anyway, but for R_MIPS_GOT_PAGE relocations
+# a failure of the following assertion:
+#
+# BFD_ASSERT (h->dynindx >= global_got_dynindx);
+#
+# would additionally be reported in mips_elf_global_got_index(), because
+# at this point h->dynindx for the undefined symbol would be set to -1.
+# Other kinds of GOT relocations allocate a GOT index for the symbol
+# referred and set its h->dynindx in _bfd_mips_elf_check_relocs(), but
+# R_MIPS_GOT_PAGE relocations only allocate a GOT page at that point and
+# for undefined symbols the page never gets resolved any further.
+
+	.abicalls
+	.text
+	.globl	foo
+	.type	foo, @function
+	.ent	foo
+foo:
+	li	$2, %got_page(bar)
+	.end	foo
+	.size	foo, . - foo
Index: ld/testsuite/ld-mips-elf/mips-elf.exp
===================================================================
--- ld/testsuite/ld-mips-elf/mips-elf.exp	(revision 255134)
+++ ld/testsuite/ld-mips-elf/mips-elf.exp	(working copy)
@@ -502,3 +502,5 @@ if { $linux_gnu } {
 	 "got-vers-1.so"}
     }
 }
+
+run_dump_test "undefined"
Index: bfd/elfxx-mips.c
===================================================================
--- bfd/elfxx-mips.c	(revision 255134)
+++ bfd/elfxx-mips.c	(working copy)
@@ -4956,15 +4956,17 @@ mips_elf_calculate_relocation (bfd *abfd
 	     http://techpubs.sgi.com/library/manuals/4000/007-4658-001/pdf/007-4658-001.pdf  */
 	  symbol = 0;
 	}
+      else if ((*info->callbacks->undefined_symbol)
+	       (info, h->root.root.root.string, input_bfd,
+		input_section, relocation->r_offset,
+		(info->unresolved_syms_in_objects == RM_GENERATE_ERROR)
+		 || ELF_ST_VISIBILITY (h->root.other)))
+	{
+	  return bfd_reloc_undefined;
+	}
       else
 	{
-	  if (! ((*info->callbacks->undefined_symbol)
-		 (info, h->root.root.root.string, input_bfd,
-		  input_section, relocation->r_offset,
-		  (info->unresolved_syms_in_objects == RM_GENERATE_ERROR)
-		   || ELF_ST_VISIBILITY (h->root.other))))
-	    return bfd_reloc_undefined;
-	  symbol = 0;
+	  return bfd_reloc_notsupported;
 	}
 
       target_is_16_bit_code_p = ELF_ST_IS_MIPS16 (h->root.other);