From 6ff9c4cd2787c3f5493501aca8d94f02651de2db Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 15 Apr 2020 11:28:46 -0700 Subject: [PATCH 01/24] Compute instance exports on demand. Instead having instances eagerly compute a Vec of Externs, and bumping the refcount for each Extern, compute Externs on demand. This also enables `Instance::get_export` to avoid doing a linear search. This also means that the closure returned by `get0` and friends now holds an `InstanceHandle` to dynamically hold the instance live rather than being scoped to a lifetime. --- crates/api/src/externals.rs | 8 ++-- crates/api/src/func.rs | 23 +++++----- crates/api/src/instance.rs | 38 +++++++---------- crates/c-api/src/instance.rs | 1 - crates/runtime/src/instance.rs | 46 ++++++++++++-------- crates/wast/src/wast.rs | 4 +- src/commands/run.rs | 23 +++++----- tests/all/custom_signal_handler.rs | 26 ++++++------ tests/all/func.rs | 68 +++++++++++++++--------------- tests/all/globals.rs | 2 +- tests/all/import_calling_export.rs | 21 +++++---- tests/all/traps.rs | 38 ++++++++++++----- 12 files changed, 161 insertions(+), 137 deletions(-) diff --git a/crates/api/src/externals.rs b/crates/api/src/externals.rs index 4ea3ca3dd1aa..ea3a623c1dfa 100644 --- a/crates/api/src/externals.rs +++ b/crates/api/src/externals.rs @@ -34,7 +34,7 @@ impl Extern { /// Returns the underlying `Func`, if this external is a function. /// /// Returns `None` if this is not a function. - pub fn func(&self) -> Option<&Func> { + pub fn func(self) -> Option { match self { Extern::Func(func) => Some(func), _ => None, @@ -44,7 +44,7 @@ impl Extern { /// Returns the underlying `Global`, if this external is a global. /// /// Returns `None` if this is not a global. - pub fn global(&self) -> Option<&Global> { + pub fn global(self) -> Option { match self { Extern::Global(global) => Some(global), _ => None, @@ -54,7 +54,7 @@ impl Extern { /// Returns the underlying `Table`, if this external is a table. /// /// Returns `None` if this is not a table. - pub fn table(&self) -> Option<&Table> { + pub fn table(self) -> Option { match self { Extern::Table(table) => Some(table), _ => None, @@ -64,7 +64,7 @@ impl Extern { /// Returns the underlying `Memory`, if this external is a memory. /// /// Returns `None` if this is not a memory. - pub fn memory(&self) -> Option<&Memory> { + pub fn memory(self) -> Option { match self { Extern::Memory(memory) => Some(memory), _ => None, diff --git a/crates/api/src/func.rs b/crates/api/src/func.rs index d70195946e18..6635fa3b4a31 100644 --- a/crates/api/src/func.rs +++ b/crates/api/src/func.rs @@ -38,7 +38,7 @@ use wasmtime_runtime::{ExportFunction, VMTrampoline}; /// let store = Store::default(); /// let module = Module::new(&store, r#"(module (func (export "foo")))"#)?; /// let instance = Instance::new(&module, &[])?; -/// let foo = instance.exports()[0].func().expect("export wasn't a function"); +/// let foo = instance.exports().next().unwrap().func().expect("export wasn't a function"); /// /// // Work with `foo` as a `Func` at this point, such as calling it /// // dynamically... @@ -88,7 +88,7 @@ use wasmtime_runtime::{ExportFunction, VMTrampoline}; /// "#, /// )?; /// let instance = Instance::new(&module, &[add.into()])?; -/// let call_add_twice = instance.exports()[0].func().expect("export wasn't a function"); +/// let call_add_twice = instance.exports().next().unwrap().func().expect("export wasn't a function"); /// let call_add_twice = call_add_twice.get0::()?; /// /// assert_eq!(call_add_twice()?, 10); @@ -149,8 +149,8 @@ macro_rules! getters { )*) => ($( $(#[$doc])* #[allow(non_snake_case)] - pub fn $name<'a, $($args,)* R>(&'a self) - -> anyhow::Result Result + 'a> + pub fn $name<$($args,)* R>(self) + -> anyhow::Result Result> where $($args: WasmTy,)* R: WasmTy, @@ -174,7 +174,6 @@ macro_rules! getters { // ... and then once we've passed the typechecks we can hand out our // object since our `transmute` below should be safe! - let f = self.wasmtime_function(); Ok(move |$($args: $args),*| -> Result { unsafe { let fnptr = mem::transmute::< @@ -184,11 +183,11 @@ macro_rules! getters { *mut VMContext, $($args,)* ) -> R, - >(f.address); + >(self.export.address); let mut ret = None; $(let $args = $args.into_abi();)* - wasmtime_runtime::catch_traps(f.vmctx, || { - ret = Some(fnptr(f.vmctx, ptr::null_mut(), $($args,)*)); + wasmtime_runtime::catch_traps(self.export.vmctx, || { + ret = Some(fnptr(self.export.vmctx, ptr::null_mut(), $($args,)*)); }).map_err(Trap::from_jit)?; Ok(ret.unwrap()) } @@ -340,7 +339,7 @@ impl Func { /// "#, /// )?; /// let instance = Instance::new(&module, &[add.into()])?; - /// let foo = instance.exports()[0].func().unwrap().get2::()?; + /// let foo = instance.exports().next().unwrap().func().unwrap().get2::()?; /// assert_eq!(foo(1, 2)?, 3); /// # Ok(()) /// # } @@ -371,7 +370,7 @@ impl Func { /// "#, /// )?; /// let instance = Instance::new(&module, &[add.into()])?; - /// let foo = instance.exports()[0].func().unwrap().get2::()?; + /// let foo = instance.exports().next().unwrap().func().unwrap().get2::()?; /// assert_eq!(foo(1, 2)?, 3); /// assert!(foo(i32::max_value(), 1).is_err()); /// # Ok(()) @@ -404,7 +403,7 @@ impl Func { /// "#, /// )?; /// let instance = Instance::new(&module, &[debug.into()])?; - /// let foo = instance.exports()[0].func().unwrap().get0::<()>()?; + /// let foo = instance.exports().next().unwrap().func().unwrap().get0::<()>()?; /// foo()?; /// # Ok(()) /// # } @@ -460,7 +459,7 @@ impl Func { /// "#, /// )?; /// let instance = Instance::new(&module, &[log_str.into()])?; - /// let foo = instance.exports()[0].func().unwrap().get0::<()>()?; + /// let foo = instance.exports().next().unwrap().func().unwrap().get0::<()>()?; /// foo()?; /// # Ok(()) /// # } diff --git a/crates/api/src/instance.rs b/crates/api/src/instance.rs index fd576d987f49..c93e484c7a88 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/src/instance.rs @@ -68,7 +68,6 @@ fn instantiate( pub struct Instance { pub(crate) instance_handle: InstanceHandle, module: Module, - exports: Box<[Extern]>, } impl Instance { @@ -145,20 +144,9 @@ impl Instance { Box::new(info), )?; - let mut exports = Vec::with_capacity(module.exports().len()); - for export in module.exports() { - let name = export.name().to_string(); - let export = instance_handle.lookup(&name).expect("export"); - exports.push(Extern::from_wasmtime_export( - store, - instance_handle.clone(), - export, - )); - } Ok(Instance { instance_handle, module: module.clone(), - exports: exports.into_boxed_slice(), }) } @@ -186,8 +174,15 @@ impl Instance { /// export you'll need to consult [`Module::exports`]. The list returned /// here maps 1:1 with the list that [`Module::exports`] returns, and /// [`ExportType`](crate::ExportType) contains the name of each export. - pub fn exports(&self) -> &[Extern] { - &self.exports + pub fn exports<'me>(&'me self) -> impl Iterator + 'me { + let instance_handle = &self.instance_handle; + let store = self.module.store(); + self.instance_handle + .exports() + .map(move |(_, entity_index)| { + let export = instance_handle.lookup_by_declaration(entity_index); + Extern::from_wasmtime_export(store, instance_handle.clone(), export) + }) } /// Looks up an exported [`Extern`] value by name. @@ -196,14 +191,13 @@ impl Instance { /// the value, if found. /// /// Returns `None` if there was no export named `name`. - pub fn get_export(&self, name: &str) -> Option<&Extern> { - let (i, _) = self - .module - .exports() - .iter() - .enumerate() - .find(|(_, e)| e.name() == name)?; - Some(&self.exports()[i]) + pub fn get_export(&self, name: &str) -> Option { + let export = self.instance_handle.lookup(&name)?; + Some(Extern::from_wasmtime_export( + self.module.store(), + self.instance_handle.clone(), + export, + )) } #[doc(hidden)] diff --git a/crates/c-api/src/instance.rs b/crates/c-api/src/instance.rs index bbeb5626e17e..113e1baa83a2 100644 --- a/crates/c-api/src/instance.rs +++ b/crates/c-api/src/instance.rs @@ -145,7 +145,6 @@ pub extern "C" fn wasm_instance_exports(instance: &wasm_instance_t, out: &mut wa let instance = &instance.instance.borrow(); instance .exports() - .iter() .map(|e| match e { Extern::Func(f) => ExternHost::Func(HostRef::new(f.clone())), Extern::Global(f) => ExternHost::Global(HostRef::new(f.clone())), diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 205215c5cb64..6eb157d5a5d9 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -383,22 +383,35 @@ impl Instance { None => return Ok(()), }; - let (callee_address, callee_vmctx) = match self.module.local.defined_func_index(start_index) - { - Some(defined_index) => { - let body = *self - .finished_functions - .get(defined_index) - .expect("function index is out of bounds"); - (body as *const _, self.vmctx_ptr()) - } - None => { - assert_lt!(start_index.index(), self.module.imported_funcs.len()); - let import = self.imported_function(start_index); - (import.body, import.vmctx) - } - }; + self.invoke_function_index(start_index) + .map_err(InstantiationError::StartTrap) + } + fn invoke_function_index(&self, callee_index: FuncIndex) -> Result<(), Trap> { + let (callee_address, callee_vmctx) = + match self.module.local.defined_func_index(callee_index) { + Some(defined_index) => { + let body = *self + .finished_functions + .get(defined_index) + .expect("function index is out of bounds"); + (body as *const _, self.vmctx_ptr()) + } + None => { + assert_lt!(callee_index.index(), self.module.imported_funcs.len()); + let import = self.imported_function(callee_index); + (import.body, import.vmctx) + } + }; + + self.invoke_function(callee_vmctx, callee_address) + } + + fn invoke_function( + &self, + callee_vmctx: *mut VMContext, + callee_address: *const VMFunctionBody, + ) -> Result<(), Trap> { // Make the call. unsafe { catch_traps(callee_vmctx, || { @@ -407,7 +420,6 @@ impl Instance { unsafe extern "C" fn(*mut VMContext, *mut VMContext), >(callee_address)(callee_vmctx, self.vmctx_ptr()) }) - .map_err(InstantiationError::StartTrap) } } @@ -1394,7 +1406,7 @@ pub enum InstantiationError { #[error("Trap occurred during instantiation")] Trap(Trap), - /// A compilation error occured. + /// A trap occurred while running the wasm start function. #[error("Trap occurred while invoking start function")] StartTrap(Trap), } diff --git a/crates/wast/src/wast.rs b/crates/wast/src/wast.rs index b4866072cd97..2275ebd6b7c1 100644 --- a/crates/wast/src/wast.rs +++ b/crates/wast/src/wast.rs @@ -66,9 +66,9 @@ impl WastContext { } } - fn get_export(&self, module: Option<&str>, name: &str) -> Result<&Extern> { + fn get_export(&self, module: Option<&str>, name: &str) -> Result { match module { - Some(module) => self.linker.get_one_by_name(module, name), + Some(module) => self.linker.get_one_by_name(module, name).map(Extern::clone), None => self .current .as_ref() diff --git a/src/commands/run.rs b/src/commands/run.rs index 5908215ed21b..429020908514 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -248,19 +248,16 @@ impl RunCommand { } fn invoke_export(&self, instance: Instance, name: &str) -> Result<()> { - let pos = instance - .module() - .exports() - .iter() - .enumerate() - .find(|(_, e)| e.name() == name); - let (ty, export) = match pos { - Some((i, ty)) => match (ty.ty(), &instance.exports()[i]) { - (wasmtime::ExternType::Func(ty), wasmtime::Extern::Func(f)) => (ty, f), - _ => bail!("export of `{}` wasn't a function", name), - }, - None => bail!("failed to find export of `{}` in module", name), + let func = if let Some(export) = instance.get_export(name) { + if let Some(func) = export.func() { + func + } else { + bail!("export of `{}` wasn't a function", name) + } + } else { + bail!("failed to find export of `{}` in module", name) }; + let ty = func.ty(); if ty.params().len() > 0 { eprintln!( "warning: using `--invoke` with a function that takes arguments \ @@ -288,7 +285,7 @@ impl RunCommand { // Invoke the function and then afterwards print all the results that came // out, if there are any. - let results = export + let results = func .call(&values) .with_context(|| format!("failed to invoke `{}`", name))?; if !results.is_empty() { diff --git a/tests/all/custom_signal_handler.rs b/tests/all/custom_signal_handler.rs index 27d14fc910a5..685c740372a2 100644 --- a/tests/all/custom_signal_handler.rs +++ b/tests/all/custom_signal_handler.rs @@ -105,8 +105,7 @@ mod tests { }); } - let exports = instance.exports(); - assert!(!exports.is_empty()); + let mut exports = instance.exports(); // these invoke wasmtime_call_trampoline from action.rs { @@ -130,7 +129,9 @@ mod tests { // these invoke wasmtime_call_trampoline from callable.rs { - let read_func = exports[0] + let read_func = exports + .next() + .unwrap() .func() .expect("expected a 'read' func in the module"); println!("calling read..."); @@ -139,7 +140,9 @@ mod tests { } { - let read_out_of_bounds_func = exports[1] + let read_out_of_bounds_func = exports + .next() + .unwrap() .func() .expect("expected a 'read_out_of_bounds' func in the module"); println!("calling read_out_of_bounds..."); @@ -216,8 +219,8 @@ mod tests { // First instance1 { - let exports1 = instance1.exports(); - assert!(!exports1.is_empty()); + let mut exports1 = instance1.exports(); + assert!(exports1.next().is_some()); println!("calling instance1.read..."); let result = invoke_export(&instance1, "read").expect("read succeeded"); @@ -231,8 +234,8 @@ mod tests { // And then instance2 { - let exports2 = instance2.exports(); - assert!(!exports2.is_empty()); + let mut exports2 = instance2.exports(); + assert!(exports2.next().is_some()); println!("calling instance2.read..."); let result = invoke_export(&instance2, "read").expect("read succeeded"); @@ -262,11 +265,10 @@ mod tests { }); } - let instance1_exports = instance1.exports(); - assert!(!instance1_exports.is_empty()); - let instance1_read = instance1_exports[0].clone(); + let mut instance1_exports = instance1.exports(); + let instance1_read = instance1_exports.next().unwrap(); - // instance2 wich calls 'instance1.read' + // instance2 which calls 'instance1.read' let module2 = Module::new(&store, WAT2)?; let instance2 = Instance::new(&module2, &[instance1_read])?; // since 'instance2.run' calls 'instance1.read' we need to set up the signal handler to handle diff --git a/tests/all/func.rs b/tests/all/func.rs index 431de864536e..4914e6e7ccee 100644 --- a/tests/all/func.rs +++ b/tests/all/func.rs @@ -207,33 +207,33 @@ fn trap_import() -> Result<()> { fn get_from_wrapper() { let store = Store::default(); let f = Func::wrap(&store, || {}); - assert!(f.get0::<()>().is_ok()); - assert!(f.get0::().is_err()); - assert!(f.get1::<(), ()>().is_ok()); - assert!(f.get1::().is_err()); - assert!(f.get1::().is_err()); - assert!(f.get2::<(), (), ()>().is_ok()); - assert!(f.get2::().is_err()); - assert!(f.get2::().is_err()); + assert!(f.clone().get0::<()>().is_ok()); + assert!(f.clone().get0::().is_err()); + assert!(f.clone().get1::<(), ()>().is_ok()); + assert!(f.clone().get1::().is_err()); + assert!(f.clone().get1::().is_err()); + assert!(f.clone().get2::<(), (), ()>().is_ok()); + assert!(f.clone().get2::().is_err()); + assert!(f.clone().get2::().is_err()); let f = Func::wrap(&store, || -> i32 { loop {} }); - assert!(f.get0::().is_ok()); + assert!(f.clone().get0::().is_ok()); let f = Func::wrap(&store, || -> f32 { loop {} }); - assert!(f.get0::().is_ok()); + assert!(f.clone().get0::().is_ok()); let f = Func::wrap(&store, || -> f64 { loop {} }); - assert!(f.get0::().is_ok()); + assert!(f.clone().get0::().is_ok()); let f = Func::wrap(&store, |_: i32| {}); - assert!(f.get1::().is_ok()); - assert!(f.get1::().is_err()); - assert!(f.get1::().is_err()); - assert!(f.get1::().is_err()); + assert!(f.clone().get1::().is_ok()); + assert!(f.clone().get1::().is_err()); + assert!(f.clone().get1::().is_err()); + assert!(f.clone().get1::().is_err()); let f = Func::wrap(&store, |_: i64| {}); - assert!(f.get1::().is_ok()); + assert!(f.clone().get1::().is_ok()); let f = Func::wrap(&store, |_: f32| {}); - assert!(f.get1::().is_ok()); + assert!(f.clone().get1::().is_ok()); let f = Func::wrap(&store, |_: f64| {}); - assert!(f.get1::().is_ok()); + assert!(f.clone().get1::().is_ok()); } #[test] @@ -241,16 +241,16 @@ fn get_from_signature() { let store = Store::default(); let ty = FuncType::new(Box::new([]), Box::new([])); let f = Func::new(&store, ty, |_, _, _| panic!()); - assert!(f.get0::<()>().is_ok()); - assert!(f.get0::().is_err()); - assert!(f.get1::().is_err()); + assert!(f.clone().get0::<()>().is_ok()); + assert!(f.clone().get0::().is_err()); + assert!(f.clone().get1::().is_err()); let ty = FuncType::new(Box::new([ValType::I32]), Box::new([ValType::F64])); let f = Func::new(&store, ty, |_, _, _| panic!()); - assert!(f.get0::<()>().is_err()); - assert!(f.get0::().is_err()); - assert!(f.get1::().is_err()); - assert!(f.get1::().is_ok()); + assert!(f.clone().get0::<()>().is_err()); + assert!(f.clone().get0::().is_err()); + assert!(f.clone().get1::().is_err()); + assert!(f.clone().get1::().is_ok()); } #[test] @@ -270,17 +270,17 @@ fn get_from_module() -> anyhow::Result<()> { )?; let instance = Instance::new(&module, &[])?; let f0 = instance.get_export("f0").unwrap().func().unwrap(); - assert!(f0.get0::<()>().is_ok()); - assert!(f0.get0::().is_err()); + assert!(f0.clone().get0::<()>().is_ok()); + assert!(f0.clone().get0::().is_err()); let f1 = instance.get_export("f1").unwrap().func().unwrap(); - assert!(f1.get0::<()>().is_err()); - assert!(f1.get1::().is_ok()); - assert!(f1.get1::().is_err()); + assert!(f1.clone().get0::<()>().is_err()); + assert!(f1.clone().get1::().is_ok()); + assert!(f1.clone().get1::().is_err()); let f2 = instance.get_export("f2").unwrap().func().unwrap(); - assert!(f2.get0::<()>().is_err()); - assert!(f2.get0::().is_ok()); - assert!(f2.get1::().is_err()); - assert!(f2.get1::().is_err()); + assert!(f2.clone().get0::<()>().is_err()); + assert!(f2.clone().get0::().is_ok()); + assert!(f2.clone().get1::().is_err()); + assert!(f2.clone().get1::().is_err()); Ok(()) } diff --git a/tests/all/globals.rs b/tests/all/globals.rs index 8f5406d3984d..c8350eb1c1ed 100644 --- a/tests/all/globals.rs +++ b/tests/all/globals.rs @@ -70,7 +70,7 @@ fn use_after_drop() -> anyhow::Result<()> { "#, )?; let instance = Instance::new(&module, &[])?; - let g = instance.exports()[0].global().unwrap().clone(); + let g = instance.exports().next().unwrap().global().unwrap(); assert_eq!(g.get().i32(), Some(100)); g.set(101.into())?; drop(instance); diff --git a/tests/all/import_calling_export.rs b/tests/all/import_calling_export.rs index 44db6f39f374..001ff6a92344 100644 --- a/tests/all/import_calling_export.rs +++ b/tests/all/import_calling_export.rs @@ -40,18 +40,20 @@ fn test_import_calling_export() { let instance = Instance::new(&module, imports.as_slice()).expect("failed to instantiate module"); - let exports = instance.exports(); - assert!(!exports.is_empty()); + let mut exports = instance.exports(); - let run_func = exports[0] + let run_func = exports + .next() + .unwrap() .func() .expect("expected a run func in the module"); *other.borrow_mut() = Some( - exports[1] + exports + .next() + .unwrap() .func() - .expect("expected an other func in the module") - .clone(), + .expect("expected an other func in the module"), ); run_func.call(&[]).expect("expected function not to trap"); @@ -84,10 +86,11 @@ fn test_returns_incorrect_type() -> Result<()> { let imports = vec![callback_func.into()]; let instance = Instance::new(&module, imports.as_slice())?; - let exports = instance.exports(); - assert!(!exports.is_empty()); + let mut exports = instance.exports(); - let run_func = exports[0] + let run_func = exports + .next() + .unwrap() .func() .expect("expected a run func in the module"); diff --git a/tests/all/traps.rs b/tests/all/traps.rs index 755fa8120368..bd08537169d9 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -17,7 +17,10 @@ fn test_trap_return() -> Result<()> { let hello_func = Func::new(&store, hello_type, |_, _, _| Err(Trap::new("test 123"))); let instance = Instance::new(&module, &[hello_func.into()])?; - let run_func = instance.exports()[0] + let run_func = instance + .exports() + .next() + .unwrap() .func() .expect("expected function export"); @@ -44,7 +47,10 @@ fn test_trap_trace() -> Result<()> { let module = Module::new(&store, wat)?; let instance = Instance::new(&module, &[])?; - let run_func = instance.exports()[0] + let run_func = instance + .exports() + .next() + .unwrap() .func() .expect("expected function export"); @@ -91,7 +97,10 @@ fn test_trap_trace_cb() -> Result<()> { let module = Module::new(&store, wat)?; let instance = Instance::new(&module, &[fn_func.into()])?; - let run_func = instance.exports()[0] + let run_func = instance + .exports() + .next() + .unwrap() .func() .expect("expected function export"); @@ -123,7 +132,10 @@ fn test_trap_stack_overflow() -> Result<()> { let module = Module::new(&store, wat)?; let instance = Instance::new(&module, &[])?; - let run_func = instance.exports()[0] + let run_func = instance + .exports() + .next() + .unwrap() .func() .expect("expected function export"); @@ -159,7 +171,10 @@ fn trap_display_pretty() -> Result<()> { let module = Module::new(&store, wat)?; let instance = Instance::new(&module, &[])?; - let run_func = instance.exports()[0] + let run_func = instance + .exports() + .next() + .unwrap() .func() .expect("expected function export"); @@ -192,7 +207,7 @@ fn trap_display_multi_module() -> Result<()> { let module = Module::new(&store, wat)?; let instance = Instance::new(&module, &[])?; - let bar = instance.exports()[0].clone(); + let bar = instance.exports().next().unwrap(); let wat = r#" (module $b @@ -203,7 +218,10 @@ fn trap_display_multi_module() -> Result<()> { "#; let module = Module::new(&store, wat)?; let instance = Instance::new(&module, &[bar])?; - let bar2 = instance.exports()[0] + let bar2 = instance + .exports() + .next() + .unwrap() .func() .expect("expected function export"); @@ -268,14 +286,14 @@ fn rust_panic_import() -> Result<()> { Func::wrap(&store, || panic!("this is another panic")).into(), ], )?; - let func = instance.exports()[0].func().unwrap().clone(); + let func = instance.exports().next().unwrap().func().unwrap(); let err = panic::catch_unwind(AssertUnwindSafe(|| { drop(func.call(&[])); })) .unwrap_err(); assert_eq!(err.downcast_ref::<&'static str>(), Some(&"this is a panic")); - let func = instance.exports()[1].func().unwrap().clone(); + let func = instance.exports().nth(1).unwrap().func().unwrap(); let err = panic::catch_unwind(AssertUnwindSafe(|| { drop(func.call(&[])); })) @@ -333,7 +351,7 @@ fn mismatched_arguments() -> Result<()> { let module = Module::new(&store, &binary)?; let instance = Instance::new(&module, &[])?; - let func = instance.exports()[0].func().unwrap().clone(); + let func = instance.exports().next().unwrap().func().unwrap(); assert_eq!( func.call(&[]).unwrap_err().to_string(), "expected 1 arguments, got 0" From 725355d07dab4275990e4695ff76abe0afe998e3 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 15 Apr 2020 14:49:45 -0700 Subject: [PATCH 02/24] Compute module imports and exports on demand too. And compute Extern::ty on demand too. --- crates/api/src/externals.rs | 65 ++-- crates/api/src/func.rs | 68 ++-- crates/api/src/instance.rs | 4 +- crates/api/src/linker.rs | 44 +-- crates/api/src/module.rs | 302 ++++++------------ crates/api/src/types.rs | 2 +- crates/c-api/src/module.rs | 2 - crates/c-api/src/wasi.rs | 2 +- crates/environ/src/module.rs | 19 +- crates/environ/src/module_environ.rs | 103 +++--- crates/fuzzing/src/oracles/dummy.rs | 31 +- crates/jit/src/imports.rs | 231 +++++++------- crates/obj/src/context.rs | 4 +- crates/obj/src/function.rs | 4 +- crates/runtime/src/instance.rs | 10 +- .../test-programs/tests/wasm_tests/runtime.rs | 1 - crates/wast/src/wast.rs | 2 +- src/commands/run.rs | 7 +- tests/misc_testsuite/threads.wast | 2 +- 19 files changed, 392 insertions(+), 511 deletions(-) diff --git a/crates/api/src/externals.rs b/crates/api/src/externals.rs index ea3a623c1dfa..83997c4e7d9d 100644 --- a/crates/api/src/externals.rs +++ b/crates/api/src/externals.rs @@ -74,10 +74,10 @@ impl Extern { /// Returns the type associated with this `Extern`. pub fn ty(&self) -> ExternType { match self { - Extern::Func(ft) => ExternType::Func(ft.ty().clone()), - Extern::Memory(ft) => ExternType::Memory(ft.ty().clone()), - Extern::Table(tt) => ExternType::Table(tt.ty().clone()), - Extern::Global(gt) => ExternType::Global(gt.ty().clone()), + Extern::Func(ft) => ExternType::Func(ft.ty()), + Extern::Memory(ft) => ExternType::Memory(ft.ty()), + Extern::Table(tt) => ExternType::Table(tt.ty()), + Extern::Global(gt) => ExternType::Global(gt.ty()), } } @@ -164,7 +164,6 @@ impl From
for Extern { #[derive(Clone)] pub struct Global { store: Store, - ty: GlobalType, wasmtime_export: wasmtime_runtime::ExportGlobal, wasmtime_handle: InstanceHandle, } @@ -191,27 +190,40 @@ impl Global { let (wasmtime_handle, wasmtime_export) = generate_global_export(store, &ty, val)?; Ok(Global { store: store.clone(), - ty, wasmtime_export, wasmtime_handle, }) } /// Returns the underlying type of this `global`. - pub fn ty(&self) -> &GlobalType { - &self.ty + pub fn ty(&self) -> GlobalType { + // The original export is coming from wasmtime_runtime itself we should + // support all the types coming out of it, so assert such here. + GlobalType::from_wasmtime_global(&self.wasmtime_export.global) + .expect("core wasm global type should be supported") + } + + /// Returns the underlying mutability of this `global`. + pub fn mutability(&self) -> Mutability { + if self.wasmtime_export.global.mutability { + Mutability::Var + } else { + Mutability::Const + } } /// Returns the current [`Val`] of this global. pub fn get(&self) -> Val { unsafe { let definition = &mut *self.wasmtime_export.definition; - match self.ty().content() { + let ty = ValType::from_wasmtime_type(self.wasmtime_export.global.ty) + .expect("core wasm type should be supported"); + match ty { ValType::I32 => Val::from(*definition.as_i32()), ValType::I64 => Val::from(*definition.as_i64()), ValType::F32 => Val::F32(*definition.as_u32()), ValType::F64 => Val::F64(*definition.as_u64()), - _ => unimplemented!("Global::get for {:?}", self.ty().content()), + _ => unimplemented!("Global::get for {:?}", ty), } } } @@ -223,15 +235,13 @@ impl Global { /// Returns an error if this global has a different type than `Val`, or if /// it's not a mutable global. pub fn set(&self, val: Val) -> Result<()> { - if self.ty().mutability() != Mutability::Var { + if self.mutability() != Mutability::Var { bail!("immutable global cannot be set"); } - if val.ty() != *self.ty().content() { - bail!( - "global of type {:?} cannot be set to {:?}", - self.ty().content(), - val.ty() - ); + let ty = ValType::from_wasmtime_type(self.wasmtime_export.global.ty) + .expect("core wasm type should be supported"); + if val.ty() != ty { + bail!("global of type {:?} cannot be set to {:?}", ty, val.ty()); } if !val.comes_from_same_store(&self.store) { bail!("cross-`Store` values are not supported"); @@ -254,13 +264,8 @@ impl Global { store: &Store, wasmtime_handle: InstanceHandle, ) -> Global { - // The original export is coming from wasmtime_runtime itself we should - // support all the types coming out of it, so assert such here. - let ty = GlobalType::from_wasmtime_global(&wasmtime_export.global) - .expect("core wasm global type should be supported"); Global { store: store.clone(), - ty: ty, wasmtime_export, wasmtime_handle, } @@ -285,7 +290,6 @@ impl Global { #[derive(Clone)] pub struct Table { store: Store, - ty: TableType, wasmtime_handle: InstanceHandle, wasmtime_export: wasmtime_runtime::ExportTable, } @@ -326,7 +330,6 @@ impl Table { Ok(Table { store: store.clone(), - ty, wasmtime_handle, wasmtime_export, }) @@ -334,8 +337,8 @@ impl Table { /// Returns the underlying type of this table, including its element type as /// well as the maximum/minimum lower bounds. - pub fn ty(&self) -> &TableType { - &self.ty + pub fn ty(&self) -> TableType { + TableType::from_wasmtime_table(&self.wasmtime_export.table.table) } fn wasmtime_table_index(&self) -> wasm::DefinedTableIndex { @@ -432,10 +435,8 @@ impl Table { store: &Store, wasmtime_handle: wasmtime_runtime::InstanceHandle, ) -> Table { - let ty = TableType::from_wasmtime_table(&wasmtime_export.table.table); Table { store: store.clone(), - ty, wasmtime_handle, wasmtime_export, } @@ -651,7 +652,6 @@ impl Table { #[derive(Clone)] pub struct Memory { store: Store, - ty: MemoryType, wasmtime_handle: InstanceHandle, wasmtime_export: wasmtime_runtime::ExportMemory, } @@ -684,7 +684,6 @@ impl Memory { generate_memory_export(store, &ty).expect("generated memory"); Memory { store: store.clone(), - ty, wasmtime_handle, wasmtime_export, } @@ -706,8 +705,8 @@ impl Memory { /// # Ok(()) /// # } /// ``` - pub fn ty(&self) -> &MemoryType { - &self.ty + pub fn ty(&self) -> MemoryType { + MemoryType::from_wasmtime_memory(&self.wasmtime_export.memory.memory) } /// Returns this memory as a slice view that can be read natively in Rust. @@ -838,10 +837,8 @@ impl Memory { store: &Store, wasmtime_handle: wasmtime_runtime::InstanceHandle, ) -> Memory { - let ty = MemoryType::from_wasmtime_memory(&wasmtime_export.memory.memory); Memory { store: store.clone(), - ty: ty, wasmtime_handle, wasmtime_export, } diff --git a/crates/api/src/func.rs b/crates/api/src/func.rs index 6635fa3b4a31..89a1a2fd4bd6 100644 --- a/crates/api/src/func.rs +++ b/crates/api/src/func.rs @@ -138,7 +138,6 @@ pub struct Func { store: Store, instance: InstanceHandle, export: ExportFunction, - ty: FuncType, trampoline: VMTrampoline, } @@ -157,7 +156,8 @@ macro_rules! getters { { // Verify all the paramers match the expected parameters, and that // there are no extra parameters... - let mut params = self.ty().params().iter().cloned(); + let ty = self.ty(); + let mut params = ty.params().iter().cloned(); let n = 0; $( let n = n + 1; @@ -167,7 +167,7 @@ macro_rules! getters { ensure!(params.next().is_none(), "Type mismatch: too many arguments (expected {})", n); // ... then do the same for the results... - let mut results = self.ty().results().iter().cloned(); + let mut results = ty.results().iter().cloned(); R::matches(&mut results) .context("Type mismatch in return type")?; ensure!(results.next().is_none(), "Type mismatch: too many return values (expected 1)"); @@ -271,7 +271,6 @@ impl Func { crate::trampoline::generate_func_export(&ty, func, store).expect("generated func"); Func { store: store.clone(), - ty, instance, export, trampoline, @@ -469,18 +468,42 @@ impl Func { } /// Returns the underlying wasm type that this `Func` has. - pub fn ty(&self) -> &FuncType { - &self.ty + pub fn ty(&self) -> FuncType { + // Signatures should always be registered in the store's registry of + // shared signatures, so we should be able to unwrap safely here. + let sig = self + .store + .compiler() + .signatures() + .lookup(self.export.signature) + .expect("failed to lookup signature"); + + // This is only called with `Export::Function`, and since it's coming + // from wasmtime_runtime itself we should support all the types coming + // out of it, so assert such here. + FuncType::from_wasmtime_signature(&sig).expect("core wasm signature should be supported") } /// Returns the number of parameters that this function takes. pub fn param_arity(&self) -> usize { - self.ty.params().len() + let sig = self + .store + .compiler() + .signatures() + .lookup(self.export.signature) + .expect("failed to lookup signature"); + sig.params.len() } /// Returns the number of results this function produces. pub fn result_arity(&self) -> usize { - self.ty.results().len() + let sig = self + .store + .compiler() + .signatures() + .lookup(self.export.signature) + .expect("failed to lookup signature"); + sig.returns.len() } /// Invokes this function with the `params` given, returning the results and @@ -498,18 +521,19 @@ impl Func { // this function. This involves checking to make sure we have the right // number and types of arguments as well as making sure everything is // from the same `Store`. - if self.ty.params().len() != params.len() { + let my_ty = self.ty(); + if my_ty.params().len() != params.len() { bail!( "expected {} arguments, got {}", - self.ty.params().len(), + my_ty.params().len(), params.len() ); } - let mut values_vec = vec![0; max(params.len(), self.ty.results().len())]; + let mut values_vec = vec![0; max(params.len(), my_ty.results().len())]; // Store the argument values into `values_vec`. - let param_tys = self.ty.params().iter(); + let param_tys = my_ty.params().iter(); for ((arg, slot), ty) in params.iter().zip(&mut values_vec).zip(param_tys) { if arg.ty() != *ty { bail!("argument type mismatch"); @@ -537,8 +561,8 @@ impl Func { } // Load the return values out of `values_vec`. - let mut results = Vec::with_capacity(self.ty.results().len()); - for (index, ty) in self.ty.results().iter().enumerate() { + let mut results = Vec::with_capacity(my_ty.results().len()); + for (index, ty) in my_ty.results().iter().enumerate() { unsafe { let ptr = values_vec.as_ptr().add(index); results.push(Val::read_value_from(ptr, ty)); @@ -557,20 +581,6 @@ impl Func { store: &Store, instance: InstanceHandle, ) -> Self { - // Signatures should always be registered in the store's registry of - // shared signatures, so we should be able to unwrap safely here. - let sig = store - .compiler() - .signatures() - .lookup(export.signature) - .expect("failed to lookup signature"); - - // This is only called with `Export::Function`, and since it's coming - // from wasmtime_runtime itself we should support all the types coming - // out of it, so assert such here. - let ty = FuncType::from_wasmtime_signature(sig) - .expect("core wasm signature should be supported"); - // Each function signature in a module should have a trampoline stored // on that module as well, so unwrap the result here since otherwise // it's a bug in wasmtime. @@ -582,7 +592,6 @@ impl Func { instance, export, trampoline, - ty, store: store.clone(), } } @@ -1094,7 +1103,6 @@ macro_rules! impl_into_func { .expect("failed to generate export"); Func { store: store.clone(), - ty, instance, export, trampoline, diff --git a/crates/api/src/instance.rs b/crates/api/src/instance.rs index c93e484c7a88..2e699d1421fd 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/src/instance.rs @@ -126,11 +126,11 @@ impl Instance { } } - if imports.len() != module.imports().len() { + if imports.len() != module.num_imports() { bail!( "wrong number of imports provided, {} != {}", imports.len(), - module.imports().len() + module.num_imports() ); } diff --git a/crates/api/src/linker.rs b/crates/api/src/linker.rs index 694c2dc21b75..311d8b132655 100644 --- a/crates/api/src/linker.rs +++ b/crates/api/src/linker.rs @@ -166,7 +166,7 @@ impl Linker { if !item.comes_from_same_store(&self.store) { bail!("all linker items must be from the same store"); } - self.insert(module, name, &item.ty(), item)?; + self.insert(module, name, item.ty(), item)?; Ok(self) } @@ -264,8 +264,8 @@ impl Linker { if !Store::same(&self.store, instance.store()) { bail!("all linker items must be from the same store"); } - for (export, item) in instance.module().exports().iter().zip(instance.exports()) { - self.insert(module_name, export.name(), export.ty(), item.clone())?; + for (export, item) in instance.module().exports().zip(instance.exports()) { + self.insert(module_name, export.name(), export.ty().clone(), item)?; } Ok(self) } @@ -291,7 +291,7 @@ impl Linker { Ok(()) } - fn insert(&mut self, module: &str, name: &str, ty: &ExternType, item: Extern) -> Result<()> { + fn insert(&mut self, module: &str, name: &str, ty: ExternType, item: Extern) -> Result<()> { let key = self.import_key(module, name, ty); match self.map.entry(key) { Entry::Occupied(o) if !self.allow_shadowing => bail!( @@ -310,7 +310,7 @@ impl Linker { Ok(()) } - fn import_key(&mut self, module: &str, name: &str, ty: &ExternType) -> ImportKey { + fn import_key(&mut self, module: &str, name: &str, ty: ExternType) -> ImportKey { ImportKey { module: self.intern_str(module), name: self.intern_str(name), @@ -318,10 +318,10 @@ impl Linker { } } - fn import_kind(&self, ty: &ExternType) -> ImportKind { + fn import_kind(&self, ty: ExternType) -> ImportKind { match ty { - ExternType::Func(f) => ImportKind::Func(f.clone()), - ExternType::Global(f) => ImportKind::Global(f.clone()), + ExternType::Func(f) => ImportKind::Func(f), + ExternType::Global(f) => ImportKind::Global(f), ExternType::Memory(_) => ImportKind::Memory, ExternType::Table(_) => ImportKind::Table, } @@ -378,8 +378,8 @@ impl Linker { pub fn instantiate(&self, module: &Module) -> Result { let mut imports = Vec::new(); for import in module.imports() { - if let Some(item) = self.get(import) { - imports.push(item.clone()); + if let Some(item) = self.get(&import) { + imports.push(item); continue; } @@ -429,23 +429,27 @@ impl Linker { /// /// Note that multiple `Extern` items may be defined for the same /// module/name pair. - pub fn iter(&self) -> impl Iterator { - self.map - .iter() - .map(move |(key, item)| (&*self.strings[key.module], &*self.strings[key.name], item)) + pub fn iter(&self) -> impl Iterator { + self.map.iter().map(move |(key, item)| { + ( + &*self.strings[key.module], + &*self.strings[key.name], + item.clone(), + ) + }) } /// Looks up a value in this `Linker` which matches the `import` type /// provided. /// /// Returns `None` if no match was found. - pub fn get(&self, import: &ImportType) -> Option<&Extern> { + pub fn get(&self, import: &ImportType) -> Option { let key = ImportKey { module: *self.string2idx.get(import.module())?, name: *self.string2idx.get(import.name())?, - kind: self.import_kind(import.ty()), + kind: self.import_kind(import.ty().clone()), }; - self.map.get(&key) + self.map.get(&key).cloned() } /// Returns all items defined for the `module` and `name` pair. @@ -468,10 +472,10 @@ impl Linker { /// Returns the single item defined for the `module` and `name` pair. /// /// Unlike the similar [`Linker::get_by_name`] method this function returns - /// a single `&Extern` item. If the `module` and `name` pair isn't defined + /// a single `Extern` item. If the `module` and `name` pair isn't defined /// in this linker then an error is returned. If more than one value exists /// for the `module` and `name` pairs, then an error is returned as well. - pub fn get_one_by_name(&self, module: &str, name: &str) -> Result<&Extern> { + pub fn get_one_by_name(&self, module: &str, name: &str) -> Result { let mut items = self.get_by_name(module, name); let ret = items .next() @@ -479,6 +483,6 @@ impl Linker { if items.next().is_some() { bail!("too many items named `{}` in `{}`", name, module); } - Ok(ret) + Ok(ret.clone()) } } diff --git a/crates/api/src/module.rs b/crates/api/src/module.rs index 1060167a9db7..d46b23b262cd 100644 --- a/crates/api/src/module.rs +++ b/crates/api/src/module.rs @@ -1,66 +1,14 @@ use crate::frame_info::GlobalFrameInfoRegistration; use crate::runtime::Store; use crate::types::{ - ExportType, ExternType, FuncType, GlobalType, ImportType, Limits, MemoryType, Mutability, - TableType, ValType, + ExportType, ExternType, FuncType, GlobalType, ImportType, MemoryType, TableType, }; -use anyhow::{bail, Error, Result}; +use anyhow::{Error, Result}; use std::path::Path; use std::sync::{Arc, Mutex}; -use wasmparser::{validate, ExternalKind, ImportSectionEntryType, ModuleReader, SectionCode}; +use wasmparser::validate; use wasmtime_jit::CompiledModule; -fn into_memory_type(mt: wasmparser::MemoryType) -> Result { - if mt.shared { - bail!("shared memories are not supported yet"); - } - Ok(MemoryType::new(Limits::new( - mt.limits.initial, - mt.limits.maximum, - ))) -} - -fn into_global_type(gt: wasmparser::GlobalType) -> GlobalType { - let mutability = if gt.mutable { - Mutability::Var - } else { - Mutability::Const - }; - GlobalType::new(into_valtype(>.content_type), mutability) -} - -// `into_valtype` is used for `map` which requires `&T`. -#[allow(clippy::trivially_copy_pass_by_ref)] -fn into_valtype(ty: &wasmparser::Type) -> ValType { - use wasmparser::Type::*; - match ty { - I32 => ValType::I32, - I64 => ValType::I64, - F32 => ValType::F32, - F64 => ValType::F64, - V128 => ValType::V128, - AnyFunc => ValType::FuncRef, - AnyRef => ValType::AnyRef, - _ => unimplemented!("types in into_valtype"), - } -} - -fn into_func_type(mt: wasmparser::FuncType) -> FuncType { - assert_eq!(mt.form, wasmparser::Type::Func); - let params = mt.params.iter().map(into_valtype).collect::>(); - let returns = mt.returns.iter().map(into_valtype).collect::>(); - FuncType::new(params.into_boxed_slice(), returns.into_boxed_slice()) -} - -fn into_table_type(tt: wasmparser::TableType) -> TableType { - assert!( - tt.element_type == wasmparser::Type::AnyFunc || tt.element_type == wasmparser::Type::AnyRef - ); - let ty = into_valtype(&tt.element_type); - let limits = Limits::new(tt.limits.initial, tt.limits.maximum); - TableType::new(ty, limits) -} - /// A compiled WebAssembly module, ready to be instantiated. /// /// A `Module` is a compiled in-memory representation of an input WebAssembly @@ -134,8 +82,6 @@ pub struct Module { struct ModuleInner { store: Store, - imports: Box<[ImportType]>, - exports: Box<[ExportType]>, compiled: CompiledModule, frame_info_registration: Mutex>>>, } @@ -332,9 +278,7 @@ impl Module { /// be somewhat valid for decoding purposes, and the basics of decoding can /// still fail. pub unsafe fn from_binary_unchecked(store: &Store, binary: &[u8]) -> Result { - let mut ret = Module::compile(store, binary)?; - ret.read_imports_and_exports(binary)?; - Ok(ret) + Module::compile(store, binary) } /// Validates `binary` input data as a WebAssembly binary given the @@ -372,8 +316,6 @@ impl Module { Ok(Module { inner: Arc::new(ModuleInner { store: store.clone(), - imports: Box::new([]), - exports: Box::new([]), compiled, frame_info_registration: Mutex::new(None), }), @@ -433,7 +375,7 @@ impl Module { /// # fn main() -> anyhow::Result<()> { /// # let store = Store::default(); /// let module = Module::new(&store, "(module)")?; - /// assert_eq!(module.imports().len(), 0); + /// assert_eq!(module.num_imports(), 0); /// # Ok(()) /// # } /// ``` @@ -450,8 +392,8 @@ impl Module { /// ) /// "#; /// let module = Module::new(&store, wat)?; - /// assert_eq!(module.imports().len(), 1); - /// let import = &module.imports()[0]; + /// assert_eq!(module.num_imports(), 1); + /// let import = module.imports().next().unwrap(); /// assert_eq!(import.module(), "host"); /// assert_eq!(import.name(), "foo"); /// match import.ty() { @@ -461,8 +403,49 @@ impl Module { /// # Ok(()) /// # } /// ``` - pub fn imports(&self) -> &[ImportType] { - &self.inner.imports + pub fn imports<'me>(&'me self) -> impl Iterator + 'me { + let inner = self.inner.clone(); + self.inner.compiled.module_ref().imports.iter().map( + move |(module_name, field_name, import)| { + let module = inner.compiled.module_ref(); + match import { + wasmtime_environ::Export::Function(func_index) => { + let sig_index = module.local.functions[*func_index]; + let sig = &module.local.signatures[sig_index]; + let ty = FuncType::from_wasmtime_signature(sig) + .expect("core wasm function type should be supported") + .into(); + ImportType::new(module_name, field_name, ty) + } + wasmtime_environ::Export::Table(table_index) => { + let ty = TableType::from_wasmtime_table( + &module.local.table_plans[*table_index].table, + ) + .into(); + ImportType::new(module_name, field_name, ty) + } + wasmtime_environ::Export::Memory(memory_index) => { + let ty = MemoryType::from_wasmtime_memory( + &module.local.memory_plans[*memory_index].memory, + ) + .into(); + ImportType::new(module_name, field_name, ty) + } + wasmtime_environ::Export::Global(global_index) => { + let ty = + GlobalType::from_wasmtime_global(&module.local.globals[*global_index]) + .expect("core wasm global type should be supported") + .into(); + ImportType::new(module_name, field_name, ty) + } + } + }, + ) + } + + /// Return the number of imports in this module. + pub fn num_imports(&self) -> usize { + self.inner.compiled.module_ref().imports.len() } /// Returns the list of exports that this [`Module`] has and will be @@ -482,7 +465,7 @@ impl Module { /// # fn main() -> anyhow::Result<()> { /// # let store = Store::default(); /// let module = Module::new(&store, "(module)")?; - /// assert!(module.exports().is_empty()); + /// assert!(module.exports().next().is_none()); /// # Ok(()) /// # } /// ``` @@ -500,16 +483,16 @@ impl Module { /// ) /// "#; /// let module = Module::new(&store, wat)?; - /// assert_eq!(module.exports().len(), 2); + /// assert_eq!(module.num_exports(), 2); /// - /// let foo = &module.exports()[0]; + /// let foo = module.exports().next().unwrap(); /// assert_eq!(foo.name(), "foo"); /// match foo.ty() { /// ExternType::Func(_) => { /* ... */ } /// _ => panic!("unexpected export type!"), /// } /// - /// let memory = &module.exports()[1]; + /// let memory = module.exports().nth(1).unwrap(); /// assert_eq!(memory.name(), "memory"); /// match memory.ty() { /// ExternType::Memory(_) => { /* ... */ } @@ -518,148 +501,51 @@ impl Module { /// # Ok(()) /// # } /// ``` - pub fn exports(&self) -> &[ExportType] { - &self.inner.exports - } - - /// Returns the [`Store`] that this [`Module`] was compiled into. - pub fn store(&self) -> &Store { - &self.inner.store - } - - fn read_imports_and_exports(&mut self, binary: &[u8]) -> Result<()> { - let inner = Arc::get_mut(&mut self.inner).unwrap(); - let mut reader = ModuleReader::new(binary)?; - let mut imports = Vec::new(); - let mut exports = Vec::new(); - let mut memories = Vec::new(); - let mut tables = Vec::new(); - let mut func_sig = Vec::new(); - let mut sigs = Vec::new(); - let mut globals = Vec::new(); - while !reader.eof() { - let section = reader.read()?; - match section.code { - SectionCode::Memory => { - let section = section.get_memory_section_reader()?; - memories.reserve_exact(section.get_count() as usize); - for entry in section { - memories.push(into_memory_type(entry?)?); - } - } - SectionCode::Type => { - let section = section.get_type_section_reader()?; - sigs.reserve_exact(section.get_count() as usize); - for entry in section { - sigs.push(into_func_type(entry?)); - } - } - SectionCode::Function => { - let section = section.get_function_section_reader()?; - func_sig.reserve_exact(section.get_count() as usize); - for entry in section { - func_sig.push(entry?); - } - } - SectionCode::Global => { - let section = section.get_global_section_reader()?; - globals.reserve_exact(section.get_count() as usize); - for entry in section { - globals.push(into_global_type(entry?.ty)); - } - } - SectionCode::Table => { - let section = section.get_table_section_reader()?; - tables.reserve_exact(section.get_count() as usize); - for entry in section { - tables.push(into_table_type(entry?)) + pub fn exports<'me>(&'me self) -> impl Iterator + 'me { + let inner = self.inner.clone(); + self.inner + .compiled + .module_ref() + .exports + .iter() + .map(move |(name, ty)| { + let module = inner.compiled.module_ref(); + let r#type = match ty { + wasmtime_environ::Export::Function(func_index) => { + let sig_index = module.local.functions[*func_index]; + let sig = &module.local.signatures[sig_index]; + ExternType::Func( + FuncType::from_wasmtime_signature(sig) + .expect("core wasm function type should be supported"), + ) } - } - SectionCode::Import => { - let section = section.get_import_section_reader()?; - imports.reserve_exact(section.get_count() as usize); - for entry in section { - let entry = entry?; - let r#type = match entry.ty { - ImportSectionEntryType::Function(index) => { - func_sig.push(index); - let sig = &sigs[index as usize]; - ExternType::Func(sig.clone()) - } - ImportSectionEntryType::Table(tt) => { - let table = into_table_type(tt); - tables.push(table.clone()); - ExternType::Table(table) - } - ImportSectionEntryType::Memory(mt) => { - let memory = into_memory_type(mt)?; - memories.push(memory.clone()); - ExternType::Memory(memory) - } - ImportSectionEntryType::Global(gt) => { - let global = into_global_type(gt); - globals.push(global.clone()); - ExternType::Global(global) - } - }; - imports.push(ImportType::new(entry.module, entry.field, r#type)); + wasmtime_environ::Export::Table(table_index) => { + ExternType::Table(TableType::from_wasmtime_table( + &module.local.table_plans[*table_index].table, + )) } - } - SectionCode::Export => { - let section = section.get_export_section_reader()?; - exports.reserve_exact(section.get_count() as usize); - for entry in section { - let entry = entry?; - let r#type = match entry.kind { - ExternalKind::Function => { - let sig_index = func_sig[entry.index as usize] as usize; - let sig = &sigs[sig_index]; - ExternType::Func(sig.clone()) - } - ExternalKind::Table => { - ExternType::Table(tables[entry.index as usize].clone()) - } - ExternalKind::Memory => { - ExternType::Memory(memories[entry.index as usize].clone()) - } - ExternalKind::Global => { - ExternType::Global(globals[entry.index as usize].clone()) - } - }; - exports.push(ExportType::new(entry.field, r#type)); + wasmtime_environ::Export::Memory(memory_index) => { + ExternType::Memory(MemoryType::from_wasmtime_memory( + &module.local.memory_plans[*memory_index].memory, + )) } - } - SectionCode::Custom { - name: "webidl-bindings", - .. - } - | SectionCode::Custom { - name: "wasm-interface-types", - .. - } => { - bail!( - "\ -support for interface types has temporarily been removed from `wasmtime` - -for more information about this temoprary you can read on the issue online: - - https://github.com/bytecodealliance/wasmtime/issues/1271 - -and for re-adding support for interface types you can see this issue: + wasmtime_environ::Export::Global(global_index) => ExternType::Global( + GlobalType::from_wasmtime_global(&module.local.globals[*global_index]) + .expect("core wasm global type should be supported"), + ), + }; + ExportType::new(name, r#type) + }) + } - https://github.com/bytecodealliance/wasmtime/issues/677 -" - ); - } - _ => { - // skip other sections - } - } - } + /// Return the number of exports in this module. + pub fn num_exports(&self) -> usize { + self.inner.compiled.module_ref().exports.len() + } - inner.imports = imports.into(); - inner.exports = exports.into(); - Ok(()) + /// Returns the [`Store`] that this [`Module`] was compiled into. + pub fn store(&self) -> &Store { + &self.inner.store } /// Register this module's stack frame information into the global scope. diff --git a/crates/api/src/types.rs b/crates/api/src/types.rs index 7d39c8047f8c..605d1be786f4 100644 --- a/crates/api/src/types.rs +++ b/crates/api/src/types.rs @@ -247,7 +247,7 @@ impl FuncType { /// Returns `None` if any types in the signature can't be converted to the /// types in this crate, but that should very rarely happen and largely only /// indicate a bug in our cranelift integration. - pub(crate) fn from_wasmtime_signature(signature: ir::Signature) -> Option { + pub(crate) fn from_wasmtime_signature(signature: &ir::Signature) -> Option { let params = signature .params .iter() diff --git a/crates/c-api/src/module.rs b/crates/c-api/src/module.rs index 4d1b2ec0c8fb..21aac85d2ac4 100644 --- a/crates/c-api/src/module.rs +++ b/crates/c-api/src/module.rs @@ -46,12 +46,10 @@ pub extern "C" fn wasmtime_module_new( handle_result(Module::from_binary(store, binary), |module| { let imports = module .imports() - .iter() .map(|i| wasm_importtype_t::new(i.clone())) .collect::>(); let exports = module .exports() - .iter() .map(|e| wasm_exporttype_t::new(e.clone())) .collect::>(); let module = Box::new(wasm_module_t { diff --git a/crates/c-api/src/wasi.rs b/crates/c-api/src/wasi.rs index 96b3798090db..8514beddbb06 100644 --- a/crates/c-api/src/wasi.rs +++ b/crates/c-api/src/wasi.rs @@ -331,7 +331,7 @@ pub extern "C" fn wasi_instance_bind_import<'a>( } }; - if export.ty() != import.ty.ty().func()? { + if &export.ty() != import.ty.ty().func()? { return None; } diff --git a/crates/environ/src/module.rs b/crates/environ/src/module.rs index 3264d4664ec2..eeb2880925dc 100644 --- a/crates/environ/src/module.rs +++ b/crates/environ/src/module.rs @@ -150,18 +150,8 @@ pub struct Module { /// function. pub local: ModuleLocal, - /// Names of imported functions, as well as the index of the import that - /// performed this import. - pub imported_funcs: PrimaryMap, - - /// Names of imported tables. - pub imported_tables: PrimaryMap, - - /// Names of imported memories. - pub imported_memories: PrimaryMap, - - /// Names of imported globals. - pub imported_globals: PrimaryMap, + /// All import records, in the order they are declared in the module. + pub imports: Vec<(String, String, Export)>, /// Exported entities. pub exports: IndexMap, @@ -226,10 +216,7 @@ impl Module { Self { id: NEXT_ID.fetch_add(1, SeqCst), name: None, - imported_funcs: PrimaryMap::new(), - imported_tables: PrimaryMap::new(), - imported_memories: PrimaryMap::new(), - imported_globals: PrimaryMap::new(), + imports: Vec::new(), exports: IndexMap::new(), start_func: None, table_elements: Vec::new(), diff --git a/crates/environ/src/module_environ.rs b/crates/environ/src/module_environ.rs index b4489e45f446..d2707a05cc2c 100644 --- a/crates/environ/src/module_environ.rs +++ b/crates/environ/src/module_environ.rs @@ -8,7 +8,7 @@ use cranelift_entity::PrimaryMap; use cranelift_wasm::{ self, translate_module, DataIndex, DefinedFuncIndex, ElemIndex, FuncIndex, Global, GlobalIndex, Memory, MemoryIndex, ModuleTranslationState, SignatureIndex, Table, TableIndex, - TargetEnvironment, WasmResult, + TargetEnvironment, WasmError, WasmResult, }; use std::convert::TryFrom; use std::sync::Arc; @@ -57,7 +57,6 @@ impl<'data> ModuleTranslation<'data> { pub struct ModuleEnvironment<'data> { /// The result to be filled in. result: ModuleTranslation<'data>, - imports: u32, } impl<'data> ModuleEnvironment<'data> { @@ -72,7 +71,6 @@ impl<'data> ModuleEnvironment<'data> { tunables: tunables.clone(), module_translation: None, }, - imports: 0, } } @@ -123,6 +121,14 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data Ok(()) } + fn reserve_imports(&mut self, num: u32) -> WasmResult<()> { + Ok(self + .result + .module + .imports + .reserve_exact(usize::try_from(num).unwrap())) + } + fn declare_func_import( &mut self, sig_index: SignatureIndex, @@ -131,37 +137,33 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data ) -> WasmResult<()> { debug_assert_eq!( self.result.module.local.functions.len(), - self.result.module.imported_funcs.len(), + self.result.module.local.num_imported_funcs, "Imported functions must be declared first" ); - self.result.module.local.functions.push(sig_index); - - self.result.module.imported_funcs.push(( - String::from(module), - String::from(field), - self.imports, + let func_index = self.result.module.local.functions.push(sig_index); + self.result.module.imports.push(( + module.to_owned(), + field.to_owned(), + Export::Function(func_index), )); self.result.module.local.num_imported_funcs += 1; - self.imports += 1; Ok(()) } fn declare_table_import(&mut self, table: Table, module: &str, field: &str) -> WasmResult<()> { debug_assert_eq!( self.result.module.local.table_plans.len(), - self.result.module.imported_tables.len(), + self.result.module.local.num_imported_tables, "Imported tables must be declared first" ); let plan = TablePlan::for_table(table, &self.result.tunables); - self.result.module.local.table_plans.push(plan); - - self.result.module.imported_tables.push(( - String::from(module), - String::from(field), - self.imports, + let table_index = self.result.module.local.table_plans.push(plan); + self.result.module.imports.push(( + module.to_owned(), + field.to_owned(), + Export::Table(table_index), )); self.result.module.local.num_imported_tables += 1; - self.imports += 1; Ok(()) } @@ -173,19 +175,20 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data ) -> WasmResult<()> { debug_assert_eq!( self.result.module.local.memory_plans.len(), - self.result.module.imported_memories.len(), + self.result.module.local.num_imported_memories, "Imported memories must be declared first" ); + if memory.shared { + return Err(WasmError::Unsupported("shared memories".to_owned())); + } let plan = MemoryPlan::for_memory(memory, &self.result.tunables); - self.result.module.local.memory_plans.push(plan); - - self.result.module.imported_memories.push(( - String::from(module), - String::from(field), - self.imports, + let memory_index = self.result.module.local.memory_plans.push(plan); + self.result.module.imports.push(( + module.to_owned(), + field.to_owned(), + Export::Memory(memory_index), )); self.result.module.local.num_imported_memories += 1; - self.imports += 1; Ok(()) } @@ -197,26 +200,16 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data ) -> WasmResult<()> { debug_assert_eq!( self.result.module.local.globals.len(), - self.result.module.imported_globals.len(), + self.result.module.local.num_imported_globals, "Imported globals must be declared first" ); - self.result.module.local.globals.push(global); - - self.result.module.imported_globals.push(( - String::from(module), - String::from(field), - self.imports, + let global_index = self.result.module.local.globals.push(global); + self.result.module.imports.push(( + module.to_owned(), + field.to_owned(), + Export::Global(global_index), )); self.result.module.local.num_imported_globals += 1; - self.imports += 1; - Ok(()) - } - - fn finish_imports(&mut self) -> WasmResult<()> { - self.result.module.imported_funcs.shrink_to_fit(); - self.result.module.imported_tables.shrink_to_fit(); - self.result.module.imported_memories.shrink_to_fit(); - self.result.module.imported_globals.shrink_to_fit(); Ok(()) } @@ -262,6 +255,9 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data } fn declare_memory(&mut self, memory: Memory) -> WasmResult<()> { + if memory.shared { + return Err(WasmError::Unsupported("shared memories".to_owned())); + } let plan = MemoryPlan::for_memory(memory, &self.result.tunables); self.result.module.local.memory_plans.push(plan); Ok(()) @@ -421,6 +417,27 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data .insert(func_index, name.to_string()); Ok(()) } + + fn custom_section(&mut self, name: &'data str, _data: &'data [u8]) -> WasmResult<()> { + match name { + "webidl-bindings" | "wasm-interface-types" => Err(WasmError::Unsupported( + "\ +Support for interface types has temporarily been removed from `wasmtime`. + +For more information about this temoprary you can read on the issue online: + + https://github.com/bytecodealliance/wasmtime/issues/1271 + +and for re-adding support for interface types you can see this issue: + + https://github.com/bytecodealliance/wasmtime/issues/677 +" + .to_owned(), + )), + // skip other sections + _ => Ok(()), + } + } } /// Add environment-specific function parameters. diff --git a/crates/fuzzing/src/oracles/dummy.rs b/crates/fuzzing/src/oracles/dummy.rs index 6db1ab14e291..3cf79d7031ee 100644 --- a/crates/fuzzing/src/oracles/dummy.rs +++ b/crates/fuzzing/src/oracles/dummy.rs @@ -6,19 +6,24 @@ use wasmtime::{ }; /// Create a set of dummy functions/globals/etc for the given imports. -pub fn dummy_imports(store: &Store, import_tys: &[ImportType]) -> Result, Trap> { - let mut imports = Vec::with_capacity(import_tys.len()); - for imp in import_tys { - imports.push(match imp.ty() { - ExternType::Func(func_ty) => Extern::Func(dummy_func(&store, func_ty.clone())), - ExternType::Global(global_ty) => { - Extern::Global(dummy_global(&store, global_ty.clone())?) - } - ExternType::Table(table_ty) => Extern::Table(dummy_table(&store, table_ty.clone())?), - ExternType::Memory(mem_ty) => Extern::Memory(dummy_memory(&store, mem_ty.clone())), - }); - } - Ok(imports) +pub fn dummy_imports( + store: &Store, + import_tys: impl Iterator, +) -> Result, Trap> { + import_tys + .map(|imp| { + Ok(match imp.ty() { + ExternType::Func(func_ty) => Extern::Func(dummy_func(&store, func_ty.clone())), + ExternType::Global(global_ty) => { + Extern::Global(dummy_global(&store, global_ty.clone())?) + } + ExternType::Table(table_ty) => { + Extern::Table(dummy_table(&store, table_ty.clone())?) + } + ExternType::Memory(mem_ty) => Extern::Memory(dummy_memory(&store, mem_ty.clone())), + }) + }) + .collect() } /// Construct a dummy function for the given function type diff --git a/crates/jit/src/imports.rs b/crates/jit/src/imports.rs index adc76ccffcaa..9a6f19df41ae 100644 --- a/crates/jit/src/imports.rs +++ b/crates/jit/src/imports.rs @@ -3,6 +3,7 @@ use crate::resolver::Resolver; use more_asserts::assert_ge; use std::collections::HashSet; +use std::convert::TryInto; use wasmtime_environ::entity::PrimaryMap; use wasmtime_environ::wasm::{Global, GlobalInit, Memory, Table, TableElementType}; use wasmtime_environ::{MemoryPlan, MemoryStyle, Module, TablePlan}; @@ -22,156 +23,140 @@ pub fn resolve_imports( ) -> Result { let mut dependencies = HashSet::new(); - let mut function_imports = PrimaryMap::with_capacity(module.imported_funcs.len()); - for (index, (module_name, field, import_idx)) in module.imported_funcs.iter() { - match resolver.resolve(*import_idx, module_name, field) { - Some(export_value) => match export_value { - Export::Function(f) => { - let import_signature = &module.local.signatures[module.local.functions[index]]; - let signature = signatures.lookup(f.signature).unwrap(); - if signature != *import_signature { - // TODO: If the difference is in the calling convention, - // we could emit a wrapper function to fix it up. - return Err(LinkError(format!( - "{}/{}: incompatible import type: exported function with signature {} \ - incompatible with function import with signature {}", - module_name, field, signature, import_signature - ))); - } - dependencies.insert(unsafe { InstanceHandle::from_vmctx(f.vmctx) }); - function_imports.push(VMFunctionImport { - body: f.address, - vmctx: f.vmctx, - }); - } - Export::Table(_) | Export::Memory(_) | Export::Global(_) => { + let mut function_imports = PrimaryMap::with_capacity(module.local.num_imported_funcs); + let mut table_imports = PrimaryMap::with_capacity(module.local.num_imported_tables); + let mut memory_imports = PrimaryMap::with_capacity(module.local.num_imported_memories); + let mut global_imports = PrimaryMap::with_capacity(module.local.num_imported_globals); + + for (import_idx, (module_name, field_name, import)) in module.imports.iter().enumerate() { + let import_idx = import_idx.try_into().unwrap(); + let export = resolver.resolve(import_idx, module_name, field_name); + + match (import, &export) { + (wasmtime_environ::Export::Function(func_index), Some(Export::Function(f))) => { + let import_signature = + &module.local.signatures[module.local.functions[*func_index]]; + let signature = signatures.lookup(f.signature).unwrap(); + if signature != *import_signature { + // TODO: If the difference is in the calling convention, + // we could emit a wrapper function to fix it up. return Err(LinkError(format!( - "{}/{}: incompatible import type: export incompatible with function import", - module_name, field + "{}/{}: incompatible import type: exported function with signature {} \ + incompatible with function import with signature {}", + module_name, field_name, signature, import_signature ))); } - }, - None => { + dependencies.insert(unsafe { InstanceHandle::from_vmctx(f.vmctx) }); + function_imports.push(VMFunctionImport { + body: f.address, + vmctx: f.vmctx, + }); + } + (wasmtime_environ::Export::Function(_), Some(_)) => { + return Err(LinkError(format!( + "{}/{}: incompatible import type: export incompatible with function import", + module_name, field_name + ))); + } + (wasmtime_environ::Export::Function(_), None) => { return Err(LinkError(format!( "{}/{}: unknown import function: function not provided", - module_name, field + module_name, field_name ))); } - } - } - let mut table_imports = PrimaryMap::with_capacity(module.imported_tables.len()); - for (index, (module_name, field, import_idx)) in module.imported_tables.iter() { - match resolver.resolve(*import_idx, module_name, field) { - Some(export_value) => match export_value { - Export::Table(t) => { - let import_table = &module.local.table_plans[index]; - if !is_table_compatible(&t.table, import_table) { - return Err(LinkError(format!( - "{}/{}: incompatible import type: exported table incompatible with \ - table import", - module_name, field, - ))); - } - dependencies.insert(unsafe { InstanceHandle::from_vmctx(t.vmctx) }); - table_imports.push(VMTableImport { - from: t.definition, - vmctx: t.vmctx, - }); - } - Export::Global(_) | Export::Memory(_) | Export::Function(_) => { + (wasmtime_environ::Export::Table(table_index), Some(Export::Table(t))) => { + let import_table = &module.local.table_plans[*table_index]; + if !is_table_compatible(&t.table, import_table) { return Err(LinkError(format!( - "{}/{}: incompatible import type: export incompatible with table import", - module_name, field + "{}/{}: incompatible import type: exported table incompatible with \ + table import", + module_name, field_name, ))); } - }, - None => { + dependencies.insert(unsafe { InstanceHandle::from_vmctx(t.vmctx) }); + table_imports.push(VMTableImport { + from: t.definition, + vmctx: t.vmctx, + }); + } + (wasmtime_environ::Export::Table(_), Some(_)) => { return Err(LinkError(format!( - "unknown import: no provided import table for {}/{}", - module_name, field + "{}/{}: incompatible import type: export incompatible with table import", + module_name, field_name + ))); + } + (wasmtime_environ::Export::Table(_), None) => { + return Err(LinkError(format!( + "{}/{}: unknown import table: table not provided", + module_name, field_name ))); } - } - } - - let mut memory_imports = PrimaryMap::with_capacity(module.imported_memories.len()); - for (index, (module_name, field, import_idx)) in module.imported_memories.iter() { - match resolver.resolve(*import_idx, module_name, field) { - Some(export_value) => match export_value { - Export::Memory(m) => { - let import_memory = &module.local.memory_plans[index]; - if !is_memory_compatible(&m.memory, import_memory) { - return Err(LinkError(format!( - "{}/{}: incompatible import type: exported memory incompatible with \ - memory import", - module_name, field - ))); - } - - // Sanity-check: Ensure that the imported memory has at least - // guard-page protections the importing module expects it to have. - if let ( - MemoryStyle::Static { bound }, - MemoryStyle::Static { - bound: import_bound, - }, - ) = (m.memory.style, &import_memory.style) - { - assert_ge!(bound, *import_bound); - } - assert_ge!(m.memory.offset_guard_size, import_memory.offset_guard_size); - dependencies.insert(unsafe { InstanceHandle::from_vmctx(m.vmctx) }); - memory_imports.push(VMMemoryImport { - from: m.definition, - vmctx: m.vmctx, - }); - } - Export::Table(_) | Export::Global(_) | Export::Function(_) => { + (wasmtime_environ::Export::Memory(memory_index), Some(Export::Memory(m))) => { + let import_memory = &module.local.memory_plans[*memory_index]; + if !is_memory_compatible(&m.memory, import_memory) { return Err(LinkError(format!( - "{}/{}: incompatible import type: export incompatible with memory import", - module_name, field + "{}/{}: incompatible import type: exported memory incompatible with \ + memory import", + module_name, field_name ))); } - }, - None => { + + // Sanity-check: Ensure that the imported memory has at least + // guard-page protections the importing module expects it to have. + if let ( + MemoryStyle::Static { bound }, + MemoryStyle::Static { + bound: import_bound, + }, + ) = (&m.memory.style, &import_memory.style) + { + assert_ge!(*bound, *import_bound); + } + assert_ge!(m.memory.offset_guard_size, import_memory.offset_guard_size); + + dependencies.insert(unsafe { InstanceHandle::from_vmctx(m.vmctx) }); + memory_imports.push(VMMemoryImport { + from: m.definition, + vmctx: m.vmctx, + }); + } + (wasmtime_environ::Export::Memory(_), Some(_)) => { return Err(LinkError(format!( - "unknown import: no provided import memory for {}/{}", - module_name, field + "{}/{}: incompatible import type: export incompatible with memory import", + module_name, field_name + ))); + } + (wasmtime_environ::Export::Memory(_), None) => { + return Err(LinkError(format!( + "{}/{}: unknown import memory: memory not provided", + module_name, field_name ))); } - } - } - let mut global_imports = PrimaryMap::with_capacity(module.imported_globals.len()); - for (index, (module_name, field, import_idx)) in module.imported_globals.iter() { - match resolver.resolve(*import_idx, module_name, field) { - Some(export_value) => match export_value { - Export::Table(_) | Export::Memory(_) | Export::Function(_) => { + (wasmtime_environ::Export::Global(global_index), Some(Export::Global(g))) => { + let imported_global = module.local.globals[*global_index]; + if !is_global_compatible(&g.global, &imported_global) { return Err(LinkError(format!( "{}/{}: incompatible import type: exported global incompatible with \ - global import", - module_name, field - ))); - } - Export::Global(g) => { - let imported_global = module.local.globals[index]; - if !is_global_compatible(&g.global, &imported_global) { - return Err(LinkError(format!( - "{}/{}: incompatible import type: exported global incompatible with \ global import", - module_name, field - ))); - } - dependencies.insert(unsafe { InstanceHandle::from_vmctx(g.vmctx) }); - global_imports.push(VMGlobalImport { from: g.definition }); + module_name, field_name + ))); } - }, - None => { + dependencies.insert(unsafe { InstanceHandle::from_vmctx(g.vmctx) }); + global_imports.push(VMGlobalImport { from: g.definition }); + } + (wasmtime_environ::Export::Global(_), Some(_)) => { + return Err(LinkError(format!( + "{}/{}: incompatible import type: export incompatible with global import", + module_name, field_name + ))); + } + (wasmtime_environ::Export::Global(_), None) => { return Err(LinkError(format!( - "unknown import: no provided import global for {}/{}", - module_name, field + "{}/{}: unknown import global: global not provided", + module_name, field_name ))); } } diff --git a/crates/obj/src/context.rs b/crates/obj/src/context.rs index 9faa5768a22d..70a99d7b7823 100644 --- a/crates/obj/src/context.rs +++ b/crates/obj/src/context.rs @@ -42,7 +42,7 @@ pub fn layout_vmcontext( } } - let num_tables_imports = module.imported_tables.len(); + let num_tables_imports = module.local.num_imported_tables; let mut table_relocs = Vec::with_capacity(module.local.table_plans.len() - num_tables_imports); for (index, table) in module.local.table_plans.iter().skip(num_tables_imports) { let def_index = module.local.defined_table_index(index).unwrap(); @@ -66,7 +66,7 @@ pub fn layout_vmcontext( }); } - let num_globals_imports = module.imported_globals.len(); + let num_globals_imports = module.local.num_imported_globals; for (index, global) in module.local.globals.iter().skip(num_globals_imports) { let def_index = module.local.defined_global_index(index).unwrap(); let offset = ofs.vmctx_vmglobal_definition(def_index) as usize; diff --git a/crates/obj/src/function.rs b/crates/obj/src/function.rs index e5598b2937e9..b3b3dfaa2820 100644 --- a/crates/obj/src/function.rs +++ b/crates/obj/src/function.rs @@ -11,7 +11,7 @@ pub fn declare_functions( module: &Module, relocations: &Relocations, ) -> Result<()> { - for i in 0..module.imported_funcs.len() { + for i in 0..module.local.num_imported_funcs { let string_name = format!("_wasm_function_{}", i); obj.declare(string_name, Decl::function_import())?; } @@ -32,7 +32,7 @@ pub fn emit_functions( ) -> Result<()> { debug_assert!( module.start_func.is_none() - || module.start_func.unwrap().index() >= module.imported_funcs.len(), + || module.start_func.unwrap().index() >= module.local.num_imported_funcs, "imported start functions not supported yet" ); diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 6eb157d5a5d9..0e6d6c8a3df0 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -398,7 +398,7 @@ impl Instance { (body as *const _, self.vmctx_ptr()) } None => { - assert_lt!(callee_index.index(), self.module.imported_funcs.len()); + assert_lt!(callee_index.index(), self.module.local.num_imported_funcs); let import = self.imported_function(callee_index); (import.body, import.vmctx) } @@ -1216,7 +1216,7 @@ fn check_memory_init_bounds( /// Allocate memory for just the tables of the current module. fn create_tables(module: &Module) -> BoxedSlice { - let num_imports = module.imported_tables.len(); + let num_imports = module.local.num_imported_tables; let mut tables: PrimaryMap = PrimaryMap::with_capacity(module.local.table_plans.len() - num_imports); for table in &module.local.table_plans.values().as_slice()[num_imports..] { @@ -1303,7 +1303,7 @@ fn create_memories( module: &Module, mem_creator: &dyn RuntimeMemoryCreator, ) -> Result>, InstantiationError> { - let num_imports = module.imported_memories.len(); + let num_imports = module.local.num_imported_memories; let mut memories: PrimaryMap = PrimaryMap::with_capacity(module.local.memory_plans.len() - num_imports); for plan in &module.local.memory_plans.values().as_slice()[num_imports..] { @@ -1348,7 +1348,7 @@ fn initialize_memories( /// Allocate memory for just the globals of the current module, /// with initializers applied. fn create_globals(module: &Module) -> BoxedSlice { - let num_imports = module.imported_globals.len(); + let num_imports = module.local.num_imported_globals; let mut vmctx_globals = PrimaryMap::with_capacity(module.local.globals.len() - num_imports); for _ in &module.local.globals.values().as_slice()[num_imports..] { @@ -1360,7 +1360,7 @@ fn create_globals(module: &Module) -> BoxedSlice, name: &str) -> Result { match module { - Some(module) => self.linker.get_one_by_name(module, name).map(Extern::clone), + Some(module) => self.linker.get_one_by_name(module, name), None => self .current .as_ref() diff --git a/src/commands/run.rs b/src/commands/run.rs index 429020908514..60e7267a26b9 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -199,7 +199,6 @@ impl RunCommand { // Resolve import using module_registry. let imports = module .imports() - .iter() .map(|i| { let export = match i.module() { "wasi_snapshot_preview1" => { @@ -231,11 +230,7 @@ impl RunCommand { // If a function to invoke was given, invoke it. if let Some(name) = self.invoke.as_ref() { self.invoke_export(instance, name)?; - } else if module - .exports() - .iter() - .any(|export| export.name().is_empty()) - { + } else if module.exports().any(|export| export.name().is_empty()) { // Launch the default command export. self.invoke_export(instance, "")?; } else { diff --git a/tests/misc_testsuite/threads.wast b/tests/misc_testsuite/threads.wast index 4c98ae2c3fcb..035cc529e465 100644 --- a/tests/misc_testsuite/threads.wast +++ b/tests/misc_testsuite/threads.wast @@ -1 +1 @@ -(assert_invalid (module (memory 1 1 shared)) "not supported") +(assert_invalid (module (memory 1 1 shared)) "Unsupported feature: shared memories") From e058c0464f77a100fa322f93ce8ac1191441fcf8 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 16 Apr 2020 10:16:58 -0700 Subject: [PATCH 03/24] Rename wasmtime_environ::Export to EntityIndex. This helps differentiate it from other Export types in the tree, and describes what it is. --- crates/api/src/module.rs | 17 +++++++++-------- crates/api/src/trampoline/func.rs | 6 +++--- crates/api/src/trampoline/global.rs | 9 ++++----- crates/api/src/trampoline/memory.rs | 9 ++++----- crates/api/src/trampoline/table.rs | 9 ++++----- crates/environ/src/lib.rs | 2 +- crates/environ/src/module.rs | 16 ++++++++-------- crates/environ/src/module_environ.rs | 20 ++++++++++---------- crates/jit/src/imports.rs | 26 +++++++++++++------------- crates/runtime/src/instance.rs | 18 +++++++++--------- 10 files changed, 65 insertions(+), 67 deletions(-) diff --git a/crates/api/src/module.rs b/crates/api/src/module.rs index d46b23b262cd..671f22646965 100644 --- a/crates/api/src/module.rs +++ b/crates/api/src/module.rs @@ -7,6 +7,7 @@ use anyhow::{Error, Result}; use std::path::Path; use std::sync::{Arc, Mutex}; use wasmparser::validate; +use wasmtime_environ::EntityIndex; use wasmtime_jit::CompiledModule; /// A compiled WebAssembly module, ready to be instantiated. @@ -409,7 +410,7 @@ impl Module { move |(module_name, field_name, import)| { let module = inner.compiled.module_ref(); match import { - wasmtime_environ::Export::Function(func_index) => { + EntityIndex::Function(func_index) => { let sig_index = module.local.functions[*func_index]; let sig = &module.local.signatures[sig_index]; let ty = FuncType::from_wasmtime_signature(sig) @@ -417,21 +418,21 @@ impl Module { .into(); ImportType::new(module_name, field_name, ty) } - wasmtime_environ::Export::Table(table_index) => { + EntityIndex::Table(table_index) => { let ty = TableType::from_wasmtime_table( &module.local.table_plans[*table_index].table, ) .into(); ImportType::new(module_name, field_name, ty) } - wasmtime_environ::Export::Memory(memory_index) => { + EntityIndex::Memory(memory_index) => { let ty = MemoryType::from_wasmtime_memory( &module.local.memory_plans[*memory_index].memory, ) .into(); ImportType::new(module_name, field_name, ty) } - wasmtime_environ::Export::Global(global_index) => { + EntityIndex::Global(global_index) => { let ty = GlobalType::from_wasmtime_global(&module.local.globals[*global_index]) .expect("core wasm global type should be supported") @@ -511,7 +512,7 @@ impl Module { .map(move |(name, ty)| { let module = inner.compiled.module_ref(); let r#type = match ty { - wasmtime_environ::Export::Function(func_index) => { + EntityIndex::Function(func_index) => { let sig_index = module.local.functions[*func_index]; let sig = &module.local.signatures[sig_index]; ExternType::Func( @@ -519,17 +520,17 @@ impl Module { .expect("core wasm function type should be supported"), ) } - wasmtime_environ::Export::Table(table_index) => { + EntityIndex::Table(table_index) => { ExternType::Table(TableType::from_wasmtime_table( &module.local.table_plans[*table_index].table, )) } - wasmtime_environ::Export::Memory(memory_index) => { + EntityIndex::Memory(memory_index) => { ExternType::Memory(MemoryType::from_wasmtime_memory( &module.local.memory_plans[*memory_index].memory, )) } - wasmtime_environ::Export::Global(global_index) => ExternType::Global( + EntityIndex::Global(global_index) => ExternType::Global( GlobalType::from_wasmtime_global(&module.local.globals[*global_index]) .expect("core wasm global type should be supported"), ), diff --git a/crates/api/src/trampoline/func.rs b/crates/api/src/trampoline/func.rs index 072d99dd2f3b..cafe26b47cc3 100644 --- a/crates/api/src/trampoline/func.rs +++ b/crates/api/src/trampoline/func.rs @@ -10,7 +10,7 @@ use std::mem; use std::panic::{self, AssertUnwindSafe}; use wasmtime_environ::entity::PrimaryMap; use wasmtime_environ::isa::TargetIsa; -use wasmtime_environ::{ir, settings, CompiledFunction, Export, Module}; +use wasmtime_environ::{ir, settings, CompiledFunction, EntityIndex, Module}; use wasmtime_jit::trampoline::ir::{ ExternalName, Function, InstBuilder, MemFlags, StackSlotData, StackSlotKind, }; @@ -228,7 +228,7 @@ pub fn create_handle_with_function( let func_id = module.local.functions.push(sig_id); module .exports - .insert("trampoline".to_string(), Export::Function(func_id)); + .insert("trampoline".to_string(), EntityIndex::Function(func_id)); let trampoline = make_trampoline(isa.as_ref(), &mut code_memory, &mut fn_builder_ctx, &sig); finished_functions.push(trampoline); @@ -288,7 +288,7 @@ pub unsafe fn create_handle_with_raw_function( let func_id = module.local.functions.push(sig_id); module .exports - .insert("trampoline".to_string(), Export::Function(func_id)); + .insert("trampoline".to_string(), EntityIndex::Function(func_id)); finished_functions.push(func); let sig_id = store.compiler().signatures().register(&sig); trampolines.insert(sig_id, trampoline); diff --git a/crates/api/src/trampoline/global.rs b/crates/api/src/trampoline/global.rs index 1499d18bee82..db2366fc92ad 100644 --- a/crates/api/src/trampoline/global.rs +++ b/crates/api/src/trampoline/global.rs @@ -3,7 +3,7 @@ use crate::Store; use crate::{GlobalType, Mutability, Val}; use anyhow::{bail, Result}; use wasmtime_environ::entity::PrimaryMap; -use wasmtime_environ::{wasm, Module}; +use wasmtime_environ::{wasm, EntityIndex, Module}; use wasmtime_runtime::InstanceHandle; pub fn create_global(store: &Store, gt: &GlobalType, val: Val) -> Result { @@ -26,10 +26,9 @@ pub fn create_global(store: &Store, gt: &GlobalType, val: Val) -> Result Result Result { @@ -21,10 +21,9 @@ pub fn create_handle_with_table(store: &Store, table: &TableType) -> Result, } -/// An entity to export. +/// An index of an entity. #[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)] -pub enum Export { - /// Function export. +pub enum EntityIndex { + /// Function index. Function(FuncIndex), - /// Table export. + /// Table index. Table(TableIndex), - /// Memory export. + /// Memory index. Memory(MemoryIndex), - /// Global export. + /// Global index. Global(GlobalIndex), } @@ -151,10 +151,10 @@ pub struct Module { pub local: ModuleLocal, /// All import records, in the order they are declared in the module. - pub imports: Vec<(String, String, Export)>, + pub imports: Vec<(String, String, EntityIndex)>, /// Exported entities. - pub exports: IndexMap, + pub exports: IndexMap, /// The module "start" function, if present. pub start_func: Option, diff --git a/crates/environ/src/module_environ.rs b/crates/environ/src/module_environ.rs index d2707a05cc2c..378d419fd401 100644 --- a/crates/environ/src/module_environ.rs +++ b/crates/environ/src/module_environ.rs @@ -1,5 +1,5 @@ use crate::func_environ::FuncEnvironment; -use crate::module::{Export, MemoryPlan, Module, TableElements, TablePlan}; +use crate::module::{EntityIndex, MemoryPlan, Module, TableElements, TablePlan}; use crate::tunables::Tunables; use cranelift_codegen::ir; use cranelift_codegen::ir::{AbiParam, ArgumentPurpose}; @@ -87,7 +87,7 @@ impl<'data> ModuleEnvironment<'data> { Ok(self.result) } - fn declare_export(&mut self, export: Export, name: &str) -> WasmResult<()> { + fn declare_export(&mut self, export: EntityIndex, name: &str) -> WasmResult<()> { self.result .module .exports @@ -144,7 +144,7 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data self.result.module.imports.push(( module.to_owned(), field.to_owned(), - Export::Function(func_index), + EntityIndex::Function(func_index), )); self.result.module.local.num_imported_funcs += 1; Ok(()) @@ -161,7 +161,7 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data self.result.module.imports.push(( module.to_owned(), field.to_owned(), - Export::Table(table_index), + EntityIndex::Table(table_index), )); self.result.module.local.num_imported_tables += 1; Ok(()) @@ -186,7 +186,7 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data self.result.module.imports.push(( module.to_owned(), field.to_owned(), - Export::Memory(memory_index), + EntityIndex::Memory(memory_index), )); self.result.module.local.num_imported_memories += 1; Ok(()) @@ -207,7 +207,7 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data self.result.module.imports.push(( module.to_owned(), field.to_owned(), - Export::Global(global_index), + EntityIndex::Global(global_index), )); self.result.module.local.num_imported_globals += 1; Ok(()) @@ -286,19 +286,19 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data } fn declare_func_export(&mut self, func_index: FuncIndex, name: &str) -> WasmResult<()> { - self.declare_export(Export::Function(func_index), name) + self.declare_export(EntityIndex::Function(func_index), name) } fn declare_table_export(&mut self, table_index: TableIndex, name: &str) -> WasmResult<()> { - self.declare_export(Export::Table(table_index), name) + self.declare_export(EntityIndex::Table(table_index), name) } fn declare_memory_export(&mut self, memory_index: MemoryIndex, name: &str) -> WasmResult<()> { - self.declare_export(Export::Memory(memory_index), name) + self.declare_export(EntityIndex::Memory(memory_index), name) } fn declare_global_export(&mut self, global_index: GlobalIndex, name: &str) -> WasmResult<()> { - self.declare_export(Export::Global(global_index), name) + self.declare_export(EntityIndex::Global(global_index), name) } fn declare_start_func(&mut self, func_index: FuncIndex) -> WasmResult<()> { diff --git a/crates/jit/src/imports.rs b/crates/jit/src/imports.rs index 9a6f19df41ae..ab5c2972c5d7 100644 --- a/crates/jit/src/imports.rs +++ b/crates/jit/src/imports.rs @@ -6,7 +6,7 @@ use std::collections::HashSet; use std::convert::TryInto; use wasmtime_environ::entity::PrimaryMap; use wasmtime_environ::wasm::{Global, GlobalInit, Memory, Table, TableElementType}; -use wasmtime_environ::{MemoryPlan, MemoryStyle, Module, TablePlan}; +use wasmtime_environ::{EntityIndex, MemoryPlan, MemoryStyle, Module, TablePlan}; use wasmtime_runtime::{ Export, Imports, InstanceHandle, LinkError, SignatureRegistry, VMFunctionImport, VMGlobalImport, VMMemoryImport, VMTableImport, @@ -33,7 +33,7 @@ pub fn resolve_imports( let export = resolver.resolve(import_idx, module_name, field_name); match (import, &export) { - (wasmtime_environ::Export::Function(func_index), Some(Export::Function(f))) => { + (EntityIndex::Function(func_index), Some(Export::Function(f))) => { let import_signature = &module.local.signatures[module.local.functions[*func_index]]; let signature = signatures.lookup(f.signature).unwrap(); @@ -52,20 +52,20 @@ pub fn resolve_imports( vmctx: f.vmctx, }); } - (wasmtime_environ::Export::Function(_), Some(_)) => { + (EntityIndex::Function(_), Some(_)) => { return Err(LinkError(format!( "{}/{}: incompatible import type: export incompatible with function import", module_name, field_name ))); } - (wasmtime_environ::Export::Function(_), None) => { + (EntityIndex::Function(_), None) => { return Err(LinkError(format!( "{}/{}: unknown import function: function not provided", module_name, field_name ))); } - (wasmtime_environ::Export::Table(table_index), Some(Export::Table(t))) => { + (EntityIndex::Table(table_index), Some(Export::Table(t))) => { let import_table = &module.local.table_plans[*table_index]; if !is_table_compatible(&t.table, import_table) { return Err(LinkError(format!( @@ -80,20 +80,20 @@ pub fn resolve_imports( vmctx: t.vmctx, }); } - (wasmtime_environ::Export::Table(_), Some(_)) => { + (EntityIndex::Table(_), Some(_)) => { return Err(LinkError(format!( "{}/{}: incompatible import type: export incompatible with table import", module_name, field_name ))); } - (wasmtime_environ::Export::Table(_), None) => { + (EntityIndex::Table(_), None) => { return Err(LinkError(format!( "{}/{}: unknown import table: table not provided", module_name, field_name ))); } - (wasmtime_environ::Export::Memory(memory_index), Some(Export::Memory(m))) => { + (EntityIndex::Memory(memory_index), Some(Export::Memory(m))) => { let import_memory = &module.local.memory_plans[*memory_index]; if !is_memory_compatible(&m.memory, import_memory) { return Err(LinkError(format!( @@ -122,20 +122,20 @@ pub fn resolve_imports( vmctx: m.vmctx, }); } - (wasmtime_environ::Export::Memory(_), Some(_)) => { + (EntityIndex::Memory(_), Some(_)) => { return Err(LinkError(format!( "{}/{}: incompatible import type: export incompatible with memory import", module_name, field_name ))); } - (wasmtime_environ::Export::Memory(_), None) => { + (EntityIndex::Memory(_), None) => { return Err(LinkError(format!( "{}/{}: unknown import memory: memory not provided", module_name, field_name ))); } - (wasmtime_environ::Export::Global(global_index), Some(Export::Global(g))) => { + (EntityIndex::Global(global_index), Some(Export::Global(g))) => { let imported_global = module.local.globals[*global_index]; if !is_global_compatible(&g.global, &imported_global) { return Err(LinkError(format!( @@ -147,13 +147,13 @@ pub fn resolve_imports( dependencies.insert(unsafe { InstanceHandle::from_vmctx(g.vmctx) }); global_imports.push(VMGlobalImport { from: g.definition }); } - (wasmtime_environ::Export::Global(_), Some(_)) => { + (EntityIndex::Global(_), Some(_)) => { return Err(LinkError(format!( "{}/{}: incompatible import type: export incompatible with global import", module_name, field_name ))); } - (wasmtime_environ::Export::Global(_), None) => { + (EntityIndex::Global(_), None) => { return Err(LinkError(format!( "{}/{}: unknown import global: global not provided", module_name, field_name diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 0e6d6c8a3df0..367d66bd9640 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -31,7 +31,7 @@ use wasmtime_environ::wasm::{ DataIndex, DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex, ElemIndex, FuncIndex, GlobalIndex, GlobalInit, MemoryIndex, SignatureIndex, TableIndex, }; -use wasmtime_environ::{ir, DataInitializer, Module, TableElements, VMOffsets}; +use wasmtime_environ::{ir, DataInitializer, EntityIndex, Module, TableElements, VMOffsets}; cfg_if::cfg_if! { if #[cfg(unix)] { @@ -296,9 +296,9 @@ impl Instance { } /// Lookup an export with the given export declaration. - pub fn lookup_by_declaration(&self, export: &wasmtime_environ::Export) -> Export { + pub fn lookup_by_declaration(&self, export: &EntityIndex) -> Export { match export { - wasmtime_environ::Export::Function(index) => { + EntityIndex::Function(index) => { let signature = self.signature_id(self.module.local.functions[*index]); let (address, vmctx) = if let Some(def_index) = self.module.local.defined_func_index(*index) { @@ -317,7 +317,7 @@ impl Instance { } .into() } - wasmtime_environ::Export::Table(index) => { + EntityIndex::Table(index) => { let (definition, vmctx) = if let Some(def_index) = self.module.local.defined_table_index(*index) { (self.table_ptr(def_index), self.vmctx_ptr()) @@ -332,7 +332,7 @@ impl Instance { } .into() } - wasmtime_environ::Export::Memory(index) => { + EntityIndex::Memory(index) => { let (definition, vmctx) = if let Some(def_index) = self.module.local.defined_memory_index(*index) { (self.memory_ptr(def_index), self.vmctx_ptr()) @@ -347,7 +347,7 @@ impl Instance { } .into() } - wasmtime_environ::Export::Global(index) => ExportGlobal { + EntityIndex::Global(index) => ExportGlobal { definition: if let Some(def_index) = self.module.local.defined_global_index(*index) { self.global_ptr(def_index) @@ -366,7 +366,7 @@ impl Instance { /// Specifically, it provides access to the key-value pairs, where they keys /// are export names, and the values are export declarations which can be /// resolved `lookup_by_declaration`. - pub fn exports(&self) -> indexmap::map::Iter { + pub fn exports(&self) -> indexmap::map::Iter { self.module.exports.iter() } @@ -1031,7 +1031,7 @@ impl InstanceHandle { } /// Lookup an export with the given export declaration. - pub fn lookup_by_declaration(&self, export: &wasmtime_environ::Export) -> Export { + pub fn lookup_by_declaration(&self, export: &EntityIndex) -> Export { self.instance().lookup_by_declaration(export) } @@ -1040,7 +1040,7 @@ impl InstanceHandle { /// Specifically, it provides access to the key-value pairs, where the keys /// are export names, and the values are export declarations which can be /// resolved `lookup_by_declaration`. - pub fn exports(&self) -> indexmap::map::Iter { + pub fn exports(&self) -> indexmap::map::Iter { self.instance().exports() } From 05f7534d6afc70f61ec0a2174b06dba0bbb76b28 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 16 Apr 2020 07:10:44 -0700 Subject: [PATCH 04/24] Add a utility function for computing an ExternType. --- crates/api/src/module.rs | 89 ++++++++++++++-------------------------- 1 file changed, 30 insertions(+), 59 deletions(-) diff --git a/crates/api/src/module.rs b/crates/api/src/module.rs index 671f22646965..e2aa1a059430 100644 --- a/crates/api/src/module.rs +++ b/crates/api/src/module.rs @@ -407,39 +407,9 @@ impl Module { pub fn imports<'me>(&'me self) -> impl Iterator + 'me { let inner = self.inner.clone(); self.inner.compiled.module_ref().imports.iter().map( - move |(module_name, field_name, import)| { - let module = inner.compiled.module_ref(); - match import { - EntityIndex::Function(func_index) => { - let sig_index = module.local.functions[*func_index]; - let sig = &module.local.signatures[sig_index]; - let ty = FuncType::from_wasmtime_signature(sig) - .expect("core wasm function type should be supported") - .into(); - ImportType::new(module_name, field_name, ty) - } - EntityIndex::Table(table_index) => { - let ty = TableType::from_wasmtime_table( - &module.local.table_plans[*table_index].table, - ) - .into(); - ImportType::new(module_name, field_name, ty) - } - EntityIndex::Memory(memory_index) => { - let ty = MemoryType::from_wasmtime_memory( - &module.local.memory_plans[*memory_index].memory, - ) - .into(); - ImportType::new(module_name, field_name, ty) - } - EntityIndex::Global(global_index) => { - let ty = - GlobalType::from_wasmtime_global(&module.local.globals[*global_index]) - .expect("core wasm global type should be supported") - .into(); - ImportType::new(module_name, field_name, ty) - } - } + move |(module_name, name, entity_index)| { + let r#type = entity_type(entity_index, inner.compiled.module_ref()); + ImportType::new(module_name, name, r#type) }, ) } @@ -509,32 +479,8 @@ impl Module { .module_ref() .exports .iter() - .map(move |(name, ty)| { - let module = inner.compiled.module_ref(); - let r#type = match ty { - EntityIndex::Function(func_index) => { - let sig_index = module.local.functions[*func_index]; - let sig = &module.local.signatures[sig_index]; - ExternType::Func( - FuncType::from_wasmtime_signature(sig) - .expect("core wasm function type should be supported"), - ) - } - EntityIndex::Table(table_index) => { - ExternType::Table(TableType::from_wasmtime_table( - &module.local.table_plans[*table_index].table, - )) - } - EntityIndex::Memory(memory_index) => { - ExternType::Memory(MemoryType::from_wasmtime_memory( - &module.local.memory_plans[*memory_index].memory, - )) - } - EntityIndex::Global(global_index) => ExternType::Global( - GlobalType::from_wasmtime_global(&module.local.globals[*global_index]) - .expect("core wasm global type should be supported"), - ), - }; + .map(move |(name, entity_index)| { + let r#type = entity_type(entity_index, inner.compiled.module_ref()); ExportType::new(name, r#type) }) } @@ -562,3 +508,28 @@ impl Module { return ret; } } + +/// Translate from a `EntityIndex` into an `ExternType`. +fn entity_type(entity_index: &EntityIndex, module: &wasmtime_environ::Module) -> ExternType { + match entity_index { + EntityIndex::Function(func_index) => { + let sig_index = module.local.functions[*func_index]; + let sig = &module.local.signatures[sig_index]; + FuncType::from_wasmtime_signature(sig) + .expect("core wasm function type should be supported") + .into() + } + EntityIndex::Table(table_index) => { + TableType::from_wasmtime_table(&module.local.table_plans[*table_index].table).into() + } + EntityIndex::Memory(memory_index) => { + MemoryType::from_wasmtime_memory(&module.local.memory_plans[*memory_index].memory) + .into() + } + EntityIndex::Global(global_index) => { + GlobalType::from_wasmtime_global(&module.local.globals[*global_index]) + .expect("core wasm global type should be supported") + .into() + } + } +} From d1f2996aa818b36fe6a976c6aaf73923008e5606 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 16 Apr 2020 07:37:35 -0700 Subject: [PATCH 05/24] Add a utility function for looking up a function's signature. --- crates/api/src/module.rs | 3 +-- crates/environ/src/cranelift.rs | 2 +- crates/environ/src/func_environ.rs | 4 ++-- crates/environ/src/module.rs | 5 +++++ crates/jit/src/imports.rs | 3 +-- 5 files changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/api/src/module.rs b/crates/api/src/module.rs index e2aa1a059430..311cf3b7593e 100644 --- a/crates/api/src/module.rs +++ b/crates/api/src/module.rs @@ -513,8 +513,7 @@ impl Module { fn entity_type(entity_index: &EntityIndex, module: &wasmtime_environ::Module) -> ExternType { match entity_index { EntityIndex::Function(func_index) => { - let sig_index = module.local.functions[*func_index]; - let sig = &module.local.signatures[sig_index]; + let sig = module.local.func_signature(*func_index); FuncType::from_wasmtime_signature(sig) .expect("core wasm function type should be supported") .into() diff --git a/crates/environ/src/cranelift.rs b/crates/environ/src/cranelift.rs index f9b8b4be8026..7d78024a41ad 100644 --- a/crates/environ/src/cranelift.rs +++ b/crates/environ/src/cranelift.rs @@ -203,7 +203,7 @@ fn compile(env: CompileEnv<'_>) -> Result cranelift_wasm::FuncEnvironment for FuncEnvironment<'m func: &mut ir::Function, index: FuncIndex, ) -> WasmResult { - let sigidx = self.module.functions[index]; - let signature = func.import_signature(self.module.signatures[sigidx].clone()); + let sig = self.module.func_signature(index); + let signature = func.import_signature(sig.clone()); let name = get_func_name(index); Ok(func.import_function(ir::ExtFuncData { name, diff --git a/crates/environ/src/module.rs b/crates/environ/src/module.rs index 36ae28ee566f..9a079e157b02 100644 --- a/crates/environ/src/module.rs +++ b/crates/environ/src/module.rs @@ -331,4 +331,9 @@ impl ModuleLocal { pub fn is_imported_global(&self, index: GlobalIndex) -> bool { index.index() < self.num_imported_globals } + + /// Convenience method for looking up the signature of a function. + pub fn func_signature(&self, func_index: FuncIndex) -> &ir::Signature { + &self.signatures[self.functions[func_index]] + } } diff --git a/crates/jit/src/imports.rs b/crates/jit/src/imports.rs index ab5c2972c5d7..743b6e512e4e 100644 --- a/crates/jit/src/imports.rs +++ b/crates/jit/src/imports.rs @@ -34,8 +34,7 @@ pub fn resolve_imports( match (import, &export) { (EntityIndex::Function(func_index), Some(Export::Function(f))) => { - let import_signature = - &module.local.signatures[module.local.functions[*func_index]]; + let import_signature = module.local.func_signature(*func_index); let signature = signatures.lookup(f.signature).unwrap(); if signature != *import_signature { // TODO: If the difference is in the calling convention, From 677ab4e58ac5c02f29c653f51ec6bc8cadbe6149 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 16 Apr 2020 10:06:37 -0700 Subject: [PATCH 06/24] Add a utility function for computing the ValType of a Global. --- crates/api/src/externals.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/api/src/externals.rs b/crates/api/src/externals.rs index 83997c4e7d9d..3b89715b6797 100644 --- a/crates/api/src/externals.rs +++ b/crates/api/src/externals.rs @@ -203,6 +203,12 @@ impl Global { .expect("core wasm global type should be supported") } + /// Returns the value type of this `global`. + pub fn val_type(&self) -> ValType { + ValType::from_wasmtime_type(self.wasmtime_export.global.ty) + .expect("core wasm type should be supported") + } + /// Returns the underlying mutability of this `global`. pub fn mutability(&self) -> Mutability { if self.wasmtime_export.global.mutability { @@ -216,14 +222,12 @@ impl Global { pub fn get(&self) -> Val { unsafe { let definition = &mut *self.wasmtime_export.definition; - let ty = ValType::from_wasmtime_type(self.wasmtime_export.global.ty) - .expect("core wasm type should be supported"); - match ty { + match self.val_type() { ValType::I32 => Val::from(*definition.as_i32()), ValType::I64 => Val::from(*definition.as_i64()), ValType::F32 => Val::F32(*definition.as_u32()), ValType::F64 => Val::F64(*definition.as_u64()), - _ => unimplemented!("Global::get for {:?}", ty), + ty => unimplemented!("Global::get for {:?}", ty), } } } @@ -238,8 +242,7 @@ impl Global { if self.mutability() != Mutability::Var { bail!("immutable global cannot be set"); } - let ty = ValType::from_wasmtime_type(self.wasmtime_export.global.ty) - .expect("core wasm type should be supported"); + let ty = self.val_type(); if val.ty() != ty { bail!("global of type {:?} cannot be set to {:?}", ty, val.ty()); } From 8d5c016449942b2e3e3e87a9a0a8a7a950c17eee Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 16 Apr 2020 10:48:43 -0700 Subject: [PATCH 07/24] Fix a typo in a comment. --- crates/runtime/src/instance.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 367d66bd9640..dc5a6698c7f6 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -363,7 +363,7 @@ impl Instance { /// Return an iterator over the exports of this instance. /// - /// Specifically, it provides access to the key-value pairs, where they keys + /// Specifically, it provides access to the key-value pairs, where the keys /// are export names, and the values are export declarations which can be /// resolved `lookup_by_declaration`. pub fn exports(&self) -> indexmap::map::Iter { From d9a813cb8afa9de99db61496c50234575bada3e3 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 16 Apr 2020 11:12:46 -0700 Subject: [PATCH 08/24] Simplify module imports and exports. --- crates/api/src/module.rs | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/crates/api/src/module.rs b/crates/api/src/module.rs index 311cf3b7593e..f91b22a68ee7 100644 --- a/crates/api/src/module.rs +++ b/crates/api/src/module.rs @@ -405,13 +405,14 @@ impl Module { /// # } /// ``` pub fn imports<'me>(&'me self) -> impl Iterator + 'me { - let inner = self.inner.clone(); - self.inner.compiled.module_ref().imports.iter().map( - move |(module_name, name, entity_index)| { - let r#type = entity_type(entity_index, inner.compiled.module_ref()); + let module = self.inner.compiled.module_ref(); + module + .imports + .iter() + .map(move |(module_name, name, entity_index)| { + let r#type = entity_type(entity_index, module); ImportType::new(module_name, name, r#type) - }, - ) + }) } /// Return the number of imports in this module. @@ -473,16 +474,11 @@ impl Module { /// # } /// ``` pub fn exports<'me>(&'me self) -> impl Iterator + 'me { - let inner = self.inner.clone(); - self.inner - .compiled - .module_ref() - .exports - .iter() - .map(move |(name, entity_index)| { - let r#type = entity_type(entity_index, inner.compiled.module_ref()); - ExportType::new(name, r#type) - }) + let module = self.inner.compiled.module_ref(); + module.exports.iter().map(move |(name, entity_index)| { + let r#type = entity_type(entity_index, module); + ExportType::new(name, r#type) + }) } /// Return the number of exports in this module. From 73c82b786da8825b967205bf1887454785926d4c Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 16 Apr 2020 16:19:26 -0700 Subject: [PATCH 09/24] Make `Instance::exports` return the export names. This significantly simplifies the public API, as it's relatively common to need the names, and this avoids the need to do a zip with `Module::exports`. This also changes `ImportType` and `ExportType` to have public members instead of private members and accessors, as I find that simplifies the usage particularly in cases where there are temporary instances. --- crates/api/src/externals.rs | 43 +++++++++++++ crates/api/src/instance.rs | 24 ++++---- crates/api/src/linker.rs | 24 ++++---- crates/api/src/module.rs | 25 +++++--- crates/api/src/types.rs | 61 +++---------------- crates/c-api/src/instance.rs | 2 +- crates/c-api/src/types/export.rs | 9 ++- crates/c-api/src/types/import.rs | 12 ++-- crates/c-api/src/vec.rs | 4 +- crates/c-api/src/wasi.rs | 11 ++-- crates/fuzzing/src/oracles/dummy.rs | 2 +- .../test-programs/tests/wasm_tests/runtime.rs | 8 +-- examples/wasi/main.rs | 7 +-- src/commands/run.rs | 20 +++--- tests/all/custom_signal_handler.rs | 2 +- tests/all/traps.rs | 2 +- 16 files changed, 133 insertions(+), 123 deletions(-) diff --git a/crates/api/src/externals.rs b/crates/api/src/externals.rs index 3b89715b6797..bf561678dbf6 100644 --- a/crates/api/src/externals.rs +++ b/crates/api/src/externals.rs @@ -890,3 +890,46 @@ pub unsafe trait MemoryCreator: Send + Sync { /// Create new LinearMemory fn new_memory(&self, ty: MemoryType) -> Result, String>; } + +// Exports + +/// An exported WebAssembly value. +/// +/// This type is primarily accessed from the +/// [`Instance::exports`](crate::Instance::exports) accessor and describes what +/// names and items are exported from a wasm instance. +#[derive(Clone)] +pub struct Export<'instance> { + /// The name of the export. + pub name: &'instance str, + + /// The value of the export. + pub external: Extern, +} + +impl<'instance> Export<'instance> { + /// Shorthand for `self.external.func()`. + pub fn func(self) -> Option { + self.external.func() + } + + /// Shorthand for `self.external.table()`. + pub fn table(self) -> Option
{ + self.external.table() + } + + /// Shorthand for `self.external.memory()`. + pub fn memory(self) -> Option { + self.external.memory() + } + + /// Shorthand for `self.external.global()`. + pub fn global(self) -> Option { + self.external.global() + } + + /// Shorthand for `self.external.ty()`. + pub fn ty(&self) -> ExternType { + self.external.ty() + } +} diff --git a/crates/api/src/instance.rs b/crates/api/src/instance.rs index 2e699d1421fd..d64e24cf1731 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/src/instance.rs @@ -1,18 +1,18 @@ -use crate::externals::Extern; +use crate::externals::{Export, Extern}; use crate::module::Module; use crate::runtime::{Config, Store}; use crate::trap::Trap; use anyhow::{bail, Error, Result}; use std::any::Any; use wasmtime_jit::{CompiledModule, Resolver}; -use wasmtime_runtime::{Export, InstanceHandle, InstantiationError, SignatureRegistry}; +use wasmtime_runtime::{InstanceHandle, InstantiationError, SignatureRegistry}; struct SimpleResolver<'a> { imports: &'a [Extern], } impl Resolver for SimpleResolver<'_> { - fn resolve(&mut self, idx: u32, _name: &str, _field: &str) -> Option { + fn resolve(&mut self, idx: u32, _name: &str, _field: &str) -> Option { self.imports .get(idx as usize) .map(|i| i.get_wasmtime_export()) @@ -169,19 +169,21 @@ impl Instance { /// Returns the list of exported items from this [`Instance`]. /// - /// Note that the exports here do not have names associated with them, - /// they're simply the values that are exported. To learn the value of each - /// export you'll need to consult [`Module::exports`]. The list returned - /// here maps 1:1 with the list that [`Module::exports`] returns, and - /// [`ExportType`](crate::ExportType) contains the name of each export. - pub fn exports<'me>(&'me self) -> impl Iterator + 'me { + /// The actual returned value is a tuple containing the name of an export and + /// information about the export itself. The list returned here maps 1:1 with + /// the list that [`Module::exports`] returns, and [`ExportType`](crate::ExportType) + /// contains the name of each export. + pub fn exports<'me>(&'me self) -> impl Iterator + 'me { let instance_handle = &self.instance_handle; let store = self.module.store(); self.instance_handle .exports() - .map(move |(_, entity_index)| { + .map(move |(name, entity_index)| { let export = instance_handle.lookup_by_declaration(entity_index); - Extern::from_wasmtime_export(store, instance_handle.clone(), export) + Export { + name, + external: Extern::from_wasmtime_export(store, instance_handle.clone(), export), + } }) } diff --git a/crates/api/src/linker.rs b/crates/api/src/linker.rs index 311d8b132655..2c44731742d5 100644 --- a/crates/api/src/linker.rs +++ b/crates/api/src/linker.rs @@ -264,8 +264,8 @@ impl Linker { if !Store::same(&self.store, instance.store()) { bail!("all linker items must be from the same store"); } - for (export, item) in instance.module().exports().zip(instance.exports()) { - self.insert(module_name, export.name(), export.ty().clone(), item)?; + for export in instance.exports() { + self.insert(module_name, &export.name, export.ty(), export.external)?; } Ok(self) } @@ -385,8 +385,8 @@ impl Linker { let mut options = String::new(); for i in self.map.keys() { - if &*self.strings[i.module] != import.module() - || &*self.strings[i.name] != import.name() + if &*self.strings[i.module] != import.module + || &*self.strings[i.name] != import.name { continue; } @@ -397,8 +397,8 @@ impl Linker { if options.len() == 0 { bail!( "unknown import: `{}::{}` has not been defined", - import.module(), - import.name() + import.module, + import.name ) } @@ -406,9 +406,9 @@ impl Linker { "incompatible import type for `{}::{}` specified\n\ desired signature was: {:?}\n\ signatures available:\n\n{}", - import.module(), - import.name(), - import.ty(), + import.module, + import.name, + import.ty, options, ) } @@ -445,9 +445,9 @@ impl Linker { /// Returns `None` if no match was found. pub fn get(&self, import: &ImportType) -> Option { let key = ImportKey { - module: *self.string2idx.get(import.module())?, - name: *self.string2idx.get(import.name())?, - kind: self.import_kind(import.ty().clone()), + module: *self.string2idx.get(import.module.as_str())?, + name: *self.string2idx.get(import.name.as_str())?, + kind: self.import_kind(import.ty.clone()), }; self.map.get(&key).cloned() } diff --git a/crates/api/src/module.rs b/crates/api/src/module.rs index f91b22a68ee7..0f55f0b4ee7f 100644 --- a/crates/api/src/module.rs +++ b/crates/api/src/module.rs @@ -395,9 +395,9 @@ impl Module { /// let module = Module::new(&store, wat)?; /// assert_eq!(module.num_imports(), 1); /// let import = module.imports().next().unwrap(); - /// assert_eq!(import.module(), "host"); - /// assert_eq!(import.name(), "foo"); - /// match import.ty() { + /// assert_eq!(import.module, "host"); + /// assert_eq!(import.name, "foo"); + /// match import.ty { /// ExternType::Func(_) => { /* ... */ } /// _ => panic!("unexpected import type!"), /// } @@ -411,7 +411,11 @@ impl Module { .iter() .map(move |(module_name, name, entity_index)| { let r#type = entity_type(entity_index, module); - ImportType::new(module_name, name, r#type) + ImportType { + module: module_name.to_owned(), + name: name.to_owned(), + ty: r#type, + } }) } @@ -458,15 +462,15 @@ impl Module { /// assert_eq!(module.num_exports(), 2); /// /// let foo = module.exports().next().unwrap(); - /// assert_eq!(foo.name(), "foo"); - /// match foo.ty() { + /// assert_eq!(foo.name, "foo"); + /// match foo.ty { /// ExternType::Func(_) => { /* ... */ } /// _ => panic!("unexpected export type!"), /// } /// /// let memory = module.exports().nth(1).unwrap(); - /// assert_eq!(memory.name(), "memory"); - /// match memory.ty() { + /// assert_eq!(memory.name, "memory"); + /// match memory.ty { /// ExternType::Memory(_) => { /* ... */ } /// _ => panic!("unexpected export type!"), /// } @@ -477,7 +481,10 @@ impl Module { let module = self.inner.compiled.module_ref(); module.exports.iter().map(move |(name, entity_index)| { let r#type = entity_type(entity_index, module); - ExportType::new(name, r#type) + ExportType { + name: name.to_owned(), + ty: r#type, + } }) } diff --git a/crates/api/src/types.rs b/crates/api/src/types.rs index 605d1be786f4..2a6316f90488 100644 --- a/crates/api/src/types.rs +++ b/crates/api/src/types.rs @@ -392,37 +392,14 @@ impl MemoryType { /// imported from as well as the type of item that's being imported. #[derive(Debug, Clone, Hash, Eq, PartialEq)] pub struct ImportType { - module: String, - name: String, - ty: ExternType, -} - -impl ImportType { - /// Creates a new import descriptor which comes from `module` and `name` and - /// is of type `ty`. - pub fn new(module: &str, name: &str, ty: ExternType) -> ImportType { - ImportType { - module: module.to_string(), - name: name.to_string(), - ty, - } - } + /// The module of the import. + pub module: String, - /// Returns the module name that this import is expected to come from. - pub fn module(&self) -> &str { - &self.module - } - - /// Returns the field name of the module that this import is expected to - /// come from. - pub fn name(&self) -> &str { - &self.name - } + /// The field of the import. + pub name: String, - /// Returns the expected type of this import. - pub fn ty(&self) -> &ExternType { - &self.ty - } + /// The type of the import. + pub ty: ExternType, } // Export Types @@ -435,27 +412,9 @@ impl ImportType { /// exported. #[derive(Debug, Clone, Hash, Eq, PartialEq)] pub struct ExportType { - name: String, - ty: ExternType, -} - -impl ExportType { - /// Creates a new export which is exported with the given `name` and has the - /// given `ty`. - pub fn new(name: &str, ty: ExternType) -> ExportType { - ExportType { - name: name.to_string(), - ty, - } - } + /// The name of the export. + pub name: String, - /// Returns the name by which this export is known by. - pub fn name(&self) -> &str { - &self.name - } - - /// Returns the type of this export. - pub fn ty(&self) -> &ExternType { - &self.ty - } + /// The type of the export. + pub ty: ExternType, } diff --git a/crates/c-api/src/instance.rs b/crates/c-api/src/instance.rs index 113e1baa83a2..1afa3686d3d4 100644 --- a/crates/c-api/src/instance.rs +++ b/crates/c-api/src/instance.rs @@ -145,7 +145,7 @@ pub extern "C" fn wasm_instance_exports(instance: &wasm_instance_t, out: &mut wa let instance = &instance.instance.borrow(); instance .exports() - .map(|e| match e { + .map(|e| match e.external { Extern::Func(f) => ExternHost::Func(HostRef::new(f.clone())), Extern::Global(f) => ExternHost::Global(HostRef::new(f.clone())), Extern::Memory(f) => ExternHost::Memory(HostRef::new(f.clone())), diff --git a/crates/c-api/src/types/export.rs b/crates/c-api/src/types/export.rs index 90d69b96e6e1..94be6ba49958 100644 --- a/crates/c-api/src/types/export.rs +++ b/crates/c-api/src/types/export.rs @@ -30,18 +30,21 @@ pub extern "C" fn wasm_exporttype_new( ) -> Option> { let name = name.take(); let name = str::from_utf8(&name).ok()?; - let ty = ExportType::new(name, ty.ty()); + let ty = ExportType { + name: name.to_string(), + ty: ty.ty(), + }; Some(Box::new(wasm_exporttype_t::new(ty))) } #[no_mangle] pub extern "C" fn wasm_exporttype_name(et: &wasm_exporttype_t) -> &wasm_name_t { et.name_cache - .get_or_init(|| wasm_name_t::from_name(&et.ty.name())) + .get_or_init(|| wasm_name_t::from_name(et.ty.name.clone())) } #[no_mangle] pub extern "C" fn wasm_exporttype_type(et: &wasm_exporttype_t) -> &wasm_externtype_t { et.type_cache - .get_or_init(|| wasm_externtype_t::new(et.ty.ty().clone())) + .get_or_init(|| wasm_externtype_t::new(et.ty.ty.clone())) } diff --git a/crates/c-api/src/types/import.rs b/crates/c-api/src/types/import.rs index 66306d2add5b..f380de575695 100644 --- a/crates/c-api/src/types/import.rs +++ b/crates/c-api/src/types/import.rs @@ -35,24 +35,28 @@ pub extern "C" fn wasm_importtype_new( let name = name.take(); let module = str::from_utf8(&module).ok()?; let name = str::from_utf8(&name).ok()?; - let ty = ImportType::new(module, name, ty.ty()); + let ty = ImportType { + module: module.to_owned(), + name: name.to_owned(), + ty: ty.ty(), + }; Some(Box::new(wasm_importtype_t::new(ty))) } #[no_mangle] pub extern "C" fn wasm_importtype_module(it: &wasm_importtype_t) -> &wasm_name_t { it.module_cache - .get_or_init(|| wasm_name_t::from_name(&it.ty.module())) + .get_or_init(|| wasm_name_t::from_name(it.ty.module.clone())) } #[no_mangle] pub extern "C" fn wasm_importtype_name(it: &wasm_importtype_t) -> &wasm_name_t { it.name_cache - .get_or_init(|| wasm_name_t::from_name(&it.ty.name())) + .get_or_init(|| wasm_name_t::from_name(it.ty.name.clone())) } #[no_mangle] pub extern "C" fn wasm_importtype_type(it: &wasm_importtype_t) -> &wasm_externtype_t { it.type_cache - .get_or_init(|| wasm_externtype_t::new(it.ty.ty().clone())) + .get_or_init(|| wasm_externtype_t::new(it.ty.ty.clone())) } diff --git a/crates/c-api/src/vec.rs b/crates/c-api/src/vec.rs index 15918b29c9da..8a7d7395a23c 100644 --- a/crates/c-api/src/vec.rs +++ b/crates/c-api/src/vec.rs @@ -9,8 +9,8 @@ use std::slice; pub type wasm_name_t = wasm_byte_vec_t; impl wasm_name_t { - pub(crate) fn from_name(name: &str) -> wasm_name_t { - name.to_string().into_bytes().into() + pub(crate) fn from_name(name: String) -> wasm_name_t { + name.into_bytes().into() } } diff --git a/crates/c-api/src/wasi.rs b/crates/c-api/src/wasi.rs index 8514beddbb06..45906ccb379d 100644 --- a/crates/c-api/src/wasi.rs +++ b/crates/c-api/src/wasi.rs @@ -7,6 +7,7 @@ use std::fs::File; use std::os::raw::{c_char, c_int}; use std::path::{Path, PathBuf}; use std::slice; +use std::str; use wasi_common::{ old::snapshot_0::WasiCtxBuilder as WasiSnapshot0CtxBuilder, preopen_dir, WasiCtxBuilder as WasiPreview1CtxBuilder, @@ -312,26 +313,26 @@ pub extern "C" fn wasi_instance_bind_import<'a>( instance: &'a mut wasi_instance_t, import: &wasm_importtype_t, ) -> Option<&'a wasm_extern_t> { - let module = import.ty.module(); - let name = import.ty.name(); + let module = &import.ty.module; + let name = str::from_utf8(import.ty.name.as_bytes()).ok()?; let export = match &instance.wasi { WasiInstance::Preview1(wasi) => { if module != "wasi_snapshot_preview1" { return None; } - wasi.get_export(name)? + wasi.get_export(&name)? } WasiInstance::Snapshot0(wasi) => { if module != "wasi_unstable" { return None; } - wasi.get_export(name)? + wasi.get_export(&name)? } }; - if &export.ty() != import.ty.ty().func()? { + if &export.ty() != import.ty.ty.func()? { return None; } diff --git a/crates/fuzzing/src/oracles/dummy.rs b/crates/fuzzing/src/oracles/dummy.rs index 3cf79d7031ee..6027775caeda 100644 --- a/crates/fuzzing/src/oracles/dummy.rs +++ b/crates/fuzzing/src/oracles/dummy.rs @@ -12,7 +12,7 @@ pub fn dummy_imports( ) -> Result, Trap> { import_tys .map(|imp| { - Ok(match imp.ty() { + Ok(match imp.ty { ExternType::Func(func_ty) => Extern::Func(dummy_func(&store, func_ty.clone())), ExternType::Global(global_ty) => { Extern::Global(dummy_global(&store, global_ty.clone())?) diff --git a/crates/test-programs/tests/wasm_tests/runtime.rs b/crates/test-programs/tests/wasm_tests/runtime.rs index fe67d4c5e812..54883c076e40 100644 --- a/crates/test-programs/tests/wasm_tests/runtime.rs +++ b/crates/test-programs/tests/wasm_tests/runtime.rs @@ -52,15 +52,11 @@ pub fn instantiate( let imports = module .imports() .map(|i| { - let field_name = i.name(); + let field_name = &i.name; if let Some(export) = snapshot1.get_export(field_name) { Ok(export.clone().into()) } else { - bail!( - "import {} was not found in module {}", - field_name, - i.module(), - ) + bail!("import {} was not found in module {}", field_name, i.module,) } }) .collect::, _>>()?; diff --git a/examples/wasi/main.rs b/examples/wasi/main.rs index 9216bce52fac..ab5ba8009082 100644 --- a/examples/wasi/main.rs +++ b/examples/wasi/main.rs @@ -17,16 +17,15 @@ fn main() -> Result<()> { let wasi = Wasi::new(&store, WasiCtx::new(std::env::args())?); let mut imports = Vec::new(); for import in module.imports() { - if import.module() == "wasi_snapshot_preview1" { - if let Some(export) = wasi.get_export(import.name()) { + if import.module == "wasi_snapshot_preview1" { + if let Some(export) = wasi.get_export(&import.name) { imports.push(Extern::from(export.clone())); continue; } } panic!( "couldn't find import for `{}::{}`", - import.module(), - import.name() + import.module, import.name ); } diff --git a/src/commands/run.rs b/src/commands/run.rs index 60e7267a26b9..4a8c61a30c05 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -190,7 +190,7 @@ impl RunCommand { store: &Store, module_registry: &ModuleRegistry, path: &Path, - ) -> Result<(Instance, Module)> { + ) -> Result { // Read the wasm module binary either as `*.wat` or a raw binary let data = wat::parse_file(path)?; @@ -200,20 +200,16 @@ impl RunCommand { let imports = module .imports() .map(|i| { - let export = match i.module() { + let export = match i.module.as_str() { "wasi_snapshot_preview1" => { - module_registry.wasi_snapshot_preview1.get_export(i.name()) + module_registry.wasi_snapshot_preview1.get_export(&i.name) } - "wasi_unstable" => module_registry.wasi_unstable.get_export(i.name()), + "wasi_unstable" => module_registry.wasi_unstable.get_export(&i.name), other => bail!("import module `{}` was not found", other), }; match export { Some(export) => Ok(export.clone().into()), - None => bail!( - "import `{}` was not found in module `{}`", - i.name(), - i.module() - ), + None => bail!("import `{}` was not found in module `{}`", i.name, i.module), } }) .collect::, _>>()?; @@ -221,16 +217,16 @@ impl RunCommand { let instance = Instance::new(&module, &imports) .context(format!("failed to instantiate {:?}", path))?; - Ok((instance, module)) + Ok(instance) } fn handle_module(&self, store: &Store, module_registry: &ModuleRegistry) -> Result<()> { - let (instance, module) = Self::instantiate_module(store, module_registry, &self.module)?; + let instance = Self::instantiate_module(store, module_registry, &self.module)?; // If a function to invoke was given, invoke it. if let Some(name) = self.invoke.as_ref() { self.invoke_export(instance, name)?; - } else if module.exports().any(|export| export.name().is_empty()) { + } else if instance.exports().any(|export| export.name.is_empty()) { // Launch the default command export. self.invoke_export(instance, "")?; } else { diff --git a/tests/all/custom_signal_handler.rs b/tests/all/custom_signal_handler.rs index 685c740372a2..d234ac6ba77a 100644 --- a/tests/all/custom_signal_handler.rs +++ b/tests/all/custom_signal_handler.rs @@ -270,7 +270,7 @@ mod tests { // instance2 which calls 'instance1.read' let module2 = Module::new(&store, WAT2)?; - let instance2 = Instance::new(&module2, &[instance1_read])?; + let instance2 = Instance::new(&module2, &[instance1_read.external])?; // since 'instance2.run' calls 'instance1.read' we need to set up the signal handler to handle // SIGSEGV originating from within the memory of instance1 unsafe { diff --git a/tests/all/traps.rs b/tests/all/traps.rs index bd08537169d9..5a8c10658906 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -217,7 +217,7 @@ fn trap_display_multi_module() -> Result<()> { ) "#; let module = Module::new(&store, wat)?; - let instance = Instance::new(&module, &[bar])?; + let instance = Instance::new(&module, &[bar.external])?; let bar2 = instance .exports() .next() From a7bf93edfdab31575c105bbbce345e2cc51376cf Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 16 Apr 2020 16:22:32 -0700 Subject: [PATCH 10/24] Remove `Instance::module`. This doesn't quite remove `Instance`'s `module` member, it gets a step closer. --- crates/api/src/instance.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/crates/api/src/instance.rs b/crates/api/src/instance.rs index d64e24cf1731..7be541658cea 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/src/instance.rs @@ -158,15 +158,6 @@ impl Instance { 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 - } - /// Returns the list of exported items from this [`Instance`]. /// /// The actual returned value is a tuple containing the name of an export and From 9cf5fb779cebac91e2b3f3ab4b94939b51303f6d Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 16 Apr 2020 16:39:39 -0700 Subject: [PATCH 11/24] Use a InstanceHandle utility function. --- crates/runtime/src/debug_builtins.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/runtime/src/debug_builtins.rs b/crates/runtime/src/debug_builtins.rs index 831bbdd3a8d8..3a6cfc8b81c2 100644 --- a/crates/runtime/src/debug_builtins.rs +++ b/crates/runtime/src/debug_builtins.rs @@ -16,7 +16,7 @@ pub unsafe extern "C" fn resolve_vmctx_memory_ptr(p: *const u32) -> *const u8 { ); let handle = InstanceHandle::from_vmctx(VMCTX_AND_MEMORY.0); assert!( - VMCTX_AND_MEMORY.1 < handle.instance().module().local.memory_plans.len(), + VMCTX_AND_MEMORY.1 < handle.module().local.memory_plans.len(), "memory index for debugger is out of bounds" ); let index = MemoryIndex::new(VMCTX_AND_MEMORY.1); From 842bd953750cadcc937562fe7d262a5a43420434 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 16 Apr 2020 16:40:01 -0700 Subject: [PATCH 12/24] Don't consume self in the `Func::get*` methods. Instead, just create a closure containing the instance handle and the export for them to call. --- crates/api/src/func.rs | 19 +++++++++--- tests/all/func.rs | 68 +++++++++++++++++++++--------------------- 2 files changed, 49 insertions(+), 38 deletions(-) diff --git a/crates/api/src/func.rs b/crates/api/src/func.rs index 89a1a2fd4bd6..e2ee519150df 100644 --- a/crates/api/src/func.rs +++ b/crates/api/src/func.rs @@ -148,7 +148,7 @@ macro_rules! getters { )*) => ($( $(#[$doc])* #[allow(non_snake_case)] - pub fn $name<$($args,)* R>(self) + pub fn $name<$($args,)* R>(&self) -> anyhow::Result Result> where $($args: WasmTy,)* @@ -172,6 +172,17 @@ macro_rules! getters { .context("Type mismatch in return type")?; ensure!(results.next().is_none(), "Type mismatch: too many return values (expected 1)"); + // Pass the instance into the closure so that we keep it live for the lifetime + // of the closure. Pass the export in so that we can call it. + struct ClosureState { + _instance: InstanceHandle, + export: ExportFunction + } + let closure = ClosureState { + _instance: self.instance.clone(), + export: self.export.clone() + }; + // ... and then once we've passed the typechecks we can hand out our // object since our `transmute` below should be safe! Ok(move |$($args: $args),*| -> Result { @@ -183,11 +194,11 @@ macro_rules! getters { *mut VMContext, $($args,)* ) -> R, - >(self.export.address); + >(closure.export.address); let mut ret = None; $(let $args = $args.into_abi();)* - wasmtime_runtime::catch_traps(self.export.vmctx, || { - ret = Some(fnptr(self.export.vmctx, ptr::null_mut(), $($args,)*)); + wasmtime_runtime::catch_traps(closure.export.vmctx, || { + ret = Some(fnptr(closure.export.vmctx, ptr::null_mut(), $($args,)*)); }).map_err(Trap::from_jit)?; Ok(ret.unwrap()) } diff --git a/tests/all/func.rs b/tests/all/func.rs index 4914e6e7ccee..431de864536e 100644 --- a/tests/all/func.rs +++ b/tests/all/func.rs @@ -207,33 +207,33 @@ fn trap_import() -> Result<()> { fn get_from_wrapper() { let store = Store::default(); let f = Func::wrap(&store, || {}); - assert!(f.clone().get0::<()>().is_ok()); - assert!(f.clone().get0::().is_err()); - assert!(f.clone().get1::<(), ()>().is_ok()); - assert!(f.clone().get1::().is_err()); - assert!(f.clone().get1::().is_err()); - assert!(f.clone().get2::<(), (), ()>().is_ok()); - assert!(f.clone().get2::().is_err()); - assert!(f.clone().get2::().is_err()); + assert!(f.get0::<()>().is_ok()); + assert!(f.get0::().is_err()); + assert!(f.get1::<(), ()>().is_ok()); + assert!(f.get1::().is_err()); + assert!(f.get1::().is_err()); + assert!(f.get2::<(), (), ()>().is_ok()); + assert!(f.get2::().is_err()); + assert!(f.get2::().is_err()); let f = Func::wrap(&store, || -> i32 { loop {} }); - assert!(f.clone().get0::().is_ok()); + assert!(f.get0::().is_ok()); let f = Func::wrap(&store, || -> f32 { loop {} }); - assert!(f.clone().get0::().is_ok()); + assert!(f.get0::().is_ok()); let f = Func::wrap(&store, || -> f64 { loop {} }); - assert!(f.clone().get0::().is_ok()); + assert!(f.get0::().is_ok()); let f = Func::wrap(&store, |_: i32| {}); - assert!(f.clone().get1::().is_ok()); - assert!(f.clone().get1::().is_err()); - assert!(f.clone().get1::().is_err()); - assert!(f.clone().get1::().is_err()); + assert!(f.get1::().is_ok()); + assert!(f.get1::().is_err()); + assert!(f.get1::().is_err()); + assert!(f.get1::().is_err()); let f = Func::wrap(&store, |_: i64| {}); - assert!(f.clone().get1::().is_ok()); + assert!(f.get1::().is_ok()); let f = Func::wrap(&store, |_: f32| {}); - assert!(f.clone().get1::().is_ok()); + assert!(f.get1::().is_ok()); let f = Func::wrap(&store, |_: f64| {}); - assert!(f.clone().get1::().is_ok()); + assert!(f.get1::().is_ok()); } #[test] @@ -241,16 +241,16 @@ fn get_from_signature() { let store = Store::default(); let ty = FuncType::new(Box::new([]), Box::new([])); let f = Func::new(&store, ty, |_, _, _| panic!()); - assert!(f.clone().get0::<()>().is_ok()); - assert!(f.clone().get0::().is_err()); - assert!(f.clone().get1::().is_err()); + assert!(f.get0::<()>().is_ok()); + assert!(f.get0::().is_err()); + assert!(f.get1::().is_err()); let ty = FuncType::new(Box::new([ValType::I32]), Box::new([ValType::F64])); let f = Func::new(&store, ty, |_, _, _| panic!()); - assert!(f.clone().get0::<()>().is_err()); - assert!(f.clone().get0::().is_err()); - assert!(f.clone().get1::().is_err()); - assert!(f.clone().get1::().is_ok()); + assert!(f.get0::<()>().is_err()); + assert!(f.get0::().is_err()); + assert!(f.get1::().is_err()); + assert!(f.get1::().is_ok()); } #[test] @@ -270,17 +270,17 @@ fn get_from_module() -> anyhow::Result<()> { )?; let instance = Instance::new(&module, &[])?; let f0 = instance.get_export("f0").unwrap().func().unwrap(); - assert!(f0.clone().get0::<()>().is_ok()); - assert!(f0.clone().get0::().is_err()); + assert!(f0.get0::<()>().is_ok()); + assert!(f0.get0::().is_err()); let f1 = instance.get_export("f1").unwrap().func().unwrap(); - assert!(f1.clone().get0::<()>().is_err()); - assert!(f1.clone().get1::().is_ok()); - assert!(f1.clone().get1::().is_err()); + assert!(f1.get0::<()>().is_err()); + assert!(f1.get1::().is_ok()); + assert!(f1.get1::().is_err()); let f2 = instance.get_export("f2").unwrap().func().unwrap(); - assert!(f2.clone().get0::<()>().is_err()); - assert!(f2.clone().get0::().is_ok()); - assert!(f2.clone().get1::().is_err()); - assert!(f2.clone().get1::().is_err()); + assert!(f2.get0::<()>().is_err()); + assert!(f2.get0::().is_ok()); + assert!(f2.get1::().is_err()); + assert!(f2.get1::().is_err()); Ok(()) } From 1148814a85a629d06d54cdeac00d9c292e0ca7e2 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 16 Apr 2020 16:44:08 -0700 Subject: [PATCH 13/24] Use `ExactSizeIterator` to avoid needing separate `num_*` methods. --- crates/api/src/instance.rs | 6 +++--- crates/api/src/module.rs | 20 +++++--------------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/crates/api/src/instance.rs b/crates/api/src/instance.rs index 7be541658cea..d45177745f96 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/src/instance.rs @@ -126,11 +126,11 @@ impl Instance { } } - if imports.len() != module.num_imports() { + if imports.len() != module.imports().len() { bail!( "wrong number of imports provided, {} != {}", imports.len(), - module.num_imports() + module.imports().len() ); } @@ -164,7 +164,7 @@ impl Instance { /// information about the export itself. The list returned here maps 1:1 with /// the list that [`Module::exports`] returns, and [`ExportType`](crate::ExportType) /// contains the name of each export. - pub fn exports<'me>(&'me self) -> impl Iterator + 'me { + pub fn exports<'me>(&'me self) -> impl ExactSizeIterator + 'me { let instance_handle = &self.instance_handle; let store = self.module.store(); self.instance_handle diff --git a/crates/api/src/module.rs b/crates/api/src/module.rs index 0f55f0b4ee7f..b417f3cfcd16 100644 --- a/crates/api/src/module.rs +++ b/crates/api/src/module.rs @@ -376,7 +376,7 @@ impl Module { /// # fn main() -> anyhow::Result<()> { /// # let store = Store::default(); /// let module = Module::new(&store, "(module)")?; - /// assert_eq!(module.num_imports(), 0); + /// assert_eq!(module.imports().len(), 0); /// # Ok(()) /// # } /// ``` @@ -393,7 +393,7 @@ impl Module { /// ) /// "#; /// let module = Module::new(&store, wat)?; - /// assert_eq!(module.num_imports(), 1); + /// assert_eq!(module.imports().len(), 1); /// let import = module.imports().next().unwrap(); /// assert_eq!(import.module, "host"); /// assert_eq!(import.name, "foo"); @@ -404,7 +404,7 @@ impl Module { /// # Ok(()) /// # } /// ``` - pub fn imports<'me>(&'me self) -> impl Iterator + 'me { + pub fn imports<'me>(&'me self) -> impl ExactSizeIterator + 'me { let module = self.inner.compiled.module_ref(); module .imports @@ -419,11 +419,6 @@ impl Module { }) } - /// Return the number of imports in this module. - pub fn num_imports(&self) -> usize { - self.inner.compiled.module_ref().imports.len() - } - /// Returns the list of exports that this [`Module`] has and will be /// available after instantiation. /// @@ -459,7 +454,7 @@ impl Module { /// ) /// "#; /// let module = Module::new(&store, wat)?; - /// assert_eq!(module.num_exports(), 2); + /// assert_eq!(module.exports().len(), 2); /// /// let foo = module.exports().next().unwrap(); /// assert_eq!(foo.name, "foo"); @@ -477,7 +472,7 @@ impl Module { /// # Ok(()) /// # } /// ``` - pub fn exports<'me>(&'me self) -> impl Iterator + 'me { + pub fn exports<'me>(&'me self) -> impl ExactSizeIterator + 'me { let module = self.inner.compiled.module_ref(); module.exports.iter().map(move |(name, entity_index)| { let r#type = entity_type(entity_index, module); @@ -488,11 +483,6 @@ impl Module { }) } - /// Return the number of exports in this module. - pub fn num_exports(&self) -> usize { - self.inner.compiled.module_ref().exports.len() - } - /// Returns the [`Store`] that this [`Module`] was compiled into. pub fn store(&self) -> &Store { &self.inner.store From e32dcf5ec57598aa989529707b815f589425a33c Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 16 Apr 2020 17:05:31 -0700 Subject: [PATCH 14/24] Rename `Extern::func()` etc. to `into_func()` etc. --- crates/api/src/externals.rs | 36 +++++++++---------- crates/api/src/func.rs | 12 +++---- crates/api/src/instance.rs | 5 --- crates/fuzzing/src/oracles.rs | 2 -- .../test-programs/tests/wasm_tests/runtime.rs | 2 +- crates/wast/src/wast.rs | 4 +-- docs/lang-rust.md | 2 +- docs/wasm-wat.md | 2 +- examples/fib-debug/main.rs | 2 +- examples/gcd.rs | 2 +- examples/hello.rs | 2 +- examples/linking.rs | 5 ++- examples/memory.rs | 8 ++--- examples/multi.rs | 4 +-- examples/wasi/main.rs | 2 +- src/commands/run.rs | 2 +- tests/all/custom_signal_handler.rs | 12 ++++--- tests/all/func.rs | 6 ++-- tests/all/globals.rs | 2 +- tests/all/import_calling_export.rs | 6 ++-- tests/all/import_indexes.rs | 2 +- tests/all/invoke_func_via_table.rs | 2 +- tests/all/linker.rs | 2 +- tests/all/memory_creator.rs | 2 +- tests/all/traps.rs | 20 +++++------ 25 files changed, 73 insertions(+), 73 deletions(-) diff --git a/crates/api/src/externals.rs b/crates/api/src/externals.rs index bf561678dbf6..5b1470ed9c51 100644 --- a/crates/api/src/externals.rs +++ b/crates/api/src/externals.rs @@ -34,7 +34,7 @@ impl Extern { /// Returns the underlying `Func`, if this external is a function. /// /// Returns `None` if this is not a function. - pub fn func(self) -> Option { + pub fn into_func(self) -> Option { match self { Extern::Func(func) => Some(func), _ => None, @@ -44,7 +44,7 @@ impl Extern { /// Returns the underlying `Global`, if this external is a global. /// /// Returns `None` if this is not a global. - pub fn global(self) -> Option { + pub fn into_global(self) -> Option { match self { Extern::Global(global) => Some(global), _ => None, @@ -54,7 +54,7 @@ impl Extern { /// Returns the underlying `Table`, if this external is a table. /// /// Returns `None` if this is not a table. - pub fn table(self) -> Option
{ + pub fn into_table(self) -> Option
{ match self { Extern::Table(table) => Some(table), _ => None, @@ -64,7 +64,7 @@ impl Extern { /// Returns the underlying `Memory`, if this external is a memory. /// /// Returns `None` if this is not a memory. - pub fn memory(self) -> Option { + pub fn into_memory(self) -> Option { match self { Extern::Memory(memory) => Some(memory), _ => None, @@ -702,7 +702,7 @@ impl Memory { /// let store = Store::default(); /// let module = Module::new(&store, "(module (memory (export \"mem\") 1))")?; /// let instance = Instance::new(&module, &[])?; - /// let memory = instance.get_export("mem").unwrap().memory().unwrap(); + /// let memory = instance.get_export("mem").unwrap().into_memory().unwrap(); /// let ty = memory.ty(); /// assert_eq!(ty.limits().min(), 1); /// # Ok(()) @@ -814,7 +814,7 @@ impl Memory { /// let store = Store::default(); /// let module = Module::new(&store, "(module (memory (export \"mem\") 1 2))")?; /// let instance = Instance::new(&module, &[])?; - /// let memory = instance.get_export("mem").unwrap().memory().unwrap(); + /// let memory = instance.get_export("mem").unwrap().into_memory().unwrap(); /// /// assert_eq!(memory.size(), 1); /// assert_eq!(memory.grow(1)?, 1); @@ -908,24 +908,24 @@ pub struct Export<'instance> { } impl<'instance> Export<'instance> { - /// Shorthand for `self.external.func()`. - pub fn func(self) -> Option { - self.external.func() + /// Shorthand for `self.external.into_func()`. + pub fn into_func(self) -> Option { + self.external.into_func() } - /// Shorthand for `self.external.table()`. - pub fn table(self) -> Option
{ - self.external.table() + /// Shorthand for `self.external.into_table()`. + pub fn into_table(self) -> Option
{ + self.external.into_table() } - /// Shorthand for `self.external.memory()`. - pub fn memory(self) -> Option { - self.external.memory() + /// Shorthand for `self.external.into_memory()`. + pub fn into_memory(self) -> Option { + self.external.into_memory() } - /// Shorthand for `self.external.global()`. - pub fn global(self) -> Option { - self.external.global() + /// Shorthand for `self.external.into_global()`. + pub fn into_global(self) -> Option { + self.external.into_global() } /// Shorthand for `self.external.ty()`. diff --git a/crates/api/src/func.rs b/crates/api/src/func.rs index e2ee519150df..67d6b80a2aa1 100644 --- a/crates/api/src/func.rs +++ b/crates/api/src/func.rs @@ -38,7 +38,7 @@ use wasmtime_runtime::{ExportFunction, VMTrampoline}; /// let store = Store::default(); /// let module = Module::new(&store, r#"(module (func (export "foo")))"#)?; /// let instance = Instance::new(&module, &[])?; -/// let foo = instance.exports().next().unwrap().func().expect("export wasn't a function"); +/// let foo = instance.exports().next().unwrap().into_func().expect("export wasn't a function"); /// /// // Work with `foo` as a `Func` at this point, such as calling it /// // dynamically... @@ -88,7 +88,7 @@ use wasmtime_runtime::{ExportFunction, VMTrampoline}; /// "#, /// )?; /// let instance = Instance::new(&module, &[add.into()])?; -/// let call_add_twice = instance.exports().next().unwrap().func().expect("export wasn't a function"); +/// let call_add_twice = instance.exports().next().unwrap().into_func().expect("export wasn't a function"); /// let call_add_twice = call_add_twice.get0::()?; /// /// assert_eq!(call_add_twice()?, 10); @@ -349,7 +349,7 @@ impl Func { /// "#, /// )?; /// let instance = Instance::new(&module, &[add.into()])?; - /// let foo = instance.exports().next().unwrap().func().unwrap().get2::()?; + /// let foo = instance.exports().next().unwrap().into_func().unwrap().get2::()?; /// assert_eq!(foo(1, 2)?, 3); /// # Ok(()) /// # } @@ -380,7 +380,7 @@ impl Func { /// "#, /// )?; /// let instance = Instance::new(&module, &[add.into()])?; - /// let foo = instance.exports().next().unwrap().func().unwrap().get2::()?; + /// let foo = instance.exports().next().unwrap().into_func().unwrap().get2::()?; /// assert_eq!(foo(1, 2)?, 3); /// assert!(foo(i32::max_value(), 1).is_err()); /// # Ok(()) @@ -413,7 +413,7 @@ impl Func { /// "#, /// )?; /// let instance = Instance::new(&module, &[debug.into()])?; - /// let foo = instance.exports().next().unwrap().func().unwrap().get0::<()>()?; + /// let foo = instance.exports().next().unwrap().into_func().unwrap().get0::<()>()?; /// foo()?; /// # Ok(()) /// # } @@ -469,7 +469,7 @@ impl Func { /// "#, /// )?; /// let instance = Instance::new(&module, &[log_str.into()])?; - /// let foo = instance.exports().next().unwrap().func().unwrap().get0::<()>()?; + /// let foo = instance.exports().next().unwrap().into_func().unwrap().get0::<()>()?; /// foo()?; /// # Ok(()) /// # } diff --git a/crates/api/src/instance.rs b/crates/api/src/instance.rs index d45177745f96..2ac135a9bae9 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/src/instance.rs @@ -159,11 +159,6 @@ impl Instance { } /// Returns the list of exported items from this [`Instance`]. - /// - /// The actual returned value is a tuple containing the name of an export and - /// information about the export itself. The list returned here maps 1:1 with - /// the list that [`Module::exports`] returns, and [`ExportType`](crate::ExportType) - /// contains the name of each export. pub fn exports<'me>(&'me self) -> impl ExactSizeIterator + 'me { let instance_handle = &self.instance_handle; let store = self.module.store(); diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index ecb9be11e1f7..838f076ce736 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -171,7 +171,6 @@ pub fn differential_execution( let funcs = module .exports() - .iter() .filter_map(|e| { if let ExternType::Func(_) = e.ty() { Some(e.name()) @@ -378,7 +377,6 @@ pub fn make_api_calls(api: crate::generators::api::ApiCalls) { let funcs = instance .exports() - .iter() .filter_map(|e| match e { Extern::Func(f) => Some(f.clone()), _ => None, diff --git a/crates/test-programs/tests/wasm_tests/runtime.rs b/crates/test-programs/tests/wasm_tests/runtime.rs index 54883c076e40..688d5f965241 100644 --- a/crates/test-programs/tests/wasm_tests/runtime.rs +++ b/crates/test-programs/tests/wasm_tests/runtime.rs @@ -72,7 +72,7 @@ pub fn instantiate( .clone(); export - .func() + .into_func() .context("expected export to be a func")? .call(&[])?; diff --git a/crates/wast/src/wast.rs b/crates/wast/src/wast.rs index 2ac3bbd74098..96fbc3ad4258 100644 --- a/crates/wast/src/wast.rs +++ b/crates/wast/src/wast.rs @@ -156,7 +156,7 @@ impl WastContext { ) -> Result { let func = self .get_export(instance_name, field)? - .func() + .into_func() .ok_or_else(|| anyhow!("no function named `{}`", field))?; Ok(match func.call(args) { Ok(result) => Outcome::Ok(result.into()), @@ -168,7 +168,7 @@ impl WastContext { fn get(&mut self, instance_name: Option<&str>, field: &str) -> Result { let global = self .get_export(instance_name, field)? - .global() + .into_global() .ok_or_else(|| anyhow!("no global named `{}`", field))?; Ok(Outcome::Ok(vec![global.get()])) } diff --git a/docs/lang-rust.md b/docs/lang-rust.md index c87d1bffcea5..3d0ff52c1778 100644 --- a/docs/lang-rust.md +++ b/docs/lang-rust.md @@ -76,7 +76,7 @@ fn main() -> Result<(), Box> { // run it. let answer = instance.get_export("answer") .expect("export named `answer` not found") - .func() + .into_func() .expect("export `answer` was not a function"); // There's a few ways we can call the `answer` `Func` value. The easiest diff --git a/docs/wasm-wat.md b/docs/wasm-wat.md index e1f519b332f3..db7c23268128 100644 --- a/docs/wasm-wat.md +++ b/docs/wasm-wat.md @@ -47,7 +47,7 @@ let wat = r#" "#; let module = Module::new(&store, wat)?; let instance = Instance::new(&module, &[])?; -let add = instance.get_export("add").and_then(|f| f.func()).unwrap(); +let add = instance.get_export("add").and_then(|f| f.into_func()).unwrap(); let add = add.get2::()?; println!("1 + 2 = {}", add(1, 2)?); # Ok(()) diff --git a/examples/fib-debug/main.rs b/examples/fib-debug/main.rs index 79a5841c4af0..7028a4ed786f 100644 --- a/examples/fib-debug/main.rs +++ b/examples/fib-debug/main.rs @@ -23,7 +23,7 @@ fn main() -> Result<()> { // Invoke `fib` export let fib = instance .get_export("fib") - .and_then(|e| e.func()) + .and_then(|e| e.into_func()) .ok_or(anyhow::format_err!("failed to find `fib` function export"))? .get1::()?; println!("fib(6) = {}", fib(6)?); diff --git a/examples/gcd.rs b/examples/gcd.rs index f310672f3eec..f9832b494e2b 100644 --- a/examples/gcd.rs +++ b/examples/gcd.rs @@ -17,7 +17,7 @@ fn main() -> Result<()> { // Invoke `gcd` export let gcd = instance .get_export("gcd") - .and_then(|e| e.func()) + .and_then(|e| e.into_func()) .ok_or(anyhow::format_err!("failed to find `gcd` function export"))? .get2::()?; diff --git a/examples/hello.rs b/examples/hello.rs index a23974942ad6..85d8926c31c6 100644 --- a/examples/hello.rs +++ b/examples/hello.rs @@ -36,7 +36,7 @@ fn main() -> Result<()> { println!("Extracting export..."); let run = instance .get_export("run") - .and_then(|e| e.func()) + .and_then(|e| e.into_func()) .ok_or(anyhow::format_err!("failed to find `run` function export"))? .get0::<()>()?; diff --git a/examples/linking.rs b/examples/linking.rs index 88dfb9956cda..8a0188ecb9b4 100644 --- a/examples/linking.rs +++ b/examples/linking.rs @@ -26,7 +26,10 @@ fn main() -> Result<()> { // And with that we can perform the final link and the execute the module. let linking1 = linker.instantiate(&linking1)?; - let run = linking1.get_export("run").and_then(|e| e.func()).unwrap(); + let run = linking1 + .get_export("run") + .and_then(|e| e.into_func()) + .unwrap(); let run = run.get0::<()>()?; run()?; Ok(()) diff --git a/examples/memory.rs b/examples/memory.rs index b86ecc34bcb4..d60cb18a85b2 100644 --- a/examples/memory.rs +++ b/examples/memory.rs @@ -19,21 +19,21 @@ fn main() -> Result<()> { // Load up our exports from the instance let memory = instance .get_export("memory") - .and_then(|e| e.memory()) + .and_then(|e| e.into_memory()) .ok_or(anyhow::format_err!("failed to find `memory` export"))?; let size = instance .get_export("size") - .and_then(|e| e.func()) + .and_then(|e| e.into_func()) .ok_or(anyhow::format_err!("failed to find `size` export"))? .get0::()?; let load = instance .get_export("load") - .and_then(|e| e.func()) + .and_then(|e| e.into_func()) .ok_or(anyhow::format_err!("failed to find `load` export"))? .get1::()?; let store = instance .get_export("store") - .and_then(|e| e.func()) + .and_then(|e| e.into_func()) .ok_or(anyhow::format_err!("failed to find `store` export"))? .get2::()?; diff --git a/examples/multi.rs b/examples/multi.rs index 9d04ec4e2952..0357c91776dc 100644 --- a/examples/multi.rs +++ b/examples/multi.rs @@ -44,7 +44,7 @@ fn main() -> Result<()> { println!("Extracting export..."); let g = instance .get_export("g") - .and_then(|e| e.func()) + .and_then(|e| e.into_func()) .ok_or(format_err!("failed to find export `g`"))?; // Call `$g`. @@ -61,7 +61,7 @@ fn main() -> Result<()> { println!("Calling export \"round_trip_many\"..."); let round_trip_many = instance .get_export("round_trip_many") - .and_then(|e| e.func()) + .and_then(|e| e.into_func()) .ok_or(format_err!("failed to find export `round_trip_many`"))?; let args = vec![ Val::I64(0), diff --git a/examples/wasi/main.rs b/examples/wasi/main.rs index ab5ba8009082..9ae8b8bbd5b1 100644 --- a/examples/wasi/main.rs +++ b/examples/wasi/main.rs @@ -34,7 +34,7 @@ fn main() -> Result<()> { let instance = Instance::new(&module, &imports)?; let start = instance .get_export("_start") - .and_then(|e| e.func()) + .and_then(|e| e.into_func()) .unwrap(); let start = start.get0::<()>()?; start()?; diff --git a/src/commands/run.rs b/src/commands/run.rs index 4a8c61a30c05..e7197bfbae5d 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -240,7 +240,7 @@ impl RunCommand { fn invoke_export(&self, instance: Instance, name: &str) -> Result<()> { let func = if let Some(export) = instance.get_export(name) { - if let Some(func) = export.func() { + if let Some(func) = export.into_func() { func } else { bail!("export of `{}` wasn't a function", name) diff --git a/tests/all/custom_signal_handler.rs b/tests/all/custom_signal_handler.rs index d234ac6ba77a..55287172c5cf 100644 --- a/tests/all/custom_signal_handler.rs +++ b/tests/all/custom_signal_handler.rs @@ -41,7 +41,7 @@ mod tests { let ret = instance .get_export(func_name) .unwrap() - .func() + .into_func() .unwrap() .call(&[])?; Ok(ret) @@ -49,7 +49,11 @@ mod tests { // Locate "memory" export, get base address and size and set memory protection to PROT_NONE fn set_up_memory(instance: &Instance) -> (*mut u8, usize) { - let mem_export = instance.get_export("memory").unwrap().memory().unwrap(); + let mem_export = instance + .get_export("memory") + .unwrap() + .into_memory() + .unwrap(); let base = mem_export.data_ptr(); let length = mem_export.data_size(); @@ -132,7 +136,7 @@ mod tests { let read_func = exports .next() .unwrap() - .func() + .into_func() .expect("expected a 'read' func in the module"); println!("calling read..."); let result = read_func.call(&[]).expect("expected function not to trap"); @@ -143,7 +147,7 @@ mod tests { let read_out_of_bounds_func = exports .next() .unwrap() - .func() + .into_func() .expect("expected a 'read_out_of_bounds' func in the module"); println!("calling read_out_of_bounds..."); let trap = read_out_of_bounds_func diff --git a/tests/all/func.rs b/tests/all/func.rs index 431de864536e..f04f6cee5e6e 100644 --- a/tests/all/func.rs +++ b/tests/all/func.rs @@ -269,14 +269,14 @@ fn get_from_module() -> anyhow::Result<()> { "#, )?; let instance = Instance::new(&module, &[])?; - let f0 = instance.get_export("f0").unwrap().func().unwrap(); + let f0 = instance.get_export("f0").unwrap().into_func().unwrap(); assert!(f0.get0::<()>().is_ok()); assert!(f0.get0::().is_err()); - let f1 = instance.get_export("f1").unwrap().func().unwrap(); + let f1 = instance.get_export("f1").unwrap().into_func().unwrap(); assert!(f1.get0::<()>().is_err()); assert!(f1.get1::().is_ok()); assert!(f1.get1::().is_err()); - let f2 = instance.get_export("f2").unwrap().func().unwrap(); + let f2 = instance.get_export("f2").unwrap().into_func().unwrap(); assert!(f2.get0::<()>().is_err()); assert!(f2.get0::().is_ok()); assert!(f2.get1::().is_err()); diff --git a/tests/all/globals.rs b/tests/all/globals.rs index c8350eb1c1ed..067474bce20e 100644 --- a/tests/all/globals.rs +++ b/tests/all/globals.rs @@ -70,7 +70,7 @@ fn use_after_drop() -> anyhow::Result<()> { "#, )?; let instance = Instance::new(&module, &[])?; - let g = instance.exports().next().unwrap().global().unwrap(); + let g = instance.exports().next().unwrap().into_global().unwrap(); assert_eq!(g.get().i32(), Some(100)); g.set(101.into())?; drop(instance); diff --git a/tests/all/import_calling_export.rs b/tests/all/import_calling_export.rs index 001ff6a92344..2d38b11e14ca 100644 --- a/tests/all/import_calling_export.rs +++ b/tests/all/import_calling_export.rs @@ -45,14 +45,14 @@ fn test_import_calling_export() { let run_func = exports .next() .unwrap() - .func() + .into_func() .expect("expected a run func in the module"); *other.borrow_mut() = Some( exports .next() .unwrap() - .func() + .into_func() .expect("expected an other func in the module"), ); @@ -91,7 +91,7 @@ fn test_returns_incorrect_type() -> Result<()> { let run_func = exports .next() .unwrap() - .func() + .into_func() .expect("expected a run func in the module"); let trap = run_func diff --git a/tests/all/import_indexes.rs b/tests/all/import_indexes.rs index befef2214388..86a7c3783109 100644 --- a/tests/all/import_indexes.rs +++ b/tests/all/import_indexes.rs @@ -43,7 +43,7 @@ fn same_import_names_still_distinct() -> anyhow::Result<()> { ]; let instance = Instance::new(&module, &imports)?; - let func = instance.get_export("foo").unwrap().func().unwrap(); + let func = instance.get_export("foo").unwrap().into_func().unwrap(); let results = func.call(&[])?; assert_eq!(results.len(), 1); match results[0] { diff --git a/tests/all/invoke_func_via_table.rs b/tests/all/invoke_func_via_table.rs index 28ba779ec998..57a2ae51bc12 100644 --- a/tests/all/invoke_func_via_table.rs +++ b/tests/all/invoke_func_via_table.rs @@ -19,7 +19,7 @@ fn test_invoke_func_via_table() -> Result<()> { let f = instance .get_export("table") .unwrap() - .table() + .into_table() .unwrap() .get(0) .unwrap() diff --git a/tests/all/linker.rs b/tests/all/linker.rs index 2fcb58980579..3430aeb9c529 100644 --- a/tests/all/linker.rs +++ b/tests/all/linker.rs @@ -90,7 +90,7 @@ fn interposition() -> Result<()> { )?; } let instance = linker.instantiate(&module)?; - let func = instance.get_export("export").unwrap().func().unwrap(); + let func = instance.get_export("export").unwrap().into_func().unwrap(); let func = func.get0::()?; assert_eq!(func()?, 112); Ok(()) diff --git a/tests/all/memory_creator.rs b/tests/all/memory_creator.rs index e5825da431ab..cddb89464224 100644 --- a/tests/all/memory_creator.rs +++ b/tests/all/memory_creator.rs @@ -173,7 +173,7 @@ mod not_for_windows { instance2 .get_export("memory") .unwrap() - .memory() + .into_memory() .unwrap() .size(), 2 diff --git a/tests/all/traps.rs b/tests/all/traps.rs index 5a8c10658906..42537734f616 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -21,7 +21,7 @@ fn test_trap_return() -> Result<()> { .exports() .next() .unwrap() - .func() + .into_func() .expect("expected function export"); let e = run_func @@ -51,7 +51,7 @@ fn test_trap_trace() -> Result<()> { .exports() .next() .unwrap() - .func() + .into_func() .expect("expected function export"); let e = run_func @@ -101,7 +101,7 @@ fn test_trap_trace_cb() -> Result<()> { .exports() .next() .unwrap() - .func() + .into_func() .expect("expected function export"); let e = run_func @@ -136,7 +136,7 @@ fn test_trap_stack_overflow() -> Result<()> { .exports() .next() .unwrap() - .func() + .into_func() .expect("expected function export"); let e = run_func @@ -175,7 +175,7 @@ fn trap_display_pretty() -> Result<()> { .exports() .next() .unwrap() - .func() + .into_func() .expect("expected function export"); let e = run_func.call(&[]).err().expect("error calling function"); @@ -222,7 +222,7 @@ fn trap_display_multi_module() -> Result<()> { .exports() .next() .unwrap() - .func() + .into_func() .expect("expected function export"); let e = bar2.call(&[]).err().expect("error calling function"); @@ -286,14 +286,14 @@ fn rust_panic_import() -> Result<()> { Func::wrap(&store, || panic!("this is another panic")).into(), ], )?; - let func = instance.exports().next().unwrap().func().unwrap(); + let func = instance.exports().next().unwrap().into_func().unwrap(); let err = panic::catch_unwind(AssertUnwindSafe(|| { drop(func.call(&[])); })) .unwrap_err(); assert_eq!(err.downcast_ref::<&'static str>(), Some(&"this is a panic")); - let func = instance.exports().nth(1).unwrap().func().unwrap(); + let func = instance.exports().nth(1).unwrap().into_func().unwrap(); let err = panic::catch_unwind(AssertUnwindSafe(|| { drop(func.call(&[])); })) @@ -351,7 +351,7 @@ fn mismatched_arguments() -> Result<()> { let module = Module::new(&store, &binary)?; let instance = Instance::new(&module, &[])?; - let func = instance.exports().next().unwrap().func().unwrap(); + let func = instance.exports().next().unwrap().into_func().unwrap(); assert_eq!( func.call(&[]).unwrap_err().to_string(), "expected 1 arguments, got 0" @@ -435,7 +435,7 @@ fn present_after_module_drop() -> Result<()> { let store = Store::default(); let module = Module::new(&store, r#"(func (export "foo") unreachable)"#)?; let instance = Instance::new(&module, &[])?; - let func = instance.exports()[0].func().unwrap().clone(); + let func = instance.exports().next().unwrap().into_func().unwrap(); println!("asserting before we drop modules"); assert_trap(func.call(&[]).unwrap_err().downcast()?); From 5ddf2b3b7fa1c0a887077c6cc040dcfff38373e7 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 16 Apr 2020 17:33:41 -0700 Subject: [PATCH 15/24] Revise examples to avoid using `nth`. --- crates/api/src/module.rs | 5 +++-- tests/all/traps.rs | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/api/src/module.rs b/crates/api/src/module.rs index b417f3cfcd16..9bae3668ee07 100644 --- a/crates/api/src/module.rs +++ b/crates/api/src/module.rs @@ -456,14 +456,15 @@ impl Module { /// let module = Module::new(&store, wat)?; /// assert_eq!(module.exports().len(), 2); /// - /// let foo = module.exports().next().unwrap(); + /// let mut exports = module.exports(); + /// let foo = exports.next().unwrap(); /// assert_eq!(foo.name, "foo"); /// match foo.ty { /// ExternType::Func(_) => { /* ... */ } /// _ => panic!("unexpected export type!"), /// } /// - /// let memory = module.exports().nth(1).unwrap(); + /// let memory = exports.next().unwrap(); /// assert_eq!(memory.name, "memory"); /// match memory.ty { /// ExternType::Memory(_) => { /* ... */ } diff --git a/tests/all/traps.rs b/tests/all/traps.rs index 42537734f616..153938c4674e 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -286,14 +286,15 @@ fn rust_panic_import() -> Result<()> { Func::wrap(&store, || panic!("this is another panic")).into(), ], )?; - let func = instance.exports().next().unwrap().into_func().unwrap(); + let mut exports = instance.exports(); + let func = exports.next().unwrap().into_func().unwrap(); let err = panic::catch_unwind(AssertUnwindSafe(|| { drop(func.call(&[])); })) .unwrap_err(); assert_eq!(err.downcast_ref::<&'static str>(), Some(&"this is a panic")); - let func = instance.exports().nth(1).unwrap().into_func().unwrap(); + let func = exports.next().unwrap().into_func().unwrap(); let err = panic::catch_unwind(AssertUnwindSafe(|| { drop(func.call(&[])); })) From 366e8c4af47f48c26482051caadf8a406d94953d Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 17 Apr 2020 17:57:22 -0700 Subject: [PATCH 16/24] Add convenience methods to instance for getting specific extern types. --- crates/api/src/externals.rs | 4 +-- crates/api/src/instance.rs | 35 ++++++++++++++++++- crates/fuzzing/src/oracles.rs | 8 ++--- .../test-programs/tests/wasm_tests/runtime.rs | 5 +-- docs/wasm-wat.md | 2 +- examples/fib-debug/main.rs | 3 +- examples/gcd.rs | 3 +- examples/hello.rs | 3 +- examples/linking.rs | 5 +-- examples/memory.rs | 12 +++---- examples/multi.rs | 6 ++-- examples/wasi/main.rs | 5 +-- tests/all/custom_signal_handler.rs | 13 ++----- tests/all/func.rs | 6 ++-- tests/all/import_indexes.rs | 2 +- tests/all/invoke_func_via_table.rs | 4 +-- tests/all/linker.rs | 2 +- tests/all/memory_creator.rs | 10 +----- 18 files changed, 61 insertions(+), 67 deletions(-) diff --git a/crates/api/src/externals.rs b/crates/api/src/externals.rs index 5b1470ed9c51..e94fc40632b8 100644 --- a/crates/api/src/externals.rs +++ b/crates/api/src/externals.rs @@ -702,7 +702,7 @@ impl Memory { /// let store = Store::default(); /// let module = Module::new(&store, "(module (memory (export \"mem\") 1))")?; /// let instance = Instance::new(&module, &[])?; - /// let memory = instance.get_export("mem").unwrap().into_memory().unwrap(); + /// let memory = instance.get_memory("mem").unwrap(); /// let ty = memory.ty(); /// assert_eq!(ty.limits().min(), 1); /// # Ok(()) @@ -814,7 +814,7 @@ impl Memory { /// let store = Store::default(); /// let module = Module::new(&store, "(module (memory (export \"mem\") 1 2))")?; /// let instance = Instance::new(&module, &[])?; - /// let memory = instance.get_export("mem").unwrap().into_memory().unwrap(); + /// let memory = instance.get_memory("mem").unwrap(); /// /// assert_eq!(memory.size(), 1); /// assert_eq!(memory.grow(1)?, 1); diff --git a/crates/api/src/instance.rs b/crates/api/src/instance.rs index 2ac135a9bae9..ad49cadac398 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/src/instance.rs @@ -1,4 +1,5 @@ -use crate::externals::{Export, Extern}; +use crate::externals::{Export, Extern, Global, Memory, Table}; +use crate::func::Func; use crate::module::Module; use crate::runtime::{Config, Store}; use crate::trap::Trap; @@ -188,6 +189,38 @@ impl Instance { )) } + /// Looks up an exported [`Func`] value by name. + /// + /// Returns `None` if there was no export named `name`, or if there was but + /// it wasn't a function. + pub fn get_func(&self, name: &str) -> Option { + self.get_export(name)?.into_func() + } + + /// Looks up an exported [`Table`] value by name. + /// + /// Returns `None` if there was no export named `name`, or if there was but + /// it wasn't a table. + pub fn get_table(&self, name: &str) -> Option
{ + self.get_export(name)?.into_table() + } + + /// Looks up an exported [`Memory`] value by name. + /// + /// Returns `None` if there was no export named `name`, or if there was but + /// it wasn't a memory. + pub fn get_memory(&self, name: &str) -> Option { + self.get_export(name)?.into_memory() + } + + /// Looks up an exported [`Global`] value by name. + /// + /// Returns `None` if there was no export named `name`, or if there was but + /// it wasn't a global. + pub fn get_global(&self, name: &str) -> Option { + self.get_export(name)?.into_global() + } + #[doc(hidden)] pub fn handle(&self) -> &InstanceHandle { &self.instance_handle diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index 838f076ce736..b786714a2015 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -185,13 +185,11 @@ pub fn differential_execution( // infinite loop when calling another export. init_hang_limit(&instance); - let f = match instance + let f = instance .get_export(&name) .expect("instance should have export from module") - { - Extern::Func(f) => f.clone(), - _ => panic!("export should be a function"), - }; + .into_func() + .expect("export should be a function"); let ty = f.ty(); let params = match dummy::dummy_values(ty.params()) { diff --git a/crates/test-programs/tests/wasm_tests/runtime.rs b/crates/test-programs/tests/wasm_tests/runtime.rs index 688d5f965241..f8785cc21b7d 100644 --- a/crates/test-programs/tests/wasm_tests/runtime.rs +++ b/crates/test-programs/tests/wasm_tests/runtime.rs @@ -66,12 +66,9 @@ pub fn instantiate( bin_name, ))?; - let export = instance + instance .get_export("_start") .context("expected a _start export")? - .clone(); - - export .into_func() .context("expected export to be a func")? .call(&[])?; diff --git a/docs/wasm-wat.md b/docs/wasm-wat.md index db7c23268128..0f9153771696 100644 --- a/docs/wasm-wat.md +++ b/docs/wasm-wat.md @@ -47,7 +47,7 @@ let wat = r#" "#; let module = Module::new(&store, wat)?; let instance = Instance::new(&module, &[])?; -let add = instance.get_export("add").and_then(|f| f.into_func()).unwrap(); +let add = instance.get_func("add").unwrap(); let add = add.get2::()?; println!("1 + 2 = {}", add(1, 2)?); # Ok(()) diff --git a/examples/fib-debug/main.rs b/examples/fib-debug/main.rs index 7028a4ed786f..ff35b8c83c10 100644 --- a/examples/fib-debug/main.rs +++ b/examples/fib-debug/main.rs @@ -22,8 +22,7 @@ fn main() -> Result<()> { // Invoke `fib` export let fib = instance - .get_export("fib") - .and_then(|e| e.into_func()) + .get_func("fib") .ok_or(anyhow::format_err!("failed to find `fib` function export"))? .get1::()?; println!("fib(6) = {}", fib(6)?); diff --git a/examples/gcd.rs b/examples/gcd.rs index f9832b494e2b..067ab7b2dff8 100644 --- a/examples/gcd.rs +++ b/examples/gcd.rs @@ -16,8 +16,7 @@ fn main() -> Result<()> { // Invoke `gcd` export let gcd = instance - .get_export("gcd") - .and_then(|e| e.into_func()) + .get_func("gcd") .ok_or(anyhow::format_err!("failed to find `gcd` function export"))? .get2::()?; diff --git a/examples/hello.rs b/examples/hello.rs index 85d8926c31c6..00772b22eb01 100644 --- a/examples/hello.rs +++ b/examples/hello.rs @@ -35,8 +35,7 @@ fn main() -> Result<()> { // Next we poke around a bit to extract the `run` function from the module. println!("Extracting export..."); let run = instance - .get_export("run") - .and_then(|e| e.into_func()) + .get_func("run") .ok_or(anyhow::format_err!("failed to find `run` function export"))? .get0::<()>()?; diff --git a/examples/linking.rs b/examples/linking.rs index 8a0188ecb9b4..4f9548739e6c 100644 --- a/examples/linking.rs +++ b/examples/linking.rs @@ -26,10 +26,7 @@ fn main() -> Result<()> { // And with that we can perform the final link and the execute the module. let linking1 = linker.instantiate(&linking1)?; - let run = linking1 - .get_export("run") - .and_then(|e| e.into_func()) - .unwrap(); + let run = linking1.get_func("run").unwrap(); let run = run.get0::<()>()?; run()?; Ok(()) diff --git a/examples/memory.rs b/examples/memory.rs index d60cb18a85b2..6f952dbb21b1 100644 --- a/examples/memory.rs +++ b/examples/memory.rs @@ -18,22 +18,18 @@ fn main() -> Result<()> { // Load up our exports from the instance let memory = instance - .get_export("memory") - .and_then(|e| e.into_memory()) + .get_memory("memory") .ok_or(anyhow::format_err!("failed to find `memory` export"))?; let size = instance - .get_export("size") - .and_then(|e| e.into_func()) + .get_func("size") .ok_or(anyhow::format_err!("failed to find `size` export"))? .get0::()?; let load = instance - .get_export("load") - .and_then(|e| e.into_func()) + .get_func("load") .ok_or(anyhow::format_err!("failed to find `load` export"))? .get1::()?; let store = instance - .get_export("store") - .and_then(|e| e.into_func()) + .get_func("store") .ok_or(anyhow::format_err!("failed to find `store` export"))? .get2::()?; diff --git a/examples/multi.rs b/examples/multi.rs index 0357c91776dc..0a5af6389913 100644 --- a/examples/multi.rs +++ b/examples/multi.rs @@ -43,8 +43,7 @@ fn main() -> Result<()> { // Extract exports. println!("Extracting export..."); let g = instance - .get_export("g") - .and_then(|e| e.into_func()) + .get_func("g") .ok_or(format_err!("failed to find export `g`"))?; // Call `$g`. @@ -60,8 +59,7 @@ fn main() -> Result<()> { // Call `$round_trip_many`. println!("Calling export \"round_trip_many\"..."); let round_trip_many = instance - .get_export("round_trip_many") - .and_then(|e| e.into_func()) + .get_func("round_trip_many") .ok_or(format_err!("failed to find export `round_trip_many`"))?; let args = vec![ Val::I64(0), diff --git a/examples/wasi/main.rs b/examples/wasi/main.rs index 9ae8b8bbd5b1..66151937eca4 100644 --- a/examples/wasi/main.rs +++ b/examples/wasi/main.rs @@ -32,10 +32,7 @@ fn main() -> Result<()> { // Instance our module with the imports we've created, then we can run the // standard wasi `_start` function. let instance = Instance::new(&module, &imports)?; - let start = instance - .get_export("_start") - .and_then(|e| e.into_func()) - .unwrap(); + let start = instance.get_func("_start").unwrap(); let start = start.get0::<()>()?; start()?; Ok(()) diff --git a/tests/all/custom_signal_handler.rs b/tests/all/custom_signal_handler.rs index 55287172c5cf..f40614d5fe2a 100644 --- a/tests/all/custom_signal_handler.rs +++ b/tests/all/custom_signal_handler.rs @@ -38,22 +38,13 @@ mod tests { "#; fn invoke_export(instance: &Instance, func_name: &str) -> Result> { - let ret = instance - .get_export(func_name) - .unwrap() - .into_func() - .unwrap() - .call(&[])?; + let ret = instance.get_func(func_name).unwrap().call(&[])?; Ok(ret) } // Locate "memory" export, get base address and size and set memory protection to PROT_NONE fn set_up_memory(instance: &Instance) -> (*mut u8, usize) { - let mem_export = instance - .get_export("memory") - .unwrap() - .into_memory() - .unwrap(); + let mem_export = instance.get_memory("memory").unwrap(); let base = mem_export.data_ptr(); let length = mem_export.data_size(); diff --git a/tests/all/func.rs b/tests/all/func.rs index f04f6cee5e6e..1cbb3e081406 100644 --- a/tests/all/func.rs +++ b/tests/all/func.rs @@ -269,14 +269,14 @@ fn get_from_module() -> anyhow::Result<()> { "#, )?; let instance = Instance::new(&module, &[])?; - let f0 = instance.get_export("f0").unwrap().into_func().unwrap(); + let f0 = instance.get_func("f0").unwrap(); assert!(f0.get0::<()>().is_ok()); assert!(f0.get0::().is_err()); - let f1 = instance.get_export("f1").unwrap().into_func().unwrap(); + let f1 = instance.get_func("f1").unwrap(); assert!(f1.get0::<()>().is_err()); assert!(f1.get1::().is_ok()); assert!(f1.get1::().is_err()); - let f2 = instance.get_export("f2").unwrap().into_func().unwrap(); + let f2 = instance.get_func("f2").unwrap(); assert!(f2.get0::<()>().is_err()); assert!(f2.get0::().is_ok()); assert!(f2.get1::().is_err()); diff --git a/tests/all/import_indexes.rs b/tests/all/import_indexes.rs index 86a7c3783109..d13d100a9f0f 100644 --- a/tests/all/import_indexes.rs +++ b/tests/all/import_indexes.rs @@ -43,7 +43,7 @@ fn same_import_names_still_distinct() -> anyhow::Result<()> { ]; let instance = Instance::new(&module, &imports)?; - let func = instance.get_export("foo").unwrap().into_func().unwrap(); + let func = instance.get_func("foo").unwrap(); let results = func.call(&[])?; assert_eq!(results.len(), 1); match results[0] { diff --git a/tests/all/invoke_func_via_table.rs b/tests/all/invoke_func_via_table.rs index 57a2ae51bc12..8f59fb17d8d5 100644 --- a/tests/all/invoke_func_via_table.rs +++ b/tests/all/invoke_func_via_table.rs @@ -17,9 +17,7 @@ fn test_invoke_func_via_table() -> Result<()> { let instance = Instance::new(&module, &[]).context("> Error instantiating module!")?; let f = instance - .get_export("table") - .unwrap() - .into_table() + .get_table("table") .unwrap() .get(0) .unwrap() diff --git a/tests/all/linker.rs b/tests/all/linker.rs index 3430aeb9c529..efca285267f0 100644 --- a/tests/all/linker.rs +++ b/tests/all/linker.rs @@ -90,7 +90,7 @@ fn interposition() -> Result<()> { )?; } let instance = linker.instantiate(&module)?; - let func = instance.get_export("export").unwrap().into_func().unwrap(); + let func = instance.get_func("export").unwrap(); let func = func.get0::()?; assert_eq!(func()?, 112); Ok(()) diff --git a/tests/all/memory_creator.rs b/tests/all/memory_creator.rs index cddb89464224..ed80cc72c95f 100644 --- a/tests/all/memory_creator.rs +++ b/tests/all/memory_creator.rs @@ -169,15 +169,7 @@ mod not_for_windows { assert_eq!(*mem_creator.num_created_memories.lock().unwrap(), 2); - assert_eq!( - instance2 - .get_export("memory") - .unwrap() - .into_memory() - .unwrap() - .size(), - 2 - ); + assert_eq!(instance2.get_memory("memory").unwrap().size(), 2); // we take the lock outside the assert, so it won't get poisoned on assert failure let tot_pages = *mem_creator.num_total_pages.lock().unwrap(); From 18112567d061e778ac307888db4b33cd2cfd7cf4 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 17 Apr 2020 21:25:59 -0700 Subject: [PATCH 17/24] Use the convenience functions in more tests and examples. --- crates/api/src/func.rs | 12 +++---- docs/lang-rust.md | 6 ++-- tests/all/custom_signal_handler.rs | 14 +++----- tests/all/globals.rs | 2 +- tests/all/import_calling_export.rs | 20 ++++------- tests/all/traps.rs | 55 +++++++----------------------- 6 files changed, 31 insertions(+), 78 deletions(-) diff --git a/crates/api/src/func.rs b/crates/api/src/func.rs index 67d6b80a2aa1..719b65fbf804 100644 --- a/crates/api/src/func.rs +++ b/crates/api/src/func.rs @@ -38,7 +38,7 @@ use wasmtime_runtime::{ExportFunction, VMTrampoline}; /// let store = Store::default(); /// let module = Module::new(&store, r#"(module (func (export "foo")))"#)?; /// let instance = Instance::new(&module, &[])?; -/// let foo = instance.exports().next().unwrap().into_func().expect("export wasn't a function"); +/// let foo = instance.get_func("foo").expect("export wasn't a function"); /// /// // Work with `foo` as a `Func` at this point, such as calling it /// // dynamically... @@ -88,7 +88,7 @@ use wasmtime_runtime::{ExportFunction, VMTrampoline}; /// "#, /// )?; /// let instance = Instance::new(&module, &[add.into()])?; -/// let call_add_twice = instance.exports().next().unwrap().into_func().expect("export wasn't a function"); +/// let call_add_twice = instance.get_func("call_add_twice").expect("export wasn't a function"); /// let call_add_twice = call_add_twice.get0::()?; /// /// assert_eq!(call_add_twice()?, 10); @@ -349,7 +349,7 @@ impl Func { /// "#, /// )?; /// let instance = Instance::new(&module, &[add.into()])?; - /// let foo = instance.exports().next().unwrap().into_func().unwrap().get2::()?; + /// let foo = instance.get_func("foo").unwrap().get2::()?; /// assert_eq!(foo(1, 2)?, 3); /// # Ok(()) /// # } @@ -380,7 +380,7 @@ impl Func { /// "#, /// )?; /// let instance = Instance::new(&module, &[add.into()])?; - /// let foo = instance.exports().next().unwrap().into_func().unwrap().get2::()?; + /// let foo = instance.get_func("foo").unwrap().get2::()?; /// assert_eq!(foo(1, 2)?, 3); /// assert!(foo(i32::max_value(), 1).is_err()); /// # Ok(()) @@ -413,7 +413,7 @@ impl Func { /// "#, /// )?; /// let instance = Instance::new(&module, &[debug.into()])?; - /// let foo = instance.exports().next().unwrap().into_func().unwrap().get0::<()>()?; + /// let foo = instance.get_func("foo").unwrap().get0::<()>()?; /// foo()?; /// # Ok(()) /// # } @@ -469,7 +469,7 @@ impl Func { /// "#, /// )?; /// let instance = Instance::new(&module, &[log_str.into()])?; - /// let foo = instance.exports().next().unwrap().into_func().unwrap().get0::<()>()?; + /// let foo = instance.get_func("foo").unwrap().get0::<()>()?; /// foo()?; /// # Ok(()) /// # } diff --git a/docs/lang-rust.md b/docs/lang-rust.md index 3d0ff52c1778..e95f61687af5 100644 --- a/docs/lang-rust.md +++ b/docs/lang-rust.md @@ -74,10 +74,8 @@ fn main() -> Result<(), Box> { // The `Instance` gives us access to various exported functions and items, // which we access here to pull out our `answer` exported function and // run it. - let answer = instance.get_export("answer") - .expect("export named `answer` not found") - .into_func() - .expect("export `answer` was not a function"); + let answer = instance.get_func("answer") + .expect("`answer` was not an exported function"); // There's a few ways we can call the `answer` `Func` value. The easiest // is to statically assert its signature with `get0` (in this case asserting diff --git a/tests/all/custom_signal_handler.rs b/tests/all/custom_signal_handler.rs index f40614d5fe2a..0b967cbf92b9 100644 --- a/tests/all/custom_signal_handler.rs +++ b/tests/all/custom_signal_handler.rs @@ -100,8 +100,6 @@ mod tests { }); } - let mut exports = instance.exports(); - // these invoke wasmtime_call_trampoline from action.rs { println!("calling read..."); @@ -124,10 +122,8 @@ mod tests { // these invoke wasmtime_call_trampoline from callable.rs { - let read_func = exports - .next() - .unwrap() - .into_func() + let read_func = instance + .get_func("read") .expect("expected a 'read' func in the module"); println!("calling read..."); let result = read_func.call(&[]).expect("expected function not to trap"); @@ -135,10 +131,8 @@ mod tests { } { - let read_out_of_bounds_func = exports - .next() - .unwrap() - .into_func() + let read_out_of_bounds_func = instance + .get_func("read_out_of_bounds") .expect("expected a 'read_out_of_bounds' func in the module"); println!("calling read_out_of_bounds..."); let trap = read_out_of_bounds_func diff --git a/tests/all/globals.rs b/tests/all/globals.rs index 067474bce20e..d0e6896c1b98 100644 --- a/tests/all/globals.rs +++ b/tests/all/globals.rs @@ -70,7 +70,7 @@ fn use_after_drop() -> anyhow::Result<()> { "#, )?; let instance = Instance::new(&module, &[])?; - let g = instance.exports().next().unwrap().into_global().unwrap(); + let g = instance.get_global("foo").unwrap(); assert_eq!(g.get().i32(), Some(100)); g.set(101.into())?; drop(instance); diff --git a/tests/all/import_calling_export.rs b/tests/all/import_calling_export.rs index 2d38b11e14ca..199af5d32483 100644 --- a/tests/all/import_calling_export.rs +++ b/tests/all/import_calling_export.rs @@ -42,17 +42,13 @@ fn test_import_calling_export() { let mut exports = instance.exports(); - let run_func = exports - .next() - .unwrap() - .into_func() + let run_func = instance + .get_func("run") .expect("expected a run func in the module"); *other.borrow_mut() = Some( - exports - .next() - .unwrap() - .into_func() + instance + .get_func("other") .expect("expected an other func in the module"), ); @@ -86,12 +82,8 @@ fn test_returns_incorrect_type() -> Result<()> { let imports = vec![callback_func.into()]; let instance = Instance::new(&module, imports.as_slice())?; - let mut exports = instance.exports(); - - let run_func = exports - .next() - .unwrap() - .into_func() + let run_func = instance + .get_func("run") .expect("expected a run func in the module"); let trap = run_func diff --git a/tests/all/traps.rs b/tests/all/traps.rs index 153938c4674e..5858dc3d8f8e 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -17,12 +17,7 @@ fn test_trap_return() -> Result<()> { let hello_func = Func::new(&store, hello_type, |_, _, _| Err(Trap::new("test 123"))); let instance = Instance::new(&module, &[hello_func.into()])?; - let run_func = instance - .exports() - .next() - .unwrap() - .into_func() - .expect("expected function export"); + let run_func = instance.get_func("run").expect("expected function export"); let e = run_func .call(&[]) @@ -47,12 +42,7 @@ fn test_trap_trace() -> Result<()> { let module = Module::new(&store, wat)?; let instance = Instance::new(&module, &[])?; - let run_func = instance - .exports() - .next() - .unwrap() - .into_func() - .expect("expected function export"); + let run_func = instance.get_func("run").expect("expected function export"); let e = run_func .call(&[]) @@ -97,12 +87,7 @@ fn test_trap_trace_cb() -> Result<()> { let module = Module::new(&store, wat)?; let instance = Instance::new(&module, &[fn_func.into()])?; - let run_func = instance - .exports() - .next() - .unwrap() - .into_func() - .expect("expected function export"); + let run_func = instance.get_func("run").expect("expected function export"); let e = run_func .call(&[]) @@ -132,12 +117,7 @@ fn test_trap_stack_overflow() -> Result<()> { let module = Module::new(&store, wat)?; let instance = Instance::new(&module, &[])?; - let run_func = instance - .exports() - .next() - .unwrap() - .into_func() - .expect("expected function export"); + let run_func = instance.get_func("run").expect("expected function export"); let e = run_func .call(&[]) @@ -171,12 +151,7 @@ fn trap_display_pretty() -> Result<()> { let module = Module::new(&store, wat)?; let instance = Instance::new(&module, &[])?; - let run_func = instance - .exports() - .next() - .unwrap() - .into_func() - .expect("expected function export"); + let run_func = instance.get_func("bar").expect("expected function export"); let e = run_func.call(&[]).err().expect("error calling function"); assert_eq!( @@ -207,7 +182,7 @@ fn trap_display_multi_module() -> Result<()> { let module = Module::new(&store, wat)?; let instance = Instance::new(&module, &[])?; - let bar = instance.exports().next().unwrap(); + let bar = instance.get_export("bar").unwrap(); let wat = r#" (module $b @@ -217,13 +192,8 @@ fn trap_display_multi_module() -> Result<()> { ) "#; let module = Module::new(&store, wat)?; - let instance = Instance::new(&module, &[bar.external])?; - let bar2 = instance - .exports() - .next() - .unwrap() - .into_func() - .expect("expected function export"); + let instance = Instance::new(&module, &[bar])?; + let bar2 = instance.get_func("bar2").expect("expected function export"); let e = bar2.call(&[]).err().expect("error calling function"); assert_eq!( @@ -286,15 +256,14 @@ fn rust_panic_import() -> Result<()> { Func::wrap(&store, || panic!("this is another panic")).into(), ], )?; - let mut exports = instance.exports(); - let func = exports.next().unwrap().into_func().unwrap(); + let func = instance.get_func("foo").unwrap(); let err = panic::catch_unwind(AssertUnwindSafe(|| { drop(func.call(&[])); })) .unwrap_err(); assert_eq!(err.downcast_ref::<&'static str>(), Some(&"this is a panic")); - let func = exports.next().unwrap().into_func().unwrap(); + let func = instance.get_func("bar").unwrap(); let err = panic::catch_unwind(AssertUnwindSafe(|| { drop(func.call(&[])); })) @@ -352,7 +321,7 @@ fn mismatched_arguments() -> Result<()> { let module = Module::new(&store, &binary)?; let instance = Instance::new(&module, &[])?; - let func = instance.exports().next().unwrap().into_func().unwrap(); + let func = instance.get_func("foo").unwrap(); assert_eq!( func.call(&[]).unwrap_err().to_string(), "expected 1 arguments, got 0" @@ -436,7 +405,7 @@ fn present_after_module_drop() -> Result<()> { let store = Store::default(); let module = Module::new(&store, r#"(func (export "foo") unreachable)"#)?; let instance = Instance::new(&module, &[])?; - let func = instance.exports().next().unwrap().into_func().unwrap(); + let func = instance.get_func("foo").unwrap(); println!("asserting before we drop modules"); assert_trap(func.call(&[]).unwrap_err().downcast()?); From ad9414d4f20693b448e928b6ea07de2f9a652c1c Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 17 Apr 2020 22:51:57 -0700 Subject: [PATCH 18/24] Avoid cloning strings for `ImportType` and `ExportType`. --- cranelift/wasm/src/translation_utils.rs | 10 +- crates/api/src/linker.rs | 22 +-- crates/api/src/module.rs | 62 ++------ crates/api/src/types.rs | 133 ++++++++++++++++-- crates/c-api/src/module.rs | 4 +- crates/c-api/src/types/export.rs | 21 ++- crates/c-api/src/types/import.rs | 28 ++-- crates/c-api/src/wasi.rs | 6 +- crates/fuzzing/src/oracles/dummy.rs | 18 +-- .../test-programs/tests/wasm_tests/runtime.rs | 8 +- examples/wasi/main.rs | 7 +- src/commands/run.rs | 12 +- tests/all/import_calling_export.rs | 2 - 13 files changed, 205 insertions(+), 128 deletions(-) diff --git a/cranelift/wasm/src/translation_utils.rs b/cranelift/wasm/src/translation_utils.rs index eb12db950f68..74bc04399ed6 100644 --- a/cranelift/wasm/src/translation_utils.rs +++ b/cranelift/wasm/src/translation_utils.rs @@ -68,7 +68,7 @@ pub struct ElemIndex(u32); entity_impl!(ElemIndex); /// WebAssembly global. -#[derive(Debug, Clone, Copy, Hash)] +#[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] pub struct Global { /// The type of the value stored in the global. pub ty: ir::Type, @@ -79,7 +79,7 @@ pub struct Global { } /// Globals are initialized via the `const` operators or by referring to another import. -#[derive(Debug, Clone, Copy, Hash)] +#[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] pub enum GlobalInit { /// An `i32.const`. I32Const(i32), @@ -102,7 +102,7 @@ pub enum GlobalInit { } /// WebAssembly table. -#[derive(Debug, Clone, Copy, Hash)] +#[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] pub struct Table { /// The type of data stored in elements of the table. pub ty: TableElementType, @@ -113,7 +113,7 @@ pub struct Table { } /// WebAssembly table element. Can be a function or a scalar type. -#[derive(Debug, Clone, Copy, Hash)] +#[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] pub enum TableElementType { /// A scalar type. Val(ir::Type), @@ -122,7 +122,7 @@ pub enum TableElementType { } /// WebAssembly linear memory. -#[derive(Debug, Clone, Copy, Hash)] +#[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] pub struct Memory { /// The minimum number of pages in the memory. pub minimum: u32, diff --git a/crates/api/src/linker.rs b/crates/api/src/linker.rs index 2c44731742d5..c7f83bdbf2f2 100644 --- a/crates/api/src/linker.rs +++ b/crates/api/src/linker.rs @@ -265,7 +265,7 @@ impl Linker { bail!("all linker items must be from the same store"); } for export in instance.exports() { - self.insert(module_name, &export.name, export.ty(), export.external)?; + self.insert(module_name, export.name, export.ty(), export.external)?; } Ok(self) } @@ -385,8 +385,8 @@ impl Linker { let mut options = String::new(); for i in self.map.keys() { - if &*self.strings[i.module] != import.module - || &*self.strings[i.name] != import.name + if &*self.strings[i.module] != import.module() + || &*self.strings[i.name] != import.name() { continue; } @@ -397,8 +397,8 @@ impl Linker { if options.len() == 0 { bail!( "unknown import: `{}::{}` has not been defined", - import.module, - import.name + import.module(), + import.name() ) } @@ -406,9 +406,9 @@ impl Linker { "incompatible import type for `{}::{}` specified\n\ desired signature was: {:?}\n\ signatures available:\n\n{}", - import.module, - import.name, - import.ty, + import.module(), + import.name(), + import.ty(), options, ) } @@ -445,9 +445,9 @@ impl Linker { /// Returns `None` if no match was found. pub fn get(&self, import: &ImportType) -> Option { let key = ImportKey { - module: *self.string2idx.get(import.module.as_str())?, - name: *self.string2idx.get(import.name.as_str())?, - kind: self.import_kind(import.ty.clone()), + module: *self.string2idx.get(import.module())?, + name: *self.string2idx.get(import.name())?, + kind: self.import_kind(import.ty()), }; self.map.get(&key).cloned() } diff --git a/crates/api/src/module.rs b/crates/api/src/module.rs index 9bae3668ee07..360f869f0963 100644 --- a/crates/api/src/module.rs +++ b/crates/api/src/module.rs @@ -1,13 +1,10 @@ use crate::frame_info::GlobalFrameInfoRegistration; use crate::runtime::Store; -use crate::types::{ - ExportType, ExternType, FuncType, GlobalType, ImportType, MemoryType, TableType, -}; +use crate::types::{EntityType, ExportType, ImportType}; use anyhow::{Error, Result}; use std::path::Path; use std::sync::{Arc, Mutex}; use wasmparser::validate; -use wasmtime_environ::EntityIndex; use wasmtime_jit::CompiledModule; /// A compiled WebAssembly module, ready to be instantiated. @@ -395,27 +392,23 @@ impl Module { /// let module = Module::new(&store, wat)?; /// assert_eq!(module.imports().len(), 1); /// let import = module.imports().next().unwrap(); - /// assert_eq!(import.module, "host"); - /// assert_eq!(import.name, "foo"); - /// match import.ty { + /// assert_eq!(import.module(), "host"); + /// assert_eq!(import.name(), "foo"); + /// match import.ty() { /// ExternType::Func(_) => { /* ... */ } /// _ => panic!("unexpected import type!"), /// } /// # Ok(()) /// # } /// ``` - pub fn imports<'me>(&'me self) -> impl ExactSizeIterator + 'me { + pub fn imports<'me>(&'me self) -> impl ExactSizeIterator> + 'me { let module = self.inner.compiled.module_ref(); module .imports .iter() .map(move |(module_name, name, entity_index)| { - let r#type = entity_type(entity_index, module); - ImportType { - module: module_name.to_owned(), - name: name.to_owned(), - ty: r#type, - } + let r#type = EntityType::new(entity_index, module); + ImportType::new(module_name, name, r#type) }) } @@ -458,29 +451,26 @@ impl Module { /// /// let mut exports = module.exports(); /// let foo = exports.next().unwrap(); - /// assert_eq!(foo.name, "foo"); - /// match foo.ty { + /// assert_eq!(foo.name(), "foo"); + /// match foo.ty() { /// ExternType::Func(_) => { /* ... */ } /// _ => panic!("unexpected export type!"), /// } /// /// let memory = exports.next().unwrap(); - /// assert_eq!(memory.name, "memory"); - /// match memory.ty { + /// assert_eq!(memory.name(), "memory"); + /// match memory.ty() { /// ExternType::Memory(_) => { /* ... */ } /// _ => panic!("unexpected export type!"), /// } /// # Ok(()) /// # } /// ``` - pub fn exports<'me>(&'me self) -> impl ExactSizeIterator + 'me { + pub fn exports<'me>(&'me self) -> impl ExactSizeIterator> + 'me { let module = self.inner.compiled.module_ref(); module.exports.iter().map(move |(name, entity_index)| { - let r#type = entity_type(entity_index, module); - ExportType { - name: name.to_owned(), - ty: r#type, - } + let r#type = EntityType::new(entity_index, module); + ExportType::new(name, r#type) }) } @@ -502,27 +492,3 @@ impl Module { return ret; } } - -/// Translate from a `EntityIndex` into an `ExternType`. -fn entity_type(entity_index: &EntityIndex, module: &wasmtime_environ::Module) -> ExternType { - match entity_index { - EntityIndex::Function(func_index) => { - let sig = module.local.func_signature(*func_index); - FuncType::from_wasmtime_signature(sig) - .expect("core wasm function type should be supported") - .into() - } - EntityIndex::Table(table_index) => { - TableType::from_wasmtime_table(&module.local.table_plans[*table_index].table).into() - } - EntityIndex::Memory(memory_index) => { - MemoryType::from_wasmtime_memory(&module.local.memory_plans[*memory_index].memory) - .into() - } - EntityIndex::Global(global_index) => { - GlobalType::from_wasmtime_global(&module.local.globals[*global_index]) - .expect("core wasm global type should be supported") - .into() - } - } -} diff --git a/crates/api/src/types.rs b/crates/api/src/types.rs index 2a6316f90488..dc011f1762a7 100644 --- a/crates/api/src/types.rs +++ b/crates/api/src/types.rs @@ -1,4 +1,5 @@ -use wasmtime_environ::{ir, wasm}; +use std::fmt; +use wasmtime_environ::{ir, wasm, EntityIndex}; // Type Representations @@ -382,6 +383,53 @@ impl MemoryType { } } +// Entity Types + +#[derive(Clone, Hash, Eq, PartialEq)] +pub(crate) enum EntityType<'module> { + Function(&'module ir::Signature), + Table(&'module wasm::Table), + Memory(&'module wasm::Memory), + Global(&'module wasm::Global), +} + +impl<'module> EntityType<'module> { + /// Translate from a `EntityIndex` into an `ExternType`. + pub(crate) fn new( + entity_index: &EntityIndex, + module: &'module wasmtime_environ::Module, + ) -> EntityType<'module> { + match entity_index { + EntityIndex::Function(func_index) => { + let sig = module.local.func_signature(*func_index); + EntityType::Function(&sig) + } + EntityIndex::Table(table_index) => { + EntityType::Table(&module.local.table_plans[*table_index].table) + } + EntityIndex::Memory(memory_index) => { + EntityType::Memory(&module.local.memory_plans[*memory_index].memory) + } + EntityIndex::Global(global_index) => { + EntityType::Global(&module.local.globals[*global_index]) + } + } + } + + fn extern_type(&self) -> ExternType { + match self { + EntityType::Function(sig) => FuncType::from_wasmtime_signature(sig) + .expect("core wasm function type should be supported") + .into(), + EntityType::Table(table) => TableType::from_wasmtime_table(table).into(), + EntityType::Memory(memory) => MemoryType::from_wasmtime_memory(memory).into(), + EntityType::Global(global) => GlobalType::from_wasmtime_global(global) + .expect("core wasm global type should be supported") + .into(), + } + } +} + // Import Types /// A descriptor for an imported value into a wasm module. @@ -390,16 +438,54 @@ impl MemoryType { /// [`Module::imports`](crate::Module::imports) API. Each [`ImportType`] /// describes an import into the wasm module with the module/name that it's /// imported from as well as the type of item that's being imported. -#[derive(Debug, Clone, Hash, Eq, PartialEq)] -pub struct ImportType { +#[derive(Clone, Hash, Eq, PartialEq)] +pub struct ImportType<'module> { /// The module of the import. - pub module: String, + module: &'module str, /// The field of the import. - pub name: String, + name: &'module str, /// The type of the import. - pub ty: ExternType, + ty: EntityType<'module>, +} + +impl<'module> ImportType<'module> { + /// Creates a new import descriptor which comes from `module` and `name` and + /// is of type `ty`. + pub(crate) fn new( + module: &'module str, + name: &'module str, + ty: EntityType<'module>, + ) -> ImportType<'module> { + ImportType { module, name, ty } + } + + /// Returns the module name that this import is expected to come from. + pub fn module(&'module self) -> &'module str { + self.module + } + + /// Returns the field name of the module that this import is expected to + /// come from. + pub fn name(&'module self) -> &'module str { + self.name + } + + /// Returns the expected type of this import. + pub fn ty(&'module self) -> ExternType { + self.ty.extern_type() + } +} + +impl<'module> fmt::Debug for ImportType<'module> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ImportType") + .field("module", &self.module().to_owned()) + .field("name", &self.name().to_owned()) + .field("ty", &self.ty()) + .finish() + } } // Export Types @@ -410,11 +496,38 @@ pub struct ImportType { /// [`Module::exports`](crate::Module::exports) accessor and describes what /// names are exported from a wasm module and the type of the item that is /// exported. -#[derive(Debug, Clone, Hash, Eq, PartialEq)] -pub struct ExportType { +#[derive(Clone, Hash, Eq, PartialEq)] +pub struct ExportType<'module> { /// The name of the export. - pub name: String, + name: &'module str, /// The type of the export. - pub ty: ExternType, + ty: EntityType<'module>, +} + +impl<'module> ExportType<'module> { + /// Creates a new export which is exported with the given `name` and has the + /// given `ty`. + pub(crate) fn new(name: &'module str, ty: EntityType<'module>) -> ExportType<'module> { + ExportType { name, ty } + } + + /// Returns the name by which this export is known by. + pub fn name(&'module self) -> &'module str { + self.name + } + + /// Returns the type of this export. + pub fn ty(&'module self) -> ExternType { + self.ty.extern_type() + } +} + +impl<'module> fmt::Debug for ExportType<'module> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ExportType") + .field("name", &self.name().to_owned()) + .field("ty", &self.ty()) + .finish() + } } diff --git a/crates/c-api/src/module.rs b/crates/c-api/src/module.rs index 21aac85d2ac4..0363632601d5 100644 --- a/crates/c-api/src/module.rs +++ b/crates/c-api/src/module.rs @@ -46,11 +46,11 @@ pub extern "C" fn wasmtime_module_new( handle_result(Module::from_binary(store, binary), |module| { let imports = module .imports() - .map(|i| wasm_importtype_t::new(i.clone())) + .map(|i| wasm_importtype_t::new(i.module().to_owned(), i.name().to_owned(), i.ty())) .collect::>(); let exports = module .exports() - .map(|e| wasm_exporttype_t::new(e.clone())) + .map(|e| wasm_exporttype_t::new(e.name().to_owned(), e.ty())) .collect::>(); let module = Box::new(wasm_module_t { module: HostRef::new(module), diff --git a/crates/c-api/src/types/export.rs b/crates/c-api/src/types/export.rs index 94be6ba49958..3d83cc41faca 100644 --- a/crates/c-api/src/types/export.rs +++ b/crates/c-api/src/types/export.rs @@ -1,12 +1,12 @@ use crate::{wasm_externtype_t, wasm_name_t}; use once_cell::unsync::OnceCell; -use std::str; -use wasmtime::ExportType; +use wasmtime::ExternType; #[repr(C)] #[derive(Clone)] pub struct wasm_exporttype_t { - ty: ExportType, + name: String, + ty: ExternType, name_cache: OnceCell, type_cache: OnceCell, } @@ -14,8 +14,9 @@ pub struct wasm_exporttype_t { wasmtime_c_api_macros::declare_ty!(wasm_exporttype_t); impl wasm_exporttype_t { - pub(crate) fn new(ty: ExportType) -> wasm_exporttype_t { + pub(crate) fn new(name: String, ty: ExternType) -> wasm_exporttype_t { wasm_exporttype_t { + name, ty, name_cache: OnceCell::new(), type_cache: OnceCell::new(), @@ -29,22 +30,18 @@ pub extern "C" fn wasm_exporttype_new( ty: Box, ) -> Option> { let name = name.take(); - let name = str::from_utf8(&name).ok()?; - let ty = ExportType { - name: name.to_string(), - ty: ty.ty(), - }; - Some(Box::new(wasm_exporttype_t::new(ty))) + let name = String::from_utf8(name).ok()?; + Some(Box::new(wasm_exporttype_t::new(name, ty.ty()))) } #[no_mangle] pub extern "C" fn wasm_exporttype_name(et: &wasm_exporttype_t) -> &wasm_name_t { et.name_cache - .get_or_init(|| wasm_name_t::from_name(et.ty.name.clone())) + .get_or_init(|| wasm_name_t::from_name(et.name.clone())) } #[no_mangle] pub extern "C" fn wasm_exporttype_type(et: &wasm_exporttype_t) -> &wasm_externtype_t { et.type_cache - .get_or_init(|| wasm_externtype_t::new(et.ty.ty.clone())) + .get_or_init(|| wasm_externtype_t::new(et.ty.clone())) } diff --git a/crates/c-api/src/types/import.rs b/crates/c-api/src/types/import.rs index f380de575695..58f1516a79aa 100644 --- a/crates/c-api/src/types/import.rs +++ b/crates/c-api/src/types/import.rs @@ -1,12 +1,13 @@ use crate::{wasm_externtype_t, wasm_name_t}; use once_cell::unsync::OnceCell; -use std::str; -use wasmtime::ImportType; +use wasmtime::ExternType; #[repr(C)] #[derive(Clone)] pub struct wasm_importtype_t { - pub(crate) ty: ImportType, + pub(crate) module: String, + pub(crate) name: String, + pub(crate) ty: ExternType, module_cache: OnceCell, name_cache: OnceCell, type_cache: OnceCell, @@ -15,8 +16,10 @@ pub struct wasm_importtype_t { wasmtime_c_api_macros::declare_ty!(wasm_importtype_t); impl wasm_importtype_t { - pub(crate) fn new(ty: ImportType) -> wasm_importtype_t { + pub(crate) fn new(module: String, name: String, ty: ExternType) -> wasm_importtype_t { wasm_importtype_t { + module, + name, ty, module_cache: OnceCell::new(), name_cache: OnceCell::new(), @@ -33,30 +36,25 @@ pub extern "C" fn wasm_importtype_new( ) -> Option> { let module = module.take(); let name = name.take(); - let module = str::from_utf8(&module).ok()?; - let name = str::from_utf8(&name).ok()?; - let ty = ImportType { - module: module.to_owned(), - name: name.to_owned(), - ty: ty.ty(), - }; - Some(Box::new(wasm_importtype_t::new(ty))) + let module = String::from_utf8(module).ok()?; + let name = String::from_utf8(name).ok()?; + Some(Box::new(wasm_importtype_t::new(module, name, ty.ty()))) } #[no_mangle] pub extern "C" fn wasm_importtype_module(it: &wasm_importtype_t) -> &wasm_name_t { it.module_cache - .get_or_init(|| wasm_name_t::from_name(it.ty.module.clone())) + .get_or_init(|| wasm_name_t::from_name(it.module.clone())) } #[no_mangle] pub extern "C" fn wasm_importtype_name(it: &wasm_importtype_t) -> &wasm_name_t { it.name_cache - .get_or_init(|| wasm_name_t::from_name(it.ty.name.clone())) + .get_or_init(|| wasm_name_t::from_name(it.name.clone())) } #[no_mangle] pub extern "C" fn wasm_importtype_type(it: &wasm_importtype_t) -> &wasm_externtype_t { it.type_cache - .get_or_init(|| wasm_externtype_t::new(it.ty.ty.clone())) + .get_or_init(|| wasm_externtype_t::new(it.ty.clone())) } diff --git a/crates/c-api/src/wasi.rs b/crates/c-api/src/wasi.rs index 45906ccb379d..dffb00f1bad8 100644 --- a/crates/c-api/src/wasi.rs +++ b/crates/c-api/src/wasi.rs @@ -313,8 +313,8 @@ pub extern "C" fn wasi_instance_bind_import<'a>( instance: &'a mut wasi_instance_t, import: &wasm_importtype_t, ) -> Option<&'a wasm_extern_t> { - let module = &import.ty.module; - let name = str::from_utf8(import.ty.name.as_bytes()).ok()?; + let module = &import.module; + let name = str::from_utf8(import.name.as_bytes()).ok()?; let export = match &instance.wasi { WasiInstance::Preview1(wasi) => { @@ -332,7 +332,7 @@ pub extern "C" fn wasi_instance_bind_import<'a>( } }; - if &export.ty() != import.ty.ty.func()? { + if &export.ty() != import.ty.func()? { return None; } diff --git a/crates/fuzzing/src/oracles/dummy.rs b/crates/fuzzing/src/oracles/dummy.rs index 6027775caeda..2e904ec1a304 100644 --- a/crates/fuzzing/src/oracles/dummy.rs +++ b/crates/fuzzing/src/oracles/dummy.rs @@ -6,21 +6,17 @@ use wasmtime::{ }; /// Create a set of dummy functions/globals/etc for the given imports. -pub fn dummy_imports( +pub fn dummy_imports<'module>( store: &Store, - import_tys: impl Iterator, + import_tys: impl Iterator>, ) -> Result, Trap> { import_tys .map(|imp| { - Ok(match imp.ty { - ExternType::Func(func_ty) => Extern::Func(dummy_func(&store, func_ty.clone())), - ExternType::Global(global_ty) => { - Extern::Global(dummy_global(&store, global_ty.clone())?) - } - ExternType::Table(table_ty) => { - Extern::Table(dummy_table(&store, table_ty.clone())?) - } - ExternType::Memory(mem_ty) => Extern::Memory(dummy_memory(&store, mem_ty.clone())), + Ok(match imp.ty() { + ExternType::Func(func_ty) => Extern::Func(dummy_func(&store, func_ty)), + ExternType::Global(global_ty) => Extern::Global(dummy_global(&store, global_ty)?), + ExternType::Table(table_ty) => Extern::Table(dummy_table(&store, table_ty)?), + ExternType::Memory(mem_ty) => Extern::Memory(dummy_memory(&store, mem_ty)), }) }) .collect() diff --git a/crates/test-programs/tests/wasm_tests/runtime.rs b/crates/test-programs/tests/wasm_tests/runtime.rs index f8785cc21b7d..0157a9c61400 100644 --- a/crates/test-programs/tests/wasm_tests/runtime.rs +++ b/crates/test-programs/tests/wasm_tests/runtime.rs @@ -52,11 +52,15 @@ pub fn instantiate( let imports = module .imports() .map(|i| { - let field_name = &i.name; + let field_name = i.name(); if let Some(export) = snapshot1.get_export(field_name) { Ok(export.clone().into()) } else { - bail!("import {} was not found in module {}", field_name, i.module,) + bail!( + "import {} was not found in module {}", + field_name, + i.module() + ) } }) .collect::, _>>()?; diff --git a/examples/wasi/main.rs b/examples/wasi/main.rs index 66151937eca4..8971364cd1b2 100644 --- a/examples/wasi/main.rs +++ b/examples/wasi/main.rs @@ -17,15 +17,16 @@ fn main() -> Result<()> { let wasi = Wasi::new(&store, WasiCtx::new(std::env::args())?); let mut imports = Vec::new(); for import in module.imports() { - if import.module == "wasi_snapshot_preview1" { - if let Some(export) = wasi.get_export(&import.name) { + if import.module() == "wasi_snapshot_preview1" { + if let Some(export) = wasi.get_export(import.name()) { imports.push(Extern::from(export.clone())); continue; } } panic!( "couldn't find import for `{}::{}`", - import.module, import.name + import.module(), + import.name() ); } diff --git a/src/commands/run.rs b/src/commands/run.rs index e7197bfbae5d..03edcd6f8cc8 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -200,16 +200,20 @@ impl RunCommand { let imports = module .imports() .map(|i| { - let export = match i.module.as_str() { + let export = match i.module() { "wasi_snapshot_preview1" => { - module_registry.wasi_snapshot_preview1.get_export(&i.name) + module_registry.wasi_snapshot_preview1.get_export(i.name()) } - "wasi_unstable" => module_registry.wasi_unstable.get_export(&i.name), + "wasi_unstable" => module_registry.wasi_unstable.get_export(i.name()), other => bail!("import module `{}` was not found", other), }; match export { Some(export) => Ok(export.clone().into()), - None => bail!("import `{}` was not found in module `{}`", i.name, i.module), + None => bail!( + "import `{}` was not found in module `{}`", + i.name(), + i.module() + ), } }) .collect::, _>>()?; diff --git a/tests/all/import_calling_export.rs b/tests/all/import_calling_export.rs index 199af5d32483..97c95384d04d 100644 --- a/tests/all/import_calling_export.rs +++ b/tests/all/import_calling_export.rs @@ -40,8 +40,6 @@ fn test_import_calling_export() { let instance = Instance::new(&module, imports.as_slice()).expect("failed to instantiate module"); - let mut exports = instance.exports(); - let run_func = instance .get_func("run") .expect("expected a run func in the module"); From b2b5e8d734a6921acc0777fbccc6143230f2c2ad Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 20 Apr 2020 09:05:28 -0700 Subject: [PATCH 19/24] Remove more obviated clone() calls. --- crates/api/src/externals.rs | 2 +- crates/api/src/linker.rs | 2 +- crates/api/src/trampoline/func.rs | 4 ++-- crates/c-api/src/extern.rs | 8 ++++---- crates/c-api/src/func.rs | 10 +++++----- crates/c-api/src/global.rs | 2 +- crates/c-api/src/instance.rs | 8 ++++---- crates/c-api/src/memory.rs | 2 +- crates/c-api/src/table.rs | 2 +- crates/c-api/src/wasi.rs | 2 +- crates/c-api/src/wat2wasm.rs | 2 +- 11 files changed, 22 insertions(+), 22 deletions(-) diff --git a/crates/api/src/externals.rs b/crates/api/src/externals.rs index e94fc40632b8..4ba931fd0794 100644 --- a/crates/api/src/externals.rs +++ b/crates/api/src/externals.rs @@ -374,7 +374,7 @@ impl Table { /// Returns the current size of this table. pub fn size(&self) -> u32 { - unsafe { (&*self.wasmtime_export.definition).current_elements } + unsafe { (*self.wasmtime_export.definition).current_elements } } /// Grows the size of this table by `delta` more elements, initialization diff --git a/crates/api/src/linker.rs b/crates/api/src/linker.rs index c7f83bdbf2f2..09cbe72ab20a 100644 --- a/crates/api/src/linker.rs +++ b/crates/api/src/linker.rs @@ -283,7 +283,7 @@ impl Linker { let items = self .iter() .filter(|(m, _, _)| *m == module) - .map(|(_, name, item)| (name.to_string(), item.clone())) + .map(|(_, name, item)| (name.to_string(), item)) .collect::>(); for (name, item) in items { self.define(as_module, &name, item)?; diff --git a/crates/api/src/trampoline/func.rs b/crates/api/src/trampoline/func.rs index cafe26b47cc3..0c5f36c625a7 100644 --- a/crates/api/src/trampoline/func.rs +++ b/crates/api/src/trampoline/func.rs @@ -212,7 +212,7 @@ pub fn create_handle_with_function( let pointer_type = isa.pointer_type(); let sig = match ft.get_wasmtime_signature(pointer_type) { - Some(sig) => sig.clone(), + Some(sig) => sig, None => bail!("not a supported core wasm signature {:?}", ft), }; @@ -276,7 +276,7 @@ pub unsafe fn create_handle_with_raw_function( let pointer_type = isa.pointer_type(); let sig = match ft.get_wasmtime_signature(pointer_type) { - Some(sig) => sig.clone(), + Some(sig) => sig, None => bail!("not a supported core wasm signature {:?}", ft), }; diff --git a/crates/c-api/src/extern.rs b/crates/c-api/src/extern.rs index e5d6fc615299..b882082faf99 100644 --- a/crates/c-api/src/extern.rs +++ b/crates/c-api/src/extern.rs @@ -41,10 +41,10 @@ pub extern "C" fn wasm_extern_kind(e: &wasm_extern_t) -> wasm_externkind_t { #[no_mangle] pub extern "C" fn wasm_extern_type(e: &wasm_extern_t) -> Box { let ty = match &e.which { - ExternHost::Func(f) => ExternType::Func(f.borrow().ty().clone()), - ExternHost::Global(f) => ExternType::Global(f.borrow().ty().clone()), - ExternHost::Table(f) => ExternType::Table(f.borrow().ty().clone()), - ExternHost::Memory(f) => ExternType::Memory(f.borrow().ty().clone()), + ExternHost::Func(f) => ExternType::Func(f.borrow().ty()), + ExternHost::Global(f) => ExternType::Global(f.borrow().ty()), + ExternHost::Table(f) => ExternType::Table(f.borrow().ty()), + ExternHost::Memory(f) => ExternType::Memory(f.borrow().ty()), }; Box::new(wasm_externtype_t::new(ty)) } diff --git a/crates/c-api/src/func.rs b/crates/c-api/src/func.rs index 9c9718216ac3..3c58d1ca46a3 100644 --- a/crates/c-api/src/func.rs +++ b/crates/c-api/src/func.rs @@ -246,7 +246,7 @@ fn _wasmtime_func_call( #[no_mangle] pub extern "C" fn wasm_func_type(f: &wasm_func_t) -> Box { - Box::new(wasm_functype_t::new(f.func().borrow().ty().clone())) + Box::new(wasm_functype_t::new(f.func().borrow().ty())) } #[no_mangle] @@ -272,10 +272,10 @@ pub unsafe extern "C" fn wasmtime_caller_export_get( let name = str::from_utf8(name.as_slice()).ok()?; let export = caller.caller.get_export(name)?; let which = match export { - Extern::Func(f) => ExternHost::Func(HostRef::new(f.clone())), - Extern::Global(g) => ExternHost::Global(HostRef::new(g.clone())), - Extern::Memory(m) => ExternHost::Memory(HostRef::new(m.clone())), - Extern::Table(t) => ExternHost::Table(HostRef::new(t.clone())), + Extern::Func(f) => ExternHost::Func(HostRef::new(f)), + Extern::Global(g) => ExternHost::Global(HostRef::new(g)), + Extern::Memory(m) => ExternHost::Memory(HostRef::new(m)), + Extern::Table(t) => ExternHost::Table(HostRef::new(t)), }; Some(Box::new(wasm_extern_t { which })) } diff --git a/crates/c-api/src/global.rs b/crates/c-api/src/global.rs index 4c6f025d65b3..fb3f163bab78 100644 --- a/crates/c-api/src/global.rs +++ b/crates/c-api/src/global.rs @@ -71,7 +71,7 @@ pub extern "C" fn wasm_global_as_extern(g: &wasm_global_t) -> &wasm_extern_t { #[no_mangle] pub extern "C" fn wasm_global_type(g: &wasm_global_t) -> Box { - let globaltype = g.global().borrow().ty().clone(); + let globaltype = g.global().borrow().ty(); Box::new(wasm_globaltype_t::new(globaltype)) } diff --git a/crates/c-api/src/instance.rs b/crates/c-api/src/instance.rs index 1afa3686d3d4..128e53fdf862 100644 --- a/crates/c-api/src/instance.rs +++ b/crates/c-api/src/instance.rs @@ -146,10 +146,10 @@ pub extern "C" fn wasm_instance_exports(instance: &wasm_instance_t, out: &mut wa instance .exports() .map(|e| match e.external { - Extern::Func(f) => ExternHost::Func(HostRef::new(f.clone())), - Extern::Global(f) => ExternHost::Global(HostRef::new(f.clone())), - Extern::Memory(f) => ExternHost::Memory(HostRef::new(f.clone())), - Extern::Table(f) => ExternHost::Table(HostRef::new(f.clone())), + Extern::Func(f) => ExternHost::Func(HostRef::new(f)), + Extern::Global(f) => ExternHost::Global(HostRef::new(f)), + Extern::Memory(f) => ExternHost::Memory(HostRef::new(f)), + Extern::Table(f) => ExternHost::Table(HostRef::new(f)), }) .collect() }); diff --git a/crates/c-api/src/memory.rs b/crates/c-api/src/memory.rs index ab814bc2960c..d77f01c4ea97 100644 --- a/crates/c-api/src/memory.rs +++ b/crates/c-api/src/memory.rs @@ -51,7 +51,7 @@ pub extern "C" fn wasm_memory_as_extern(m: &wasm_memory_t) -> &wasm_extern_t { #[no_mangle] pub extern "C" fn wasm_memory_type(m: &wasm_memory_t) -> Box { - let ty = m.memory().borrow().ty().clone(); + let ty = m.memory().borrow().ty(); Box::new(wasm_memorytype_t::new(ty)) } diff --git a/crates/c-api/src/table.rs b/crates/c-api/src/table.rs index 8f4c0dca043e..a52b79048b53 100644 --- a/crates/c-api/src/table.rs +++ b/crates/c-api/src/table.rs @@ -54,7 +54,7 @@ pub unsafe extern "C" fn wasm_table_new( #[no_mangle] pub extern "C" fn wasm_table_type(t: &wasm_table_t) -> Box { - let ty = t.table().borrow().ty().clone(); + let ty = t.table().borrow().ty(); Box::new(wasm_tabletype_t::new(ty)) } diff --git a/crates/c-api/src/wasi.rs b/crates/c-api/src/wasi.rs index dffb00f1bad8..a3b3ef8e8444 100644 --- a/crates/c-api/src/wasi.rs +++ b/crates/c-api/src/wasi.rs @@ -297,7 +297,7 @@ pub unsafe extern "C" fn wasi_instance_new( })), Err(e) => { *trap = Box::into_raw(Box::new(wasm_trap_t { - trap: HostRef::new(Trap::new(e.to_string())), + trap: HostRef::new(Trap::new(e)), })); None diff --git a/crates/c-api/src/wat2wasm.rs b/crates/c-api/src/wat2wasm.rs index 696ee579a8e7..4223da67da15 100644 --- a/crates/c-api/src/wat2wasm.rs +++ b/crates/c-api/src/wat2wasm.rs @@ -10,6 +10,6 @@ pub extern "C" fn wasmtime_wat2wasm( Err(_) => return bad_utf8(), }; handle_result(wat::parse_str(wat).map_err(|e| e.into()), |bytes| { - ret.set_buffer(bytes.into()) + ret.set_buffer(bytes) }) } From e4b07c542bf86eac46dd6b35f72c86c6e903c97c Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 20 Apr 2020 09:25:35 -0700 Subject: [PATCH 20/24] Simplify `Func`'s closure state. --- crates/api/src/func.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/crates/api/src/func.rs b/crates/api/src/func.rs index 719b65fbf804..b6a4dec62f57 100644 --- a/crates/api/src/func.rs +++ b/crates/api/src/func.rs @@ -174,14 +174,8 @@ macro_rules! getters { // Pass the instance into the closure so that we keep it live for the lifetime // of the closure. Pass the export in so that we can call it. - struct ClosureState { - _instance: InstanceHandle, - export: ExportFunction - } - let closure = ClosureState { - _instance: self.instance.clone(), - export: self.export.clone() - }; + let instance = self.instance.clone(); + let export = self.export.clone(); // ... and then once we've passed the typechecks we can hand out our // object since our `transmute` below should be safe! @@ -194,12 +188,17 @@ macro_rules! getters { *mut VMContext, $($args,)* ) -> R, - >(closure.export.address); + >(export.address); let mut ret = None; $(let $args = $args.into_abi();)* - wasmtime_runtime::catch_traps(closure.export.vmctx, || { - ret = Some(fnptr(closure.export.vmctx, ptr::null_mut(), $($args,)*)); + wasmtime_runtime::catch_traps(export.vmctx, || { + ret = Some(fnptr(export.vmctx, ptr::null_mut(), $($args,)*)); }).map_err(Trap::from_jit)?; + + // We're holding this handle just to ensure that the instance stays + // live while we call into it. + drop(&instance); + Ok(ret.unwrap()) } }) From b53c7c1093913391bbe0b56874ff0cdccca7e277 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 20 Apr 2020 11:35:11 -0700 Subject: [PATCH 21/24] Make wasmtime::Export's fields private. This makes them more consistent with ExportType. --- crates/api/src/externals.rs | 56 ++++++++++++++++++++---------- crates/api/src/instance.rs | 8 ++--- crates/api/src/linker.rs | 8 ++--- crates/c-api/src/instance.rs | 2 +- src/commands/run.rs | 2 +- tests/all/custom_signal_handler.rs | 2 +- 6 files changed, 48 insertions(+), 30 deletions(-) diff --git a/crates/api/src/externals.rs b/crates/api/src/externals.rs index 4ba931fd0794..576c92e15e37 100644 --- a/crates/api/src/externals.rs +++ b/crates/api/src/externals.rs @@ -91,11 +91,11 @@ impl Extern { } pub(crate) fn from_wasmtime_export( + wasmtime_export: wasmtime_runtime::Export, store: &Store, instance_handle: InstanceHandle, - export: wasmtime_runtime::Export, ) -> Extern { - match export { + match wasmtime_export { wasmtime_runtime::Export::Function(f) => { Extern::Func(Func::from_wasmtime_function(f, store, instance_handle)) } @@ -901,35 +901,55 @@ pub unsafe trait MemoryCreator: Send + Sync { #[derive(Clone)] pub struct Export<'instance> { /// The name of the export. - pub name: &'instance str, + name: &'instance str, - /// The value of the export. - pub external: Extern, + /// The definition of the export. + definition: Extern, } impl<'instance> Export<'instance> { - /// Shorthand for `self.external.into_func()`. + /// Creates a new export which is exported with the given `name` and has the + /// given `definition`. + pub(crate) fn new(name: &'instance str, definition: Extern) -> Export<'instance> { + Export { name, definition } + } + + /// Returns the name by which this export is known. + pub fn name(&self) -> &'instance str { + self.name + } + + /// Return the `ExternType` of this export. + pub fn ty(&self) -> ExternType { + self.definition.ty() + } + + /// Consume this `Export` and return the contained `Extern`. + pub fn into_extern(self) -> Extern { + self.definition + } + + /// Consume this `Export` and return the contained `Func`, if it's a function, + /// or `None` otherwise. pub fn into_func(self) -> Option { - self.external.into_func() + self.definition.into_func() } - /// Shorthand for `self.external.into_table()`. + /// Consume this `Export` and return the contained `Table`, if it's a table, + /// or `None` otherwise. pub fn into_table(self) -> Option
{ - self.external.into_table() + self.definition.into_table() } - /// Shorthand for `self.external.into_memory()`. + /// Consume this `Export` and return the contained `Memory`, if it's a memory, + /// or `None` otherwise. pub fn into_memory(self) -> Option { - self.external.into_memory() + self.definition.into_memory() } - /// Shorthand for `self.external.into_global()`. + /// Consume this `Export` and return the contained `Global`, if it's a global, + /// or `None` otherwise. pub fn into_global(self) -> Option { - self.external.into_global() - } - - /// Shorthand for `self.external.ty()`. - pub fn ty(&self) -> ExternType { - self.external.ty() + self.definition.into_global() } } diff --git a/crates/api/src/instance.rs b/crates/api/src/instance.rs index ad49cadac398..e40f12a44d1f 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/src/instance.rs @@ -167,10 +167,8 @@ impl Instance { .exports() .map(move |(name, entity_index)| { let export = instance_handle.lookup_by_declaration(entity_index); - Export { - name, - external: Extern::from_wasmtime_export(store, instance_handle.clone(), export), - } + let extern_ = Extern::from_wasmtime_export(export, store, instance_handle.clone()); + Export::new(name, extern_) }) } @@ -183,9 +181,9 @@ impl Instance { pub fn get_export(&self, name: &str) -> Option { let export = self.instance_handle.lookup(&name)?; Some(Extern::from_wasmtime_export( + export, self.module.store(), self.instance_handle.clone(), - export, )) } diff --git a/crates/api/src/linker.rs b/crates/api/src/linker.rs index 09cbe72ab20a..888ad1cffb4c 100644 --- a/crates/api/src/linker.rs +++ b/crates/api/src/linker.rs @@ -166,7 +166,7 @@ impl Linker { if !item.comes_from_same_store(&self.store) { bail!("all linker items must be from the same store"); } - self.insert(module, name, item.ty(), item)?; + self.insert(module, name, item)?; Ok(self) } @@ -265,7 +265,7 @@ impl Linker { bail!("all linker items must be from the same store"); } for export in instance.exports() { - self.insert(module_name, export.name, export.ty(), export.external)?; + self.insert(module_name, export.name(), export.into_extern())?; } Ok(self) } @@ -291,8 +291,8 @@ impl Linker { Ok(()) } - fn insert(&mut self, module: &str, name: &str, ty: ExternType, item: Extern) -> Result<()> { - let key = self.import_key(module, name, ty); + fn insert(&mut self, module: &str, name: &str, item: Extern) -> Result<()> { + let key = self.import_key(module, name, item.ty()); match self.map.entry(key) { Entry::Occupied(o) if !self.allow_shadowing => bail!( "import of `{}::{}` with kind {:?} defined twice", diff --git a/crates/c-api/src/instance.rs b/crates/c-api/src/instance.rs index 128e53fdf862..0ac864f5c3b1 100644 --- a/crates/c-api/src/instance.rs +++ b/crates/c-api/src/instance.rs @@ -145,7 +145,7 @@ pub extern "C" fn wasm_instance_exports(instance: &wasm_instance_t, out: &mut wa let instance = &instance.instance.borrow(); instance .exports() - .map(|e| match e.external { + .map(|e| match e.into_extern() { Extern::Func(f) => ExternHost::Func(HostRef::new(f)), Extern::Global(f) => ExternHost::Global(HostRef::new(f)), Extern::Memory(f) => ExternHost::Memory(HostRef::new(f)), diff --git a/src/commands/run.rs b/src/commands/run.rs index 03edcd6f8cc8..95c795d582a4 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -230,7 +230,7 @@ impl RunCommand { // If a function to invoke was given, invoke it. if let Some(name) = self.invoke.as_ref() { self.invoke_export(instance, name)?; - } else if instance.exports().any(|export| export.name.is_empty()) { + } else if instance.exports().any(|export| export.name().is_empty()) { // Launch the default command export. self.invoke_export(instance, "")?; } else { diff --git a/tests/all/custom_signal_handler.rs b/tests/all/custom_signal_handler.rs index 0b967cbf92b9..2f1be828b990 100644 --- a/tests/all/custom_signal_handler.rs +++ b/tests/all/custom_signal_handler.rs @@ -259,7 +259,7 @@ mod tests { // instance2 which calls 'instance1.read' let module2 = Module::new(&store, WAT2)?; - let instance2 = Instance::new(&module2, &[instance1_read.external])?; + let instance2 = Instance::new(&module2, &[instance1_read.into_extern()])?; // since 'instance2.run' calls 'instance1.read' we need to set up the signal handler to handle // SIGSEGV originating from within the memory of instance1 unsafe { From 8050fea3d28314994843980f7ea30fda005570f1 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 20 Apr 2020 11:41:02 -0700 Subject: [PATCH 22/24] Fix compilation error. --- crates/fuzzing/src/oracles.rs | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index b786714a2015..b842ab2d0fc5 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -169,28 +169,14 @@ pub fn differential_execution( } }; - let funcs = module - .exports() - .filter_map(|e| { - if let ExternType::Func(_) = e.ty() { - Some(e.name()) - } else { - None - } - }) - .collect::>(); - - for name in funcs { + for (name, f) in instance.exports().filter_map(|e| { + let name = e.name(); + e.into_func().map(|f| (name, f)) + }) { // Always call the hang limit initializer first, so that we don't // infinite loop when calling another export. init_hang_limit(&instance); - let f = instance - .get_export(&name) - .expect("instance should have export from module") - .into_func() - .expect("export should be a function"); - let ty = f.ty(); let params = match dummy::dummy_values(ty.params()) { Ok(p) => p, @@ -375,7 +361,7 @@ pub fn make_api_calls(api: crate::generators::api::ApiCalls) { let funcs = instance .exports() - .filter_map(|e| match e { + .filter_map(|e| match e.into_extern() { Extern::Func(f) => Some(f.clone()), _ => None, }) From 8d96fffe0d4dff4bbf53973752a630a7bef3ae93 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 20 Apr 2020 13:02:40 -0700 Subject: [PATCH 23/24] Make a lifetime parameter explicit, and use better lifetime names. Instead of 'me, use 'instance and 'module to make it clear what the lifetime is. --- crates/api/src/instance.rs | 4 +++- crates/api/src/module.rs | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/api/src/instance.rs b/crates/api/src/instance.rs index e40f12a44d1f..c1b66ebe211b 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/src/instance.rs @@ -160,7 +160,9 @@ impl Instance { } /// Returns the list of exported items from this [`Instance`]. - pub fn exports<'me>(&'me self) -> impl ExactSizeIterator + 'me { + pub fn exports<'instance>( + &'instance self, + ) -> impl ExactSizeIterator> + 'instance { let instance_handle = &self.instance_handle; let store = self.module.store(); self.instance_handle diff --git a/crates/api/src/module.rs b/crates/api/src/module.rs index 360f869f0963..c741d434e579 100644 --- a/crates/api/src/module.rs +++ b/crates/api/src/module.rs @@ -401,7 +401,9 @@ impl Module { /// # Ok(()) /// # } /// ``` - pub fn imports<'me>(&'me self) -> impl ExactSizeIterator> + 'me { + pub fn imports<'module>( + &'module self, + ) -> impl ExactSizeIterator> + 'module { let module = self.inner.compiled.module_ref(); module .imports @@ -466,7 +468,9 @@ impl Module { /// # Ok(()) /// # } /// ``` - pub fn exports<'me>(&'me self) -> impl ExactSizeIterator> + 'me { + pub fn exports<'module>( + &'module self, + ) -> impl ExactSizeIterator> + 'module { let module = self.inner.compiled.module_ref(); module.exports.iter().map(move |(name, entity_index)| { let r#type = EntityType::new(entity_index, module); From 2ce07d0a463b6b2144382c851b3185a720285230 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 20 Apr 2020 13:08:14 -0700 Subject: [PATCH 24/24] More lifetime cleanups. --- crates/api/src/types.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/api/src/types.rs b/crates/api/src/types.rs index dc011f1762a7..5b3513f6b112 100644 --- a/crates/api/src/types.rs +++ b/crates/api/src/types.rs @@ -462,18 +462,18 @@ impl<'module> ImportType<'module> { } /// Returns the module name that this import is expected to come from. - pub fn module(&'module self) -> &'module str { + pub fn module(&self) -> &'module str { self.module } /// Returns the field name of the module that this import is expected to /// come from. - pub fn name(&'module self) -> &'module str { + pub fn name(&self) -> &'module str { self.name } /// Returns the expected type of this import. - pub fn ty(&'module self) -> ExternType { + pub fn ty(&self) -> ExternType { self.ty.extern_type() } } @@ -512,13 +512,13 @@ impl<'module> ExportType<'module> { ExportType { name, ty } } - /// Returns the name by which this export is known by. - pub fn name(&'module self) -> &'module str { + /// Returns the name by which this export is known. + pub fn name(&self) -> &'module str { self.name } /// Returns the type of this export. - pub fn ty(&'module self) -> ExternType { + pub fn ty(&self) -> ExternType { self.ty.extern_type() } }