Re: [PATCH, MIPS] Relax alignment check for LDPC

[Richard Sandiford <rdsandiford at googlemail dot com>]

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
>> The R6 LDPC instruction requires an 8-byte aligned load address but
>> does not require LDPC itself to be 8-byte aligned. Unfortunately the
>> tests for LPDC were aligning the LDPC instruction so did not catch
>> this issue.
>>
>> The alignment check for BFD_RELOC_MIPS_18_PCREL_S3 was incorrectly
>> using the valP value as the target address of the reloc when it is
>> actually the calculated value to be patched into an instruction.
>> Since BFD_RELOC_MIPS_18_PCREL_S3 is recorded as relative to the
>> current PC rather than PC & ~7 then valP can end up as a multiple
>> of 4 rather than 8 even if the target address is correctly 8-byte
>> aligned.  Instead of recalculating the true target address from
>> the fixup I just check that valP is a 4-byte multiple leaving the
>> linker to do the accurate check as it could end up misaligning the
>> target anyway.
>> [...]
>> diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
>> index c9266db..3adebf4 100644
>> --- a/gas/config/tc-mips.c
>> +++ b/gas/config/tc-mips.c
>> @@ -15009,7 +15009,11 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg ATTRIBUTE_UNUSED)
>>        break;
>>  
>>      case BFD_RELOC_MIPS_18_PCREL_S3:
>> -      if ((*valP & 0x7) != 0)
>> +      /* There is little point checking for a correctly aligned destination
>> +	 for R_MIPS_PC18_S3 as only the linker will know for sure if the
>> +	 destination address ends up 8-byte aligned or not.  Check for a
>> +	 minimum of 4-byte alignment instead.  */
>> +      if ((*valP & 0x3) != 0)
>>  	as_bad_where (fixP->fx_file, fixP->fx_line,
>>  		      _("PC-relative access to misaligned address (%lx)"),
>>  		      (long) *valP);
>
> It seems a bit dangerous to drop a bit on the floor though (which I assume
> we'd end up doing for REL).  Also, in practice, the relocation is only safe
> if the target has 8-byte alignment at assembly time.  I.e. the section that
> the target is in should have at least 8 bytes alignment and the symbol
> itself should have an offset that's divisible by 8.
>
> So I think we should either check S_GET_VALUE (fx_addsy) + fx_offset
> is aligned or (probably better, if it works) check that the low 3 bits
> of *valP are the same as the low three bits of:
>
>     fixP->fx_where + fixP->fx_frag->fr_address

BTW, it might also be good to tweak the error message, since the current
one makes it sound like *valP is the address.

Thanks,
Richard