This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: [patch] MIPS: Incorrect calculation for R_MIPS_LO16 relocs
Here's another way of fixing Maciej's testcase. It stops gas from
turning R_MIPS_{LO,HI,GOT}16 relocations against mergeable symbols
into section-relative ones.
If we were only now adding support for mergeable sections and split REL
relocations, I'm sure we wouldn't want to do it like this. Something like
Maciej's patch would be cleaner.
Unfortunately, there's a long history of not putting HI16s before LO16s,
and if we patched the linker now, it would silently mishandle existing
references to mergeable section symbols + offsets. Patching gas seems
much less invasive and keeps the existing ABI.
I tested this on:
mips64-elf mips64el-elf mips-elf mipsel-elf mips-ecoff
mips-linux-gnu mipsel-linux-gnu mips64-linux-gnu mips64el-linux-gnu
no regressions. I've reattached Maciej's testcase with a couple of
trivial changes to the *.d file. (Specifically: there's no need to
force little-endian code, since the testcase works with either endianness.
There's also no need to use -melf32ltsmip, which not all configurations
support, since the testcase works for both REL and RELA targets. Of course,
we're mostly interested in REL here, but the test should still pass for
RELA as well.)
Note the patch introduces a new macro, HAVE_IN_PLACE_OFFSETS. Using a
simple macro like that seemed more efficient than finding & checking
the underlying HOWTO.
I've also moved the function-wide comment above mips_fix_adjustable so
that individual statements are commented instead. I thought that was
better than trying to work the new comment into the existing one.
Anyway, just sending this is an alternative...
Richard
gas/
* config/tc-mips.c (HAVE_IN_PLACE_OFFSETS): New macro.
(mips_fix_adjustable): If the full addend is going to split into more
than one in-place addend, return 0 for relocations against mergeable
sections. Associate comments with code.
gas/testsuite/
* gas/mips/elf-rel7.d: Expect relocations against bar to refer to bar.
* gas/mips/elf-refl19.d: Likewise L2.
Index: gas/config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.266
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.266 tc-mips.c
*** gas/config/tc-mips.c 15 Jun 2004 01:16:35 -0000 1.266
--- gas/config/tc-mips.c 29 Jun 2004 11:28:15 -0000
*************** #define HAVE_32BIT_ADDRESSES
*** 287,292 ****
--- 287,295 ----
#define HAVE_64BIT_ADDRESSES (! HAVE_32BIT_ADDRESSES)
+ /* True if we'll generate relocations with in-place offsets. */
+ #define HAVE_IN_PLACE_OFFSETS (!HAVE_NEWABI)
+
/* Addresses are loaded in different ways, depending on the address size
in use. The n32 ABI Documentation also mandates the use of additions
with overflow checking, but existing implementations don't follow it. */
*************** md_estimate_size_before_relax (fragS *fr
*** 12693,12707 ****
}
/* This is called to see whether a reloc against a defined symbol
! should be converted into a reloc against a section. Don't adjust
! MIPS16 jump relocations, so we don't have to worry about the format
! of the offset in the .o file. Don't adjust relocations against
! mips16 symbols, so that the linker can find them if it needs to set
! up a stub. */
int
mips_fix_adjustable (fixS *fixp)
{
if (fixp->fx_r_type == BFD_RELOC_MIPS16_JMP)
return 0;
--- 12696,12708 ----
}
/* This is called to see whether a reloc against a defined symbol
! should be converted into a reloc against a section. */
int
mips_fix_adjustable (fixS *fixp)
{
+ /* Don't adjust MIPS16 jump relocations, so we don't have to worry
+ about the format of the offset in the .o file. */
if (fixp->fx_r_type == BFD_RELOC_MIPS16_JMP)
return 0;
*************** mips_fix_adjustable (fixS *fixp)
*** 12712,12718 ****
--- 12713,12740 ----
if (fixp->fx_addsy == NULL)
return 1;
+ /* If symbol SYM is in a mergeable section, relocations of the form
+ SYM + 0 can usually be made section-relative. The mergeable data
+ is then identified by the section offset rather than by the symbol.
+
+ However, if we're generating REL LO16 relocations, the offset is split
+ between the LO16 and parterning high part relocation. The linker will
+ need to recalculate the complete offset in order to correctly identify
+ the merge data.
+
+ The linker has traditionally not looked for the parterning high part
+ relocation, and has thus allowed orphaned R_MIPS_LO16 relocations to be
+ placed anywhere. Rather than break backwards compatibility by changing
+ this, it seems better not to force the issue, and instead keep the
+ original symbol. This will work with either linker behavior. */
+ if ((fixp->fx_r_type == BFD_RELOC_LO16 || reloc_needs_lo_p (fixp->fx_r_type))
+ && HAVE_IN_PLACE_OFFSETS
+ && (S_GET_SEGMENT (fixp->fx_addsy)->flags & SEC_MERGE) != 0)
+ return 0;
+
#ifdef OBJ_ELF
+ /* Don't adjust relocations against mips16 symbols, so that the linker
+ can find them if it needs to set up a stub. */
if (OUTPUT_FLAVOR == bfd_target_elf_flavour
&& S_GET_OTHER (fixp->fx_addsy) == STO_MIPS16
&& fixp->fx_subsy == NULL)
Index: gas/testsuite/gas/mips/elf-rel7.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/elf-rel7.d,v
retrieving revision 1.2
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.2 elf-rel7.d
*** gas/testsuite/gas/mips/elf-rel7.d 7 May 2003 05:08:20 -0000 1.2
--- gas/testsuite/gas/mips/elf-rel7.d 29 Jun 2004 11:28:15 -0000
***************
*** 6,14 ****
Disassembly of section \.text:
0+00 <.*> lui a0,0x0
! 0: R_MIPS_HI16 .barsec
! 0+04 <.*> lw a0,8\(a0\)
! 4: R_MIPS_LO16 .barsec
0+08 <.*> lui a0,0x0
8: R_MIPS_HI16 bar
0+0c <.*> lw a0,4\(a0\)
--- 6,14 ----
Disassembly of section \.text:
0+00 <.*> lui a0,0x0
! 0: R_MIPS_HI16 bar
! 0+04 <.*> lw a0,0\(a0\)
! 4: R_MIPS_LO16 bar
0+08 <.*> lui a0,0x0
8: R_MIPS_HI16 bar
0+0c <.*> lw a0,4\(a0\)
Index: gas/testsuite/gas/mips/elf-rel19.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/elf-rel19.d,v
retrieving revision 1.1
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.1 elf-rel19.d
*** gas/testsuite/gas/mips/elf-rel19.d 23 Jan 2004 12:58:22 -0000 1.1
--- gas/testsuite/gas/mips/elf-rel19.d 29 Jun 2004 11:28:15 -0000
*************** 00000000 <.*>:
*** 10,19 ****
# Relocation agsinst .rodata.str1.1
#
.*: 8f840000 lw a0,0\(gp\)
! .*: R_MIPS_GOT16 \.rodata\.str1\.1
.*: 00000000 nop
! .*: 24840004 addiu a0,a0,4
! .*: R_MIPS_LO16 \.rodata\.str1\.1
#
# Relocation agsinst L2 + 2
#
--- 10,19 ----
# Relocation agsinst .rodata.str1.1
#
.*: 8f840000 lw a0,0\(gp\)
! .*: R_MIPS_GOT16 L2
.*: 00000000 nop
! .*: 24840000 addiu a0,a0,0
! .*: R_MIPS_LO16 L2
#
# Relocation agsinst L2 + 2
#
Index: ld/testsuite/ld-mips-elf/mips-elf.exp
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-mips-elf/mips-elf.exp,v
retrieving revision 1.17
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.17 mips-elf.exp
*** ld/testsuite/ld-mips-elf/mips-elf.exp 24 Apr 2004 00:20:13 -0000 1.17
--- ld/testsuite/ld-mips-elf/mips-elf.exp 29 Jun 2004 11:28:15 -0000
*************** if $has_newabi {
*** 74,76 ****
--- 74,77 ----
}
}
run_dump_test "reloc-2"
+ run_dump_test "reloc-merge-lo16"
*** /dev/null Fri Apr 23 00:21:55 2004
--- ld/testsuite/ld-mips-elf/reloc-merge-lo16.d Tue Jun 29 12:20:30 2004
***************
*** 0 ****
--- 1,16 ----
+ #name: MIPS ELF lo16 merge
+ #source: reloc-merge-lo16.s
+ #ld: -Treloc-merge-lo16.ld
+ #objdump: -td --prefix-addresses --show-raw-insn
+
+ # Test lo16 reloc calculation with string merging.
+
+ .*: +file format .*mips.*
+ #...
+ 0+80fe70 l .rodata 0+000000 g
+ 0+400000 g F .text 0+000000 __start
+ #...
+ 0+400000 <[^>]*> 3c020081 lui v0,0x81
+ 0+400004 <[^>]*> 2443fe70 addiu v1,v0,-400
+ 0+400008 <[^>]*> 2442fe70 addiu v0,v0,-400
+ \.\.\.
*** /dev/null Fri Apr 23 00:21:55 2004
--- ld/testsuite/ld-mips-elf/reloc-merge-lo16.ld Tue Jun 29 08:32:48 2004
***************
*** 0 ****
--- 1,9 ----
+ ENTRY(__start)
+ SECTIONS
+ {
+ . = 0x0400000;
+ .text : { *(.text) }
+ . = 0x0800000;
+ .rodata : { *(.rodata.*) }
+ /DISCARD/ : { *(*) }
+ }
*** /dev/null Fri Apr 23 00:21:55 2004
--- ld/testsuite/ld-mips-elf/reloc-merge-lo16.s Tue Jun 29 08:32:48 2004
***************
*** 0 ****
--- 1,28 ----
+ .section .rodata.str1.4,"aMS", @progbits, 1
+ .macro fillstr char
+ .rept 0x3fff - \char
+ .byte \char
+ .endr
+ .byte 0
+ .endm
+ fillstr 'a'
+ fillstr 'h'
+ fillstr 'c'
+ fillstr 'd'
+ fillstr 'g'
+ fillstr 'f'
+ g:
+ fillstr 'g'
+ fillstr 'h'
+
+ .text
+ .globl __start
+ .ent __start
+ .type __start, @function
+ __start:
+ lui $2, %hi(g)
+ addiu $3, $2, %lo(g)
+ addiu $2, $2, %lo(g)
+ .end __start
+
+ .space 16