From 87acdb26cb5d8e5470ba9d7042abac17753be6fa Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 27 May 2021 07:42:32 -0700 Subject: [PATCH 1/2] Add the ability to cache typechecking an instance This commit adds the abilty to cache the type-checked imports of an instance if an instance is going to be instantiated multiple times. This can also be useful to do a "dry run" of instantiation where no wasm code is run but it's double-checked that a `Linker` possesses everything necessary to instantiate the provided module. This should ideally help cut down repeated instantiation costs slightly by avoiding type-checking and allocation a `Vec` on each instantiation. It's expected though that the impact on instantiation time is quite small and likely not super significant. The functionality, though, of pre-checking can be useful for some embeddings. --- crates/wasmtime/src/func.rs | 6 +- crates/wasmtime/src/func/typed.rs | 2 +- crates/wasmtime/src/instance.rs | 343 ++++++++++++++++++-------- crates/wasmtime/src/lib.rs | 4 +- crates/wasmtime/src/linker.rs | 183 +++++++++----- crates/wasmtime/src/store.rs | 2 +- crates/wasmtime/src/types/matching.rs | 60 ++++- tests/all/linker.rs | 32 +++ 8 files changed, 457 insertions(+), 175 deletions(-) diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index 8e6cafac959b..e821e5affc5b 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -677,7 +677,7 @@ impl Func { /// initiates a panic. Also panics if `store` does not own this function. pub fn call(&self, mut store: impl AsContextMut, params: &[Val]) -> Result> { assert!( - !cfg!(feature = "async") || !store.as_context().async_support(), + !store.as_context().async_support(), "must use `call_async` when async support is enabled on the config", ); let my_ty = self.ty(&store); @@ -1894,6 +1894,10 @@ impl HostFunc { let idx = self.export.anyfunc.as_ref().type_index; store.register_host_trampoline(idx, self.trampoline); } + + pub(crate) fn sig_index(&self) -> VMSharedSignatureIndex { + unsafe { self.export.anyfunc.as_ref().type_index } + } } impl Drop for HostFunc { diff --git a/crates/wasmtime/src/func/typed.rs b/crates/wasmtime/src/func/typed.rs index ccc7979c2094..a01565f1dcca 100644 --- a/crates/wasmtime/src/func/typed.rs +++ b/crates/wasmtime/src/func/typed.rs @@ -73,7 +73,7 @@ where pub fn call(&self, mut store: impl AsContextMut, params: Params) -> Result { let mut store = store.as_context_mut().opaque(); assert!( - !cfg!(feature = "async") || !store.async_support(), + !store.async_support(), "must use `call_async` with async stores" ); unsafe { self._call(&mut store, params) } diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index 899cc581b121..783d11ec8115 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -1,3 +1,4 @@ +use crate::linker::Definition; use crate::store::{InstanceId, StoreData, StoreOpaque, StoreOpaqueSend, Stored}; use crate::types::matching; use crate::{ @@ -9,7 +10,8 @@ use std::mem; use std::sync::Arc; use wasmtime_environ::entity::PrimaryMap; use wasmtime_environ::wasm::{ - EntityIndex, FuncIndex, GlobalIndex, InstanceIndex, MemoryIndex, ModuleIndex, TableIndex, + EntityIndex, EntityType, FuncIndex, GlobalIndex, InstanceIndex, MemoryIndex, ModuleIndex, + TableIndex, }; use wasmtime_environ::Initializer; use wasmtime_runtime::{ @@ -96,32 +98,14 @@ impl Instance { module: &Module, imports: &[Extern], ) -> Result { - Instance::_new(&mut store.as_context_mut().opaque(), module, imports) - } - - fn _new( - store: &mut StoreOpaque<'_>, - module: &Module, - imports: &[Extern], - ) -> Result { - assert!( - !store.async_support(), - "cannot use `new` when async support is enabled on the config" - ); - - // NB: this is the same code as `Instance::new_async`. It's intentionally - // small but should be kept in sync (modulo the async bits). - let mut i = Instantiator::new(store, module, imports)?; - loop { - if let Some((id, instance)) = i.step(store)? { - if let Some(start) = store.instance(id).module().start_func { - Instantiator::start_raw(store, id, start)?; - } - if let Some(instance) = instance { - break Ok(instance); - } - } - } + // This unsafety comes from `Instantiator::new` where we must typecheck + // first, which we are sure to do here. + let mut i = unsafe { + let mut cx = store.as_context_mut().opaque(); + typecheck_externs(&mut cx, module, imports)?; + Instantiator::new(&mut cx, module, ImportSource::Externs(imports))? + }; + i.run(store.as_context_mut().opaque()) } /// Same as [`Instance::new`], except for usage in [asynchronous stores]. @@ -151,37 +135,13 @@ impl Instance { where T: Send, { - Instance::_new_async(store.as_context_mut().opaque_send(), module, imports).await - } - - #[cfg(feature = "async")] - async fn _new_async<'a>( - mut store: StoreOpaqueSend<'a>, - module: &Module, - imports: &[Extern], - ) -> Result { - assert!( - store.async_support(), - "cannot use `new_async` without enabling async support on the config" - ); - - // NB: this is the same code as `Instance::new`. It's intentionally - // small but should be kept in sync (modulo the async bits). - let mut i = Instantiator::new(&mut store.opaque(), module, imports)?; - loop { - let step = i.step(&mut store.opaque())?; - if let Some((id, instance)) = step { - let start = store.instance(id).module().start_func; - if let Some(start) = start { - store - .on_fiber(|store| Instantiator::start_raw(store, id, start)) - .await??; - } - if let Some(instance) = instance { - break Ok(instance); - } - } - } + // See `new` for unsafety comments + let mut i = unsafe { + let mut cx = store.as_context_mut().opaque(); + typecheck_externs(&mut cx, module, imports)?; + Instantiator::new(&mut cx, module, ImportSource::Externs(imports))? + }; + i.run_async(store.as_context_mut().opaque_send()).await } pub(crate) fn from_wasmtime(handle: RuntimeInstance, store: &mut StoreOpaque) -> Instance { @@ -336,7 +296,8 @@ struct ImportsBuilder<'a> { } enum ImportSource<'a> { - Runtime(&'a [Extern]), + Externs(&'a [Extern]), + Definitions(&'a [Definition]), Outer { initializer: usize }, } @@ -345,33 +306,82 @@ impl<'a> Instantiator<'a> { /// directives of a module. /// /// This doesn't do much work itself beyond setting things up. - fn new( + /// + /// # Unsafety + /// + /// This function is unsafe for a few reasons: + /// + /// * This assumes that `imports` has already been typechecked and is of the + /// appropriate length. It is memory unsafe if the types of `imports` are + /// not what `module` expects. + /// + /// * The `imports` must be safely able to get inserted into `store`. This + /// only applies if `ImportSource::Definitions` is used because this will + /// internally call `Definition::to_extern` which requires that any + /// host functions in the list were created with an original `T` as the + /// store that's being inserted into. + /// + /// * The `imports` must all come from the `store` specified. + unsafe fn new( store: &mut StoreOpaque<'_>, module: &Module, - imports: &'a [Extern], + imports: ImportSource<'a>, ) -> Result> { if !Engine::same(store.engine(), module.engine()) { bail!("cross-`Engine` instantiation is not currently supported"); } - // Perform some pre-flight checks before we get into the meat of - // instantiation. - let expected = module.compiled_module().module().imports().count(); - if expected != imports.len() { - bail!("expected {} imports, found {}", expected, imports.len()); - } - for import in imports { - if !import.comes_from_same_store(&store) { - bail!("cross-`Store` instantiation is not currently supported"); - } - } - Ok(Instantiator { in_progress: Vec::new(), - cur: ImportsBuilder::new(module, ImportSource::Runtime(imports)), + cur: ImportsBuilder::new(module, imports), }) } + fn run(&mut self, mut store: StoreOpaque<'_>) -> Result { + assert!( + !store.async_support(), + "cannot use `new` when async support is enabled on the config" + ); + + // NB: this is the same code as `run_async`. It's intentionally + // small but should be kept in sync (modulo the async bits). + loop { + if let Some((id, instance)) = self.step(&mut store)? { + if let Some(start) = store.instance(id).module().start_func { + Instantiator::start_raw(&mut store, id, start)?; + } + if let Some(instance) = instance { + break Ok(instance); + } + } + } + } + + #[cfg(feature = "async")] + async fn run_async(&mut self, mut store: StoreOpaqueSend<'_>) -> Result { + assert!( + store.async_support(), + "cannot use `new_async` without enabling async support on the config" + ); + + // NB: this is the same code as `run`. It's intentionally + // small but should be kept in sync (modulo the async bits). + loop { + let step = self.step(&mut store.opaque())?; + if let Some((id, instance)) = step { + let start = store.instance(id).module().start_func; + if let Some(start) = start { + store + .on_fiber(|store| Instantiator::start_raw(store, id, start)) + .await??; + } + if let Some(instance) = instance { + break Ok(instance); + } + } + } + } + /// Processes the next initializer for the next instance being created /// without running any wasm code. /// @@ -410,7 +420,7 @@ impl<'a> Instantiator<'a> { .initializers .get(self.cur.initializer - 1) { - Some(Initializer::Import { index, name, field }) => { + Some(Initializer::Import { name, field, .. }) => { match &mut self.cur.src { // If imports are coming from the runtime-provided list // (e.g. the root module being instantiated) then we @@ -418,27 +428,18 @@ impl<'a> Instantiator<'a> { // // Note the `unwrap` here should be ok given the validation // above in `Instantiation::new`. - ImportSource::Runtime(list) => { + ImportSource::Externs(list) => { let (head, remaining) = list.split_first().unwrap(); *list = remaining; - let expected_ty = - self.cur.module.compiled_module().module().type_of(*index); - matching::MatchCx { - signatures: self.cur.module.signatures(), - types: self.cur.module.types(), - store_data: store.store_data(), - engine: store.engine(), - } - .extern_(&expected_ty, head) - .with_context(|| { - let extra = match field { - Some(name) => format!("::{}", name), - None => String::new(), - }; - format!("incompatible import type for `{}{}`", name, extra) - })?; self.cur.push(head.clone(), store); } + ImportSource::Definitions(list) => { + let (head, remaining) = list.split_first().unwrap(); + *list = remaining; + // This unsafety is encapsulated with + // `Instantiator::new`, documented above. + self.cur.push(unsafe { head.to_extern(store) }, store); + } // Otherwise if arguments are coming from our outer // instance due to a recursive instantiation then we @@ -738,25 +739,155 @@ impl<'a> ImportsBuilder<'a> { pub(crate) type RuntimeInstance = Arc>; -/// An internal structure to this crate to build an `Instance` from a list of -/// items with names. This is intended to stay private for now, it'll need an -/// audit of APIs if publicly exported. -#[derive(Default)] -pub(crate) struct InstanceBuilder { - items: RuntimeInstance, +/// An instance, pre-instantiation, that is ready to be instantiated. +/// +/// This structure represents an instance *just before* it was instantiated, +/// after all type-checking and imports have been resolved. The only thing left +/// to do for this instance is to actually run the process of instantiation. +/// +/// Note that an `InstancePre` may not be tied to any particular [`Store`] if +/// none of the imports it closed over are tied to any particular [`Store`]. +/// +/// This structure is created through the [`Linker::instantiate_pre`] method, +/// which also has some more information and examples. +/// +/// [`Store`]: crate::Store +/// [`Linker::instantiate_pre`]: crate::Linker::instantiate_pre +pub struct InstancePre { + module: Module, + items: Vec, + _marker: std::marker::PhantomData T>, } -impl InstanceBuilder { - pub(crate) fn new() -> InstanceBuilder { - InstanceBuilder::default() +impl InstancePre { + pub(crate) unsafe fn new( + store: &mut StoreOpaque, + module: &Module, + items: Vec, + ) -> Result> { + typecheck_defs(store, module, &items)?; + Ok(InstancePre { + module: module.clone(), + items, + _marker: std::marker::PhantomData, + }) } - pub(crate) fn insert(&mut self, name: &str, item: impl Into) { - let items = Arc::get_mut(&mut self.items).unwrap(); - items.insert(name.to_string(), item.into()); + /// Instantiates this instance, creating a new instance within the provided + /// `store`. + /// + /// This function will run the actual process of instantiation to + /// completion. This will use all of the previously-closed-over items as + /// imports to instantiate the module that this was originally created with. + /// + /// For more information about instantiation see [`Instance::new`]. + /// + /// # Panics + /// + /// Panics if any import closed over by this [`InstancePre`] isn't owned by + /// `store`, or if `store` has async support enabled. + pub fn instantiate(&self, mut store: impl AsContextMut) -> Result { + let mut store = store.as_context_mut().opaque(); + // For the unsafety here the typecheck happened at creation time of this + // structure and then othrewise the `T` of `InstancePre` connects any + // host functions we have in our definition list to the `store` that was + // passed in. + unsafe { + self.ensure_comes_from_same_store(&store)?; + Instantiator::new( + &mut store, + &self.module, + ImportSource::Definitions(&self.items), + )? + .run(store) + } } - pub(crate) fn finish(self, store: &mut StoreOpaque) -> Instance { - Instance::from_wasmtime(self.items, store) + /// Creates a new instance, running the start function asynchronously + /// instead of inline. + /// + /// For more information about asynchronous instantiation see the + /// documentation on [`Instance::new_async`]. + /// + /// # Panics + /// + /// Panics if any import closed over by this [`InstancePre`] isn't owned by + /// `store`, or if `store` does not have async support enabled. + pub async fn instantiate_async( + &self, + mut store: impl AsContextMut, + ) -> Result + where + T: Send, + { + // For the unsafety here see above + let mut i = unsafe { + let mut store = store.as_context_mut().opaque(); + self.ensure_comes_from_same_store(&store)?; + Instantiator::new( + &mut store, + &self.module, + ImportSource::Definitions(&self.items), + )? + }; + i.run_async(store.as_context_mut().opaque_send()).await + } + + fn ensure_comes_from_same_store(&self, store: &StoreOpaque<'_>) -> Result<()> { + for import in self.items.iter() { + if !import.comes_from_same_store(store) { + bail!("cross-`Store` instantiation is not currently supported"); + } + } + Ok(()) + } +} + +fn typecheck_externs(store: &mut StoreOpaque, module: &Module, imports: &[Extern]) -> Result<()> { + for import in imports { + if !import.comes_from_same_store(store) { + bail!("cross-`Store` instantiation is not currently supported"); + } + } + typecheck(store, module, imports, |cx, ty, item| cx.extern_(ty, item)) +} + +fn typecheck_defs(store: &mut StoreOpaque, module: &Module, imports: &[Definition]) -> Result<()> { + for import in imports { + if !import.comes_from_same_store(store) { + bail!("cross-`Store` instantiation is not currently supported"); + } + } + typecheck(store, module, imports, |cx, ty, item| { + cx.definition(ty, item) + }) +} + +fn typecheck( + store: &mut StoreOpaque, + module: &Module, + imports: &[I], + check: impl Fn(&matching::MatchCx<'_>, &EntityType, &I) -> Result<()>, +) -> Result<()> { + let env_module = module.compiled_module().module(); + let expected = env_module.imports().count(); + if expected != imports.len() { + bail!("expected {} imports, found {}", expected, imports.len()); + } + let cx = matching::MatchCx { + signatures: module.signatures(), + types: module.types(), + store_data: store.store_data(), + engine: store.engine(), + }; + for ((name, field, expected_ty), actual) in env_module.imports().zip(imports) { + check(&cx, &expected_ty, actual).with_context(|| { + let extra = match field { + Some(name) => format!("::{}", name), + None => String::new(), + }; + format!("incompatible import type for `{}{}`", name, extra) + })?; } + Ok(()) } diff --git a/crates/wasmtime/src/lib.rs b/crates/wasmtime/src/lib.rs index c71baf742ef9..34da2a696492 100644 --- a/crates/wasmtime/src/lib.rs +++ b/crates/wasmtime/src/lib.rs @@ -387,7 +387,7 @@ pub use crate::config::*; pub use crate::engine::*; pub use crate::externals::*; pub use crate::func::*; -pub use crate::instance::Instance; +pub use crate::instance::{Instance, InstancePre}; pub use crate::limits::*; pub use crate::linker::*; pub use crate::memory::*; @@ -428,6 +428,8 @@ fn _assert_send_sync() { _assert::>(); _assert::>(); _assert::(); + _assert::>(); + _assert::>(); #[cfg(feature = "async")] fn _call_async(s: &mut Store<()>, f: Func) { diff --git a/crates/wasmtime/src/linker.rs b/crates/wasmtime/src/linker.rs index 0329c1b0b069..21ec2787de22 100644 --- a/crates/wasmtime/src/linker.rs +++ b/crates/wasmtime/src/linker.rs @@ -1,5 +1,6 @@ use crate::func::HostFunc; -use crate::instance::InstanceBuilder; +use crate::instance::InstancePre; +use crate::store::StoreOpaque; use crate::{ AsContextMut, Caller, Engine, Extern, ExternType, Func, FuncType, ImportType, Instance, IntoFunc, Module, Trap, Val, @@ -75,7 +76,7 @@ pub struct Linker { engine: Engine, string2idx: HashMap, usize>, strings: Vec>, - map: HashMap>, + map: HashMap, allow_shadowing: bool, allow_unknown_exports: bool, _marker: marker::PhantomData T>, @@ -87,12 +88,11 @@ struct ImportKey { module: usize, } -enum Definition { +#[derive(Clone)] +pub(crate) enum Definition { Extern(Extern), - HostFunc { - func: Arc, - t: marker::PhantomData T>, - }, + HostFunc(Arc), + Instance(Arc>), } macro_rules! generate_wrap_async_func { @@ -284,13 +284,7 @@ impl Linker { ) -> Result<&mut Self> { let func = HostFunc::new(&self.engine, ty, func); let key = self.import_key(module, Some(name)); - self.insert( - key, - Definition::HostFunc { - func: Arc::new(func), - t: marker::PhantomData, - }, - )?; + self.insert(key, Definition::HostFunc(Arc::new(func)))?; Ok(self) } @@ -394,13 +388,7 @@ impl Linker { ) -> Result<&mut Self> { let func = HostFunc::wrap(&self.engine, func); let key = self.import_key(module, Some(name)); - self.insert( - key, - Definition::HostFunc { - func: Arc::new(func), - t: marker::PhantomData, - }, - )?; + self.insert(key, Definition::HostFunc(Arc::new(func)))?; Ok(self) } @@ -590,7 +578,10 @@ impl Linker { mut store: impl AsContextMut, module_name: &str, module: &Module, - ) -> Result<&mut Self> { + ) -> Result<&mut Self> + where + T: 'static, + { match ModuleKind::categorize(module)? { ModuleKind::Command => self.command(store, module_name, module), ModuleKind::Reactor => { @@ -614,18 +605,20 @@ impl Linker { mut store: impl AsContextMut, module_name: &str, module: &Module, - ) -> Result<&mut Self> { + ) -> Result<&mut Self> + where + T: 'static, + { for export in module.exports() { if let Some(func_ty) = export.ty().func() { - let imports = self.compute_imports(&mut store, module)?; - let module = module.clone(); + let instance_pre = self.instantiate_pre(&mut store, module)?; let export_name = export.name().to_owned(); let func = Func::new( &mut store, func_ty.clone(), move |mut caller, params, results| { // Create a new instance for this command execution. - let instance = Instance::new(&mut caller, &module, &imports)?; + let instance = instance_pre.instantiate(&mut caller)?; // `unwrap()` everything here because we know the instance contains a // function export with the given name and signature because we're @@ -739,7 +732,7 @@ impl Linker { Ok(()) } - fn insert(&mut self, key: ImportKey, item: Definition) -> Result<()> { + fn insert(&mut self, key: ImportKey, item: Definition) -> Result<()> { match self.map.entry(key) { Entry::Occupied(_) if !self.allow_shadowing => { let module = &self.strings[key.module]; @@ -831,8 +824,7 @@ impl Linker { mut store: impl AsContextMut, module: &Module, ) -> Result { - let imports = self.compute_imports(&mut store, module)?; - Instance::new(store, module, &imports) + self.instantiate_pre(&mut store, module)?.instantiate(store) } /// Attempts to instantiate the `module` provided. This is the same as @@ -847,22 +839,72 @@ impl Linker { where T: Send, { - let imports = self.compute_imports(&mut store, module)?; - Instance::new_async(store, module, &imports).await + self.instantiate_pre(&mut store, module)? + .instantiate_async(store) + .await } - fn compute_imports( + /// Performs all checks necessary for instantiating `module` with this + /// linker within `store`, 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 + /// returned [`InstancePre`] represents a ready-to-be-instantiated module, + /// which can also be instantiated multiple times if desired. + /// + /// # Panics + /// + /// This method will panic if any item defined in this linker used by + /// `module` is not owned by `store`. + /// + /// # Examples + /// + /// ``` + /// # use wasmtime::*; + /// # fn main() -> anyhow::Result<()> { + /// # let engine = Engine::default(); + /// # let mut store = Store::new(&engine, ()); + /// let mut linker = Linker::new(&engine); + /// linker.func_wrap("host", "double", |x: i32| x * 2)?; + /// + /// let wat = r#" + /// (module + /// (import "host" "double" (func (param i32) (result i32))) + /// ) + /// "#; + /// let module = Module::new(&engine, wat)?; + /// let instance_pre = linker.instantiate_pre(&mut store, &module)?; + /// + /// // Finish instantiation after the type-checking has all completed... + /// let instance = instance_pre.instantiate(&mut store)?; + /// + /// // ... and we can even continue to keep instantiating if desired! + /// instance_pre.instantiate(&mut store)?; + /// instance_pre.instantiate(&mut store)?; + /// + /// // Note that functions defined in a linker with `func_wrap` and similar + /// // constructors are not owned by any particular `Store`, so we can also + /// // instantiate our `instance_pre` in other stores because no imports + /// // belong to the original store. + /// let mut new_store = Store::new(&engine, ()); + /// instance_pre.instantiate(&mut new_store)?; + /// # Ok(()) + /// # } + /// ``` + pub fn instantiate_pre( &self, mut store: impl AsContextMut, module: &Module, - ) -> Result> { - module + ) -> Result> { + let imports = module .imports() .map(|import| { - self.get_by_import(&mut store, &import) + self._get_by_import(&import) .ok_or_else(|| self.link_error(&import)) }) - .collect() + .collect::>()?; + unsafe { InstancePre::new(&mut store.as_context_mut().opaque(), module, imports) } } fn link_error(&self, import: &ImportType) -> Error { @@ -887,10 +929,12 @@ impl Linker { mut store: impl AsContextMut + 'p, ) -> impl Iterator + 'p { self.map.iter().map(move |(key, item)| { + let mut store = store.as_context_mut().opaque(); ( &*self.strings[key.module], &*self.strings[key.name], - item.to_extern(&mut store), + // Should be safe since `T` is connecting the linker and store + unsafe { item.to_extern(&mut store) }, ) }) } @@ -902,10 +946,16 @@ impl Linker { /// [`Linker`]. pub fn get( &self, - store: impl AsContextMut, + mut store: impl AsContextMut, module: &str, name: Option<&str>, ) -> Option { + let mut store = store.as_context_mut().opaque(); + // Should be safe since `T` is connecting the linker and store + Some(unsafe { self._get(module, name)?.to_extern(&mut store) }) + } + + fn _get(&self, module: &str, name: Option<&str>) -> Option<&Definition> { let key = ImportKey { module: *self.string2idx.get(module)?, name: match name { @@ -913,7 +963,7 @@ impl Linker { None => usize::max_value(), }, }; - Some(self.map.get(&key)?.to_extern(store)) + self.map.get(&key) } /// Looks up a value in this `Linker` which matches the `import` type @@ -925,8 +975,14 @@ impl Linker { mut store: impl AsContextMut, import: &ImportType, ) -> Option { - if let Some(item) = self.get(&mut store, import.module(), import.name()) { - return Some(item); + let mut store = store.as_context_mut().opaque(); + // Should be safe since `T` is connecting the linker and store + Some(unsafe { self._get_by_import(import)?.to_extern(&mut store) }) + } + + fn _get_by_import(&self, import: &ImportType) -> Option { + if let Some(item) = self._get(import.module(), import.name()) { + return Some(item.clone()); } if import.name().is_some() { @@ -949,12 +1005,12 @@ impl Linker { // it each time a module is instantiated. For now though while the // module linking proposal is under development this should hopefully // suffice. - let mut builder = InstanceBuilder::new(); + let mut map = indexmap::IndexMap::new(); for export in t.exports() { - let item = self.get(&mut store, import.module(), Some(export.name()))?; - builder.insert(export.name(), item); + let item = self._get(import.module(), Some(export.name()))?; + map.insert(export.name().to_string(), item.clone()); } - return Some(builder.finish(&mut store.as_context_mut().opaque()).into()); + return Some(Definition::Instance(Arc::new(map))); } None @@ -999,31 +1055,30 @@ impl Default for Linker { } } -impl Definition { - fn to_extern(&self, mut store: impl AsContextMut) -> Extern { +impl Definition { + /// 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(), - - // Note the unsafety here is due to calling `to_func`. The - // requirement here is that the `T` that was originally used to - // create the `HostFunc` matches the `T` on the store. This is done - // with the `T` on `Definition` (and `Linker`) as well as the - // `Data=T` bound above. - Definition::HostFunc { func, .. } => unsafe { - func.to_func(&mut store.as_context_mut().opaque()).into() - }, + Definition::HostFunc(func) => func.to_func(store).into(), + Definition::Instance(i) => { + let items = Arc::new( + i.iter() + .map(|(name, item)| (name.clone(), item.to_extern(store))) + .collect(), + ); + Instance::from_wasmtime(items, store).into() + } } } -} -impl Clone for Definition { - fn clone(&self) -> Definition { + pub(crate) fn comes_from_same_store(&self, store: &StoreOpaque) -> bool { match self { - Definition::Extern(e) => Definition::Extern(e.clone()), - Definition::HostFunc { func, t } => Definition::HostFunc { - func: func.clone(), - t: *t, - }, + Definition::Extern(e) => e.comes_from_same_store(store), + Definition::HostFunc(_func) => true, + Definition::Instance(i) => i.values().all(|e| e.comes_from_same_store(store)), } } } diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index b3c9a9bd0f80..059247ac3c61 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -575,7 +575,7 @@ impl StoreInner { } pub fn async_support(&self) -> bool { - self.engine().config().async_support + cfg!(feature = "async") && self.engine().config().async_support } pub fn engine(&self) -> &Engine { diff --git a/crates/wasmtime/src/types/matching.rs b/crates/wasmtime/src/types/matching.rs index 1f5e1a5d0b0a..cb22f97b229b 100644 --- a/crates/wasmtime/src/types/matching.rs +++ b/crates/wasmtime/src/types/matching.rs @@ -1,3 +1,4 @@ +use crate::linker::Definition; use crate::store::StoreData; use crate::{signatures::SignatureCollection, Engine, Extern}; use anyhow::{bail, Context, Result}; @@ -5,6 +6,7 @@ use wasmtime_environ::wasm::{ EntityType, Global, InstanceTypeIndex, Memory, ModuleTypeIndex, SignatureIndex, Table, }; use wasmtime_jit::TypeTables; +use wasmtime_runtime::VMSharedSignatureIndex; pub struct MatchCx<'a> { pub signatures: &'a SignatureCollection, @@ -73,8 +75,24 @@ impl MatchCx<'_> { } pub fn func(&self, expected: SignatureIndex, actual: &crate::Func) -> Result<()> { + self.vmshared_signature_index(expected, actual.sig_index(self.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, + actual: VMSharedSignatureIndex, + ) -> Result<()> { let matches = match self.signatures.shared_signature(expected) { - Some(idx) => actual.sig_index(self.store_data) == idx, + Some(idx) => actual == idx, // If our expected signature isn't registered, then there's no way // that `actual` can match it. None => false, @@ -295,4 +313,44 @@ impl MatchCx<'_> { EntityType::Event(_) => 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)), + }, + Definition::Instance(items) => match expected { + EntityType::Instance(expected) => { + for (name, expected) in self.types.instance_signatures[*expected].exports.iter() + { + match items.get(name) { + Some(item) => { + self.definition(expected, item).with_context(|| { + format!("instance export {:?} incompatible", name) + })?; + } + None => bail!("instance type missing export {:?}", name), + } + } + Ok(()) + } + _ => bail!("expected {}, but found instance", entity_desc(expected)), + }, + } + } +} + +fn entity_desc(ty: &EntityType) -> &'static str { + match ty { + EntityType::Global(_) => "global", + EntityType::Table(_) => "table", + EntityType::Memory(_) => "memory", + EntityType::Function(_) => "func", + EntityType::Instance(_) => "instance", + EntityType::Module(_) => "module", + EntityType::Event(_) => "event", + } } diff --git a/tests/all/linker.rs b/tests/all/linker.rs index 26809ce9faee..51d42ab3366c 100644 --- a/tests/all/linker.rs +++ b/tests/all/linker.rs @@ -308,3 +308,35 @@ fn alias_one() -> Result<()> { assert!(linker.get(&mut store, "c", Some("d")).is_some()); Ok(()) } + +#[test] +fn instance_pre() -> Result<()> { + let engine = Engine::default(); + let mut linker = Linker::new(&engine); + linker.func_wrap("", "", || {})?; + + let module = Module::new(&engine, r#"(module (import "" "" (func)))"#)?; + let instance_pre = linker.instantiate_pre(&mut Store::new(&engine, ()), &module)?; + instance_pre.instantiate(&mut Store::new(&engine, ()))?; + instance_pre.instantiate(&mut Store::new(&engine, ()))?; + + let mut store = Store::new(&engine, ()); + let global = Global::new( + &mut store, + GlobalType::new(ValType::I32, Mutability::Const), + 1.into(), + )?; + linker.define("", "g", global)?; + + let module = Module::new( + &engine, + r#"(module + (import "" "" (func)) + (import "" "g" (global i32)) + )"#, + )?; + let instance_pre = linker.instantiate_pre(&mut store, &module)?; + instance_pre.instantiate(&mut store)?; + instance_pre.instantiate(&mut store)?; + Ok(()) +} From 20fcb34417418d47a7308cc0e1a8018bf1785553 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 3 Jun 2021 07:51:29 -0700 Subject: [PATCH 2/2] Fix build with async --- crates/wasmtime/src/instance.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index 783d11ec8115..da544825b3a4 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -813,6 +813,8 @@ impl InstancePre { /// /// Panics if any import closed over by this [`InstancePre`] isn't owned by /// `store`, or if `store` does not have async support enabled. + #[cfg(feature = "async")] + #[cfg_attr(nightlydoc, doc(cfg(feature = "async")))] pub async fn instantiate_async( &self, mut store: impl AsContextMut,