[PATCH] ARM: gas not detecting invalid operands for Thumb2 ADD{S} and SUB{S}

[PATCH] ARM: gas not detecting invalid operands for Thumb2 ADD{S} and SUB{S}

[Paul Carroll]

[-- Attachment #1: Type: text/plain, Size: 2755 bytes --]

(I don't have write permission into the Binutils CVS, so someone else 
will be merging the final patch.)

In ARMv6T2 and ARMv7 Thumb2, the ADD, ADDS, SUB, and SUBS instructions 
added several new instruction forms. One of the new forms allowed is:

    ADD{S}<c>.W <Rd>,SP,<Rm>{,<shift>}
    SUB{S}<c>.W <Rd>,SP,<Rm>{,<shift>}

According to DDI-0406B page A8-30

    d = UInt(Rd); m = UInt(Rm); setflags = (S == '1');
    (shift_t, shift_n) = DecodeImmShift(type, imm3:imm2);
    if d == 13&&   (shift_t != SRType_LSL || shift_n>   3) then 
UNPREDICTABLE;
    if d == 15 || BadReg(m) then UNPREDICTABLE;

The tc-arm.c file in the gas/config directory was already detecting the 
'd==15' condition. But, there was no validation of the shift type or 
shift value when the first register specified was SP.
This patch adds that check.

The patch could be reorganized a little, to move the added constraints 
out of
encode_thumb32_shifted_operand() and into do_t_add_sub().  That would 
get rid of
passing an argument into encode_thumb32_shifted_operand() but add some 
minimal duplicate
code into do_t_add_sub().  Either would produce identical behavior.
I will leave it to others as to whether the patch should be reorganized.

I also have 3 new test cases for the gas/arm test suite - add, 
addthumb2, and addthumb2err. The 'add' and 'addthumb2' test cases 
represented valid versions of ADD, ADDS, SUB, and SUBS. The 
'addthumb2err' test case represents operand combinations that should 
have generated an error. These invalid tests use SP for the first 2 
operands and r0 as the 3rd operand. The LSR, ASR, ROR, and RRX shift 
types are used with valid shift values, which is not legal. Then the LSL 
shift type is used with a shift value of 4, which is also not legal.

2011-02-25  Paul Carroll <pcarroll@codesourcery.com>

         gas/
         * config/tc-arm.c (encode_thumb32_shifted_operand): Added
         constraints for shift type and value for Thumb2 ADD{S} and
         SUB{S} instructions.
         (do_t_add_sub): Adding argument to encode calls to indicate if
         first 2 operands are both SP.
         (do_t_arit3, do_t_arit3c, do_t_mov_cmp, do_t_mvn_tst, do_t_orn)
         (do_t_rsb, do_t_shift): Adding FALSE argument to encode calls
         since that argument is not used by them.

         gas/testsuite/
         * gas/arm/add.s: new test, allowable ADD{S} and SUB{S} insns.
         * gas/arm/add.d: new result file.
         * gas/arm/addthumb32.s: new test, allowable ADD{S} and SUB{S} 
insns.
         * gas/arm/addthumb32.d: new result file.
         * gas/arm/addthumb32err.s: new test, bad ADD{S} and SUB{S} insns.
         * gas/arm/addthumb32err.d: new result file.
         * gas/arm/addthumb32err.l: new error file.

[-- Attachment #2: ARM_ADD.patch --]
[-- Type: text/plain, Size: 19431 bytes --]

Index: src/gas/config/tc-arm.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-arm.c,v
retrieving revision 1.476
diff -u -p -r1.476 tc-arm.c
--- src/gas/config/tc-arm.c	18 Jan 2011 14:10:44 -0000	1.476
+++ src/gas/config/tc-arm.c	28 Feb 2011 18:35:38 -0000
@@ -8914,13 +8914,17 @@ do_xsc_mra (void)
    it into inst.instruction in the format used by Thumb32.  */
 
 static void
-encode_thumb32_shifted_operand (int i)
+encode_thumb32_shifted_operand (int i, bfd_boolean is_add_sub_sp_sp)
 {
   unsigned int value = inst.reloc.exp.X_add_number;
   unsigned int shift = inst.operands[i].shift_kind;
 
   constraint (inst.operands[i].immisreg,
 	      _("shift by register not allowed in thumb mode"));
+  constraint (is_add_sub_sp_sp && value > 3,
+	      _("shift value over 3 not allowed in thumb mode"));
+  constraint (is_add_sub_sp_sp && shift != SHIFT_LSL,
+	      _("only LSL shift allowed in thumb mode"));
   inst.instruction |= inst.operands[i].reg;
   if (shift == SHIFT_RRX)
     inst.instruction |= SHIFT_ROR << 4;
@@ -9307,7 +9311,7 @@ do_t_add_sub (void)
 	  inst.instruction = THUMB_OP32 (inst.instruction);
 	  inst.instruction |= Rd << 8;
 	  inst.instruction |= Rs << 16;
-	  encode_thumb32_shifted_operand (2);
+	  encode_thumb32_shifted_operand (2, Rd == REG_SP && Rs == REG_SP);
 	}
     }
   else
@@ -9458,7 +9462,7 @@ do_t_arit3 (void)
 	  inst.instruction = THUMB_OP32 (inst.instruction);
 	  inst.instruction |= Rd << 8;
 	  inst.instruction |= Rs << 16;
-	  encode_thumb32_shifted_operand (2);
+	  encode_thumb32_shifted_operand (2, FALSE);
 	}
     }
   else
@@ -9555,7 +9559,7 @@ do_t_arit3c (void)
 	  inst.instruction = THUMB_OP32 (inst.instruction);
 	  inst.instruction |= Rd << 8;
 	  inst.instruction |= Rs << 16;
-	  encode_thumb32_shifted_operand (2);
+	  encode_thumb32_shifted_operand (2, FALSE);
 	}
     }
   else
@@ -10612,7 +10616,7 @@ do_t_mov_cmp (void)
 	    {
 	      inst.instruction = THUMB_OP32 (inst.instruction);
 	      inst.instruction |= Rn << r0off;
-	      encode_thumb32_shifted_operand (1);
+	      encode_thumb32_shifted_operand (1, FALSE);
 	    }
 	}
       else
@@ -10783,7 +10787,7 @@ do_t_mvn_tst (void)
 	      if (inst.instruction < 0xffff)
 		inst.instruction = THUMB_OP32 (inst.instruction);
 	      inst.instruction |= Rn << r0off;
-	      encode_thumb32_shifted_operand (1);
+	      encode_thumb32_shifted_operand (1, FALSE);
 	    }
 	}
     }
@@ -11082,7 +11086,7 @@ do_t_orn (void)
       constraint (inst.operands[2].shifted
 		  && inst.operands[2].immisreg,
 		  _("shift must be constant"));
-      encode_thumb32_shifted_operand (2);
+      encode_thumb32_shifted_operand (2, FALSE);
     }
 }
 
@@ -11284,7 +11288,7 @@ do_t_rsb (void)
 	}
     }
   else
-    encode_thumb32_shifted_operand (2);
+    encode_thumb32_shifted_operand (2, FALSE);
 }
 
 static void
@@ -11354,7 +11358,7 @@ do_t_shift (void)
 	      inst.instruction = THUMB_OP32 (THUMB_SETS_FLAGS (inst.instruction)
 					     ? T_MNEM_movs : T_MNEM_mov);
 	      inst.instruction |= inst.operands[0].reg << 8;
-	      encode_thumb32_shifted_operand (1);
+	      encode_thumb32_shifted_operand (1, FALSE);
 	      /* Prevent the incorrect generation of an ARM_IMMEDIATE fixup.  */
 	      inst.reloc.type = BFD_RELOC_UNUSED;
 	    }
Index: src/gas/testsuite/gas/arm/add.d
===================================================================
RCS file: src/gas/testsuite/gas/arm/add.d
diff -N src/gas/testsuite/gas/arm/add.d
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ src/gas/testsuite/gas/arm/add.d	28 Feb 2011 18:35:38 -0000
@@ -0,0 +1,97 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: ARM Add{S} and Sub{S} instructions
+#as: -march=armv7-a
+
+# Test some ARM instructions:
+
+.*: +file format .*arm.*
+
+Disassembly of section .text:
+0+000 <.*> e08dd200 	add	sp, sp, r0, lsl #4
+0+004 <.*> e08dd1a0 	add	sp, sp, r0, lsr #3
+0+008 <.*> e08dd1c0 	add	sp, sp, r0, asr #3
+0+00c <.*> e08dd1e0 	add	sp, sp, r0, ror #3
+0+010 <.*> e08dd060 	add	sp, sp, r0, rrx
+0+014 <.*> e09dd200 	adds	sp, sp, r0, lsl #4
+0+018 <.*> e09dd1a0 	adds	sp, sp, r0, lsr #3
+0+01c <.*> e09dd1c0 	adds	sp, sp, r0, asr #3
+0+020 <.*> e09dd1e0 	adds	sp, sp, r0, ror #3
+0+024 <.*> e09dd060 	adds	sp, sp, r0, rrx
+0+028 <.*> e04dd200 	sub	sp, sp, r0, lsl #4
+0+02c <.*> e04dd1a0 	sub	sp, sp, r0, lsr #3
+0+030 <.*> e04dd1c0 	sub	sp, sp, r0, asr #3
+0+034 <.*> e04dd1e0 	sub	sp, sp, r0, ror #3
+0+038 <.*> e04dd060 	sub	sp, sp, r0, rrx
+0+03c <.*> e05dd200 	subs	sp, sp, r0, lsl #4
+0+040 <.*> e05dd1a0 	subs	sp, sp, r0, lsr #3
+0+044 <.*> e05dd1c0 	subs	sp, sp, r0, asr #3
+0+048 <.*> e05dd1e0 	subs	sp, sp, r0, ror #3
+0+04c <.*> e05dd060 	subs	sp, sp, r0, rrx
+0+050 <.*> e08dd000 	add	sp, sp, r0
+0+054 <.*> e08dd080 	add	sp, sp, r0, lsl #1
+0+058 <.*> e08dd100 	add	sp, sp, r0, lsl #2
+0+05c <.*> e08dd180 	add	sp, sp, r0, lsl #3
+0+060 <.*> e09dd000 	adds	sp, sp, r0
+0+064 <.*> e09dd080 	adds	sp, sp, r0, lsl #1
+0+068 <.*> e09dd100 	adds	sp, sp, r0, lsl #2
+0+06c <.*> e09dd180 	adds	sp, sp, r0, lsl #3
+0+070 <.*> e04dd000 	sub	sp, sp, r0
+0+074 <.*> e04dd080 	sub	sp, sp, r0, lsl #1
+0+078 <.*> e04dd100 	sub	sp, sp, r0, lsl #2
+0+07c <.*> e04dd180 	sub	sp, sp, r0, lsl #3
+0+080 <.*> e05dd000 	subs	sp, sp, r0
+0+084 <.*> e05dd080 	subs	sp, sp, r0, lsl #1
+0+088 <.*> e05dd100 	subs	sp, sp, r0, lsl #2
+0+08c <.*> e05dd180 	subs	sp, sp, r0, lsl #3
+0+090 <.*> e08dd004 	add	sp, sp, r4
+0+094 <.*> e08dd084 	add	sp, sp, r4, lsl #1
+0+098 <.*> e08dd104 	add	sp, sp, r4, lsl #2
+0+09c <.*> e08dd184 	add	sp, sp, r4, lsl #3
+0+0a0 <.*> e09dd004 	adds	sp, sp, r4
+0+0a4 <.*> e09dd084 	adds	sp, sp, r4, lsl #1
+0+0a8 <.*> e09dd104 	adds	sp, sp, r4, lsl #2
+0+0ac <.*> e09dd184 	adds	sp, sp, r4, lsl #3
+0+0b0 <.*> e04dd004 	sub	sp, sp, r4
+0+0b4 <.*> e04dd084 	sub	sp, sp, r4, lsl #1
+0+0b8 <.*> e04dd104 	sub	sp, sp, r4, lsl #2
+0+0bc <.*> e04dd184 	sub	sp, sp, r4, lsl #3
+0+0c0 <.*> e05dd004 	subs	sp, sp, r4
+0+0c4 <.*> e05dd084 	subs	sp, sp, r4, lsl #1
+0+0c8 <.*> e05dd104 	subs	sp, sp, r4, lsl #2
+0+0cc <.*> e05dd184 	subs	sp, sp, r4, lsl #3
+0+0d0 <.*> e08d4000 	add	r4, sp, r0
+0+0d4 <.*> e08d4080 	add	r4, sp, r0, lsl #1
+0+0d8 <.*> e08d4100 	add	r4, sp, r0, lsl #2
+0+0dc <.*> e08d4180 	add	r4, sp, r0, lsl #3
+0+0e0 <.*> e08d4200 	add	r4, sp, r0, lsl #4
+0+0e4 <.*> e08d41a0 	add	r4, sp, r0, lsr #3
+0+0e8 <.*> e08d41c0 	add	r4, sp, r0, asr #3
+0+0ec <.*> e08d41e0 	add	r4, sp, r0, ror #3
+0+0f0 <.*> e08d4060 	add	r4, sp, r0, rrx
+0+0f4 <.*> e09d4000 	adds	r4, sp, r0
+0+0f8 <.*> e09d4080 	adds	r4, sp, r0, lsl #1
+0+0fc <.*> e09d4100 	adds	r4, sp, r0, lsl #2
+0+100 <.*> e09d4180 	adds	r4, sp, r0, lsl #3
+0+104 <.*> e09d4200 	adds	r4, sp, r0, lsl #4
+0+108 <.*> e09d41a0 	adds	r4, sp, r0, lsr #3
+0+10c <.*> e09d41c0 	adds	r4, sp, r0, asr #3
+0+110 <.*> e09d41e0 	adds	r4, sp, r0, ror #3
+0+114 <.*> e09d4060 	adds	r4, sp, r0, rrx
+0+118 <.*> e04d4000 	sub	r4, sp, r0
+0+11c <.*> e04d4080 	sub	r4, sp, r0, lsl #1
+0+120 <.*> e04d4100 	sub	r4, sp, r0, lsl #2
+0+124 <.*> e04d4180 	sub	r4, sp, r0, lsl #3
+0+128 <.*> e04d4200 	sub	r4, sp, r0, lsl #4
+0+12c <.*> e04d41a0 	sub	r4, sp, r0, lsr #3
+0+130 <.*> e04d41c0 	sub	r4, sp, r0, asr #3
+0+134 <.*> e04d41e0 	sub	r4, sp, r0, ror #3
+0+138 <.*> e04d4060 	sub	r4, sp, r0, rrx
+0+13c <.*> e05d4000 	subs	r4, sp, r0
+0+140 <.*> e05d4080 	subs	r4, sp, r0, lsl #1
+0+144 <.*> e05d4100 	subs	r4, sp, r0, lsl #2
+0+148 <.*> e05d4180 	subs	r4, sp, r0, lsl #3
+0+14c <.*> e05d4200 	subs	r4, sp, r0, lsl #4
+0+150 <.*> e05d41a0 	subs	r4, sp, r0, lsr #3
+0+154 <.*> e05d41c0 	subs	r4, sp, r0, asr #3
+0+158 <.*> e05d41e0 	subs	r4, sp, r0, ror #3
+0+15c <.*> e05d4060 	subs	r4, sp, r0, rrx
Index: src/gas/testsuite/gas/arm/add.s
===================================================================
RCS file: src/gas/testsuite/gas/arm/add.s
diff -N src/gas/testsuite/gas/arm/add.s
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ src/gas/testsuite/gas/arm/add.s	28 Feb 2011 18:35:38 -0000
@@ -0,0 +1,95 @@
+@ test case of ADD{S} and SUB{S} instructions in ARM mode
+
+	.text
+	.align	2
+	add sp, sp, r0, LSL #4
+	add sp, sp, r0, LSR #3
+	add sp, sp, r0, ASR #3
+	add sp, sp, r0, ROR #3
+	add sp, sp, r0, RRX
+	adds sp, sp, r0, LSL #4
+	adds sp, sp, r0, LSR #3
+	adds sp, sp, r0, ASR #3
+	adds sp, sp, r0, ROR #3
+	adds sp, sp, r0, RRX
+	sub sp, sp, r0, LSL #4
+	sub sp, sp, r0, LSR #3
+	sub sp, sp, r0, ASR #3
+	sub sp, sp, r0, ROR #3
+	sub sp, sp, r0, RRX
+	subs sp, sp, r0, LSL #4
+	subs sp, sp, r0, LSR #3
+	subs sp, sp, r0, ASR #3
+	subs sp, sp, r0, ROR #3
+	subs sp, sp, r0, RRX
+
+	add sp, sp, r0
+	add sp, sp, r0, LSL #1
+	add sp, sp, r0, LSL #2
+	add sp, sp, r0, LSL #3
+	adds sp, sp, r0
+	adds sp, sp, r0, LSL #1
+	adds sp, sp, r0, LSL #2
+	adds sp, sp, r0, LSL #3
+	sub sp, sp, r0
+	sub sp, sp, r0, LSL #1
+	sub sp, sp, r0, LSL #2
+	sub sp, sp, r0, LSL #3
+	subs sp, sp, r0
+	subs sp, sp, r0, LSL #1
+	subs sp, sp, r0, LSL #2
+	subs sp, sp, r0, LSL #3
+
+	add sp, sp, r4
+	add sp, sp, r4, LSL #1
+	add sp, sp, r4, LSL #2
+	add sp, sp, r4, LSL #3
+	adds sp, sp, r4
+	adds sp, sp, r4, LSL #1
+	adds sp, sp, r4, LSL #2
+	adds sp, sp, r4, LSL #3
+	sub sp, sp, r4
+	sub sp, sp, r4, LSL #1
+	sub sp, sp, r4, LSL #2
+	sub sp, sp, r4, LSL #3
+	subs sp, sp, r4
+	subs sp, sp, r4, LSL #1
+	subs sp, sp, r4, LSL #2
+	subs sp, sp, r4, LSL #3
+
+	add r4, sp, r0
+	add r4, sp, r0, LSL #1
+	add r4, sp, r0, LSL #2
+	add r4, sp, r0, LSL #3
+	add r4, sp, r0, LSL #4
+	add r4, sp, r0, LSR #3
+	add r4, sp, r0, ASR #3
+	add r4, sp, r0, ROR #3
+	add r4, sp, r0, RRX
+	adds r4, sp, r0
+	adds r4, sp, r0, LSL #1
+	adds r4, sp, r0, LSL #2
+	adds r4, sp, r0, LSL #3
+	adds r4, sp, r0, LSL #4
+	adds r4, sp, r0, LSR #3
+	adds r4, sp, r0, ASR #3
+	adds r4, sp, r0, ROR #3
+	adds r4, sp, r0, RRX
+	sub r4, sp, r0
+	sub r4, sp, r0, LSL #1
+	sub r4, sp, r0, LSL #2
+	sub r4, sp, r0, LSL #3
+	sub r4, sp, r0, LSL #4
+	sub r4, sp, r0, LSR #3
+	sub r4, sp, r0, ASR #3
+	sub r4, sp, r0, ROR #3
+	sub r4, sp, r0, RRX
+	subs r4, sp, r0
+	subs r4, sp, r0, LSL #1
+	subs r4, sp, r0, LSL #2
+	subs r4, sp, r0, LSL #3
+	subs r4, sp, r0, LSL #4
+	subs r4, sp, r0, LSR #3
+	subs r4, sp, r0, ASR #3
+	subs r4, sp, r0, ROR #3
+	subs r4, sp, r0, RRX
Index: src/gas/testsuite/gas/arm/addthumb2.d
===================================================================
RCS file: src/gas/testsuite/gas/arm/addthumb2.d
diff -N src/gas/testsuite/gas/arm/addthumb2.d
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ src/gas/testsuite/gas/arm/addthumb2.d	28 Feb 2011 18:35:38 -0000
@@ -0,0 +1,77 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: Thumb2 Add{S} and Sub{S} instructions
+#as: -march=armv7-a
+
+# Test some Thumb2 instructions:
+
+.*: +file format .*arm.*
+
+Disassembly of section .text:
+0+000 <.*> 4485      	add	sp, r0
+0+002 <.*> eb0d 0d40 	add.w	sp, sp, r0, lsl #1
+0+006 <.*> eb0d 0d80 	add.w	sp, sp, r0, lsl #2
+0+00a <.*> eb0d 0dc0 	add.w	sp, sp, r0, lsl #3
+0+00e <.*> eb1d 0d00 	adds.w	sp, sp, r0
+0+012 <.*> eb1d 0d40 	adds.w	sp, sp, r0, lsl #1
+0+016 <.*> eb1d 0d80 	adds.w	sp, sp, r0, lsl #2
+0+01a <.*> eb1d 0dc0 	adds.w	sp, sp, r0, lsl #3
+0+01e <.*> ebad 0d00 	sub.w	sp, sp, r0
+0+022 <.*> ebad 0d40 	sub.w	sp, sp, r0, lsl #1
+0+026 <.*> ebad 0d80 	sub.w	sp, sp, r0, lsl #2
+0+02a <.*> ebad 0dc0 	sub.w	sp, sp, r0, lsl #3
+0+02e <.*> ebbd 0d00 	subs.w	sp, sp, r0
+0+032 <.*> ebbd 0d40 	subs.w	sp, sp, r0, lsl #1
+0+036 <.*> ebbd 0d80 	subs.w	sp, sp, r0, lsl #2
+0+03a <.*> ebbd 0dc0 	subs.w	sp, sp, r0, lsl #3
+0+03e <.*> 44a5      	add	sp, r4
+0+040 <.*> eb0d 0d44 	add.w	sp, sp, r4, lsl #1
+0+044 <.*> eb0d 0d84 	add.w	sp, sp, r4, lsl #2
+0+048 <.*> eb0d 0dc4 	add.w	sp, sp, r4, lsl #3
+0+04c <.*> eb1d 0d04 	adds.w	sp, sp, r4
+0+050 <.*> eb1d 0d44 	adds.w	sp, sp, r4, lsl #1
+0+054 <.*> eb1d 0d84 	adds.w	sp, sp, r4, lsl #2
+0+058 <.*> eb1d 0dc4 	adds.w	sp, sp, r4, lsl #3
+0+05c <.*> ebad 0d04 	sub.w	sp, sp, r4
+0+060 <.*> ebad 0d44 	sub.w	sp, sp, r4, lsl #1
+0+064 <.*> ebad 0d84 	sub.w	sp, sp, r4, lsl #2
+0+068 <.*> ebad 0dc4 	sub.w	sp, sp, r4, lsl #3
+0+06c <.*> ebbd 0d04 	subs.w	sp, sp, r4
+0+070 <.*> ebbd 0d44 	subs.w	sp, sp, r4, lsl #1
+0+074 <.*> ebbd 0d84 	subs.w	sp, sp, r4, lsl #2
+0+078 <.*> ebbd 0dc4 	subs.w	sp, sp, r4, lsl #3
+0+07c <.*> eb0d 0400 	add.w	r4, sp, r0
+0+080 <.*> eb0d 0440 	add.w	r4, sp, r0, lsl #1
+0+084 <.*> eb0d 0480 	add.w	r4, sp, r0, lsl #2
+0+088 <.*> eb0d 04c0 	add.w	r4, sp, r0, lsl #3
+0+08c <.*> eb0d 1400 	add.w	r4, sp, r0, lsl #4
+0+090 <.*> eb0d 04d0 	add.w	r4, sp, r0, lsr #3
+0+094 <.*> eb0d 04e0 	add.w	r4, sp, r0, asr #3
+0+098 <.*> eb0d 04f0 	add.w	r4, sp, r0, ror #3
+0+09c <.*> eb0d 0430 	add.w	r4, sp, r0, rrx
+0+0a0 <.*> eb1d 0400 	adds.w	r4, sp, r0
+0+0a4 <.*> eb1d 0440 	adds.w	r4, sp, r0, lsl #1
+0+0a8 <.*> eb1d 0480 	adds.w	r4, sp, r0, lsl #2
+0+0ac <.*> eb1d 04c0 	adds.w	r4, sp, r0, lsl #3
+0+0b0 <.*> eb1d 1400 	adds.w	r4, sp, r0, lsl #4
+0+0b4 <.*> eb1d 04d0 	adds.w	r4, sp, r0, lsr #3
+0+0b8 <.*> eb1d 04e0 	adds.w	r4, sp, r0, asr #3
+0+0bc <.*> eb1d 04f0 	adds.w	r4, sp, r0, ror #3
+0+0c0 <.*> eb1d 0430 	adds.w	r4, sp, r0, rrx
+0+0c4 <.*> ebad 0400 	sub.w	r4, sp, r0
+0+0c8 <.*> ebad 0440 	sub.w	r4, sp, r0, lsl #1
+0+0cc <.*> ebad 0480 	sub.w	r4, sp, r0, lsl #2
+0+0d0 <.*> ebad 04c0 	sub.w	r4, sp, r0, lsl #3
+0+0d4 <.*> ebad 1400 	sub.w	r4, sp, r0, lsl #4
+0+0d8 <.*> ebad 04d0 	sub.w	r4, sp, r0, lsr #3
+0+0dc <.*> ebad 04e0 	sub.w	r4, sp, r0, asr #3
+0+0e0 <.*> ebad 04f0 	sub.w	r4, sp, r0, ror #3
+0+0e4 <.*> ebad 0430 	sub.w	r4, sp, r0, rrx
+0+0e8 <.*> ebbd 0400 	subs.w	r4, sp, r0
+0+0ec <.*> ebbd 0440 	subs.w	r4, sp, r0, lsl #1
+0+0f0 <.*> ebbd 0480 	subs.w	r4, sp, r0, lsl #2
+0+0f4 <.*> ebbd 04c0 	subs.w	r4, sp, r0, lsl #3
+0+0f8 <.*> ebbd 1400 	subs.w	r4, sp, r0, lsl #4
+0+0fc <.*> ebbd 04d0 	subs.w	r4, sp, r0, lsr #3
+0+100 <.*> ebbd 04e0 	subs.w	r4, sp, r0, asr #3
+0+104 <.*> ebbd 04f0 	subs.w	r4, sp, r0, ror #3
+0+108 <.*> ebbd 0430 	subs.w	r4, sp, r0, rrx
Index: src/gas/testsuite/gas/arm/addthumb2.s
===================================================================
RCS file: src/gas/testsuite/gas/arm/addthumb2.s
diff -N src/gas/testsuite/gas/arm/addthumb2.s
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ src/gas/testsuite/gas/arm/addthumb2.s	28 Feb 2011 18:35:38 -0000
@@ -0,0 +1,76 @@
+	.syntax unified
+	.file	"s6163c.c"
+	.text
+	.align	2
+	.thumb
+
+	add sp, sp, r0
+	add sp, sp, r0, LSL #1
+	add sp, sp, r0, LSL #2
+	add sp, sp, r0, LSL #3
+	adds sp, sp, r0
+	adds sp, sp, r0, LSL #1
+	adds sp, sp, r0, LSL #2
+	adds sp, sp, r0, LSL #3
+	sub sp, sp, r0
+	sub sp, sp, r0, LSL #1
+	sub sp, sp, r0, LSL #2
+	sub sp, sp, r0, LSL #3
+	subs sp, sp, r0
+	subs sp, sp, r0, LSL #1
+	subs sp, sp, r0, LSL #2
+	subs sp, sp, r0, LSL #3
+
+	add sp, sp, r4
+	add sp, sp, r4, LSL #1
+	add sp, sp, r4, LSL #2
+	add sp, sp, r4, LSL #3
+	adds sp, sp, r4
+	adds sp, sp, r4, LSL #1
+	adds sp, sp, r4, LSL #2
+	adds sp, sp, r4, LSL #3
+	sub sp, sp, r4
+	sub sp, sp, r4, LSL #1
+	sub sp, sp, r4, LSL #2
+	sub sp, sp, r4, LSL #3
+	subs sp, sp, r4
+	subs sp, sp, r4, LSL #1
+	subs sp, sp, r4, LSL #2
+	subs sp, sp, r4, LSL #3
+
+	add r4, sp, r0
+	add r4, sp, r0, LSL #1
+	add r4, sp, r0, LSL #2
+	add r4, sp, r0, LSL #3
+	add r4, sp, r0, LSL #4
+	add r4, sp, r0, LSR #3
+	add r4, sp, r0, ASR #3
+	add r4, sp, r0, ROR #3
+	add r4, sp, r0, RRX
+	adds r4, sp, r0
+	adds r4, sp, r0, LSL #1
+	adds r4, sp, r0, LSL #2
+	adds r4, sp, r0, LSL #3
+	adds r4, sp, r0, LSL #4
+	adds r4, sp, r0, LSR #3
+	adds r4, sp, r0, ASR #3
+	adds r4, sp, r0, ROR #3
+	adds r4, sp, r0, RRX
+	sub r4, sp, r0
+	sub r4, sp, r0, LSL #1
+	sub r4, sp, r0, LSL #2
+	sub r4, sp, r0, LSL #3
+	sub r4, sp, r0, LSL #4
+	sub r4, sp, r0, LSR #3
+	sub r4, sp, r0, ASR #3
+	sub r4, sp, r0, ROR #3
+	sub r4, sp, r0, RRX
+	subs r4, sp, r0
+	subs r4, sp, r0, LSL #1
+	subs r4, sp, r0, LSL #2
+	subs r4, sp, r0, LSL #3
+	subs r4, sp, r0, LSL #4
+	subs r4, sp, r0, LSR #3
+	subs r4, sp, r0, ASR #3
+	subs r4, sp, r0, ROR #3
+	subs r4, sp, r0, RRX
Index: src/gas/testsuite/gas/arm/addthumb2err.d
===================================================================
RCS file: src/gas/testsuite/gas/arm/addthumb2err.d
diff -N src/gas/testsuite/gas/arm/addthumb2err.d
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ src/gas/testsuite/gas/arm/addthumb2err.d	28 Feb 2011 18:35:38 -0000
@@ -0,0 +1,7 @@
+#name: bad Thumb2 Add{S} and Sub{S} instructions
+#as: -march=armv7-a
+#error-output: addthumb2err.l
+
+# Test some Thumb2 instructions:
+
+.*: +file format .*arm.*
Index: src/gas/testsuite/gas/arm/addthumb2err.l
===================================================================
RCS file: src/gas/testsuite/gas/arm/addthumb2err.l
diff -N src/gas/testsuite/gas/arm/addthumb2err.l
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ src/gas/testsuite/gas/arm/addthumb2err.l	28 Feb 2011 18:35:38 -0000
@@ -0,0 +1,21 @@
+[^:]*: Assembler messages:
+[^:]*:8: Error: shift value over 3 not allowed in thumb mode -- `add sp,sp,r0,LSL#4'
+[^:]*:9: Error: only LSL shift allowed in thumb mode -- `add sp,sp,r0,LSR#3'
+[^:]*:10: Error: only LSL shift allowed in thumb mode -- `add sp,sp,r0,ASR#3'
+[^:]*:11: Error: only LSL shift allowed in thumb mode -- `add sp,sp,r0,ROR#3'
+[^:]*:12: Error: only LSL shift allowed in thumb mode -- `add sp,sp,r0,RRX'
+[^:]*:13: Error: shift value over 3 not allowed in thumb mode -- `adds sp,sp,r0,LSL#4'
+[^:]*:14: Error: only LSL shift allowed in thumb mode -- `adds sp,sp,r0,LSR#3'
+[^:]*:15: Error: only LSL shift allowed in thumb mode -- `adds sp,sp,r0,ASR#3'
+[^:]*:16: Error: only LSL shift allowed in thumb mode -- `adds sp,sp,r0,ROR#3'
+[^:]*:17: Error: only LSL shift allowed in thumb mode -- `adds sp,sp,r0,RRX'
+[^:]*:18: Error: shift value over 3 not allowed in thumb mode -- `sub sp,sp,r0,LSL#4'
+[^:]*:19: Error: only LSL shift allowed in thumb mode -- `sub sp,sp,r0,LSR#3'
+[^:]*:20: Error: only LSL shift allowed in thumb mode -- `sub sp,sp,r0,ASR#3'
+[^:]*:21: Error: only LSL shift allowed in thumb mode -- `sub sp,sp,r0,ROR#3'
+[^:]*:22: Error: only LSL shift allowed in thumb mode -- `sub sp,sp,r0,RRX'
+[^:]*:23: Error: shift value over 3 not allowed in thumb mode -- `subs sp,sp,r0,LSL#4'
+[^:]*:24: Error: only LSL shift allowed in thumb mode -- `subs sp,sp,r0,LSR#3'
+[^:]*:25: Error: only LSL shift allowed in thumb mode -- `subs sp,sp,r0,ASR#3'
+[^:]*:26: Error: only LSL shift allowed in thumb mode -- `subs sp,sp,r0,ROR#3'
+[^:]*:27: Error: only LSL shift allowed in thumb mode -- `subs sp,sp,r0,RRX'
Index: src/gas/testsuite/gas/arm/addthumb2err.s
===================================================================
RCS file: src/gas/testsuite/gas/arm/addthumb2err.s
diff -N src/gas/testsuite/gas/arm/addthumb2err.s
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ src/gas/testsuite/gas/arm/addthumb2err.s	28 Feb 2011 18:35:38 -0000
@@ -0,0 +1,27 @@
+	.syntax unified
+	.file	"s6163c.c"
+	.text
+	.align	2
+	.thumb
+
+	# Bad instructions
+	add sp, sp, r0, LSL #4
+	add sp, sp, r0, LSR #3
+	add sp, sp, r0, ASR #3
+	add sp, sp, r0, ROR #3
+	add sp, sp, r0, RRX
+	adds sp, sp, r0, LSL #4
+	adds sp, sp, r0, LSR #3
+	adds sp, sp, r0, ASR #3
+	adds sp, sp, r0, ROR #3
+	adds sp, sp, r0, RRX
+	sub sp, sp, r0, LSL #4
+	sub sp, sp, r0, LSR #3
+	sub sp, sp, r0, ASR #3
+	sub sp, sp, r0, ROR #3
+	sub sp, sp, r0, RRX
+	subs sp, sp, r0, LSL #4
+	subs sp, sp, r0, LSR #3
+	subs sp, sp, r0, ASR #3
+	subs sp, sp, r0, ROR #3
+	subs sp, sp, r0, RRX

Re: [PATCH] ARM: gas not detecting invalid operands for Thumb2 ADD{S} and SUB{S}

[Paul Brook]

> In ARMv6T2 and ARMv7 Thumb2, the ADD, ADDS, SUB, and SUBS instructions
> added several new instruction forms. One of the new forms allowed is:
> 
>     ADD{S}<c>.W <Rd>,SP,<Rm>{,<shift>}
>     SUB{S}<c>.W <Rd>,SP,<Rm>{,<shift>}

The "added new forms" is mainly an artifact of how the new documentation is 
structured.  If you're starting from the ARM instruction set (or from the 
encodings) then it's actually new restrictions on where r13 (aka SP) may be 
used.

The patch looks ok, though I think the testcases could use some 
reorganisation.  I'd prefer two assembly files, one with insns valid in both 
ARM and Thumb mode, the other which is only valid in ARM mode.  Assemble both 
in both modes.  There should already be existing tests
(e.g. sp-pc-usage-t) that cover the former.

> +@ test case of ADD{S} and SUB{S} instructions in ARM mode

Too vague.  You're testing use of SP in these insns.

> +	.file	"s6163c.c"

Looks bogus.

Paul

Re: [PATCH] ARM: gas not detecting invalid operands for Thumb2 ADD{S} and SUB{S}

[Paul Carroll]

On 3/11/2011 6:26 AM, Paul Brook wrote:
> The patch looks ok, though I think the testcases could use some
> reorganisation.  I'd prefer two assembly files, one with insns valid in both
> ARM and Thumb mode, the other which is only valid in ARM mode.  Assemble both
> in both modes.  There should already be existing tests
> (e.g. sp-pc-usage-t) that cover the former.

Maybe the best thing to do is just remove the 2 positive test cases and 
keep only the 1 negative test case.
I'll just assume the test suite does enough validation with existing tests.

>> +@ test case of ADD{S} and SUB{S} instructions in ARM mode
> Too vague.  You're testing use of SP in these insns.

For the negative test case, I can adjust the comment to this:
     # Test of invalid operands for ADD{S} and SUB{S} instructions
     # in Thumb2 mode.  The instruction form being testing
     # involves having the first 2 operands be SP.

>> +	.file	"s6163c.c"
> Looks bogus.

That can easily be removed from the negative test case.

Re: [PATCH] ARM: gas not detecting invalid operands for Thumb2 ADD{S} and SUB{S}

[Nick Clifton]

Hi Paul,

> This is a resubmission of a patch I originally posted February 28th, 2011.
> Here is a link to that original patch:

Approved and applied.  Note - you did not include any changelog entries 
with your patch, so I created some for you.

Cheers
   Nick

Re: [PATCH] ARM: gas not detecting invalid operands for Thumb2 ADD{S} and SUB{S}

[Paul Carroll]

[-- Attachment #1: Type: text/plain, Size: 1757 bytes --]

(I don't have write permission into the Binutils CVS, so someone else 
will be merging the final patch.)

This is a resubmission of a patch I originally posted February 28th, 2011.
Here is a link to that original patch: 
http://sourceware.org/ml/binutils/2011-02/msg00418.html
That was followed by a response from Paul Brook on March 11th, 2011:
http://sourceware.org/ml/binutils/2011-03/msg00212.html
This revised patch should address his concerns.

In ARMv6T2 and ARMv7 Thumb2, the ADD, ADDS, SUB, and SUBS instructions 
added several new restrictions for when the first 2 registers are SP. 
These forms are:

    ADD{S}<c>.W<Rd>,SP,<Rm>{,<shift>}
    SUB{S}<c>.W<Rd>,SP,<Rm>{,<shift>}


According to DDI-0406B page A8-30

d = UInt(Rd); m = UInt(Rm); setflags = (S == '1');
(shift_t, shift_n) = DecodeImmShift(type, imm3:imm2);
if d == 13&& (shift_t != SRType_LSL || shift_n> 3) then UNPREDICTABLE;
if d == 15 || BadReg(m) then UNPREDICTABLE;

The tc-arm.c file in the gas/config directory was already detecting the 
'd==15' condition. But, there was no validation of the shift type or 
shift value when the first register specified was SP.
This patch adds that check.

This latest patch is a little different from the previous patch.  Since 
I have no evidence that these added checks will be useful to other 
instructions, I moved the constraints into the function that is specific 
to these 4 instructions.  That means I didn't need to add an argument to 
encode_thumb32_shifted_operand() nor need to add dummy arguments to a 
lot of calls to that function.

Since we can presume there are sufficient test cases for the valid uses 
of these instructions, I dropped back to just a single test case that 
shows error conditions for the above conditions.

[-- Attachment #2: ARM_ADD.patch --]
[-- Type: text/plain, Size: 4585 bytes --]

Index: gas/config/tc-arm.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-arm.c,v
retrieving revision 1.478
diff -u -p -r1.478 tc-arm.c
--- gas/config/tc-arm.c	14 Mar 2011 16:04:12 -0000	1.478
+++ gas/config/tc-arm.c	11 Apr 2011 18:21:22 -0000
@@ -9257,6 +9257,9 @@ do_t_add_sub (void)
 	}
       else
 	{
+	  unsigned int value = inst.reloc.exp.X_add_number;
+	  unsigned int shift = inst.operands[2].shift_kind;
+
 	  Rn = inst.operands[2].reg;
 	  /* See if we can do this with a 16-bit instruction.  */
 	  if (!inst.operands[2].shifted && inst.size_req != 4)
@@ -9307,6 +9310,10 @@ do_t_add_sub (void)
 	  inst.instruction = THUMB_OP32 (inst.instruction);
 	  inst.instruction |= Rd << 8;
 	  inst.instruction |= Rs << 16;
+	  constraint (Rd == REG_SP && Rs == REG_SP && value > 3,
+		      _("shift value over 3 not allowed in thumb mode"));
+	  constraint (Rd == REG_SP && Rs == REG_SP && shift != SHIFT_LSL,
+		      _("only LSL shift allowed in thumb mode"));
 	  encode_thumb32_shifted_operand (2);
 	}
     }
Index: gas/testsuite/gas/arm/addthumb2err.d
===================================================================
RCS file: gas/testsuite/gas/arm/addthumb2err.d
diff -N gas/testsuite/gas/arm/addthumb2err.d
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/arm/addthumb2err.d	11 Apr 2011 18:21:22 -0000
@@ -0,0 +1,7 @@
+#name: bad Thumb2 Add{S} and Sub{S} instructions
+#as: -march=armv7-a
+#error-output: addthumb2err.l
+
+# Test some Thumb2 instructions:
+
+.*: +file format .*arm.*
Index: gas/testsuite/gas/arm/addthumb2err.l
===================================================================
RCS file: gas/testsuite/gas/arm/addthumb2err.l
diff -N gas/testsuite/gas/arm/addthumb2err.l
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/arm/addthumb2err.l	11 Apr 2011 18:21:22 -0000
@@ -0,0 +1,21 @@
+[^:]*: Assembler messages:
+[^:]*:9: Error: shift value over 3 not allowed in thumb mode -- `add sp,sp,r0,LSL#4'
+[^:]*:10: Error: only LSL shift allowed in thumb mode -- `add sp,sp,r0,LSR#3'
+[^:]*:11: Error: only LSL shift allowed in thumb mode -- `add sp,sp,r0,ASR#3'
+[^:]*:12: Error: only LSL shift allowed in thumb mode -- `add sp,sp,r0,ROR#3'
+[^:]*:13: Error: only LSL shift allowed in thumb mode -- `add sp,sp,r0,RRX'
+[^:]*:14: Error: shift value over 3 not allowed in thumb mode -- `adds sp,sp,r0,LSL#4'
+[^:]*:15: Error: only LSL shift allowed in thumb mode -- `adds sp,sp,r0,LSR#3'
+[^:]*:16: Error: only LSL shift allowed in thumb mode -- `adds sp,sp,r0,ASR#3'
+[^:]*:17: Error: only LSL shift allowed in thumb mode -- `adds sp,sp,r0,ROR#3'
+[^:]*:18: Error: only LSL shift allowed in thumb mode -- `adds sp,sp,r0,RRX'
+[^:]*:19: Error: shift value over 3 not allowed in thumb mode -- `sub sp,sp,r0,LSL#4'
+[^:]*:20: Error: only LSL shift allowed in thumb mode -- `sub sp,sp,r0,LSR#3'
+[^:]*:21: Error: only LSL shift allowed in thumb mode -- `sub sp,sp,r0,ASR#3'
+[^:]*:22: Error: only LSL shift allowed in thumb mode -- `sub sp,sp,r0,ROR#3'
+[^:]*:23: Error: only LSL shift allowed in thumb mode -- `sub sp,sp,r0,RRX'
+[^:]*:24: Error: shift value over 3 not allowed in thumb mode -- `subs sp,sp,r0,LSL#4'
+[^:]*:25: Error: only LSL shift allowed in thumb mode -- `subs sp,sp,r0,LSR#3'
+[^:]*:26: Error: only LSL shift allowed in thumb mode -- `subs sp,sp,r0,ASR#3'
+[^:]*:27: Error: only LSL shift allowed in thumb mode -- `subs sp,sp,r0,ROR#3'
+[^:]*:28: Error: only LSL shift allowed in thumb mode -- `subs sp,sp,r0,RRX'
Index: gas/testsuite/gas/arm/addthumb2err.s
===================================================================
RCS file: gas/testsuite/gas/arm/addthumb2err.s
diff -N gas/testsuite/gas/arm/addthumb2err.s
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/arm/addthumb2err.s	11 Apr 2011 18:21:22 -0000
@@ -0,0 +1,28 @@
+	.syntax unified
+	.text
+	.align	2
+	.thumb
+
+	# Test of invalid operands for ADD{S} and SUB{S} instructions
+	# in Thumb2 mode.  The instruction form being testing
+	# involves having the first 2 operands be SP.
+	add sp, sp, r0, LSL #4
+	add sp, sp, r0, LSR #3
+	add sp, sp, r0, ASR #3
+	add sp, sp, r0, ROR #3
+	add sp, sp, r0, RRX
+	adds sp, sp, r0, LSL #4
+	adds sp, sp, r0, LSR #3
+	adds sp, sp, r0, ASR #3
+	adds sp, sp, r0, ROR #3
+	adds sp, sp, r0, RRX
+	sub sp, sp, r0, LSL #4
+	sub sp, sp, r0, LSR #3
+	sub sp, sp, r0, ASR #3
+	sub sp, sp, r0, ROR #3
+	sub sp, sp, r0, RRX
+	subs sp, sp, r0, LSL #4
+	subs sp, sp, r0, LSR #3
+	subs sp, sp, r0, ASR #3
+	subs sp, sp, r0, ROR #3
+	subs sp, sp, r0, RRX