[PATCH] Fix MIPS symbol difference calculation

Kwok Cheung Yeung kcy@codesourcery.com
Fri May 30 16:01:00 GMT 2014


When targetting MIPS, gas can miscalculate the difference between two 
symbols when used in %hi relocation operators.

e.g.
	.text
foo:
	li	$2, %hi(LAB3-LAB1)
LAB1:
	addiu	$2, %lo(LAB3-LAB1)
LAB2:
	nop
	nop
	nop
	nop
LAB3:
	.fill 32760
	li	$2, %hi(LAB2-LAB4)
LAB4:
	addiu	$2, %lo(LAB2-LAB4)

The problem occurs because fixups are created for all the relocation 
operators except %lo(LAB2-LAB4), because both LAB2 and LAB4 have been 
encountered by the time it is processed.

adjust_reloc_syms() in write.c changes these relocs to be relative to 
.text, so the second and third relocs effectively become:

%lo(.text-LAB1+0x18)
%hi(.text-LAB4+0x8)

mips_frob_file() in tc-mips.c then mistakenly pairs these two together 
when looking for a corresponding %lo for the %hi, because there is no 
corresponding %lo(LAB2-LAB4) fixup, and the earlier %lo appears to be an 
offset from the same symbol .text (ignoring the subtracted symbol). 
mips_frob_file() then overwrites the offset of the %hi with that of the 
%lo, so it becomes:

%hi(.text-LAB4+0x18)

This corrupts the offset, so fixup_segment() later ends up calculating 
the wrong value for this %hi.

This patch works around this by preventing %hi expressions from being 
processed by mips_frob_file() if it contains a subtracted symbol. This 
should be safe as the relocation-pairing is only necessary if the 
relocations actually make it into the output object file, but GAS cannot 
express the difference between two symbols as a relocation on MIPS, so 
any such fixups that are not fully resolvable will result in an error 
anyway.

fixup_has_matching_lo_p() is also modified to check the subtracted 
symbol to avoid mispairings of remaining %hi relocations with %lo 
relocations containing a subtracted symbol.

I have also added the above example as a DejaGnu test.

Kwok
-------------- next part --------------
commit eadb554890624733b20ff6ec6094aeb86b33e0f7
Author: Kwok Cheung Yeung <kcy@codesourcery.com>
Date:   Fri May 30 14:00:24 2014 +0100

    Fix MIPS symbol difference calculation in %hi operators.
    
    This fixes a problem where the difference between two symbols
    may be miscalculated by the assembler when used in a %hi
    relocation operator.
    
    gas/
    	* config/tc-mips.c (fixup_has_matching_lo_p): Also match
    	`fx_subsy' fixup fields.
    	(append_insn): Don't put fixups on `mips_hi_fixup_list' that have
    	non-NULL `fx_subsy' field.
    
    gas/testsuite/
    	* gas/mips/mips.exp: Add new hilo-diff-subsy tests.
    	* gas/mips/hilo-diff-subsy.d: New test file.
    	* gas/mips/hilo-diff-subsy-n32.d: New test file.
    	* gas/mips/hilo-diff-subsy-n64.d: New test file.
    	* gas/mips/micromips@hilo-diff-subsy.d: New test file.
    	* gas/mips/micromips@hilo-diff-subsy-n32.d: New test file.
    	* gas/mips/micromips@hilo-diff-subsy-n64.d: New test file.
    	* gas/mips/mips16@hilo-diff-subsy.d: New test file.
    	* gas/mips/hilo-diff-subsy.s: New test file.

diff --git a/gas/ChangeLog b/gas/ChangeLog
index 98186d3..7c31e30 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,11 @@
+2014-05-30  Kwok Cheung Yeung  <kcy@codesourcery.com>
+            Maciej W. Rozycki  <macro@codesourcery.com>
+
+	* config/tc-mips.c (fixup_has_matching_lo_p): Also match 
+	`fx_subsy' fixup fields.
+	(append_insn): Don't put fixups on `mips_hi_fixup_list' that have
+	non-NULL `fx_subsy' field.
+
 2014-05-22  Alan Modra  <amodra@gmail.com>
 
 	* listing.c (listing_warning, listing_error): Add space after colon.
diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
index 4814a69..ec36cfd 100644
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -3971,6 +3971,7 @@ fixup_has_matching_lo_p (fixS *fixp)
   return (fixp->fx_next != NULL
 	  && fixp->fx_next->fx_r_type == matching_lo_reloc (fixp->fx_r_type)
 	  && fixp->fx_addsy == fixp->fx_next->fx_addsy
+	  && fixp->fx_subsy == fixp->fx_next->fx_subsy
 	  && fixp->fx_offset == fixp->fx_next->fx_offset);
 }
 
@@ -7031,7 +7032,8 @@ append_insn (struct mips_cl_insn *ip, expressionS *address_expr,
 	  if (mips_relax.first_fixup == 0)
 	    mips_relax.first_fixup = ip->fixp[0];
 	}
-      else if (reloc_needs_lo_p (*reloc_type))
+      else if (reloc_needs_lo_p (*reloc_type)
+	       && ip->fixp[0]->fx_subsy == NULL)
 	{
 	  struct mips_hi_fixup *hi_fixup;
 
diff --git a/gas/testsuite/ChangeLog b/gas/testsuite/ChangeLog
index 855add8..41ec441 100644
--- a/gas/testsuite/ChangeLog
+++ b/gas/testsuite/ChangeLog
@@ -1,3 +1,16 @@
+2014-05-30  Kwok Cheung Yeung  <kcy@codesourcery.com>
+            Maciej W. Rozycki  <macro@codesourcery.com>
+
+	* gas/mips/mips.exp: Add new hilo-diff-subsy tests.
+	* gas/mips/hilo-diff-subsy.d: New test file.
+	* gas/mips/hilo-diff-subsy-n32.d: New test file.
+	* gas/mips/hilo-diff-subsy-n64.d: New test file.
+	* gas/mips/micromips@hilo-diff-subsy.d: New test file.
+	* gas/mips/micromips@hilo-diff-subsy-n32.d: New test file.
+	* gas/mips/micromips@hilo-diff-subsy-n64.d: New test file.
+	* gas/mips/mips16@hilo-diff-subsy.d: New test file.
+	* gas/mips/hilo-diff-subsy.s: New test file.
+
 2014-05-22  Alan Modra  <amodra@gmail.com>
 
 	* gas/d30v/bittest.l: Update for changed whitespace.
diff --git a/gas/testsuite/gas/mips/hilo-diff-subsy-n32.d b/gas/testsuite/gas/mips/hilo-diff-subsy-n32.d
new file mode 100644
index 0000000..200fa84
--- /dev/null
+++ b/gas/testsuite/gas/mips/hilo-diff-subsy-n32.d
@@ -0,0 +1,5 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS hilo-diff-subsy n32
+#as: -n32
+#source: hilo-diff-subsy.s
+#dump: hilo-diff-subsy.d
diff --git a/gas/testsuite/gas/mips/hilo-diff-subsy-n64.d b/gas/testsuite/gas/mips/hilo-diff-subsy-n64.d
new file mode 100644
index 0000000..7a0d0e2
--- /dev/null
+++ b/gas/testsuite/gas/mips/hilo-diff-subsy-n64.d
@@ -0,0 +1,5 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS hilo-diff-subsy n64
+#as: -64
+#source: hilo-diff-subsy.s
+#dump: hilo-diff-subsy.d
diff --git a/gas/testsuite/gas/mips/hilo-diff-subsy.d b/gas/testsuite/gas/mips/hilo-diff-subsy.d
new file mode 100644
index 0000000..ecaf0b9
--- /dev/null
+++ b/gas/testsuite/gas/mips/hilo-diff-subsy.d
@@ -0,0 +1,14 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS hilo-diff-subsy o32
+#as: -32
+#source: hilo-diff-subsy.s
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+[0-9a-f]+ <[^>]*> 24020000 	li	v0,0
+[0-9a-f]+ <[^>]*> 24420014 	addiu	v0,v0,20
+#...
+[0-9a-f]+ <[^>]*> 2402ffff 	li	v0,-1
+[0-9a-f]+ <[^>]*> 24427ff4 	addiu	v0,v0,32756
+#pass
diff --git a/gas/testsuite/gas/mips/hilo-diff-subsy.s b/gas/testsuite/gas/mips/hilo-diff-subsy.s
new file mode 100644
index 0000000..8efedbf
--- /dev/null
+++ b/gas/testsuite/gas/mips/hilo-diff-subsy.s
@@ -0,0 +1,15 @@
+	.text
+foo:
+	li	$2, %hi(LAB3-LAB1)
+LAB1:
+	addiu	$2, %lo(LAB3-LAB1)
+LAB2:
+	nop
+	nop
+	nop
+	nop
+LAB3:
+	.fill 32760
+	li	$2, %hi(LAB2-LAB4)
+LAB4:
+	addiu	$2, %lo(LAB2-LAB4)
diff --git a/gas/testsuite/gas/mips/micromips@hilo-diff-subsy-n32.d b/gas/testsuite/gas/mips/micromips@hilo-diff-subsy-n32.d
new file mode 100644
index 0000000..931a68f
--- /dev/null
+++ b/gas/testsuite/gas/mips/micromips@hilo-diff-subsy-n32.d
@@ -0,0 +1,5 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS hilo-diff-subsy n32
+#as: -n32
+#source: hilo-diff-subsy.s
+#dump: micromips@hilo-diff-subsy.d
diff --git a/gas/testsuite/gas/mips/micromips@hilo-diff-subsy-n64.d b/gas/testsuite/gas/mips/micromips@hilo-diff-subsy-n64.d
new file mode 100644
index 0000000..dfbcd2d
--- /dev/null
+++ b/gas/testsuite/gas/mips/micromips@hilo-diff-subsy-n64.d
@@ -0,0 +1,5 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS hilo-diff-subsy n64
+#as: -64
+#source: hilo-diff-subsy.s
+#dump: micromips@hilo-diff-subsy.d
diff --git a/gas/testsuite/gas/mips/micromips@hilo-diff-subsy.d b/gas/testsuite/gas/mips/micromips@hilo-diff-subsy.d
new file mode 100644
index 0000000..f75de82
--- /dev/null
+++ b/gas/testsuite/gas/mips/micromips@hilo-diff-subsy.d
@@ -0,0 +1,14 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS hilo-diff-subsy o32
+#as: -32
+#source: hilo-diff-subsy.s
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+[0-9a-f]+ <[^>]*> 3040 0000 	li	v0,0
+[0-9a-f]+ <[^>]*> 3042 000b 	addiu	v0,v0,11
+#...
+[0-9a-f]+ <[^>]*> 3040ffff 	andi	zero,v0,0xffff
+[0-9a-f]+ <[^>]*> 3042 7ffc 	addiu	v0,v0,32764
+#pass
diff --git a/gas/testsuite/gas/mips/mips.exp b/gas/testsuite/gas/mips/mips.exp
index c3135ca..793f29e 100644
--- a/gas/testsuite/gas/mips/mips.exp
+++ b/gas/testsuite/gas/mips/mips.exp
@@ -1149,6 +1149,13 @@ if { [istarget mips*-*-vxworks*] } {
 	run_dump_test_arches "hilo-diff-eb-n64" [mips_arch_list_matching mips3]
 	run_dump_test_arches "hilo-diff-el-n64" [mips_arch_list_matching mips3]
     }
+    run_dump_test_arches "hilo-diff-subsy" [mips_arch_list_all]
+    if $has_newabi {
+	run_dump_test_arches "hilo-diff-subsy-n32" \
+				[mips_arch_list_matching mips3]
+	run_dump_test_arches "hilo-diff-subsy-n64" \
+				[mips_arch_list_matching mips3]
+    }
     run_dump_test_arches "lui"		[mips_arch_list_matching mips1]
     run_list_test_arches "lui-1" "-32"	[mips_arch_list_matching mips1]
     run_list_test_arches "lui-2" "-32"	[mips_arch_list_matching mips1]
diff --git a/gas/testsuite/gas/mips/mips16@hilo-diff-subsy.d b/gas/testsuite/gas/mips/mips16@hilo-diff-subsy.d
new file mode 100644
index 0000000..54f515a
--- /dev/null
+++ b/gas/testsuite/gas/mips/mips16@hilo-diff-subsy.d
@@ -0,0 +1,14 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS hilo-diff-subsy o32
+#as: -32
+#source: hilo-diff-subsy.s
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+[0-9a-f]+ <[^>]*> f000 6a00 	li	v0,0
+[0-9a-f]+ <[^>]*> f000 4a0b 	addiu	v0,11
+#...
+[0-9a-f]+ <[^>]*> f7ff 6a1f 	li	v0,65535
+[0-9a-f]+ <[^>]*> f7ef 4a1c 	addiu	v0,32764
+#pass


More information about the Binutils mailing list