[PATCH] [MIPS] Compact EH Infrastructure R_MIPS_PC32
Moore, Catherine
Catherine_Moore@mentor.com
Fri May 3 16:38:00 GMT 2013
> -----Original Message-----
> From: Richard Sandiford [mailto:rdsandiford@googlemail.com]
> Sent: Thursday, May 02, 2013 10:20 AM
> To: Moore, Catherine
> Cc: binutils@sourceware.org
> Subject: Re: [PATCH] [MIPS] Compact EH Infrastructure R_MIPS_PC32
>
> "Moore, Catherine" <Catherine_Moore@mentor.com> writes:
> >> -----Original Message-----
> >> From: Richard Sandiford [mailto:rdsandiford@googlemail.com]
> >> Sent: Monday, April 22, 2013 2:49 PM
> >> Subject: Re: [PATCH] [MIPS] Compact EH Infrastructure R_MIPS_PC32
> >>
> >> "Moore, Catherine" <Catherine_Moore@mentor.com> writes:
> >> > @@ -17782,6 +17787,10 @@
> >> > if (fixp->fx_addsy == NULL)
> >> > return 1;
> >> >
> >> > + /* Alow relocs used for EH tables. */ if (fixp->fx_r_type ==
> >> > + BFD_RELOC_32_PCREL)
> >> > + return 1;
> >> > +
> >> > /* If symbol SYM is in a mergeable section, relocations of the form
> >> > SYM + 0 can usually be made section-relative. The mergeable data
> >> > is then identified by the section offset rather than by the symbol.
> >>
> >> I think this is really trying to circumvent the later:
> >
> > That is likely true, but
> >>
> >> /* There is no place to store an in-place offset for JALR relocations.
> >> Likewise an in-range offset of PC-relative relocations may overflow
> >> the in-place relocatable field if recalculated against the start
> >> address of the symbol's containing section. */
> >> if (HAVE_IN_PLACE_ADDENDS
> >> && (fixp->fx_pcrel || jalr_reloc_p (fixp->fx_r_type)))
> >> return 0;
> >>
> >> I'm not sure it's correct for ELF64 though, where the range of the
> >> relocation is still smaller than the address range. I realise the
> >> cases where it matters are very much corner cases, but it still seems
> wrong.
> >>
> >> Maybe something like:
> >>
> >> /* In general, converting to a section-relative relocation can
> >> increase the addend. Make sure that we won't lose bits. */
> >> if (HAVE_IN_PLACE_ADDENDS)
> >> {
> >> reloc_howto_type *howto;
> >> bfd_vma addr_mask;
> >>
> >> howto = bfd_reloc_type_lookup (stdoutput, fixp->fx_r_type);
> >> addr_mask = ((bfd_vma) 1 << (HAVE_64BIT_OBJECTS ? 63 : 31) << 1) -
> 1;
> >> if ((howto->src_mask & addr_mask) != addr_mask)
> >> return 0;
> >> }
> >>
> >> would be better (completely untested).
> >
> > This suggestion isn't correct either.
>
> Oh well, thanks for trying.
>
> > This fails to convert relocs that have a src_mask that is less than 32
> > bits such as R_MIPS_LO16. It also incorrectly converts JALR
> > relocations. Did you really mean to say:
> >
> > If (HAVE_IN_PLACE_ADDENDS
> > && (fixp->fx_pcrel || jalr_reloc (fixp->fx_r_type))
> >
> > For the initial condition? Testing with this looks good.
>
> Thanks for assuming that I meant something sensible, but I'm afraid I really
> did mean the original :-) I should have predicted the problem with it though.
>
> I don't understand the "also incorrectly converts JALR relocations"
> bit though. If so, how does keeping the jalr_reloc test help?
> I can imagine the same problem applies to JAL relocations.
I meant that without the jalr_reloc test, the jalr relocations weren't being handled properly.
>
> The blanket fixp->fx_pcrel is in that case probably wrong too.
> Although one isn't defined at the moment, there's no reason in principle why
> we couldn't have HI/LO pairs for PC-relative relocs.
>
> So, how about:
>
> /* Return true if RELOC is a PC-relative relocation that does not have
> full address range. */
>
> static inline bfd_boolean
> limited_pcrel_reloc_p (bfd_reloc_code_real_type reloc) (
> switch (reloc)
> {
> case BFD_RELOC_16_PCREL_S2:
> case BFD_RELOC_MICROMIPS_7_PCREL_S1:
> case BFD_RELOC_MICROMIPS_10_PCREL_S1:
> case BFD_RELOC_MICROMIPS_16_PCREL_S1:
> return TRUE;
>
> case BFD_RELOC_32_PCREL:
> return HAVE_64BIT_ADDRESSES;
>
> default:
> return FALSE;
> }
> }
>
> in the "jalr_reloc_p" part of tc-mips.c and:
>
> /* There is no place to store an in-place offset for JALR relocations.
> Likewise an in-range offset of limited PC-relative relocations may
> overflow the in-place relocatable field if recalculated against the
> start address of the symbol's containing section. */
> if (HAVE_IN_PLACE_ADDENDS
> && (limited_pcrel_reloc_p (fixp->fx_r_type)
> || jalr_reloc_p (fixp->fx_r_type)))
> return 0;
>
> ?
>
Yes. This looks much better. A new version of the patch is attached, although I would like to commit the mips_fix_adjustable/limited_pc_reloc change separately from the relocation patch, if that's okay.
Thanks,
Catherine
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mips-reloc-pc32.patch3
Type: application/octet-stream
Size: 9272 bytes
Desc: mips-reloc-pc32.patch3
URL: <https://sourceware.org/pipermail/binutils/attachments/20130503/fa4cc439/attachment.obj>
More information about the Binutils
mailing list