Disallow values to cross stores (#1016)

* Disallow values to cross stores

Lots of internals in the wasmtime-{jit,runtime} crates are highly
unsafe, so it's up to the `wasmtime` API crate to figure out how to make
it safe. One guarantee we need to provide is that values never cross
between stores. For example you can't take a function in one store and
move it over into a different instance in a different store. This
dynamic check can't be performed at compile time and it's up to
`wasmtime` to do the check itself.

This adds a number of checks, but not all of them, to the codebase for
now. This primarily adds checks around instantiation, globals, and
tables. The main hole in this is functions, where you can pass in
arguments or return values that are not from the right store. For now
though we can't compile modules with `anyref` parameters/returns anyway,
so we should be good. Eventually when that is supported we'll need to
put the guards in place.

Closes #958

* Clarify how values test they come from stores

* Allow null anyref to initialize tables

crates/api/src/externals.rs

@@ -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 {
@@ -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,
@@ -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,
@@ -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 {
@@ -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,
@@ -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.
@@ -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,
@@ -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,
@@ -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,

crates/api/src/func.rs

@@ -143,7 +143,7 @@ use wasmtime_runtime::{InstanceHandle, VMContext, VMFunctionBody};
 /// ```
 #[derive(Clone)]
 pub struct Func {
-    _store: Store,
+    store: Store,
     callable: Rc<dyn WrappedCallable + 'static>,
     ty: FuncType,
 }
@@ -510,7 +510,7 @@ impl Func {
         callable: Rc<dyn WrappedCallable + 'static>,
     ) -> Func {
         Func {
-            _store: store.clone(),
+            store: store.clone(),
             callable,
             ty,
         }
@@ -541,6 +541,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())
@@ -689,6 +696,10 @@ impl Func {
         /// See the [`Func::get1`] method for more documentation.
         (get15, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11, A12, A13, A14, A15)
     }
+
+    pub(crate) fn store(&self) -> &Store {
+        &self.store
+    }
 }
 
 impl fmt::Debug for Func {

crates/api/src/instance.rs

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

crates/api/src/values.rs

@@ -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 {
@@ -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(),
@@ -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()

crates/api/tests/externals.rs

@@ -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(())
+}