fix more incomplete cases, report arm instruction length properly

3 files changed, 140 insertions, 12 deletions

diff --git a/src/armv7.rs b/src/armv7.rs
index 91b65bd..73530fb 100644
--- a/src/armv7.rs
+++ b/src/armv7.rs
@@ -1313,8 +1313,14 @@ pub struct Instruction {
     pub condition: ConditionCode,
     pub opcode: Opcode,
     pub operands: [Operand; 4],
+    /// does this instruction update flags, while variants that do not update flags exist?
     pub s: bool,
+    /// is this a 32-bit thumb instruction?
+    pub wide: bool,
+    /// and if it is a 32-bit thumb instruction, should the .w suffix be shown?
     pub thumb_w: bool,
+    /// and generally speaking, was this just a thumb-encoded instruction?
+    pub thumb: bool,
 }
 
 #[derive(Debug, PartialEq)]
@@ -1363,6 +1369,8 @@ impl Default for Instruction {
             operands: [Operand::Nothing, Operand::Nothing, Operand::Nothing, Operand::Nothing],
             s: false,
             thumb_w: false,
+            wide: false,
+            thumb: false,
         }
     }
 }
@@ -1376,6 +1384,14 @@ impl Instruction {
         self.thumb_w = value;
     }
     pub fn w(&self) -> bool { self.thumb_w }
+    pub(crate) fn set_wide(&mut self, value: bool) {
+        self.wide = value;
+    }
+    pub fn wide(&self) -> bool { self.wide }
+    pub(crate) fn set_thumb(&mut self, value: bool) {
+        self.thumb = value;
+    }
+    pub fn thumb(&self) -> bool { self.thumb }
 }
 
 fn format_reg_list<T: fmt::Write, C: fmt::Display, Y: YaxColors<C>>(f: &mut T, mut list: u16, colors: &Y) -> Result<(), fmt::Error> {
@@ -1485,10 +1501,15 @@ impl Display for Instruction {
 impl LengthedInstruction for Instruction {
     type Unit = AddressDiff<<ARMv7 as Arch>::Address>;
     fn min_size() -> Self::Unit {
+        // TODO: this is contingent on the decoder mode...
         AddressDiff::from_const(4)
     }
     fn len(&self) -> Self::Unit {
-        AddressDiff::from_const(4)
+        if self.thumb && !self.wide {
+            AddressDiff::from_const(2)
+        } else {
+            AddressDiff::from_const(4)
+        }
     }
 }
 
@@ -1777,10 +1798,12 @@ impl Decoder<Instruction> for InstDecoder {
     type Error = DecodeError;
 
     fn decode_into<T: IntoIterator<Item=u8>>(&self, inst: &mut Instruction, bytes: T) -> Result<(), Self::Error> {
+        inst.set_w(false);
+        inst.set_wide(false);
         if self.thumb {
             return thumb::decode_into(&self, inst, bytes);
         } else {
-            inst.set_w(false);
+            inst.set_thumb(false);
         }
 
         fn read_word<T: IntoIterator<Item=u8>>(bytes: T) -> Result<u32, DecodeError> {
diff --git a/src/armv7/thumb.rs b/src/armv7/thumb.rs
index dc2b0e9..6028e7b 100644
--- a/src/armv7/thumb.rs
+++ b/src/armv7/thumb.rs
@@ -99,7 +99,12 @@ fn DecodeImmShift(reg: u8, ty: u8, imm5: u8) -> RegShift {
 
 #[allow(non_snake_case)]
 pub fn decode_into<T: IntoIterator<Item=u8>>(decoder: &InstDecoder, inst: &mut Instruction, bytes: T) -> Result<(), DecodeError> {
-    inst.set_w(false);
+    // these are cleared in `armv7::InstDecoder::decode_into`.
+    // they must be reset when switching out of thumb decoding or decoding a new thumb instruction,
+    // which that `decode_into` is the entrypoint for in all cases.
+    // inst.set_w(false);
+    // inst.set_wide(false);
+    inst.set_thumb(true);
     let mut iter = bytes.into_iter();
     let instr: u16 =
         ((iter.next().ok_or(DecodeError::ExhaustedInput)? as u16)      ) |
@@ -113,6 +118,7 @@ pub fn decode_into<T: IntoIterator<Item=u8>>(decoder: &InstDecoder, inst: &mut I
     // `A6.1 Thumb instruction set encoding`
     if opword >= 0b11101 {
         inst.set_w(true);
+        inst.set_wide(true);
 
         // 32b instruction - `A6-228, 32-bit Thumb instruction encoding`
         // opword low bits 01, 10, and 11 correspond to `op1` in table `A6-9`
@@ -709,7 +715,7 @@ pub fn decode_into<T: IntoIterator<Item=u8>>(decoder: &InstDecoder, inst: &mut I
                     let rm = lower2[0..4].load::<u16>();
 
                     let shift = RegShift::from_raw(
-                        0b00000 | // reg-imm shift. TODO: probably need to change the const
+                        0b00000 | // reg-imm shift
                         rm as u16 |
                         (imm2 << 7) | (imm3 << 9) |
                         tp << 5
@@ -760,31 +766,92 @@ pub fn decode_into<T: IntoIterator<Item=u8>>(decoder: &InstDecoder, inst: &mut I
                                 let tp = (lower >> 4) & 0b11;
                                 let imm2 = (lower >> 6) & 0b11;
                                 let imm3 = (lower >> 12) & 0b111;
+                                let imm5 = (imm3 << 2) | imm2;
                                 match tp {
                                     0b00 => {
-                                        if imm2 | imm3 == 0 {
+                                        if imm5 == 0 {
                                             // `MOV (register, Thumb)` (`A8-487`)
-                                            return Err(DecodeError::Incomplete);
+                                            // encoding T3
+                                            inst.set_w(true);
+                                            let rm = lower2[..4].load::<u8>();
+                                            let rd = lower2[8..12].load::<u8>();
+                                            inst.opcode = Opcode::MOV;
+                                            inst.operands = [
+                                                Operand::Reg(Reg::from_u8(rd)),
+                                                Operand::Reg(Reg::from_u8(rm)),
+                                                Operand::Nothing,
+                                                Operand::Nothing,
+                                            ];
                                         } else {
                                             // `LSL (immediate)` (`A8-469`)
-                                            return Err(DecodeError::Incomplete);
+                                            // encoding T2
+                                            inst.set_w(true);
+                                            let rm = lower2[..4].load::<u8>();
+                                            let rd = lower2[8..12].load::<u8>();
+                                            inst.opcode = Opcode::LSL;
+                                            inst.operands = [
+                                                Operand::Reg(Reg::from_u8(rd)),
+                                                Operand::Reg(Reg::from_u8(rm)),
+                                                Operand::Imm12(imm5),
+                                                Operand::Nothing,
+                                            ];
                                         }
                                     },
                                     0b01 => {
                                         // `LSR (immediate)` (`A8-473`)
-                                        return Err(DecodeError::Incomplete);
+                                        // encoding T2
+                                        inst.set_w(true);
+                                        let rm = lower2[..4].load::<u8>();
+                                        let rd = lower2[8..12].load::<u8>();
+                                        inst.opcode = Opcode::LSR;
+                                        inst.operands = [
+                                            Operand::Reg(Reg::from_u8(rd)),
+                                            Operand::Reg(Reg::from_u8(rm)),
+                                            Operand::Imm12(imm5),
+                                            Operand::Nothing,
+                                        ];
                                     }
                                     0b10 => {
                                         // `ASR (immediate)` (`A8-328`)
-                                        return Err(DecodeError::Incomplete);
+                                        // encoding T2
+                                        inst.set_w(true);
+                                        let rm = lower2[..4].load::<u8>();
+                                        let rd = lower2[8..12].load::<u8>();
+                                        inst.opcode = Opcode::ASR;
+                                        inst.operands = [
+                                            Operand::Reg(Reg::from_u8(rd)),
+                                            Operand::Reg(Reg::from_u8(rm)),
+                                            Operand::Imm12(imm5),
+                                            Operand::Nothing,
+                                        ];
                                     }
                                     0b11 => {
-                                        if imm2 | imm3 == 0 {
+                                        if imm5 == 0 {
                                             // `RRX` (`A8-573`)
-                                            return Err(DecodeError::Incomplete);
+                                            // encoding T1
+                                            inst.set_w(false);
+                                            let rm = lower2[..4].load::<u8>();
+                                            let rd = lower2[8..12].load::<u8>();
+                                            inst.opcode = Opcode::RRX;
+                                            inst.operands = [
+                                                Operand::Reg(Reg::from_u8(rd)),
+                                                Operand::Reg(Reg::from_u8(rm)),
+                                                Operand::Nothing,
+                                                Operand::Nothing,
+                                            ];
                                         } else {
                                             // `ROR (immediate)` (`A8-569`)
-                                            return Err(DecodeError::Incomplete);
+                                            // encoding T1
+                                            inst.set_w(false);
+                                            let rm = lower2[..4].load::<u8>();
+                                            let rd = lower2[8..12].load::<u8>();
+                                            inst.opcode = Opcode::ASR;
+                                            inst.operands = [
+                                                Operand::Reg(Reg::from_u8(rd)),
+                                                Operand::Reg(Reg::from_u8(rm)),
+                                                Operand::Imm12(imm5),
+                                                Operand::Nothing,
+                                            ];
                                         }
                                     }
                                     _ => {
diff --git a/test/armv7.rs b/test/armv7.rs
index c2f3300..6257f5b 100644
--- a/test/armv7.rs
+++ b/test/armv7.rs
@@ -110,6 +110,8 @@ fn test_decode_str_ldr() {
             ],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_decode(
@@ -125,6 +127,8 @@ fn test_decode_str_ldr() {
             ],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_decode(
@@ -140,6 +144,8 @@ fn test_decode_str_ldr() {
             ],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_decode(
@@ -155,6 +161,8 @@ fn test_decode_str_ldr() {
             ],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_decode(
@@ -170,6 +178,8 @@ fn test_decode_str_ldr() {
             ],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_decode(
@@ -185,6 +195,8 @@ fn test_decode_str_ldr() {
             ],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_all([0x10, 0x00, 0x7f, 0xe5], "ldrb r0, [pc, -0x10]!");
@@ -350,6 +362,8 @@ fn test_decode_pop() {
             ],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_display(
@@ -369,6 +383,8 @@ fn test_decode_pop() {
             ],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_display(
@@ -388,6 +404,8 @@ fn test_decode_pop() {
             ],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_display(
@@ -411,6 +429,8 @@ fn test_decode_mov() {
             ],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_display([0x0d, 0x20, 0xa0, 0xe1], "mov r2, sp");
@@ -428,6 +448,8 @@ fn test_decode_mov() {
             ],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
 }
@@ -442,6 +464,8 @@ fn test_decode_arithmetic() {
             operands: [Operand::Reg(Reg::from_u8(1)), Operand::Reg(Reg::from_u8(0)), Operand::RegShift(RegShift::from_raw(0xd18)), Operand::Nothing],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_display(
@@ -456,6 +480,8 @@ fn test_decode_arithmetic() {
             operands: [Operand::Reg(Reg::from_u8(3)), Operand::Reg(Reg::from_u8(15)), Operand::Reg(Reg::from_u8(3)), Operand::Nothing],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_decode(
@@ -466,6 +492,8 @@ fn test_decode_arithmetic() {
             operands: [Operand::Reg(Reg::from_u8(3)), Operand::Reg(Reg::from_u8(6)), Operand::Reg(Reg::from_u8(3)), Operand::Nothing],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_decode(
@@ -476,6 +504,8 @@ fn test_decode_arithmetic() {
             operands: [Operand::Reg(Reg::from_u8(3)), Operand::Reg(Reg::from_u8(0)), Operand::RegShift(RegShift::from_raw(0x143)), Operand::Nothing],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_decode(
@@ -486,6 +516,8 @@ fn test_decode_arithmetic() {
             operands: [Operand::Reg(Reg::from_u8(5)), Operand::Reg(Reg::from_u8(3)), Operand::Imm32(1), Operand::Nothing],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
 }
@@ -553,6 +585,8 @@ fn test_decode_mul() {
             ],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_decode(
@@ -568,6 +602,8 @@ fn test_decode_mul() {
             ],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
     test_decode(
@@ -583,6 +619,8 @@ fn test_decode_mul() {
             ],
             s: false,
             thumb_w: false,
+            thumb: false,
+            wide: false,
         }
     );
 }