* RFA: Fix MIPS failure for ld-elf/merge test
@ 2003-05-04 21:45 Daniel Jacobowitz
2003-05-05 6:00 ` Alan Modra
2003-05-05 22:08 ` Eric Christopher
0 siblings, 2 replies; 5+ messages in thread
From: Daniel Jacobowitz @ 2003-05-04 21:45 UTC (permalink / raw)
To: binutils
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA: Fix MIPS failure for ld-elf/merge test
2003-05-04 21:45 RFA: Fix MIPS failure for ld-elf/merge test Daniel Jacobowitz
@ 2003-05-05 6:00 ` Alan Modra
2003-05-05 22:08 ` Eric Christopher
1 sibling, 0 replies; 5+ messages in thread
From: Alan Modra @ 2003-05-05 6:00 UTC (permalink / raw)
To: binutils
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA: Fix MIPS failure for ld-elf/merge test
2003-05-04 21:45 RFA: Fix MIPS failure for ld-elf/merge test Daniel Jacobowitz
2003-05-05 6:00 ` Alan Modra
@ 2003-05-05 22:08 ` Eric Christopher
2003-05-06 0:05 ` Daniel Jacobowitz
1 sibling, 1 reply; 5+ messages in thread
From: Eric Christopher @ 2003-05-05 22:08 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: binutils
> 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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA: Fix MIPS failure for ld-elf/merge test
2003-05-05 22:08 ` Eric Christopher
@ 2003-05-06 0:05 ` Daniel Jacobowitz
2003-05-06 0:11 ` Eric Christopher
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Jacobowitz @ 2003-05-06 0:05 UTC (permalink / raw)
To: Eric Christopher; +Cc: binutils
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA: Fix MIPS failure for ld-elf/merge test
2003-05-06 0:05 ` Daniel Jacobowitz
@ 2003-05-06 0:11 ` Eric Christopher
0 siblings, 0 replies; 5+ messages in thread
From: Eric Christopher @ 2003-05-06 0:11 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: binutils
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2003-05-06 0:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-05-04 21:45 RFA: Fix MIPS failure for ld-elf/merge test Daniel Jacobowitz
2003-05-05 6:00 ` Alan Modra
2003-05-05 22:08 ` Eric Christopher
2003-05-06 0:05 ` Daniel Jacobowitz
2003-05-06 0:11 ` Eric Christopher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).