From fc974dd56f72aee208fbbe69dcf49aecc351eed4 Mon Sep 17 00:00:00 2001 From: George Cosma Date: Thu, 25 Jul 2024 13:47:00 +0300 Subject: [PATCH] chore: Convert panics to errors Signed-off-by: George Cosma --- src/core/error.rs | 18 +++++++++- src/core/reader/mod.rs | 2 +- src/core/reader/section_header.rs | 2 +- src/core/reader/types/import.rs | 6 ++-- src/execution/mod.rs | 59 ++++++++++++++++++------------- src/validation/mod.rs | 12 +++++-- 6 files changed, 66 insertions(+), 33 deletions(-) diff --git a/src/core/error.rs b/src/core/error.rs index 121f7363..f9ac034a 100644 --- a/src/core/error.rs +++ b/src/core/error.rs @@ -1,4 +1,4 @@ -use crate::core::indices::GlobalIdx; +use crate::core::indices::{FuncIdx, GlobalIdx}; use core::fmt::{Display, Formatter}; use core::str::Utf8Error; @@ -38,8 +38,12 @@ pub enum Error { InvalidMutType(u8), MoreThanOneMemory, InvalidGlobalIdx(GlobalIdx), + InvalidFunctionIdx(FuncIdx), GlobalIsConst, RuntimeError(RuntimeError), + NotYetImplemented(&'static str), + InvalidParameterTypes, + InvalidResultTypes, } impl Display for Error { @@ -108,12 +112,24 @@ impl Display for Error { Error::InvalidGlobalIdx(idx) => f.write_fmt(format_args!( "An invalid global index `{idx}` was specified" )), + Error::InvalidFunctionIdx(idx) => f.write_fmt(format_args!( + "An invalid function index `{idx}` was specified" + )), Error::GlobalIsConst => f.write_str("A const global cannot be written to"), Error::RuntimeError(err) => match err { RuntimeError::DivideBy0 => f.write_str("Divide by zero is not permitted"), RuntimeError::UnrepresentableResult => f.write_str("Result is unrepresentable"), RuntimeError::FunctionNotFound => f.write_str("Function not found"), }, + Error::NotYetImplemented(msg) => { + f.write_fmt(format_args!("Not yet implemented: {msg}")) + } + Error::InvalidParameterTypes => { + f.write_str("The given parameter types do not match the function's signature") + } + Error::InvalidResultTypes => { + f.write_str("The given result types do not match the function's signature") + } } } } diff --git a/src/core/reader/mod.rs b/src/core/reader/mod.rs index 545f25a8..f21429ed 100644 --- a/src/core/reader/mod.rs +++ b/src/core/reader/mod.rs @@ -89,7 +89,7 @@ impl<'a> WasmReader<'a> { let bytes = &self.full_wasm_binary[self.pc..(self.pc + N)]; self.pc += N; - Ok(bytes.try_into().expect("the slice length to be exactly N")) + bytes.try_into().map_err(|_err| Error::Eof) } /// Read the current byte without advancing the [`pc`](Self::pc) diff --git a/src/core/reader/section_header.rs b/src/core/reader/section_header.rs index 29fdccfa..7f88b83e 100644 --- a/src/core/reader/section_header.rs +++ b/src/core/reader/section_header.rs @@ -87,7 +87,7 @@ impl WasmReadable for SectionHeader { let size: u32 = wasm.read_var_u32().unwrap_validated(); let contents_span = wasm .make_span(size as usize) - .expect("TODO remove this expect"); + .expect("Expected to be able to read section due to prior validation"); SectionHeader { ty, diff --git a/src/core/reader/types/import.rs b/src/core/reader/types/import.rs index 966ae947..75833b21 100644 --- a/src/core/reader/types/import.rs +++ b/src/core/reader/types/import.rs @@ -60,9 +60,9 @@ impl WasmReadable for ImportDesc { fn read(wasm: &mut WasmReader) -> Result { let desc = match wasm.read_u8()? { 0x00 => Self::Func(wasm.read_var_u32()? as TypeIdx), - 0x01 => todo!("read TableType"), - 0x02 => todo!("read MemType"), - 0x03 => todo!("read GlobalType"), + 0x01 => return Err(Error::NotYetImplemented("read TableType")), + 0x02 => return Err(Error::NotYetImplemented("read MemType")), + 0x03 => return Err(Error::NotYetImplemented("read GlobalType")), other => return Err(Error::InvalidImportDesc(other)), }; diff --git a/src/execution/mod.rs b/src/execution/mod.rs index abba6568..4ab8a333 100644 --- a/src/execution/mod.rs +++ b/src/execution/mod.rs @@ -14,7 +14,7 @@ use crate::execution::store::{FuncInst, GlobalInst, MemInst, Store}; use crate::execution::value::Value; use crate::validation::code::read_declared_locals; use crate::value::InteropValueList; -use crate::Error::RuntimeError; +use crate::Error::{self, RuntimeError}; use crate::RuntimeError::{DivideBy0, FunctionNotFound, UnrepresentableResult}; use crate::{Result, ValidationInfo}; @@ -51,7 +51,7 @@ where pub fn new_with_hooks(validation_info: &'_ ValidationInfo<'b>, hook_set: H) -> Result { trace!("Starting instantiation of bytecode"); - let store = Self::init_store(validation_info); + let store = Self::init_store(validation_info)?; let mut instance = RuntimeInstance { wasm_bytecode: validation_info.wasm, @@ -100,15 +100,20 @@ where params: Param, ) -> Result { // -=-= Verification =-=- - let func_inst = self.store.funcs.get(func_idx).expect("valid FuncIdx"); + // TODO(george-cosma): Do we want this to be a RuntimeError(FunctionNotFound)? + let func_inst = self + .store + .funcs + .get(func_idx) + .ok_or(Error::InvalidFunctionIdx(func_idx))?; let func_ty = self.types.get(func_inst.ty).unwrap_validated(); // Check correct function parameters and return types if func_ty.params.valtypes != Param::TYS { - panic!("Invalid `Param` generics"); + return Err(Error::InvalidParameterTypes); } if func_ty.returns.valtypes != Returns::TYS { - panic!("Invalid `Returns` generics"); + return Err(Error::InvalidResultTypes); } // -=-= Invoke the function =-=- @@ -143,19 +148,24 @@ where ret_types: &[ValType], ) -> Result> { // -=-= Verification =-=- - let func_inst = self.store.funcs.get(func_idx).expect("valid FuncIdx"); + // TODO(george-cosma): Do we want this to be a RuntimeError(FunctionNotFound)? + let func_inst = self + .store + .funcs + .get(func_idx) + .ok_or(Error::InvalidFunctionIdx(func_idx))?; let func_ty = self.types.get(func_inst.ty).unwrap_validated(); // Verify that the given parameters match the function parameters let param_types = params.iter().map(|v| v.to_ty()).collect::>(); if func_ty.params.valtypes != param_types { - panic!("Invalid parameters for function"); + return Err(Error::InvalidParameterTypes); } // Verify that the given return types match the function return types if func_ty.returns.valtypes != ret_types { - panic!("Invalid return types for function"); + return Err(Error::InvalidResultTypes); } // -=-= Invoke the function =-=- @@ -169,7 +179,12 @@ where let error = self.function(func_idx, &mut stack); error?; - let func_inst = self.store.funcs.get(func_idx).expect("valid FuncIdx"); + // TODO(george-cosma): Do we want this to be a RuntimeError(FunctionNotFound)? + let func_inst = self + .store + .funcs + .get(func_idx) + .ok_or(Error::InvalidFunctionIdx(func_idx))?; let func_ty = self.types.get(func_inst.ty).unwrap_validated(); // Pop return values from stack @@ -263,9 +278,9 @@ where let address = address as usize; mem.data.get(address..(address + 4)) }) - .expect("TODO trap here"); + .expect("TODO trap here"); // ??? - let data: [u8; 4] = data.try_into().expect("this to be exactly 4 bytes"); + let data: [u8; 4] = data.try_into().expect("this to be exactly 4 bytes"); // ??? u32::from_le_bytes(data) }; @@ -289,7 +304,7 @@ where let address = address as usize; mem.data.get_mut(address..(address + 4)) }) - .expect("TODO trap here"); + .expect("TODO trap here"); // ??? memory_location.copy_from_slice(&data_to_store.to_le_bytes()); trace!("Instruction: i32.store [{relative_address} {data_to_store}] -> []"); @@ -663,7 +678,7 @@ where Ok(()) } - fn init_store(validation_info: &ValidationInfo) -> Store { + fn init_store(validation_info: &ValidationInfo) -> Result { let function_instances: Vec = { let mut wasm_reader = WasmReader::new(validation_info.wasm); @@ -673,25 +688,21 @@ where functions .zip(func_blocks) .map(|(ty, func)| { - wasm_reader - .move_start_to(*func) - .expect("function index to be in the bounds of the WASM binary"); + wasm_reader.move_start_to(*func)?; let (locals, bytes_read) = wasm_reader .measure_num_read_bytes(read_declared_locals) .unwrap_validated(); - let code_expr = wasm_reader - .make_span(func.len() - bytes_read) - .expect("TODO remove this expect"); + let code_expr = wasm_reader.make_span(func.len() - bytes_read)?; - FuncInst { + Ok(FuncInst { ty: *ty, locals, code_expr, - } + }) }) - .collect() + .collect::>>()? }; let memory_instances: Vec = validation_info @@ -714,10 +725,10 @@ where }) .collect(); - Store { + Ok(Store { funcs: function_instances, mems: memory_instances, globals: global_instances, - } + }) } } diff --git a/src/validation/mod.rs b/src/validation/mod.rs index 9d8f08d2..c6141c28 100644 --- a/src/validation/mod.rs +++ b/src/validation/mod.rs @@ -121,13 +121,17 @@ pub fn validate(wasm: &[u8]) -> Result { while (skip_section(&mut wasm, &mut header)?).is_some() {} let _: Option<()> = handle_section(&mut wasm, &mut header, SectionTy::Element, |_, _| { - todo!("element section not yet supported") + Err(Error::NotYetImplemented( + "element section not yet supported", + )) })?; while (skip_section(&mut wasm, &mut header)?).is_some() {} let _: Option<()> = handle_section(&mut wasm, &mut header, SectionTy::DataCount, |_, _| { - todo!("data count section not yet supported") + Err(Error::NotYetImplemented( + "data count section not yet supported", + )) })?; while (skip_section(&mut wasm, &mut header)?).is_some() {} @@ -142,7 +146,7 @@ pub fn validate(wasm: &[u8]) -> Result { while (skip_section(&mut wasm, &mut header)?).is_some() {} let _: Option<()> = handle_section(&mut wasm, &mut header, SectionTy::Data, |_, _| { - todo!("data section not yet supported") + Err(Error::NotYetImplemented("data section not yet supported")) })?; while (skip_section(&mut wasm, &mut header)?).is_some() {} @@ -183,6 +187,8 @@ fn handle_section Result>( ) -> Result> { match &header { Some(SectionHeader { ty, .. }) if *ty == section_ty => { + // We just checked that the header is of type "Some", so + // it is safe to unwrap here. let h = header.take().unwrap(); trace!("Handling section {:?}", h.ty); let ret = handler(wasm, h)?;