Revert "Implement a reflection API in WASIX"

Author: zebreus

lib/wasi-types/src/wasi/bindings.rs

@@ -2466,56 +2466,6 @@ impl core::fmt::Debug for WasmValueType {
     }
 }
 
-// TODO: if necessary, must be implemented in wit-bindgen
-unsafe impl ValueType for WasmValueType {
-    #[inline]
-    fn zero_padding_bytes(&self, _bytes: &mut [MaybeUninit<u8>]) {}
-}
-
-impl TryFrom<wasmer::Type> for WasmValueType {
-    type Error = ();
-    fn try_from(value: wasmer::Type) -> Result<Self, Self::Error> {
-        match value {
-            wasmer::Type::I32 => Ok(Self::I32),
-            wasmer::Type::I64 => Ok(Self::I64),
-            wasmer::Type::F32 => Ok(Self::F32),
-            wasmer::Type::F64 => Ok(Self::F64),
-            wasmer::Type::V128 => Ok(Self::V128),
-            _ => Err(()),
-        }
-    }
-}
-impl From<WasmValueType> for wasmer::Type {
-    fn from(value: WasmValueType) -> Self {
-        match value {
-            WasmValueType::I32 => Self::I32,
-            WasmValueType::I64 => Self::I64,
-            WasmValueType::F32 => Self::F32,
-            WasmValueType::F64 => Self::F64,
-            WasmValueType::V128 => Self::V128,
-        }
-    }
-}
-
-#[doc = " A structure representing the reflection information for a function signature"]
-#[repr(C)]
-#[derive(Debug, Copy, Clone)]
-pub struct ReflectionResult {
-    #[doc = " Whether the result is cacheable"]
-    pub cacheable: Bool,
-    #[doc = " Number of arguments the function takes"]
-    pub arguments: u16,
-    #[doc = " Number of results the function returns"]
-    pub results: u16,
-}
-
-unsafe impl wasmer_types::ValueType for ReflectionResult {
-    #[inline]
-    fn zero_padding_bytes(&self, bytes: &mut [MaybeUninit<u8>]) {
-        bytes[1].write(0);
-    }
-}
-
 #[repr(C)]
 #[derive(Copy, Clone)]
 pub struct AddrUnspec {

lib/wasix/src/lib.rs

@@ -510,7 +510,6 @@ fn wasix_exports_32(mut store: &mut impl AsStoreMut, env: &FunctionEnv<WasiEnv>)
         "args_get" => Function::new_typed_with_env(&mut store, env, args_get::<Memory32>),
         "args_sizes_get" => Function::new_typed_with_env(&mut store, env, args_sizes_get::<Memory32>),
         "call_dynamic" => Function::new_typed_with_env(&mut store, env, call_dynamic::<Memory32>),
-        "reflect_signature" => Function::new_typed_with_env(&mut store, env, reflect_signature::<Memory32>),
         "clock_res_get" => Function::new_typed_with_env(&mut store, env, clock_res_get::<Memory32>),
         "clock_time_get" => Function::new_typed_with_env(&mut store, env, clock_time_get::<Memory32>),
         "clock_time_set" => Function::new_typed_with_env(&mut store, env, clock_time_set),
@@ -650,7 +649,6 @@ fn wasix_exports_64(mut store: &mut impl AsStoreMut, env: &FunctionEnv<WasiEnv>)
         "args_get" => Function::new_typed_with_env(&mut store, env, args_get::<Memory64>),
         "args_sizes_get" => Function::new_typed_with_env(&mut store, env, args_sizes_get::<Memory64>),
         "call_dynamic" => Function::new_typed_with_env(&mut store, env, call_dynamic::<Memory64>),
-        "reflect_signature" => Function::new_typed_with_env(&mut store, env, reflect_signature::<Memory64>),
         "clock_res_get" => Function::new_typed_with_env(&mut store, env, clock_res_get::<Memory64>),
         "clock_time_get" => Function::new_typed_with_env(&mut store, env, clock_time_get::<Memory64>),
         "clock_time_set" => Function::new_typed_with_env(&mut store, env, clock_time_set),

lib/wasix/src/state/handles/mod.rs

@@ -217,32 +217,6 @@ pub enum WasiModuleTreeHandles {
     },
 }
 
-#[derive(thiserror::Error, Debug, Clone)]
-pub enum FunctionLookupError {
-    #[error("No function found at the requested index `{0}`")]
-    /// Function not found at the index
-    Empty(u32),
-    #[error("Table index `{0}` out of bounds")]
-    /// Table index out of bounds
-    OutOfBounds(u32),
-    #[error("The table does not contain functions")]
-    /// The table does not contain functions
-    NotAFunctionTable,
-    #[error("No indirect function table available")]
-    /// No indirect function table available
-    NoIndirectFunctionTable,
-}
-impl From<FunctionLookupError> for Errno {
-    fn from(e: FunctionLookupError) -> Self {
-        match e {
-            FunctionLookupError::Empty(_) => Errno::Inval,
-            FunctionLookupError::OutOfBounds(_) => Errno::Inval,
-            FunctionLookupError::NotAFunctionTable => Errno::Inval,
-            FunctionLookupError::NoIndirectFunctionTable => Errno::Notsup,
-        }
-    }
-}
-
 impl WasiModuleTreeHandles {
     /// Can be used to get the `WasiModuleInstanceHandles` of the main module.
     /// If access to the side modules' instance handles is required, one must go
@@ -323,48 +297,32 @@ impl WasiModuleTreeHandles {
         }
     }
 
-    /// Helper function to look up a function in the indirect function table
-    ///
-    /// * Returns an Errno if an error occurred.
-    /// * Returns `Ok(None)` if the index is out of bounds.
-    /// * Returns `Ok(Some(None))` if there is no function at the index.
-    /// * Returns `Ok(Some(Some(function)))` if there is a function at the index.
     pub fn indirect_function_table_lookup(
         &self,
         store: &mut impl AsStoreMut,
         index: u32,
-    ) -> Result<Function, FunctionLookupError> {
+    ) -> Result<Option<Function>, Errno> {
         let value = self
             .main_module_instance_handles()
             .indirect_function_table
             .as_ref()
-            .ok_or(FunctionLookupError::NoIndirectFunctionTable)?
+            .ok_or(Errno::Notsup)?
             .get(store, index);
         let Some(value) = value else {
             trace!(
                 function_id = index,
                 "Function not found in indirect function table"
             );
-            return Err(FunctionLookupError::OutOfBounds(index));
+            return Ok(None);
         };
         let Value::FuncRef(funcref) = value else {
             error!("Function table contains something other than a funcref");
-            return Err(FunctionLookupError::NotAFunctionTable);
+            return Err(Errno::Inval);
         };
         let Some(funcref) = funcref else {
             trace!(function_id = index, "No function at the supplied index");
-            return Err(FunctionLookupError::Empty(index));
+            return Ok(None);
         };
-        Ok(funcref)
-    }
-
-    /// Check if an indirect_function_table entry is reserved for closures.
-    ///
-    /// Returns false if the entry is not reserved for closures.
-    pub fn is_closure(&self, function_id: u32) -> bool {
-        match self {
-            WasiModuleTreeHandles::Static(_) => false,
-            WasiModuleTreeHandles::Dynamic { linker, .. } => linker.is_closure(function_id),
-        }
+        Ok(Some(funcref))
     }
 }

lib/wasix/src/state/linker.rs

@@ -260,7 +260,7 @@
 
 use std::{
     borrow::Cow,
-    collections::{BTreeMap, HashMap},
+    collections::{BTreeMap, BTreeSet, HashMap},
     ffi::OsStr,
     ops::{Deref, DerefMut},
     path::{Path, PathBuf},
@@ -817,9 +817,7 @@ struct LinkerState {
     heap_base: u64,
 
     /// Tracks which slots in the function table are currently used for closures
-    ///
-    /// True if the closure is currently in use, false otherwise.
-    allocated_closure_functions: BTreeMap<u32, bool>,
+    used_closure_functions: BTreeSet<u32>,
     /// Slots in the indirect function table that were allocated for closures but are currently not in use.
     /// These can be given out without needing to lock all threads.
     available_closure_functions: Vec<u32>,
@@ -851,7 +849,7 @@ pub struct Linker {
 }
 
 // This macro exists to ensure we don't get into a deadlock with another pending
-// DL operation. The linker state must be locked for write *ONLY THROUGH THIS
+// DL operation. the linker state must be locked for write *ONLY THROUGH THIS
 // MACRO*. Bad things happen otherwise.
 // We also need a lock on the specific group's state here, because if there is a
 // pending DL operation we need to apply, that'll require mutable access to the
@@ -1065,7 +1063,7 @@ impl Linker {
             side_modules_by_name: HashMap::new(),
             next_module_handle: MAIN_MODULE_HANDLE.0 + 1,
             memory_allocator: MemoryAllocator::new(),
-            allocated_closure_functions: BTreeMap::new(),
+            used_closure_functions: BTreeSet::new(),
             available_closure_functions: Vec::new(),
             heap_base: stack_high,
             symbol_resolution_records: HashMap::new(),
@@ -1430,9 +1428,7 @@ impl Linker {
 
         // Use a previously allocated slot if possible
         if let Some(function_index) = linker_state.available_closure_functions.pop() {
-            linker_state
-                .allocated_closure_functions
-                .insert(function_index, true);
+            linker_state.used_closure_functions.insert(function_index);
             return Ok(function_index);
         }
 
@@ -1450,13 +1446,8 @@ impl Linker {
             linker_state
                 .available_closure_functions
                 .push(function_index + i);
-            linker_state
-                .allocated_closure_functions
-                .insert(function_index + i, false);
         }
-        linker_state
-            .allocated_closure_functions
-            .insert(function_index, true);
+        linker_state.used_closure_functions.insert(function_index);
 
         self.synchronize_link_operation(
             DlOperation::AllocateFunctionTable {
@@ -1488,32 +1479,13 @@ impl Linker {
         );
         write_linker_state!(linker_state, self, group_state, ctx);
 
-        let Some(entry) = linker_state
-            .allocated_closure_functions
-            .get_mut(&function_id)
-        else {
-            // Not allocated
-            return Ok(());
-        };
-        if !*entry {
-            // Not used
+        if !linker_state.used_closure_functions.remove(&function_id) {
+            // Not used, nothing to do
             return Ok(());
         }
-
-        *entry = false;
         linker_state.available_closure_functions.push(function_id);
-        Ok(())
-    }
 
-    /// Check if an indirect_function_table entry is reserved for closures.
-    ///
-    /// Returns false if the entry is not reserved for closures.
-    pub fn is_closure(&self, function_id: u32) -> bool {
-        // TODO: Check if this can result in a deadlock
-        let linker = self.linker_state.read().unwrap();
-        linker
-            .allocated_closure_functions
-            .contains_key(&function_id)
+        Ok(())
     }
 
     /// Loads a side module from the given path, linking it against the existing module tree

lib/wasix/src/syscalls/wasix/call_dynamic.rs

@@ -139,7 +139,7 @@ pub fn call_dynamic<M: MemorySize>(
     let function = wasi_try_ok!(env
         .inner()
         .indirect_function_table_lookup(&mut store, function_id)
-        .map_err(Errno::from));
+        .and_then(|f| f.ok_or(Errno::Inval)));
 
     let function_type = function.ty(&store);
 

lib/wasix/src/syscalls/wasix/mod.rs

@@ -45,7 +45,6 @@ mod proc_signals_sizes_get;
 mod proc_snapshot;
 mod proc_spawn;
 mod proc_spawn2;
-mod reflect_signature;
 mod resolve;
 mod sched_yield;
 mod sock_accept;
@@ -132,7 +131,6 @@ pub use proc_signals_sizes_get::*;
 pub use proc_snapshot::*;
 pub use proc_spawn::*;
 pub use proc_spawn2::*;
-pub use reflect_signature::*;
 pub use resolve::*;
 pub use sched_yield::*;
 pub use sock_accept::*;