From 9ff3909995255d2a62e7ea103eb44ece2c38457c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 14 Sep 2021 11:23:43 -0700 Subject: [PATCH 1/2] Add `*_unchecked` variants of `Func` APIs for the C API This commit is what is hopefully going to be my last installment within the saga of optimizing function calls in/out of WebAssembly modules in the C API. This is yet another alternative approach to #3345 (sorry) but also contains everything necessary to make the C API fast. As in #3345 the general idea is just moving checks out of the call path in the same style of `TypedFunc`. This new strategy takes inspiration from previously learned attempts effectively "just" exposes how we previously passed `*mut u128` through trampolines for arguments/results. This storage format is formalized through a new `ValRaw` union that is exposed from the `wasmtime` crate. By doing this it made it relatively easy to expose two new APIs: * `Func::new_unchecked` * `Func::call_unchecked` These are the same as their checked equivalents except that they're `unsafe` and they work with `*mut ValRaw` rather than safe slices of `Val`. Working with these eschews type checks and such and requires callers/embedders to do the right thing. These two new functions are then exposed via the C API with new functions, enabling C to have a fast-path of calling/defining functions. This fast path is akin to `Func::wrap` in Rust, although that API can't be built in C due to C not having generics in the same way that Rust has. For some benchmarks, the benchmarks here are: * `nop` - Call a wasm function from the host that does nothing and returns nothing. * `i64` - Call a wasm function from the host, the wasm function calls a host function, and the host function returns an `i64` all the way out to the original caller. * `many` - Call a wasm function from the host, the wasm calls host function with 5 `i32` parameters, and then an `i64` result is returned back to the original host * `i64` host - just the overhead of the wasm calling the host, so the wasm calls the host function in a loop. * `many` host - same as `i64` host, but calling the `many` host function. All numbers in this table are in nanoseconds, and this is just one measurement as well so there's bound to be some variation in the precise numbers here. | Name | Rust | C (before) | C (after) | |-----------|------|------------|-----------| | nop | 19 | 112 | 25 | | i64 | 22 | 207 | 32 | | many | 27 | 189 | 34 | | i64 host | 2 | 38 | 5 | | many host | 7 | 75 | 8 | The main conclusion here is that the C API is significantly faster than before when using the `*_unchecked` variants of APIs. The Rust implementation is still the ceiling (or floor I guess?) for performance The main reason that C is slower than Rust is that a little bit more has to travel through memory where on the Rust side of things we can monomorphize and inline a bit more to get rid of that. Overall though the costs are way way down from where they were originally and I don't plan on doing a whole lot more myself at this time. There's various things we theoretically could do I've considered but implementation-wise I think they'll be much more weighty. --- crates/c-api/include/wasmtime/func.h | 128 ++++++++++++ crates/c-api/include/wasmtime/linker.h | 23 +++ crates/c-api/include/wasmtime/val.h | 54 +++++ crates/c-api/src/func.rs | 60 +++++- crates/c-api/src/linker.rs | 29 ++- crates/c-api/src/val.rs | 24 ++- crates/runtime/src/externref.rs | 1 + crates/runtime/src/lib.rs | 2 +- crates/runtime/src/vmcontext.rs | 20 +- crates/wasmtime/src/externals.rs | 30 +-- crates/wasmtime/src/func.rs | 266 ++++++++++++++++--------- crates/wasmtime/src/linker.rs | 20 +- crates/wasmtime/src/ref.rs | 59 ++++++ crates/wasmtime/src/store.rs | 16 +- crates/wasmtime/src/trampoline/func.rs | 8 +- crates/wasmtime/src/values.rs | 126 +++++------- 16 files changed, 651 insertions(+), 215 deletions(-) diff --git a/crates/c-api/include/wasmtime/func.h b/crates/c-api/include/wasmtime/func.h index f254d922aa0f..83339e8b2b80 100644 --- a/crates/c-api/include/wasmtime/func.h +++ b/crates/c-api/include/wasmtime/func.h @@ -87,6 +87,75 @@ WASM_API_EXTERN void wasmtime_func_new( wasmtime_func_t *ret ); +/** + * \brief Callback signature for #wasmtime_func_new_unchecked. + * + * This is the function signature for host functions that can be made accessible + * to WebAssembly. The arguments to this function are: + * + * \param env user-provided argument passed to #wasmtime_func_new_unchecked + * \param caller a temporary object that can only be used during this function + * call. Used to acquire #wasmtime_context_t or caller's state + * \param args_and_results storage space for both the parameters to the + * function as well as the results of the function. The size of this + * array depends on the function type that the host function is created + * with, but it will be the maximum of the number of parameters and + * number of results. + * + * This callback can optionally return a #wasm_trap_t indicating that a trap + * should be raised in WebAssembly. It's expected that in this case the caller + * relinquishes ownership of the trap and it is passed back to the engine. + * + * This differs from #wasmtime_func_callback_t in that the payload of + * `args_and_results` does not have type information, nor does it have sizing + * information. This is especially unsafe because it's only valid within the + * particular #wasm_functype_t that the function was created with. The onus is + * on the embedder to ensure that `args_and_results` are all read correctly + * for parameters and all written for results within the execution of a + * function. + * + * Parameters will be listed starting at index 0 in the `args_and_results` + * array. Results are also written starting at index 0, which will overwrite + * the arguments. + */ +typedef wasm_trap_t* (*wasmtime_func_unchecked_callback_t)( + void *env, + wasmtime_caller_t* caller, + wasmtime_val_raw_t *args_and_results); + +/** + * \brief Creates a new host function in the same manner of #wasmtime_func_new, + * but the function-to-call has no type information available at runtime. + * + * This function is very similar to #wasmtime_func_new. The difference is that + * this version is "more unsafe" in that when the host callback is invoked there + * is no type information and no checks that the right types of values are + * produced. The onus is on the consumer of this API to ensure that all + * invariants are upheld such as: + * + * * The host callback reads parameters correctly and interprets their types + * correctly. + * * If a trap doesn't happen then all results are written to the results + * pointer. All results must have the correct type. + * * Types such as `funcref` cannot cross stores. + * * Types such as `externref` have valid reference counts. + * + * It's generally only recommended to use this if your application can wrap + * this in a safe embedding. This should not be frequently used due to the + * number of invariants that must be upheld on the wasm<->host boundary. On the + * upside, though, this flavor of host function will be faster to call than + * those created by #wasmtime_func_new (hence the reason for this function's + * existence). + */ +WASM_API_EXTERN void wasmtime_func_new_unchecked( + wasmtime_context_t *store, + const wasm_functype_t* type, + wasmtime_func_unchecked_callback_t callback, + void *env, + void (*finalizer)(void*), + wasmtime_func_t *ret +); + /** * \brief Returns the type of the function specified * @@ -142,6 +211,39 @@ WASM_API_EXTERN wasmtime_error_t *wasmtime_func_call( wasm_trap_t **trap ); +/** + * \brief Call a WebAssembly function in an "unchecked" fashion. + * + * This function is similar to #wasmtime_func_call except that there is no type + * information provided with the arguments (or sizing information). Consequently + * this is less safe to call since it's up to the caller to ensure that `args` + * has an appropriate size and all the parameters are configured with their + * appropriate values/types. Additionally all the results must be interpreted + * correctly if this function returns successfully. + * + * Parameters must be specified starting at index 0 in the `args_and_results` + * array. Results are written starting at index 0, which will overwrite + * the arguments. + * + * Callers must ensure that various correctness variants are upheld when this + * API is called such as: + * + * * The `args_and_results` pointer has enough space to hold all the parameters + * and all the results (but not at the same time). + * * Parameters must all be configured as if they were the correct type. + * * Values such as `externref` and `funcref` are valid within the store being + * called. + * + * When in doubt it's much safer to call #wasmtime_func_call. This function is + * faster than that function, but the tradeoff is that embeddings must uphold + * more invariants rather than relying on Wasmtime to check them for you. + */ +WASM_API_EXTERN wasm_trap_t *wasmtime_func_call_unchecked( + wasmtime_context_t *store, + const wasmtime_func_t *func, + wasmtime_val_raw_t *args_and_results +); + /** * \brief Loads a #wasmtime_extern_t from the caller's context * @@ -172,6 +274,32 @@ WASM_API_EXTERN bool wasmtime_caller_export_get( */ WASM_API_EXTERN wasmtime_context_t* wasmtime_caller_context(wasmtime_caller_t* caller); +/** + * \brief Converts a `raw` nonzero `funcref` value from #wasmtime_val_raw_t + * into a #wasmtime_func_t. + * + * This function can be used to interpret nonzero values of the `funcref` field + * of the #wasmtime_val_raw_t structure. It is assumed that `raw` does not have + * a value of 0, or otherwise the program will abort. + * + * Note that this function is unchecked and unsafe. It's only safe to pass + * values learned from #wasmtime_val_raw_t with the same corresponding + * #wasmtime_context_t that they were produced from. Providing arbitrary values + * to `raw` here or cross-context values with `context` is UB. + */ +WASM_API_EXTERN void wasmtime_func_from_raw( + wasmtime_context_t* context, + size_t raw, + wasmtime_func_t *ret); + +/** + * \brief Converts a `func` which belongs to `context` into a `usize` + * parameter that is suitable for insertion into a #wasmtime_val_raw_t. + */ +WASM_API_EXTERN size_t wasmtime_func_to_raw( + wasmtime_context_t* context, + const wasmtime_func_t *func); + #ifdef __cplusplus } // extern "C" #endif diff --git a/crates/c-api/include/wasmtime/linker.h b/crates/c-api/include/wasmtime/linker.h index 09bc0bb10a24..edd52442dfb8 100644 --- a/crates/c-api/include/wasmtime/linker.h +++ b/crates/c-api/include/wasmtime/linker.h @@ -102,6 +102,8 @@ WASM_API_EXTERN wasmtime_error_t* wasmtime_linker_define( * Note that this function does not create a #wasmtime_func_t. This creates a * store-independent function within the linker, allowing this function * definition to be used with multiple stores. + * + * For more information about host callbacks see #wasmtime_func_new. */ WASM_API_EXTERN wasmtime_error_t* wasmtime_linker_define_func( wasmtime_linker_t *linker, @@ -115,6 +117,27 @@ WASM_API_EXTERN wasmtime_error_t* wasmtime_linker_define_func( void (*finalizer)(void*) ); +/** + * \brief Defines a new function in this linker. + * + * This is the same as #wasmtime_linker_define_func except that it's the analog + * of #wasmtime_func_new_unchecked instead of #wasmtime_func_new. Be sure to + * consult the documentation of #wasmtime_linker_define_func for argument + * information as well as #wasmtime_func_new_unchecked for why this is an + * unsafe API. + */ +WASM_API_EXTERN wasmtime_error_t* wasmtime_linker_define_func_unchecked( + wasmtime_linker_t *linker, + const char *module, + size_t module_len, + const char *name, + size_t name_len, + const wasm_functype_t *ty, + wasmtime_func_unchecked_callback_t cb, + void *data, + void (*finalizer)(void*) +); + /** * \brief Defines WASI functions in this linker. * diff --git a/crates/c-api/include/wasmtime/val.h b/crates/c-api/include/wasmtime/val.h index 43b40ff77e8b..1664887811e7 100644 --- a/crates/c-api/include/wasmtime/val.h +++ b/crates/c-api/include/wasmtime/val.h @@ -63,6 +63,23 @@ WASM_API_EXTERN wasmtime_externref_t *wasmtime_externref_clone(wasmtime_externre */ WASM_API_EXTERN void wasmtime_externref_delete(wasmtime_externref_t *ref); +/** + * \brief Converts a raw `externref` value coming from #wasmtime_val_raw_t into + * a #wasmtime_externref_t. + */ +WASM_API_EXTERN wasmtime_externref_t *wasmtime_externref_from_raw(wasmtime_context_t *context, size_t raw); + +/** + * \brief Converts a #wasmtime_externref_t to a raw value suitable for storing + * into a #wasmtime_val_raw_t. + * + * Note that after this function is called callers must not execute GC within + * the store or the raw value returned here will become invalid. + */ +WASM_API_EXTERN size_t wasmtime_externref_to_raw( + wasmtime_context_t *context, + wasmtime_externref_t *ref); + /// \brief Discriminant stored in #wasmtime_val::kind typedef uint8_t wasmtime_valkind_t; /// \brief Value of #wasmtime_valkind_t meaning that #wasmtime_val_t is an i32 @@ -117,6 +134,43 @@ typedef union wasmtime_valunion { wasmtime_v128 v128; } wasmtime_valunion_t; +/** + * \typedef wasmtime_val_raw_t + * \brief Convenience alias for #wasmtime_val_raw + * + * \union wasmtime_val_raw + * \brief Container for possible wasm values. + * + * This type is used on conjunction with #wasmtime_func_new_unchecked as well + * as #wasmtime_func_call_unchecked. Instances of this type do not have type + * information associated with them, it's up to the embedder to figure out + * how to interpret the bits contained within, often using some other channel + * to determine the type. + */ +typedef union wasmtime_val_raw { + /// Field for when this val is a WebAssembly `i32` value. + int32_t i32; + /// Field for when this val is a WebAssembly `i64` value. + int64_t i64; + /// Field for when this val is a WebAssembly `f32` value. + float32_t f32; + /// Field for when this val is a WebAssembly `f64` value. + float64_t f64; + /// Field for when this val is a WebAssembly `v128` value. + wasmtime_v128 v128; + /// Field for when this val is a WebAssembly `funcref` value. + /// + /// If this is set to 0 then it's a null funcref, otherwise this must be + /// passed to `wasmtime_func_from_raw` to determine the `wasmtime_func_t`. + size_t funcref; + /// Field for when this val is a WebAssembly `externref` value. + /// + /// If this is set to 0 then it's a null externref, otherwise this must be + /// passed to `wasmtime_externref_from_raw` to determine the + /// `wasmtime_externref_t`. + size_t externref; +} wasmtime_val_raw_t; + /** * \typedef wasmtime_val_t * \brief Convenience alias for #wasmtime_val_t diff --git a/crates/c-api/src/func.rs b/crates/c-api/src/func.rs index fe875d951f4b..76ba0759c022 100644 --- a/crates/c-api/src/func.rs +++ b/crates/c-api/src/func.rs @@ -8,7 +8,7 @@ use std::mem::{self, MaybeUninit}; use std::panic::{self, AssertUnwindSafe}; use std::ptr; use std::str; -use wasmtime::{AsContextMut, Caller, Extern, Func, Trap, Val}; +use wasmtime::{AsContextMut, Caller, Extern, Func, Trap, Val, ValRaw}; #[derive(Clone)] #[repr(transparent)] @@ -208,6 +208,9 @@ pub type wasmtime_func_callback_t = extern "C" fn( usize, ) -> Option>; +pub type wasmtime_func_unchecked_callback_t = + extern "C" fn(*mut c_void, *mut wasmtime_caller_t, *mut ValRaw) -> Option>; + #[no_mangle] pub unsafe extern "C" fn wasmtime_func_new( store: CStoreContextMut<'_>, @@ -271,6 +274,35 @@ pub(crate) unsafe fn c_callback_to_rust_fn( } } +#[no_mangle] +pub unsafe extern "C" fn wasmtime_func_new_unchecked( + store: CStoreContextMut<'_>, + ty: &wasm_functype_t, + callback: wasmtime_func_unchecked_callback_t, + data: *mut c_void, + finalizer: Option, + func: &mut Func, +) { + let ty = ty.ty().ty.clone(); + let cb = c_unchecked_callback_to_rust_fn(callback, data, finalizer); + *func = Func::new_unchecked(store, ty, cb); +} + +pub(crate) unsafe fn c_unchecked_callback_to_rust_fn( + callback: wasmtime_func_unchecked_callback_t, + data: *mut c_void, + finalizer: Option, +) -> impl Fn(Caller<'_, crate::StoreData>, *mut ValRaw) -> Result<(), Trap> { + let foreign = crate::ForeignData { data, finalizer }; + move |caller, values| { + let mut caller = wasmtime_caller_t { caller }; + match callback(foreign.data, &mut caller, values) { + None => Ok(()), + Some(trap) => Err(trap.trap), + } + } +} + #[no_mangle] pub unsafe extern "C" fn wasmtime_func_call( mut store: CStoreContextMut<'_>, @@ -329,6 +361,18 @@ pub unsafe extern "C" fn wasmtime_func_call( } } +#[no_mangle] +pub unsafe extern "C" fn wasmtime_func_call_unchecked( + store: CStoreContextMut<'_>, + func: &Func, + args_and_results: *mut ValRaw, +) -> *mut wasm_trap_t { + match func.call_unchecked(store, args_and_results) { + Ok(()) => ptr::null_mut(), + Err(trap) => Box::into_raw(Box::new(wasm_trap_t::new(trap))), + } +} + #[no_mangle] pub extern "C" fn wasmtime_func_type( store: CStoreContext<'_>, @@ -362,3 +406,17 @@ pub unsafe extern "C" fn wasmtime_caller_export_get( crate::initialize(item, which.into()); true } + +#[no_mangle] +pub unsafe extern "C" fn wasmtime_func_from_raw( + store: CStoreContextMut<'_>, + raw: usize, + func: &mut Func, +) { + *func = Func::from_raw(store, raw).unwrap(); +} + +#[no_mangle] +pub unsafe extern "C" fn wasmtime_func_to_raw(store: CStoreContextMut<'_>, func: &Func) -> usize { + func.to_raw(store) +} diff --git a/crates/c-api/src/linker.rs b/crates/c-api/src/linker.rs index 1b582b50c7f1..1ff5624a5257 100644 --- a/crates/c-api/src/linker.rs +++ b/crates/c-api/src/linker.rs @@ -1,7 +1,6 @@ -use crate::func::c_callback_to_rust_fn; use crate::{ bad_utf8, handle_result, wasm_engine_t, wasm_functype_t, wasm_trap_t, wasmtime_error_t, - wasmtime_extern_t, wasmtime_func_callback_t, wasmtime_module_t, CStoreContextMut, + wasmtime_extern_t, wasmtime_module_t, CStoreContextMut, }; use std::ffi::c_void; use std::mem::MaybeUninit; @@ -64,17 +63,39 @@ pub unsafe extern "C" fn wasmtime_linker_define_func( name: *const u8, name_len: usize, ty: &wasm_functype_t, - callback: wasmtime_func_callback_t, + callback: crate::wasmtime_func_callback_t, data: *mut c_void, finalizer: Option, ) -> Option> { let ty = ty.ty().ty.clone(); let module = to_str!(module, module_len); let name = to_str!(name, name_len); - let cb = c_callback_to_rust_fn(callback, data, finalizer); + let cb = crate::func::c_callback_to_rust_fn(callback, data, finalizer); handle_result(linker.linker.func_new(module, name, ty, cb), |_linker| ()) } +#[no_mangle] +pub unsafe extern "C" fn wasmtime_linker_define_func_unchecked( + linker: &mut wasmtime_linker_t, + module: *const u8, + module_len: usize, + name: *const u8, + name_len: usize, + ty: &wasm_functype_t, + callback: crate::wasmtime_func_unchecked_callback_t, + data: *mut c_void, + finalizer: Option, +) -> Option> { + let ty = ty.ty().ty.clone(); + let module = to_str!(module, module_len); + let name = to_str!(name, name_len); + let cb = crate::func::c_unchecked_callback_to_rust_fn(callback, data, finalizer); + handle_result( + linker.linker.func_new_unchecked(module, name, ty, cb), + |_linker| (), + ) +} + #[cfg(feature = "wasi")] #[no_mangle] pub extern "C" fn wasmtime_linker_define_wasi( diff --git a/crates/c-api/src/val.rs b/crates/c-api/src/val.rs index c088390cb2d9..a98ece8bfeb8 100644 --- a/crates/c-api/src/val.rs +++ b/crates/c-api/src/val.rs @@ -1,5 +1,8 @@ use crate::r#ref::{ref_to_val, WasmRefInner}; -use crate::{from_valtype, into_valtype, wasm_ref_t, wasm_valkind_t, wasmtime_valkind_t, WASM_I32}; +use crate::{ + from_valtype, into_valtype, wasm_ref_t, wasm_valkind_t, wasmtime_valkind_t, CStoreContextMut, + WASM_I32, +}; use std::ffi::c_void; use std::mem::{self, ManuallyDrop, MaybeUninit}; use std::ptr; @@ -288,3 +291,22 @@ pub extern "C" fn wasmtime_externref_clone(externref: ManuallyDrop) - #[no_mangle] pub extern "C" fn wasmtime_externref_delete(_val: Option) {} + +#[no_mangle] +pub unsafe extern "C" fn wasmtime_externref_to_raw( + cx: CStoreContextMut<'_>, + val: Option>, +) -> usize { + match val { + Some(ptr) => ptr.to_raw(cx), + None => 0, + } +} + +#[no_mangle] +pub unsafe extern "C" fn wasmtime_externref_from_raw( + _cx: CStoreContextMut<'_>, + val: usize, +) -> Option { + ExternRef::from_raw(val) +} diff --git a/crates/runtime/src/externref.rs b/crates/runtime/src/externref.rs index 340dcf9f2c61..7d110a87ad86 100644 --- a/crates/runtime/src/externref.rs +++ b/crates/runtime/src/externref.rs @@ -344,6 +344,7 @@ impl VMExternRef { /// Nor does this method increment the reference count. You must ensure /// that `self` (or some other clone of `self`) stays alive until /// `clone_from_raw` is called. + #[inline] pub fn as_raw(&self) -> *mut u8 { let ptr = self.0.cast::().as_ptr(); ptr diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index 0be1abec7711..4a2ca4d3a28b 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -56,7 +56,7 @@ pub use crate::traphandlers::{ pub use crate::vmcontext::{ VMCallerCheckedAnyfunc, VMContext, VMFunctionBody, VMFunctionImport, VMGlobalDefinition, VMGlobalImport, VMInterrupts, VMInvokeArgument, VMMemoryDefinition, VMMemoryImport, - VMSharedSignatureIndex, VMTableDefinition, VMTableImport, VMTrampoline, + VMSharedSignatureIndex, VMTableDefinition, VMTableImport, VMTrampoline, ValRaw, }; /// Version number of this crate. diff --git a/crates/runtime/src/vmcontext.rs b/crates/runtime/src/vmcontext.rs index 21660c47dd42..027f414daca9 100644 --- a/crates/runtime/src/vmcontext.rs +++ b/crates/runtime/src/vmcontext.rs @@ -790,10 +790,28 @@ impl VMContext { } } +/// A "raw" and unsafe representation of a WebAssembly value. +/// +/// This is provided for use with the `Func::new_unchecked` and +/// `Func::call_unchecked` APIs. In general it's unlikely you should be using +/// this from Rust, rather using APIs like `Func::wrap` and `TypedFunc::call`. +#[allow(missing_docs)] +#[repr(C)] +#[derive(Copy, Clone)] +pub union ValRaw { + pub i32: i32, + pub i64: i64, + pub f32: u32, + pub f64: u64, + pub v128: u128, + pub funcref: usize, + pub externref: usize, +} + /// Trampoline function pointer type. pub type VMTrampoline = unsafe extern "C" fn( *mut VMContext, // callee vmctx *mut VMContext, // caller vmctx *const VMFunctionBody, // function we're actually calling - *mut u128, // space for arguments and return values + *mut ValRaw, // space for arguments and return values ); diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs index b20a00b9e837..5a5fe3eb9e56 100644 --- a/crates/wasmtime/src/externals.rs +++ b/crates/wasmtime/src/externals.rs @@ -1,6 +1,5 @@ use crate::store::{StoreData, StoreOpaque, Stored}; use crate::trampoline::{generate_global_export, generate_table_export}; -use crate::values::{from_checked_anyfunc, into_checked_anyfunc}; use crate::{ AsContext, AsContextMut, ExternRef, ExternType, Func, GlobalType, Instance, Memory, Module, Mutability, TableType, Trap, Val, ValType, @@ -307,7 +306,7 @@ impl Global { .map(|inner| ExternRef { inner }), ), ValType::FuncRef => { - from_checked_anyfunc(definition.as_anyfunc() as *mut _, store.0) + Val::FuncRef(Func::from_raw(store, definition.as_anyfunc() as usize)) } ValType::V128 => Val::V128(*definition.as_u128()), } @@ -438,28 +437,8 @@ impl Table { } fn _new(store: &mut StoreOpaque, ty: TableType, init: Val) -> Result { - if init.ty() != ty.element() { - bail!( - "table initialization value type {:?} does not have expected type {:?}", - init.ty(), - ty.element(), - ); - } let wasmtime_export = generate_table_export(store, &ty)?; - - let init: runtime::TableElement = match ty.element() { - ValType::FuncRef => into_checked_anyfunc(init, store)?.into(), - ValType::ExternRef => init - .externref() - .ok_or_else(|| { - anyhow!("table initialization value does not have expected type `externref`") - })? - .map(|x| x.inner) - .into(), - ty => bail!("unsupported table element type: {:?}", ty), - }; - - // Initialize entries with the init value. + let init = init.into_table_element(store, ty.element())?; unsafe { let table = Table::from_wasmtime_table(wasmtime_export, store); (*table.wasmtime_table(store)) @@ -503,7 +482,10 @@ impl Table { let table = self.wasmtime_table(store); unsafe { match (*table).get(index)? { - runtime::TableElement::FuncRef(f) => Some(from_checked_anyfunc(f, store)), + runtime::TableElement::FuncRef(f) => { + let func = Func::from_caller_checked_anyfunc(store, f); + Some(Val::FuncRef(func)) + } runtime::TableElement::ExternRef(None) => Some(Val::ExternRef(None)), runtime::TableElement::ExternRef(Some(x)) => { Some(Val::ExternRef(Some(ExternRef { inner: x }))) diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index 942c6ed42676..5615d438c278 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -1,7 +1,7 @@ use crate::store::{StoreData, StoreOpaque, Stored}; use crate::{ AsContext, AsContextMut, CallHook, Engine, Extern, FuncType, Instance, InterruptHandle, - StoreContext, StoreContextMut, Trap, Val, ValType, + StoreContext, StoreContextMut, Trap, Val, ValRaw, ValType, }; use anyhow::{bail, Context as _, Result}; use std::error::Error; @@ -306,20 +306,48 @@ impl Func { #[cfg(compiler)] #[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))] // see build.rs pub fn new( - mut store: impl AsContextMut, + store: impl AsContextMut, ty: FuncType, func: impl Fn(Caller<'_, T>, &[Val], &mut [Val]) -> Result<(), Trap> + Send + Sync + 'static, ) -> Self { - let store = store.as_context_mut().0; - - // part of this unsafety is about matching the `T` to a `Store`, - // which is done through the `AsContextMut` bound above. + let ty_clone = ty.clone(); unsafe { - let host = HostFunc::new(store.engine(), ty, func); - host.into_func(store) + Func::new_unchecked(store, ty, move |caller, values| { + Func::invoke(caller, &ty_clone, values, &func) + }) } } + /// Creates a new [`Func`] with the given arguments, although has fewer + /// runtime checks than [`Func::new`]. + /// + /// This function takes a callback of a different signature than + /// [`Func::new`], instead receiving a raw pointer with a list of [`ValRaw`] + /// structures. These values have no type information associated with them + /// so it's up to the caller to provide a function that will correctly + /// interpret the list of values as those coming from the `ty` specified. + /// + /// If you're calling this from Rust it's recommended to either instead use + /// [`Func::new`] or [`Func::wrap`]. The [`Func::wrap`] API, in particular, + /// is both safer and faster than this API. + /// + /// # Unsafety + /// + /// This function is not safe because it's not known at compile time that + /// the `func` provided correctly interprets the argument types provided to + /// it, or that the results it produces will be of the correct type. + #[cfg(compiler)] + #[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))] // see build.rs + pub unsafe fn new_unchecked( + mut store: impl AsContextMut, + ty: FuncType, + func: impl Fn(Caller<'_, T>, *mut ValRaw) -> Result<(), Trap> + Send + Sync + 'static, + ) -> Self { + let store = store.as_context_mut().0; + let host = HostFunc::new_unchecked(store.engine(), ty, func); + host.into_func(store) + } + /// Creates a new host-defined WebAssembly function which, when called, /// will run the asynchronous computation defined by `func` to completion /// and then return the result to WebAssembly. @@ -412,9 +440,9 @@ impl Func { pub(crate) unsafe fn from_caller_checked_anyfunc( store: &mut StoreOpaque, - anyfunc: *mut VMCallerCheckedAnyfunc, - ) -> Option { - let anyfunc = NonNull::new(anyfunc)?; + raw: *mut VMCallerCheckedAnyfunc, + ) -> Option { + let anyfunc = NonNull::new(raw)?; debug_assert!(anyfunc.as_ref().type_index != VMSharedSignatureIndex::default()); let export = ExportFunction { anyfunc }; Some(Func::from_wasmtime_function(export, store)) @@ -684,6 +712,84 @@ impl Func { self.call_impl(&mut store.as_context_mut(), params, results) } + /// Invokes this function in an "unchecked" fashion, reading parameters and + /// writing results to `params_and_returns`. + /// + /// This function is the same as [`Func::call`] except that the arguments + /// and results both use a different representation. If possible it's + /// recommended to use [`Func::call`] if safety isn't necessary or to use + /// [`Func::typed`] in conjunction with [`TypedFunc::call`] since that's + /// both safer and faster than this method of invoking a function. + /// + /// Note that if this function takes `externref` arguments then it will + /// **not** automatically GC unlike the [`Func::call`] and + /// [`TypedFunc::call`] functions. This means that if this function is + /// invoked many times with new `ExternRef` values and no other GC happens + /// via any other means then no values will get collected. + /// + /// # Unsafety + /// + /// This function is unsafe because the `params_and_returns` argument is not + /// validated at all. It must uphold invariants such as: + /// + /// * It's a valid pointer to an array + /// * It has enough space to store all parameters + /// * It has enough space to store all results (not at the same time as + /// parameters) + /// * Parameters are initially written to the array and have the correct + /// types and such. + /// * Reference types like `externref` and `funcref` are valid at the + /// time of this call and for the `store` specified. + /// + /// These invariants are all upheld for you with [`Func::call`] and + /// [`TypedFunc::call`]. + pub unsafe fn call_unchecked( + &self, + mut store: impl AsContextMut, + params_and_returns: *mut ValRaw, + ) -> Result<(), Trap> { + let mut store = store.as_context_mut(); + let data = &store.0.store_data()[self.0]; + let trampoline = data.trampoline(); + let anyfunc = data.export().anyfunc; + invoke_wasm_and_catch_traps(&mut store, |callee| { + trampoline( + (*anyfunc.as_ptr()).vmctx, + callee, + (*anyfunc.as_ptr()).func_ptr.as_ptr(), + params_and_returns, + ) + }) + } + + /// Converts the raw representation of a `funcref` into an `Option` + /// + /// This is intended to be used in conjunction with [`Func::new_unchecked`], + /// [`Func::call_unchecked`], and [`ValRaw`] with its `funcref` field. + /// + /// # Unsafety + /// + /// This function is not safe because `raw` is not validated at all. The + /// caller must guarantee that `raw` is owned by the `store` provided and is + /// valid within the `store`. + pub unsafe fn from_raw(mut store: impl AsContextMut, raw: usize) -> Option { + Func::from_caller_checked_anyfunc(store.as_context_mut().0, raw as *mut _) + } + + /// Extracts the raw value of this `Func`, which is owned by `store`. + /// + /// This function returns a value that's suitable for writing into the + /// `funcref` field of the [`ValRaw`] structure. + /// + /// # Unsafety + /// + /// The returned value is only valid for as long as the store is alive and + /// this function is properly rooted within it. Additionally this function + /// should not be liberally used since it's a very low-level knob. + pub unsafe fn to_raw(&self, store: impl AsContext) -> usize { + self.caller_checked_anyfunc(store.as_context().0).as_ptr() as usize + } + /// Invokes this function with the `params` given, returning the results /// asynchronously. /// @@ -766,80 +872,46 @@ impl Func { bail!("cross-`Store` values are not currently supported"); } } - let externref_params = ty.as_wasm_func_type().externref_params_count(); - - let mut values_vec = write_params(store.0, externref_params, params, results)?; - // Call the trampoline. - unsafe { - 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.as_ptr()).vmctx, - callee, - (*anyfunc.as_ptr()).func_ptr.as_ptr(), - values_vec.as_mut_ptr(), - ) - })?; + let values_vec_size = params.len().max(ty.results().len()); + + // Whenever we pass `externref`s from host code to Wasm code, they + // go into the `VMExternRefActivationsTable`. But the table might be + // at capacity already, so check for that. If it is at capacity + // (unlikely) then do a GC to free up space. This is necessary + // because otherwise we would either keep filling up the bump chunk + // and making it larger and larger or we would always take the slow + // path when inserting references into the table. + if ty.as_wasm_func_type().externref_params_count() + > store + .0 + .externref_activations_table() + .bump_capacity_remaining() + { + store.gc(); } - read_results(store.0, self, values_vec, results); - return Ok(()); - - fn write_params( - store: &mut StoreOpaque, - externref_params: usize, - params: &[Val], - results: &mut [Val], - ) -> Result> { - let values_vec_size = params.len().max(results.len()); - - let mut values_vec = store.take_wasm_u128_storage(); - debug_assert!(values_vec.is_empty()); - values_vec.extend((0..values_vec_size).map(|_| 0)); - - // Whenever we pass `externref`s from host code to Wasm code, they - // go into the `VMExternRefActivationsTable`. But the table might be - // at capacity already, so check for that. If it is at capacity - // (unlikely) then do a GC to free up space. This is necessary - // because otherwise we would either keep filling up the bump chunk - // and making it larger and larger or we would always take the slow - // path when inserting references into the table. - if externref_params - > store - .externref_activations_table() - .bump_capacity_remaining() - { - store.gc(); - } - - // Store the argument values into `values_vec`. - for (arg, slot) in params.iter().zip(&mut values_vec) { - unsafe { - arg.write_value_without_gc(store, slot); - } + // Store the argument values into `values_vec`. + let mut values_vec = store.0.take_wasm_val_raw_storage(); + debug_assert!(values_vec.is_empty()); + values_vec.resize_with(values_vec_size, || ValRaw { i32: 0 }); + for (arg, slot) in params.iter().cloned().zip(&mut values_vec) { + unsafe { + *slot = arg.to_raw(&mut *store); } + } - Ok(values_vec) + unsafe { + self.call_unchecked(&mut *store, values_vec.as_mut_ptr())?; } - fn read_results( - store: &mut StoreOpaque, - func: &Func, - mut values_vec: Vec, - results: &mut [Val], - ) { - for (i, (ptr, dst)) in values_vec.iter().zip(results).enumerate() { - let ty = store[func.0].ty.results().nth(i).unwrap(); - unsafe { - *dst = Val::read_value_from(store, ptr, ty); - } - } - values_vec.truncate(0); - store.save_wasm_u128_storage(values_vec); + for ((i, slot), val) in results.iter_mut().enumerate().zip(&values_vec) { + let ty = store[self.0].ty.results().nth(i).unwrap(); + *slot = unsafe { Val::from_raw(&mut *store, *val, ty) }; } + values_vec.truncate(0); + store.0.save_wasm_val_raw_storage(values_vec); + Ok(()) } #[inline] @@ -890,7 +962,7 @@ impl Func { fn invoke( mut caller: Caller<'_, T>, ty: &FuncType, - values_vec: *mut u128, + values_vec: *mut ValRaw, func: &dyn Fn(Caller<'_, T>, &[Val], &mut [Val]) -> Result<(), Trap>, ) -> Result<(), Trap> { caller.store.0.call_hook(CallHook::CallingHost)?; @@ -909,10 +981,7 @@ impl Func { let nparams = ty.params().len(); val_vec.reserve(nparams + ty.results().len()); for (i, ty) in ty.params().enumerate() { - unsafe { - let val = Val::read_value_from(caller.store.0, values_vec.add(i), ty); - val_vec.push(val); - } + val_vec.push(unsafe { Val::from_raw(&mut caller.store, *values_vec.add(i), ty) }) } val_vec.extend((0..ty.results().len()).map(|_| Val::null())); @@ -946,7 +1015,7 @@ impl Func { )); } unsafe { - ret.write_value_without_gc(caller.store.0, values_vec.add(i)); + *values_vec.add(i) = ret.to_raw(&mut caller.store); } } @@ -1276,7 +1345,7 @@ pub unsafe trait WasmRet { fn func_type(params: impl Iterator) -> FuncType; #[doc(hidden)] - unsafe fn wrap_trampoline(ptr: *mut u128, f: impl FnOnce(Self::Retptr) -> Self::Abi); + unsafe fn wrap_trampoline(ptr: *mut ValRaw, f: impl FnOnce(Self::Retptr) -> Self::Abi); // Utilities used to convert an instance of this type to a `Result` // explicitly, used when wrapping async functions which always bottom-out @@ -1313,7 +1382,7 @@ where FuncType::new(params, Some(::valtype())) } - unsafe fn wrap_trampoline(ptr: *mut u128, f: impl FnOnce(Self::Retptr) -> Self::Abi) { + unsafe fn wrap_trampoline(ptr: *mut ValRaw, f: impl FnOnce(Self::Retptr) -> Self::Abi) { *ptr.cast::() = f(()); } @@ -1353,7 +1422,7 @@ where T::func_type(params) } - unsafe fn wrap_trampoline(ptr: *mut u128, f: impl FnOnce(Self::Retptr) -> Self::Abi) { + unsafe fn wrap_trampoline(ptr: *mut ValRaw, f: impl FnOnce(Self::Retptr) -> Self::Abi) { T::wrap_trampoline(ptr, f) } @@ -1399,7 +1468,7 @@ macro_rules! impl_wasm_host_results { } #[allow(unused_assignments)] - unsafe fn wrap_trampoline(mut _ptr: *mut u128, f: impl FnOnce(Self::Retptr) -> Self::Abi) { + unsafe fn wrap_trampoline(mut _ptr: *mut ValRaw, f: impl FnOnce(Self::Retptr) -> Self::Abi) { let ($($t,)*) = <($($t::Abi,)*) as HostAbi>::call(f); $( *_ptr.cast() = $t; @@ -1866,7 +1935,7 @@ macro_rules! impl_into_func { callee_vmctx: *mut VMContext, caller_vmctx: *mut VMContext, ptr: *const VMFunctionBody, - args: *mut u128, + args: *mut ValRaw, ) where $($args: WasmTy,)* @@ -1956,14 +2025,23 @@ impl HostFunc { func: impl Fn(Caller<'_, T>, &[Val], &mut [Val]) -> Result<(), Trap> + Send + Sync + 'static, ) -> Self { let ty_clone = ty.clone(); - - // Create a trampoline that converts raw u128 values to `Val` - let func = move |caller_vmctx, values_vec: *mut u128| unsafe { - Caller::with(caller_vmctx, |caller| { - Func::invoke(caller, &ty_clone, values_vec, &func) + unsafe { + HostFunc::new_unchecked(engine, ty, move |caller, values| { + Func::invoke(caller, &ty_clone, values, &func) }) - }; + } + } + /// Analog of [`Func::new_unchecked`] + #[cfg(compiler)] + pub unsafe fn new_unchecked( + engine: &Engine, + ty: FuncType, + func: impl Fn(Caller<'_, T>, *mut ValRaw) -> Result<(), Trap> + Send + Sync + 'static, + ) -> Self { + let func = move |caller_vmctx, values: *mut ValRaw| unsafe { + Caller::::with(caller_vmctx, |caller| func(caller, values)) + }; let (instance, trampoline) = crate::trampoline::create_function(&ty, func, engine) .expect("failed to create function"); HostFunc::_new(engine, instance, trampoline) diff --git a/crates/wasmtime/src/linker.rs b/crates/wasmtime/src/linker.rs index a84b497959eb..9eb91506eb03 100644 --- a/crates/wasmtime/src/linker.rs +++ b/crates/wasmtime/src/linker.rs @@ -3,7 +3,7 @@ use crate::instance::{InstanceData, InstancePre}; use crate::store::StoreOpaque; use crate::{ AsContextMut, Caller, Engine, Extern, ExternType, Func, FuncType, ImportType, Instance, - IntoFunc, Module, StoreContextMut, Trap, Val, + IntoFunc, Module, StoreContextMut, Trap, Val, ValRaw, }; use anyhow::{anyhow, bail, Context, Error, Result}; use log::warn; @@ -315,6 +315,24 @@ impl Linker { Ok(self) } + /// Creates a [`Func::new_unchecked`]-style function named in this linker. + /// + /// For more information see [`Linker::func_wrap`]. + #[cfg(compiler)] + #[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))] // see build.rs + pub unsafe fn func_new_unchecked( + &mut self, + module: &str, + name: &str, + ty: FuncType, + func: impl Fn(Caller<'_, T>, *mut ValRaw) -> Result<(), Trap> + Send + Sync + 'static, + ) -> Result<&mut Self> { + let func = HostFunc::new_unchecked(&self.engine, ty, func); + let key = self.import_key(module, Some(name)); + self.insert(key, Definition::HostFunc(Arc::new(func)))?; + Ok(self) + } + /// Creates a [`Func::new_async`]-style function named in this linker. /// /// For more information see [`Linker::func_wrap`]. diff --git a/crates/wasmtime/src/ref.rs b/crates/wasmtime/src/ref.rs index 9af6057a43d7..f999b96d80a2 100644 --- a/crates/wasmtime/src/ref.rs +++ b/crates/wasmtime/src/ref.rs @@ -1,5 +1,6 @@ #![allow(missing_docs)] +use crate::AsContextMut; use std::any::Any; use wasmtime_runtime::VMExternRef; @@ -40,4 +41,62 @@ impl ExternRef { pub fn ptr_eq(&self, other: &ExternRef) -> bool { VMExternRef::eq(&self.inner, &other.inner) } + + /// Creates a new strongly-owned [`ExternRef`] from the raw value provided. + /// + /// This is intended to be used in conjunction with [`Func::new_unchecked`], + /// [`Func::call_unchecked`], and [`ValRaw`] with its `externref` field. + /// + /// This function assumes that `raw` is an externref value which is + /// currently rooted within the [`Store`]. + /// + /// # Unsafety + /// + /// This function is particularly `unsafe` because `raw` not only must be a + /// valid externref value produced prior by `to_raw` but it must also be + /// correctly rooted within the store. When arguments are provided to a + /// callback with [`Func::new_unchecked`], for example, or returned via + /// [`Func::call_unchecked`], if a GC is performed within the store then + /// floating externref values are not rooted and will be GC'd, meaning that + /// this function will no longer be safe to call with the values cleaned up. + /// This function must be invoked *before* possible GC operations can happen + /// (such as calling wasm). + /// + /// When in doubt try to not use this. Instead use the safe Rust APIs of + /// [`TypedFunc`] and friends. + /// + /// [`Func::call_unchecked`]: crate::Func::call_unchecked + /// [`Func::new_unchecked`]: crate::Func::new_unchecked + /// [`Store`]: crate::Store + /// [`TypedFunc`]: crate::TypedFunc + /// [`ValRaw`]: crate::ValRaw + pub unsafe fn from_raw(raw: usize) -> Option { + let raw = raw as *mut u8; + if raw.is_null() { + None + } else { + Some(ExternRef { + inner: VMExternRef::clone_from_raw(raw), + }) + } + } + + /// Converts this [`ExternRef`] to a raw value suitable to store within a + /// [`ValRaw`]. + /// + /// # Unsafety + /// + /// Produces a raw value which is only safe to pass into a store if a GC + /// doesn't happen between when the value is produce and when it's passed + /// into the store. + /// + /// [`ValRaw`]: crate::ValRaw + pub unsafe fn to_raw(&self, mut store: impl AsContextMut) -> usize { + let externref_ptr = self.inner.as_raw(); + store + .as_context_mut() + .0 + .insert_vmexternref_without_gc(self.inner.clone()); + externref_ptr as usize + } } diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index e797984bc773..365574bd115d 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -76,7 +76,7 @@ //! contents of `StoreOpaque`. This is an invariant that we, as the authors of //! `wasmtime`, must uphold for the public interface to be safe. -use crate::{module::ModuleRegistry, Engine, Module, Trap, Val}; +use crate::{module::ModuleRegistry, Engine, Module, Trap, Val, ValRaw}; use anyhow::{bail, Result}; use std::cell::UnsafeCell; use std::collections::HashMap; @@ -276,7 +276,7 @@ pub struct StoreOpaque { hostcall_val_storage: Vec, /// Same as `hostcall_val_storage`, but for the direction of the host /// calling wasm. - wasm_u128_storage: Vec, + wasm_val_raw_storage: Vec, } #[cfg(feature = "async")] @@ -433,7 +433,7 @@ impl Store { store_data: StoreData::new(), default_callee, hostcall_val_storage: Vec::new(), - wasm_u128_storage: Vec::new(), + wasm_val_raw_storage: Vec::new(), }, limiter: None, call_hook: None, @@ -1182,16 +1182,16 @@ impl StoreOpaque { /// Same as `take_hostcall_val_storage`, but for the direction of the host /// calling wasm. #[inline] - pub fn take_wasm_u128_storage(&mut self) -> Vec { - mem::take(&mut self.wasm_u128_storage) + pub fn take_wasm_val_raw_storage(&mut self) -> Vec { + mem::take(&mut self.wasm_val_raw_storage) } /// Same as `save_hostcall_val_storage`, but for the direction of the host /// calling wasm. #[inline] - pub fn save_wasm_u128_storage(&mut self, storage: Vec) { - if storage.capacity() > self.wasm_u128_storage.capacity() { - self.wasm_u128_storage = storage; + pub fn save_wasm_val_raw_storage(&mut self, storage: Vec) { + if storage.capacity() > self.wasm_val_raw_storage.capacity() { + self.wasm_val_raw_storage = storage; } } } diff --git a/crates/wasmtime/src/trampoline/func.rs b/crates/wasmtime/src/trampoline/func.rs index 3c7e8c84c660..5c2e2a1b8417 100644 --- a/crates/wasmtime/src/trampoline/func.rs +++ b/crates/wasmtime/src/trampoline/func.rs @@ -1,6 +1,6 @@ //! Support for a calling of an imported function. -use crate::{Engine, FuncType, Trap}; +use crate::{Engine, FuncType, Trap, ValRaw}; use anyhow::Result; use std::any::Any; use std::panic::{self, AssertUnwindSafe}; @@ -21,9 +21,9 @@ struct TrampolineState { unsafe extern "C" fn stub_fn( vmctx: *mut VMContext, caller_vmctx: *mut VMContext, - values_vec: *mut u128, + values_vec: *mut ValRaw, ) where - F: Fn(*mut VMContext, *mut u128) -> Result<(), Trap> + 'static, + F: Fn(*mut VMContext, *mut ValRaw) -> Result<(), Trap> + 'static, { // 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 @@ -72,7 +72,7 @@ pub fn create_function( engine: &Engine, ) -> Result<(InstanceHandle, VMTrampoline)> where - F: Fn(*mut VMContext, *mut u128) -> Result<(), Trap> + Send + Sync + 'static, + F: Fn(*mut VMContext, *mut ValRaw) -> Result<(), Trap> + Send + Sync + 'static, { let mut obj = engine.compiler().object()?; let (t1, t2) = engine.compiler().emit_trampoline_obj( diff --git a/crates/wasmtime/src/values.rs b/crates/wasmtime/src/values.rs index c92e511e2072..c9b10a8190e3 100644 --- a/crates/wasmtime/src/values.rs +++ b/crates/wasmtime/src/values.rs @@ -1,9 +1,11 @@ use crate::r#ref::ExternRef; use crate::store::StoreOpaque; -use crate::{Func, ValType}; +use crate::{AsContextMut, Func, ValType}; use anyhow::{bail, Result}; use std::ptr; -use wasmtime_runtime::{self as runtime, VMExternRef}; +use wasmtime_runtime::TableElement; + +pub use wasmtime_runtime::ValRaw; /// Possible runtime values that a WebAssembly module can either consume or /// produce. @@ -93,55 +95,52 @@ impl Val { } } - pub(crate) unsafe fn write_value_without_gc(&self, store: &mut StoreOpaque, 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), - Val::F32(u) => ptr::write(p as *mut u32, u), - Val::F64(u) => ptr::write(p as *mut u64, u), - Val::V128(b) => ptr::write(p as *mut u128, b), - Val::ExternRef(None) => ptr::write(p, 0), - Val::ExternRef(Some(ref x)) => { - let externref_ptr = x.inner.as_raw(); - store.insert_vmexternref_without_gc(x.clone().inner); - ptr::write(p as *mut *mut u8, externref_ptr) + /// Convenience method to convert this [`Val`] into a [`ValRaw`]. + /// + /// # Unsafety + /// + /// This method is unsafe for the reasons that [`ExternRef::to_raw`] and + /// [`Func::to_raw`] are unsafe. + pub unsafe fn to_raw(&self, store: impl AsContextMut) -> ValRaw { + match self { + Val::I32(i) => ValRaw { i32: *i }, + Val::I64(i) => ValRaw { i64: *i }, + Val::F32(u) => ValRaw { f32: *u }, + Val::F64(u) => ValRaw { f64: *u }, + Val::V128(b) => ValRaw { v128: *b }, + Val::ExternRef(e) => { + let externref = match e { + Some(e) => e.to_raw(store), + None => 0, + }; + ValRaw { externref } + } + Val::FuncRef(f) => { + let funcref = match f { + Some(f) => f.to_raw(store), + None => 0, + }; + ValRaw { funcref } } - Val::FuncRef(f) => ptr::write( - p as *mut *mut runtime::VMCallerCheckedAnyfunc, - if let Some(f) = f { - f.caller_checked_anyfunc(store).as_ptr() - } else { - ptr::null_mut() - }, - ), } } - pub(crate) unsafe fn read_value_from( - store: &mut StoreOpaque, - p: *const u128, - ty: ValType, - ) -> Val { + /// Convenience method to convert a [`ValRaw`] into a [`Val`]. + /// + /// # Unsafety + /// + /// This method is unsafe for the reasons that [`ExternRef::from_raw`] and + /// [`Func::from_raw`] are unsafe. Additionaly there's no guarantee + /// otherwise that `raw` should have the type `ty` specified. + pub unsafe fn from_raw(store: impl AsContextMut, raw: ValRaw, 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)), - 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)), - ValType::ExternRef => { - let raw = ptr::read(p as *const *mut u8); - if raw.is_null() { - Val::ExternRef(None) - } else { - Val::ExternRef(Some(ExternRef { - inner: VMExternRef::clone_from_raw(raw), - })) - } - } - ValType::FuncRef => { - let func = ptr::read(p as *const *mut runtime::VMCallerCheckedAnyfunc); - from_checked_anyfunc(func, store) - } + ValType::I32 => Val::I32(raw.i32), + ValType::I64 => Val::I64(raw.i64), + ValType::F32 => Val::F32(raw.f32), + ValType::F64 => Val::F64(raw.f64), + ValType::V128 => Val::V128(raw.v128), + ValType::ExternRef => Val::ExternRef(ExternRef::from_raw(raw.externref)), + ValType::FuncRef => Val::FuncRef(Func::from_raw(store, raw.funcref)), } } @@ -189,25 +188,21 @@ impl Val { self, store: &mut StoreOpaque, ty: ValType, - ) -> Result { + ) -> Result { match (self, ty) { (Val::FuncRef(Some(f)), ValType::FuncRef) => { if !f.comes_from_same_store(store) { bail!("cross-`Store` values are not supported in tables"); } - Ok(runtime::TableElement::FuncRef( + Ok(TableElement::FuncRef( f.caller_checked_anyfunc(store).as_ptr(), )) } - (Val::FuncRef(None), ValType::FuncRef) => { - Ok(runtime::TableElement::FuncRef(ptr::null_mut())) - } + (Val::FuncRef(None), ValType::FuncRef) => Ok(TableElement::FuncRef(ptr::null_mut())), (Val::ExternRef(Some(x)), ValType::ExternRef) => { - Ok(runtime::TableElement::ExternRef(Some(x.inner))) - } - (Val::ExternRef(None), ValType::ExternRef) => { - Ok(runtime::TableElement::ExternRef(None)) + Ok(TableElement::ExternRef(Some(x.inner))) } + (Val::ExternRef(None), ValType::ExternRef) => Ok(TableElement::ExternRef(None)), _ => bail!("value does not match table element type"), } } @@ -293,24 +288,3 @@ impl From for Val { Val::V128(val) } } - -pub(crate) fn into_checked_anyfunc( - val: Val, - store: &mut StoreOpaque, -) -> Result<*mut wasmtime_runtime::VMCallerCheckedAnyfunc> { - if !val.comes_from_same_store(store) { - bail!("cross-`Store` values are not supported"); - } - Ok(match val { - Val::FuncRef(None) => ptr::null_mut(), - Val::FuncRef(Some(f)) => f.caller_checked_anyfunc(store).as_ptr(), - _ => bail!("val is not funcref"), - }) -} - -pub(crate) unsafe fn from_checked_anyfunc( - anyfunc: *mut wasmtime_runtime::VMCallerCheckedAnyfunc, - store: &mut StoreOpaque, -) -> Val { - Val::FuncRef(Func::from_caller_checked_anyfunc(store, anyfunc)) -} From a4e86903ec5be938134fb7b5990b9063d78e7a97 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 24 Sep 2021 11:37:25 -0700 Subject: [PATCH 2/2] Tweak `wasmtime_externref_t` API comments --- crates/c-api/include/wasmtime/val.h | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/c-api/include/wasmtime/val.h b/crates/c-api/include/wasmtime/val.h index 1664887811e7..c17f605cd82b 100644 --- a/crates/c-api/include/wasmtime/val.h +++ b/crates/c-api/include/wasmtime/val.h @@ -66,6 +66,9 @@ WASM_API_EXTERN void wasmtime_externref_delete(wasmtime_externref_t *ref); /** * \brief Converts a raw `externref` value coming from #wasmtime_val_raw_t into * a #wasmtime_externref_t. + * + * Note that the returned #wasmtime_externref_t is an owned value that must be + * deleted via #wasmtime_externref_delete by the caller if it is non-null. */ WASM_API_EXTERN wasmtime_externref_t *wasmtime_externref_from_raw(wasmtime_context_t *context, size_t raw); @@ -73,12 +76,15 @@ WASM_API_EXTERN wasmtime_externref_t *wasmtime_externref_from_raw(wasmtime_conte * \brief Converts a #wasmtime_externref_t to a raw value suitable for storing * into a #wasmtime_val_raw_t. * - * Note that after this function is called callers must not execute GC within - * the store or the raw value returned here will become invalid. + * Note that the returned underlying value is not tracked by Wasmtime's garbage + * collector until it enters WebAssembly. This means that a GC may release the + * context's reference to the raw value, making the raw value invalid within the + * context of the store. Do not perform a GC between calling this function and + * passing it to WebAssembly. */ WASM_API_EXTERN size_t wasmtime_externref_to_raw( wasmtime_context_t *context, - wasmtime_externref_t *ref); + const wasmtime_externref_t *ref); /// \brief Discriminant stored in #wasmtime_val::kind typedef uint8_t wasmtime_valkind_t;