This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH 9/9] MIPS/GAS: Correct MIPS16 HI16/LO16 reloc pairing
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: binutils at sourceware dot org
- Cc: Richard Sandiford <rdsandiford at googlemail dot com>, Catherine Moore <clm at codesourcery dot com>, gnu-mips-sgxx at codesourcery dot com
- Date: Fri, 2 Jul 2010 17:15:36 +0100 (BST)
- Subject: [PATCH 9/9] MIPS/GAS: Correct MIPS16 HI16/LO16 reloc pairing
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"