[BFD][AARCH64]Properly truncate no overflow checking relocation value for load/store immediate.

[Renlin Li]

Hi Peter,

Thanks for the comment!
(I noticed that when I am working on the fix. But I forgot why I didn't do it there)

I update the patch by using PG_OFFSET when resolving the relocation.
Regression test Okay.

Regards,
Renlin

bfd/ChangeLog:

2018-04-27  Renlin Li  <renlin.li@arm.com>

	* elfxx-aarch64.c (_bfd_aarch64_elf_resolve_relocation): Use PG_OFFSET
	to resolve BFD_RELOC_AARCH64_TLSLD_LDST16_DTPREL_LO12_NC,
	BFD_RELOC_AARCH64_TLSLD_LDST32_DTPREL_LO12_NC,
	BFD_RELOC_AARCH64_TLSLD_LDST64_DTPREL_LO12_NC,
	BFD_RELOC_AARCH64_TLSLD_LDST8_DTPREL_LO12_NC,
	BFD_RELOC_AARCH64_TLSLE_LDST16_TPREL_LO12_NC,
	BFD_RELOC_AARCH64_TLSLE_LDST32_TPREL_LO12_NC,
	BFD_RELOC_AARCH64_TLSLE_LDST64_TPREL_LO12_NC,
	BFD_RELOC_AARCH64_TLSLE_LDST8_TPREL_LO12_NC.

ld/ChangeLog:

2018-04-27 Renlin Li  <renlin.li@arm.com>

      * testsuite/ld-aarch64/emit-relocs-115.d: Update test with new value.
      * testsuite/ld-aarch64/emit-relocs-534.d: Likewise.
      * testsuite/ld-aarch64/emit-relocs-555.d: Likewise.

On 04/26/2018 03:53 PM, Peter Smith wrote:
> Hello Renlin,
> 
> I've checked that the patch works on an example I had that previously
> gave different results to Gold and an in-progress implementation in
> LLD. To the best of my knowledge the bitsizes in the HOWTO and the
> method to truncate the result will work. One thing that I'm a bit
> curious about is that relocations such as
> BFD_RELOC_AARCH64_LDST64_LO12 use "value = PG_OFFSET (value +
> addend);" to calculate the addend, whereas relocations like
> BFD_RELOC_AARCH64_TLSLD_LDST32_DTPREL_LO12_NC use "value = value +
> addend". In the former case the truncation is done in
> _bfd_aarch64_elf_resolve_relocation whereas in the latter you are
> having to truncate in _bfd_aarch64_elf_put_addend; it seems like you
> might be able to use PG_OFFSET for all the LO12 relocations and not
> need to put an extra truncation step in? I'm no expert in the BFD
> codebase, so there may be good reasons not to do that so just treat
> that as an observation.
> 
> Thanks for the fix.
> 
> Peter
> 
> On 26 April 2018 at 14:05, Renlin Li <renlin.li@foss.arm.com> wrote:
>> Hi all,
>>
>>
>> In aarch64, there are relocations for the unsigned immediate offset of
>> load/store instruction.
>> The size of the immediate field is 12-bit. In the scaled case, not all
>> 12-bit will be used.
>>
>> For example: R_AARCH64_LD64_GOT_LO12_NC
>> Set the LD/ST immediate field to bits [11:3] of relocated value. No overflow
>> check.
>>
>> In this case, the top 3 bits of the immediate field of the load instruction
>> should
>> be all 0. Only 9 bits from the value should be written into the instruction
>> encoding.
>>
>> For relocations with over-flow check, if the value is larger than 12-bits,
>> it
>> will be caught by the checking mechanism.
>>
>> But for relocations without no overflow check, the value could be larger
>> than
>> 12 bits, after scaling, it might take more bits than the specification
>> specified. The value should be properly truncated.
>>
>> The patch here correct the bitsize field of a few relocations, and the
>> bitsize
>> field is used to properly truncate the value written into the instruction
>> encoding.
>>
>> Okay to commit?
>>
>> bfd/ChangeLog:
>>
>> 2018-04-26  Renlin Li  <renlin.li@arm.com>
>>
>>          * elfnn-aarch64.c (elfNN_aarch64_howto_table): Correct the bitsize
>>          field of R_AARCH64_LD64_GOT_LO12_NC, R_AARCH64_P32_LD32_GOT_LO12_NC,
>>          R_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC,
>>          R_AARCH64_P32_LD32_GOTTPREL_LO12_NC,
>>          R_AARCH64_LDST16_ABS_LO12_NC,
>>          R_AARCH64_P32_TLSDESC_LD32_LO12_NC.
>>          R_AARCH64_P32_TLSDESC_LD32_LO12_NC.
>>          R_AARCH64_TLSDESC_LD64_LO12_NC.
>>          R_AARCH64_LDST32_ABS_LO12_NC,
>>          R_AARCH64_LDST64_ABS_LO12_NC,
>>          R_AARCH64_LDST128_ABS_LO12_NC,
>>          * elfxx-aarch64.c (_bfd_aarch64_elf_put_addend): Truncate the
>>          immediate value for load/store instruction.
>>
>> ld/ChangeLog:
>>
>> 2018-04-26 Renlin Li  <renlin.li@arm.com>
>>
>>          * testsuite/ld-aarch64/emit-relocs-115.d: Update test with new
>> value.
>>          * testsuite/ld-aarch64/emit-relocs-534.d: Likewise.
>>          * testsuite/ld-aarch64/emit-relocs-555.d: Likewise.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: tmp.diff
Type: text/x-patch
Size: 4928 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20180427/6bb72420/attachment.bin>