Re: MIPS md_apply_fix()(?) problem.

[Richard Sandiford <rsandifo at redhat dot com>]

cgd@broadcom.com writes:
> So, I ran across a rather serious problem, which i believe is caused
> by Richard S's change:
> 
> 2001-08-03  Richard Sandiford  <rsandifo@redhat.com>
> 
>         * config/tc-mips.c (md_apply_fix): Don't subtract the symbol value
>         from GPREL addends.

Knew it'd come back to haunt me.  Thanks for all the debugging work!

I'm afraid the best I can do at the moment is hack around with it even
more.  It'd be really nice to do this stuff properly one day...

The patch above tried to stop md_apply_fix3() from subtracting the
symbol value twice from GPREL addends.  The one below lets it happen and
makes tc_gen_reloc add the symbol's value back in.  The original munged
addend can then be used for other kinds of reloc by the variant frag stuff.

Seems to fix the case and shows no regressions on mips-elf or
mips-ecoff.  The test case for the original bug was MIPS reloc 4, which
still passes after this patch.

I'll try to come up with a test case next week, if Chris doesn't beat
me to it.

Richard

        * config/tc-mips.c (mips_need_elf_addend_fixup): New, extracted from...
        (md_apply_fix3): ...here.  Don't prevent the symbol value being
        subtracted twice from GPREL addends.
        (tc_gen_reloc): Add the symbol value to a GPREL addend if it was
        subtracted by the previous function.

Index: config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.96
diff -c -p -d -r1.96 tc-mips.c
*** config/tc-mips.c	2001/11/15 21:28:56	1.96
--- config/tc-mips.c	2001/11/16 23:05:40
*************** static const char *mips_isa_to_str PARAM
*** 734,739 ****
--- 734,742 ----
  static const char *mips_cpu_to_str PARAMS ((int));
  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 *));
+ #endif
  
  /* Return values of my_getSmallExpression().  */
  
*************** mips_force_relocation (fixp)
*** 10218,10223 ****
--- 10221,10245 ----
  	      || fixp->fx_r_type == BFD_RELOC_PCREL_LO16));
  }
  
+ #ifdef OBJ_ELF
+ static 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))));
+ }
+ #endif
+ 
  /* Apply a fixup to the object file.  */
  
  void
*************** md_apply_fix3 (fixP, valP, seg)
*** 10257,10281 ****
  #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
! 	      && 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
--- 10279,10290 ----
  #ifdef OBJ_ELF
    if (fixP->fx_addsy != NULL && OUTPUT_FLAVOR == bfd_target_elf_flavour)
      {
!       if (mips_need_elf_addend_fixup (fixP))
  	{
  	  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
*************** tc_gen_reloc (section, fixp)
*** 12250,12255 ****
--- 12259,12277 ----
  			bfd_get_reloc_code_name (code));
  	}
      }
+ 
+ #ifdef OBJ_ELF
+   /* md_apply_fix3 has a double-subtraction hack to get
+      bfd_install_relocation to behave nicely.  GPREL relocations are
+      handled correctly without this hack, so undo it here.  We can't
+      stop md_apply_fix3 from subtracting twice in the first place since
+      the fake addend is required for variant frags above.  */
+   if (fixp->fx_addsy != NULL && OUTPUT_FLAVOR == bfd_target_elf_flavour
+       && code == BFD_RELOC_MIPS_GPREL
+       && reloc->addend != 0
+       && mips_need_elf_addend_fixup (fixp))
+     reloc->addend += S_GET_VALUE (fixp->fx_addsy);
+ #endif
  
    /* To support a PC relative reloc when generating embedded PIC code
       for ECOFF, we use a Cygnus extension.  We check for that here to