From 9c82f366fc1e0ae8088660abe51d3708d42132f8 Mon Sep 17 00:00:00 2001 From: Henry Gressmann Date: Mon, 15 Jan 2024 18:44:16 +0100 Subject: chore: change panics to UnimplementedFeature errors Signed-off-by: Henry Gressmann --- crates/parser/src/conversion.rs | 6 +-- crates/tinywasm/src/func.rs | 2 +- crates/tinywasm/src/runtime/executor/mod.rs | 16 ++++--- crates/tinywasm/src/runtime/stack/call_stack.rs | 49 ++++------------------ crates/tinywasm/src/store.rs | 55 +++++++++++++++++++++---- crates/types/src/lib.rs | 11 +++++ 6 files changed, 80 insertions(+), 59 deletions(-) (limited to 'crates') diff --git a/crates/parser/src/conversion.rs b/crates/parser/src/conversion.rs index b9e674e..7b36dcd 100644 --- a/crates/parser/src/conversion.rs +++ b/crates/parser/src/conversion.rs @@ -1,5 +1,5 @@ use alloc::{boxed::Box, format, string::ToString, vec::Vec}; -use log::info; +use log::debug; use tinywasm_types::{ BlockArgs, ConstInstruction, ElementItem, Export, ExternalKind, FuncType, Global, GlobalType, Import, ImportKind, Instruction, MemArg, MemoryArch, MemoryType, TableType, ValType, @@ -322,7 +322,7 @@ pub fn process_operators<'a>( let mut labels_ptrs = Vec::new(); // indexes into the instructions array for op in ops { - info!("op: {:?}", op); + debug!("op: {:?}", op); let op = op?; validator.op(offset, &op)?; @@ -359,7 +359,7 @@ pub fn process_operators<'a>( } End => { if let Some(label_pointer) = labels_ptrs.pop() { - info!("ending block: {:?}", instructions[label_pointer]); + debug!("ending block: {:?}", instructions[label_pointer]); let current_instr_ptr = instructions.len(); diff --git a/crates/tinywasm/src/func.rs b/crates/tinywasm/src/func.rs index b0b359d..7a6fd1c 100644 --- a/crates/tinywasm/src/func.rs +++ b/crates/tinywasm/src/func.rs @@ -68,7 +68,7 @@ impl FuncHandle { let result_m = func_ty.results.len(); // 1. Assert: m values are on the top of the stack (Ensured by validation) - debug_assert!(stack.values.len() >= result_m); + assert!(stack.values.len() >= result_m); // 2. Pop m values from the stack let res = stack.values.last_n(result_m)?; diff --git a/crates/tinywasm/src/runtime/executor/mod.rs b/crates/tinywasm/src/runtime/executor/mod.rs index 541c0a7..fe27d5a 100644 --- a/crates/tinywasm/src/runtime/executor/mod.rs +++ b/crates/tinywasm/src/runtime/executor/mod.rs @@ -7,7 +7,6 @@ use crate::{ CallFrame, Error, LabelArgs, ModuleInstance, Result, Store, }; use alloc::vec::Vec; -use log::info; use tinywasm_types::Instruction; mod macros; @@ -104,7 +103,7 @@ fn exec_one( store: &mut Store, module: &ModuleInstance, ) -> Result { - info!("ptr: {} instr: {:?}", cf.instr_ptr, instr); + debug!("ptr: {} instr: {:?}", cf.instr_ptr, instr); use tinywasm_types::Instruction::*; match instr { @@ -217,7 +216,11 @@ fn exec_one( .collect::>>()?; if instr.len() != *len { - panic!("Expected {} BrLabel instructions, got {}", len, instr.len()); + panic!( + "Expected {} BrLabel instructions, got {}, this should have been validated by the parser", + len, + instr.len() + ); } let idx = stack.values.pop_t::()? as usize; @@ -241,7 +244,7 @@ fn exec_one( }, EndFunc => { - debug_assert!( + assert!( cf.labels.len() == 0, "endfunc: block frames not empty, this should have been validated by the parser" ); @@ -499,7 +502,10 @@ fn exec_one( i => { log::error!("unimplemented instruction: {:?}", i); - panic!("Unimplemented instruction: {:?}", i) + return Err(Error::UnsupportedFeature(alloc::format!( + "unimplemented instruction: {:?}", + i + ))); } }; diff --git a/crates/tinywasm/src/runtime/stack/call_stack.rs b/crates/tinywasm/src/runtime/stack/call_stack.rs index 239659c..c4397c2 100644 --- a/crates/tinywasm/src/runtime/stack/call_stack.rs +++ b/crates/tinywasm/src/runtime/stack/call_stack.rs @@ -38,28 +38,13 @@ impl CallStack { Ok(self.stack.pop().unwrap()) } - #[inline] - pub(crate) fn _top(&self) -> Result<&CallFrame> { - assert!(self.top <= self.stack.len()); - if self.top == 0 { - return Err(Error::CallStackEmpty); - } - Ok(&self.stack[self.top - 1]) - } - - #[inline] - pub(crate) fn _top_mut(&mut self) -> Result<&mut CallFrame> { - assert!(self.top <= self.stack.len()); - if self.top == 0 { - return Err(Error::CallStackEmpty); - } - Ok(&mut self.stack[self.top - 1]) - } - #[inline] pub(crate) fn push(&mut self, call_frame: CallFrame) { - assert!(self.top <= self.stack.len()); - assert!(self.stack.len() <= CALL_STACK_MAX_SIZE); + assert!(self.top <= self.stack.len(), "stack is too small"); + assert!( + self.stack.len() <= CALL_STACK_MAX_SIZE, + "call stack size exceeded, this should have been caught" + ); self.top += 1; self.stack.push(call_frame); @@ -68,7 +53,6 @@ impl CallStack { #[derive(Debug, Clone)] pub(crate) struct CallFrame { - // having real pointers here would be nice :( but we can't really do that in safe rust pub(crate) instr_ptr: usize, pub(crate) func_ptr: usize, @@ -115,19 +99,6 @@ impl CallFrame { } } - // self.instr_ptr = block_frame.instr_ptr; - // value_stack.trim(block_frame.stack_ptr); - - // // // Adjusting how to trim the blocks stack based on the block type - // // let trim_index = match block_frame.block { - // // // if we are breaking to a loop, we want to jump back to the start of the loop - // // BlockFrameInner::Loop => block_index as usize - 1, - // // // if we are breaking to any other block, we want to jump to the end of the block - // // // TODO: check if this is correct - // // BlockFrameInner::If | BlockFrameInner::Else | BlockFrameInner::Block => block_index as usize - 1, - // // }; - - // self.block_frames.trim(block_index as usize); Some(()) } @@ -155,19 +126,13 @@ impl CallFrame { #[inline] pub(crate) fn set_local(&mut self, local_index: usize, value: RawWasmValue) { - if local_index >= self.local_count { - panic!("Invalid local index"); - } - + assert!(local_index < self.local_count, "Invalid local index"); self.locals[local_index] = value; } #[inline] pub(crate) fn get_local(&self, local_index: usize) -> RawWasmValue { - if local_index >= self.local_count { - panic!("Invalid local index"); - } - + assert!(local_index < self.local_count, "Invalid local index"); self.locals[local_index] } } diff --git a/crates/tinywasm/src/store.rs b/crates/tinywasm/src/store.rs index 564232d..73eb503 100644 --- a/crates/tinywasm/src/store.rs +++ b/crates/tinywasm/src/store.rs @@ -202,6 +202,21 @@ impl Store { Ok(global_addrs) } + pub(crate) fn eval_i32_const(&self, const_instr: &tinywasm_types::ConstInstruction) -> Result { + use tinywasm_types::ConstInstruction::*; + let val = match const_instr { + I32Const(i) => *i, + GlobalGet(addr) => { + let addr = *addr as usize; + let global = self.data.globals[addr].clone(); + let val = global.borrow().value; + i32::from(val) + } + _ => return Err(Error::Other("expected i32".to_string())), + }; + Ok(val) + } + pub(crate) fn eval_const(&self, const_instr: &tinywasm_types::ConstInstruction) -> Result { use tinywasm_types::ConstInstruction::*; let val = match const_instr { @@ -227,27 +242,50 @@ impl Store { let elem_count = self.data.elems.len(); let mut elem_addrs = Vec::with_capacity(elem_count); for (i, elem) in elems.into_iter().enumerate() { - match elem.kind { + let items = match elem.kind { // doesn't need to be initialized, can be initialized lazily using the `table.init` instruction - ElementKind::Passive => {} + ElementKind::Passive => None, + // TODO: ElementKind::Passive => Some(elem.items.iter().map(|item| item.addr()).collect()), // this one is active, so we need to initialize it (essentially a `table.init` instruction) - ElementKind::Active { .. } => { + ElementKind::Active { offset, table } => { + let init = elem + .items + .iter() + .map(|item| { + item.addr().ok_or_else(|| { + Error::UnsupportedFeature(format!("const expression other than ref: {:?}", item)) + }) + }) + .collect::>>()?; + // a. Let n be the length of the vector elem[i].init + let n = elem.items.len(); + // b. Execute the instruction sequence einstrs + let table_idx = self.eval_i32_const(&offset)? as usize; + // c. Execute the instruction i32.const 0 + let elem_idx = 0; + // d. Execute the instruction i32.const n + let elem_count = n; + // e. Execute the instruction table.init tableidx i + // self.data.tables[table_idx].elements[elem_idx..elem_count].copy_from_slice(&init); + // f. Execute the instruction elm.drop i + None } // this one is not available to the runtime but needs to be initialized to declare references ElementKind::Declared => { // a. Execute the instruction elm.drop i + None } - } + }; - self.data.elems.push(ElemInstance::new(elem.kind, idx)); + self.data.elems.push(ElemInstance::new(elem.kind, idx, items)); elem_addrs.push((i + elem_count) as Addr); } @@ -377,7 +415,7 @@ pub(crate) struct MemoryInstance { impl MemoryInstance { pub(crate) fn new(kind: MemoryType, owner: ModuleInstanceAddr) -> Self { - debug_assert!(kind.page_count_initial <= kind.page_count_max.unwrap_or(MAX_PAGES as u64)); + assert!(kind.page_count_initial <= kind.page_count_max.unwrap_or(MAX_PAGES as u64)); log::debug!("initializing memory with {} pages", kind.page_count_initial); Self { @@ -480,12 +518,13 @@ impl GlobalInstance { #[derive(Debug)] pub(crate) struct ElemInstance { kind: ElementKind, + items: Option>, // none is the element was dropped owner: ModuleInstanceAddr, // index into store.module_instances } impl ElemInstance { - pub(crate) fn new(kind: ElementKind, owner: ModuleInstanceAddr) -> Self { - Self { kind, owner } + pub(crate) fn new(kind: ElementKind, owner: ModuleInstanceAddr, items: Option>) -> Self { + Self { kind, owner, items } } } diff --git a/crates/types/src/lib.rs b/crates/types/src/lib.rs index d3bf81a..4ea7b55 100644 --- a/crates/types/src/lib.rs +++ b/crates/types/src/lib.rs @@ -434,3 +434,14 @@ pub enum ElementItem { Func(FuncAddr), Expr(ConstInstruction), } + +impl ElementItem { + pub fn addr(&self) -> Option { + match self { + Self::Func(addr) => Some(*addr), + Self::Expr(ConstInstruction::RefFunc(addr)) => Some(*addr), + Self::Expr(ConstInstruction::RefNull(_ty)) => Some(0), + _ => None, + } + } +} -- cgit v1.3.1