fix various armv8 and armv8 panics that should be Err.

in fact the decoder should _never_ panic. included here are tests that
cover the entire 32-bit instruction space and ensure that decoding and
display do not panic. these tests run uncomfortably slowly (1168s to
decode the 4b "instruction" sequences on my desktop), but verify that
panics are no longer an issue.

8 files changed, 125 insertions, 35 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 14a97e7..33b9c48 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,7 @@
+## 0.1.2
+* fix some instructions in aarch64 decoding panicking with "unreachable"
+  instead of returning an `Err(DecodeError::Incomplete)`.
+
 ## 0.1.1
 * fix incorrect `yaxpeax_arch::Arch` impl for `std`-enabled builds
   (DecodeError did not implement `std::error::Error` in those build environments)
diff --git a/src/armv7.rs b/src/armv7.rs
index 5894bc4..e858566 100644
--- a/src/armv7.rs
+++ b/src/armv7.rs
@@ -1367,11 +1367,12 @@ pub enum StatusRegMask {
 }
 
 impl StatusRegMask {
-    fn from_raw(raw: u8) -> StatusRegMask {
+    fn from_raw(raw: u8) -> Result<StatusRegMask, DecodeError> {
         if raw == 0 {
-            panic!("invalid status reg mask value");
+            // invalid status reg mask value
+            return Err(DecodeError::InvalidOperand);
         }
-        [
+        Ok([
             StatusRegMask::CPSR_C, // actually unreachable
             StatusRegMask::CPSR_C,
             StatusRegMask::CPSR_X,
@@ -1404,7 +1405,7 @@ impl StatusRegMask {
             StatusRegMask::SPSR_FSC,
             StatusRegMask::SPSR_FSX,
             StatusRegMask::SPSR_FSXC,
-        ][raw as usize]
+        ][raw as usize])
     }
 }
 
@@ -2925,7 +2926,11 @@ impl Decoder<ARMv7> for InstDecoder {
                                                 }
                                             }
                                             inst.opcode = Opcode::MSR;
-                                            inst.operands[0] = Reg::from_sysm(R != 0, SYSm).expect("from_sysm works");
+                                            inst.operands[0] = if let Some(reg) = Reg::from_sysm(R != 0, SYSm) {
+                                                reg
+                                            } else {
+                                                return Err(DecodeError::InvalidOperand);
+                                            };
                                             inst.operands[1] = Operand::Reg(Reg::from_u8(word as u8 & 0b1111));
                                             inst.operands[2] = Operand::Nothing;
                                             inst.operands[3] = Operand::Nothing;
@@ -2937,7 +2942,11 @@ impl Decoder<ARMv7> for InstDecoder {
                                             }
                                             inst.opcode = Opcode::MRS;
                                             inst.operands[0] = Operand::Reg(Reg::from_u8((word >> 12) as u8 & 0b1111));
-                                            inst.operands[1] = Reg::from_sysm(R != 0, SYSm).expect("from_sysm works");
+                                            inst.operands[1] = if let Some(reg) = Reg::from_sysm(R != 0, SYSm) {
+                                                reg
+                                            } else {
+                                                return Err(DecodeError::InvalidOperand);
+                                            };
                                             inst.operands[2] = Operand::Nothing;
                                             inst.operands[3] = Operand::Nothing;
                                         }
@@ -2966,7 +2975,7 @@ impl Decoder<ARMv7> for InstDecoder {
                                                 let mask = (word >> 16) & 0b1111;
                                                 if mask & 0b11 == 0 {
                                                     if self.mode.is_user() {
-                                                        inst.operands[0] = Operand::StatusRegMask(StatusRegMask::from_raw(mask as u8));
+                                                        inst.operands[0] = Operand::StatusRegMask(StatusRegMask::from_raw(mask as u8)?);
                                                         inst.operands[1] = Operand::Reg(Reg::from_u8(word as u8 & 0b1111));
                                                         inst.operands[2] = Operand::Nothing;
                                                         inst.operands[3] = Operand::Nothing;
@@ -2979,7 +2988,7 @@ impl Decoder<ARMv7> for InstDecoder {
                                                     if self.mode.is_system() {
                                                         // bit 22 is the high bit of opcode, so..
                                                         let R = (word >> 22) as u8 & 1;
-                                                        inst.operands[0] = Operand::StatusRegMask(StatusRegMask::from_raw((R << 4) | mask as u8));
+                                                        inst.operands[0] = Operand::StatusRegMask(StatusRegMask::from_raw((R << 4) | mask as u8)?);
                                                         inst.operands[1] = Operand::Reg(Reg::from_u8(word as u8 & 0b1111));
                                                         inst.operands[2] = Operand::Nothing;
                                                         inst.operands[3] = Operand::Nothing;
@@ -2998,7 +3007,7 @@ impl Decoder<ARMv7> for InstDecoder {
                                                 let mask = (word >> 16) & 0b1111;
                                                 // bit 22 is the high bit of opcode, so..
                                                 let R = (word >> 22) & 1;
-                                                inst.operands[0] = Operand::StatusRegMask(StatusRegMask::from_raw((R << 4) as u8 | mask as u8));
+                                                inst.operands[0] = Operand::StatusRegMask(StatusRegMask::from_raw((R << 4) as u8 | mask as u8)?);
                                                 inst.operands[1] = Operand::Reg(Reg::from_u8(word as u8 & 0b1111));
                                                 inst.operands[2] = Operand::Nothing;
                                                 inst.operands[3] = Operand::Nothing;
@@ -3397,7 +3406,7 @@ impl Decoder<ARMv7> for InstDecoder {
                                         }
                                     }
                                     inst.operands = [
-                                        Operand::StatusRegMask(StatusRegMask::from_raw(op1 as u8)),
+                                        Operand::StatusRegMask(StatusRegMask::from_raw(op1 as u8)?),
                                         Operand::Imm32(word & 0xfff),
                                         Operand::Nothing,
                                         Operand::Nothing,
@@ -3414,7 +3423,7 @@ impl Decoder<ARMv7> for InstDecoder {
                                 }
                             }
                             inst.operands = [
-                                Operand::StatusRegMask(StatusRegMask::from_raw((word >> 16) as u8 & 0b1111 | 0b10000)),
+                                Operand::StatusRegMask(StatusRegMask::from_raw((word >> 16) as u8 & 0b1111 | 0b10000)?),
                                 Operand::Imm32(word & 0xfff),
                                 Operand::Nothing,
                                 Operand::Nothing,
diff --git a/src/armv7/thumb.rs b/src/armv7/thumb.rs
index 8017f01..e721691 100644
--- a/src/armv7/thumb.rs
+++ b/src/armv7/thumb.rs
@@ -2956,6 +2956,9 @@ pub fn decode_into<T: Reader<<ARMv7 as Arch>::Address, <ARMv7 as Arch>::Word>>(d
                                     return Err(DecodeError::InvalidOpcode);
                                 }
 
+                                if op1 == 0 {
+                                    return Err(DecodeError::InvalidOpcode);
+                                }
                                 let opcode_idx = (op1 - 1) * 3 + op2;
 
                                 let rn = instr2[0..4].load::<u8>();
diff --git a/src/armv8/a64.rs b/src/armv8/a64.rs
index 68f2337..4c9da27 100644
--- a/src/armv8/a64.rs
+++ b/src/armv8/a64.rs
@@ -7,6 +7,8 @@ use yaxpeax_arch::{Arch, AddressDiff, Decoder, LengthedInstruction, Reader, Read
 
 #[allow(non_snake_case)]
 mod docs {
+    use crate::armv8::a64::DecodeError;
+
     #[test]
     fn test_ones() {
         assert_eq!(Ones(0), 0x00);
@@ -77,9 +79,12 @@ mod docs {
     }
 
     // helper functions from the ARMv8 Architecture Reference Manual
-    pub fn DecodeBitMasks_32(immN: u8, imms: u8, immr: u8) -> (u32, u32) {
+    pub fn DecodeBitMasks_32(immN: u8, imms: u8, immr: u8) -> Result<(u32, u32), DecodeError> {
         // should the !imms be ~imms
         let len = HighestSetBit(7, ((immN << 6) | ((!imms) & 0x3f)) as u64);
+        if len == 0xff {
+            return Err(DecodeError::InvalidOperand);
+        }
 
         let levels = (Ones(len) & 0x3f) as u8; // should ZeroExtend to at least 6 bits, but this is u8.
 
@@ -93,12 +98,15 @@ mod docs {
         let telem = Ones(d + 1);
         let wmask = Replicate(ROR(welem, esize, R), esize, 32) as u32;
         let tmask = Replicate(telem, esize, 32) as u32;
-        (wmask, tmask)
+        Ok((wmask, tmask))
     }
 
-    pub fn DecodeBitMasks_64(immN: u8, imms: u8, immr: u8) -> (u64, u64) {
+    pub fn DecodeBitMasks_64(immN: u8, imms: u8, immr: u8) -> Result<(u64, u64), DecodeError> {
         // should the !imms be ~imms
         let len = HighestSetBit(7, ((immN << 6) | ((!imms) & 0x3f)) as u64);
+        if len == 0xff {
+            return Err(DecodeError::InvalidOperand);
+        }
 
         let levels = (Ones(len) & 0x3f) as u8; // should ZeroExtend to at least 6 bits, but this is u8.
 
@@ -112,7 +120,7 @@ mod docs {
         let telem = Ones(d + 1);
         let wmask = Replicate(ROR(welem, esize, R), esize, 64);
         let tmask = Replicate(telem, esize, 64);
-        (wmask, tmask)
+        Ok((wmask, tmask))
     }
 
     pub fn DecodeShift(op: u8) -> super::ShiftStyle {
@@ -1560,8 +1568,8 @@ impl Decoder<ARMv8> for InstDecoder {
                             Operand::Register(size, Rd),
                             Operand::Register(size, Rn),
                             match size {
-                                SizeCode::W => Operand::Immediate(docs::DecodeBitMasks_32(N as u8, imms as u8, immr as u8).0),
-                                SizeCode::X => Operand::Imm64(docs::DecodeBitMasks_64(N as u8, imms as u8, immr as u8).0)
+                                SizeCode::W => Operand::Immediate(docs::DecodeBitMasks_32(N as u8, imms as u8, immr as u8)?.0),
+                                SizeCode::X => Operand::Imm64(docs::DecodeBitMasks_64(N as u8, imms as u8, immr as u8)?.0)
                             },
                             Operand::Nothing
                         ];
@@ -1785,7 +1793,7 @@ impl Decoder<ARMv8> for InstDecoder {
                                         Operand::Nothing,
                                     ];
                                 } else {
-                                    unreachable!("Lo checked for validity already");
+                                    return Err(DecodeError::InvalidOpcode);
                                 };
                                 if size == 0b00 {
                                     match (Lo1, o0) {
@@ -1949,7 +1957,8 @@ impl Decoder<ARMv8> for InstDecoder {
                                 SizeCode::X
                             }
                             0b11 => {
-                                unimplemented!("PRFM is not supported");
+                                // PRFM is not supported
+                                return Err(DecodeError::IncompleteDecoder);
                             }
                             _ => {
                                 unreachable!("opc is two bits");
@@ -1997,13 +2006,15 @@ impl Decoder<ARMv8> for InstDecoder {
                         // load/store no-allocate pair (offset)
                         // V == 0
                         let opc_L = ((word >> 22) & 1) | ((word >> 29) & 0x6);
-                        unimplemented!("C3.3.7 V==0, opc_L: {}", opc_L);
+                        // eprintln!("C3.3.7 V==0, opc_L: {}", opc_L);
+                        return Err(DecodeError::IncompleteDecoder);
                     },
                     0b10100 => {
                         // load/store no-allocate pair (offset)
                         // V == 1
                         let opc_L = ((word >> 22) & 1) | ((word >> 29) & 0x6);
-                        unimplemented!("C3.3.7 V==1, opc_L: {}", opc_L);
+                        // eprintln!("C3.3.7 V==1, opc_L: {}", opc_L);
+                        return Err(DecodeError::IncompleteDecoder);
                     },
                     0b10001 => {
                         // load/store register pair (post-indexed)
@@ -2120,7 +2131,8 @@ impl Decoder<ARMv8> for InstDecoder {
                         // load/store register pair (offset)
                         // V == 1
                         let opc_L = ((word >> 22) & 1) | ((word >> 29) & 0x6);
-                        unimplemented!("C3.3.14 V==1, opc_L: {}", opc_L);
+                        // eprintln!("C3.3.14 V==1, opc_L: {}", opc_L);
+                        return Err(DecodeError::IncompleteDecoder);
                     },
                     0b10011 => {
                         // load/store register pair (pre-indexed)
@@ -2178,7 +2190,8 @@ impl Decoder<ARMv8> for InstDecoder {
                         // load/store register pair (pre-indexed)
                         // V == 1
                         let opc_L = ((word >> 22) & 1) | ((word >> 29) & 0x6);
-                        unimplemented!("C3.3.16 V==1, opc_L: {}", opc_L);
+                        // eprintln!("C3.3.16 V==1, opc_L: {}", opc_L);
+                        return Err(DecodeError::IncompleteDecoder);
                     },
                     0b11000 |
                     0b11001 => {
@@ -2257,7 +2270,8 @@ impl Decoder<ARMv8> for InstDecoder {
                                         SizeCode::X
                                     },
                                     0b1110 => {
-                                        unimplemented!("PRFM is not supported yet");
+                                        // eprintln!("PRFM is not supported yet");
+                                        return Err(DecodeError::IncompleteDecoder);
 //                                        inst.opcode = Opcode::PRFM;
 //                                        SizeCode::X
                                     },
@@ -2371,7 +2385,8 @@ impl Decoder<ARMv8> for InstDecoder {
                                             SizeCode::X
                                         }
                                         0b1110 => {
-                                            unimplemented!("PRFUM not handled yet");
+                                            // eprintln!("PRFUM not handled yet");
+                                            return Err(DecodeError::IncompleteDecoder);
                                         },
                                         0b1111 => {
                                             inst.opcode = Opcode::Invalid;
@@ -2556,7 +2571,7 @@ impl Decoder<ARMv8> for InstDecoder {
                          * unprivileged, immediate pre-indexd, register offset}
                          * V == 1
                          */
-                        unimplemented!("load/store register (unscaled immediate), load/store register (immediate post-indexed), V==1");
+                        return Err(DecodeError::IncompleteDecoder);
                     }
                     0b11010 |
                     0b11011 => {
@@ -2686,7 +2701,8 @@ impl Decoder<ARMv8> for InstDecoder {
                                 ];
                             }
                             0b1110 => {
-                                unimplemented!("PRFM not yet supported");
+                                // PRFM not yet supported
+                                return Err(DecodeError::IncompleteDecoder);
                             }
                             0b1111 => { inst.opcode = Opcode::Invalid; }
                             _ => { unreachable!("size and opc are four bits"); }
@@ -2696,23 +2712,23 @@ impl Decoder<ARMv8> for InstDecoder {
                     0b11111 => {
                         // load/store register (unsigned immediate)
                         // V == 1
-                        unimplemented!("load/store register (unsigned immediate) V==1");
+                        return Err(DecodeError::IncompleteDecoder);
                     },
                     0b00100 => {
                         // AdvSIMD load/store multiple structures
-                        unimplemented!("AdvSIMD load/store multiple structures");
+                        return Err(DecodeError::IncompleteDecoder);
                     },
                     0b00101 => {
                         // AdvSIMD load/store multiple structures (post-indexed)
-                        unimplemented!("AdvSIMD load/store multiple structures (post-indexed)");
+                        return Err(DecodeError::IncompleteDecoder);
                     },
                     0b00110 => {
                         // AdvSIMD load/store single structure
-                        unimplemented!("AdvSIMD load/store single structure");
+                        return Err(DecodeError::IncompleteDecoder);
                     },
                     0b00111 => {
                         // AdvSIMD load/store single structure (post-indexed)
-                        unimplemented!("AdvSIMD load/store single structures (post-indexed)");
+                        return Err(DecodeError::IncompleteDecoder);
                     }
                     _ => {
                         inst.opcode = Opcode::Invalid;
@@ -2952,7 +2968,7 @@ impl Decoder<ARMv8> for InstDecoder {
                                 }
                                 0b001 => {
                                     inst.opcode = Opcode::SYS;
-                                    panic!("TODO");
+                                    return Err(DecodeError::IncompleteDecoder);
                                 }
                                 0b010 |
                                 0b011 => {
@@ -2963,7 +2979,7 @@ impl Decoder<ARMv8> for InstDecoder {
                                 }
                                 0b101 => {
                                     inst.opcode = Opcode::SYSL;
-                                    panic!("TODO");
+                                    return Err(DecodeError::IncompleteDecoder);
                                 }
                                 0b110 |
                                 0b111 => {
@@ -3047,7 +3063,7 @@ impl Decoder<ARMv8> for InstDecoder {
                     }
                     _ => {
                         // TODO: invalid
-                        panic!("Illegal instruction");
+                        return Err(DecodeError::InvalidOpcode);
                     }
                 }
             },
diff --git a/test/armv7.rs b/test/armv7.rs
index 7feb774..a60b50a 100644
--- a/test/armv7.rs
+++ b/test/armv7.rs
@@ -106,6 +106,11 @@ fn test_display(data: [u8; 4], expected: &'static str) {
 }
 
 #[test]
+fn test_unpredictable_instructions() {
+    test_invalid([0x00, 0x02, 0x08, 0x01]); // msr with invalid machine register
+}
+
+#[test]
 fn test_decode_str_ldr() {
     test_decode(
         [0x24, 0xc0, 0x9f, 0xe5],
diff --git a/test/armv7/thumb.rs b/test/armv7/thumb.rs
index fafc65c..c74da37 100644
--- a/test/armv7/thumb.rs
+++ b/test/armv7/thumb.rs
@@ -77,6 +77,11 @@ fn test_display(data: &[u8], expected: &'static str) {
 }
 
 #[test]
+fn test_unpredictable_instructions() {
+    test_invalid(&[0x80, 0xfa, 0x40, 0x00]);
+}
+
+#[test]
 fn test_decode_add_cases() {
     test_display(
         &[0x01, 0x44],
diff --git a/test/armv8/a64.rs b/test/armv8/a64.rs
index 6dd9d9b..0e25c93 100644
--- a/test/armv8/a64.rs
+++ b/test/armv8/a64.rs
@@ -45,6 +45,13 @@ fn test_neon() {
 }
 
 #[test]
+fn test_unpredictable() {
+    // could be stx/ldx but Lo1 is `x1` and invalid.
+    test_err([0x00, 0x00, 0x20, 0x08], DecodeError::InvalidOpcode);
+    test_err([0x00, 0xfc, 0x00, 0x12], DecodeError::InvalidOperand);
+}
+
+#[test]
 fn test_display_misc() {
     test_display(
         [0xc0, 0x03, 0x5f, 0xd6],
diff --git a/test/test.rs b/test/test.rs
index fcf680a..b333fc1 100644
--- a/test/test.rs
+++ b/test/test.rs
@@ -7,3 +7,44 @@ extern crate yaxpeax_arm;
 
 mod armv7;
 mod armv8;
+
+use yaxpeax_arch::{Arch, Decoder, Reader, U8Reader};
+use std::fmt;
+
+#[test]
+fn test_armv7_does_not_panic() {
+    let armv7 = <yaxpeax_arm::armv7::ARMv7 as Arch>::Decoder::default();
+
+    for i in 0..=u32::MAX {
+        let bytes = i.to_le_bytes();
+        let res = armv7.decode(&mut U8Reader::new(&bytes));
+        if let Ok(instr) = res {
+            let s = instr.to_string();
+        }
+    }
+}
+#[test]
+fn test_armv7_thumb_does_not_panic() {
+    let mut armv7_t = <yaxpeax_arm::armv7::ARMv7 as Arch>::Decoder::default();
+    armv7_t.set_thumb_mode(true);
+
+    for i in 0..=u32::MAX {
+        let bytes = i.to_le_bytes();
+        let res = armv7_t.decode(&mut U8Reader::new(&bytes));
+        if let Ok(instr) = res {
+            let s = instr.to_string();
+        }
+    }
+}
+#[test]
+fn test_armv8_does_not_panic() {
+    let armv8 = <yaxpeax_arm::armv8::a64::ARMv8 as Arch>::Decoder::default();
+
+    for i in 0..=u32::MAX {
+        let bytes = i.to_le_bytes();
+        let res = armv8.decode(&mut U8Reader::new(&bytes));
+        if let Ok(instr) = res {
+            let s = instr.to_string();
+        }
+    }
+}