T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix running enter/exit hooks on start functions (#3001)

Browse Source 
This commit fixes running the store's enter/exit hooks into wasm which
accidentally weren't run for an instance's `start` function. The fix
here was mostly to just sink the enter/exit hook much lower in the code
to `invoke_wasm_and_catch_traps`, which is the common entry point for
all wasm calls.

This did involve propagating the `StoreContext<T>` generic rather than
using `StoreOpaque` unfortunately, but it is overally not too too much
code and we generally wanted most of it inlined anyway.
This commit is contained in:
Alex Crichton
2021-06-21 16:31:10 -05:00
committed by GitHub
parent 18cd2f681c
commit 8760bccc8e
6 changed files with 193 additions and 178 deletions
Download Patch File Download Diff File Expand all files Collapse all files

181
crates/wasmtime/src/func.rs
View File

@@ -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<Box<[Val]>> {
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<T>(
&self,
store: &mut StoreOpaque<'_>,
store: &mut StoreContextMut<'_, T>,
my_ty: FuncType,
params: &[Val],
) -> Result<Box<[Val]>> {
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<Vec<u128>> {
// 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<VMCallerCheckedAnyfunc> {
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<T>(
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<T>(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<T>(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);

37
crates/wasmtime/src/func/typed.rs
View File

@@ -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<Results, Trap> {
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<Results, Trap> {
unsafe fn _call<T>(
&self,
store: &mut StoreContextMut<'_, T>,
params: Params,
) -> Result<Results, Trap> {
// 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(),
))
}
}

45
crates/wasmtime/src/instance.rs
View File

@@ -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<Instance, Error> {
fn run<T>(&mut self, store: &mut StoreContextMut<'_, T>) -> Result<Instance, Error> {
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<Instance, Error> {
async fn run_async<T>(&mut self, store: &mut StoreContextMut<'_, T>) -> Result<Instance, Error>
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<T>(
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<T> InstancePre<T> {
/// 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<Data = T>) -> Result<Instance> {
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<T>` 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<T> InstancePre<T> {
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<()> {

24
crates/wasmtime/src/store.rs
View File

@@ -921,7 +921,7 @@ impl StoreInnermost {
}
}
impl StoreOpaqueSend<'_> {
impl<T> 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<R>(
pub(crate) async fn on_fiber<R>(
&mut self,
func: impl FnOnce(&mut StoreOpaque<'_>) -> R + Send,
) -> Result<R, Trap> {
let config = self.engine.config();
debug_assert!(self.async_support());
func: impl FnOnce(&mut StoreContextMut<'_, T>) -> R + Send,
) -> Result<R, Trap>
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(())
}
})

47
crates/wasmtime/src/store/context.rs
View File

@@ -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
}
}

37
tests/all/native_hooks.rs
View File

@@ -200,6 +200,43 @@ async fn call_linked_func_async() -> Result<(), Error> {
Ok(())
}
#[test]
fn instantiate() -> Result<(), Error> {
let mut store = Store::<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(&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,