[GAS,ARM] Invalid LDR immediate transformation

Message ID 56532CF5.5070300@st.com
State New
Headers show

Commit Message

Christophe MONAT Nov. 23, 2015, 3:12 p.m.
"Load immediate values using LDR Rd, =const" as described in
<http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0473l/dom1359731147386.html> 
is currently broken.

The symptom appears with the following fragment that transforms a
'ldr' into a 'cmp':

$ cat weird-a53.s
         .syntax unified
         .thumb
         .text
         .align  2
         .global         foo
         .thumb_func
         .type   foo, %function
foo:
          ldr     r12, =#0x99

$ arm-none-eabi-as weird-a53.s
$ arm-none-eabi-objdump -t
(snip)
Disassembly of section .text:

00000000 <foo>:
      0:   2c99            cmp     r4, #153        ; 0x99
      2:   46c0            nop                     ; (mov r8, r8)

It was originally reported by an internal user on cortex-a53 in
AArch32, but the issue is unrelated to a specific target and shows up
for any architecture supporting this transformation, eg from armv6t2.

The issue happens because a T1 encoding is used with a high register,
and the bits of the register number leak into the opcode, leading to
the transformation into a compare.

It looks like this is exposed in binutils trunk since the changes for
this <https://sourceware.org/bugzilla/show_bug.cgi?id=18500> have been
merged into the trunk.

I have attached a patch, tested on an x86_64 host in arm-none-eabi
arm-linux-eabi arm-nacl, no regression.

Waiting for any feedback, thanks.
--C
commit 791bba5593322493a86d35c4bc0e3e1bc09dae0b
Author: Christophe Monat <christophe.monat@st.com>
Date:   Mon Nov 23 11:07:25 2015 +0100

    gas/ChangeLog:
    
    2015-11-23  Christophe Monat <christophe.monat@st.com>
    
    	* config/tc-arm.c (move_or_literal_pool): Do not transform ldr
    	ri,=imm into movs when ri is a high register in T1.
    
    gas/testsuite/ChangeLog:
    
    2015-11-23  Christophe Monat <christophe.monat@st.com>
    
    	* gas/arm/thumb2_ldr_immediate_armv6t2.s: Added high register
    	tests.
    	* gas/arm/thumb2_ldr_immediate_armv6t2.d: Accounted for new test
    	cases.
    	* gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s: New.
    	* gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d: New.

Patch hide | download patch | download mbox

diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index efc522a..6c2f6e8 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -7839,7 +7839,8 @@  move_or_literal_pool (int i, enum lit_type t, bfd_boolean mode_3)
 	{
 	  if (thumb_p)
 	    {
-	      if ((v & ~0xFF) == 0)
+	      /* This can be encoded only for a low register.  */
+	      if ((v & ~0xFF) == 0 && (inst.operands[i].reg < 8))
 		{
 		  /* This can be done with a mov(1) instruction.  */
 		  inst.instruction = T_OPCODE_MOV_I8 | (inst.operands[i].reg << 8);
diff --git a/gas/testsuite/gas/arm/thumb2_ldr_immediate_armv6t2.d b/gas/testsuite/gas/arm/thumb2_ldr_immediate_armv6t2.d
index 698371a..ad15604 100644
--- a/gas/testsuite/gas/arm/thumb2_ldr_immediate_armv6t2.d
+++ b/gas/testsuite/gas/arm/thumb2_ldr_immediate_armv6t2.d
@@ -11,5 +11,9 @@  Disassembly of section .text:
 0[0-9a-f]+ <[^>]+> f44f 228e 	mov.w	r2, #290816	.*
 0[0-9a-f]+ <[^>]+> f6cf 7232 	movt	r2, #65330	.*
 0[0-9a-f]+ <[^>]+> f241 32f1 	movw	r2, #5105	.*
-
-
+0[0-9a-f]+ <[^>]+> f04f 3872 	mov.w	r8, #1920103026	.*
+0[0-9a-f]+ <[^>]+> f04f 2863 	mov.w	r8, #1660969728	.*
+0[0-9a-f]+ <[^>]+> f04f 1851 	mov.w	r8, #5308497	.*
+0[0-9a-f]+ <[^>]+> f44f 298e 	mov.w	r9, #290816	.*
+0[0-9a-f]+ <[^>]+> f6cf 7932 	movt	r9, #65330	.*
+0[0-9a-f]+ <[^>]+> f241 39f1 	movw	r9, #5105	.*
diff --git a/gas/testsuite/gas/arm/thumb2_ldr_immediate_armv6t2.s b/gas/testsuite/gas/arm/thumb2_ldr_immediate_armv6t2.s
index 22a5014..4b94c91 100644
--- a/gas/testsuite/gas/arm/thumb2_ldr_immediate_armv6t2.s
+++ b/gas/testsuite/gas/arm/thumb2_ldr_immediate_armv6t2.s
@@ -9,4 +9,10 @@  thumb2_ldr:
 	ldr r2,=0x00047000
 	ldr r2,=0xFF320000
 	ldr r2,=0x000013F1
+	ldr r8,=0x72727272
+	ldr r8,=0x63006300
+	ldr r8,=0x00510051
+	ldr r9,=0x00047000
+	ldr r9,=0xFF320000
+	ldr r9,=0x000013F1
 	.pool
diff --git a/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d b/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d
new file mode 100644
index 0000000..55b5f17
--- /dev/null
+++ b/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.d
@@ -0,0 +1,24 @@ 
+# name: Ldr small immediate high registers on armv6t2
+# as: -march=armv6t2
+# objdump: -dr --prefix-addresses --show-raw-insn
+# not-target: *-*-*coff *-*-pe *-*-wince *-*-*aout* *-*-netbsd *-*-riscix*
+
+.*: +file format .*arm.*
+
+Disassembly of section \.text:
+0[0-9a-f]+ <[^>]+> 2000[[:space:]]+movs[[:space:]]+r0, #0.*
+0[0-9a-f]+ <[^>]+> 2108[[:space:]]+movs[[:space:]]+r1, #8.*
+0[0-9a-f]+ <[^>]+> 2251[[:space:]]+movs[[:space:]]+r2, #81.*
+0[0-9a-f]+ <[^>]+> 231f[[:space:]]+movs[[:space:]]+r3, #31.*
+0[0-9a-f]+ <[^>]+> 242f[[:space:]]+movs[[:space:]]+r4, #47.*
+0[0-9a-f]+ <[^>]+> 253f[[:space:]]+movs[[:space:]]+r5, #63.*
+0[0-9a-f]+ <[^>]+> 2680[[:space:]]+movs[[:space:]]+r6, #128.*
+0[0-9a-f]+ <[^>]+> 27ff[[:space:]]+movs[[:space:]]+r7, #255.*
+0[0-9a-f]+ <[^>]+> f04f 0800[[:space:]]+mov\.w[[:space:]]+r8, #0.*
+0[0-9a-f]+ <[^>]+> f04f 0908[[:space:]]+mov\.w[[:space:]]+r9, #8.*
+0[0-9a-f]+ <[^>]+> f04f 0a51[[:space:]]+mov\.w[[:space:]]+sl, #81.*
+0[0-9a-f]+ <[^>]+> f04f 0b1f[[:space:]]+mov\.w[[:space:]]+fp, #31.*
+0[0-9a-f]+ <[^>]+> f04f 0c2f[[:space:]]+mov\.w[[:space:]]+ip, #47.*
+0[0-9a-f]+ <[^>]+> f04f 0d3f[[:space:]]+mov\.w[[:space:]]+sp, #63.*
+0[0-9a-f]+ <[^>]+> f04f 0e80[[:space:]]+mov\.w[[:space:]]+lr, #128.*
+0[0-9a-f]+ <[^>]+> f04f 0fff[[:space:]]+mov\.w[[:space:]]+pc, #255.*
diff --git a/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s b/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s
new file mode 100644
index 0000000..d225410
--- /dev/null
+++ b/gas/testsuite/gas/arm/thumb2_ldr_immediate_highregs_armv6t2.s
@@ -0,0 +1,24 @@ 
+	.thumb
+	.syntax unified
+	.thumb_func
+thumb2_ldr:
+	# These can be encoded into movs since constant is small
+	# And register can be encoded in 3 bits
+	ldr r0,=0x00
+	ldr r1,=0x08
+	ldr r2,=0x51
+	ldr r3,=0x1F
+	ldr r4,=0x2F
+	ldr r5,=0x3F
+	ldr r6,=0x80
+	ldr r7,=0xFF
+	# These shall be encoded into mov.w
+	# Since register cannot be encoded in 3 bits
+	ldr r8,=0x00
+	ldr r9,=0x08
+	ldr r10,=0x51
+	ldr r11,=0x1F
+	ldr r12,=0x2F
+	ldr r13,=0x3F
+	ldr r14,=0x80
+	ldr r15,=0xFF