chore: minor PR edits and clippy fixes

- Move test to crate directory
- Remove types argument from HostFuncs
- Update CHANGELOG.md

Signed-off-by: Henry Gressmann <[email protected]>

Author: explodingcamera

CHANGELOG.md

@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Fixed archive **no_std** support which was broken in the previous release, and added more tests to ensure it stays working
+- Check returns in untyped host functions ([#27](https://github.com/explodingcamera/tinywasm/pull/27)) (thanks [@WhaleKit](https://github.com/WhaleKit))
 
 ## [0.8.0] - 2024-08-29
 

Cargo.lock

@@ -23,6 +23,7 @@ libm={version="0.2", default-features=false}
 wasm-testsuite={version="0.3.3"}
 indexmap="2.7"
 wast={workspace=true}
+wat={workspace=true}
 eyre={workspace=true}
 pretty_env_logger={workspace=true}
 criterion={workspace=true}

crates/tinywasm/Cargo.toml

@@ -1,4 +1,5 @@
 use alloc::string::{String, ToString};
+use alloc::vec::Vec;
 use core::{fmt::Display, ops::ControlFlow};
 use tinywasm_types::FuncType;
 
@@ -20,18 +21,20 @@ pub enum Error {
     /// An unknown error occurred
     Other(String),
 
-    /// A function did not return a value
-    FuncDidNotReturn,
+    /// A host function returned an invalid value
+    InvalidHostFnReturn {
+        /// The expected type
+        expected: FuncType,
+        /// The actual value
+        actual: Vec<tinywasm_types::WasmValue>,
+    },
 
     /// An invalid label type was encountered
     InvalidLabelType,
 
     /// The store is not the one that the module instance was instantiated in
     InvalidStore,
 
-    /// A host function did not return results expected of it's type
-    InvalidHostFnReturn,
-
     #[cfg(feature = "std")]
     /// An I/O error occurred
     Io(crate::std::io::Error),
@@ -186,9 +189,10 @@ impl Display for Error {
             Self::InvalidLabelType => write!(f, "invalid label type"),
             Self::Other(message) => write!(f, "unknown error: {message}"),
             Self::UnsupportedFeature(feature) => write!(f, "unsupported feature: {feature}"),
-            Self::FuncDidNotReturn => write!(f, "function did not return"),
+            Self::InvalidHostFnReturn { expected, actual } => {
+                write!(f, "invalid host function return: expected={expected:?}, actual={actual:?}")
+            }
             Self::InvalidStore => write!(f, "invalid store"),
-            Self::InvalidHostFnReturn=>write!(f, "Host function returned wrong type(s)")
         }
     }
 }

crates/tinywasm/src/error.rs

@@ -6,7 +6,7 @@ use alloc::vec::Vec;
 use core::fmt::Debug;
 
 use crate::func::{FromWasmValueTuple, IntoWasmValueTuple, ValTypesFromTuple};
-use crate::{cold, log, Error, LinkingError, MemoryRef, MemoryRefMut, Result};
+use crate::{log, LinkingError, MemoryRef, MemoryRefMut, Result};
 use tinywasm_types::*;
 
 /// The internal representation of a function
@@ -42,12 +42,11 @@ impl HostFunction {
 
     /// Call the function
     pub fn call(&self, ctx: FuncContext<'_>, args: &[WasmValue]) -> Result<Vec<WasmValue>> {
-        (self.func)(ctx, args, &self.ty.results)
+        (self.func)(ctx, args)
     }
 }
 
-// third argument - expected return types, for validation
-pub(crate) type HostFuncInner = Box<dyn Fn(FuncContext<'_>, &[WasmValue], &[ValType]) -> Result<Vec<WasmValue>>>;
+pub(crate) type HostFuncInner = Box<dyn Fn(FuncContext<'_>, &[WasmValue]) -> Result<Vec<WasmValue>>>;
 
 /// The context of a host-function call
 #[derive(Debug)]
@@ -140,19 +139,26 @@ impl Extern {
         ty: &tinywasm_types::FuncType,
         func: impl Fn(FuncContext<'_>, &[WasmValue]) -> Result<Vec<WasmValue>> + 'static,
     ) -> Self {
-        let wrapper = move |ctx: FuncContext<'_>, params: &[WasmValue], res_ty: &[ValType]| {
-            let ret = func(ctx, params)?;
-            let types_match = res_ty.iter().cloned().eq(ret.iter().map(WasmValue::val_type));
-            if types_match {
-                Ok(ret)
-            } else {
-                cold();
-                let got_types = ret.iter().map(WasmValue::val_type).collect::<Box<[_]>>();
-                log::error!("Return from host function type mismatch: expected {:?}, got {:?}", res_ty, got_types);
-                Err(Error::InvalidHostFnReturn)
-            }
+        let _ty = ty.clone();
+        let inner_func = move |ctx: FuncContext<'_>, args: &[WasmValue]| -> Result<Vec<WasmValue>> {
+            let _ty = _ty.clone();
+            let result = func(ctx, args)?;
+
+            if result.len() != _ty.results.len() {
+                return Err(crate::Error::InvalidHostFnReturn { expected: _ty.clone(), actual: result });
+            };
+
+            result.iter().zip(_ty.results.iter()).try_for_each(|(val, ty)| {
+                if val.val_type() != *ty {
+                    return Err(crate::Error::InvalidHostFnReturn { expected: _ty.clone(), actual: result.clone() });
+                }
+                Ok(())
+            })?;
+
+            Ok(result)
         };
-        Self::Function(Function::Host(Rc::new(HostFunction { func: Box::new(wrapper), ty: ty.clone() })))
+
+        Self::Function(Function::Host(Rc::new(HostFunction { func: Box::new(inner_func), ty: ty.clone() })))
     }
 
     /// Create a new typed function import
@@ -161,7 +167,7 @@ impl Extern {
         P: FromWasmValueTuple + ValTypesFromTuple,
         R: IntoWasmValueTuple + ValTypesFromTuple + Debug,
     {
-        let inner_func = move |ctx: FuncContext<'_>, args: &[WasmValue], _: &[ValType]| -> Result<Vec<WasmValue>> {
+        let inner_func = move |ctx: FuncContext<'_>, args: &[WasmValue]| -> Result<Vec<WasmValue>> {
             let args = P::from_wasm_value_tuple(args)?;
             let result = func(ctx, args)?;
             Ok(result.into_wasm_value_tuple().to_vec())

crates/tinywasm/src/imports.rs

@@ -1,9 +1,9 @@
 use eyre::Result;
+use std::fmt::Write;
 use tinywasm::{
     types::{FuncType, ValType, WasmValue},
     Extern, FuncContext, Imports, Module, Store,
 };
-use wat;
 
 const VAL_LISTS: &[&[WasmValue]] = &[
     &[],
@@ -13,7 +13,7 @@ const VAL_LISTS: &[&[WasmValue]] = &[
     &[WasmValue::I32(0), WasmValue::F64(0.0), WasmValue::I32(0)], // reorder
     &[WasmValue::RefExtern(0), WasmValue::F64(0.0), WasmValue::I32(0)], // all different types
 ];
-//(f64, i32, i32) and (f64) can be used to "match_none"
+// (f64, i32, i32) and (f64) can be used to "match_none"
 
 fn get_type_lists() -> impl Iterator<Item = impl Iterator<Item = ValType> + Clone> + Clone {
     VAL_LISTS.iter().map(|l| l.iter().map(WasmValue::val_type))
@@ -24,7 +24,7 @@ fn get_modules() -> Vec<(Module, FuncType, Vec<WasmValue>)> {
     for res_types in get_type_lists() {
         for (arg_types, arg_vals) in val_and_tys.clone() {
             let ty = FuncType { results: res_types.clone().collect(), params: arg_types.collect() };
-            result.push((proxy_module(&ty), ty, arg_vals.iter().cloned().collect()));
+            result.push((proxy_module(&ty), ty, arg_vals.to_vec()));
         }
     }
     result
@@ -64,9 +64,9 @@ fn test_return_invalid_type() -> Result<()> {
 fn test_linking_invalid_untyped_func() -> Result<()> {
     // try to import host functions with function types no matching those expected by modules
     let mod_list = get_modules();
-    for (module, actual_func_ty,_) in &mod_list {
-        for (_, func_ty_to_try,_) in &mod_list {
-            let tried_fn = Extern::func(&func_ty_to_try, |_: FuncContext<'_>, _| panic!("not intended to be called"));
+    for (module, actual_func_ty, _) in &mod_list {
+        for (_, func_ty_to_try, _) in &mod_list {
+            let tried_fn = Extern::func(func_ty_to_try, |_: FuncContext<'_>, _| panic!("not intended to be called"));
             let mut store = Store::default();
             let mut imports = Imports::new();
             imports.define("host", "hfn", tried_fn).unwrap();
@@ -107,11 +107,12 @@ fn test_linking_invalid_typed_func() -> Result<()> {
         for typed_fn in matching_none.clone() {
             let mut store = Store::default();
             let mut imports = Imports::new();
-            imports.define("host", "hfn", typed_fn.clone()).unwrap();
+            imports.define("host", "hfn", typed_fn).unwrap();
             let link_failure = module.clone().instantiate(&mut store, Some(imports));
             link_failure.expect_err("no func in matching_none list should link to any mod");
         }
     }
+
     // the valid cases are well-checked in other tests
     Ok(())
 }
@@ -145,7 +146,11 @@ fn proxy_module(func_ty: &FuncType) -> Module {
     let results_text = join_surround(results, "result");
     let params_text = join_surround(params, "param");
 
-    let params_gets: String = params.iter().enumerate().map(|(num, _)| format!("(local.get {num})\n")).collect();
+    // let params_gets: String = params.iter().enumerate().map(|(num, _)| format!("(local.get {num})\n")).collect();
+    let params_gets: String = params.iter().enumerate().fold(String::new(), |mut acc, (num, _)| {
+        let _ = writeln!(acc, "(local.get {num})", num = num);
+        acc
+    });
 
     let result_drops = "(drop)\n".repeat(results.len()).to_string();
     let wasm_text = format!(
@@ -164,6 +169,5 @@ fn proxy_module(func_ty: &FuncType) -> Module {
     "#
     );
     let wasm = wat::parse_str(wasm_text).expect("failed to parse wat");
-    let res = Module::parse_bytes(&wasm).expect("failed to make module");
-    res
+    Module::parse_bytes(&wasm).expect("failed to make module")
 }