From 53e42916d1af1c974bced08f42aa218cd458e573 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Mon, 13 Nov 2023 11:44:18 +0100 Subject: [PATCH 1/4] Explain what is unsupported in the interpreter This is a first step towards #42, but the most useful one for now. --- crates/interpreter/CHANGELOG.md | 6 ++++++ crates/interpreter/Cargo.lock | 2 +- crates/interpreter/Cargo.toml | 2 +- crates/interpreter/src/error.rs | 18 ++++++++++++++--- crates/interpreter/src/lib.rs | 12 +++++++++++ crates/interpreter/src/parser.rs | 34 +++++++++++++++++++------------- crates/interpreter/src/toctou.rs | 8 ++++---- crates/interpreter/tests/spec.rs | 12 +++++------ crates/runner-host/Cargo.lock | 2 +- crates/runner-nordic/Cargo.lock | 2 +- crates/scheduler/Cargo.lock | 2 +- crates/scheduler/Cargo.toml | 2 +- 12 files changed, 69 insertions(+), 33 deletions(-) diff --git a/crates/interpreter/CHANGELOG.md b/crates/interpreter/CHANGELOG.md index c7b0715a..f51b26ea 100644 --- a/crates/interpreter/CHANGELOG.md +++ b/crates/interpreter/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## 0.2.0 + +### Major + +- Change `Error::Unsupported` to take a reason with the `debug` feature + ## 0.1.4 ### Patch diff --git a/crates/interpreter/Cargo.lock b/crates/interpreter/Cargo.lock index a078ec93..49cd5915 100644 --- a/crates/interpreter/Cargo.lock +++ b/crates/interpreter/Cargo.lock @@ -107,7 +107,7 @@ checksum = "e51733f11c9c4f72aa0c160008246859e340b00807569a0da0e7a1079b27ba85" [[package]] name = "wasefire-interpreter" -version = "0.1.4" +version = "0.2.0-git" dependencies = [ "lazy_static", "libm", diff --git a/crates/interpreter/Cargo.toml b/crates/interpreter/Cargo.toml index 8525d035..e6ff17ea 100644 --- a/crates/interpreter/Cargo.toml +++ b/crates/interpreter/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "wasefire-interpreter" -version = "0.1.4" +version = "0.2.0-git" authors = ["Julien Cretin "] license = "Apache-2.0" publish = true diff --git a/crates/interpreter/src/error.rs b/crates/interpreter/src/error.rs index 1379fa95..4fb35fd0 100644 --- a/crates/interpreter/src/error.rs +++ b/crates/interpreter/src/error.rs @@ -22,13 +22,25 @@ pub enum Error { NotFound, /// An argument is not supported. - Unsupported, + Unsupported(Unsupported), /// Execution trapped. // TODO: In debug mode, trap could describe the reason (like resource exhaustion, etc). Trap, } +#[cfg(not(feature = "debug"))] +pub type Unsupported = (); + +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +#[non_exhaustive] +#[cfg(feature = "debug")] +pub enum Unsupported { + Opcode(u8), + OpcodeFc(u32), + MaxLocals, +} + #[cfg(feature = "debug")] pub fn print_backtrace() { let backtrace = std::backtrace::Backtrace::capture(); @@ -49,10 +61,10 @@ pub fn not_found() -> Error { Error::NotFound } -pub fn unsupported() -> Error { +pub fn unsupported(reason: Unsupported) -> Error { #[cfg(feature = "debug")] print_backtrace(); - Error::Unsupported + Error::Unsupported(reason) } pub fn trap() -> Error { diff --git a/crates/interpreter/src/lib.rs b/crates/interpreter/src/lib.rs index 1320dc56..a4baa639 100644 --- a/crates/interpreter/src/lib.rs +++ b/crates/interpreter/src/lib.rs @@ -116,6 +116,16 @@ extern crate alloc; +macro_rules! if_debug { + ($unit:expr) => {{ + #[cfg(feature = "debug")] + let unit = $unit; + #[cfg(not(feature = "debug"))] + let unit = (); + unit + }}; +} + macro_rules! support_if { ($feature:literal[$($var:ident)*], $then:expr, $else:expr) => {{ #[cfg(feature = $feature)] @@ -141,6 +151,8 @@ mod toctou; mod valid; pub use error::Error; +#[cfg(feature = "debug")] +pub use error::Unsupported; pub use exec::{Call, InstId, RunAnswer, RunResult, Store, StoreId, Val, MEMORY_ALIGN}; pub use module::Module; pub use syntax::{GlobalType, ImportDesc, Limits, Mut, RefType, TableType, ValType}; diff --git a/crates/interpreter/src/parser.rs b/crates/interpreter/src/parser.rs index 1dfca303..34e3134c 100644 --- a/crates/interpreter/src/parser.rs +++ b/crates/interpreter/src/parser.rs @@ -15,6 +15,8 @@ use alloc::vec::Vec; use core::marker::PhantomData; +#[cfg(feature = "debug")] +use crate::error::*; use crate::syntax::*; use crate::toctou::*; @@ -304,7 +306,7 @@ impl<'m, M: Mode> Parser<'m, M> { x @ 0x2a ..= 0x2b => support_if!( "float-types"[x], Instr::FLoad((x - 0x2a).into(), self.parse_memarg()?), - M::unsupported()? + M::unsupported(if_debug!(Unsupported::Opcode(x)))? ), x @ 0x2c ..= 0x35 => Instr::ILoad_( ((x - 0x2c) / 2).into(), @@ -315,7 +317,7 @@ impl<'m, M: Mode> Parser<'m, M> { x @ 0x38 ..= 0x39 => support_if!( "float-types"[x], Instr::FStore((x - 0x38).into(), self.parse_memarg()?), - M::unsupported()? + M::unsupported(if_debug!(Unsupported::Opcode(x)))? ), x @ 0x3a ..= 0x3e => Instr::IStore_((x - 0x3a).into(), self.parse_memarg()?), 0x3f => { @@ -331,12 +333,12 @@ impl<'m, M: Mode> Parser<'m, M> { 0x43 => support_if!( "float-types"[], Instr::F32Const(u32::from_le_bytes(self.parse_bytes(4)?.try_into().unwrap())), - M::unsupported()? + M::unsupported(if_debug!(Unsupported::Opcode(0x43)))? ), 0x44 => support_if!( "float-types"[], Instr::F64Const(u64::from_le_bytes(self.parse_bytes(8)?.try_into().unwrap())), - M::unsupported()? + M::unsupported(if_debug!(Unsupported::Opcode(0x44)))? ), x @ 0x45 ..= 0x5a => { let n = Nx::from((x - 0x45) / 11); @@ -348,7 +350,7 @@ impl<'m, M: Mode> Parser<'m, M> { x @ 0x5b ..= 0x66 => support_if!( "float-types"[x], Instr::FRelOp(((x - 0x5b) / 6).into(), ((x - 0x5b) % 6).into()), - M::unsupported()? + M::unsupported(if_debug!(Unsupported::Opcode(x)))? ), x @ 0x67 ..= 0x8a => { let n = Nx::from((x - 0x67) / 18); @@ -366,7 +368,7 @@ impl<'m, M: Mode> Parser<'m, M> { y => Instr::FBinOp(n, (y - 7).into()), } }, - M::unsupported()? + M::unsupported(if_debug!(Unsupported::Opcode(x)))? ), 0xa7 => Instr::CvtOp(CvtOp::Wrap), x @ 0xa8 ..= 0xab => support_if!( @@ -376,7 +378,7 @@ impl<'m, M: Mode> Parser<'m, M> { ((x - 0xa8) / 2).into(), ((x - 0xa8) % 2).into(), )), - M::unsupported()? + M::unsupported(if_debug!(Unsupported::Opcode(x)))? ), x @ 0xac ..= 0xad => Instr::CvtOp(CvtOp::Extend((x - 0xac).into())), x @ 0xae ..= 0xb1 => support_if!( @@ -386,7 +388,7 @@ impl<'m, M: Mode> Parser<'m, M> { ((x - 0xae) / 2).into(), ((x - 0xae) % 2).into(), )), - M::unsupported()? + M::unsupported(if_debug!(Unsupported::Opcode(x)))? ), x @ 0xb2 ..= 0xbb => support_if!( "float-types"[x], @@ -400,17 +402,17 @@ impl<'m, M: Mode> Parser<'m, M> { y => Instr::CvtOp(CvtOp::Convert(n, (y / 2).into(), (y % 2).into())), } }, - M::unsupported()? + M::unsupported(if_debug!(Unsupported::Opcode(x)))? ), x @ 0xbc ..= 0xbd => support_if!( "float-types"[x], Instr::CvtOp(CvtOp::IReinterpret((x - 0xbc).into())), - M::unsupported()? + M::unsupported(if_debug!(Unsupported::Opcode(x)))? ), x @ 0xbe ..= 0xbf => support_if!( "float-types"[x], Instr::CvtOp(CvtOp::FReinterpret((x - 0xbe).into())), - M::unsupported()? + M::unsupported(if_debug!(Unsupported::Opcode(x)))? ), x @ 0xc0 ..= 0xc4 => Instr::IExtend((x - 0xc0).into()), 0xd0 => Instr::RefNull(self.parse_reftype()?), @@ -424,7 +426,7 @@ impl<'m, M: Mode> Parser<'m, M> { ((x & 2 != 0) as u8).into(), ((x & 1) as u8).into(), )), - M::unsupported()? + M::unsupported(if_debug!(Unsupported::OpcodeFc(x)))? ), 8 => { let x = self.parse_dataidx()?; @@ -453,7 +455,11 @@ impl<'m, M: Mode> Parser<'m, M> { 17 => Instr::TableFill(self.parse_tableidx()?), _ => M::invalid()?, }, - 0xfd => support_if!("vector-types"[], unimplemented!(), M::unsupported()?), + 0xfd => support_if!( + "vector-types"[], + unimplemented!(), + M::unsupported(if_debug!(Unsupported::Opcode(0xfd)))? + ), _ => M::invalid()?, }) } @@ -462,7 +468,7 @@ impl<'m, M: Mode> Parser<'m, M> { for _ in 0 .. self.parse_vec()? { let len = self.parse_u32()? as usize; if locals.len().checked_add(len).map_or(true, |x| x > MAX_LOCALS) { - return M::unsupported(); + return M::unsupported(if_debug!(Unsupported::MaxLocals)); } let val = self.parse_valtype()?; locals.extend(core::iter::repeat(val).take(len)); diff --git a/crates/interpreter/src/toctou.rs b/crates/interpreter/src/toctou.rs index 732194f3..7dd9476b 100644 --- a/crates/interpreter/src/toctou.rs +++ b/crates/interpreter/src/toctou.rs @@ -20,7 +20,7 @@ pub trait Mode { fn invalid() -> Result; - fn unsupported() -> Result; + fn unsupported(reason: Unsupported) -> Result; fn check(cond: impl FnOnce() -> bool) -> Result<(), Self::Error>; @@ -41,8 +41,8 @@ impl Mode for Check { Err(invalid()) } - fn unsupported() -> Result { - Err(unsupported()) + fn unsupported(reason: Unsupported) -> Result { + Err(unsupported(reason)) } fn check(cond: impl FnOnce() -> bool) -> Result<(), Self::Error> { @@ -75,7 +75,7 @@ impl Mode for Use { unreachable!() } - fn unsupported() -> Result { + fn unsupported(_: Unsupported) -> Result { Self::invalid() } diff --git a/crates/interpreter/tests/spec.rs b/crates/interpreter/tests/spec.rs index 64915e55..6cdfa933 100644 --- a/crates/interpreter/tests/spec.rs +++ b/crates/interpreter/tests/spec.rs @@ -41,7 +41,7 @@ fn test(name: &str) { match directive { WastDirective::Wat(QuoteWat::Wat(Wat::Module(mut m))) => { env.instantiate(name, &m.encode().unwrap()); - if !matches!(env.inst, Err(Error::Unsupported)) { + if !matches!(env.inst, Err(Error::Unsupported(_))) { env.register_id(m.id, env.inst.unwrap()); } } @@ -120,7 +120,7 @@ impl<'m> Env<'m> { fn set_inst(&mut self, inst: Result) { match inst { - Ok(_) | Err(Error::Unsupported) => (), + Ok(_) | Err(Error::Unsupported(_)) => (), Err(e) => panic!("{e:?}"), } self.inst = inst; @@ -296,7 +296,7 @@ fn assert_invoke(env: &mut Env, invoke: WastInvoke) { fn assert_malformed(mut wat: QuoteWat) { if let Ok(wasm) = wat.encode() { let module = Module::new(&wasm); - if !matches!(module, Err(Error::Unsupported)) { + if !matches!(module, Err(Error::Unsupported(_))) { assert_eq!(module.err(), Some(Error::Invalid)); } } @@ -305,21 +305,21 @@ fn assert_malformed(mut wat: QuoteWat) { fn assert_invalid(mut wat: QuoteWat) { let wasm = wat.encode().unwrap(); let module = Module::new(&wasm); - if !matches!(module, Err(Error::Unsupported)) { + if !matches!(module, Err(Error::Unsupported(_))) { assert_eq!(module.err(), Some(Error::Invalid)); } } fn assert_exhaustion(env: &mut Env, call: WastInvoke) { let result = wast_invoke(env, call); - if !matches!(result, Err(Error::Unsupported)) { + if !matches!(result, Err(Error::Unsupported(_))) { assert_eq!(result, Err(Error::Trap)); } } fn assert_unlinkable(env: &mut Env, mut wat: Wat) { let inst = env.maybe_instantiate("", &wat.encode().unwrap()); - if !matches!(inst, Err(Error::Unsupported)) { + if !matches!(inst, Err(Error::Unsupported(_))) { assert_eq!(inst.err(), Some(Error::NotFound)); } } diff --git a/crates/runner-host/Cargo.lock b/crates/runner-host/Cargo.lock index 2265d5bf..ce45421e 100644 --- a/crates/runner-host/Cargo.lock +++ b/crates/runner-host/Cargo.lock @@ -1756,7 +1756,7 @@ dependencies = [ [[package]] name = "wasefire-interpreter" -version = "0.1.4" +version = "0.2.0-git" dependencies = [ "num_enum", "paste", diff --git a/crates/runner-nordic/Cargo.lock b/crates/runner-nordic/Cargo.lock index 097e79e4..d7f0672b 100644 --- a/crates/runner-nordic/Cargo.lock +++ b/crates/runner-nordic/Cargo.lock @@ -1053,7 +1053,7 @@ dependencies = [ [[package]] name = "wasefire-interpreter" -version = "0.1.4" +version = "0.2.0-git" dependencies = [ "num_enum", "paste", diff --git a/crates/scheduler/Cargo.lock b/crates/scheduler/Cargo.lock index 1d92f7f9..dffdc940 100644 --- a/crates/scheduler/Cargo.lock +++ b/crates/scheduler/Cargo.lock @@ -444,7 +444,7 @@ dependencies = [ [[package]] name = "wasefire-interpreter" -version = "0.1.4" +version = "0.2.0-git" dependencies = [ "num_enum", "paste", diff --git a/crates/scheduler/Cargo.toml b/crates/scheduler/Cargo.toml index 3b95edc8..96db0c7b 100644 --- a/crates/scheduler/Cargo.toml +++ b/crates/scheduler/Cargo.toml @@ -26,7 +26,7 @@ wasefire-logger = { version = "0.1.3", path = "../logger" } wasefire-store = { version = "0.2.2", path = "../store" } [dependencies.wasefire-interpreter] -version = "0.1.4" +version = "0.2.0-git" path = "../interpreter" features = ["toctou"] optional = true From b806d604e31fad48299ecca1183bc72a661993c1 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Mon, 13 Nov 2023 11:56:40 +0100 Subject: [PATCH 2/4] Fix changelog --- crates/interpreter/CHANGELOG.md | 2 +- crates/scheduler/CHANGELOG.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/interpreter/CHANGELOG.md b/crates/interpreter/CHANGELOG.md index f51b26ea..4eccd2b9 100644 --- a/crates/interpreter/CHANGELOG.md +++ b/crates/interpreter/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## 0.2.0 +## 0.2.0-git ### Major diff --git a/crates/scheduler/CHANGELOG.md b/crates/scheduler/CHANGELOG.md index fe7312c7..5a6dbbae 100644 --- a/crates/scheduler/CHANGELOG.md +++ b/crates/scheduler/CHANGELOG.md @@ -81,4 +81,4 @@ ## 0.1.0 - + From 4a020ef319d7196e6372e0e93a96a30ab4e91018 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Mon, 13 Nov 2023 11:58:36 +0100 Subject: [PATCH 3/4] Fix visibility of Unsupported --- crates/interpreter/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/interpreter/src/lib.rs b/crates/interpreter/src/lib.rs index a4baa639..886e5a81 100644 --- a/crates/interpreter/src/lib.rs +++ b/crates/interpreter/src/lib.rs @@ -150,9 +150,7 @@ mod syntax; mod toctou; mod valid; -pub use error::Error; -#[cfg(feature = "debug")] -pub use error::Unsupported; +pub use error::{Error, Unsupported}; pub use exec::{Call, InstId, RunAnswer, RunResult, Store, StoreId, Val, MEMORY_ALIGN}; pub use module::Module; pub use syntax::{GlobalType, ImportDesc, Limits, Mut, RefType, TableType, ValType}; From 94459c61586a31e9d6398ed552f91a27d039836d Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Mon, 13 Nov 2023 12:00:43 +0100 Subject: [PATCH 4/4] Fix changelog --- crates/interpreter/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/interpreter/CHANGELOG.md b/crates/interpreter/CHANGELOG.md index 4eccd2b9..bbc67b6d 100644 --- a/crates/interpreter/CHANGELOG.md +++ b/crates/interpreter/CHANGELOG.md @@ -38,4 +38,4 @@ ## 0.1.0 - +