Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow values to cross stores #1016

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 26 additions & 6 deletions crates/api/src/externals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,16 @@ impl Extern {
}
}
}

pub(crate) fn comes_from_same_store(&self, store: &Store) -> bool {
let my_store = match self {
Extern::Func(f) => f.store(),
Extern::Global(g) => &g.store,
Extern::Memory(m) => &m.store,
Extern::Table(t) => &t.store,
};
Store::same(my_store, store)
}
}

impl From<Func> for Extern {
Expand Down Expand Up @@ -153,7 +163,7 @@ impl From<Table> for Extern {
/// instances are equivalent in their functionality.
#[derive(Clone)]
pub struct Global {
_store: Store,
store: Store,
ty: GlobalType,
wasmtime_export: wasmtime_runtime::Export,
wasmtime_handle: InstanceHandle,
Expand All @@ -172,12 +182,15 @@ impl Global {
/// Returns an error if the `ty` provided does not match the type of the
/// value `val`.
pub fn new(store: &Store, ty: GlobalType, val: Val) -> Result<Global> {
if !val.comes_from_same_store(store) {
bail!("cross-`Store` globals are not supported");
}
if val.ty() != *ty.content() {
bail!("value provided does not match the type of this global");
}
let (wasmtime_handle, wasmtime_export) = generate_global_export(store, &ty, val)?;
Ok(Global {
_store: store.clone(),
store: store.clone(),
ty,
wasmtime_export,
wasmtime_handle,
Expand Down Expand Up @@ -227,6 +240,9 @@ impl Global {
val.ty()
);
}
if !val.comes_from_same_store(&self.store) {
bail!("cross-`Store` values are not supported");
}
let definition = unsafe { &mut *self.wasmtime_global_definition() };
unsafe {
match val {
Expand Down Expand Up @@ -259,7 +275,7 @@ impl Global {
let ty = GlobalType::from_wasmtime_global(&global)
.expect("core wasm global type should be supported");
Global {
_store: store.clone(),
store: store.clone(),
ty: ty,
wasmtime_export: export,
wasmtime_handle,
Expand Down Expand Up @@ -421,6 +437,10 @@ impl Table {
src_index: u32,
len: u32,
) -> Result<()> {
if !Store::same(&dst_table.store, &src_table.store) {
bail!("cross-`Store` table copies are not supported");
}

// NB: We must use the `dst_table`'s `wasmtime_handle` for the
// `dst_table_index` and vice versa for `src_table` since each table can
// come from different modules.
Expand Down Expand Up @@ -488,7 +508,7 @@ impl Table {
/// implemented though!
#[derive(Clone)]
pub struct Memory {
_store: Store,
store: Store,
ty: MemoryType,
wasmtime_handle: InstanceHandle,
wasmtime_export: wasmtime_runtime::Export,
Expand All @@ -504,7 +524,7 @@ impl Memory {
let (wasmtime_handle, wasmtime_export) =
generate_memory_export(store, &ty).expect("generated memory");
Memory {
_store: store.clone(),
store: store.clone(),
ty,
wasmtime_handle,
wasmtime_export,
Expand Down Expand Up @@ -634,7 +654,7 @@ impl Memory {
};
let ty = MemoryType::from_wasmtime_memory(&memory.memory);
Memory {
_store: store.clone(),
store: store.clone(),
ty: ty,
wasmtime_handle: instance_handle,
wasmtime_export: export,
Expand Down
15 changes: 13 additions & 2 deletions crates/api/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use wasmtime_runtime::{InstanceHandle, VMContext, VMFunctionBody};
/// instances are equivalent in their functionality.
#[derive(Clone)]
pub struct Func {
_store: Store,
store: Store,
callable: Rc<dyn WrappedCallable + 'static>,
ty: FuncType,
}
Expand Down Expand Up @@ -272,7 +272,7 @@ impl Func {
callable: Rc<dyn WrappedCallable + 'static>,
) -> Func {
Func {
_store: store.clone(),
store: store.clone(),
callable,
ty,
}
Expand Down Expand Up @@ -303,6 +303,13 @@ impl Func {
/// This function should not panic unless the underlying function itself
/// initiates a panic.
pub fn call(&self, params: &[Val]) -> Result<Box<[Val]>, Trap> {
for param in params {
if !param.comes_from_same_store(&self.store) {
return Err(Trap::new(
"cross-`Store` values are not currently supported",
));
}
}
let mut results = vec![Val::null(); self.result_arity()];
self.callable.call(params, &mut results)?;
Ok(results.into_boxed_slice())
Expand Down Expand Up @@ -421,6 +428,10 @@ impl Func {
/// See the [`Func::get1`] method for more documentation.
(get10, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10)
}

pub(crate) fn store(&self) -> &Store {
&self.store
}
}

impl fmt::Debug for Func {
Expand Down
11 changes: 10 additions & 1 deletion crates/api/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::externals::Extern;
use crate::module::Module;
use crate::runtime::{Config, Store};
use crate::trap::Trap;
use anyhow::{Error, Result};
use anyhow::{bail, Error, Result};
use wasmtime_jit::{CompiledModule, Resolver};
use wasmtime_runtime::{Export, InstanceHandle, InstantiationError};

Expand Down Expand Up @@ -112,6 +112,15 @@ impl Instance {
/// [`ExternType`]: crate::ExternType
pub fn new(module: &Module, imports: &[Extern]) -> Result<Instance, Error> {
let store = module.store();

// For now we have a restriction that the `Store` that we're working
// with is the same for everything involved here.
for import in imports {
if !import.comes_from_same_store(store) {
bail!("cross-`Store` instantiation is not currently supported");
}
}

let config = store.engine().config();
let instance_handle = instantiate(config, module.compiled_module(), imports)?;

Expand Down
21 changes: 20 additions & 1 deletion crates/api/src/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,22 @@ impl Val {
pub fn unwrap_anyref(&self) -> AnyRef {
self.anyref().expect("expected anyref")
}

pub(crate) fn comes_from_same_store(&self, store: &Store) -> bool {
match self {
Val::FuncRef(f) => Store::same(store, f.store()),

// TODO: need to implement this once we actually finalize what
// `anyref` will look like and it's actually implemented to pass it
// to compiled wasm as well.
Val::AnyRef(AnyRef::Ref(_)) | Val::AnyRef(AnyRef::Other(_)) => false,
Val::AnyRef(AnyRef::Null) => true,

// Integers have no association with any particular store, so
// they're always considered as "yes I came from that store",
Val::I32(_) | Val::I64(_) | Val::F32(_) | Val::F64(_) | Val::V128(_) => true,
}
}
}

impl From<i32> for Val {
Expand Down Expand Up @@ -175,6 +191,9 @@ pub(crate) fn into_checked_anyfunc(
val: Val,
store: &Store,
) -> Result<wasmtime_runtime::VMCallerCheckedAnyfunc> {
if !val.comes_from_same_store(store) {
bail!("cross-`Store` values are not supported");
}
Ok(match val {
Val::AnyRef(AnyRef::Null) => wasmtime_runtime::VMCallerCheckedAnyfunc {
func_ptr: ptr::null(),
Expand Down Expand Up @@ -206,7 +225,7 @@ pub(crate) fn from_checked_anyfunc(
store: &Store,
) -> Val {
if item.type_index == wasmtime_runtime::VMSharedSignatureIndex::default() {
return Val::AnyRef(AnyRef::Null);
Val::AnyRef(AnyRef::Null);
}
let signature = store
.compiler()
Expand Down
58 changes: 58 additions & 0 deletions crates/api/tests/externals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,61 @@ fn bad_tables() {
assert!(t.grow(1, Val::I32(0)).is_err());
assert_eq!(t.size(), 1);
}

#[test]
fn cross_store() -> anyhow::Result<()> {
let mut cfg = Config::new();
cfg.wasm_reference_types(true);
let store1 = Store::new(&Engine::new(&cfg));
let store2 = Store::new(&Engine::new(&cfg));

// ============ Cross-store instantiation ==============

let func = Func::wrap0(&store2, || {});
let ty = GlobalType::new(ValType::I32, Mutability::Const);
let global = Global::new(&store2, ty, Val::I32(0))?;
let ty = MemoryType::new(Limits::new(1, None));
let memory = Memory::new(&store2, ty);
let ty = TableType::new(ValType::FuncRef, Limits::new(1, None));
let table = Table::new(&store2, ty, Val::AnyRef(AnyRef::Null))?;

let need_func = Module::new(&store1, r#"(module (import "" "" (func)))"#)?;
assert!(Instance::new(&need_func, &[func.into()]).is_err());

let need_global = Module::new(&store1, r#"(module (import "" "" (global i32)))"#)?;
assert!(Instance::new(&need_global, &[global.into()]).is_err());

let need_table = Module::new(&store1, r#"(module (import "" "" (table 1 funcref)))"#)?;
assert!(Instance::new(&need_table, &[table.into()]).is_err());

let need_memory = Module::new(&store1, r#"(module (import "" "" (memory 1)))"#)?;
assert!(Instance::new(&need_memory, &[memory.into()]).is_err());

// ============ Cross-store globals ==============

let store1val = Val::FuncRef(Func::wrap0(&store1, || {}));
let store2val = Val::FuncRef(Func::wrap0(&store2, || {}));

let ty = GlobalType::new(ValType::FuncRef, Mutability::Var);
assert!(Global::new(&store2, ty.clone(), store1val.clone()).is_err());
if let Ok(g) = Global::new(&store2, ty.clone(), store2val.clone()) {
assert!(g.set(store1val.clone()).is_err());
}

// ============ Cross-store tables ==============

let ty = TableType::new(ValType::FuncRef, Limits::new(1, None));
assert!(Table::new(&store2, ty.clone(), store1val.clone()).is_err());
let t1 = Table::new(&store2, ty.clone(), store2val.clone())?;
assert!(t1.set(0, store1val.clone()).is_err());
assert!(t1.grow(0, store1val.clone()).is_err());
let t2 = Table::new(&store1, ty.clone(), store1val.clone())?;
assert!(Table::copy(&t1, 0, &t2, 0, 0).is_err());

// ============ Cross-store funcs ==============

// TODO: need to actually fill this out once we support anyref params/locals
// let module = Module::new(&store1, r#"(module (func (export "a") (param funcref)))"#)?;

Ok(())
}