From bbd0941d3eaa1b61fded4b8daee1dd6472721acf Mon Sep 17 00:00:00 2001 From: vms Date: Sun, 7 Jun 2020 22:57:30 +0300 Subject: [PATCH] fix bug with leaked imports --- examples/ipfs_node/src/imports.rs | 29 +++++- examples/ipfs_node/src/node.rs | 3 +- examples/ipfs_node/wasm/ipfs_node/src/lib.rs | 4 +- examples/ipfs_node/wasm/ipfs_rpc/src/lib.rs | 6 +- examples/ipfs_node/wasm/ipfs_rpc/wit | 18 ++-- fce/src/vm/fce.rs | 11 +- fce/src/vm/module/fce_module.rs | 100 +++++++++++++------ fce/src/vm/module/wit_function.rs | 44 +++----- fce/src/vm/module/wit_instance.rs | 8 +- 9 files changed, 134 insertions(+), 89 deletions(-) diff --git a/examples/ipfs_node/src/imports.rs b/examples/ipfs_node/src/imports.rs index ba22f626..b716b2e2 100644 --- a/examples/ipfs_node/src/imports.rs +++ b/examples/ipfs_node/src/imports.rs @@ -27,14 +27,38 @@ pub(super) fn log_utf8_string(ctx: &mut Ctx, offset: i32, size: i32) { } } +pub(super) fn ipfs(ctx: &mut Ctx, offset: i32, size: i32) -> i32 { + use wasmer_core::memory::ptr::{Array, WasmPtr}; + + let wasm_ptr = WasmPtr::::new(offset as _); + match wasm_ptr.get_utf8_string(ctx.memory(0), size as _) { + Some(msg) => println!("ipfs"), + None => println!("ipfs\n"), + } + + 0x1337 +} + +#[derive(Clone)] +struct T {} + +impl Drop for T { + fn drop(&mut self) { + println!("drop T"); + } +} + pub(super) fn create_host_import_func(host_cmd: String) -> DynamicFunc<'static> { use wasmer_core::types::Value; use wasmer_core::types::Type; use wasmer_core::types::FuncSig; + let t = T{}; let func = move |ctx: &mut Ctx, inputs: &[Value]| -> Vec { use wasmer_core::memory::ptr::{Array, WasmPtr}; + let _t = t.clone(); + println!("inputs size is {}", inputs.len()); // TODO: refactor this let array_ptr = inputs[1].to_u128() as i32; @@ -44,9 +68,10 @@ pub(super) fn create_host_import_func(host_cmd: String) -> DynamicFunc<'static> let wasm_ptr = WasmPtr::::new(array_ptr as _); match wasm_ptr.get_utf8_string(ctx.memory(0), array_size as _) { Some(msg) => print!("{}", msg), - None => print!("ipfs node logger: incorrect UTF8 string's been supplied to logger"), + None => print!("callback: incorrect UTF8 string's been supplied to logger"), } - vec![] + + vec![Value::I32(0x1337)] }; DynamicFunc::new( diff --git a/examples/ipfs_node/src/node.rs b/examples/ipfs_node/src/node.rs index bfa43524..4c96b176 100644 --- a/examples/ipfs_node/src/node.rs +++ b/examples/ipfs_node/src/node.rs @@ -81,6 +81,7 @@ impl IpfsNode { self.process .load_module(rpc_module_name, wasm_rpc, self.rpc_module_config.clone())?; + let call_result = self.process.call( rpc_module_name, "invoke", @@ -131,9 +132,9 @@ impl IpfsNode { if let Some(imports) = module_config.imports { for (import_name, host_cmd) in imports { - println!("{} - {}", import_name, host_cmd); let host_import = create_host_import_func(host_cmd); namespace.insert(import_name, host_import); + //namespace.insert(import_name, func!(crate::imports::ipfs)); } } diff --git a/examples/ipfs_node/wasm/ipfs_node/src/lib.rs b/examples/ipfs_node/wasm/ipfs_node/src/lib.rs index 3ea6b43d..7dc0ac4d 100644 --- a/examples/ipfs_node/wasm/ipfs_node/src/lib.rs +++ b/examples/ipfs_node/wasm/ipfs_node/src/lib.rs @@ -33,9 +33,9 @@ pub unsafe fn put(file_content_ptr: *mut u8, file_content_size: usize) { log_utf8_string(msg.as_ptr() as _, msg.len() as _); let cmd = format!("put {}", file_content); - ipfs(cmd.as_ptr() as _, cmd.len() as _); + let ipfs_result = ipfs(cmd.as_ptr() as _, cmd.len() as _); - let after_ipfs = format!("after ipfs call"); + let after_ipfs = format!("after ipfs call: {} \n", ipfs_result); log_utf8_string(after_ipfs.as_ptr() as _, after_ipfs.len() as _); let result = "IPFS node: hash is asdasdsad".to_string(); diff --git a/examples/ipfs_node/wasm/ipfs_rpc/src/lib.rs b/examples/ipfs_node/wasm/ipfs_rpc/src/lib.rs index f1e19a6b..5c20fbd5 100644 --- a/examples/ipfs_node/wasm/ipfs_rpc/src/lib.rs +++ b/examples/ipfs_node/wasm/ipfs_rpc/src/lib.rs @@ -35,9 +35,11 @@ pub unsafe fn invoke(file_content_ptr: *mut u8, file_content_size: usize) { *RESULT_SIZE.get_mut(), *RESULT_SIZE.get_mut(), ); - let msg = format!("from Wasm rpc: hash is {}\n", hash); - log_utf8_string(msg.as_ptr() as _, msg.len() as _); + let result_msg = format!("result from Wasm rpc: {}\n", hash); + *RESULT_PTR.get_mut() = result_msg.as_ptr() as _; + *RESULT_SIZE.get_mut() = result_msg.len(); + std::mem::forget(result_msg); } #[link(wasm_import_module = "host")] diff --git a/examples/ipfs_node/wasm/ipfs_rpc/wit b/examples/ipfs_node/wasm/ipfs_rpc/wit index 5c4f1b20..38b831c0 100644 --- a/examples/ipfs_node/wasm/ipfs_rpc/wit +++ b/examples/ipfs_node/wasm/ipfs_rpc/wit @@ -52,14 +52,15 @@ arg.get 0 string.lower_memory call-core 2 ;; call invoke - call-core 3 ;; call get_result_size - call-core 4 ;; call get_result_ptr + call-core 4 ;; call get_result_size + call-core 3 ;; call get_result_ptr string.lift_memory - call-core 3 ;; call get_result_size - call-core 4 ;; call get_result_ptr + call-core 4 ;; call get_result_size + call-core 3 ;; call get_result_ptr call-core 1 ;; call deallocate ) +;; adapter for import function ipfs.get (@interface func (type 9) arg.get 0 arg.get 1 @@ -70,10 +71,11 @@ call-core 0 ;; call allocate swap2 string.lower_memory - call-core 6 ;; call set_result_size - call-core 5 ;; call set_result_ptr + call-core 5 ;; call set_result_size + call-core 6 ;; call set_result_ptr ) +;; adapter for import function ipfs.put (@interface func (type 10) arg.get 0 arg.get 1 @@ -84,8 +86,8 @@ call-core 0 ;; call allocate swap2 string.lower_memory - call-core 6 ;; call set_result_size - call-core 5 ;; call set_result_ptr + call-core 5 ;; call set_result_size + call-core 6 ;; call set_result_ptr ) ;; Implementations diff --git a/fce/src/vm/fce.rs b/fce/src/vm/fce.rs index d8ddf279..83b413af 100644 --- a/fce/src/vm/fce.rs +++ b/fce/src/vm/fce.rs @@ -25,12 +25,12 @@ use std::collections::HashMap; pub struct FCE { // set of modules registered inside FCE - modules: HashMap>, + modules: HashMap, } impl Drop for FCE { fn drop(&mut self) { - // println!("FCE dropped"); + println!("FCE dropped"); } } @@ -57,9 +57,7 @@ impl WasmProcess for FCE { ) -> Result, FCEError> { match self.modules.get_mut(module_name) { // TODO: refactor errors - Some(mut module) => unsafe { - Ok(Arc::get_mut_unchecked(&mut module).call(func_name, argument)?) - }, + Some(mut module) => module.call(func_name, argument), None => { println!("no such module"); Err(FCEError::NoSuchModule) @@ -83,7 +81,7 @@ impl WasmProcess for FCE { match self.modules.entry(module_name.into()) { Entry::Vacant(entry) => { - entry.insert(Arc::new(module)); + entry.insert(module); Ok(()) } Entry::Occupied(_) => Err(FCEError::NonUniqueModuleName), @@ -105,7 +103,6 @@ impl WasmProcess for FCE { match self.modules.get(module_name) { Some(module) => { let signatures = module - .as_ref() .get_exports_signatures() .map(|(name, inputs, outputs)| NodeFunction { name, diff --git a/fce/src/vm/module/fce_module.rs b/fce/src/vm/module/fce_module.rs index de95135f..09874870 100644 --- a/fce/src/vm/module/fce_module.rs +++ b/fce/src/vm/module/fce_module.rs @@ -28,14 +28,36 @@ use std::collections::HashMap; use std::convert::TryInto; use std::mem::MaybeUninit; use std::sync::Arc; +use std::borrow::BorrowMut; type WITInterpreter = Interpreter>; -struct WITModuleFunc { - interpreter: WITInterpreter, - inputs: Vec, - outputs: Vec, +#[derive(Clone)] +pub(super) struct WITModuleFunc { + interpreter: Arc, + pub(super) inputs: Vec, + pub(super) outputs: Vec, +} + +#[derive(Clone)] +pub(super) struct Callable { + pub(super) wit_instance: Arc, + pub(super) wit_module_func: WITModuleFunc, +} + +impl Callable { + pub fn call(&mut self, args: &[IValue]) -> Result, FCEError> { + use wasmer_wit::interpreter::stack::Stackable; + + let result = self.wit_module_func + .interpreter + .run(args, Arc::make_mut(&mut self.wit_instance))? + .as_slice() + .to_owned(); + + Ok(result) + } } pub struct FCEModule { @@ -43,32 +65,36 @@ pub struct FCEModule { // that internally keep pointer to Wasmer instance. #[allow(unused)] wamser_instance: WasmerInstance, - wit_instance: Arc, - exports_funcs: HashMap, + import_object: ImportObject, + + // TODO: replace with dyn Trait + pub(super) exports_funcs: HashMap>, } impl Drop for FCEModule { fn drop(&mut self) { - // println!("FCEModule dropped: {:?}", self.exports_funcs.keys()); + println!("FCEModule dropped: {:?}", self.exports_funcs.keys()); } } impl FCEModule { pub fn new( wasm_bytes: &[u8], - imports: ImportObject, - modules: &HashMap>, + fce_imports: ImportObject, + modules: &HashMap, ) -> Result { let wasmer_module = compile(&wasm_bytes)?; let wit = extract_wit(&wasmer_module)?; let fce_wit = FCEWITInterfaces::new(wit); - let wit_exports = Self::instantiate_wit_exports(&fce_wit)?; let mut wit_instance = Arc::new_uninit(); let mut import_object = Self::adjust_wit_imports(&fce_wit, wit_instance.clone())?; - import_object.extend(imports); + let mut fce_imports = fce_imports; - let wasmer_instance = wasmer_module.instantiate(&import_object)?; + fce_imports.extend(import_object.clone()); + + + let wasmer_instance = wasmer_module.instantiate(&fce_imports)?; let wit_instance = unsafe { // get_mut_unchecked here is safe because currently only this modules have reference to @@ -78,24 +104,21 @@ impl FCEModule { std::mem::transmute::<_, Arc>(wit_instance) }; + let exports_funcs = Self::instantiate_wit_exports(wit_instance.clone(), &fce_wit)?; + Ok(Self { wamser_instance: wasmer_instance, - wit_instance, - exports_funcs: wit_exports, + import_object, + exports_funcs, }) } pub fn call(&mut self, function_name: &str, args: &[IValue]) -> Result, FCEError> { use wasmer_wit::interpreter::stack::Stackable; - match self.exports_funcs.get(function_name) { + match self.exports_funcs.get_mut(function_name) { Some(func) => { - let result = func - .interpreter - .run(args, Arc::make_mut(&mut self.wit_instance))? - .as_slice() - .to_owned(); - Ok(result) + Arc::make_mut(func).call(args) } None => Err(FCEError::NoSuchFunction(format!( "{} hasn't been found while calling", @@ -109,7 +132,7 @@ impl FCEModule { function_name: &str, ) -> Result<(&Vec, &Vec), FCEError> { match self.exports_funcs.get(function_name) { - Some(func) => Ok((&func.inputs, &func.outputs)), + Some(func) => Ok((&func.wit_module_func.inputs, &func.wit_module_func.outputs)), None => { for func in self.exports_funcs.iter() { println!("{}", func.0); @@ -126,14 +149,19 @@ impl FCEModule { pub fn get_exports_signatures( &self, ) -> impl Iterator, &Vec)> { - self.exports_funcs - .iter() - .map(|(func_name, func)| (func_name, &func.inputs, &func.outputs)) + self.exports_funcs.iter().map(|(func_name, func)| { + ( + func_name, + &func.wit_module_func.inputs, + &func.wit_module_func.outputs, + ) + }) } fn instantiate_wit_exports( + wit_instance: Arc, wit: &FCEWITInterfaces<'_>, - ) -> Result, FCEError> { + ) -> Result>, FCEError> { use fce_wit_interfaces::WITAstType; wit.implementations() @@ -161,14 +189,18 @@ impl FCEModule { inputs, outputs, .. } => { let interpreter: WITInterpreter = adapter_instructions.try_into()?; + let wit_module_func = WITModuleFunc { + interpreter: Arc::new(interpreter), + inputs: inputs.clone(), + outputs: outputs.clone(), + }; Ok(( export_function_name.to_string(), - WITModuleFunc { - interpreter, - inputs: inputs.clone(), - outputs: outputs.clone(), - }, + Arc::new(Callable { + wit_instance: wit_instance.clone(), + wit_module_func, + }), )) } _ => Err(FCEError::IncorrectWIT(format!( @@ -177,7 +209,7 @@ impl FCEModule { ))), } }) - .collect::, FCEError>>() + .collect::>, FCEError>>() } // this function deals only with import functions that have an adaptor implementation @@ -194,7 +226,7 @@ impl FCEModule { impl Drop for T { fn drop(&mut self) { - // println!("drop T"); + println!("fce_module imports: drop T"); } } @@ -215,6 +247,8 @@ impl FCEModule { use super::type_converters::wval_to_ival; let t_copied = t.clone(); + println!("dyn_func_from_raw_import: {:?}", inputs); + // copy here because otherwise wit_instance will be consumed by the closure let wit_instance_callable = wit_instance.clone(); let converted_inputs = inputs.iter().map(wval_to_ival).collect::>(); diff --git a/fce/src/vm/module/wit_function.rs b/fce/src/vm/module/wit_function.rs index ffaf0496..1ede5093 100644 --- a/fce/src/vm/module/wit_function.rs +++ b/fce/src/vm/module/wit_function.rs @@ -17,6 +17,7 @@ use super::wit_prelude::FCEError; use super::fce_module::FCEModule; use super::{IType, IValue, WValue}; +use crate::vm::module::fce_module::Callable; use wasmer_wit::interpreter::wasm; use wasmer_core::instance::DynFunc; @@ -32,10 +33,7 @@ enum WITFunctionInner { }, Import { // TODO: use dyn Callable here - wit_module: Arc, - func_name: String, - inputs: Vec, - outputs: Vec, + callable: Arc, }, } @@ -49,10 +47,10 @@ impl Drop for WITFunction { fn drop(&mut self) { match &self.inner { WITFunctionInner::Export { func, .. } => { - // println!("WITFunction export dropped: {:?}", func.signature()); + println!("WITFunction export dropped: {:?}", func.signature()); } - WITFunctionInner::Import { func_name, .. } => { - // println!("WITFunction import dropped: {:?}", func_name); + WITFunctionInner::Import { callable } => { + println!("WITFunction import dropped: {:?}", callable.wit_module_func.inputs); } } } @@ -86,18 +84,13 @@ impl WITFunction { /// Creates function from a module import. pub(super) fn from_import( - wit_module: Arc, + wit_module: &FCEModule, func_name: String, ) -> Result { - let func_type = wit_module.as_ref().get_func_signature(&func_name)?; - let inputs = func_type.0.clone(); - let outputs = func_type.1.clone(); + let callable = wit_module.exports_funcs.get(&func_name).unwrap().clone(); let inner = WITFunctionInner::Import { - wit_module, - func_name, - inputs, - outputs, + callable }; Ok(Self { inner }) @@ -108,28 +101,28 @@ impl wasm::structures::LocalImport for WITFunction { fn inputs_cardinality(&self) -> usize { match &self.inner { WITFunctionInner::Export { ref inputs, .. } => inputs.len(), - WITFunctionInner::Import { ref inputs, .. } => inputs.len(), + WITFunctionInner::Import { ref callable, .. } => callable.wit_module_func.inputs.len(), } } fn outputs_cardinality(&self) -> usize { match &self.inner { WITFunctionInner::Export { ref outputs, .. } => outputs.len(), - WITFunctionInner::Import { ref outputs, .. } => outputs.len(), + WITFunctionInner::Import { ref callable, .. } => callable.wit_module_func.outputs.len(), } } fn inputs(&self) -> &[IType] { match &self.inner { WITFunctionInner::Export { ref inputs, .. } => inputs, - WITFunctionInner::Import { ref inputs, .. } => inputs, + WITFunctionInner::Import { ref callable, .. } => &callable.wit_module_func.inputs, } } fn outputs(&self) -> &[IType] { match &self.inner { WITFunctionInner::Export { ref outputs, .. } => outputs, - WITFunctionInner::Import { ref outputs, .. } => outputs, + WITFunctionInner::Import { ref callable, .. } => &callable.wit_module_func.outputs, } } @@ -143,18 +136,9 @@ impl wasm::structures::LocalImport for WITFunction { .map(|result| result.iter().map(wval_to_ival).collect()) .map_err(|_| ()), WITFunctionInner::Import { - wit_module, - func_name, - .. + callable } => { - let mut wit_module_caller = wit_module.clone(); - unsafe { - // get_mut_unchecked here is safe because it is single-threaded environment - // without cyclic reference between modules - Arc::get_mut_unchecked(&mut wit_module_caller) - .call(func_name, arguments) - .map_err(|_| ()) - } + Arc::make_mut(&mut callable.clone()).call(arguments).map_err(|_| ()) } } } diff --git a/fce/src/vm/module/wit_instance.rs b/fce/src/vm/module/wit_instance.rs index a7e640d2..bc2487e8 100644 --- a/fce/src/vm/module/wit_instance.rs +++ b/fce/src/vm/module/wit_instance.rs @@ -35,7 +35,7 @@ pub(super) struct WITInstance { impl Drop for WITInstance { fn drop(&mut self) { - // println!("WITInstance dropped"); + println!("WITInstance dropped"); } } @@ -43,7 +43,7 @@ impl WITInstance { pub(super) fn new( wasmer_instance: &WasmerInstance, wit: &FCEWITInterfaces<'_>, - modules: &HashMap>, + modules: &HashMap, ) -> Result { let mut exports = Self::extract_raw_exports(&wasmer_instance, wit)?; let imports = Self::extract_imports(modules, wit, exports.len())?; @@ -80,7 +80,7 @@ impl WITInstance { /// Extracts only those imports that don't have implementations. fn extract_imports( - modules: &HashMap>, + modules: &HashMap, wit: &FCEWITInterfaces<'_>, start_index: usize, ) -> Result, FCEError> { @@ -92,7 +92,7 @@ impl WITInstance { .enumerate() .map(|(idx, import)| match modules.get(import.namespace) { Some(module) => { - let func = WITFunction::from_import(module.clone(), import.name.to_string())?; + let func = WITFunction::from_import(module, import.name.to_string())?; Ok((start_index + idx as usize, func)) } None => Err(FCEError::NoSuchModule),