From ea72c621f07644609c87252d5fb4454d8d8f0e49 Mon Sep 17 00:00:00 2001 From: Peter Huene Date: Wed, 14 Apr 2021 13:45:56 -0700 Subject: [PATCH] Remove the stack map registry. This commit removes the stack map registry and instead uses the existing information from the store's module registry to lookup stack maps. A trait is now used to pass the lookup context to the runtime, implemented by `Store` to do the lookup. With this change, module registration in `Store` is now entirely limited to inserting the module into the module registry. --- crates/environ/src/vmoffsets.rs | 18 +- crates/jit/src/instantiate.rs | 11 +- crates/runtime/src/externref.rs | 201 ++---------------- crates/runtime/src/instance.rs | 8 +- crates/runtime/src/instance/allocator.rs | 8 +- .../runtime/src/instance/allocator/pooling.rs | 4 +- .../src/instance/allocator/pooling/uffd.rs | 2 +- crates/runtime/src/libcalls.rs | 8 +- crates/wasmtime/src/func/typed.rs | 2 +- crates/wasmtime/src/instance.rs | 5 +- crates/wasmtime/src/module/registry.rs | 82 ++++++- crates/wasmtime/src/store.rs | 57 ++--- crates/wasmtime/src/trampoline.rs | 6 +- .../wasmtime/src/trampoline/create_handle.rs | 4 +- crates/wasmtime/src/trampoline/func.rs | 4 +- crates/wasmtime/src/values.rs | 2 +- 16 files changed, 158 insertions(+), 264 deletions(-) diff --git a/crates/environ/src/vmoffsets.rs b/crates/environ/src/vmoffsets.rs index e5d6604610..20f3b8a298 100644 --- a/crates/environ/src/vmoffsets.rs +++ b/crates/environ/src/vmoffsets.rs @@ -6,7 +6,7 @@ // struct VMContext { // interrupts: *const VMInterrupts, // externref_activations_table: *mut VMExternRefActivationsTable, -// stack_map_registry: *mut StackMapRegistry, +// stack_map_lookup: *const dyn StackMapLookup, // signature_ids: [VMSharedSignatureIndex; module.num_signature_ids], // imported_functions: [VMFunctionImport; module.num_imported_functions], // imported_tables: [VMTableImport; module.num_imported_tables], @@ -77,7 +77,7 @@ pub struct VMOffsets { // precalculated offsets of various member fields interrupts: u32, externref_activations_table: u32, - stack_map_registry: u32, + stack_map_lookup: u32, signature_ids: u32, imported_functions: u32, imported_tables: u32, @@ -149,7 +149,7 @@ impl From for VMOffsets { num_defined_globals: fields.num_defined_globals, interrupts: 0, externref_activations_table: 0, - stack_map_registry: 0, + stack_map_lookup: 0, signature_ids: 0, imported_functions: 0, imported_tables: 0, @@ -168,13 +168,13 @@ impl From for VMOffsets { .interrupts .checked_add(u32::from(fields.pointer_size)) .unwrap(); - ret.stack_map_registry = ret + ret.stack_map_lookup = ret .externref_activations_table .checked_add(u32::from(fields.pointer_size)) .unwrap(); ret.signature_ids = ret - .stack_map_registry - .checked_add(u32::from(fields.pointer_size)) + .stack_map_lookup + .checked_add(u32::from(fields.pointer_size * 2)) .unwrap(); ret.imported_functions = ret .signature_ids @@ -507,10 +507,10 @@ impl VMOffsets { self.externref_activations_table } - /// The offset of the `*mut StackMapRegistry` member. + /// The offset of the `*const dyn StackMapLookup` member. #[inline] - pub fn vmctx_stack_map_registry(&self) -> u32 { - self.stack_map_registry + pub fn vmctx_stack_map_lookup(&self) -> u32 { + self.stack_map_lookup } /// The offset of the `signature_ids` array. diff --git a/crates/jit/src/instantiate.rs b/crates/jit/src/instantiate.rs index 2d155e3d7b..5527051ecd 100644 --- a/crates/jit/src/instantiate.rs +++ b/crates/jit/src/instantiate.rs @@ -362,11 +362,18 @@ impl CompiledModule { } /// Gets the function information for a given function index. - pub fn func_info(&self, index: DefinedFuncIndex) -> (&FunctionAddressMap, &[TrapInformation]) { + pub fn func_info( + &self, + index: DefinedFuncIndex, + ) -> ( + &FunctionAddressMap, + &[TrapInformation], + &[StackMapInformation], + ) { self.artifacts .funcs .get(index) - .map(|f| (&f.address_map, f.traps.as_ref())) + .map(|f| (&f.address_map, f.traps.as_ref(), f.stack_maps.as_ref())) .expect("defined function should be present") } diff --git a/crates/runtime/src/externref.rs b/crates/runtime/src/externref.rs index 02943c1c01..0606a941e6 100644 --- a/crates/runtime/src/externref.rs +++ b/crates/runtime/src/externref.rs @@ -103,14 +103,12 @@ use std::alloc::Layout; use std::any::Any; use std::cell::{Cell, RefCell, UnsafeCell}; use std::cmp::Ordering; -use std::collections::BTreeMap; use std::collections::HashSet; use std::hash::{Hash, Hasher}; use std::mem; use std::ops::Deref; use std::ptr::{self, NonNull}; -use std::rc::Rc; -use wasmtime_environ::{ir::StackMap, StackMapInformation}; +use wasmtime_environ::ir::StackMap; /// An external reference to some opaque data. /// @@ -596,10 +594,10 @@ impl VMExternRefActivationsTable { pub unsafe fn insert_with_gc( &self, externref: VMExternRef, - stack_maps_registry: &StackMapRegistry, + stack_map_lookup: &dyn StackMapLookup, ) { if let Err(externref) = self.try_insert(externref) { - self.gc_and_insert_slow(externref, stack_maps_registry); + self.gc_and_insert_slow(externref, stack_map_lookup); } } @@ -607,9 +605,9 @@ impl VMExternRefActivationsTable { unsafe fn gc_and_insert_slow( &self, externref: VMExternRef, - stack_maps_registry: &StackMapRegistry, + stack_map_lookup: &dyn StackMapLookup, ) { - gc(stack_maps_registry, self); + gc(stack_map_lookup, self); // Might as well insert right into the hash set, rather than the bump // chunk, since we are already on a slow path and we get de-duplication @@ -743,182 +741,21 @@ impl VMExternRefActivationsTable { } } -/// A registry of stack maps for currently active Wasm modules. -#[derive(Default)] -pub struct StackMapRegistry { - inner: RefCell, +/// Used by the runtime to lookup a stack map from a PC value. +pub trait StackMapLookup { + /// Lookup the stack map at a program counter (PC) value. + fn lookup(&self, pc: usize) -> Option<*const StackMap>; } -#[derive(Default)] -struct StackMapRegistryInner { - /// A map from the highest pc in a module, to its stack maps. - /// - /// For details, see the comment above `GlobalModuleRegistry`. - ranges: BTreeMap, -} +pub(crate) struct EmptyStackMapLookup; -#[derive(Debug)] -struct ModuleStackMaps { - /// The range of PCs that this module covers. Different modules must always - /// have distinct ranges. - range: std::ops::Range, - - /// A map from a PC in this module (that is a GC safepoint) to its - /// associated stack map. If `None` then it means that the PC is the start - /// of a range which has no stack map. - pc_to_stack_map: Vec<(usize, Option>)>, -} - -impl StackMapRegistry { - /// Register the stack maps for a given module. - /// - /// The stack maps should be given as an iterator over a function's PC range - /// in memory (that is, where the JIT actually allocated and emitted the - /// function's code at), and the stack maps and code offsets within that - /// range for each of its GC safepoints. - pub fn register_stack_maps<'a>( - &self, - stack_maps: impl IntoIterator, &'a [StackMapInformation])>, - ) { - let mut min = usize::max_value(); - let mut max = 0; - let mut pc_to_stack_map = vec![]; - let mut last_is_none_marker = true; - - for (range, infos) in stack_maps { - let len = range.end - range.start; - - min = std::cmp::min(min, range.start); - max = std::cmp::max(max, range.end); - - // Add a marker between functions indicating that this function's pc - // starts with no stack map so when our binary search later on finds - // a pc between the start of the function and the function's first - // stack map it doesn't think the previous stack map is our stack - // map. - // - // We skip this if the previous entry pushed was also a `None` - // marker, in which case the starting pc already has no stack map. - // This is also skipped if the first `code_offset` is zero since - // what we'll push applies for the first pc anyway. - if !last_is_none_marker && (infos.is_empty() || infos[0].code_offset > 0) { - pc_to_stack_map.push((range.start, None)); - last_is_none_marker = true; - } - - for info in infos { - assert!((info.code_offset as usize) < len); - pc_to_stack_map.push(( - range.start + (info.code_offset as usize), - Some(Rc::new(info.stack_map.clone())), - )); - last_is_none_marker = false; - } - } - - if pc_to_stack_map.is_empty() { - // Nothing to register. - return; - } - - let module_stack_maps = ModuleStackMaps { - range: min..max, - pc_to_stack_map, - }; - - let mut inner = self.inner.borrow_mut(); - - // Assert that this chunk of ranges doesn't collide with any other known - // chunks. - if let Some((_, prev)) = inner.ranges.range(max..).next() { - assert!(prev.range.start > max); - } - if let Some((prev_end, _)) = inner.ranges.range(..=min).next_back() { - assert!(*prev_end < min); - } - - let old = inner.ranges.insert(max, module_stack_maps); - assert!(old.is_none()); - } - - /// Lookup the stack map for the given PC, if any. - pub fn lookup_stack_map(&self, pc: usize) -> Option> { - let inner = self.inner.borrow(); - let stack_maps = inner.module_stack_maps(pc)?; - - // Do a binary search to find the stack map for the given PC. - // - // Because GC safepoints are technically only associated with a single - // PC, we should ideally only care about `Ok(index)` values returned - // from the binary search. However, safepoints are inserted right before - // calls, and there are two things that can disturb the PC/offset - // associated with the safepoint versus the PC we actually use to query - // for the stack map: - // - // 1. The `backtrace` crate gives us the PC in a frame that will be - // *returned to*, and where execution will continue from, rather than - // the PC of the call we are currently at. So we would need to - // disassemble one instruction backwards to query the actual PC for - // the stack map. - // - // TODO: One thing we *could* do to make this a little less error - // prone, would be to assert/check that the nearest GC safepoint - // found is within `max_encoded_size(any kind of call instruction)` - // our queried PC for the target architecture. - // - // 2. Cranelift's stack maps only handle the stack, not - // registers. However, some references that are arguments to a call - // may need to be in registers. In these cases, what Cranelift will - // do is: - // - // a. spill all the live references, - // b. insert a GC safepoint for those references, - // c. reload the references into registers, and finally - // d. make the call. - // - // Step (c) adds drift between the GC safepoint and the location of - // the call, which is where we actually walk the stack frame and - // collect its live references. - // - // Luckily, the spill stack slots for the live references are still - // up to date, so we can still find all the on-stack roots. - // Furthermore, we do not have a moving GC, so we don't need to worry - // whether the following code will reuse the references in registers - // (which would not have been updated to point to the moved objects) - // or reload from the stack slots (which would have been updated to - // point to the moved objects). - let index = match stack_maps - .pc_to_stack_map - .binary_search_by_key(&pc, |(pc, _stack_map)| *pc) - { - // Exact hit. - Ok(i) => i, - - // `Err(0)` means that the associated stack map would have been the - // first element in the array if this pc had an associated stack - // map, but this pc does not have an associated stack map. This can - // only happen inside a Wasm frame if there are no live refs at this - // pc. - Err(0) => return None, - - Err(n) => n - 1, - }; - - let stack_map = stack_maps.pc_to_stack_map[index].1.as_ref()?.clone(); - Some(stack_map) +impl StackMapLookup for EmptyStackMapLookup { + fn lookup(&self, _pc: usize) -> Option<*const StackMap> { + None } } -impl StackMapRegistryInner { - fn module_stack_maps(&self, pc: usize) -> Option<&ModuleStackMaps> { - let (end, stack_maps) = self.ranges.range(pc..).next()?; - if pc < stack_maps.range.start || *end < pc { - None - } else { - Some(stack_maps) - } - } -} +pub(crate) const EMPTY_STACK_MAP_LOOKUP: EmptyStackMapLookup = EmptyStackMapLookup; #[derive(Debug, Default)] struct DebugOnly { @@ -965,7 +802,7 @@ impl std::ops::DerefMut for DebugOnly { /// Additionally, you must have registered the stack maps for every Wasm module /// that has frames on the stack with the given `stack_maps_registry`. pub unsafe fn gc( - stack_maps_registry: &StackMapRegistry, + stack_map_lookup: &dyn StackMapLookup, externref_activations_table: &VMExternRefActivationsTable, ) { // We borrow the precise stack roots `RefCell` for the whole duration of @@ -1003,7 +840,7 @@ pub unsafe fn gc( if cfg!(debug_assertions) { // Assert that there aren't any Wasm frames on the stack. backtrace::trace(|frame| { - let stack_map = stack_maps_registry.lookup_stack_map(frame.ip() as usize); + let stack_map = stack_map_lookup.lookup(frame.ip() as usize); assert!(stack_map.is_none()); true }); @@ -1048,11 +885,11 @@ pub unsafe fn gc( let pc = frame.ip() as usize; let sp = frame.sp() as usize; - if let Some(stack_map) = stack_maps_registry.lookup_stack_map(pc) { + if let Some(stack_map) = stack_map_lookup.lookup(pc) { debug_assert!(sp != 0, "we should always get a valid SP for Wasm frames"); - for i in 0..(stack_map.mapped_words() as usize) { - if stack_map.get_bit(i) { + for i in 0..((*stack_map).mapped_words() as usize) { + if (*stack_map).get_bit(i) { // Stack maps have one bit per word in the frame, and the // zero^th bit is the *lowest* addressed word in the frame, // i.e. the closest to the SP. So to get the `i`^th word in diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index bf4234f6b8..33a2583cff 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -3,7 +3,7 @@ //! `InstanceHandle` is a reference-counting handle for an `Instance`. use crate::export::Export; -use crate::externref::{StackMapRegistry, VMExternRefActivationsTable}; +use crate::externref::{StackMapLookup, VMExternRefActivationsTable}; use crate::memory::{Memory, RuntimeMemoryCreator}; use crate::table::{Table, TableElement}; use crate::traphandlers::Trap; @@ -249,9 +249,9 @@ impl Instance { unsafe { self.vmctx_plus_offset(self.offsets.vmctx_externref_activations_table()) } } - /// Return a pointer to the `StackMapRegistry`. - pub fn stack_map_registry(&self) -> *mut *mut StackMapRegistry { - unsafe { self.vmctx_plus_offset(self.offsets.vmctx_stack_map_registry()) } + /// Return a pointer to the `StackMapLookup`. + pub fn stack_map_lookup(&self) -> *mut *const dyn StackMapLookup { + unsafe { self.vmctx_plus_offset(self.offsets.vmctx_stack_map_lookup()) } } /// Return a reference to the vmctx used by compiled wasm code. diff --git a/crates/runtime/src/instance/allocator.rs b/crates/runtime/src/instance/allocator.rs index d236b4723c..14a9d2eff3 100644 --- a/crates/runtime/src/instance/allocator.rs +++ b/crates/runtime/src/instance/allocator.rs @@ -1,4 +1,4 @@ -use crate::externref::{StackMapRegistry, VMExternRefActivationsTable}; +use crate::externref::{StackMapLookup, VMExternRefActivationsTable, EMPTY_STACK_MAP_LOOKUP}; use crate::imports::Imports; use crate::instance::{Instance, InstanceHandle, RuntimeMemoryCreator}; use crate::memory::{DefaultMemoryCreator, Memory}; @@ -57,8 +57,8 @@ pub struct InstanceAllocationRequest<'a> { /// The pointer to the reference activations table to use for the instance. pub externref_activations_table: *mut VMExternRefActivationsTable, - /// The pointer to the stack map registry to use for the instance. - pub stack_map_registry: *mut StackMapRegistry, + /// The pointer to the stack map lookup to use for the instance. + pub stack_map_lookup: Option<*const dyn StackMapLookup>, } /// An link error while instantiating a module. @@ -447,7 +447,7 @@ unsafe fn initialize_vmcontext(instance: &Instance, req: InstanceAllocationReque *instance.interrupts() = req.interrupts; *instance.externref_activations_table() = req.externref_activations_table; - *instance.stack_map_registry() = req.stack_map_registry; + *instance.stack_map_lookup() = req.stack_map_lookup.unwrap_or(&EMPTY_STACK_MAP_LOOKUP); // Initialize shared signatures let mut ptr = instance.signature_ids_ptr(); diff --git a/crates/runtime/src/instance/allocator/pooling.rs b/crates/runtime/src/instance/allocator/pooling.rs index d9604c348f..6b89ce603e 100644 --- a/crates/runtime/src/instance/allocator/pooling.rs +++ b/crates/runtime/src/instance/allocator/pooling.rs @@ -1370,7 +1370,7 @@ mod test { host_state: Box::new(()), interrupts: std::ptr::null(), externref_activations_table: std::ptr::null_mut(), - stack_map_registry: std::ptr::null_mut(), + stack_map_lookup: None, }, ) .expect("allocation should succeed"), @@ -1394,7 +1394,7 @@ mod test { host_state: Box::new(()), interrupts: std::ptr::null(), externref_activations_table: std::ptr::null_mut(), - stack_map_registry: std::ptr::null_mut(), + stack_map_lookup: None, }, ) { Err(InstantiationError::Limit(3)) => {} diff --git a/crates/runtime/src/instance/allocator/pooling/uffd.rs b/crates/runtime/src/instance/allocator/pooling/uffd.rs index 5578ca1746..fb811680bb 100644 --- a/crates/runtime/src/instance/allocator/pooling/uffd.rs +++ b/crates/runtime/src/instance/allocator/pooling/uffd.rs @@ -523,7 +523,7 @@ mod test { host_state: Box::new(()), interrupts: ptr::null(), externref_activations_table: ptr::null_mut(), - stack_map_registry: ptr::null_mut(), + stack_map_lookup: None, }, ) .expect("instance should allocate"), diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs index 026738a8a1..58eb585312 100644 --- a/crates/runtime/src/libcalls.rs +++ b/crates/runtime/src/libcalls.rs @@ -449,8 +449,8 @@ pub unsafe extern "C" fn wasmtime_activations_table_insert_with_gc( let externref = VMExternRef::clone_from_raw(externref); let instance = (&mut *vmctx).instance(); let activations_table = &**instance.externref_activations_table(); - let registry = &**instance.stack_map_registry(); - activations_table.insert_with_gc(externref, registry); + let stack_map_lookup = &**instance.stack_map_lookup(); + activations_table.insert_with_gc(externref, stack_map_lookup); } /// Perform a Wasm `global.get` for `externref` globals. @@ -466,8 +466,8 @@ pub unsafe extern "C" fn wasmtime_externref_global_get( Some(externref) => { let raw = externref.as_raw(); let activations_table = &**instance.externref_activations_table(); - let registry = &**instance.stack_map_registry(); - activations_table.insert_with_gc(externref, registry); + let stack_map_lookup = &**instance.stack_map_lookup(); + activations_table.insert_with_gc(externref, stack_map_lookup); raw } } diff --git a/crates/wasmtime/src/func/typed.rs b/crates/wasmtime/src/func/typed.rs index c0a1d834da..b0fe8b574f 100644 --- a/crates/wasmtime/src/func/typed.rs +++ b/crates/wasmtime/src/func/typed.rs @@ -207,7 +207,7 @@ unsafe impl WasmTy for Option { unsafe { store .externref_activations_table() - .insert_with_gc(x.inner, store.stack_map_registry()); + .insert_with_gc(x.inner, &*store.stack_map_lookup()); } abi } else { diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index 00cc90fd82..a30ef98414 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -13,7 +13,7 @@ use wasmtime_environ::wasm::{ }; use wasmtime_environ::Initializer; use wasmtime_runtime::{ - Imports, InstanceAllocationRequest, InstantiationError, RuntimeInstance, StackMapRegistry, + Imports, InstanceAllocationRequest, InstantiationError, RuntimeInstance, StackMapLookup, VMContext, VMExternRefActivationsTable, VMFunctionBody, VMFunctionImport, VMGlobalImport, VMMemoryImport, VMTableImport, }; @@ -525,8 +525,7 @@ impl<'a> Instantiator<'a> { externref_activations_table: self.store.externref_activations_table() as *const VMExternRefActivationsTable as *mut _, - stack_map_registry: self.store.stack_map_registry() as *const StackMapRegistry - as *mut _, + stack_map_lookup: Some(self.store.stack_map_lookup() as *const dyn StackMapLookup), })?; // After we've created the `InstanceHandle` we still need to run diff --git a/crates/wasmtime/src/module/registry.rs b/crates/wasmtime/src/module/registry.rs index 161ef408d6..bec0434a91 100644 --- a/crates/wasmtime/src/module/registry.rs +++ b/crates/wasmtime/src/module/registry.rs @@ -43,6 +43,11 @@ impl ModuleRegistry { self.module(pc)?.lookup_trap_info(pc) } + /// Looks up a stack map from a program counter. + pub fn lookup_stack_map<'a>(&'a self, pc: usize) -> Option<&'a ir::StackMap> { + self.module(pc)?.lookup_stack_map(pc) + } + fn module(&self, pc: usize) -> Option<&RegisteredModule> { let (end, info) = self.0.range(pc..).next()?; if pc < info.start || *end < pc { @@ -53,13 +58,13 @@ impl ModuleRegistry { } /// Registers a new module with the registry. - pub fn register(&mut self, module: &Module) -> bool { + pub fn register(&mut self, module: &Module) { let compiled_module = module.compiled_module(); let (start, end) = compiled_module.code().range(); // Ignore modules with no code, finished functions, or if the module is already registered if start == end || compiled_module.finished_functions().is_empty() { - return false; + return; } // The module code range is exclusive for end, so make it inclusive as it @@ -67,7 +72,7 @@ impl ModuleRegistry { let end = end - 1; if self.0.get(&end).is_some() { - return false; + return; } // Assert that this module's code doesn't collide with any other registered modules @@ -90,7 +95,6 @@ impl ModuleRegistry { assert!(prev.is_none()); GLOBAL_MODULES.lock().unwrap().register(start, end, module); - true } /// Looks up a trampoline from a shared signature index. @@ -135,7 +139,7 @@ impl RegisteredModule { /// if no information can be found. pub fn lookup_frame_info(&self, pc: usize) -> Option { let (index, offset) = self.func(pc)?; - let (addr_map, _) = self.module.func_info(index); + let (addr_map, _, _) = self.module.func_info(index); let pos = Self::instr_pos(offset, addr_map); // In debug mode for now assert that we found a mapping for `pc` within @@ -197,13 +201,77 @@ impl RegisteredModule { /// Fetches trap information about a program counter in a backtrace. pub fn lookup_trap_info(&self, pc: usize) -> Option<&TrapInformation> { let (index, offset) = self.func(pc)?; - let (_, traps) = self.module.func_info(index); + let (_, traps, _) = self.module.func_info(index); let idx = traps .binary_search_by_key(&offset, |info| info.code_offset) .ok()?; Some(&traps[idx]) } + /// Looks up a stack map from a program counter + pub fn lookup_stack_map(&self, pc: usize) -> Option<&ir::StackMap> { + let (index, offset) = self.func(pc)?; + let (_, _, stack_maps) = self.module.func_info(index); + + // Do a binary search to find the stack map for the given offset. + // + // Because GC safepoints are technically only associated with a single + // PC, we should ideally only care about `Ok(index)` values returned + // from the binary search. However, safepoints are inserted right before + // calls, and there are two things that can disturb the PC/offset + // associated with the safepoint versus the PC we actually use to query + // for the stack map: + // + // 1. The `backtrace` crate gives us the PC in a frame that will be + // *returned to*, and where execution will continue from, rather than + // the PC of the call we are currently at. So we would need to + // disassemble one instruction backwards to query the actual PC for + // the stack map. + // + // TODO: One thing we *could* do to make this a little less error + // prone, would be to assert/check that the nearest GC safepoint + // found is within `max_encoded_size(any kind of call instruction)` + // our queried PC for the target architecture. + // + // 2. Cranelift's stack maps only handle the stack, not + // registers. However, some references that are arguments to a call + // may need to be in registers. In these cases, what Cranelift will + // do is: + // + // a. spill all the live references, + // b. insert a GC safepoint for those references, + // c. reload the references into registers, and finally + // d. make the call. + // + // Step (c) adds drift between the GC safepoint and the location of + // the call, which is where we actually walk the stack frame and + // collect its live references. + // + // Luckily, the spill stack slots for the live references are still + // up to date, so we can still find all the on-stack roots. + // Furthermore, we do not have a moving GC, so we don't need to worry + // whether the following code will reuse the references in registers + // (which would not have been updated to point to the moved objects) + // or reload from the stack slots (which would have been updated to + // point to the moved objects). + + let index = match stack_maps.binary_search_by_key(&offset, |i| i.code_offset) { + // Exact hit. + Ok(i) => i, + + // `Err(0)` means that the associated stack map would have been the + // first element in the array if this pc had an associated stack + // map, but this pc does not have an associated stack map. This can + // only happen inside a Wasm frame if there are no live refs at this + // pc. + Err(0) => return None, + + Err(i) => i - 1, + }; + + Some(&stack_maps[index].stack_map) + } + fn func(&self, pc: usize) -> Option<(DefinedFuncIndex, u32)> { let (index, start, _) = self.module.func_by_pc(pc)?; Some((index, (pc - start) as u32)) @@ -260,7 +328,7 @@ impl GlobalModuleRegistry { match info.module.func(pc) { Some((index, offset)) => { - let (addr_map, _) = info.module.module.func_info(index); + let (addr_map, _, _) = info.module.module.func_info(index); RegisteredModule::instr_pos(offset, addr_map).is_some() } None => false, diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index 5cbe0119b5..f337b39975 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -16,11 +16,11 @@ use std::rc::Rc; use std::sync::Arc; use std::task::{Context, Poll}; use wasmtime_environ::wasm::WasmFuncType; -use wasmtime_jit::{CompiledModule, ModuleCode}; +use wasmtime_jit::ModuleCode; use wasmtime_runtime::{ - InstanceAllocator, InstanceHandle, OnDemandInstanceAllocator, SignalHandler, StackMapRegistry, - TrapInfo, VMContext, VMExternRef, VMExternRefActivationsTable, VMInterrupts, - VMSharedSignatureIndex, VMTrampoline, + InstanceAllocator, InstanceHandle, OnDemandInstanceAllocator, SignalHandler, TrapInfo, + VMContext, VMExternRef, VMExternRefActivationsTable, VMInterrupts, VMSharedSignatureIndex, + VMTrampoline, }; /// Used to associate instances with the store. @@ -75,7 +75,6 @@ pub(crate) struct StoreInner { instances: RefCell>, signal_handler: RefCell>>>, externref_activations_table: VMExternRefActivationsTable, - stack_map_registry: StackMapRegistry, modules: RefCell, trampolines: RefCell, // Numbers of resources instantiated in this store. @@ -139,7 +138,6 @@ impl Store { instances: RefCell::new(Vec::new()), signal_handler: RefCell::new(None), externref_activations_table: VMExternRefActivationsTable::new(), - stack_map_registry: StackMapRegistry::default(), modules: RefCell::new(ModuleRegistry::default()), trampolines: RefCell::new(TrampolineMap::default()), instance_count: Default::default(), @@ -207,26 +205,7 @@ impl Store { pub(crate) fn register_module(&self, module: &Module) { // Register the module with the registry - if !self.inner.modules.borrow_mut().register(module) { - return; - } - - // We need to know about all the stack maps of all instantiated modules - // so when performing a GC we know about all wasm frames that we find - // on the stack. - self.register_stack_maps(module.compiled_module()); - } - - fn register_stack_maps(&self, module: &CompiledModule) { - self.stack_map_registry() - .register_stack_maps(module.stack_maps().map(|(func, stack_maps)| unsafe { - let ptr = (*func).as_ptr(); - let len = (*func).len(); - let start = ptr as usize; - let end = ptr as usize + len; - let range = start..end; - (range, stack_maps) - })); + self.inner.modules.borrow_mut().register(module); } // This is used to register a `Func` with the store @@ -448,10 +427,6 @@ impl Store { } #[inline] - pub(crate) fn stack_map_registry(&self) -> &StackMapRegistry { - &self.inner.stack_map_registry - } - pub(crate) fn modules(&self) -> &RefCell { &self.inner.modules } @@ -471,20 +446,21 @@ impl Store { /// /// It is fine to call this several times: only the first call will have an effect. pub unsafe fn notify_switched_thread(&self) { - wasmtime_runtime::init_traps(frame_info::GlobalFrameInfo::is_wasm_pc) + wasmtime_runtime::init_traps(crate::module::GlobalModuleRegistry::is_wasm_pc) .expect("failed to initialize per-threads traps"); } + #[inline] + pub(crate) fn stack_map_lookup(&self) -> *const dyn wasmtime_runtime::StackMapLookup { + self.inner.as_ref() + } + /// Perform garbage collection of `ExternRef`s. pub fn gc(&self) { // For this crate's API, we ensure that `set_stack_canary` invariants - // are upheld for all host-->Wasm calls, and we register every module - // used with this store in `self.inner.stack_map_registry`. + // are upheld for all host-->Wasm calls. unsafe { - wasmtime_runtime::gc( - &self.inner.stack_map_registry, - &self.inner.externref_activations_table, - ); + wasmtime_runtime::gc(self.inner.as_ref(), &self.inner.externref_activations_table); } } @@ -951,6 +927,13 @@ impl Drop for StoreInner { } } +impl wasmtime_runtime::StackMapLookup for StoreInner { + fn lookup(&self, pc: usize) -> Option<*const wasmtime_environ::ir::StackMap> { + // The address of the stack map is stable for the lifetime of the store + self.modules.borrow().lookup_stack_map(pc).map(|m| m as _) + } +} + /// A threadsafe handle used to interrupt instances executing within a /// particular `Store`. /// diff --git a/crates/wasmtime/src/trampoline.rs b/crates/wasmtime/src/trampoline.rs index 7cb588f7d4..21eac2a134 100644 --- a/crates/wasmtime/src/trampoline.rs +++ b/crates/wasmtime/src/trampoline.rs @@ -19,8 +19,8 @@ use std::sync::Arc; use wasmtime_environ::{entity::PrimaryMap, wasm, Module}; use wasmtime_runtime::{ Imports, InstanceAllocationRequest, InstanceAllocator, InstanceHandle, - OnDemandInstanceAllocator, StackMapRegistry, VMExternRefActivationsTable, VMFunctionBody, - VMFunctionImport, VMSharedSignatureIndex, + OnDemandInstanceAllocator, VMExternRefActivationsTable, VMFunctionBody, VMFunctionImport, + VMSharedSignatureIndex, }; /// A wrapper around `wasmtime_runtime::InstanceHandle` which pairs it with the @@ -77,7 +77,7 @@ fn create_handle( externref_activations_table: store.externref_activations_table() as *const VMExternRefActivationsTable as *mut _, - stack_map_registry: store.stack_map_registry() as *const StackMapRegistry as *mut _, + stack_map_lookup: Some(store.stack_map_lookup()), }, )?; diff --git a/crates/wasmtime/src/trampoline/create_handle.rs b/crates/wasmtime/src/trampoline/create_handle.rs index dde937f81c..182e421499 100644 --- a/crates/wasmtime/src/trampoline/create_handle.rs +++ b/crates/wasmtime/src/trampoline/create_handle.rs @@ -9,7 +9,7 @@ use wasmtime_environ::entity::PrimaryMap; use wasmtime_environ::wasm::DefinedFuncIndex; use wasmtime_environ::Module; use wasmtime_runtime::{ - Imports, InstanceAllocationRequest, InstanceAllocator, StackMapRegistry, + Imports, InstanceAllocationRequest, InstanceAllocator, StackMapLookup, VMExternRefActivationsTable, VMFunctionBody, VMFunctionImport, VMSharedSignatureIndex, }; @@ -43,7 +43,7 @@ pub(crate) fn create_handle( externref_activations_table: store.externref_activations_table() as *const VMExternRefActivationsTable as *mut _, - stack_map_registry: store.stack_map_registry() as *const StackMapRegistry as *mut _, + stack_map_lookup: &store, })?; Ok(store.add_instance(handle, true)) diff --git a/crates/wasmtime/src/trampoline/func.rs b/crates/wasmtime/src/trampoline/func.rs index f2402efa6c..0265c0b3c1 100644 --- a/crates/wasmtime/src/trampoline/func.rs +++ b/crates/wasmtime/src/trampoline/func.rs @@ -283,7 +283,7 @@ pub fn create_function( host_state: Box::new(trampoline_state), interrupts: std::ptr::null(), externref_activations_table: std::ptr::null_mut(), - stack_map_registry: std::ptr::null_mut(), + stack_map_lookup: None, })?, trampoline, )) @@ -315,7 +315,7 @@ pub unsafe fn create_raw_function( host_state, interrupts: std::ptr::null(), externref_activations_table: std::ptr::null_mut(), - stack_map_registry: std::ptr::null_mut(), + stack_map_lookup: None, })?, ) } diff --git a/crates/wasmtime/src/values.rs b/crates/wasmtime/src/values.rs index 4dc6474513..9e10f0aa83 100644 --- a/crates/wasmtime/src/values.rs +++ b/crates/wasmtime/src/values.rs @@ -98,7 +98,7 @@ impl Val { let externref_ptr = x.inner.as_raw(); store .externref_activations_table() - .insert_with_gc(x.inner, store.stack_map_registry()); + .insert_with_gc(x.inner, &*store.stack_map_lookup()); ptr::write(p as *mut *mut u8, externref_ptr) } Val::FuncRef(f) => ptr::write(