Check all of the appropriate bits when parsing op1 for CDP2/MCR2/MRC2

3 files changed, 21 insertions, 0 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 2daae7f..02c60e5 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -14,6 +14,8 @@ several fixes from @Grond66:
   decoder is operating in thumb mode.
 * ARMv7: fix incorrect handling of registers lsr or asr by an immediate 32.
   yaxpeax-arm incorrectly reported a shift of 0, but should have reported 32.
+* ARMv7: reject unconditional instructions with op1=1111xxxx. these were
+  decoded as CDP, MCR, or MRC when the bit pattern is actually undefined.
 
 thank you for the patches!
 
diff --git a/src/armv7.rs b/src/armv7.rs
index ada486a..2901508 100644
--- a/src/armv7.rs
+++ b/src/armv7.rs
@@ -1492,6 +1492,17 @@ impl Decoder<ARMv7> for InstDecoder {
                         }
                     }
                     0b11 => {
+                        // We know the instruction looks like this...
+                        // |1 1 1 1|1 1 1|x x x x|x|x x x x|x x x x|x x x x x|x x|x|x x x x|
+                        //                ^ We need to check that this bit is zero, if any of the
+                        //                  instructions decoded in this block are to match
+                        //                  correctly.
+                        // See A5-214 for the table that shows the required forms for these
+                        // instructions.
+                        if (op1 >> 4) & 1 != 0 {
+                            return Err(DecodeError::InvalidOpcode);
+                        }
+
                         // operands are shared between cdp2 and mcr2/mrc2, but Rt is repurposed as
                         // CRd
                         let CRm = word as u8 & 0b1111;
diff --git a/tests/armv7/mod.rs b/tests/armv7/mod.rs
index 632bd3e..7fff4a8 100644
--- a/tests/armv7/mod.rs
+++ b/tests/armv7/mod.rs
@@ -714,6 +714,14 @@ fn test_register_shift_rotate() {
     test_all([0x62, 0x00, 0x01, 0xe0], "and r0, r1, r2, rrx");
 }
 
+#[test]
+fn test_decode_mrc2() {
+    // The LSB of the last byte being set makes op1 not match any row in
+    // A5.7 Unconditional Instructions, but previously this was incorrectly decoded as `mrc2`.
+    test_invalid([0xbc, 0xec, 0xff, 0xff]);
+    test_armv6([0xbc, 0xec, 0xff, 0xfe], "mrc2 p12, 7, lr, c15, c12, 5");
+}
+
 static INSTRUCTION_BYTES: [u8; 4 * 60] = [
         0x24, 0xc0, 0x9f, 0xe5,
         0x00, 0xb0, 0xa0, 0xe3,