This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]