This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
[PATCH RFA] fixing the MIPS elf embedded-pic problems.
- From: cgd at broadcom dot com
- To: binutils at sources dot redhat dot com
- Date: 29 Jan 2002 11:39:57 -0800
- Subject: [PATCH RFA] fixing the MIPS elf embedded-pic problems.
So, the following patch addresses the MIPS elf embedded-pic hi/lo
reloc failures that we've seen (which are currently represented by a
test case).
In a nutshell, BFD_RELOC_PCREL_LO16 and the corresponding
BFD_RELOC_PCREL_HI16_S are supposed to be relative to the LO16 reloc.
They weren't, and that caused a couple of edge cases which were
potentially fatal.
The change to tc-mips.c is effectively a large no-op _except_ for the
HI16_S case, where the relocation is made relative to the LO16 half.
However, since it's scary looking, i did my best to test this change
more rigorously than normal. More on that below.
Side-effects: it's no longer allowed for PCREL_LO16 reloc to have
multiple PCREL_HI16_S relocs associated with it. (non-pcrel is still
allowed to do that.) I have no reason to believe that this is a
problem. (we're obviously the largest user, probably by far, of
-membedded-pic in ELF, and it doesn't cause the compiler a problem for
our code.)
Testing: not only has this been in use here for 5 or 6 weeks, it has
been tested by:
* building binutils for targets mips-elf, mips64-elf,
mips-linux, mipsel-linux (from host x86-linux), make check
with no new failures (and the prev. related testcase
failures fixed, of course 8-).
* building gcc in a combined tree w/ binutils, newlib, etc.,
for target mips-elf (host x86-linux), with and without the
change, and verifying that 'gmake -k check-gcc
RUNTESTFLAGS=--target_board=mips-sim' indicated no new
failures.
(I'd like to get this in sooner rather than later, so that the next
release will have working embedded-pic support.)
chris
============================================================================
[ bfd/ChangeLog ]
2002-01-29 Chris Demetriou <cgd@broadcom.com>
* elf32-mips.c: Add additional comments about HI16 relocation
processing.
(_bfd_mips_elf_hi16_reloc): Don't subtract address here for
pc-relative relocations. (Reverts change made on 2001-10-31.)
(_bfd_mips_elf_lo16_reloc): Subtract address of LO16 part here
for pc-relative relocations.
(mips_elf_calculate_relocation): Add a comment about a kludge
in the R_MIPS_GNU_REL_HI16 handling.
(_bfd_mips_elf_relocate_section): Implement that kludge;
adjust pc-relative HI16 relocation for difference in HI16 and
LO16 addresses, since it can't easily be done in
mips_elf_calculate_relocation.
[ gas/ChangeLog ]
2002-01-29 Chris Demetriou <cgd@broadcom.com>
* config/tc-mips.c (tc_gen_reloc): Arrange for
BFD_RELOC_PCREL_HI16_S relocations to be output relative to
their LO16 parts, even for ELF.
Index: bfd/elf32-mips.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-mips.c,v
retrieving revision 1.139
diff -u -p -r1.139 elf32-mips.c
--- elf32-mips.c 2002/01/21 10:29:09 1.139
+++ elf32-mips.c 2002/01/29 19:18:33
@@ -1640,11 +1640,17 @@ static reloc_howto_type elf_mips_gnu_vte
/* Do a R_MIPS_HI16 relocation. This has to be done in combination
with a R_MIPS_LO16 reloc, because there is a carry from the LO16 to
the HI16. Here we just save the information we need; we do the
- actual relocation when we see the LO16. MIPS ELF requires that the
- LO16 immediately follow the HI16. As a GNU extension, we permit an
+ actual relocation when we see the LO16.
+
+ MIPS ELF requires that the LO16 immediately follow the HI16. As a
+ GNU extension, for non-pc-relative relocations, we permit an
arbitrary number of HI16 relocs to be associated with a single LO16
reloc. This extension permits gcc to output the HI and LO relocs
- itself. */
+ itself.
+
+ This cannot be done for PC-relative relocations because both the HI16
+ and LO16 parts of the relocations must be done relative to the LO16
+ part, and there can be carry to or borrow from the HI16 part. */
struct mips_hi16
{
@@ -1727,8 +1733,6 @@ _bfd_mips_elf_hi16_reloc (abfd,
relocation += symbol->section->output_section->vma;
relocation += symbol->section->output_offset;
relocation += reloc_entry->addend;
- if (reloc_entry->howto->pc_relative)
- relocation -= reloc_entry->address;
if (reloc_entry->address > input_section->_cooked_size)
return bfd_reloc_outofrange;
@@ -1794,6 +1798,12 @@ _bfd_mips_elf_lo16_reloc (abfd,
val = ((insn & 0xffff) << 16) + vallo;
val += l->addend;
+ /* If PC-relative, we need to subtract out the address of the LO
+ half of the HI/LO. (The actual relocation is relative
+ to that instruction.) */
+ if (reloc_entry->howto->pc_relative)
+ val -= reloc_entry->address;
+
/* At this point, "val" has the value of the combined HI/LO
pair. If the low order 16 bits (which will be used for
the LO16 insn) are negative, then we will need an
@@ -6864,6 +6874,11 @@ mips_elf_calculate_relocation (abfd,
break;
case R_MIPS_GNU_REL_HI16:
+ /* Instead of subtracting 'p' here, we should be subtracting the
+ equivalent value for the LO part of the reloc, since the value
+ here is relative to that address. Because that's not easy to do,
+ we adjust 'addend' in _bfd_mips_elf_relocate_section(). See also
+ the comment there for more information. */
value = mips_elf_high (addend + symbol - p);
value &= howto->dst_mask;
break;
@@ -7390,6 +7405,16 @@ _bfd_mips_elf_relocate_section (output_b
/* Compute the combined addend. */
addend += l;
+
+ /* If PC-relative, subtract the difference between the
+ address of the LO part of the reloc and the address of
+ the HI part. The relocation is relative to the LO
+ part, but mips_elf_calculate_relocation() doesn't know
+ it address or the difference from the HI part, so
+ we subtract that difference here. See also the
+ comment in mips_elf_calculate_relocation(). */
+ if (r_type == R_MIPS_GNU_REL_HI16)
+ addend -= (lo16_relocation->r_offset - rel->r_offset);
}
else if (r_type == R_MIPS16_GPREL)
{
Index: gas/config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.105
diff -u -p -r1.105 tc-mips.c
--- tc-mips.c 2002/01/16 21:30:47 1.105
+++ tc-mips.c 2002/01/29 19:18:38
@@ -12269,32 +12269,43 @@ tc_gen_reloc (section, fixp)
as_fatal (_("Double check fx_r_type in tc-mips.c:tc_gen_reloc"));
fixp->fx_r_type = BFD_RELOC_GPREL32;
}
- else if (fixp->fx_pcrel == 0 || OUTPUT_FLAVOR == bfd_target_elf_flavour)
- reloc->addend = fixp->fx_addnumber;
else if (fixp->fx_r_type == BFD_RELOC_PCREL_LO16)
{
- /* We use a special addend for an internal RELLO reloc. */
- if (symbol_section_p (fixp->fx_addsy))
- reloc->addend = reloc->address - S_GET_VALUE (fixp->fx_subsy);
+ if (OUTPUT_FLAVOR == bfd_target_elf_flavour)
+ reloc->addend = fixp->fx_addnumber;
else
- reloc->addend = fixp->fx_addnumber + reloc->address;
+ {
+ /* We use a special addend for an internal RELLO reloc. */
+ if (symbol_section_p (fixp->fx_addsy))
+ reloc->addend = reloc->address - S_GET_VALUE (fixp->fx_subsy);
+ else
+ reloc->addend = fixp->fx_addnumber + reloc->address;
+ }
}
else if (fixp->fx_r_type == BFD_RELOC_PCREL_HI16_S)
{
assert (fixp->fx_next != NULL
&& fixp->fx_next->fx_r_type == BFD_RELOC_PCREL_LO16);
- /* We use a special addend for an internal RELHI reloc. The
- reloc is relative to the RELLO; adjust the addend
+
+ /* The reloc is relative to the RELLO; adjust the addend
accordingly. */
- if (symbol_section_p (fixp->fx_addsy))
- reloc->addend = (fixp->fx_next->fx_frag->fr_address
- + fixp->fx_next->fx_where
- - S_GET_VALUE (fixp->fx_subsy));
+ if (OUTPUT_FLAVOR == bfd_target_elf_flavour)
+ reloc->addend = fixp->fx_next->fx_addnumber;
else
- reloc->addend = (fixp->fx_addnumber
- + fixp->fx_next->fx_frag->fr_address
- + fixp->fx_next->fx_where);
+ {
+ /* We use a special addend for an internal RELHI reloc. */
+ if (symbol_section_p (fixp->fx_addsy))
+ reloc->addend = (fixp->fx_next->fx_frag->fr_address
+ + fixp->fx_next->fx_where
+ - S_GET_VALUE (fixp->fx_subsy));
+ else
+ reloc->addend = (fixp->fx_addnumber
+ + fixp->fx_next->fx_frag->fr_address
+ + fixp->fx_next->fx_where);
+ }
}
+ else if (fixp->fx_pcrel == 0 || OUTPUT_FLAVOR == bfd_target_elf_flavour)
+ reloc->addend = fixp->fx_addnumber;
else
{
if (OUTPUT_FLAVOR != bfd_target_aout_flavour)