Redesign Store and StoreInner internal error handling (#1679)

* redesign Store and StoreInner internal error handling

* adjust Wasmi executor for redesigned Store[Inner]

* fix intradoc links

* apply rustfmt

Author: Robbepop

crates/wasmi/src/engine/executor/instrs/call.rs

@@ -10,7 +10,7 @@ use crate::{
     },
     func::{FuncEntity, HostFuncEntity},
     ir::{index, Op, Slot, SlotSpan},
-    store::{CallHooks, PrunedStore, StoreInner},
+    store::{CallHooks, PrunedStore, StoreError, StoreInner},
     Error,
     Func,
     Instance,
@@ -42,16 +42,21 @@ pub fn dispatch_host_func(
         usize::from(len_params),
         usize::from(len_results),
     );
-    store
-        .call_host_func(&host_func, instance, params_results, call_hooks)
-        .inspect_err(|_error| {
+    match store.call_host_func(&host_func, instance, params_results, call_hooks) {
+        Err(StoreError::Internal(error)) => {
+            panic!("`call.host`: internal interpreter error: {error}")
+        }
+        Err(StoreError::External(error)) => {
             // Note: We drop the values that have been temporarily added to
             //       the stack to act as parameter and result buffer for the
             //       called host function. Since the host function failed we
             //       need to clean up the temporary buffer values here.
             //       This is required for resumable calls to work properly.
             value_stack.drop(usize::from(max_inout));
-        })?;
+            return Err(error);
+        }
+        _ => {}
+    }
     Ok((len_params, len_results))
 }
 

crates/wasmi/src/engine/executor/instrs/memory.rs

@@ -7,7 +7,7 @@ use crate::{
         Op,
         Slot,
     },
-    store::{PrunedStore, StoreInner},
+    store::{PrunedStore, StoreError, StoreInner},
     Error,
     TrapCode,
 };
@@ -46,7 +46,7 @@ impl Executor<'_> {
     /// Executes an [`Op::DataDrop`].
     pub fn execute_data_drop(&mut self, store: &mut StoreInner, segment_index: Data) {
         let segment = self.get_data_segment(segment_index);
-        store.resolve_data_segment_mut(&segment).drop_bytes();
+        store.resolve_data_mut(&segment).drop_bytes();
         self.next_instr();
     }
 
@@ -88,20 +88,24 @@ impl Executor<'_> {
                 unsafe { self.cache.update_memory(store.inner_mut()) };
                 return_value
             }
-            Err(MemoryError::OutOfBoundsGrowth | MemoryError::OutOfSystemMemory) => {
+            Err(StoreError::External(
+                MemoryError::OutOfBoundsGrowth | MemoryError::OutOfSystemMemory,
+            )) => {
                 let memory_ty = store.inner().resolve_memory(&memory).ty();
                 match memory_ty.is_64() {
                     true => u64::MAX,
                     false => u64::from(u32::MAX),
                 }
             }
-            Err(MemoryError::OutOfFuel { required_fuel }) => {
+            Err(StoreError::External(MemoryError::OutOfFuel { required_fuel })) => {
                 return Err(Error::from(ResumableOutOfFuelError::new(required_fuel)))
             }
-            Err(MemoryError::ResourceLimiterDeniedAllocation) => {
+            Err(StoreError::External(MemoryError::ResourceLimiterDeniedAllocation)) => {
                 return Err(Error::from(TrapCode::GrowthOperationLimited))
             }
-            Err(error) => panic!("encountered an unexpected error: {error}"),
+            Err(error) => {
+                panic!("`table.grow`: internal interpreter error: {error}")
+            }
         };
         self.set_stack_slot(result, return_value);
         self.try_next_instr_at(2)

crates/wasmi/src/engine/executor/instrs/table.rs

@@ -9,7 +9,7 @@ use crate::{
         Op,
         Slot,
     },
-    store::{PrunedStore, StoreInner},
+    store::{PrunedStore, StoreError, StoreInner},
     Error,
     TrapCode,
 };
@@ -224,20 +224,24 @@ impl Executor<'_> {
         let value = self.get_stack_slot(value);
         let return_value = match store.grow_table(&table, delta, value) {
             Ok(return_value) => return_value,
-            Err(TableError::GrowOutOfBounds | TableError::OutOfSystemMemory) => {
+            Err(StoreError::External(
+                TableError::GrowOutOfBounds | TableError::OutOfSystemMemory,
+            )) => {
                 let table_ty = store.inner().resolve_table(&table).ty();
                 match table_ty.is_64() {
                     true => u64::MAX,
                     false => u64::from(u32::MAX),
                 }
             }
-            Err(TableError::OutOfFuel { required_fuel }) => {
+            Err(StoreError::External(TableError::OutOfFuel { required_fuel })) => {
                 return Err(Error::from(ResumableOutOfFuelError::new(required_fuel)))
             }
-            Err(TableError::ResourceLimiterDeniedAllocation) => {
+            Err(StoreError::External(TableError::ResourceLimiterDeniedAllocation)) => {
                 return Err(Error::from(TrapCode::GrowthOperationLimited))
             }
-            Err(error) => panic!("encountered unexpected error: {error}"),
+            Err(error) => {
+                panic!("`memory.grow`: internal interpreter error: {error}")
+            }
         };
         self.set_stack_slot(result, return_value);
         self.try_next_instr_at(2)
@@ -246,7 +250,7 @@ impl Executor<'_> {
     /// Executes an [`Op::ElemDrop`].
     pub fn execute_element_drop(&mut self, store: &mut StoreInner, segment_index: Elem) {
         let segment = self.get_element_segment(segment_index);
-        store.resolve_element_segment_mut(&segment).drop_items();
+        store.resolve_element_mut(&segment).drop_items();
         self.next_instr();
     }
 }

crates/wasmi/src/global.rs

@@ -56,6 +56,10 @@ impl Global {
     }
 
     /// Returns the [`GlobalType`] of the global variable.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `ctx` does not own this [`Global`].
     pub fn ty(&self, ctx: impl AsContext) -> GlobalType {
         ctx.as_context().store.inner.resolve_global(self).ty()
     }

crates/wasmi/src/reftype.rs

@@ -126,7 +126,7 @@ impl ExternRef {
     ///
     /// Panics if `ctx` does not own this [`ExternRef`].
     pub fn data<'a, T: 'a>(&self, ctx: impl Into<StoreContext<'a, T>>) -> &'a dyn Any {
-        ctx.into().store.inner.resolve_external_object(self).data()
+        ctx.into().store.inner.resolve_externref(self).data()
     }
 }
 

crates/wasmi/src/store/error.rs

@@ -0,0 +1,119 @@
+use core::{any::type_name, fmt};
+
+#[cfg(doc)]
+use super::{PrunedStore, Store, StoreInner};
+
+/// An error occurred on [`Store<T>`] or [`StoreInner`] methods.
+#[derive(Debug, Copy, Clone)]
+pub enum StoreError<E> {
+    /// An error representing an internal error, a.k.a. a bug or invalid behavior within Wasmi.
+    Internal(InternalStoreError),
+    /// An external error forwarded by the [`Store`] or [`StoreInner`].
+    External(E),
+}
+
+impl<E: fmt::Display> fmt::Display for StoreError<E> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            StoreError::Internal(error) => fmt::Display::fmt(error, f),
+            StoreError::External(error) => fmt::Display::fmt(error, f),
+        }
+    }
+}
+
+/// A [`Store`] or [`StoreInner`] internal error, a.k.a. bug or invalid behavior within Wasmi.
+#[derive(Debug, Copy, Clone)]
+pub struct InternalStoreError {
+    kind: InternalStoreErrorKind,
+}
+
+impl InternalStoreError {
+    /// Creates a new [`InternalStoreError`].
+    fn new(kind: InternalStoreErrorKind) -> Self {
+        Self { kind }
+    }
+
+    /// An error indicating that a [`Store`] resource could not be found.
+    #[cold]
+    #[inline]
+    pub fn not_found() -> Self {
+        Self::new(InternalStoreErrorKind::EntityNotFound)
+    }
+
+    /// An error indicating that a [`Store`] resource does not originate from the store.
+    #[cold]
+    #[inline]
+    pub fn store_mismatch() -> Self {
+        Self::new(InternalStoreErrorKind::StoreMismatch)
+    }
+
+    /// An error indicating that restoring a [`PrunedStore`] to a [`Store<T>`] mismatched `T`.
+    #[cold]
+    #[inline]
+    pub fn restore_type_mismatch<T>() -> Self {
+        Self::new(InternalStoreErrorKind::RestoreTypeMismatch(
+            RestoreTypeMismatchError::new::<T>(),
+        ))
+    }
+}
+
+impl fmt::Display for InternalStoreError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let message = match &self.kind {
+            InternalStoreErrorKind::RestoreTypeMismatch(error) => {
+                return fmt::Display::fmt(error, f)
+            }
+            InternalStoreErrorKind::StoreMismatch => "store owner mismatch",
+            InternalStoreErrorKind::EntityNotFound => "entity not found",
+        };
+        write!(f, "failed to resolve entity: {message}")
+    }
+}
+
+#[derive(Debug, Copy, Clone)]
+enum InternalStoreErrorKind {
+    /// An error when restoring a [`PrunedStore`] with an incorrect `T` for [`Store<T>`].
+    RestoreTypeMismatch(RestoreTypeMismatchError),
+    /// An error indicating that a store resource does not originate from the given store.
+    StoreMismatch,
+    /// An error indicating that a store resource was not found.
+    EntityNotFound,
+}
+
+impl<E> StoreError<E> {
+    /// Create a new [`StoreError`] from the external `error`.
+    pub fn external(error: E) -> Self {
+        Self::External(error)
+    }
+}
+
+impl<E> From<InternalStoreError> for StoreError<E> {
+    fn from(error: InternalStoreError) -> Self {
+        Self::Internal(error)
+    }
+}
+
+/// Error occurred when restoring a [`PrunedStore`] to a [`Store<T>`] with an mismatching `T`.
+#[derive(Debug, Copy, Clone)]
+struct RestoreTypeMismatchError {
+    type_name: fn() -> &'static str,
+}
+
+impl RestoreTypeMismatchError {
+    /// Create a new [`RestoreTypeMismatchError`].
+    pub fn new<T>() -> Self {
+        Self {
+            type_name: type_name::<T>,
+        }
+    }
+}
+
+impl fmt::Display for RestoreTypeMismatchError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "failed to unprune store due to type mismatch: {}",
+            (self.type_name)()
+        )
+    }
+}