This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
[PATCH' RFA] fix generation of MIPS -membedded-pic jumps
So, after further discussion with Eric, it was decided that this patch
really probably wasn't the right thing.
So, I've now got a new patch which may also not be the right thing,
but at least isn't so obviously wrong. (If only because it's much,
much less obvious. 8-)
Anyway, after tracing through the code to see where the symbol value
was being added in incorrectly for the jump-to-global-symbol case, I
traced through the code in write.c that uses the undocumented macros
TC_RELOC_RTSYM_LOC_FIXUP and TC_FIX_ADJUSTABLE.
MIPS didn't currently define them, but I was able to come up with
definitions (restricted to ELF and the relocation that was having the
problem) that seem to produce the correct result. Given that they're
not documented, it's hard to verify that i'm using them correctly.
However, in context, it doesn't look like i'm using them
_incorrectly_. 8-)
This is the tc-mips.h change, and the jal-empic-elf{,-2} tests. (the
all-but-last hunk of the tc-mips.c change also were related to this.)
Once that was done (which addresses a slightly more complex version of
my previous test-case), I turned my attention to a thornier problem:
handling of the 'value == 0' case in the MIPS md_apply_fix3.
I came up with some instructions which explicitly target that for the
embedded-PIC case, calculated by hand the relocations expected in the
assembly output, generated a test case, and, uh, tweaked until I got
those results.
This is the last hunk of the tc-mips.c change, and the jal-empic-elf-3
test.
Reading the diffs, it's fairly clear that these changes only impact
the -membedded-pic jumps/branches. (Alas, in this change, even that
isn't truly obvious. 8-)
For testing:
Cross-compiled our local tools (based on 2002-01-31 trunk, with
relatively few mods to existing code in binutils) with this change,
from host x86-linux, for:
* a "mips-elf"-like target, and used that to build our Big
-membedded-pic Test Case and verified that it worked.
(I also verified, by use of an assert, that as I expected
it never hits the 'value == 0' tweaks.)
* mips-linux. Built a kernel and userland, and verified
that they all seem OK.
* mips-netbsd. Same thing.
(Our tools have no patches to chunks of code related to this patch --
other than attempts at fixing this problem, which I backed out.)
Also, gmake && gmake check in current head-of-branch binutils for
mips-elf, mips64-elf, and mips-linux.
Given that, _really_, no i mean really, this change doesn't impact
anything other than these particular -membedded-pic relocs, I didn't
go to the trouble of a gcc+sim check. I can if people want me to.
I want this for both the trunk and the branch; this is the last thing
keeping -membedded-pic from working for ELF in the stock binutils
sources.
cgd
===================================================================
[gas/ChangeLog]
2002-02-12 Chris Demetriou <cgd@broadcom.com>
* config/tc-mips.c (mips_need_elf_addend_fixup): Make this
function externally visible. Also, restructure it into
a sequence of indpendent 'if' statements for easier debugging.
(md_apply_fix3): When deposting the relocation value into
relocations because 'value == 0', handle ELF BFD_RELOC_16_PCREL_S2
in a way that produces the correct results.
* config/tc-mips.h (TC_RELOC_RTSYM_LOC_FIXUP, TC_FIX_ADJUSTABLE):
New macros, defined so that ELF BFD_RELOC_16_PCREL_S2 relocations
are produced correctly.
[gas/testsuite/ChangeLog]
2002-02-12 Chris Demetriou <cgd@broadcom.com>
* gas/mips/jal-empic-elf.d: New file.
* gas/mips/jal-empic-elf-2.d: Likewise.
* gas/mips/jal-empic-elf-2.s: Likewise.
* gas/mips/jal-empic-elf-3.d: Likewise.
* gas/mips/jal-empic-elf-3.s: Likewise.
* gas/mips/mips.exp: Run the jal-empic-elf,
jal-empic-elf-2, and jal-empic-elf-3 tests.
Index: gas/config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.107
diff -u -p -r1.107 tc-mips.c
--- tc-mips.c 2002/02/08 22:25:36 1.107
+++ tc-mips.c 2002/02/12 23:36:05
@@ -744,7 +744,7 @@ static const char *mips_cpu_to_str PARAM
static int validate_mips_insn PARAMS ((const struct mips_opcode *));
static void show PARAMS ((FILE *, char *, int *, int *));
#ifdef OBJ_ELF
-static int mips_need_elf_addend_fixup PARAMS ((fixS *));
+int mips_need_elf_addend_fixup PARAMS ((fixS *));
#endif
/* Return values of my_getSmallExpression(). */
@@ -10400,21 +10400,25 @@ mips_force_relocation (fixp)
}
#ifdef OBJ_ELF
-static int
+int
mips_need_elf_addend_fixup (fixP)
fixS *fixP;
{
- return (S_GET_OTHER (fixP->fx_addsy) == STO_MIPS16
- || ((S_IS_WEAK (fixP->fx_addsy)
- || S_IS_EXTERN (fixP->fx_addsy))
- && !S_IS_COMMON (fixP->fx_addsy))
- || (symbol_used_in_reloc_p (fixP->fx_addsy)
- && (((bfd_get_section_flags (stdoutput,
- S_GET_SEGMENT (fixP->fx_addsy))
- & SEC_LINK_ONCE) != 0)
- || !strncmp (segment_name (S_GET_SEGMENT (fixP->fx_addsy)),
- ".gnu.linkonce",
- sizeof (".gnu.linkonce") - 1))));
+ if (S_GET_OTHER (fixP->fx_addsy) == STO_MIPS16)
+ return 1;
+ if ((S_IS_WEAK (fixP->fx_addsy)
+ || S_IS_EXTERN (fixP->fx_addsy))
+ && !S_IS_COMMON (fixP->fx_addsy))
+ return 1;
+ if (symbol_used_in_reloc_p (fixP->fx_addsy)
+ && (((bfd_get_section_flags (stdoutput,
+ S_GET_SEGMENT (fixP->fx_addsy))
+ & SEC_LINK_ONCE) != 0)
+ || !strncmp (segment_name (S_GET_SEGMENT (fixP->fx_addsy)),
+ ".gnu.linkonce",
+ sizeof (".gnu.linkonce") - 1)))
+ return 1;
+ return 0;
}
#endif
@@ -10698,7 +10702,24 @@ md_apply_fix3 (fixP, valP, seg)
do the store, so it must be done here. This is probably
a bug somewhere. */
if (!fixP->fx_done)
- value -= fixP->fx_frag->fr_address + fixP->fx_where;
+ {
+ value -= fixP->fx_frag->fr_address + fixP->fx_where;
+
+ /* The following was cobbled together using trial and error.
+ It too is fragile and untrustworthy. If you change this
+ code or the code above, be sure that the tests in
+ jal-empic-elf-3 which target this case still _hit_ this
+ case and that they produce the correct results. */
+ if (OUTPUT_FLAVOR == bfd_target_elf_flavour
+ && fixP->fx_r_type == BFD_RELOC_16_PCREL_S2
+ && fixP->fx_addsy != NULL)
+ {
+ value += S_GET_VALUE (fixP->fx_addsy);
+ if (S_IS_DEFINED(fixP->fx_addsy)
+ && ! mips_need_elf_addend_fixup (fixP))
+ value += fixP->fx_frag->fr_address + fixP->fx_where;
+ }
+ }
value = (offsetT) value >> 2;
Index: gas/config/tc-mips.h
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.h,v
retrieving revision 1.14
diff -u -p -r1.14 tc-mips.h
--- tc-mips.h 2001/12/18 14:35:34 1.14
+++ tc-mips.h 2002/02/12 23:36:05
@@ -111,6 +111,19 @@ extern int mips_fix_adjustable PARAMS ((
#define TC_FORCE_RELOCATION(fixp) mips_force_relocation (fixp)
extern int mips_force_relocation PARAMS ((struct fix *));
+/* Embedded PIC jumps to global symbols need special handling, too. */
+#ifdef OBJ_ELF
+#define TC_RELOC_RTSYM_LOC_FIXUP(FIX) \
+ (OUTPUT_FLAVOR != bfd_target_elf_flavour \
+ || (FIX)->fx_r_type != BFD_RELOC_16_PCREL_S2)
+
+#define TC_FIX_ADJUSTABLE(FIX) \
+ (OUTPUT_FLAVOR != bfd_target_elf_flavour \
+ || (FIX)->fx_r_type != BFD_RELOC_16_PCREL_S2 \
+ || !mips_need_elf_addend_fixup (FIX))
+#endif
+extern int mips_need_elf_addend_fixup PARAMS ((struct fix *));
+
/* Register mask variables. These are set by the MIPS assembly code
and used by ECOFF and possibly other object file formats. */
extern unsigned long mips_gprmask;
Index: gas/testsuite/gas/mips/jal-empic-elf-2.d
===================================================================
RCS file: jal-empic-elf-2.d
diff -N jal-empic-elf-2.d
--- /dev/null Tue May 5 13:32:27 1998
+++ jal-empic-elf-2.d Tue Feb 12 15:36:07 2002
@@ -0,0 +1,57 @@
+#objdump: -dr --prefix-addresses -mmips:3000 --show-raw-insn
+#name: MIPS jal-empic-elf-2
+#as: -mips1 -membedded-pic
+
+# Test the jal macro harder with -membedded-pic.
+
+.*: +file format .*mips.*
+
+Disassembly of section .text:
+ \.\.\.
+ \.\.\.
+0+0018 <[^>]*> 0411ffff bal 0+0018 <g1\+0xc>
+[ ]*18: R_MIPS_GNU_REL16_S2 g1
+0+001c <[^>]*> 00000000 nop
+0+0020 <[^>]*> 04110002 bal 0+002c <g1\+0x20>
+[ ]*20: R_MIPS_GNU_REL16_S2 .text
+0+0024 <[^>]*> 00000000 nop
+0+0028 <[^>]*> 0411ffff bal 0+0028 <g1\+0x1c>
+[ ]*28: R_MIPS_GNU_REL16_S2 e1
+0+002c <[^>]*> 00000000 nop
+0+0030 <[^>]*> 1000ffff b 0+0030 <g1\+0x24>
+[ ]*30: R_MIPS_GNU_REL16_S2 g1
+0+0034 <[^>]*> 00000000 nop
+0+0038 <[^>]*> 10000002 b 0+0044 <g1\+0x38>
+[ ]*38: R_MIPS_GNU_REL16_S2 .text
+0+003c <[^>]*> 00000000 nop
+0+0040 <[^>]*> 1000ffff b 0+0040 <g1\+0x34>
+[ ]*40: R_MIPS_GNU_REL16_S2 e1
+0+0044 <[^>]*> 00000000 nop
+0+0048 <[^>]*> 0411fffc bal 0+003c <g1\+0x30>
+[ ]*48: R_MIPS_GNU_REL16_S2 g1
+0+004c <[^>]*> 00000000 nop
+0+0050 <[^>]*> 0411ffff bal 0+0050 <g1\+0x44>
+[ ]*50: R_MIPS_GNU_REL16_S2 .text
+0+0054 <[^>]*> 00000000 nop
+0+0058 <[^>]*> 0411fffc bal 0+004c <g1\+0x40>
+[ ]*58: R_MIPS_GNU_REL16_S2 e1
+0+005c <[^>]*> 00000000 nop
+0+0060 <[^>]*> 04110002 bal 0+006c <g1\+0x60>
+[ ]*60: R_MIPS_GNU_REL16_S2 g1
+0+0064 <[^>]*> 00000000 nop
+0+0068 <[^>]*> 04110005 bal 0+0080 <g1\+0x74>
+[ ]*68: R_MIPS_GNU_REL16_S2 .text
+0+006c <[^>]*> 00000000 nop
+0+0070 <[^>]*> 04110002 bal 0+007c <g1\+0x70>
+[ ]*70: R_MIPS_GNU_REL16_S2 e1
+0+0074 <[^>]*> 00000000 nop
+0+0078 <[^>]*> 0411ffe5 bal 0+0010 <g1\+0x4>
+[ ]*78: R_MIPS_GNU_REL16_S2 g1
+0+007c <[^>]*> 00000000 nop
+0+0080 <[^>]*> 0411ffe0 bal 0+0004 <g1\-0x8>
+[ ]*80: R_MIPS_GNU_REL16_S2 .text
+0+0084 <[^>]*> 00000000 nop
+0+0088 <[^>]*> 0411ffde bal 0+0004 <g1\-0x8>
+[ ]*88: R_MIPS_GNU_REL16_S2 e1
+0+008c <[^>]*> 00000000 nop
+ \.\.\.
Index: gas/testsuite/gas/mips/jal-empic-elf-2.s
===================================================================
RCS file: jal-empic-elf-2.s
diff -N jal-empic-elf-2.s
--- /dev/null Tue May 5 13:32:27 1998
+++ jal-empic-elf-2.s Tue Feb 12 15:36:07 2002
@@ -0,0 +1,28 @@
+# Source file used to test the jal macro even harder
+ # some space so offets won't be 0.
+ .space 0xc
+
+ .globl g1 .text
+g1:
+l1:
+ # some more space, so offset from label won't be 0.
+ .space 0xc
+
+ jal g1 # 0x18
+ jal l1 # 0x20
+ jal e1 # 0x28
+
+ j g1 # 0x30
+ j l1 # 0x38
+ j e1 # 0x40
+
+ jal g1 - 0xc # 0x48
+ jal l1 - 0xc # 0x50
+ jal e1 - 0xc # 0x58
+
+ jal g1 + 0xc # 0x60
+ jal l1 + 0xc # 0x68
+ jal e1 + 0xc # 0x70
+
+# Force at least 8 (non-delay-slot) zero bytes, to make 'objdump' print ...
+ .space 8
Index: gas/testsuite/gas/mips/jal-empic-elf-3.d
===================================================================
RCS file: jal-empic-elf-3.d
diff -N jal-empic-elf-3.d
--- /dev/null Tue May 5 13:32:27 1998
+++ jal-empic-elf-3.d Tue Feb 12 15:36:07 2002
@@ -0,0 +1,21 @@
+#objdump: -dr --prefix-addresses -mmips:3000 --show-raw-insn
+#name: MIPS jal-empic-elf-2
+#as: -mips1 -membedded-pic
+
+# Test the jal macro harder with -membedded-pic.
+
+.*: +file format .*mips.*
+
+Disassembly of section .text:
+ \.\.\.
+ \.\.\.
+0+0018 <[^>]*> 0411fffd bal 0+0010 <g1\+0x4>
+[ ]*18: R_MIPS_GNU_REL16_S2 g1
+0+001c <[^>]*> 00000000 nop
+0+0020 <[^>]*> 0411fff8 bal 0+0004 <g1\-0x8>
+[ ]*20: R_MIPS_GNU_REL16_S2 .text
+0+0024 <[^>]*> 00000000 nop
+0+0028 <[^>]*> 0411fff6 bal 0+0004 <g1\-0x8>
+[ ]*28: R_MIPS_GNU_REL16_S2 e1
+0+002c <[^>]*> 00000000 nop
+ \.\.\.
Index: gas/testsuite/gas/mips/jal-empic-elf-3.s
===================================================================
RCS file: jal-empic-elf-3.s
diff -N jal-empic-elf-3.s
--- /dev/null Tue May 5 13:32:27 1998
+++ jal-empic-elf-3.s Tue Feb 12 15:36:07 2002
@@ -0,0 +1,18 @@
+# Source file used to test the jal macro even harder
+ # some space so offets won't be 0.
+ .space 0xc
+
+ .globl g1 .text
+g1:
+l1:
+ # some more space, so offset from label won't be 0.
+ .space 0xc
+
+ # Hit the case where 'value == 0' in the BFD_RELOC_16_PCREL_S2
+ # handling in tc-mips.c:md_apply_fix3().
+ jal g1 - 0x08 # 0x18
+ jal l1 - 0x28 # 0x20
+ jal e1 - 0x24 # 0x28
+
+# Force at least 8 (non-delay-slot) zero bytes, to make 'objdump' print ...
+ .space 8
Index: gas/testsuite/gas/mips/jal-empic-elf.d
===================================================================
RCS file: jal-empic-elf.d
diff -N jal-empic-elf.d
--- /dev/null Tue May 5 13:32:27 1998
+++ jal-empic-elf.d Tue Feb 12 15:36:07 2002
@@ -0,0 +1,26 @@
+#objdump: -dr --prefix-addresses -mmips:3000 --show-raw-insn
+#name: MIPS jal-empic-elf
+#as: -mips1 -membedded-pic
+#source: jal.s
+
+# Test the jal macro with -membedded-pic.
+
+.*: +file format .*mips.*
+
+Disassembly of section .text:
+0+0000 <[^>]*> 0320f809 jalr t9
+0+0004 <[^>]*> 00000000 nop
+0+0008 <[^>]*> 03202009 jalr a0,t9
+0+000c <[^>]*> 00000000 nop
+0+0010 <[^>]*> 0411ffff bal 0+0010 <text_label\+0x10>
+[ ]*10: R_MIPS_GNU_REL16_S2 text_label
+0+0014 <[^>]*> 00000000 nop
+0+0018 <[^>]*> 0411ffff bal 0+0018 <text_label\+0x18>
+[ ]*18: R_MIPS_GNU_REL16_S2 external_text_label
+0+001c <[^>]*> 00000000 nop
+0+0020 <[^>]*> 1000ffff b 0+0020 <text_label\+0x20>
+[ ]*20: R_MIPS_GNU_REL16_S2 text_label
+0+0024 <[^>]*> 00000000 nop
+0+0028 <[^>]*> 1000ffff b 0+0028 <text_label\+0x28>
+[ ]*28: R_MIPS_GNU_REL16_S2 external_text_label
+0+002c <[^>]*> 00000000 nop
Index: gas/testsuite/gas/mips/mips.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/mips.exp,v
retrieving revision 1.28
diff -u -p -r1.28 mips.exp
--- mips.exp 2002/02/09 07:18:54 1.28
+++ mips.exp 2002/02/12 23:36:07
@@ -70,6 +70,9 @@ if { [istarget mips*-*-*] } then {
# It appears that it broke between 2000-03-11 00:00UTC and
# 2000-03-12 00:00 UTC.
if $ecoff { run_dump_test "jal-empic" }
+ if $elf { run_dump_test "jal-empic-elf" }
+ if $elf { run_dump_test "jal-empic-elf-2" }
+ if $elf { run_dump_test "jal-empic-elf-3" }
if !$aout { run_dump_test "la" }
if $elf { run_dump_test "la-svr4pic" }
if $elf { run_dump_test "la-xgot" }