[PATCH 9/9] MIPS/GAS: Correct MIPS16 HI16/LO16 reloc pairing

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

Hello,

 In some circumstances MIPS GAS fails to pair MIPS16 HI16 relocations with 
their LO16 counterparts, which is required by the o32 ABI.  It happens 
when they are against local symbols and there are only HI16 relocations 
after the actual symbol definition.

 In this case GAS processes relocations physically preceding the symbol 
definition such that they are associated with the so called "converted 
symbol", that refers to the actual symbol definition indirectly.  
Relocations physically following the symbol definition refer the actual 
symbol definition directly.  As a result the symbols used in each of the 
two types of reference are not the same one and a direct comparison fails 
leaving HI16 relocations orphaned.

 The problem has been noticed elsewhere and a function to compare symbols 
taking converted symbols into account introduced together with a fix for 
the other problem.  I have used the function for HI16/LO16 relocation 
processing in the MIPS backend now making the issue go away -- relocations 
are now paired correctly.

 The problem would only affect MIPS16 relocations, because with standard 
MIPS code such relocations are converted to refer to the section the local 
symbol has been associated with, making any converted symbols go away.

 NB the presence of the relocation associated with the branch is required 
for some reason to trigger the bug here and the offending HI16 relocation 
is moved ahead of the former relocation while fixups are processed.  It is 
most likely an artefact of how these fixups are processed; I haven't 
investigated it further.  The relocation associated with the branch is 
discarded before output is written, except with microMIPS code.  Which 
actually behaves the same as MIPS16 here and the test case will be updated 
accordingly with the addition of microMIPS support.

2010-07-02  Maciej W. Rozycki  <macro@codesourcery.com>

	gas/
	* config/tc-mips.c (mips_frob_file): Use symbol_same_p to match
	symbols.

	gas/testsuite/
	* gas/mips/elf-rel27.d: New test for HI16/LO16 relocation
	pairing.
	* gas/mips/elf-rel27.s: Source for the new test.
	* gas/mips/mips.exp: Create "mips16" architecture.  Define
	"mips" property.  Adjust conditions involving negated properties
	throughout to require "mips1" as appropriate.  Run the new test.
	(mips_arch_destroy): New procedure.

 NB the requirement to add "mips1" annotation to these tests is a strong 
indication they would benefit from the new architecture-specific override 
feature (!gpr_ilocks tests could easily be merged with gpr_ilocks ones).

 OK to commit?

  Maciej

binutils-fsf-2.20.51-20100702-gas-hi-fixup-11.patch
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2010-07-02 02:05:30.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2010-07-02 02:11:46.000000000 +0100
@@ -12234,7 +12234,7 @@
 	    hi_pos = pos;
 
 	  if ((*pos)->fx_r_type == looking_for_rtype
-	      && (*pos)->fx_addsy == l->fixp->fx_addsy
+	      && symbol_same_p ((*pos)->fx_addsy, l->fixp->fx_addsy)
 	      && (*pos)->fx_offset >= l->fixp->fx_offset
 	      && (lo_pos == NULL
 		  || (*pos)->fx_offset < (*lo_pos)->fx_offset
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/elf-rel27.d
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/elf-rel27.d	2010-07-02 02:11:46.000000000 +0100
@@ -0,0 +1,10 @@
+#PROG: readelf
+#readelf: -Wr
+#name: MIPS ELF reloc 27
+#as: -32
+
+Relocation section '\.rel\.text' at offset .* contains [34] entries:
+ *Offset * Info * Type * Sym\. Value * Symbol's Name
+[0-9a-f]+ * [0-9a-f]+ R_(MIPS|MIPS16)_HI16 * [0-9a-f]+ * (\.text|\.L0)
+[0-9a-f]+ * [0-9a-f]+ R_(MIPS|MIPS16)_HI16 * [0-9a-f]+ * (\.text|\.L0)
+[0-9a-f]+ * [0-9a-f]+ R_(MIPS|MIPS16)_LO16 * [0-9a-f]+ * (\.text|\.L0)
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/elf-rel27.s
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/elf-rel27.s	2010-07-02 02:05:34.000000000 +0100
@@ -0,0 +1,9 @@
+	.text
+foo:
+	li	$5, %hi(.L0)
+	sll	$5, 16
+	addiu	$5, %lo(.L0)
+.L0:
+	b	.L0
+	li	$5, %hi(.L0)
+	sll	$5, 16
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips.exp
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/testsuite/gas/mips/mips.exp	2010-07-02 02:05:30.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips.exp	2010-07-02 02:11:46.000000000 +0100
@@ -146,6 +146,19 @@
     }
 }
 
+# mips_arch_destroy ARCH
+#
+# The opposite of the above.  This function removes an entry from
+# the architecture data array, for the architecture or CPU named ARCH.
+
+proc mips_arch_destroy {arch} {
+    global mips_arches
+
+    if { [info exists mips_arches($arch)] } {
+	unset mips_arches($arch)
+    }
+}
+
 # mips_arch_list_all
 #
 # This function returns the list of all names of entries in the
@@ -330,7 +343,7 @@
 # can't easily handle that; do NOT list those targets as defaulting
 # to any architecture.
 mips_arch_init
-mips_arch_create mips1 	32	{}	{} \
+mips_arch_create mips1 	32	{}	{ mips } \
 			{ -march=mips1 -mtune=mips1 } { -mmips:3000 }
 mips_arch_create mips2 	32	mips1	{ gpr_ilocks } \
 		        { -march=mips2 -mtune=mips2 } { -mmips:6000 }
@@ -354,6 +367,8 @@
 			{ -march=mips64r2 -mtune=mips64r2 } \
 			{ -mmips:isa64r2 } \
 			{ mipsisa64r2-*-* mipsisa64r2el-*-* }
+mips_arch_create mips16	32	{}	{ mips } \
+			{ -march=mips1 -mips16 } { -mmips:16 }
 mips_arch_create r3000 	32	mips1	{} \
 			{ -march=r3000 -mtune=r3000 } { -mmips:3000 }
 mips_arch_create r3900 	32	mips1	{ gpr_ilocks } \
@@ -403,6 +418,9 @@
     if { $ecoff } {
 	set no_mips16 1
     }
+    if { $no_mips16 } {
+	mips_arch_destroy mips16
+    }
     
     run_dump_test_arches "abs"		[mips_arch_list_matching mips1]
     run_dump_test_arches "add"		[mips_arch_list_matching mips1]
@@ -456,10 +474,11 @@
     if !$aout {
 	# XXX FIXME: Has mips2 and later insns with mips1 disassemblies.
 	# (Should split and then use appropriate arch lists.)
-	run_dump_test_arches "lb"	[mips_arch_list_matching !mips2]
+	run_dump_test_arches "lb"	[mips_arch_list_matching mips1 !mips2]
     }
     if $elf {
-	run_dump_test_arches "lb-svr4pic" [mips_arch_list_matching !gpr_ilocks]
+	run_dump_test_arches "lb-svr4pic" \
+				[mips_arch_list_matching mips1 !gpr_ilocks]
 	run_dump_test_arches "lb-svr4pic-ilocks" [mips_arch_list_matching gpr_ilocks]
     }
     if $elf {
@@ -496,7 +515,7 @@
     run_dump_test_arches "mips5"	[mips_arch_list_matching mips5]
     run_dump_test "mul"
 
-    run_dump_test_arches "rol"		[mips_arch_list_matching !ror]
+    run_dump_test_arches "rol"		[mips_arch_list_matching mips1 !ror]
     run_dump_test_arches "rol-hw" 	[mips_arch_list_matching ror]
 
     run_dump_test_arches "rol64"	[mips_arch_list_matching gpr64 !ror]
@@ -516,9 +535,11 @@
 	run_dump_test "usw"
 	run_dump_test "usd"
     }
-    run_dump_test_arches "ulw2-eb"	[mips_arch_list_matching !gpr_ilocks]
+    run_dump_test_arches "ulw2-eb" \
+				[mips_arch_list_matching mips1 !gpr_ilocks]
     run_dump_test_arches "ulw2-eb-ilocks" [mips_arch_list_matching gpr_ilocks]
-    run_dump_test_arches "ulw2-el"	[mips_arch_list_matching !gpr_ilocks]
+    run_dump_test_arches "ulw2-el" \
+				[mips_arch_list_matching mips1 !gpr_ilocks]
     run_dump_test_arches "ulw2-el-ilocks" [mips_arch_list_matching gpr_ilocks]
 
     run_dump_test_arches "uld2-eb" [mips_arch_list_matching mips3]
@@ -705,6 +726,8 @@
 	run_dump_test "elf-rel25a"
 	run_dump_test "elf-rel26"
 
+	run_dump_test_arches "elf-rel27" [mips_arch_list_matching mips]
+
 	if { !$no_mips16 } {
 	    run_dump_test "${tmips}mips${el}16-e"
 	    run_dump_test "${tmips}mips${el}16-f"