From 8b86f795b734f2cf437bb1ed76fd21d564a9d7ab Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Mon, 10 May 2021 16:42:10 -0700 Subject: [PATCH 1/3] 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 2/3] 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 3/3] 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 {