chore: Convert panics to errors

george-cosma · george-cosma · commit ce788d8fe288 · 2024-07-25T13:47:00.000+03:00
diff --git a/src/core/error.rs b/src/core/error.rs
@@ -5,6 +5,8 @@ use core::str::Utf8Error;
 use crate::core::reader::section_header::SectionTy;
 use crate::core::reader::types::ValType;
 
+use super::indices::FuncIdx;
+
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum RuntimeError {
     DivideBy0,
@@ -38,8 +40,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 +114,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
@@ -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"))
+        Ok(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
@@ -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
@@ -60,9 +60,9 @@ impl WasmReadable for ImportDesc {
     fn read(wasm: &mut WasmReader) -> Result<Self> {
         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
@@ -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<Self> {
         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,16 @@ where
         params: Param,
     ) -> Result<Returns> {
         // -=-= 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 +144,20 @@ where
         ret_types: &[ValType],
     ) -> Result<Vec<Value>> {
         // -=-= 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::<Vec<_>>();
 
         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,9 +171,10 @@ 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
         let return_values = func_ty
             .returns
@@ -263,9 +266,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 +292,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 +666,7 @@ where
         Ok(())
     }
 
-    fn init_store(validation_info: &ValidationInfo) -> Store {
+    fn init_store(validation_info: &ValidationInfo) -> Result<Store> {
         let function_instances: Vec<FuncInst> = {
             let mut wasm_reader = WasmReader::new(validation_info.wasm);
 
@@ -674,24 +677,22 @@ where
                 .zip(func_blocks)
                 .map(|(ty, func)| {
                     wasm_reader
-                        .move_start_to(*func)
-                        .expect("function index to be in the bounds of the WASM binary");
+                        .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");
+                        .make_span(func.len() - bytes_read)?;
 
-                    FuncInst {
+                    Ok(FuncInst {
                         ty: *ty,
                         locals,
                         code_expr,
-                    }
+                    })
                 })
-                .collect()
+                .collect::<Result<Vec<FuncInst>>>()?
         };
 
         let memory_instances: Vec<MemInst> = validation_info
@@ -714,10 +715,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
@@ -121,13 +121,13 @@ pub fn validate(wasm: &[u8]) -> Result<ValidationInfo> {
     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 +142,7 @@ pub fn validate(wasm: &[u8]) -> Result<ValidationInfo> {
     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 +183,8 @@ fn handle_section<T, F: FnOnce(&mut WasmReader, SectionHeader) -> Result<T>>(
 ) -> Result<Option<T>> {
     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)?;

Original file line number	Diff line number	Diff line change
`@@ -89,7 +89,7 @@ impl<'a> WasmReader<'a> {`
`89`	`89`	`let bytes = &self.full_wasm_binary[self.pc..(self.pc + N)];`
`90`	`90`	`self.pc += N;`
`91`	`91`
`92`		`- Ok(bytes.try_into().expect("the slice length to be exactly N"))`
	`92`	`+ Ok(bytes.try_into().map_err(\|_err\| Error::Eof)?)`
`93`	`93`	`}`
`94`	`94`
`95`	`95`	/// Read the current byte without advancing the [`pc`](Self::pc)