diff --git a/lib/api/src/module.rs b/lib/api/src/module.rs index 97a175240..ebc3856cc 100644 --- a/lib/api/src/module.rs +++ b/lib/api/src/module.rs @@ -184,7 +184,17 @@ impl Module { &self, resolver: &dyn Resolver, ) -> Result { - self.store.engine().instantiate(&self.compiled, resolver) + unsafe { + let instance_handle = self.store.engine().instantiate(&self.compiled, resolver)?; + + // After the instance handle is created, we need to initialize + // the data, call the start function and so. However, if any + // of this steps traps, we still need to keep the instance alive + // as some of the Instance elements may have placed in other + // instance tables. + self.compiled.finish_instantiation(&instance_handle)?; + Ok(instance_handle) + } } /// Returns the name of the current module. diff --git a/lib/engine-jit/src/engine.rs b/lib/engine-jit/src/engine.rs index 14dfa4015..64a3a6325 100644 --- a/lib/engine-jit/src/engine.rs +++ b/lib/engine-jit/src/engine.rs @@ -121,7 +121,7 @@ impl Engine for JITEngine { } /// Instantiates a WebAssembly module - fn instantiate( + unsafe fn instantiate( &self, compiled_module: &Arc, resolver: &dyn Resolver, diff --git a/lib/engine-jit/src/module.rs b/lib/engine-jit/src/module.rs index b39fd7e2a..581692044 100644 --- a/lib/engine-jit/src/module.rs +++ b/lib/engine-jit/src/module.rs @@ -207,17 +207,7 @@ impl CompiledModule { ) -> Result { let jit_compiler = jit.compiler(); let tunables = jit.tunables(); - let is_bulk_memory: bool = self.serializable.features.bulk_memory; let sig_registry: &SignatureRegistry = jit_compiler.signatures(); - let data_initializers = self - .serializable - .data_initializers - .iter() - .map(|init| DataInitializer { - location: init.location.clone(), - data: &*init.data, - }) - .collect::>(); let imports = resolve_imports( &self.serializable.module, &sig_registry, @@ -250,14 +240,23 @@ impl CompiledModule { finished_tables, finished_globals, imports, - &data_initializers, self.signatures.clone(), - is_bulk_memory, host_state, ) .map_err(|trap| InstantiationError::Start(RuntimeError::from_trap(trap))) } + /// Returns data initializers to pass to `InstanceHandle::initialize` + pub fn data_initializers(&self) -> Vec> { + self.serializable + .data_initializers + .iter() + .map(|init| DataInitializer { + location: init.location.clone(), + data: &*init.data, + }) + .collect::>() + } /// Register this module's stack frame information into the global scope. /// /// This is required to ensure that any traps can be properly symbolicated. @@ -280,6 +279,16 @@ unsafe impl Sync for CompiledModule {} unsafe impl Send for CompiledModule {} impl BaseCompiledModule for CompiledModule { + unsafe fn finish_instantiation( + &self, + handle: &InstanceHandle, + ) -> Result<(), InstantiationError> { + let is_bulk_memory: bool = self.serializable.features.bulk_memory; + handle + .finish_instantiation(is_bulk_memory, &self.data_initializers()) + .map_err(|trap| InstantiationError::Start(RuntimeError::from_trap(trap))) + } + fn module(&self) -> &Arc { &self.serializable.module } diff --git a/lib/engine/src/engine.rs b/lib/engine/src/engine.rs index 8bbccef4c..f1cb520e1 100644 --- a/lib/engine/src/engine.rs +++ b/lib/engine/src/engine.rs @@ -32,7 +32,7 @@ pub trait Engine { fn compile(&self, binary: &[u8]) -> Result, CompileError>; /// Instantiates a WebAssembly module - fn instantiate( + unsafe fn instantiate( &self, compiled_module: &Arc, resolver: &dyn Resolver, diff --git a/lib/engine/src/module.rs b/lib/engine/src/module.rs index 82b742c74..95720d11a 100644 --- a/lib/engine/src/module.rs +++ b/lib/engine/src/module.rs @@ -1,4 +1,6 @@ +use crate::error::InstantiationError; use std::sync::Arc; +use wasmer_runtime::InstanceHandle; use wasmer_runtime::Module; use downcast_rs::{impl_downcast, DowncastSync}; @@ -6,6 +8,16 @@ use downcast_rs::{impl_downcast, DowncastSync}; /// The `CompiledModule` trait is used by engine implementors, such /// as a JIT or Native execution. pub trait CompiledModule: DowncastSync { + /// Finish instantiation of a `InstanceHandle` + /// + /// # Unsafety + /// + /// See `InstanceHandle::finish_instantiation` + unsafe fn finish_instantiation( + &self, + handle: &InstanceHandle, + ) -> Result<(), InstantiationError>; + /// Return a reference-counting pointer to a module. fn module(&self) -> &Arc; diff --git a/lib/engine/src/resolver.rs b/lib/engine/src/resolver.rs index 05efa2b5c..32d9fc08d 100644 --- a/lib/engine/src/resolver.rs +++ b/lib/engine/src/resolver.rs @@ -3,7 +3,6 @@ use crate::error::{ImportError, LinkError}; use more_asserts::assert_ge; -use std::collections::HashSet; use wasm_common::entity::PrimaryMap; use wasm_common::{ExternType, ImportIndex, MemoryIndex, TableIndex}; use wasmer_runtime::{ @@ -95,8 +94,6 @@ pub fn resolve_imports( memory_plans: &PrimaryMap, _table_plans: &PrimaryMap, ) -> Result { - let dependencies = HashSet::new(); - let mut function_imports = PrimaryMap::with_capacity(module.num_imported_funcs); let mut table_imports = PrimaryMap::with_capacity(module.num_imported_tables); let mut memory_imports = PrimaryMap::with_capacity(module.num_imported_memories); @@ -125,16 +122,12 @@ pub fn resolve_imports( } match resolved { Export::Function(ref f) => { - // TODO: Syrus - fix this - // dependencies.insert(unsafe { InstanceHandle::from_vmctx(f.vmctx) }); function_imports.push(VMFunctionImport { body: f.address, vmctx: f.vmctx, }); } Export::Table(ref t) => { - // TODO: Syrus - fix this - // dependencies.insert(unsafe { InstanceHandle::from_vmctx(t.vmctx) }); table_imports.push(VMTableImport { definition: t.definition, from: t.from, @@ -168,16 +161,13 @@ pub fn resolve_imports( } } - // TODO: Syrus - fix this - // dependencies.insert(unsafe { InstanceHandle::from_vmctx(m.vmctx) }); memory_imports.push(VMMemoryImport { definition: m.definition, from: m.from, }); } + Export::Global(ref g) => { - // TODO: Syrus - fix this - // dependencies.insert(unsafe { InstanceHandle::from_vmctx(g.vmctx) }); global_imports.push(VMGlobalImport { definition: g.definition, }); @@ -186,7 +176,6 @@ pub fn resolve_imports( } Ok(Imports::new( - dependencies, function_imports, table_imports, memory_imports, diff --git a/lib/runtime/src/imports.rs b/lib/runtime/src/imports.rs index 9b8c100d6..ff9c3359a 100644 --- a/lib/runtime/src/imports.rs +++ b/lib/runtime/src/imports.rs @@ -1,4 +1,3 @@ -use crate::instance::InstanceHandle; use crate::vmcontext::{VMFunctionImport, VMGlobalImport, VMMemoryImport, VMTableImport}; use std::collections::HashSet; use wasm_common::entity::{BoxedSlice, PrimaryMap}; @@ -7,9 +6,6 @@ use wasm_common::{FuncIndex, GlobalIndex, MemoryIndex, TableIndex}; /// Resolved import pointers. #[derive(Clone)] pub struct Imports { - /// The set of instances that the imports depend on. - pub dependencies: HashSet, - /// Resolved addresses for imported functions. pub functions: BoxedSlice, @@ -26,14 +22,12 @@ pub struct Imports { impl Imports { /// Construct a new `Imports` instance. pub fn new( - dependencies: HashSet, function_imports: PrimaryMap, table_imports: PrimaryMap, memory_imports: PrimaryMap, global_imports: PrimaryMap, ) -> Self { Self { - dependencies, functions: function_imports.into_boxed_slice(), tables: table_imports.into_boxed_slice(), memories: memory_imports.into_boxed_slice(), @@ -44,7 +38,6 @@ impl Imports { /// Construct a new `Imports` instance with no imports. pub fn none() -> Self { Self { - dependencies: HashSet::new(), functions: PrimaryMap::new().into_boxed_slice(), tables: PrimaryMap::new().into_boxed_slice(), memories: PrimaryMap::new().into_boxed_slice(), diff --git a/lib/runtime/src/instance.rs b/lib/runtime/src/instance.rs index f430f8657..91a172ae4 100644 --- a/lib/runtime/src/instance.rs +++ b/lib/runtime/src/instance.rs @@ -35,7 +35,7 @@ cfg_if::cfg_if! { impl InstanceHandle { /// Set a custom signal handler - pub fn set_signal_handler(&mut self, handler: H) + pub fn set_signal_handler(&self, handler: H) where H: 'static + Fn(libc::c_int, *const libc::siginfo_t, *const libc::c_void) -> bool, { @@ -47,7 +47,7 @@ cfg_if::cfg_if! { impl InstanceHandle { /// Set a custom signal handler - pub fn set_signal_handler(&mut self, handler: H) + pub fn set_signal_handler(&self, handler: H) where H: 'static + Fn(winapi::um::winnt::PEXCEPTION_POINTERS) -> bool, { @@ -62,14 +62,6 @@ cfg_if::cfg_if! { /// This is repr(C) to ensure that the vmctx field is last. #[repr(C)] pub(crate) struct Instance { - /// The number of references to this `Instance`. - refcount: Cell, - - /// `Instance`s from which this `Instance` imports. These won't - /// create reference cycles because wasm instances can't cyclically - /// import from each other. - dependencies: HashSet, - /// The `Module` this `Instance` was instantiated from. module: Arc, @@ -788,9 +780,7 @@ impl InstanceHandle { finished_tables: BoxedSlice, finished_globals: BoxedSlice, imports: Imports, - data_initializers: &[DataInitializer<'_>], vmshared_signatures: BoxedSlice, - is_bulk_memory: bool, host_state: Box, ) -> Result { let vmctx_tables = finished_tables @@ -813,8 +803,6 @@ impl InstanceHandle { let handle = { let instance = Instance { - refcount: Cell::new(1), - dependencies: imports.dependencies, module, offsets, memories: finished_memories, @@ -883,29 +871,42 @@ impl InstanceHandle { VMBuiltinFunctionsArray::initialized(), ); + // Ensure that our signal handlers are ready for action. + init_traps(); + + // Perform infallible initialization in this constructor, while fallible + // initialization is deferred to the `initialize` method. + initialize_passive_elements(instance); + initialize_globals(instance); + + Ok(handle) + } + + /// Finishes the instantiation process started by `Instance::new`. + /// + /// Only safe to call immediately after instantiation. + pub unsafe fn finish_instantiation( + &self, + is_bulk_memory: bool, + data_initializers: &[DataInitializer<'_>], + ) -> Result<(), Trap> { // Check initializer bounds before initializing anything. Only do this // when bulk memory is disabled, since the bulk memory proposal changes // instantiation such that the intermediate results of failed // initializations are visible. if !is_bulk_memory { - check_table_init_bounds(instance)?; - check_memory_init_bounds(instance, data_initializers)?; + check_table_init_bounds(self.instance())?; + check_memory_init_bounds(self.instance(), data_initializers)?; } // Apply the initializers. - initialize_tables(instance)?; - initialize_passive_elements(instance); - initialize_memories(instance, data_initializers)?; - initialize_globals(instance); - - // Ensure that our signal handlers are ready for action. - init_traps(); + initialize_tables(self.instance())?; + initialize_memories(self.instance(), data_initializers)?; // The WebAssembly spec specifies that the start function is // invoked automatically at instantiation time. - instance.invoke_start_function()?; - - Ok(handle) + self.instance().invoke_start_function()?; + Ok(()) } /// Create a new `InstanceHandle` pointing at the instance @@ -916,7 +917,6 @@ impl InstanceHandle { /// be a `VMContext` allocated as part of an `Instance`. pub unsafe fn from_vmctx(vmctx: *mut VMContext) -> Self { let instance = (&mut *vmctx).instance(); - instance.refcount.set(instance.refcount.get() + 1); Self { instance: instance as *const Instance as *mut Instance, } @@ -1031,32 +1031,36 @@ impl InstanceHandle { pub(crate) fn instance(&self) -> &Instance { unsafe { &*(self.instance as *const Instance) } } + + /// Deallocates memory associated with this instance. + /// + /// Note that this is unsafe because there might be other handles to this + /// `InstanceHandle` elsewhere, and there's nothing preventing usage of + /// this handle after this function is called. + pub unsafe fn dealloc(&self) { + let instance = self.instance(); + let layout = instance.alloc_layout(); + ptr::drop_in_place(self.instance); + alloc::dealloc(self.instance.cast(), layout); + } } impl Clone for InstanceHandle { fn clone(&self) -> Self { - let instance = self.instance(); - instance.refcount.set(instance.refcount.get() + 1); Self { instance: self.instance, } } } -impl Drop for InstanceHandle { - fn drop(&mut self) { - let instance = self.instance(); - let count = instance.refcount.get(); - instance.refcount.set(count - 1); - if count == 1 { - let layout = instance.alloc_layout(); - unsafe { - ptr::drop_in_place(self.instance); - alloc::dealloc(self.instance.cast(), layout); - } - } - } -} +// TODO: uncomment this, as we need to store the handles +// in the store, and once the store is dropped, then the instances +// will too. +// impl Drop for InstanceHandle { +// fn drop(&mut self) { +// unsafe { self.dealloc() } +// } +// } fn check_table_init_bounds(instance: &Instance) -> Result<(), Trap> { let module = Arc::clone(&instance.module);