wip: feedback

src/core/error.rs

@@ -173,15 +173,15 @@ impl Display for Error {
                 elem_idx
             )),
             Error::DifferentRefTypes(rref1, rref2) => f.write_fmt(format_args!(
-                "RefType {} is NOT equal to RefType {}",
+                "RefType {:?} is NOT equal to RefType {:?}",
                 rref1, rref2
             )),
             Error::ExpectedARefType(found_valtype) => f.write_fmt(format_args!(
                 "Expected a RefType, found a {:?} instead",
                 found_valtype
             )),
             Error::WrongRefTypeForInteropValue(ref_given, ref_wanted) => f.write_fmt(format_args!(
-                "Wrong RefType for InteropValue: Given {} - Needed {}",
+                "Wrong RefType for InteropValue: Given {:?} - Needed {:?}",
                 ref_given, ref_wanted
             )),
             Error::FunctionIsNotDefined(func_idx) => f.write_fmt(format_args!(

src/core/reader/types/element.rs

@@ -18,7 +18,7 @@ impl Debug for ElemType {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         write!(
             f,
-            "ElemType {{\n\tinit: {:?},\n\tmode: {:?},\n\t#ty: {}\n}}",
+            "ElemType {{\n\tinit: {:?},\n\tmode: {:?},\n\t#ty: {:?}\n}}",
             self.init,
             self.mode,
             self.init.ty()
@@ -105,7 +105,7 @@ impl ElemType {
             };
 
             match reftype_or_elemkind {
-                Some(rty) => trace!("REFTYPE: {}", rty),
+                Some(rty) => trace!("REFTYPE: {:?}", rty),
                 None => {
                     trace!("REFTYPE NONE!")
                 }

src/core/reader/types/mod.rs

@@ -3,7 +3,7 @@
 //! See: <https://webassembly.github.io/spec/core/binary/types.html>
 
 use alloc::vec::Vec;
-use core::fmt::{Debug, Display, Formatter};
+use core::fmt::{Debug, Formatter};
 
 use crate::core::reader::{WasmReadable, WasmReader};
 use crate::execution::assert_validated::UnwrapValidatedExt;
@@ -87,19 +87,6 @@ pub enum RefType {
     ExternRef,
 }
 
-impl Display for RefType {
-    fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
-        write!(
-            f,
-            "{}",
-            match self {
-                Self::ExternRef => "ExternRef",
-                Self::FuncRef => "FuncRef",
-            }
-        )
-    }
-}
-
 impl RefType {
     /// TODO: we have to make sure they are NOT null Refs, but still, they are not valid ones as we cast them from RefTypes which don't hold addresses per-se
     pub fn to_null_ref(&self) -> Ref {

src/execution/const_interpreter_loop.rs

@@ -6,8 +6,6 @@ use crate::{
     NumType, RefType, ValType, Value,
 };
 
-use super::store::FuncInst;
-
 /// Execute a previosly-validated constant expression. These type of expressions are used for initializing global
 /// variables.
 ///
@@ -33,7 +31,6 @@ pub(crate) fn run_const(
     mut wasm: WasmReader,
     stack: &mut Stack,
     _imported_globals: (), /*todo!*/
-    funcs: &[FuncInst],
 ) {
     use crate::core::reader::types::opcode::*;
     loop {
@@ -105,17 +102,11 @@ pub(crate) fn run_const(
                 let reftype = RefType::read_unvalidated(&mut wasm);
 
                 stack.push_value(Value::Ref(reftype.to_null_ref()));
-                trace!("Instruction: ref.null '{}' -> [{}]", reftype, reftype);
+                trace!("Instruction: ref.null '{:?}' -> [{:?}]", reftype, reftype);
             }
             REF_FUNC => {
-                // not unwrap validated because, guess what? not validated
-                let func_idx = wasm.read_var_u32().unwrap() as usize;
-                if func_idx >= funcs.len() {
-                    panic!(
-                        "Out of bounds ref.func ({func_idx}) access (max: {})",
-                        funcs.len()
-                    );
-                }
+                // we already checked for the func_idx to be in bounds during validation
+                let func_idx = wasm.read_var_u32().unwrap_validated() as usize;
                 stack.push_value(Value::Ref(Ref::Func(FuncAddr::new(Some(func_idx)))));
             }
             other => {
@@ -129,14 +120,14 @@ pub(crate) fn run_const_span(
     wasm: &[u8],
     span: &Span,
     imported_globals: (),
-    funcs: &[FuncInst],
+    // funcs: &[FuncInst],
 ) -> Option<Value> {
     let mut wasm = WasmReader::new(wasm);
 
     wasm.move_start_to(*span).unwrap_validated();
 
     let mut stack = Stack::new();
-    run_const(wasm, &mut stack, imported_globals, funcs);
+    run_const(wasm, &mut stack, imported_globals);
 
     stack.peek_unknown_value()
 }

src/execution/interpreter_loop.rs

@@ -143,7 +143,10 @@ pub(super) fn run<H: HookSet>(
                     Ref::Extern(_) => unreachable!(),
                 };
 
-                let func_to_call_inst = store.funcs.get(func_addr).unwrap_validated();
+                let func_to_call_inst = store
+                    .funcs
+                    .get(func_addr.unwrap_validated())
+                    .unwrap_validated();
 
                 let func_ty_actual_index = func_to_call_inst.ty;
 
@@ -156,7 +159,7 @@ pub(super) fn run<H: HookSet>(
 
                 trace!("Instruction: call_indirect [{func_addr:?}]");
                 let locals = Locals::new(params, remaining_locals);
-                stack.push_stackframe(func_addr, func_ty, locals, wasm.pc);
+                stack.push_stackframe(func_addr.unwrap_validated(), func_ty, locals, wasm.pc);
 
                 wasm.move_start_to(func_to_call_inst.code_expr)
                     .unwrap_validated();
@@ -1982,13 +1985,13 @@ pub(super) fn run<H: HookSet>(
                 let reftype = RefType::read_unvalidated(&mut wasm);
 
                 stack.push_value(Value::Ref(reftype.to_null_ref()));
-                trace!("Instruction: ref.null '{}' -> [{}]", reftype, reftype);
+                trace!("Instruction: ref.null '{:?}' -> [{:?}]", reftype, reftype);
             }
             REF_IS_NULL => {
                 let rref = stack.pop_unknown_ref();
                 let is_null = match rref {
-                    Ref::Extern(rref) => rref.is_null,
-                    Ref::Func(rref) => rref.is_null,
+                    Ref::Extern(rref) => rref.addr.is_none(),
+                    Ref::Func(rref) => rref.addr.is_none(),
                 };
 
                 let res = if is_null { 1 } else { 0 };

src/execution/mod.rs

@@ -366,8 +366,7 @@ where
                         .iter()
                         .map(|expr| {
                             get_address_offset(
-                                run_const_span(validation_info.wasm, expr, (), &function_instances)
-                                    .unwrap_validated(),
+                                run_const_span(validation_info.wasm, expr, ()).unwrap_validated(),
                             )
                         })
                         .collect(),
@@ -410,13 +409,8 @@ where
                         assert!(tables.len() > table_idx);
 
                         let offset = get_address_offset(
-                            run_const_span(
-                                validation_info.wasm,
-                                &active_elem.offset,
-                                (),
-                                &function_instances,
-                            )
-                            .unwrap_validated(),
+                            run_const_span(validation_info.wasm, &active_elem.offset, ())
+                                .unwrap_validated(),
                         ) as usize;
 
                         let table = &mut tables[table_idx];
@@ -457,7 +451,7 @@ where
                         let mut wasm = WasmReader::new(validation_info.wasm);
                         wasm.move_start_to(active_data.offset).unwrap_validated();
                         let mut stack = Stack::new();
-                        run_const(wasm, &mut stack, (), &[]);
+                        run_const(wasm, &mut stack, ());
                         let value = stack.peek_unknown_value();
                         if value.is_none() {
                             panic!("No value on the stack for data segment offset");
@@ -508,7 +502,7 @@ where
                     wasm.move_start_to(global.init_expr).unwrap_validated();
                     // We shouldn't need to clear the stack. If validation is correct, it will remain empty after execution.
 
-                    run_const(wasm, &mut stack, (), &[]);
+                    run_const(wasm, &mut stack, ());
                     let value = stack.pop_value(global.ty.ty);
 
                     GlobalInst {

src/execution/value.rs

@@ -280,14 +280,14 @@ impl Ref {
 
     pub fn is_null(&self) -> bool {
         match self {
-            Self::Extern(extern_addr) => extern_addr.is_null,
-            Self::Func(func_addr) => func_addr.is_null,
+            Self::Extern(extern_addr) => extern_addr.addr.is_none(),
+            Self::Func(func_addr) => func_addr.addr.is_none(),
         }
     }
 
     pub fn is_specific_func(&self, func_id: u32) -> bool {
         match self {
-            Self::Func(func_addr) => !func_addr.is_null && func_addr.addr == func_id as usize,
+            Self::Func(func_addr) => func_addr.addr == Some(func_id as usize),
             _ => unreachable!(),
         }
     }
@@ -296,23 +296,21 @@ impl Ref {
 impl Display for Ref {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         match self {
-            Ref::Func(func_addr) => write!(f, "FuncRef({})", func_addr),
-            Ref::Extern(extern_addr) => write!(f, "ExternRef({})", extern_addr),
+            Ref::Func(func_addr) => write!(f, "FuncRef({:?})", func_addr),
+            Ref::Extern(extern_addr) => write!(f, "ExternRef({:?})", extern_addr),
         }
     }
 }
 
 #[derive(Clone, Copy, PartialEq)]
 pub struct FuncAddr {
-    pub is_null: bool,
-    // it is the idx of the function in the current module
-    pub addr: usize,
+    pub addr: Option<usize>,
 }
 
 impl Debug for FuncAddr {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        match self.is_null {
-            false => write!(f, "FuncAddr {{\n\taddr: {}\n}}", self.addr),
+        match self.addr.is_none() {
+            false => write!(f, "FuncAddr {{\n\taddr: {}\n}}", self.addr.unwrap()),
             true => write!(f, "FuncAddr {{ NULL }}"),
         }
     }
@@ -322,28 +320,24 @@ impl FuncAddr {
     pub fn new(addr: Option<usize>) -> Self {
         match addr {
             None => Self::null(),
-            Some(u) => Self {
-                addr: u,
-                is_null: false,
-            },
+            Some(u) => Self { addr: Some(u) },
         }
     }
     pub fn null() -> Self {
-        Self {
-            addr: 0,
-            is_null: true,
-        }
+        Self { addr: None }
     }
     pub fn get_value(&self) -> usize {
-        if self.is_null {
-            Self::get_reserved_value() as usize
-        } else {
-            self.addr
+        match self.addr {
+            None => Self::get_reserved_value() as usize,
+            Some(val) => val,
         }
     }
     pub fn get_reserved_value() -> u32 {
         u32::MAX
     }
+    pub fn is_null(&self) -> bool {
+        self.addr.is_none()
+    }
 }
 
 impl Default for FuncAddr {
@@ -352,43 +346,25 @@ impl Default for FuncAddr {
     }
 }
 
-impl Display for FuncAddr {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        if self.is_null {
-            write!(f, "FuncAddr {{ NULL }}")
-        } else {
-            write!(f, "FuncAddr {{ addr: {} }}", self.addr)
-        }
-    }
-}
-
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub struct ExternAddr {
-    pub is_null: bool,
-    pub addr: usize,
+    pub addr: Option<usize>,
 }
 
 impl ExternAddr {
     pub fn new(addr: Option<usize>) -> Self {
         match addr {
             None => Self::null(),
-            Some(u) => Self {
-                addr: u,
-                is_null: false,
-            },
+            Some(u) => Self { addr: Some(u) },
         }
     }
     pub fn null() -> Self {
-        Self {
-            addr: 0,
-            is_null: true,
-        }
+        Self { addr: None }
     }
     pub fn get_value(&self) -> usize {
-        if self.is_null {
-            Self::get_reserved_value() as usize
-        } else {
-            self.addr
+        match self.addr {
+            None => Self::get_reserved_value() as usize,
+            Some(val) => val,
         }
     }
     pub fn get_reserved_value() -> u32 {
@@ -402,16 +378,6 @@ impl Default for ExternAddr {
     }
 }
 
-impl Display for ExternAddr {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        if self.is_null {
-            write!(f, "ExternAddr {{ NULL }}")
-        } else {
-            write!(f, "ExternAddr {{ addr: () }}") // No actual addr, so printing ()
-        }
-    }
-}
-
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub enum RefValueTy {
     Func,

src/validation/read_constant_expression.rs

@@ -132,7 +132,7 @@ pub fn read_constant_instructions(
                 stack.push_valtype(ValType::RefType(RefType::read(wasm)?));
             }
             REF_FUNC => {
-                let func_idx = wasm.read_var_u32().unwrap() as usize;
+                let func_idx = wasm.read_var_u32()? as usize;
                 match funcs {
                     Some(funcs) => {
                         if func_idx >= funcs.len() {

tests/table.rs

@@ -129,8 +129,8 @@ fn table_elem_test() {
         .for_each(|(i, rref)| match *rref {
             wasm::value::Ref::Extern(_) => panic!(),
             wasm::value::Ref::Func(func_addr) => {
-                assert!(!func_addr.is_null);
-                assert!(wanted[i] == func_addr.addr)
+                assert!(func_addr.addr.is_some());
+                assert!(wanted[i] == func_addr.addr.unwrap())
             }
         });
     // assert!(instance.store.tables)
@@ -170,7 +170,7 @@ fn table_get_set_test() {
 
         match rref {
             Ref::Func(funcaddr) => {
-                assert!(!funcaddr.is_null)
+                assert!(!funcaddr.is_null())
             }
             _ => panic!("Expected a FuncRef"),
         }
@@ -186,7 +186,7 @@ fn table_get_set_test() {
 
         match rref {
             Ref::Func(funcaddr) => {
-                assert!(funcaddr.is_null)
+                assert!(funcaddr.is_null())
             }
             _ => panic!("Expected a FuncRef"),
         }
@@ -204,7 +204,7 @@ fn table_get_set_test() {
 
         match rref {
             Ref::Func(funcaddr) => {
-                assert!(!funcaddr.is_null)
+                assert!(!funcaddr.is_null())
             }
             _ => panic!("Expected a FuncRef"),
         }