From 63d80fc50959b3798848ce85f189cd4147fc3abc Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 2 Feb 2023 11:54:20 -0600 Subject: [PATCH] Remove the need to have a `Store` for an `InstancePre` (#5683) * Remove the need to have a `Store` for an `InstancePre` This commit relaxes a requirement of the `InstancePre` API, notably its construction via `Linker::instantiate_pre`. Previously this function required a `Store` to be present to be able to perform type-checking on the contents of the linker, and now this requirement has been removed. Items stored within a linker are either a `HostFunc`, which has type information inside of it, or an `Extern`, which doesn't have type information inside of it. Due to the usage of `Extern` this is why a `Store` was required during the `InstancePre` construction process, it's used to extract the type of an `Extern`. This commit implements a solution where the type information of an `Extern` is stored alongside the `Extern` itself, meaning that the `InstancePre` construction process no longer requires a `Store`. One caveat of this implementation is that some items, such as tables and memories, technically have a "dynamic type" where during type checking their current size is consulted to match against the minimum size required of an import. This no longer works when using `Linker::instantiate_pre` as the current size used is the one when it was inserted into the linker rather than the one available at instantiation time. It's hoped, however, that this is a relatively esoteric use case that doesn't impact many real-world users. Additionally note that this is an API-breaking change. Not only is the `Store` argument removed from `Linker::instantiate_pre`, but some other methods such as `Linker::define` grew a `Store` argument as the type needs to be extracted when an item is inserted into a linker. Closes #5675 * Fix the C API * Fix benchmark compilation * Add C API docs * Update crates/wasmtime/src/linker.rs Co-authored-by: Andrew Brown --------- Co-authored-by: Andrew Brown --- benches/instantiation.rs | 4 +- crates/c-api/include/wasmtime/linker.h | 2 + crates/c-api/src/linker.rs | 5 +- crates/fuzzing/src/oracles.rs | 127 ++++++++---------- crates/fuzzing/src/oracles/dummy.rs | 7 +- crates/wasmtime/src/component/instance.rs | 5 +- crates/wasmtime/src/externals.rs | 14 +- crates/wasmtime/src/instance.rs | 26 ++-- crates/wasmtime/src/linker.rs | 154 ++++++++++++++++++---- crates/wasmtime/src/types/matching.rs | 68 ++-------- crates/wast/src/spectest.rs | 14 +- tests/all/call_hook.rs | 2 +- tests/all/linker.rs | 44 +++---- 13 files changed, 245 insertions(+), 227 deletions(-) diff --git a/benches/instantiation.rs b/benches/instantiation.rs index ae272c1fcb..33a1e2f2ba 100644 --- a/benches/instantiation.rs +++ b/benches/instantiation.rs @@ -46,7 +46,7 @@ fn bench_sequential(c: &mut Criterion, path: &Path) { let mut linker = Linker::new(&engine); wasmtime_wasi::add_to_linker(&mut linker, |cx| cx).unwrap(); let pre = linker - .instantiate_pre(&mut store(&engine), &module) + .instantiate_pre(&module) .expect("failed to pre-instantiate"); (engine, pre) }); @@ -77,7 +77,7 @@ fn bench_parallel(c: &mut Criterion, path: &Path) { wasmtime_wasi::add_to_linker(&mut linker, |cx| cx).unwrap(); let pre = Arc::new( linker - .instantiate_pre(&mut store(&engine), &module) + .instantiate_pre(&module) .expect("failed to pre-instantiate"), ); (engine, pre) diff --git a/crates/c-api/include/wasmtime/linker.h b/crates/c-api/include/wasmtime/linker.h index edd52442df..453ef73d64 100644 --- a/crates/c-api/include/wasmtime/linker.h +++ b/crates/c-api/include/wasmtime/linker.h @@ -59,6 +59,7 @@ WASM_API_EXTERN void wasmtime_linker_allow_shadowing(wasmtime_linker_t* linker, * \brief Defines a new item in this linker. * * \param linker the linker the name is being defined in. + * \param store the store that the `item` is owned by. * \param module the module name the item is defined under. * \param module_len the byte length of `module` * \param name the field name the item is defined under @@ -73,6 +74,7 @@ WASM_API_EXTERN void wasmtime_linker_allow_shadowing(wasmtime_linker_t* linker, */ WASM_API_EXTERN wasmtime_error_t* wasmtime_linker_define( wasmtime_linker_t *linker, + wasmtime_context_t *store, const char *module, size_t module_len, const char *name, diff --git a/crates/c-api/src/linker.rs b/crates/c-api/src/linker.rs index d5ad429fdd..31a5aeed45 100644 --- a/crates/c-api/src/linker.rs +++ b/crates/c-api/src/linker.rs @@ -1,6 +1,6 @@ use crate::{ bad_utf8, handle_result, wasm_engine_t, wasm_functype_t, wasm_trap_t, wasmtime_error_t, - wasmtime_extern_t, wasmtime_module_t, CStoreContextMut, + wasmtime_extern_t, wasmtime_module_t, CStoreContext, CStoreContextMut, }; use std::ffi::c_void; use std::mem::MaybeUninit; @@ -41,6 +41,7 @@ macro_rules! to_str { #[no_mangle] pub unsafe extern "C" fn wasmtime_linker_define( linker: &mut wasmtime_linker_t, + store: CStoreContext<'_>, module: *const u8, module_len: usize, name: *const u8, @@ -51,7 +52,7 @@ pub unsafe extern "C" fn wasmtime_linker_define( let module = to_str!(module, module_len); let name = to_str!(name, name_len); let item = item.to_extern(); - handle_result(linker.define(module, name, item), |_linker| ()) + handle_result(linker.define(&store, module, name, item), |_linker| ()) } #[no_mangle] diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index e453b20549..2ed29e6a33 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -544,45 +544,40 @@ pub fn table_ops( // test case. const MAX_GCS: usize = 5; - linker - .define( - "", - "gc", - // NB: use `Func::new` so that this can still compile on the old x86 - // backend, where `IntoFunc` isn't implemented for multi-value - // returns. - Func::new( - &mut store, - FuncType::new( - vec![], - vec![ValType::ExternRef, ValType::ExternRef, ValType::ExternRef], - ), - { - let num_dropped = num_dropped.clone(); - let expected_drops = expected_drops.clone(); - let num_gcs = num_gcs.clone(); - move |mut caller: Caller<'_, StoreLimits>, _params, results| { - log::info!("table_ops: GC"); - if num_gcs.fetch_add(1, SeqCst) < MAX_GCS { - caller.gc(); - } - - let a = ExternRef::new(CountDrops(num_dropped.clone())); - let b = ExternRef::new(CountDrops(num_dropped.clone())); - let c = ExternRef::new(CountDrops(num_dropped.clone())); - - log::info!("table_ops: make_refs() -> ({:p}, {:p}, {:p})", a, b, c); - - expected_drops.fetch_add(3, SeqCst); - results[0] = Some(a).into(); - results[1] = Some(b).into(); - results[2] = Some(c).into(); - Ok(()) - } - }, - ), - ) - .unwrap(); + // NB: use `Func::new` so that this can still compile on the old x86 + // backend, where `IntoFunc` isn't implemented for multi-value + // returns. + let func = Func::new( + &mut store, + FuncType::new( + vec![], + vec![ValType::ExternRef, ValType::ExternRef, ValType::ExternRef], + ), + { + let num_dropped = num_dropped.clone(); + let expected_drops = expected_drops.clone(); + let num_gcs = num_gcs.clone(); + move |mut caller: Caller<'_, StoreLimits>, _params, results| { + log::info!("table_ops: GC"); + if num_gcs.fetch_add(1, SeqCst) < MAX_GCS { + caller.gc(); + } + + let a = ExternRef::new(CountDrops(num_dropped.clone())); + let b = ExternRef::new(CountDrops(num_dropped.clone())); + let c = ExternRef::new(CountDrops(num_dropped.clone())); + + log::info!("table_ops: make_refs() -> ({:p}, {:p}, {:p})", a, b, c); + + expected_drops.fetch_add(3, SeqCst); + results[0] = Some(a).into(); + results[1] = Some(b).into(); + results[2] = Some(c).into(); + Ok(()) + } + }, + ); + linker.define(&store, "", "gc", func).unwrap(); linker .func_wrap("", "take_refs", { @@ -624,37 +619,29 @@ pub fn table_ops( }) .unwrap(); - linker - .define( - "", - "make_refs", - // NB: use `Func::new` so that this can still compile on the old - // x86 backend, where `IntoFunc` isn't implemented for - // multi-value returns. - Func::new( - &mut store, - FuncType::new( - vec![], - vec![ValType::ExternRef, ValType::ExternRef, ValType::ExternRef], - ), - { - let num_dropped = num_dropped.clone(); - let expected_drops = expected_drops.clone(); - move |_caller, _params, results| { - log::info!("table_ops: make_refs"); - expected_drops.fetch_add(3, SeqCst); - results[0] = - Some(ExternRef::new(CountDrops(num_dropped.clone()))).into(); - results[1] = - Some(ExternRef::new(CountDrops(num_dropped.clone()))).into(); - results[2] = - Some(ExternRef::new(CountDrops(num_dropped.clone()))).into(); - Ok(()) - } - }, - ), - ) - .unwrap(); + // NB: use `Func::new` so that this can still compile on the old + // x86 backend, where `IntoFunc` isn't implemented for + // multi-value returns. + let func = Func::new( + &mut store, + FuncType::new( + vec![], + vec![ValType::ExternRef, ValType::ExternRef, ValType::ExternRef], + ), + { + let num_dropped = num_dropped.clone(); + let expected_drops = expected_drops.clone(); + move |_caller, _params, results| { + log::info!("table_ops: make_refs"); + expected_drops.fetch_add(3, SeqCst); + results[0] = Some(ExternRef::new(CountDrops(num_dropped.clone()))).into(); + results[1] = Some(ExternRef::new(CountDrops(num_dropped.clone()))).into(); + results[2] = Some(ExternRef::new(CountDrops(num_dropped.clone()))).into(); + Ok(()) + } + }, + ); + linker.define(&store, "", "make_refs", func).unwrap(); let instance = linker.instantiate(&mut store, &module).unwrap(); let run = instance.get_func(&mut store, "run").unwrap(); diff --git a/crates/fuzzing/src/oracles/dummy.rs b/crates/fuzzing/src/oracles/dummy.rs index 7e9601560f..df4ec56621 100644 --- a/crates/fuzzing/src/oracles/dummy.rs +++ b/crates/fuzzing/src/oracles/dummy.rs @@ -8,12 +8,9 @@ pub fn dummy_linker<'module, T>(store: &mut Store, module: &Module) -> Result let mut linker = Linker::new(store.engine()); linker.allow_shadowing(true); for import in module.imports() { + let extern_ = dummy_extern(store, import.ty())?; linker - .define( - import.module(), - import.name(), - dummy_extern(store, import.ty())?, - ) + .define(&store, import.module(), import.name(), extern_) .unwrap(); } Ok(linker) diff --git a/crates/wasmtime/src/component/instance.rs b/crates/wasmtime/src/component/instance.rs index 1064f02e8b..5c00b8bc3c 100644 --- a/crates/wasmtime/src/component/instance.rs +++ b/crates/wasmtime/src/component/instance.rs @@ -1,6 +1,7 @@ use crate::component::func::HostFunc; use crate::component::{Component, ComponentNamedList, Func, Lift, Lower, TypedFunc}; use crate::instance::OwnedImports; +use crate::linker::DefinitionType; use crate::store::{StoreOpaque, Stored}; use crate::{AsContextMut, Module, StoreContextMut}; use anyhow::{anyhow, Context, Result}; @@ -439,13 +440,13 @@ impl<'a> Instantiator<'a> { } let val = unsafe { crate::Extern::from_wasmtime_export(export, store) }; + let ty = DefinitionType::from(store, &val); crate::types::matching::MatchCx { - store, engine: store.engine(), signatures: module.signatures(), types: module.types(), } - .extern_(&expected, &val) + .definition(&expected, &ty) .expect("unexpected typecheck failure"); } } diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs index 50eb511398..152ec64393 100644 --- a/crates/wasmtime/src/externals.rs +++ b/crates/wasmtime/src/externals.rs @@ -138,16 +138,6 @@ impl Extern { Extern::Table(t) => store.store_data().contains(t.0), } } - - pub(crate) fn desc(&self) -> &'static str { - match self { - Extern::Func(_) => "function", - Extern::Table(_) => "table", - Extern::Memory(_) => "memory", - Extern::SharedMemory(_) => "shared memory", - Extern::Global(_) => "global", - } - } } impl From for Extern { @@ -233,8 +223,8 @@ impl Global { /// )?; /// /// let mut linker = Linker::new(&engine); - /// linker.define("", "i32-const", i32_const)?; - /// linker.define("", "f64-mut", f64_mut)?; + /// linker.define(&store, "", "i32-const", i32_const)?; + /// linker.define(&store, "", "f64-mut", f64_mut)?; /// /// let instance = linker.instantiate(&mut store, &module)?; /// // ... diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index 045dad494c..cbb2f0b2ae 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -1,4 +1,4 @@ -use crate::linker::Definition; +use crate::linker::{Definition, DefinitionType}; use crate::store::{InstanceId, StoreOpaque, Stored}; use crate::types::matching; use crate::{ @@ -157,7 +157,10 @@ impl Instance { bail!("cross-`Store` instantiation is not currently supported"); } } - typecheck(store, module, imports, |cx, ty, item| cx.extern_(ty, item))?; + typecheck(module, imports, |cx, ty, item| { + let item = DefinitionType::from(store, item); + cx.definition(ty, &item) + })?; let mut owned_imports = OwnedImports::new(module); for import in imports { owned_imports.push(import, store); @@ -679,19 +682,8 @@ impl InstancePre { /// This method is unsafe as the `T` of the `InstancePre` is not /// guaranteed to be the same as the `T` within the `Store`, the caller must /// verify that. - pub(crate) unsafe fn new( - store: &mut StoreOpaque, - module: &Module, - items: Vec, - ) -> Result> { - for import in items.iter() { - if !import.comes_from_same_store(store) { - bail!("cross-`Store` instantiation is not currently supported"); - } - } - typecheck(store, module, &items, |cx, ty, item| { - cx.definition(ty, item) - })?; + pub(crate) unsafe fn new(module: &Module, items: Vec) -> Result> { + typecheck(module, &items, |cx, ty, item| cx.definition(ty, &item.ty()))?; let host_funcs = items .iter() @@ -813,7 +805,6 @@ fn pre_instantiate_raw( } fn typecheck( - store: &mut StoreOpaque, module: &Module, imports: &[I], check: impl Fn(&matching::MatchCx<'_>, &EntityType, &I) -> Result<()>, @@ -826,8 +817,7 @@ fn typecheck( let cx = matching::MatchCx { signatures: module.signatures(), types: module.types(), - store: store, - engine: store.engine(), + engine: module.engine(), }; for ((name, field, expected_ty), actual) in env_module.imports().zip(imports) { check(&cx, &expected_ty, actual) diff --git a/crates/wasmtime/src/linker.rs b/crates/wasmtime/src/linker.rs index 3f4782434f..0cda89000d 100644 --- a/crates/wasmtime/src/linker.rs +++ b/crates/wasmtime/src/linker.rs @@ -2,8 +2,8 @@ use crate::func::HostFunc; use crate::instance::InstancePre; use crate::store::StoreOpaque; use crate::{ - AsContextMut, Caller, Engine, Extern, ExternType, Func, FuncType, ImportType, Instance, - IntoFunc, Module, StoreContextMut, Val, ValRaw, + AsContext, AsContextMut, Caller, Engine, Extern, ExternType, Func, FuncType, ImportType, + Instance, IntoFunc, Module, StoreContextMut, Val, ValRaw, }; use anyhow::{bail, Context, Result}; use log::warn; @@ -113,10 +113,25 @@ struct ImportKey { #[derive(Clone)] pub(crate) enum Definition { - Extern(Extern), + Extern(Extern, DefinitionType), HostFunc(Arc), } +/// This is a sort of slimmed down `ExternType` which notably doesn't have a +/// `FuncType`, which is an allocation, and additionally retains the current +/// size of the table/memory. +#[derive(Clone)] +pub(crate) enum DefinitionType { + Func(wasmtime_runtime::VMSharedSignatureIndex), + Global(wasmtime_environ::Global), + // Note that tables and memories store not only the original type + // information but additionally the current size of the table/memory, as + // this is used during linking since the min size specified in the type may + // no longer be the current size of the table/memory. + Table(wasmtime_environ::Table, u32), + Memory(wasmtime_environ::Memory, u64), +} + macro_rules! generate_wrap_async_func { ($num:tt $($args:ident)*) => (paste::paste!{ /// Asynchronous analog of [`Linker::func_wrap`]. @@ -296,7 +311,7 @@ impl Linker { /// let mut linker = Linker::new(&engine); /// let ty = GlobalType::new(ValType::I32, Mutability::Const); /// let global = Global::new(&mut store, ty, Val::I32(0x1234))?; - /// linker.define("host", "offset", global)?; + /// linker.define(&store, "host", "offset", global)?; /// /// let wat = r#" /// (module @@ -312,12 +327,14 @@ impl Linker { /// ``` pub fn define( &mut self, + store: impl AsContext, module: &str, name: &str, item: impl Into, ) -> Result<&mut Self> { + let store = store.as_context(); let key = self.import_key(module, Some(name)); - self.insert(key, Definition::Extern(item.into()))?; + self.insert(key, Definition::new(store.0, item.into()))?; Ok(self) } @@ -327,9 +344,15 @@ impl Linker { /// This is only relevant when working with the module linking proposal /// where one-level names are allowed (in addition to two-level names). /// Otherwise this method need not be used. - pub fn define_name(&mut self, name: &str, item: impl Into) -> Result<&mut Self> { + pub fn define_name( + &mut self, + store: impl AsContext, + name: &str, + item: impl Into, + ) -> Result<&mut Self> { + let store = store.as_context(); let key = self.import_key(name, None); - self.insert(key, Definition::Extern(item.into()))?; + self.insert(key, Definition::new(store.0, item.into()))?; Ok(self) } @@ -542,9 +565,18 @@ impl Linker { module_name: &str, instance: Instance, ) -> Result<&mut Self> { - for export in instance.exports(store.as_context_mut()) { - let key = self.import_key(module_name, Some(export.name())); - self.insert(key, Definition::Extern(export.into_extern()))?; + let mut store = store.as_context_mut(); + let exports = instance + .exports(&mut store) + .map(|e| { + ( + self.import_key(module_name, Some(e.name())), + e.into_extern(), + ) + }) + .collect::>(); + for (key, export) in exports { + self.insert(key, Definition::new(store.0, export))?; } Ok(self) } @@ -814,11 +846,11 @@ impl Linker { let mut store = store.as_context_mut(); for export in module.exports() { if let Some(func_ty) = export.ty().func() { - let instance_pre = self.instantiate_pre(&mut store, module)?; + let instance_pre = self.instantiate_pre(module)?; let export_name = export.name().to_owned(); let func = mk_func(&mut store, func_ty, export_name, instance_pre); let key = self.import_key(module_name, Some(export.name())); - self.insert(key, Definition::Extern(func.into()))?; + self.insert(key, Definition::new(store.0, func.into()))?; } else if export.name() == "memory" && export.ty().memory().is_some() { // Allow an exported "memory" memory for now. } else if export.name() == "__indirect_function_table" && export.ty().table().is_some() @@ -1003,7 +1035,8 @@ impl Linker { mut store: impl AsContextMut, module: &Module, ) -> Result { - self.instantiate_pre(&mut store, module)?.instantiate(store) + self._instantiate_pre(module, Some(store.as_context_mut().0))? + .instantiate(store) } /// Attempts to instantiate the `module` provided. This is the same as @@ -1018,14 +1051,13 @@ impl Linker { where T: Send, { - self.instantiate_pre(&mut store, module)? + self._instantiate_pre(module, Some(store.as_context_mut().0))? .instantiate_async(store) .await } /// Performs all checks necessary for instantiating `module` with this - /// linker within `store`, except that instantiation doesn't actually - /// finish. + /// linker, except that instantiation doesn't actually finish. /// /// This method is used for front-loading type-checking information as well /// as collecting the imports to use to instantiate a module with. The @@ -1060,7 +1092,7 @@ impl Linker { /// ) /// "#; /// let module = Module::new(&engine, wat)?; - /// let instance_pre = linker.instantiate_pre(&mut store, &module)?; + /// let instance_pre = linker.instantiate_pre(&module)?; /// /// // Finish instantiation after the type-checking has all completed... /// let instance = instance_pre.instantiate(&mut store)?; @@ -1078,17 +1110,36 @@ impl Linker { /// # Ok(()) /// # } /// ``` - pub fn instantiate_pre( + pub fn instantiate_pre(&self, module: &Module) -> Result> { + self._instantiate_pre(module, None) + } + + /// This is split out to optionally take a `store` so that when the + /// `.instantiate` API is used we can get fresh up-to-date type information + /// for memories and their current size, if necessary. + /// + /// Note that providing a `store` here is not required for correctness + /// per-se. If one is not provided, such as the with the `instantiate_pre` + /// API, then the type information used for memories and tables will reflect + /// their size when inserted into the linker rather than their current size. + /// This isn't expected to be much of a problem though since + /// per-store-`Linker` types are likely using `.instantiate(..)` and + /// per-`Engine` linkers don't have memories/tables in them. + fn _instantiate_pre( &self, - mut store: impl AsContextMut, module: &Module, + store: Option<&StoreOpaque>, ) -> Result> { - let store = store.as_context_mut().0; - let imports = module + let mut imports = module .imports() .map(|import| self._get_by_import(&import)) - .collect::>()?; - unsafe { InstancePre::new(store, module, imports) } + .collect::, _>>()?; + if let Some(store) = store { + for import in imports.iter_mut() { + import.update_size(store); + } + } + unsafe { InstancePre::new(module, imports) } } /// Returns an iterator over all items defined in this `Linker`, in @@ -1217,12 +1268,24 @@ impl Default for Linker { } impl Definition { + fn new(store: &StoreOpaque, item: Extern) -> Definition { + let ty = DefinitionType::from(store, &item); + Definition::Extern(item, ty) + } + + pub(crate) fn ty(&self) -> DefinitionType { + match self { + Definition::Extern(_, ty) => ty.clone(), + Definition::HostFunc(func) => DefinitionType::Func(func.sig_index()), + } + } + /// Note the unsafety here is due to calling `HostFunc::to_func`. The /// requirement here is that the `T` that was originally used to create the /// `HostFunc` matches the `T` on the store. pub(crate) unsafe fn to_extern(&self, store: &mut StoreOpaque) -> Extern { match self { - Definition::Extern(e) => e.clone(), + Definition::Extern(e, _) => e.clone(), Definition::HostFunc(func) => func.to_func(store).into(), } } @@ -1231,17 +1294,56 @@ impl Definition { /// `HostFunc::to_func_store_rooted`. pub(crate) unsafe fn to_extern_store_rooted(&self, store: &mut StoreOpaque) -> Extern { match self { - Definition::Extern(e) => e.clone(), + Definition::Extern(e, _) => e.clone(), Definition::HostFunc(func) => func.to_func_store_rooted(store).into(), } } pub(crate) fn comes_from_same_store(&self, store: &StoreOpaque) -> bool { match self { - Definition::Extern(e) => e.comes_from_same_store(store), + Definition::Extern(e, _) => e.comes_from_same_store(store), Definition::HostFunc(_func) => true, } } + + fn update_size(&mut self, store: &StoreOpaque) { + match self { + Definition::Extern(Extern::Memory(m), DefinitionType::Memory(_, size)) => { + *size = m.internal_size(store); + } + Definition::Extern(Extern::SharedMemory(m), DefinitionType::Memory(_, size)) => { + *size = m.size(); + } + Definition::Extern(Extern::Table(m), DefinitionType::Table(_, size)) => { + *size = m.internal_size(store); + } + _ => {} + } + } +} + +impl DefinitionType { + pub(crate) fn from(store: &StoreOpaque, item: &Extern) -> DefinitionType { + let data = store.store_data(); + match item { + Extern::Func(f) => DefinitionType::Func(f.sig_index(data)), + Extern::Table(t) => DefinitionType::Table(*t.wasmtime_ty(data), t.internal_size(store)), + Extern::Global(t) => DefinitionType::Global(*t.wasmtime_ty(data)), + Extern::Memory(t) => { + DefinitionType::Memory(*t.wasmtime_ty(data), t.internal_size(store)) + } + Extern::SharedMemory(t) => DefinitionType::Memory(*t.ty().wasmtime_memory(), t.size()), + } + } + + pub(crate) fn desc(&self) -> &'static str { + match self { + DefinitionType::Func(_) => "function", + DefinitionType::Table(..) => "table", + DefinitionType::Memory(..) => "memory", + DefinitionType::Global(_) => "global", + } + } } /// Modules can be interpreted either as Commands or Reactors. diff --git a/crates/wasmtime/src/types/matching.rs b/crates/wasmtime/src/types/matching.rs index 4e20473597..11b460fa79 100644 --- a/crates/wasmtime/src/types/matching.rs +++ b/crates/wasmtime/src/types/matching.rs @@ -1,6 +1,5 @@ -use crate::linker::Definition; -use crate::store::StoreOpaque; -use crate::{signatures::SignatureCollection, Engine, Extern}; +use crate::linker::DefinitionType; +use crate::{signatures::SignatureCollection, Engine}; use anyhow::{anyhow, bail, Result}; use wasmtime_environ::{ EntityType, Global, Memory, ModuleTypes, SignatureIndex, Table, WasmFuncType, WasmType, @@ -10,47 +9,10 @@ use wasmtime_runtime::VMSharedSignatureIndex; pub struct MatchCx<'a> { pub signatures: &'a SignatureCollection, pub types: &'a ModuleTypes, - pub store: &'a StoreOpaque, pub engine: &'a Engine, } impl MatchCx<'_> { - pub fn global(&self, expected: &Global, actual: &crate::Global) -> Result<()> { - global_ty(expected, actual.wasmtime_ty(self.store.store_data())) - } - - pub fn table(&self, expected: &Table, actual: &crate::Table) -> Result<()> { - table_ty( - expected, - actual.wasmtime_ty(self.store.store_data()), - Some(actual.internal_size(self.store)), - ) - } - - pub fn memory(&self, expected: &Memory, actual: &crate::Memory) -> Result<()> { - memory_ty( - expected, - actual.wasmtime_ty(self.store.store_data()), - Some(actual.internal_size(self.store)), - ) - } - - pub fn shared_memory(&self, expected: &Memory, actual: &crate::SharedMemory) -> Result<()> { - memory_ty(expected, actual.ty().wasmtime_memory(), Some(actual.size())) - } - - pub fn func(&self, expected: SignatureIndex, actual: &crate::Func) -> Result<()> { - self.vmshared_signature_index(expected, actual.sig_index(self.store.store_data())) - } - - pub(crate) fn host_func( - &self, - expected: SignatureIndex, - actual: &crate::func::HostFunc, - ) -> Result<()> { - self.vmshared_signature_index(expected, actual.sig_index()) - } - pub fn vmshared_signature_index( &self, expected: SignatureIndex, @@ -79,39 +41,31 @@ impl MatchCx<'_> { } /// Validates that the `expected` type matches the type of `actual` - pub fn extern_(&self, expected: &EntityType, actual: &Extern) -> Result<()> { + pub(crate) fn definition(&self, expected: &EntityType, actual: &DefinitionType) -> Result<()> { match expected { EntityType::Global(expected) => match actual { - Extern::Global(actual) => self.global(expected, actual), + DefinitionType::Global(actual) => global_ty(expected, actual), _ => bail!("expected global, but found {}", actual.desc()), }, EntityType::Table(expected) => match actual { - Extern::Table(actual) => self.table(expected, actual), + DefinitionType::Table(actual, cur_size) => { + table_ty(expected, actual, Some(*cur_size)) + } _ => bail!("expected table, but found {}", actual.desc()), }, EntityType::Memory(expected) => match actual { - Extern::Memory(actual) => self.memory(expected, actual), - Extern::SharedMemory(actual) => self.shared_memory(expected, actual), + DefinitionType::Memory(actual, cur_size) => { + memory_ty(expected, actual, Some(*cur_size)) + } _ => bail!("expected memory, but found {}", actual.desc()), }, EntityType::Function(expected) => match actual { - Extern::Func(actual) => self.func(*expected, actual), + DefinitionType::Func(actual) => self.vmshared_signature_index(*expected, *actual), _ => bail!("expected func, but found {}", actual.desc()), }, EntityType::Tag(_) => unimplemented!(), } } - - /// Validates that the `expected` type matches the type of `actual` - pub(crate) fn definition(&self, expected: &EntityType, actual: &Definition) -> Result<()> { - match actual { - Definition::Extern(e) => self.extern_(expected, e), - Definition::HostFunc(f) => match expected { - EntityType::Function(expected) => self.host_func(*expected, f), - _ => bail!("expected {}, but found func", entity_desc(expected)), - }, - } - } } #[cfg_attr(not(feature = "component-model"), allow(dead_code))] diff --git a/crates/wast/src/spectest.rs b/crates/wast/src/spectest.rs index f7ff59d55e..b699d24aa1 100644 --- a/crates/wast/src/spectest.rs +++ b/crates/wast/src/spectest.rs @@ -24,32 +24,32 @@ pub fn link_spectest( let ty = GlobalType::new(ValType::I32, Mutability::Const); let g = Global::new(&mut *store, ty, Val::I32(666))?; - linker.define("spectest", "global_i32", g)?; + linker.define(&mut *store, "spectest", "global_i32", g)?; let ty = GlobalType::new(ValType::I64, Mutability::Const); let g = Global::new(&mut *store, ty, Val::I64(666))?; - linker.define("spectest", "global_i64", g)?; + linker.define(&mut *store, "spectest", "global_i64", g)?; let ty = GlobalType::new(ValType::F32, Mutability::Const); let g = Global::new(&mut *store, ty, Val::F32(0x4426_8000))?; - linker.define("spectest", "global_f32", g)?; + linker.define(&mut *store, "spectest", "global_f32", g)?; let ty = GlobalType::new(ValType::F64, Mutability::Const); let g = Global::new(&mut *store, ty, Val::F64(0x4084_d000_0000_0000))?; - linker.define("spectest", "global_f64", g)?; + linker.define(&mut *store, "spectest", "global_f64", g)?; let ty = TableType::new(ValType::FuncRef, 10, Some(20)); let table = Table::new(&mut *store, ty, Val::FuncRef(None))?; - linker.define("spectest", "table", table)?; + linker.define(&mut *store, "spectest", "table", table)?; let ty = MemoryType::new(1, Some(2)); let memory = Memory::new(&mut *store, ty)?; - linker.define("spectest", "memory", memory)?; + linker.define(&mut *store, "spectest", "memory", memory)?; if use_shared_memory { let ty = MemoryType::shared(1, 1); let memory = Memory::new(&mut *store, ty)?; - linker.define("spectest", "shared_memory", memory)?; + linker.define(&mut *store, "spectest", "shared_memory", memory)?; } Ok(()) diff --git a/tests/all/call_hook.rs b/tests/all/call_hook.rs index 7184f06dbb..4dc7417342 100644 --- a/tests/all/call_hook.rs +++ b/tests/all/call_hook.rs @@ -262,7 +262,7 @@ async fn call_linked_func_async() -> Result<(), Error> { let mut linker = Linker::new(&engine); - linker.define("host", "f", f)?; + linker.define(&mut store, "host", "f", f)?; let wat = r#" (module diff --git a/tests/all/linker.rs b/tests/all/linker.rs index 0f1c38c6ca..4048f6894e 100644 --- a/tests/all/linker.rs +++ b/tests/all/linker.rs @@ -56,36 +56,36 @@ fn link_twice_bad() -> Result<()> { // globals let ty = GlobalType::new(ValType::I32, Mutability::Const); let global = Global::new(&mut store, ty, Val::I32(0))?; - linker.define("g", "1", global.clone())?; - assert!(linker.define("g", "1", global.clone()).is_err()); + linker.define(&mut store, "g", "1", global.clone())?; + assert!(linker.define(&mut store, "g", "1", global.clone()).is_err()); let ty = GlobalType::new(ValType::I32, Mutability::Var); let global = Global::new(&mut store, ty, Val::I32(0))?; - linker.define("g", "2", global.clone())?; - assert!(linker.define("g", "2", global.clone()).is_err()); + linker.define(&mut store, "g", "2", global.clone())?; + assert!(linker.define(&mut store, "g", "2", global.clone()).is_err()); let ty = GlobalType::new(ValType::I64, Mutability::Const); let global = Global::new(&mut store, ty, Val::I64(0))?; - linker.define("g", "3", global.clone())?; - assert!(linker.define("g", "3", global.clone()).is_err()); + linker.define(&mut store, "g", "3", global.clone())?; + assert!(linker.define(&mut store, "g", "3", global.clone()).is_err()); // memories let ty = MemoryType::new(1, None); let memory = Memory::new(&mut store, ty)?; - linker.define("m", "", memory.clone())?; - assert!(linker.define("m", "", memory.clone()).is_err()); + linker.define(&mut store, "m", "", memory.clone())?; + assert!(linker.define(&mut store, "m", "", memory.clone()).is_err()); let ty = MemoryType::new(2, None); let memory = Memory::new(&mut store, ty)?; - assert!(linker.define("m", "", memory.clone()).is_err()); + assert!(linker.define(&mut store, "m", "", memory.clone()).is_err()); // tables let ty = TableType::new(ValType::FuncRef, 1, None); let table = Table::new(&mut store, ty, Val::FuncRef(None))?; - linker.define("t", "", table.clone())?; - assert!(linker.define("t", "", table.clone()).is_err()); + linker.define(&mut store, "t", "", table.clone())?; + assert!(linker.define(&mut store, "t", "", table.clone()).is_err()); let ty = TableType::new(ValType::FuncRef, 2, None); let table = Table::new(&mut store, ty, Val::FuncRef(None))?; - assert!(linker.define("t", "", table.clone()).is_err()); + assert!(linker.define(&mut store, "t", "", table.clone()).is_err()); Ok(()) } @@ -100,11 +100,8 @@ fn function_interposition() -> Result<()> { )?; for _ in 0..4 { let instance = linker.instantiate(&mut store, &module)?; - linker.define( - "red", - "green", - instance.get_export(&mut store, "green").unwrap().clone(), - )?; + let green = instance.get_export(&mut store, "green").unwrap().clone(); + linker.define(&mut store, "red", "green", green)?; module = Module::new( store.engine(), r#"(module @@ -137,11 +134,8 @@ fn function_interposition_renamed() -> Result<()> { )?; for _ in 0..4 { let instance = linker.instantiate(&mut store, &module)?; - linker.define( - "red", - "green", - instance.get_export(&mut store, "export").unwrap().clone(), - )?; + let export = instance.get_export(&mut store, "export").unwrap().clone(); + linker.define(&mut store, "red", "green", export)?; module = Module::new( store.engine(), r#"(module @@ -334,7 +328,7 @@ fn instance_pre() -> Result<()> { linker.func_wrap("", "", || {})?; let module = Module::new(&engine, r#"(module (import "" "" (func)))"#)?; - let instance_pre = linker.instantiate_pre(&mut Store::new(&engine, ()), &module)?; + let instance_pre = linker.instantiate_pre(&module)?; instance_pre.instantiate(&mut Store::new(&engine, ()))?; instance_pre.instantiate(&mut Store::new(&engine, ()))?; @@ -344,7 +338,7 @@ fn instance_pre() -> Result<()> { GlobalType::new(ValType::I32, Mutability::Const), 1.into(), )?; - linker.define("", "g", global)?; + linker.define(&mut store, "", "g", global)?; let module = Module::new( &engine, @@ -353,7 +347,7 @@ fn instance_pre() -> Result<()> { (import "" "g" (global i32)) )"#, )?; - let instance_pre = linker.instantiate_pre(&mut store, &module)?; + let instance_pre = linker.instantiate_pre(&module)?; instance_pre.instantiate(&mut store)?; instance_pre.instantiate(&mut store)?; Ok(())