This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH] [MIPS] Compact EH Infrastructure R_MIPS_PC32
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: "Moore\, Catherine" <Catherine_Moore at mentor dot com>
- Cc: "binutils\ at sourceware dot org" <binutils at sourceware dot org>
- Date: Thu, 02 May 2013 15:20:01 +0100
- Subject: Re: [PATCH] [MIPS] Compact EH Infrastructure R_MIPS_PC32
- References: <FD3DCEAC5B03E9408544A1E416F11242F8F96D19 at NA-MBX-04 dot mgc dot mentorg dot com> <87a9oql7u3 dot fsf at talisman dot default> <FD3DCEAC5B03E9408544A1E416F11242F8FB332D at NA-MBX-01 dot mgc dot mentorg dot com>
"Moore, Catherine" <Catherine_Moore@mentor.com> writes:
>> -----Original Message-----
>> From: Richard Sandiford [mailto:email@example.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.
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)
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. */
&& (limited_pcrel_reloc_p (fixp->fx_r_type)
|| jalr_reloc_p (fixp->fx_r_type)))