diff --git a/CHANGELOG.md b/CHANGELOG.md index 71c945933bf..56e5d3db974 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### Added +- [#1837](https://github.com/wasmerio/wasmer/pull/1837) It is now possible to use exports of an `Intance` even after the `Instance` has been freed - [#1831](https://github.com/wasmerio/wasmer/pull/1831) Added support for Apple Silicon chips (`arm64-apple-darwin`) - [#1649](https://github.com/wasmerio/wasmer/pull/1649) Add outline of migration to 1.0.0 docs. diff --git a/lib/api/src/externals/function.rs b/lib/api/src/externals/function.rs index b65462e3af3..def361e1078 100644 --- a/lib/api/src/externals/function.rs +++ b/lib/api/src/externals/function.rs @@ -100,6 +100,7 @@ impl Function { vmctx, signature: ty.clone(), call_trampoline: None, + instance_allocator: None, }, } } @@ -152,6 +153,7 @@ impl Function { vmctx, signature: ty.clone(), call_trampoline: None, + instance_allocator: None, }, } } @@ -196,6 +198,7 @@ impl Function { signature, kind: VMFunctionKind::Static, call_trampoline: None, + instance_allocator: None, }, } } @@ -252,6 +255,7 @@ impl Function { vmctx, signature, call_trampoline: None, + instance_allocator: None, }, } } @@ -293,6 +297,7 @@ impl Function { vmctx, signature, call_trampoline: None, + instance_allocator: None, }, } } @@ -474,7 +479,7 @@ impl Function { Ok(results.into_boxed_slice()) } - pub(crate) fn from_export(store: &Store, wasmer_export: ExportFunction) -> Self { + pub(crate) fn from_vm_export(store: &Store, wasmer_export: ExportFunction) -> Self { if let Some(trampoline) = wasmer_export.call_trampoline { Self { store: store.clone(), diff --git a/lib/api/src/externals/global.rs b/lib/api/src/externals/global.rs index a306b9a94e5..add92598ce3 100644 --- a/lib/api/src/externals/global.rs +++ b/lib/api/src/externals/global.rs @@ -180,7 +180,7 @@ impl Global { Ok(()) } - pub(crate) fn from_export(store: &Store, wasmer_export: ExportGlobal) -> Self { + pub(crate) fn from_vm_export(store: &Store, wasmer_export: ExportGlobal) -> Self { Self { store: store.clone(), global: wasmer_export.from, @@ -218,6 +218,7 @@ impl<'a> Exportable<'a> for Global { fn to_export(&self) -> Export { ExportGlobal { from: self.global.clone(), + instance_allocator: None, } .into() } diff --git a/lib/api/src/externals/memory.rs b/lib/api/src/externals/memory.rs index 32583c82fef..f03583d30c3 100644 --- a/lib/api/src/externals/memory.rs +++ b/lib/api/src/externals/memory.rs @@ -220,7 +220,7 @@ impl Memory { unsafe { MemoryView::new(base as _, length as u32) } } - pub(crate) fn from_export(store: &Store, wasmer_export: ExportMemory) -> Self { + pub(crate) fn from_vm_export(store: &Store, wasmer_export: ExportMemory) -> Self { Self { store: store.clone(), memory: wasmer_export.from, @@ -248,6 +248,7 @@ impl<'a> Exportable<'a> for Memory { fn to_export(&self) -> Export { ExportMemory { from: self.memory.clone(), + instance_allocator: None, } .into() } diff --git a/lib/api/src/externals/mod.rs b/lib/api/src/externals/mod.rs index c9521870059..6f49228a56b 100644 --- a/lib/api/src/externals/mod.rs +++ b/lib/api/src/externals/mod.rs @@ -46,13 +46,13 @@ impl Extern { } } - /// Create an `Extern` from an `Export`. - pub fn from_export(store: &Store, export: Export) -> Self { + /// Create an `Extern` from an `wasmer_vm::Export`. + pub fn from_vm_export(store: &Store, export: Export) -> Self { match export { - Export::Function(f) => Self::Function(Function::from_export(store, f)), - Export::Memory(m) => Self::Memory(Memory::from_export(store, m)), - Export::Global(g) => Self::Global(Global::from_export(store, g)), - Export::Table(t) => Self::Table(Table::from_export(store, t)), + Export::Function(f) => Self::Function(Function::from_vm_export(store, f)), + Export::Memory(m) => Self::Memory(Memory::from_vm_export(store, m)), + Export::Global(g) => Self::Global(Global::from_vm_export(store, g)), + Export::Table(t) => Self::Table(Table::from_vm_export(store, t)), } } } diff --git a/lib/api/src/externals/table.rs b/lib/api/src/externals/table.rs index 4bcbf257584..27725d8e004 100644 --- a/lib/api/src/externals/table.rs +++ b/lib/api/src/externals/table.rs @@ -139,7 +139,7 @@ impl Table { Ok(()) } - pub(crate) fn from_export(store: &Store, wasmer_export: ExportTable) -> Self { + pub(crate) fn from_vm_export(store: &Store, wasmer_export: ExportTable) -> Self { Self { store: store.clone(), table: wasmer_export.from, @@ -156,6 +156,7 @@ impl<'a> Exportable<'a> for Table { fn to_export(&self) -> Export { ExportTable { from: self.table.clone(), + instance_allocator: None, } .into() } diff --git a/lib/api/src/instance.rs b/lib/api/src/instance.rs index 50b67739d6d..21986eea8e1 100644 --- a/lib/api/src/instance.rs +++ b/lib/api/src/instance.rs @@ -80,7 +80,7 @@ impl Instance { .map(|export| { let name = export.name().to_string(); let export = handle.lookup(&name).expect("export"); - let extern_ = Extern::from_export(store, export); + let extern_ = Extern::from_vm_export(store, export); (name, extern_) }) .collect::(); diff --git a/lib/api/src/native.rs b/lib/api/src/native.rs index d1c6e78fd51..a119435020f 100644 --- a/lib/api/src/native.rs +++ b/lib/api/src/native.rs @@ -70,6 +70,7 @@ where signature, kind: other.arg_kind, call_trampoline: None, + instance_allocator: None, } } } @@ -90,6 +91,7 @@ where 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 80924c33cbd..2e3a378fcd9 100644 --- a/lib/api/src/types.rs +++ b/lib/api/src/types.rs @@ -81,8 +81,9 @@ impl ValFuncRef for Val { kind: wasmer_vm::VMFunctionKind::Static, vmctx: item.vmctx, call_trampoline: None, + instance_allocator: None, }; - let f = Function::from_export(store, export); + let f = Function::from_vm_export(store, export); Self::FuncRef(f) } } diff --git a/lib/api/tests/instance.rs b/lib/api/tests/instance.rs new file mode 100644 index 00000000000..69733877424 --- /dev/null +++ b/lib/api/tests/instance.rs @@ -0,0 +1,39 @@ +use anyhow::Result; +use wasmer::*; + +#[test] +fn exports_work_after_multiple_instances_have_been_freed() -> Result<()> { + let store = Store::default(); + let module = Module::new( + &store, + " + (module + (type $sum_t (func (param i32 i32) (result i32))) + (func $sum_f (type $sum_t) (param $x i32) (param $y i32) (result i32) + local.get $x + local.get $y + i32.add) + (export \"sum\" (func $sum_f))) +", + )?; + + let import_object = ImportObject::new(); + let instance = Instance::new(&module, &import_object)?; + let instance2 = instance.clone(); + let instance3 = instance.clone(); + + // The function is cloned to “break” the connection with `instance`. + let sum = instance.exports.get_function("sum")?.clone(); + + drop(instance); + drop(instance2); + drop(instance3); + + // All instances have been dropped, but `sum` continues to work! + assert_eq!( + sum.call(&[Value::I32(1), Value::I32(2)])?.into_vec(), + vec![Value::I32(3)], + ); + + Ok(()) +} diff --git a/lib/c-api/src/wasm_c_api/wasi/mod.rs b/lib/c-api/src/wasm_c_api/wasi/mod.rs index 68516c66a12..13823b4568c 100644 --- a/lib/c-api/src/wasm_c_api/wasi/mod.rs +++ b/lib/c-api/src/wasm_c_api/wasi/mod.rs @@ -335,7 +335,7 @@ unsafe fn wasi_get_imports_inner( import_type.name() ), })); - let inner = Extern::from_export(store, export); + let inner = Extern::from_vm_export(store, export); Some(Box::new(wasm_extern_t { instance: None, diff --git a/lib/deprecated/runtime-core/src/module.rs b/lib/deprecated/runtime-core/src/module.rs index 7ef483dcb85..b43166dd346 100644 --- a/lib/deprecated/runtime-core/src/module.rs +++ b/lib/deprecated/runtime-core/src/module.rs @@ -159,12 +159,12 @@ impl Module { ( (namespace, name), - new::wasmer::Extern::from_export(store, Export::Function(function)), + new::wasmer::Extern::from_vm_export(store, Export::Function(function)), ) } export => ( (namespace, name), - new::wasmer::Extern::from_export(store, export), + new::wasmer::Extern::from_vm_export(store, export), ), }) .for_each(|((namespace, name), extern_)| { diff --git a/lib/vm/src/export.rs b/lib/vm/src/export.rs index 86b09befbb5..9a4532d8402 100644 --- a/lib/vm/src/export.rs +++ b/lib/vm/src/export.rs @@ -2,6 +2,7 @@ // Attributions: https://github.com/wasmerio/wasmer/blob/master/ATTRIBUTIONS.md use crate::global::Global; +use crate::instance::InstanceAllocator; use crate::memory::{Memory, MemoryStyle}; use crate::table::{Table, TableStyle}; use crate::vmcontext::{VMFunctionBody, VMFunctionEnvironment, VMFunctionKind, VMTrampoline}; @@ -29,15 +30,27 @@ pub enum Export { pub struct ExportFunction { /// The address of the native-code function. pub address: *const VMFunctionBody, + /// Pointer to the containing `VMContext`. pub vmctx: VMFunctionEnvironment, + /// The function type, used for compatibility checking. pub signature: FunctionType, - /// The function kind (specifies the calling convention for the function). + + /// The function kind (specifies the calling convention for the + /// function). pub kind: VMFunctionKind, - /// Address of the function call trampoline owned by the same VMContext that owns the VMFunctionBody. - /// May be None when the function is a host-function (FunctionType == Dynamic or vmctx == nullptr). + + /// Address of the function call trampoline owned by the same + /// VMContext that owns the VMFunctionBody. + /// + /// May be `None` when the function is a host function (`FunctionType` + /// == `Dynamic` or `vmctx` == `nullptr`). pub call_trampoline: Option, + + /// A “reference” to the instance through the + /// `InstanceAllocator`. `None` if it is a host function. + pub instance_allocator: Option, } /// # Safety @@ -59,6 +72,10 @@ impl From for Export { pub struct ExportTable { /// Pointer to the containing `Table`. pub from: Arc, + + /// A “reference” to the instance through the + /// `InstanceAllocator`. `None` if it is a host function. + pub instance_allocator: Option, } /// # Safety @@ -66,6 +83,7 @@ pub struct ExportTable { /// correct use of the raw table from multiple threads via `definition` requires `unsafe` /// and is the responsibilty of the user of this type. unsafe impl Send for ExportTable {} + /// # Safety /// This is correct because the values directly in `definition` should be considered immutable /// and the type is both `Send` and `Clone` (thus marking it `Sync` adds no new behavior, it @@ -100,6 +118,10 @@ impl From for Export { pub struct ExportMemory { /// Pointer to the containing `Memory`. pub from: Arc, + + /// A “reference” to the instance through the + /// `InstanceAllocator`. `None` if it is a host function. + pub instance_allocator: Option, } /// # Safety @@ -107,6 +129,7 @@ pub struct ExportMemory { /// correct use of the raw memory from multiple threads via `definition` requires `unsafe` /// and is the responsibilty of the user of this type. unsafe impl Send for ExportMemory {} + /// # Safety /// This is correct because the values directly in `definition` should be considered immutable /// and the type is both `Send` and `Clone` (thus marking it `Sync` adds no new behavior, it @@ -141,6 +164,10 @@ impl From for Export { pub struct ExportGlobal { /// The global declaration, used for compatibility checking. pub from: Arc, + + /// A “reference” to the instance through the + /// `InstanceAllocator`. `None` if it is a host function. + pub instance_allocator: Option, } /// # Safety @@ -148,6 +175,7 @@ pub struct ExportGlobal { /// correct use of the raw global from multiple threads via `definition` requires `unsafe` /// and is the responsibilty of the user of this type. unsafe impl Send for ExportGlobal {} + /// # Safety /// This is correct because the values directly in `definition` should be considered immutable /// from the perspective of users of this type and the type is both `Send` and `Clone` (thus diff --git a/lib/vm/src/instance.rs b/lib/vm/src/instance.rs index f01fd7904ac..15f024bc889 100644 --- a/lib/vm/src/instance.rs +++ b/lib/vm/src/instance.rs @@ -1,9 +1,12 @@ // This file contains code from external sources. // Attributions: https://github.com/wasmerio/wasmer/blob/master/ATTRIBUTIONS.md -//! An `Instance` contains all the runtime state used by execution of a -//! wasm module (except its callstack and register state). An -//! `InstanceHandle` is a reference-counting handle for an `Instance`. +//! An `Instance` contains all the runtime state used by execution of +//! a WebAssembly module (except its callstack and register state). An +//! `InstanceAllocator` is a wrapper around `Instance` that manages +//! how it is allocated and deallocated. An `InstanceHandle` is a +//! wrapper around an `InstanceAllocator`. + use crate::export::Export; use crate::global::Global; use crate::imports::Imports; @@ -25,8 +28,9 @@ use std::any::Any; use std::cell::{Cell, RefCell}; use std::collections::HashMap; use std::convert::{TryFrom, TryInto}; +use std::fmt; use std::ptr::NonNull; -use std::sync::Arc; +use std::sync::{atomic, Arc}; use std::{mem, ptr, slice}; use wasmer_types::entity::{packed_option::ReservedValue, BoxedSlice, EntityRef, PrimaryMap}; use wasmer_types::{ @@ -35,37 +39,12 @@ use wasmer_types::{ SignatureIndex, TableIndex, TableInitializer, }; -cfg_if::cfg_if! { - if #[cfg(unix)] { - pub type SignalHandler = dyn Fn(libc::c_int, *const libc::siginfo_t, *const libc::c_void) -> bool; - - impl InstanceHandle { - /// Set a custom signal handler - pub fn set_signal_handler(&self, handler: H) - where - H: 'static + Fn(libc::c_int, *const libc::siginfo_t, *const libc::c_void) -> bool, - { - self.instance().signal_handler.set(Some(Box::new(handler))); - } - } - } else if #[cfg(target_os = "windows")] { - pub type SignalHandler = dyn Fn(winapi::um::winnt::PEXCEPTION_POINTERS) -> bool; - - impl InstanceHandle { - /// Set a custom signal handler - pub fn set_signal_handler(&self, handler: H) - where - H: 'static + Fn(winapi::um::winnt::PEXCEPTION_POINTERS) -> bool, - { - self.instance().signal_handler.set(Some(Box::new(handler))); - } - } - } -} - /// A WebAssembly instance. /// -/// This is repr(C) to ensure that the vmctx field is last. +/// The type is dynamically-sized. Indeed, the `vmctx` field can +/// contain various data. That's why the type has a C representation +/// to ensure that the `vmctx` field is last. See the documentation of +/// the `vmctx` field to learn more. #[repr(C)] pub(crate) struct Instance { /// The `ModuleInfo` this `Instance` was instantiated from. @@ -104,12 +83,19 @@ pub(crate) struct Instance { /// Handler run when `SIGBUS`, `SIGFPE`, `SIGILL`, or `SIGSEGV` are caught by the instance thread. pub(crate) signal_handler: Cell>>, - /// Additional context used by compiled wasm code. This field is last, and - /// represents a dynamically-sized array that extends beyond the nominal - /// end of the struct (similar to a flexible array member). + /// Additional context used by compiled WebAssembly code. This + /// field is last, and represents a dynamically-sized array that + /// extends beyond the nominal end of the struct (similar to a + /// flexible array member). vmctx: VMContext, } +impl fmt::Debug for Instance { + fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.debug_struct("Instance").finish() + } +} + #[allow(clippy::cast_ptr_alignment)] impl Instance { /// Helper function to access various locations offset from our `*mut @@ -126,11 +112,11 @@ impl Instance { unsafe { *self.signature_ids_ptr().add(index) } } - pub(crate) fn module(&self) -> &Arc { + fn module(&self) -> &Arc { &self.module } - pub(crate) fn module_ref(&self) -> &ModuleInfo { + fn module_ref(&self) -> &ModuleInfo { &*self.module } @@ -209,7 +195,7 @@ impl Instance { } /// Get a locally defined or imported memory. - pub(crate) fn get_memory(&self, index: MemoryIndex) -> VMMemoryDefinition { + fn get_memory(&self, index: MemoryIndex) -> VMMemoryDefinition { if let Some(local_index) = self.module.local_memory_index(index) { self.memory(local_index) } else { @@ -273,95 +259,15 @@ impl Instance { } /// Return a reference to the vmctx used by compiled wasm code. - pub fn vmctx(&self) -> &VMContext { + fn vmctx(&self) -> &VMContext { &self.vmctx } /// Return a raw pointer to the vmctx used by compiled wasm code. - pub fn vmctx_ptr(&self) -> *mut VMContext { + fn vmctx_ptr(&self) -> *mut VMContext { self.vmctx() as *const VMContext as *mut VMContext } - /// Lookup an export with the given name. - pub fn lookup(&self, field: &str) -> Option { - let export = self.module.exports.get(field)?; - - Some(self.lookup_by_declaration(&export)) - } - - /// Lookup an export with the given export declaration. - pub fn lookup_by_declaration(&self, export: &ExportIndex) -> Export { - match export { - ExportIndex::Function(index) => { - let sig_index = &self.module.functions[*index]; - let (address, vmctx) = if let Some(def_index) = self.module.local_func_index(*index) - { - ( - self.functions[def_index].0 as *const _, - VMFunctionEnvironment { - vmctx: self.vmctx_ptr(), - }, - ) - } else { - let import = self.imported_function(*index); - (import.body, import.environment) - }; - let call_trampoline = Some(self.function_call_trampolines[*sig_index]); - let signature = self.module.signatures[*sig_index].clone(); - ExportFunction { - address, - // Any function received is already static at this point as: - // 1. All locally defined functions in the Wasm have a static signature. - // 2. All the imported functions are already static (because - // they point to the trampolines rather than the dynamic addresses). - kind: VMFunctionKind::Static, - signature, - vmctx, - call_trampoline, - } - .into() - } - ExportIndex::Table(index) => { - let from = if let Some(def_index) = self.module.local_table_index(*index) { - self.tables[def_index].clone() - } else { - let import = self.imported_table(*index); - import.from.clone() - }; - ExportTable { from }.into() - } - ExportIndex::Memory(index) => { - let from = if let Some(def_index) = self.module.local_memory_index(*index) { - self.memories[def_index].clone() - } else { - let import = self.imported_memory(*index); - import.from.clone() - }; - ExportMemory { from }.into() - } - ExportIndex::Global(index) => { - let from = { - if let Some(def_index) = self.module.local_global_index(*index) { - self.globals[def_index].clone() - } else { - let import = self.imported_global(*index); - import.from.clone() - } - }; - ExportGlobal { from }.into() - } - } - } - - /// Return an iterator over the exports of this instance. - /// - /// Specifically, it provides access to the key-value pairs, where the keys - /// are export names, and the values are export declarations which can be - /// resolved `lookup_by_declaration`. - pub fn exports(&self) -> indexmap::map::Iter { - self.module.exports.iter() - } - /// Return a reference to the custom state attached to this instance. #[inline] pub fn host_state(&self) -> &dyn Any { @@ -406,7 +312,7 @@ impl Instance { } } - /// Return the offset from the vmctx pointer to its containing Instance. + /// Return the offset from the vmctx pointer to its containing `Instance`. #[inline] pub(crate) fn vmctx_offset() -> isize { offset_of!(Self, vmctx) as isize @@ -511,7 +417,7 @@ impl Instance { result } - // Get table element by index. + /// Get table element by index. fn table_get( &self, table_index: LocalTableIndex, @@ -523,6 +429,7 @@ impl Instance { .get(index) } + /// Set table element by index. fn table_set( &self, table_index: LocalTableIndex, @@ -535,14 +442,6 @@ impl Instance { .set(index, val) } - fn alloc_layout(offsets: &VMOffsets) -> Layout { - let size = mem::size_of::() - .checked_add(usize::try_from(offsets.size_of_vmctx()).unwrap()) - .unwrap(); - let align = mem::align_of::(); - Layout::from_size_align(size, align).unwrap() - } - /// Get a `VMCallerCheckedAnyfunc` for the given `FunctionIndex`. fn get_caller_checked_anyfunc(&self, index: FunctionIndex) -> VMCallerCheckedAnyfunc { if index == FunctionIndex::reserved_value() { @@ -563,6 +462,7 @@ impl Instance { let import = self.imported_function(index); (import.body, import.environment) }; + VMCallerCheckedAnyfunc { func_ptr, type_index, @@ -761,110 +661,285 @@ impl Instance { } } -/// A handle holding an `Instance` of a WebAssembly module. -#[derive(Hash, PartialEq, Eq)] -pub struct InstanceHandle { - instance: *mut Instance, +/// An `InstanceAllocator` is responsible to allocate, to deallocate, +/// and to give access to an `Instance`, in such a way that `Instance` +/// is unique, can be shared, safely, across threads, without +/// duplicating the pointer in multiple locations. `InstanceAllocator` +/// must be the only “owner” of an `Instance`. +/// +/// Consequently, one must not share `Instance` but +/// `InstanceAllocator`. It acts like an Atomically Reference Counted +/// to `Instance`. In short, `InstanceAllocator` is roughly a +/// simplified version of `std::sync::Arc`. +/// +/// It is important to remind that `Instance` is dynamically-sized +/// based on `VMOffsets`: The `Instance.vmctx` field represents a +/// dynamically-sized array that extends beyond the nominal end of the +/// type. So in order to create an instance of it, we must: +/// +/// 1. Define the correct layout for `Instance` (size and alignment), +/// 2. Allocate it properly. +/// +/// The `InstanceAllocator::instance_layout` computes the correct +/// layout to represent the wanted `Instance`. +/// +/// Then `InstanceAllocator::allocate_instance` will use this layout +/// to allocate an empty `Instance` properly. This allocation must be +/// freed with `InstanceAllocator::deallocate_instance` if and only if +/// it has been set correctly. The `Drop` implementation of +/// `InstanceAllocator` calls its `deallocate_instance` method without +/// checking if this property holds, only when `Self.strong` is equal +/// to 1. +/// +/// Note for the curious reader: `InstanceHandle::allocate_instance` +/// and `InstanceHandle::new` will respectively allocate a proper +/// `Instance` and will fill it correctly. +/// +/// A little bit of background: The initial goal was to be able to +/// shared an `Instance` between an `InstanceHandle` and the module +/// exports, so that one can drop a `InstanceHandle` but still being +/// able to use the exports properly. +/// +/// This structure has a C representation because `Instance` is +/// dynamically-sized, and the `instance` field must be last. +#[derive(Debug)] +#[repr(C)] +pub struct InstanceAllocator { + /// Number of `Self` in the nature. It increases when `Self` is + /// cloned, and it decreases when `Self` is dropped. + strong: Arc, + + /// The layout of `Instance` (which can vary). + instance_layout: Layout, - /// Whether `Self` owns `self.instance`. It's not always the case; - /// e.g. when `Self` is built with `Self::from_vmctx`. + /// The `Instance` itself. It must be the last field of + /// `InstanceAllocator` since `Instance` is dyamically-sized. /// - /// This information is necessary to know whether `Self` should - /// deallocate `self.instance`. - owned_instance: bool, + /// `Instance` must not be dropped manually by Rust, because it's + /// allocated manually with `alloc` and a specific layout (Rust + /// would be able to drop `Instance` itself but it will imply a + /// memory leak because of `alloc`). + /// + /// No one in the code has a copy of the `Instance`'s + /// pointer. `Self` is the only one. + instance: NonNull, } -/// # Safety -/// This is safe because there is no thread-specific logic in `InstanceHandle`. -/// TODO: this needs extra review -unsafe impl Send for InstanceHandle {} - -impl InstanceHandle { - /// Allocates an instance for use with `InstanceHandle::new`. +impl InstanceAllocator { + /// A soft limit on the amount of references that may be made to an `InstanceAllocator`. /// - /// Returns the instance pointer and the [`VMOffsets`] that describe the - /// memory buffer pointed to by the instance pointer. - pub fn allocate_instance(module: &ModuleInfo) -> (NonNull, VMOffsets) { - let offsets = VMOffsets::new(mem::size_of::<*const u8>() as u8, module); + /// Going above this limit will make the program to panic at exactly + /// `MAX_REFCOUNT` references. + const MAX_REFCOUNT: usize = std::usize::MAX - 1; + + /// Create a new `InstanceAllocator`. It allocates nothing. It + /// fills nothing. The `Instance` must be already valid and + /// filled. `self_ptr` and `self_layout` must be the pointer and + /// the layout returned by `Self::allocate_self` used to build + /// `Self`. + fn new(instance: NonNull, instance_layout: Layout) -> Self { + Self { + strong: Arc::new(atomic::AtomicUsize::new(1)), + instance_layout, + instance, + } + } + + /// Calculate the appropriate layout for `Instance`. + fn instance_layout(offsets: &VMOffsets) -> Layout { + let vmctx_size = usize::try_from(offsets.size_of_vmctx()) + .expect("Failed to convert the size of `vmctx` to a `usize`"); + + let instance_vmctx_layout = + Layout::array::(vmctx_size).expect("Failed to create a layout for `VMContext`"); - let layout = Instance::alloc_layout(&offsets); + let (instance_layout, _offset) = Layout::new::() + .extend(instance_vmctx_layout) + .expect("Failed to extend to `Instance` layout to include `VMContext`"); + + instance_layout.pad_to_align() + } + + /// Allocate `Instance` (it is an uninitialized pointer). + /// + /// `offsets` is used to compute the layout with `Self::instance_layout`. + fn allocate_instance(offsets: &VMOffsets) -> (NonNull, Layout) { + let layout = Self::instance_layout(offsets); #[allow(clippy::cast_ptr_alignment)] let instance_ptr = unsafe { alloc::alloc(layout) as *mut Instance }; + let ptr = if let Some(ptr) = NonNull::new(instance_ptr) { - ptr.cast() + ptr } else { alloc::handle_alloc_error(layout); }; - (ptr, offsets) + (ptr, layout) } - /// Get the locations of where the local `VMMemoryDefinition`s should be stored. - /// - /// This function lets us create `Memory` objects on the host with backing - /// memory in the VM. + /// Deallocate `Instance`. /// /// # Safety - /// - `instance_ptr` must point to enough memory that all of the offsets in - /// `offsets` point to valid locations in memory. - pub unsafe fn memory_definition_locations( - instance_ptr: NonNull, - offsets: &VMOffsets, - ) -> Vec> { - let num_memories = offsets.num_local_memories; - let num_memories = usize::try_from(num_memories).unwrap(); - let mut out = Vec::with_capacity(num_memories); - // TODO: better encapsulate this logic, this shouldn't be duplicated - let base_ptr = instance_ptr.as_ptr().add(std::mem::size_of::()); - for i in 0..num_memories { - let mem_offset = offsets.vmctx_vmmemory_definition(LocalMemoryIndex::new(i)); - let mem_offset = usize::try_from(mem_offset).unwrap(); + /// + /// `Self.instance` must be correctly set and filled before being + /// dropped and deallocated. + unsafe fn deallocate_instance(&mut self) { + let instance_ptr = self.instance.as_ptr(); - let new_ptr = NonNull::new_unchecked(base_ptr.add(mem_offset)); + ptr::drop_in_place(instance_ptr); + std::alloc::dealloc(instance_ptr as *mut u8, self.instance_layout); + } - out.push(new_ptr.cast()); - } - out + /// Get the number of strong references pointing to this + /// `InstanceAllocator`. + pub fn strong_count(&self) -> usize { + self.strong.load(atomic::Ordering::SeqCst) } - /// Get the locations of where the `VMTableDefinition`s should be stored. - /// - /// This function lets us create `Table` objects on the host with backing - /// memory in the VM. + /// Get a reference to the `Instance`. + #[inline] + pub(crate) fn as_ref<'a>(&'a self) -> &'a Instance { + // SAFETY: The pointer is properly aligned, it is + // “dereferencable”, it points to an initialized memory of + // `Instance`, and the reference has the lifetime `'a`. + unsafe { self.instance.as_ref() } + } +} + +/// TODO: Review this super carefully. +unsafe impl Send for InstanceAllocator {} +unsafe impl Sync for InstanceAllocator {} + +impl Clone for InstanceAllocator { + /// Makes a clone of `InstanceAllocator`. /// - /// # Safety - /// - `instance_ptr` must point to enough memory that all of the offsets in - /// `offsets` point to valid locations in memory. - pub unsafe fn table_definition_locations( - instance_ptr: NonNull, - offsets: &VMOffsets, - ) -> Vec> { - let num_tables = offsets.num_local_tables; - let num_tables = usize::try_from(num_tables).unwrap(); - let mut out = Vec::with_capacity(num_tables); - // TODO: better encapsulate this logic, this shouldn't be duplicated - let base_ptr = instance_ptr.as_ptr().add(std::mem::size_of::()); - for i in 0..num_tables { - let table_offset = offsets.vmctx_vmtable_definition(LocalTableIndex::new(i)); - let table_offset = usize::try_from(table_offset).unwrap(); + /// This creates another `InstanceAllocator` using the same + /// `instance` pointer, increasing the strong reference count. + #[inline] + fn clone(&self) -> Self { + // Using a relaxed ordering is alright here, as knowledge of + // the original reference prevents other threads from + // erroneously deleting the object. + // + // As explained in the [Boost documentation][1]: + // + // > Increasing the reference counter can always be done with + // > `memory_order_relaxed`: New references to an object can + // > only be formed from an existing reference, and passing an + // > existing reference from one thread to another must already + // > provide any required synchronization. + // + // [1]: https://www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html + let old_size = self.strong.fetch_add(1, atomic::Ordering::Relaxed); + + // However we need to guard against massive refcounts in case + // someone is `mem::forget`ing `InstanceAllocator`. If we + // don't do this the count can overflow and users will + // use-after free. We racily saturate to `isize::MAX` on the + // assumption that there aren't ~2 billion threads + // incrementing the reference count at once. This branch will + // never be taken in any realistic program. + // + // We abort because such a program is incredibly degenerate, + // and we don't care to support it. + + if old_size > Self::MAX_REFCOUNT { + panic!("Too many references of `InstanceAllocator`"); + } - let new_ptr = NonNull::new_unchecked(base_ptr.add(table_offset)); + Self { + strong: self.strong.clone(), + instance_layout: self.instance_layout, + instance: self.instance.clone(), + } + } +} - out.push(new_ptr.cast()); +impl PartialEq for InstanceAllocator { + /// Two `InstanceAllocator` are equal if and only if + /// `Self.instance` points to the same location. + fn eq(&self, other: &Self) -> bool { + self.instance == other.instance + } +} + +impl Drop for InstanceAllocator { + /// Drop the `InstanceAllocator`. + /// + /// This will decrement the strong reference count. If it reaches + /// 1, then the `Self.instance` will be deallocated with + /// `Self::deallocate_instance`. + fn drop(&mut self) { + // Because `fetch_sub` is already atomic, we do not need to + // synchronize with other thread. + if self.strong.fetch_sub(1, atomic::Ordering::Release) != 1 { + return; } - out + + // This fence is needed to prevent reordering of use of the data and + // deletion of the data. Because it is marked `Release`, the decreasing + // of the reference count synchronizes with this `Acquire` fence. This + // means that use of the data happens before decreasing the reference + // count, which happens before this fence, which happens before the + // deletion of the data. + // + // As explained in the [Boost documentation][1]: + // + // > It is important to enforce any possible access to the object in one + // > thread (through an existing reference) to *happen before* deleting + // > the object in a different thread. This is achieved by a "release" + // > operation after dropping a reference (any access to the object + // > through this reference must obviously happened before), and an + // > "acquire" operation before deleting the object. + // + // [1]: https://www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html + atomic::fence(atomic::Ordering::Acquire); + + // Now we can deallocate the instance. Note that we don't + // check the pointer to `Instance` is correctly initialized, + // but the way `InstanceHandle` creates the + // `InstanceAllocator` ensures that. + unsafe { Self::deallocate_instance(self) }; + } +} + +/// A handle holding an `InstanceAllocator`, which holds an `Instance` +/// of a WebAssembly module. +/// +/// This is more or less a public facade of the private `Instance`, +/// providing useful higher-level API. +#[derive(Debug, PartialEq)] +pub struct InstanceHandle { + /// The `InstanceAllocator`. See its documentation to learn more. + instance: InstanceAllocator, +} + +impl InstanceHandle { + /// Allocates an instance for use with `InstanceHandle::new`. + /// + /// Returns the instance pointer and the [`VMOffsets`] that describe the + /// memory buffer pointed to by the instance pointer. + /// + /// It should ideally return `NonNull` rather than + /// `NonNull`, however `Instance` is private, and we want to + /// keep it private. + pub fn allocate_instance(module: &ModuleInfo) -> (NonNull, VMOffsets) { + let offsets = VMOffsets::new(mem::size_of::<*const u8>() as u8, module); + let (instance_ptr, _instance_layout) = InstanceAllocator::allocate_instance(&offsets); + + (instance_ptr.cast(), offsets) } - /// Create a new `InstanceHandle` pointing at a new `Instance`. + /// Create a new `InstanceHandle` pointing at a new `InstanceAllocator`. /// /// # Safety /// /// This method is not necessarily inherently unsafe to call, but in general /// the APIs of an `Instance` are quite unsafe and have not been really /// audited for safety that much. As a result the unsafety here on this - /// method is a low-overhead way of saying "this is an extremely unsafe type - /// to work with". + /// method is a low-overhead way of saying “this is an extremely unsafe type + /// to work with”. /// /// Extreme care must be taken when working with `InstanceHandle` and it's /// recommended to have relatively intimate knowledge of how it works @@ -875,8 +950,7 @@ impl InstanceHandle { /// However the following must be taken care of before calling this function: /// - `instance_ptr` must point to valid memory sufficiently large /// for the `Instance`. `instance_ptr` will be owned by - /// `InstanceHandle`, see `InstanceHandle::owned_instance` to - /// learn more. + /// `InstanceAllocator`, see `InstanceAllocator` to learn more. /// - The memory at `instance.tables_ptr()` must be initialized with data for /// all the local tables. /// - The memory at `instance.memories_ptr()` must be initialized with data for @@ -895,17 +969,20 @@ impl InstanceHandle { vmshared_signatures: BoxedSlice, host_state: Box, ) -> Result { - let instance_ptr = instance_ptr.cast::().as_ptr(); + // `NonNull` here actually means `NonNull`. See + // `Self::allocate_instance` to understand why. + let instance_ptr: NonNull = instance_ptr.cast(); let vmctx_globals = finished_globals .values() .map(|m| m.vmglobal()) .collect::>() .into_boxed_slice(); - let passive_data = RefCell::new(module.passive_data.clone()); let handle = { + let instance_layout = InstanceAllocator::instance_layout(&offsets); + // Create the `Instance`. The unique, the One. let instance = Instance { module, offsets, @@ -920,14 +997,24 @@ impl InstanceHandle { signal_handler: Cell::new(None), vmctx: VMContext {}, }; - ptr::write(instance_ptr, instance); + + // `instance` is moved at `instance_ptr`. This pointer has + // been allocated by `Self::allocate_instance` (so by + // `InstanceAllocator::allocate_instance`. + ptr::write(instance_ptr.as_ptr(), instance); + + // Now `instance_ptr` is correctly initialized! + + // `instance_ptr` is passed to `InstanceAllocator`, which + // makes it the only “owner” (it doesn't own the value, + // it's just the semantics we define). + let instance_allocator = InstanceAllocator::new(instance_ptr, instance_layout); Self { - instance: instance_ptr, - owned_instance: true, + instance: instance_allocator, } }; - let instance = handle.instance(); + let instance = handle.instance().as_ref(); ptr::copy( vmshared_signatures.values().as_slice().as_ptr(), @@ -978,6 +1065,88 @@ impl InstanceHandle { Ok(handle) } + /// Return a reference to the contained `Instance`. + pub(crate) fn instance(&self) -> &InstanceAllocator { + &self.instance + } + + /// Get the locations of where the local `VMMemoryDefinition`s should be stored. + /// + /// This function lets us create `Memory` objects on the host with backing + /// memory in the VM. + /// + /// # Safety + /// - `instance_ptr` must point to enough memory that all of the + /// offsets in `offsets` point to valid locations in memory, + /// i.e. `instance_ptr` must have been allocated by + /// `InstanceHandle::allocate_instance`. + pub unsafe fn memory_definition_locations( + instance_ptr: NonNull, + offsets: &VMOffsets, + ) -> Vec> { + // `NonNull` here actually means `NonNull`. See + // `Self::allocate_instance` to understand why. + let instance_ptr: NonNull = instance_ptr.cast(); + + let num_memories = offsets.num_local_memories; + let num_memories = usize::try_from(num_memories).unwrap(); + let mut out = Vec::with_capacity(num_memories); + + // We need to do some pointer arithmetic now. The unit is `u8`. + let ptr = instance_ptr.cast::().as_ptr(); + + // + let base_ptr = ptr.add(mem::size_of::()); + + for i in 0..num_memories { + let mem_offset = offsets.vmctx_vmmemory_definition(LocalMemoryIndex::new(i)); + let mem_offset = usize::try_from(mem_offset).unwrap(); + + let new_ptr = NonNull::new_unchecked(base_ptr.add(mem_offset)); + + out.push(new_ptr.cast()); + } + + out + } + + /// Get the locations of where the `VMTableDefinition`s should be stored. + /// + /// This function lets us create `Table` objects on the host with backing + /// memory in the VM. + /// + /// # Safety + /// - `instance_ptr` must point to enough memory that all of the + /// offsets in `offsets` point to valid locations in memory, + /// i.e. `instance_ptr` must have been allocated by + /// `InstanceHandle::allocate_instance`. + pub unsafe fn table_definition_locations( + instance_ptr: NonNull, + offsets: &VMOffsets, + ) -> Vec> { + // `NonNull` here actually means `NonNull`. See + // `Self::allocate_instance` to understand why. + let instance_ptr: NonNull = instance_ptr.cast(); + + let num_tables = offsets.num_local_tables; + let num_tables = usize::try_from(num_tables).unwrap(); + let mut out = Vec::with_capacity(num_tables); + + // We need to do some pointer arithmetic now. The unit is `u8`. + let ptr = instance_ptr.cast::().as_ptr(); + let base_ptr = ptr.add(std::mem::size_of::()); + + for i in 0..num_tables { + let table_offset = offsets.vmctx_vmtable_definition(LocalTableIndex::new(i)); + let table_offset = usize::try_from(table_offset).unwrap(); + + let new_ptr = NonNull::new_unchecked(base_ptr.add(table_offset)); + + out.push(new_ptr.cast()); + } + out + } + /// Finishes the instantiation process started by `Instance::new`. /// /// # Safety @@ -987,64 +1156,132 @@ impl InstanceHandle { &self, data_initializers: &[DataInitializer<'_>], ) -> Result<(), Trap> { - check_table_init_bounds(self.instance())?; - check_memory_init_bounds(self.instance(), data_initializers)?; + let instance = self.instance().as_ref(); + check_table_init_bounds(instance)?; + check_memory_init_bounds(instance, data_initializers)?; // Apply the initializers. - initialize_tables(self.instance())?; - initialize_memories(self.instance(), data_initializers)?; + initialize_tables(instance)?; + initialize_memories(instance, data_initializers)?; // The WebAssembly spec specifies that the start function is // invoked automatically at instantiation time. - self.instance().invoke_start_function()?; + instance.invoke_start_function()?; Ok(()) } - /// Create a new `InstanceHandle` pointing at the instance - /// pointed to by the given `VMContext` pointer. - /// - /// # Safety - /// This is unsafe because it doesn't work on just any `VMContext`, it must - /// be a `VMContext` allocated as part of an `Instance`. - pub unsafe fn from_vmctx(vmctx: *const VMContext) -> Self { - let instance = (&*vmctx).instance(); - - Self { - instance: instance as *const Instance as *mut Instance, - - // We consider `vmctx` owns the `Instance`, not `Self`. - owned_instance: false, - } - } - /// Return a reference to the vmctx used by compiled wasm code. pub fn vmctx(&self) -> &VMContext { - self.instance().vmctx() + self.instance().as_ref().vmctx() } /// Return a raw pointer to the vmctx used by compiled wasm code. pub fn vmctx_ptr(&self) -> *mut VMContext { - self.instance().vmctx_ptr() + self.instance().as_ref().vmctx_ptr() } /// Return a reference-counting pointer to a module. pub fn module(&self) -> &Arc { - self.instance().module() + self.instance().as_ref().module() } /// Return a reference to a module. pub fn module_ref(&self) -> &ModuleInfo { - self.instance().module_ref() + self.instance().as_ref().module_ref() } /// Lookup an export with the given name. pub fn lookup(&self, field: &str) -> Option { - self.instance().lookup(field) + let export = self.module().exports.get(field)?; + + Some(self.lookup_by_declaration(&export)) } /// Lookup an export with the given export declaration. pub fn lookup_by_declaration(&self, export: &ExportIndex) -> Export { - self.instance().lookup_by_declaration(export) + let instance = self.instance().clone(); + let instance_ref = instance.as_ref(); + + match export { + ExportIndex::Function(index) => { + let sig_index = &instance_ref.module.functions[*index]; + let (address, vmctx) = + if let Some(def_index) = instance_ref.module.local_func_index(*index) { + ( + instance_ref.functions[def_index].0 as *const _, + VMFunctionEnvironment { + vmctx: instance_ref.vmctx_ptr(), + }, + ) + } else { + let import = instance_ref.imported_function(*index); + (import.body, import.environment) + }; + let call_trampoline = Some(instance_ref.function_call_trampolines[*sig_index]); + let signature = instance_ref.module.signatures[*sig_index].clone(); + + ExportFunction { + address, + // Any function received is already static at this point as: + // 1. All locally defined functions in the Wasm have a static signature. + // 2. All the imported functions are already static (because + // they point to the trampolines rather than the dynamic addresses). + kind: VMFunctionKind::Static, + signature, + vmctx, + call_trampoline, + instance_allocator: Some(instance), + } + .into() + } + + ExportIndex::Table(index) => { + let from = if let Some(def_index) = instance_ref.module.local_table_index(*index) { + instance_ref.tables[def_index].clone() + } else { + let import = instance_ref.imported_table(*index); + import.from.clone() + }; + + ExportTable { + from, + instance_allocator: Some(instance), + } + .into() + } + + ExportIndex::Memory(index) => { + let from = if let Some(def_index) = instance_ref.module.local_memory_index(*index) { + instance_ref.memories[def_index].clone() + } else { + let import = instance_ref.imported_memory(*index); + import.from.clone() + }; + + ExportMemory { + from, + instance_allocator: Some(instance), + } + .into() + } + + ExportIndex::Global(index) => { + let from = { + if let Some(def_index) = instance_ref.module.local_global_index(*index) { + instance_ref.globals[def_index].clone() + } else { + let import = instance_ref.imported_global(*index); + import.from.clone() + } + }; + + ExportGlobal { + from, + instance_allocator: Some(instance), + } + .into() + } + } } /// Return an iterator over the exports of this instance. @@ -1053,17 +1290,17 @@ impl InstanceHandle { /// are export names, and the values are export declarations which can be /// resolved `lookup_by_declaration`. pub fn exports(&self) -> indexmap::map::Iter { - self.instance().exports() + self.module().exports.iter() } /// Return a reference to the custom state attached to this instance. pub fn host_state(&self) -> &dyn Any { - self.instance().host_state() + self.instance().as_ref().host_state() } /// Return the memory index for the given `VMMemoryDefinition` in this instance. pub fn memory_index(&self, memory: &VMMemoryDefinition) -> LocalMemoryIndex { - self.instance().memory_index(memory) + self.instance().as_ref().memory_index(memory) } /// Grow memory in this instance by the specified amount of pages. @@ -1078,12 +1315,12 @@ impl InstanceHandle { where IntoPages: Into, { - self.instance().memory_grow(memory_index, delta) + self.instance().as_ref().memory_grow(memory_index, delta) } /// Return the table index for the given `VMTableDefinition` in this instance. pub fn table_index(&self, table: &VMTableDefinition) -> LocalTableIndex { - self.instance().table_index(table) + self.instance().as_ref().table_index(table) } /// Grow table in this instance by the specified amount of pages. @@ -1091,7 +1328,7 @@ impl InstanceHandle { /// Returns `None` if memory can't be grown by the specified amount /// of pages. pub fn table_grow(&self, table_index: LocalTableIndex, delta: u32) -> Option { - self.instance().table_grow(table_index, delta) + self.instance().as_ref().table_grow(table_index, delta) } /// Get table element reference. @@ -1102,7 +1339,7 @@ impl InstanceHandle { table_index: LocalTableIndex, index: u32, ) -> Option { - self.instance().table_get(table_index, index) + self.instance().as_ref().table_get(table_index, index) } /// Set table element reference. @@ -1114,38 +1351,39 @@ impl InstanceHandle { index: u32, val: VMCallerCheckedAnyfunc, ) -> Result<(), Trap> { - self.instance().table_set(table_index, index, val) + self.instance().as_ref().table_set(table_index, index, val) } /// Get a table defined locally within this module. pub fn get_local_table(&self, index: LocalTableIndex) -> &dyn Table { - self.instance().get_local_table(index) + self.instance().as_ref().get_local_table(index) } +} - /// Return a reference to the contained `Instance`. - pub(crate) fn instance(&self) -> &Instance { - unsafe { &*(self.instance as *const Instance) } - } +cfg_if::cfg_if! { + if #[cfg(unix)] { + pub type SignalHandler = dyn Fn(libc::c_int, *const libc::siginfo_t, *const libc::c_void) -> bool; - /// Deallocates memory associated with this instance. - /// - /// # Safety - /// - /// 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(&instance.offsets); - ptr::drop_in_place(self.instance); - alloc::dealloc(self.instance.cast(), layout); - } -} + impl InstanceHandle { + /// Set a custom signal handler + pub fn set_signal_handler(&self, handler: H) + where + H: 'static + Fn(libc::c_int, *const libc::siginfo_t, *const libc::c_void) -> bool, + { + self.instance().as_ref().signal_handler.set(Some(Box::new(handler))); + } + } + } else if #[cfg(target_os = "windows")] { + pub type SignalHandler = dyn Fn(winapi::um::winnt::PEXCEPTION_POINTERS) -> bool; -impl Drop for InstanceHandle { - fn drop(&mut self) { - if self.owned_instance { - unsafe { self.dealloc() } + impl InstanceHandle { + /// Set a custom signal handler + pub fn set_signal_handler(&self, handler: H) + where + H: 'static + Fn(winapi::um::winnt::PEXCEPTION_POINTERS) -> bool, + { + self.instance().as_ref().signal_handler.set(Some(Box::new(handler))); + } } } } diff --git a/lib/vm/src/trap/traphandlers.rs b/lib/vm/src/trap/traphandlers.rs index de6582cf4b6..bfd0b656dcf 100644 --- a/lib/vm/src/trap/traphandlers.rs +++ b/lib/vm/src/trap/traphandlers.rs @@ -5,7 +5,7 @@ //! signalhandling mechanisms. use super::trapcode::TrapCode; -use crate::instance::{InstanceHandle, SignalHandler}; +use crate::instance::{Instance, SignalHandler}; use crate::vmcontext::{VMFunctionBody, VMFunctionEnvironment, VMTrampoline}; use backtrace::Backtrace; use std::any::Any; @@ -577,9 +577,15 @@ impl CallThreadState { }) } - fn any_instance(&self, func: impl Fn(&InstanceHandle) -> bool) -> bool { + fn any_instance(&self, func: impl Fn(&Instance) -> bool) -> bool { unsafe { - if func(&InstanceHandle::from_vmctx(self.vmctx.vmctx)) { + if func( + self.vmctx + .vmctx + .as_ref() + .expect("`VMContext` is null in `any_instance`") + .instance(), + ) { return true; } match self.prev { @@ -632,13 +638,13 @@ impl CallThreadState { // First up see if any instance registered has a custom trap handler, // in which case run them all. If anything handles the trap then we // return that the trap was handled. - let any_instance = self.any_instance(|i| { - let handler = match i.instance().signal_handler.replace(None) { + let any_instance = self.any_instance(|instance: &Instance| { + let handler = match instance.signal_handler.replace(None) { Some(handler) => handler, None => return false, }; let result = call_handler(&handler); - i.instance().signal_handler.set(Some(handler)); + instance.signal_handler.set(Some(handler)); result }); diff --git a/lib/vm/src/vmoffsets.rs b/lib/vm/src/vmoffsets.rs index b18408165d3..3ca2073cb34 100644 --- a/lib/vm/src/vmoffsets.rs +++ b/lib/vm/src/vmoffsets.rs @@ -95,20 +95,20 @@ impl VMOffsets { impl VMOffsets { /// The offset of the `body` field. #[allow(clippy::erasing_op)] - pub fn vmfunction_import_body(&self) -> u8 { + pub const fn vmfunction_import_body(&self) -> u8 { 0 * self.pointer_size } /// The offset of the `vmctx` field. #[allow(clippy::identity_op)] - pub fn vmfunction_import_vmctx(&self) -> u8 { + pub const fn vmfunction_import_vmctx(&self) -> u8 { 1 * self.pointer_size } /// Return the size of [`VMFunctionImport`]. /// /// [`VMFunctionImport`]: crate::vmcontext::VMFunctionImport - pub fn size_of_vmfunction_import(&self) -> u8 { + pub const fn size_of_vmfunction_import(&self) -> u8 { 2 * self.pointer_size } } @@ -119,20 +119,20 @@ impl VMOffsets { impl VMOffsets { /// The offset of the `address` field. #[allow(clippy::erasing_op)] - pub fn vmdynamicfunction_import_context_address(&self) -> u8 { + pub const fn vmdynamicfunction_import_context_address(&self) -> u8 { 0 * self.pointer_size } /// The offset of the `ctx` field. #[allow(clippy::identity_op)] - pub fn vmdynamicfunction_import_context_ctx(&self) -> u8 { + pub const fn vmdynamicfunction_import_context_ctx(&self) -> u8 { 1 * self.pointer_size } /// Return the size of [`VMDynamicFunctionContext`]. /// /// [`VMDynamicFunctionContext`]: crate::vmcontext::VMDynamicFunctionContext - pub fn size_of_vmdynamicfunction_import_context(&self) -> u8 { + pub const fn size_of_vmdynamicfunction_import_context(&self) -> u8 { 2 * self.pointer_size } }