From 8b86f795b734f2cf437bb1ed76fd21d564a9d7ab Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Mon, 10 May 2021 16:42:10 -0700 Subject: [PATCH 01/14] Improved trap handling --- lib/api/src/externals/function.rs | 1 + lib/api/src/module.rs | 3 +- lib/api/src/native.rs | 5 +- lib/api/src/store.rs | 26 +- lib/compiler-cranelift/src/sink.rs | 3 +- lib/engine/src/artifact.rs | 5 +- lib/engine/src/trap/error.rs | 15 +- lib/engine/src/trap/frame_info.rs | 9 + lib/engine/src/trap/mod.rs | 4 +- lib/vm/src/instance/mod.rs | 55 +--- lib/vm/src/libcalls.rs | 6 +- lib/vm/src/table.rs | 6 +- lib/vm/src/trap/helpers.c | 4 +- lib/vm/src/trap/mod.rs | 2 +- lib/vm/src/trap/trapcode.rs | 46 +--- lib/vm/src/trap/traphandlers.rs | 424 ++++++++++++++++++----------- lib/vm/src/vmcontext.rs | 4 +- 17 files changed, 360 insertions(+), 258 deletions(-) diff --git a/lib/api/src/externals/function.rs b/lib/api/src/externals/function.rs index f8a8d1cb35d..aaf91d78bda 100644 --- a/lib/api/src/externals/function.rs +++ b/lib/api/src/externals/function.rs @@ -480,6 +480,7 @@ impl Function { // Call the trampoline. if let Err(error) = unsafe { wasmer_call_trampoline( + &self.store, self.exported.vm_function.vmctx, trampoline, self.exported.vm_function.address, diff --git a/lib/api/src/module.rs b/lib/api/src/module.rs index 8cb8fdd7939..1832849be39 100644 --- a/lib/api/src/module.rs +++ b/lib/api/src/module.rs @@ -275,7 +275,8 @@ impl Module { // of this steps traps, we still need to keep the instance alive // as some of the Instance elements may have placed in other // instance tables. - self.artifact.finish_instantiation(&instance_handle)?; + self.artifact + .finish_instantiation(&self.store, &instance_handle)?; Ok(instance_handle) } diff --git a/lib/api/src/native.rs b/lib/api/src/native.rs index 5f46254534c..3744f559edc 100644 --- a/lib/api/src/native.rs +++ b/lib/api/src/native.rs @@ -131,6 +131,7 @@ macro_rules! impl_native_traits { }; unsafe { wasmer_vm::wasmer_call_trampoline( + &self.store, self.vmctx(), trampoline, self.address(), @@ -145,8 +146,8 @@ macro_rules! impl_native_traits { // TODO: we can probably remove this copy by doing some clever `transmute`s. // we know it's not overlapping because `using_rets_array` is false std::ptr::copy_nonoverlapping(src_pointer, - rets_list, - num_rets); + rets_list, + num_rets); } } Ok(Rets::from_array(rets_list_array)) diff --git a/lib/api/src/store.rs b/lib/api/src/store.rs index 2371fd18b2c..e5f88d5d3d2 100644 --- a/lib/api/src/store.rs +++ b/lib/api/src/store.rs @@ -1,10 +1,12 @@ use crate::tunables::BaseTunables; use loupe::MemoryUsage; +use std::any::Any; use std::fmt; use std::sync::Arc; #[cfg(all(feature = "compiler", feature = "engine"))] use wasmer_compiler::CompilerConfig; -use wasmer_engine::{Engine, Tunables}; +use wasmer_engine::{is_wasm_pc, Engine, Tunables}; +use wasmer_vm::{init_traps, SignalHandler, TrapInfo}; /// The store represents all global state that can be manipulated by /// WebAssembly programs. It consists of the runtime representation @@ -28,10 +30,7 @@ impl Store { where E: Engine + ?Sized, { - Self { - engine: engine.cloned(), - tunables: Arc::new(BaseTunables::for_target(engine.target())), - } + Self::new_with_tunables(engine, BaseTunables::for_target(engine.target())) } /// Creates a new `Store` with a specific [`Engine`] and [`Tunables`]. @@ -39,6 +38,10 @@ impl Store { where E: Engine + ?Sized, { + // Make sure the signal handlers are installed. + // This is required for handling traps. + init_traps(is_wasm_pc); + Self { engine: engine.cloned(), tunables: Arc::new(tunables), @@ -69,6 +72,19 @@ impl PartialEq for Store { } } +unsafe impl TrapInfo for Store { + #[inline] + fn as_any(&self) -> &dyn Any { + self + } + fn custom_signal_handler(&self, call: &dyn Fn(&SignalHandler) -> bool) -> bool { + // if let Some(handler) = &*self.inner.signal_handler.borrow() { + // return call(handler); + // } + false + } +} + // We only implement default if we have assigned a default compiler and engine #[cfg(all(feature = "default-compiler", feature = "default-engine"))] impl Default for Store { diff --git a/lib/compiler-cranelift/src/sink.rs b/lib/compiler-cranelift/src/sink.rs index 54f802912bc..8a0459d2e22 100644 --- a/lib/compiler-cranelift/src/sink.rs +++ b/lib/compiler-cranelift/src/sink.rs @@ -124,8 +124,9 @@ fn translate_ir_trapcode(trap: ir::TrapCode) -> TrapCode { ir::TrapCode::IntegerDivisionByZero => TrapCode::IntegerDivisionByZero, ir::TrapCode::BadConversionToInteger => TrapCode::BadConversionToInteger, ir::TrapCode::UnreachableCodeReached => TrapCode::UnreachableCodeReached, - ir::TrapCode::Interrupt => TrapCode::Interrupt, + ir::TrapCode::Interrupt => unimplemented!("Interrupts not supported"), ir::TrapCode::User(_user_code) => unimplemented!("User trap code not supported"), + // ir::TrapCode::Interrupt => TrapCode::Interrupt, // ir::TrapCode::User(user_code) => TrapCode::User(user_code), } } diff --git a/lib/engine/src/artifact.rs b/lib/engine/src/artifact.rs index a706dbdd75c..56c6f9e5003 100644 --- a/lib/engine/src/artifact.rs +++ b/lib/engine/src/artifact.rs @@ -14,7 +14,7 @@ use wasmer_types::{ }; use wasmer_vm::{ FuncDataRegistry, FunctionBodyPtr, InstanceAllocator, InstanceHandle, MemoryStyle, ModuleInfo, - TableStyle, VMSharedSignatureIndex, VMTrampoline, + TableStyle, TrapInfo, VMSharedSignatureIndex, VMTrampoline, }; /// An `Artifact` is the product that the `Engine` @@ -161,6 +161,7 @@ pub trait Artifact: Send + Sync + Upcastable + MemoryUsage { /// See [`InstanceHandle::finish_instantiation`]. unsafe fn finish_instantiation( &self, + trap_info: &dyn TrapInfo, handle: &InstanceHandle, ) -> Result<(), InstantiationError> { let data_initializers = self @@ -172,7 +173,7 @@ pub trait Artifact: Send + Sync + Upcastable + MemoryUsage { }) .collect::>(); handle - .finish_instantiation(&data_initializers) + .finish_instantiation(trap_info, &data_initializers) .map_err(|trap| InstantiationError::Start(RuntimeError::from_trap(trap))) } } diff --git a/lib/engine/src/trap/error.rs b/lib/engine/src/trap/error.rs index 98c2effeb94..6fadc1ae8be 100644 --- a/lib/engine/src/trap/error.rs +++ b/lib/engine/src/trap/error.rs @@ -78,6 +78,19 @@ impl RuntimeError { ), } } + Trap::OOM { backtrace } => { + unimplemented!("OOM memory errors are not yet handled"); + // match error.downcast::() { + // // The error is already a RuntimeError, we return it directly + // Ok(runtime_error) => *runtime_error, + // Err(e) => Self::new_with_trace( + // &info, + // None, + // RuntimeErrorSource::User(e), + // Backtrace::new_unresolved(), + // ), + // } + } // A trap caused by an error on the generated machine code for a Wasm function Trap::Wasm { pc, @@ -92,7 +105,7 @@ impl RuntimeError { Self::new_with_trace(&info, Some(pc), RuntimeErrorSource::Trap(code), backtrace) } // A trap triggered manually from the Wasmer runtime - Trap::Runtime { + Trap::Lib { trap_code, backtrace, } => Self::new_with_trace(&info, None, RuntimeErrorSource::Trap(trap_code), backtrace), diff --git a/lib/engine/src/trap/frame_info.rs b/lib/engine/src/trap/frame_info.rs index cb155ada5b0..2309f38bfed 100644 --- a/lib/engine/src/trap/frame_info.rs +++ b/lib/engine/src/trap/frame_info.rs @@ -42,6 +42,15 @@ pub struct GlobalFrameInfo { ranges: BTreeMap, } +/// Returns whether the `pc`, according to globally registered information, +/// is a wasm trap or not. +pub fn is_wasm_pc(pc: usize) -> bool { + let frame_info = FRAME_INFO.read().unwrap(); + let module_info = frame_info.module_info(pc); + let is_wasm_pc = module_info.is_some(); + is_wasm_pc +} + /// An RAII structure used to unregister a module's frame information when the /// module is destroyed. #[derive(MemoryUsage)] diff --git a/lib/engine/src/trap/mod.rs b/lib/engine/src/trap/mod.rs index d1eac6b420b..ca7b548c690 100644 --- a/lib/engine/src/trap/mod.rs +++ b/lib/engine/src/trap/mod.rs @@ -2,6 +2,6 @@ mod error; mod frame_info; pub use error::RuntimeError; pub use frame_info::{ - register as register_frame_info, FrameInfo, FunctionExtent, GlobalFrameInfoRegistration, - FRAME_INFO, + is_wasm_pc, register as register_frame_info, FrameInfo, FunctionExtent, + GlobalFrameInfoRegistration, FRAME_INFO, }; diff --git a/lib/vm/src/instance/mod.rs b/lib/vm/src/instance/mod.rs index 5941a57d3a6..38e951b29b8 100644 --- a/lib/vm/src/instance/mod.rs +++ b/lib/vm/src/instance/mod.rs @@ -19,7 +19,7 @@ use crate::global::Global; use crate::imports::Imports; use crate::memory::{Memory, MemoryError}; use crate::table::{Table, TableElement}; -use crate::trap::{catch_traps, init_traps, Trap, TrapCode}; +use crate::trap::{catch_traps, init_traps, Trap, TrapCode, TrapInfo}; use crate::vmcontext::{ VMBuiltinFunctionsArray, VMCallerCheckedAnyfunc, VMContext, VMFunctionBody, VMFunctionEnvironment, VMFunctionImport, VMFunctionKind, VMGlobalDefinition, VMGlobalImport, @@ -98,10 +98,6 @@ pub(crate) struct Instance { /// Hosts can store arbitrary per-instance information here. host_state: Box, - /// Handler run when `SIGBUS`, `SIGFPE`, `SIGILL`, or `SIGSEGV` are caught by the instance thread. - #[loupe(skip)] - pub(crate) signal_handler: Cell>>, - /// Functions to operate on host environments in the imports /// and pointers to the environments. /// @@ -397,7 +393,7 @@ impl Instance { } /// Invoke the WebAssembly start function of the instance, if one is present. - fn invoke_start_function(&self) -> Result<(), Trap> { + fn invoke_start_function(&self, trap_info: &dyn TrapInfo) -> Result<(), Trap> { let start_index = match self.module.start_function { Some(idx) => idx, None => return Ok(()), @@ -426,7 +422,7 @@ impl Instance { // Make the call. unsafe { - catch_traps(callee_vmctx, || { + catch_traps(trap_info, || { mem::transmute::<*const VMFunctionBody, unsafe extern "C" fn(VMFunctionEnvironment)>( callee_address, )(callee_vmctx) @@ -669,7 +665,7 @@ impl Instance { .map_or(true, |n| n as usize > elem.len()) || dst.checked_add(len).map_or(true, |m| m > table.size()) { - return Err(Trap::new_from_runtime(TrapCode::TableAccessOutOfBounds)); + return Err(Trap::lib(TrapCode::TableAccessOutOfBounds)); } for (dst, src) in (dst..dst + len).zip(src..src + len) { @@ -702,7 +698,7 @@ impl Instance { .checked_add(len) .map_or(true, |n| n as usize > table_size) { - return Err(Trap::new_from_runtime(TrapCode::TableAccessOutOfBounds)); + return Err(Trap::lib(TrapCode::TableAccessOutOfBounds)); } for i in start_index..(start_index + len) { @@ -821,7 +817,7 @@ impl Instance { .checked_add(len) .map_or(true, |m| m > memory.current_length) { - return Err(Trap::new_from_runtime(TrapCode::HeapAccessOutOfBounds)); + return Err(Trap::lib(TrapCode::HeapAccessOutOfBounds)); } let src_slice = &data[src as usize..(src + len) as usize]; @@ -935,7 +931,6 @@ impl InstanceHandle { passive_data, host_state, funcrefs, - signal_handler: Cell::new(None), imported_function_envs, vmctx: VMContext {}, }; @@ -1000,9 +995,6 @@ impl InstanceHandle { VMBuiltinFunctionsArray::initialized(), ); - // Ensure that our signal handlers are ready for action. - init_traps(); - // Perform infallible initialization in this constructor, while fallible // initialization is deferred to the `initialize` method. initialize_passive_elements(instance); @@ -1023,6 +1015,7 @@ impl InstanceHandle { /// Only safe to call immediately after instantiation. pub unsafe fn finish_instantiation( &self, + trap_info: &dyn TrapInfo, data_initializers: &[DataInitializer<'_>], ) -> Result<(), Trap> { let instance = self.instance().as_ref(); @@ -1033,7 +1026,7 @@ impl InstanceHandle { // The WebAssembly spec specifies that the start function is // invoked automatically at instantiation time. - instance.invoke_start_function()?; + instance.invoke_start_function(trap_info)?; Ok(()) } @@ -1270,34 +1263,6 @@ impl InstanceHandle { } } -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().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 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))); - } - } - } -} - /// Compute the offset for a memory data initializer. fn get_memory_init_start(init: &DataInitializer<'_>, instance: &Instance) -> usize { let mut start = init.location.offset; @@ -1363,7 +1328,7 @@ fn initialize_tables(instance: &Instance) -> Result<(), Trap> { .checked_add(init.elements.len()) .map_or(true, |end| end > table.size() as usize) { - return Err(Trap::new_from_runtime(TrapCode::TableAccessOutOfBounds)); + return Err(Trap::lib(TrapCode::TableAccessOutOfBounds)); } for (i, func_idx) in init.elements.iter().enumerate() { @@ -1421,7 +1386,7 @@ fn initialize_memories( .checked_add(init.data.len()) .map_or(true, |end| end > memory.current_length.try_into().unwrap()) { - return Err(Trap::new_from_runtime(TrapCode::HeapAccessOutOfBounds)); + return Err(Trap::lib(TrapCode::HeapAccessOutOfBounds)); } unsafe { diff --git a/lib/vm/src/libcalls.rs b/lib/vm/src/libcalls.rs index 1d74e9abc9c..e60704d02ed 100644 --- a/lib/vm/src/libcalls.rs +++ b/lib/vm/src/libcalls.rs @@ -339,7 +339,7 @@ pub unsafe extern "C" fn wasmer_vm_table_get( // TODO: type checking, maybe have specialized accessors match instance.table_get(table_index, elem_index) { Some(table_ref) => table_ref.into(), - None => raise_lib_trap(Trap::new_from_runtime(TrapCode::TableAccessOutOfBounds)), + None => raise_lib_trap(Trap::lib(TrapCode::TableAccessOutOfBounds)), } } @@ -360,7 +360,7 @@ pub unsafe extern "C" fn wasmer_vm_imported_table_get( // TODO: type checking, maybe have specialized accessors match instance.imported_table_get(table_index, elem_index) { Some(table_ref) => table_ref.into(), - None => raise_lib_trap(Trap::new_from_runtime(TrapCode::TableAccessOutOfBounds)), + None => raise_lib_trap(Trap::lib(TrapCode::TableAccessOutOfBounds)), } } @@ -668,7 +668,7 @@ pub unsafe extern "C" fn wasmer_vm_data_drop(vmctx: *mut VMContext, data_index: /// `wasmer_call_trampoline` must have been previously called. #[no_mangle] pub unsafe extern "C" fn wasmer_vm_raise_trap(trap_code: TrapCode) -> ! { - let trap = Trap::new_from_runtime(trap_code); + let trap = Trap::lib(trap_code); raise_lib_trap(trap) } diff --git a/lib/vm/src/table.rs b/lib/vm/src/table.rs index 80ddd656409..6f8abe4fdd1 100644 --- a/lib/vm/src/table.rs +++ b/lib/vm/src/table.rs @@ -83,11 +83,11 @@ pub trait Table: fmt::Debug + Send + Sync + MemoryUsage { .checked_add(len) .map_or(true, |n| n > src_table.size()) { - return Err(Trap::new_from_runtime(TrapCode::TableAccessOutOfBounds)); + return Err(Trap::lib(TrapCode::TableAccessOutOfBounds)); } if dst_index.checked_add(len).map_or(true, |m| m > self.size()) { - return Err(Trap::new_from_runtime(TrapCode::TableAccessOutOfBounds)); + return Err(Trap::lib(TrapCode::TableAccessOutOfBounds)); } let srcs = src_index..src_index + len; @@ -411,7 +411,7 @@ impl Table for LinearTable { Ok(()) } - None => Err(Trap::new_from_runtime(TrapCode::TableAccessOutOfBounds)), + None => Err(Trap::lib(TrapCode::TableAccessOutOfBounds)), } } diff --git a/lib/vm/src/trap/helpers.c b/lib/vm/src/trap/helpers.c index a85324de559..4cfc883b897 100644 --- a/lib/vm/src/trap/helpers.c +++ b/lib/vm/src/trap/helpers.c @@ -3,7 +3,7 @@ #include -int RegisterSetjmp( +int register_setjmp( void **buf_storage, void (*body)(void*), void *payload) { @@ -16,7 +16,7 @@ int RegisterSetjmp( return 1; } -void Unwind(void *JmpBuf) { +void unwind(void *JmpBuf) { jmp_buf *buf = (jmp_buf*) JmpBuf; longjmp(*buf, 1); } diff --git a/lib/vm/src/trap/mod.rs b/lib/vm/src/trap/mod.rs index f21d28144fe..89a891df233 100644 --- a/lib/vm/src/trap/mod.rs +++ b/lib/vm/src/trap/mod.rs @@ -9,6 +9,6 @@ mod traphandlers; pub use trapcode::TrapCode; pub use traphandlers::{ catch_traps, catch_traps_with_result, raise_lib_trap, raise_user_trap, wasmer_call_trampoline, - Trap, + SignalHandler, Trap, TrapInfo, }; pub use traphandlers::{init_traps, resume_panic}; diff --git a/lib/vm/src/trap/trapcode.rs b/lib/vm/src/trap/trapcode.rs index 953008c5d23..5393f8d9981 100644 --- a/lib/vm/src/trap/trapcode.rs +++ b/lib/vm/src/trap/trapcode.rs @@ -61,17 +61,8 @@ pub enum TrapCode { /// Code that was supposed to have been unreachable was reached. UnreachableCodeReached = 10, - /// Execution has potentially run too long and may be interrupted. - /// This trap is resumable. - Interrupt = 11, - /// An atomic memory access was attempted with an unaligned pointer. UnalignedAtomic = 12, - - /// A trap indicating that the runtime was unable to allocate sufficient memory. - VMOutOfMemory = 13, - // /// A user-defined trap code. - // User(u16), } impl TrapCode { @@ -89,10 +80,7 @@ impl TrapCode { Self::IntegerDivisionByZero => "integer divide by zero", Self::BadConversionToInteger => "invalid conversion to integer", Self::UnreachableCodeReached => "unreachable", - Self::Interrupt => "interrupt", Self::UnalignedAtomic => "unaligned atomic access", - Self::VMOutOfMemory => "out of memory", - // Self::User(_) => unreachable!(), } } } @@ -111,10 +99,7 @@ impl Display for TrapCode { Self::IntegerDivisionByZero => "int_divz", Self::BadConversionToInteger => "bad_toint", Self::UnreachableCodeReached => "unreachable", - Self::Interrupt => "interrupt", Self::UnalignedAtomic => "unalign_atom", - Self::VMOutOfMemory => "oom", - // User(x) => return write!(f, "user{}", x), }; f.write_str(identifier) } @@ -124,23 +109,19 @@ impl FromStr for TrapCode { type Err = (); fn from_str(s: &str) -> Result { - use self::TrapCode::*; match s { - "stk_ovf" => Ok(StackOverflow), - "heap_get_oob" => Ok(HeapAccessOutOfBounds), - "heap_misaligned" => Ok(HeapMisaligned), - "table_get_oob" => Ok(TableAccessOutOfBounds), - "oob" => Ok(OutOfBounds), - "icall_null" => Ok(IndirectCallToNull), - "bad_sig" => Ok(BadSignature), - "int_ovf" => Ok(IntegerOverflow), - "int_divz" => Ok(IntegerDivisionByZero), - "bad_toint" => Ok(BadConversionToInteger), - "unreachable" => Ok(UnreachableCodeReached), - "interrupt" => Ok(Interrupt), - "unalign_atom" => Ok(UnalignedAtomic), - "oom" => Ok(VMOutOfMemory), - // _ if s.starts_with("user") => s[4..].parse().map(User).map_err(|_| ()), + "stk_ovf" => Ok(TrapCode::StackOverflow), + "heap_get_oob" => Ok(TrapCode::HeapAccessOutOfBounds), + "heap_misaligned" => Ok(TrapCode::HeapMisaligned), + "table_get_oob" => Ok(TrapCode::TableAccessOutOfBounds), + "oob" => Ok(TrapCode::OutOfBounds), + "icall_null" => Ok(TrapCode::IndirectCallToNull), + "bad_sig" => Ok(TrapCode::BadSignature), + "int_ovf" => Ok(TrapCode::IntegerOverflow), + "int_divz" => Ok(TrapCode::IntegerDivisionByZero), + "bad_toint" => Ok(TrapCode::BadConversionToInteger), + "unreachable" => Ok(TrapCode::UnreachableCodeReached), + "unalign_atom" => Ok(TrapCode::UnalignedAtomic), _ => Err(()), } } @@ -151,7 +132,7 @@ mod tests { use super::*; // Everything but user-defined codes. - const CODES: [TrapCode; 13] = [ + const CODES: [TrapCode; 12] = [ TrapCode::StackOverflow, TrapCode::HeapAccessOutOfBounds, TrapCode::HeapMisaligned, @@ -163,7 +144,6 @@ mod tests { TrapCode::IntegerDivisionByZero, TrapCode::BadConversionToInteger, TrapCode::UnreachableCodeReached, - TrapCode::Interrupt, TrapCode::UnalignedAtomic, ]; diff --git a/lib/vm/src/trap/traphandlers.rs b/lib/vm/src/trap/traphandlers.rs index be2b68ff5e7..3c45900595d 100644 --- a/lib/vm/src/trap/traphandlers.rs +++ b/lib/vm/src/trap/traphandlers.rs @@ -5,24 +5,33 @@ //! signalhandling mechanisms. use super::trapcode::TrapCode; -use crate::instance::{Instance, SignalHandler}; use crate::vmcontext::{VMFunctionBody, VMFunctionEnvironment, VMTrampoline}; use backtrace::Backtrace; use std::any::Any; -use std::cell::Cell; +use std::cell::{Cell, UnsafeCell}; use std::error::Error; use std::io; use std::mem; use std::ptr; use std::sync::Once; +cfg_if::cfg_if! { + if #[cfg(unix)] { + /// Function which may handle custom signals while processing traps. + pub type SignalHandler = dyn Fn(libc::c_int, *const libc::siginfo_t, *const libc::c_void) -> bool; + } else if #[cfg(target_os = "windows")] { + /// Function which may handle custom signals while processing traps. + pub type SignalHandler = dyn Fn(winapi::um::winnt::PEXCEPTION_POINTERS) -> bool; + } +} + extern "C" { - fn RegisterSetjmp( + fn register_setjmp( jmp_buf: *mut *const u8, callback: extern "C" fn(*mut u8), payload: *mut u8, ) -> i32; - fn Unwind(jmp_buf: *const u8) -> !; + fn unwind(jmp_buf: *const u8) -> !; } cfg_if::cfg_if! { @@ -160,7 +169,7 @@ cfg_if::cfg_if! { } else if jmp_buf as usize == 1 { true } else { - Unwind(jmp_buf) + unwind(jmp_buf) } }); @@ -338,44 +347,52 @@ cfg_if::cfg_if! { } else if jmp_buf as usize == 1 { EXCEPTION_CONTINUE_EXECUTION } else { - Unwind(jmp_buf) + unwind(jmp_buf) } }) } } } -/// This function performs the low-overhead signal handler initialization that -/// we want to do eagerly to ensure a more-deterministic global process state. +/// Globally-set callback to determine whether a program counter is actually a +/// wasm trap. /// -/// This is especially relevant for signal handlers since handler ordering -/// depends on installation order: the wasm signal handler must run *before* -/// the other crash handlers and since POSIX signal handlers work LIFO, this -/// function needs to be called at the end of the startup process, after other -/// handlers have been installed. This function can thus be called multiple -/// times, having no effect after the first call. -pub fn init_traps() { - static INIT: Once = Once::new(); - INIT.call_once(real_init); -} +/// This is initialized during `init_traps` below. The definition lives within +/// `wasmer` currently. +static mut IS_WASM_PC: fn(usize) -> bool = |_| false; -fn real_init() { - unsafe { +/// This function is required to be called before any WebAssembly is entered. +/// This will configure global state such as signal handlers to prepare the +/// process to receive wasm traps. +/// +/// This function must not only be called globally once before entering +/// WebAssembly but it must also be called once-per-thread that enters +/// WebAssembly. Currently in wasmer's integration this function is called on +/// creation of a `Store`. +/// +/// The `is_wasm_pc` argument is used when a trap happens to determine if a +/// program counter is the pc of an actual wasm trap or not. This is then used +/// to disambiguate faults that happen due to wasm and faults that happen due to +/// bugs in Rust or elsewhere. +pub fn init_traps(is_wasm_pc: fn(usize) -> bool) { + static INIT: Once = Once::new(); + INIT.call_once(|| unsafe { + IS_WASM_PC = is_wasm_pc; platform_init(); - } + }); } /// Raises a user-defined trap immediately. /// /// This function performs as-if a wasm trap was just executed, only the trap /// has a dynamic payload associated with it which is user-provided. This trap -/// payload is then returned from `wasmer_call` and `wasmer_call_trampoline` -/// below. +/// payload is then returned from `catch_traps` below. /// /// # Safety /// -/// Only safe to call when wasm code is on the stack, aka `wasmer_call` or -/// `wasmer_call_trampoline` must have been previously called. +/// Only safe to call when wasm code is on the stack, aka `catch_traps` must +/// have been previously called. Additionally no Rust destructors can be on the +/// stack. They will be skipped and not executed. pub unsafe fn raise_user_trap(data: Box) -> ! { tls::with(|info| info.unwrap().unwind_with(UnwindReason::UserTrap(data))) } @@ -383,13 +400,13 @@ pub unsafe fn raise_user_trap(data: Box) -> ! { /// Raises a trap from inside library code immediately. /// /// This function performs as-if a wasm trap was just executed. This trap -/// payload is then returned from `wasmer_call` and `wasmer_call_trampoline` -/// below. +/// payload is then returned from `catch_traps` below. /// /// # Safety /// -/// Only safe to call when wasm code is on the stack, aka `wasmer_call` or -/// `wasmer_call_trampoline` must have been previously called. +/// Only safe to call when wasm code is on the stack, aka `catch_traps` must +/// have been previously called. Additionally no Rust destructors can be on the +/// stack. They will be skipped and not executed. pub unsafe fn raise_lib_trap(trap: Trap) -> ! { tls::with(|info| info.unwrap().unwind_with(UnwindReason::LibTrap(trap))) } @@ -399,8 +416,9 @@ pub unsafe fn raise_lib_trap(trap: Trap) -> ! { /// /// # Safety /// -/// Only safe to call when wasm code is on the stack, aka `wasmer_call` or -/// `wasmer_call_trampoline` must have been previously called. +/// Only safe to call when wasm code is on the stack, aka `catch_traps` must +/// have been previously called. Additionally no Rust destructors can be on the +/// stack. They will be skipped and not executed. pub unsafe fn resume_panic(payload: Box) -> ! { tls::with(|info| info.unwrap().unwind_with(UnwindReason::Panic(payload))) } @@ -427,9 +445,11 @@ pub enum Trap { /// A user-raised trap through `raise_user_trap`. User(Box), - /// A trap raised from machine code generated from Wasm + /// A trap raised from the Wasm generated code + /// + /// Note: this trap is deterministic (assuming a deterministic host implementation) Wasm { - /// The program counter in generated code where this trap happened. + /// The program counter in JIT code where this trap happened. pc: usize, /// Native stack backtrace at the time the trap occurred backtrace: Backtrace, @@ -437,45 +457,54 @@ pub enum Trap { signal_trap: Option, }, - /// A trap raised manually from the Wasmer VM - Runtime { + /// A trap raised from a wasm libcall + /// + /// Note: this trap is deterministic (assuming a deterministic host implementation) + Lib { /// Code of the trap. trap_code: TrapCode, /// Native stack backtrace at the time the trap occurred backtrace: Backtrace, }, + + /// A trap indicating that the runtime was unable to allocate sufficient memory. + /// + /// Note: this trap is undeterministic, since it depends on the host system. + OOM { + /// Native stack backtrace at the time the OOM occurred + backtrace: Backtrace, + }, } impl Trap { - /// Construct a new VM `Trap` with the given the program counter, backtrace and an optional - /// trap code associated with the signal received from the kernel. - /// Wasm traps are Traps that are triggered by the chip when running generated - /// code for a Wasm function. - pub fn new_from_wasm(pc: usize, backtrace: Backtrace, signal_trap: Option) -> Self { - Self::Wasm { + /// Construct a new Wasm trap with the given source location and backtrace. + /// + /// Internally saves a backtrace when constructed. + pub fn wasm(pc: usize, backtrace: Backtrace, signal_trap: Option) -> Self { + Trap::Wasm { pc, backtrace, signal_trap, } } - /// Construct a new runtime `Trap` with the given trap code. - /// Runtime traps are Traps that are triggered manually from the VM. + /// Construct a new Wasm trap with the given trap code. /// /// Internally saves a backtrace when constructed. - pub fn new_from_runtime(trap_code: TrapCode) -> Self { + pub fn lib(trap_code: TrapCode) -> Self { let backtrace = Backtrace::new_unresolved(); - Self::Runtime { + Trap::Lib { trap_code, backtrace, } } - /// Construct a new Out of Memory (OOM) `Trap`. + /// Construct a new OOM trap with the given source location and trap code. /// /// Internally saves a backtrace when constructed. - pub fn new_from_user(error: Box) -> Self { - Self::User(error) + pub fn oom() -> Self { + let backtrace = Backtrace::new_unresolved(); + Trap::OOM { backtrace } } } @@ -495,34 +524,29 @@ impl Trap { /// Wildly unsafe because it calls raw function pointers and reads/writes raw /// function pointers. pub unsafe fn wasmer_call_trampoline( + trap_info: &impl TrapInfo, vmctx: VMFunctionEnvironment, trampoline: VMTrampoline, callee: *const VMFunctionBody, values_vec: *mut u8, ) -> Result<(), Trap> { - catch_traps(vmctx, || { + catch_traps(trap_info, || { mem::transmute::<_, extern "C" fn(VMFunctionEnvironment, *const VMFunctionBody, *mut u8)>( trampoline, - )(vmctx, callee, values_vec) + )(vmctx, callee, values_vec); }) } /// Catches any wasm traps that happen within the execution of `closure`, /// returning them as a `Result`. /// -/// # Safety -/// -/// Highly unsafe since `closure` won't have any destructors run. -pub unsafe fn catch_traps(vmctx: VMFunctionEnvironment, mut closure: F) -> Result<(), Trap> +/// Highly unsafe since `closure` won't have any dtors run. +pub unsafe fn catch_traps(trap_info: &dyn TrapInfo, mut closure: F) -> Result<(), Trap> where F: FnMut(), { - // Ensure that we have our sigaltstack installed. - #[cfg(unix)] - setup_unix_sigaltstack()?; - - return CallThreadState::new(vmctx).with(|cx| { - RegisterSetjmp( + return CallThreadState::new(trap_info).with(|cx| { + register_setjmp( cx.jmp_buf.as_ptr(), call_closure::, &mut closure as *mut F as *mut u8, @@ -547,14 +571,14 @@ where /// /// Check [`catch_traps`]. pub unsafe fn catch_traps_with_result( - vmctx: VMFunctionEnvironment, + trap_info: &dyn TrapInfo, mut closure: F, ) -> Result where F: FnMut() -> R, { - let mut global_results = mem::MaybeUninit::::uninit(); - catch_traps(vmctx, || { + let mut global_results = MaybeUninit::::uninit(); + catch_traps(trap_info, || { global_results.as_mut_ptr().write(closure()); })?; Ok(global_results.assume_init()) @@ -562,91 +586,89 @@ where /// Temporary state stored on the stack which is registered in the `tls` module /// below for calls into wasm. -pub struct CallThreadState { - unwind: Cell, +pub struct CallThreadState<'a> { + unwind: UnsafeCell>, jmp_buf: Cell<*const u8>, reset_guard_page: Cell, - prev: Option<*const CallThreadState>, - vmctx: VMFunctionEnvironment, + prev: Cell, + trap_info: &'a (dyn TrapInfo + 'a), handling_trap: Cell, } +/// A package of functionality needed by `catch_traps` to figure out what to do +/// when handling a trap. +/// +/// Note that this is an `unsafe` trait at least because it's being run in the +/// context of a synchronous signal handler, so it needs to be careful to not +/// access too much state in answering these queries. +pub unsafe trait TrapInfo { + /// Converts this object into an `Any` to dynamically check its type. + fn as_any(&self) -> &dyn Any; + + /// Uses `call` to call a custom signal handler, if one is specified. + /// + /// Returns `true` if `call` returns true, otherwise returns `false`. + fn custom_signal_handler(&self, call: &dyn Fn(&SignalHandler) -> bool) -> bool; +} + enum UnwindReason { - None, + /// A panic caused by the host Panic(Box), + /// A custom error triggered by the user UserTrap(Box), + /// A Trap triggered by a wasm libcall LibTrap(Trap), - RuntimeTrap { + /// A trap caused by the Wasm generated code + WasmTrap { backtrace: Backtrace, pc: usize, signal_trap: Option, }, } -impl CallThreadState { - fn new(vmctx: VMFunctionEnvironment) -> Self { +impl<'a> CallThreadState<'a> { + #[inline] + fn new(trap_info: &'a (dyn TrapInfo + 'a)) -> CallThreadState<'a> { Self { - unwind: Cell::new(UnwindReason::None), - vmctx, + unwind: UnsafeCell::new(MaybeUninit::uninit()), jmp_buf: Cell::new(ptr::null()), reset_guard_page: Cell::new(false), - prev: None, + prev: Cell::new(ptr::null()), + trap_info, handling_trap: Cell::new(false), } } - fn with(mut self, closure: impl FnOnce(&Self) -> i32) -> Result<(), Trap> { - tls::with(|prev| { - self.prev = prev.map(|p| p as *const _); - let ret = tls::set(&self, || closure(&self)); - match self.unwind.replace(UnwindReason::None) { - UnwindReason::None => { - debug_assert_eq!(ret, 1); - Ok(()) - } - UnwindReason::UserTrap(data) => { - debug_assert_eq!(ret, 0); - Err(Trap::new_from_user(data)) - } - UnwindReason::LibTrap(trap) => Err(trap), - UnwindReason::RuntimeTrap { - backtrace, - pc, - signal_trap, - } => { - debug_assert_eq!(ret, 0); - Err(Trap::new_from_wasm(pc, backtrace, signal_trap)) - } - UnwindReason::Panic(panic) => { - debug_assert_eq!(ret, 0); - std::panic::resume_unwind(panic) - } + fn with(mut self, closure: impl FnOnce(&CallThreadState) -> i32) -> Result<(), Trap> { + let ret = tls::set(&self, || closure(&self))?; + if ret != 0 { + return Ok(()); + } + match unsafe { (*self.unwind.get()).as_ptr().read() } { + UnwindReason::UserTrap(data) => { + debug_assert_eq!(ret, 0); + Err(Trap::User(data)) } - }) - } - - fn any_instance(&self, func: impl Fn(&Instance) -> bool) -> bool { - unsafe { - if func( - self.vmctx - .vmctx - .as_ref() - .expect("`VMContext` is null in `any_instance`") - .instance(), - ) { - return true; + UnwindReason::LibTrap(trap) => Err(trap), + UnwindReason::WasmTrap { + backtrace, + pc, + signal_trap, + } => { + debug_assert_eq!(ret, 0); + Err(Trap::wasm(pc, backtrace, signal_trap)) } - match self.prev { - Some(prev) => (*prev).any_instance(func), - None => false, + UnwindReason::Panic(panic) => { + debug_assert_eq!(ret, 0); + std::panic::resume_unwind(panic) } } } fn unwind_with(&self, reason: UnwindReason) -> ! { - self.unwind.replace(reason); unsafe { - Unwind(self.jmp_buf.get()); + (*self.unwind.get()).as_mut_ptr().write(reason); + unwind(self.jmp_buf.get()); } } @@ -683,21 +705,10 @@ impl CallThreadState { return ptr::null(); } - // 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 + // First up see if we have a custom trap handler, + // in which case run it. If anything handles the trap then we // return that the trap was handled. - 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); - instance.signal_handler.set(Some(handler)); - result - }); - - if any_instance { - self.handling_trap.set(false); + if self.trap_info.custom_signal_handler(&call_handler) { return 1 as *const _; } @@ -714,17 +725,21 @@ impl CallThreadState { } let backtrace = Backtrace::new_unresolved(); self.reset_guard_page.set(reset_guard_page); - self.unwind.replace(UnwindReason::RuntimeTrap { - backtrace, - signal_trap, - pc: pc as usize, - }); + unsafe { + (*self.unwind.get()) + .as_mut_ptr() + .write(UnwindReason::WasmTrap { + backtrace, + signal_trap, + pc: pc as usize, + }); + } self.handling_trap.set(false); self.jmp_buf.get() } } -impl Drop for CallThreadState { +impl<'a> Drop for CallThreadState<'a> { fn drop(&mut self) { if self.reset_guard_page.get() { reset_guard_page(); @@ -739,39 +754,138 @@ impl Drop for CallThreadState { // the caller to the trap site. mod tls { use super::CallThreadState; - use std::cell::Cell; + use crate::Trap; + use std::mem; use std::ptr; - thread_local!(static PTR: Cell<*const CallThreadState> = Cell::new(ptr::null())); + pub use raw::Ptr; + + // An even *more* inner module for dealing with TLS. This actually has the + // thread local variable and has functions to access the variable. + // + // Note that this is specially done to fully encapsulate that the accessors + // for tls must not be inlined. Wasmtime's async support employs stack + // switching which can resume execution on different OS threads. This means + // that borrows of our TLS pointer must never live across accesses because + // otherwise the access may be split across two threads and cause unsafety. + // + // This also means that extra care is taken by the runtime to save/restore + // these TLS values when the runtime may have crossed threads. + mod raw { + use super::CallThreadState; + use crate::Trap; + use std::cell::Cell; + use std::ptr; + + pub type Ptr = *const CallThreadState<'static>; + + // The first entry here is the `Ptr` which is what's used as part of the + // public interface of this module. The second entry is a boolean which + // allows the runtime to perform per-thread initialization if necessary + // for handling traps (e.g. setting up ports on macOS and sigaltstack on + // Unix). + thread_local!(static PTR: Cell<(Ptr, bool)> = Cell::new((ptr::null(), false))); + + #[inline(never)] // see module docs for why this is here + pub fn replace(val: Ptr) -> Result { + PTR.with(|p| { + // When a new value is configured that means that we may be + // entering WebAssembly so check to see if this thread has + // performed per-thread initialization for traps. + let (prev, mut initialized) = p.get(); + if !initialized { + super::super::lazy_per_thread_init()?; + initialized = true; + } + p.set((val, initialized)); + Ok(prev) + }) + } + + #[inline(never)] // see module docs for why this is here + pub fn get() -> Ptr { + PTR.with(|p| p.get().0) + } + } + + /// Opaque state used to help control TLS state across stack switches for + /// async support. + pub struct TlsRestore(raw::Ptr); + + impl TlsRestore { + /// Takes the TLS state that is currently configured and returns a + /// token that is used to replace it later. + /// + /// This is not a safe operation since it's intended to only be used + /// with stack switching found with fibers and async wasmer. + pub unsafe fn take() -> Result { + // Our tls pointer must be set at this time, and it must not be + // null. We need to restore the previous pointer since we're + // removing ourselves from the call-stack, and in the process we + // null out our own previous field for safety in case it's + // accidentally used later. + let raw = raw::get(); + assert!(!raw.is_null()); + let prev = (*raw).prev.replace(ptr::null()); + raw::replace(prev)?; + Ok(TlsRestore(raw)) + } + + /// Restores a previous tls state back into this thread's TLS. + /// + /// This is unsafe because it's intended to only be used within the + /// context of stack switching within wasmtime. + pub unsafe fn replace(self) -> Result<(), super::Trap> { + // We need to configure our previous TLS pointer to whatever is in + // TLS at this time, and then we set the current state to ourselves. + let prev = raw::get(); + assert!((*self.0).prev.get().is_null()); + (*self.0).prev.set(prev); + raw::replace(self.0)?; + Ok(()) + } + } /// Configures thread local state such that for the duration of the /// execution of `closure` any call to `with` will yield `ptr`, unless this /// is recursively called again. - pub fn set(ptr: &CallThreadState, closure: impl FnOnce() -> R) -> R { - struct Reset<'a, T: Copy>(&'a Cell, T); + pub fn set(state: &CallThreadState<'_>, closure: impl FnOnce() -> R) -> Result { + struct Reset<'a, 'b>(&'a CallThreadState<'b>); - impl Drop for Reset<'_, T> { + impl Drop for Reset<'_, '_> { + #[inline] fn drop(&mut self) { - self.0.set(self.1); + raw::replace(self.0.prev.replace(ptr::null())) + .expect("tls should be previously initialized"); } } - PTR.with(|p| { - let _r = Reset(p, p.replace(ptr)); - closure() - }) + // Note that this extension of the lifetime to `'static` should be + // safe because we only ever access it below with an anonymous + // lifetime, meaning `'static` never leaks out of this module. + let ptr = unsafe { + mem::transmute::<*const CallThreadState<'_>, *const CallThreadState<'static>>(state) + }; + let prev = raw::replace(ptr)?; + state.prev.set(prev); + let _reset = Reset(state); + Ok(closure()) } /// Returns the last pointer configured with `set` above. Panics if `set` /// has not been previously called. - pub fn with(closure: impl FnOnce(Option<&CallThreadState>) -> R) -> R { - PTR.with(|ptr| { - let p = ptr.get(); - unsafe { closure(if p.is_null() { None } else { Some(&*p) }) } - }) + pub fn with(closure: impl FnOnce(Option<&CallThreadState<'_>>) -> R) -> R { + let p = raw::get(); + unsafe { closure(if p.is_null() { None } else { Some(&*p) }) } } } +#[cfg(not(unix))] +pub fn lazy_per_thread_init() -> Result<(), Trap> { + // Unused on Windows + Ok(()) +} + /// A module for registering a custom alternate signal stack (sigaltstack). /// /// Rust's libstd installs an alternate stack with size `SIGSTKSZ`, which is not @@ -779,7 +893,7 @@ mod tls { /// and registering our own alternate stack that is large enough and has a guard /// page. #[cfg(unix)] -fn setup_unix_sigaltstack() -> Result<(), Trap> { +pub fn lazy_per_thread_init() -> Result<(), Trap> { use std::cell::RefCell; use std::ptr::null_mut; @@ -834,7 +948,7 @@ fn setup_unix_sigaltstack() -> Result<(), Trap> { 0, ); if ptr == libc::MAP_FAILED { - return Err(Trap::new_from_runtime(TrapCode::VMOutOfMemory)); + return Err(Trap::oom()); } // Prepare the stack with readable/writable memory and then register it diff --git a/lib/vm/src/vmcontext.rs b/lib/vm/src/vmcontext.rs index 9d294e9fcc1..ec7d6ca06bf 100644 --- a/lib/vm/src/vmcontext.rs +++ b/lib/vm/src/vmcontext.rs @@ -384,7 +384,7 @@ impl VMMemoryDefinition { .checked_add(len) .map_or(true, |m| m > self.current_length) { - return Err(Trap::new_from_runtime(TrapCode::HeapAccessOutOfBounds)); + return Err(Trap::lib(TrapCode::HeapAccessOutOfBounds)); } let dst = usize::try_from(dst).unwrap(); @@ -414,7 +414,7 @@ impl VMMemoryDefinition { .checked_add(len) .map_or(true, |m| m > self.current_length) { - return Err(Trap::new_from_runtime(TrapCode::HeapAccessOutOfBounds)); + return Err(Trap::lib(TrapCode::HeapAccessOutOfBounds)); } let dst = isize::try_from(dst).unwrap(); From b5ce9b5c6e5adef565ed5fc9468589d0d4489133 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Mon, 10 May 2021 16:50:40 -0700 Subject: [PATCH 02/14] Fixed linting --- lib/api/src/store.rs | 2 +- lib/engine/src/trap/error.rs | 2 +- lib/vm/src/instance/mod.rs | 4 ++-- lib/vm/src/trap/mod.rs | 2 +- lib/vm/src/trap/traphandlers.rs | 3 ++- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/api/src/store.rs b/lib/api/src/store.rs index e5f88d5d3d2..da956bc1812 100644 --- a/lib/api/src/store.rs +++ b/lib/api/src/store.rs @@ -77,7 +77,7 @@ unsafe impl TrapInfo for Store { fn as_any(&self) -> &dyn Any { self } - fn custom_signal_handler(&self, call: &dyn Fn(&SignalHandler) -> bool) -> bool { + fn custom_signal_handler(&self, _call: &dyn Fn(&SignalHandler) -> bool) -> bool { // if let Some(handler) = &*self.inner.signal_handler.borrow() { // return call(handler); // } diff --git a/lib/engine/src/trap/error.rs b/lib/engine/src/trap/error.rs index 6fadc1ae8be..cb5a4a4355f 100644 --- a/lib/engine/src/trap/error.rs +++ b/lib/engine/src/trap/error.rs @@ -78,7 +78,7 @@ impl RuntimeError { ), } } - Trap::OOM { backtrace } => { + Trap::OOM { backtrace: _ } => { unimplemented!("OOM memory errors are not yet handled"); // match error.downcast::() { // // The error is already a RuntimeError, we return it directly diff --git a/lib/vm/src/instance/mod.rs b/lib/vm/src/instance/mod.rs index 38e951b29b8..99b7b755754 100644 --- a/lib/vm/src/instance/mod.rs +++ b/lib/vm/src/instance/mod.rs @@ -19,7 +19,7 @@ use crate::global::Global; use crate::imports::Imports; use crate::memory::{Memory, MemoryError}; use crate::table::{Table, TableElement}; -use crate::trap::{catch_traps, init_traps, Trap, TrapCode, TrapInfo}; +use crate::trap::{catch_traps, Trap, TrapCode, TrapInfo}; use crate::vmcontext::{ VMBuiltinFunctionsArray, VMCallerCheckedAnyfunc, VMContext, VMFunctionBody, VMFunctionEnvironment, VMFunctionImport, VMFunctionKind, VMGlobalDefinition, VMGlobalImport, @@ -32,7 +32,7 @@ use loupe::{MemoryUsage, MemoryUsageTracker}; use memoffset::offset_of; use more_asserts::assert_lt; use std::any::Any; -use std::cell::{Cell, RefCell}; +use std::cell::RefCell; use std::collections::HashMap; use std::convert::{TryFrom, TryInto}; use std::ffi; diff --git a/lib/vm/src/trap/mod.rs b/lib/vm/src/trap/mod.rs index 89a891df233..a885798f666 100644 --- a/lib/vm/src/trap/mod.rs +++ b/lib/vm/src/trap/mod.rs @@ -9,6 +9,6 @@ mod traphandlers; pub use trapcode::TrapCode; pub use traphandlers::{ catch_traps, catch_traps_with_result, raise_lib_trap, raise_user_trap, wasmer_call_trampoline, - SignalHandler, Trap, TrapInfo, + SignalHandler, TlsRestore, Trap, TrapInfo, }; pub use traphandlers::{init_traps, resume_panic}; diff --git a/lib/vm/src/trap/traphandlers.rs b/lib/vm/src/trap/traphandlers.rs index 3c45900595d..5ba4c703217 100644 --- a/lib/vm/src/trap/traphandlers.rs +++ b/lib/vm/src/trap/traphandlers.rs @@ -14,6 +14,7 @@ use std::io; use std::mem; use std::ptr; use std::sync::Once; +pub use tls::TlsRestore; cfg_if::cfg_if! { if #[cfg(unix)] { @@ -639,7 +640,7 @@ impl<'a> CallThreadState<'a> { } } - fn with(mut self, closure: impl FnOnce(&CallThreadState) -> i32) -> Result<(), Trap> { + fn with(self, closure: impl FnOnce(&CallThreadState) -> i32) -> Result<(), Trap> { let ret = tls::set(&self, || closure(&self))?; if ret != 0 { return Ok(()); From 1863fcbc201977c863a4caf03887a11589855ea6 Mon Sep 17 00:00:00 2001 From: Syrus Date: Mon, 10 May 2021 17:27:07 -0700 Subject: [PATCH 03/14] Implement unimplemented trap oom/handler code --- lib/api/src/store.rs | 5 ++--- lib/engine/src/trap/error.rs | 18 ++++++------------ 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/lib/api/src/store.rs b/lib/api/src/store.rs index da956bc1812..60143643a54 100644 --- a/lib/api/src/store.rs +++ b/lib/api/src/store.rs @@ -78,9 +78,8 @@ unsafe impl TrapInfo for Store { self } fn custom_signal_handler(&self, _call: &dyn Fn(&SignalHandler) -> bool) -> bool { - // if let Some(handler) = &*self.inner.signal_handler.borrow() { - // return call(handler); - // } + // Setting a custom handler is implemented for now, please open an issue + // in the Wasmer Github repo if you are interested in this. false } } diff --git a/lib/engine/src/trap/error.rs b/lib/engine/src/trap/error.rs index cb5a4a4355f..08535c1488c 100644 --- a/lib/engine/src/trap/error.rs +++ b/lib/engine/src/trap/error.rs @@ -16,6 +16,7 @@ pub struct RuntimeError { #[derive(Debug)] enum RuntimeErrorSource { Generic(String), + OOM, User(Box), Trap(TrapCode), } @@ -25,6 +26,7 @@ impl fmt::Display for RuntimeErrorSource { match self { Self::Generic(s) => write!(f, "{}", s), Self::User(s) => write!(f, "{}", s), + Self::OOM => write!(f, "Wasmer VM out of memory"), Self::Trap(s) => write!(f, "{}", s.message()), } } @@ -66,6 +68,7 @@ impl RuntimeError { pub fn from_trap(trap: Trap) -> Self { let info = FRAME_INFO.read().unwrap(); match trap { + // A user error Trap::User(error) => { match error.downcast::() { // The error is already a RuntimeError, we return it directly @@ -78,18 +81,9 @@ impl RuntimeError { ), } } - Trap::OOM { backtrace: _ } => { - unimplemented!("OOM memory errors are not yet handled"); - // match error.downcast::() { - // // The error is already a RuntimeError, we return it directly - // Ok(runtime_error) => *runtime_error, - // Err(e) => Self::new_with_trace( - // &info, - // None, - // RuntimeErrorSource::User(e), - // Backtrace::new_unresolved(), - // ), - // } + // A trap caused by the VM being Out of Memory + Trap::OOM { backtrace } => { + Self::new_with_trace(&info, None, RuntimeErrorSource::OOM, backtrace) } // A trap caused by an error on the generated machine code for a Wasm function Trap::Wasm { From f39f9502a2a9615db416b2cdecc2300c1df56c70 Mon Sep 17 00:00:00 2001 From: Syrus Date: Mon, 10 May 2021 18:27:33 -0700 Subject: [PATCH 04/14] Use sigsetjmp/siglongjmp in linux --- lib/vm/build.rs | 16 +++++++++++++--- lib/vm/src/trap/handlers.c | 37 +++++++++++++++++++++++++++++++++++++ lib/vm/src/trap/helpers.c | 22 ---------------------- 3 files changed, 50 insertions(+), 25 deletions(-) create mode 100644 lib/vm/src/trap/handlers.c delete mode 100644 lib/vm/src/trap/helpers.c diff --git a/lib/vm/build.rs b/lib/vm/build.rs index 206d9927638..278eb570db7 100644 --- a/lib/vm/build.rs +++ b/lib/vm/build.rs @@ -1,9 +1,19 @@ //! Runtime build script compiles C code using setjmp for trap handling. +use std::env; + fn main() { - println!("cargo:rerun-if-changed=src/trap/helpers.c"); + println!("cargo:rerun-if-changed=src/trap/handlers.c"); + cc::Build::new() .warnings(true) - .file("src/trap/helpers.c") - .compile("helpers"); + .define( + &format!( + "CFG_TARGET_OS_{}", + env::var("CARGO_CFG_TARGET_OS").unwrap().to_uppercase() + ), + None, + ) + .file("src/trap/handlers.c") + .compile("handlers"); } diff --git a/lib/vm/src/trap/handlers.c b/lib/vm/src/trap/handlers.c new file mode 100644 index 00000000000..0cbb33efe6d --- /dev/null +++ b/lib/vm/src/trap/handlers.c @@ -0,0 +1,37 @@ +// This file contains partial code from other sources. +// Attributions: https://github.com/wasmerio/wasmer/blob/master/ATTRIBUTIONS.md + +#include + +// Note that `sigsetjmp` and `siglongjmp` are used here where possible to +// explicitly pass a 0 argument to `sigsetjmp` that we don't need to preserve +// the process signal mask. This should make this call a bit faster b/c it +// doesn't need to touch the kernel signal handling routines. +// In case of macOS, stackoverflow +#if defined(CFG_TARGET_OS_WINDOWS) || defined(CFG_TARGET_OS_MACOS) +#define platform_setjmp(buf) setjmp(buf) +#define platform_longjmp(buf, arg) longjmp(buf, arg) +#define platform_jmp_buf jmp_buf +#else +#define platform_setjmp(buf) sigsetjmp(buf, 0) +#define platform_longjmp(buf, arg) siglongjmp(buf, arg) +#define platform_jmp_buf sigjmp_buf +#endif + +int register_setjmp( + void **buf_storage, + void (*body)(void*), + void *payload) { + platform_jmp_buf buf; + if (platform_setjmp(buf) != 0) { + return 0; + } + *buf_storage = &buf; + body(payload); + return 1; +} + +void unwind(void *JmpBuf) { + platform_jmp_buf *buf = (platform_jmp_buf*) JmpBuf; + platform_longjmp(*buf, 1); +} diff --git a/lib/vm/src/trap/helpers.c b/lib/vm/src/trap/helpers.c deleted file mode 100644 index 4cfc883b897..00000000000 --- a/lib/vm/src/trap/helpers.c +++ /dev/null @@ -1,22 +0,0 @@ -// This file contains partial code from other sources. -// Attributions: https://github.com/wasmerio/wasmer/blob/master/ATTRIBUTIONS.md - -#include - -int register_setjmp( - void **buf_storage, - void (*body)(void*), - void *payload) { - jmp_buf buf; - if (setjmp(buf) != 0) { - return 0; - } - *buf_storage = &buf; - body(payload); - return 1; -} - -void unwind(void *JmpBuf) { - jmp_buf *buf = (jmp_buf*) JmpBuf; - longjmp(*buf, 1); -} From d858b15c622a427c3e114dbdfdc2c2e039671ad5 Mon Sep 17 00:00:00 2001 From: Syrus Date: Mon, 10 May 2021 18:28:46 -0700 Subject: [PATCH 05/14] Move MaybeUninit to the top --- lib/vm/src/trap/traphandlers.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/vm/src/trap/traphandlers.rs b/lib/vm/src/trap/traphandlers.rs index 5ba4c703217..848d99d19dd 100644 --- a/lib/vm/src/trap/traphandlers.rs +++ b/lib/vm/src/trap/traphandlers.rs @@ -11,7 +11,7 @@ use std::any::Any; use std::cell::{Cell, UnsafeCell}; use std::error::Error; use std::io; -use std::mem; +use std::mem::{self, MaybeUninit}; use std::ptr; use std::sync::Once; pub use tls::TlsRestore; @@ -37,8 +37,6 @@ extern "C" { cfg_if::cfg_if! { if #[cfg(unix)] { - use std::mem::MaybeUninit; - static mut PREV_SIGSEGV: MaybeUninit = MaybeUninit::uninit(); static mut PREV_SIGBUS: MaybeUninit = MaybeUninit::uninit(); static mut PREV_SIGILL: MaybeUninit = MaybeUninit::uninit(); @@ -765,7 +763,7 @@ mod tls { // thread local variable and has functions to access the variable. // // Note that this is specially done to fully encapsulate that the accessors - // for tls must not be inlined. Wasmtime's async support employs stack + // for tls must not be inlined. Wasmer's async support will employ stack // switching which can resume execution on different OS threads. This means // that borrows of our TLS pointer must never live across accesses because // otherwise the access may be split across two threads and cause unsafety. @@ -835,7 +833,7 @@ mod tls { /// Restores a previous tls state back into this thread's TLS. /// /// This is unsafe because it's intended to only be used within the - /// context of stack switching within wasmtime. + /// context of stack switching within wasmer. pub unsafe fn replace(self) -> Result<(), super::Trap> { // We need to configure our previous TLS pointer to whatever is in // TLS at this time, and then we set the current state to ourselves. From f85382d07fdd057b3f5a2ab73b1790b1e5ab83a8 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Tue, 11 May 2021 11:13:35 -0700 Subject: [PATCH 06/14] Not use unwind feature --- lib/compiler-cranelift/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compiler-cranelift/Cargo.toml b/lib/compiler-cranelift/Cargo.toml index d6c45996984..d39d07aeacb 100644 --- a/lib/compiler-cranelift/Cargo.toml +++ b/lib/compiler-cranelift/Cargo.toml @@ -36,7 +36,7 @@ lazy_static = "1.4" maintenance = { status = "actively-developed" } [features] -default = ["std", "enable-serde", "unwind"] +default = ["std", "enable-serde"] unwind = ["cranelift-codegen/unwind", "gimli"] enable-serde = ["wasmer-compiler/enable-serde", "wasmer-types/enable-serde", "cranelift-entity/enable-serde"] std = ["cranelift-codegen/std", "cranelift-frontend/std", "wasmer-compiler/std", "wasmer-types/std"] From 90135576fae836e877c4975bcb6e58bc51a3e4e5 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Tue, 11 May 2021 11:14:12 -0700 Subject: [PATCH 07/14] Split debug inco in macos --- Cargo.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index c8dffcea9b3..4f280e31b8b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -148,6 +148,9 @@ test-jit = [ # that raise signals because that interferes with tarpaulin. coverage = [] +[profile.dev] +split-debuginfo = "unpacked" + [[bench]] name = "static_and_dynamic_functions" harness = false From f3384bb912214f4d174fc6eafd5dc6c8f61eb9ce Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Tue, 11 May 2021 13:08:00 -0700 Subject: [PATCH 08/14] Address comments --- lib/compiler-cranelift/Cargo.toml | 2 +- lib/engine/src/trap/frame_info.rs | 5 ++--- lib/vm/src/trap/handlers.c | 4 ++-- lib/vm/src/trap/trapcode.rs | 2 +- lib/vm/src/trap/traphandlers.rs | 34 +++++++++++++++++-------------- 5 files changed, 25 insertions(+), 22 deletions(-) diff --git a/lib/compiler-cranelift/Cargo.toml b/lib/compiler-cranelift/Cargo.toml index d39d07aeacb..d6c45996984 100644 --- a/lib/compiler-cranelift/Cargo.toml +++ b/lib/compiler-cranelift/Cargo.toml @@ -36,7 +36,7 @@ lazy_static = "1.4" maintenance = { status = "actively-developed" } [features] -default = ["std", "enable-serde"] +default = ["std", "enable-serde", "unwind"] unwind = ["cranelift-codegen/unwind", "gimli"] enable-serde = ["wasmer-compiler/enable-serde", "wasmer-types/enable-serde", "cranelift-entity/enable-serde"] std = ["cranelift-codegen/std", "cranelift-frontend/std", "wasmer-compiler/std", "wasmer-types/std"] diff --git a/lib/engine/src/trap/frame_info.rs b/lib/engine/src/trap/frame_info.rs index 2309f38bfed..90422018abe 100644 --- a/lib/engine/src/trap/frame_info.rs +++ b/lib/engine/src/trap/frame_info.rs @@ -45,10 +45,9 @@ pub struct GlobalFrameInfo { /// Returns whether the `pc`, according to globally registered information, /// is a wasm trap or not. pub fn is_wasm_pc(pc: usize) -> bool { - let frame_info = FRAME_INFO.read().unwrap(); + let frame_info = FRAME_INFO.read().expect("can't acquire the FrameInfo lock"); let module_info = frame_info.module_info(pc); - let is_wasm_pc = module_info.is_some(); - is_wasm_pc + module_info.is_some() } /// An RAII structure used to unregister a module's frame information when the diff --git a/lib/vm/src/trap/handlers.c b/lib/vm/src/trap/handlers.c index 0cbb33efe6d..6ae17f72dff 100644 --- a/lib/vm/src/trap/handlers.c +++ b/lib/vm/src/trap/handlers.c @@ -18,7 +18,7 @@ #define platform_jmp_buf sigjmp_buf #endif -int register_setjmp( +int wasmer_register_setjmp( void **buf_storage, void (*body)(void*), void *payload) { @@ -31,7 +31,7 @@ int register_setjmp( return 1; } -void unwind(void *JmpBuf) { +void wasmer_unwind(void *JmpBuf) { platform_jmp_buf *buf = (platform_jmp_buf*) JmpBuf; platform_longjmp(*buf, 1); } diff --git a/lib/vm/src/trap/trapcode.rs b/lib/vm/src/trap/trapcode.rs index 5393f8d9981..1fd02176401 100644 --- a/lib/vm/src/trap/trapcode.rs +++ b/lib/vm/src/trap/trapcode.rs @@ -62,7 +62,7 @@ pub enum TrapCode { UnreachableCodeReached = 10, /// An atomic memory access was attempted with an unaligned pointer. - UnalignedAtomic = 12, + UnalignedAtomic = 11, } impl TrapCode { diff --git a/lib/vm/src/trap/traphandlers.rs b/lib/vm/src/trap/traphandlers.rs index 848d99d19dd..4108fe2f2af 100644 --- a/lib/vm/src/trap/traphandlers.rs +++ b/lib/vm/src/trap/traphandlers.rs @@ -27,12 +27,12 @@ cfg_if::cfg_if! { } extern "C" { - fn register_setjmp( + fn wasmer_register_setjmp( jmp_buf: *mut *const u8, callback: extern "C" fn(*mut u8), payload: *mut u8, ) -> i32; - fn unwind(jmp_buf: *const u8) -> !; + fn wasmer_unwind(jmp_buf: *const u8) -> !; } cfg_if::cfg_if! { @@ -168,7 +168,7 @@ cfg_if::cfg_if! { } else if jmp_buf as usize == 1 { true } else { - unwind(jmp_buf) + wasmer_unwind(jmp_buf) } }); @@ -346,7 +346,7 @@ cfg_if::cfg_if! { } else if jmp_buf as usize == 1 { EXCEPTION_CONTINUE_EXECUTION } else { - unwind(jmp_buf) + wasmer_unwind(jmp_buf) } }) } @@ -390,8 +390,9 @@ pub fn init_traps(is_wasm_pc: fn(usize) -> bool) { /// # Safety /// /// Only safe to call when wasm code is on the stack, aka `catch_traps` must -/// have been previously called. Additionally no Rust destructors can be on the -/// stack. They will be skipped and not executed. +/// have been previous called and not yet returned. +/// Additionally no Rust destructors may be on the stack. +/// They will be skipped and not executed. pub unsafe fn raise_user_trap(data: Box) -> ! { tls::with(|info| info.unwrap().unwind_with(UnwindReason::UserTrap(data))) } @@ -404,8 +405,9 @@ pub unsafe fn raise_user_trap(data: Box) -> ! { /// # Safety /// /// Only safe to call when wasm code is on the stack, aka `catch_traps` must -/// have been previously called. Additionally no Rust destructors can be on the -/// stack. They will be skipped and not executed. +/// have been previous called and not yet returned. +/// Additionally no Rust destructors may be on the stack. +/// They will be skipped and not executed. pub unsafe fn raise_lib_trap(trap: Trap) -> ! { tls::with(|info| info.unwrap().unwind_with(UnwindReason::LibTrap(trap))) } @@ -448,7 +450,7 @@ pub enum Trap { /// /// Note: this trap is deterministic (assuming a deterministic host implementation) Wasm { - /// The program counter in JIT code where this trap happened. + /// The program counter in generated code where this trap happened. pc: usize, /// Native stack backtrace at the time the trap occurred backtrace: Backtrace, @@ -468,7 +470,7 @@ pub enum Trap { /// A trap indicating that the runtime was unable to allocate sufficient memory. /// - /// Note: this trap is undeterministic, since it depends on the host system. + /// Note: this trap is nondeterministic, since it depends on the host system. OOM { /// Native stack backtrace at the time the OOM occurred backtrace: Backtrace, @@ -545,7 +547,7 @@ where F: FnMut(), { return CallThreadState::new(trap_info).with(|cx| { - register_setjmp( + wasmer_register_setjmp( cx.jmp_buf.as_ptr(), call_closure::, &mut closure as *mut F as *mut u8, @@ -667,7 +669,7 @@ impl<'a> CallThreadState<'a> { fn unwind_with(&self, reason: UnwindReason) -> ! { unsafe { (*self.unwind.get()).as_mut_ptr().write(reason); - unwind(self.jmp_buf.get()); + wasmer_unwind(self.jmp_buf.get()); } } @@ -815,6 +817,8 @@ mod tls { /// Takes the TLS state that is currently configured and returns a /// token that is used to replace it later. /// + /// # Safety + /// /// This is not a safe operation since it's intended to only be used /// with stack switching found with fibers and async wasmer. pub unsafe fn take() -> Result { @@ -832,6 +836,8 @@ mod tls { /// Restores a previous tls state back into this thread's TLS. /// + /// # Safety + /// /// This is unsafe because it's intended to only be used within the /// context of stack switching within wasmer. pub unsafe fn replace(self) -> Result<(), super::Trap> { @@ -862,9 +868,7 @@ mod tls { // Note that this extension of the lifetime to `'static` should be // safe because we only ever access it below with an anonymous // lifetime, meaning `'static` never leaks out of this module. - let ptr = unsafe { - mem::transmute::<*const CallThreadState<'_>, *const CallThreadState<'static>>(state) - }; + let ptr = unsafe { mem::transmute::<*const CallThreadState<'_>, _>(state) }; let prev = raw::replace(ptr)?; state.prev.set(prev); let _reset = Reset(state); From df83605aadcff8f2cb46f72692d94074ef8b7b13 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Tue, 11 May 2021 13:19:35 -0700 Subject: [PATCH 09/14] Renamed TrapInfo to TrapHandler --- lib/api/src/store.rs | 6 +++--- lib/engine/src/artifact.rs | 6 +++--- lib/vm/src/instance/mod.rs | 10 +++++----- lib/vm/src/trap/mod.rs | 2 +- lib/vm/src/trap/traphandlers.rs | 30 +++++++++++++++--------------- 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/lib/api/src/store.rs b/lib/api/src/store.rs index 60143643a54..4132faca2dc 100644 --- a/lib/api/src/store.rs +++ b/lib/api/src/store.rs @@ -6,7 +6,7 @@ use std::sync::Arc; #[cfg(all(feature = "compiler", feature = "engine"))] use wasmer_compiler::CompilerConfig; use wasmer_engine::{is_wasm_pc, Engine, Tunables}; -use wasmer_vm::{init_traps, SignalHandler, TrapInfo}; +use wasmer_vm::{init_traps, TrapHandler, TrapHandlerFn}; /// The store represents all global state that can be manipulated by /// WebAssembly programs. It consists of the runtime representation @@ -72,12 +72,12 @@ impl PartialEq for Store { } } -unsafe impl TrapInfo for Store { +unsafe impl TrapHandler for Store { #[inline] fn as_any(&self) -> &dyn Any { self } - fn custom_signal_handler(&self, _call: &dyn Fn(&SignalHandler) -> bool) -> bool { + fn custom_trap_handler(&self, _call: &dyn Fn(&TrapHandlerFn) -> bool) -> bool { // Setting a custom handler is implemented for now, please open an issue // in the Wasmer Github repo if you are interested in this. false diff --git a/lib/engine/src/artifact.rs b/lib/engine/src/artifact.rs index 56c6f9e5003..00146067c3d 100644 --- a/lib/engine/src/artifact.rs +++ b/lib/engine/src/artifact.rs @@ -14,7 +14,7 @@ use wasmer_types::{ }; use wasmer_vm::{ FuncDataRegistry, FunctionBodyPtr, InstanceAllocator, InstanceHandle, MemoryStyle, ModuleInfo, - TableStyle, TrapInfo, VMSharedSignatureIndex, VMTrampoline, + TableStyle, TrapHandler, VMSharedSignatureIndex, VMTrampoline, }; /// An `Artifact` is the product that the `Engine` @@ -161,7 +161,7 @@ pub trait Artifact: Send + Sync + Upcastable + MemoryUsage { /// See [`InstanceHandle::finish_instantiation`]. unsafe fn finish_instantiation( &self, - trap_info: &dyn TrapInfo, + trap_handler: &dyn TrapHandler, handle: &InstanceHandle, ) -> Result<(), InstantiationError> { let data_initializers = self @@ -173,7 +173,7 @@ pub trait Artifact: Send + Sync + Upcastable + MemoryUsage { }) .collect::>(); handle - .finish_instantiation(trap_info, &data_initializers) + .finish_instantiation(trap_handler, &data_initializers) .map_err(|trap| InstantiationError::Start(RuntimeError::from_trap(trap))) } } diff --git a/lib/vm/src/instance/mod.rs b/lib/vm/src/instance/mod.rs index 99b7b755754..70fbde8a18d 100644 --- a/lib/vm/src/instance/mod.rs +++ b/lib/vm/src/instance/mod.rs @@ -19,7 +19,7 @@ use crate::global::Global; use crate::imports::Imports; use crate::memory::{Memory, MemoryError}; use crate::table::{Table, TableElement}; -use crate::trap::{catch_traps, Trap, TrapCode, TrapInfo}; +use crate::trap::{catch_traps, Trap, TrapCode, TrapHandler}; use crate::vmcontext::{ VMBuiltinFunctionsArray, VMCallerCheckedAnyfunc, VMContext, VMFunctionBody, VMFunctionEnvironment, VMFunctionImport, VMFunctionKind, VMGlobalDefinition, VMGlobalImport, @@ -393,7 +393,7 @@ impl Instance { } /// Invoke the WebAssembly start function of the instance, if one is present. - fn invoke_start_function(&self, trap_info: &dyn TrapInfo) -> Result<(), Trap> { + fn invoke_start_function(&self, trap_handler: &dyn TrapHandler) -> Result<(), Trap> { let start_index = match self.module.start_function { Some(idx) => idx, None => return Ok(()), @@ -422,7 +422,7 @@ impl Instance { // Make the call. unsafe { - catch_traps(trap_info, || { + catch_traps(trap_handler, || { mem::transmute::<*const VMFunctionBody, unsafe extern "C" fn(VMFunctionEnvironment)>( callee_address, )(callee_vmctx) @@ -1015,7 +1015,7 @@ impl InstanceHandle { /// Only safe to call immediately after instantiation. pub unsafe fn finish_instantiation( &self, - trap_info: &dyn TrapInfo, + trap_handler: &dyn TrapHandler, data_initializers: &[DataInitializer<'_>], ) -> Result<(), Trap> { let instance = self.instance().as_ref(); @@ -1026,7 +1026,7 @@ impl InstanceHandle { // The WebAssembly spec specifies that the start function is // invoked automatically at instantiation time. - instance.invoke_start_function(trap_info)?; + instance.invoke_start_function(trap_handler)?; Ok(()) } diff --git a/lib/vm/src/trap/mod.rs b/lib/vm/src/trap/mod.rs index a885798f666..26b6c2a5321 100644 --- a/lib/vm/src/trap/mod.rs +++ b/lib/vm/src/trap/mod.rs @@ -9,6 +9,6 @@ mod traphandlers; pub use trapcode::TrapCode; pub use traphandlers::{ catch_traps, catch_traps_with_result, raise_lib_trap, raise_user_trap, wasmer_call_trampoline, - SignalHandler, TlsRestore, Trap, TrapInfo, + TlsRestore, Trap, TrapHandler, TrapHandlerFn, }; pub use traphandlers::{init_traps, resume_panic}; diff --git a/lib/vm/src/trap/traphandlers.rs b/lib/vm/src/trap/traphandlers.rs index 4108fe2f2af..c6858b8f17d 100644 --- a/lib/vm/src/trap/traphandlers.rs +++ b/lib/vm/src/trap/traphandlers.rs @@ -19,10 +19,10 @@ pub use tls::TlsRestore; cfg_if::cfg_if! { if #[cfg(unix)] { /// Function which may handle custom signals while processing traps. - pub type SignalHandler = dyn Fn(libc::c_int, *const libc::siginfo_t, *const libc::c_void) -> bool; + pub type TrapHandlerFn = dyn Fn(libc::c_int, *const libc::siginfo_t, *const libc::c_void) -> bool; } else if #[cfg(target_os = "windows")] { /// Function which may handle custom signals while processing traps. - pub type SignalHandler = dyn Fn(winapi::um::winnt::PEXCEPTION_POINTERS) -> bool; + pub type TrapHandlerFn = dyn Fn(winapi::um::winnt::PEXCEPTION_POINTERS) -> bool; } } @@ -525,13 +525,13 @@ impl Trap { /// Wildly unsafe because it calls raw function pointers and reads/writes raw /// function pointers. pub unsafe fn wasmer_call_trampoline( - trap_info: &impl TrapInfo, + trap_handler: &impl TrapHandler, vmctx: VMFunctionEnvironment, trampoline: VMTrampoline, callee: *const VMFunctionBody, values_vec: *mut u8, ) -> Result<(), Trap> { - catch_traps(trap_info, || { + catch_traps(trap_handler, || { mem::transmute::<_, extern "C" fn(VMFunctionEnvironment, *const VMFunctionBody, *mut u8)>( trampoline, )(vmctx, callee, values_vec); @@ -542,11 +542,11 @@ pub unsafe fn wasmer_call_trampoline( /// returning them as a `Result`. /// /// Highly unsafe since `closure` won't have any dtors run. -pub unsafe fn catch_traps(trap_info: &dyn TrapInfo, mut closure: F) -> Result<(), Trap> +pub unsafe fn catch_traps(trap_handler: &dyn TrapHandler, mut closure: F) -> Result<(), Trap> where F: FnMut(), { - return CallThreadState::new(trap_info).with(|cx| { + return CallThreadState::new(trap_handler).with(|cx| { wasmer_register_setjmp( cx.jmp_buf.as_ptr(), call_closure::, @@ -572,14 +572,14 @@ where /// /// Check [`catch_traps`]. pub unsafe fn catch_traps_with_result( - trap_info: &dyn TrapInfo, + trap_handler: &dyn TrapHandler, mut closure: F, ) -> Result where F: FnMut() -> R, { let mut global_results = MaybeUninit::::uninit(); - catch_traps(trap_info, || { + catch_traps(trap_handler, || { global_results.as_mut_ptr().write(closure()); })?; Ok(global_results.assume_init()) @@ -592,7 +592,7 @@ pub struct CallThreadState<'a> { jmp_buf: Cell<*const u8>, reset_guard_page: Cell, prev: Cell, - trap_info: &'a (dyn TrapInfo + 'a), + trap_handler: &'a (dyn TrapHandler + 'a), handling_trap: Cell, } @@ -602,14 +602,14 @@ pub struct CallThreadState<'a> { /// Note that this is an `unsafe` trait at least because it's being run in the /// context of a synchronous signal handler, so it needs to be careful to not /// access too much state in answering these queries. -pub unsafe trait TrapInfo { +pub unsafe trait TrapHandler { /// Converts this object into an `Any` to dynamically check its type. fn as_any(&self) -> &dyn Any; /// Uses `call` to call a custom signal handler, if one is specified. /// /// Returns `true` if `call` returns true, otherwise returns `false`. - fn custom_signal_handler(&self, call: &dyn Fn(&SignalHandler) -> bool) -> bool; + fn custom_trap_handler(&self, call: &dyn Fn(&TrapHandlerFn) -> bool) -> bool; } enum UnwindReason { @@ -629,13 +629,13 @@ enum UnwindReason { impl<'a> CallThreadState<'a> { #[inline] - fn new(trap_info: &'a (dyn TrapInfo + 'a)) -> CallThreadState<'a> { + fn new(trap_handler: &'a (dyn TrapHandler + 'a)) -> CallThreadState<'a> { Self { unwind: UnsafeCell::new(MaybeUninit::uninit()), jmp_buf: Cell::new(ptr::null()), reset_guard_page: Cell::new(false), prev: Cell::new(ptr::null()), - trap_info, + trap_handler, handling_trap: Cell::new(false), } } @@ -695,7 +695,7 @@ impl<'a> CallThreadState<'a> { pc: *const u8, reset_guard_page: bool, signal_trap: Option, - call_handler: impl Fn(&SignalHandler) -> bool, + call_handler: impl Fn(&TrapHandlerFn) -> bool, ) -> *const u8 { // If we hit a fault while handling a previous trap, that's quite bad, // so bail out and let the system handle this recursive segfault. @@ -709,7 +709,7 @@ impl<'a> CallThreadState<'a> { // First up see if we have a custom trap handler, // in which case run it. If anything handles the trap then we // return that the trap was handled. - if self.trap_info.custom_signal_handler(&call_handler) { + if self.trap_handler.custom_trap_handler(&call_handler) { return 1 as *const _; } From c9243b2a667959020fe78946d41a78481a3fbfdf Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Wed, 12 May 2021 12:23:12 -0700 Subject: [PATCH 10/14] Fixed comments --- lib/engine/src/trap/frame_info.rs | 2 +- lib/vm/src/trap/traphandlers.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/engine/src/trap/frame_info.rs b/lib/engine/src/trap/frame_info.rs index 90422018abe..51ba70188cf 100644 --- a/lib/engine/src/trap/frame_info.rs +++ b/lib/engine/src/trap/frame_info.rs @@ -45,7 +45,7 @@ pub struct GlobalFrameInfo { /// Returns whether the `pc`, according to globally registered information, /// is a wasm trap or not. pub fn is_wasm_pc(pc: usize) -> bool { - let frame_info = FRAME_INFO.read().expect("can't acquire the FrameInfo lock"); + let frame_info = FRAME_INFO.read().unwrap(); let module_info = frame_info.module_info(pc); module_info.is_some() } diff --git a/lib/vm/src/trap/traphandlers.rs b/lib/vm/src/trap/traphandlers.rs index c6858b8f17d..336c1792771 100644 --- a/lib/vm/src/trap/traphandlers.rs +++ b/lib/vm/src/trap/traphandlers.rs @@ -418,7 +418,7 @@ pub unsafe fn raise_lib_trap(trap: Trap) -> ! { /// # Safety /// /// Only safe to call when wasm code is on the stack, aka `catch_traps` must -/// have been previously called. Additionally no Rust destructors can be on the +/// have been previously called and not returned. Additionally no Rust destructors can be on the /// stack. They will be skipped and not executed. pub unsafe fn resume_panic(payload: Box) -> ! { tls::with(|info| info.unwrap().unwind_with(UnwindReason::Panic(payload))) @@ -876,7 +876,7 @@ mod tls { } /// Returns the last pointer configured with `set` above. Panics if `set` - /// has not been previously called. + /// has not been previously called and not returned. pub fn with(closure: impl FnOnce(Option<&CallThreadState<'_>>) -> R) -> R { let p = raw::get(); unsafe { closure(if p.is_null() { None } else { Some(&*p) }) } From eb030f0657bc2b1f7b0264b3962674e3e7132e38 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Wed, 12 May 2021 12:46:54 -0700 Subject: [PATCH 11/14] Added extra comments --- lib/vm/src/trap/traphandlers.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/vm/src/trap/traphandlers.rs b/lib/vm/src/trap/traphandlers.rs index 336c1792771..a8e689f20d0 100644 --- a/lib/vm/src/trap/traphandlers.rs +++ b/lib/vm/src/trap/traphandlers.rs @@ -645,9 +645,12 @@ impl<'a> CallThreadState<'a> { if ret != 0 { return Ok(()); } + // We will only reach this path if ret == 0. And that will + // only happen if a trap did happen. As such, it's safe to + // assume that the `unwind` field is already initialized + // at this moment. match unsafe { (*self.unwind.get()).as_ptr().read() } { UnwindReason::UserTrap(data) => { - debug_assert_eq!(ret, 0); Err(Trap::User(data)) } UnwindReason::LibTrap(trap) => Err(trap), @@ -656,11 +659,9 @@ impl<'a> CallThreadState<'a> { pc, signal_trap, } => { - debug_assert_eq!(ret, 0); Err(Trap::wasm(pc, backtrace, signal_trap)) } UnwindReason::Panic(panic) => { - debug_assert_eq!(ret, 0); std::panic::resume_unwind(panic) } } From 8a5a33179eac0a4465210895e601078e5e4894ec Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Wed, 12 May 2021 13:06:40 -0700 Subject: [PATCH 12/14] Implemented custom trap handler --- lib/api/src/store.rs | 27 ++++++++++++++++++++++----- lib/vm/src/trap/traphandlers.rs | 12 +++--------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/lib/api/src/store.rs b/lib/api/src/store.rs index 4132faca2dc..3381aeb15e5 100644 --- a/lib/api/src/store.rs +++ b/lib/api/src/store.rs @@ -2,7 +2,7 @@ use crate::tunables::BaseTunables; use loupe::MemoryUsage; use std::any::Any; use std::fmt; -use std::sync::Arc; +use std::sync::{Arc, RwLock}; #[cfg(all(feature = "compiler", feature = "engine"))] use wasmer_compiler::CompilerConfig; use wasmer_engine::{is_wasm_pc, Engine, Tunables}; @@ -22,6 +22,8 @@ use wasmer_vm::{init_traps, TrapHandler, TrapHandlerFn}; pub struct Store { engine: Arc, tunables: Arc, + #[loupe(skip)] + trap_handler: Arc>>>, } impl Store { @@ -33,6 +35,12 @@ impl Store { Self::new_with_tunables(engine, BaseTunables::for_target(engine.target())) } + /// Set the trap handler in this store. + pub fn set_trap_handler(&self, handler: Option>) { + let mut m = self.trap_handler.write().unwrap(); + *m = handler; + } + /// Creates a new `Store` with a specific [`Engine`] and [`Tunables`]. pub fn new_with_tunables(engine: &E, tunables: impl Tunables + Send + Sync + 'static) -> Self where @@ -45,6 +53,7 @@ impl Store { Self { engine: engine.cloned(), tunables: Arc::new(tunables), + trap_handler: Arc::new(RwLock::new(None)), } } @@ -77,13 +86,21 @@ unsafe impl TrapHandler for Store { fn as_any(&self) -> &dyn Any { self } - fn custom_trap_handler(&self, _call: &dyn Fn(&TrapHandlerFn) -> bool) -> bool { - // Setting a custom handler is implemented for now, please open an issue - // in the Wasmer Github repo if you are interested in this. - false + + fn custom_trap_handler(&self, call: &dyn Fn(&TrapHandlerFn) -> bool) -> bool { + if let Some(handler) = *&self.trap_handler.read().unwrap().as_ref() { + call(handler) + } else { + false + } } } +// This is required to be able to set the trap_handler in the +// Store. +unsafe impl Send for Store {} +unsafe impl Sync for Store {} + // We only implement default if we have assigned a default compiler and engine #[cfg(all(feature = "default-compiler", feature = "default-engine"))] impl Default for Store { diff --git a/lib/vm/src/trap/traphandlers.rs b/lib/vm/src/trap/traphandlers.rs index a8e689f20d0..c463c290f32 100644 --- a/lib/vm/src/trap/traphandlers.rs +++ b/lib/vm/src/trap/traphandlers.rs @@ -650,20 +650,14 @@ impl<'a> CallThreadState<'a> { // assume that the `unwind` field is already initialized // at this moment. match unsafe { (*self.unwind.get()).as_ptr().read() } { - UnwindReason::UserTrap(data) => { - Err(Trap::User(data)) - } + UnwindReason::UserTrap(data) => Err(Trap::User(data)), UnwindReason::LibTrap(trap) => Err(trap), UnwindReason::WasmTrap { backtrace, pc, signal_trap, - } => { - Err(Trap::wasm(pc, backtrace, signal_trap)) - } - UnwindReason::Panic(panic) => { - std::panic::resume_unwind(panic) - } + } => Err(Trap::wasm(pc, backtrace, signal_trap)), + UnwindReason::Panic(panic) => std::panic::resume_unwind(panic), } } From 030c9a4af23835612573f84275059d4421c06bd9 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Wed, 12 May 2021 13:14:31 -0700 Subject: [PATCH 13/14] Last fix --- lib/vm/src/trap/traphandlers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vm/src/trap/traphandlers.rs b/lib/vm/src/trap/traphandlers.rs index c463c290f32..143829905e4 100644 --- a/lib/vm/src/trap/traphandlers.rs +++ b/lib/vm/src/trap/traphandlers.rs @@ -418,7 +418,7 @@ pub unsafe fn raise_lib_trap(trap: Trap) -> ! { /// # Safety /// /// Only safe to call when wasm code is on the stack, aka `catch_traps` must -/// have been previously called and not returned. Additionally no Rust destructors can be on the +/// have been previously called and not returned. Additionally no Rust destructors may be on the /// stack. They will be skipped and not executed. pub unsafe fn resume_panic(payload: Box) -> ! { tls::with(|info| info.unwrap().unwind_with(UnwindReason::Panic(payload))) From 29fd4301f418af4bcc88bae557eb0e7b27137704 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Wed, 12 May 2021 13:47:58 -0700 Subject: [PATCH 14/14] Fix default store --- lib/api/src/store.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/api/src/store.rs b/lib/api/src/store.rs index 3381aeb15e5..4bee9e996b6 100644 --- a/lib/api/src/store.rs +++ b/lib/api/src/store.rs @@ -141,10 +141,7 @@ impl Default for Store { let config = get_config(); let engine = get_engine(config); let tunables = BaseTunables::for_target(engine.target()); - Store { - engine: Arc::new(engine), - tunables: Arc::new(tunables), - } + Self::new_with_tunables(&engine, tunables) } }