From 1eaea6ecf1e0b0845e64d3d5cff833aa85e0ff71 Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Wed, 2 Dec 2020 16:56:05 -0800 Subject: [PATCH 1/9] Fix memory leak in host function envs --- lib/api/src/externals/function.rs | 77 ++++++++++++++++++++++--------- lib/api/src/native.rs | 11 +++-- lib/api/src/types.rs | 5 +- lib/engine/src/artifact.rs | 8 ++-- lib/engine/src/export.rs | 7 ++- lib/engine/src/resolver.rs | 44 +++++------------- lib/vm/src/export.rs | 8 ++-- lib/vm/src/imports.rs | 38 ++++----------- lib/vm/src/instance.rs | 42 +++++++++++++---- lib/vm/src/lib.rs | 2 +- 10 files changed, 135 insertions(+), 107 deletions(-) diff --git a/lib/api/src/externals/function.rs b/lib/api/src/externals/function.rs index bd629d1b944..64780a822aa 100644 --- a/lib/api/src/externals/function.rs +++ b/lib/api/src/externals/function.rs @@ -12,6 +12,7 @@ pub use inner::{UnsafeMutableEnv, WithUnsafeMutableEnv}; use std::cmp::max; use std::fmt; +use std::sync::Arc; use wasmer_engine::{Export, ExportFunction}; use wasmer_vm::{ raise_user_trap, resume_panic, wasmer_call_trampoline, VMCallerCheckedAnyfunc, @@ -87,10 +88,11 @@ impl Function { where F: Fn(&[Val]) -> Result, RuntimeError> + 'static, { - let dynamic_ctx = VMDynamicFunctionContext::from_context(VMDynamicFunctionWithoutEnv { - func: Box::new(func), - function_type: ty.clone(), - }); + let dynamic_ctx: VMDynamicFunctionContext = + VMDynamicFunctionContext::from_context(VMDynamicFunctionWithoutEnv { + func: Box::new(func), + function_type: ty.clone(), + }); // We don't yet have the address with the Wasm ABI signature. // The engine linker will replace the address with one pointing to a // generated dynamic trampoline. @@ -98,20 +100,27 @@ impl Function { let vmctx = VMFunctionEnvironment { host_env: Box::into_raw(Box::new(dynamic_ctx)) as *mut _, }; + let host_env_drop_fn: Option = + Some(|ptr: *mut std::ffi::c_void| { + unsafe { + Box::from_raw(ptr as *mut VMDynamicFunctionContext) + }; + }); Self { store: store.clone(), definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: false }), exported: ExportFunction { import_init_function_ptr: None, - vm_function: VMExportFunction { + host_env_drop_fn, + vm_function: Arc::new(VMExportFunction { address, kind: VMFunctionKind::Dynamic, vmctx, signature: ty.clone(), call_trampoline: None, instance_allocator: None, - }, + }), }, } } @@ -143,11 +152,12 @@ impl Function { F: Fn(&Env, &[Val]) -> Result, RuntimeError> + 'static, Env: Sized + WasmerEnv + 'static, { - let dynamic_ctx = VMDynamicFunctionContext::from_context(VMDynamicFunctionWithEnv { - env: Box::new(env), - func: Box::new(func), - function_type: ty.clone(), - }); + let dynamic_ctx: VMDynamicFunctionContext> = + VMDynamicFunctionContext::from_context(VMDynamicFunctionWithEnv { + env: Box::new(env), + func: Box::new(func), + function_type: ty.clone(), + }); // We don't yet have the address with the Wasm ABI signature. // The engine linker will replace the address with one pointing to a // generated dynamic trampoline. @@ -155,26 +165,44 @@ impl Function { let vmctx = VMFunctionEnvironment { host_env: Box::into_raw(Box::new(dynamic_ctx)) as *mut _, }; - // TODO: look into removing transmute by changing API type signatures + let import_init_function_ptr: fn(_, _) -> Result<(), _> = + |ptr: *mut std::ffi::c_void, instance: *const std::ffi::c_void| { + let ptr = ptr as *mut VMDynamicFunctionContext>; + unsafe { + let env = &mut *ptr; + let env: &mut Env = &mut *env.ctx.env; + let instance = &*(instance as *const crate::Instance); + Env::init_with_instance(env, instance) + } + }; let import_init_function_ptr = Some(unsafe { std::mem::transmute:: Result<(), _>, fn(_, _) -> Result<(), _>>( - Env::init_with_instance, + import_init_function_ptr, ) }); + let host_env_drop_fn: Option = + Some(|ptr: *mut std::ffi::c_void| { + unsafe { + Box::from_raw( + ptr as *mut VMDynamicFunctionContext>, + ) + }; + }); Self { store: store.clone(), definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: true }), exported: ExportFunction { import_init_function_ptr, - vm_function: VMExportFunction { + host_env_drop_fn, + vm_function: Arc::new(VMExportFunction { address, kind: VMFunctionKind::Dynamic, vmctx, signature: ty.clone(), call_trampoline: None, instance_allocator: None, - }, + }), }, } } @@ -221,14 +249,15 @@ impl Function { // TODO: figure out what's going on in this function: it takes an `Env` // param but also marks itself as not having an env import_init_function_ptr: None, - vm_function: VMExportFunction { + host_env_drop_fn: None, + vm_function: Arc::new(VMExportFunction { address, vmctx, signature, kind: VMFunctionKind::Static, call_trampoline: None, instance_allocator: None, - }, + }), }, } } @@ -278,6 +307,11 @@ impl Function { let vmctx = VMFunctionEnvironment { host_env: Box::into_raw(box_env) as *mut _, }; + let host_env_drop_fn: Option = + Some(|ptr: *mut std::ffi::c_void| { + unsafe { Box::from_raw(ptr as *mut Env) }; + }); + // TODO: look into removing transmute by changing API type signatures let import_init_function_ptr = Some(unsafe { std::mem::transmute:: Result<(), _>, fn(_, _) -> Result<(), _>>( @@ -291,14 +325,15 @@ impl Function { definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: true }), exported: ExportFunction { import_init_function_ptr, - vm_function: VMExportFunction { + host_env_drop_fn, + vm_function: Arc::new(VMExportFunction { address, kind: VMFunctionKind::Static, vmctx, signature, call_trampoline: None, instance_allocator: None, - }, + }), }, } } @@ -344,14 +379,14 @@ impl Function { definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: true }), exported: ExportFunction { import_init_function_ptr, - vm_function: VMExportFunction { + vm_function: Arc::new(VMExportFunction { address, kind: VMFunctionKind::Static, vmctx, signature, call_trampoline: None, instance_allocator: None, - }, + }), }, } } diff --git a/lib/api/src/native.rs b/lib/api/src/native.rs index 6938cf25d8f..8c4ea531379 100644 --- a/lib/api/src/native.rs +++ b/lib/api/src/native.rs @@ -15,6 +15,7 @@ use crate::externals::function::{ }; use crate::{FromToNativeWasmType, Function, FunctionType, RuntimeError, Store, WasmTypeList}; use std::panic::{catch_unwind, AssertUnwindSafe}; +use std::sync::Arc; use wasmer_engine::ExportFunction; use wasmer_types::NativeWasmType; use wasmer_vm::{ @@ -88,14 +89,15 @@ where Self { // TODO: import_init_function_ptr: None, - vm_function: VMExportFunction { + host_env_drop_fn: None, + vm_function: Arc::new(VMExportFunction { address: other.address, vmctx: other.vmctx, signature, kind: other.arg_kind, call_trampoline: None, instance_allocator: None, - }, + }), } } } @@ -113,14 +115,15 @@ where exported: ExportFunction { // TODO: import_init_function_ptr: None, - vm_function: VMExportFunction { + host_env_drop_fn: None, + vm_function: Arc::new(VMExportFunction { address: other.address, vmctx: other.vmctx, signature, kind: other.arg_kind, call_trampoline: None, instance_allocator: None, - }, + }), }, } } diff --git a/lib/api/src/types.rs b/lib/api/src/types.rs index cf1b7dab146..a8e72f7b4b6 100644 --- a/lib/api/src/types.rs +++ b/lib/api/src/types.rs @@ -77,7 +77,8 @@ impl ValFuncRef for Val { // TODO: // figure out if we ever need a value here: need testing with complicated import patterns import_init_function_ptr: None, - vm_function: wasmer_vm::VMExportFunction { + host_env_drop_fn: None, + vm_function: std::sync::Arc::new(wasmer_vm::VMExportFunction { address: item.func_ptr, signature, // All functions in tables are already Static (as dynamic functions @@ -86,7 +87,7 @@ impl ValFuncRef for Val { vmctx: item.vmctx, call_trampoline: None, instance_allocator: None, - }, + }), }; let f = Function::from_vm_export(store, export); Self::FuncRef(f) diff --git a/lib/engine/src/artifact.rs b/lib/engine/src/artifact.rs index 43ff9506c06..b91ddd66380 100644 --- a/lib/engine/src/artifact.rs +++ b/lib/engine/src/artifact.rs @@ -96,7 +96,7 @@ pub trait Artifact: Send + Sync + Upcastable { let module = self.module(); let (instance_ptr, offsets) = InstanceHandle::allocate_instance(&module); - let (imports, import_initializers) = { + let (imports, import_envs) = { let mut imports = resolve_imports( &module, resolver, @@ -108,9 +108,9 @@ pub trait Artifact: Send + Sync + Upcastable { // Get the `WasmerEnv::init_with_instance` function pointers and the pointers // to the envs to call it on. - let import_initializers: Vec<(_, _)> = imports.get_import_initializers(); + let import_envs = imports.get_import_initializers(); - (imports, import_initializers) + (imports, import_envs) }; // Get pointers to where metadata about local memories should live in VM memory. @@ -146,7 +146,7 @@ pub trait Artifact: Send + Sync + Upcastable { imports, self.signatures().clone(), host_state, - import_initializers, + import_envs, ) .map_err(|trap| InstantiationError::Start(RuntimeError::from_trap(trap)))?; Ok(handle) diff --git a/lib/engine/src/export.rs b/lib/engine/src/export.rs index 8f972c3f79f..d7533345fe2 100644 --- a/lib/engine/src/export.rs +++ b/lib/engine/src/export.rs @@ -3,6 +3,8 @@ use wasmer_vm::{ VMExportTable, }; +use std::sync::Arc; + /// The value of an export passed from one instance to another. #[derive(Debug, Clone)] pub enum Export { @@ -36,6 +38,7 @@ impl From for Export { VMExport::Function(vm_function) => Export::Function(ExportFunction { vm_function, import_init_function_ptr: None, + host_env_drop_fn: None, }), VMExport::Memory(vm_memory) => Export::Memory(ExportMemory { vm_memory }), VMExport::Table(vm_table) => Export::Table(ExportTable { vm_table }), @@ -49,12 +52,14 @@ impl From for Export { #[derive(Debug, Clone, PartialEq)] pub struct ExportFunction { /// The VM function, containing most of the data. - pub vm_function: VMExportFunction, + pub vm_function: Arc, /// Function pointer to `WasmerEnv::init_with_instance(&mut self, instance: &Instance)`. /// /// This function is called to finish setting up the environment after /// we create the `api::Instance`. pub import_init_function_ptr: Option, + /// The destructor to free the host environment. + pub host_env_drop_fn: Option, } impl From for Export { diff --git a/lib/engine/src/resolver.rs b/lib/engine/src/resolver.rs index f8d3ca40d88..8b7f75ab6b6 100644 --- a/lib/engine/src/resolver.rs +++ b/lib/engine/src/resolver.rs @@ -3,13 +3,14 @@ use crate::{Export, ImportError, LinkError}; use more_asserts::assert_ge; +use std::sync::Arc; use wasmer_types::entity::{BoxedSlice, EntityRef, PrimaryMap}; use wasmer_types::{ExternType, FunctionIndex, ImportIndex, MemoryIndex, TableIndex}; use wasmer_vm::{ - FunctionBodyPtr, Imports, MemoryStyle, ModuleInfo, TableStyle, VMDynamicFunctionContext, - VMFunctionBody, VMFunctionImport, VMFunctionKind, VMGlobalImport, VMMemoryImport, - VMTableImport, + FunctionBodyPtr, ImportEnv, Imports, MemoryStyle, ModuleInfo, TableStyle, + VMDynamicFunctionContext, VMFunctionBody, VMFunctionImport, VMFunctionKind, VMGlobalImport, + VMMemoryImport, VMTableImport, }; /// Import resolver connects imports with available exported values. @@ -155,36 +156,13 @@ pub fn resolve_imports( } match resolved { Export::Function(ref f) => { - let (address, env_ptr) = match f.vm_function.kind { + let address = match f.vm_function.kind { VMFunctionKind::Dynamic => { // If this is a dynamic imported function, // the address of the function is the address of the // reverse trampoline. let index = FunctionIndex::new(function_imports.len()); - let address = finished_dynamic_function_trampolines[index].0 - as *mut VMFunctionBody as _; - let env_ptr = if f.import_init_function_ptr.is_some() { - // Our function env looks like: - // Box>> - // Which we can interpret as `*const *const Env` (due to - // the precise layout of these types via `repr(C)`) - // We extract the `*const Env`: - unsafe { - // Box> - let dyn_func_ctx_ptr = f.vm_function.vmctx.host_env - as *mut VMDynamicFunctionContext<*mut std::ffi::c_void>; - // maybe report error here if it's null? - // invariants of these types are not enforced. - - // &VMDynamicFunctionContext<...> - let dyn_func_ctx = &*dyn_func_ctx_ptr; - dyn_func_ctx.ctx - } - } else { - std::ptr::null_mut() - }; - - (address, env_ptr) + finished_dynamic_function_trampolines[index].0 as *mut VMFunctionBody as _ // TODO: We should check that the f.vmctx actually matches // the shape of `VMDynamicFunctionImportContext` @@ -198,9 +176,7 @@ pub fn resolve_imports( assert!(num_params < 9, "Only native functions with less than 9 arguments are allowed in Apple Silicon (for now). Received {} in the import {}.{}", num_params, module_name, field); } - (f.vm_function.address, unsafe { - f.vm_function.vmctx.host_env - }) + f.vm_function.address } }; function_imports.push(VMFunctionImport { @@ -208,7 +184,11 @@ pub fn resolve_imports( environment: f.vm_function.vmctx, }); - host_function_env_initializers.push((f.import_init_function_ptr, env_ptr)); + host_function_env_initializers.push(Arc::new(ImportEnv { + env: unsafe { f.vm_function.vmctx.host_env }, + initializer: f.import_init_function_ptr, + destructor: f.host_env_drop_fn, + })); } Export::Table(ref t) => { table_imports.push(VMTableImport { diff --git a/lib/vm/src/export.rs b/lib/vm/src/export.rs index 45b6e9be133..ec0af502fd3 100644 --- a/lib/vm/src/export.rs +++ b/lib/vm/src/export.rs @@ -10,10 +10,10 @@ use std::sync::Arc; use wasmer_types::{FunctionType, MemoryType, TableType}; /// The value of an export passed from one instance to another. -#[derive(Debug, Clone)] +#[derive(Debug)] pub enum VMExport { /// A function export value. - Function(VMExportFunction), + Function(Arc), /// A table export value. Table(VMExportTable), @@ -26,7 +26,7 @@ pub enum VMExport { } /// A function export value. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, PartialEq)] pub struct VMExportFunction { /// The address of the native-code function. pub address: *const VMFunctionBody, @@ -63,7 +63,7 @@ unsafe impl Sync for VMExportFunction {} impl From for VMExport { fn from(func: VMExportFunction) -> Self { - Self::Function(func) + Self::Function(Arc::new(func)) } } diff --git a/lib/vm/src/imports.rs b/lib/vm/src/imports.rs index 0f9482e4092..1d3fd87030f 100644 --- a/lib/vm/src/imports.rs +++ b/lib/vm/src/imports.rs @@ -1,8 +1,9 @@ // This file contains code from external sources. // Attributions: https://github.com/wasmerio/wasmer/blob/master/ATTRIBUTIONS.md -use crate::instance::{ImportInitializerFuncPtr, ImportInitializerThunks}; +use crate::instance::ImportEnv; use crate::vmcontext::{VMFunctionImport, VMGlobalImport, VMMemoryImport, VMTableImport}; +use std::sync::Arc; use wasmer_types::entity::{BoxedSlice, PrimaryMap}; use wasmer_types::{FunctionIndex, GlobalIndex, MemoryIndex, TableIndex}; @@ -17,9 +18,7 @@ pub struct Imports { /// space may affect Wasm runtime performance due to increased cache pressure. /// /// We make it optional so that we can free the data after use. - pub host_function_env_initializers: Option< - BoxedSlice, *mut std::ffi::c_void)>, - >, + pub host_function_env_initializers: Option>>, /// Resolved addresses for imported tables. pub tables: BoxedSlice, @@ -35,10 +34,7 @@ impl Imports { /// Construct a new `Imports` instance. pub fn new( function_imports: PrimaryMap, - host_function_env_initializers: PrimaryMap< - FunctionIndex, - (Option, *mut std::ffi::c_void), - >, + host_function_env_initializers: PrimaryMap>, table_imports: PrimaryMap, memory_imports: PrimaryMap, global_imports: PrimaryMap, @@ -56,7 +52,7 @@ impl Imports { pub fn none() -> Self { Self { functions: PrimaryMap::new().into_boxed_slice(), - host_function_env_initializers: Some(PrimaryMap::new().into_boxed_slice()), + host_function_env_initializers: None, tables: PrimaryMap::new().into_boxed_slice(), memories: PrimaryMap::new().into_boxed_slice(), globals: PrimaryMap::new().into_boxed_slice(), @@ -68,25 +64,9 @@ impl Imports { /// /// This function can only be called once, it deletes the data it returns after /// returning it to ensure that it's not called more than once. - pub fn get_import_initializers(&mut self) -> ImportInitializerThunks { - let result = if let Some(inner) = &self.host_function_env_initializers { - inner - .values() - .cloned() - .map(|(func_init, env_ptr)| { - let host_env = if func_init.is_some() { - env_ptr - } else { - std::ptr::null_mut() - }; - (func_init, host_env) - }) - .collect() - } else { - vec![] - }; - // ensure we only call these functions once and free this now useless memory. - self.host_function_env_initializers = None; - result + pub fn get_import_initializers(&mut self) -> BoxedSlice> { + self.host_function_env_initializers + .take() + .unwrap_or_else(|| PrimaryMap::new().into_boxed_slice()) } } diff --git a/lib/vm/src/instance.rs b/lib/vm/src/instance.rs index d8f546d59c8..dc87a0cb711 100644 --- a/lib/vm/src/instance.rs +++ b/lib/vm/src/instance.rs @@ -100,7 +100,8 @@ pub(crate) struct Instance { /// /// TODO: Be sure to test with serialize/deserialize and imported /// functions from other Wasm modules. - import_initializers: ImportInitializerThunks, + /// TODO: update this comment + import_envs: BoxedSlice>, /// Additional context used by compiled WebAssembly code. This /// field is last, and represents a dynamically-sized array that @@ -109,6 +110,27 @@ pub(crate) struct Instance { vmctx: VMContext, } +/// TODO: figure out what to do with Clone here +/// We can probably remove the Arc... just need to figure that out. +/// TODO: +#[derive(Debug)] +pub struct ImportEnv { + /// TODO: + pub env: *mut std::ffi::c_void, + /// TODO: + pub initializer: Option, + /// TODO: + pub destructor: Option, +} + +impl Drop for ImportEnv { + fn drop(&mut self) { + if let Some(destructor) = self.destructor { + (destructor)(self.env); + } + } +} + impl fmt::Debug for Instance { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { formatter.debug_struct("Instance").finish() @@ -155,7 +177,7 @@ impl Instance { &self, index: FunctionIndex, ) -> Option { - self.import_initializers[index.as_u32() as usize].0 + self.import_envs[index].initializer } /// Return a pointer to the `VMFunctionImport`s. @@ -1007,7 +1029,7 @@ impl InstanceHandle { imports: Imports, vmshared_signatures: BoxedSlice, host_state: Box, - import_initializers: ImportInitializerThunks, + import_envs: BoxedSlice>, ) -> Result { // `NonNull` here actually means `NonNull`. See // `Self::allocate_instance` to understand why. @@ -1035,7 +1057,7 @@ impl InstanceHandle { passive_data, host_state, signal_handler: Cell::new(None), - import_initializers, + import_envs, vmctx: VMContext {}, }; @@ -1411,18 +1433,20 @@ impl InstanceHandle { ) -> Result<(), Err> { let instance_ref = self.instance.as_mut(); - for (func, env) in instance_ref.import_initializers.drain(..) { - if let Some(ref f) = func { + for ImportEnv { + env, initializer, .. + } in instance_ref.import_envs.values().map(|v| &**v) + { + if let Some(ref f) = initializer { // transmute our function pointer into one with the correct error type let f = mem::transmute::< &ImportInitializerFuncPtr, &fn(*mut ffi::c_void, *const ffi::c_void) -> Result<(), Err>, >(f); - f(env, instance_ptr)?; + f(*env, instance_ptr)?; + //*initializer = None; } } - // free memory now that it's empty. - instance_ref.import_initializers.shrink_to_fit(); Ok(()) } diff --git a/lib/vm/src/lib.rs b/lib/vm/src/lib.rs index ecbadeca794..893739988de 100644 --- a/lib/vm/src/lib.rs +++ b/lib/vm/src/lib.rs @@ -39,7 +39,7 @@ pub mod libcalls; pub use crate::export::*; pub use crate::global::*; pub use crate::imports::Imports; -pub use crate::instance::{ImportInitializerFuncPtr, InstanceHandle}; +pub use crate::instance::{ImportEnv, ImportInitializerFuncPtr, InstanceHandle}; pub use crate::memory::{LinearMemory, Memory, MemoryError, MemoryStyle}; pub use crate::mmap::Mmap; pub use crate::module::{ExportsIterator, ImportsIterator, ModuleInfo}; From e43d9d27326d756ec1eff1001d8ce9a18f9b2575 Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Fri, 4 Dec 2020 16:04:03 -0800 Subject: [PATCH 2/9] Experimental: clone host envs during construction --- Cargo.lock | 1 - Cargo.toml | 8 +-- examples/imports_function_env.rs | 2 +- lib/api/src/env.rs | 34 +++++------ lib/api/src/externals/function.rs | 95 ++++++++++++++++++++----------- lib/api/src/native.rs | 6 +- lib/api/src/types.rs | 3 +- lib/emscripten/src/lib.rs | 2 +- lib/engine/src/export.rs | 39 ++++++++++--- lib/engine/src/lib.rs | 4 +- lib/engine/src/resolver.rs | 47 +++++++++++---- lib/vm/src/imports.rs | 7 +-- lib/vm/src/instance.rs | 31 +++++++--- tests/compilers/imports.rs | 49 ++++++++++++++++ 14 files changed, 231 insertions(+), 97 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f4df59f4861..3268401e26a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2826,7 +2826,6 @@ dependencies = [ "wasmer-compiler-cranelift", "wasmer-compiler-llvm", "wasmer-compiler-singlepass", - "wasmer-emscripten", "wasmer-engine", "wasmer-engine-dummy", "wasmer-engine-jit", diff --git a/Cargo.toml b/Cargo.toml index 79ebcef8c14..1ba3c8d9269 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ wasmer-compiler = { version = "1.0.0-beta1", path = "lib/compiler" } wasmer-compiler-cranelift = { version = "1.0.0-beta1", path = "lib/compiler-cranelift", optional = true } wasmer-compiler-singlepass = { version = "1.0.0-beta1", path = "lib/compiler-singlepass", optional = true } wasmer-compiler-llvm = { version = "1.0.0-beta1", path = "lib/compiler-llvm", optional = true } -wasmer-emscripten = { version = "1.0.0-beta1", path = "lib/emscripten", optional = true } +#wasmer-emscripten = { version = "1.0.0-beta1", path = "lib/emscripten", optional = true } wasmer-engine = { version = "1.0.0-beta1", path = "lib/engine" } wasmer-engine-jit = { version = "1.0.0-beta1", path = "lib/engine-jit", optional = true } wasmer-engine-native = { version = "1.0.0-beta1", path = "lib/engine-native", optional = true } @@ -38,7 +38,7 @@ members = [ "lib/compiler-singlepass", "lib/compiler-llvm", "lib/derive", - "lib/emscripten", +# "lib/emscripten", "lib/engine", "lib/engine-jit", "lib/engine-native", @@ -81,7 +81,7 @@ default = [ "object-file", "cache", "wasi", - "emscripten", +# "emscripten", "middlewares", ] engine = [] @@ -100,7 +100,7 @@ object-file = [ cache = ["wasmer-cache"] wast = ["wasmer-wast"] wasi = ["wasmer-wasi"] -emscripten = ["wasmer-emscripten"] +#emscripten = ["wasmer-emscripten"] wat = ["wasmer/wat"] compiler = [ "wasmer/compiler", diff --git a/examples/imports_function_env.rs b/examples/imports_function_env.rs index 66d61bf7040..07bf7bbf26d 100644 --- a/examples/imports_function_env.rs +++ b/examples/imports_function_env.rs @@ -70,7 +70,7 @@ fn main() -> Result<(), Box> { // possible to know the size of the `Env` at compile time (i.e it has to // implement the `Sized` trait) and that it implement the `WasmerEnv` trait. // We derive a default implementation of `WasmerEnv` here. - #[derive(WasmerEnv)] + #[derive(WasmerEnv, Clone)] struct Env { counter: Arc>, } diff --git a/lib/api/src/env.rs b/lib/api/src/env.rs index 53c37d6d2ea..03d1376990e 100644 --- a/lib/api/src/env.rs +++ b/lib/api/src/env.rs @@ -30,12 +30,12 @@ impl From for HostEnvInitError { /// ``` /// use wasmer::{WasmerEnv, LazyInit, Memory, NativeFunc}; /// -/// #[derive(WasmerEnv)] +/// #[derive(WasmerEnv, Clone)] /// pub struct MyEnvWithNoInstanceData { /// non_instance_data: u8, /// } /// -/// #[derive(WasmerEnv)] +/// #[derive(WasmerEnv, Clone)] /// pub struct MyEnvWithInstanceData { /// non_instance_data: u8, /// #[wasmer(export)] @@ -54,6 +54,7 @@ impl From for HostEnvInitError { /// This trait can also be implemented manually: /// ``` /// # use wasmer::{WasmerEnv, LazyInit, Memory, Instance, HostEnvInitError}; +/// #[derive(Clone)] /// pub struct MyEnv { /// memory: LazyInit, /// } @@ -66,7 +67,7 @@ impl From for HostEnvInitError { /// } /// } /// ``` -pub trait WasmerEnv { +pub trait WasmerEnv: Clone { /// The function that Wasmer will call on your type to let it finish /// setting up the environment with data from the `Instance`. /// @@ -94,28 +95,29 @@ impl WasmerEnv for isize {} impl WasmerEnv for char {} impl WasmerEnv for bool {} impl WasmerEnv for String {} -impl WasmerEnv for ::std::sync::atomic::AtomicBool {} -impl WasmerEnv for ::std::sync::atomic::AtomicI8 {} -impl WasmerEnv for ::std::sync::atomic::AtomicU8 {} -impl WasmerEnv for ::std::sync::atomic::AtomicI16 {} -impl WasmerEnv for ::std::sync::atomic::AtomicU16 {} -impl WasmerEnv for ::std::sync::atomic::AtomicI32 {} -impl WasmerEnv for ::std::sync::atomic::AtomicU32 {} -impl WasmerEnv for ::std::sync::atomic::AtomicI64 {} -impl WasmerEnv for ::std::sync::atomic::AtomicUsize {} -impl WasmerEnv for ::std::sync::atomic::AtomicIsize {} -impl WasmerEnv for dyn ::std::any::Any {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicBool {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicI8 {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicU8 {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicI16 {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicU16 {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicI32 {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicU32 {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicI64 {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicUsize {} +impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicIsize {} +//impl WasmerEnv for dyn ::std::any::Any + Clone {} impl WasmerEnv for Box { fn init_with_instance(&mut self, instance: &Instance) -> Result<(), HostEnvInitError> { (&mut **self).init_with_instance(instance) } } -impl WasmerEnv for &'static mut T { +/*impl WasmerEnv for &'static T { fn init_with_instance(&mut self, instance: &Instance) -> Result<(), HostEnvInitError> { + T::init_with_instance() (*self).init_with_instance(instance) } -} +}*/ /// Lazily init an item pub struct LazyInit { diff --git a/lib/api/src/externals/function.rs b/lib/api/src/externals/function.rs index 64780a822aa..b99f4313755 100644 --- a/lib/api/src/externals/function.rs +++ b/lib/api/src/externals/function.rs @@ -13,7 +13,7 @@ pub use inner::{UnsafeMutableEnv, WithUnsafeMutableEnv}; use std::cmp::max; use std::fmt; use std::sync::Arc; -use wasmer_engine::{Export, ExportFunction}; +use wasmer_engine::{Export, ExportFunction, ExportFunctionMetadata}; use wasmer_vm::{ raise_user_trap, resume_panic, wasmer_call_trampoline, VMCallerCheckedAnyfunc, VMDynamicFunctionContext, VMExportFunction, VMFunctionBody, VMFunctionEnvironment, @@ -97,22 +97,32 @@ impl Function { // The engine linker will replace the address with one pointing to a // generated dynamic trampoline. let address = std::ptr::null() as *const VMFunctionBody; - let vmctx = VMFunctionEnvironment { - host_env: Box::into_raw(Box::new(dynamic_ctx)) as *mut _, + let host_env = Box::into_raw(Box::new(dynamic_ctx)) as *mut _; + let vmctx = VMFunctionEnvironment { host_env }; + let host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void = |ptr| { + let duped_env = unsafe { + let ptr: *mut VMDynamicFunctionContext = ptr as _; + let item: &VMDynamicFunctionContext = &*ptr; + item.clone() + }; + Box::into_raw(Box::new(duped_env)) as _ + }; + let host_env_drop_fn: fn(*mut std::ffi::c_void) = |ptr: *mut std::ffi::c_void| { + unsafe { + Box::from_raw(ptr as *mut VMDynamicFunctionContext) + }; }; - let host_env_drop_fn: Option = - Some(|ptr: *mut std::ffi::c_void| { - unsafe { - Box::from_raw(ptr as *mut VMDynamicFunctionContext) - }; - }); Self { store: store.clone(), definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: false }), exported: ExportFunction { - import_init_function_ptr: None, - host_env_drop_fn, + metadata: Some(Arc::new(ExportFunctionMetadata { + host_env, + import_init_function_ptr: None, + host_env_clone_fn, + host_env_drop_fn, + })), vm_function: Arc::new(VMExportFunction { address, kind: VMFunctionKind::Dynamic, @@ -162,9 +172,8 @@ impl Function { // The engine linker will replace the address with one pointing to a // generated dynamic trampoline. let address = std::ptr::null() as *const VMFunctionBody; - let vmctx = VMFunctionEnvironment { - host_env: Box::into_raw(Box::new(dynamic_ctx)) as *mut _, - }; + let host_env = Box::into_raw(Box::new(dynamic_ctx)) as *mut _; + let vmctx = VMFunctionEnvironment { host_env }; let import_init_function_ptr: fn(_, _) -> Result<(), _> = |ptr: *mut std::ffi::c_void, instance: *const std::ffi::c_void| { let ptr = ptr as *mut VMDynamicFunctionContext>; @@ -180,21 +189,30 @@ impl Function { import_init_function_ptr, ) }); - let host_env_drop_fn: Option = - Some(|ptr: *mut std::ffi::c_void| { - unsafe { - Box::from_raw( - ptr as *mut VMDynamicFunctionContext>, - ) - }; - }); + let host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void = |ptr| { + let duped_env = unsafe { + let ptr: *mut VMDynamicFunctionContext> = ptr as _; + let item: &VMDynamicFunctionContext> = &*ptr; + item.clone() + }; + Box::into_raw(Box::new(duped_env)) as _ + }; + let host_env_drop_fn: fn(*mut std::ffi::c_void) = |ptr: *mut std::ffi::c_void| { + unsafe { + Box::from_raw(ptr as *mut VMDynamicFunctionContext>) + }; + }; Self { store: store.clone(), definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: true }), exported: ExportFunction { - import_init_function_ptr, - host_env_drop_fn, + metadata: Some(Arc::new(ExportFunctionMetadata { + host_env, + import_init_function_ptr, + host_env_clone_fn, + host_env_drop_fn, + })), vm_function: Arc::new(VMExportFunction { address, kind: VMFunctionKind::Dynamic, @@ -248,8 +266,7 @@ impl Function { exported: ExportFunction { // TODO: figure out what's going on in this function: it takes an `Env` // param but also marks itself as not having an env - import_init_function_ptr: None, - host_env_drop_fn: None, + metadata: None, vm_function: Arc::new(VMExportFunction { address, vmctx, @@ -304,13 +321,19 @@ impl Function { // In the case of Host-defined functions `VMContext` is whatever environment // the user want to attach to the function. let box_env = Box::new(env); - let vmctx = VMFunctionEnvironment { - host_env: Box::into_raw(box_env) as *mut _, + let host_env = Box::into_raw(box_env) as *mut _; + let vmctx = VMFunctionEnvironment { host_env }; + let host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void = |ptr| { + let duped_env = unsafe { + let ptr: *mut Env = ptr as _; + let item: &Env = &*ptr; + item.clone() + }; + Box::into_raw(Box::new(duped_env)) as _ + }; + let host_env_drop_fn: fn(*mut std::ffi::c_void) = |ptr: *mut std::ffi::c_void| { + unsafe { Box::from_raw(ptr as *mut Env) }; }; - let host_env_drop_fn: Option = - Some(|ptr: *mut std::ffi::c_void| { - unsafe { Box::from_raw(ptr as *mut Env) }; - }); // TODO: look into removing transmute by changing API type signatures let import_init_function_ptr = Some(unsafe { @@ -324,8 +347,12 @@ impl Function { store: store.clone(), definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: true }), exported: ExportFunction { - import_init_function_ptr, - host_env_drop_fn, + metadata: Some(Arc::new(ExportFunctionMetadata { + host_env, + import_init_function_ptr, + host_env_clone_fn, + host_env_drop_fn, + })), vm_function: Arc::new(VMExportFunction { address, kind: VMFunctionKind::Static, diff --git a/lib/api/src/native.rs b/lib/api/src/native.rs index 8c4ea531379..306270d07e1 100644 --- a/lib/api/src/native.rs +++ b/lib/api/src/native.rs @@ -88,8 +88,7 @@ where let signature = FunctionType::new(Args::wasm_types(), Rets::wasm_types()); Self { // TODO: - import_init_function_ptr: None, - host_env_drop_fn: None, + metadata: None, vm_function: Arc::new(VMExportFunction { address: other.address, vmctx: other.vmctx, @@ -114,8 +113,7 @@ where definition: other.definition, exported: ExportFunction { // TODO: - import_init_function_ptr: None, - host_env_drop_fn: None, + metadata: None, vm_function: Arc::new(VMExportFunction { address: other.address, vmctx: other.vmctx, diff --git a/lib/api/src/types.rs b/lib/api/src/types.rs index a8e72f7b4b6..92ef8a405fa 100644 --- a/lib/api/src/types.rs +++ b/lib/api/src/types.rs @@ -76,8 +76,7 @@ impl ValFuncRef for Val { let export = wasmer_engine::ExportFunction { // TODO: // figure out if we ever need a value here: need testing with complicated import patterns - import_init_function_ptr: None, - host_env_drop_fn: None, + metadata: None, vm_function: std::sync::Arc::new(wasmer_vm::VMExportFunction { address: item.func_ptr, signature, diff --git a/lib/emscripten/src/lib.rs b/lib/emscripten/src/lib.rs index 84ad5b08447..da32dc9cede 100644 --- a/lib/emscripten/src/lib.rs +++ b/lib/emscripten/src/lib.rs @@ -122,7 +122,7 @@ lazy_static! { const GLOBAL_BASE: u32 = 1024; const STATIC_BASE: u32 = GLOBAL_BASE; -#[derive(WasmerEnv, Default)] +#[derive(WasmerEnv, Clone, Default)] pub struct EmscriptenData { pub globals: EmscriptenGlobalsData, diff --git a/lib/engine/src/export.rs b/lib/engine/src/export.rs index d7533345fe2..37ca3607628 100644 --- a/lib/engine/src/export.rs +++ b/lib/engine/src/export.rs @@ -37,8 +37,7 @@ impl From for Export { match other { VMExport::Function(vm_function) => Export::Function(ExportFunction { vm_function, - import_init_function_ptr: None, - host_env_drop_fn: None, + metadata: None, }), VMExport::Memory(vm_memory) => Export::Memory(ExportMemory { vm_memory }), VMExport::Table(vm_table) => Export::Table(ExportTable { vm_table }), @@ -47,19 +46,41 @@ impl From for Export { } } -/// A function export value with an extra function pointer to initialize -/// host environments. -#[derive(Debug, Clone, PartialEq)] -pub struct ExportFunction { - /// The VM function, containing most of the data. - pub vm_function: Arc, +/// TODO: rename, etc. +#[derive(Debug, PartialEq)] +pub struct ExportFunctionMetadata { + /// duplicated here so we can free it.... + /// TODO: refactor all this stuff so it's less of a nightmare. + pub host_env: *mut std::ffi::c_void, /// Function pointer to `WasmerEnv::init_with_instance(&mut self, instance: &Instance)`. /// /// This function is called to finish setting up the environment after /// we create the `api::Instance`. + // This one is optional for now because dynamic host envs need the rest + // of this without the init fn pub import_init_function_ptr: Option, + /// TODO: + pub host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, /// The destructor to free the host environment. - pub host_env_drop_fn: Option, + pub host_env_drop_fn: fn(*mut std::ffi::c_void), +} + +impl Drop for ExportFunctionMetadata { + fn drop(&mut self) { + if !self.host_env.is_null() { + (self.host_env_drop_fn)(self.host_env); + } + } +} + +/// A function export value with an extra function pointer to initialize +/// host environments. +#[derive(Debug, Clone, PartialEq)] +pub struct ExportFunction { + /// The VM function, containing most of the data. + pub vm_function: Arc, + /// TODO: + pub metadata: Option>, } impl From for Export { diff --git a/lib/engine/src/lib.rs b/lib/engine/src/lib.rs index 557b1b039d2..e5754535823 100644 --- a/lib/engine/src/lib.rs +++ b/lib/engine/src/lib.rs @@ -34,7 +34,9 @@ pub use crate::engine::{Engine, EngineId}; pub use crate::error::{ DeserializeError, ImportError, InstantiationError, LinkError, SerializeError, }; -pub use crate::export::{Export, ExportFunction, ExportGlobal, ExportMemory, ExportTable}; +pub use crate::export::{ + Export, ExportFunction, ExportFunctionMetadata, ExportGlobal, ExportMemory, ExportTable, +}; pub use crate::resolver::{ resolve_imports, ChainableNamedResolver, NamedResolver, NamedResolverChain, NullResolver, Resolver, diff --git a/lib/engine/src/resolver.rs b/lib/engine/src/resolver.rs index 8b7f75ab6b6..471b3c9b912 100644 --- a/lib/engine/src/resolver.rs +++ b/lib/engine/src/resolver.rs @@ -1,16 +1,15 @@ //! Define the `Resolver` trait, allowing custom resolution for external //! references. -use crate::{Export, ImportError, LinkError}; +use crate::{Export, ExportFunctionMetadata, ImportError, LinkError}; use more_asserts::assert_ge; -use std::sync::Arc; use wasmer_types::entity::{BoxedSlice, EntityRef, PrimaryMap}; use wasmer_types::{ExternType, FunctionIndex, ImportIndex, MemoryIndex, TableIndex}; use wasmer_vm::{ - FunctionBodyPtr, ImportEnv, Imports, MemoryStyle, ModuleInfo, TableStyle, - VMDynamicFunctionContext, VMFunctionBody, VMFunctionImport, VMFunctionKind, VMGlobalImport, - VMMemoryImport, VMTableImport, + FunctionBodyPtr, ImportEnv, Imports, MemoryStyle, ModuleInfo, TableStyle, VMFunctionBody, + VMFunctionEnvironment, VMFunctionImport, VMFunctionKind, VMGlobalImport, VMMemoryImport, + VMTableImport, }; /// Import resolver connects imports with available exported values. @@ -179,16 +178,42 @@ pub fn resolve_imports( f.vm_function.address } }; + + // TODO: add lots of documentation for this really strange + // looking code. + // Also, keep eyes open for refactor opportunities to clean + // this all up. + let env = if let Some(ExportFunctionMetadata { + host_env_clone_fn: clone, + .. + }) = f.metadata.as_ref().map(|x| &**x) + { + // TODO: maybe start adding asserts in all these + // unsafe blocks to prevent future changes from + // horribly breaking things. + unsafe { + assert!(!f.vm_function.vmctx.host_env.is_null()); + (clone)(f.vm_function.vmctx.host_env) + } + } else { + unsafe { + assert!(f.vm_function.vmctx.host_env.is_null()); + f.vm_function.vmctx.host_env + } + }; + function_imports.push(VMFunctionImport { body: address, - environment: f.vm_function.vmctx, + environment: VMFunctionEnvironment { host_env: env }, }); - host_function_env_initializers.push(Arc::new(ImportEnv { - env: unsafe { f.vm_function.vmctx.host_env }, - initializer: f.import_init_function_ptr, - destructor: f.host_env_drop_fn, - })); + host_function_env_initializers.push(ImportEnv { + env, + // TODO: consider just passing metadata directly + initializer: f.metadata.as_ref().and_then(|m| m.import_init_function_ptr), + clone: f.metadata.as_ref().map(|m| m.host_env_clone_fn), + destructor: f.metadata.as_ref().map(|m| m.host_env_drop_fn), + }); } Export::Table(ref t) => { table_imports.push(VMTableImport { diff --git a/lib/vm/src/imports.rs b/lib/vm/src/imports.rs index 1d3fd87030f..d50b79826f1 100644 --- a/lib/vm/src/imports.rs +++ b/lib/vm/src/imports.rs @@ -3,7 +3,6 @@ use crate::instance::ImportEnv; use crate::vmcontext::{VMFunctionImport, VMGlobalImport, VMMemoryImport, VMTableImport}; -use std::sync::Arc; use wasmer_types::entity::{BoxedSlice, PrimaryMap}; use wasmer_types::{FunctionIndex, GlobalIndex, MemoryIndex, TableIndex}; @@ -18,7 +17,7 @@ pub struct Imports { /// space may affect Wasm runtime performance due to increased cache pressure. /// /// We make it optional so that we can free the data after use. - pub host_function_env_initializers: Option>>, + pub host_function_env_initializers: Option>, /// Resolved addresses for imported tables. pub tables: BoxedSlice, @@ -34,7 +33,7 @@ impl Imports { /// Construct a new `Imports` instance. pub fn new( function_imports: PrimaryMap, - host_function_env_initializers: PrimaryMap>, + host_function_env_initializers: PrimaryMap, table_imports: PrimaryMap, memory_imports: PrimaryMap, global_imports: PrimaryMap, @@ -64,7 +63,7 @@ impl Imports { /// /// This function can only be called once, it deletes the data it returns after /// returning it to ensure that it's not called more than once. - pub fn get_import_initializers(&mut self) -> BoxedSlice> { + pub fn get_import_initializers(&mut self) -> BoxedSlice { self.host_function_env_initializers .take() .unwrap_or_else(|| PrimaryMap::new().into_boxed_slice()) diff --git a/lib/vm/src/instance.rs b/lib/vm/src/instance.rs index dc87a0cb711..14413f1440b 100644 --- a/lib/vm/src/instance.rs +++ b/lib/vm/src/instance.rs @@ -45,11 +45,6 @@ use wasmer_types::{ pub type ImportInitializerFuncPtr = fn(*mut std::ffi::c_void, *const std::ffi::c_void) -> Result<(), *mut std::ffi::c_void>; -/// This type holds thunks (delayed computations) for initializing the imported -/// function's environments with the [`Instance`]. -pub(crate) type ImportInitializerThunks = - Vec<(Option, *mut std::ffi::c_void)>; - /// A WebAssembly instance. /// /// The type is dynamically-sized. Indeed, the `vmctx` field can @@ -101,7 +96,7 @@ pub(crate) struct Instance { /// TODO: Be sure to test with serialize/deserialize and imported /// functions from other Wasm modules. /// TODO: update this comment - import_envs: BoxedSlice>, + import_envs: BoxedSlice, /// Additional context used by compiled WebAssembly code. This /// field is last, and represents a dynamically-sized array that @@ -118,11 +113,29 @@ pub struct ImportEnv { /// TODO: pub env: *mut std::ffi::c_void, /// TODO: + pub clone: Option *mut std::ffi::c_void>, + /// TODO: pub initializer: Option, /// TODO: pub destructor: Option, } +impl Clone for ImportEnv { + fn clone(&self) -> Self { + let env = if let Some(clone) = self.clone { + (clone)(self.env) + } else { + self.env + }; + Self { + env, + clone: self.clone.clone(), + initializer: self.initializer.clone(), + destructor: self.destructor.clone(), + } + } +} + impl Drop for ImportEnv { fn drop(&mut self) { if let Some(destructor) = self.destructor { @@ -1029,7 +1042,7 @@ impl InstanceHandle { imports: Imports, vmshared_signatures: BoxedSlice, host_state: Box, - import_envs: BoxedSlice>, + import_envs: BoxedSlice, ) -> Result { // `NonNull` here actually means `NonNull`. See // `Self::allocate_instance` to understand why. @@ -1435,7 +1448,7 @@ impl InstanceHandle { for ImportEnv { env, initializer, .. - } in instance_ref.import_envs.values().map(|v| &**v) + } in instance_ref.import_envs.values_mut() { if let Some(ref f) = initializer { // transmute our function pointer into one with the correct error type @@ -1444,7 +1457,7 @@ impl InstanceHandle { &fn(*mut ffi::c_void, *const ffi::c_void) -> Result<(), Err>, >(f); f(*env, instance_ptr)?; - //*initializer = None; + *initializer = None; } } diff --git a/tests/compilers/imports.rs b/tests/compilers/imports.rs index 3d35944f4db..cfed0c69239 100644 --- a/tests/compilers/imports.rs +++ b/tests/compilers/imports.rs @@ -343,3 +343,52 @@ fn dynamic_function_with_env_wasmer_env_init_works() -> Result<()> { f.call()?; Ok(()) } + +#[test] +fn multi_use_host_fn_manages_memory_correctly() -> Result<()> { + let store = get_store(false); + let module = get_module2(&store)?; + + #[allow(dead_code)] + #[derive(Clone)] + struct Env { + memory: LazyInit, + }; + + impl WasmerEnv for Env { + fn init_with_instance(&mut self, instance: &Instance) -> Result<(), HostEnvInitError> { + dbg!("Initing the env!"); + let memory = instance.exports.get_memory("memory")?.clone(); + self.memory.initialize(memory); + Ok(()) + } + } + + let env: Env = Env { + memory: LazyInit::default(), + }; + fn host_fn(env: &Env) { + dbg!(env as *const _); + assert!(env.memory.get_ref().is_some()); + println!("Hello, world!"); + } + + let imports = imports! { + "host" => { + "fn" => Function::new_native_with_env(&store, env.clone(), host_fn), + }, + }; + let instance1 = Instance::new(&module, &imports)?; + let instance2 = Instance::new(&module, &imports)?; + { + let f1: NativeFunc<(), ()> = instance1.exports.get_native_function("main")?; + f1.call()?; + } + drop(instance1); + { + let f2: NativeFunc<(), ()> = instance2.exports.get_native_function("main")?; + f2.call()?; + } + drop(instance2); + Ok(()) +} From 4ec8c9e71adee5534761eb3ddb8cfc4c5162ac83 Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Fri, 4 Dec 2020 17:07:01 -0800 Subject: [PATCH 3/9] Fix issue with dynamic host env --- lib/api/src/externals/function.rs | 55 ++++++++++++++++++++----------- lib/api/src/native.rs | 9 +++-- lib/api/src/types.rs | 4 +-- lib/engine/src/export.rs | 5 ++- lib/engine/src/resolver.rs | 2 +- lib/vm/src/export.rs | 6 ++-- lib/vm/src/vmcontext.rs | 9 +++++ 7 files changed, 58 insertions(+), 32 deletions(-) diff --git a/lib/api/src/externals/function.rs b/lib/api/src/externals/function.rs index b99f4313755..a21ca3a4612 100644 --- a/lib/api/src/externals/function.rs +++ b/lib/api/src/externals/function.rs @@ -90,7 +90,7 @@ impl Function { { let dynamic_ctx: VMDynamicFunctionContext = VMDynamicFunctionContext::from_context(VMDynamicFunctionWithoutEnv { - func: Box::new(func), + func: Arc::new(func), function_type: ty.clone(), }); // We don't yet have the address with the Wasm ABI signature. @@ -100,7 +100,7 @@ impl Function { let host_env = Box::into_raw(Box::new(dynamic_ctx)) as *mut _; let vmctx = VMFunctionEnvironment { host_env }; let host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void = |ptr| { - let duped_env = unsafe { + let duped_env: VMDynamicFunctionContext = unsafe { let ptr: *mut VMDynamicFunctionContext = ptr as _; let item: &VMDynamicFunctionContext = &*ptr; item.clone() @@ -123,14 +123,14 @@ impl Function { host_env_clone_fn, host_env_drop_fn, })), - vm_function: Arc::new(VMExportFunction { + vm_function: VMExportFunction { address, kind: VMFunctionKind::Dynamic, vmctx, signature: ty.clone(), call_trampoline: None, instance_allocator: None, - }), + }, }, } } @@ -164,8 +164,11 @@ impl Function { { let dynamic_ctx: VMDynamicFunctionContext> = VMDynamicFunctionContext::from_context(VMDynamicFunctionWithEnv { - env: Box::new(env), - func: Box::new(func), + env: { + let e = Box::new(env); + e + }, + func: Arc::new(func), function_type: ty.clone(), }); // We don't yet have the address with the Wasm ABI signature. @@ -190,7 +193,7 @@ impl Function { ) }); let host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void = |ptr| { - let duped_env = unsafe { + let duped_env: VMDynamicFunctionContext> = unsafe { let ptr: *mut VMDynamicFunctionContext> = ptr as _; let item: &VMDynamicFunctionContext> = &*ptr; item.clone() @@ -213,14 +216,14 @@ impl Function { host_env_clone_fn, host_env_drop_fn, })), - vm_function: Arc::new(VMExportFunction { + vm_function: VMExportFunction { address, kind: VMFunctionKind::Dynamic, vmctx, signature: ty.clone(), call_trampoline: None, instance_allocator: None, - }), + }, }, } } @@ -267,14 +270,14 @@ impl Function { // TODO: figure out what's going on in this function: it takes an `Env` // param but also marks itself as not having an env metadata: None, - vm_function: Arc::new(VMExportFunction { + vm_function: VMExportFunction { address, vmctx, signature, kind: VMFunctionKind::Static, call_trampoline: None, instance_allocator: None, - }), + }, }, } } @@ -320,8 +323,7 @@ impl Function { // Wasm-defined functions have a `VMContext`. // In the case of Host-defined functions `VMContext` is whatever environment // the user want to attach to the function. - let box_env = Box::new(env); - let host_env = Box::into_raw(box_env) as *mut _; + let host_env = Box::into_raw(Box::new(env)) as *mut _; let vmctx = VMFunctionEnvironment { host_env }; let host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void = |ptr| { let duped_env = unsafe { @@ -353,14 +355,14 @@ impl Function { host_env_clone_fn, host_env_drop_fn, })), - vm_function: Arc::new(VMExportFunction { + vm_function: VMExportFunction { address, kind: VMFunctionKind::Static, vmctx, signature, call_trampoline: None, instance_allocator: None, - }), + }, }, } } @@ -405,15 +407,17 @@ impl Function { store: store.clone(), definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: true }), exported: ExportFunction { - import_init_function_ptr, - vm_function: Arc::new(VMExportFunction { + metadata: Some(Arc::new(ExportFunctionMetadata { + import_init_function_ptr, + })), + vm_function: VMExportFunction { address, kind: VMFunctionKind::Static, vmctx, signature, call_trampoline: None, instance_allocator: None, - }), + }, }, } } @@ -775,9 +779,10 @@ pub(crate) trait VMDynamicFunction { fn function_type(&self) -> &FunctionType; } +#[derive(Clone)] pub(crate) struct VMDynamicFunctionWithoutEnv { #[allow(clippy::type_complexity)] - func: Box Result, RuntimeError> + 'static>, + func: Arc Result, RuntimeError> + 'static>, function_type: FunctionType, } @@ -799,7 +804,17 @@ where env: Box, function_type: FunctionType, #[allow(clippy::type_complexity)] - func: Box Result, RuntimeError> + 'static>, + func: Arc Result, RuntimeError> + 'static>, +} + +impl Clone for VMDynamicFunctionWithEnv { + fn clone(&self) -> Self { + Self { + env: self.env.clone(), + function_type: self.function_type.clone(), + func: self.func.clone(), + } + } } impl VMDynamicFunction for VMDynamicFunctionWithEnv diff --git a/lib/api/src/native.rs b/lib/api/src/native.rs index 306270d07e1..c7619339458 100644 --- a/lib/api/src/native.rs +++ b/lib/api/src/native.rs @@ -15,7 +15,6 @@ use crate::externals::function::{ }; use crate::{FromToNativeWasmType, Function, FunctionType, RuntimeError, Store, WasmTypeList}; use std::panic::{catch_unwind, AssertUnwindSafe}; -use std::sync::Arc; use wasmer_engine::ExportFunction; use wasmer_types::NativeWasmType; use wasmer_vm::{ @@ -89,14 +88,14 @@ where Self { // TODO: metadata: None, - vm_function: Arc::new(VMExportFunction { + vm_function: VMExportFunction { address: other.address, vmctx: other.vmctx, signature, kind: other.arg_kind, call_trampoline: None, instance_allocator: None, - }), + }, } } } @@ -114,14 +113,14 @@ where exported: ExportFunction { // TODO: metadata: None, - vm_function: Arc::new(VMExportFunction { + vm_function: VMExportFunction { address: other.address, vmctx: other.vmctx, signature, kind: other.arg_kind, call_trampoline: None, instance_allocator: None, - }), + }, }, } } diff --git a/lib/api/src/types.rs b/lib/api/src/types.rs index 92ef8a405fa..edab902bad1 100644 --- a/lib/api/src/types.rs +++ b/lib/api/src/types.rs @@ -77,7 +77,7 @@ impl ValFuncRef for Val { // TODO: // figure out if we ever need a value here: need testing with complicated import patterns metadata: None, - vm_function: std::sync::Arc::new(wasmer_vm::VMExportFunction { + vm_function: wasmer_vm::VMExportFunction { address: item.func_ptr, signature, // All functions in tables are already Static (as dynamic functions @@ -86,7 +86,7 @@ impl ValFuncRef for Val { vmctx: item.vmctx, call_trampoline: None, instance_allocator: None, - }), + }, }; let f = Function::from_vm_export(store, export); Self::FuncRef(f) diff --git a/lib/engine/src/export.rs b/lib/engine/src/export.rs index 37ca3607628..903405f4286 100644 --- a/lib/engine/src/export.rs +++ b/lib/engine/src/export.rs @@ -65,8 +65,11 @@ pub struct ExportFunctionMetadata { pub host_env_drop_fn: fn(*mut std::ffi::c_void), } +// We have to free `host_env` here because we always clone it before using it +// so all the `host_env`s freed at the `Instance` level won't touch the original. impl Drop for ExportFunctionMetadata { fn drop(&mut self) { + dbg!("DROPPING ORIGINAL HOST ENV!"); if !self.host_env.is_null() { (self.host_env_drop_fn)(self.host_env); } @@ -78,7 +81,7 @@ impl Drop for ExportFunctionMetadata { #[derive(Debug, Clone, PartialEq)] pub struct ExportFunction { /// The VM function, containing most of the data. - pub vm_function: Arc, + pub vm_function: VMExportFunction, /// TODO: pub metadata: Option>, } diff --git a/lib/engine/src/resolver.rs b/lib/engine/src/resolver.rs index 471b3c9b912..f8e63cafc97 100644 --- a/lib/engine/src/resolver.rs +++ b/lib/engine/src/resolver.rs @@ -197,7 +197,7 @@ pub fn resolve_imports( } } else { unsafe { - assert!(f.vm_function.vmctx.host_env.is_null()); + //assert!(f.vm_function.vmctx.host_env.is_null()); f.vm_function.vmctx.host_env } }; diff --git a/lib/vm/src/export.rs b/lib/vm/src/export.rs index ec0af502fd3..3a167fe22e8 100644 --- a/lib/vm/src/export.rs +++ b/lib/vm/src/export.rs @@ -13,7 +13,7 @@ use wasmer_types::{FunctionType, MemoryType, TableType}; #[derive(Debug)] pub enum VMExport { /// A function export value. - Function(Arc), + Function(VMExportFunction), /// A table export value. Table(VMExportTable), @@ -26,7 +26,7 @@ pub enum VMExport { } /// A function export value. -#[derive(Debug, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub struct VMExportFunction { /// The address of the native-code function. pub address: *const VMFunctionBody, @@ -63,7 +63,7 @@ unsafe impl Sync for VMExportFunction {} impl From for VMExport { fn from(func: VMExportFunction) -> Self { - Self::Function(Arc::new(func)) + Self::Function(func) } } diff --git a/lib/vm/src/vmcontext.rs b/lib/vm/src/vmcontext.rs index 43bdcd59d1f..4f478485423 100644 --- a/lib/vm/src/vmcontext.rs +++ b/lib/vm/src/vmcontext.rs @@ -106,6 +106,15 @@ pub struct VMDynamicFunctionContext { pub ctx: T, } +impl Clone for VMDynamicFunctionContext { + fn clone(&self) -> Self { + Self { + address: self.address, + ctx: self.ctx.clone(), + } + } +} + #[cfg(test)] mod test_vmdynamicfunction_import_context { use super::VMDynamicFunctionContext; From ca4736cca996b27cc18089b3af5ead62d81aeb4e Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Fri, 4 Dec 2020 17:40:37 -0800 Subject: [PATCH 4/9] Fix tests in C API and deprecated API --- lib/api/src/env.rs | 7 ++ lib/api/src/externals/function.rs | 18 +++- lib/c-api/src/deprecated/import/mod.rs | 21 +++-- lib/c-api/src/deprecated/instance.rs | 7 +- lib/c-api/src/deprecated/module.rs | 3 +- .../src/wasm_c_api/externals/function.rs | 2 +- lib/c-api/wasmer_wasm.h | 3 + lib/deprecated/runtime-core/Cargo.lock | 90 ++++++++++++++----- lib/deprecated/runtime-core/src/typed_func.rs | 6 +- lib/deprecated/runtime/Cargo.lock | 90 ++++++++++++++----- 10 files changed, 178 insertions(+), 69 deletions(-) diff --git a/lib/api/src/env.rs b/lib/api/src/env.rs index 03d1376990e..51e9a1bbf97 100644 --- a/lib/api/src/env.rs +++ b/lib/api/src/env.rs @@ -112,6 +112,13 @@ impl WasmerEnv for Box { } } +impl WasmerEnv for ::std::sync::Arc<::std::sync::Mutex> { + fn init_with_instance(&mut self, instance: &Instance) -> Result<(), HostEnvInitError> { + let mut guard = self.lock().unwrap(); + guard.init_with_instance(instance) + } +} + /*impl WasmerEnv for &'static T { fn init_with_instance(&mut self, instance: &Instance) -> Result<(), HostEnvInitError> { T::init_with_instance() diff --git a/lib/api/src/externals/function.rs b/lib/api/src/externals/function.rs index a21ca3a4612..0442c8ebd6b 100644 --- a/lib/api/src/externals/function.rs +++ b/lib/api/src/externals/function.rs @@ -393,9 +393,20 @@ impl Function { let address = function.address(); let box_env = Box::new(env); - let vmctx = VMFunctionEnvironment { - host_env: Box::into_raw(box_env) as *mut _, + let host_env = Box::into_raw(box_env) as *mut _; + let vmctx = VMFunctionEnvironment { host_env }; + let host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void = |ptr| { + let duped_env: Env = { + let ptr: *mut Env = ptr as _; + let item: &Env = &*ptr; + item.clone() + }; + Box::into_raw(Box::new(duped_env)) as _ + }; + let host_env_drop_fn: fn(*mut std::ffi::c_void) = |ptr: *mut std::ffi::c_void| { + Box::from_raw(ptr as *mut Env); }; + let signature = function.ty(); // TODO: look into removing transmute by changing API type signatures let import_init_function_ptr = Some(std::mem::transmute::< @@ -408,6 +419,9 @@ impl Function { definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: true }), exported: ExportFunction { metadata: Some(Arc::new(ExportFunctionMetadata { + host_env, + host_env_clone_fn, + host_env_drop_fn, import_init_function_ptr, })), vm_function: VMExportFunction { diff --git a/lib/c-api/src/deprecated/import/mod.rs b/lib/c-api/src/deprecated/import/mod.rs index c6e1cc58b7b..658ddf50222 100644 --- a/lib/c-api/src/deprecated/import/mod.rs +++ b/lib/c-api/src/deprecated/import/mod.rs @@ -15,7 +15,7 @@ use std::ffi::{c_void, CStr}; use std::os::raw::c_char; use std::ptr::{self, NonNull}; use std::slice; -// use std::sync::Arc; +use std::sync::{Arc, Mutex}; // use std::convert::TryFrom, use std::collections::HashMap; use wasmer::{ @@ -48,7 +48,7 @@ pub(crate) struct CAPIImportObject { /// List of pointers to `LegacyEnv`s used to patch imported functions to be able to /// pass a the "vmctx" as the first argument. /// Needed here because of extending import objects. - pub(crate) instance_pointers_to_update: Vec>, + pub(crate) instance_pointers_to_update: Vec>>, } #[repr(C)] @@ -399,7 +399,6 @@ pub unsafe extern "C" fn wasmer_import_object_imports_destroy( let function_wrapper: Box = Box::from_raw(import.value.func as *mut _); let _: Box = Box::from_raw(function_wrapper.func.as_ptr()); - let _: Box = Box::from_raw(function_wrapper.legacy_env.as_ptr()); } wasmer_import_export_kind::WASM_GLOBAL => { let _: Box = Box::from_raw(import.value.global as *mut _); @@ -464,7 +463,7 @@ pub unsafe extern "C" fn wasmer_import_object_extend( let func_export = (*func_wrapper).func.as_ptr(); import_object .instance_pointers_to_update - .push((*func_wrapper).legacy_env); + .push((*func_wrapper).legacy_env.clone()); Extern::Function((&*func_export).clone()) } wasmer_import_export_kind::WASM_GLOBAL => { @@ -635,7 +634,7 @@ pub unsafe extern "C" fn wasmer_import_func_params_arity( /// struct used to pass in context to functions (which must be back-patched) -#[derive(Debug, Default, WasmerEnv)] +#[derive(Debug, Default, WasmerEnv, Clone)] pub(crate) struct LegacyEnv { pub(crate) instance_ptr: Option>, } @@ -654,7 +653,7 @@ impl LegacyEnv { #[derive(Debug)] pub(crate) struct FunctionWrapper { pub(crate) func: NonNull, - pub(crate) legacy_env: NonNull, + pub(crate) legacy_env: Arc>, } /// Creates new host function, aka imported function. `func` is a @@ -691,13 +690,14 @@ pub unsafe extern "C" fn wasmer_import_func_new( let store = get_global_store(); - let env_ptr = Box::into_raw(Box::new(LegacyEnv::default())); + let env = Arc::new(Mutex::new(LegacyEnv::default())); - let func = Function::new_with_env(store, &func_type, &mut *env_ptr, move |env, args| { + let func = Function::new_with_env(store, &func_type, env.clone(), move |env, args| { use libffi::high::call::{call, Arg}; use libffi::low::CodePtr; + let env_guard = env.lock().unwrap(); - let ctx_ptr = env.ctx_ptr(); + let ctx_ptr = env_guard.ctx_ptr(); let ctx_ptr_val = ctx_ptr as *const _ as isize as i64; let ffi_args: Vec = { @@ -735,7 +735,7 @@ pub unsafe extern "C" fn wasmer_import_func_new( let function_wrapper = FunctionWrapper { func: NonNull::new_unchecked(Box::into_raw(Box::new(func))), - legacy_env: NonNull::new_unchecked(env_ptr), + legacy_env: env.clone(), }; Box::into_raw(Box::new(function_wrapper)) as *mut wasmer_import_func_t } @@ -865,7 +865,6 @@ pub unsafe extern "C" fn wasmer_import_func_returns_arity( pub unsafe extern "C" fn wasmer_import_func_destroy(func: Option>) { if let Some(func) = func { let function_wrapper = Box::from_raw(func.cast::().as_ptr()); - let _legacy_env = Box::from_raw(function_wrapper.legacy_env.as_ptr()); let _function = Box::from_raw(function_wrapper.func.as_ptr()); } } diff --git a/lib/c-api/src/deprecated/instance.rs b/lib/c-api/src/deprecated/instance.rs index 5ca24ee6ffa..b5d5cf39ddc 100644 --- a/lib/c-api/src/deprecated/instance.rs +++ b/lib/c-api/src/deprecated/instance.rs @@ -186,7 +186,7 @@ pub unsafe extern "C" fn wasmer_instantiate( wasmer_import_export_kind::WASM_FUNCTION => { let func_wrapper = import.value.func as *mut FunctionWrapper; let func_export = (*func_wrapper).func.as_ptr(); - instance_pointers_to_update.push((*func_wrapper).legacy_env); + instance_pointers_to_update.push((*func_wrapper).legacy_env.clone()); Extern::Function((&*func_export).clone()) } wasmer_import_export_kind::WASM_GLOBAL => { @@ -229,8 +229,9 @@ pub unsafe extern "C" fn wasmer_instantiate( ctx_data: None, }; let c_api_instance_pointer = Box::into_raw(Box::new(c_api_instance)); - for mut to_update in instance_pointers_to_update { - to_update.as_mut().instance_ptr = Some(NonNull::new_unchecked(c_api_instance_pointer)); + for to_update in instance_pointers_to_update { + let mut to_update_guard = to_update.lock().unwrap(); + to_update_guard.instance_ptr = Some(NonNull::new_unchecked(c_api_instance_pointer)); } *instance = c_api_instance_pointer as *mut wasmer_instance_t; wasmer_result_t::WASMER_OK diff --git a/lib/c-api/src/deprecated/module.rs b/lib/c-api/src/deprecated/module.rs index 4a160898b60..4b72ab960ab 100644 --- a/lib/c-api/src/deprecated/module.rs +++ b/lib/c-api/src/deprecated/module.rs @@ -192,7 +192,8 @@ pub unsafe extern "C" fn wasmer_module_import_instantiate( }; let c_api_instance_pointer = Box::into_raw(Box::new(c_api_instance)); for to_update in import_object.instance_pointers_to_update.iter_mut() { - to_update.as_mut().instance_ptr = Some(NonNull::new_unchecked(c_api_instance_pointer)); + let mut to_update_guard = to_update.lock().unwrap(); + to_update_guard.instance_ptr = Some(NonNull::new_unchecked(c_api_instance_pointer)); } *instance = c_api_instance_pointer as *mut wasmer_instance_t; diff --git a/lib/c-api/src/wasm_c_api/externals/function.rs b/lib/c-api/src/wasm_c_api/externals/function.rs index f34baa3a370..94e1c58c49a 100644 --- a/lib/c-api/src/wasm_c_api/externals/function.rs +++ b/lib/c-api/src/wasm_c_api/externals/function.rs @@ -101,7 +101,7 @@ pub unsafe extern "C" fn wasm_func_new_with_env( let func_sig = &function_type.inner().function_type; let num_rets = func_sig.results().len(); - #[derive(wasmer::WasmerEnv)] + #[derive(wasmer::WasmerEnv, Clone)] #[repr(C)] struct WrapperEnv { env: *mut c_void, diff --git a/lib/c-api/wasmer_wasm.h b/lib/c-api/wasmer_wasm.h index 9f1a47e8a1e..40a8c375dcb 100644 --- a/lib/c-api/wasmer_wasm.h +++ b/lib/c-api/wasmer_wasm.h @@ -29,6 +29,9 @@ # define DEPRECATED(message) __declspec(deprecated(message)) #endif +// The `jit` feature has been enabled for this build. +#define WASMER_JIT_ENABLED + // The `compiler` feature has been enabled for this build. #define WASMER_COMPILER_ENABLED diff --git a/lib/deprecated/runtime-core/Cargo.lock b/lib/deprecated/runtime-core/Cargo.lock index 93de172f07a..f480eebb9bd 100644 --- a/lib/deprecated/runtime-core/Cargo.lock +++ b/lib/deprecated/runtime-core/Cargo.lock @@ -237,7 +237,7 @@ dependencies = [ "const_fn", "crossbeam-utils", "lazy_static", - "memoffset", + "memoffset 0.5.5", "scopeguard", ] @@ -538,7 +538,7 @@ dependencies = [ "lazy_static", "libc", "regex", - "semver", + "semver 0.9.0", ] [[package]] @@ -593,6 +593,15 @@ dependencies = [ "autocfg", ] +[[package]] +name = "memoffset" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "157b4208e3059a8f9e78d559edc658e13df41410cb3ae03979c83130067fdd87" +dependencies = [ + "autocfg", +] + [[package]] name = "miniz_oxide" version = "0.4.0" @@ -635,9 +644,9 @@ checksum = "1ab52be62400ca80aa00285d25253d7f7c437b7375c4de678f5405d3afe82ca5" [[package]] name = "object" -version = "0.21.1" +version = "0.22.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "37fd5004feb2ce328a52b0b3d01dbf4ffff72583493900ed15f22d4111c51693" +checksum = "8d3b63360ec3cb337817c2dbd47ab4a0f170d285d8e5a2064600f3def1402397" dependencies = [ "crc32fast", "indexmap", @@ -675,6 +684,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "pest" +version = "2.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "10f4872ae94d7b90ae48754df22fd42ad52ce740b8f370b03da4835417403e53" +dependencies = [ + "ucd-trie", +] + [[package]] name = "plain" version = "0.2.3" @@ -882,7 +900,7 @@ version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "138e3e0acb6c9fb258b19b67cb8abd63c00679d2851805ea151465464fe9030a" dependencies = [ - "semver", + "semver 0.9.0", ] [[package]] @@ -917,7 +935,16 @@ version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d7eb9ef2c18661902cc47e535f9bc51b78acd254da71d375c2f6720d9a40403" dependencies = [ - "semver-parser", + "semver-parser 0.7.0", +] + +[[package]] +name = "semver" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f301af10236f6df4160f7c3f04eec6dbc70ace82d23326abad5edee88801c6b6" +dependencies = [ + "semver-parser 0.10.1", ] [[package]] @@ -926,6 +953,15 @@ version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" +[[package]] +name = "semver-parser" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42ef146c2ad5e5f4b037cd6ce2ebb775401729b19a82040c1beac9d36c7d1428" +dependencies = [ + "pest", +] + [[package]] name = "serde" version = "1.0.114" @@ -957,9 +993,9 @@ dependencies = [ [[package]] name = "smallvec" -version = "1.4.1" +version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3757cb9d89161a2f24e1cf78efa0c1fcff485d18e3f55e0aa3480824ddaa0f3f" +checksum = "7acad6f34eb9e8a259d3283d1e8c1d34d7415943d4895f65cc73813c7396fc85" [[package]] name = "stable_deref_trait" @@ -1087,6 +1123,12 @@ version = "1.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "373c8a200f9e67a0c95e62a4f52fbf80c23b4381c05a17845531982fa99e6b33" +[[package]] +name = "ucd-trie" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56dee185309b50d1f11bfedef0fe6d036842e3fb77413abef29f8f8d1c5d4c1c" + [[package]] name = "unicode-xid" version = "0.2.1" @@ -1107,7 +1149,7 @@ checksum = "cccddf32554fecc6acb585f82a32a72e28b48f8c4c1883ddfeeeaa96f7d8e519" [[package]] name = "wasmer" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "cfg-if 0.1.10", "indexmap", @@ -1130,7 +1172,7 @@ dependencies = [ [[package]] name = "wasmer-cache" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "blake3", "hex", @@ -1141,7 +1183,7 @@ dependencies = [ [[package]] name = "wasmer-compiler" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "enumset", "raw-cpuid", @@ -1157,7 +1199,7 @@ dependencies = [ [[package]] name = "wasmer-compiler-cranelift" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "cranelift-codegen", "cranelift-frontend", @@ -1174,7 +1216,7 @@ dependencies = [ [[package]] name = "wasmer-compiler-llvm" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "byteorder", "cc", @@ -1186,7 +1228,7 @@ dependencies = [ "rayon", "regex", "rustc_version", - "semver", + "semver 0.11.0", "smallvec", "target-lexicon", "wasmer-compiler", @@ -1196,7 +1238,7 @@ dependencies = [ [[package]] name = "wasmer-compiler-singlepass" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "byteorder", "dynasm", @@ -1213,7 +1255,7 @@ dependencies = [ [[package]] name = "wasmer-derive" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "proc-macro-error", "proc-macro2", @@ -1223,7 +1265,7 @@ dependencies = [ [[package]] name = "wasmer-engine" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "backtrace", "bincode", @@ -1241,7 +1283,7 @@ dependencies = [ [[package]] name = "wasmer-engine-jit" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "bincode", "cfg-if 0.1.10", @@ -1257,7 +1299,7 @@ dependencies = [ [[package]] name = "wasmer-engine-native" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "bincode", "cfg-if 0.1.10", @@ -1276,9 +1318,9 @@ dependencies = [ [[package]] name = "wasmer-object" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ - "object 0.21.1", + "object 0.22.0", "thiserror", "wasmer-compiler", "wasmer-types", @@ -1304,7 +1346,7 @@ dependencies = [ [[package]] name = "wasmer-types" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "cranelift-entity", "serde", @@ -1312,14 +1354,14 @@ dependencies = [ [[package]] name = "wasmer-vm" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "backtrace", "cc", "cfg-if 0.1.10", "indexmap", "libc", - "memoffset", + "memoffset 0.6.1", "more-asserts", "region", "serde", diff --git a/lib/deprecated/runtime-core/src/typed_func.rs b/lib/deprecated/runtime-core/src/typed_func.rs index 0bfe47d61a5..eb942015948 100644 --- a/lib/deprecated/runtime-core/src/typed_func.rs +++ b/lib/deprecated/runtime-core/src/typed_func.rs @@ -235,11 +235,11 @@ use std::{ /// Initially, it holds an empty `vm::Ctx`, but it is replaced by the /// `vm::Ctx` from `instance::PreInstance` in /// `module::Module::instantiate`. -#[derive(WasmerEnv)] +#[derive(WasmerEnv, Clone)] pub(crate) struct DynamicCtx { pub(crate) vmctx: Rc>, inner_func: - Box Result, RuntimeError> + Send + 'static>, + Rc Result, RuntimeError> + Send + 'static>, } impl DynamicFunc { @@ -251,7 +251,7 @@ impl DynamicFunc { // Create an empty `vm::Ctx`, that is going to be overwritten by `Instance::new`. let ctx = DynamicCtx { vmctx: Rc::new(RefCell::new(unsafe { vm::Ctx::new_uninit() })), - inner_func: Box::new(func), + inner_func: Rc::new(func), }; // Wrapper to safely extract a `&mut vm::Ctx` to pass diff --git a/lib/deprecated/runtime/Cargo.lock b/lib/deprecated/runtime/Cargo.lock index 69d6013f5a3..1e296021f33 100644 --- a/lib/deprecated/runtime/Cargo.lock +++ b/lib/deprecated/runtime/Cargo.lock @@ -237,7 +237,7 @@ dependencies = [ "const_fn", "crossbeam-utils", "lazy_static", - "memoffset", + "memoffset 0.5.5", "scopeguard", ] @@ -538,7 +538,7 @@ dependencies = [ "lazy_static", "libc", "regex", - "semver", + "semver 0.9.0", ] [[package]] @@ -593,6 +593,15 @@ dependencies = [ "autocfg", ] +[[package]] +name = "memoffset" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "157b4208e3059a8f9e78d559edc658e13df41410cb3ae03979c83130067fdd87" +dependencies = [ + "autocfg", +] + [[package]] name = "miniz_oxide" version = "0.4.0" @@ -635,9 +644,9 @@ checksum = "1ab52be62400ca80aa00285d25253d7f7c437b7375c4de678f5405d3afe82ca5" [[package]] name = "object" -version = "0.21.1" +version = "0.22.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "37fd5004feb2ce328a52b0b3d01dbf4ffff72583493900ed15f22d4111c51693" +checksum = "8d3b63360ec3cb337817c2dbd47ab4a0f170d285d8e5a2064600f3def1402397" dependencies = [ "crc32fast", "indexmap", @@ -675,6 +684,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "pest" +version = "2.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "10f4872ae94d7b90ae48754df22fd42ad52ce740b8f370b03da4835417403e53" +dependencies = [ + "ucd-trie", +] + [[package]] name = "plain" version = "0.2.3" @@ -882,7 +900,7 @@ version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "138e3e0acb6c9fb258b19b67cb8abd63c00679d2851805ea151465464fe9030a" dependencies = [ - "semver", + "semver 0.9.0", ] [[package]] @@ -917,7 +935,16 @@ version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d7eb9ef2c18661902cc47e535f9bc51b78acd254da71d375c2f6720d9a40403" dependencies = [ - "semver-parser", + "semver-parser 0.7.0", +] + +[[package]] +name = "semver" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f301af10236f6df4160f7c3f04eec6dbc70ace82d23326abad5edee88801c6b6" +dependencies = [ + "semver-parser 0.10.1", ] [[package]] @@ -926,6 +953,15 @@ version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" +[[package]] +name = "semver-parser" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42ef146c2ad5e5f4b037cd6ce2ebb775401729b19a82040c1beac9d36c7d1428" +dependencies = [ + "pest", +] + [[package]] name = "serde" version = "1.0.114" @@ -957,9 +993,9 @@ dependencies = [ [[package]] name = "smallvec" -version = "1.4.1" +version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3757cb9d89161a2f24e1cf78efa0c1fcff485d18e3f55e0aa3480824ddaa0f3f" +checksum = "7acad6f34eb9e8a259d3283d1e8c1d34d7415943d4895f65cc73813c7396fc85" [[package]] name = "stable_deref_trait" @@ -1087,6 +1123,12 @@ version = "1.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "373c8a200f9e67a0c95e62a4f52fbf80c23b4381c05a17845531982fa99e6b33" +[[package]] +name = "ucd-trie" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56dee185309b50d1f11bfedef0fe6d036842e3fb77413abef29f8f8d1c5d4c1c" + [[package]] name = "unicode-xid" version = "0.2.1" @@ -1107,7 +1149,7 @@ checksum = "cccddf32554fecc6acb585f82a32a72e28b48f8c4c1883ddfeeeaa96f7d8e519" [[package]] name = "wasmer" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "cfg-if 0.1.10", "indexmap", @@ -1130,7 +1172,7 @@ dependencies = [ [[package]] name = "wasmer-cache" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "blake3", "hex", @@ -1141,7 +1183,7 @@ dependencies = [ [[package]] name = "wasmer-compiler" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "enumset", "raw-cpuid", @@ -1157,7 +1199,7 @@ dependencies = [ [[package]] name = "wasmer-compiler-cranelift" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "cranelift-codegen", "cranelift-frontend", @@ -1174,7 +1216,7 @@ dependencies = [ [[package]] name = "wasmer-compiler-llvm" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "byteorder", "cc", @@ -1186,7 +1228,7 @@ dependencies = [ "rayon", "regex", "rustc_version", - "semver", + "semver 0.11.0", "smallvec", "target-lexicon", "wasmer-compiler", @@ -1196,7 +1238,7 @@ dependencies = [ [[package]] name = "wasmer-compiler-singlepass" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "byteorder", "dynasm", @@ -1213,7 +1255,7 @@ dependencies = [ [[package]] name = "wasmer-derive" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "proc-macro-error", "proc-macro2", @@ -1223,7 +1265,7 @@ dependencies = [ [[package]] name = "wasmer-engine" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "backtrace", "bincode", @@ -1241,7 +1283,7 @@ dependencies = [ [[package]] name = "wasmer-engine-jit" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "bincode", "cfg-if 0.1.10", @@ -1257,7 +1299,7 @@ dependencies = [ [[package]] name = "wasmer-engine-native" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "bincode", "cfg-if 0.1.10", @@ -1276,9 +1318,9 @@ dependencies = [ [[package]] name = "wasmer-object" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ - "object 0.21.1", + "object 0.22.0", "thiserror", "wasmer-compiler", "wasmer-types", @@ -1311,7 +1353,7 @@ dependencies = [ [[package]] name = "wasmer-types" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "cranelift-entity", "serde", @@ -1319,14 +1361,14 @@ dependencies = [ [[package]] name = "wasmer-vm" -version = "1.0.0-alpha5" +version = "1.0.0-beta1" dependencies = [ "backtrace", "cc", "cfg-if 0.1.10", "indexmap", "libc", - "memoffset", + "memoffset 0.6.1", "more-asserts", "region", "serde", From 6e95c50dc23811d6a4ee68ba991a3dc7ce874936 Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Tue, 8 Dec 2020 11:02:26 -0800 Subject: [PATCH 5/9] Address feedback --- lib/api/src/env.rs | 8 ------- lib/api/src/externals/function.rs | 40 +++++++++++++++---------------- lib/api/src/native.rs | 8 +++---- lib/engine/src/export.rs | 21 ++++++++++++---- tests/compilers/imports.rs | 2 -- 5 files changed, 40 insertions(+), 39 deletions(-) diff --git a/lib/api/src/env.rs b/lib/api/src/env.rs index 51e9a1bbf97..69bbd003fa4 100644 --- a/lib/api/src/env.rs +++ b/lib/api/src/env.rs @@ -105,7 +105,6 @@ impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicU32 {} impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicI64 {} impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicUsize {} impl<'a> WasmerEnv for &'a ::std::sync::atomic::AtomicIsize {} -//impl WasmerEnv for dyn ::std::any::Any + Clone {} impl WasmerEnv for Box { fn init_with_instance(&mut self, instance: &Instance) -> Result<(), HostEnvInitError> { (&mut **self).init_with_instance(instance) @@ -119,13 +118,6 @@ impl WasmerEnv for ::std::sync::Arc<::std::sync::Mutex> { } } -/*impl WasmerEnv for &'static T { - fn init_with_instance(&mut self, instance: &Instance) -> Result<(), HostEnvInitError> { - T::init_with_instance() - (*self).init_with_instance(instance) - } -}*/ - /// Lazily init an item pub struct LazyInit { /// The data to be initialized diff --git a/lib/api/src/externals/function.rs b/lib/api/src/externals/function.rs index 91eaae4e07e..950747bf6c6 100644 --- a/lib/api/src/externals/function.rs +++ b/lib/api/src/externals/function.rs @@ -88,8 +88,8 @@ impl Function { where F: Fn(&[Val]) -> Result, RuntimeError> + 'static, { - let dynamic_ctx: VMDynamicFunctionContext = - VMDynamicFunctionContext::from_context(VMDynamicFunctionWithoutEnv { + let dynamic_ctx: VMDynamicFunctionContext = + VMDynamicFunctionContext::from_context(DynamicFunctionWithoutEnv { func: Arc::new(func), function_type: ty.clone(), }); @@ -100,16 +100,16 @@ impl Function { let host_env = Box::into_raw(Box::new(dynamic_ctx)) as *mut _; let vmctx = VMFunctionEnvironment { host_env }; let host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void = |ptr| { - let duped_env: VMDynamicFunctionContext = unsafe { - let ptr: *mut VMDynamicFunctionContext = ptr as _; - let item: &VMDynamicFunctionContext = &*ptr; + let duped_env: VMDynamicFunctionContext = unsafe { + let ptr: *mut VMDynamicFunctionContext = ptr as _; + let item: &VMDynamicFunctionContext = &*ptr; item.clone() }; Box::into_raw(Box::new(duped_env)) as _ }; let host_env_drop_fn: fn(*mut std::ffi::c_void) = |ptr: *mut std::ffi::c_void| { unsafe { - Box::from_raw(ptr as *mut VMDynamicFunctionContext) + Box::from_raw(ptr as *mut VMDynamicFunctionContext) }; }; @@ -162,8 +162,8 @@ impl Function { F: Fn(&Env, &[Val]) -> Result, RuntimeError> + 'static, Env: Sized + WasmerEnv + 'static, { - let dynamic_ctx: VMDynamicFunctionContext> = - VMDynamicFunctionContext::from_context(VMDynamicFunctionWithEnv { + let dynamic_ctx: VMDynamicFunctionContext> = + VMDynamicFunctionContext::from_context(DynamicFunctionWithEnv { env: { let e = Box::new(env); e @@ -179,7 +179,7 @@ impl Function { let vmctx = VMFunctionEnvironment { host_env }; let import_init_function_ptr: fn(_, _) -> Result<(), _> = |ptr: *mut std::ffi::c_void, instance: *const std::ffi::c_void| { - let ptr = ptr as *mut VMDynamicFunctionContext>; + let ptr = ptr as *mut VMDynamicFunctionContext>; unsafe { let env = &mut *ptr; let env: &mut Env = &mut *env.ctx.env; @@ -193,16 +193,16 @@ impl Function { ) }); let host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void = |ptr| { - let duped_env: VMDynamicFunctionContext> = unsafe { - let ptr: *mut VMDynamicFunctionContext> = ptr as _; - let item: &VMDynamicFunctionContext> = &*ptr; + let duped_env: VMDynamicFunctionContext> = unsafe { + let ptr: *mut VMDynamicFunctionContext> = ptr as _; + let item: &VMDynamicFunctionContext> = &*ptr; item.clone() }; Box::into_raw(Box::new(duped_env)) as _ }; let host_env_drop_fn: fn(*mut std::ffi::c_void) = |ptr: *mut std::ffi::c_void| { unsafe { - Box::from_raw(ptr as *mut VMDynamicFunctionContext>) + Box::from_raw(ptr as *mut VMDynamicFunctionContext>) }; }; @@ -795,13 +795,13 @@ pub(crate) trait VMDynamicFunction { } #[derive(Clone)] -pub(crate) struct VMDynamicFunctionWithoutEnv { +pub(crate) struct DynamicFunctionWithoutEnv { #[allow(clippy::type_complexity)] func: Arc Result, RuntimeError> + 'static>, function_type: FunctionType, } -impl VMDynamicFunction for VMDynamicFunctionWithoutEnv { +impl VMDynamicFunction for DynamicFunctionWithoutEnv { fn call(&self, args: &[Val]) -> Result, RuntimeError> { (*self.func)(&args) } @@ -810,19 +810,17 @@ impl VMDynamicFunction for VMDynamicFunctionWithoutEnv { } } -#[repr(C)] -pub(crate) struct VMDynamicFunctionWithEnv +pub(crate) struct DynamicFunctionWithEnv where Env: Sized + 'static, { - // This field _must_ come first in this struct. - env: Box, function_type: FunctionType, #[allow(clippy::type_complexity)] func: Arc Result, RuntimeError> + 'static>, + env: Box, } -impl Clone for VMDynamicFunctionWithEnv { +impl Clone for DynamicFunctionWithEnv { fn clone(&self) -> Self { Self { env: self.env.clone(), @@ -832,7 +830,7 @@ impl Clone for VMDynamicFunctionWithEnv { } } -impl VMDynamicFunction for VMDynamicFunctionWithEnv +impl VMDynamicFunction for DynamicFunctionWithEnv where Env: Sized + 'static, { diff --git a/lib/api/src/native.rs b/lib/api/src/native.rs index c7619339458..9eaff1f0c40 100644 --- a/lib/api/src/native.rs +++ b/lib/api/src/native.rs @@ -10,8 +10,8 @@ use std::marker::PhantomData; use crate::externals::function::{ - FunctionDefinition, HostFunctionDefinition, VMDynamicFunction, VMDynamicFunctionWithEnv, - VMDynamicFunctionWithoutEnv, WasmFunctionDefinition, + DynamicFunctionWithEnv, DynamicFunctionWithoutEnv, FunctionDefinition, HostFunctionDefinition, + VMDynamicFunction, WasmFunctionDefinition, }; use crate::{FromToNativeWasmType, Function, FunctionType, RuntimeError, Store, WasmTypeList}; use std::panic::{catch_unwind, AssertUnwindSafe}; @@ -205,13 +205,13 @@ macro_rules! impl_native_traits { VMFunctionKind::Dynamic => { let params_list = [ $( $x.to_native().to_value() ),* ]; let results = if !has_env { - type VMContextWithoutEnv = VMDynamicFunctionContext; + type VMContextWithoutEnv = VMDynamicFunctionContext; unsafe { let ctx = self.vmctx.host_env as *mut VMContextWithoutEnv; (*ctx).ctx.call(¶ms_list)? } } else { - type VMContextWithEnv = VMDynamicFunctionContext>; + type VMContextWithEnv = VMDynamicFunctionContext>; unsafe { let ctx = self.vmctx.host_env as *mut VMContextWithEnv; (*ctx).ctx.call(¶ms_list)? diff --git a/lib/engine/src/export.rs b/lib/engine/src/export.rs index 903405f4286..96334c978d7 100644 --- a/lib/engine/src/export.rs +++ b/lib/engine/src/export.rs @@ -46,11 +46,25 @@ impl From for Export { } } -/// TODO: rename, etc. +/// Extra metadata about `ExportFunction`s. +/// +/// The metadata acts as a kind of manual virtual dispatch. We store the +/// user-supplied `WasmerEnv` as a void pointer and have methods on it +/// that have been adapted to accept a void pointer. #[derive(Debug, PartialEq)] pub struct ExportFunctionMetadata { - /// duplicated here so we can free it.... - /// TODO: refactor all this stuff so it's less of a nightmare. + /// This field is stored here to be accessible by `Drop`. + /// + /// At the time it was added, it's not accessed anywhere outside of + /// the `Drop` implementation. This field is the "master copy" of the env, + /// that is, the original env passed in by the user. Every time we create + /// an `Instance` we clone this with the `host_env_clone_fn` field. + /// + /// Thus, we only bother to store the master copy at all here so that + /// we can free it. + /// + /// See `wasmer_vm::export::VMExportFunction::vmctx` for the version of + /// this pointer that is used by the VM when creating an `Instance`. pub host_env: *mut std::ffi::c_void, /// Function pointer to `WasmerEnv::init_with_instance(&mut self, instance: &Instance)`. /// @@ -69,7 +83,6 @@ pub struct ExportFunctionMetadata { // so all the `host_env`s freed at the `Instance` level won't touch the original. impl Drop for ExportFunctionMetadata { fn drop(&mut self) { - dbg!("DROPPING ORIGINAL HOST ENV!"); if !self.host_env.is_null() { (self.host_env_drop_fn)(self.host_env); } diff --git a/tests/compilers/imports.rs b/tests/compilers/imports.rs index cfed0c69239..57c9383c19c 100644 --- a/tests/compilers/imports.rs +++ b/tests/compilers/imports.rs @@ -357,7 +357,6 @@ fn multi_use_host_fn_manages_memory_correctly() -> Result<()> { impl WasmerEnv for Env { fn init_with_instance(&mut self, instance: &Instance) -> Result<(), HostEnvInitError> { - dbg!("Initing the env!"); let memory = instance.exports.get_memory("memory")?.clone(); self.memory.initialize(memory); Ok(()) @@ -368,7 +367,6 @@ fn multi_use_host_fn_manages_memory_correctly() -> Result<()> { memory: LazyInit::default(), }; fn host_fn(env: &Env) { - dbg!(env as *const _); assert!(env.memory.get_ref().is_some()); println!("Hello, world!"); } From be3c381169a2e1493e368e81e59945dc4de4e33e Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Wed, 9 Dec 2020 12:26:53 -0800 Subject: [PATCH 6/9] Make `NativeFunc` `Clone` --- Cargo.lock | 1 + Cargo.toml | 8 ++++---- lib/api/src/native.rs | 8 +++----- lib/emscripten/src/lib.rs | 4 ++-- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3a1ae3f31f9..3046aafb0a4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2807,6 +2807,7 @@ dependencies = [ "wasmer-compiler-cranelift", "wasmer-compiler-llvm", "wasmer-compiler-singlepass", + "wasmer-emscripten", "wasmer-engine", "wasmer-engine-dummy", "wasmer-engine-jit", diff --git a/Cargo.toml b/Cargo.toml index bfb49a17805..bdf29749ea6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ wasmer-compiler = { version = "1.0.0-beta1", path = "lib/compiler" } wasmer-compiler-cranelift = { version = "1.0.0-beta1", path = "lib/compiler-cranelift", optional = true } wasmer-compiler-singlepass = { version = "1.0.0-beta1", path = "lib/compiler-singlepass", optional = true } wasmer-compiler-llvm = { version = "1.0.0-beta1", path = "lib/compiler-llvm", optional = true } -#wasmer-emscripten = { version = "1.0.0-beta1", path = "lib/emscripten", optional = true } +wasmer-emscripten = { version = "1.0.0-beta1", path = "lib/emscripten", optional = true } wasmer-engine = { version = "1.0.0-beta1", path = "lib/engine" } wasmer-engine-jit = { version = "1.0.0-beta1", path = "lib/engine-jit", optional = true } wasmer-engine-native = { version = "1.0.0-beta1", path = "lib/engine-native", optional = true } @@ -38,7 +38,7 @@ members = [ "lib/compiler-singlepass", "lib/compiler-llvm", "lib/derive", -# "lib/emscripten", + "lib/emscripten", "lib/engine", "lib/engine-jit", "lib/engine-native", @@ -81,7 +81,7 @@ default = [ "object-file", "cache", "wasi", -# "emscripten", + "emscripten", "middlewares", ] engine = [] @@ -100,7 +100,7 @@ object-file = [ cache = ["wasmer-cache"] wast = ["wasmer-wast"] wasi = ["wasmer-wasi"] -#emscripten = ["wasmer-emscripten"] +emscripten = ["wasmer-emscripten"] wat = ["wasmer/wat"] compiler = [ "wasmer/compiler", diff --git a/lib/api/src/native.rs b/lib/api/src/native.rs index 6d93dbbc137..98dee8675b1 100644 --- a/lib/api/src/native.rs +++ b/lib/api/src/native.rs @@ -13,17 +13,15 @@ use crate::externals::function::{ DynamicFunctionWithEnv, DynamicFunctionWithoutEnv, FunctionDefinition, HostFunctionDefinition, VMDynamicFunction, WasmFunctionDefinition, }; -use crate::{FromToNativeWasmType, Function, FunctionType, RuntimeError, Store, WasmTypeList}; +use crate::{FromToNativeWasmType, Function, RuntimeError, Store, WasmTypeList}; use std::panic::{catch_unwind, AssertUnwindSafe}; use wasmer_engine::ExportFunction; use wasmer_types::NativeWasmType; -use wasmer_vm::{ - VMDynamicFunctionContext, VMExportFunction, VMFunctionBody, VMFunctionEnvironment, - VMFunctionKind, -}; +use wasmer_vm::{VMDynamicFunctionContext, VMFunctionBody, VMFunctionEnvironment, VMFunctionKind}; /// A WebAssembly function that can be called natively /// (using the Native ABI). +#[derive(Clone)] pub struct NativeFunc { definition: FunctionDefinition, store: Store, diff --git a/lib/emscripten/src/lib.rs b/lib/emscripten/src/lib.rs index da32dc9cede..22f4224aa4f 100644 --- a/lib/emscripten/src/lib.rs +++ b/lib/emscripten/src/lib.rs @@ -14,7 +14,7 @@ extern crate log; use lazy_static::lazy_static; -use std::cell::UnsafeCell; +use std::cell::Cell; use std::collections::HashMap; use std::f64; use std::path::PathBuf; @@ -136,7 +136,7 @@ pub struct EmscriptenData { pub memset: LazyInit>, #[wasmer(export)] pub stack_alloc: LazyInit>, - pub jumps: Vec>, + pub jumps: Vec>, pub opened_dirs: HashMap>, #[wasmer(export)] From ec4d6e6d108fcdb61aae31c524a60686c575f1ca Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Thu, 10 Dec 2020 15:20:57 -0800 Subject: [PATCH 7/9] Address feedback, make code more explicit about the different cases --- lib/api/src/externals/function.rs | 14 +-- lib/engine/src/export.rs | 35 +++++- lib/engine/src/resolver.rs | 34 +++--- lib/vm/src/instance.rs | 175 +++++++++++++++++++++++------- 4 files changed, 192 insertions(+), 66 deletions(-) diff --git a/lib/api/src/externals/function.rs b/lib/api/src/externals/function.rs index eb4b4ae59f5..98df122cd8a 100644 --- a/lib/api/src/externals/function.rs +++ b/lib/api/src/externals/function.rs @@ -117,12 +117,12 @@ impl Function { store: store.clone(), definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: false }), exported: ExportFunction { - metadata: Some(Arc::new(ExportFunctionMetadata { + metadata: Some(Arc::new(ExportFunctionMetadata::new( host_env, - import_init_function_ptr: None, + None, host_env_clone_fn, host_env_drop_fn, - })), + ))), vm_function: VMExportFunction { address, kind: VMFunctionKind::Dynamic, @@ -210,12 +210,12 @@ impl Function { store: store.clone(), definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: true }), exported: ExportFunction { - metadata: Some(Arc::new(ExportFunctionMetadata { + metadata: Some(Arc::new(ExportFunctionMetadata::new( host_env, import_init_function_ptr, host_env_clone_fn, host_env_drop_fn, - })), + ))), vm_function: VMExportFunction { address, kind: VMFunctionKind::Dynamic, @@ -349,12 +349,12 @@ impl Function { store: store.clone(), definition: FunctionDefinition::Host(HostFunctionDefinition { has_env: true }), exported: ExportFunction { - metadata: Some(Arc::new(ExportFunctionMetadata { + metadata: Some(Arc::new(ExportFunctionMetadata::new( host_env, import_init_function_ptr, host_env_clone_fn, host_env_drop_fn, - })), + ))), vm_function: VMExportFunction { address, kind: VMFunctionKind::Static, diff --git a/lib/engine/src/export.rs b/lib/engine/src/export.rs index 96334c978d7..005c9a73fb2 100644 --- a/lib/engine/src/export.rs +++ b/lib/engine/src/export.rs @@ -51,6 +51,9 @@ impl From for Export { /// The metadata acts as a kind of manual virtual dispatch. We store the /// user-supplied `WasmerEnv` as a void pointer and have methods on it /// that have been adapted to accept a void pointer. +/// +/// This struct owns the original `host_env`, thus when it gets dropped +/// it calls the `drop` function on it. #[derive(Debug, PartialEq)] pub struct ExportFunctionMetadata { /// This field is stored here to be accessible by `Drop`. @@ -65,18 +68,36 @@ pub struct ExportFunctionMetadata { /// /// See `wasmer_vm::export::VMExportFunction::vmctx` for the version of /// this pointer that is used by the VM when creating an `Instance`. - pub host_env: *mut std::ffi::c_void, + pub(crate) host_env: *mut std::ffi::c_void, /// Function pointer to `WasmerEnv::init_with_instance(&mut self, instance: &Instance)`. /// /// This function is called to finish setting up the environment after /// we create the `api::Instance`. // This one is optional for now because dynamic host envs need the rest // of this without the init fn - pub import_init_function_ptr: Option, - /// TODO: - pub host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, + pub(crate) import_init_function_ptr: Option, + /// A function analogous to `Clone::clone` that returns a leaked `Box`. + pub(crate) host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, /// The destructor to free the host environment. - pub host_env_drop_fn: fn(*mut std::ffi::c_void), + pub(crate) host_env_drop_fn: fn(*mut std::ffi::c_void), +} + +impl ExportFunctionMetadata { + /// Create an `ExportFunctionMetadata` type with information about + /// the exported function. + pub fn new( + host_env: *mut std::ffi::c_void, + import_init_function_ptr: Option, + host_env_clone_fn: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, + host_env_drop_fn: fn(*mut std::ffi::c_void), + ) -> Self { + Self { + host_env, + import_init_function_ptr, + host_env_clone_fn, + host_env_drop_fn, + } + } } // We have to free `host_env` here because we always clone it before using it @@ -95,7 +116,9 @@ impl Drop for ExportFunctionMetadata { pub struct ExportFunction { /// The VM function, containing most of the data. pub vm_function: VMExportFunction, - /// TODO: + /// Contains functions necessary to create and initialize host envs + /// with each `Instance` as well as being responsible for the + /// underlying memory of the host env. pub metadata: Option>, } diff --git a/lib/engine/src/resolver.rs b/lib/engine/src/resolver.rs index f8e63cafc97..c4f18a46a44 100644 --- a/lib/engine/src/resolver.rs +++ b/lib/engine/src/resolver.rs @@ -179,10 +179,7 @@ pub fn resolve_imports( } }; - // TODO: add lots of documentation for this really strange - // looking code. - // Also, keep eyes open for refactor opportunities to clean - // this all up. + // Clone the host env for this `Instance`. let env = if let Some(ExportFunctionMetadata { host_env_clone_fn: clone, .. @@ -196,10 +193,10 @@ pub fn resolve_imports( (clone)(f.vm_function.vmctx.host_env) } } else { - unsafe { - //assert!(f.vm_function.vmctx.host_env.is_null()); - f.vm_function.vmctx.host_env - } + // No `clone` function means we're dealing with some + // other kind of `vmctx`, not a host env of any + // kind. + unsafe { f.vm_function.vmctx.host_env } }; function_imports.push(VMFunctionImport { @@ -207,13 +204,20 @@ pub fn resolve_imports( environment: VMFunctionEnvironment { host_env: env }, }); - host_function_env_initializers.push(ImportEnv { - env, - // TODO: consider just passing metadata directly - initializer: f.metadata.as_ref().and_then(|m| m.import_init_function_ptr), - clone: f.metadata.as_ref().map(|m| m.host_env_clone_fn), - destructor: f.metadata.as_ref().map(|m| m.host_env_drop_fn), - }); + let initializer = f.metadata.as_ref().and_then(|m| m.import_init_function_ptr); + let clone = f.metadata.as_ref().map(|m| m.host_env_clone_fn); + let destructor = f.metadata.as_ref().map(|m| m.host_env_drop_fn); + let import_env = if let (Some(clone), Some(destructor)) = (clone, destructor) { + if let Some(initializer) = initializer { + ImportEnv::new_host_env(env, clone, initializer, destructor) + } else { + ImportEnv::new_dynamic_host_env_with_no_inner_env(env, clone, destructor) + } + } else { + ImportEnv::new_no_env() + }; + + host_function_env_initializers.push(import_env); } Export::Table(ref t) => { table_imports.push(VMTableImport { diff --git a/lib/vm/src/instance.rs b/lib/vm/src/instance.rs index 14413f1440b..c648d4e34b8 100644 --- a/lib/vm/src/instance.rs +++ b/lib/vm/src/instance.rs @@ -105,41 +105,135 @@ pub(crate) struct Instance { vmctx: VMContext, } -/// TODO: figure out what to do with Clone here -/// We can probably remove the Arc... just need to figure that out. -/// TODO: +/// A collection of data about host envs. +/// +/// Due to invariants in the `HostEnv` variant, this type should only +/// be constructed by one of the given methods, not directly. #[derive(Debug)] -pub struct ImportEnv { - /// TODO: - pub env: *mut std::ffi::c_void, - /// TODO: - pub clone: Option *mut std::ffi::c_void>, - /// TODO: - pub initializer: Option, - /// TODO: - pub destructor: Option, +pub enum ImportEnv { + /// The `vmctx` pointer does not refer to a host env, there is no + /// metadata about it. + NoEnv, + /// We're dealing with a user-defined host env. + /// + /// This host env may be either unwrapped (the user-supplied host env + /// directly) or wrapped. i.e. in the case of Dynamic functions, we + /// store our own extra data along with the user supplied env, + /// thus the `env` pointer here points to the outermost type. + HostEnv { + /// The function environment. This is not always the user-supplied + /// env. + env: *mut std::ffi::c_void, + /// A clone function for duplicating the env. + clone: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, + /// This type must start out as Some. `None` indicates that the + /// initializer has been called and must not be called again. + initializer: Option, + /// The destructor to clean up the type in `env`. + destructor: fn(*mut std::ffi::c_void), + }, + /// Like `Self::HostEnv` but contains no user supplied env. + /// + /// i.e. Dynamic functions without an env. + DynamicHostEnvWithNoInnerEnv { + /// Some type that does not contain a user supplied env at all, + /// thus we have no need to initialize it. + env: *mut std::ffi::c_void, + /// A clone function for duplicating the env. + clone: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, + /// The destructor to clean up the type in `env`. + destructor: fn(*mut std::ffi::c_void), + }, +} + +impl ImportEnv { + /// Make a new `Self::HostEnv`. + pub fn new_host_env( + env: *mut std::ffi::c_void, + clone: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, + initializer: ImportInitializerFuncPtr, + destructor: fn(*mut std::ffi::c_void), + ) -> Self { + Self::HostEnv { + env, + clone, + initializer: Some(initializer), + destructor, + } + } + + /// Make a new `Self::DynamicHostEnvWithNoInnerEnv`. + pub fn new_dynamic_host_env_with_no_inner_env( + env: *mut std::ffi::c_void, + clone: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, + destructor: fn(*mut std::ffi::c_void), + ) -> Self { + Self::DynamicHostEnvWithNoInnerEnv { + env, + clone, + destructor, + } + } + + /// Make a new `Self::NoEnv`. + pub fn new_no_env() -> Self { + Self::NoEnv + } + + fn initializer(&self) -> Option { + match self { + Self::HostEnv { initializer, .. } => *initializer, + _ => None, + } + } } impl Clone for ImportEnv { fn clone(&self) -> Self { - let env = if let Some(clone) = self.clone { - (clone)(self.env) - } else { - self.env - }; - Self { - env, - clone: self.clone.clone(), - initializer: self.initializer.clone(), - destructor: self.destructor.clone(), + match &self { + Self::NoEnv => Self::NoEnv, + Self::HostEnv { + env, + clone, + destructor, + initializer, + } => { + let new_env = (*clone)(*env); + Self::HostEnv { + env: new_env, + clone: *clone, + destructor: *destructor, + initializer: *initializer, + } + } + Self::DynamicHostEnvWithNoInnerEnv { + env, + clone, + destructor, + } => { + let new_env = (*clone)(*env); + Self::DynamicHostEnvWithNoInnerEnv { + env: new_env, + clone: *clone, + destructor: *destructor, + } + } } } } impl Drop for ImportEnv { fn drop(&mut self) { - if let Some(destructor) = self.destructor { - (destructor)(self.env); + match self { + ImportEnv::DynamicHostEnvWithNoInnerEnv { + env, destructor, .. + } + | ImportEnv::HostEnv { + env, destructor, .. + } => { + (destructor)(*env); + } + ImportEnv::NoEnv => (), } } } @@ -190,7 +284,7 @@ impl Instance { &self, index: FunctionIndex, ) -> Option { - self.import_envs[index].initializer + self.import_envs[index].initializer() } /// Return a pointer to the `VMFunctionImport`s. @@ -1446,21 +1540,26 @@ impl InstanceHandle { ) -> Result<(), Err> { let instance_ref = self.instance.as_mut(); - for ImportEnv { - env, initializer, .. - } in instance_ref.import_envs.values_mut() - { - if let Some(ref f) = initializer { - // transmute our function pointer into one with the correct error type - let f = mem::transmute::< - &ImportInitializerFuncPtr, - &fn(*mut ffi::c_void, *const ffi::c_void) -> Result<(), Err>, - >(f); - f(*env, instance_ptr)?; - *initializer = None; + for import_env in instance_ref.import_envs.values_mut() { + match import_env { + ImportEnv::HostEnv { + env, + ref mut initializer, + .. + } => { + if let Some(f) = initializer { + // transmute our function pointer into one with the correct error type + let f = mem::transmute::< + &ImportInitializerFuncPtr, + &fn(*mut ffi::c_void, *const ffi::c_void) -> Result<(), Err>, + >(f); + f(*env, instance_ptr)?; + } + *initializer = None; + } + ImportEnv::DynamicHostEnvWithNoInnerEnv { .. } | ImportEnv::NoEnv => (), } } - Ok(()) } } From bd01d6a203e991332fcb885351b56344c0c0dd1d Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Fri, 11 Dec 2020 15:47:12 -0800 Subject: [PATCH 8/9] Address feedback --- lib/engine/src/artifact.rs | 8 ++++---- lib/engine/src/resolver.rs | 12 ++++++------ lib/vm/src/imports.rs | 8 ++++---- lib/vm/src/instance.rs | 36 +++++++++++++++++------------------- lib/vm/src/lib.rs | 2 +- 5 files changed, 32 insertions(+), 34 deletions(-) diff --git a/lib/engine/src/artifact.rs b/lib/engine/src/artifact.rs index b91ddd66380..25953e84be9 100644 --- a/lib/engine/src/artifact.rs +++ b/lib/engine/src/artifact.rs @@ -96,7 +96,7 @@ pub trait Artifact: Send + Sync + Upcastable { let module = self.module(); let (instance_ptr, offsets) = InstanceHandle::allocate_instance(&module); - let (imports, import_envs) = { + let (imports, import_function_envs) = { let mut imports = resolve_imports( &module, resolver, @@ -108,9 +108,9 @@ pub trait Artifact: Send + Sync + Upcastable { // Get the `WasmerEnv::init_with_instance` function pointers and the pointers // to the envs to call it on. - let import_envs = imports.get_import_initializers(); + let import_function_envs = imports.get_imported_function_envs(); - (imports, import_envs) + (imports, import_function_envs) }; // Get pointers to where metadata about local memories should live in VM memory. @@ -146,7 +146,7 @@ pub trait Artifact: Send + Sync + Upcastable { imports, self.signatures().clone(), host_state, - import_envs, + import_function_envs, ) .map_err(|trap| InstantiationError::Start(RuntimeError::from_trap(trap)))?; Ok(handle) diff --git a/lib/engine/src/resolver.rs b/lib/engine/src/resolver.rs index c4f18a46a44..c151ea7b852 100644 --- a/lib/engine/src/resolver.rs +++ b/lib/engine/src/resolver.rs @@ -7,7 +7,7 @@ use wasmer_types::entity::{BoxedSlice, EntityRef, PrimaryMap}; use wasmer_types::{ExternType, FunctionIndex, ImportIndex, MemoryIndex, TableIndex}; use wasmer_vm::{ - FunctionBodyPtr, ImportEnv, Imports, MemoryStyle, ModuleInfo, TableStyle, VMFunctionBody, + FunctionBodyPtr, ImportFunctionEnv, Imports, MemoryStyle, ModuleInfo, TableStyle, VMFunctionBody, VMFunctionEnvironment, VMFunctionImport, VMFunctionKind, VMGlobalImport, VMMemoryImport, VMTableImport, }; @@ -207,17 +207,17 @@ pub fn resolve_imports( let initializer = f.metadata.as_ref().and_then(|m| m.import_init_function_ptr); let clone = f.metadata.as_ref().map(|m| m.host_env_clone_fn); let destructor = f.metadata.as_ref().map(|m| m.host_env_drop_fn); - let import_env = if let (Some(clone), Some(destructor)) = (clone, destructor) { + let import_function_env = if let (Some(clone), Some(destructor)) = (clone, destructor) { if let Some(initializer) = initializer { - ImportEnv::new_host_env(env, clone, initializer, destructor) + ImportFunctionEnv::new_host_env(env, clone, initializer, destructor) } else { - ImportEnv::new_dynamic_host_env_with_no_inner_env(env, clone, destructor) + ImportFunctionEnv::new_dynamic_host_env_with_no_inner_env(env, clone, destructor) } } else { - ImportEnv::new_no_env() + ImportFunctionEnv::new_no_env() }; - host_function_env_initializers.push(import_env); + host_function_env_initializers.push(import_function_env); } Export::Table(ref t) => { table_imports.push(VMTableImport { diff --git a/lib/vm/src/imports.rs b/lib/vm/src/imports.rs index d50b79826f1..428f1e3be77 100644 --- a/lib/vm/src/imports.rs +++ b/lib/vm/src/imports.rs @@ -1,7 +1,7 @@ // This file contains code from external sources. // Attributions: https://github.com/wasmerio/wasmer/blob/master/ATTRIBUTIONS.md -use crate::instance::ImportEnv; +use crate::instance::ImportFunctionEnv; use crate::vmcontext::{VMFunctionImport, VMGlobalImport, VMMemoryImport, VMTableImport}; use wasmer_types::entity::{BoxedSlice, PrimaryMap}; use wasmer_types::{FunctionIndex, GlobalIndex, MemoryIndex, TableIndex}; @@ -17,7 +17,7 @@ pub struct Imports { /// space may affect Wasm runtime performance due to increased cache pressure. /// /// We make it optional so that we can free the data after use. - pub host_function_env_initializers: Option>, + pub host_function_env_initializers: Option>, /// Resolved addresses for imported tables. pub tables: BoxedSlice, @@ -33,7 +33,7 @@ impl Imports { /// Construct a new `Imports` instance. pub fn new( function_imports: PrimaryMap, - host_function_env_initializers: PrimaryMap, + host_function_env_initializers: PrimaryMap, table_imports: PrimaryMap, memory_imports: PrimaryMap, global_imports: PrimaryMap, @@ -63,7 +63,7 @@ impl Imports { /// /// This function can only be called once, it deletes the data it returns after /// returning it to ensure that it's not called more than once. - pub fn get_import_initializers(&mut self) -> BoxedSlice { + pub fn get_imported_function_envs(&mut self) -> BoxedSlice { self.host_function_env_initializers .take() .unwrap_or_else(|| PrimaryMap::new().into_boxed_slice()) diff --git a/lib/vm/src/instance.rs b/lib/vm/src/instance.rs index c648d4e34b8..e2ea898f379 100644 --- a/lib/vm/src/instance.rs +++ b/lib/vm/src/instance.rs @@ -89,14 +89,12 @@ pub(crate) struct Instance { /// Handler run when `SIGBUS`, `SIGFPE`, `SIGILL`, or `SIGSEGV` are caught by the instance thread. pub(crate) signal_handler: Cell>>, - /// Functions to initialize the host environments in the imports - /// and pointers to the environments. These function pointers all - /// come from `WasmerEnv::init_with_instance`. + /// Functions to operate on host environments in the imports + /// and pointers to the environments. /// /// TODO: Be sure to test with serialize/deserialize and imported /// functions from other Wasm modules. - /// TODO: update this comment - import_envs: BoxedSlice, + imported_function_envs: BoxedSlice, /// Additional context used by compiled WebAssembly code. This /// field is last, and represents a dynamically-sized array that @@ -110,7 +108,7 @@ pub(crate) struct Instance { /// Due to invariants in the `HostEnv` variant, this type should only /// be constructed by one of the given methods, not directly. #[derive(Debug)] -pub enum ImportEnv { +pub enum ImportFunctionEnv { /// The `vmctx` pointer does not refer to a host env, there is no /// metadata about it. NoEnv, @@ -146,7 +144,7 @@ pub enum ImportEnv { }, } -impl ImportEnv { +impl ImportFunctionEnv { /// Make a new `Self::HostEnv`. pub fn new_host_env( env: *mut std::ffi::c_void, @@ -188,7 +186,7 @@ impl ImportEnv { } } -impl Clone for ImportEnv { +impl Clone for ImportFunctionEnv { fn clone(&self) -> Self { match &self { Self::NoEnv => Self::NoEnv, @@ -222,18 +220,18 @@ impl Clone for ImportEnv { } } -impl Drop for ImportEnv { +impl Drop for ImportFunctionEnv { fn drop(&mut self) { match self { - ImportEnv::DynamicHostEnvWithNoInnerEnv { + ImportFunctionEnv::DynamicHostEnvWithNoInnerEnv { env, destructor, .. } - | ImportEnv::HostEnv { + | ImportFunctionEnv::HostEnv { env, destructor, .. } => { (destructor)(*env); } - ImportEnv::NoEnv => (), + ImportFunctionEnv::NoEnv => (), } } } @@ -284,7 +282,7 @@ impl Instance { &self, index: FunctionIndex, ) -> Option { - self.import_envs[index].initializer() + self.imported_function_envs[index].initializer() } /// Return a pointer to the `VMFunctionImport`s. @@ -1136,7 +1134,7 @@ impl InstanceHandle { imports: Imports, vmshared_signatures: BoxedSlice, host_state: Box, - import_envs: BoxedSlice, + imported_function_envs: BoxedSlice, ) -> Result { // `NonNull` here actually means `NonNull`. See // `Self::allocate_instance` to understand why. @@ -1164,7 +1162,7 @@ impl InstanceHandle { passive_data, host_state, signal_handler: Cell::new(None), - import_envs, + imported_function_envs, vmctx: VMContext {}, }; @@ -1540,9 +1538,9 @@ impl InstanceHandle { ) -> Result<(), Err> { let instance_ref = self.instance.as_mut(); - for import_env in instance_ref.import_envs.values_mut() { - match import_env { - ImportEnv::HostEnv { + for import_function_env in instance_ref.imported_function_envs.values_mut() { + match import_function_env { + ImportFunctionEnv::HostEnv { env, ref mut initializer, .. @@ -1557,7 +1555,7 @@ impl InstanceHandle { } *initializer = None; } - ImportEnv::DynamicHostEnvWithNoInnerEnv { .. } | ImportEnv::NoEnv => (), + ImportFunctionEnv::DynamicHostEnvWithNoInnerEnv { .. } | ImportFunctionEnv::NoEnv => (), } } Ok(()) diff --git a/lib/vm/src/lib.rs b/lib/vm/src/lib.rs index 893739988de..0b75ce58a9a 100644 --- a/lib/vm/src/lib.rs +++ b/lib/vm/src/lib.rs @@ -39,7 +39,7 @@ pub mod libcalls; pub use crate::export::*; pub use crate::global::*; pub use crate::imports::Imports; -pub use crate::instance::{ImportEnv, ImportInitializerFuncPtr, InstanceHandle}; +pub use crate::instance::{ImportFunctionEnv, ImportInitializerFuncPtr, InstanceHandle}; pub use crate::memory::{LinearMemory, Memory, MemoryError, MemoryStyle}; pub use crate::mmap::Mmap; pub use crate::module::{ExportsIterator, ImportsIterator, ModuleInfo}; From 27f7e7e255830783475b5827d4066da54bf5ebe6 Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Tue, 15 Dec 2020 08:31:48 -0800 Subject: [PATCH 9/9] Remove variant from `ImportFunctionEnv` --- lib/engine/src/resolver.rs | 13 +++--- lib/vm/src/imports.rs | 5 +++ lib/vm/src/instance/mod.rs | 86 ++++++-------------------------------- 3 files changed, 23 insertions(+), 81 deletions(-) diff --git a/lib/engine/src/resolver.rs b/lib/engine/src/resolver.rs index 85f20618d29..acc1c800ca5 100644 --- a/lib/engine/src/resolver.rs +++ b/lib/engine/src/resolver.rs @@ -209,15 +209,14 @@ pub fn resolve_imports( let destructor = f.metadata.as_ref().map(|m| m.host_env_drop_fn); let import_function_env = if let (Some(clone), Some(destructor)) = (clone, destructor) { - if let Some(initializer) = initializer { - ImportFunctionEnv::new_host_env(env, clone, initializer, destructor) - } else { - ImportFunctionEnv::new_dynamic_host_env_with_no_inner_env( - env, clone, destructor, - ) + ImportFunctionEnv::Env { + env, + clone, + initializer, + destructor, } } else { - ImportFunctionEnv::new_no_env() + ImportFunctionEnv::NoEnv }; host_function_env_initializers.push(import_function_env); diff --git a/lib/vm/src/imports.rs b/lib/vm/src/imports.rs index 428f1e3be77..22bead4f730 100644 --- a/lib/vm/src/imports.rs +++ b/lib/vm/src/imports.rs @@ -17,6 +17,11 @@ pub struct Imports { /// space may affect Wasm runtime performance due to increased cache pressure. /// /// We make it optional so that we can free the data after use. + /// + /// We move this data in `get_imported_function_envs` because there's + /// no value to keeping it around; host functions must be initialized + /// exactly once so we save some memory and improve correctness by + /// moving this data. pub host_function_env_initializers: Option>, /// Resolved addresses for imported tables. diff --git a/lib/vm/src/instance/mod.rs b/lib/vm/src/instance/mod.rs index 570191113c5..f36702baf83 100644 --- a/lib/vm/src/instance/mod.rs +++ b/lib/vm/src/instance/mod.rs @@ -107,10 +107,7 @@ pub(crate) struct Instance { vmctx: VMContext, } -/// A collection of data about host envs. -/// -/// Due to invariants in the `HostEnv` variant, this type should only -/// be constructed by one of the given methods, not directly. +/// A collection of data about host envs used by imported functions. #[derive(Debug)] pub enum ImportFunctionEnv { /// The `vmctx` pointer does not refer to a host env, there is no @@ -122,69 +119,26 @@ pub enum ImportFunctionEnv { /// directly) or wrapped. i.e. in the case of Dynamic functions, we /// store our own extra data along with the user supplied env, /// thus the `env` pointer here points to the outermost type. - HostEnv { + Env { /// The function environment. This is not always the user-supplied /// env. env: *mut std::ffi::c_void, /// A clone function for duplicating the env. clone: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, - /// This type must start out as Some. `None` indicates that the - /// initializer has been called and must not be called again. + /// This field is not always present. When it is present, it + /// should be set to `None` after use to prevent double + /// initialization. initializer: Option, /// The destructor to clean up the type in `env`. destructor: fn(*mut std::ffi::c_void), }, - /// Like `Self::HostEnv` but contains no user supplied env. - /// - /// i.e. Dynamic functions without an env. - DynamicHostEnvWithNoInnerEnv { - /// Some type that does not contain a user supplied env at all, - /// thus we have no need to initialize it. - env: *mut std::ffi::c_void, - /// A clone function for duplicating the env. - clone: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, - /// The destructor to clean up the type in `env`. - destructor: fn(*mut std::ffi::c_void), - }, } impl ImportFunctionEnv { - /// Make a new `Self::HostEnv`. - pub fn new_host_env( - env: *mut std::ffi::c_void, - clone: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, - initializer: ImportInitializerFuncPtr, - destructor: fn(*mut std::ffi::c_void), - ) -> Self { - Self::HostEnv { - env, - clone, - initializer: Some(initializer), - destructor, - } - } - - /// Make a new `Self::DynamicHostEnvWithNoInnerEnv`. - pub fn new_dynamic_host_env_with_no_inner_env( - env: *mut std::ffi::c_void, - clone: fn(*mut std::ffi::c_void) -> *mut std::ffi::c_void, - destructor: fn(*mut std::ffi::c_void), - ) -> Self { - Self::DynamicHostEnvWithNoInnerEnv { - env, - clone, - destructor, - } - } - - /// Make a new `Self::NoEnv`. - pub fn new_no_env() -> Self { - Self::NoEnv - } - + /// Get the `initializer` function pointer if it exists. fn initializer(&self) -> Option { match self { - Self::HostEnv { initializer, .. } => *initializer, + Self::Env { initializer, .. } => *initializer, _ => None, } } @@ -194,32 +148,20 @@ impl Clone for ImportFunctionEnv { fn clone(&self) -> Self { match &self { Self::NoEnv => Self::NoEnv, - Self::HostEnv { + Self::Env { env, clone, destructor, initializer, } => { let new_env = (*clone)(*env); - Self::HostEnv { + Self::Env { env: new_env, clone: *clone, destructor: *destructor, initializer: *initializer, } } - Self::DynamicHostEnvWithNoInnerEnv { - env, - clone, - destructor, - } => { - let new_env = (*clone)(*env); - Self::DynamicHostEnvWithNoInnerEnv { - env: new_env, - clone: *clone, - destructor: *destructor, - } - } } } } @@ -227,10 +169,7 @@ impl Clone for ImportFunctionEnv { impl Drop for ImportFunctionEnv { fn drop(&mut self) { match self { - ImportFunctionEnv::DynamicHostEnvWithNoInnerEnv { - env, destructor, .. - } - | ImportFunctionEnv::HostEnv { + ImportFunctionEnv::Env { env, destructor, .. } => { (destructor)(*env); @@ -1396,7 +1335,7 @@ impl InstanceHandle { for import_function_env in instance_ref.imported_function_envs.values_mut() { match import_function_env { - ImportFunctionEnv::HostEnv { + ImportFunctionEnv::Env { env, ref mut initializer, .. @@ -1411,8 +1350,7 @@ impl InstanceHandle { } *initializer = None; } - ImportFunctionEnv::DynamicHostEnvWithNoInnerEnv { .. } - | ImportFunctionEnv::NoEnv => (), + ImportFunctionEnv::NoEnv => (), } } Ok(())