From a18371eb9118644203669fe8bc813ab8aea44d37 Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Thu, 26 Mar 2020 16:24:23 -0700 Subject: [PATCH] Implement `instance.exports` field syntax --- CHANGELOG.md | 2 +- lib/api-tests/tests/high_level_api.rs | 14 +- lib/runtime-core-tests/tests/imports.rs | 8 +- lib/runtime-core/src/export.rs | 4 +- lib/runtime-core/src/instance.rs | 299 +++++++++++++----------- 5 files changed, 181 insertions(+), 146 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1d7658b9..6047f59c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ - [#1313](https://github.com/wasmerio/wasmer/pull/1313) Add new high-level public API through `wasmer` crate. Includes many updates including: - Minor improvement: `imports!` macro now handles no trailing comma as well as a trailing comma in namespaces and between namespaces. - New methods on `Module`: `exports`, `imports`, and `custom_sections`. - - TODO: update this when the method name changes. New way to get exports from an instance with `let func_name: Func = instance.exports_new().get("func_name");`. + - New way to get exports from an instance with `let func_name: Func = instance.exports.get("func_name");`. - Improved `Table` APIs including `set` which now allows setting functions directly. TODO: update this more if `Table::get` gets made public in this PR - TODO: finish the list of changes here - [#1303](https://github.com/wasmerio/wasmer/pull/1303) NaN canonicalization for singlepass backend. diff --git a/lib/api-tests/tests/high_level_api.rs b/lib/api-tests/tests/high_level_api.rs index 88136c15b..22aaf93b3 100644 --- a/lib/api-tests/tests/high_level_api.rs +++ b/lib/api-tests/tests/high_level_api.rs @@ -128,19 +128,19 @@ fn it_works() { }; let instance = module.instantiate(&import_object).unwrap(); - let ret_2: Func<(), i32> = instance.exports_new().get("ret_2").unwrap(); - let ret_4: Func<(), i32> = instance.exports_new().get("ret_4").unwrap(); - let set_test_global: Func = instance.exports_new().get("set_test_global").unwrap(); - let update_outside_global: Func = instance.exports_new().get("update_outside_global").unwrap(); + let ret_2: Func<(), i32> = instance.exports.get("ret_2").unwrap(); + let ret_4: Func<(), i32> = instance.exports.get("ret_4").unwrap(); + let set_test_global: Func = instance.exports.get("set_test_global").unwrap(); + let update_outside_global: Func = instance.exports.get("update_outside_global").unwrap(); assert_eq!(ret_2.call(), Ok(2)); assert_eq!(ret_4.call(), Ok(4)); - let _test_table: Table = instance.exports_new().get("test-table").unwrap(); + let _test_table: Table = instance.exports.get("test-table").unwrap(); // TODO: when table get is stablized test this - let test_global: Global = instance.exports_new().get("test-global").unwrap(); - // TODO: do we want to make global.get act like exports_new().get()? + let test_global: Global = instance.exports.get("test-global").unwrap(); + // TODO: do we want to make global.get act like exports.get()? assert_eq!(test_global.get(), Value::I32(3)); set_test_global.call(17).unwrap(); assert_eq!(test_global.get(), Value::I32(17)); diff --git a/lib/runtime-core-tests/tests/imports.rs b/lib/runtime-core-tests/tests/imports.rs index bdebc62f1..77754ae81 100644 --- a/lib/runtime-core-tests/tests/imports.rs +++ b/lib/runtime-core-tests/tests/imports.rs @@ -30,13 +30,13 @@ fn new_api_works() { let import_object = imports! {}; let instance = module.instantiate(&import_object).unwrap(); - let my_global: Global = instance.exports_new().get("my_global").unwrap(); + let my_global: Global = instance.exports.get("my_global").unwrap(); assert_eq!(my_global.get(), Value::I32(45)); - let double: Func = instance.exports_new().get("double").unwrap(); + let double: Func = instance.exports.get("double").unwrap(); assert_eq!(double.call(5).unwrap(), 10); - let add_one: DynFunc = instance.exports_new().get("add_one").unwrap(); + let add_one: DynFunc = instance.exports.get("add_one").unwrap(); assert_eq!(add_one.call(&[Value::I32(5)]).unwrap(), &[Value::I32(6)]); - let add_one_memory: Option = instance.exports_new().get("my_global"); + let add_one_memory: Option = instance.exports.get("my_global"); assert!(add_one_memory.is_none()); } diff --git a/lib/runtime-core/src/export.rs b/lib/runtime-core/src/export.rs index b97668d0a..7eb322663 100644 --- a/lib/runtime-core/src/export.rs +++ b/lib/runtime-core/src/export.rs @@ -2,7 +2,7 @@ //! including memories, tables, globals, and functions. use crate::{ global::Global, - instance::{Instance, InstanceInner}, + instance::{Exports, InstanceInner}, memory::Memory, module::ExportIndex, module::ModuleInner, @@ -102,5 +102,5 @@ impl<'a> Iterator for ExportIter<'a> { pub trait Exportable<'a>: Sized { /// Implementation of how to get the export corresponding to the implementing type /// from an [`Instance`] by name. - fn get_self(instance: &'a Instance, name: &str) -> Option; + fn get_self(exports: &'a Exports, name: &str) -> Option; } diff --git a/lib/runtime-core/src/instance.rs b/lib/runtime-core/src/instance.rs index 5261b60e8..af854a8a8 100644 --- a/lib/runtime-core/src/instance.rs +++ b/lib/runtime-core/src/instance.rs @@ -56,16 +56,13 @@ pub struct Instance { /// Reference to the module used to instantiate this instance. pub module: Arc, inner: Pin>, + /// The exports of this instance. TODO: document this + pub exports: Exports, #[allow(dead_code)] import_object: ImportObject, } impl Instance { - /// Access the new exports API. - /// TODO: rename etc. - pub fn exports_new(&self) -> Exports { - Exports { instance: self } - } pub(crate) fn new(module: Arc, imports: &ImportObject) -> Result { // We need the backing and import_backing to create a vm::Ctx, but we need // a vm::Ctx to create a backing and an import_backing. The solution is to create an @@ -97,9 +94,15 @@ impl Instance { }; Box::leak(vmctx); + let exports = Exports { + module: module.clone(), + instance_inner: &*inner as *const InstanceInner, + }; + let instance = Instance { module, inner, + exports, import_object: imports.clone_ref(), }; @@ -183,77 +186,7 @@ impl Instance { Args: WasmTypeList, Rets: WasmTypeList, { - let export_index = - self.module - .info - .exports - .get(name) - .ok_or_else(|| ResolveError::ExportNotFound { - name: name.to_string(), - })?; - - if let ExportIndex::Func(func_index) = export_index { - let sig_index = *self - .module - .info - .func_assoc - .get(*func_index) - .expect("broken invariant, incorrect func index"); - let signature = - SigRegistry.lookup_signature_ref(&self.module.info.signatures[sig_index]); - - if signature.params() != Args::types() || signature.returns() != Rets::types() { - Err(ResolveError::Signature { - expected: (*signature).clone(), - found: Args::types().to_vec(), - })?; - } - - let ctx = match func_index.local_or_import(&self.module.info) { - LocalOrImport::Local(_) => self.inner.vmctx, - LocalOrImport::Import(imported_func_index) => unsafe { - self.inner.import_backing.vm_functions[imported_func_index] - .func_ctx - .as_ref() - } - .vmctx - .as_ptr(), - }; - - let func_wasm_inner = self - .module - .runnable_module - .get_trampoline(&self.module.info, sig_index) - .unwrap(); - - let (func_ptr, func_env) = match func_index.local_or_import(&self.module.info) { - LocalOrImport::Local(local_func_index) => ( - self.module - .runnable_module - .get_func(&self.module.info, local_func_index) - .unwrap(), - None, - ), - LocalOrImport::Import(import_func_index) => { - let imported_func = &self.inner.import_backing.vm_functions[import_func_index]; - - ( - NonNull::new(imported_func.func as *mut _).unwrap(), - unsafe { imported_func.func_ctx.as_ref() }.func_env, - ) - } - }; - - let typed_func: Func = - unsafe { Func::from_raw_parts(func_wasm_inner, func_ptr, func_env, ctx) }; - - Ok(typed_func) - } else { - Err(ResolveError::ExportWrongType { - name: name.to_string(), - } - .into()) - } + func(&*self.module, &self.inner, name) } /// Resolve a function by name. @@ -292,37 +225,7 @@ impl Instance { /// # } /// ``` pub fn dyn_func(&self, name: &str) -> ResolveResult { - let export_index = - self.module - .info - .exports - .get(name) - .ok_or_else(|| ResolveError::ExportNotFound { - name: name.to_string(), - })?; - - if let ExportIndex::Func(func_index) = export_index { - let sig_index = *self - .module - .info - .func_assoc - .get(*func_index) - .expect("broken invariant, incorrect func index"); - let signature = - SigRegistry.lookup_signature_ref(&self.module.info.signatures[sig_index]); - - Ok(DynFunc { - signature, - module: &self.module, - instance_inner: &self.inner, - func_index: *func_index, - }) - } else { - Err(ResolveError::ExportWrongType { - name: name.to_string(), - } - .into()) - } + dyn_func(&*self.module, &self.inner, name) } /// Call an exported WebAssembly function given the export name. @@ -419,6 +322,124 @@ impl Instance { } } +/// Private function implementing the inner logic of `Instance::func` +// TODO: reevaluate this lifetime +fn func<'a, Args, Rets>( + module: &'a ModuleInner, + inst_inner: &'a InstanceInner, + name: &str, +) -> ResolveResult> +where + Args: WasmTypeList, + Rets: WasmTypeList, +{ + let export_index = + module + .info + .exports + .get(name) + .ok_or_else(|| ResolveError::ExportNotFound { + name: name.to_string(), + })?; + + if let ExportIndex::Func(func_index) = export_index { + let sig_index = *module + .info + .func_assoc + .get(*func_index) + .expect("broken invariant, incorrect func index"); + let signature = SigRegistry.lookup_signature_ref(&module.info.signatures[sig_index]); + + if signature.params() != Args::types() || signature.returns() != Rets::types() { + Err(ResolveError::Signature { + expected: (*signature).clone(), + found: Args::types().to_vec(), + })?; + } + + let ctx = match func_index.local_or_import(&module.info) { + LocalOrImport::Local(_) => inst_inner.vmctx, + LocalOrImport::Import(imported_func_index) => unsafe { + inst_inner.import_backing.vm_functions[imported_func_index] + .func_ctx + .as_ref() + } + .vmctx + .as_ptr(), + }; + + let func_wasm_inner = module + .runnable_module + .get_trampoline(&module.info, sig_index) + .unwrap(); + + let (func_ptr, func_env) = match func_index.local_or_import(&module.info) { + LocalOrImport::Local(local_func_index) => ( + module + .runnable_module + .get_func(&module.info, local_func_index) + .unwrap(), + None, + ), + LocalOrImport::Import(import_func_index) => { + let imported_func = &inst_inner.import_backing.vm_functions[import_func_index]; + + ( + NonNull::new(imported_func.func as *mut _).unwrap(), + unsafe { imported_func.func_ctx.as_ref() }.func_env, + ) + } + }; + + let typed_func: Func = + unsafe { Func::from_raw_parts(func_wasm_inner, func_ptr, func_env, ctx) }; + + Ok(typed_func) + } else { + Err(ResolveError::ExportWrongType { + name: name.to_string(), + } + .into()) + } +} + +/// Private function implementing the inner logic of `Instance::dyn_func` +fn dyn_func<'a>( + module: &'a ModuleInner, + inst_inner: &'a InstanceInner, + name: &str, +) -> ResolveResult> { + let export_index = + module + .info + .exports + .get(name) + .ok_or_else(|| ResolveError::ExportNotFound { + name: name.to_string(), + })?; + + if let ExportIndex::Func(func_index) = export_index { + let sig_index = *module + .info + .func_assoc + .get(*func_index) + .expect("broken invariant, incorrect func index"); + let signature = SigRegistry.lookup_signature_ref(&module.info.signatures[sig_index]); + + Ok(DynFunc { + signature, + module: &module, + instance_inner: &inst_inner, + func_index: *func_index, + }) + } else { + Err(ResolveError::ExportWrongType { + name: name.to_string(), + } + .into()) + } +} + impl InstanceInner { pub(crate) fn get_export_from_index( &self, @@ -823,11 +844,11 @@ impl<'a> DynFunc<'a> { } impl<'a> Exportable<'a> for Memory { - fn get_self(instance: &'a Instance, name: &str) -> Option { - let export_index = instance.module.info.exports.get(name)?; + fn get_self(exports: &'a Exports, name: &str) -> Option { + let (inst_inner, module) = exports.get_inner(); + let export_index = module.info.exports.get(name)?; if let ExportIndex::Memory(idx) = export_index { - let module = instance.module.borrow(); - Some(instance.inner.get_memory_from_index(module, *idx)) + Some(inst_inner.get_memory_from_index(module, *idx)) } else { None } @@ -835,11 +856,11 @@ impl<'a> Exportable<'a> for Memory { } impl<'a> Exportable<'a> for Table { - fn get_self(instance: &'a Instance, name: &str) -> Option { - let export_index = instance.module.info.exports.get(name)?; + fn get_self(exports: &'a Exports, name: &str) -> Option { + let (inst_inner, module) = exports.get_inner(); + let export_index = module.info.exports.get(name)?; if let ExportIndex::Table(idx) = export_index { - let module = instance.module.borrow(); - Some(instance.inner.get_table_from_index(module, *idx)) + Some(inst_inner.get_table_from_index(module, *idx)) } else { None } @@ -847,11 +868,11 @@ impl<'a> Exportable<'a> for Table { } impl<'a> Exportable<'a> for Global { - fn get_self(instance: &'a Instance, name: &str) -> Option { - let export_index = instance.module.info.exports.get(name)?; + fn get_self(exports: &'a Exports, name: &str) -> Option { + let (inst_inner, module) = exports.get_inner(); + let export_index = module.info.exports.get(name)?; if let ExportIndex::Global(idx) = export_index { - let module = instance.module.borrow(); - Some(instance.inner.get_global_from_index(module, *idx)) + Some(inst_inner.get_global_from_index(module, *idx)) } else { None } @@ -859,26 +880,33 @@ impl<'a> Exportable<'a> for Global { } impl<'a> Exportable<'a> for DynFunc<'a> { - fn get_self(instance: &'a Instance, name: &str) -> Option { - instance.dyn_func(name).ok() + fn get_self(exports: &'a Exports, name: &str) -> Option { + let (inst_inner, module) = exports.get_inner(); + dyn_func(module, inst_inner, name).ok() } } impl<'a, Args: WasmTypeList, Rets: WasmTypeList> Exportable<'a> for Func<'a, Args, Rets, Wasm> { - fn get_self(instance: &'a Instance, name: &str) -> Option { - instance.func(name).ok() + fn get_self(exports: &'a Exports, name: &str) -> Option { + let (inst_inner, module) = exports.get_inner(); + func(module, inst_inner, name).ok() } } /// `Exports` is used to get exports like [`Func`]s, [`DynFunc`]s, [`Memory`]s, /// [`Global`]s, and [`Table`]s from an [`Instance`]. /// -/// Use [`Instance::exports_new`] to get an `Exports` from an [`Instance`]. -pub struct Exports<'a> { - instance: &'a Instance, +/// Use `Instance.exports` to get an `Exports` from an [`Instance`]. +pub struct Exports { + // We want to avoid the borrow checker here. + // This is safe because + // 1. `Exports` can't be constructed or copied (so can't safely outlive `Instance`) + // 2. `InstanceInner` is `Pin>`, thus we know that it will not move. + instance_inner: *const InstanceInner, + module: Arc, } -impl<'a> Exports<'a> { +impl Exports { /// Get an export. /// /// ``` @@ -887,21 +915,28 @@ impl<'a> Exports<'a> { /// # use wasmer_runtime_core::types::Value; /// # fn example_fn(instance: &Instance) -> Option<()> { /// // We can get a function as a static `Func` - /// let func: Func = instance.exports_new().get("my_func")?; + /// let func: Func = instance.exports.get("my_func")?; /// let _result = func.call(42); /// /// // Or we can get it as a dynamic `DynFunc` - /// let dyn_func: DynFunc = instance.exports_new().get("my_func")?; + /// let dyn_func: DynFunc = instance.exports.get("my_func")?; /// let _result= dyn_func.call(&[Value::I32(42)]); /// /// // We can also get other exports like `Global`s, `Memory`s, and `Table`s - /// let _counter: Global = instance.exports_new().get("counter")?; + /// let _counter: Global = instance.exports.get("counter")?; /// /// # Some(()) /// # } /// ``` - pub fn get>(&self, name: &str) -> Option { - T::get_self(self.instance, name) + pub fn get<'a, T: Exportable<'a>>(&'a self, name: &str) -> Option { + T::get_self(self, name) + } + + /// This method must remain private. + fn get_inner(&self) -> (&InstanceInner, &ModuleInner) { + let inst_inner = unsafe { &*self.instance_inner }; + let module = self.module.borrow(); + (inst_inner, module) } }