RFA: Fix MIPS failure for ld-elf/merge test

RFA: Fix MIPS failure for ld-elf/merge test

[Daniel Jacobowitz]

I'd really appreciate more eyes on this one.  I'm touching the black voodoo
of working around bfd_install_relocation.

The failure is visible if you look at the assembled file for merge.s. 
Before I started working on this, it looked like:

RELOCATION RECORDS FOR [.text]:
OFFSET   TYPE              VALUE 
00000000 R_MIPS_32         .rodata.str
00000004 R_MIPS_32         .rodata.str
00000008 R_MIPS_PC32       .LC0
0000000c R_MIPS_PC32       .LC1


Contents of section .text:
 0000 00000000 00000004 00000004 00000010  ................


Note the offset of 0x10.  That's not right; it should be the difference
between the PC and .LT0, which is only 0x8.


The fix is in two parts.  The first one: in a patch last year, Richard
Sandiford changed mips_need_elf_addend_fixup to check SEC_MERGE.  However,
he left it conditioned on symbol_useed_in_reloc_p.  That's relevant (?? I
think? Maybe it shouldn't be there at all?) to the SEC_LINK_ONCE check; but
it doesn't matter for SEC_MERGE.  If I move the SEC_MERGE check out to a
separate if statement, the offset drops from 0x10 to 0xc.

That's still wrong; but now it's wrong by four bytes.  The value of .LC1 is
4 at this point, which pointed me at the second problem.  In md_apply_fix3,
we check howto->pcrel_offset corresponding to fx_r_type; but tc_gen_reloc may
change fx_r_type.  Since all MIPS pcrel relocs are pcrel_offset, I just
removed the check.  That dropped the offset to 0x8.

This fixes a test failure on mips-linux and mipsel-linux.  It introduces no
regressions on mips-linux.  OK?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-05-04  Daniel Jacobowitz  <drow@mvista.com>

	* config/tc-mips.c (mips_need_elf_addend_fixup): Correct check for
	SEC_MERGE.
	(md_apply_fix3): Remove check for howto->pcrel_offset.

--- /opt/src/binutils/binutils/branch-src/gas/config/tc-mips.c	2003-04-27 16:17:31.000000000 -0400
+++ config/tc-mips.c	2003-05-04 17:33:30.000000000 -0400
@@ -11595,11 +11595,15 @@ mips_need_elf_addend_fixup (fixP)
   if (symbol_used_in_reloc_p (fixP->fx_addsy)
       && (((bfd_get_section_flags (stdoutput,
 				   S_GET_SEGMENT (fixP->fx_addsy))
-	    & (SEC_LINK_ONCE | SEC_MERGE)) != 0)
+	    & SEC_LINK_ONCE) != 0)
 	  || !strncmp (segment_name (S_GET_SEGMENT (fixP->fx_addsy)),
 		       ".gnu.linkonce",
 		       sizeof (".gnu.linkonce") - 1)))
     return 1;
+  if ((bfd_get_section_flags (stdoutput,
+			      S_GET_SEGMENT (fixP->fx_addsy))
+       & SEC_MERGE) != 0)
+    return 1;
   return 0;
 }
 #endif
@@ -11658,15 +11662,25 @@ md_apply_fix3 (fixP, valP, seg)
 	  value -= symval;
 
 	  howto = bfd_reloc_type_lookup (stdoutput, fixP->fx_r_type);
-	  if (value != 0 && howto && howto->partial_inplace
-	      && (! fixP->fx_pcrel || howto->pcrel_offset))
+	  if (value != 0 && howto && howto->partial_inplace)
 	    {
 	      /* 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.
 
-		 howto->pcrel_offset is added for R_MIPS_PC16, which is
-		 generated for code like
+		 The condition above used to include
+		 "&& (! fixP->fx_pcrel || howto->pcrel_offset)".
+
+		 However, howto can't be trusted here, because we
+		 might change the reloc type in tc_gen_reloc.  We can
+		 check howto->partial_inplace because that conversion
+		 happens to preserve howto->partial_inplace; but it
+		 does not preserve howto->pcrel_offset.  I've just
+		 eliminated the check, because all MIPS PC-relative
+		 relocations are marked howto->pcrel_offset.
+
+		 howto->pcrel_offset was originally added for
+		 R_MIPS_PC16, which is generated for code like
 
 		 	globl g1 .text
 			.text

Re: RFA: Fix MIPS failure for ld-elf/merge test

[Alan Modra]

On Sun, May 04, 2003 at 05:45:56PM -0400, Daniel Jacobowitz wrote:
> The fix is in two parts.  The first one: in a patch last year, Richard
> Sandiford changed mips_need_elf_addend_fixup to check SEC_MERGE.  However,
> he left it conditioned on symbol_useed_in_reloc_p.  That's relevant (?? I
> think? Maybe it shouldn't be there at all?) to the SEC_LINK_ONCE check; but
> it doesn't matter for SEC_MERGE.  If I move the SEC_MERGE check out to a
> separate if statement, the offset drops from 0x10 to 0xc.

symbol_used_in_reloc_p should be removed.  The flag isn't set
early enough for md_apply_fix3.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: RFA: Fix MIPS failure for ld-elf/merge test

[Eric Christopher]

> This fixes a test failure on mips-linux and mipsel-linux.  It introduces no
> regressions on mips-linux.  OK?

Can you fix up your patch with Alan's comments in mind?

-eric

-- 
Eric Christopher <echristo@redhat.com>

Re: RFA: Fix MIPS failure for ld-elf/merge test

[Daniel Jacobowitz]

On Mon, May 05, 2003 at 03:08:56PM -0700, Eric Christopher wrote:
> 
> > This fixes a test failure on mips-linux and mipsel-linux.  It introduces no
> > regressions on mips-linux.  OK?
> 
> Can you fix up your patch with Alan's comments in mind?

Sure, easy enough.  Second hunk and explanation unchanged.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-05-05  Daniel Jacobowitz  <drow@mvista.com>

	* config/tc-mips.c (mips_need_elf_addend_fixup): Remove
	symbol_used_in_reloc_p check.
	(md_apply_fix3): Remove check for howto->pcrel_offset.

Index: gas/config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.204
diff -u -p -r1.204 tc-mips.c
--- gas/config/tc-mips.c	25 Apr 2003 04:40:09 -0000	1.204
+++ gas/config/tc-mips.c	6 May 2003 00:04:24 -0000
@@ -11592,13 +11592,12 @@ mips_need_elf_addend_fixup (fixP)
 	  || S_IS_EXTERNAL (fixP->fx_addsy))
       && !S_IS_COMMON (fixP->fx_addsy))
     return 1;
-  if (symbol_used_in_reloc_p (fixP->fx_addsy)
-      && (((bfd_get_section_flags (stdoutput,
-				   S_GET_SEGMENT (fixP->fx_addsy))
-	    & (SEC_LINK_ONCE | SEC_MERGE)) != 0)
-	  || !strncmp (segment_name (S_GET_SEGMENT (fixP->fx_addsy)),
-		       ".gnu.linkonce",
-		       sizeof (".gnu.linkonce") - 1)))
+  if (((bfd_get_section_flags (stdoutput,
+			       S_GET_SEGMENT (fixP->fx_addsy))
+	& (SEC_LINK_ONCE | SEC_MERGE)) != 0)
+      || !strncmp (segment_name (S_GET_SEGMENT (fixP->fx_addsy)),
+		   ".gnu.linkonce",
+		   sizeof (".gnu.linkonce") - 1))
     return 1;
   return 0;
 }
@@ -11658,15 +11657,25 @@ md_apply_fix3 (fixP, valP, seg)
 	  value -= symval;
 
 	  howto = bfd_reloc_type_lookup (stdoutput, fixP->fx_r_type);
-	  if (value != 0 && howto && howto->partial_inplace
-	      && (! fixP->fx_pcrel || howto->pcrel_offset))
+	  if (value != 0 && howto && howto->partial_inplace)
 	    {
 	      /* 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.
 
-		 howto->pcrel_offset is added for R_MIPS_PC16, which is
-		 generated for code like
+		 The condition above used to include
+		 "&& (! fixP->fx_pcrel || howto->pcrel_offset)".
+
+		 However, howto can't be trusted here, because we
+		 might change the reloc type in tc_gen_reloc.  We can
+		 check howto->partial_inplace because that conversion
+		 happens to preserve howto->partial_inplace; but it
+		 does not preserve howto->pcrel_offset.  I've just
+		 eliminated the check, because all MIPS PC-relative
+		 relocations are marked howto->pcrel_offset.
+
+		 howto->pcrel_offset was originally added for
+		 R_MIPS_PC16, which is generated for code like
 
 		 	globl g1 .text
 			.text

Re: RFA: Fix MIPS failure for ld-elf/merge test

[Eric Christopher]

On Mon, 2003-05-05 at 17:05, Daniel Jacobowitz wrote:
> On Mon, May 05, 2003 at 03:08:56PM -0700, Eric Christopher wrote:
> > 
> > > This fixes a test failure on mips-linux and mipsel-linux.  It introduces no
> > > regressions on mips-linux.  OK?
> > 
> > Can you fix up your patch with Alan's comments in mind?
> 
> Sure, easy enough.  Second hunk and explanation unchanged.

OK.

-eric

-- 
Eric Christopher <echristo@redhat.com>