From: Paul Carroll <pcarroll@codesourcery.com>
To: binutils@sourceware.org
Subject: Re: [PATCH] ARM: gas not detecting invalid operands for Thumb2 ADD{S} and SUB{S}
Date: Mon, 11 Apr 2011 19:42:00 -0000 [thread overview]
Message-ID: <4DA359A5.3060508@codesourcery.com> (raw)
[-- 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
next reply other threads:[~2011-04-11 19:42 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-11 19:42 Paul Carroll [this message]
2011-06-30 13:44 ` Nick Clifton
-- strict thread matches above, loose matches on Subject: below --
2011-02-28 18:40 Paul Carroll
2011-03-11 13:26 ` Paul Brook
2011-03-11 18:01 ` Paul Carroll
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DA359A5.3060508@codesourcery.com \
--to=pcarroll@codesourcery.com \
--cc=binutils@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).