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

[Renlin Li <renlin dot li at foss dot arm dot com>]

Ping~



On 04/27/2018 10:46 PM, Renlin Li wrote:
> 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.