From 420dcd76fd0d684291901c7a6afeb481481dea7e Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 13 Jan 2020 17:50:57 -0600 Subject: [PATCH] Don't require `Store` in `Instance` constructor (#810) * Don't require `Store` in `Instance` constructor This can be inferred from the `Module` argument. Additionally add a `store` accessor to an `Instance` in case it's needed to instantiate another `Module`. cc #708 * Update more constructors * Fix a doctest * Don't ignore store in `wasm_instance_new` * Run rustfmt --- crates/api/examples/gcd.rs | 2 +- crates/api/examples/hello.rs | 3 +-- crates/api/examples/memory.rs | 2 +- crates/api/examples/multi.rs | 4 ++-- crates/api/src/callable.rs | 1 - crates/api/src/instance.rs | 20 ++++++++++++++++--- crates/api/src/runtime.rs | 4 ++++ crates/api/src/wasm.rs | 16 +++++++++++++-- crates/api/tests/import-indexes.rs | 2 +- crates/api/tests/import_calling_export.rs | 2 +- crates/api/tests/traps.rs | 2 +- crates/fuzzing/src/oracles.rs | 4 ++-- crates/misc/py/src/lib.rs | 2 +- crates/misc/rust/macro/src/lib.rs | 2 +- .../test-programs/tests/wasm_tests/runtime.rs | 2 +- crates/test-programs/wasi-tests/Cargo.lock | 2 +- crates/wast/src/wast.rs | 2 +- src/commands/run.rs | 2 +- tests/custom_signal_handler.rs | 10 +++++----- 19 files changed, 56 insertions(+), 28 deletions(-) diff --git a/crates/api/examples/gcd.rs b/crates/api/examples/gcd.rs index c510a176ca..43a29fcf01 100644 --- a/crates/api/examples/gcd.rs +++ b/crates/api/examples/gcd.rs @@ -50,7 +50,7 @@ fn main() -> anyhow::Result<()> { .0; // Instantiate the module. - let instance = Instance::new(&store, &module, &[])?; + let instance = Instance::new(&module, &[])?; // Invoke `gcd` export let gcd = instance.exports()[gcd_index].func().expect("gcd"); diff --git a/crates/api/examples/hello.rs b/crates/api/examples/hello.rs index 8924dfcb7d..c33b2c4495 100644 --- a/crates/api/examples/hello.rs +++ b/crates/api/examples/hello.rs @@ -48,8 +48,7 @@ fn main() -> Result<()> { // Note that this is where the wasm `start` function, if any, would run. println!("Instantiating module..."); let imports = vec![hello_func.into()]; - let instance = - Instance::new(&store, &module, &imports).context("> Error instantiating module!")?; + let instance = Instance::new(&module, &imports).context("> Error instantiating module!")?; // Next we poke around a bit to extract the `run` function from the module. println!("Extracting export..."); diff --git a/crates/api/examples/memory.rs b/crates/api/examples/memory.rs index f9df469669..a167e6f1ac 100644 --- a/crates/api/examples/memory.rs +++ b/crates/api/examples/memory.rs @@ -90,7 +90,7 @@ fn main() -> Result<(), Error> { // Instantiate. println!("Instantiating module..."); - let instance = Instance::new(&store, &module, &[]).context("> Error instantiating module!")?; + let instance = Instance::new(&module, &[]).context("> Error instantiating module!")?; // Extract export. println!("Extracting export..."); diff --git a/crates/api/examples/multi.rs b/crates/api/examples/multi.rs index d3236501cb..830d899138 100644 --- a/crates/api/examples/multi.rs +++ b/crates/api/examples/multi.rs @@ -67,8 +67,8 @@ fn main() -> Result<()> { // Instantiate. println!("Instantiating module..."); let imports = vec![callback_func.into()]; - let instance = Instance::new(&store, &module, imports.as_slice()) - .context("Error instantiating module!")?; + let instance = + Instance::new(&module, imports.as_slice()).context("Error instantiating module!")?; // Extract exports. println!("Extracting export..."); diff --git a/crates/api/src/callable.rs b/crates/api/src/callable.rs index 4aefa8b206..139959f359 100644 --- a/crates/api/src/callable.rs +++ b/crates/api/src/callable.rs @@ -59,7 +59,6 @@ use wasmtime_runtime::Export; /// /// // Create module instance that imports our function /// let instance = wasmtime::Instance::new( -/// &store, /// &module, /// &[times_two_function.into()] /// )?; diff --git a/crates/api/src/instance.rs b/crates/api/src/instance.rs index 145e25f2c2..7d1d8c18e3 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/src/instance.rs @@ -68,7 +68,8 @@ pub struct Instance { } impl Instance { - pub fn new(store: &Store, module: &Module, externs: &[Extern]) -> Result { + pub fn new(module: &Module, externs: &[Extern]) -> Result { + let store = module.store(); let context = store.context().clone(); let exports = store.global_exports().clone(); let (mut instance_handle, contexts) = instantiate_in_context( @@ -100,14 +101,27 @@ impl Instance { }) } - pub fn exports(&self) -> &[Extern] { - &self.exports + /// Returns the associated [`Store`] that this `Instance` is compiled into. + /// + /// This is the [`Store`] that generally serves as a sort of global cache + /// for various instance-related things. + pub fn store(&self) -> &Store { + self.module.store() } + /// Returns the associated [`Module`] that this `Instance` instantiated. + /// + /// The corresponding [`Module`] here is a static version of this `Instance` + /// which can be used to learn information such as naming information about + /// various functions. pub fn module(&self) -> &Module { &self.module } + pub fn exports(&self) -> &[Extern] { + &self.exports + } + pub fn find_export_by_name(&self, name: &str) -> Option<&Extern> { let (i, _) = self .module diff --git a/crates/api/src/runtime.rs b/crates/api/src/runtime.rs index d2a4117950..7476bc44c9 100644 --- a/crates/api/src/runtime.rs +++ b/crates/api/src/runtime.rs @@ -397,6 +397,10 @@ impl Store { .get(&type_index) .cloned() } + + pub(crate) fn ptr_eq(a: &Store, b: &Store) -> bool { + Rc::ptr_eq(&a.inner, &b.inner) + } } impl Default for Store { diff --git a/crates/api/src/wasm.rs b/crates/api/src/wasm.rs index f87ca77b8e..083671406f 100644 --- a/crates/api/src/wasm.rs +++ b/crates/api/src/wasm.rs @@ -686,7 +686,6 @@ pub unsafe extern "C" fn wasm_instance_new( imports: *const *const wasm_extern_t, result: *mut *mut wasm_trap_t, ) -> *mut wasm_instance_t { - let store = &(*store).store.borrow(); let mut externs: Vec = Vec::with_capacity((*module).imports.len()); for i in 0..(*module).imports.len() { let import = *imports.add(i); @@ -697,8 +696,21 @@ pub unsafe extern "C" fn wasm_instance_new( ExternHost::Memory(e) => Extern::Memory(e.borrow().clone()), }); } + let store = &(*store).store.borrow(); let module = &(*module).module.borrow(); - match Instance::new(store, module, &externs) { + // FIXME(WebAssembly/wasm-c-api#126) what else can we do with the `store` + // argument? + if !Store::ptr_eq(&store, module.store()) { + if !result.is_null() { + let trap = Trap::new("wasm_store_t must match store in wasm_module_t"); + let trap = Box::new(wasm_trap_t { + trap: HostRef::new(trap), + }); + (*result) = Box::into_raw(trap); + } + return ptr::null_mut(); + } + match Instance::new(module, &externs) { Ok(instance) => { let instance = Box::new(wasm_instance_t { instance: HostRef::new(instance), diff --git a/crates/api/tests/import-indexes.rs b/crates/api/tests/import-indexes.rs index 18847da8e6..8406a8b63a 100644 --- a/crates/api/tests/import-indexes.rs +++ b/crates/api/tests/import-indexes.rs @@ -55,7 +55,7 @@ fn same_import_names_still_distinct() -> anyhow::Result<()> { ) .into(), ]; - let instance = Instance::new(&store, &module, &imports)?; + let instance = Instance::new(&module, &imports)?; let func = instance.find_export_by_name("foo").unwrap().func().unwrap(); let results = func.call(&[])?; diff --git a/crates/api/tests/import_calling_export.rs b/crates/api/tests/import_calling_export.rs index 71991ae09b..aad031d173 100644 --- a/crates/api/tests/import_calling_export.rs +++ b/crates/api/tests/import_calling_export.rs @@ -47,7 +47,7 @@ fn test_import_calling_export() { let imports = vec![callback_func.into()]; let instance = - Instance::new(&store, &module, imports.as_slice()).expect("failed to instantiate module"); + Instance::new(&module, imports.as_slice()).expect("failed to instantiate module"); let exports = instance.exports(); assert!(!exports.is_empty()); diff --git a/crates/api/tests/traps.rs b/crates/api/tests/traps.rs index 9973236578..aea6404a9b 100644 --- a/crates/api/tests/traps.rs +++ b/crates/api/tests/traps.rs @@ -29,7 +29,7 @@ fn test_trap_return() -> Result<(), String> { let hello_func = Func::new(&store, hello_type, Rc::new(HelloCallback)); let imports = vec![hello_func.into()]; - let instance = Instance::new(&store, &module, &imports) + let instance = Instance::new(&module, &imports) .map_err(|e| format!("failed to instantiate module: {:?}", e))?; let run_func = instance.exports()[0] .func() diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index 2dab05f457..bb5860c9a1 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -60,7 +60,7 @@ pub fn instantiate(wasm: &[u8], strategy: Strategy) { // aren't caught during validation or compilation. For example, an imported // table might not have room for an element segment that we want to // initialize into it. - let _result = Instance::new(&store, &module, &imports); + let _result = Instance::new(&module, &imports); } /// Compile the Wasm buffer, and implicitly fail if we have an unexpected @@ -152,7 +152,7 @@ pub fn make_api_calls(api: crate::generators::api::ApiCalls) { // aren't caught during validation or compilation. For example, an imported // table might not have room for an element segment that we want to // initialize into it. - if let Ok(instance) = Instance::new(store.as_ref().unwrap(), &module, &imports) { + if let Ok(instance) = Instance::new(&module, &imports) { instances.insert(id, instance); } } diff --git a/crates/misc/py/src/lib.rs b/crates/misc/py/src/lib.rs index 1c7584a40c..61a6132c53 100644 --- a/crates/misc/py/src/lib.rs +++ b/crates/misc/py/src/lib.rs @@ -120,7 +120,7 @@ pub fn instantiate( } } - let instance = wasmtime::Instance::new(&store, &module, &imports) + let instance = wasmtime::Instance::new(&module, &imports) .map_err(|t| PyErr::new::(format!("instantiated with trap {:?}", t)))?; let module = Py::new(py, Module { module })?; diff --git a/crates/misc/rust/macro/src/lib.rs b/crates/misc/rust/macro/src/lib.rs index 2d91664dd5..0cbc1e027c 100644 --- a/crates/misc/rust/macro/src/lib.rs +++ b/crates/misc/rust/macro/src/lib.rs @@ -75,7 +75,7 @@ fn generate_load(item: &syn::ItemTrait) -> syn::Result { } } let instance = - Instance::new(&store, &module, &imports).map_err(|t| format_err!("instantiation trap: {:?}", t))?; + Instance::new(&module, &imports).map_err(|t| format_err!("instantiation trap: {:?}", t))?; Ok(#name { instance, data }) } diff --git a/crates/test-programs/tests/wasm_tests/runtime.rs b/crates/test-programs/tests/wasm_tests/runtime.rs index c24ce862c3..28eef2878d 100644 --- a/crates/test-programs/tests/wasm_tests/runtime.rs +++ b/crates/test-programs/tests/wasm_tests/runtime.rs @@ -61,7 +61,7 @@ pub fn instantiate(data: &[u8], bin_name: &str, workspace: Option<&Path>) -> any }) .collect::, _>>()?; - let instance = Instance::new(&store, &module, &imports).context(format!( + let instance = Instance::new(&module, &imports).context(format!( "error while instantiating Wasm module '{}'", bin_name, ))?; diff --git a/crates/test-programs/wasi-tests/Cargo.lock b/crates/test-programs/wasi-tests/Cargo.lock index 83b94cc7b8..b1962b4c0e 100644 --- a/crates/test-programs/wasi-tests/Cargo.lock +++ b/crates/test-programs/wasi-tests/Cargo.lock @@ -17,7 +17,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "wasi-tests" -version = "0.7.0" +version = "0.9.0" dependencies = [ "libc 0.2.66 (registry+https://github.com/rust-lang/crates.io-index)", "more-asserts 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/crates/wast/src/wast.rs b/crates/wast/src/wast.rs index 2564bac34e..fc71896ac9 100644 --- a/crates/wast/src/wast.rs +++ b/crates/wast/src/wast.rs @@ -99,7 +99,7 @@ impl WastContext { .clone(); imports.push(export); } - let instance = match Instance::new(&self.store, &module, &imports) { + let instance = match Instance::new(&module, &imports) { Ok(i) => i, Err(e) => return e.downcast::().map(Outcome::Trap), }; diff --git a/src/commands/run.rs b/src/commands/run.rs index 3dd9a48033..4426245c71 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -259,7 +259,7 @@ impl RunCommand { }) .collect::, _>>()?; - let instance = Instance::new(store, &module, &imports) + let instance = Instance::new(&module, &imports) .context(format!("failed to instantiate {:?}", path))?; Ok((instance, module, data)) diff --git a/tests/custom_signal_handler.rs b/tests/custom_signal_handler.rs index 8f0e1c83e4..40f9080998 100644 --- a/tests/custom_signal_handler.rs +++ b/tests/custom_signal_handler.rs @@ -108,7 +108,7 @@ mod tests { let store = Store::new(&engine); let data = wat::parse_str(WAT1)?; let module = Module::new(&store, &data)?; - let instance = Instance::new(&store, &module, &[])?; + let instance = Instance::new(&module, &[])?; let (base, length) = set_up_memory(&instance); instance.set_signal_handler(move |signum, siginfo, _| { @@ -165,7 +165,7 @@ mod tests { // Set up multiple instances - let instance1 = Instance::new(&store, &module, &[])?; + let instance1 = Instance::new(&module, &[])?; let instance1_handler_triggered = Rc::new(AtomicBool::new(false)); { @@ -192,7 +192,7 @@ mod tests { }); } - let instance2 = Instance::new(&store, &module, &[]).expect("failed to instantiate module"); + let instance2 = Instance::new(&module, &[]).expect("failed to instantiate module"); let instance2_handler_triggered = Rc::new(AtomicBool::new(false)); { @@ -261,7 +261,7 @@ mod tests { // instance1 which defines 'read' let data1 = wat::parse_str(WAT1)?; let module1 = Module::new(&store, &data1)?; - let instance1 = Instance::new(&store, &module1, &[])?; + let instance1 = Instance::new(&module1, &[])?; let (base1, length1) = set_up_memory(&instance1); instance1.set_signal_handler(move |signum, siginfo, _| { println!("instance1"); @@ -275,7 +275,7 @@ mod tests { // instance2 wich calls 'instance1.read' let data2 = wat::parse_str(WAT2)?; let module2 = Module::new(&store, &data2)?; - let instance2 = Instance::new(&store, &module2, &[instance1_read])?; + let instance2 = Instance::new(&module2, &[instance1_read])?; // since 'instance2.run' calls 'instance1.read' we need to set up the signal handler to handle // SIGSEGV originating from within the memory of instance1 instance2.set_signal_handler(move |signum, siginfo, _| {