Add fuzzing, fix things that were discovered (#13)

Fixes #11, with three bugs discovered by fuzzing:

- `_NIP2r` was incorrect in assembly, with two digits swapped
- `_SWP2r` was incorrect in assembly, with a single digit mistake
- `sft` was incorrect in the interpreter implementation, because
  wrapping_shl/shr doesn't return zeros if you shift out of range

Author: mkeeter

Cargo.lock

@@ -6,6 +6,7 @@ members = [
     "raven-cli",
     "raven-gui",
 ]
+exclude = ["fuzz"]
 
 [workspace.dependencies]
 anyhow = "1.0.83"

Cargo.toml

@@ -20,6 +20,14 @@ The `raven-uxn` crate includes two implementations of the Uxn CPU:
   shims on either side), and runs 40-50% faster than the reference
   implementation
 
+The native interpreter can be checked against the safe interpreter with fuzz
+testing:
+
+```console
+cargo install cargo-fuzz # this only needs to be run once
+cargo +nightly fuzz run --release fuzz-native
+```
+
 --------------------------------------------------------------------------------
 
 The Varvara implementation (`raven-varvara`) includes all peripherals, and has
@@ -37,7 +45,7 @@ The repository includes two applications built on these libraries:
 
 --------------------------------------------------------------------------------
 
-© 2024 Matthew Keeter  
+© 2024-2025 Matthew Keeter  
 Released under the [Mozilla Public License 2.0](https://github.com/mkeeter/fidget/blob/main/LICENSE.txt)
 
 The repository includes ROMs compiled from the `uxnemu` reference

README.md

@@ -0,0 +1,4 @@
+target
+corpus
+artifacts
+coverage

fuzz/.gitignore

@@ -0,0 +1,19 @@
+[package]
+name = "fuzz"
+version = "0.0.0"
+publish = false
+edition = "2021"
+
+[package.metadata]
+cargo-fuzz = true
+
+[dependencies]
+libfuzzer-sys = "0.4"
+uxn = { path = "../raven-uxn", package = "raven-uxn", features = ["native"] }
+
+[[bin]]
+name = "fuzz-native"
+path = "src/native.rs"
+test = false
+doc = false
+bench = false

fuzz/Cargo.lock

@@ -0,0 +1,69 @@
+#![no_main]
+
+use libfuzzer_sys::fuzz_target;
+use uxn::{Backend, EmptyDevice, Uxn, UxnRam};
+
+fuzz_target!(|data: &[u8]| {
+    let mut ram_v = UxnRam::new();
+    let mut vm_v = Uxn::new(&mut ram_v, Backend::Interpreter);
+
+    let mut ram_n = UxnRam::new();
+    let mut vm_n = Uxn::new(&mut ram_n, Backend::Native);
+
+    // Don't load any programs that require auxiliary memory
+    if !vm_v.reset(data).is_empty() {
+        return;
+    }
+    assert!(vm_n.reset(data).is_empty());
+
+    // Use the VM-backed evaluator, halting if we take more than 65K cycles
+    let Some(pc_v) =
+        vm_v.run_until(&mut EmptyDevice, 0x100, |_uxn, _dev, i| i > 65536)
+    else {
+        return;
+    };
+    let pc_n = vm_n.run(&mut EmptyDevice, 0x100);
+
+    let mut failed = false;
+
+    if pc_v != pc_n {
+        println!("PC mismatch: {pc_v:#04x} != {pc_n:#04x}");
+        failed = true;
+    }
+    for i in 0..=65535 {
+        let a = vm_v.ram_read_byte(i);
+        let b = vm_n.ram_read_byte(i);
+        if a != b {
+            println!("RAM mismatch at {i:#04x}: {a:#02x} != {b:#02x}");
+            failed = true;
+        }
+    }
+    if vm_v.ret() != vm_n.ret() {
+        println!(
+            "return mismatch:\n  bytecode: {:?}\n    native: {:?}",
+            vm_v.ret(),
+            vm_n.ret()
+        );
+        failed = true;
+    }
+    if vm_v.stack() != vm_n.stack() {
+        println!(
+            "stack mismatch:\n  bytecode: {:?}\n    native: {:?}",
+            vm_v.stack(),
+            vm_n.stack()
+        );
+        failed = true;
+    }
+    if failed {
+        print!("Instructions:\n  ");
+        for (i, d) in data.iter().enumerate() {
+            print!(
+                "{}{}",
+                if i == 0 { "" } else { " " },
+                uxn::op::NAMES[usize::from(*d)]
+            );
+        }
+        println!();
+        panic!("mismatch found");
+    }
+});

fuzz/Cargo.toml

@@ -20,7 +20,7 @@ const fn ret(flags: u8) -> bool {
 pub const DEV_SIZE: usize = 16;
 
 /// Simple circular stack, with room for 256 items
-#[derive(Debug)]
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub struct Stack {
     data: [u8; 256],
 
@@ -147,17 +147,17 @@ impl Value {
         }
     }
     #[inline]
-    fn wrapping_shr(&self, i: u32) -> Self {
+    fn shr(&self, i: u32) -> Self {
         match self {
-            Value::Short(v) => Value::Short(v.wrapping_shr(i)),
-            Value::Byte(v) => Value::Byte(v.wrapping_shr(i)),
+            Value::Short(v) => Value::Short(v.checked_shr(i).unwrap_or(0)),
+            Value::Byte(v) => Value::Byte(v.checked_shr(i).unwrap_or(0)),
         }
     }
     #[inline]
-    fn wrapping_shl(&self, i: u32) -> Self {
+    fn shl(&self, i: u32) -> Self {
         match self {
-            Value::Short(v) => Value::Short(v.wrapping_shl(i)),
-            Value::Byte(v) => Value::Byte(v.wrapping_shl(i)),
+            Value::Short(v) => Value::Short(v.checked_shl(i).unwrap_or(0)),
+            Value::Byte(v) => Value::Byte(v.checked_shl(i).unwrap_or(0)),
         }
     }
 }
@@ -400,21 +400,45 @@ impl<'a> Uxn<'a> {
     #[inline]
     pub fn run<D: Device>(&mut self, dev: &mut D, mut pc: u16) -> u16 {
         match self.backend {
-            Backend::Interpreter => {
-                loop {
-                    let op = self.next(&mut pc);
-                    let Some(next) = self.op(op, dev, pc) else {
-                        break;
-                    };
-                    pc = next;
-                }
-                pc
-            }
+            Backend::Interpreter => loop {
+                let op = self.next(&mut pc);
+                let Some(next) = self.op(op, dev, pc) else {
+                    break pc;
+                };
+                pc = next;
+            },
             #[cfg(feature = "native")]
             Backend::Native => native::entry(self, dev, pc),
         }
     }
 
+    /// Runs until the program terminates or we hit a stop condition
+    ///
+    /// Returns the new program counter if the program terminated, or `None` if
+    /// the stop condition was reached.
+    ///
+    /// This function always uses the interpreter, ignoring
+    /// [`self.backend`](Self::backend).
+    #[inline]
+    pub fn run_until<D: Device, F: Fn(&Self, &D, usize) -> bool>(
+        &mut self,
+        dev: &mut D,
+        mut pc: u16,
+        stop: F,
+    ) -> Option<u16> {
+        for i in 0.. {
+            let op = self.next(&mut pc);
+            let Some(next) = self.op(op, dev, pc) else {
+                return Some(pc);
+            };
+            pc = next;
+            if stop(self, dev, i) {
+                return None;
+            }
+        }
+        unreachable!()
+    }
+
     /// Converts raw ports memory into a [`Ports`] object
     #[inline]
     pub fn dev<D: Ports>(&self) -> &D {
@@ -1638,7 +1662,7 @@ impl<'a> Uxn<'a> {
         let shr = u32::from(shift & 0xF);
         let shl = u32::from(shift >> 4);
         let v = s.pop();
-        s.push(v.wrapping_shr(shr).wrapping_shl(shl));
+        s.push(v.shr(shr).shl(shl));
         Some(pc)
     }
 }
@@ -1728,9 +1752,9 @@ pub use ram::UxnRam;
 
 ////////////////////////////////////////////////////////////////////////////////
 
-#[cfg(test)]
-#[allow(unused, non_upper_case_globals)]
-mod op {
+/// Opcode names and constants
+#[allow(non_upper_case_globals, missing_docs)]
+pub mod op {
     pub const BRK: u8 = 0x0;
     pub const INC: u8 = 0x1;
     pub const POP: u8 = 0x2;

fuzz/src/native.rs

@@ -877,9 +877,9 @@ _NIP2r:
     ldrb w10, [x2, x3]
     rpop
     strb w9, [x2, x3]
-    sub x11, x1, #1
+    sub x11, x3, #1
     and x11, x11, #0xff
-    strb w10, [x2, x31]
+    strb w10, [x2, x11]
     next
 
 _SWP2r:
@@ -889,7 +889,7 @@ _SWP2r:
     strb w12, [x2, x3]
 
     rpeek w11, x9, 1
-    rpeek w12, x10, 2
+    rpeek w12, x10, 3
 
     strb w11, [x2, x10]
     strb w12, [x2, x9]