T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove global state for trap registration (#909)

Browse Source 
* Remove global state for trap registration

There's a number of changes brought about in this commit, motivated by a
few things. One motivation was to remove an instance of using
`lazy_static!` in an effort to remove global state and encapsulate it
wherever possible. A second motivation came when investigating a
slowly-compiling wasm module (a bit too slowly) where a good chunk of
time was spent in managing trap registrations.

The specific change made here is that `TrapRegistry` is now stored
inside of a `Compiler` instead of inside a global. Additionally traps
are "bulk registered" for a module rather than one-by-one. This form of
bulk-registration allows optimizing the locks used here, where a lock is
only held for a module at-a-time instead of once-per-function.

With these changes the "unregister" logic has also been tweaked a bit
here and there to continue to work. As a nice side effect the `Compiler`
type now has one fewer field that requires actual mutability and has
been updated for multi-threaded compilation, nudging us closer to a
world where we can support multi-threaded compilation. Yay!

In terms of performance improvements, a local wasm test file that
previously took 3 seconds to compile is now 10% faster to compile,
taking ~2.7 seconds now.

* Perform trap resolution after unwinding

This avoids taking locks in signal handlers which feels a bit iffy...

* Remove `TrapRegistration::dummy()`

Avoid an case where you're trying to lookup trap information from a
dummy module for something that happened in a different module.

* Tweak some comments
This commit is contained in:
Alex Crichton
2020-02-06 12:40:50 -06:00
committed by GitHub
parent 9dffaf9d57
commit 348c597a8e
14 changed files with 233 additions and 131 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
crates/runtime/src/instance.rs
View File

@@ -15,6 +15,7 @@ use crate::vmcontext::{
VMGlobalDefinition, VMGlobalImport, VMMemoryDefinition, VMMemoryImport, VMSharedSignatureIndex,
VMTableDefinition, VMTableImport,
};
use crate::TrapRegistration;
use memoffset::offset_of;
use more_asserts::assert_lt;
use std::any::Any;
@@ -100,6 +101,10 @@ pub(crate) struct Instance {
/// Handler run when `SIGBUS`, `SIGFPE`, `SIGILL`, or `SIGSEGV` are caught by the instance thread.
pub(crate) signal_handler: Cell<Option<Box<SignalHandler>>>,
/// Handle to our registration of traps so signals know what trap to return
/// when a segfault/sigill happens.
pub(crate) trap_registration: TrapRegistration,
/// 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).
@@ -534,6 +539,7 @@ impl InstanceHandle {
/// safety.
pub unsafe fn new(
module: Arc<Module>,
trap_registration: TrapRegistration,
finished_functions: BoxedSlice<DefinedFuncIndex, *const VMFunctionBody>,
imports: Imports,
data_initializers: &[DataInitializer<'_>],
@@ -582,6 +588,7 @@ impl InstanceHandle {
dbg_jit_registration,
host_state,
signal_handler: Cell::new(None),
trap_registration,
vmctx: VMContext {},
};
ptr::write(instance_ptr, instance);

3
crates/runtime/src/lib.rs
View File

@@ -43,8 +43,7 @@ pub use crate::instance::{InstanceHandle, InstantiationError, LinkError};
pub use crate::jit_int::GdbJitImageRegistration;
pub use crate::mmap::Mmap;
pub use crate::sig_registry::SignatureRegistry;
pub use crate::trap_registry::TrapDescription;
pub use crate::trap_registry::{get_mut_trap_registry, get_trap_registry, TrapRegistrationGuard};
pub use crate::trap_registry::{TrapDescription, TrapRegistration, TrapRegistry};
pub use crate::traphandlers::resume_panic;
pub use crate::traphandlers::{raise_user_trap, wasmtime_call, wasmtime_call_trampoline, Trap};
pub use crate::vmcontext::{

158
crates/runtime/src/trap_registry.rs
View File

@@ -1,19 +1,64 @@
use lazy_static::lazy_static;
use std::collections::HashMap;
use std::collections::{BTreeMap, HashMap};
use std::fmt;
use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard};
use std::sync::{Arc, RwLock};
use wasmtime_environ::ir;
lazy_static! {
static ref REGISTRY: RwLock<TrapRegistry> = RwLock::new(TrapRegistry::default());
}
/// The registry maintains descriptions of traps in currently allocated functions.
#[derive(Default)]
pub struct TrapRegistry {
// This data structure is intended to be safe to use across many threads
// since this is stored inside of a `Compiler` which, eventually, will be
// used across many threads. To that end this is internally use an `Arc`
// plus an `RwLock`.
//
// The problem that this data structure is solving is that when a
// segfault/illegal instruction happens we need to answer "given this
// hardware program counter what is the wasm reason this trap is being
// raised"?
//
// The way this is answered here is done to minimize the amount of
// synchronization (in theory) and have something like so:
//
// * Each module bulk-registers a list of in-memory pc addresses that have
// traps. We assume that the range of traps for each module are always
// disjoint.
// * Each key in this `BTreeMap` is the highest trapping address and the
// value contains the lowest address as well as all the individual
// addresses in their own `HashMap`.
// * Registration then looks by calculating the start/end and inserting
// into this map (with some assertions about disjointed-ness)
// * Lookup is done in two layers. First we find the corresponding entry
// in the map and verify that a program counter falls in the start/end
// range. Next we look up the address in the `traps` hash map below.
//
// The `register_traps` function works by returning an RAII guard that owns
// a handle to this `Arc` as well, and when that type is dropped it will
// automatically remove all trap information from this `ranges` list.
ranges: Arc<RwLock<BTreeMap<usize, TrapGroup>>>,
}
#[derive(Debug)]
struct TrapGroup {
/// The lowest key in the `trap` field.
///
/// This represents the start of the range of this group of traps, and the
/// end of the range for this group of traps is stored as the key in the
/// `ranges` struct above in `TrapRegistry`.
start: usize,
/// All known traps in this group, mapped from program counter to the
/// description of the trap itself.
traps: HashMap<usize, TrapDescription>,
}
/// RAII structure returned from `TrapRegistry::register_trap` to unregister
/// trap information on drop.
#[derive(Clone)]
pub struct TrapRegistration {
ranges: Arc<RwLock<BTreeMap<usize, TrapGroup>>>,
end: Option<usize>,
}
/// Description of a trap.
#[derive(Clone, Copy, PartialEq, Debug)]
pub struct TrapDescription {
@@ -52,50 +97,77 @@ fn trap_code_to_expected_string(trap_code: ir::TrapCode) -> String {
}
}
/// RAII guard for deregistering traps
pub struct TrapRegistrationGuard(usize);
impl TrapRegistry {
/// Registers a new trap.
/// Returns a RAII guard that deregisters the trap when dropped.
pub fn register_trap(
&mut self,
address: usize,
source_loc: ir::SourceLoc,
trap_code: ir::TrapCode,
) -> TrapRegistrationGuard {
let entry = TrapDescription {
source_loc,
trap_code,
};
let previous_trap = self.traps.insert(address, entry);
assert!(previous_trap.is_none());
TrapRegistrationGuard(address)
}
/// Registers a list of traps.
///
/// Returns a RAII guard that deregisters all traps when dropped.
pub fn register_traps(
&self,
list: impl IntoIterator<Item = (usize, ir::SourceLoc, ir::TrapCode)>,
) -> TrapRegistration {
let mut start = usize::max_value();
let mut end = 0;
let mut traps = HashMap::new();
for (addr, source_loc, trap_code) in list.into_iter() {
traps.insert(
addr,
TrapDescription {
source_loc,
trap_code,
},
);
if addr < start {
start = addr;
}
if addr > end {
end = addr;
}
}
if traps.len() == 0 {
return TrapRegistration {
ranges: self.ranges.clone(),
end: None,
};
}
let mut ranges = self.ranges.write().unwrap();
fn deregister_trap(&mut self, address: usize) {
assert!(self.traps.remove(&address).is_some());
}
// Sanity check that no other group of traps overlaps with our
// registration...
if let Some((_, prev)) = ranges.range(end..).next() {
assert!(prev.start > end);
}
if let Some((prev_end, _)) = ranges.range(..=start).next_back() {
assert!(*prev_end < start);
}
// ... and then register ourselves
assert!(ranges.insert(end, TrapGroup { start, traps }).is_none());
TrapRegistration {
ranges: self.ranges.clone(),
end: Some(end),
}
}
}
impl TrapRegistration {
/// Gets a trap description at given address.
pub fn get_trap(&self, address: usize) -> Option<TrapDescription> {
self.traps.get(&address).copied()
let ranges = self.ranges.read().ok()?;
let (end, group) = ranges.range(address..).next()?;
if group.start <= address && address <= *end {
group.traps.get(&address).copied()
} else {
None
}
}
}
impl Drop for TrapRegistrationGuard {
impl Drop for TrapRegistration {
fn drop(&mut self) {
let mut registry = get_mut_trap_registry();
registry.deregister_trap(self.0);
if let Some(end) = self.end {
if let Ok(mut ranges) = self.ranges.write() {
ranges.remove(&end);
}
}
}
}
/// Gets guarded writable reference to traps registry
pub fn get_mut_trap_registry() -> RwLockWriteGuard<'static, TrapRegistry> {
REGISTRY.write().expect("trap registry lock got poisoned")
}
/// Gets guarded readable reference to traps registry
pub fn get_trap_registry() -> RwLockReadGuard<'static, TrapRegistry> {
REGISTRY.read().expect("trap registry lock got poisoned")
}

45
crates/runtime/src/traphandlers.rs
View File

@@ -2,7 +2,6 @@
//! signalhandling mechanisms.
use crate::instance::{InstanceHandle, SignalHandler};
use crate::trap_registry::get_trap_registry;
use crate::trap_registry::TrapDescription;
use crate::vmcontext::{VMContext, VMFunctionBody};
use backtrace::Backtrace;
@@ -78,8 +77,7 @@ cfg_if::cfg_if! {
/// Only safe to call when wasm code is on the stack, aka `wasmtime_call` or
/// `wasmtime_call_trampoline` must have been previously called.
pub unsafe fn raise_user_trap(data: Box<dyn Error + Send + Sync>) -> ! {
let trap = Trap::User(data);
tls::with(|info| info.unwrap().unwind_with(UnwindReason::Trap(trap)))
tls::with(|info| info.unwrap().unwind_with(UnwindReason::UserTrap(data)))
}
/// Carries a Rust panic across wasm code and resumes the panic on the other
@@ -187,7 +185,8 @@ pub struct CallThreadState {
enum UnwindReason {
None,
Panic(Box<dyn Any + Send>),
Trap(Trap),
UserTrap(Box<dyn Error + Send + Sync>),
Trap { backtrace: Backtrace, pc: usize },
}
impl CallThreadState {
@@ -210,9 +209,25 @@ impl CallThreadState {
debug_assert_eq!(ret, 1);
Ok(())
}
UnwindReason::Trap(trap) => {
UnwindReason::UserTrap(data) => {
debug_assert_eq!(ret, 0);
Err(trap)
Err(Trap::User(data))
}
UnwindReason::Trap { backtrace, pc } => {
debug_assert_eq!(ret, 0);
let instance = unsafe { InstanceHandle::from_vmctx(self.vmctx) };
Err(Trap::Wasm {
desc: instance
.instance()
.trap_registration
.get_trap(pc)
.unwrap_or_else(|| TrapDescription {
source_loc: ir::SourceLoc::default(),
trap_code: ir::TrapCode::StackOverflow,
}),
backtrace,
})
}
UnwindReason::Panic(panic) => {
debug_assert_eq!(ret, 0);
@@ -289,20 +304,12 @@ impl CallThreadState {
if self.jmp_buf.get().is_null() {
return ptr::null();
}
let registry = get_trap_registry();
let trap = Trap::Wasm {
desc: registry
.get_trap(pc as usize)
.unwrap_or_else(|| TrapDescription {
source_loc: ir::SourceLoc::default(),
trap_code: ir::TrapCode::StackOverflow,
}),
backtrace: Backtrace::new_unresolved(),
};
let backtrace = Backtrace::new_unresolved();
self.reset_guard_page.set(reset_guard_page);
self.unwind.replace(UnwindReason::Trap(trap));
self.unwind.replace(UnwindReason::Trap {
backtrace,
pc: pc as usize,
});
self.jmp_buf.get()
}
}