This is the mail archive of the mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] [MIPS] Compact EH Infrastructure R_MIPS_PC32

"Moore, Catherine" <> writes:
>> 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.

Sorry, I still don't get it.  We're talking about the difference between:


which you said didn't handle jalr relocs correctly, and:

      && (fixp->fx_pcrel || jalr_reloc_p (fixp->fx_r_type)))

where jalr_reloc_p returns true for jalr relocs.  Aren't the two the same
for jalr relocs?  The same BLAH is executed in both cases.  I suppose it
doesn't matter now though.

>> 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:
>>       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.  */
>>       && (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.

Either way's fine.  Patch is OK with a suitable changelog.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]