From d8d4bf81b260ea18f56475fbde96b6662a365cf8 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 17 Jun 2021 14:27:48 -0500 Subject: [PATCH] Reimplement how instance exports are stored/loaded (#2984) * Reimplement how instance exports are stored/loaded This commit internally refactors how instance exports are handled and fixes two issues. One issue is that when we instantiate an instance we no longer forcibly load all items from the instance immediately, deferring insertion of each item into the store data tables to happen later as necessary. The next issue is that repeated calls to `Caller::get_export` would continuously insert items into the store data tables. While working as intended this was undesirable because it would continuously push onto a vector that only got deallocated once the entire store was deallocate. Now it's routed to `Instance::get_export` which doesn't have this behavior. Closes #2916 Closes #2983 * Just define our own `Either` --- crates/runtime/src/instance.rs | 4 +- crates/wasmtime/src/func.rs | 30 ++- crates/wasmtime/src/instance.rs | 360 ++++++++++++++++++-------- crates/wasmtime/src/linker.rs | 4 +- crates/wasmtime/src/store.rs | 4 - crates/wasmtime/src/store/context.rs | 9 +- crates/wasmtime/src/store/data.rs | 78 ++++-- crates/wasmtime/src/types/matching.rs | 48 +++- 8 files changed, 372 insertions(+), 165 deletions(-) diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 5474d430c7..5dc34f5464 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -143,7 +143,7 @@ impl Instance { .cast() } - pub(crate) fn module(&self) -> &Module { + pub(crate) fn module(&self) -> &Arc { &self.module } @@ -957,7 +957,7 @@ impl InstanceHandle { } /// Return a reference to a module. - pub fn module(&self) -> &Module { + pub fn module(&self) -> &Arc { self.instance().module() } diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index fb31d0863f..5a577500b3 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -1,6 +1,6 @@ use crate::store::{StoreData, StoreOpaque, StoreOpaqueSend, Stored}; use crate::{ - AsContext, AsContextMut, Engine, Extern, FuncType, InterruptHandle, StoreContext, + AsContext, AsContextMut, Engine, Extern, FuncType, Instance, InterruptHandle, StoreContext, StoreContextMut, Trap, Val, ValType, }; use anyhow::{bail, Context as _, Result}; @@ -1514,19 +1514,21 @@ impl Caller<'_, T> { /// It's recommended to take care when calling this API and gracefully /// handling a `None` return value. pub fn get_export(&mut self, name: &str) -> Option { - unsafe { - let index = self.caller.module().exports.get(name)?; - match index { - // Only allow memory/functions for now to emulate what interface - // types will once provide - EntityIndex::Memory(_) | EntityIndex::Function(_) => { - Some(Extern::from_wasmtime_export( - self.caller.lookup_by_declaration(&index), - &mut self.store.as_context_mut().opaque(), - )) - } - _ => None, - } + // All instances created have a `host_state` with a pointer pointing + // back to themselves. If this caller doesn't have that `host_state` + // then it probably means it was a host-created object like `Func::new` + // which doesn't have any exports we want to return anyway. + match self + .caller + .host_state() + .downcast_ref::()? + .get_export(&mut self.store, name)? + { + Extern::Func(f) => Some(Extern::Func(f)), + Extern::Memory(f) => Some(Extern::Memory(f)), + // Intentionally ignore other Extern items here since this API is + // supposed to be a temporary stop-gap until interface types. + _ => None, } } diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index da544825b3..0801b10e00 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -1,9 +1,10 @@ use crate::linker::Definition; +use crate::signatures::SignatureCollection; use crate::store::{InstanceId, StoreData, StoreOpaque, StoreOpaqueSend, Stored}; use crate::types::matching; use crate::{ - AsContext, AsContextMut, Engine, Export, Extern, Func, Global, InstanceType, Memory, Module, - StoreContextMut, Table, Trap, TypedFunc, + AsContext, AsContextMut, Engine, Export, Extern, ExternType, Func, Global, InstanceType, + Memory, Module, StoreContextMut, Table, Trap, TypedFunc, }; use anyhow::{anyhow, bail, Context, Error, Result}; use std::mem; @@ -14,6 +15,7 @@ use wasmtime_environ::wasm::{ TableIndex, }; use wasmtime_environ::Initializer; +use wasmtime_jit::TypeTables; use wasmtime_runtime::{ Imports, InstanceAllocationRequest, InstantiationError, VMContext, VMFunctionBody, VMFunctionImport, VMGlobalImport, VMMemoryImport, VMTableImport, @@ -34,7 +36,28 @@ use wasmtime_runtime::{ /// available as [`Instance::new`]. #[derive(Copy, Clone, Debug)] #[repr(transparent)] -pub struct Instance(Stored); +pub struct Instance(Stored); + +pub(crate) enum InstanceData { + /// This variant is used for instances created through instantiation of a + /// module, e.g. `Instance::new` or various linker methods. + Instantiated { + /// The id of the instance within the store, used to find the original + /// `InstanceHandle`. + id: InstanceId, + /// A lazily-populated list of exports of this instance. The order of + /// exports here matches the order of the exports in the the original + /// module. + exports: Vec>, + /// The type information of the module that this was instantiated with. + types: Arc, + signatures: Arc, + }, + + /// This variant is used for synthetically created instances via `Linker` + /// APIs. This is only used for the module linking proposal at this time. + Synthetic(Arc>), +} impl Instance { /// Creates a new [`Instance`] from the previously compiled [`Module`] and @@ -144,7 +167,7 @@ impl Instance { i.run_async(store.as_context_mut().opaque_send()).await } - pub(crate) fn from_wasmtime(handle: RuntimeInstance, store: &mut StoreOpaque) -> Instance { + pub(crate) fn from_wasmtime(handle: InstanceData, store: &mut StoreOpaque) -> Instance { Instance(store.store_data_mut().insert(handle)) } @@ -155,15 +178,25 @@ impl Instance { /// Panics if `store` does not own this instance. pub fn ty(&self, store: impl AsContext) -> InstanceType { let store = store.as_context(); - let items = &store[self.0]; let mut ty = InstanceType::new(); - for (name, item) in items.iter() { - ty.add_named_export(name, item.ty(&store)); + match &store[self.0] { + InstanceData::Synthetic(items) => { + for (name, item) in items.iter() { + ty.add_named_export(name, item.ty(&store)); + } + } + InstanceData::Instantiated { id, types, .. } => { + let module = store.0.instance(*id).module(); + for (name, idx) in module.exports.iter() { + let export_ty = module.type_of(*idx); + ty.add_named_export(name, ExternType::from_wasmtime(types, &export_ty)); + } + } } ty } - pub(crate) fn items<'a>(&self, store: &'a StoreData) -> &'a RuntimeInstance { + pub(crate) fn data<'a>(&self, store: &'a StoreData) -> &'a InstanceData { &store[self.0] } @@ -180,10 +213,80 @@ impl Instance { &'a self, store: impl Into>, ) -> impl ExactSizeIterator> + 'a { - let items = &store.into().store_data()[self.0]; - items - .iter() - .map(|(name, item)| Export::new(name, item.clone())) + self._exports(store.into().opaque()) + } + + fn _exports<'a>( + &'a self, + mut store: StoreOpaque<'a>, + ) -> impl ExactSizeIterator> + 'a { + // If this is an `Instantiated` instance then all the `exports` may not + // be filled in. Fill them all in now if that's the case. + if let InstanceData::Instantiated { exports, id, .. } = &store[self.0] { + if exports.iter().any(|e| e.is_none()) { + let module = Arc::clone(store.instance(*id).module()); + for name in module.exports.keys() { + self._get_export(&mut store, name); + } + } + } + + let inner = store.into_inner(); + return match &inner.store_data()[self.0] { + InstanceData::Synthetic(names) => { + Either::A(names.iter().map(|(k, v)| Export::new(k, v.clone()))) + } + InstanceData::Instantiated { exports, id, .. } => { + let module = inner.instance(*id).module(); + Either::B( + module + .exports + .iter() + .zip(exports) + .map(|((name, _), export)| Export::new(name, export.clone().unwrap())), + ) + } + }; + + enum Either { + A(A), + B(B), + } + + impl Iterator for Either + where + A: Iterator, + B: Iterator, + { + type Item = A::Item; + + fn next(&mut self) -> Option { + match self { + Either::A(a) => a.next(), + Either::B(b) => b.next(), + } + } + + fn size_hint(&self) -> (usize, Option) { + match self { + Either::A(a) => a.size_hint(), + Either::B(b) => b.size_hint(), + } + } + } + + impl ExactSizeIterator for Either + where + A: ExactSizeIterator, + B: ExactSizeIterator, + { + fn len(&self) -> usize { + match self { + Either::A(a) => a.len(), + Either::B(b) => b.len(), + } + } + } } /// Looks up an exported [`Extern`] value by name. @@ -196,9 +299,35 @@ impl Instance { /// # Panics /// /// Panics if `store` does not own this instance. - pub fn get_export(&self, store: impl AsContextMut, name: &str) -> Option { - let store = store.as_context(); - store[self.0].get(name).cloned() + pub fn get_export(&self, mut store: impl AsContextMut, name: &str) -> Option { + self._get_export(&mut store.as_context_mut().opaque(), name) + } + + fn _get_export(&self, store: &mut StoreOpaque<'_>, name: &str) -> Option { + match &store[self.0] { + // Synthetic instances always have their entire list of exports + // already specified. + InstanceData::Synthetic(names) => names.get(name).cloned(), + + // Instantiated instances will lazily fill in exports, so we process + // all that lazy logic here. + InstanceData::Instantiated { id, exports, .. } => { + let instance = store.instance(*id); + let (i, _, index) = instance.module().exports.get_full(name)?; + if let Some(export) = &exports[i] { + return Some(export.clone()); + } + let item = unsafe { + Extern::from_wasmtime_export(instance.lookup_by_declaration(index), store) + }; + let exports = match &mut store[self.0] { + InstanceData::Instantiated { exports, .. } => exports, + _ => unreachable!(), + }; + exports[i] = Some(item.clone()); + Some(item) + } + } } /// Looks up an exported [`Func`] value by name. @@ -346,11 +475,11 @@ impl<'a> Instantiator<'a> { // 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, start, toplevel)) = self.step(&mut store)? { + if let Some(start) = start { + Instantiator::start_raw(&mut store, instance, start)?; } - if let Some(instance) = instance { + if toplevel { break Ok(instance); } } @@ -368,14 +497,13 @@ impl<'a> Instantiator<'a> { // 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((instance, start, toplevel)) = step { if let Some(start) = start { store - .on_fiber(|store| Instantiator::start_raw(store, id, start)) + .on_fiber(|store| Instantiator::start_raw(store, instance, start)) .await??; } - if let Some(instance) = instance { + if toplevel { break Ok(instance); } } @@ -405,7 +533,7 @@ impl<'a> Instantiator<'a> { fn step( &mut self, store: &mut StoreOpaque<'_>, - ) -> Result)>> { + ) -> Result, bool)>> { if self.cur.initializer == 0 { store.bump_resource_counts(&self.cur.module)?; } @@ -484,7 +612,7 @@ impl<'a> Instantiator<'a> { // type-checking since only valid modules should reach this point. Some(Initializer::AliasInstanceExport { instance, export }) => { let instance = self.cur.instances[*instance]; - let export = store[instance.0][export].clone(); + let export = instance._get_export(store, export).unwrap(); self.cur.push(export, store); } @@ -550,24 +678,26 @@ impl<'a> Instantiator<'a> { // Note that in all cases we return the raw instance handle to get // the start function executed by the outer context. None => { - let instance = self.instantiate_raw(store)?; - let items = self.runtime_instance(store, instance); - let items = match self.in_progress.pop() { + let (instance, start) = self.instantiate_raw(store)?; + let toplevel = match self.in_progress.pop() { Some(imports) => { self.cur = imports; - self.cur.instances.push(items); - None + self.cur.instances.push(instance); + false } - None => Some(items), + None => true, }; - return Ok(Some((instance, items))); + return Ok(Some((instance, start, toplevel))); } } Ok(None) } - fn instantiate_raw(&mut self, store: &mut StoreOpaque<'_>) -> Result { + fn instantiate_raw( + &mut self, + store: &mut StoreOpaque<'_>, + ) -> Result<(Instance, Option)> { let compiled_module = self.cur.module.compiled_module(); // Register the module just before instantiation to ensure we keep the module @@ -575,26 +705,34 @@ impl<'a> Instantiator<'a> { store.modules_mut().register(&self.cur.module); unsafe { - let mut instance = store - .engine() - .allocator() - .allocate(InstanceAllocationRequest { - module: compiled_module.module().clone(), - finished_functions: compiled_module.finished_functions(), - imports: self.cur.build(), - shared_signatures: self.cur.module.signatures().as_module_map().into(), - host_state: Box::new(()), - store: Some(store.traitobj), - })?; + // The first thing we do is issue an instance allocation request + // to the instance allocator. This, on success, will give us an + // instance handle. + // + // Note that the `host_state` here is a pointer back to the + // `Instance` we'll be returning from this function. This is a + // circular reference so we can't construct it before we construct + // this instance, so we determine what the ID is and then assert + // it's the same later when we do actually insert it. + let instance_to_be = store.store_data().next_id::(); + let mut instance_handle = + store + .engine() + .allocator() + .allocate(InstanceAllocationRequest { + module: compiled_module.module().clone(), + finished_functions: compiled_module.finished_functions(), + imports: self.cur.build(), + shared_signatures: self.cur.module.signatures().as_module_map().into(), + host_state: Box::new(Instance(instance_to_be)), + store: Some(store.traitobj), + })?; - // After we've created the `InstanceHandle` we still need to run - // initialization to set up data/elements/etc. We do this after - // adding the `InstanceHandle` to the store though. This is required - // for safety because the start function (for example) may trap, but - // element initializers may have run which placed elements into - // other instance's tables. This means that from this point on, - // regardless of whether initialization is successful, we need to - // keep the instance alive. + // The instance still has lots of setup, for example + // data/elements/start/etc. This can all fail, but even on failure + // the instance may persist some state via previous successful + // initialization. For this reason once we have an instance handle + // we immediately insert it into the store to keep it alive. // // Note that we `clone` the instance handle just to make easier // working the the borrow checker here easier. Technically the `&mut @@ -602,12 +740,69 @@ impl<'a> Instantiator<'a> { // conflicts with the borrow on `store.engine`) but this doesn't // matter in practice since initialization isn't even running any // code here anyway. - let id = store.add_instance(instance.clone(), false); + let id = store.add_instance(instance_handle.clone(), false); + + // Additionally, before we start doing fallible instantiation, we + // do one more step which is to insert an `InstanceData` + // corresponding to this instance. This `InstanceData` can be used + // via `Caller::get_export` if our instance's state "leaks" into + // other instances, even if we don't return successfully from this + // function. + // + // We don't actually load all exports from the instance at this + // time, instead preferring to lazily load them as they're demanded. + // For module/instance exports, though, those aren't actually + // stored in the instance handle so we need to immediately handle + // those here. + let instance = { + let exports = compiled_module + .module() + .exports + .values() + .map(|index| { + // Note that instances and modules are not handled by + // `wasmtime_runtime`, they're handled by us in this crate. That + // means we need to handle that here, otherwise we defer to the + // instance to load the values. + match *index { + EntityIndex::Instance(i) => { + Some(Extern::Instance(self.cur.instances[i].clone())) + } + EntityIndex::Module(i) => { + Some(Extern::Module(self.cur.modules[i].clone())) + } + _ => None, + } + }) + .collect(); + let data = InstanceData::Instantiated { + id, + exports, + types: Arc::clone(self.cur.module.types()), + signatures: Arc::clone(self.cur.module.signatures()), + }; + Instance::from_wasmtime(data, store) + }; + + // double-check our guess of what the new instance's ID would be + // was actually correct. + assert_eq!(instance.0, instance_to_be); + + // Now that we've recorded all information we need to about this + // instance within a `Store` we can start performing fallible + // initialization. Note that we still defer the `start` function to + // later since that may need to run asynchronously. + // + // If this returns an error (or if the start function traps) then + // any other initialization which may have succeeded which placed + // items from this instance into other instances should be ok when + // those items are loaded and run we'll have all the metadata to + // look at them. store .engine() .allocator() .initialize( - &mut instance, + &mut instance_handle, compiled_module.module(), store.engine().config().features.bulk_memory, ) @@ -618,18 +813,18 @@ impl<'a> Instantiator<'a> { } })?; - Ok(id) + Ok((instance, compiled_module.module().start_func)) } } - fn start_raw( - store: &mut StoreOpaque<'_>, - instance: InstanceId, - start: FuncIndex, - ) -> Result<()> { + fn start_raw(store: &mut StoreOpaque<'_>, instance: Instance, start: FuncIndex) -> Result<()> { + let id = match &store[instance.0] { + InstanceData::Instantiated { id, .. } => *id, + InstanceData::Synthetic(_) => return Ok(()), + }; // If a start function is present, invoke it. Make sure we use all the // trap-handling configuration in `store` as well. - let instance = store.instance(instance); + let instance = store.instance(id); let f = match instance.lookup_by_declaration(&EntityIndex::Function(start)) { wasmtime_runtime::Export::Function(f) => f, _ => unreachable!(), // valid modules shouldn't hit this @@ -647,45 +842,6 @@ impl<'a> Instantiator<'a> { } Ok(()) } - - fn runtime_instance(&mut self, store: &mut StoreOpaque<'_>, instance: InstanceId) -> Instance { - // We use an unsafe `clone()` here to work around the borrow checker. - // Technically our instance is a borrow of `store`, but we need the - // borrow again later when calling `Extern::from_wasmtime_export` (and a - // mutable one at that). - // - // The mutability in `from_wasmtime_export` only mutates `StoreData` - // since we're adding ids, but it definitely doesn't deallocate - // `instance` (nothing does that except `Drop` for `Store`), so this in - // theory should be safe. - let instance = unsafe { store.instance(instance).clone() }; - - // FIXME(#2916) we should ideally just store the `InstanceId` within the - // store itself. There should be no reason we have to allocate a hash - // map here and allocate a bunch of strings, that's quite wasteful if - // only one or two exports are used. Additionally this can push items - // into the `Store` which never end up getting used. - let exports = instance - .module() - .exports - .iter() - .map(|(name, index)| { - // Note that instances and modules are not handled by - // `wasmtime_runtime`, they're handled by us in this crate. That - // means we need to handle that here, otherwise we defer to the - // instance to load the values. - let item = match index { - EntityIndex::Instance(i) => Extern::Instance(self.cur.instances[*i].clone()), - EntityIndex::Module(i) => Extern::Module(self.cur.modules[*i].clone()), - index => unsafe { - Extern::from_wasmtime_export(instance.lookup_by_declaration(index), store) - }, - }; - (name.clone(), item) - }) - .collect(); - Instance::from_wasmtime(Arc::new(exports), store) - } } impl<'a> ImportsBuilder<'a> { @@ -737,8 +893,6 @@ impl<'a> ImportsBuilder<'a> { } } -pub(crate) type RuntimeInstance = Arc>; - /// An instance, pre-instantiation, that is ready to be instantiated. /// /// This structure represents an instance *just before* it was instantiated, @@ -879,7 +1033,7 @@ fn typecheck( let cx = matching::MatchCx { signatures: module.signatures(), types: module.types(), - store_data: store.store_data(), + store: store, engine: store.engine(), }; for ((name, field, expected_ty), actual) in env_module.imports().zip(imports) { diff --git a/crates/wasmtime/src/linker.rs b/crates/wasmtime/src/linker.rs index 6fc1c4546d..22ddead971 100644 --- a/crates/wasmtime/src/linker.rs +++ b/crates/wasmtime/src/linker.rs @@ -1,5 +1,5 @@ use crate::func::HostFunc; -use crate::instance::InstancePre; +use crate::instance::{InstanceData, InstancePre}; use crate::store::StoreOpaque; use crate::{ AsContextMut, Caller, Engine, Extern, ExternType, Func, FuncType, ImportType, Instance, @@ -1083,7 +1083,7 @@ impl Definition { .map(|(name, item)| (name.clone(), item.to_extern(store))) .collect(), ); - Instance::from_wasmtime(items, store).into() + Instance::from_wasmtime(InstanceData::Synthetic(items), store).into() } } } diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index be906b0645..b10c1b5c91 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -629,10 +629,6 @@ impl<'a, T> StoreContextMut<'a, T> { self.0 .out_of_fuel_async_yield(injection_count, fuel_to_inject) } - - pub(crate) fn store_data(self) -> &'a StoreData { - self.0.store_data() - } } impl StoreInner { diff --git a/crates/wasmtime/src/store/context.rs b/crates/wasmtime/src/store/context.rs index 94a81619ea..6513c7d2de 100644 --- a/crates/wasmtime/src/store/context.rs +++ b/crates/wasmtime/src/store/context.rs @@ -9,7 +9,7 @@ use std::ops::{Deref, DerefMut}; // representation of this `struct` is a pointer for now. If the representation // changes then the C API will need to be updated #[repr(transparent)] -pub struct StoreContext<'a, T>(pub(super) &'a StoreInner); +pub struct StoreContext<'a, T>(pub(crate) &'a StoreInner); /// A temporary handle to a [`&mut Store`][`Store`]. /// @@ -219,8 +219,11 @@ pub struct StoreOpaque<'a> { pub traitobj: *mut dyn wasmtime_runtime::Store, } -pub trait Opaque {} -impl Opaque for T {} +impl<'a> StoreOpaque<'a> { + pub fn into_inner(self) -> &'a StoreInnermost { + self.inner + } +} // Deref impls to forward all methods on `StoreOpaque` to `StoreInner`. impl<'a> Deref for StoreOpaque<'a> { diff --git a/crates/wasmtime/src/store/data.rs b/crates/wasmtime/src/store/data.rs index 789c9fb111..757599f075 100644 --- a/crates/wasmtime/src/store/data.rs +++ b/crates/wasmtime/src/store/data.rs @@ -3,7 +3,7 @@ use crate::{StoreContext, StoreContextMut}; use std::fmt; use std::marker; use std::num::NonZeroU64; -use std::ops::Index; +use std::ops::{Index, IndexMut}; use std::sync::atomic::{AtomicU64, Ordering::SeqCst}; // This is defined here, in a private submodule, so we can explicitly reexport @@ -18,35 +18,22 @@ pub struct StoreData { funcs: Vec, tables: Vec, globals: Vec, - instances: Vec, + instances: Vec, memories: Vec, } pub trait StoredData: Sized { - fn get<'a>(id: Stored, data: &'a StoreData) -> &'a Self; - fn insert(self, data: &mut StoreData) -> Stored; - fn assert_contained(id: Stored, data: &StoreData); + fn list(data: &StoreData) -> &Vec; + fn list_mut(data: &mut StoreData) -> &mut Vec; } macro_rules! impl_store_data { ($($field:ident => $t:ty,)*) => ($( impl StoredData for $t { #[inline] - fn get<'a>(id: Stored, data: &'a StoreData) -> &'a Self { - assert!(id.store_id() == data.id, - "object used with the wrong store"); - &data.$field[id.index()] - } - - fn insert(self, data: &mut StoreData) -> Stored { - let index = data.$field.len(); - data.$field.push(self); - Stored::new(data.id, index) - } - - fn assert_contained(id: Stored, data: &StoreData) { - assert!(id.index() < data.$field.len()); - } + fn list(data: &StoreData) -> &Vec { &data.$field } + #[inline] + fn list_mut(data: &mut StoreData) -> &mut Vec { &mut data.$field } } )*) } @@ -55,7 +42,7 @@ impl_store_data! { funcs => crate::func::FuncData, tables => wasmtime_runtime::ExportTable, globals => wasmtime_runtime::ExportGlobal, - instances => crate::instance::RuntimeInstance, + instances => crate::instance::InstanceData, memories => wasmtime_runtime::ExportMemory, } @@ -85,7 +72,17 @@ impl StoreData { where T: StoredData, { - data.insert(self) + let list = T::list_mut(self); + let index = list.len(); + list.push(data); + Stored::new(self.id, index) + } + + pub fn next_id(&self) -> Stored + where + T: StoredData, + { + Stored::new(self.id, T::list(self).len()) } pub fn contains(&self, id: Stored) -> bool @@ -98,7 +95,7 @@ impl StoreData { // this should be true as an invariant of our API, but double-check with // debug assertions enabled. if cfg!(debug_assertions) { - T::assert_contained(id, self); + assert!(id.index() < T::list(self).len()); } true } @@ -112,7 +109,25 @@ where #[inline] fn index(&self, index: Stored) -> &Self::Output { - T::get(index, self) + assert!( + index.store_id() == self.id, + "object used with the wrong store" + ); + &T::list(self)[index.index()] + } +} + +impl IndexMut> for StoreData +where + T: StoredData, +{ + #[inline] + fn index_mut(&mut self, index: Stored) -> &mut Self::Output { + assert!( + index.store_id() == self.id, + "object used with the wrong store" + ); + &mut T::list_mut(self)[index.index()] } } @@ -154,6 +169,15 @@ where self.store_data().index(index) } } +impl IndexMut for StoreOpaque<'_> +where + StoreData: IndexMut, +{ + #[inline] + fn index_mut(&mut self, index: I) -> &mut Self::Output { + self.store_data_mut().index_mut(index) + } +} #[repr(C)] // used by reference in the C API pub struct Stored { @@ -180,6 +204,12 @@ impl Stored { } } +impl PartialEq for Stored { + fn eq(&self, other: &Stored) -> bool { + self.store_id == other.store_id && self.index == other.index + } +} + impl Copy for Stored {} impl Clone for Stored { diff --git a/crates/wasmtime/src/types/matching.rs b/crates/wasmtime/src/types/matching.rs index cb22f97b22..9709288c4d 100644 --- a/crates/wasmtime/src/types/matching.rs +++ b/crates/wasmtime/src/types/matching.rs @@ -1,5 +1,6 @@ +use crate::instance::InstanceData; use crate::linker::Definition; -use crate::store::StoreData; +use crate::store::StoreInnermost; use crate::{signatures::SignatureCollection, Engine, Extern}; use anyhow::{bail, Context, Result}; use wasmtime_environ::wasm::{ @@ -11,13 +12,13 @@ use wasmtime_runtime::VMSharedSignatureIndex; pub struct MatchCx<'a> { pub signatures: &'a SignatureCollection, pub types: &'a TypeTables, - pub store_data: &'a StoreData, + pub store: &'a StoreInnermost, pub engine: &'a Engine, } impl MatchCx<'_> { pub fn global(&self, expected: &Global, actual: &crate::Global) -> Result<()> { - self.global_ty(expected, actual.wasmtime_ty(self.store_data)) + self.global_ty(expected, actual.wasmtime_ty(self.store.store_data())) } fn global_ty(&self, expected: &Global, actual: &Global) -> Result<()> { @@ -32,7 +33,7 @@ impl MatchCx<'_> { } pub fn table(&self, expected: &Table, actual: &crate::Table) -> Result<()> { - self.table_ty(expected, actual.wasmtime_ty(self.store_data)) + self.table_ty(expected, actual.wasmtime_ty(self.store.store_data())) } fn table_ty(&self, expected: &Table, actual: &Table) -> Result<()> { @@ -54,7 +55,7 @@ impl MatchCx<'_> { } pub fn memory(&self, expected: &Memory, actual: &crate::Memory) -> Result<()> { - self.memory_ty(expected, actual.wasmtime_ty(self.store_data)) + self.memory_ty(expected, actual.wasmtime_ty(self.store.store_data())) } fn memory_ty(&self, expected: &Memory, actual: &Memory) -> Result<()> { @@ -75,7 +76,7 @@ impl MatchCx<'_> { } pub fn func(&self, expected: SignatureIndex, actual: &crate::Func) -> Result<()> { - self.vmshared_signature_index(expected, actual.sig_index(self.store_data)) + self.vmshared_signature_index(expected, actual.sig_index(self.store.store_data())) } pub(crate) fn host_func( @@ -105,14 +106,35 @@ impl MatchCx<'_> { } pub fn instance(&self, expected: InstanceTypeIndex, actual: &crate::Instance) -> Result<()> { - let items = actual.items(self.store_data); for (name, expected) in self.types.instance_signatures[expected].exports.iter() { - match items.get(name) { - Some(item) => { - self.extern_(expected, item) - .with_context(|| format!("instance export {:?} incompatible", name))?; + match actual.data(self.store.store_data()) { + InstanceData::Synthetic(names) => match names.get(name) { + Some(item) => { + self.extern_(expected, item) + .with_context(|| format!("instance export {:?} incompatible", name))?; + } + None => bail!("instance type missing export {:?}", name), + }, + InstanceData::Instantiated { + id, + types, + signatures, + .. + } => { + let module = self.store.instance(*id).module(); + match module.exports.get(name) { + Some(index) => { + let actual_ty = module.type_of(*index); + self.extern_ty_matches(expected, &actual_ty, signatures, types) + .with_context(|| { + format!("instance export {:?} incompatible", name) + })?; + } + None => bail!("instance type missing export {:?}", name), + } + + // let } - None => bail!("instance type missing export {:?}", name), } } Ok(()) @@ -169,7 +191,7 @@ impl MatchCx<'_> { MatchCx { signatures: actual_signatures, types: actual_types, - store_data: self.store_data, + store: self.store, engine: self.engine, } .extern_ty_matches(&actual_ty, expected_ty, self.signatures, self.types)