diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index 5a577500b395..0698e386103b 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -1,4 +1,4 @@ -use crate::store::{StoreData, StoreOpaque, StoreOpaqueSend, Stored}; +use crate::store::{StoreData, StoreInnermost, StoreOpaque, Stored}; use crate::{ AsContext, AsContextMut, Engine, Extern, FuncType, Instance, InterruptHandle, StoreContext, StoreContextMut, Trap, Val, ValType, @@ -681,10 +681,7 @@ impl Func { "must use `call_async` when async support is enabled on the config", ); let my_ty = self.ty(&store); - store.as_context_mut().0.exiting_native_hook()?; - let r = self.call_impl(&mut store.as_context_mut().opaque(), my_ty, params); - store.as_context_mut().0.entering_native_hook()?; - r + self.call_impl(&mut store.as_context_mut(), my_ty, params) } /// Invokes this function with the `params` given, returning the results @@ -719,105 +716,110 @@ impl Func { where T: Send, { - let my_ty = self.ty(&store); - store.as_context_mut().0.exiting_native_hook()?; - let r = self - ._call_async(store.as_context_mut().opaque_send(), my_ty, params) - .await; - store.as_context_mut().0.entering_native_hook()?; - r - } - - #[cfg(feature = "async")] - async fn _call_async( - &self, - mut store: StoreOpaqueSend<'_>, - my_ty: FuncType, - params: &[Val], - ) -> Result> { + let mut store = store.as_context_mut(); assert!( - store.async_support(), + store.0.async_support(), "cannot use `call_async` without enabling async support in the config", ); + let my_ty = self.ty(&store); let result = store .on_fiber(|store| self.call_impl(store, my_ty, params)) .await??; Ok(result) } - fn call_impl( + fn call_impl( &self, - store: &mut StoreOpaque<'_>, + store: &mut StoreContextMut<'_, T>, my_ty: FuncType, params: &[Val], ) -> Result> { - let data = &store[self.0]; - let trampoline = data.trampoline(); - let anyfunc = data.export().anyfunc; - // We need to perform a dynamic check that the arguments given to us - // match the signature of this function and are appropriate to pass to - // this function. This involves checking to make sure we have the right - // number and types of arguments as well as making sure everything is - // from the same `Store`. - if my_ty.params().len() != params.len() { - bail!( - "expected {} arguments, got {}", - my_ty.params().len(), - params.len() - ); - } - - let mut values_vec = vec![0; max(params.len(), my_ty.results().len())]; - - // Store the argument values into `values_vec`. - let param_tys = my_ty.params(); - for ((arg, slot), ty) in params.iter().cloned().zip(&mut values_vec).zip(param_tys) { - if arg.ty() != ty { - bail!( - "argument type mismatch: found {} but expected {}", - arg.ty(), - ty - ); - } - if !arg.comes_from_same_store(store) { - bail!("cross-`Store` values are not currently supported"); - } - unsafe { - arg.write_value_to(store, slot); - } - } + let mut values_vec = write_params(&mut store.as_context_mut().opaque(), &my_ty, params)?; // Call the trampoline. unsafe { - let anyfunc = anyfunc.as_ref(); + let data = &store.0.store_data()[self.0]; + let trampoline = data.trampoline(); + let anyfunc = data.export().anyfunc; invoke_wasm_and_catch_traps(store, |callee| { trampoline( - anyfunc.vmctx, + (*anyfunc.as_ptr()).vmctx, callee, - anyfunc.func_ptr.as_ptr(), + (*anyfunc.as_ptr()).func_ptr.as_ptr(), values_vec.as_mut_ptr(), ) })?; } - // Load the return values out of `values_vec`. - let mut results = Vec::with_capacity(my_ty.results().len()); - for (index, ty) in my_ty.results().enumerate() { - unsafe { - let ptr = values_vec.as_ptr().add(index); - results.push(Val::read_value_from(store, ptr, ty)); + return Ok(read_results( + &mut store.as_context_mut().opaque(), + &my_ty, + &values_vec, + )); + + fn write_params( + store: &mut StoreOpaque<'_>, + ty: &FuncType, + params: &[Val], + ) -> Result> { + // We need to perform a dynamic check that the arguments given to us + // match the signature of this function and are appropriate to pass to + // this function. This involves checking to make sure we have the right + // number and types of arguments as well as making sure everything is + // from the same `Store`. + if ty.params().len() != params.len() { + bail!( + "expected {} arguments, got {}", + ty.params().len(), + params.len() + ); } + + let mut values_vec = vec![0; max(params.len(), ty.results().len())]; + + // Store the argument values into `values_vec`. + let param_tys = ty.params(); + for ((arg, slot), ty) in params.iter().cloned().zip(&mut values_vec).zip(param_tys) { + if arg.ty() != ty { + bail!( + "argument type mismatch: found {} but expected {}", + arg.ty(), + ty + ); + } + if !arg.comes_from_same_store(store) { + bail!("cross-`Store` values are not currently supported"); + } + unsafe { + arg.write_value_to(store, slot); + } + } + + Ok(values_vec) } - Ok(results.into()) + fn read_results( + store: &mut StoreOpaque<'_>, + ty: &FuncType, + values_vec: &[u128], + ) -> Box<[Val]> { + let mut results = Vec::with_capacity(ty.results().len()); + for (index, ty) in ty.results().enumerate() { + unsafe { + let ptr = &values_vec[index]; + results.push(Val::read_value_from(store, ptr, ty)); + } + } + results.into() + } } #[inline] pub(crate) fn caller_checked_anyfunc( &self, - store: &StoreOpaque, + store: &StoreInnermost, ) -> NonNull { - store[self.0].export().anyfunc + store.store_data()[self.0].export().anyfunc } pub(crate) unsafe fn from_wasmtime_function( @@ -1033,28 +1035,39 @@ impl Func { /// /// The `closure` provided receives a default "callee" `VMContext` parameter it /// can pass to the called wasm function, if desired. -#[inline] -pub(crate) fn invoke_wasm_and_catch_traps( - store: &mut StoreOpaque<'_>, +pub(crate) fn invoke_wasm_and_catch_traps( + store: &mut StoreContextMut<'_, T>, closure: impl FnMut(*mut VMContext), ) -> Result<(), Trap> { unsafe { - let exit = if store.externref_activations_table().stack_canary().is_some() { + let exit = if store + .0 + .externref_activations_table() + .stack_canary() + .is_some() + { false } else { enter_wasm(store)?; true }; + if let Err(trap) = store.0.exiting_native_hook() { + if exit { + exit_wasm(store); + } + return Err(trap); + } let result = wasmtime_runtime::catch_traps( - store.vminterrupts(), - store.signal_handler(), - store.default_callee(), + store.0.vminterrupts(), + store.0.signal_handler(), + store.0.default_callee(), closure, ); if exit { exit_wasm(store); } + store.0.entering_native_hook()?; result.map_err(Trap::from_runtime) } } @@ -1076,8 +1089,7 @@ pub(crate) fn invoke_wasm_and_catch_traps( /// /// This function may fail if the the stack limit can't be set because an /// interrupt already happened. -#[inline] -fn enter_wasm(store: &mut StoreOpaque<'_>) -> Result<(), Trap> { +fn enter_wasm(store: &mut StoreContextMut<'_, T>) -> Result<(), Trap> { let stack_pointer = psm::stack_pointer() as usize; // Determine the stack pointer where, after which, any wasm code will @@ -1107,7 +1119,7 @@ fn enter_wasm(store: &mut StoreOpaque<'_>) -> Result<(), Trap> { // synchronize with any other memory it's hoped that the choice of `Relaxed` // here should be correct for our use case. let wasm_stack_limit = stack_pointer - store.engine().config().max_wasm_stack; - let interrupts = store.interrupts(); + let interrupts = store.0.interrupts(); match interrupts.stack_limit.swap(wasm_stack_limit, Relaxed) { wasmtime_environ::INTERRUPTED => { // This means that an interrupt happened before we actually @@ -1123,18 +1135,19 @@ fn enter_wasm(store: &mut StoreOpaque<'_>) -> Result<(), Trap> { n => debug_assert_eq!(usize::max_value(), n), } store + .0 .externref_activations_table() .set_stack_canary(Some(stack_pointer)); Ok(()) } -#[inline] -fn exit_wasm(store: &mut StoreOpaque<'_>) { - store.externref_activations_table().set_stack_canary(None); +fn exit_wasm(store: &mut StoreContextMut<'_, T>) { + store.0.externref_activations_table().set_stack_canary(None); // see docs above for why this uses `Relaxed` store + .0 .interrupts() .stack_limit .store(usize::max_value(), Relaxed); diff --git a/crates/wasmtime/src/func/typed.rs b/crates/wasmtime/src/func/typed.rs index a51d5a379c8f..af284473a72a 100644 --- a/crates/wasmtime/src/func/typed.rs +++ b/crates/wasmtime/src/func/typed.rs @@ -1,6 +1,6 @@ use super::{invoke_wasm_and_catch_traps, HostAbi}; use crate::store::StoreOpaque; -use crate::{AsContextMut, ExternRef, Func, Trap, ValType}; +use crate::{AsContextMut, ExternRef, Func, StoreContextMut, Trap, ValType}; use anyhow::{bail, Result}; use std::marker; use std::mem::{self, MaybeUninit}; @@ -71,15 +71,12 @@ where /// This function will panic if it is called when the underlying [`Func`] is /// connected to an asynchronous store. pub fn call(&self, mut store: impl AsContextMut, params: Params) -> Result { - store.as_context_mut().0.exiting_native_hook()?; - let mut store_opaque = store.as_context_mut().opaque(); + let mut store = store.as_context_mut(); assert!( - !store_opaque.async_support(), + !store.0.async_support(), "must use `call_async` with async stores" ); - let r = unsafe { self._call(&mut store_opaque, params) }; - store.as_context_mut().0.entering_native_hook()?; - r + unsafe { self._call(&mut store, params) } } /// Invokes this WebAssembly function with the specified parameters. @@ -103,24 +100,25 @@ where where T: Send, { - store.as_context_mut().0.exiting_native_hook()?; - let mut store_opaque = store.as_context_mut().opaque_send(); + let mut store = store.as_context_mut(); assert!( - store_opaque.async_support(), + store.0.async_support(), "must use `call` with non-async stores" ); - let r = store_opaque + store .on_fiber(|store| unsafe { self._call(store, params) }) - .await?; - store.as_context_mut().0.entering_native_hook()?; - r + .await? } - unsafe fn _call(&self, store: &mut StoreOpaque<'_>, params: Params) -> Result { + unsafe fn _call( + &self, + store: &mut StoreContextMut<'_, T>, + params: Params, + ) -> Result { // Validate that all runtime values flowing into this store indeed // belong within this store, otherwise it would be unsafe for store // values to cross each other. - let params = match params.into_abi(store) { + let params = match params.into_abi(&mut store.as_context_mut().opaque()) { Some(abi) => abi, None => { return Err(Trap::new( @@ -135,7 +133,7 @@ where // other side of a C++ shim, so it can never be inlined enough to make // the memory go away, so the size matters here for performance. let mut captures = ( - self.func.caller_checked_anyfunc(store), + self.func.caller_checked_anyfunc(store.0), MaybeUninit::uninit(), params, false, @@ -156,7 +154,10 @@ where let (_, ret, _, returned) = captures; debug_assert_eq!(result.is_ok(), returned); result?; - Ok(Results::from_abi(store, ret.assume_init())) + Ok(Results::from_abi( + &mut store.as_context_mut().opaque(), + ret.assume_init(), + )) } } diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index 0801b10e00ab..2450779eb8e4 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -1,6 +1,6 @@ use crate::linker::Definition; use crate::signatures::SignatureCollection; -use crate::store::{InstanceId, StoreData, StoreOpaque, StoreOpaqueSend, Stored}; +use crate::store::{InstanceId, StoreData, StoreOpaque, Stored}; use crate::types::matching; use crate::{ AsContext, AsContextMut, Engine, Export, Extern, ExternType, Func, Global, InstanceType, @@ -128,7 +128,7 @@ impl Instance { typecheck_externs(&mut cx, module, imports)?; Instantiator::new(&mut cx, module, ImportSource::Externs(imports))? }; - i.run(store.as_context_mut().opaque()) + i.run(&mut store.as_context_mut()) } /// Same as [`Instance::new`], except for usage in [asynchronous stores]. @@ -164,7 +164,7 @@ impl Instance { typecheck_externs(&mut cx, module, imports)?; Instantiator::new(&mut cx, module, ImportSource::Externs(imports))? }; - i.run_async(store.as_context_mut().opaque_send()).await + i.run_async(&mut store.as_context_mut()).await } pub(crate) fn from_wasmtime(handle: InstanceData, store: &mut StoreOpaque) -> Instance { @@ -466,18 +466,20 @@ impl<'a> Instantiator<'a> { }) } - fn run(&mut self, mut store: StoreOpaque<'_>) -> Result { + fn run(&mut self, store: &mut StoreContextMut<'_, T>) -> Result { assert!( - !store.async_support(), + !store.0.async_support(), "cannot use `new` when async support is enabled on the config" ); // NB: this is the same code as `run_async`. It's intentionally // small but should be kept in sync (modulo the async bits). loop { - if let Some((instance, start, toplevel)) = self.step(&mut store)? { + if let Some((instance, start, toplevel)) = + self.step(&mut store.as_context_mut().opaque())? + { if let Some(start) = start { - Instantiator::start_raw(&mut store, instance, start)?; + Instantiator::start_raw(store, instance, start)?; } if toplevel { break Ok(instance); @@ -487,16 +489,19 @@ impl<'a> Instantiator<'a> { } #[cfg(feature = "async")] - async fn run_async(&mut self, mut store: StoreOpaqueSend<'_>) -> Result { + async fn run_async(&mut self, store: &mut StoreContextMut<'_, T>) -> Result + where + T: Send, + { assert!( - store.async_support(), + store.0.async_support(), "cannot use `new_async` without enabling async support on the config" ); // NB: this is the same code as `run`. It's intentionally // small but should be kept in sync (modulo the async bits). loop { - let step = self.step(&mut store.opaque())?; + let step = self.step(&mut store.as_context_mut().opaque())?; if let Some((instance, start, toplevel)) = step { if let Some(start) = start { store @@ -817,14 +822,18 @@ impl<'a> Instantiator<'a> { } } - fn start_raw(store: &mut StoreOpaque<'_>, instance: Instance, start: FuncIndex) -> Result<()> { - let id = match &store[instance.0] { + fn start_raw( + store: &mut StoreContextMut<'_, T>, + instance: Instance, + start: FuncIndex, + ) -> Result<()> { + let id = match &store.0.store_data()[instance.0] { InstanceData::Instantiated { id, .. } => *id, InstanceData::Synthetic(_) => return Ok(()), }; // If a start function is present, invoke it. Make sure we use all the // trap-handling configuration in `store` as well. - let instance = store.instance(id); + let instance = store.0.instance(id); let f = match instance.lookup_by_declaration(&EntityIndex::Function(start)) { wasmtime_runtime::Export::Function(f) => f, _ => unreachable!(), // valid modules shouldn't hit this @@ -941,20 +950,20 @@ impl InstancePre { /// Panics if any import closed over by this [`InstancePre`] isn't owned by /// `store`, or if `store` has async support enabled. pub fn instantiate(&self, mut store: impl AsContextMut) -> Result { - let mut store = store.as_context_mut().opaque(); // For the unsafety here the typecheck happened at creation time of this // structure and then othrewise the `T` of `InstancePre` connects any // host functions we have in our definition list to the `store` that was // passed in. - unsafe { + let mut instantiator = unsafe { + let mut store = store.as_context_mut().opaque(); self.ensure_comes_from_same_store(&store)?; Instantiator::new( &mut store, &self.module, ImportSource::Definitions(&self.items), )? - .run(store) - } + }; + instantiator.run(&mut store.as_context_mut()) } /// Creates a new instance, running the start function asynchronously @@ -986,7 +995,7 @@ impl InstancePre { ImportSource::Definitions(&self.items), )? }; - i.run_async(store.as_context_mut().opaque_send()).await + i.run_async(&mut store.as_context_mut()).await } fn ensure_comes_from_same_store(&self, store: &StoreOpaque<'_>) -> Result<()> { diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index b10c1b5c916c..503a1a15551f 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -921,7 +921,7 @@ impl StoreInnermost { } } -impl StoreOpaqueSend<'_> { +impl StoreContextMut<'_, T> { /// Executes a synchronous computation `func` asynchronously on a new fiber. /// /// This function will convert the synchronous `func` into an asynchronous @@ -932,21 +932,23 @@ impl StoreOpaqueSend<'_> { /// necessary to suspend the fiber later on and poll sub-futures. It's hoped /// that the various comments are illuminating as to what's going on here. #[cfg(feature = "async")] - pub async fn on_fiber( + pub(crate) async fn on_fiber( &mut self, - func: impl FnOnce(&mut StoreOpaque<'_>) -> R + Send, - ) -> Result { - let config = self.engine.config(); - - debug_assert!(self.async_support()); + func: impl FnOnce(&mut StoreContextMut<'_, T>) -> R + Send, + ) -> Result + where + T: Send, + { + let config = self.engine().config(); + debug_assert!(self.0.async_support()); debug_assert!(config.async_stack_size > 0); let mut slot = None; let future = { - let current_poll_cx = self.async_state.current_poll_cx.get(); - let current_suspend = self.async_state.current_suspend.get(); + let current_poll_cx = self.0.async_state.current_poll_cx.get(); + let current_suspend = self.0.async_state.current_suspend.get(); let stack = self - .engine + .engine() .allocator() .allocate_fiber_stack() .map_err(|e| Trap::from(anyhow::Error::from(e)))?; @@ -970,7 +972,7 @@ impl StoreOpaqueSend<'_> { let _reset = Reset(current_suspend, *current_suspend); *current_suspend = suspend; - *slot = Some(func(&mut self.opaque())); + *slot = Some(func(self)); Ok(()) } }) diff --git a/crates/wasmtime/src/store/context.rs b/crates/wasmtime/src/store/context.rs index 6513c7d2dee9..a90ba317cacb 100644 --- a/crates/wasmtime/src/store/context.rs +++ b/crates/wasmtime/src/store/context.rs @@ -50,18 +50,6 @@ impl<'a, T> StoreContextMut<'a, T> { } } - /// Same as `opaque`, but produces a value which implements the `Send` - /// trait. Useful for futures. - pub(crate) fn opaque_send(mut self) -> StoreOpaqueSend<'a> - where - T: Send, - { - StoreOpaqueSend { - traitobj: self.traitobj(), - inner: self.0, - } - } - fn traitobj(&mut self) -> *mut dyn wasmtime_runtime::Store { unsafe { std::mem::transmute::< @@ -241,38 +229,3 @@ impl<'a> DerefMut for StoreOpaque<'a> { &mut *self.inner } } - -pub struct StoreOpaqueSend<'a> { - /// The actual pointer to the `StoreInner` internals. - inner: &'a mut StoreInnermost, - pub traitobj: *mut dyn wasmtime_runtime::Store, -} - -// This is needed due to the raw pointer `traitobj`, which is simply a pointer -// to `inner` which is already `Send`. -unsafe impl Send for StoreOpaqueSend<'_> {} - -impl StoreOpaqueSend<'_> { - pub fn opaque(&mut self) -> StoreOpaque<'_> { - StoreOpaque { - inner: &mut *self.inner, - traitobj: self.traitobj, - } - } -} - -impl<'a> Deref for StoreOpaqueSend<'a> { - type Target = StoreInnermost; - - #[inline] - fn deref(&self) -> &Self::Target { - &*self.inner - } -} - -impl<'a> DerefMut for StoreOpaqueSend<'a> { - #[inline] - fn deref_mut(&mut self) -> &mut Self::Target { - &mut *self.inner - } -} diff --git a/tests/all/native_hooks.rs b/tests/all/native_hooks.rs index 2a2288550b83..9dc0b331dcfd 100644 --- a/tests/all/native_hooks.rs +++ b/tests/all/native_hooks.rs @@ -200,6 +200,43 @@ async fn call_linked_func_async() -> Result<(), Error> { Ok(()) } +#[test] +fn instantiate() -> Result<(), Error> { + let mut store = Store::::default(); + store.entering_native_code_hook(State::entering_native); + store.exiting_native_code_hook(State::exiting_native); + + let m = Module::new(store.engine(), "(module)")?; + Instance::new(&mut store, &m, &[])?; + assert_eq!(store.data().switches_into_native, 0); + + let m = Module::new(store.engine(), "(module (func) (start 0))")?; + Instance::new(&mut store, &m, &[])?; + assert_eq!(store.data().switches_into_native, 1); + + Ok(()) +} + +#[tokio::test] +async fn instantiate_async() -> Result<(), Error> { + let mut config = Config::new(); + config.async_support(true); + let engine = Engine::new(&config)?; + let mut store = Store::new(&engine, State::default()); + store.entering_native_code_hook(State::entering_native); + store.exiting_native_code_hook(State::exiting_native); + + let m = Module::new(store.engine(), "(module)")?; + Instance::new_async(&mut store, &m, &[]).await?; + assert_eq!(store.data().switches_into_native, 0); + + let m = Module::new(store.engine(), "(module (func) (start 0))")?; + Instance::new_async(&mut store, &m, &[]).await?; + assert_eq!(store.data().switches_into_native, 1); + + Ok(()) +} + enum Context { Native, Vm,