fuzz cases: only 64 system registers, display should never panic

6 files changed, 162 insertions, 70 deletions

diff --git a/fuzz/.gitignore b/fuzz/.gitignore
new file mode 100644
index 0000000..a092511
--- /dev/null
+++ b/fuzz/.gitignore
@@ -0,0 +1,3 @@
+target
+corpus
+artifacts
diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml
new file mode 100644
index 0000000..5804b25
--- /dev/null
+++ b/fuzz/Cargo.toml
@@ -0,0 +1,30 @@
+[package]
+name = "yaxpeax-hexagon-fuzz"
+version = "0.0.0"
+authors = ["iximeow <me@iximeow.net>"]
+publish = false
+edition = "2021"
+
+[package.metadata]
+cargo-fuzz = true
+
+[dependencies]
+libfuzzer-sys = "0.4"
+yaxpeax-hexagon = { path = ".." }
+yaxpeax-arch = "0.3.1"
+
+# Prevent this from interfering with workspaces
+[workspace]
+members = ["."]
+
+[[bin]]
+name = "no-panic"
+path = "fuzz_targets/no-panic.rs"
+test = false
+doc = false
+
+[[bin]]
+name = "fresh-decode"
+path = "fuzz_targets/fresh-decode.rs"
+test = false
+doc = false
diff --git a/fuzz/fuzz_targets/fresh-decode.rs b/fuzz/fuzz_targets/fresh-decode.rs
new file mode 100644
index 0000000..76a402e
--- /dev/null
+++ b/fuzz/fuzz_targets/fresh-decode.rs
@@ -0,0 +1,21 @@
+//! decoding into a pre-existing instruction should not result in different outcomes compared to
+//! decoding into a fresh instruction. if decoding succeeds, both outcomes should be equal.
+
+#![no_main]
+use libfuzzer_sys::fuzz_target;
+
+use yaxpeax_arch::Decoder;
+
+fuzz_target!(|data: &[u8]| {
+    let decoder = yaxpeax_hexagon::InstDecoder::default();
+
+    let mut reused_inst = yaxpeax_hexagon::InstructionPacket::default();
+
+    let mut words = yaxpeax_arch::U8Reader::new(data);
+    // test decoding, may be ok or not, but should not panic
+    if let Ok(()) = decoder.decode_into(&mut reused_inst, &mut words) {
+        let mut words = yaxpeax_arch::U8Reader::new(data);
+        let fresh_inst = decoder.decode(&mut words).expect("decoded before, can decode again");
+        assert_eq!(reused_inst, fresh_inst);
+    }
+});
diff --git a/src/display.rs b/src/display.rs
index 6827108..90dfca4 100644
--- a/src/display.rs
+++ b/src/display.rs
@@ -40,20 +40,29 @@ fn assign_mode_label(mode: Option<AssignMode>) -> &'static str {
     }
 }
 
+fn display_or_partial<T: fmt::Display + ?Sized>(t: Option<&T>) -> String {
+    match t {
+        Some(t) => { t.to_string() },
+        None => { "<invalid>".to_string() }
+    }
+}
+
 impl fmt::Display for Instruction {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         if special_display_rules(&self.opcode) {
             match self.opcode {
                 Opcode::AndAnd => {
-                    return write!(f, "{} = and({}, and({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = and({}, and({}, {}))",
+                        display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 }
                 Opcode::AndOr => {
-                    return write!(f, "{} = and({}, or({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = and({}, or({}, {}))",
+                        display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 }
                 Opcode::OrAnd => {
-                    return write!(f, "{} = or({}, and({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = or({}, and({}, {}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 }
                 Opcode::AndNot => {
@@ -65,125 +74,125 @@ impl fmt::Display for Instruction {
                             assign_mode_label(self.flags.assign_mode),
                             self.sources[0], self.sources[1]);
                     } else {
-                        return write!(f, "{} {} and({}, ~{})", self.dest.as_ref().unwrap(),
+                        return write!(f, "{} {} and({}, ~{})", display_or_partial(self.dest.as_ref()),
                             assign_mode_label(self.flags.assign_mode),
                             self.sources[0], self.sources[1]);
                     }
                 }
                 Opcode::OrOr => {
-                    return write!(f, "{} = or({}, or({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = or({}, or({}, {}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 }
                 Opcode::AndAndNot => {
-                    return write!(f, "{} = and({}, and({}, !{}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = and({}, and({}, !{}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 }
                 Opcode::AndOrNot => {
-                    return write!(f, "{} = and({}, or({}, !{}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = and({}, or({}, !{}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 }
                 Opcode::OrAndNot => {
-                    return write!(f, "{} = or({}, and({}, !{}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = or({}, and({}, !{}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 }
                 Opcode::OrNot => {
-                    return write!(f, "{} {} or({}, !{})", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} {} or({}, !{})", display_or_partial(self.dest.as_ref()),
                         assign_mode_label(self.flags.assign_mode),
                         self.sources[0], self.sources[1]);
                 }
                 Opcode::OrOrNot => {
-                    return write!(f, "{} = or({}, or({}, !{}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = or({}, or({}, !{}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 }
                 Opcode::AndLsr => {
-                    return write!(f, "{} = and({}, lsr({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = and({}, lsr({}, {}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 },
                 Opcode::OrLsr => {
-                    return write!(f, "{} = or({}, lsr({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = or({}, lsr({}, {}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 },
                 Opcode::AddLsr => {
-                    return write!(f, "{} = add({}, lsr({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = add({}, lsr({}, {}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 },
                 Opcode::SubLsr => {
-                    return write!(f, "{} = sub({}, lsr({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = sub({}, lsr({}, {}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 },
                 Opcode::AddLsl => {
-                    return write!(f, "{} = add({}, lsl({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = add({}, lsl({}, {}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 },
                 Opcode::AddAsl => {
-                    return write!(f, "{} = add({}, asl({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = add({}, asl({}, {}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 },
                 Opcode::SubAsl => {
-                    return write!(f, "{} = sub({}, asl({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = sub({}, asl({}, {}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 },
                 Opcode::AndAsl => {
-                    return write!(f, "{} = and({}, asl({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = and({}, asl({}, {}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 },
                 Opcode::OrAsl => {
-                    return write!(f, "{} = or({}, asl({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = or({}, asl({}, {}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 },
                 Opcode::AddAdd => {
-                    return write!(f, "{} = add({}, add({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = add({}, add({}, {}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 },
                 Opcode::AddSub => {
-                    return write!(f, "{} = add({}, sub({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = add({}, sub({}, {}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 },
                 Opcode::AddMpyi => {
-                    return write!(f, "{} = add({}, mpyi({}, {}))", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = add({}, mpyi({}, {}))", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1], self.sources[2]);
                 },
                 Opcode::MpyiPos => {
-                    return write!(f, "{} = +mpyi({}, {})", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = +mpyi({}, {})", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1]);
                 },
                 Opcode::MpyiNeg => {
-                    return write!(f, "{} = -mpyi({}, {})", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = -mpyi({}, {})", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1]);
                 },
                 Opcode::AddClb => {
-                    return write!(f, "{} = add(clb({}), {})", self.dest.as_ref().unwrap(),
+                    return write!(f, "{} = add(clb({}), {})", display_or_partial(self.dest.as_ref()),
                         self.sources[0], self.sources[1]);
                 }
                 Opcode::Vacsh => {
                     return write!(f, "{}, {} = {}({}, {})",
-                        self.dest.as_ref().unwrap(), self.alt_dest.as_ref().unwrap(),
+                        display_or_partial(self.dest.as_ref()), display_or_partial(self.alt_dest.as_ref()),
                         self.opcode, self.sources[0], self.sources[1]);
                 }
                 Opcode::Vminub => {
                     if let Some(alt_dest) = self.alt_dest.as_ref() {
                         return write!(f, "{}, {} = {}({}, {})",
-                            self.dest.as_ref().unwrap(), alt_dest,
+                            display_or_partial(self.dest.as_ref()), alt_dest,
                             self.opcode, self.sources[0], self.sources[1]);
                     } else {
                         return write!(f, "{} = {}({}, {})",
-                            self.dest.as_ref().unwrap(),
+                            display_or_partial(self.dest.as_ref()),
                             self.opcode, self.sources[0], self.sources[1]);
                     }
                 }
                 Opcode::SfRecipa => {
                     return write!(f, "{}, {} = {}({}, {})",
-                        self.dest.as_ref().unwrap(), self.alt_dest.as_ref().unwrap(),
+                        display_or_partial(self.dest.as_ref()), display_or_partial(self.alt_dest.as_ref()),
                         self.opcode, self.sources[0], self.sources[1]);
                 }
                 Opcode::SfInvsqrta => {
                     return write!(f, "{}, {} = {}({})",
-                        self.dest.as_ref().unwrap(), self.alt_dest.as_ref().unwrap(),
+                        display_or_partial(self.dest.as_ref()), display_or_partial(self.alt_dest.as_ref()),
                         self.opcode, self.sources[0]);
                 }
                 Opcode::Any8VcmpbEq => {
                     return write!(f, "{} = {}any8(vcmpb.eq({}, {}))",
-                        self.dest.as_ref().unwrap(),
+                        display_or_partial(self.dest.as_ref()),
                         if self.flags.negated { "!" } else { "" },
                         self.sources[0], self.sources[1]);
                 }
@@ -211,7 +220,7 @@ impl fmt::Display for Instruction {
                     Some(BranchHint::NotTaken) => { ":nt" },
                     None => { "" },
                 },
-                self.dest.as_ref().unwrap()
+                display_or_partial(self.dest.as_ref())
             );
         }
         // handle cmp+jump first; this includes elements that would be misformatted below (like
@@ -221,25 +230,31 @@ impl fmt::Display for Instruction {
             Opcode::CmpGtuJump, Opcode::TestClrJump,
         ];
         if COMPARE_JUMPS.contains(&self.opcode) {
-            let predicate = self.flags.predicate.as_ref().unwrap();
-            let preg = Operand::pred(predicate.num());
+            if let Some(predicate) = self.flags.predicate.as_ref() {
+                let preg = Operand::pred(predicate.num());
 
-            let hint_label = match self.flags.branch_hint.unwrap() {
-                BranchHint::Taken => { "t" },
-                BranchHint::NotTaken => { "nt" },
-            };
+                let hint_label = match self.flags.branch_hint {
+                    Some(BranchHint::Taken) => { "t" },
+                    Some(BranchHint::NotTaken) => { "nt" },
+                    None => {
+                        "<invalid decode>"
+                    }
+                };
 
-            write!(f, "{} = {}({}, {}); if ({}{}.new) jump:{} {}",
-                preg,
-                self.opcode.cmp_str().unwrap(),
-                self.sources[0],
-                self.sources[1],
-                if predicate.negated() { "!" } else { "" },
-                preg,
-                hint_label,
-                self.dest.as_ref().unwrap(),
-            )?;
-            return Ok(());
+                write!(f, "{} = {}({}, {}); if ({}{}.new) jump:{} {}",
+                    preg,
+                    display_or_partial(self.opcode.cmp_str()),
+                    self.sources[0],
+                    self.sources[1],
+                    if predicate.negated() { "!" } else { "" },
+                    preg,
+                    hint_label,
+                    display_or_partial(self.dest.as_ref()),
+                )?;
+                return Ok(());
+            } else {
+                return write!(f, "<invalid decode>");
+            }
         }
 
         if let Some(predication) = self.flags.predicate {
@@ -309,9 +324,9 @@ impl fmt::Display for Instruction {
         // TransferRegisterJump and TransferImmediateJump also have special display rules...
         if self.opcode == Opcode::TransferRegisterJump || self.opcode == Opcode::TransferImmediateJump {
             write!(f, "{} = {}; jump {}",
-                self.alt_dest.as_ref().unwrap(),
+                display_or_partial(self.alt_dest.as_ref()),
                 self.sources[0],
-                self.dest.as_ref().unwrap(),
+                display_or_partial(self.dest.as_ref()),
             )?;
             return Ok(());
         }
@@ -327,11 +342,11 @@ impl fmt::Display for Instruction {
                 None => { "" },
             };
             write!(f, "if ({}({}, {})) jump{} {}",
-                self.opcode.cmp_str().unwrap(), // (obvious, but) decoder bug if this fails
+                display_or_partial(self.opcode.cmp_str()), // (obvious, but) decoder bug if this fails
                 self.sources[0],
                 self.sources[1],
                 hint_label,
-                self.dest.unwrap(),
+                display_or_partial(self.dest.as_ref()),
             )?;
             return Ok(());
         }
@@ -347,7 +362,7 @@ impl fmt::Display for Instruction {
                     Some(BranchHint::NotTaken) => ":nt",
                     None => "",
                 },
-                self.dest.unwrap())?;
+                display_or_partial(self.dest.as_ref()))?;
             return Ok(());
         }
 
diff --git a/src/lib.rs b/src/lib.rs
index 115d78e..6c89dec 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -112,7 +112,7 @@ impl Predicate {
     }
 }
 
-#[derive(Debug, Copy, Clone, Default)]
+#[derive(Debug, Copy, Clone, Default, PartialEq)]
 pub struct LoopEnd {
     loops_ended: u8
 }
@@ -163,6 +163,16 @@ pub struct InstructionPacket {
     loop_effect: LoopEnd,
 }
 
+impl PartialEq for InstructionPacket {
+    fn eq(&self, other: &Self) -> bool {
+        let instructions = self.instruction_count as usize;
+        self.instruction_count == other.instruction_count &&
+            self.word_count == other.word_count &&
+            self.loop_effect == other.loop_effect &&
+            self.instructions[..instructions] == other.instructions[..instructions]
+    }
+}
+
 /// a decoded `hexagon` instruction. this is only one of potentially several instructions in an
 /// [`InstructionPacket`].
 ///
@@ -1204,8 +1214,8 @@ trait DecodeHandler<T: Reader<<Hexagon as Arch>::Address, <Hexagon as Arch>::Wor
     fn read_inst_word(&mut self, words: &mut T) -> Result<u32, <Hexagon as Arch>::DecodeError>;
     fn on_decode_start(&mut self) {}
     fn on_decode_end(&mut self) {}
-    fn start_instruction(&mut self);
-    fn end_instruction(&mut self);
+    fn start_instruction(&mut self) -> Result<(), <Hexagon as Arch>::DecodeError> { Ok(()) }
+    fn end_instruction(&mut self) -> Result<(), <Hexagon as Arch>::DecodeError> { Ok(()) }
     fn on_loop_end(&mut self, loop_num: u8);
     fn on_opcode_decoded(&mut self, _opcode: Opcode) -> Result<(), <Hexagon as Arch>::DecodeError> { Ok(()) }
     fn on_source_decoded(&mut self, _operand: Operand) -> Result<(), <Hexagon as Arch>::DecodeError> { Ok(()) }
@@ -1368,9 +1378,13 @@ impl<T: yaxpeax_arch::Reader<<Hexagon as Arch>::Address, <Hexagon as Arch>::Word
         self.read_u32(words)
     }
     fn on_word_read(&mut self, _word: <Hexagon as Arch>::Word) { }
-    fn start_instruction(&mut self) { }
-    fn end_instruction(&mut self) {
+    fn start_instruction(&mut self) -> Result<(), <Hexagon as Arch>::DecodeError> {
+        opcode_check!(self.instruction_count < self.instructions.len() as u8);
+        Ok(())
+    }
+    fn end_instruction(&mut self) -> Result<(), <Hexagon as Arch>::DecodeError> {
         self.instruction_count += 1;
+        Ok(())
     }
 }
 
@@ -1981,12 +1995,12 @@ fn decode_packet<
                         parse_lo: fn(&mut H, u16, &mut Option<u32>) -> Result<(), <Hexagon as Arch>::DecodeError>,
                         parse_hi: fn(&mut H, u16, &mut Option<u32>) -> Result<(), <Hexagon as Arch>::DecodeError>,
                     ) -> Result<(), yaxpeax_arch::StandardDecodeError> {
-                        self.handler.start_instruction();
+                        self.handler.start_instruction()?;
                         parse_lo(self.handler, self.subinstr_low, self.extender)?;
-                        self.handler.end_instruction();
-                        self.handler.start_instruction();
+                        self.handler.end_instruction()?;
+                        self.handler.start_instruction()?;
                         parse_hi(self.handler, self.subinstr_high, self.extender)?;
-                        self.handler.end_instruction();
+                        self.handler.end_instruction()?;
                         Ok(())
                     }
                 }
@@ -2080,7 +2094,7 @@ fn decode_packet<
         if iclass == 0b0000 {
             extender = Some((inst & 0x3fff) | ((inst >> 2) & 0x3ffc000));
         } else {
-            handler.start_instruction();
+            handler.start_instruction()?;
             decode_instruction(decoder, handler, inst, &mut extender)?;
             // V73 section 10.9:
             // > The constant extender serves as a prefix for an instruction: it does not execute
@@ -2095,7 +2109,7 @@ fn decode_packet<
                 // if the extender was not consumed, the instruction (packet) is invalid
                 return Err(DecodeError::InvalidOpcode);
             }
-            handler.end_instruction();
+            handler.end_instruction()?;
         }
 
         current_word += 1;
@@ -3113,9 +3127,9 @@ fn decode_instruction<
                 0b0111000 => {
                     // not in V73! store to supervisor register?
                     let sssss = reg_b16(inst);
-                    let ddddddd = inst & 0b111_1111;
+                    let dddddd = inst & 0b11_1111;
                     handler.on_opcode_decoded(Opcode::TransferRegister)?;
-                    handler.on_dest_decoded(Operand::sr(ddddddd as u8))?;
+                    handler.on_dest_decoded(Operand::sr(dddddd as u8))?;
                     handler.on_source_decoded(Operand::gpr(sssss))?;
                 }
                 // TODO: 1001000 loop0 goes here
@@ -3152,9 +3166,9 @@ fn decode_instruction<
                 0b1110110 | 0b1110111 => {
                     // not in V73! load supervisor register?
                     let sssss = reg_b0(inst);
-                    let ddddddd = (inst >> 16) & 0b111_1111;
+                    let dddddd = (inst >> 16) & 0b11_1111;
                     handler.on_opcode_decoded(Opcode::TransferRegister)?;
-                    handler.on_source_decoded(Operand::sr(ddddddd as u8))?;
+                    handler.on_source_decoded(Operand::sr(dddddd as u8))?;
                     handler.on_dest_decoded(Operand::gpr(sssss))?;
                 }
                 0b1111000 | 0b1111001 |
@@ -3763,7 +3777,8 @@ fn decode_instruction<
                 handler.on_opcode_decoded(Opcode::Nop)?;
             }
             _ => {
-                unreachable!("impossible pattern");
+                // anything left unhandled is invalid..
+                opcode_check!(false);
             }
             }
         }
diff --git a/tests/from_brain.rs b/tests/from_brain.rs
index a72f561..9c33dbf 100644
--- a/tests/from_brain.rs
+++ b/tests/from_brain.rs
@@ -2460,3 +2460,11 @@ fn inst_1111() {
 
     test_display(&0b1111_1101000_00100_11_1_00011_001_00110u32.to_le_bytes(), "{ if (p1.new) r7:6 = contains(r4, r3) }");
 }
+
+#[test]
+fn no_panic() {
+    test_invalid(
+        &[253, 67, 36, 65, 68, 143, 143, 143, 143, 143, 143, 143, 3, 0, 2, 0, 143],
+        DecodeError::InvalidOpcode
+    );
+}