From f5fe9b70c9bddcd823c1d3dfa3da1202e1d811a6 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 18 Mar 2020 14:06:05 -0700 Subject: [PATCH 1/9] Remove `WrappedCallable` indirection At this point `Func` has evolved quite a bit since inception and the `WrappedCallable` trait I don't believe is needed any longer. This should help clean up a few entry points by having fewer traits in play. --- crates/api/src/callable.rs | 135 ------------------------------ crates/api/src/func.rs | 128 +++++++++++++++++++++------- crates/api/src/trampoline/func.rs | 3 +- crates/api/src/trampoline/mod.rs | 5 +- 4 files changed, 102 insertions(+), 169 deletions(-) diff --git a/crates/api/src/callable.rs b/crates/api/src/callable.rs index 39791c810fe5..1f2f4d763296 100644 --- a/crates/api/src/callable.rs +++ b/crates/api/src/callable.rs @@ -1,12 +1,5 @@ -use crate::runtime::Store; -use crate::trampoline::generate_func_export; use crate::trap::Trap; -use crate::types::FuncType; use crate::values::Val; -use std::cmp::max; -use std::ptr; -use std::rc::Rc; -use wasmtime_runtime::{ExportFunction, InstanceHandle, VMTrampoline}; /// A trait representing a function that can be imported and called from inside /// WebAssembly. @@ -84,131 +77,3 @@ pub trait Callable { /// function. Produces a `Trap` if the function encounters any errors. fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), Trap>; } - -pub(crate) trait WrappedCallable { - fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), Trap>; - fn wasmtime_handle(&self) -> &InstanceHandle; - fn wasmtime_function(&self) -> &ExportFunction; -} - -pub(crate) struct WasmtimeFn { - store: Store, - instance: InstanceHandle, - export: ExportFunction, - trampoline: VMTrampoline, -} - -impl WasmtimeFn { - pub fn new( - store: &Store, - instance: InstanceHandle, - export: ExportFunction, - trampoline: VMTrampoline, - ) -> WasmtimeFn { - WasmtimeFn { - store: store.clone(), - instance, - export, - trampoline, - } - } -} - -impl WrappedCallable for WasmtimeFn { - fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), Trap> { - let f = self.wasmtime_function(); - let signature = self - .store - .compiler() - .signatures() - .lookup(f.signature) - .expect("missing signature"); - if signature.params.len() - 2 != params.len() { - return Err(Trap::new(format!( - "expected {} arguments, got {}", - signature.params.len() - 2, - params.len() - ))); - } - if signature.returns.len() != results.len() { - return Err(Trap::new(format!( - "expected {} results, got {}", - signature.returns.len(), - results.len() - ))); - } - - let mut values_vec = vec![0; max(params.len(), results.len())]; - - // Store the argument values into `values_vec`. - let param_tys = signature.params.iter().skip(2); - for ((arg, slot), ty) in params.iter().zip(&mut values_vec).zip(param_tys) { - if arg.ty().get_wasmtime_type() != Some(ty.value_type) { - return Err(Trap::new("argument type mismatch")); - } - unsafe { - arg.write_value_to(slot); - } - } - - // Call the trampoline. - if let Err(error) = unsafe { - wasmtime_runtime::wasmtime_call_trampoline( - f.vmctx, - ptr::null_mut(), - self.trampoline, - f.address, - values_vec.as_mut_ptr() as *mut u8, - ) - } { - return Err(Trap::from_jit(error)); - } - - // Load the return values out of `values_vec`. - for (index, abi_param) in signature.returns.iter().enumerate() { - unsafe { - let ptr = values_vec.as_ptr().add(index); - - results[index] = Val::read_value_from(ptr, abi_param.value_type); - } - } - - Ok(()) - } - fn wasmtime_handle(&self) -> &InstanceHandle { - &self.instance - } - fn wasmtime_function(&self) -> &ExportFunction { - &self.export - } -} - -pub struct NativeCallable { - callable: Rc, - instance: InstanceHandle, - export: ExportFunction, -} - -impl NativeCallable { - pub(crate) fn new(callable: Rc, ft: &FuncType, store: &Store) -> Self { - let (instance, export) = - generate_func_export(ft, &callable, store).expect("generated func"); - NativeCallable { - callable, - instance, - export, - } - } -} - -impl WrappedCallable for NativeCallable { - fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), Trap> { - self.callable.call(params, results) - } - fn wasmtime_handle(&self) -> &InstanceHandle { - &self.instance - } - fn wasmtime_function(&self) -> &ExportFunction { - &self.export - } -} diff --git a/crates/api/src/func.rs b/crates/api/src/func.rs index 82843d4581a0..0c4c9e16c8d4 100644 --- a/crates/api/src/func.rs +++ b/crates/api/src/func.rs @@ -1,12 +1,13 @@ -use crate::callable::{NativeCallable, WasmtimeFn, WrappedCallable}; use crate::{Callable, Extern, FuncType, Memory, Store, Trap, Val, ValType}; use anyhow::{ensure, Context as _}; +use std::cmp::max; use std::fmt; use std::mem; use std::panic::{self, AssertUnwindSafe}; use std::ptr; use std::rc::Rc; use wasmtime_runtime::{Export, InstanceHandle, VMContext, VMFunctionBody}; +use wasmtime_runtime::{ExportFunction, VMTrampoline}; /// A WebAssembly function which can be called. /// @@ -144,8 +145,10 @@ use wasmtime_runtime::{Export, InstanceHandle, VMContext, VMFunctionBody}; #[derive(Clone)] pub struct Func { store: Store, - callable: Rc, + instance: InstanceHandle, + export: ExportFunction, ty: FuncType, + trampoline: VMTrampoline, } macro_rules! getters { @@ -220,8 +223,15 @@ impl Func { /// signature given, error or traps may occur if it does not respect the /// `ty` signature. pub fn new(store: &Store, ty: FuncType, callable: Rc) -> Self { - let callable = Rc::new(NativeCallable::new(callable, &ty, &store)); - Func::from_wrapped(store, ty, callable) + let (instance, export, trampoline) = + crate::trampoline::generate_func_export(&ty, &callable, store).expect("generated func"); + Func { + store: store.clone(), + ty, + instance, + export, + trampoline, + } } /// Creates a new `Func` from the given Rust closure. @@ -414,18 +424,6 @@ impl Func { func.into_func(store) } - fn from_wrapped( - store: &Store, - ty: FuncType, - callable: Rc, - ) -> Func { - Func { - store: store.clone(), - callable, - ty, - } - } - /// Returns the underlying wasm type that this `Func` has. pub fn ty(&self) -> &FuncType { &self.ty @@ -451,26 +449,84 @@ impl Func { /// This function should not panic unless the underlying function itself /// initiates a panic. pub fn call(&self, params: &[Val]) -> Result, Trap> { - for param in params { - if !param.comes_from_same_store(&self.store) { - return Err(Trap::new( - "cross-`Store` values are not currently supported", - )); + // for param in params { + // if !param.comes_from_same_store(&self.store) { + // return Err(Trap::new( + // "cross-`Store` values are not currently supported", + // )); + // } + // } + // let mut results = vec![Val::null(); self.result_arity()]; + // self.callable.call(params, &mut results)?; + // Ok(results.into_boxed_slice()) + let signature = self + .store + .compiler() + .signatures() + .lookup(self.export.signature) + .expect("missing signature"); + if signature.params.len() - 2 != params.len() { + return Err(Trap::new(format!( + "expected {} arguments, got {}", + signature.params.len() - 2, + params.len() + ))); + } + // if signature.returns.len() != results.len() { + // return Err(Trap::new(format!( + // "expected {} results, got {}", + // signature.returns.len(), + // results.len() + // ))); + // } + + let mut values_vec = vec![0; max(params.len(), signature.returns.len())]; + + // Store the argument values into `values_vec`. + let param_tys = signature.params.iter().skip(2); + for ((arg, slot), ty) in params.iter().zip(&mut values_vec).zip(param_tys) { + if arg.ty().get_wasmtime_type() != Some(ty.value_type) { + return Err(Trap::new("argument type mismatch")); + } + unsafe { + arg.write_value_to(slot); + } + } + + // Call the trampoline. + if let Err(error) = unsafe { + wasmtime_runtime::wasmtime_call_trampoline( + self.export.vmctx, + ptr::null_mut(), + self.trampoline, + self.export.address, + values_vec.as_mut_ptr() as *mut u8, + ) + } { + return Err(Trap::from_jit(error)); + } + + // Load the return values out of `values_vec`. + let mut results = Vec::with_capacity(signature.returns.len()); + for (index, abi_param) in signature.returns.iter().enumerate() { + unsafe { + let ptr = values_vec.as_ptr().add(index); + + results.push(Val::read_value_from(ptr, abi_param.value_type)); } } - let mut results = vec![Val::null(); self.result_arity()]; - self.callable.call(params, &mut results)?; - Ok(results.into_boxed_slice()) + + Ok(results.into()) } pub(crate) fn wasmtime_function(&self) -> &wasmtime_runtime::ExportFunction { - self.callable.wasmtime_function() + &self.export } pub(crate) fn from_wasmtime_function( export: wasmtime_runtime::ExportFunction, store: &Store, - instance_handle: InstanceHandle, + instance: InstanceHandle, ) -> Self { // Signatures should always be registered in the store's registry of // shared signatures, so we should be able to unwrap safely here. @@ -489,12 +545,17 @@ impl Func { // Each function signature in a module should have a trampoline stored // on that module as well, so unwrap the result here since otherwise // it's a bug in wasmtime. - let trampoline = instance_handle + let trampoline = instance .trampoline(export.signature) .expect("failed to retrieve trampoline from module"); - let callable = WasmtimeFn::new(store, instance_handle, export, trampoline); - Func::from_wrapped(store, ty, Rc::new(callable)) + Func { + instance, + export, + trampoline, + ty, + store: store.clone(), + } } getters! { @@ -998,8 +1059,13 @@ macro_rules! impl_into_func { Box::new((self, store_clone)), ) .expect("failed to generate export"); - let callable = Rc::new(WasmtimeFn::new(store, instance, export, trampoline)); - Func::from_wrapped(store, ty, callable) + Func { + store: store.clone(), + ty, + instance, + export, + trampoline, + } } } } diff --git a/crates/api/src/trampoline/func.rs b/crates/api/src/trampoline/func.rs index 2e15c6b74148..7e94c8de10ce 100644 --- a/crates/api/src/trampoline/func.rs +++ b/crates/api/src/trampoline/func.rs @@ -251,7 +251,7 @@ pub fn create_handle_with_function( ft: &FuncType, func: &Rc, store: &Store, -) -> Result { +) -> Result<(InstanceHandle, VMTrampoline)> { let isa = { let isa_builder = native::builder(); let flag_builder = settings::builder(); @@ -312,6 +312,7 @@ pub fn create_handle_with_function( trampolines, Box::new(trampoline_state), ) + .map(|instance| (instance, trampoline)) } pub unsafe fn create_handle_with_raw_function( diff --git a/crates/api/src/trampoline/mod.rs b/crates/api/src/trampoline/mod.rs index cc05aff0cad0..7d99b62a51ec 100644 --- a/crates/api/src/trampoline/mod.rs +++ b/crates/api/src/trampoline/mod.rs @@ -23,10 +23,11 @@ pub fn generate_func_export( ) -> Result<( wasmtime_runtime::InstanceHandle, wasmtime_runtime::ExportFunction, + VMTrampoline, )> { - let instance = create_handle_with_function(ft, func, store)?; + let (instance, trampoline) = create_handle_with_function(ft, func, store)?; match instance.lookup("trampoline").expect("trampoline export") { - wasmtime_runtime::Export::Function(f) => Ok((instance, f)), + wasmtime_runtime::Export::Function(f) => Ok((instance, f, trampoline)), _ => unreachable!(), } } From 1351004c05b688a57c2c2517328a094fc2b075ef Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 19 Mar 2020 07:51:51 -0700 Subject: [PATCH 2/9] Remove the `Callable` trait This commit removes the `wasmtime::Callable` trait, changing the signature of `Func::new` to take an appropriately typed `Fn`. Additionally the function now always takes `&Caller` like `Func::wrap` optionally can, to empower `Func::new` to have the same capabilities of `Func::wrap`. --- crates/api/src/callable.rs | 79 ------------- crates/api/src/func.rs | 137 +++++++++++++--------- crates/api/src/lib.rs | 2 - crates/api/src/trampoline/func.rs | 80 ++----------- crates/api/src/trampoline/mod.rs | 7 +- crates/api/src/values.rs | 13 +- crates/api/tests/func.rs | 11 +- crates/api/tests/import-indexes.rs | 37 ++---- crates/api/tests/import_calling_export.rs | 53 +++------ crates/api/tests/traps.rs | 53 ++------- 10 files changed, 145 insertions(+), 327 deletions(-) delete mode 100644 crates/api/src/callable.rs diff --git a/crates/api/src/callable.rs b/crates/api/src/callable.rs deleted file mode 100644 index 1f2f4d763296..000000000000 --- a/crates/api/src/callable.rs +++ /dev/null @@ -1,79 +0,0 @@ -use crate::trap::Trap; -use crate::values::Val; - -/// A trait representing a function that can be imported and called from inside -/// WebAssembly. -/// # Example -/// ``` -/// use wasmtime::Val; -/// -/// struct TimesTwo; -/// -/// impl wasmtime::Callable for TimesTwo { -/// fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), wasmtime::Trap> { -/// let mut value = params[0].unwrap_i32(); -/// value *= 2; -/// results[0] = value.into(); -/// -/// Ok(()) -/// } -/// } -/// -/// # fn main () -> Result<(), Box> { -/// // Simple module that imports our host function ("times_two") and re-exports -/// // it as "run". -/// let wat = r#" -/// (module -/// (func $times_two (import "" "times_two") (param i32) (result i32)) -/// (func -/// (export "run") -/// (param i32) -/// (result i32) -/// (local.get 0) -/// (call $times_two)) -/// ) -/// "#; -/// -/// // Initialise environment and our module. -/// let store = wasmtime::Store::default(); -/// let module = wasmtime::Module::new(&store, wat)?; -/// -/// // Define the type of the function we're going to call. -/// let times_two_type = wasmtime::FuncType::new( -/// // Parameters -/// Box::new([wasmtime::ValType::I32]), -/// // Results -/// Box::new([wasmtime::ValType::I32]) -/// ); -/// -/// // Build a reference to the "times_two" function that can be used. -/// let times_two_function = -/// wasmtime::Func::new(&store, times_two_type, std::rc::Rc::new(TimesTwo)); -/// -/// // Create module instance that imports our function -/// let instance = wasmtime::Instance::new( -/// &module, -/// &[times_two_function.into()] -/// )?; -/// -/// // Get "run" function from the exports. -/// let run_function = instance.exports()[0].func().unwrap(); -/// -/// // Borrow and call "run". Returning any error message from Wasm as a string. -/// let original = 5i32; -/// let results = run_function -/// .call(&[original.into()]) -/// .map_err(|trap| trap.to_string())?; -/// -/// // Compare that the results returned matches what we expect. -/// assert_eq!(original * 2, results[0].unwrap_i32()); -/// # Ok(()) -/// # } -/// ``` -pub trait Callable { - /// What is called when the function is invoked in WebAssembly. - /// `params` is an immutable list of parameters provided to the function. - /// `results` is mutable list of results to be potentially set by your - /// function. Produces a `Trap` if the function encounters any errors. - fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), Trap>; -} diff --git a/crates/api/src/func.rs b/crates/api/src/func.rs index 0c4c9e16c8d4..529535cc54e2 100644 --- a/crates/api/src/func.rs +++ b/crates/api/src/func.rs @@ -1,11 +1,10 @@ -use crate::{Callable, Extern, FuncType, Memory, Store, Trap, Val, ValType}; +use crate::{Extern, FuncType, Memory, Store, Trap, Val, ValType}; use anyhow::{ensure, Context as _}; use std::cmp::max; use std::fmt; use std::mem; use std::panic::{self, AssertUnwindSafe}; use std::ptr; -use std::rc::Rc; use wasmtime_runtime::{Export, InstanceHandle, VMContext, VMFunctionBody}; use wasmtime_runtime::{ExportFunction, VMTrampoline}; @@ -101,19 +100,6 @@ use wasmtime_runtime::{ExportFunction, VMTrampoline}; /// /// ``` /// # use wasmtime::*; -/// use std::rc::Rc; -/// -/// struct Double; -/// -/// impl Callable for Double { -/// fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), Trap> { -/// let mut value = params[0].unwrap_i32(); -/// value *= 2; -/// results[0] = value.into(); -/// Ok(()) -/// } -/// } -/// /// # fn main() -> anyhow::Result<()> { /// let store = Store::default(); /// @@ -123,7 +109,12 @@ use wasmtime_runtime::{ExportFunction, VMTrampoline}; /// Box::new([wasmtime::ValType::I32]), /// Box::new([wasmtime::ValType::I32]) /// ); -/// let double = Func::new(&store, double_type, Rc::new(Double)); +/// let double = Func::new(&store, double_type, |_, params, results| { +/// let mut value = params[0].unwrap_i32(); +/// value *= 2; +/// results[0] = value.into(); +/// Ok(()) +/// }); /// /// let module = Module::new( /// &store, @@ -216,15 +207,69 @@ impl Func { /// * `ty` - the signature of this function, used to indicate what the /// inputs and outputs are, which must be WebAssembly types. /// - /// * `callable` - a type implementing the [`Callable`] trait which - /// is the implementation of this `Func` value. + /// * `func` - the native code invoked whenever this `Func` will be called. + /// This closure is provided a [`Caller`] as its first argument to learn + /// information about the caller, and then it's passed a list of + /// parameters as a slice along with a mutable slice of where to write + /// results. /// - /// Note that the implementation of `callable` must adhere to the `ty` + /// Note that the implementation of `func` must adhere to the `ty` /// signature given, error or traps may occur if it does not respect the /// `ty` signature. - pub fn new(store: &Store, ty: FuncType, callable: Rc) -> Self { + /// + /// Additionally note that this is quite a dynamic function since signatures + /// are not statically known. For a more performant `Func` it's recommended + /// to use [`Func::wrap`] if you can because with statically known + /// signatures the engine can optimize the implementation much more. + pub fn new( + store: &Store, + ty: FuncType, + func: impl Fn(&Caller<'_>, &[Val], &mut [Val]) -> Result<(), Trap> + 'static, + ) -> Self { + let store_clone = store.clone(); + let ty_clone = ty.clone(); + + // Create our actual trampoline function which translates from a bunch + // of bit patterns on the stack to actual instances of `Val` being + // passed to the given function. + let func = Box::new(move |caller_vmctx, values_vec: *mut i128| { + // We have a dynamic guarantee that `values_vec` has the right + // number of arguments and the right types of arguments. As a result + // we should be able to safely run through them all and read them. + let mut args = Vec::with_capacity(ty_clone.params().len()); + for (i, ty) in ty_clone.params().iter().enumerate() { + unsafe { + args.push(Val::read_value_from(values_vec.offset(i as isize), ty)); + } + } + let mut returns = vec![Val::null(); ty_clone.results().len()]; + func( + &Caller { + store: &store_clone, + caller_vmctx, + }, + &args, + &mut returns, + )?; + + // Unlike our arguments we need to dynamically check that the return + // values produced are correct. There could be a bug in `func` that + // produces the wrong number or wrong types of values, and we need + // to catch that here. + for (i, (ret, ty)) in returns.iter_mut().zip(ty_clone.results()).enumerate() { + if ret.ty() != *ty { + return Err(Trap::new( + "function attempted to return an incompatible value", + )); + } + unsafe { + ret.write_value_to(values_vec.add(i)); + } + } + Ok(()) + }); let (instance, export, trampoline) = - crate::trampoline::generate_func_export(&ty, &callable, store).expect("generated func"); + crate::trampoline::generate_func_export(&ty, func, store).expect("generated func"); Func { store: store.clone(), ty, @@ -449,45 +494,32 @@ impl Func { /// This function should not panic unless the underlying function itself /// initiates a panic. pub fn call(&self, params: &[Val]) -> Result, Trap> { - // for param in params { - // if !param.comes_from_same_store(&self.store) { - // return Err(Trap::new( - // "cross-`Store` values are not currently supported", - // )); - // } - // } - // let mut results = vec![Val::null(); self.result_arity()]; - // self.callable.call(params, &mut results)?; - // Ok(results.into_boxed_slice()) - let signature = self - .store - .compiler() - .signatures() - .lookup(self.export.signature) - .expect("missing signature"); - if signature.params.len() - 2 != params.len() { + // 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 self.ty.params().len() != params.len() { return Err(Trap::new(format!( "expected {} arguments, got {}", - signature.params.len() - 2, + self.ty.params().len(), params.len() ))); } - // if signature.returns.len() != results.len() { - // return Err(Trap::new(format!( - // "expected {} results, got {}", - // signature.returns.len(), - // results.len() - // ))); - // } - let mut values_vec = vec![0; max(params.len(), signature.returns.len())]; + let mut values_vec = vec![0; max(params.len(), self.ty.results().len())]; // Store the argument values into `values_vec`. - let param_tys = signature.params.iter().skip(2); + let param_tys = self.ty.params().iter(); for ((arg, slot), ty) in params.iter().zip(&mut values_vec).zip(param_tys) { - if arg.ty().get_wasmtime_type() != Some(ty.value_type) { + if arg.ty() != *ty { return Err(Trap::new("argument type mismatch")); } + if !arg.comes_from_same_store(&self.store) { + return Err(Trap::new( + "cross-`Store` values are not currently supported", + )); + } unsafe { arg.write_value_to(slot); } @@ -507,12 +539,11 @@ impl Func { } // Load the return values out of `values_vec`. - let mut results = Vec::with_capacity(signature.returns.len()); - for (index, abi_param) in signature.returns.iter().enumerate() { + let mut results = Vec::with_capacity(self.ty.results().len()); + for (index, ty) in self.ty.results().iter().enumerate() { unsafe { let ptr = values_vec.as_ptr().add(index); - - results.push(Val::read_value_from(ptr, abi_param.value_type)); + results.push(Val::read_value_from(ptr, ty)); } } diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index 785b864d60a7..b6e5cb5ee9f7 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -8,7 +8,6 @@ #![deny(missing_docs, intra_doc_link_resolution_failure)] -mod callable; mod externals; mod frame_info; mod func; @@ -21,7 +20,6 @@ mod trap; mod types; mod values; -pub use crate::callable::Callable; pub use crate::externals::*; pub use crate::frame_info::FrameInfo; pub use crate::func::*; diff --git a/crates/api/src/trampoline/func.rs b/crates/api/src/trampoline/func.rs index 7e94c8de10ce..98a03741743c 100644 --- a/crates/api/src/trampoline/func.rs +++ b/crates/api/src/trampoline/func.rs @@ -1,18 +1,15 @@ //! Support for a calling of an imported function. use super::create_handle::create_handle; -use crate::{Callable, FuncType, Store, Trap, Val}; +use crate::{FuncType, Store, Trap}; use anyhow::{bail, Result}; use std::any::Any; use std::cmp; use std::collections::HashMap; use std::mem; use std::panic::{self, AssertUnwindSafe}; -use std::rc::Rc; -use wasmtime_environ::entity::{EntityRef, PrimaryMap}; -use wasmtime_environ::ir::types; +use wasmtime_environ::entity::PrimaryMap; use wasmtime_environ::isa::TargetIsa; -use wasmtime_environ::wasm::FuncIndex; use wasmtime_environ::{ ir, settings, CompiledFunction, CompiledFunctionUnwindInfo, Export, Module, }; @@ -26,21 +23,14 @@ use wasmtime_jit::{native, CodeMemory}; use wasmtime_runtime::{InstanceHandle, VMContext, VMFunctionBody, VMTrampoline}; struct TrampolineState { - func: Rc, + func: Box Result<(), Trap>>, #[allow(dead_code)] code_memory: CodeMemory, } -impl TrampolineState { - fn new(func: Rc, code_memory: CodeMemory) -> Self { - TrampolineState { func, code_memory } - } -} - unsafe extern "C" fn stub_fn( vmctx: *mut VMContext, - _caller_vmctx: *mut VMContext, - call_id: u32, + caller_vmctx: *mut VMContext, values_vec: *mut i128, ) { // Here we are careful to use `catch_unwind` to ensure Rust panics don't @@ -56,7 +46,9 @@ unsafe extern "C" fn stub_fn( // below will trigger a longjmp, which won't run local destructors if we // have any. To prevent leaks we avoid having any local destructors by // avoiding local variables. - let result = panic::catch_unwind(AssertUnwindSafe(|| call_stub(vmctx, call_id, values_vec))); + let result = panic::catch_unwind(AssertUnwindSafe(|| { + call_stub(vmctx, caller_vmctx, values_vec) + })); match result { Ok(Ok(())) => {} @@ -76,45 +68,15 @@ unsafe extern "C" fn stub_fn( unsafe fn call_stub( vmctx: *mut VMContext, - call_id: u32, + caller_vmctx: *mut VMContext, values_vec: *mut i128, ) -> Result<(), Trap> { let instance = InstanceHandle::from_vmctx(vmctx); - - let (args, returns_len) = { - let module = instance.module_ref(); - let signature = - &module.local.signatures[module.local.functions[FuncIndex::new(call_id as usize)]]; - - let mut args = Vec::new(); - for i in 2..signature.params.len() { - args.push(Val::read_value_from( - values_vec.offset(i as isize - 2), - signature.params[i].value_type, - )) - } - (args, signature.returns.len()) - }; - - let mut returns = vec![Val::null(); returns_len]; let state = &instance .host_state() .downcast_ref::() .expect("state"); - state.func.call(&args, &mut returns)?; - - let module = instance.module_ref(); - let signature = - &module.local.signatures[module.local.functions[FuncIndex::new(call_id as usize)]]; - for (i, ret) in returns.iter_mut().enumerate() { - if ret.ty().get_wasmtime_type() != Some(signature.returns[i].value_type) { - return Err(Trap::new( - "`Callable` attempted to return an incompatible value", - )); - } - ret.write_value_to(values_vec.add(i)); - } - Ok(()) + (state.func)(caller_vmctx, values_vec) } } @@ -123,7 +85,6 @@ fn make_trampoline( isa: &dyn TargetIsa, code_memory: &mut CodeMemory, fn_builder_ctx: &mut FunctionBuilderContext, - call_id: u32, signature: &ir::Signature, ) -> *mut [VMFunctionBody] { // Mostly reverse copy of the similar method from wasmtime's @@ -140,9 +101,6 @@ fn make_trampoline( // Add the caller `vmctx` parameter. stub_sig.params.push(ir::AbiParam::new(pointer_type)); - // Add the `call_id` parameter. - stub_sig.params.push(ir::AbiParam::new(types::I32)); - // Add the `values_vec` parameter. stub_sig.params.push(ir::AbiParam::new(pointer_type)); @@ -188,14 +146,8 @@ fn make_trampoline( let block_params = builder.func.dfg.block_params(block0); let vmctx_ptr_val = block_params[0]; let caller_vmctx_ptr_val = block_params[1]; - let call_id_val = builder.ins().iconst(types::I32, call_id as i64); - let callee_args = vec![ - vmctx_ptr_val, - caller_vmctx_ptr_val, - call_id_val, - values_vec_ptr_val, - ]; + let callee_args = vec![vmctx_ptr_val, caller_vmctx_ptr_val, values_vec_ptr_val]; let new_sig = builder.import_signature(stub_sig); @@ -249,7 +201,7 @@ fn make_trampoline( pub fn create_handle_with_function( ft: &FuncType, - func: &Rc, + func: Box Result<(), Trap>>, store: &Store, ) -> Result<(InstanceHandle, VMTrampoline)> { let isa = { @@ -277,13 +229,7 @@ pub fn create_handle_with_function( module .exports .insert("trampoline".to_string(), Export::Function(func_id)); - let trampoline = make_trampoline( - isa.as_ref(), - &mut code_memory, - &mut fn_builder_ctx, - func_id.index() as u32, - &sig, - ); + let trampoline = make_trampoline(isa.as_ref(), &mut code_memory, &mut fn_builder_ctx, &sig); finished_functions.push(trampoline); // ... and then we also need a trampoline with the standard "trampoline ABI" @@ -304,7 +250,7 @@ pub fn create_handle_with_function( // code memory (makes it executable) and ensuring all our various bits of // state make it into the instance constructors. code_memory.publish(); - let trampoline_state = TrampolineState::new(func.clone(), code_memory); + let trampoline_state = TrampolineState { func, code_memory }; create_handle( module, store, diff --git a/crates/api/src/trampoline/mod.rs b/crates/api/src/trampoline/mod.rs index 7d99b62a51ec..681a5d19b615 100644 --- a/crates/api/src/trampoline/mod.rs +++ b/crates/api/src/trampoline/mod.rs @@ -10,15 +10,14 @@ use self::func::create_handle_with_function; use self::global::create_global; use self::memory::create_handle_with_memory; use self::table::create_handle_with_table; -use super::{Callable, FuncType, GlobalType, MemoryType, Store, TableType, Val}; +use crate::{FuncType, GlobalType, MemoryType, Store, TableType, Trap, Val}; use anyhow::Result; use std::any::Any; -use std::rc::Rc; -use wasmtime_runtime::{VMFunctionBody, VMTrampoline}; +use wasmtime_runtime::{VMContext, VMFunctionBody, VMTrampoline}; pub fn generate_func_export( ft: &FuncType, - func: &Rc, + func: Box Result<(), Trap>>, store: &Store, ) -> Result<( wasmtime_runtime::InstanceHandle, diff --git a/crates/api/src/values.rs b/crates/api/src/values.rs index d775685b937e..e7f67af4bef3 100644 --- a/crates/api/src/values.rs +++ b/crates/api/src/values.rs @@ -2,7 +2,6 @@ use crate::r#ref::AnyRef; use crate::{Func, Store, ValType}; use anyhow::{bail, Result}; use std::ptr; -use wasmtime_environ::ir; /// Possible runtime values that a WebAssembly module can either consume or /// produce. @@ -92,13 +91,13 @@ impl Val { } } - pub(crate) unsafe fn read_value_from(p: *const i128, ty: ir::Type) -> Val { + pub(crate) unsafe fn read_value_from(p: *const i128, ty: &ValType) -> Val { match ty { - ir::types::I32 => Val::I32(ptr::read(p as *const i32)), - ir::types::I64 => Val::I64(ptr::read(p as *const i64)), - ir::types::F32 => Val::F32(ptr::read(p as *const u32)), - ir::types::F64 => Val::F64(ptr::read(p as *const u64)), - ir::types::I8X16 => Val::V128(ptr::read(p as *const u128)), + ValType::I32 => Val::I32(ptr::read(p as *const i32)), + ValType::I64 => Val::I64(ptr::read(p as *const i64)), + ValType::F32 => Val::F32(ptr::read(p as *const u32)), + ValType::F64 => Val::F64(ptr::read(p as *const u64)), + ValType::V128 => Val::V128(ptr::read(p as *const u128)), _ => unimplemented!("Val::read_value_from"), } } diff --git a/crates/api/tests/func.rs b/crates/api/tests/func.rs index 9e014f21ff75..a43d2ef735a7 100644 --- a/crates/api/tests/func.rs +++ b/crates/api/tests/func.rs @@ -1,5 +1,4 @@ use anyhow::Result; -use std::rc::Rc; use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; use wasmtime::*; @@ -238,21 +237,15 @@ fn get_from_wrapper() { #[test] fn get_from_signature() { - struct Foo; - impl Callable for Foo { - fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), Trap> { - panic!() - } - } let store = Store::default(); let ty = FuncType::new(Box::new([]), Box::new([])); - let f = Func::new(&store, ty, Rc::new(Foo)); + let f = Func::new(&store, ty, |_, _, _| panic!()); assert!(f.get0::<()>().is_ok()); assert!(f.get0::().is_err()); assert!(f.get1::().is_err()); let ty = FuncType::new(Box::new([ValType::I32]), Box::new([ValType::F64])); - let f = Func::new(&store, ty, Rc::new(Foo)); + let f = Func::new(&store, ty, |_, _, _| panic!()); assert!(f.get0::<()>().is_err()); assert!(f.get0::().is_err()); assert!(f.get1::().is_err()); diff --git a/crates/api/tests/import-indexes.rs b/crates/api/tests/import-indexes.rs index e1e0d4123870..befef2214388 100644 --- a/crates/api/tests/import-indexes.rs +++ b/crates/api/tests/import-indexes.rs @@ -1,4 +1,3 @@ -use std::rc::Rc; use wasmtime::*; #[test] @@ -15,28 +14,6 @@ fn same_import_names_still_distinct() -> anyhow::Result<()> { ) "#; - struct Ret1; - - impl Callable for Ret1 { - fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), Trap> { - assert!(params.is_empty()); - assert_eq!(results.len(), 1); - results[0] = 1i32.into(); - Ok(()) - } - } - - struct Ret2; - - impl Callable for Ret2 { - fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), Trap> { - assert!(params.is_empty()); - assert_eq!(results.len(), 1); - results[0] = 2.0f32.into(); - Ok(()) - } - } - let store = Store::default(); let module = Module::new(&store, WAT)?; @@ -44,13 +21,23 @@ fn same_import_names_still_distinct() -> anyhow::Result<()> { Func::new( &store, FuncType::new(Box::new([]), Box::new([ValType::I32])), - Rc::new(Ret1), + |_, params, results| { + assert!(params.is_empty()); + assert_eq!(results.len(), 1); + results[0] = 1i32.into(); + Ok(()) + }, ) .into(), Func::new( &store, FuncType::new(Box::new([]), Box::new([ValType::F32])), - Rc::new(Ret2), + |_, params, results| { + assert!(params.is_empty()); + assert_eq!(results.len(), 1); + results[0] = 2.0f32.into(); + Ok(()) + }, ) .into(), ]; diff --git a/crates/api/tests/import_calling_export.rs b/crates/api/tests/import_calling_export.rs index c755e9bb5807..6281d4830c43 100644 --- a/crates/api/tests/import_calling_export.rs +++ b/crates/api/tests/import_calling_export.rs @@ -15,33 +15,24 @@ fn test_import_calling_export() { ) "#; - struct Callback { - pub other: RefCell>, - } - - impl Callable for Callback { - fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), Trap> { - self.other - .borrow() - .as_ref() - .expect("expected a function ref") - .call(&[]) - .expect("expected function not to trap"); - Ok(()) - } - } - let store = Store::default(); let module = Module::new(&store, WAT).expect("failed to create module"); - let callback = Rc::new(Callback { - other: RefCell::new(None), - }); + let other = Rc::new(RefCell::new(None::)); + let other2 = other.clone(); let callback_func = Func::new( &store, FuncType::new(Box::new([]), Box::new([])), - callback.clone(), + move |_, _, _| { + other2 + .borrow() + .as_ref() + .expect("expected a function ref") + .call(&[]) + .expect("expected function not to trap"); + Ok(()) + }, ); let imports = vec![callback_func.into()]; @@ -55,7 +46,7 @@ fn test_import_calling_export() { .func() .expect("expected a run func in the module"); - *callback.other.borrow_mut() = Some( + *other.borrow_mut() = Some( exports[1] .func() .expect("expected an other func in the module") @@ -76,25 +67,17 @@ fn test_returns_incorrect_type() { ) "#; - struct EvilCallback; - - impl Callable for EvilCallback { - fn call(&self, _params: &[Val], results: &mut [Val]) -> Result<(), Trap> { - // Evil! Returns I64 here instead of promised in the signature I32. - results[0] = Val::I64(228); - Ok(()) - } - } - let store = Store::default(); let module = Module::new(&store, WAT).expect("failed to create module"); - let callback = Rc::new(EvilCallback); - let callback_func = Func::new( &store, FuncType::new(Box::new([]), Box::new([ValType::I32])), - callback.clone(), + |_, _, results| { + // Evil! Returns I64 here instead of promised in the signature I32. + results[0] = Val::I64(228); + Ok(()) + }, ); let imports = vec![callback_func.into()]; @@ -111,6 +94,6 @@ fn test_returns_incorrect_type() { let trap = run_func.call(&[]).expect_err("the execution should fail"); assert_eq!( trap.message(), - "`Callable` attempted to return an incompatible value" + "function attempted to return an incompatible value" ); } diff --git a/crates/api/tests/traps.rs b/crates/api/tests/traps.rs index 0705b09427f4..67abb3746f08 100644 --- a/crates/api/tests/traps.rs +++ b/crates/api/tests/traps.rs @@ -1,18 +1,9 @@ use anyhow::Result; use std::panic::{self, AssertUnwindSafe}; -use std::rc::Rc; use wasmtime::*; #[test] fn test_trap_return() -> Result<()> { - struct HelloCallback; - - impl Callable for HelloCallback { - fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), Trap> { - Err(Trap::new("test 123")) - } - } - let store = Store::default(); let wat = r#" (module @@ -23,7 +14,9 @@ fn test_trap_return() -> Result<()> { let module = Module::new(&store, wat)?; let hello_type = FuncType::new(Box::new([]), Box::new([])); - let hello_func = Func::new(&store, hello_type, Rc::new(HelloCallback)); + let hello_func = Func::new(&store, hello_type, |_, _, _| { + Err(Trap::new("test 123")) + }); let instance = Instance::new(&module, &[hello_func.into()])?; let run_func = instance.exports()[0] @@ -74,14 +67,6 @@ fn test_trap_trace() -> Result<()> { #[test] fn test_trap_trace_cb() -> Result<()> { - struct ThrowCallback; - - impl Callable for ThrowCallback { - fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), Trap> { - Err(Trap::new("cb throw")) - } - } - let store = Store::default(); let wat = r#" (module $hello_mod @@ -92,7 +77,7 @@ fn test_trap_trace_cb() -> Result<()> { "#; let fn_type = FuncType::new(Box::new([]), Box::new([])); - let fn_func = Func::new(&store, fn_type, Rc::new(ThrowCallback)); + let fn_func = Func::new(&store, fn_type, |_, _, _| Err(Trap::new("cb throw"))); let module = Module::new(&store, wat)?; let instance = Instance::new(&module, &[fn_func.into()])?; @@ -223,14 +208,6 @@ wasm backtrace: #[test] fn trap_start_function_import() -> Result<()> { - struct ReturnTrap; - - impl Callable for ReturnTrap { - fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), Trap> { - Err(Trap::new("user trap")) - } - } - let store = Store::default(); let binary = wat::parse_str( r#" @@ -243,7 +220,7 @@ fn trap_start_function_import() -> Result<()> { let module = Module::new(&store, &binary)?; let sig = FuncType::new(Box::new([]), Box::new([])); - let func = Func::new(&store, sig, Rc::new(ReturnTrap)); + let func = Func::new(&store, sig, |_, _, _| Err(Trap::new("user trap"))); let err = Instance::new(&module, &[func.into()]).err().unwrap(); assert_eq!(err.downcast_ref::().unwrap().message(), "user trap"); Ok(()) @@ -251,14 +228,6 @@ fn trap_start_function_import() -> Result<()> { #[test] fn rust_panic_import() -> Result<()> { - struct Panic; - - impl Callable for Panic { - fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), Trap> { - panic!("this is a panic"); - } - } - let store = Store::default(); let binary = wat::parse_str( r#" @@ -273,7 +242,7 @@ fn rust_panic_import() -> Result<()> { let module = Module::new(&store, &binary)?; let sig = FuncType::new(Box::new([]), Box::new([])); - let func = Func::new(&store, sig, Rc::new(Panic)); + let func = Func::new(&store, sig, |_, _, _| panic!("this is a panic")); let instance = Instance::new( &module, &[ @@ -302,14 +271,6 @@ fn rust_panic_import() -> Result<()> { #[test] fn rust_panic_start_function() -> Result<()> { - struct Panic; - - impl Callable for Panic { - fn call(&self, _params: &[Val], _results: &mut [Val]) -> Result<(), Trap> { - panic!("this is a panic"); - } - } - let store = Store::default(); let binary = wat::parse_str( r#" @@ -322,7 +283,7 @@ fn rust_panic_start_function() -> Result<()> { let module = Module::new(&store, &binary)?; let sig = FuncType::new(Box::new([]), Box::new([])); - let func = Func::new(&store, sig, Rc::new(Panic)); + let func = Func::new(&store, sig, |_, _, _| panic!("this is a panic")); let err = panic::catch_unwind(AssertUnwindSafe(|| { drop(Instance::new(&module, &[func.into()])); })) From dca57d94795e962ba69ffa8468e00efc36db988f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 19 Mar 2020 07:57:32 -0700 Subject: [PATCH 3/9] Add a test for an already-fixed issue Closes #849 --- crates/api/tests/func.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/api/tests/func.rs b/crates/api/tests/func.rs index a43d2ef735a7..479839694305 100644 --- a/crates/api/tests/func.rs +++ b/crates/api/tests/func.rs @@ -385,3 +385,13 @@ fn caller_memory() -> anyhow::Result<()> { Instance::new(&module, &[f.into()])?; Ok(()) } + +#[test] +fn func_write_nothing() -> anyhow::Result<()> { + let store = Store::default(); + let ty = FuncType::new(Box::new([]), Box::new([ValType::I32])); + let f = Func::new(&store, ty, |_, _, _| Ok(())); + let err = f.call(&[]).unwrap_err(); + assert_eq!(err.message(), "function attempted to return an incompatible value"); + Ok(()) +} From caded3c44f31ec5422cb9001911244734572e664 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 19 Mar 2020 08:12:36 -0700 Subject: [PATCH 4/9] rustfmt --- crates/api/tests/func.rs | 5 ++++- crates/api/tests/traps.rs | 4 +--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/api/tests/func.rs b/crates/api/tests/func.rs index 479839694305..be2cd15e7a0a 100644 --- a/crates/api/tests/func.rs +++ b/crates/api/tests/func.rs @@ -392,6 +392,9 @@ fn func_write_nothing() -> anyhow::Result<()> { let ty = FuncType::new(Box::new([]), Box::new([ValType::I32])); let f = Func::new(&store, ty, |_, _, _| Ok(())); let err = f.call(&[]).unwrap_err(); - assert_eq!(err.message(), "function attempted to return an incompatible value"); + assert_eq!( + err.message(), + "function attempted to return an incompatible value" + ); Ok(()) } diff --git a/crates/api/tests/traps.rs b/crates/api/tests/traps.rs index 67abb3746f08..75513d765a79 100644 --- a/crates/api/tests/traps.rs +++ b/crates/api/tests/traps.rs @@ -14,9 +14,7 @@ fn test_trap_return() -> Result<()> { let module = Module::new(&store, wat)?; let hello_type = FuncType::new(Box::new([]), Box::new([])); - let hello_func = Func::new(&store, hello_type, |_, _, _| { - Err(Trap::new("test 123")) - }); + let hello_func = Func::new(&store, hello_type, |_, _, _| Err(Trap::new("test 123"))); let instance = Instance::new(&module, &[hello_func.into()])?; let run_func = instance.exports()[0] From af3dac166c10fac3bb77f3d6ce880ae351a46042 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 19 Mar 2020 08:51:37 -0700 Subject: [PATCH 5/9] Update more locations for `Callable` --- crates/api/src/trampoline/func.rs | 2 +- crates/c-api/src/lib.rs | 119 ++++++++++++---------------- crates/fuzzing/src/oracles/dummy.rs | 28 ++----- crates/misc/py/src/function.rs | 85 +++++++------------- examples/multi.rs | 23 ++---- 5 files changed, 98 insertions(+), 159 deletions(-) diff --git a/crates/api/src/trampoline/func.rs b/crates/api/src/trampoline/func.rs index 98a03741743c..5e01aad70390 100644 --- a/crates/api/src/trampoline/func.rs +++ b/crates/api/src/trampoline/func.rs @@ -80,7 +80,7 @@ unsafe extern "C" fn stub_fn( } } -/// Create a trampoline for invoking a Callable. +/// Create a trampoline for invoking a function. fn make_trampoline( isa: &dyn TargetIsa, code_memory: &mut CodeMemory, diff --git a/crates/c-api/src/lib.rs b/crates/c-api/src/lib.rs index f9b52a62c6bb..b1d7eb5051f5 100644 --- a/crates/c-api/src/lib.rs +++ b/crates/c-api/src/lib.rs @@ -7,12 +7,11 @@ use std::cell::RefCell; use std::panic::{self, AssertUnwindSafe}; -use std::rc::Rc; use std::{mem, ptr, slice}; use wasmtime::{ - AnyRef, Callable, Config, Engine, ExportType, Extern, ExternType, Func, FuncType, Global, - GlobalType, HostInfo, HostRef, ImportType, Instance, Limits, Memory, MemoryType, Module, Store, - Table, TableType, Trap, Val, ValType, + AnyRef, Config, Engine, ExportType, Extern, ExternType, Func, FuncType, Global, GlobalType, + HostInfo, HostRef, ImportType, Instance, Limits, Memory, MemoryType, Module, Store, Table, + TableType, Trap, Val, ValType, }; mod ext; @@ -622,78 +621,34 @@ impl wasm_val_t { } } -struct Callback { +#[no_mangle] +pub unsafe extern "C" fn wasm_func_new( + store: *mut wasm_store_t, + ty: *const wasm_functype_t, callback: wasm_func_callback_t, -} - -impl Callable for Callback { - fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), Trap> { - let params = params - .iter() - .map(|p| wasm_val_t::from_val(p)) - .collect::>(); - let mut out_results = vec![wasm_val_t::default(); results.len()]; - let func = self.callback.expect("wasm_func_callback_t fn"); - let out = unsafe { func(params.as_ptr(), out_results.as_mut_ptr()) }; - if !out.is_null() { - let trap: Box = unsafe { Box::from_raw(out) }; - return Err(trap.trap.borrow().clone()); - } - for i in 0..results.len() { - results[i] = out_results[i].val(); - } - Ok(()) - } -} - -struct CallbackWithEnv { - callback: wasm_func_callback_with_env_t, - env: *mut std::ffi::c_void, - finalizer: std::option::Option, -} - -impl Callable for CallbackWithEnv { - fn call(&self, params: &[Val], results: &mut [Val]) -> Result<(), Trap> { +) -> *mut wasm_func_t { + let store = &(*store).store.borrow(); + let ty = (*ty).functype.clone(); + let func = Func::new(store, ty, move |_, params, results| { let params = params .iter() .map(|p| wasm_val_t::from_val(p)) .collect::>(); let mut out_results = vec![wasm_val_t::default(); results.len()]; - let func = self.callback.expect("wasm_func_callback_with_env_t fn"); - let out = unsafe { func(self.env, params.as_ptr(), out_results.as_mut_ptr()) }; + let func = callback.expect("wasm_func_callback_t fn"); + let out = func(params.as_ptr(), out_results.as_mut_ptr()); if !out.is_null() { - let trap: Box = unsafe { Box::from_raw(out) }; + let trap: Box = Box::from_raw(out); return Err(trap.trap.borrow().clone()); } for i in 0..results.len() { results[i] = out_results[i].val(); } Ok(()) - } -} - -impl Drop for CallbackWithEnv { - fn drop(&mut self) { - if let Some(finalizer) = self.finalizer { - unsafe { - finalizer(self.env); - } - } - } -} - -#[no_mangle] -pub unsafe extern "C" fn wasm_func_new( - store: *mut wasm_store_t, - ty: *const wasm_functype_t, - callback: wasm_func_callback_t, -) -> *mut wasm_func_t { - let store = &(*store).store.borrow(); - let ty = (*ty).functype.clone(); - let callback = Rc::new(Callback { callback }); + }); let func = Box::new(wasm_func_t { ext: wasm_extern_t { - which: ExternHost::Func(HostRef::new(Func::new(store, ty, callback))), + which: ExternHost::Func(HostRef::new(func)), }, }); Box::into_raw(func) @@ -925,18 +880,48 @@ pub unsafe extern "C" fn wasm_func_new_with_env( ty: *const wasm_functype_t, callback: wasm_func_callback_with_env_t, env: *mut std::ffi::c_void, - finalizer: std::option::Option, + finalizer: Option, ) -> *mut wasm_func_t { let store = &(*store).store.borrow(); let ty = (*ty).functype.clone(); - let callback = Rc::new(CallbackWithEnv { - callback, - env, - finalizer, + + // Create a small object which will run the finalizer when it's dropped, and + // then we move this `run_finalizer` object into the closure below (via the + // `drop(&run_finalizer)` statement so it's all dropped when the closure is + // dropped. + struct RunOnDrop(F); + impl Drop for RunOnDrop { + fn drop(&mut self) { + (self.0)(); + } + } + let run_finalizer = RunOnDrop(move || { + if let Some(finalizer) = finalizer { + finalizer(env); + } + }); + let func = Func::new(store, ty, move |_, params, results| { + drop(&run_finalizer); + let params = params + .iter() + .map(|p| wasm_val_t::from_val(p)) + .collect::>(); + let mut out_results = vec![wasm_val_t::default(); results.len()]; + let func = callback.expect("wasm_func_callback_with_env_t fn"); + let out = func(env, params.as_ptr(), out_results.as_mut_ptr()); + if !out.is_null() { + let trap: Box = Box::from_raw(out); + return Err(trap.trap.borrow().clone()); + } + for i in 0..results.len() { + results[i] = out_results[i].val(); + } + Ok(()) }); + let func = Box::new(wasm_func_t { ext: wasm_extern_t { - which: ExternHost::Func(HostRef::new(Func::new(store, ty, callback))), + which: ExternHost::Func(HostRef::new(func)), }, }); Box::into_raw(func) diff --git a/crates/fuzzing/src/oracles/dummy.rs b/crates/fuzzing/src/oracles/dummy.rs index b06297d37618..b7b6b4b2dec0 100644 --- a/crates/fuzzing/src/oracles/dummy.rs +++ b/crates/fuzzing/src/oracles/dummy.rs @@ -1,8 +1,7 @@ //! Dummy implementations of things that a Wasm module can import. -use std::rc::Rc; use wasmtime::{ - Callable, Extern, ExternType, Func, FuncType, Global, GlobalType, ImportType, Memory, + Extern, ExternType, Func, FuncType, Global, GlobalType, ImportType, Memory, MemoryType, Store, Table, TableType, Trap, Val, ValType, }; @@ -11,7 +10,7 @@ pub fn dummy_imports(store: &Store, import_tys: &[ImportType]) -> Result Extern::Func(DummyFunc::new(&store, func_ty.clone())), + ExternType::Func(func_ty) => Extern::Func(dummy_func(&store, func_ty.clone())), ExternType::Global(global_ty) => { Extern::Global(dummy_global(&store, global_ty.clone())?) } @@ -22,27 +21,14 @@ pub fn dummy_imports(store: &Store, import_tys: &[ImportType]) -> Result Func { - let callable = DummyFunc(ty.clone()); - Func::new(store, ty, Rc::new(callable) as _) - } -} - -impl Callable for DummyFunc { - fn call(&self, _params: &[Val], results: &mut [Val]) -> Result<(), Trap> { - for (ret_ty, result) in self.0.results().iter().zip(results) { +/// Construct a dummy function for the given function type +pub fn dummy_func(store: &Store, ty: FuncType) -> Func { + Func::new(store, ty.clone(), move |_, _, results| { + for (ret_ty, result) in ty.results().iter().zip(results) { *result = dummy_value(ret_ty)?; } - Ok(()) - } + }) } /// Construct a dummy value for the given value type. diff --git a/crates/misc/py/src/function.rs b/crates/misc/py/src/function.rs index 36f8f909d136..45443e23e40b 100644 --- a/crates/misc/py/src/function.rs +++ b/crates/misc/py/src/function.rs @@ -4,7 +4,6 @@ use crate::value::{pyobj_to_value, value_to_pyobj}; use pyo3::exceptions::Exception; use pyo3::prelude::*; use pyo3::types::{PyAny, PyDict, PyTuple}; -use std::rc::Rc; // TODO support non-export functions #[pyclass] @@ -52,27 +51,33 @@ fn parse_annotation_type(s: &str) -> wasmtime::ValType { _ => panic!("unknown type in annotations"), } } +pub fn wrap_into_pyfunction(store: &wasmtime::Store, callable: &PyAny) -> PyResult { + if !callable.hasattr("__annotations__")? { + // TODO support calls without annotations? + return Err(PyErr::new::( + "import is not a function".to_string(), + )); + } -struct WrappedFn { - func: PyObject, - returns_types: Vec, -} - -impl WrappedFn { - pub fn new(func: PyObject, returns_types: Vec) -> Self { - WrappedFn { - func, - returns_types, + let annot = callable.getattr("__annotations__")?.cast_as::()?; + let mut params = Vec::new(); + let mut returns = Vec::new(); + for (name, value) in annot.iter() { + let ty = parse_annotation_type(&value.to_string()); + match name.to_string().as_str() { + "return" => returns.push(ty), + _ => params.push(ty), } } -} -impl wasmtime::Callable for WrappedFn { - fn call( - &self, - params: &[wasmtime::Val], - returns: &mut [wasmtime::Val], - ) -> Result<(), wasmtime::Trap> { + let ft = wasmtime::FuncType::new( + params.into_boxed_slice(), + returns.clone().into_boxed_slice(), + ); + + let gil = Python::acquire_gil(); + let func = callable.to_object(gil.python()); + Ok(wasmtime::Func::new(store, ft, move |_, params, results| { let gil = Python::acquire_gil(); let py = gil.python(); @@ -81,14 +86,13 @@ impl wasmtime::Callable for WrappedFn { .map(|p| match p { wasmtime::Val::I32(i) => i.clone().into_py(py), wasmtime::Val::I64(i) => i.clone().into_py(py), - _ => { - panic!(); - } + wasmtime::Val::F32(i) => i.clone().into_py(py), + wasmtime::Val::F64(i) => i.clone().into_py(py), + _ => panic!(), }) .collect::>(); - let result = self - .func + let result = func .call(py, PyTuple::new(py, params), None) .expect("TODO: convert result to trap"); @@ -101,9 +105,9 @@ impl wasmtime::Callable for WrappedFn { PyTuple::new(py, &[result]) } }; - for (i, ty) in self.returns_types.iter().enumerate() { + for (i, ty) in returns.iter().enumerate() { let result_item = result.get_item(i); - returns[i] = match ty { + results[i] = match ty { wasmtime::ValType::I32 => wasmtime::Val::I32(result_item.extract::().unwrap()), wasmtime::ValType::I64 => wasmtime::Val::I64(result_item.extract::().unwrap()), _ => { @@ -112,34 +116,5 @@ impl wasmtime::Callable for WrappedFn { }; } Ok(()) - } -} - -pub fn wrap_into_pyfunction(store: &wasmtime::Store, callable: &PyAny) -> PyResult { - if !callable.hasattr("__annotations__")? { - // TODO support calls without annotations? - return Err(PyErr::new::( - "import is not a function".to_string(), - )); - } - - let annot = callable.getattr("__annotations__")?.cast_as::()?; - let mut params = Vec::new(); - let mut returns = Vec::new(); - for (name, value) in annot.iter() { - let ty = parse_annotation_type(&value.to_string()); - match name.to_string().as_str() { - "return" => returns.push(ty), - _ => params.push(ty), - } - } - - let ft = wasmtime::FuncType::new( - params.into_boxed_slice(), - returns.clone().into_boxed_slice(), - ); - - let gil = Python::acquire_gil(); - let wrapped = WrappedFn::new(callable.to_object(gil.python()), returns); - Ok(wasmtime::Func::new(store, ft, Rc::new(wrapped))) + })) } diff --git a/examples/multi.rs b/examples/multi.rs index 86574477882c..9d04ec4e2952 100644 --- a/examples/multi.rs +++ b/examples/multi.rs @@ -8,22 +8,8 @@ // You can execute this example with `cargo run --example multi` use anyhow::{format_err, Result}; -use std::rc::Rc; use wasmtime::*; -struct Callback; - -impl Callable for Callback { - fn call(&self, args: &[Val], results: &mut [Val]) -> Result<(), Trap> { - println!("Calling back..."); - println!("> {} {}", args[0].unwrap_i32(), args[1].unwrap_i64()); - - results[0] = Val::I64(args[1].unwrap_i64() + 1); - results[1] = Val::I32(args[0].unwrap_i32() + 1); - Ok(()) - } -} - fn main() -> Result<()> { // Configure our `Store`, but be sure to use a `Config` that enables the // wasm multi-value feature since it's not stable yet. @@ -41,7 +27,14 @@ fn main() -> Result<()> { Box::new([ValType::I32, ValType::I64]), Box::new([ValType::I64, ValType::I32]), ); - let callback_func = Func::new(&store, callback_type, Rc::new(Callback)); + let callback_func = Func::new(&store, callback_type, |_, args, results| { + println!("Calling back..."); + println!("> {} {}", args[0].unwrap_i32(), args[1].unwrap_i64()); + + results[0] = Val::I64(args[1].unwrap_i64() + 1); + results[1] = Val::I32(args[0].unwrap_i32() + 1); + Ok(()) + }); // Instantiate. println!("Instantiating module..."); From fa0e2b28fe85bf455e6c15139ed7a25f8294a194 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 19 Mar 2020 08:54:27 -0700 Subject: [PATCH 6/9] rustfmt --- crates/fuzzing/src/oracles/dummy.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fuzzing/src/oracles/dummy.rs b/crates/fuzzing/src/oracles/dummy.rs index b7b6b4b2dec0..6db1ab14e291 100644 --- a/crates/fuzzing/src/oracles/dummy.rs +++ b/crates/fuzzing/src/oracles/dummy.rs @@ -1,8 +1,8 @@ //! Dummy implementations of things that a Wasm module can import. use wasmtime::{ - Extern, ExternType, Func, FuncType, Global, GlobalType, ImportType, Memory, - MemoryType, Store, Table, TableType, Trap, Val, ValType, + Extern, ExternType, Func, FuncType, Global, GlobalType, ImportType, Memory, MemoryType, Store, + Table, TableType, Trap, Val, ValType, }; /// Create a set of dummy functions/globals/etc for the given imports. From 5b68a722021e85de2a3df38c76b5b21a5507d285 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 19 Mar 2020 10:28:31 -0700 Subject: [PATCH 7/9] Remove a stray leading borrow --- crates/api/src/func.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/api/src/func.rs b/crates/api/src/func.rs index 529535cc54e2..c8721491a141 100644 --- a/crates/api/src/func.rs +++ b/crates/api/src/func.rs @@ -224,7 +224,7 @@ impl Func { pub fn new( store: &Store, ty: FuncType, - func: impl Fn(&Caller<'_>, &[Val], &mut [Val]) -> Result<(), Trap> + 'static, + func: impl Fn(Caller<'_>, &[Val], &mut [Val]) -> Result<(), Trap> + 'static, ) -> Self { let store_clone = store.clone(); let ty_clone = ty.clone(); @@ -244,7 +244,7 @@ impl Func { } let mut returns = vec![Val::null(); ty_clone.results().len()]; func( - &Caller { + Caller { store: &store_clone, caller_vmctx, }, From 0689e8ac9a52ec5a6d114ef4a35526367b8b4ee2 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 19 Mar 2020 11:12:44 -0700 Subject: [PATCH 8/9] Review feedback --- crates/api/src/func.rs | 6 +++--- crates/api/src/trampoline/func.rs | 8 ++++---- crates/api/src/trampoline/mod.rs | 2 +- crates/api/src/values.rs | 4 ++-- crates/runtime/src/traphandlers.rs | 8 ++------ 5 files changed, 12 insertions(+), 16 deletions(-) diff --git a/crates/api/src/func.rs b/crates/api/src/func.rs index c8721491a141..f876bb1c1f77 100644 --- a/crates/api/src/func.rs +++ b/crates/api/src/func.rs @@ -232,14 +232,14 @@ impl Func { // Create our actual trampoline function which translates from a bunch // of bit patterns on the stack to actual instances of `Val` being // passed to the given function. - let func = Box::new(move |caller_vmctx, values_vec: *mut i128| { + let func = Box::new(move |caller_vmctx, values_vec: *mut u128| { // We have a dynamic guarantee that `values_vec` has the right // number of arguments and the right types of arguments. As a result // we should be able to safely run through them all and read them. let mut args = Vec::with_capacity(ty_clone.params().len()); for (i, ty) in ty_clone.params().iter().enumerate() { unsafe { - args.push(Val::read_value_from(values_vec.offset(i as isize), ty)); + args.push(Val::read_value_from(values_vec.add(i), ty)); } } let mut returns = vec![Val::null(); ty_clone.results().len()]; @@ -532,7 +532,7 @@ impl Func { ptr::null_mut(), self.trampoline, self.export.address, - values_vec.as_mut_ptr() as *mut u8, + values_vec.as_mut_ptr(), ) } { return Err(Trap::from_jit(error)); diff --git a/crates/api/src/trampoline/func.rs b/crates/api/src/trampoline/func.rs index 5e01aad70390..4d64482deacc 100644 --- a/crates/api/src/trampoline/func.rs +++ b/crates/api/src/trampoline/func.rs @@ -23,7 +23,7 @@ use wasmtime_jit::{native, CodeMemory}; use wasmtime_runtime::{InstanceHandle, VMContext, VMFunctionBody, VMTrampoline}; struct TrampolineState { - func: Box Result<(), Trap>>, + func: Box Result<(), Trap>>, #[allow(dead_code)] code_memory: CodeMemory, } @@ -31,7 +31,7 @@ struct TrampolineState { unsafe extern "C" fn stub_fn( vmctx: *mut VMContext, caller_vmctx: *mut VMContext, - values_vec: *mut i128, + values_vec: *mut u128, ) { // Here we are careful to use `catch_unwind` to ensure Rust panics don't // unwind past us. The primary reason for this is that Rust considers it UB @@ -69,7 +69,7 @@ unsafe extern "C" fn stub_fn( unsafe fn call_stub( vmctx: *mut VMContext, caller_vmctx: *mut VMContext, - values_vec: *mut i128, + values_vec: *mut u128, ) -> Result<(), Trap> { let instance = InstanceHandle::from_vmctx(vmctx); let state = &instance @@ -201,7 +201,7 @@ fn make_trampoline( pub fn create_handle_with_function( ft: &FuncType, - func: Box Result<(), Trap>>, + func: Box Result<(), Trap>>, store: &Store, ) -> Result<(InstanceHandle, VMTrampoline)> { let isa = { diff --git a/crates/api/src/trampoline/mod.rs b/crates/api/src/trampoline/mod.rs index 681a5d19b615..bb43651803c3 100644 --- a/crates/api/src/trampoline/mod.rs +++ b/crates/api/src/trampoline/mod.rs @@ -17,7 +17,7 @@ use wasmtime_runtime::{VMContext, VMFunctionBody, VMTrampoline}; pub fn generate_func_export( ft: &FuncType, - func: Box Result<(), Trap>>, + func: Box Result<(), Trap>>, store: &Store, ) -> Result<( wasmtime_runtime::InstanceHandle, diff --git a/crates/api/src/values.rs b/crates/api/src/values.rs index e7f67af4bef3..a9387a7338b0 100644 --- a/crates/api/src/values.rs +++ b/crates/api/src/values.rs @@ -80,7 +80,7 @@ impl Val { } } - pub(crate) unsafe fn write_value_to(&self, p: *mut i128) { + pub(crate) unsafe fn write_value_to(&self, p: *mut u128) { match self { Val::I32(i) => ptr::write(p as *mut i32, *i), Val::I64(i) => ptr::write(p as *mut i64, *i), @@ -91,7 +91,7 @@ impl Val { } } - pub(crate) unsafe fn read_value_from(p: *const i128, ty: &ValType) -> Val { + pub(crate) unsafe fn read_value_from(p: *const u128, ty: &ValType) -> Val { match ty { ValType::I32 => Val::I32(ptr::read(p as *const i32)), ValType::I64 => Val::I64(ptr::read(p as *const i64)), diff --git a/crates/runtime/src/traphandlers.rs b/crates/runtime/src/traphandlers.rs index 351d71ce4ec5..b239173cc1b3 100644 --- a/crates/runtime/src/traphandlers.rs +++ b/crates/runtime/src/traphandlers.rs @@ -9,7 +9,6 @@ use std::any::Any; use std::cell::Cell; use std::error::Error; use std::fmt; -use std::mem; use std::ptr; use wasmtime_environ::ir; @@ -172,13 +171,10 @@ pub unsafe fn wasmtime_call_trampoline( caller_vmctx: *mut VMContext, trampoline: VMTrampoline, callee: *const VMFunctionBody, - values_vec: *mut u8, + values_vec: *mut u128, ) -> Result<(), Trap> { catch_traps(vmctx, || { - mem::transmute::< - _, - extern "C" fn(*mut VMContext, *mut VMContext, *const VMFunctionBody, *mut u8), - >(trampoline)(vmctx, caller_vmctx, callee, values_vec) + trampoline(vmctx, caller_vmctx, callee, values_vec) }) } From 2ed216445b436070e241c00fcd01691d60784f4b Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 19 Mar 2020 11:17:09 -0700 Subject: [PATCH 9/9] Remove unneeded `wasmtime_call_trampoline` shim --- crates/api/src/func.rs | 15 +++++----- crates/runtime/src/lib.rs | 4 +-- crates/runtime/src/traphandlers.rs | 48 ++++++++---------------------- 3 files changed, 21 insertions(+), 46 deletions(-) diff --git a/crates/api/src/func.rs b/crates/api/src/func.rs index f876bb1c1f77..e596caf46dbc 100644 --- a/crates/api/src/func.rs +++ b/crates/api/src/func.rs @@ -527,13 +527,14 @@ impl Func { // Call the trampoline. if let Err(error) = unsafe { - wasmtime_runtime::wasmtime_call_trampoline( - self.export.vmctx, - ptr::null_mut(), - self.trampoline, - self.export.address, - values_vec.as_mut_ptr(), - ) + wasmtime_runtime::catch_traps(self.export.vmctx, || { + (self.trampoline)( + self.export.vmctx, + ptr::null_mut(), + self.export.address, + values_vec.as_mut_ptr(), + ) + }) } { return Err(Trap::from_jit(error)); } diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index 212defdff843..4359db4806db 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -45,9 +45,7 @@ pub use crate::sig_registry::SignatureRegistry; pub use crate::table::Table; pub use crate::trap_registry::{TrapDescription, TrapRegistration, TrapRegistry}; pub use crate::traphandlers::resume_panic; -pub use crate::traphandlers::{ - catch_traps, raise_lib_trap, raise_user_trap, wasmtime_call_trampoline, Trap, -}; +pub use crate::traphandlers::{catch_traps, raise_lib_trap, raise_user_trap, Trap}; pub use crate::vmcontext::{ VMCallerCheckedAnyfunc, VMContext, VMFunctionBody, VMFunctionImport, VMGlobalDefinition, VMGlobalImport, VMInvokeArgument, VMMemoryDefinition, VMMemoryImport, VMSharedSignatureIndex, diff --git a/crates/runtime/src/traphandlers.rs b/crates/runtime/src/traphandlers.rs index b239173cc1b3..64a59a9090dc 100644 --- a/crates/runtime/src/traphandlers.rs +++ b/crates/runtime/src/traphandlers.rs @@ -3,7 +3,7 @@ use crate::instance::{InstanceHandle, SignalHandler}; use crate::trap_registry::TrapDescription; -use crate::vmcontext::{VMContext, VMFunctionBody, VMTrampoline}; +use crate::vmcontext::VMContext; use backtrace::Backtrace; use std::any::Any; use std::cell::Cell; @@ -61,13 +61,13 @@ cfg_if::cfg_if! { /// /// This function performs as-if a wasm trap was just executed, only the trap /// has a dynamic payload associated with it which is user-provided. This trap -/// payload is then returned from `wasmtime_call` an `wasmtime_call_trampoline` -/// below. +/// payload is then returned from `catch_traps` below. /// /// # Safety /// -/// Only safe to call when wasm code is on the stack, aka `wasmtime_call` or -/// `wasmtime_call_trampoline` must have been previously called. +/// Only safe to call when wasm code is on the stack, aka `catch_traps` must +/// have been previously called. Additionally no Rust destructors can be on the +/// stack. They will be skipped and not executed. pub unsafe fn raise_user_trap(data: Box) -> ! { tls::with(|info| info.unwrap().unwind_with(UnwindReason::UserTrap(data))) } @@ -75,13 +75,13 @@ pub unsafe fn raise_user_trap(data: Box) -> ! { /// Raises a trap from inside library code immediately. /// /// This function performs as-if a wasm trap was just executed. This trap -/// payload is then returned from `wasmtime_call` and `wasmtime_call_trampoline` -/// below. +/// payload is then returned from `catch_traps` below. /// /// # Safety /// -/// Only safe to call when wasm code is on the stack, aka `wasmtime_call` or -/// `wasmtime_call_trampoline` must have been previously called. +/// Only safe to call when wasm code is on the stack, aka `catch_traps` must +/// have been previously called. Additionally no Rust destructors can be on the +/// stack. They will be skipped and not executed. pub unsafe fn raise_lib_trap(trap: Trap) -> ! { tls::with(|info| info.unwrap().unwind_with(UnwindReason::LibTrap(trap))) } @@ -91,8 +91,9 @@ pub unsafe fn raise_lib_trap(trap: Trap) -> ! { /// /// # Safety /// -/// Only safe to call when wasm code is on the stack, aka `wasmtime_call` or -/// `wasmtime_call_trampoline` must have been previously called. +/// Only safe to call when wasm code is on the stack, aka `catch_traps` must +/// have been previously called. Additionally no Rust destructors can be on the +/// stack. They will be skipped and not executed. pub unsafe fn resume_panic(payload: Box) -> ! { tls::with(|info| info.unwrap().unwind_with(UnwindReason::Panic(payload))) } @@ -153,31 +154,6 @@ impl Trap { } } -/// Call the wasm function pointed to by `callee`. -/// -/// * `vmctx` - the callee vmctx argument -/// * `caller_vmctx` - the caller vmctx argument -/// * `trampoline` - the jit-generated trampoline whose ABI takes 4 values, the -/// callee vmctx, the caller vmctx, the `callee` argument below, and then the -/// `values_vec` argument. -/// * `callee` - the third argument to the `trampoline` function -/// * `values_vec` - points to a buffer which holds the incoming arguments, and to -/// which the outgoing return values will be written. -/// -/// Wildly unsafe because it calls raw function pointers and reads/writes raw -/// function pointers. -pub unsafe fn wasmtime_call_trampoline( - vmctx: *mut VMContext, - caller_vmctx: *mut VMContext, - trampoline: VMTrampoline, - callee: *const VMFunctionBody, - values_vec: *mut u128, -) -> Result<(), Trap> { - catch_traps(vmctx, || { - trampoline(vmctx, caller_vmctx, callee, values_vec) - }) -} - /// Catches any wasm traps that happen within the execution of `closure`, /// returning them as a `Result`. ///