RE: [PATCH] MIPS: microMIPS ASE support

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

On Tue, 22 Feb 2011, Fu, Chao-Ying wrote:

> > > > > +  /* For microMIPS PC relative relocations, we cannot 
> > > convert it to
> > > > > +     against a section.  If we do, it will mess up the 
> > > fixp->fx_offset.  */
> > > > >    if (fixp->fx_r_type == BFD_RELOC_VTABLE_INHERIT
> > > > > -      || fixp->fx_r_type == BFD_RELOC_VTABLE_ENTRY)
> > > > > +      || fixp->fx_r_type == BFD_RELOC_VTABLE_ENTRY
> > > > > +      || fixp->fx_r_type == BFD_RELOC_MICROMIPS_7_PCREL_S1
> > > > > +      || fixp->fx_r_type == BFD_RELOC_MICROMIPS_10_PCREL_S1
> > > > > +      || fixp->fx_r_type == BFD_RELOC_MICROMIPS_16_PCREL_S1)
> > > > 
> > > > "to be against a section".  That's not a helpful comment though.
> > > > _How_ will it mess up fixp->fx_offset?  Give the reader a clue why
> > > > the problem applies to BFD_RELOC_MICROMIPS_16_PCREL_S1 but not
> > > > to something like BFD_RELOC_16_PCREL_S2.
> 
>   GAS resolves all MIPS pc-relative relocation types inside assembling, so there are no issues for
> BFD_RELOC_16_PCREL_S2.  For microMIPS, GAS leaves them to the linker, so we hit new issues for
> microMIPS pc-relative relocation types.  Thanks!

 Thanks for the hint -- now I can see what's going on.

 The problem is for REL targets we have a limited number of relocatable 
bits in the instruction to store the in-place addend.  The space does not 
cover the whole address space and because these are PC-relative 
relocations, if converted to section-relative ones, then they can overflow 
even for legitimate symbol references.

 As such the change belongs next to the check for BFD_RELOC_MIPS_JALR 
relocations and likewise it does not apply to RELA targets.  I have 
updated the change accordingly.

 Contrary to what you say the problem does apply to standard MIPS code 
too, because BFD_RELOC_16_PCREL_S2 relocs against defined symbols are not 
always resolved internally by GAS and R_MIPS_PC16 relocs are produced as 
appropriate.  Consider the following program:

$ cat bsec.s
	.text
	.space	0x40000
.Lbar:
	addu	$2, $3, $4

	.section .init, "ax", @progbits
foo:
	b	.Lbar

As .Lbar is a local defined symbol from another section, a relocation is 
placed into the output file for the static linker to resolve based on the 
final section layout.  The program does not build though:

$ mips-sde-elf-as -o bsec.o bsec.s
bsec.s: Assembler messages:
bsec.s:8: Error: relocation overflow

This is because .Lbar is too far into .text for the PC-relative offset to 
fit into the relocatable field of the R_MIPS_PC16 reloc.  Therefore this 
reference shouldn't be made section-relative either.  This observation 
makes this change not specific to microMIPS relocations at all, so I have 
separated it and I'm providing it below.  I have removed the original hunk 
from the microMIPS patch altogether now.

 And for the record, MIPS16 branches produce no reloc at all (hence 
excluded from the test case), even though they probably should.

2011-02-24  Maciej W. Rozycki  <macro@codesourcery.com>

	gas/
	* config/tc-mips.c (mips_fix_adjustable): On REL targets also 
	reject PC-relative relocations.

	gas/testsuite/
	* gas/mips/branch-misc-2.d: Adjust for relocation change.
	* gas/mips/branch-misc-2pic.d: Likewise.
	* gas/mips/branch-misc-4.d: New test for PC-relative relocation
	overflow.
	* gas/mips/branch-misc-4-64.d: Likewise.
	* gas/mips/branch-misc-4.s: Source for the new tests.
	* testsuite/gas/mips/mips.exp: Run the new tests.

 This change has been regression-tested for the mips-sde-elf and 
mips-linux-gnu targets.  OK to apply?

  Maciej

binutils-gas-mips-fix-pc-rel.diff
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2011-02-24 10:14:39.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2011-02-24 10:22:22.000000000 +0000
@@ -14229,8 +14229,12 @@ mips_fix_adjustable (fixS *fixp)
       && (S_GET_SEGMENT (fixp->fx_addsy)->flags & SEC_MERGE) != 0)
     return 0;
 
-  /* There is no place to store an in-place offset for JALR relocations.  */
-  if (fixp->fx_r_type == BFD_RELOC_MIPS_JALR && HAVE_IN_PLACE_ADDENDS)
+  /* There is no place to store an in-place offset for JALR relocations.
+     Likewise an in-range offset of PC-relative relocations may overflow
+     the in-place relocatable field if recalculated against the start
+     address of the symbol's containing section.  */
+  if (HAVE_IN_PLACE_ADDENDS
+      && (fixp->fx_pcrel || fixp->fx_r_type == BFD_RELOC_MIPS_JALR))
     return 0;
 
 #ifdef OBJ_ELF
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-2.d
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/testsuite/gas/mips/branch-misc-2.d	2011-02-24 10:14:33.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-2.d	2011-02-24 10:20:45.000000000 +0000
@@ -39,6 +39,6 @@
 [ 	]*b0: R_MIPS_PC16	x2
 0+00b4 <[^>]*> 00000000 	nop
 0+00b8 <[^>]*> 1000ffff 	b	000000b8 <g6\+0x10>
-[ 	]*b8: R_MIPS_PC16	\.data
+[ 	]*b8: R_MIPS_PC16	\.Ldata
 0+00bc <[^>]*> 00000000 	nop
 	\.\.\.
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-2pic.d
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/testsuite/gas/mips/branch-misc-2pic.d	2011-02-24 10:14:33.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-2pic.d	2011-02-24 10:20:45.000000000 +0000
@@ -40,6 +40,6 @@
 [ 	]*b0: R_MIPS_PC16	x2
 0+00b4 <[^>]*> 00000000 	nop
 0+00b8 <[^>]*> 1000ffff 	b	000000b8 <g6\+0x10>
-[ 	]*b8: R_MIPS_PC16	\.data
+[ 	]*b8: R_MIPS_PC16	\.Ldata
 0+00bc <[^>]*> 00000000 	nop
 	\.\.\.
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-4.d
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-4.d	2011-02-24 10:22:59.000000000 +0000
@@ -0,0 +1,26 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS branch-misc-4
+#as: -32
+
+# Verify PC-relative relocations do not overflow.
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+	\.\.\.
+([0-9a-f]+) <[^>]*> 1000ffff 	b	\1 <foo>
+[ 	]*[0-9a-f]+: R_MIPS_PC16	bar
+[0-9a-f]+ <[^>]*> 00000000 	nop
+([0-9a-f]+) <[^>]*> 1000ffff 	b	\1 <\.Lfoo>
+[ 	]*[0-9a-f]+: R_MIPS_PC16	\.Lbar
+[0-9a-f]+ <[^>]*> 00000000 	nop
+	\.\.\.
+
+Disassembly of section \.init:
+([0-9a-f]+) <[^>]*> 1000ffff 	b	\1 <bar>
+[ 	]*[0-9a-f]+: R_MIPS_PC16	foo
+[0-9a-f]+ <[^>]*> 00000000 	nop
+([0-9a-f]+) <[^>]*> 1000ffff 	b	\1 <\.Lbar>
+[ 	]*[0-9a-f]+: R_MIPS_PC16	\.Lfoo
+[0-9a-f]+ <[^>]*> 00000000 	nop
+	\.\.\.
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-4.s
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-4.s	2011-02-24 10:14:40.000000000 +0000
@@ -0,0 +1,28 @@
+# Source file to verify PC-relative relocations do not overflow.
+
+	.text
+	.space	0x40000
+	.globl	foo
+	.ent	foo
+foo:
+	b	bar
+.Lfoo:
+	b	.Lbar
+	.end	foo
+
+# Force at least 8 (non-delay-slot) zero bytes, to make 'objdump' print ...
+	.align	2
+	.space	8
+
+	.section .init, "ax", @progbits
+	.globl	bar
+	.ent	bar
+bar:
+	b	foo
+.Lbar:
+	b	.Lfoo
+	.end	bar
+
+# Force at least 8 (non-delay-slot) zero bytes, to make 'objdump' print ...
+	.align	2
+	.space	8
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips.exp
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/testsuite/gas/mips/mips.exp	2011-02-24 10:14:39.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips.exp	2011-02-24 10:20:46.000000000 +0000
@@ -838,6 +838,10 @@ if { [istarget mips*-*-vxworks*] } {
 	run_dump_test_arches "loc-swap"	[mips_arch_list_all]
 	run_dump_test_arches "loc-swap-dis" \
 					[mips_arch_list_all]
+	run_dump_test_arches "branch-misc-4" \
+					[mips_arch_list_matching mips1]
+	run_dump_test_arches "branch-misc-4-64" \
+					[mips_arch_list_matching mips3]
     }
 
     if $has_newabi {
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-4-64.d
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-4-64.d	2011-02-24 10:14:40.000000000 +0000
@@ -0,0 +1,35 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS branch-misc-4-64
+#as: -64
+#source: branch-misc-4.s
+
+# Verify PC-relative relocations do not overflow.
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+	\.\.\.
+[0-9a-f]+ <[^>]*> 10000000 	b	[0-9a-f]+ <foo\+0x[0-9a-f]+>
+[ 	]*[0-9a-f]+: R_MIPS_PC16	bar\+0xf+fffc
+[ 	]*[0-9a-f]+: R_MIPS_NONE	\*ABS\*\+0xf+fffc
+[ 	]*[0-9a-f]+: R_MIPS_NONE	\*ABS\*\+0xf+fffc
+[0-9a-f]+ <[^>]*> 00000000 	nop
+[0-9a-f]+ <[^>]*> 10000000 	b	[0-9a-f]+ <foo\+0x[0-9a-f]+>
+[ 	]*[0-9a-f]+: R_MIPS_PC16	\.init\+0x4
+[ 	]*[0-9a-f]+: R_MIPS_NONE	\*ABS\*\+0x4
+[ 	]*[0-9a-f]+: R_MIPS_NONE	\*ABS\*\+0x4
+[0-9a-f]+ <[^>]*> 00000000 	nop
+	\.\.\.
+
+Disassembly of section \.init:
+[0-9a-f]+ <[^>]*> 10000000 	b	[0-9a-f]+ <bar\+0x[0-9a-f]+>
+[ 	]*[0-9a-f]+: R_MIPS_PC16	foo\+0xf+fffc
+[ 	]*[0-9a-f]+: R_MIPS_NONE	\*ABS\*\+0xf+fffc
+[ 	]*[0-9a-f]+: R_MIPS_NONE	\*ABS\*\+0xf+fffc
+[0-9a-f]+ <[^>]*> 00000000 	nop
+[0-9a-f]+ <[^>]*> 10000000 	b	[0-9a-f]+ <bar\+0x[0-9a-f]+>
+[ 	]*[0-9a-f]+: R_MIPS_PC16	\.text\+0x40004
+[ 	]*[0-9a-f]+: R_MIPS_NONE	\*ABS\*\+0x40004
+[ 	]*[0-9a-f]+: R_MIPS_NONE	\*ABS\*\+0x40004
+[0-9a-f]+ <[^>]*> 00000000 	nop
+	\.\.\.