Commit: PR 22773: Reject illegal immediate values for Thumb ORR instructions

Message ID 8737247s39.fsf@redhat.com
State New
Headers show
Series
  • Commit: PR 22773: Reject illegal immediate values for Thumb ORR instructions
Related show

Commit Message

Nick Clifton Feb. 13, 2018, 4:49 p.m.
Hi Guys,

  I am applying the patch below to fix a problem with the ARM
  assembler.  It was silently accepting an illegal immediate value for
  an ORR instruction, and converting it into a MOV instruction instead.
  The patch fixes this so that the assembler will now reject the invalid
  value.

Cheers
  Nick

gas/ChangeLog
2018-02-13  Nick Clifton  <nickc@redhat.com>

	PR 22773
	* config/tc-arm.c (md_apply_fix): Test Rn field of Thumb ORR
	instruction before assuming that it is a MOV instruction.
	* testsuite/gas/arm/pr22773.s: New test.
	* testsuite/gas/arm/pr22773.d: New test driver.
	* testsuite/gas/arm/pr22773.l: New expected output.

Comments

Andre Vieira (lists) March 29, 2018, 9:19 a.m. | #1
Hi Nick,

Thank you for this fix! Any chance you could backport this to the 2.30
branch? I checked and it applies cleanly and tests look good too for
arm-none-eabi.

Cheers,
Andre

On 13/02/18 16:49, Nick Clifton wrote:
> Hi Guys,

> 

>   I am applying the patch below to fix a problem with the ARM

>   assembler.  It was silently accepting an illegal immediate value for

>   an ORR instruction, and converting it into a MOV instruction instead.

>   The patch fixes this so that the assembler will now reject the invalid

>   value.

> 

> Cheers

>   Nick

> 

> gas/ChangeLog

> 2018-02-13  Nick Clifton  <nickc@redhat.com>

> 

> 	PR 22773

> 	* config/tc-arm.c (md_apply_fix): Test Rn field of Thumb ORR

> 	instruction before assuming that it is a MOV instruction.

> 	* testsuite/gas/arm/pr22773.s: New test.

> 	* testsuite/gas/arm/pr22773.d: New test driver.

> 	* testsuite/gas/arm/pr22773.l: New expected output.

> 

> diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c

> index 9786f14762..7a5c02b2bb 100644

> --- a/gas/config/tc-arm.c

> +++ b/gas/config/tc-arm.c

> @@ -23596,12 +23596,14 @@ md_apply_fix (fixS *	fixP,

>  	      /* MOV accepts both Thumb2 modified immediate (T2 encoding) and

>  		 UINT16 (T3 encoding), MOVW only accepts UINT16.  When

>  		 disassembling, MOV is preferred when there is no encoding

> -		 overlap.

> -		 NOTE: MOV is using ORR opcode under Thumb 2 mode.  */

> +		 overlap.  */

>  	      if (((newval >> T2_DATA_OP_SHIFT) & 0xf) == T2_OPCODE_ORR

> +		  /* NOTE: MOV uses the ORR opcode in Thumb 2 mode

> +		     but with the Rn field [19:16] set to 1111.  */

> +		  && (((newval >> 16) & 0xf) == 0xf)

>  		  && ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2_v8m)

>  		  && !((newval >> T2_SBIT_SHIFT) & 0x1)

> -		  && value >= 0 && value <=0xffff)

> +		  && value >= 0 && value <= 0xffff)

>  		{

>  		  /* Toggle bit[25] to change encoding from T2 to T3.  */

>  		  newval ^= 1 << 25;

> --- /dev/null	2018-02-13 08:01:57.617024533 +0000

> +++ gas/testsuite/gas/arm/pr22773.s	2018-02-13 16:35:32.293364259 +0000

> @@ -0,0 +1,9 @@

> +	.syntax unified

> +	.cpu cortex-m4

> +	.thumb

> +

> +	.section  .text

> +

> +	orr r1, #12800	 	/* This is OK.  */

> +	orr r1, #12801		/* This cannot be encoded in Thumb mode.  */

> +	/* GAS used to accept it though, and produce a MOV instruction instead.  */

> --- /dev/null	2018-02-13 08:01:57.617024533 +0000

> +++ gas/testsuite/gas/arm/pr22773.d	2018-02-13 16:31:37.298292983 +0000

> @@ -0,0 +1,2 @@

> +# name: PR 22773: Invalid immediate constants produce incorrect instruction

> +# error-output: pr22773.l

> --- /dev/null	2018-02-13 08:01:57.617024533 +0000

> +++ gas/testsuite/gas/arm/pr22773.l	2018-02-13 16:29:55.610561563 +0000

> @@ -0,0 +1,3 @@

> +[^:]*: Assembler messages:

> +[^:]*:8: Error: invalid constant \(3201\) after fixup

> +#pass

>
Nick Clifton March 29, 2018, 11:32 a.m. | #2
Hi Andre,

> Thank you for this fix! Any chance you could backport this to the 2.30

> branch? 


Done.

Cheers
  Nick

Patch

diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 9786f14762..7a5c02b2bb 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -23596,12 +23596,14 @@  md_apply_fix (fixS *	fixP,
 	      /* MOV accepts both Thumb2 modified immediate (T2 encoding) and
 		 UINT16 (T3 encoding), MOVW only accepts UINT16.  When
 		 disassembling, MOV is preferred when there is no encoding
-		 overlap.
-		 NOTE: MOV is using ORR opcode under Thumb 2 mode.  */
+		 overlap.  */
 	      if (((newval >> T2_DATA_OP_SHIFT) & 0xf) == T2_OPCODE_ORR
+		  /* NOTE: MOV uses the ORR opcode in Thumb 2 mode
+		     but with the Rn field [19:16] set to 1111.  */
+		  && (((newval >> 16) & 0xf) == 0xf)
 		  && ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v6t2_v8m)
 		  && !((newval >> T2_SBIT_SHIFT) & 0x1)
-		  && value >= 0 && value <=0xffff)
+		  && value >= 0 && value <= 0xffff)
 		{
 		  /* Toggle bit[25] to change encoding from T2 to T3.  */
 		  newval ^= 1 << 25;
--- /dev/null	2018-02-13 08:01:57.617024533 +0000
+++ gas/testsuite/gas/arm/pr22773.s	2018-02-13 16:35:32.293364259 +0000
@@ -0,0 +1,9 @@ 
+	.syntax unified
+	.cpu cortex-m4
+	.thumb
+
+	.section  .text
+
+	orr r1, #12800	 	/* This is OK.  */
+	orr r1, #12801		/* This cannot be encoded in Thumb mode.  */
+	/* GAS used to accept it though, and produce a MOV instruction instead.  */
--- /dev/null	2018-02-13 08:01:57.617024533 +0000
+++ gas/testsuite/gas/arm/pr22773.d	2018-02-13 16:31:37.298292983 +0000
@@ -0,0 +1,2 @@ 
+# name: PR 22773: Invalid immediate constants produce incorrect instruction
+# error-output: pr22773.l
--- /dev/null	2018-02-13 08:01:57.617024533 +0000
+++ gas/testsuite/gas/arm/pr22773.l	2018-02-13 16:29:55.610561563 +0000
@@ -0,0 +1,3 @@ 
+[^:]*: Assembler messages:
+[^:]*:8: Error: invalid constant \(3201\) after fixup
+#pass