T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove the need to have a Store for an InstancePre (#5683)

Browse Source 
* 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<T>` 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<T>`.

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 <andrew.brown@intel.com>

---------

Co-authored-by: Andrew Brown <andrew.brown@intel.com>
This commit is contained in:
Alex Crichton
2023-02-02 11:54:20 -06:00
committed by GitHub
parent f5f517e811
commit 63d80fc509
13 changed files with 242 additions and 224 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
crates/wasmtime/src/component/instance.rs
View File

@@ -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");
}
}

14
crates/wasmtime/src/externals.rs
View File

@@ -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<Func> 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)?;
/// // ...

26
crates/wasmtime/src/instance.rs
View File

@@ -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<T> InstancePre<T> {
/// This method is unsafe as the `T` of the `InstancePre<T>` 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<Definition>,
) -> Result<InstancePre<T>> {
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<Definition>) -> Result<InstancePre<T>> {
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<I>(
store: &mut StoreOpaque,
module: &Module,
imports: &[I],
check: impl Fn(&matching::MatchCx<'_>, &EntityType, &I) -> Result<()>,
@@ -826,8 +817,7 @@ fn typecheck<I>(
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)

154
crates/wasmtime/src/linker.rs
View File

@@ -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<HostFunc>),
}
/// 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<T> Linker<T> {
/// 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<T> Linker<T> {
/// ```
pub fn define(
&mut self,
store: impl AsContext<Data = T>,
module: &str,
name: &str,
item: impl Into<Extern>,
) -> 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<T> Linker<T> {
/// 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<Extern>) -> Result<&mut Self> {
pub fn define_name(
&mut self,
store: impl AsContext<Data = T>,
name: &str,
item: impl Into<Extern>,
) -> 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<T> Linker<T> {
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::<Vec<_>>();
for (key, export) in exports {
self.insert(key, Definition::new(store.0, export))?;
}
Ok(self)
}
@@ -814,11 +846,11 @@ impl<T> Linker<T> {
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<T> Linker<T> {
mut store: impl AsContextMut<Data = T>,
module: &Module,
) -> Result<Instance> {
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<T> Linker<T> {
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<T> Linker<T> {
/// )
/// "#;
/// 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<T> Linker<T> {
/// # Ok(())
/// # }
/// ```
pub fn instantiate_pre(
pub fn instantiate_pre(&self, module: &Module) -> Result<InstancePre<T>> {
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<Data = T>,
module: &Module,
store: Option<&StoreOpaque>,
) -> Result<InstancePre<T>> {
let store = store.as_context_mut().0;
let imports = module
let mut imports = module
.imports()
.map(|import| self._get_by_import(&import))
.collect::<Result<_, _>>()?;
unsafe { InstancePre::new(store, module, imports) }
.collect::<Result<Vec<_>, _>>()?;
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<T> Default for Linker<T> {
}
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.

68
crates/wasmtime/src/types/matching.rs
View File

@@ -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))]