Fix imm20 range check in MSP430 port of gas

7 files changed, 45 insertions, 11 deletions

diff --git a/gas/config/tc-msp430.c b/gas/config/tc-msp430.c
index 27b1d83..7ce061b 100644
--- a/gas/config/tc-msp430.c
+++ b/gas/config/tc-msp430.c
@@ -3584,7 +3584,13 @@ msp430_operands (struct msp430_opcode_s * opcode, char * line)
 		if (op1.exp.X_op == O_constant)
 		  {
 		    n = op1.exp.X_add_number;
-		    if (n > 0xfffff || n < - (0x7ffff))
+		    /* Strictly speaking the positive value test should be for "n > 0x7ffff"
+		       but traditionally when specifying immediates as hex values any valid
+		       bit pattern is allowed.  Hence "suba #0xfffff, r6" is allowed, and so
+		       the positive value test has to be "n > 0xfffff".
+		       FIXME: We could pre-parse the expression to find out if it starts with
+		       0x and only then allow positive values > 0x7fffff.  */
+		    if (n > 0xfffff || n < -0x80000)
 		      {
 			as_bad (_("expected value of first argument of %s to fit into 20-bits"),
 				opcode->name);
diff --git a/gas/testsuite/gas/msp430/bad.l b/gas/testsuite/gas/msp430/bad.l
index 7b68583..3ea1b7b 100644
--- a/gas/testsuite/gas/msp430/bad.l
+++ b/gas/testsuite/gas/msp430/bad.l
@@ -6,13 +6,16 @@
 [^:]*:10: Warning: no size modifier after period, .w assumed
 [^:]*:10: Warning: a NOP might be needed here, before this interrupt state change
 [^:]*:11: Error: instruction bis.a does not exist
-[^:]*:16: Warning: a NOP might also be needed here, after the instruction that changed interrupt state
-[^:]*:16: Warning: a NOP might be needed here, before an interrupt enable instruction
-[^:]*:25: Warning: a NOP might be needed here, after an interrupt disable instruction
-[^:]*:26: Warning: a NOP might be needed here, after an interrupt enable instruction
-[^:]*:29: Warning: a NOP might be needed here, after an interrupt disable instruction
-[^:]*:31: Warning: a NOP might be needed here, after an interrupt disable instruction
-[^:]*:32: Warning: a NOP might be needed here, after an interrupt disable instruction
-[^:]*:33: Warning: a NOP might be needed here, after an interrupt disable instruction
-[^:]*:34: Warning: a NOP might be needed here, after an interrupt enable instruction
+[^:]*:14: Warning: a NOP might also be needed here, after the instruction that changed interrupt state
+[^:]*:14: Error: expected value of first argument of adda to fit into 20-bits
+[^:]*:15: Error: expected value of first argument of adda to fit into 20-bits
+[^:]*:22: Error: expected value of first argument of adda to fit into 20-bits
+[^:]*:27: Warning: a NOP might be needed here, before an interrupt enable instruction
+[^:]*:36: Warning: a NOP might be needed here, after an interrupt disable instruction
+[^:]*:37: Warning: a NOP might be needed here, after an interrupt enable instruction
+[^:]*:40: Warning: a NOP might be needed here, after an interrupt disable instruction
+[^:]*:42: Warning: a NOP might be needed here, after an interrupt disable instruction
+[^:]*:43: Warning: a NOP might be needed here, after an interrupt disable instruction
+[^:]*:44: Warning: a NOP might be needed here, after an interrupt disable instruction
+[^:]*:45: Warning: a NOP might be needed here, after an interrupt enable instruction
 [^:]*: Warning: a NOP might be needed after the interrupt state change at the end of the file
diff --git a/gas/testsuite/gas/msp430/bad.s b/gas/testsuite/gas/msp430/bad.s
index ae2db2f..8649031 100644
--- a/gas/testsuite/gas/msp430/bad.s
+++ b/gas/testsuite/gas/msp430/bad.s
@@ -10,6 +10,17 @@
 	mov.	r1, r2
 	bis.a	#8, r2
 
+	;;  Make sure that the range checking gets #imm20 correct.
+	adda    #-524289, r12
+	adda	#0x180000, r10
+	;;  An immediate of #524288 will not trigger an error because
+	;;  positive values up to 1048575 are allowed.  This is because
+	;;  assembler programmers often specify bit patterns as immediate
+	;;  values in hex knowing that they will fit, despite the fact
+	;;  that those same values, when viewed as integers, are out of range.
+	;;  eg: adda 0xfffff, r1
+	adda	#1048576, r11
+	
 ;;; FIXME: Add more tests of assembler error detection here.
 
 	;;  A NOP is needed *before* an EINT instruction.
@@ -17,7 +28,7 @@
 	nop
 	;; And *after* a DINT instruction.
 	dint
-	
+
 	;;  Changing interrupt states in two successive instructions
 	;;  might cause an interrupt to be missed.  The assembler
 	;;  should warn about this, if the -mz command line option
@@ -35,3 +46,4 @@
 	;;  We will also get a warning if the last instruction in the file
 	;;  changes the interrupt state, since this file could be linked
 	;;  with another that starts with an interrupt change.
+
diff --git a/gas/testsuite/gas/msp430/msp430x.d b/gas/testsuite/gas/msp430/msp430x.d
index ecaef8f..fe4c634 100644
--- a/gas/testsuite/gas/msp430/msp430x.d
+++ b/gas/testsuite/gas/msp430/msp430x.d
@@ -230,3 +230,7 @@ Disassembly of section .text:
 0+0406 <[^>]*> 84 18 44 11 	rpt r4 \{ rrax.a	r4		;
 0+040a <[^>]*> 44 18 45 55 	rpt #5 \{ rlax.b	r5		;
 0+040e <[^>]*> 05 18 46 66 	rpt #6 \{ rlcx.a	r6		;
+0+0412 <[^>]*> ac 08 00 00 	adda	#524288,r12	;0x80000
+0+0416 <[^>]*> ab 07 ff ff 	adda	#524287,r11	;0x7ffff
+0+041a <[^>]*> ac 08 00 00 	adda	#524288,r12	;0x80000
+0+041e <[^>]*> ab 07 ff ff 	adda	#524287,r11	;0x7ffff
diff --git a/gas/testsuite/gas/msp430/msp430x.s b/gas/testsuite/gas/msp430/msp430x.s
index 8fef882..e6ae903 100644
--- a/gas/testsuite/gas/msp430/msp430x.s
+++ b/gas/testsuite/gas/msp430/msp430x.s
@@ -281,3 +281,9 @@ foo:
 	rpt r4 { rrax.a r4
 	rpt #5 { rlax.b r5
 	rpt #6 { rlcx.a r6
+
+	;;  Make sure that the range checking gets #imm20 correct.
+	adda    #-524288, r12
+	adda	#524287, r11
+	adda    #-0x80000, r12
+	adda	#0x7ffff, r11
diff --git a/gas/testsuite/gas/msp430/opcode.d b/gas/testsuite/gas/msp430/opcode.d
index 9212d89..04d1081 100644
--- a/gas/testsuite/gas/msp430/opcode.d
+++ b/gas/testsuite/gas/msp430/opcode.d
@@ -43,3 +43,4 @@ Disassembly of section .text:
 0+062 <[^>]*> 3f 40 f0 00 	mov	#240,	r15	;#0x00f0
 0+066 <[^>]*> 30 40 00 00 	br	#0x0000		;
 0+06a <[^>]*> 92 52 00 02 72 01 	add	&0x0200,&0x0172	;0x0200
+
diff --git a/gas/testsuite/gas/msp430/opcode.s b/gas/testsuite/gas/msp430/opcode.s
index 4924a60..fc29b8c 100644
--- a/gas/testsuite/gas/msp430/opcode.s
+++ b/gas/testsuite/gas/msp430/opcode.s
@@ -55,3 +55,5 @@ main:
 	;; This next instruction triggered a bug which
 	;; was fixed by a patch to msp430-dis.c on Jan 2, 2004
 	add	&0x200, &0x172
+
+