diff --git a/crates/wasmtime/src/runtime/component/func/host.rs b/crates/wasmtime/src/runtime/component/func/host.rs index 990673105607..096a2a28b555 100644 --- a/crates/wasmtime/src/runtime/component/func/host.rs +++ b/crates/wasmtime/src/runtime/component/func/host.rs @@ -683,15 +683,12 @@ where { let cx = unsafe { VMComponentContext::from_opaque(cx) }; unsafe { - ComponentInstance::from_vmctx(cx, |store, instance| { + ComponentInstance::enter_host_from_wasm(cx, |store, instance| { let mut store = store.unchecked_context_mut(); - - crate::runtime::vm::catch_unwind_and_record_trap(|| { - store.0.call_hook(CallHook::CallingHost)?; - let res = func(store.as_context_mut(), instance); - store.0.call_hook(CallHook::ReturningFromHost)?; - res - }) + store.0.call_hook(CallHook::CallingHost)?; + let res = func(store.as_context_mut(), instance); + store.0.call_hook(CallHook::ReturningFromHost)?; + res }) } } diff --git a/crates/wasmtime/src/runtime/func.rs b/crates/wasmtime/src/runtime/func.rs index 101a94cdef41..df1c76f04dc1 100644 --- a/crates/wasmtime/src/runtime/func.rs +++ b/crates/wasmtime/src/runtime/func.rs @@ -1,10 +1,11 @@ use crate::prelude::*; use crate::runtime::Uninhabited; use crate::runtime::vm::{ - InterpreterRef, SendSyncPtr, StoreBox, VMArrayCallHostFuncContext, VMCommonStackInformation, - VMContext, VMFuncRef, VMFunctionImport, VMOpaqueContext, VMStoreContext, + self, InterpreterRef, SendSyncPtr, StoreBox, VMArrayCallHostFuncContext, + VMCommonStackInformation, VMContext, VMFuncRef, VMFunctionImport, VMOpaqueContext, + VMStoreContext, }; -use crate::store::{AutoAssertNoGc, StoreId, StoreOpaque}; +use crate::store::{AutoAssertNoGc, InstanceId, StoreId, StoreOpaque}; use crate::type_registry::RegisteredType; use crate::{ AsContext, AsContextMut, CallHook, Engine, Extern, FuncType, Instance, ModuleExport, Ref, @@ -2041,46 +2042,29 @@ impl Caller<'_, T> { /// Executes `f` with an appropriate `Caller`. /// - /// This is the entrypoint for host functions in core wasm and converts from - /// `VMContext` to `Caller` - /// - /// # Safety - /// - /// This requires that `caller` is safe to wrap up as a `Caller`, - /// effectively meaning that we just entered the host from wasm. - /// Additionally this `Caller`'s `T` parameter must match the actual `T` in - /// the store of the vmctx of `caller`. - unsafe fn with(caller: NonNull, f: F) -> R + /// This is the entrypoint for host functions in core wasm. The `store` and + /// `instance` are created from `Instance::enter_host_from_wasm` and this + /// will further invoke the host function that `f` refers to. + fn with(mut store: StoreContextMut, caller: InstanceId, f: F) -> R where F: FnOnce(Caller<'_, T>) -> R, { - // SAFETY: it's a contract of this function itself that `from_vmctx` is - // safe to call. Additionally it's a contract of this function itself - // that the `T` of `Caller` matches the store. - unsafe { - crate::runtime::vm::InstanceAndStore::from_vmctx(caller, |pair| { - let (instance, store) = pair.unpack_mut(); - let mut store = store.unchecked_context_mut::(); - let caller = Instance::from_wasmtime(instance.id(), store.0); + let caller = Instance::from_wasmtime(caller, store.0); - let (gc_lifo_scope, ret) = { - let gc_lifo_scope = store.0.gc_roots().enter_lifo_scope(); + let (gc_lifo_scope, ret) = { + let gc_lifo_scope = store.0.gc_roots().enter_lifo_scope(); - let ret = f(Caller { - store: store.as_context_mut(), - caller, - }); + let ret = f(Caller { + store: store.as_context_mut(), + caller, + }); - (gc_lifo_scope, ret) - }; + (gc_lifo_scope, ret) + }; - // Safe to recreate a mutable borrow of the store because `ret` - // cannot be borrowing from the store. - store.0.exit_gc_lifo_scope(gc_lifo_scope); + store.0.exit_gc_lifo_scope(gc_lifo_scope); - ret - }) - } + ret } fn sub_caller(&mut self) -> Caller<'_, T> { @@ -2409,10 +2393,14 @@ impl HostContext { // closure and then run it as part of `Caller::with`. // // SAFETY: this is an entrypoint of wasm which requires correct type - // ascription of `T` itself, meaning that this should be safe to call. - crate::runtime::vm::catch_unwind_and_record_trap(move || unsafe { - Caller::with(caller_vmctx, run) - }) + // ascription of `T` itself, meaning that this should be safe to call + // both `enter_host_from_wasm` as well as `unchecked_context_mut`. + unsafe { + vm::Instance::enter_host_from_wasm(caller_vmctx, |store, instance| { + let store = store.unchecked_context_mut(); + Caller::with(store, instance.id(), run) + }) + } } } @@ -2485,20 +2473,23 @@ impl HostFunc { T: 'static, { assert!(ty.comes_from_same_engine(engine)); - // SAFETY: This is only only called in the raw entrypoint of wasm - // meaning that `caller_vmctx` is appropriate to read, and additionally - // the later usage of `{,in}to_func` will connect `T` to an actual - // store's `T` to ensure it's the same. - let func = move |caller_vmctx, values: &mut [ValRaw]| unsafe { - Caller::::with(caller_vmctx, |mut caller| { - caller.store.0.call_hook(CallHook::CallingHost)?; - let result = func(caller.sub_caller(), values)?; - caller.store.0.call_hook(CallHook::ReturningFromHost)?; - Ok(result) - }) - }; - let ctx = crate::trampoline::create_array_call_function(&ty, func) - .expect("failed to create function"); + let ctx = crate::trampoline::create_array_call_function( + &ty, + move |store, instance, values: &mut [ValRaw]| { + // SAFETY: the later usage of `{,in}to_func` will connect `T` to + // an actual store's `T` to ensure it's the same. This means + // that the store this is invoked with always has `T` as a type + // parameter which should make this cast safe. + let store = unsafe { store.unchecked_context_mut::() }; + Caller::with(store, instance, |mut caller| { + caller.store.0.call_hook(CallHook::CallingHost)?; + let result = func(caller.sub_caller(), values)?; + caller.store.0.call_hook(CallHook::ReturningFromHost)?; + Ok(result) + }) + }, + ) + .expect("failed to create function"); HostFunc::_new(engine, ctx.into()) } diff --git a/crates/wasmtime/src/runtime/trampoline/func.rs b/crates/wasmtime/src/runtime/trampoline/func.rs index 4b6a47a49157..8a8f94a3b2fe 100644 --- a/crates/wasmtime/src/runtime/trampoline/func.rs +++ b/crates/wasmtime/src/runtime/trampoline/func.rs @@ -1,7 +1,10 @@ //! Support for a calling of an imported function. use crate::prelude::*; -use crate::runtime::vm::{StoreBox, VMArrayCallHostFuncContext, VMContext, VMOpaqueContext}; +use crate::runtime::vm::{ + Instance, StoreBox, VMArrayCallHostFuncContext, VMContext, VMOpaqueContext, VMStore, +}; +use crate::store::InstanceId; use crate::type_registry::RegisteredType; use crate::{FuncType, ValRaw}; use core::ptr::NonNull; @@ -32,33 +35,37 @@ unsafe extern "C" fn array_call_shim( values_vec_len: usize, ) -> bool where - F: Fn(NonNull, &mut [ValRaw]) -> Result<()> + 'static, + F: Fn(&mut dyn VMStore, InstanceId, &mut [ValRaw]) -> Result<()> + 'static, { - // Be sure to catch Rust panics to manually shepherd them across the wasm - // boundary, and then otherwise delegate as normal. - crate::runtime::vm::catch_unwind_and_record_trap(|| { - // SAFETY: this function itself requires that the `vmctx` is valid to - // use here. - let state = unsafe { - let vmctx = VMArrayCallHostFuncContext::from_opaque(vmctx); - vmctx.as_ref().host_state() - }; + // SAFETY: this is an entrypoint of wasm calling a host and our parameters + // should reflect that making `enter_host_from_wasm` suitable. Further + // unsafe operations are commented below. + unsafe { + Instance::enter_host_from_wasm(caller_vmctx, |store, instance| { + let instance = instance.id(); + // SAFETY: this function itself requires that the `vmctx` is valid to + // use here. + let state = { + let vmctx = VMArrayCallHostFuncContext::from_opaque(vmctx); + vmctx.as_ref().host_state() + }; - // Double-check ourselves in debug mode, but we control - // the `Any` here so an unsafe downcast should also - // work. - // - // SAFETY: this function is only usable with `TrampolineState`. - let state = unsafe { - debug_assert!(state.is::>()); - &*(state as *const _ as *const TrampolineState) - }; - let mut values_vec = NonNull::slice_from_raw_parts(values_vec, values_vec_len); - // SAFETY: it's a contract of this function itself that the values - // provided are valid to view as a slice. - let values_vec = unsafe { values_vec.as_mut() }; - (state.func)(caller_vmctx, values_vec) - }) + // Double-check ourselves in debug mode, but we control + // the `Any` here so an unsafe downcast should also + // work. + // + // SAFETY: this function is only usable with `TrampolineState`. + let state = { + debug_assert!(state.is::>()); + &*(state as *const _ as *const TrampolineState) + }; + let mut values_vec = NonNull::slice_from_raw_parts(values_vec, values_vec_len); + // SAFETY: it's a contract of this function itself that the values + // provided are valid to view as a slice. + let values_vec = values_vec.as_mut(); + (state.func)(store, instance, values_vec) + }) + } } pub fn create_array_call_function( @@ -66,7 +73,7 @@ pub fn create_array_call_function( func: F, ) -> Result> where - F: Fn(NonNull, &mut [ValRaw]) -> Result<()> + Send + Sync + 'static, + F: Fn(&mut dyn VMStore, InstanceId, &mut [ValRaw]) -> Result<()> + Send + Sync + 'static, { let array_call = array_call_shim::; diff --git a/crates/wasmtime/src/runtime/vm.rs b/crates/wasmtime/src/runtime/vm.rs index 70ebc795f17f..77dbbda88332 100644 --- a/crates/wasmtime/src/runtime/vm.rs +++ b/crates/wasmtime/src/runtime/vm.rs @@ -97,8 +97,8 @@ pub use crate::runtime::vm::gc::*; pub use crate::runtime::vm::imports::Imports; pub use crate::runtime::vm::instance::{ GcHeapAllocationIndex, Instance, InstanceAllocationRequest, InstanceAllocator, - InstanceAllocatorImpl, InstanceAndStore, InstanceHandle, MemoryAllocationIndex, - OnDemandInstanceAllocator, StorePtr, TableAllocationIndex, initialize_instance, + InstanceAllocatorImpl, InstanceHandle, MemoryAllocationIndex, OnDemandInstanceAllocator, + StorePtr, TableAllocationIndex, initialize_instance, }; #[cfg(feature = "pooling-allocator")] pub use crate::runtime::vm::instance::{ diff --git a/crates/wasmtime/src/runtime/vm/component.rs b/crates/wasmtime/src/runtime/vm/component.rs index f9c5de3e9906..0e3168283be5 100644 --- a/crates/wasmtime/src/runtime/vm/component.rs +++ b/crates/wasmtime/src/runtime/vm/component.rs @@ -11,9 +11,9 @@ use crate::runtime::component::ComponentInstanceId; use crate::runtime::vm::instance::{InstanceLayout, OwnedInstance, OwnedVMContext}; use crate::runtime::vm::vmcontext::VMFunctionBody; use crate::runtime::vm::{ - SendSyncPtr, VMArrayCallFunction, VMFuncRef, VMGlobalDefinition, VMMemoryDefinition, - VMOpaqueContext, VMStore, VMStoreRawPtr, VMTableImport, VMWasmCallFunction, ValRaw, VmPtr, - VmSafe, + HostResult, SendSyncPtr, VMArrayCallFunction, VMFuncRef, VMGlobalDefinition, + VMMemoryDefinition, VMOpaqueContext, VMStore, VMStoreRawPtr, VMTableImport, VMWasmCallFunction, + ValRaw, VmPtr, VmSafe, catch_unwind_and_record_trap, }; use crate::store::InstanceId; use alloc::alloc::Layout; @@ -203,6 +203,10 @@ impl ComponentInstance { /// Converts the `vmctx` provided into a `ComponentInstance` and runs the /// provided closure with that instance. /// + /// This function will also catch any failures that `f` produces and returns + /// an appropriate ABI value to return to wasm. This includes normal errors + /// such as traps as well as Rust-side panics which require wasm to unwind. + /// /// # Unsafety /// /// This is `unsafe` because `vmctx` cannot be guaranteed to be a valid @@ -210,10 +214,13 @@ impl ComponentInstance { /// mutable reference at this time to the instance from `vmctx`. Note that /// it must be also safe to borrow the store mutably, meaning it can't /// already be in use elsewhere. - pub unsafe fn from_vmctx( + pub unsafe fn enter_host_from_wasm( vmctx: NonNull, f: impl FnOnce(&mut dyn VMStore, Instance) -> R, - ) -> R { + ) -> R::Abi + where + R: HostResult, + { // SAFETY: it's a contract of this function that `vmctx` is a valid // allocation which can go backwards to a `ComponentInstance`. let mut ptr = unsafe { @@ -230,7 +237,7 @@ impl ComponentInstance { let store = unsafe { &mut *reference.store.0.as_ptr() }; let instance = Instance::from_wasmtime(store, reference.id); - f(store, instance) + catch_unwind_and_record_trap(store, |store| f(store, instance)) } /// Returns the `InstanceId` associated with the `vmctx` provided. diff --git a/crates/wasmtime/src/runtime/vm/component/libcalls.rs b/crates/wasmtime/src/runtime/vm/component/libcalls.rs index 09b968f19754..30fd688bc825 100644 --- a/crates/wasmtime/src/runtime/vm/component/libcalls.rs +++ b/crates/wasmtime/src/runtime/vm/component/libcalls.rs @@ -99,11 +99,11 @@ mod trampolines { { $(shims!(@validate_param $pname $param);)* - let ret = crate::runtime::vm::traphandlers::catch_unwind_and_record_trap(|| unsafe { - ComponentInstance::from_vmctx(vmctx, |store, instance| { + let ret = unsafe { + ComponentInstance::enter_host_from_wasm(vmctx, |store, instance| { shims!(@invoke $name(store, instance,) $($pname)*) }) - }); + }; shims!(@convert_ret ret $($pname: $param)*) } $( diff --git a/crates/wasmtime/src/runtime/vm/debug_builtins.rs b/crates/wasmtime/src/runtime/vm/debug_builtins.rs index 4b5d5aa81dde..e62f9d487297 100644 --- a/crates/wasmtime/src/runtime/vm/debug_builtins.rs +++ b/crates/wasmtime/src/runtime/vm/debug_builtins.rs @@ -1,7 +1,6 @@ #![doc(hidden)] -use crate::runtime::vm::instance::InstanceAndStore; -use crate::runtime::vm::vmcontext::VMContext; +use crate::runtime::vm::{Instance, VMContext}; use core::ptr::NonNull; use wasmtime_environ::{EntityRef, MemoryIndex}; use wasmtime_versioned_export_macros::versioned_export; @@ -19,8 +18,7 @@ pub unsafe extern "C" fn resolve_vmctx_memory_ptr(p: *const u32) -> *const u8 { VMCTX_AND_MEMORY.0 != NonNull::dangling(), "must call `__vmctx->set()` before resolving Wasm pointers" ); - InstanceAndStore::from_vmctx(VMCTX_AND_MEMORY.0, |handle| { - let (handle, _) = handle.unpack_mut(); + Instance::enter_host_from_wasm(VMCTX_AND_MEMORY.0, |_store, handle| { assert!( VMCTX_AND_MEMORY.1 < handle.env_module().memories.len(), "memory index for debugger is out of bounds" diff --git a/crates/wasmtime/src/runtime/vm/instance.rs b/crates/wasmtime/src/runtime/vm/instance.rs index ce807fa8ab67..49d0f4bdd4fd 100644 --- a/crates/wasmtime/src/runtime/vm/instance.rs +++ b/crates/wasmtime/src/runtime/vm/instance.rs @@ -14,8 +14,8 @@ use crate::runtime::vm::vmcontext::{ VMTableDefinition, VMTableImport, VMTagDefinition, VMTagImport, }; use crate::runtime::vm::{ - GcStore, Imports, ModuleRuntimeInfo, SendSyncPtr, VMGlobalKind, VMStore, VMStoreRawPtr, VmPtr, - VmSafe, WasmFault, + GcStore, HostResult, Imports, ModuleRuntimeInfo, SendSyncPtr, VMGlobalKind, VMStore, + VMStoreRawPtr, VmPtr, VmSafe, WasmFault, catch_unwind_and_record_trap, }; use crate::store::{InstanceId, StoreId, StoreInstanceId, StoreOpaque}; use alloc::sync::Arc; @@ -41,130 +41,6 @@ use wasmtime_wmemcheck::Wmemcheck; mod allocator; pub use allocator::*; -/// The pair of an instance and a raw pointer its associated store. -/// -/// ### Safety -/// -/// > **Note**: it's known that the documentation below is documenting an -/// > unsound pattern and we're in the process of fixing it, but it'll take -/// > some time to refactor. Notably `unpack_mut` is not sound because the -/// > returned store pointer can be used to accidentally alias the instance -/// > pointer returned as well. -/// -/// Getting a borrow of a vmctx's store is one of the fundamental bits of unsafe -/// code in Wasmtime. No matter how we architect the runtime, some kind of -/// unsafe conversion from a raw vmctx pointer that Wasm is using into a Rust -/// struct must happen. -/// -/// It is our responsibility to ensure that multiple (exclusive) borrows of the -/// vmctx's store never exist at the same time. The distinction between the -/// `Instance` type (which doesn't expose its underlying vmctx pointer or a way -/// to get a borrow of its associated store) and this type (which does) is -/// designed to help with that. -/// -/// Going from a `*mut VMContext` to a `&mut StoreInner` is naturally unsafe -/// due to the raw pointer usage, but additionally the `T` type parameter needs -/// to be the same `T` that was used to define the `dyn VMStore` trait object -/// that was stuffed into the vmctx. -/// -/// ### Usage -/// -/// Usage generally looks like: -/// -/// 1. You get a raw `*mut VMContext` from Wasm -/// -/// 2. You call `InstanceAndStore::from_vmctx` on that raw pointer -/// -/// 3. You then call `InstanceAndStore::unpack_mut` (or another helper) to get -/// the underlying `Pin<&mut Instance>` and `&mut dyn VMStore` (or `&mut -/// StoreInner`). -/// -/// 4. You then use whatever `Instance` methods you need to, each of which take -/// a store argument as necessary. -/// -/// In step (4) you no longer need to worry about double exclusive borrows of -/// the store, so long as you don't do (1-2) again. Note also that the borrow -/// checker prevents repeating step (3) if you never repeat (1-2). In general, -/// steps (1-3) should be done in a single, common, internally-unsafe, -/// plumbing-code bottleneck and the raw pointer should never be exposed to Rust -/// code that does (4) after the `InstanceAndStore` is created. Follow this -/// pattern, and everything using the resulting `Instance` and `Store` can be -/// safe code (at least, with regards to accessing the store itself). -/// -/// As an illustrative example, the common plumbing code for our various -/// libcalls performs steps (1-3) before calling into each actual libcall -/// implementation function that does (4). The plumbing code hides the raw vmctx -/// pointer and never gives out access to it to the libcall implementation -/// functions, nor does an `Instance` expose its internal vmctx pointer, which -/// would allow unsafely repeating steps (1-2). -#[repr(transparent)] -pub struct InstanceAndStore { - instance: Instance, -} - -impl InstanceAndStore { - /// Converts the provided `*mut VMContext` to an `InstanceAndStore` - /// reference and calls the provided closure with it. - /// - /// This method will move the `vmctx` pointer backwards to point to the - /// original `Instance` that precedes it. The closure is provided a - /// temporary reference to the `InstanceAndStore` with a constrained - /// lifetime to ensure that it doesn't accidentally escape. - /// - /// # Safety - /// - /// Callers must validate that the `vmctx` pointer is a valid allocation and - /// that it's valid to acquire `&mut InstanceAndStore` at this time. For - /// example this can't be called twice on the same `VMContext` to get two - /// active mutable borrows to the same `InstanceAndStore`. - /// - /// See also the safety discussion in this type's documentation. - #[inline] - pub(crate) unsafe fn from_vmctx( - vmctx: NonNull, - f: impl for<'a> FnOnce(&'a mut Self) -> R, - ) -> R { - const _: () = assert!(mem::size_of::() == mem::size_of::()); - // SAFETY: The validity of this `byte_sub` relies on `vmctx` being a - // valid allocation which is itself a contract of this function. - let mut ptr = unsafe { - vmctx - .byte_sub(mem::size_of::()) - .cast::() - }; - - // SAFETY: the ability to interpret `vmctx` as a safe pointer and - // continue on is a contract of this function itself, so the safety here - // is effectively up to callers. - unsafe { f(ptr.as_mut()) } - } - - /// Unpacks this `InstanceAndStore` into its underlying `Instance` and `dyn - /// VMStore`. - #[inline] - pub(crate) fn unpack_mut(&mut self) -> (Pin<&mut Instance>, &mut dyn VMStore) { - unsafe { - let store = &mut *self.store_ptr(); - (Pin::new_unchecked(&mut self.instance), store) - } - } - - /// Gets a pointer to this instance's `Store` which was originally - /// configured on creation. - /// - /// # Panics - /// - /// May panic if the originally configured store was `None`. That can happen - /// for host functions so host functions can't be queried what their - /// original `Store` was since it's just retained as null (since host - /// functions are shared amongst threads and don't all share the same - /// store). - #[inline] - fn store_ptr(&self) -> *mut dyn VMStore { - self.instance.store.unwrap().0.as_ptr() - } -} - /// A type that roughly corresponds to a WebAssembly instance, but is also used /// for host-defined objects. /// @@ -328,6 +204,67 @@ impl Instance { ret } + /// Encapsulated entrypoint to the host from WebAssembly, converting a raw + /// `VMContext` pointer into a `VMStore` plus an `Instance`. + /// + /// This is an entrypoint for core wasm entering back into the host. This is + /// used for both host functions and libcalls for example. This will execute + /// the closure `f` with safer Internal types than a raw `VMContext` + /// pointer. + /// + /// The closure `f` will have its errors caught, handled, and translated to + /// an ABI-safe return value to give back to wasm. This includes both normal + /// errors such as traps as well as panics. + /// + /// # Known Unsoundness + /// + /// This API is known to be unsound because it's possible to alias the + /// returned `Instance` pointer with a pointer derived safely from the store + /// provided to the closure. This signature would ideally replace + /// `Pin<&mut Instance>` with `InstanceId`. That's not quite possible yet + /// and is left for a future refactoring. + /// + /// # Safety + /// + /// Callers must ensure that `vmctx` is a valid allocation and is safe to + /// dereference at this time. That's generally only true when it's a + /// wasm-provided value and this is the first function called after entering + /// the host. Otherwise this could unsafely alias the store with a mutable + /// pointer, for example. + #[inline] + pub(crate) unsafe fn enter_host_from_wasm( + vmctx: NonNull, + f: impl FnOnce(&mut dyn VMStore, Pin<&mut Instance>) -> R, + ) -> R::Abi + where + R: HostResult, + { + // SAFETY: The validity of this `byte_sub` relies on `vmctx` being a + // valid allocation which is itself a contract of this function. + // Additionally `as_mut` requires that the pointer is valid, which is + // also a contract of this function. The lifetime of the reference will + // be constrained by the closure `f` provided to this function which + // inherently can't have the pointer escape, so the lifetime is scoped + // here. + // + // Note that this is additionally creating both an instance and a store + // as safe pointers. See the documentation on this function for known + // unsoundness here where the store can safely derive an aliasing + // mutable pointer to the instance. + let (store, instance) = unsafe { + let instance = vmctx + .byte_sub(mem::size_of::()) + .cast::() + .as_mut(); + let store = &mut *instance.store.unwrap().0.as_ptr(); + (store, Pin::new_unchecked(instance)) + }; + + // Thread the `store` and `instance` through panic/trap infrastructure + // back into `f`. + catch_unwind_and_record_trap(store, |store| f(store, instance)) + } + /// Converts the provided `*mut VMContext` to an `Instance` pointer and /// returns it with the same lifetime as `self`. /// diff --git a/crates/wasmtime/src/runtime/vm/libcalls.rs b/crates/wasmtime/src/runtime/vm/libcalls.rs index f6c6cfb846d7..5d88c1df31c3 100644 --- a/crates/wasmtime/src/runtime/vm/libcalls.rs +++ b/crates/wasmtime/src/runtime/vm/libcalls.rs @@ -97,7 +97,7 @@ use wasmtime_wmemcheck::AccessError::{ /// For more information on converting from host-defined values to Cranelift ABI /// values see the `catch_unwind_and_record_trap` function. pub mod raw { - use crate::runtime::vm::{InstanceAndStore, VMContext, f32x4, f64x2, i8x16}; + use crate::runtime::vm::{Instance, VMContext, f32x4, f64x2, i8x16}; use core::ptr::NonNull; macro_rules! libcall { @@ -120,12 +120,9 @@ pub mod raw { $( $pname : libcall!(@ty $param), )* ) $(-> libcall!(@ty $result))? { $(#[cfg($attr)])? - { - crate::runtime::vm::traphandlers::catch_unwind_and_record_trap(|| unsafe { - InstanceAndStore::from_vmctx(vmctx, |pair| { - let (instance, store) = pair.unpack_mut(); - super::$name(store, instance, $($pname),*) - }) + unsafe { + Instance::enter_host_from_wasm(vmctx, |store, instance| { + super::$name(store, instance, $($pname),*) }) } $( diff --git a/crates/wasmtime/src/runtime/vm/traphandlers.rs b/crates/wasmtime/src/runtime/vm/traphandlers.rs index 4e9867aebfca..0ce769482f34 100644 --- a/crates/wasmtime/src/runtime/vm/traphandlers.rs +++ b/crates/wasmtime/src/runtime/vm/traphandlers.rs @@ -18,7 +18,7 @@ pub use self::signals::*; use crate::runtime::module::lookup_code; use crate::runtime::store::{ExecutorRef, StoreOpaque}; use crate::runtime::vm::sys::traphandlers; -use crate::runtime::vm::{InterpreterRef, VMContext, VMStoreContext, f32x4, f64x2, i8x16}; +use crate::runtime::vm::{InterpreterRef, VMContext, VMStore, VMStoreContext, f32x4, f64x2, i8x16}; use crate::{EntryStoreContext, prelude::*}; use crate::{StoreContextMut, WasmBacktrace}; use core::cell::Cell; @@ -87,11 +87,13 @@ pub(super) unsafe fn raise_preexisting_trap() -> ! { tls::with(|info| unsafe { info.unwrap().unwind() }) } -/// Invokes the closure `f` and returns a `bool` if it succeeded. +/// Invokes the closure `f` and handles any error/panic/trap that happens +/// within. /// -/// This will invoke the closure `f` which returns a value that implements -/// `HostResult`. This trait abstracts over how host values are translated to -/// ABI values when going back into wasm. Some examples are: +/// This will invoke the closure `f` with the provided `store` and the closure +/// will return a value that implements `HostResult`. This trait abstracts over +/// how host values are translated to ABI values when going back into wasm. +/// Some examples are: /// /// * `T` - bare return types (not results) are simply returned as-is. No /// `catch_unwind` happens as if a trap can't happen then the host shouldn't @@ -109,7 +111,10 @@ pub(super) unsafe fn raise_preexisting_trap() -> ! { /// This function acts as a bridge between the two to appropriately handle /// encoding host values to Cranelift-understood ABIs via the `HostResult` /// trait. -pub fn catch_unwind_and_record_trap(f: impl FnOnce() -> R) -> R::Abi +pub fn catch_unwind_and_record_trap( + store: &mut dyn VMStore, + f: impl FnOnce(&mut dyn VMStore) -> R, +) -> R::Abi where R: HostResult, { @@ -117,7 +122,7 @@ where // return value is always provided and if unwind information is provided // (e.g. `ret` is a "false"-y value) then it's recorded in TLS for the // unwind operation that's about to happen from Cranelift-generated code. - let (ret, unwind) = R::maybe_catch_unwind(f); + let (ret, unwind) = R::maybe_catch_unwind(|| f(store)); if let Some(unwind) = unwind { tls::with(|info| info.unwrap().record_unwind(unwind)); }