More problems with MIPS gas relocations

[Richard Sandiford <r dot sandiford at redhat dot com>]

A few weeks back, HJ reported a problem with the way that MIPS gas's
md_apply_fix handles ELF relocations:

	<http://sources.redhat.com/ml/binutils/2001-06/msg00099.html>

The same hunk of code causes the following to be misassembled, but
for different reasons:

----------------------------------------------------------------------
	.section .sdata
	.global a
	.4byte 1
a:	.4byte 2

	.section .text
	la $4,a
	la $4,a+4
	la $4,a+8
	la $4,a+12
----------------------------------------------------------------------

In the relocations that have non-zero addends, the value of "a" is
incorrectly subtracted from that addend:

----------------------------------------------------------------------
Disassembly of section .text:

0000000000000000 <.text>:
   0:	27840000 	addiu	a0,gp,0
			0: R_MIPS_GPREL16	a
   4:	27840000 	addiu	a0,gp,0
			4: R_MIPS_GPREL16	a
   8:	27840004 	addiu	a0,gp,4
			8: R_MIPS_GPREL16	a
   c:	27840008 	addiu	a0,gp,8
			c: R_MIPS_GPREL16	a
----------------------------------------------------------------------

I don't know how common this situation is, but I've found some GCC
testsuite cases that fail because of it.  To requote the guilty code:

#ifdef OBJ_ELF
  if (fixP->fx_addsy != NULL && OUTPUT_FLAVOR == bfd_target_elf_flavour)
    {
      if (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))))

	{
	  valueT symval = S_GET_VALUE (fixP->fx_addsy);
	  value -= symval;
	  if (value != 0 && ! fixP->fx_pcrel)
	    {
	      /* In this case, the bfd_install_relocation routine will
		 incorrectly add the symbol value back in.  We just want
		 the addend to appear in the object file.  */
	      value -= symval;

	      [...]
	    }
	}
	[...]
    }
#endif

The condition "value != 0 && ! fixP->fx_pcrel" is trying to predict whether
the generic relocation handling in bfd_install_relocation will be used.  It
will only be used if the reloc's howto handler returns bfd_reloc_continue,
and the condition is based on the assumption that reloc's howto function
will be bfd_elf_generic_reloc.  But GPREL relocations have their own howto
function that (as far as I can tell) will always install the relocation
itself.

The patch below adds the obvious "fix": make the code check for GPREL
relocations before subtracting the symbol value.  It fixes the test case and
introduces no regressions on mips-elf, mipsel-elf and mips64-elf.  I've been
using it locally for a week or so without any apparent ill effects.  Is it
OK to apply?

Richard

[gas/]

	* config/tc-mips.c (md_apply_fix): Don't subtract the symbol value
	from GPREL addends.

[gas/testsuite/]

	* gas/mips/elf-rel4.s, gas/mips/elf-rel4.d: New test.
	* gas/mips/e32-rel4.s, gas/mips/e32-rel4.s: New test.

	* gas/mips/mips.exp: Run new tests.

Index: config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.53
diff -c -p -d -r1.53 tc-mips.c
*** config/tc-mips.c	2001/07/23 13:03:40	1.53
--- config/tc-mips.c	2001/07/25 12:31:40
*************** md_apply_fix (fixP, valueP)
*** 9613,9619 ****
  	{
  	  valueT symval = S_GET_VALUE (fixP->fx_addsy);
  	  value -= symval;
! 	  if (value != 0 && ! fixP->fx_pcrel)
  	    {
  	      /* In this case, the bfd_install_relocation routine will
  		 incorrectly add the symbol value back in.  We just want
--- 9613,9621 ----
  	{
  	  valueT symval = S_GET_VALUE (fixP->fx_addsy);
  	  value -= symval;
! 	  if (value != 0
! 	      && ! fixP->fx_pcrel
! 	      && fixP->fx_r_type != BFD_RELOC_MIPS_GPREL)
  	    {
  	      /* In this case, the bfd_install_relocation routine will
  		 incorrectly add the symbol value back in.  We just want
*** testsuite/gas/mips/mips.exp.old	Wed Jul 25 13:28:30 2001
--- testsuite/gas/mips/mips.exp	Wed Jul 25 13:31:30 2001
*************** if { [istarget mips*-*-*] } then {
*** 138,146 ****
      
  	run_dump_test "elf${el}-rel"
  	if [istarget mips64*-*-*] { 
! 	    run_dump_test "elf${el}-rel2" 
  	} {
! 	    run_dump_test "e32${el}-rel2" 
  	}
  	run_dump_test "elf${el}-rel3"
  	run_dump_test "${tmips}${el}empic"
--- 138,148 ----
      
  	run_dump_test "elf${el}-rel"
  	if [istarget mips64*-*-*] { 
! 	    run_dump_test "elf${el}-rel2"
! 	    run_dump_test "elf${el}-rel4"
  	} {
! 	    run_dump_test "e32${el}-rel2"
! 	    run_dump_test "e32${el}-rel4"
  	}
  	run_dump_test "elf${el}-rel3"
  	run_dump_test "${tmips}${el}empic"
*** /dev/null	Tue Nov 14 21:44:43 2000
--- testsuite/gas/mips/e32-rel4.d	Wed Jul 25 13:24:57 2001
***************
*** 0 ****
--- 1,14 ----
+ #objdump: --prefix-addresses -dr
+ #name: MIPS ELF reloc 4
+ 
+ .*: +file format.*
+ 
+ Disassembly of section .text:
+ 0+000 <[^>]*> addiu	a0,gp,0
+ 			0: R_MIPS_GPREL16	a
+ 0+004 <[^>]*> addiu	a0,gp,4
+ 			4: R_MIPS_GPREL16	a
+ 0+008 <[^>]*> addiu	a0,gp,8
+ 			8: R_MIPS_GPREL16	a
+ 0+00c <[^>]*> addiu	a0,gp,12
+ 			c: R_MIPS_GPREL16	a
*** /dev/null	Tue Nov 14 21:44:43 2000
--- testsuite/gas/mips/e32-rel4.s	Wed Jul 25 13:24:10 2001
***************
*** 0 ****
--- 1,12 ----
+ 
+ 	.section .sdata
+ 	.global a
+ 	.4byte 1
+ a:	.4byte 2
+ 
+ 	.section .text
+ 	la $4,a
+ 	la $4,a+4
+ 	la $4,a+8
+ 	la $4,a+12
+ 
*** /dev/null	Tue Nov 14 21:44:43 2000
--- testsuite/gas/mips/elf-rel4.d	Wed Jul 25 13:24:16 2001
***************
*** 0 ****
--- 1,14 ----
+ #objdump: --prefix-addresses -dr
+ #name: MIPS ELF reloc 4
+ 
+ .*: +file format.*
+ 
+ Disassembly of section .text:
+ 0+000 <[^>]*> daddiu	a0,gp,0
+ 			0: R_MIPS_GPREL16	a
+ 0+004 <[^>]*> daddiu	a0,gp,4
+ 			4: R_MIPS_GPREL16	a
+ 0+008 <[^>]*> daddiu	a0,gp,8
+ 			8: R_MIPS_GPREL16	a
+ 0+00c <[^>]*> daddiu	a0,gp,12
+ 			c: R_MIPS_GPREL16	a
*** /dev/null	Tue Nov 14 21:44:43 2000
--- testsuite/gas/mips/elf-rel4.s	Wed Jul 25 13:04:53 2001
***************
*** 0 ****
--- 1,12 ----
+ 
+ 	.section .sdata
+ 	.global a
+ 	.4byte 1
+ a:	.4byte 2
+ 
+ 	.section .text
+ 	la $4,a
+ 	la $4,a+4
+ 	la $4,a+8
+ 	la $4,a+12
+