feat: linker errors

explodingcamera · explodingcamera · commit ed1e181bdbef · 2024-01-24T14:34:48.000+01:00
Signed-off-by: Henry Gressmann &lt;mail@henrygressmann.de&gt;
diff --git a/crates/parser/src/conversion.rs b/crates/parser/src/conversion.rs
@@ -70,9 +70,9 @@ pub(crate) fn convert_module_import(import: wasmparser::Import<'_>) -> Result<Im
         module: import.module.to_string().into_boxed_str(),
         name: import.name.to_string().into_boxed_str(),
         kind: match import.ty {
-            wasmparser::TypeRef::Func(ty) => ImportKind::Func(ty),
+            wasmparser::TypeRef::Func(ty) => ImportKind::Function(ty),
             wasmparser::TypeRef::Table(ty) => ImportKind::Table(convert_module_table(ty)?),
-            wasmparser::TypeRef::Memory(ty) => ImportKind::Mem(convert_module_memory(ty)?),
+            wasmparser::TypeRef::Memory(ty) => ImportKind::Memory(convert_module_memory(ty)?),
             wasmparser::TypeRef::Global(ty) => {
                 ImportKind::Global(GlobalType { mutable: ty.mutable, ty: convert_valtype(&ty.content_type) })
             }
diff --git a/crates/tinywasm/src/error.rs b/crates/tinywasm/src/error.rs
@@ -5,6 +5,83 @@ use tinywasm_types::FuncType;
 #[cfg(feature = "parser")]
 use tinywasm_parser::ParseError;
 
+/// A tinywasm error
+#[derive(Debug)]
+pub enum Error {
+    #[cfg(feature = "parser")]
+    /// A parsing error occurred
+    ParseError(ParseError),
+
+    #[cfg(feature = "std")]
+    /// An I/O error occurred
+    Io(crate::std::io::Error),
+
+    /// A WebAssembly feature is not supported
+    UnsupportedFeature(String),
+
+    /// An unknown error occurred
+    Other(String),
+
+    /// A WebAssembly trap occurred
+    Trap(Trap),
+
+    /// A linking error occurred
+    Linker(LinkingError),
+
+    /// A function did not return a value
+    FuncDidNotReturn,
+
+    /// The stack is empty
+    StackUnderflow,
+
+    /// The label stack is empty
+    LabelStackUnderflow,
+
+    /// An invalid label type was encountered
+    InvalidLabelType,
+
+    /// The call stack is empty
+    CallStackEmpty,
+
+    /// The store is not the one that the module instance was instantiated in
+    InvalidStore,
+
+    /// Missing import
+    MissingImport {
+        /// The module name
+        module: String,
+        /// The import name
+        name: String,
+    },
+
+    /// Could not resolve an import
+    CouldNotResolveImport {
+        /// The module name
+        module: String,
+        /// The import name
+        name: String,
+    },
+}
+
+#[derive(Debug)]
+/// A linking error
+pub enum LinkingError {
+    /// An unknown import was encountered
+    UnknownImport {
+        /// The module name
+        module: String,
+        /// The import name
+        name: String,
+    },
+    /// A mismatched import type was encountered
+    MismatchedImportType {
+        /// The module name
+        module: String,
+        /// The import name
+        name: String,
+    },
+}
+
 #[derive(Debug)]
 /// A WebAssembly trap
 ///
@@ -84,67 +161,20 @@ impl Trap {
     }
 }
 
-#[derive(Debug)]
-/// A tinywasm error
-pub enum Error {
-    #[cfg(feature = "parser")]
-    /// A parsing error occurred
-    ParseError(ParseError),
-
-    #[cfg(feature = "std")]
-    /// An I/O error occurred
-    Io(crate::std::io::Error),
-
-    /// A WebAssembly feature is not supported
-    UnsupportedFeature(String),
-
-    /// An unknown error occurred
-    Other(String),
-
-    /// A WebAssembly trap occurred
-    Trap(Trap),
-
-    /// A function did not return a value
-    FuncDidNotReturn,
-
-    /// The stack is empty
-    StackUnderflow,
-
-    /// The label stack is empty
-    LabelStackUnderflow,
-
-    /// An invalid label type was encountered
-    InvalidLabelType,
-
-    /// The call stack is empty
-    CallStackEmpty,
-
-    /// The store is not the one that the module instance was instantiated in
-    InvalidStore,
-
-    /// Missing import
-    MissingImport {
-        /// The module name
-        module: String,
-        /// The import name
-        name: String,
-    },
-
-    /// Could not resolve an import
-    CouldNotResolveImport {
-        /// The module name
-        module: String,
-        /// The import name
-        name: String,
-    },
+impl LinkingError {
+    /// Get the message of the linking error
+    pub fn message(&self) -> &'static str {
+        match self {
+            Self::UnknownImport { .. } => "unknown import",
+            Self::MismatchedImportType { .. } => "mismatched import type",
+        }
+    }
+}
 
-    /// Invalid import type
-    InvalidImportType {
-        /// The module name
-        module: String,
-        /// The import name
-        name: String,
-    },
+impl From<LinkingError> for Error {
+    fn from(value: LinkingError) -> Self {
+        Self::Linker(value)
+    }
 }
 
 impl From<Trap> for Error {
@@ -163,6 +193,7 @@ impl Display for Error {
             Self::Io(err) => write!(f, "I/O error: {}", err),
 
             Self::Trap(trap) => write!(f, "trap: {}", trap.message()),
+            Self::Linker(err) => write!(f, "linking error: {}", err.message()),
             Self::CallStackEmpty => write!(f, "call stack empty"),
             Self::InvalidLabelType => write!(f, "invalid label type"),
             Self::Other(message) => write!(f, "unknown error: {}", message),
@@ -179,10 +210,6 @@ impl Display for Error {
             Self::CouldNotResolveImport { module, name } => {
                 write!(f, "could not resolve import: {}.{}", module, name)
             }
-
-            Self::InvalidImportType { module, name } => {
-                write!(f, "invalid import type: {}.{}", module, name)
-            }
         }
     }
 }
diff --git a/crates/tinywasm/src/imports.rs b/crates/tinywasm/src/imports.rs
@@ -99,7 +99,7 @@ pub enum Extern {
     Memory(ExternMemory),
 
     /// A function
-    Func(Function),
+    Function(Function),
 }
 
 /// A function
@@ -152,7 +152,7 @@ impl Extern {
             func(ctx, &args)
         };
 
-        Self::Func(Function::Host(HostFunction { func: Arc::new(inner_func), ty: ty.clone() }))
+        Self::Function(Function::Host(HostFunction { func: Arc::new(inner_func), ty: ty.clone() }))
     }
 
     /// Create a new typed function import
@@ -171,15 +171,15 @@ impl Extern {
 
         let ty = tinywasm_types::FuncType { params: P::val_types(), results: R::val_types() };
 
-        Self::Func(Function::Host(HostFunction { func: Arc::new(inner_func), ty }))
+        Self::Function(Function::Host(HostFunction { func: Arc::new(inner_func), ty }))
     }
 
     pub(crate) fn kind(&self) -> ExternalKind {
         match self {
             Self::Global(_) => ExternalKind::Global,
             Self::Table(_) => ExternalKind::Table,
             Self::Memory(_) => ExternalKind::Memory,
-            Self::Func(_) => ExternalKind::Func,
+            Self::Function(_) => ExternalKind::Func,
         }
     }
 }
@@ -273,6 +273,22 @@ impl Imports {
         None
     }
 
+    fn compare_types<T>(import: &Import, expected: &T, actual: &T) -> Result<()>
+    where
+        T: Debug + PartialEq,
+    {
+        if expected != actual {
+            log::error!("failed to link import {}, expected {:?}, got {:?}", import.name, expected, actual);
+            return Err(crate::LinkingError::MismatchedImportType {
+                module: import.module.to_string(),
+                name: import.name.to_string(),
+            }
+            .into());
+        }
+
+        Ok(())
+    }
+
     pub(crate) fn link(
         mut self,
         store: &mut crate::Store,
@@ -291,46 +307,50 @@ impl Imports {
 
             match val {
                 // A link to something that needs to be added to the store
-                ResolvedExtern::Extern(ex) => {
-                    // check if the kind matches
-                    let kind = ex.kind();
-                    if kind != (&import.kind).into() {
-                        return Err(crate::Error::InvalidImportType {
+                ResolvedExtern::Extern(ex) => match (ex, &import.kind) {
+                    (Extern::Global(extern_global), ImportKind::Global(ty)) => {
+                        Self::compare_types(import, &extern_global.ty, ty)?;
+                        imports.globals.push(store.add_global(extern_global.ty, extern_global.val.into(), idx)?);
+                    }
+                    (Extern::Table(extern_table), ImportKind::Table(ty)) => {
+                        Self::compare_types(import, &extern_table.ty.element_type, &ty.element_type)?;
+                        // TODO: do we need to check any limits?
+                        imports.tables.push(store.add_table(extern_table.ty, idx)?);
+                    }
+                    (Extern::Memory(extern_memory), ImportKind::Memory(ty)) => {
+                        Self::compare_types(import, &extern_memory.ty.arch, &ty.arch)?;
+                        // TODO: do we need to check any limits?
+                        imports.memories.push(store.add_mem(extern_memory.ty, idx)?);
+                    }
+                    (Extern::Function(extern_func), ImportKind::Function(ty)) => {
+                        let import_func_type = module.data.func_types.get(*ty as usize).ok_or_else(|| {
+                            crate::Error::CouldNotResolveImport {
+                                module: import.module.to_string(),
+                                name: import.name.to_string(),
+                            }
+                        })?;
+
+                        Self::compare_types(import, extern_func.ty(), import_func_type)?;
+                        imports.funcs.push(store.add_func(extern_func, *ty, idx)?);
+                    }
+                    _ => {
+                        return Err(crate::LinkingError::MismatchedImportType {
                             module: import.module.to_string(),
                             name: import.name.to_string(),
-                        });
-                    }
-
-                    // TODO: check if the type matches
-
-                    // add it to the store and get the address
-                    let addr = match ex {
-                        Extern::Global(g) => store.add_global(g.ty, g.val.into(), idx)?,
-                        Extern::Table(t) => store.add_table(t.ty, idx)?,
-                        Extern::Memory(m) => store.add_mem(m.ty, idx)?,
-                        Extern::Func(f) => {
-                            let ImportKind::Func(import_type) = import.kind else { unreachable!() };
-                            store.add_func(f, import_type, idx)?
                         }
-                    };
-
-                    // store the link
-                    match &kind {
-                        ExternalKind::Global => imports.globals.push(addr),
-                        ExternalKind::Table => imports.tables.push(addr),
-                        ExternalKind::Memory => imports.memories.push(addr),
-                        ExternalKind::Func => imports.funcs.push(addr),
+                        .into());
                     }
-                }
+                },
 
                 // A link to something already in the store
                 ResolvedExtern::Store(val) => {
                     // check if the kind matches
                     if val.kind() != (&import.kind).into() {
-                        return Err(crate::Error::InvalidImportType {
+                        return Err(crate::LinkingError::MismatchedImportType {
                             module: import.module.to_string(),
                             name: import.name.to_string(),
-                        });
+                        }
+                        .into());
                     }
 
                     // TODO: check if the type matches
diff --git a/crates/tinywasm/src/runtime/executor/macros.rs b/crates/tinywasm/src/runtime/executor/macros.rs
@@ -70,7 +70,9 @@ macro_rules! mem_store {
 /// Doing the actual conversion from float to int is a bit tricky, because
 /// we need to check for overflow. This macro generates the min/max values
 /// for a specific conversion, which are then used in the actual conversion.
-/// Rust sadly doesn't have wrapping casts for floats (yet)
+/// Rust sadly doesn't have wrapping casts for floats yet, maybe never.
+/// Alternatively, https://crates.io/crates/az could be used for this but
+/// it's not worth the dependency.
 macro_rules! float_min_max {
     (f32, i32) => {
         (-2147483904.0_f32, 2147483648.0_f32)
diff --git a/crates/tinywasm/src/runtime/executor/mod.rs b/crates/tinywasm/src/runtime/executor/mod.rs
@@ -6,7 +6,7 @@ use crate::{
     runtime::{BlockType, LabelFrame},
     CallFrame, Error, FuncContext, LabelArgs, ModuleInstance, Result, Store, Trap,
 };
-use alloc::{format, string::ToString, vec::Vec};
+use alloc::{string::ToString, vec::Vec};
 use tinywasm_types::{ElementKind, Instruction, ValType};
 
 mod macros;
@@ -40,10 +40,12 @@ impl DefaultRuntime {
                         current_module.swap(
                             store
                                 .get_module_instance(cf.func_instance.owner)
-                                .expect(&format!(
-                                    "exec expected module instance {} to exist for function",
-                                    cf.func_instance.owner
-                                ))
+                                .unwrap_or_else(|| {
+                                    panic!(
+                                        "exec expected module instance {} to exist for function",
+                                        cf.func_instance.owner
+                                    )
+                                })
                                 .clone(),
                         );
                     }
diff --git a/crates/types/src/lib.rs b/crates/types/src/lib.rs
