Re: mips gas is horribly broken

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

Ian Lance Taylor wrote:

> I was suggesting that in the assembler you change the addend and the
> object file so that the result comes out right without ever having an
> addend of zero.  That is generally how these sorts of problems are solved
> now: by adding hacks to gas.

Below I've tried to do what I think you're suggesting here.  With a small
modification (also below) it fixes HJ's test case.  It also fixes some C
testsuite cases that this bug was causing.  It is a truly gruesome hack.  In
fact, it would be nice to commit it anonymously ;-).  But it seems from this
thread that the only real solution is to overhaul bfd relocs in general,
which I suspect few people have time to do.

> Of course, each hack makes it harder to implement the right solution.

Of course.

The patch passes make check with no regressions, fixes MIPS ELF reloc 3 &
XPASSes MIPS16 reloc 2.  OK to install?

Richard



[gas]

	* config/tc-mips.c (md_apply_fix): Add hack to avoid a zero addend.

[gas/testsuite]

	* gas/mips/elf-rel3.s: Add zero word to end of file.

Index: tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.48
diff -c -p -d -r1.48 tc-mips.c
*** tc-mips.c	2001/06/11 20:40:34	1.48
--- tc-mips.c	2001/06/26 10:43:42
*************** md_apply_fix (fixP, valueP)
*** 9577,9585 ****
  	    {
  	      /* 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.
! 		 FIXME: If this makes VALUE zero, we're toast.  */
  	      value -= symval;
  	    }
  	}
  
--- 9577,9620 ----
  	    {
  	      /* 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;
+ 
+ 	      /* Make sure the addend is still non-zero.  If it became zero
+ 		 after the last operation, set it to a spurious value and
+ 		 subtract the same value from the object file's contents.  */
+ 	      if (value == 0)
+ 		{
+ 		  value = 8;
+ 
+ 		  /* The in-place addends for LO16 relocations are signed;
+ 		     leave the matching HI16 in-place addends as zero.  */
+ 		  if (fixP->fx_r_type != BFD_RELOC_HI16_S)
+ 		    {
+ 		      reloc_howto_type *howto;
+ 		      bfd_vma contents, mask, field;
+ 
+ 		      howto = bfd_reloc_type_lookup (stdoutput,
+ 						     fixP->fx_r_type);
+ 
+ 		      contents = bfd_get_bits (fixP->fx_frag->fr_literal
+ 					       + fixP->fx_where,
+ 					       fixP->fx_size * 8,
+ 					       target_big_endian);
+ 
+ 		      /* MASK has bits set where the relocation should go.
+ 			 FIELD is -value, shifted into the appropriate place
+ 			 for this relocation.  */
+ 		      mask = 1 << (howto->bitsize - 1);
+ 		      mask = (((mask - 1) << 1) | 1) << howto->bitpos;
+ 		      field = (-value >> howto->rightshift) << howto->bitpos;
+ 
+ 		      bfd_put_bits ((field & mask) | (contents & ~mask),
+ 				    fixP->fx_frag->fr_literal + fixP->fx_where,
+ 				    fixP->fx_size * 8,
+ 				    target_big_endian);
+ 		    }
+ 		}
  	    }
  	}
  

Index: elf-rel3.s
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/elf-rel3.s,v
retrieving revision 1.2
diff -c -p -d -r1.2 elf-rel3.s
*** elf-rel3.s	2001/06/09 06:25:55	1.2
--- elf-rel3.s	2001/06/25 16:35:13
*************** x:
*** 9,11 ****
--- 9,12 ----
  b:
  	.word	b+4
  	.word	x
+ 	.word	0