This is the mail archive of the binutils@sourceware.org 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: [BFD][AARCH64]Properly truncate no overflow checking relocation value for load/store immediate.


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.

commit a7b3fde0b637089dadeb37b0c5f7de27162f67e0
Author: Renlin Li <renlin.li@arm.com>
Date:   Fri Apr 27 10:48:18 2018 +0100

    [BFD][AARCH64]Properly truncate no overflow checking relocation value for load/store immediate.
    
    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.

diff --git a/bfd/elfxx-aarch64.c b/bfd/elfxx-aarch64.c
index 08e7cd9..59fdc5b 100644
--- a/bfd/elfxx-aarch64.c
+++ b/bfd/elfxx-aarch64.c
@@ -452,26 +452,18 @@ _bfd_aarch64_elf_resolve_relocation (bfd_reloc_code_real_type r_type,
     case BFD_RELOC_AARCH64_TLSLD_ADD_DTPREL_LO12:
     case BFD_RELOC_AARCH64_TLSLD_ADD_DTPREL_LO12_NC:
     case BFD_RELOC_AARCH64_TLSLD_LDST16_DTPREL_LO12:
-    case BFD_RELOC_AARCH64_TLSLD_LDST16_DTPREL_LO12_NC:
     case BFD_RELOC_AARCH64_TLSLD_LDST32_DTPREL_LO12:
-    case BFD_RELOC_AARCH64_TLSLD_LDST32_DTPREL_LO12_NC:
     case BFD_RELOC_AARCH64_TLSLD_LDST64_DTPREL_LO12:
-    case BFD_RELOC_AARCH64_TLSLD_LDST64_DTPREL_LO12_NC:
     case BFD_RELOC_AARCH64_TLSLD_LDST8_DTPREL_LO12:
-    case BFD_RELOC_AARCH64_TLSLD_LDST8_DTPREL_LO12_NC:
     case BFD_RELOC_AARCH64_TLSLD_MOVW_DTPREL_G0:
     case BFD_RELOC_AARCH64_TLSLD_MOVW_DTPREL_G0_NC:
     case BFD_RELOC_AARCH64_TLSLD_MOVW_DTPREL_G1:
     case BFD_RELOC_AARCH64_TLSLD_MOVW_DTPREL_G1_NC:
     case BFD_RELOC_AARCH64_TLSLD_MOVW_DTPREL_G2:
     case BFD_RELOC_AARCH64_TLSLE_LDST16_TPREL_LO12:
-    case BFD_RELOC_AARCH64_TLSLE_LDST16_TPREL_LO12_NC:
     case BFD_RELOC_AARCH64_TLSLE_LDST32_TPREL_LO12:
-    case BFD_RELOC_AARCH64_TLSLE_LDST32_TPREL_LO12_NC:
     case BFD_RELOC_AARCH64_TLSLE_LDST64_TPREL_LO12:
-    case BFD_RELOC_AARCH64_TLSLE_LDST64_TPREL_LO12_NC:
     case BFD_RELOC_AARCH64_TLSLE_LDST8_TPREL_LO12:
-    case BFD_RELOC_AARCH64_TLSLE_LDST8_TPREL_LO12_NC:
       value = value + addend;
       break;
 
@@ -521,7 +513,15 @@ _bfd_aarch64_elf_resolve_relocation (bfd_reloc_code_real_type r_type,
     case BFD_RELOC_AARCH64_TLSGD_ADD_LO12_NC:
     case BFD_RELOC_AARCH64_TLSIE_LD32_GOTTPREL_LO12_NC:
     case BFD_RELOC_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC:
+    case BFD_RELOC_AARCH64_TLSLD_LDST16_DTPREL_LO12_NC:
+    case BFD_RELOC_AARCH64_TLSLD_LDST32_DTPREL_LO12_NC:
+    case BFD_RELOC_AARCH64_TLSLD_LDST64_DTPREL_LO12_NC:
+    case BFD_RELOC_AARCH64_TLSLD_LDST8_DTPREL_LO12_NC:
     case BFD_RELOC_AARCH64_TLSLE_ADD_TPREL_LO12_NC:
+    case BFD_RELOC_AARCH64_TLSLE_LDST16_TPREL_LO12_NC:
+    case BFD_RELOC_AARCH64_TLSLE_LDST32_TPREL_LO12_NC:
+    case BFD_RELOC_AARCH64_TLSLE_LDST64_TPREL_LO12_NC:
+    case BFD_RELOC_AARCH64_TLSLE_LDST8_TPREL_LO12_NC:
       value = PG_OFFSET (value + addend);
       break;
 
diff --git a/ld/testsuite/ld-aarch64/emit-relocs-115.d b/ld/testsuite/ld-aarch64/emit-relocs-115.d
index f436d32..95a6e31 100644
--- a/ld/testsuite/ld-aarch64/emit-relocs-115.d
+++ b/ld/testsuite/ld-aarch64/emit-relocs-115.d
@@ -6,5 +6,5 @@
 00010000 <.text>:
    10000:	798019d6 	ldrsh	x22, \[x14, #12\]
 			10000: R_AARCH64_P32_TLSLE_LDST16_TPREL_LO12_NC	v2
-   10004:	79a72a28 	ldrsh	x8, \[x17, #5012\]
+   10004:	79872a28 	ldrsh	x8, \[x17, #916\]
 			10004: R_AARCH64_P32_TLSLE_LDST16_TPREL_LO12_NC	v3
diff --git a/ld/testsuite/ld-aarch64/emit-relocs-534.d b/ld/testsuite/ld-aarch64/emit-relocs-534.d
index 121fdc4..fe59b23 100644
--- a/ld/testsuite/ld-aarch64/emit-relocs-534.d
+++ b/ld/testsuite/ld-aarch64/emit-relocs-534.d
@@ -5,5 +5,5 @@
 0000000000010000 <.text>:
    10000:	798009d6 	ldrsh	x22, \[x14, #4\]
 			10000: R_AARCH64_TLSLD_LDST16_DTPREL_LO12_NC	v2
-   10004:	79a71a28 	ldrsh	x8, \[x17, #5004\]
+   10004:	79871a28 	ldrsh	x8, \[x17, #908\]
 			10004: R_AARCH64_TLSLD_LDST16_DTPREL_LO12_NC	v3
diff --git a/ld/testsuite/ld-aarch64/emit-relocs-555.d b/ld/testsuite/ld-aarch64/emit-relocs-555.d
index e866b60..1e33998 100644
--- a/ld/testsuite/ld-aarch64/emit-relocs-555.d
+++ b/ld/testsuite/ld-aarch64/emit-relocs-555.d
@@ -5,5 +5,5 @@
 0000000000010000 <.text>:
    10000:	798029d6 	ldrsh	x22, \[x14, #20\]
 			10000: R_AARCH64_TLSLE_LDST16_TPREL_LO12_NC	v2
-   10004:	79a73a28 	ldrsh	x8, \[x17, #5020\]
+   10004:	79873a28 	ldrsh	x8, \[x17, #924\]
 			10004: R_AARCH64_TLSLE_LDST16_TPREL_LO12_NC	v3

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