Re: MIPS md_apply_fix()(?) problem.

[Richard Sandiford <rsandifo at redhat dot com>]

> 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...

But sadly no inspiration for a better fix yet.  Is it OK to install this
patch?

> 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