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

[Paul Carroll <pcarroll at codesourcery dot com>]

(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: ARM_ADD.patch
Description: Text document