API BREAK feat(vm): move jumpdest check out of 'run' into 'verify'

crates/fogtix-vm-cli/src/bin/fogtix-vm.rs

@@ -18,6 +18,11 @@ fn main() {
     let main_mod = readfilez::read_from_file(std::fs::File::open(main_mod_path))
         .expect("unable to open/load main module");
 
+    if let Err(e) = fogtix_vm::verify(&main_mod) {
+        eprintln!("ERROR: {}", e);
+        std::process::exit(1);
+    }
+
     let mut p = fogtix_vm::Process {
         stack: Vec::new(),
         callstack: Vec::new(),
@@ -25,7 +30,9 @@ fn main() {
         instrp: 0,
     };
 
-    if let Err(e) = p.run(None) {
-        eprintln!("ERROR@{}: {}", p.instrp, e);
+    match p.run(None) {
+        Err(e) => eprintln!("ERROR@{}: {}", p.instrp, e),
+        Ok(None) => {}
+        Ok(Some(x)) => eprintln!("remote call: {}", x),
     }
 }

crates/fogtix-vm/src/lib.rs

@@ -23,43 +23,104 @@ pub type StackEntValue = u64;
 
 #[derive(Clone, Debug)]
 pub enum Error {
-    InvalidJumpTarget {
-        invoked_by: &'static str,
-        from: usize,
-        to: usize,
-    },
     InstrpOutOfBounds,
     UnparsableInstruction(Vec<u8>),
     OutOfFuel,
     NotEnoughStacked,
     DivisionByZero,
-    RemoteCall(u64),
 }
 
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            Self::InvalidJumpTarget {
-                invoked_by,
-                from,
-                to,
-            } => {
-                write!(
-                    f,
-                    "`{}` arrived from `{}` => at non-jump target `{}`",
-                    invoked_by, from, to
-                )
-            }
             Self::InstrpOutOfBounds => write!(f, "instruction pointer out-of-bounds"),
             Self::UnparsableInstruction(x) => write!(f, "reached unparsable instruction: {:x?}", x),
             Self::OutOfFuel => write!(f, "out of fuel"),
             Self::NotEnoughStacked => write!(f, "not enough operands on stack"),
             Self::DivisionByZero => write!(f, "tried to divide by zero"),
-            Self::RemoteCall(ptr) => write!(f, "tried to call remote @ {:x?}", ptr),
         }
     }
 }
 
+#[derive(Clone, Debug)]
+pub struct VerifyError {
+    pub from: usize,
+    pub kind: VerifyErrorKind,
+}
+
+#[derive(Clone, Debug)]
+pub enum VerifyErrorKind {
+    InvalidJumpTarget { invoked_by: &'static str, to: usize },
+    InstrpOutOfBounds,
+    UnparsableInstruction(Vec<u8>),
+}
+
+impl fmt::Display for VerifyError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        use VerifyErrorKind as K;
+        write!(f, "at instruction {}: ", self.from)?;
+        match &self.kind {
+            K::InvalidJumpTarget { invoked_by, to } => {
+                write!(f, "`{}` arrived at non-jump target `{}`", invoked_by, to)
+            }
+            K::InstrpOutOfBounds => write!(f, "instruction pointer out-of-bounds"),
+            K::UnparsableInstruction(x) => write!(f, "reached unparsable instruction: {:x?}", x),
+        }
+    }
+}
+
+/// this should be called before attempting to run an opcode stream to ensure
+/// that jump targets are valid (this makes it possible to avoid unnecessary
+/// checks during tight loops)
+pub fn verify(m: &[u8]) -> Result<(), VerifyError> {
+    let check_jump = |jinstr: &'static str, from: usize, to: u64| -> Result<(), VerifyError> {
+        match usize::try_from(to) {
+            Ok(to) if to == 0 => Ok(()),
+            Ok(to) if matches!(next_instr(m, to), Some(Ok((_, Instr::Label)))) => Ok(()),
+            Ok(to) => Err(VerifyError {
+                from,
+                kind: VerifyErrorKind::InvalidJumpTarget {
+                    invoked_by: jinstr,
+                    to,
+                },
+            }),
+            Err(_) => Err(VerifyError {
+                from,
+                kind: VerifyErrorKind::InstrpOutOfBounds,
+            }),
+        }
+    };
+
+    let mut instrp = 0;
+
+    while let Some(nxti) = next_instr(m, instrp) {
+        let (nxtidelta, nxti) = nxti.map_err(|code| VerifyError {
+            from: instrp,
+            kind: VerifyErrorKind::UnparsableInstruction(code.to_vec()),
+        })?;
+        assert_ne!(nxtidelta, 0);
+        instrp += nxtidelta;
+        match nxti {
+            Instr::CallLocal(x) => check_jump("call-l", instrp, x)?,
+            Instr::CallLDefer(x) => check_jump("call-l-defer", instrp, x)?,
+            Instr::JumpCond(x) => check_jump("jump-cond", instrp, x)?,
+            Instr::Label
+            | Instr::CallRemote
+            | Instr::Return
+            | Instr::Push(_)
+            | Instr::Pop(_)
+            | Instr::Dup(_)
+            | Instr::Swap(_)
+            | Instr::Shift(_)
+            | Instr::DupFrom
+            | Instr::DoMath1(_)
+            | Instr::DoMath2(_) => {}
+        }
+    }
+
+    Ok(())
+}
+
 pub struct Process<'m> {
     pub stack: Vec<StackEntValue>,
     pub m: &'m [u8],
@@ -67,35 +128,12 @@ pub struct Process<'m> {
     pub instrp: usize,
 }
 
-fn verify_jumptarget_explicit(
-    jinstr: &'static str,
-    m: &[u8],
-    from: usize,
-    to: usize,
-) -> Result<(), Error> {
-    if to == 0 {
-        Ok(())
-    } else if let Some(Ok((_, Instr::Label))) = next_instr(m, to) {
-        Ok(())
-    } else {
-        Err(Error::InvalidJumpTarget {
-            invoked_by: jinstr,
-            from,
-            to,
-        })
-    }
-}
-
 impl Process<'_> {
-    fn verify_jumptarget(&self, jinstr: &'static str, from: usize) -> Result<(), Error> {
-        verify_jumptarget_explicit(jinstr, self.m, from, self.instrp)
-    }
-
     fn stpop(&mut self) -> Result<StackEntValue, Error> {
         self.stack.pop().ok_or(Error::NotEnoughStacked)
     }
 
-    pub fn run(&mut self, mut fuel: Option<&mut u64>) -> Result<(), Error> {
+    pub fn run(&mut self, mut fuel: Option<&mut u64>) -> Result<Option<u64>, Error> {
         loop {
             let previptr = self.instrp;
             tracing::trace!("previptr = {}", previptr);
@@ -117,7 +155,7 @@ impl Process<'_> {
                     if !is_call2jump(self.m, self.instrp) {
                         self.callstack.push(self.instrp);
                     }
-                    return Err(Error::RemoteCall(self.stpop()?));
+                    return Ok(Some(self.stpop()?));
                 }
                 Instr::CallLocal(x) => {
                     if !is_call2jump(self.m, self.instrp) {
@@ -127,12 +165,9 @@ impl Process<'_> {
                         Ok(y) => y,
                         Err(_) => return Err(Error::InstrpOutOfBounds),
                     };
-                    self.verify_jumptarget("call-l", previptr)?;
                 }
                 Instr::CallLDefer(x) => match x.try_into() {
-                    Ok(pos) => {
-                        let jtip = pos;
-                        verify_jumptarget_explicit("call-l-defer", self.m, previptr, jtip)?;
+                    Ok(jtip) => {
                         if is_call2jump(self.m, self.instrp) {
                             self.instrp = jtip;
                         } else {
@@ -148,12 +183,11 @@ impl Process<'_> {
                     };
                     if self.stpop()? != 0 {
                         self.instrp = x;
-                        self.verify_jumptarget("jump-cond", previptr)?;
                     }
                 }
                 Instr::Return => match self.callstack.pop() {
                     Some(x) => self.instrp = x,
-                    None => break Ok(()),
+                    None => return Ok(None),
                 },
                 Instr::Push(v) => self.stack.push(v),
                 Instr::Pop(_) if self.stack.is_empty() => return Err(Error::NotEnoughStacked),
@@ -253,6 +287,7 @@ mod tests {
 
         #[test]
         fn doesnt_crash(inp in proptest::collection::vec(0..=u8::MAX, 0..1024)) {
+            let _ = verify(&inp[..]);
             let mut p = Process {
                 stack: Vec::new(),
                 callstack: Vec::new(),