From 68df4000c12217eff0a9575f14e0ace41efeb075 Mon Sep 17 00:00:00 2001 From: Andy Wortman Date: Thu, 11 Jun 2020 18:42:58 -0700 Subject: [PATCH 01/37] appears to be a start --- Cargo.lock | 1 + lucetc/Cargo.toml | 3 +- lucetc/src/compiler.rs | 117 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 118 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6907d9a64..711ff5fbc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1157,6 +1157,7 @@ dependencies = [ "cranelift-object", "cranelift-wasm", "env_logger", + "gimli 0.20.0", "human-size", "log", "lucet-module", diff --git a/lucetc/Cargo.toml b/lucetc/Cargo.toml index 59bd9551b..e0e656378 100644 --- a/lucetc/Cargo.toml +++ b/lucetc/Cargo.toml @@ -16,7 +16,7 @@ path = "lucetc/main.rs" [dependencies] anyhow = "1" bincode = "1.1.4" -cranelift-codegen = { path = "../wasmtime/cranelift/codegen", version = "0.64.0" } +cranelift-codegen = { path = "../wasmtime/cranelift/codegen", version = "0.64.0", features = ["unwind"] } cranelift-entity = { path = "../wasmtime/cranelift/entity", version = "0.64.0" } cranelift-native = { path = "../wasmtime/cranelift/native", version = "0.64.0" } cranelift-frontend = { path = "../wasmtime/cranelift/frontend", version = "0.64.0" } @@ -31,6 +31,7 @@ wasmparser = "0.57.0" clap="2.32" log = "0.4" env_logger = "0.6" +gimli = { version = "0.20.0", default-features = false, features = ["write"] } object = { version = "0.18.0", default-features = false, features = ["write"] } byteorder = "1.2" wabt = "0.9.2" diff --git a/lucetc/src/compiler.rs b/lucetc/src/compiler.rs index af85ed47f..dc237ae43 100644 --- a/lucetc/src/compiler.rs +++ b/lucetc/src/compiler.rs @@ -14,16 +14,18 @@ use crate::traps::{translate_trapcode, trap_sym_for_func}; use byteorder::{LittleEndian, WriteBytesExt}; use cranelift_codegen::{ binemit, ir, + isa::unwind::UnwindInfo, isa::TargetIsa, settings::{self, Configurable}, Context as ClifContext, }; use cranelift_module::{ Backend as ClifBackend, DataContext as ClifDataContext, DataId, FuncId, FuncOrDataId, - Linkage as ClifLinkage, Module as ClifModule, + Linkage as ClifLinkage, Module as ClifModule, ModuleError as ClifModuleError, }; use cranelift_object::{ObjectBackend, ObjectBuilder}; use cranelift_wasm::{translate_module, FuncTranslator, ModuleTranslationState, WasmError}; +use gimli::write::{Address, EhFrame, Writer}; use lucet_module::bindings::Bindings; use lucet_module::{ ModuleData, ModuleFeatures, SerializedModule, VersionInfo, LUCET_MODULE_SYM, MODULE_DATA_SYM, @@ -271,9 +273,23 @@ impl<'a> Compiler<'a> { let mut func_translator = FuncTranslator::new(); let mut function_manifest_ctx = ClifDataContext::new(); let mut function_manifest_bytes = Cursor::new(Vec::new()); + // let mut eh_frame_table_bytes: Cursor> = Cursor::new(Vec::new()); let mut function_map: HashMap = HashMap::new(); - for (ref func, (code, code_offset)) in self.decls.function_bodies() { + let mut frame_table = gimli::write::FrameTable::default(); + + let isa = Self::target_isa( + self.target.clone(), + self.opt_level, + &self.cpu_features, + self.canonicalize_nans, + )?; + let cie_id = frame_table.add_cie(match isa.create_systemv_cie() { + Some(cie) => cie, + None => panic!("uh oh"), + }); + + for (idx, (ref func, (code, code_offset))) in self.decls.function_bodies().enumerate() { let mut func_info = FuncInfo::new(&self.decls, self.count_instructions); let mut clif_context = ClifContext::new(); clif_context.func.name = func.name.as_externalname(); @@ -303,6 +319,36 @@ impl<'a> Compiler<'a> { let size = compiled.size; + println!( + "signature for {} is {:?}", + func.name.symbol(), + func.signature.call_conv + ); + println!("isa: {}, ptr bits: {}", isa.name(), isa.pointer_bits()); + let unwind_info = + clif_context + .create_unwind_info(isa.as_ref()) + .map_err(|compilation_error| Error::FunctionDefinition { + symbol: func.name.symbol().to_string(), + source: ClifModuleError::Compilation(compilation_error), + })?; + if let Some(unwind_info) = unwind_info { + if let UnwindInfo::SystemV(info) = unwind_info { + frame_table.add_fde( + cie_id, + info.to_fde(Address::Symbol { + symbol: idx, // func.name.symbol().to_string(), + addend: 0, + }), + ); + } else { + panic!("non-sysv unwind info in lucet module"); + } + } else { + println!("no info for {}", func.name.symbol()); + // .expect("function yields .eh_frame information"); + } + let trap_data_id = traps.write(&mut self.clif_module, func.name.symbol())?; function_map.insert(func_id, (size, trap_data_id, traps.len())); @@ -379,6 +425,73 @@ impl<'a> Compiler<'a> { self.clif_module .define_data(manifest_data_id, &function_manifest_ctx)?; + let mut eh_frame_ctx = ClifDataContext::new(); + struct EhFrameSink<'a> { + pub data: Vec, + pub data_context: &'a mut ClifDataContext, + pub decls: &'a ModuleDecls<'a>, + } + + impl<'a> Writer for EhFrameSink<'a> { + type Endian = gimli::LittleEndian; + + fn endian(&self) -> Self::Endian { + gimli::LittleEndian + } + fn len(&self) -> usize { + self.data.len() + } + fn write(&mut self, bytes: &[u8]) -> gimli::write::Result<()> { + self.data.extend_from_slice(bytes); + Ok(()) + } + fn write_at(&mut self, offset: usize, bytes: &[u8]) -> gimli::write::Result<()> { + if offset + bytes.len() > self.data.len() { + return Err(gimli::write::Error::LengthOutOfBounds); + } + self.data[offset..][..bytes.len()].copy_from_slice(bytes); + Ok(()) + } + + fn write_address(&mut self, address: Address, size: u8) -> gimli::write::Result<()> { + match address { + Address::Constant(val) => self.write_udata(val, size), + Address::Symbol { symbol, addend } => { + assert_eq!(addend, 0); + + use crate::module::UniqueFuncIndex; + let name = self + .decls + .get_func(UniqueFuncIndex::from_u32(symbol as u32)) + .expect("valid index") + .name + .as_externalname(); + let funcref = self.data_context.import_function(name); + let offset = self.data.len(); + self.data_context + .write_function_addr(offset as u32, funcref); + + self.write_udata(0, size) + } + } + } + } + let mut eh_frame = EhFrame(EhFrameSink { + data: Vec::new(), + data_context: &mut eh_frame_ctx, + decls: &self.decls, + }); + frame_table + .write_eh_frame(&mut eh_frame) + .expect("this works too"); + let eh_frame_bytes = eh_frame.0.data.into_boxed_slice(); + eh_frame_ctx.define(eh_frame_bytes); + let eh_frame_data_id = + self.clif_module + .declare_data(".eh_frame", ClifLinkage::Export, false, false, None)?; + self.clif_module + .define_data(eh_frame_data_id, &eh_frame_ctx)?; + // Write out the structure tying everything together. let mut native_data = Cursor::new(Vec::with_capacity(std::mem::size_of::())); From dfd8e25bb4815966e90dcdb1ee389a43ca15605e Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Mon, 22 Jul 2019 15:23:06 -0700 Subject: [PATCH 02/37] temporarily disable panic_unwind in hostcall macro This is for debugging purposes so that we have a test case that unwinds to `lucet_context_backstop`. This causes several of the existing test cases to fail, but if you want one that is solely focused on this behavior, run: ``` cargo test -p lucet-runtime --test host unwind ``` --- .../src/hostcall_macros.rs | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs b/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs index 75d923d75..d8a59e052 100644 --- a/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs +++ b/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs @@ -44,7 +44,27 @@ macro_rules! lucet_hostcalls { $vmctx: &lucet_runtime::vmctx::Vmctx, $( $arg: $arg_ty ),* ) -> $ret_ty { - $($body)* + #[inline(always)] + unsafe fn hostcall_impl( + $vmctx: &mut $crate::vmctx::Vmctx, + $( $arg : $arg_ty ),* + ) -> $ret_ty { + $($body)* + } + // let res = std::panic::catch_unwind(move || { + hostcall_impl(&mut $crate::vmctx::Vmctx::from_raw(vmctx_raw), $( $arg ),*) + // }); + // match res { + // Ok(res) => res, + // Err(e) => { + // if let Some(details) = e.downcast_ref::<$crate::instance::TerminationDetails>() { + // let mut vmctx = $crate::vmctx::Vmctx::from_raw(vmctx_raw); + // vmctx.terminate_no_unwind(details.clone()); + // } else { + // std::panic::resume_unwind(e); + // } + // } + // } } )* } From 6c0916d89aa442fc1dca6870ef284d08af78189b Mon Sep 17 00:00:00 2001 From: Andy Wortman Date: Mon, 29 Jul 2019 14:41:36 -0700 Subject: [PATCH 03/37] WIP, get pieces in place for backstop CFI directives to work ... when guest code has correct .eh_frame information, anyway. this currently results in the personality function being called if you run the lucet-runtime hostcall tests under gdb with the following commands ``` set args unwind --test-threads=1 --nocapture b rust_panic r ``` then, when the breakpoint (`rust_panic`) is hit, replace the first return into guest code with a return to `lucet_context_backstop`: ``` `# v-- this is the address of lucet_context_backstop` printf '\x61\x7f\xa0\x56\x55\x55\x00\x00' \ | dd `# because gdb doesnt like "set *(long long*)0xaddr = value ` \ `# v--- just finding the pid of the test debugee ` \ of=/proc/$(ps -ef | grep lucet | grep unwind | cut -d' ' -f2)/mem \ `# v-- this is where the first guest return address is `\ `# v ..for me anyway. replaces 0x00007ffff6878685` \ `# v for "guest_func___original_main". `\ bs=1 seek=$((0x7ffff6872fa8)) \ `# dd would try to truncate guest memory by default. do not do this. `\ conv=notrunc ``` (if you can figure out how to do this standalone in gdb, i'm all ears) at this point, continuing in gdb to allow the panic mechanism to run should ... call into the provided personality function! --- .../src/context/context_asm.S | 14 +++++++++++++- .../src/instance/signals.rs | 8 ++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S b/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S index f313ddea9..7fe25d516 100644 --- a/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S +++ b/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S @@ -55,13 +55,24 @@ _lucet_context_bootstrap: #endif .text +.globl libunwind_backstop_shim +.align 16 +// unsure if `simple` is necessary here. GAS docs suggest this may omit some initial CFI instructions, and i want this to be as barren as possible for now. +.cfi_startproc simple +.cfi_personality 0,win + // this nop is load-bearing. We set up the backstop with a cfi_personality function so that stack unwinding through + // guest code ends at a handler which can recover and switch back to the host. + // However: libunwind doesn't treat the start of `lucet_context_backstop` as `lucet_context_backstop` but as the end + // of `lucet_context_bootstrap`. so to get the unwinder to reach information about this function, start the unwind + // region slightly before the real address we use, and place an address after the start, unambiguously referring to + // the backstop function, on the stack for guests (and libunwind!) to eventually reach. + nop .globl lucet_context_backstop #ifdef __ELF__ .type lucet_context_backstop,@function #else .globl _lucet_context_backstop #endif -.align 16 lucet_context_backstop: _lucet_context_backstop: // Note that `rbp` here really has no relation to any stack! @@ -94,6 +105,7 @@ no_backstop_callback: #else jmp lucet_context_swap #endif +.cfi_endproc #ifdef __ELF__ .size lucet_context_backstop,.-lucet_context_backstop #endif diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs index 04b092da0..1c5e41545 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs @@ -451,6 +451,14 @@ unsafe fn setup_guest_signal_state(ostate: &mut Option) { }); } +#[no_mangle] +extern "C" fn win() { + use std::io::Write; + // ... wonder what happens if we try to call this stuff while panicked? + std::io::stdout().write_all(b"thank you libunwind! but our princess is in another castle!"); + std::io::stdout().flush(); +} + fn setup_guest_panic_hook() -> Arc) + Sync + Send + 'static>> { let saved_panic_hook = Arc::new(panic::take_hook()); let closure_saved_panic_hook = saved_panic_hook.clone(); From bf50796570988cd033575c9c94e15867c62bda07 Mon Sep 17 00:00:00 2001 From: Andy Wortman Date: Tue, 6 Aug 2019 19:01:19 -0700 Subject: [PATCH 04/37] add ld argumet to get eh_frame_hdr emitted --- lucetc/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lucetc/src/lib.rs b/lucetc/src/lib.rs index d19c56bca..d79ded350 100644 --- a/lucetc/src/lib.rs +++ b/lucetc/src/lib.rs @@ -348,6 +348,8 @@ fn link_so( let mut ld_iter = env_ld.split_whitespace(); let ld_prog = ld_iter.next().expect("LD must not be empty"); let mut cmd_ld = Command::new(ld_prog); + // TODO: sufficiently crossplatform? + cmd_ld.arg("--eh-frame-hdr"); for flag in ld_iter { cmd_ld.arg(flag); } From e6d35dc629e7f8c98d0e832dccba7b03fe9543f6 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Wed, 7 Aug 2019 15:23:30 -0700 Subject: [PATCH 05/37] temporarily copy in Rust's system unwinder bindings, and win in ph2 --- .../lucet-runtime-internals/src/instance.rs | 1 + .../src/instance/signals.rs | 40 ++- .../src/instance/unwind.rs | 239 ++++++++++++++++++ 3 files changed, 275 insertions(+), 5 deletions(-) create mode 100644 lucet-runtime/lucet-runtime-internals/src/instance/unwind.rs diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index f00342ef3..5186d2393 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -2,6 +2,7 @@ pub mod execution; mod siginfo_ext; pub mod signals; pub mod state; +mod unwind; pub use crate::instance::execution::{KillError, KillState, KillSuccess, KillSwitch}; pub use crate::instance::signals::{signal_handler_none, SignalBehavior, SignalHandler}; diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs index 1c5e41545..d2901edb7 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs @@ -451,12 +451,42 @@ unsafe fn setup_guest_signal_state(ostate: &mut Option) { }); } +use crate::instance::unwind as uw; + #[no_mangle] -extern "C" fn win() { - use std::io::Write; - // ... wonder what happens if we try to call this stuff while panicked? - std::io::stdout().write_all(b"thank you libunwind! but our princess is in another castle!"); - std::io::stdout().flush(); +extern "C" fn win( + version: c_int, + actions: uw::_Unwind_Action, + exceptionClass: u64, + exceptionObject: *mut uw::_Unwind_Exception, + context: *mut uw::_Unwind_Context, +) -> uw::_Unwind_Reason_Code { + if version != 1 { + return uw::_URC_FATAL_PHASE1_ERROR; + } + // dbg!(version); + // dbg!(actions); + // dbg!(uw::fmt_exception_class(exceptionClass)); + // dbg!(exceptionObject); + // dbg!(context); + + // CURRENT_INSTANCE.with(|current_instance| { + // eprintln!("instance state when panicking: {}", unsafe { + // ¤t_instance.borrow().unwrap().as_mut().state + // }); + // }); + + if actions as i32 & uw::_UA_SEARCH_PHASE as i32 != 0 { + return uw::_URC_HANDLER_FOUND; + } else { + use std::io::Write; + // ... wonder what happens if we try to call this stuff while panicked? + std::io::stdout() + .write_all(b"thank you libunwind! but our princess is in another castle!") + .unwrap(); + std::io::stdout().flush().unwrap(); + uw::_Unwind_Reason_Code::_URC_FATAL_PHASE2_ERROR + } } fn setup_guest_panic_hook() -> Arc) + Sync + Send + 'static>> { diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/unwind.rs b/lucet-runtime/lucet-runtime-internals/src/instance/unwind.rs new file mode 100644 index 000000000..2ae00c590 --- /dev/null +++ b/lucet-runtime/lucet-runtime-internals/src/instance/unwind.rs @@ -0,0 +1,239 @@ +#![allow(nonstandard_style)] + +macro_rules! cfg_if { + ( $( if #[cfg( $meta:meta )] { $($it1:item)* } else { $($it2:item)* } )* ) => + ( $( $( #[cfg($meta)] $it1)* $( #[cfg(not($meta))] $it2)* )* ) +} + +use libc::{c_int, c_void, uintptr_t}; + +#[repr(C)] +#[derive(Debug, Copy, Clone, PartialEq)] +pub enum _Unwind_Reason_Code { + _URC_NO_REASON = 0, + _URC_FOREIGN_EXCEPTION_CAUGHT = 1, + _URC_FATAL_PHASE2_ERROR = 2, + _URC_FATAL_PHASE1_ERROR = 3, + _URC_NORMAL_STOP = 4, + _URC_END_OF_STACK = 5, + _URC_HANDLER_FOUND = 6, + _URC_INSTALL_CONTEXT = 7, + _URC_CONTINUE_UNWIND = 8, + _URC_FAILURE = 9, // used only by ARM EHABI +} +pub use _Unwind_Reason_Code::*; + +pub type _Unwind_Exception_Class = u64; +pub fn fmt_exception_class(class: _Unwind_Exception_Class) -> String { + let mut s = String::new(); + for shift in &[56u64, 48, 40, 32, 24, 16, 8, 0] { + s.push((class >> shift & 0xff) as u8 as char); + } + s +} + +pub type _Unwind_Word = uintptr_t; +pub type _Unwind_Ptr = uintptr_t; +pub type _Unwind_Trace_Fn = + extern "C" fn(ctx: *mut _Unwind_Context, arg: *mut c_void) -> _Unwind_Reason_Code; +#[cfg(target_arch = "x86")] +pub const unwinder_private_data_size: usize = 5; + +#[cfg(target_arch = "x86_64")] +pub const unwinder_private_data_size: usize = 6; + +#[cfg(all(target_arch = "arm", not(target_os = "ios")))] +pub const unwinder_private_data_size: usize = 20; + +#[cfg(all(target_arch = "arm", target_os = "ios"))] +pub const unwinder_private_data_size: usize = 5; + +#[cfg(target_arch = "aarch64")] +pub const unwinder_private_data_size: usize = 2; + +#[cfg(target_arch = "mips")] +pub const unwinder_private_data_size: usize = 2; + +#[cfg(target_arch = "mips64")] +pub const unwinder_private_data_size: usize = 2; + +#[cfg(any(target_arch = "powerpc", target_arch = "powerpc64"))] +pub const unwinder_private_data_size: usize = 2; + +#[cfg(target_arch = "s390x")] +pub const unwinder_private_data_size: usize = 2; + +#[cfg(target_arch = "sparc64")] +pub const unwinder_private_data_size: usize = 2; + +#[cfg(target_os = "emscripten")] +pub const unwinder_private_data_size: usize = 20; + +#[repr(C)] +pub struct _Unwind_Exception { + pub exception_class: _Unwind_Exception_Class, + pub exception_cleanup: _Unwind_Exception_Cleanup_Fn, + pub private: [_Unwind_Word; unwinder_private_data_size], +} + +pub enum _Unwind_Context {} + +pub type _Unwind_Exception_Cleanup_Fn = + extern "C" fn(unwind_code: _Unwind_Reason_Code, exception: *mut _Unwind_Exception); +extern "C" { + // #[unwind(allowed)] + pub fn _Unwind_Resume(exception: *mut _Unwind_Exception) -> !; + pub fn _Unwind_DeleteException(exception: *mut _Unwind_Exception); + pub fn _Unwind_GetLanguageSpecificData(ctx: *mut _Unwind_Context) -> *mut c_void; + pub fn _Unwind_GetRegionStart(ctx: *mut _Unwind_Context) -> _Unwind_Ptr; + pub fn _Unwind_GetTextRelBase(ctx: *mut _Unwind_Context) -> _Unwind_Ptr; + pub fn _Unwind_GetDataRelBase(ctx: *mut _Unwind_Context) -> _Unwind_Ptr; +} + +cfg_if! { +if #[cfg(all(any(target_os = "ios", target_os = "netbsd", not(target_arch = "arm"))))] { + // Not ARM EHABI + #[repr(C)] + #[derive(Copy, Clone, PartialEq, Debug)] + pub enum _Unwind_Action { + _UA_SEARCH_PHASE = 1, + _UA_CLEANUP_PHASE = 2, + _UA_HANDLER_FRAME = 4, + _UA_FORCE_UNWIND = 8, + _UA_END_OF_STACK = 16, + } + pub use _Unwind_Action::*; + + extern "C" { + pub fn _Unwind_GetGR(ctx: *mut _Unwind_Context, reg_index: c_int) -> _Unwind_Word; + pub fn _Unwind_SetGR(ctx: *mut _Unwind_Context, reg_index: c_int, value: _Unwind_Word); + pub fn _Unwind_GetIP(ctx: *mut _Unwind_Context) -> _Unwind_Word; + pub fn _Unwind_SetIP(ctx: *mut _Unwind_Context, value: _Unwind_Word); + pub fn _Unwind_GetIPInfo(ctx: *mut _Unwind_Context, ip_before_insn: *mut c_int) + -> _Unwind_Word; + pub fn _Unwind_FindEnclosingFunction(pc: *mut c_void) -> *mut c_void; + } + +} else { + // ARM EHABI + #[repr(C)] + #[derive(Copy, Clone, PartialEq)] + pub enum _Unwind_State { + _US_VIRTUAL_UNWIND_FRAME = 0, + _US_UNWIND_FRAME_STARTING = 1, + _US_UNWIND_FRAME_RESUME = 2, + _US_ACTION_MASK = 3, + _US_FORCE_UNWIND = 8, + _US_END_OF_STACK = 16, + } + pub use _Unwind_State::*; + + #[repr(C)] + enum _Unwind_VRS_Result { + _UVRSR_OK = 0, + _UVRSR_NOT_IMPLEMENTED = 1, + _UVRSR_FAILED = 2, + } + #[repr(C)] + enum _Unwind_VRS_RegClass { + _UVRSC_CORE = 0, + _UVRSC_VFP = 1, + _UVRSC_FPA = 2, + _UVRSC_WMMXD = 3, + _UVRSC_WMMXC = 4, + } + use _Unwind_VRS_RegClass::*; + #[repr(C)] + enum _Unwind_VRS_DataRepresentation { + _UVRSD_UINT32 = 0, + _UVRSD_VFPX = 1, + _UVRSD_FPAX = 2, + _UVRSD_UINT64 = 3, + _UVRSD_FLOAT = 4, + _UVRSD_DOUBLE = 5, + } + use _Unwind_VRS_DataRepresentation::*; + + pub const UNWIND_POINTER_REG: c_int = 12; + pub const UNWIND_IP_REG: c_int = 15; + + extern "C" { + fn _Unwind_VRS_Get(ctx: *mut _Unwind_Context, + regclass: _Unwind_VRS_RegClass, + regno: _Unwind_Word, + repr: _Unwind_VRS_DataRepresentation, + data: *mut c_void) + -> _Unwind_VRS_Result; + + fn _Unwind_VRS_Set(ctx: *mut _Unwind_Context, + regclass: _Unwind_VRS_RegClass, + regno: _Unwind_Word, + repr: _Unwind_VRS_DataRepresentation, + data: *mut c_void) + -> _Unwind_VRS_Result; + } + + // On Android or ARM/Linux, these are implemented as macros: + + pub unsafe fn _Unwind_GetGR(ctx: *mut _Unwind_Context, reg_index: c_int) -> _Unwind_Word { + let mut val: _Unwind_Word = 0; + _Unwind_VRS_Get(ctx, _UVRSC_CORE, reg_index as _Unwind_Word, _UVRSD_UINT32, + &mut val as *mut _ as *mut c_void); + val + } + + pub unsafe fn _Unwind_SetGR(ctx: *mut _Unwind_Context, reg_index: c_int, value: _Unwind_Word) { + let mut value = value; + _Unwind_VRS_Set(ctx, _UVRSC_CORE, reg_index as _Unwind_Word, _UVRSD_UINT32, + &mut value as *mut _ as *mut c_void); + } + + pub unsafe fn _Unwind_GetIP(ctx: *mut _Unwind_Context) + -> _Unwind_Word { + let val = _Unwind_GetGR(ctx, UNWIND_IP_REG); + (val & !1) as _Unwind_Word + } + + pub unsafe fn _Unwind_SetIP(ctx: *mut _Unwind_Context, + value: _Unwind_Word) { + // Propagate thumb bit to instruction pointer + let thumb_state = _Unwind_GetGR(ctx, UNWIND_IP_REG) & 1; + let value = value | thumb_state; + _Unwind_SetGR(ctx, UNWIND_IP_REG, value); + } + + pub unsafe fn _Unwind_GetIPInfo(ctx: *mut _Unwind_Context, + ip_before_insn: *mut c_int) + -> _Unwind_Word { + *ip_before_insn = 0; + _Unwind_GetIP(ctx) + } + + // This function also doesn't exist on Android or ARM/Linux, so make it a no-op + pub unsafe fn _Unwind_FindEnclosingFunction(pc: *mut c_void) -> *mut c_void { + pc + } +} + +if #[cfg(not(all(target_os = "ios", target_arch = "arm")))] { + // Not 32-bit iOS + extern "C" { + // #[unwind(allowed)] + pub fn _Unwind_RaiseException(exception: *mut _Unwind_Exception) -> _Unwind_Reason_Code; + pub fn _Unwind_Backtrace(trace: _Unwind_Trace_Fn, + trace_argument: *mut c_void) + -> _Unwind_Reason_Code; + } +} else { + // 32-bit iOS uses SjLj and does not provide _Unwind_Backtrace() + extern "C" { + // #[unwind(allowed)] + pub fn _Unwind_SjLj_RaiseException(e: *mut _Unwind_Exception) -> _Unwind_Reason_Code; + } + + #[inline] + pub unsafe fn _Unwind_RaiseException(exc: *mut _Unwind_Exception) -> _Unwind_Reason_Code { + _Unwind_SjLj_RaiseException(exc) + } +} +} // cfg_if! From 2d2bb60b50e19edc1aa9d14bb020653356286314 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Fri, 9 Aug 2019 18:01:23 -0700 Subject: [PATCH 06/37] experimental checkin of CFA calculation expression Also includes a not-quite-working personality function for the backstop that fails because the system unwinder doesn't want us to be able to set rdi, even though that's explicitly one of the registers listed as being supported for landing pad passing purposes. --- .../src/context/context_asm.S | 12 +++- .../src/context/mod.rs | 2 +- .../lucet-runtime-internals/src/instance.rs | 2 +- .../src/instance/signals.rs | 67 ++++++++++++++----- lucet-runtime/lucet-runtime-tests/src/host.rs | 25 +++++++ 5 files changed, 87 insertions(+), 21 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S b/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S index 7fe25d516..bc7f34bcc 100644 --- a/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S +++ b/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S @@ -59,7 +59,17 @@ _lucet_context_bootstrap: .align 16 // unsure if `simple` is necessary here. GAS docs suggest this may omit some initial CFI instructions, and i want this to be as barren as possible for now. .cfi_startproc simple -.cfi_personality 0,win +// .cfi_personality 0,win + .cfi_escape 0x0f, /* DW_CFA_def_cfa_expression */ \ + 8, /* uleb128 length of expression bytes */ \ + 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ + 0x23, 0x08, /* DW_OP_plus_uconst (add 8 to the base of context to point to saved rsp) */ \ + 0x06, /* deref */ \ + 0x23, 0x08 /* add 8 for good luck? */ + + .cfi_offset 16, -8 /* return address value is stored at is CFA -8 */ + // this nop is load-bearing. We set up the backstop with a cfi_personality function so that stack unwinding through // guest code ends at a handler which can recover and switch back to the host. // However: libunwind doesn't treat the start of `lucet_context_backstop` as `lucet_context_backstop` but as the end diff --git a/lucet-runtime/lucet-runtime-internals/src/context/mod.rs b/lucet-runtime/lucet-runtime-internals/src/context/mod.rs index a6f3b13b3..1bf801c41 100644 --- a/lucet-runtime/lucet-runtime-internals/src/context/mod.rs +++ b/lucet-runtime/lucet-runtime-internals/src/context/mod.rs @@ -26,7 +26,7 @@ use thiserror::Error; #[repr(C)] pub(crate) struct GpRegs { pub(crate) rbx: u64, - pub(crate) rsp: u64, + pub rsp: u64, rbp: u64, pub(crate) rdi: u64, r12: u64, diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index 5186d2393..dfa324940 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -45,7 +45,7 @@ thread_local! { /// the swap. Meanwhile, the signal handler can run at any point during the guest function, and /// so it also must be able to immutably borrow the host context if it needs to swap back. The /// runtime borrowing constraints for a `RefCell` are therefore too strict for this variable. - pub(crate) static HOST_CTX: UnsafeCell = UnsafeCell::new(Context::new()); + pub static HOST_CTX: UnsafeCell = UnsafeCell::new(Context::new()); /// The currently-running `Instance`, if one exists. pub(crate) static CURRENT_INSTANCE: RefCell>> = RefCell::new(None); diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs index d2901edb7..9bd9ec98b 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs @@ -176,6 +176,17 @@ impl Instance { // run the body let res = f(self); + match &self.state { + State::Panicking { exception_obj } => { + eprintln!("re-raising exception!"); + unsafe { + uw::_Unwind_RaiseException(*exception_obj); + } + unreachable!() + } + st => eprintln!("state at exit: {}", st), + } + if self.ensure_signal_handler_installed { decrement_lucet_signal_state(); } @@ -457,8 +468,8 @@ use crate::instance::unwind as uw; extern "C" fn win( version: c_int, actions: uw::_Unwind_Action, - exceptionClass: u64, - exceptionObject: *mut uw::_Unwind_Exception, + exception_class: u64, + exception_obj: *mut uw::_Unwind_Exception, context: *mut uw::_Unwind_Context, ) -> uw::_Unwind_Reason_Code { if version != 1 { @@ -466,26 +477,46 @@ extern "C" fn win( } // dbg!(version); // dbg!(actions); - // dbg!(uw::fmt_exception_class(exceptionClass)); - // dbg!(exceptionObject); + // dbg!(uw::fmt_exception_class(exception_class)); + // dbg!(exception_obj); // dbg!(context); - - // CURRENT_INSTANCE.with(|current_instance| { - // eprintln!("instance state when panicking: {}", unsafe { - // ¤t_instance.borrow().unwrap().as_mut().state - // }); - // }); + // use std::io::Write; + // std::io::stdout().flush().unwrap(); if actions as i32 & uw::_UA_SEARCH_PHASE as i32 != 0 { - return uw::_URC_HANDLER_FOUND; + uw::_URC_HANDLER_FOUND } else { - use std::io::Write; - // ... wonder what happens if we try to call this stuff while panicked? - std::io::stdout() - .write_all(b"thank you libunwind! but our princess is in another castle!") - .unwrap(); - std::io::stdout().flush().unwrap(); - uw::_Unwind_Reason_Code::_URC_FATAL_PHASE2_ERROR + CURRENT_INSTANCE.with(|current_instance| unsafe { + current_instance.borrow().unwrap().as_mut().state = State::Panicking { + exception_obj: exception_obj, + }; + }); + HOST_CTX.with(|host_ctx| unsafe { + // 5 == rdi, which is reserved as a scratch register to transfer data to the landing + // pad, per the ABI... but the register indexing doesn't appear to actually work that + // way here. + uw::_Unwind_SetGR(context, 5, host_ctx.get() as uw::_Unwind_Word); + }); + unsafe { + // uw::_Unwind_SetGR(context, 0, 0xA); + // uw::_Unwind_SetGR(context, 1, 0xB); + // uw::_Unwind_SetGR(context, 2, 0xC); + // uw::_Unwind_SetGR(context, 3, 0xD); + } + unsafe { + uw::_Unwind_SetIP( + context, + crate::context::lucet_context_set as uw::_Unwind_Word, + ); + } + uw::_URC_INSTALL_CONTEXT + + // // ... wonder what happens if we try to call this stuff while panicked? + // std::io::stdout() + // .write_all(b"thank you libunwind! but our princess is in another castle!") + // .unwrap(); + // std::io::stdout().flush().unwrap(); + // uw::_Unwind_Reason_Code::_URC_FATAL_PHASE2_ERROR } } diff --git a/lucet-runtime/lucet-runtime-tests/src/host.rs b/lucet-runtime/lucet-runtime-tests/src/host.rs index 8b460fedb..017c3ce24 100644 --- a/lucet-runtime/lucet-runtime-tests/src/host.rs +++ b/lucet-runtime/lucet-runtime-tests/src/host.rs @@ -321,6 +321,31 @@ macro_rules! host_tests { } } + #[test] + fn run_hostcall_print_host_rsp() { + extern "C" { + fn hostcall_print_host_rsp(vmctx: *mut lucet_vmctx); + } + + unsafe extern "C" fn f(vmctx: *mut lucet_vmctx) { + hostcall_print_host_rsp(vmctx); + } + + let module = MockModuleBuilder::new() + .with_export_func(MockExportBuilder::new( + "f", + FunctionPointer::from_usize(f as usize), + )) + .build(); + + let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + inst.run("f", &[]).unwrap(); + } + #[test] fn run_hostcall_bad_borrow() { extern "C" { From f5255b2f484c3444a1a31c2c3c37196b2fd36a0e Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Tue, 13 Aug 2019 13:02:49 -0700 Subject: [PATCH 07/37] =?UTF-8?q?=F0=9F=8E=89=20add=20CFI=20for=20all=20sa?= =?UTF-8?q?ved=20registers=20in=20the=20backstop?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This means backtraces and panics now work across the host/guest stack boundary --- .../src/context/context_asm.S | 78 ++++++++++++++++++- .../lucet-runtime-internals/src/instance.rs | 4 +- .../src/instance/signals.rs | 21 ++--- .../src/instance/unwind.rs | 2 +- 4 files changed, 91 insertions(+), 14 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S b/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S index bc7f34bcc..c7f3bbed8 100644 --- a/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S +++ b/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S @@ -59,14 +59,88 @@ _lucet_context_bootstrap: .align 16 // unsure if `simple` is necessary here. GAS docs suggest this may omit some initial CFI instructions, and i want this to be as barren as possible for now. .cfi_startproc simple -// .cfi_personality 0,win + // .cfi_personality 0,win + + // TODO: generate these CFI instructions, perhaps using gimli + + // The idea with the call frame information here is to make any unwinder (libgcc, gdb, + // libunwind, etc) think that the previous frame is the call to `Context::swap()` that initially swapped + // us from host context into guest context. This means indicating that the values of + // callee-saved registers may be found within the saved host context, and that the canonical + // frame address is in the host stack. + + // The values we want to use are behind two dereferences (address of the parent context, parent + // context), so the normal `.cfi_*` assembler directives are not sufficient to specify their + // location. Instead, we have to use `.cfi_escape` so that we can write DWARF expressions that + // locate the values or their addresses. See the DWARF spec for more info. + + // Start by providing the canonical frame address. Even though unwinding runtimes _should_ be + // able to figure this out based on the saved rsp value, things go wrong if this is missing. + .cfi_escape 0x0f, /* DW_CFA_def_cfa_expression */ \ 8, /* uleb128 length of expression bytes */ \ 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ 0x23, 0x08, /* DW_OP_plus_uconst (add 8 to the base of context to point to saved rsp) */ \ 0x06, /* deref */ \ - 0x23, 0x08 /* add 8 for good luck? */ + 0x23, 0x08 /* add 8 to pop the return address from the call to lucet_context_swap */ + + // Now provide the saved rsp value. Note that unlike the other callee-saved register + // expressions, this is a `val-expression` so that we can increment the final value to account for + // the extra `lucet_context_swap` call frame we want to skip over + + .cfi_escape 0x16, 0x07, /* DW_CFA_val_expression(7=rsp) */ \ + 8, /* uleb128 length of expression bytes */ \ + 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ + 0x23, 0x08, /* DW_OP_plus_uconst (add 8 to the base of context to point to saved rsp) */ \ + 0x06, /* deref */ \ + 0x23, 0x08 /* add 8 to pop the return address from the call to lucet_context_swap */ + + // The remaining expressions are all very similar; they just return the address that points to + // the corresponding field on the `Context` struct + + .cfi_escape 0x10, 0x03, /* DW_CFA_expression(3=rbx) */ \ + 3, /* uleb128 length of expression bytes */ \ + 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 0x06 /* DW_OP_deref should then leave the parent context pointer on the stack */ \ + /* rbx is at offset 0 */ + + .cfi_escape 0x10, 0x06, /* DW_CFA_expression(6=rbp) */ \ + 5, /* uleb128 length of expression bytes */ \ + 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ + 0x23, 0x10 /* DW_OP_plus_uconst (add 16 to the base of context to point to saved rbp) */ + + .cfi_escape 0x10, 0x05, /* DW_CFA_expression(5=rdi) */ \ + 5, /* uleb128 length of expression bytes */ \ + 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ + 0x23, 0x18 /* DW_OP_plus_uconst (add 24 to the base of context to point to saved rdi) */ + + .cfi_escape 0x10, 0x0c, /* DW_CFA_expression(12=r12) */ \ + 5, /* uleb128 length of expression bytes */ \ + 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ + 0x23, 0x20 /* DW_OP_plus_uconst (add 32 to the base of context to point to saved r12) */ + + .cfi_escape 0x10, 0x0d, /* DW_CFA_expression(13=r13) */ \ + 5, /* uleb128 length of expression bytes */ \ + 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ + 0x23, 0x28 /* DW_OP_plus_uconst (add 40 to the base of context to point to saved r13) */ + + .cfi_escape 0x10, 0x0e, /* DW_CFA_expression(14=r14) */ \ + 5, /* uleb128 length of expression bytes */ \ + 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ + 0x23, 0x30 /* DW_OP_plus_uconst (add 48 to the base of context to point to saved r14) */ + + .cfi_escape 0x10, 0x0f, /* DW_CFA_expression(15=r15) */ \ + 5, /* uleb128 length of expression bytes */ \ + 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ + 0x23, 0x38 /* DW_OP_plus_uconst (add 56 to the base of context to point to saved r15) */ .cfi_offset 16, -8 /* return address value is stored at is CFA -8 */ diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index dfa324940..ff6e21206 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -220,7 +220,7 @@ pub struct Instance { module: Arc, /// The `Context` in which the guest program runs - pub(crate) ctx: Context, + pub ctx: Context, /// Instance state and error information pub(crate) state: State, @@ -233,7 +233,7 @@ pub struct Instance { pub lock_testpoints: Arc, /// The memory allocated for this instance - alloc: Alloc, + pub alloc: Alloc, /// Handler run for signals that do not arise from a known WebAssembly trap, or that involve /// memory outside of the current instance. diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs index 9bd9ec98b..296b56ab4 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs @@ -173,8 +173,11 @@ impl Instance { ); } - // run the body - let res = f(self); + let res = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + // run the body + f(self) + })) + .unwrap_or_else(|e| std::panic::resume_unwind(e)); match &self.state { State::Panicking { exception_obj } => { @@ -468,7 +471,7 @@ use crate::instance::unwind as uw; extern "C" fn win( version: c_int, actions: uw::_Unwind_Action, - exception_class: u64, + _exception_class: u64, exception_obj: *mut uw::_Unwind_Exception, context: *mut uw::_Unwind_Context, ) -> uw::_Unwind_Reason_Code { @@ -497,12 +500,12 @@ extern "C" fn win( // way here. uw::_Unwind_SetGR(context, 5, host_ctx.get() as uw::_Unwind_Word); }); - unsafe { - // uw::_Unwind_SetGR(context, 0, 0xA); - // uw::_Unwind_SetGR(context, 1, 0xB); - // uw::_Unwind_SetGR(context, 2, 0xC); - // uw::_Unwind_SetGR(context, 3, 0xD); - } + // unsafe { + // uw::_Unwind_SetGR(context, 0, 0xA); + // uw::_Unwind_SetGR(context, 1, 0xB); + // uw::_Unwind_SetGR(context, 2, 0xC); + // uw::_Unwind_SetGR(context, 3, 0xD); + // } unsafe { uw::_Unwind_SetIP( context, diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/unwind.rs b/lucet-runtime/lucet-runtime-internals/src/instance/unwind.rs index 2ae00c590..8951d2060 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/unwind.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/unwind.rs @@ -24,7 +24,7 @@ pub enum _Unwind_Reason_Code { pub use _Unwind_Reason_Code::*; pub type _Unwind_Exception_Class = u64; -pub fn fmt_exception_class(class: _Unwind_Exception_Class) -> String { +pub fn _fmt_exception_class(class: _Unwind_Exception_Class) -> String { let mut s = String::new(); for shift in &[56u64, 48, 40, 32, 24, 16, 8, 0] { s.push((class >> shift & 0xff) as u8 as char); From d5b2306c3200fc1f4778c5e26df29e7391052d97 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Tue, 13 Aug 2019 14:09:23 -0700 Subject: [PATCH 08/37] remove the attempt at a landing pad, and its associated code --- .../lucet-runtime-internals/src/instance.rs | 1 - .../src/instance/signals.rs | 87 +------ .../src/instance/unwind.rs | 239 ------------------ 3 files changed, 14 insertions(+), 313 deletions(-) delete mode 100644 lucet-runtime/lucet-runtime-internals/src/instance/unwind.rs diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index ff6e21206..0b5e70d48 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -2,7 +2,6 @@ pub mod execution; mod siginfo_ext; pub mod signals; pub mod state; -mod unwind; pub use crate::instance::execution::{KillError, KillState, KillSuccess, KillSwitch}; pub use crate::instance::signals::{signal_handler_none, SignalBehavior, SignalHandler}; diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs index 296b56ab4..317570f66 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs @@ -121,9 +121,9 @@ fn decrement_lucet_signal_state() { } impl Instance { - pub(crate) fn with_signals_on(&mut self, f: F) -> Result + pub(crate) fn with_signals_on(&mut self, f: F) -> Result<(), Error> where - F: FnOnce(&mut Instance) -> Result, + F: FnOnce(&mut Instance) -> Result<(), Error>, { let previous_sigstack = if self.ensure_sigstack_installed { validate_sigstack_size(self.alloc.slot().limits.signal_stack_size)?; @@ -173,22 +173,21 @@ impl Instance { ); } - let res = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let res = match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { // run the body f(self) - })) - .unwrap_or_else(|e| std::panic::resume_unwind(e)); - - match &self.state { - State::Panicking { exception_obj } => { - eprintln!("re-raising exception!"); - unsafe { - uw::_Unwind_RaiseException(*exception_obj); + })) { + Ok(res) => res, + Err(e) => match e.downcast::() { + Ok(details) => { + self.state = State::Terminated { details: *details }; + Ok(()) } - unreachable!() - } - st => eprintln!("state at exit: {}", st), - } + Err(e) => { + std::panic::resume_unwind(e); + } + }, + }; if self.ensure_signal_handler_installed { decrement_lucet_signal_state(); @@ -465,64 +464,6 @@ unsafe fn setup_guest_signal_state(ostate: &mut Option) { }); } -use crate::instance::unwind as uw; - -#[no_mangle] -extern "C" fn win( - version: c_int, - actions: uw::_Unwind_Action, - _exception_class: u64, - exception_obj: *mut uw::_Unwind_Exception, - context: *mut uw::_Unwind_Context, -) -> uw::_Unwind_Reason_Code { - if version != 1 { - return uw::_URC_FATAL_PHASE1_ERROR; - } - // dbg!(version); - // dbg!(actions); - // dbg!(uw::fmt_exception_class(exception_class)); - // dbg!(exception_obj); - // dbg!(context); - // use std::io::Write; - // std::io::stdout().flush().unwrap(); - - if actions as i32 & uw::_UA_SEARCH_PHASE as i32 != 0 { - uw::_URC_HANDLER_FOUND - } else { - CURRENT_INSTANCE.with(|current_instance| unsafe { - current_instance.borrow().unwrap().as_mut().state = State::Panicking { - exception_obj: exception_obj, - }; - }); - HOST_CTX.with(|host_ctx| unsafe { - // 5 == rdi, which is reserved as a scratch register to transfer data to the landing - // pad, per the ABI... but the register indexing doesn't appear to actually work that - // way here. - uw::_Unwind_SetGR(context, 5, host_ctx.get() as uw::_Unwind_Word); - }); - // unsafe { - // uw::_Unwind_SetGR(context, 0, 0xA); - // uw::_Unwind_SetGR(context, 1, 0xB); - // uw::_Unwind_SetGR(context, 2, 0xC); - // uw::_Unwind_SetGR(context, 3, 0xD); - // } - unsafe { - uw::_Unwind_SetIP( - context, - crate::context::lucet_context_set as uw::_Unwind_Word, - ); - } - uw::_URC_INSTALL_CONTEXT - - // // ... wonder what happens if we try to call this stuff while panicked? - // std::io::stdout() - // .write_all(b"thank you libunwind! but our princess is in another castle!") - // .unwrap(); - // std::io::stdout().flush().unwrap(); - // uw::_Unwind_Reason_Code::_URC_FATAL_PHASE2_ERROR - } -} - fn setup_guest_panic_hook() -> Arc) + Sync + Send + 'static>> { let saved_panic_hook = Arc::new(panic::take_hook()); let closure_saved_panic_hook = saved_panic_hook.clone(); diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/unwind.rs b/lucet-runtime/lucet-runtime-internals/src/instance/unwind.rs deleted file mode 100644 index 8951d2060..000000000 --- a/lucet-runtime/lucet-runtime-internals/src/instance/unwind.rs +++ /dev/null @@ -1,239 +0,0 @@ -#![allow(nonstandard_style)] - -macro_rules! cfg_if { - ( $( if #[cfg( $meta:meta )] { $($it1:item)* } else { $($it2:item)* } )* ) => - ( $( $( #[cfg($meta)] $it1)* $( #[cfg(not($meta))] $it2)* )* ) -} - -use libc::{c_int, c_void, uintptr_t}; - -#[repr(C)] -#[derive(Debug, Copy, Clone, PartialEq)] -pub enum _Unwind_Reason_Code { - _URC_NO_REASON = 0, - _URC_FOREIGN_EXCEPTION_CAUGHT = 1, - _URC_FATAL_PHASE2_ERROR = 2, - _URC_FATAL_PHASE1_ERROR = 3, - _URC_NORMAL_STOP = 4, - _URC_END_OF_STACK = 5, - _URC_HANDLER_FOUND = 6, - _URC_INSTALL_CONTEXT = 7, - _URC_CONTINUE_UNWIND = 8, - _URC_FAILURE = 9, // used only by ARM EHABI -} -pub use _Unwind_Reason_Code::*; - -pub type _Unwind_Exception_Class = u64; -pub fn _fmt_exception_class(class: _Unwind_Exception_Class) -> String { - let mut s = String::new(); - for shift in &[56u64, 48, 40, 32, 24, 16, 8, 0] { - s.push((class >> shift & 0xff) as u8 as char); - } - s -} - -pub type _Unwind_Word = uintptr_t; -pub type _Unwind_Ptr = uintptr_t; -pub type _Unwind_Trace_Fn = - extern "C" fn(ctx: *mut _Unwind_Context, arg: *mut c_void) -> _Unwind_Reason_Code; -#[cfg(target_arch = "x86")] -pub const unwinder_private_data_size: usize = 5; - -#[cfg(target_arch = "x86_64")] -pub const unwinder_private_data_size: usize = 6; - -#[cfg(all(target_arch = "arm", not(target_os = "ios")))] -pub const unwinder_private_data_size: usize = 20; - -#[cfg(all(target_arch = "arm", target_os = "ios"))] -pub const unwinder_private_data_size: usize = 5; - -#[cfg(target_arch = "aarch64")] -pub const unwinder_private_data_size: usize = 2; - -#[cfg(target_arch = "mips")] -pub const unwinder_private_data_size: usize = 2; - -#[cfg(target_arch = "mips64")] -pub const unwinder_private_data_size: usize = 2; - -#[cfg(any(target_arch = "powerpc", target_arch = "powerpc64"))] -pub const unwinder_private_data_size: usize = 2; - -#[cfg(target_arch = "s390x")] -pub const unwinder_private_data_size: usize = 2; - -#[cfg(target_arch = "sparc64")] -pub const unwinder_private_data_size: usize = 2; - -#[cfg(target_os = "emscripten")] -pub const unwinder_private_data_size: usize = 20; - -#[repr(C)] -pub struct _Unwind_Exception { - pub exception_class: _Unwind_Exception_Class, - pub exception_cleanup: _Unwind_Exception_Cleanup_Fn, - pub private: [_Unwind_Word; unwinder_private_data_size], -} - -pub enum _Unwind_Context {} - -pub type _Unwind_Exception_Cleanup_Fn = - extern "C" fn(unwind_code: _Unwind_Reason_Code, exception: *mut _Unwind_Exception); -extern "C" { - // #[unwind(allowed)] - pub fn _Unwind_Resume(exception: *mut _Unwind_Exception) -> !; - pub fn _Unwind_DeleteException(exception: *mut _Unwind_Exception); - pub fn _Unwind_GetLanguageSpecificData(ctx: *mut _Unwind_Context) -> *mut c_void; - pub fn _Unwind_GetRegionStart(ctx: *mut _Unwind_Context) -> _Unwind_Ptr; - pub fn _Unwind_GetTextRelBase(ctx: *mut _Unwind_Context) -> _Unwind_Ptr; - pub fn _Unwind_GetDataRelBase(ctx: *mut _Unwind_Context) -> _Unwind_Ptr; -} - -cfg_if! { -if #[cfg(all(any(target_os = "ios", target_os = "netbsd", not(target_arch = "arm"))))] { - // Not ARM EHABI - #[repr(C)] - #[derive(Copy, Clone, PartialEq, Debug)] - pub enum _Unwind_Action { - _UA_SEARCH_PHASE = 1, - _UA_CLEANUP_PHASE = 2, - _UA_HANDLER_FRAME = 4, - _UA_FORCE_UNWIND = 8, - _UA_END_OF_STACK = 16, - } - pub use _Unwind_Action::*; - - extern "C" { - pub fn _Unwind_GetGR(ctx: *mut _Unwind_Context, reg_index: c_int) -> _Unwind_Word; - pub fn _Unwind_SetGR(ctx: *mut _Unwind_Context, reg_index: c_int, value: _Unwind_Word); - pub fn _Unwind_GetIP(ctx: *mut _Unwind_Context) -> _Unwind_Word; - pub fn _Unwind_SetIP(ctx: *mut _Unwind_Context, value: _Unwind_Word); - pub fn _Unwind_GetIPInfo(ctx: *mut _Unwind_Context, ip_before_insn: *mut c_int) - -> _Unwind_Word; - pub fn _Unwind_FindEnclosingFunction(pc: *mut c_void) -> *mut c_void; - } - -} else { - // ARM EHABI - #[repr(C)] - #[derive(Copy, Clone, PartialEq)] - pub enum _Unwind_State { - _US_VIRTUAL_UNWIND_FRAME = 0, - _US_UNWIND_FRAME_STARTING = 1, - _US_UNWIND_FRAME_RESUME = 2, - _US_ACTION_MASK = 3, - _US_FORCE_UNWIND = 8, - _US_END_OF_STACK = 16, - } - pub use _Unwind_State::*; - - #[repr(C)] - enum _Unwind_VRS_Result { - _UVRSR_OK = 0, - _UVRSR_NOT_IMPLEMENTED = 1, - _UVRSR_FAILED = 2, - } - #[repr(C)] - enum _Unwind_VRS_RegClass { - _UVRSC_CORE = 0, - _UVRSC_VFP = 1, - _UVRSC_FPA = 2, - _UVRSC_WMMXD = 3, - _UVRSC_WMMXC = 4, - } - use _Unwind_VRS_RegClass::*; - #[repr(C)] - enum _Unwind_VRS_DataRepresentation { - _UVRSD_UINT32 = 0, - _UVRSD_VFPX = 1, - _UVRSD_FPAX = 2, - _UVRSD_UINT64 = 3, - _UVRSD_FLOAT = 4, - _UVRSD_DOUBLE = 5, - } - use _Unwind_VRS_DataRepresentation::*; - - pub const UNWIND_POINTER_REG: c_int = 12; - pub const UNWIND_IP_REG: c_int = 15; - - extern "C" { - fn _Unwind_VRS_Get(ctx: *mut _Unwind_Context, - regclass: _Unwind_VRS_RegClass, - regno: _Unwind_Word, - repr: _Unwind_VRS_DataRepresentation, - data: *mut c_void) - -> _Unwind_VRS_Result; - - fn _Unwind_VRS_Set(ctx: *mut _Unwind_Context, - regclass: _Unwind_VRS_RegClass, - regno: _Unwind_Word, - repr: _Unwind_VRS_DataRepresentation, - data: *mut c_void) - -> _Unwind_VRS_Result; - } - - // On Android or ARM/Linux, these are implemented as macros: - - pub unsafe fn _Unwind_GetGR(ctx: *mut _Unwind_Context, reg_index: c_int) -> _Unwind_Word { - let mut val: _Unwind_Word = 0; - _Unwind_VRS_Get(ctx, _UVRSC_CORE, reg_index as _Unwind_Word, _UVRSD_UINT32, - &mut val as *mut _ as *mut c_void); - val - } - - pub unsafe fn _Unwind_SetGR(ctx: *mut _Unwind_Context, reg_index: c_int, value: _Unwind_Word) { - let mut value = value; - _Unwind_VRS_Set(ctx, _UVRSC_CORE, reg_index as _Unwind_Word, _UVRSD_UINT32, - &mut value as *mut _ as *mut c_void); - } - - pub unsafe fn _Unwind_GetIP(ctx: *mut _Unwind_Context) - -> _Unwind_Word { - let val = _Unwind_GetGR(ctx, UNWIND_IP_REG); - (val & !1) as _Unwind_Word - } - - pub unsafe fn _Unwind_SetIP(ctx: *mut _Unwind_Context, - value: _Unwind_Word) { - // Propagate thumb bit to instruction pointer - let thumb_state = _Unwind_GetGR(ctx, UNWIND_IP_REG) & 1; - let value = value | thumb_state; - _Unwind_SetGR(ctx, UNWIND_IP_REG, value); - } - - pub unsafe fn _Unwind_GetIPInfo(ctx: *mut _Unwind_Context, - ip_before_insn: *mut c_int) - -> _Unwind_Word { - *ip_before_insn = 0; - _Unwind_GetIP(ctx) - } - - // This function also doesn't exist on Android or ARM/Linux, so make it a no-op - pub unsafe fn _Unwind_FindEnclosingFunction(pc: *mut c_void) -> *mut c_void { - pc - } -} - -if #[cfg(not(all(target_os = "ios", target_arch = "arm")))] { - // Not 32-bit iOS - extern "C" { - // #[unwind(allowed)] - pub fn _Unwind_RaiseException(exception: *mut _Unwind_Exception) -> _Unwind_Reason_Code; - pub fn _Unwind_Backtrace(trace: _Unwind_Trace_Fn, - trace_argument: *mut c_void) - -> _Unwind_Reason_Code; - } -} else { - // 32-bit iOS uses SjLj and does not provide _Unwind_Backtrace() - extern "C" { - // #[unwind(allowed)] - pub fn _Unwind_SjLj_RaiseException(e: *mut _Unwind_Exception) -> _Unwind_Reason_Code; - } - - #[inline] - pub unsafe fn _Unwind_RaiseException(exc: *mut _Unwind_Exception) -> _Unwind_Reason_Code { - _Unwind_SjLj_RaiseException(exc) - } -} -} // cfg_if! From 75ab74090f9a8ece83ce043c04f706670ba595ea Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Tue, 13 Aug 2019 14:10:29 -0700 Subject: [PATCH 09/37] add nested hostcall unwinding test --- .../guests/host/bindings.json | 4 +- .../guests/host/nested_error_unwind.c | 13 +++++ lucet-runtime/lucet-runtime-tests/src/host.rs | 52 +++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 lucet-runtime/lucet-runtime-tests/guests/host/nested_error_unwind.c diff --git a/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json b/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json index 383bcf316..96ef423a6 100644 --- a/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json +++ b/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json @@ -2,6 +2,8 @@ "env": { "hostcall_test_func_hello": "hostcall_test_func_hello", "hostcall_test_func_hostcall_error": "hostcall_test_func_hostcall_error", - "hostcall_test_func_hostcall_error_unwind": "hostcall_test_func_hostcall_error_unwind" + "hostcall_test_func_hostcall_error_unwind": "hostcall_test_func_hostcall_error_unwind", + "nested_error_unwind_outer": "nested_error_unwind_outer", + "nested_error_unwind_inner": "nested_error_unwind_inner" } } diff --git a/lucet-runtime/lucet-runtime-tests/guests/host/nested_error_unwind.c b/lucet-runtime/lucet-runtime-tests/guests/host/nested_error_unwind.c new file mode 100644 index 000000000..065414164 --- /dev/null +++ b/lucet-runtime/lucet-runtime-tests/guests/host/nested_error_unwind.c @@ -0,0 +1,13 @@ +#include + +extern void nested_error_unwind_outer(void (*)(void)); +extern void nested_error_unwind_inner(); + +void callback(void) { + nested_error_unwind_inner(); +} + +void entrypoint(void) +{ + nested_error_unwind_outer(callback); +} diff --git a/lucet-runtime/lucet-runtime-tests/src/host.rs b/lucet-runtime/lucet-runtime-tests/src/host.rs index 017c3ce24..533162874 100644 --- a/lucet-runtime/lucet-runtime-tests/src/host.rs +++ b/lucet-runtime/lucet-runtime-tests/src/host.rs @@ -27,6 +27,15 @@ macro_rules! host_tests { const ERROR_MESSAGE: &'static str = "hostcall_test_func_hostcall_error"; +<<<<<<< HEAD +======= + lazy_static! { + static ref HOSTCALL_MUTEX: Mutex<()> = Mutex::new(()); + static ref NESTED_OUTER: Mutex<()> = Mutex::new(()); + static ref NESTED_INNER: Mutex<()> = Mutex::new(()); + } + +>>>>>>> 3afa5c4... add nested hostcall unwinding test #[lucet_hostcall] #[no_mangle] pub fn hostcall_test_func_hostcall_error(_vmctx: &Vmctx) { @@ -208,10 +217,53 @@ macro_rules! host_tests { let _module = test_module_c("host", "trivial.c").expect("build and load module"); } +<<<<<<< HEAD #[test] fn load_nonexistent_module() { let module = DlModule::load("/non/existient/file"); assert!(module.is_err()); +======= + /// Check that if two segments of hostcall stack are present when terminating, that they + /// both get properly unwound. + #[test] + fn nested_error_unwind() { + let module = + test_module_c("host", "nested_error_unwind.c").expect("build and load module"); + let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + match inst.run("entrypoint", &[]) { + Err(Error::RuntimeTerminated(term)) => { + assert_eq!( + *term + .provided_details() + .expect("user provided termination reason") + .downcast_ref::<&'static str>() + .expect("error was static str"), + ERROR_MESSAGE + ); + } + res => panic!("unexpected result: {:?}", res), + } + + assert!(NESTED_OUTER.is_poisoned()); + assert!(NESTED_INNER.is_poisoned()); + } + + #[test] + fn run_fpe() { + let module = test_module_c("host", "fpe.c").expect("build and load module"); + let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + match inst.run("trigger_div_error", &[0u32.into()]) { + Err(Error::RuntimeFault(details)) => { + assert_eq!(details.trapcode, Some(TrapCode::IntegerDivByZero)); +>>>>>>> 3afa5c4... add nested hostcall unwinding test } #[test] From 1bd449d4fd35cc3ac749c43acafcd7826955b898 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Tue, 13 Aug 2019 15:14:23 -0700 Subject: [PATCH 10/37] add test that panics across guest with callee-saved variables --- .../guests/host/bindings.json | 4 +- .../guests/host/nested_error_unwind.c | 66 +++++++++++++++++-- lucet-runtime/lucet-runtime-tests/src/host.rs | 53 +++++++++++++++ 3 files changed, 117 insertions(+), 6 deletions(-) diff --git a/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json b/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json index 96ef423a6..8ffc90be5 100644 --- a/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json +++ b/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json @@ -4,6 +4,8 @@ "hostcall_test_func_hostcall_error": "hostcall_test_func_hostcall_error", "hostcall_test_func_hostcall_error_unwind": "hostcall_test_func_hostcall_error_unwind", "nested_error_unwind_outer": "nested_error_unwind_outer", - "nested_error_unwind_inner": "nested_error_unwind_inner" + "nested_error_unwind_inner": "nested_error_unwind_inner", + "nested_error_unwind_regs_outer": "nested_error_unwind_regs_outer", + "nested_error_unwind_regs_inner": "nested_error_unwind_regs_inner" } } diff --git a/lucet-runtime/lucet-runtime-tests/guests/host/nested_error_unwind.c b/lucet-runtime/lucet-runtime-tests/guests/host/nested_error_unwind.c index 065414164..4336e1039 100644 --- a/lucet-runtime/lucet-runtime-tests/guests/host/nested_error_unwind.c +++ b/lucet-runtime/lucet-runtime-tests/guests/host/nested_error_unwind.c @@ -1,13 +1,69 @@ #include +#include -extern void nested_error_unwind_outer(void (*)(void)); -extern void nested_error_unwind_inner(); +extern uint64_t nested_error_unwind_outer(uint64_t (*)(void)); +extern void nested_error_unwind_inner(); -void callback(void) { +uint64_t callback(void) +{ nested_error_unwind_inner(); + return 0; +} + +uint64_t entrypoint(void) +{ + return nested_error_unwind_outer(callback); +} + +extern uint64_t nested_error_unwind_regs_outer(uint64_t (*)(void)); +extern void nested_error_unwind_regs_inner(); + +uint64_t callback_regs(void) +{ + uint64_t a = 0xFFFFFFFF00000000; + uint64_t b = 0xFFFFFFFF00000001; + uint64_t c = 0xFFFFFFFF00000002; + uint64_t d = 0xFFFFFFFF00000003; + uint64_t e = 0xFFFFFFFF00000004; + uint64_t f = 0xFFFFFFFF00000005; + uint64_t g = 0xFFFFFFFF00000006; + uint64_t h = 0xFFFFFFFF00000007; + uint64_t i = 0xFFFFFFFF00000008; + uint64_t j = 0xFFFFFFFF00000009; + uint64_t k = 0xFFFFFFFF0000000A; + uint64_t l = 0xFFFFFFFF0000000B; + + a = b + c ^ 0; + b = c + d ^ 1; + c = d + e ^ 2; + d = e + f ^ 3; + e = f + g ^ 4; + f = g + h ^ 5; + g = h + i ^ 6; + h = i + j ^ 7; + i = j + k ^ 8; + j = k + l ^ 9; + k = l + a ^ 10; + l = a + b ^ 11; + + nested_error_unwind_regs_inner(); + + a = b * c & 0; + b = c * d & 1; + c = d * e & 2; + d = e * f & 3; + e = f * g & 4; + f = g * h & 5; + g = h * i & 6; + h = i * j & 7; + i = j * k & 8; + j = k * l & 9; + k = l * a & 10; + l = a * b & 11; + return l; } -void entrypoint(void) +uint64_t entrypoint_regs(void) { - nested_error_unwind_outer(callback); + return nested_error_unwind_regs_outer(callback_regs); } diff --git a/lucet-runtime/lucet-runtime-tests/src/host.rs b/lucet-runtime/lucet-runtime-tests/src/host.rs index 533162874..6f49d97d2 100644 --- a/lucet-runtime/lucet-runtime-tests/src/host.rs +++ b/lucet-runtime/lucet-runtime-tests/src/host.rs @@ -33,6 +33,30 @@ macro_rules! host_tests { static ref HOSTCALL_MUTEX: Mutex<()> = Mutex::new(()); static ref NESTED_OUTER: Mutex<()> = Mutex::new(()); static ref NESTED_INNER: Mutex<()> = Mutex::new(()); + static ref NESTED_REGS_OUTER: Mutex<()> = Mutex::new(()); + static ref NESTED_REGS_INNER: Mutex<()> = Mutex::new(()); + } + + #[inline] + unsafe fn unwind_outer(vmctx: &mut Vmctx, mutex: &Mutex<()>, cb_idx: u32) -> u64 { + let lock = mutex.lock().unwrap(); + let func = vmctx + .get_func_from_idx(0, cb_idx) + .expect("can get function by index"); + let func = std::mem::transmute:: u64>( + func.ptr.as_usize(), + ); + let res = (func)(vmctx.as_raw()); + drop(lock); + res + } + + #[allow(unreachable_code)] + #[inline] + unsafe fn unwind_inner(vmctx: &mut Vmctx, mutex: &Mutex<()>) { + let lock = mutex.lock().unwrap(); + lucet_hostcall_terminate!(ERROR_MESSAGE); + drop(lock); } >>>>>>> 3afa5c4... add nested hostcall unwinding test @@ -252,6 +276,35 @@ macro_rules! host_tests { assert!(NESTED_INNER.is_poisoned()); } + /// Like `nested_error_unwind`, but the guest code callback in between the two segments of + /// hostcall stack uses enough locals to require saving callee registers. + #[test] + fn nested_error_unwind_regs() { + let module = + test_module_c("host", "nested_error_unwind.c").expect("build and load module"); + let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + match inst.run("entrypoint_regs", &[]) { + Err(Error::RuntimeTerminated(term)) => { + assert_eq!( + *term + .provided_details() + .expect("user provided termination reason") + .downcast_ref::<&'static str>() + .expect("error was static str"), + ERROR_MESSAGE + ); + } + res => panic!("unexpected result: {:?}", res), + } + + assert!(NESTED_REGS_OUTER.is_poisoned()); + assert!(NESTED_REGS_INNER.is_poisoned()); + } + #[test] fn run_fpe() { let module = test_module_c("host", "fpe.c").expect("build and load module"); From cc36826f163a00f29ff8a94ef0335ce629f339a6 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Wed, 14 Aug 2019 14:36:25 -0700 Subject: [PATCH 11/37] update cranelift version and add callee-saved regs test --- lucetc/src/compiler.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lucetc/src/compiler.rs b/lucetc/src/compiler.rs index dc237ae43..a7d9396d0 100644 --- a/lucetc/src/compiler.rs +++ b/lucetc/src/compiler.rs @@ -294,6 +294,7 @@ impl<'a> Compiler<'a> { let mut clif_context = ClifContext::new(); clif_context.func.name = func.name.as_externalname(); clif_context.func.signature = func.signature.clone(); + clif_context.func.collect_debug_info(); func_translator .translate( From 65beadaa0cb94a837b4b7f195432d74335c6a2b9 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Thu, 15 Aug 2019 16:32:20 -0700 Subject: [PATCH 12/37] add (currently-ignored) callee-saved registers test This test works with the nightly-only `#[unwind(allowed)]` attribute, which we'll hopefully be able to help move along. --- .../guests/host/bindings.json | 4 +- .../guests/host/nested_error_unwind.c | 52 +++++++++++++++++++ lucet-runtime/lucet-runtime-tests/src/host.rs | 18 +++++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json b/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json index 8ffc90be5..43965ce9e 100644 --- a/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json +++ b/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json @@ -6,6 +6,8 @@ "nested_error_unwind_outer": "nested_error_unwind_outer", "nested_error_unwind_inner": "nested_error_unwind_inner", "nested_error_unwind_regs_outer": "nested_error_unwind_regs_outer", - "nested_error_unwind_regs_inner": "nested_error_unwind_regs_inner" + "nested_error_unwind_regs_inner": "nested_error_unwind_regs_inner", + "restore_callee_saved": "hostcall_restore_callee_saved", + "hostcall_panic": "hostcall_panic" } } diff --git a/lucet-runtime/lucet-runtime-tests/guests/host/nested_error_unwind.c b/lucet-runtime/lucet-runtime-tests/guests/host/nested_error_unwind.c index 4336e1039..1a9e09392 100644 --- a/lucet-runtime/lucet-runtime-tests/guests/host/nested_error_unwind.c +++ b/lucet-runtime/lucet-runtime-tests/guests/host/nested_error_unwind.c @@ -67,3 +67,55 @@ uint64_t entrypoint_regs(void) { return nested_error_unwind_regs_outer(callback_regs); } + +extern uint64_t restore_callee_saved(uint64_t (*)(void)); +extern void hostcall_panic(); + +uint64_t callback_restore(void) +{ + uint64_t a = 0xFFFFFFFF00000000; + uint64_t b = 0xFFFFFFFF00000001; + uint64_t c = 0xFFFFFFFF00000002; + uint64_t d = 0xFFFFFFFF00000003; + uint64_t e = 0xFFFFFFFF00000004; + uint64_t f = 0xFFFFFFFF00000005; + uint64_t g = 0xFFFFFFFF00000006; + uint64_t h = 0xFFFFFFFF00000007; + uint64_t i = 0xFFFFFFFF00000008; + uint64_t j = 0xFFFFFFFF00000009; + uint64_t k = 0xFFFFFFFF0000000A; + uint64_t l = 0xFFFFFFFF0000000B; + + a = b + c ^ 0; + b = c + d ^ 1; + c = d + e ^ 2; + d = e + f ^ 3; + e = f + g ^ 4; + f = g + h ^ 5; + g = h + i ^ 6; + h = i + j ^ 7; + i = j + k ^ 8; + j = k + l ^ 9; + k = l + a ^ 10; + l = a + b ^ 11; + + hostcall_panic(); + + a = b * c & 0; + b = c * d & 1; + c = d * e & 2; + d = e * f & 3; + e = f * g & 4; + f = g * h & 5; + g = h * i & 6; + h = i * j & 7; + i = j * k & 8; + j = k * l & 9; + k = l * a & 10; + l = a * b & 11; + return l; +} + +uint64_t entrypoint_restore(void) { + return restore_callee_saved(callback_restore); +} diff --git a/lucet-runtime/lucet-runtime-tests/src/host.rs b/lucet-runtime/lucet-runtime-tests/src/host.rs index 6f49d97d2..38cf21e77 100644 --- a/lucet-runtime/lucet-runtime-tests/src/host.rs +++ b/lucet-runtime/lucet-runtime-tests/src/host.rs @@ -305,6 +305,24 @@ macro_rules! host_tests { assert!(NESTED_REGS_INNER.is_poisoned()); } + /// Ensures that callee-saved registers are properly restored following a `catch_unwind` + /// that catches a panic. Currently disabled because it relies on UB until + /// `#[unwind(allowed)]` is stabilized. + #[ignore] + #[test] + fn restore_callee_saved() { + let module = + test_module_c("host", "nested_error_unwind.c").expect("build and load module"); + let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + assert_eq!( + u64::from(inst.run("entrypoint_restore", &[]).unwrap()), + 6148914668330025056 + ); + } + #[test] fn run_fpe() { let module = test_module_c("host", "fpe.c").expect("build and load module"); From 829124a947e32b410dc417488fe0128c5483e2c0 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Wed, 18 Sep 2019 16:27:02 -0700 Subject: [PATCH 13/37] switch to the most recent working nightly, and fix for wasi-sdk 6 --- helpers/indent.sh | 2 +- .../src/instance/signals.rs | 2 +- lucet-runtime/lucet-runtime-tests/src/host.rs | 302 ++++++++---------- rust-toolchain | 2 +- 4 files changed, 144 insertions(+), 164 deletions(-) diff --git a/helpers/indent.sh b/helpers/indent.sh index 4fd6d1658..1bc30d3f2 100755 --- a/helpers/indent.sh +++ b/helpers/indent.sh @@ -10,7 +10,7 @@ cleanup() { } trap cleanup 1 2 3 6 15 -RUSTFMT_VERSION=1.4.12-stable +RUSTFMT_VERSION=1.4.12-nightly if ! rustfmt --version | grep -q "rustfmt $RUSTFMT_VERSION"; then echo "indent requires rustfmt $RUSTFMT_VERSION" diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs index 317570f66..e0b157ef6 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs @@ -180,7 +180,7 @@ impl Instance { Ok(res) => res, Err(e) => match e.downcast::() { Ok(details) => { - self.state = State::Terminated { details: *details }; + self.state = State::Terminating { details: *details }; Ok(()) } Err(e) => { diff --git a/lucet-runtime/lucet-runtime-tests/src/host.rs b/lucet-runtime/lucet-runtime-tests/src/host.rs index 38cf21e77..c144e77c4 100644 --- a/lucet-runtime/lucet-runtime-tests/src/host.rs +++ b/lucet-runtime/lucet-runtime-tests/src/host.rs @@ -25,72 +25,6 @@ macro_rules! host_tests { assert!(module.is_err()); } - const ERROR_MESSAGE: &'static str = "hostcall_test_func_hostcall_error"; - -<<<<<<< HEAD -======= - lazy_static! { - static ref HOSTCALL_MUTEX: Mutex<()> = Mutex::new(()); - static ref NESTED_OUTER: Mutex<()> = Mutex::new(()); - static ref NESTED_INNER: Mutex<()> = Mutex::new(()); - static ref NESTED_REGS_OUTER: Mutex<()> = Mutex::new(()); - static ref NESTED_REGS_INNER: Mutex<()> = Mutex::new(()); - } - - #[inline] - unsafe fn unwind_outer(vmctx: &mut Vmctx, mutex: &Mutex<()>, cb_idx: u32) -> u64 { - let lock = mutex.lock().unwrap(); - let func = vmctx - .get_func_from_idx(0, cb_idx) - .expect("can get function by index"); - let func = std::mem::transmute:: u64>( - func.ptr.as_usize(), - ); - let res = (func)(vmctx.as_raw()); - drop(lock); - res - } - - #[allow(unreachable_code)] - #[inline] - unsafe fn unwind_inner(vmctx: &mut Vmctx, mutex: &Mutex<()>) { - let lock = mutex.lock().unwrap(); - lucet_hostcall_terminate!(ERROR_MESSAGE); - drop(lock); - } - ->>>>>>> 3afa5c4... add nested hostcall unwinding test - #[lucet_hostcall] - #[no_mangle] - pub fn hostcall_test_func_hostcall_error(_vmctx: &Vmctx) { - lucet_hostcall_terminate!(ERROR_MESSAGE); - } - - #[lucet_hostcall] - #[no_mangle] - pub fn hostcall_test_func_hello(vmctx: &Vmctx, hello_ptr: u32, hello_len: u32) { - let heap = vmctx.heap(); - let hello = heap.as_ptr() as usize + hello_ptr as usize; - if !vmctx.check_heap(hello as *const c_void, hello_len as usize) { - lucet_hostcall_terminate!("heap access"); - } - let hello = - unsafe { std::slice::from_raw_parts(hello as *const u8, hello_len as usize) }; - if hello.starts_with(b"hello") { - *vmctx.get_embed_ctx_mut::() = true; - } - } - - #[lucet_hostcall] - #[allow(unreachable_code)] - #[no_mangle] - pub fn hostcall_test_func_hostcall_error_unwind(vmctx: &Vmctx) { - let lock = vmctx.get_embed_ctx::>>(); - let _mutex_guard = lock.lock().unwrap(); - lucet_hostcall_terminate!(ERROR_MESSAGE); - drop(_mutex_guard); - } - #[lucet_hostcall] #[no_mangle] pub fn hostcall_bad_borrow(vmctx: &Vmctx) -> bool { @@ -219,7 +153,6 @@ macro_rules! host_tests { $( mod $region_id { - use lazy_static::lazy_static; use libc::c_void; use lucet_runtime::vmctx::{lucet_vmctx, Vmctx}; @@ -232,109 +165,80 @@ macro_rules! host_tests { use $crate::helpers::{FunctionPointer, HeapSpec, MockExportBuilder, MockModuleBuilder}; use $TestRegion as TestRegion; + const ERROR_MESSAGE: &'static str = "hostcall_test_func_hostcall_error"; + lazy_static! { static ref HOSTCALL_MUTEX: Arc> = Arc::new(Mutex::new(())); + static ref NESTED_OUTER: Mutex<()> = Mutex::new(()); + static ref NESTED_INNER: Mutex<()> = Mutex::new(()); + static ref NESTED_REGS_OUTER: Mutex<()> = Mutex::new(()); + static ref NESTED_REGS_INNER: Mutex<()> = Mutex::new(()); } - #[test] - fn load_module() { - let _module = test_module_c("host", "trivial.c").expect("build and load module"); + #[inline] + unsafe fn unwind_outer(vmctx: &mut Vmctx, mutex: &Mutex<()>, cb_idx: u32) -> u64 { + let lock = mutex.lock().unwrap(); + let func = vmctx + .get_func_from_idx(0, cb_idx) + .expect("can get function by index"); + let func = std::mem::transmute:: u64>( + func.ptr.as_usize(), + ); + let res = (func)(vmctx.as_raw()); + drop(lock); + res } -<<<<<<< HEAD - #[test] - fn load_nonexistent_module() { - let module = DlModule::load("/non/existient/file"); - assert!(module.is_err()); -======= - /// Check that if two segments of hostcall stack are present when terminating, that they - /// both get properly unwound. - #[test] - fn nested_error_unwind() { - let module = - test_module_c("host", "nested_error_unwind.c").expect("build and load module"); - let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - - match inst.run("entrypoint", &[]) { - Err(Error::RuntimeTerminated(term)) => { - assert_eq!( - *term - .provided_details() - .expect("user provided termination reason") - .downcast_ref::<&'static str>() - .expect("error was static str"), - ERROR_MESSAGE - ); + #[allow(unreachable_code)] + #[inline] + unsafe fn unwind_inner(vmctx: &mut Vmctx, mutex: &Mutex<()>) { + let lock = mutex.lock().unwrap(); + lucet_hostcall_terminate!(ERROR_MESSAGE); + drop(lock); } - res => panic!("unexpected result: {:?}", res), - } - assert!(NESTED_OUTER.is_poisoned()); - assert!(NESTED_INNER.is_poisoned()); - } + /* + #[lucet_hostcall] + #[no_mangle] + pub fn hostcall_test_func_hostcall_error(_vmctx: &Vmctx) { + lucet_hostcall_terminate!(ERROR_MESSAGE); + } - /// Like `nested_error_unwind`, but the guest code callback in between the two segments of - /// hostcall stack uses enough locals to require saving callee registers. - #[test] - fn nested_error_unwind_regs() { - let module = - test_module_c("host", "nested_error_unwind.c").expect("build and load module"); - let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - - match inst.run("entrypoint_regs", &[]) { - Err(Error::RuntimeTerminated(term)) => { - assert_eq!( - *term - .provided_details() - .expect("user provided termination reason") - .downcast_ref::<&'static str>() - .expect("error was static str"), - ERROR_MESSAGE - ); + #[lucet_hostcall] + #[no_mangle] + pub fn hostcall_test_func_hello(vmctx: &Vmctx, hello_ptr: u32, hello_len: u32) { + let heap = vmctx.heap(); + let hello = heap.as_ptr() as usize + hello_ptr as usize; + if !vmctx.check_heap(hello as *const c_void, hello_len as usize) { + lucet_hostcall_terminate!("heap access"); + } + let hello = + unsafe { std::slice::from_raw_parts(hello as *const u8, hello_len as usize) }; + if hello.starts_with(b"hello") { + *vmctx.get_embed_ctx_mut::() = true; + } } - res => panic!("unexpected result: {:?}", res), - } - assert!(NESTED_REGS_OUTER.is_poisoned()); - assert!(NESTED_REGS_INNER.is_poisoned()); - } + #[lucet_hostcall] + #[allow(unreachable_code)] + #[no_mangle] + pub fn hostcall_test_func_hostcall_error_unwind(vmctx: &Vmctx) { + let lock = vmctx.get_embed_ctx::>>(); + let _mutex_guard = lock.lock().unwrap(); + lucet_hostcall_terminate!(ERROR_MESSAGE); + drop(_mutex_guard); + } + */ - /// Ensures that callee-saved registers are properly restored following a `catch_unwind` - /// that catches a panic. Currently disabled because it relies on UB until - /// `#[unwind(allowed)]` is stabilized. - #[ignore] - #[test] - fn restore_callee_saved() { - let module = - test_module_c("host", "nested_error_unwind.c").expect("build and load module"); - let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - assert_eq!( - u64::from(inst.run("entrypoint_restore", &[]).unwrap()), - 6148914668330025056 - ); - } + #[test] + fn load_module() { + let _module = test_module_c("host", "trivial.c").expect("build and load module"); + } - #[test] - fn run_fpe() { - let module = test_module_c("host", "fpe.c").expect("build and load module"); - let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - - match inst.run("trigger_div_error", &[0u32.into()]) { - Err(Error::RuntimeFault(details)) => { - assert_eq!(details.trapcode, Some(TrapCode::IntegerDivByZero)); ->>>>>>> 3afa5c4... add nested hostcall unwinding test + #[test] + fn load_nonexistent_module() { + let module = DlModule::load("/non/existient/file"); + assert!(module.is_err()); } #[test] @@ -383,14 +287,14 @@ macro_rules! host_tests { .expect("instance can be created"); match inst.run("main", &[0u32.into(), 0i32.into()]) { - Err(Error::RuntimeTerminated(term)) => { + Err(Error::RuntimeTerminated(term)) => { assert_eq!( *term .provided_details() .expect("user provided termination reason") .downcast_ref::<&'static str>() .expect("error was static str"), - super::ERROR_MESSAGE + ERROR_MESSAGE ); } res => panic!("unexpected result: {:?}", res), @@ -417,7 +321,7 @@ macro_rules! host_tests { .expect("user provided termination reason") .downcast_ref::<&'static str>() .expect("error was static str"), - super::ERROR_MESSAGE + ERROR_MESSAGE ); } res => panic!("unexpected result: {:?}", res), @@ -426,6 +330,82 @@ macro_rules! host_tests { assert!(HOSTCALL_MUTEX.is_poisoned()); } + /// Check that if two segments of hostcall stack are present when terminating, that they + /// both get properly unwound. + #[test] + fn nested_error_unwind() { + let module = + test_module_c("host", "nested_error_unwind.c").expect("build and load module"); + let region = ::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + match inst.run("entrypoint", &[]) { + Err(Error::RuntimeTerminated(term)) => { + assert_eq!( + *term + .provided_details() + .expect("user provided termination reason") + .downcast_ref::<&'static str>() + .expect("error was static str"), + ERROR_MESSAGE + ); + } + res => panic!("unexpected result: {:?}", res), + } + + assert!(NESTED_OUTER.is_poisoned()); + assert!(NESTED_INNER.is_poisoned()); + } + + /// Like `nested_error_unwind`, but the guest code callback in between the two segments of + /// hostcall stack uses enough locals to require saving callee registers. + #[test] + fn nested_error_unwind_regs() { + let module = + test_module_c("host", "nested_error_unwind.c").expect("build and load module"); + let region = ::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + match inst.run("entrypoint_regs", &[]) { + Err(Error::RuntimeTerminated(term)) => { + assert_eq!( + *term + .provided_details() + .expect("user provided termination reason") + .downcast_ref::<&'static str>() + .expect("error was static str"), + ERROR_MESSAGE + ); + } + res => panic!("unexpected result: {:?}", res), + } + + assert!(NESTED_REGS_OUTER.is_poisoned()); + assert!(NESTED_REGS_INNER.is_poisoned()); + } + + /// Ensures that callee-saved registers are properly restored following a `catch_unwind` + /// that catches a panic. Currently disabled because it relies on UB until + /// `#[unwind(allowed)]` is stabilized. + #[ignore] + #[test] + fn restore_callee_saved() { + let module = + test_module_c("host", "nested_error_unwind.c").expect("build and load module"); + let region = ::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + assert_eq!( + u64::from(inst.run("entrypoint_restore", &[]).unwrap().unwrap_returned()), + 6148914668330025056 + ); + } + #[test] fn run_fpe() { let module = test_module_c("host", "fpe.c").expect("build and load module"); @@ -461,7 +441,7 @@ macro_rules! host_tests { )) .build(); - let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); + let region = ::create(1, &Limits::default()).expect("region can be created"); let mut inst = region .new_instance(module) .expect("instance can be created"); diff --git a/rust-toolchain b/rust-toolchain index 3987c4729..4ce925c82 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1 +1 @@ -1.43.1 +nightly-2020-06-06 From e7026d21e9ee227a7e01ea38291873b713884776 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Fri, 27 Sep 2019 12:44:15 -0700 Subject: [PATCH 14/37] add `#[unwind(allowed)]` to hostcalls and enable relevant test --- benchmarks/lucet-benchmarks/src/lib.rs | 2 ++ .../src/hostcall_macros.rs | 5 +++++ .../lucet-runtime-internals/src/vmctx.rs | 1 + lucet-runtime/lucet-runtime-tests/src/host.rs | 16 ++++++++++++++++ lucet-runtime/src/lib.rs | 3 +++ lucet-runtime/tests/entrypoint.rs | 2 ++ lucet-runtime/tests/guest_fault.rs | 2 ++ lucet-runtime/tests/host.rs | 2 ++ lucet-runtime/tests/strcmp.rs | 2 ++ lucet-wasi/src/lib.rs | 1 + 10 files changed, 36 insertions(+) diff --git a/benchmarks/lucet-benchmarks/src/lib.rs b/benchmarks/lucet-benchmarks/src/lib.rs index 989b582af..28a866483 100644 --- a/benchmarks/lucet-benchmarks/src/lib.rs +++ b/benchmarks/lucet-benchmarks/src/lib.rs @@ -1,3 +1,5 @@ +#![feature(unwind_attributes)] + mod compile; mod context; mod modules; diff --git a/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs b/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs index d8a59e052..c04b9d067 100644 --- a/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs +++ b/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs @@ -21,6 +21,10 @@ /// // body /// } /// ``` +/// +/// **Note:** This macro currently uses the unstable `#![feature(unwind_attributes)]`, which must be +/// enabled in any crate where the macro is used. In the long term, we hope to move back to stable +/// once [unwinding across FFI](https://github.com/rust-lang/rfcs/pull/2753) is defined. #[macro_export] #[deprecated(since = "0.5.0", note = "Use the #[lucet_hostcall] attribute instead")] macro_rules! lucet_hostcalls { @@ -40,6 +44,7 @@ macro_rules! lucet_hostcalls { #[allow(unused_unsafe)] #[$crate::lucet_hostcall] $(#[$attr])* + #[unwind(allowed)] pub unsafe extern "C" fn $name( $vmctx: &lucet_runtime::vmctx::Vmctx, $( $arg: $arg_ty ),* diff --git a/lucet-runtime/lucet-runtime-internals/src/vmctx.rs b/lucet-runtime/lucet-runtime-internals/src/vmctx.rs index 981b5be92..c08cd5e0a 100644 --- a/lucet-runtime/lucet-runtime-internals/src/vmctx.rs +++ b/lucet-runtime/lucet-runtime-internals/src/vmctx.rs @@ -295,6 +295,7 @@ impl Vmctx { /// [ongoing](https://github.com/bytecodealliance/lucet/pull/254). /// /// ```no_run + /// # #![feature(unwind_attributes)] /// use lucet_runtime_macros::lucet_hostcall; /// use lucet_runtime_internals::lucet_hostcall_terminate; /// use lucet_runtime_internals::vmctx::{lucet_vmctx, Vmctx}; diff --git a/lucet-runtime/lucet-runtime-tests/src/host.rs b/lucet-runtime/lucet-runtime-tests/src/host.rs index c144e77c4..66fc03c8a 100644 --- a/lucet-runtime/lucet-runtime-tests/src/host.rs +++ b/lucet-runtime/lucet-runtime-tests/src/host.rs @@ -406,6 +406,22 @@ macro_rules! host_tests { ); } + /// Ensures that callee-saved registers are properly restored following a `catch_unwind` + /// that catches a panic. + #[test] + fn restore_callee_saved() { + let module = + test_module_c("host", "nested_error_unwind.c").expect("build and load module"); + let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + assert_eq!( + u64::from(inst.run("entrypoint_restore", &[]).unwrap()), + 6148914668330025056 + ); + } + #[test] fn run_fpe() { let module = test_module_c("host", "fpe.c").expect("build and load module"); diff --git a/lucet-runtime/src/lib.rs b/lucet-runtime/src/lib.rs index a710f746c..663912dc8 100644 --- a/lucet-runtime/src/lib.rs +++ b/lucet-runtime/src/lib.rs @@ -83,6 +83,7 @@ //! to make a `u32` available to hostcalls: //! //! ```no_run +//! # #![feature(unwind_attributes)] //! use lucet_runtime::{DlModule, Limits, MmapRegion, Region, lucet_hostcall}; //! use lucet_runtime::vmctx::{Vmctx, lucet_vmctx}; //! @@ -198,6 +199,7 @@ //! and yield it when appropriate. //! //! ```no_run +//! # #![feature(unwind_attributes)] //! use lucet_runtime::lucet_hostcall; //! use lucet_runtime::vmctx::Vmctx; //! @@ -395,6 +397,7 @@ //! lucet-runtime-internals = { version = "0.6.1", default-features = false } //! ``` +#![feature(unwind_attributes)] #![deny(bare_trait_objects)] // This makes `lucet_runtime` in the expansion of `#[lucet_hostcall]` resolve to something diff --git a/lucet-runtime/tests/entrypoint.rs b/lucet-runtime/tests/entrypoint.rs index a3ecfb89a..c1728b46a 100644 --- a/lucet-runtime/tests/entrypoint.rs +++ b/lucet-runtime/tests/entrypoint.rs @@ -1,3 +1,5 @@ +#![feature(unwind_attributes)] + use lucet_runtime_tests::entrypoint_tests; cfg_if::cfg_if! { diff --git a/lucet-runtime/tests/guest_fault.rs b/lucet-runtime/tests/guest_fault.rs index bb4353c0a..12583bcba 100644 --- a/lucet-runtime/tests/guest_fault.rs +++ b/lucet-runtime/tests/guest_fault.rs @@ -1,3 +1,5 @@ +#![feature(unwind_attributes)] + use lucet_runtime_tests::{guest_fault_common_defs, guest_fault_tests}; guest_fault_common_defs!(); diff --git a/lucet-runtime/tests/host.rs b/lucet-runtime/tests/host.rs index 5404c2c15..19c6b8068 100644 --- a/lucet-runtime/tests/host.rs +++ b/lucet-runtime/tests/host.rs @@ -1,3 +1,5 @@ +#![feature(unwind_attributes)] + use lucet_runtime_tests::host_tests; cfg_if::cfg_if! { diff --git a/lucet-runtime/tests/strcmp.rs b/lucet-runtime/tests/strcmp.rs index badd163c1..a0a540995 100644 --- a/lucet-runtime/tests/strcmp.rs +++ b/lucet-runtime/tests/strcmp.rs @@ -1,3 +1,5 @@ +#![feature(unwind_attributes)] + use lucet_runtime_tests::strcmp_tests; cfg_if::cfg_if! { diff --git a/lucet-wasi/src/lib.rs b/lucet-wasi/src/lib.rs index fc7281209..4d20eccef 100644 --- a/lucet-wasi/src/lib.rs +++ b/lucet-wasi/src/lib.rs @@ -1,3 +1,4 @@ +#![feature(unwind_attributes)] #![deny(bare_trait_objects)] pub mod c_api; From e96dd2d8c40965bcab99bb5e850fc298e88b4e7f Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Fri, 27 Sep 2019 13:30:43 -0700 Subject: [PATCH 15/37] add unwind attribute to spectest crate, install nightly in devenv --- lucet-spectest/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lucet-spectest/src/lib.rs b/lucet-spectest/src/lib.rs index 2b7191212..3b15f950a 100644 --- a/lucet-spectest/src/lib.rs +++ b/lucet-spectest/src/lib.rs @@ -1,3 +1,4 @@ +#![feature(unwind_attributes)] #![deny(bare_trait_objects)] pub mod error; From 84d9394ec185e4cfe0275a1b84dda3a77cd0bca3 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Tue, 1 Oct 2019 11:45:11 -0700 Subject: [PATCH 16/37] wip: induce unwinding when resetting a faulted instance The current state of the repo is such that only the `fault_unwind` test is currently relevant: ``` cargo test -p lucet-runtime --test host fault_unwind -- --nocapture ``` Currently stuck figuring out how to set up the stack properly in order to return into the function that panics. If I pad the stack with a zero word in order to keep it 16-byte aligned, the unwinding runtime interprets that zero as a return address and fails. If I don't add the padding, later instructions fault because of unaligned arguments. We probably need to add a shim that uses `.cfi` directives in order to make the unwinding runtime skip over the padding. --- .../src/context/mod.rs | 36 ++++++++-------- .../lucet-runtime-internals/src/instance.rs | 39 +++++++++++++++++ .../src/instance/signals.rs | 22 ++++++++++ .../lucet-runtime-internals/src/lib.rs | 1 + .../src/sysdeps/linux.rs | 42 +++++++++++++++++++ .../guests/host/bindings.json | 1 + .../guests/host/fault_unwind.c | 15 +++++++ .../lucet-runtime-tests/src/build.rs | 3 ++ lucet-runtime/lucet-runtime-tests/src/host.rs | 14 +++++++ 9 files changed, 155 insertions(+), 18 deletions(-) create mode 100644 lucet-runtime/lucet-runtime-tests/guests/host/fault_unwind.c diff --git a/lucet-runtime/lucet-runtime-internals/src/context/mod.rs b/lucet-runtime/lucet-runtime-internals/src/context/mod.rs index 1bf801c41..37c37b484 100644 --- a/lucet-runtime/lucet-runtime-internals/src/context/mod.rs +++ b/lucet-runtime/lucet-runtime-internals/src/context/mod.rs @@ -25,15 +25,15 @@ use thiserror::Error; /// `u64`, this should be fine? #[repr(C)] pub(crate) struct GpRegs { - pub(crate) rbx: u64, + pub rbx: u64, pub rsp: u64, - rbp: u64, - pub(crate) rdi: u64, - r12: u64, - r13: u64, - r14: u64, - r15: u64, - pub(crate) rsi: u64, + pub rbp: u64, + pub rdi: u64, + pub r12: u64, + pub r13: u64, + pub r14: u64, + pub r15: u64, + pub rsi: u64, } impl GpRegs { @@ -65,15 +65,15 @@ impl GpRegs { /// . Since the members are all /// `__m128`, this should be fine? #[repr(C)] -struct FpRegs { - xmm0: __m128, - xmm1: __m128, - xmm2: __m128, - xmm3: __m128, - xmm4: __m128, - xmm5: __m128, - xmm6: __m128, - xmm7: __m128, +pub(crate) struct FpRegs { + pub xmm0: __m128, + pub xmm1: __m128, + pub xmm2: __m128, + pub xmm3: __m128, + pub xmm4: __m128, + pub xmm5: __m128, + pub xmm6: __m128, + pub xmm7: __m128, } impl FpRegs { @@ -115,7 +115,7 @@ impl FpRegs { #[repr(C, align(64))] pub struct Context { pub(crate) gpr: GpRegs, - fpr: FpRegs, + pub(crate) fpr: FpRegs, retvals_gp: [u64; 2], retval_fp: __m128, parent_ctx: *mut Context, diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index 0b5e70d48..c067e2fd3 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -592,6 +592,12 @@ impl Instance { /// /// [run_start]: struct.Instance.html#method.run pub fn reset(&mut self) -> Result<(), Error> { + // temporary heuristic for whether this should happen upon a reset; in the long term we only + // want to force an unwind if there are hostcall frames present, which we'll need to track + // via the instance + if self.ctx.gpr.rsp != 0 { + self.force_unwind().unwrap(); + } self.alloc.reset_heap(self.module.as_ref())?; let globals = unsafe { self.alloc.globals_mut() }; let mod_globals = self.module.globals(); @@ -1195,6 +1201,39 @@ impl Instance { res } + + fn force_unwind(&mut self) -> Result<(), Error> { + #[unwind(allowed)] + extern "C" fn initiate_unwind() { + panic!(TerminationDetails::ForcedUnwind); + } + + // extremely unsafe, doesn't handle any stack exhaustion edge cases yet + let stack_offset = self.ctx.gpr.rsp as usize - dbg!(self.alloc.slot().stack) as usize; + let stack_index = stack_offset / 8; + assert!(stack_offset % 8 == 0); + + let stack = unsafe { self.alloc.stack_u64_mut() }; + if stack_index % 2 == 1 { + stack[stack_index - 1] = initiate_unwind as u64; + self.ctx.gpr.rsp -= 8; + } else { + // stack[stack_index - 1] = 0xFAFAFAFAFAFAFAFA; + stack[stack_index - 2] = 0; + stack[stack_index - 1] = initiate_unwind as u64; + // stack[stack_index - 1] = 0; + // stack[stack_index - 2] = initiate_unwind as u64; + self.ctx.gpr.rsp -= 16; + } + + match self.swap_and_return() { + Ok(_) => panic!("forced unwinding shouldn't return normally"), + Err(Error::RuntimeTerminated(TerminationDetails::ForcedUnwind)) => (), + Err(e) => panic!("unexpected error: {}", e), + } + + Ok(()) + } } /// Information about a runtime fault. diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs index e0b157ef6..6bc9b7f9e 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs @@ -373,6 +373,28 @@ extern "C" fn handle_signal(signum: c_int, siginfo_ptr: *mut siginfo_t, ucontext inst.lock_testpoints .signal_handler_after_disabling_termination .check(); + + // use the ucontext to fill in the fields of the guest context; we can't use + // `Context::swap()` here because then we'd swap back to the signal handler instead of + // the point in the guest that caused the fault + ctx.save_to_context(&mut inst.ctx); + + // set up the faulting instruction pointer as the return address for `initiate_unwind`; + // extremely unsafe, doesn't handle any edge cases yet + let stack_offset = + inst.ctx.gpr.rsp as usize - dbg!(inst.alloc.slot().stack) as usize; + let stack_index = stack_offset / 8; + assert!(stack_offset % 8 == 0); + + let stack = unsafe { inst.alloc.stack_u64_mut() }; + if stack_index % 2 == 1 { + stack[stack_index - 1] = rip as u64; + inst.ctx.gpr.rsp -= 8; + } else { + stack[stack_index - 1] = 0xAFAFAFAFAFAFAFAF; + stack[stack_index - 2] = rip as u64; + inst.ctx.gpr.rsp -= 16; + } } } diff --git a/lucet-runtime/lucet-runtime-internals/src/lib.rs b/lucet-runtime/lucet-runtime-internals/src/lib.rs index 6737f9b3a..66ddc4c68 100644 --- a/lucet-runtime/lucet-runtime-internals/src/lib.rs +++ b/lucet-runtime/lucet-runtime-internals/src/lib.rs @@ -2,6 +2,7 @@ //! WebAssembly modules in lightweight sandboxes. It is intended to work with modules compiled by //! [`lucetc`](https://github.com/fastly/lucet/tree/master/lucetc). +#![feature(unwind_attributes)] #![deny(bare_trait_objects)] #[macro_use] diff --git a/lucet-runtime/lucet-runtime-internals/src/sysdeps/linux.rs b/lucet-runtime/lucet-runtime-internals/src/sysdeps/linux.rs index da58e43cb..7773e22ec 100644 --- a/lucet-runtime/lucet-runtime-internals/src/sysdeps/linux.rs +++ b/lucet-runtime/lucet-runtime-internals/src/sysdeps/linux.rs @@ -1,4 +1,10 @@ use libc::{c_void, ucontext_t, REG_RDI, REG_RIP}; +use crate::context::Context; +use libc::{ + c_void, ucontext_t, REG_R12, REG_R13, REG_R14, REG_R15, REG_RBP, REG_RBX, REG_RDI, REG_RIP, + REG_RSP, +}; +use std::arch::x86_64::{__m128, _mm_loadu_ps}; #[derive(Clone, Copy, Debug)] pub struct UContextPtr(*mut ucontext_t); @@ -27,8 +33,44 @@ impl UContextPtr { let mut mcontext = &mut unsafe { self.0.as_mut().unwrap() }.uc_mcontext; mcontext.gregs[REG_RDI as usize] = new_rdi as i64; } + + pub fn save_to_context(&self, ctx: &mut Context) { + let mcontext = &unsafe { *(self.0) }.uc_mcontext; + ctx.gpr.rbx = mcontext.gregs[REG_RBX as usize] as u64; + ctx.gpr.rsp = mcontext.gregs[REG_RSP as usize] as u64; + ctx.gpr.rbp = mcontext.gregs[REG_RBP as usize] as u64; + ctx.gpr.rdi = mcontext.gregs[REG_RDI as usize] as u64; + ctx.gpr.r12 = mcontext.gregs[REG_R12 as usize] as u64; + ctx.gpr.r13 = mcontext.gregs[REG_R13 as usize] as u64; + ctx.gpr.r14 = mcontext.gregs[REG_R14 as usize] as u64; + ctx.gpr.r15 = mcontext.gregs[REG_R15 as usize] as u64; + + let fpregs = &unsafe { *(mcontext.fpregs) }; + let xmms = fpregs._xmm[0..8] + .iter() + .map(|reg| unsafe { _mm_loadu_ps(reg.element.as_ptr() as *const u32 as *const _) }) + .collect::>(); + ctx.fpr.xmm0 = xmms[0]; + ctx.fpr.xmm1 = xmms[1]; + ctx.fpr.xmm2 = xmms[2]; + ctx.fpr.xmm3 = xmms[3]; + ctx.fpr.xmm4 = xmms[4]; + ctx.fpr.xmm5 = xmms[5]; + ctx.fpr.xmm6 = xmms[6]; + ctx.fpr.xmm7 = xmms[7]; + } } +// TODO: refactor uses of these types so that a deref instance can make sense, then move the methods +// from the ptr type into the target-specific ucontext type +// +// impl std::ops::Deref for UContextPtr { +// type Target = UContext; +// fn deref(&self) -> &Self::Target { +// &unsafe { UContext::new(self.0 as *const c_void) } +// } +// } + #[repr(C)] #[derive(Clone, Copy)] pub struct UContext { diff --git a/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json b/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json index 43965ce9e..5dae83cd9 100644 --- a/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json +++ b/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json @@ -1,5 +1,6 @@ { "env": { + "fault_unwind": "hostcall_fault_unwind", "hostcall_test_func_hello": "hostcall_test_func_hello", "hostcall_test_func_hostcall_error": "hostcall_test_func_hostcall_error", "hostcall_test_func_hostcall_error_unwind": "hostcall_test_func_hostcall_error_unwind", diff --git a/lucet-runtime/lucet-runtime-tests/guests/host/fault_unwind.c b/lucet-runtime/lucet-runtime-tests/guests/host/fault_unwind.c new file mode 100644 index 000000000..db16554c9 --- /dev/null +++ b/lucet-runtime/lucet-runtime-tests/guests/host/fault_unwind.c @@ -0,0 +1,15 @@ +#include +#include + +extern void fault_unwind(void (*)(void)); + +void callback(void) +{ + *(uint64_t *) 0xFFFFFFFF = 420; + return; +} + +void entrypoint(void) +{ + return fault_unwind(callback); +} diff --git a/lucet-runtime/lucet-runtime-tests/src/build.rs b/lucet-runtime/lucet-runtime-tests/src/build.rs index 6a22fc21d..642a1b91d 100644 --- a/lucet-runtime/lucet-runtime-tests/src/build.rs +++ b/lucet-runtime/lucet-runtime-tests/src/build.rs @@ -50,6 +50,9 @@ where let dlmodule = DlModule::load(so_file)?; + // temporary, so that gdb doesn't barf due to the .so being deleted too early + workdir.into_path(); + Ok(dlmodule) } diff --git a/lucet-runtime/lucet-runtime-tests/src/host.rs b/lucet-runtime/lucet-runtime-tests/src/host.rs index 66fc03c8a..8029385e3 100644 --- a/lucet-runtime/lucet-runtime-tests/src/host.rs +++ b/lucet-runtime/lucet-runtime-tests/src/host.rs @@ -173,6 +173,7 @@ macro_rules! host_tests { static ref NESTED_INNER: Mutex<()> = Mutex::new(()); static ref NESTED_REGS_OUTER: Mutex<()> = Mutex::new(()); static ref NESTED_REGS_INNER: Mutex<()> = Mutex::new(()); + static ref FAULT_UNWIND: Mutex<()> = Mutex::new(()); } #[inline] @@ -422,6 +423,19 @@ macro_rules! host_tests { ); } + /// Ensures that hostcall stack frames get unwound when a fault occurs in guest code. + #[test] + fn fault_unwind() { + let module = test_module_c("host", "fault_unwind.c").expect("build and load module"); + let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + inst.run("entrypoint", &[]).unwrap_err(); + inst.reset().unwrap(); + assert!(FAULT_UNWIND.is_poisoned()); + } + #[test] fn run_fpe() { let module = test_module_c("host", "fpe.c").expect("build and load module"); From c988266a662068a61206d26ed453a71cfb3f94af Mon Sep 17 00:00:00 2001 From: Andy Wortman Date: Thu, 3 Oct 2019 13:20:15 -0700 Subject: [PATCH 17/37] add stub function so we can pad stack to ABI requirements --- .../src/context/context_asm.S | 14 ++++++++++++++ .../lucet-runtime-internals/src/context/mod.rs | 4 ++++ .../lucet-runtime-internals/src/instance.rs | 4 ++-- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S b/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S index c7f3bbed8..36785d992 100644 --- a/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S +++ b/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S @@ -54,6 +54,20 @@ _lucet_context_bootstrap: .size lucet_context_bootstrap,.-lucet_context_bootstrap #endif +.text +.globl unwind_stub +.align 16 +.cfi_startproc + // load bearing nop so we can just put `unwind_stub` in places and have the unwinding mechanism understand we mean we are _in_ this function + nop +unwind_stub: +_unwind_stub: + ret +.cfi_endproc +#ifdef __ELF__ +.size unwind_stub,.-unwind_stub +#endif + .text .globl libunwind_backstop_shim .align 16 diff --git a/lucet-runtime/lucet-runtime-internals/src/context/mod.rs b/lucet-runtime/lucet-runtime-internals/src/context/mod.rs index 37c37b484..6afcae529 100644 --- a/lucet-runtime/lucet-runtime-internals/src/context/mod.rs +++ b/lucet-runtime/lucet-runtime-internals/src/context/mod.rs @@ -729,4 +729,8 @@ extern "C" { /// For more information, see `Instance::swap_and_return`, `Instance::with_activation_routine`, /// `enter_guest_region`, and `lucet_context_activate`'s assembly implementation. pub(crate) fn lucet_context_activate(); + + /// Just a stub to have a single address we can unwind through, for stack alignment when we + /// need to add a new call frame to an existing guest + pub(crate) fn unwind_stub(); } diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index c067e2fd3..bcff52247 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -1219,8 +1219,8 @@ impl Instance { self.ctx.gpr.rsp -= 8; } else { // stack[stack_index - 1] = 0xFAFAFAFAFAFAFAFA; - stack[stack_index - 2] = 0; - stack[stack_index - 1] = initiate_unwind as u64; + stack[stack_index - 1] = crate::context::unwind_stub as u64; + stack[stack_index - 2] = initiate_unwind as u64; // stack[stack_index - 1] = 0; // stack[stack_index - 2] = initiate_unwind as u64; self.ctx.gpr.rsp -= 16; From 91a777e2696dcedbb940e22afe32a62a5a4b54f7 Mon Sep 17 00:00:00 2001 From: Andy Wortman Date: Thu, 3 Oct 2019 17:05:56 -0700 Subject: [PATCH 18/37] add a helper to push values onto a guest stack --- .../lucet-runtime-internals/src/instance.rs | 56 ++++++++++++++----- .../src/instance/signals.rs | 15 +---- 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index bcff52247..58564fcbb 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -1202,30 +1202,56 @@ impl Instance { res } + fn push(&mut self, value: u64) { + let stack_offset = self.ctx.gpr.rsp as usize - self.alloc.slot().stack as usize; + let stack_index = stack_offset / 8; + assert!(stack_offset % 8 == 0); + + let stack = unsafe { self.alloc.stack_u64_mut() }; + + // check for at least one free stack slot + if stack.len() - stack_index >= 1 { + self.ctx.gpr.rsp -= 8; + stack[stack_index - 1] = value; + } else { + panic!("caused a guest stack overflow!"); + } + } + fn force_unwind(&mut self) -> Result<(), Error> { #[unwind(allowed)] extern "C" fn initiate_unwind() { panic!(TerminationDetails::ForcedUnwind); } + // The logic for this conditional can be a bit unintuitive: we _require_ that the stack + // is aligned to 8 bytes, but not 16 bytes, when pushing `initiate_unwind`. + // + // A diagram of the required layout may help: + // `XXXXX0`: ------------------ <-- call frame start -- SysV ABI requires 16-byte alignment + // `XXXXX8`: | return address | + // `XXXX..`: | ..locals etc.. | + // `XXXX..`: | ..as needed... | + // + // By the time we've gotten here, we have already pushed "return address", the address of + // wherever in the guest we want to start unwinding. If it leaves the stack 16-byte + // aligned, it's 8 bytes off from the diagram above, and we would have the call frame for + // `initiate_unwind` in violation of the SysV ABI. Functionally, this means that + // compiler-generated xmm accesses will fault due to being misaligned. + // + // So, instead, push a new return address to construct a new call frame at the right + // offset. `unwind_stub` has CFA directives so the unwinder can connect from + // `initiate_unwind` to guest/host frames to unwind. The unwinder, thankfully, has no + // preferences about stack alignment of frames being unwound. + // // extremely unsafe, doesn't handle any stack exhaustion edge cases yet - let stack_offset = self.ctx.gpr.rsp as usize - dbg!(self.alloc.slot().stack) as usize; - let stack_index = stack_offset / 8; - assert!(stack_offset % 8 == 0); - - let stack = unsafe { self.alloc.stack_u64_mut() }; - if stack_index % 2 == 1 { - stack[stack_index - 1] = initiate_unwind as u64; - self.ctx.gpr.rsp -= 8; - } else { - // stack[stack_index - 1] = 0xFAFAFAFAFAFAFAFA; - stack[stack_index - 1] = crate::context::unwind_stub as u64; - stack[stack_index - 2] = initiate_unwind as u64; - // stack[stack_index - 1] = 0; - // stack[stack_index - 2] = initiate_unwind as u64; - self.ctx.gpr.rsp -= 16; + if self.ctx.gpr.rsp % 16 == 0 { + self.push(crate::context::unwind_stub as u64); } + assert!(self.ctx.gpr.rsp % 16 == 8); + self.push(initiate_unwind as u64); + match self.swap_and_return() { Ok(_) => panic!("forced unwinding shouldn't return normally"), Err(Error::RuntimeTerminated(TerminationDetails::ForcedUnwind)) => (), diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs index 6bc9b7f9e..d94ffdc63 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs @@ -381,20 +381,7 @@ extern "C" fn handle_signal(signum: c_int, siginfo_ptr: *mut siginfo_t, ucontext // set up the faulting instruction pointer as the return address for `initiate_unwind`; // extremely unsafe, doesn't handle any edge cases yet - let stack_offset = - inst.ctx.gpr.rsp as usize - dbg!(inst.alloc.slot().stack) as usize; - let stack_index = stack_offset / 8; - assert!(stack_offset % 8 == 0); - - let stack = unsafe { inst.alloc.stack_u64_mut() }; - if stack_index % 2 == 1 { - stack[stack_index - 1] = rip as u64; - inst.ctx.gpr.rsp -= 8; - } else { - stack[stack_index - 1] = 0xAFAFAFAFAFAFAFAF; - stack[stack_index - 2] = rip as u64; - inst.ctx.gpr.rsp -= 16; - } + inst.push(rip as u64); } } From a6d2262f739a57f1e9906db06b8b52c25aeaf396 Mon Sep 17 00:00:00 2001 From: Andy Wortman Date: Thu, 3 Oct 2019 17:52:48 -0700 Subject: [PATCH 19/37] found some sharp bits --- .../src/instance/signals.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs index d94ffdc63..be20a7b99 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs @@ -381,6 +381,20 @@ extern "C" fn handle_signal(signum: c_int, siginfo_ptr: *mut siginfo_t, ucontext // set up the faulting instruction pointer as the return address for `initiate_unwind`; // extremely unsafe, doesn't handle any edge cases yet + // + // TODO(Andy) can we avoid pushing onto the guest stack until knowing we want to force + // an unwind? maybe a "last address" field on ctx. This can be populated on context + // swap out (return address of lucet_context_swap) as well as here in the signal + // handler. + // + // TODO(Andy) if the last address is obtained through the signal handler, for a signal + // received exactly when we have just executed a `call` to a guest function, we + // actually want to not push it (or push it +1?) lest we try to unwind with a return + // address == start of function, where the system unwinder will unwind for the function + // at address-1, (probably) fail to find the function, and `abort()`. + // + // if `rip` == the start of some guest function, we can probably just discard it and + // use the return address instead. inst.push(rip as u64); } } From 2cbbd9f807fc283660af92e8cbbf135f84e232bb Mon Sep 17 00:00:00 2001 From: Andy Wortman Date: Mon, 7 Oct 2019 17:23:31 -0700 Subject: [PATCH 20/37] only force an unwind if n>0 host frames are on a guest stack --- .../src/hostcall_macros.rs | 10 +++++++++- .../lucet-runtime-internals/src/instance.rs | 20 +++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs b/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs index c04b9d067..2798ecc0f 100644 --- a/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs +++ b/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs @@ -57,7 +57,13 @@ macro_rules! lucet_hostcalls { $($body)* } // let res = std::panic::catch_unwind(move || { - hostcall_impl(&mut $crate::vmctx::Vmctx::from_raw(vmctx_raw), $( $arg ),*) + #[allow(unused_imports)] + use lucet_runtime_internals::vmctx::VmctxInternal; + $crate::vmctx::Vmctx::from_raw(vmctx_raw).instance_mut().begin_hostcall(); + let res = hostcall_impl(&mut $crate::vmctx::Vmctx::from_raw(vmctx_raw), $( $arg ),*); + + $crate::vmctx::Vmctx::from_raw(vmctx_raw).instance_mut().end_hostcall(); + res // }); // match res { // Ok(res) => res, @@ -70,6 +76,8 @@ macro_rules! lucet_hostcalls { // } // } // } + // + // res } )* } diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index 58564fcbb..d892e98a2 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -264,6 +264,8 @@ pub struct Instance { /// The value passed back to the guest when resuming a yielded instance. pub(crate) resumed_val: Option>, + hostcall_count: u64, + /// `_padding` must be the last member of the structure. /// This marks where the padding starts to make the structure exactly 4096 bytes long. /// It is also used to compute the size of the structure up to that point, i.e. without padding. @@ -592,10 +594,7 @@ impl Instance { /// /// [run_start]: struct.Instance.html#method.run pub fn reset(&mut self) -> Result<(), Error> { - // temporary heuristic for whether this should happen upon a reset; in the long term we only - // want to force an unwind if there are hostcall frames present, which we'll need to track - // via the instance - if self.ctx.gpr.rsp != 0 { + if self.hostcall_count > 0 { self.force_unwind().unwrap(); } self.alloc.reset_heap(self.module.as_ref())?; @@ -845,6 +844,14 @@ impl Instance { res } + pub fn begin_hostcall(&mut self) { + self.hostcall_count += 1; + } + + pub fn end_hostcall(&mut self) { + self.hostcall_count -= 1; + } + #[inline] pub fn get_instruction_count(&self) -> Option { if self.module.is_instruction_count_instrumented() { @@ -889,6 +896,7 @@ impl Instance { ensure_sigstack_installed: true, entrypoint: None, resumed_val: None, + hostcall_count: 0, _padding: (), }; inst.set_globals_ptr(globals_ptr); @@ -982,6 +990,10 @@ impl Instance { let mut args_with_vmctx = vec![Val::from(self.alloc.slot().heap)]; args_with_vmctx.extend_from_slice(args); + // when preparing a new context for the guest to run, we must have cleaned up any existing + // host call frames on the stack. + assert_eq!(self.hostcall_count, 0); + let self_ptr = self as *mut _; Context::init_with_callback( unsafe { self.alloc.stack_u64_mut() }, From 01b1ea31613575166da9ac3706e914d6079f6461 Mon Sep 17 00:00:00 2001 From: Andy Wortman Date: Mon, 7 Oct 2019 19:11:52 -0700 Subject: [PATCH 21/37] test and support forced unwinding of guests stopped from stack overflows --- .../lucet-runtime-internals/src/alloc/mod.rs | 76 +++++++-- .../src/context/mod.rs | 4 + .../lucet-runtime-internals/src/instance.rs | 136 ++++++++++----- .../src/instance/signals.rs | 19 +-- .../lucet-runtime-internals/src/region.rs | 4 + .../src/region/mmap.rs | 23 +++ .../guests/host/bindings.json | 3 +- .../guests/host/fault_unwind.c | 18 +- lucet-runtime/lucet-runtime-tests/src/host.rs | 159 +++++++++++++++++- 9 files changed, 364 insertions(+), 78 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs b/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs index eafbf2f6c..f08612cf0 100644 --- a/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs +++ b/lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs @@ -58,6 +58,8 @@ pub struct Slot { pub limits: Limits, pub region: Weak, + + pub(crate) redzone_stack_enabled: bool, } // raw pointers require unsafe impl @@ -68,6 +70,14 @@ impl Slot { pub fn stack_top(&self) -> *mut c_void { (self.stack as usize + self.limits.stack_size) as *mut c_void } + + pub fn stack_redzone_start(&self) -> *mut c_void { + (self.stack as usize - self.stack_redzone_size()) as *mut c_void + } + + pub fn stack_redzone_size(&self) -> usize { + host_page_size() + } } /// The strategy by which a `Region` selects an allocation to back an `Instance`. @@ -167,6 +177,24 @@ impl AddrLocation { } impl Alloc { + pub(crate) fn enable_stack_redzone(&mut self) { + let slot = self + .slot + .as_mut() + .expect("alloc has a Slot when toggling stack redzone"); + slot.redzone_stack_enabled = true; + self.region.enable_stack_redzone(slot) + } + + pub(crate) fn disable_stack_redzone(&mut self) { + let slot = self + .slot + .as_mut() + .expect("alloc has a Slot when toggling stack redzone"); + slot.redzone_stack_enabled = false; + self.region.disable_stack_redzone(slot) + } + /// Where in an `Alloc` does a particular address fall? pub fn addr_location(&self, addr: *const c_void) -> AddrLocation { let addr = addr as usize; @@ -351,13 +379,41 @@ impl Alloc { std::slice::from_raw_parts_mut(self.slot().heap as *mut u64, self.heap_accessible_size / 8) } + pub(crate) fn stack_start(&self) -> *mut u8 { + let mut stack_start = self.slot().stack as usize; + + if self + .slot + .as_ref() + .expect("alloc has a slot when we want to access its stack") + .redzone_stack_enabled + { + stack_start -= host_page_size(); + } + + stack_start as *mut u8 + } + + pub(crate) fn stack_size(&self) -> usize { + let mut stack_size = self.slot().limits.stack_size; + if self + .slot + .as_ref() + .expect("alloc has a slot when we want to access its stack") + .redzone_stack_enabled + { + stack_size += host_page_size(); + } + stack_size + } + /// Return the stack as a mutable byte slice. /// /// Since the stack grows down, `alloc.stack_mut()[0]` is the top of the stack, and /// `alloc.stack_mut()[alloc.limits.stack_size - 1]` is the last byte at the bottom of the /// stack. pub unsafe fn stack_mut(&mut self) -> &mut [u8] { - std::slice::from_raw_parts_mut(self.slot().stack as *mut u8, self.slot().limits.stack_size) + std::slice::from_raw_parts_mut(self.stack_start(), self.stack_size()) } /// Return the stack as a mutable slice of 64-bit words. @@ -366,18 +422,12 @@ impl Alloc { /// `alloc.stack_mut()[alloc.limits.stack_size - 1]` is the last word at the bottom of the /// stack. pub unsafe fn stack_u64_mut(&mut self) -> &mut [u64] { - assert!( - self.slot().stack as usize % 8 == 0, - "stack is 8-byte aligned" - ); - assert!( - self.slot().limits.stack_size % 8 == 0, - "stack size is multiple of 8-bytes" - ); - std::slice::from_raw_parts_mut( - self.slot().stack as *mut u64, - self.slot().limits.stack_size / 8, - ) + let stack_start = self.stack_start(); + let stack_size = self.stack_size(); + + assert!(stack_start as usize % 8 == 0, "stack is 8-byte aligned"); + assert!(stack_size % 8 == 0, "stack size is multiple of 8-bytes"); + std::slice::from_raw_parts_mut(stack_start as *mut u64, stack_size / 8) } /// Return the globals as a slice. diff --git a/lucet-runtime/lucet-runtime-internals/src/context/mod.rs b/lucet-runtime/lucet-runtime-internals/src/context/mod.rs index 6afcae529..9a734f2c7 100644 --- a/lucet-runtime/lucet-runtime-internals/src/context/mod.rs +++ b/lucet-runtime/lucet-runtime-internals/src/context/mod.rs @@ -122,6 +122,8 @@ pub struct Context { // TODO ACF 2019-10-23: make Instance into a generic parameter? backstop_callback: *const unsafe extern "C" fn(*mut Instance), callback_data: *mut Instance, + sigset: signal::SigSet, + pub(crate) stop_addr: Option, } impl Context { @@ -135,6 +137,8 @@ impl Context { parent_ctx: ptr::null_mut(), backstop_callback: Context::default_backstop_callback as *const _, callback_data: ptr::null_mut(), + sigset: signal::SigSet::empty(), + stop_addr: None, } } diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index d892e98a2..6a3f2b9f9 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -1214,63 +1214,119 @@ impl Instance { res } - fn push(&mut self, value: u64) { - let stack_offset = self.ctx.gpr.rsp as usize - self.alloc.slot().stack as usize; + fn push(&mut self, value: u64) -> Result<(), ()> { + let stack_offset = self.ctx.gpr.rsp as usize - self.alloc.stack_start() as usize; let stack_index = stack_offset / 8; assert!(stack_offset % 8 == 0); let stack = unsafe { self.alloc.stack_u64_mut() }; // check for at least one free stack slot - if stack.len() - stack_index >= 1 { + if stack_index >= 1 { self.ctx.gpr.rsp -= 8; stack[stack_index - 1] = value; + Ok(()) } else { - panic!("caused a guest stack overflow!"); + Err(()) } } + fn with_redzone_stack T>(&mut self, f: F) -> T { + self.alloc.enable_stack_redzone(); + + let res = f(self); + + self.alloc.disable_stack_redzone(); + + res + } + + // Force a guest to unwind the stack from the specified guest address fn force_unwind(&mut self) -> Result<(), Error> { - #[unwind(allowed)] - extern "C" fn initiate_unwind() { - panic!(TerminationDetails::ForcedUnwind); - } + // if we should unwind by returning into the guest to cause a fault, do so with the redzone + // available in case the guest was at or close to overflowing. + self.with_redzone_stack(|inst| { + #[unwind(allowed)] + extern "C" fn initiate_unwind() { + panic!(TerminationDetails::ForcedUnwind); + } - // The logic for this conditional can be a bit unintuitive: we _require_ that the stack - // is aligned to 8 bytes, but not 16 bytes, when pushing `initiate_unwind`. - // - // A diagram of the required layout may help: - // `XXXXX0`: ------------------ <-- call frame start -- SysV ABI requires 16-byte alignment - // `XXXXX8`: | return address | - // `XXXX..`: | ..locals etc.. | - // `XXXX..`: | ..as needed... | - // - // By the time we've gotten here, we have already pushed "return address", the address of - // wherever in the guest we want to start unwinding. If it leaves the stack 16-byte - // aligned, it's 8 bytes off from the diagram above, and we would have the call frame for - // `initiate_unwind` in violation of the SysV ABI. Functionally, this means that - // compiler-generated xmm accesses will fault due to being misaligned. - // - // So, instead, push a new return address to construct a new call frame at the right - // offset. `unwind_stub` has CFA directives so the unwinder can connect from - // `initiate_unwind` to guest/host frames to unwind. The unwinder, thankfully, has no - // preferences about stack alignment of frames being unwound. - // - // extremely unsafe, doesn't handle any stack exhaustion edge cases yet - if self.ctx.gpr.rsp % 16 == 0 { - self.push(crate::context::unwind_stub as u64); - } + let guest_addr = inst + .ctx + .stop_addr + .expect("guest that stopped in guest code has an address it stopped at"); - assert!(self.ctx.gpr.rsp % 16 == 8); - self.push(initiate_unwind as u64); + // set up the faulting instruction pointer as the return address for `initiate_unwind`; + // extremely unsafe, doesn't handle any edge cases yet + // + // TODO(Andy) if the last address is obtained through the signal handler, for a signal + // received exactly when we have just executed a `call` to a guest function, we + // actually want to not push it (or push it +1?) lest we try to unwind with a return + // address == start of function, where the system unwinder will unwind for the function + // at address-1, (probably) fail to find the function, and `abort()`. + // + // if `rip` == the start of some guest function, we can probably just discard it and + // use the return address instead. + inst.push(guest_addr as u64) + .expect("stack has available space"); - match self.swap_and_return() { - Ok(_) => panic!("forced unwinding shouldn't return normally"), - Err(Error::RuntimeTerminated(TerminationDetails::ForcedUnwind)) => (), - Err(e) => panic!("unexpected error: {}", e), - } + // The logic for this conditional can be a bit unintuitive: we _require_ that the stack + // is aligned to 8 bytes, but not 16 bytes, when pushing `initiate_unwind`. + // + // A diagram of the required layout may help: + // `XXXXX0`: ------------------ <-- call frame start -- SysV ABI requires 16-byte alignment + // `XXXXX8`: | return address | + // `XXXX..`: | ..locals etc.. | + // `XXXX..`: | ..as needed... | + // + // Now ensure we _have_ an ABI-conformant call fame like above, by handling the case that + // could lead to an unaligned stack - the guest stack pointer currently being unaligned. + // Among other errors, a misaligned stack will result in compiler-generated xmm accesses to + // fault. + // + // Eg, we would have a stack like: + // `XXXXX8`: ------------------ <-- guest stack end, call frame start + // `XXXXX0`: | unwind_stub | + // `XXXX..`: | ..locals etc.. | + // `XXXX..`: | ..as needed... | + // + // So, instead, push a new return address to construct a new call frame at the right + // offset. `unwind_stub` has CFA directives so the unwinder can connect from + // `initiate_unwind` to guest/host frames to unwind. The unwinder, thankfully, has no + // preferences about alignment of frames being unwound. + // + // And we end up with a guest stack like this: + // `XXXXX8`: ------------------ <-- guest stack end + // `XXXXX0`: | guest ret addr | <-- guest return address to unwind through + // `XXXXX0`: ------------------ <-- call frame start -- SysV ABI requires 16-byte alignment + // `XXXXX8`: | unwind_stub | + // `XXXX..`: | ..locals etc.. | + // `XXXX..`: | ..as needed... | + if inst.ctx.gpr.rsp % 16 == 0 { + // extremely unsafe, doesn't handle any stack exhaustion edge cases yet + inst.push(crate::context::unwind_stub as u64) + .expect("stack has available space"); + } - Ok(()) + assert!(inst.ctx.gpr.rsp % 16 == 8); + // extremely unsafe, doesn't handle any stack exhaustion edge cases yet + inst.push(initiate_unwind as u64) + .expect("stack has available space"); + + inst.state = State::Ready; + + match inst.swap_and_return() { + Ok(_) => panic!("forced unwinding shouldn't return normally"), + Err(Error::RuntimeTerminated(TerminationDetails::ForcedUnwind)) => (), + Err(e) => panic!("unexpected error: {}", e), + } + + // we've unwound the stack, so we know there are no longer any host frames. + inst.hostcall_count = 0; + inst.ctx.stop_addr = None; + + Ok(()) + }) } } diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs index be20a7b99..adc9dc57d 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs @@ -378,24 +378,7 @@ extern "C" fn handle_signal(signum: c_int, siginfo_ptr: *mut siginfo_t, ucontext // `Context::swap()` here because then we'd swap back to the signal handler instead of // the point in the guest that caused the fault ctx.save_to_context(&mut inst.ctx); - - // set up the faulting instruction pointer as the return address for `initiate_unwind`; - // extremely unsafe, doesn't handle any edge cases yet - // - // TODO(Andy) can we avoid pushing onto the guest stack until knowing we want to force - // an unwind? maybe a "last address" field on ctx. This can be populated on context - // swap out (return address of lucet_context_swap) as well as here in the signal - // handler. - // - // TODO(Andy) if the last address is obtained through the signal handler, for a signal - // received exactly when we have just executed a `call` to a guest function, we - // actually want to not push it (or push it +1?) lest we try to unwind with a return - // address == start of function, where the system unwinder will unwind for the function - // at address-1, (probably) fail to find the function, and `abort()`. - // - // if `rip` == the start of some guest function, we can probably just discard it and - // use the return address instead. - inst.push(rip as u64); + inst.ctx.stop_addr = Some(rip as u64); } } diff --git a/lucet-runtime/lucet-runtime-internals/src/region.rs b/lucet-runtime/lucet-runtime-internals/src/region.rs index c72ae159f..8083fd3e8 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region.rs @@ -72,6 +72,10 @@ pub trait RegionInternal: Send + Sync { fn get_limits(&self) -> &Limits; fn as_dyn_internal(&self) -> &dyn RegionInternal; + + fn enable_stack_redzone(&self, slot: &Slot); + + fn disable_stack_redzone(&self, slot: &Slot); } /// A trait for regions that are created with a fixed capacity and limits. diff --git a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs index 99196d4b3..87de08782 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/mmap.rs @@ -276,6 +276,28 @@ impl RegionInternal for MmapRegion { fn as_dyn_internal(&self) -> &dyn RegionInternal { self } + + fn enable_stack_redzone(&self, slot: &Slot) { + unsafe { + mprotect( + slot.stack_redzone_start(), + host_page_size(), + ProtFlags::PROT_READ | ProtFlags::PROT_WRITE, + ) + .expect("can set permissions on stack redzone page") + } + } + + fn disable_stack_redzone(&self, slot: &Slot) { + unsafe { + mprotect( + slot.stack_redzone_start(), + host_page_size(), + ProtFlags::PROT_NONE, + ) + .expect("can set permissions on stack redzone page") + } + } } impl Drop for MmapRegion { @@ -412,6 +434,7 @@ impl MmapRegion { sigstack: sigstack as *mut c_void, limits: region.limits.clone(), region: Arc::downgrade(region) as Weak, + redzone_stack_enabled: false, }) } diff --git a/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json b/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json index 5dae83cd9..8a44915a3 100644 --- a/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json +++ b/lucet-runtime/lucet-runtime-tests/guests/host/bindings.json @@ -1,6 +1,7 @@ { "env": { - "fault_unwind": "hostcall_fault_unwind", + "bad_access_unwind": "hostcall_bad_access_unwind", + "stack_overflow_unwind": "hostcall_stack_overflow_unwind", "hostcall_test_func_hello": "hostcall_test_func_hello", "hostcall_test_func_hostcall_error": "hostcall_test_func_hostcall_error", "hostcall_test_func_hostcall_error_unwind": "hostcall_test_func_hostcall_error_unwind", diff --git a/lucet-runtime/lucet-runtime-tests/guests/host/fault_unwind.c b/lucet-runtime/lucet-runtime-tests/guests/host/fault_unwind.c index db16554c9..da01e11ed 100644 --- a/lucet-runtime/lucet-runtime-tests/guests/host/fault_unwind.c +++ b/lucet-runtime/lucet-runtime-tests/guests/host/fault_unwind.c @@ -1,15 +1,25 @@ #include #include -extern void fault_unwind(void (*)(void)); +extern void bad_access_unwind(void (*)(void)); +extern void stack_overflow_unwind(void (*)(void)); -void callback(void) +void do_bad_access(void) { *(uint64_t *) 0xFFFFFFFF = 420; return; } -void entrypoint(void) +void bad_access(void) { - return fault_unwind(callback); + return bad_access_unwind(do_bad_access); +} + +void do_stack_overflow(void) { + do_stack_overflow(); +} + +void stack_overflow(void) +{ + return stack_overflow_unwind(do_stack_overflow); } diff --git a/lucet-runtime/lucet-runtime-tests/src/host.rs b/lucet-runtime/lucet-runtime-tests/src/host.rs index 8029385e3..e1f919832 100644 --- a/lucet-runtime/lucet-runtime-tests/src/host.rs +++ b/lucet-runtime/lucet-runtime-tests/src/host.rs @@ -25,6 +25,160 @@ macro_rules! host_tests { assert!(module.is_err()); } + #[allow(unreachable_code)] + #[no_mangle] + pub fn hostcall_test_func_hostcall_error_unwind( + vmctx: &Vmctx, + ) -> () { + let lock = HOSTCALL_MUTEX.lock().unwrap(); + unsafe { + lucet_hostcall_terminate!(ERROR_MESSAGE); + } + drop(lock); + } + + #[lucet_hostcall] + #[no_mangle] + pub fn nested_error_unwind_outer( + vmctx: &Vmctx, + cb_idx: u32, + ) -> u64 { + unwind_outer(vmctx, &*NESTED_OUTER, cb_idx) + } + + #[lucet_hostcall] + #[no_mangle] + pub fn nested_error_unwind_inner( + vmctx: &Vmctx, + ) -> () { + unwind_inner(vmctx, &*NESTED_INNER) + } + + #[lucet_hostcall] + #[no_mangle] + pub fn nested_error_unwind_regs_outer( + vmctx: &Vmctx, + cb_idx: u32, + ) -> u64 { + unwind_outer(vmctx, &*NESTED_REGS_OUTER, cb_idx) + } + + #[lucet_hostcall] + #[no_mangle] + pub fn nested_error_unwind_regs_inner( + vmctx: &Vmctx, + ) -> () { + unwind_inner(vmctx, &*NESTED_REGS_INNER) + } + + #[lucet_hostcall] + #[no_mangle] + pub fn hostcall_panic( + _vmctx: &Vmctx, + ) -> () { + panic!("hostcall_panic"); + } + + #[lucet_hostcall] + #[no_mangle] + pub fn hostcall_restore_callee_saved( + vmctx: &Vmctx, + cb_idx: u32, + ) -> u64 { + let mut a: u64; + let mut b: u64 = 0xAAAAAAAA00000001; + let mut c: u64 = 0xAAAAAAAA00000002; + let mut d: u64 = 0xAAAAAAAA00000003; + let mut e: u64 = 0xAAAAAAAA00000004; + let mut f: u64 = 0xAAAAAAAA00000005; + let mut g: u64 = 0xAAAAAAAA00000006; + let mut h: u64 = 0xAAAAAAAA00000007; + let mut i: u64 = 0xAAAAAAAA00000008; + let mut j: u64 = 0xAAAAAAAA00000009; + let mut k: u64 = 0xAAAAAAAA0000000A; + let mut l: u64 = 0xAAAAAAAA0000000B; + + a = b.wrapping_add(c ^ 0); + b = c.wrapping_add(d ^ 1); + c = d.wrapping_add(e ^ 2); + d = e.wrapping_add(f ^ 3); + e = f.wrapping_add(g ^ 4); + f = g.wrapping_add(h ^ 5); + g = h.wrapping_add(i ^ 6); + h = i.wrapping_add(j ^ 7); + i = j.wrapping_add(k ^ 8); + j = k.wrapping_add(l ^ 9); + k = l.wrapping_add(a ^ 10); + l = a.wrapping_add(b ^ 11); + + let func = vmctx + .get_func_from_idx(0, cb_idx) + .expect("can get function by index"); + let func = std::mem::transmute:: u64>( + func.ptr.as_usize(), + ); + let vmctx_raw = vmctx.as_raw(); + let res = std::panic::catch_unwind(|| { + (func)(vmctx_raw); + }); + assert!(res.is_err()); + + a = b.wrapping_mul(c & 0); + b = c.wrapping_mul(d & 1); + c = d.wrapping_mul(e & 2); + d = e.wrapping_mul(f & 3); + e = f.wrapping_mul(g & 4); + f = g.wrapping_mul(h & 5); + g = h.wrapping_mul(i & 6); + h = i.wrapping_mul(j & 7); + i = j.wrapping_mul(k & 8); + j = k.wrapping_mul(l & 9); + k = l.wrapping_mul(a & 10); + l = a.wrapping_mul(b & 11); + + a ^ b ^ c ^ d ^ e ^ f ^ g ^ h ^ i ^ j ^ k ^ l + } + + #[lucet_hostcall] + #[no_mangle] + pub fn hostcall_stack_overflow_unwind( + vmctx: &Vmctx, + cb_idx: u32, + ) -> () { + let lock = STACK_OVERFLOW_UNWIND.lock().unwrap(); + + let func = vmctx + .get_func_from_idx(0, cb_idx) + .expect("can get function by index"); + let func = std::mem::transmute::( + func.ptr.as_usize(), + ); + let vmctx_raw = vmctx.as_raw(); + func(vmctx_raw); + + drop(lock); + } + + #[lucet_hostcall] + #[no_mangle] + pub fn hostcall_bad_access_unwind( + vmctx: &Vmctx, + cb_idx: u32, + ) -> () { + let lock = BAD_ACCESS_UNWIND.lock().unwrap(); + + let func = vmctx + .get_func_from_idx(0, cb_idx) + .expect("can get function by index"); + let func = std::mem::transmute::( + func.ptr.as_usize(), + ); + let vmctx_raw = vmctx.as_raw(); + func(vmctx_raw); + + drop(lock); + } + #[lucet_hostcall] #[no_mangle] pub fn hostcall_bad_borrow(vmctx: &Vmctx) -> bool { @@ -168,12 +322,13 @@ macro_rules! host_tests { const ERROR_MESSAGE: &'static str = "hostcall_test_func_hostcall_error"; lazy_static! { - static ref HOSTCALL_MUTEX: Arc> = Arc::new(Mutex::new(())); + static ref HOSTCALL_MUTEX: Mutex<()> = Mutex::new(()); static ref NESTED_OUTER: Mutex<()> = Mutex::new(()); static ref NESTED_INNER: Mutex<()> = Mutex::new(()); static ref NESTED_REGS_OUTER: Mutex<()> = Mutex::new(()); static ref NESTED_REGS_INNER: Mutex<()> = Mutex::new(()); - static ref FAULT_UNWIND: Mutex<()> = Mutex::new(()); + static ref BAD_ACCESS_UNWIND: Mutex<()> = Mutex::new(()); + static ref STACK_OVERFLOW_UNWIND: Mutex<()> = Mutex::new(()); } #[inline] From 61e083c8f94e02e7c11766cd2e10b7fe991fc189 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Thu, 10 Oct 2019 15:08:29 -0700 Subject: [PATCH 22/37] improve forced unwinding in cases with termination and yield - adds a `catch_unwind` around each hostcall to make sure we decrement `hostcall_count` when unwinding - handles `State::Yielded` instances by setting a pending termination flag, and then resuming --- .../src/hostcall_macros.rs | 7 +- .../lucet-runtime-internals/src/instance.rs | 194 ++++++++++-------- .../lucet-runtime-internals/src/vmctx.rs | 3 + 3 files changed, 119 insertions(+), 85 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs b/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs index 2798ecc0f..da7284763 100644 --- a/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs +++ b/lucet-runtime/lucet-runtime-internals/src/hostcall_macros.rs @@ -59,10 +59,9 @@ macro_rules! lucet_hostcalls { // let res = std::panic::catch_unwind(move || { #[allow(unused_imports)] use lucet_runtime_internals::vmctx::VmctxInternal; - $crate::vmctx::Vmctx::from_raw(vmctx_raw).instance_mut().begin_hostcall(); - let res = hostcall_impl(&mut $crate::vmctx::Vmctx::from_raw(vmctx_raw), $( $arg ),*); - - $crate::vmctx::Vmctx::from_raw(vmctx_raw).instance_mut().end_hostcall(); + let res = $crate::vmctx::Vmctx::from_raw(vmctx_raw).instance_mut().in_hostcall(|| { + hostcall_impl(&mut $crate::vmctx::Vmctx::from_raw(vmctx_raw), $( $arg ),*) + }); res // }); // match res { diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index 6a3f2b9f9..f9b582ebb 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -26,6 +26,7 @@ use std::cell::{BorrowError, BorrowMutError, Ref, RefCell, RefMut, UnsafeCell}; use std::marker::PhantomData; use std::mem; use std::ops::{Deref, DerefMut}; +use std::panic::{catch_unwind, resume_unwind, UnwindSafe}; use std::ptr::{self, NonNull}; use std::sync::Arc; @@ -266,6 +267,8 @@ pub struct Instance { hostcall_count: u64, + pub(crate) pending_termination: Option, + /// `_padding` must be the last member of the structure. /// This marks where the padding starts to make the structure exactly 4096 bytes long. /// It is also used to compute the size of the structure up to that point, i.e. without padding. @@ -281,6 +284,10 @@ pub struct Instance { /// this Instance exists in, *while* the Instance destructor is executing. impl Drop for Instance { fn drop(&mut self) { + // Initiate a forced unwind if any hostcall frames remain on the stack + if self.hostcall_count > 0 { + self.force_unwind().expect("forced unwinding succeeds"); + } // Reset magic to indicate this instance // is no longer valid self.magic = 0; @@ -595,7 +602,7 @@ impl Instance { /// [run_start]: struct.Instance.html#method.run pub fn reset(&mut self) -> Result<(), Error> { if self.hostcall_count > 0 { - self.force_unwind().unwrap(); + self.force_unwind()?; } self.alloc.reset_heap(self.module.as_ref())?; let globals = unsafe { self.alloc.globals_mut() }; @@ -617,6 +624,9 @@ impl Instance { } else { self.state = State::Ready; } + self.resumed_val = None; + self.hostcall_count = 0; + self.pending_termination = None; #[cfg(feature = "concurrent_testpoints")] { @@ -844,12 +854,17 @@ impl Instance { res } - pub fn begin_hostcall(&mut self) { + pub fn in_hostcall R + UnwindSafe, R>(&mut self, f: F) -> R { self.hostcall_count += 1; - } - - pub fn end_hostcall(&mut self) { + let res = match catch_unwind(f) { + Ok(res) => res, + Err(e) => { + self.hostcall_count -= 1; + resume_unwind(e) + } + }; self.hostcall_count -= 1; + res } #[inline] @@ -897,6 +912,7 @@ impl Instance { entrypoint: None, resumed_val: None, hostcall_count: 0, + pending_termination: None, _padding: (), }; inst.set_globals_ptr(globals_ptr); @@ -1243,90 +1259,106 @@ impl Instance { // Force a guest to unwind the stack from the specified guest address fn force_unwind(&mut self) -> Result<(), Error> { - // if we should unwind by returning into the guest to cause a fault, do so with the redzone - // available in case the guest was at or close to overflowing. - self.with_redzone_stack(|inst| { - #[unwind(allowed)] - extern "C" fn initiate_unwind() { - panic!(TerminationDetails::ForcedUnwind); + match &self.state { + State::Yielded { .. } => { + self.pending_termination = Some(TerminationDetails::ForcedUnwind); + match self.with_redzone_stack(|inst| inst.swap_and_return()) { + Err(Error::RuntimeTerminated(TerminationDetails::ForcedUnwind)) => Ok(()), + Err(e) => lucet_bail!( + "resume with forced unwind returned with an unexpected error: {}", + e + ), + Ok(_) => lucet_bail!("resume with forced unwind returned normally"), + } } + State::Faulted { .. } => { + // if we should unwind by returning into the guest to cause a fault, do so with the redzone + // available in case the guest was at or close to overflowing. + self.with_redzone_stack(|inst| { + #[unwind(allowed)] + extern "C" fn initiate_unwind() { + panic!(TerminationDetails::ForcedUnwind); + } - let guest_addr = inst - .ctx - .stop_addr - .expect("guest that stopped in guest code has an address it stopped at"); + let guest_addr = inst + .ctx + .stop_addr + .expect("guest that stopped in guest code has an address it stopped at"); + + // set up the faulting instruction pointer as the return address for `initiate_unwind`; + // extremely unsafe, doesn't handle any edge cases yet + // + // TODO(Andy) if the last address is obtained through the signal handler, for a signal + // received exactly when we have just executed a `call` to a guest function, we + // actually want to not push it (or push it +1?) lest we try to unwind with a return + // address == start of function, where the system unwinder will unwind for the function + // at address-1, (probably) fail to find the function, and `abort()`. + // + // if `rip` == the start of some guest function, we can probably just discard it and + // use the return address instead. + inst.push(guest_addr as u64) + .expect("stack has available space"); + + // The logic for this conditional can be a bit unintuitive: we _require_ that the stack + // is aligned to 8 bytes, but not 16 bytes, when pushing `initiate_unwind`. + // + // A diagram of the required layout may help: + // `XXXXX0`: ------------------ <-- call frame start -- SysV ABI requires 16-byte alignment + // `XXXXX8`: | return address | + // `XXXX..`: | ..locals etc.. | + // `XXXX..`: | ..as needed... | + // + // Now ensure we _have_ an ABI-conformant call fame like above, by handling the case that + // could lead to an unaligned stack - the guest stack pointer currently being unaligned. + // Among other errors, a misaligned stack will result in compiler-generated xmm accesses to + // fault. + // + // Eg, we would have a stack like: + // `XXXXX8`: ------------------ <-- guest stack end, call frame start + // `XXXXX0`: | unwind_stub | + // `XXXX..`: | ..locals etc.. | + // `XXXX..`: | ..as needed... | + // + // So, instead, push a new return address to construct a new call frame at the right + // offset. `unwind_stub` has CFA directives so the unwinder can connect from + // `initiate_unwind` to guest/host frames to unwind. The unwinder, thankfully, has no + // preferences about alignment of frames being unwound. + // + // And we end up with a guest stack like this: + // `XXXXX8`: ------------------ <-- guest stack end + // `XXXXX0`: | guest ret addr | <-- guest return address to unwind through + // `XXXXX0`: ------------------ <-- call frame start -- SysV ABI requires 16-byte alignment + // `XXXXX8`: | unwind_stub | + // `XXXX..`: | ..locals etc.. | + // `XXXX..`: | ..as needed... | + if inst.ctx.gpr.rsp % 16 == 0 { + // extremely unsafe, doesn't handle any stack exhaustion edge cases yet + inst.push(crate::context::unwind_stub as u64) + .expect("stack has available space"); + } - // set up the faulting instruction pointer as the return address for `initiate_unwind`; - // extremely unsafe, doesn't handle any edge cases yet - // - // TODO(Andy) if the last address is obtained through the signal handler, for a signal - // received exactly when we have just executed a `call` to a guest function, we - // actually want to not push it (or push it +1?) lest we try to unwind with a return - // address == start of function, where the system unwinder will unwind for the function - // at address-1, (probably) fail to find the function, and `abort()`. - // - // if `rip` == the start of some guest function, we can probably just discard it and - // use the return address instead. - inst.push(guest_addr as u64) - .expect("stack has available space"); + assert!(inst.ctx.gpr.rsp % 16 == 8); + // extremely unsafe, doesn't handle any stack exhaustion edge cases yet + inst.push(initiate_unwind as u64) + .expect("stack has available space"); - // The logic for this conditional can be a bit unintuitive: we _require_ that the stack - // is aligned to 8 bytes, but not 16 bytes, when pushing `initiate_unwind`. - // - // A diagram of the required layout may help: - // `XXXXX0`: ------------------ <-- call frame start -- SysV ABI requires 16-byte alignment - // `XXXXX8`: | return address | - // `XXXX..`: | ..locals etc.. | - // `XXXX..`: | ..as needed... | - // - // Now ensure we _have_ an ABI-conformant call fame like above, by handling the case that - // could lead to an unaligned stack - the guest stack pointer currently being unaligned. - // Among other errors, a misaligned stack will result in compiler-generated xmm accesses to - // fault. - // - // Eg, we would have a stack like: - // `XXXXX8`: ------------------ <-- guest stack end, call frame start - // `XXXXX0`: | unwind_stub | - // `XXXX..`: | ..locals etc.. | - // `XXXX..`: | ..as needed... | - // - // So, instead, push a new return address to construct a new call frame at the right - // offset. `unwind_stub` has CFA directives so the unwinder can connect from - // `initiate_unwind` to guest/host frames to unwind. The unwinder, thankfully, has no - // preferences about alignment of frames being unwound. - // - // And we end up with a guest stack like this: - // `XXXXX8`: ------------------ <-- guest stack end - // `XXXXX0`: | guest ret addr | <-- guest return address to unwind through - // `XXXXX0`: ------------------ <-- call frame start -- SysV ABI requires 16-byte alignment - // `XXXXX8`: | unwind_stub | - // `XXXX..`: | ..locals etc.. | - // `XXXX..`: | ..as needed... | - if inst.ctx.gpr.rsp % 16 == 0 { - // extremely unsafe, doesn't handle any stack exhaustion edge cases yet - inst.push(crate::context::unwind_stub as u64) - .expect("stack has available space"); - } + inst.state = State::Ready; - assert!(inst.ctx.gpr.rsp % 16 == 8); - // extremely unsafe, doesn't handle any stack exhaustion edge cases yet - inst.push(initiate_unwind as u64) - .expect("stack has available space"); + match inst.swap_and_return() { + Ok(_) => panic!("forced unwinding shouldn't return normally"), + Err(Error::RuntimeTerminated(TerminationDetails::ForcedUnwind)) => (), + Err(e) => panic!("unexpected error: {}", e), + } - inst.state = State::Ready; + // we've unwound the stack, so we know there are no longer any host frames. + // inst.hostcall_count = 0; + inst.ctx.stop_addr = None; - match inst.swap_and_return() { - Ok(_) => panic!("forced unwinding shouldn't return normally"), - Err(Error::RuntimeTerminated(TerminationDetails::ForcedUnwind)) => (), - Err(e) => panic!("unexpected error: {}", e), + Ok(()) + }) } - - // we've unwound the stack, so we know there are no longer any host frames. - inst.hostcall_count = 0; - inst.ctx.stop_addr = None; - - Ok(()) - }) + st => panic!("should never do forced unwinding in this state: {}", st), + } } } diff --git a/lucet-runtime/lucet-runtime-internals/src/vmctx.rs b/lucet-runtime/lucet-runtime-internals/src/vmctx.rs index c08cd5e0a..308bccef2 100644 --- a/lucet-runtime/lucet-runtime-internals/src/vmctx.rs +++ b/lucet-runtime/lucet-runtime-internals/src/vmctx.rs @@ -398,6 +398,9 @@ impl Vmctx { }; HOST_CTX.with(|host_ctx| unsafe { Context::swap(&mut inst.ctx, &mut *host_ctx.get()) }); + if let Some(td) = inst.pending_termination.take() { + panic!(td); + } } /// Take and return the value passed to From 8d98bc8295292e9303751d0a9f30db39a8d58305 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Thu, 10 Oct 2019 15:32:17 -0700 Subject: [PATCH 23/37] simplify unwinding implementation by using `Faulted` state This state already had the context information we needed to initiate unwinding, so we didn't need to add an extra field on our `Context`, or preemptively overwrite the guest context in the signal handler --- .../src/context/mod.rs | 2 -- .../lucet-runtime-internals/src/instance.rs | 30 +++++++++---------- .../src/instance/signals.rs | 2 +- .../src/sysdeps/linux.rs | 4 +-- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/context/mod.rs b/lucet-runtime/lucet-runtime-internals/src/context/mod.rs index 9a734f2c7..ac35d3ef7 100644 --- a/lucet-runtime/lucet-runtime-internals/src/context/mod.rs +++ b/lucet-runtime/lucet-runtime-internals/src/context/mod.rs @@ -123,7 +123,6 @@ pub struct Context { backstop_callback: *const unsafe extern "C" fn(*mut Instance), callback_data: *mut Instance, sigset: signal::SigSet, - pub(crate) stop_addr: Option, } impl Context { @@ -138,7 +137,6 @@ impl Context { backstop_callback: Context::default_backstop_callback as *const _, callback_data: ptr::null_mut(), sigset: signal::SigSet::empty(), - stop_addr: None, } } diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index f9b582ebb..ba8b71edc 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -1271,20 +1271,21 @@ impl Instance { Ok(_) => lucet_bail!("resume with forced unwind returned normally"), } } - State::Faulted { .. } => { + State::Faulted { context, .. } => { + #[unwind(allowed)] + extern "C" fn initiate_unwind() { + panic!(TerminationDetails::ForcedUnwind); + } + + // set up the guest context as it was when the fault was raised + context.as_ptr().save_to_context(&mut self.ctx); + + // get the instruction pointer when the fault was raised + let guest_addr = context.as_ptr().get_ip(); + // if we should unwind by returning into the guest to cause a fault, do so with the redzone // available in case the guest was at or close to overflowing. self.with_redzone_stack(|inst| { - #[unwind(allowed)] - extern "C" fn initiate_unwind() { - panic!(TerminationDetails::ForcedUnwind); - } - - let guest_addr = inst - .ctx - .stop_addr - .expect("guest that stopped in guest code has an address it stopped at"); - // set up the faulting instruction pointer as the return address for `initiate_unwind`; // extremely unsafe, doesn't handle any edge cases yet // @@ -1342,8 +1343,6 @@ impl Instance { inst.push(initiate_unwind as u64) .expect("stack has available space"); - inst.state = State::Ready; - match inst.swap_and_return() { Ok(_) => panic!("forced unwinding shouldn't return normally"), Err(Error::RuntimeTerminated(TerminationDetails::ForcedUnwind)) => (), @@ -1351,13 +1350,12 @@ impl Instance { } // we've unwound the stack, so we know there are no longer any host frames. - // inst.hostcall_count = 0; - inst.ctx.stop_addr = None; + debug_assert_eq!(inst.hostcall_count, 0); Ok(()) }) } - st => panic!("should never do forced unwinding in this state: {}", st), + st => lucet_bail!("should never do forced unwinding in this state: {}", st), } } } diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs index adc9dc57d..d0a29db86 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs @@ -276,7 +276,7 @@ extern "C" fn handle_signal(signum: c_int, siginfo_ptr: *mut siginfo_t, ucontext let trapcode = inst.module.lookup_trapcode(rip); let behavior = (inst.signal_handler)(inst, &trapcode, signum, siginfo_ptr, ucontext_ptr); - let switch_to_host = match behavior { + match behavior { SignalBehavior::Continue => { // return to the guest context without making any modifications to the instance false diff --git a/lucet-runtime/lucet-runtime-internals/src/sysdeps/linux.rs b/lucet-runtime/lucet-runtime-internals/src/sysdeps/linux.rs index 7773e22ec..8e11263d9 100644 --- a/lucet-runtime/lucet-runtime-internals/src/sysdeps/linux.rs +++ b/lucet-runtime/lucet-runtime-internals/src/sysdeps/linux.rs @@ -85,8 +85,8 @@ impl UContext { } } - pub fn as_ptr(&mut self) -> UContextPtr { - UContextPtr::new(self.context as *mut _ as *mut _) + pub fn as_ptr(&self) -> UContextPtr { + UContextPtr::new(&self.context as *const _ as *const _) } } From 9b627e2f2213c3f66c2037a765f520bb09059ec0 Mon Sep 17 00:00:00 2001 From: "Adam C. Foltzer" Date: Fri, 11 Oct 2019 14:14:29 -0700 Subject: [PATCH 24/37] add rudimentary backtrace support, still needs to be cleaned up --- Cargo.lock | 1 + .../lucet-runtime-internals/Cargo.toml | 1 + .../lucet-runtime-internals/src/c_api.rs | 5 +++ .../lucet-runtime-internals/src/instance.rs | 11 +++++ .../src/instance/internals.rs | 0 .../src/instance/signals.rs | 11 ++--- .../src/instance/state.rs | 2 + .../lucet-runtime-internals/src/module.rs | 3 ++ .../lucet-runtime-internals/src/module/dl.rs | 45 +++++++++++++++++++ .../src/module/mock.rs | 9 ++++ 10 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 lucet-runtime/lucet-runtime-internals/src/instance/internals.rs diff --git a/Cargo.lock b/Cargo.lock index 711ff5fbc..9ca8ca3b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -956,6 +956,7 @@ name = "lucet-runtime-internals" version = "0.7.0-dev" dependencies = [ "anyhow", + "backtrace", "bincode", "bitflags 1.2.1", "byteorder", diff --git a/lucet-runtime/lucet-runtime-internals/Cargo.toml b/lucet-runtime/lucet-runtime-internals/Cargo.toml index 34e92478c..7a8d1655d 100644 --- a/lucet-runtime/lucet-runtime-internals/Cargo.toml +++ b/lucet-runtime/lucet-runtime-internals/Cargo.toml @@ -14,6 +14,7 @@ lucet-module = { path = "../../lucet-module", version = "=0.7.0-dev" } lucet-runtime-macros = { path = "../lucet-runtime-macros", version = "=0.7.0-dev" } anyhow = "1.0" +backtrace = "0.3" bitflags = "1.0" bincode = "1.1.4" byteorder = "1.3" diff --git a/lucet-runtime/lucet-runtime-internals/src/c_api.rs b/lucet-runtime/lucet-runtime-internals/src/c_api.rs index 393cf4cdf..e263071c2 100644 --- a/lucet-runtime/lucet-runtime-internals/src/c_api.rs +++ b/lucet-runtime/lucet-runtime-internals/src/c_api.rs @@ -311,6 +311,10 @@ pub mod lucet_result { .map(|CTerminationDetails { details }| *details) .unwrap_or(ptr::null_mut()), }, + TerminationDetails::ForcedUnwind => lucet_terminated { + reason: lucet_terminated_reason::ForcedUnwind, + provided: std::ptr::null_mut(), + }, TerminationDetails::Remote => lucet_terminated { reason: lucet_terminated_reason::Remote, provided: std::ptr::null_mut(), @@ -368,6 +372,7 @@ pub mod lucet_result { YieldTypeMismatch, BorrowError, Provided, + ForcedUnwind, Remote, } diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index ba8b71edc..d40ee05ac 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -18,6 +18,7 @@ use crate::region::RegionInternal; use crate::sysdeps::HOST_PAGE_SIZE_EXPECTED; use crate::val::{UntypedRetVal, Val}; use crate::WASM_PAGE_SIZE; +use backtrace::Backtrace; use libc::{c_void, pthread_self, siginfo_t, uintptr_t}; use lucet_module::InstanceRuntimeData; use memoffset::offset_of; @@ -1156,6 +1157,7 @@ impl Instance { mut details, siginfo, context, + full_backtrace, } => { // Sandbox is no longer runnable. It's unsafe to determine all error details in the signal // handler, so we fill in extra details here. @@ -1166,11 +1168,15 @@ impl Instance { .module .addr_details(details.rip_addr as *const c_void)?; + details.backtrace = Some(self.module.resolve_and_trim(&full_backtrace)); + // dbg!(&details.backtrace); + // fill the state back in with the updated details in case fatal handlers need it self.state = State::Faulted { details: details.clone(), siginfo, context, + full_backtrace, }; if details.fatal { @@ -1374,6 +1380,8 @@ pub struct FaultDetails { pub rip_addr: uintptr_t, /// Extra information about the instruction pointer's location, if available. pub rip_addr_details: Option, + /// Backtrace of the frames from the guest stack, if available. + pub backtrace: Option, } impl std::fmt::Display for FaultDetails { @@ -1428,6 +1436,8 @@ pub enum TerminationDetails { Provided(Box), /// The instance was terminated by its `KillSwitch`. Remote, + /// Returned when the stack is forced to unwind on instance reset or drop. + ForcedUnwind, } impl TerminationDetails { @@ -1478,6 +1488,7 @@ impl std::fmt::Debug for TerminationDetails { TerminationDetails::YieldTypeMismatch => write!(f, "YieldTypeMismatch"), TerminationDetails::Provided(_) => write!(f, "Provided(Any)"), TerminationDetails::Remote => write!(f, "Remote"), + TerminationDetails::ForcedUnwind => write!(f, "ForcedUnwind"), } } } diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/internals.rs b/lucet-runtime/lucet-runtime-internals/src/instance/internals.rs new file mode 100644 index 000000000..e69de29bb diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs index d0a29db86..a009a5be6 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/signals.rs @@ -5,6 +5,7 @@ use crate::instance::{ HOST_CTX, }; use crate::sysdeps::UContextPtr; +use backtrace::Backtrace; use lazy_static::lazy_static; use libc::{c_int, c_void, siginfo_t, SIGBUS, SIGSEGV}; use lucet_module::TrapCode; @@ -276,7 +277,7 @@ extern "C" fn handle_signal(signum: c_int, siginfo_ptr: *mut siginfo_t, ucontext let trapcode = inst.module.lookup_trapcode(rip); let behavior = (inst.signal_handler)(inst, &trapcode, signum, siginfo_ptr, ucontext_ptr); - match behavior { + let switch_to_host = match behavior { SignalBehavior::Continue => { // return to the guest context without making any modifications to the instance false @@ -328,9 +329,11 @@ extern "C" fn handle_signal(signum: c_int, siginfo_ptr: *mut siginfo_t, ucontext // Details set to `None` here: have to wait until `verify_trap_safety` to // fill in these details, because access may not be signal safe. rip_addr_details: None, + backtrace: None, }, siginfo, context: ctx.into(), + full_backtrace: Backtrace::new_unresolved(), }; }; @@ -373,12 +376,6 @@ extern "C" fn handle_signal(signum: c_int, siginfo_ptr: *mut siginfo_t, ucontext inst.lock_testpoints .signal_handler_after_disabling_termination .check(); - - // use the ucontext to fill in the fields of the guest context; we can't use - // `Context::swap()` here because then we'd swap back to the signal handler instead of - // the point in the guest that caused the fault - ctx.save_to_context(&mut inst.ctx); - inst.ctx.stop_addr = Some(rip as u64); } } diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/state.rs b/lucet-runtime/lucet-runtime-internals/src/instance/state.rs index bb77d52da..63f87937e 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance/state.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance/state.rs @@ -1,6 +1,7 @@ use crate::instance::siginfo_ext::SiginfoExt; use crate::instance::{FaultDetails, TerminationDetails, YieldedVal}; use crate::sysdeps::UContext; +use backtrace::Backtrace; use libc::{SIGBUS, SIGSEGV}; use std::any::Any; use std::ffi::{CStr, CString}; @@ -32,6 +33,7 @@ pub enum State { details: FaultDetails, siginfo: libc::siginfo_t, context: UContext, + full_backtrace: Backtrace, }, /// The instance is in the process of terminating. diff --git a/lucet-runtime/lucet-runtime-internals/src/module.rs b/lucet-runtime/lucet-runtime-internals/src/module.rs index f0d78821b..56a2db374 100644 --- a/lucet-runtime/lucet-runtime-internals/src/module.rs +++ b/lucet-runtime/lucet-runtime-internals/src/module.rs @@ -11,6 +11,7 @@ pub use lucet_module::{ use crate::alloc::Limits; use crate::error::Error; +use backtrace::Backtrace; use libc::c_void; /// Details about a program address. @@ -69,6 +70,8 @@ pub trait ModuleInternal: Send + Sync { fn addr_details(&self, addr: *const c_void) -> Result, Error>; + fn resolve_and_trim(&self, full_bt: &Backtrace) -> Backtrace; + fn get_signature(&self, fn_id: FunctionIndex) -> &Signature; fn function_handle_from_ptr(&self, ptr: FunctionPointer) -> FunctionHandle { diff --git a/lucet-runtime/lucet-runtime-internals/src/module/dl.rs b/lucet-runtime/lucet-runtime-internals/src/module/dl.rs index f73497d8b..e8e25e1ef 100644 --- a/lucet-runtime/lucet-runtime-internals/src/module/dl.rs +++ b/lucet-runtime/lucet-runtime-internals/src/module/dl.rs @@ -1,5 +1,6 @@ use crate::error::Error; use crate::module::{AddrDetails, GlobalSpec, HeapSpec, Module, ModuleInternal, TableElement}; +use backtrace::{Backtrace, BacktraceFrame}; use libc::c_void; use libloading::Library; use lucet_module::{ @@ -331,6 +332,50 @@ impl ModuleInternal for DlModule { fn get_signature(&self, fn_id: FunctionIndex) -> &Signature { self.module.module_data.get_signature(fn_id) } + + fn resolve_and_trim(&self, full_bt: &Backtrace) -> Backtrace { + let mut bt = full_bt.clone(); + bt.resolve(); + let trimmed_frames = bt + .frames() + .iter() + .filter(|fr| match self.addr_details(fr.ip()) { + // if we can look up addr details, and it's in module code, keep the frame + Ok(Some(details)) => details.in_module_code, + _ => false, + }) + // // skip everything until the entry to the signal handler + // .skip_while(|fr| { + // fr.symbols() + // .iter() + // .find(|sym| { + // sym.name().map_or(false, |sn| { + // let name = format!("{}", sn); + // name.starts_with( + // "lucet_runtime_internals::instance::signals::handle_signal", + // ) && !name.contains("closure") + // }) + // }) + // .is_none() + // }) + // // drop the handle_signal frame + // .skip(1) + // // take all frames between handle_signal and Context::swap + // .take_while(|fr| { + // fr.symbols() + // .iter() + // .find(|sym| { + // sym.name().map_or(false, |sn| { + // format!("{}", sn) + // .starts_with("lucet_runtime_internals::context::Context::swap") + // }) + // }) + // .is_none() + // }) + .cloned() + .collect::>(); + trimmed_frames.into() + } } // TODO: PR to nix or libloading? diff --git a/lucet-runtime/lucet-runtime-internals/src/module/mock.rs b/lucet-runtime/lucet-runtime-internals/src/module/mock.rs index c64d2ef2d..d4930c9ac 100644 --- a/lucet-runtime/lucet-runtime-internals/src/module/mock.rs +++ b/lucet-runtime/lucet-runtime-internals/src/module/mock.rs @@ -1,5 +1,6 @@ use crate::error::Error; use crate::module::{AddrDetails, GlobalSpec, HeapSpec, Module, ModuleInternal, TableElement}; +use backtrace::Backtrace; use libc::c_void; use lucet_module::owned::{ OwnedExportFunction, OwnedFunctionMetadata, OwnedGlobalSpec, OwnedImportFunction, @@ -335,6 +336,14 @@ impl ModuleInternal for MockModule { Ok(None) } + fn resolve_and_trim(&self, full_bt: &Backtrace) -> Backtrace { + // for a mock module, just resolve since we can't differentiate between hostcall code and + // mock module functions + let mut bt = full_bt.clone(); + bt.resolve(); + bt + } + fn get_signature(&self, fn_id: FunctionIndex) -> &Signature { self.module_data.get_signature(fn_id) } From 7460113958b5cd7d2cd0da3e830e5216b3852663 Mon Sep 17 00:00:00 2001 From: iximeow Date: Wed, 15 Jan 2020 11:39:02 -0800 Subject: [PATCH 25/37] address rebase conflicts in host tests --- lucet-runtime/lucet-runtime-tests/src/host.rs | 126 ++++++++---------- 1 file changed, 57 insertions(+), 69 deletions(-) diff --git a/lucet-runtime/lucet-runtime-tests/src/host.rs b/lucet-runtime/lucet-runtime-tests/src/host.rs index e1f919832..600a27445 100644 --- a/lucet-runtime/lucet-runtime-tests/src/host.rs +++ b/lucet-runtime/lucet-runtime-tests/src/host.rs @@ -25,10 +25,65 @@ macro_rules! host_tests { assert!(module.is_err()); } + const ERROR_MESSAGE: &'static str = "hostcall_test_func_hostcall_error"; + + lazy_static! { + static ref HOSTCALL_MUTEX: Mutex<()> = Mutex::new(()); + static ref NESTED_OUTER: Mutex<()> = Mutex::new(()); + static ref NESTED_INNER: Mutex<()> = Mutex::new(()); + static ref NESTED_REGS_OUTER: Mutex<()> = Mutex::new(()); + static ref NESTED_REGS_INNER: Mutex<()> = Mutex::new(()); + static ref BAD_ACCESS_UNWIND: Mutex<()> = Mutex::new(()); + static ref STACK_OVERFLOW_UNWIND: Mutex<()> = Mutex::new(()); + } + #[allow(unreachable_code)] + #[inline] + unsafe fn unwind_inner(vmctx: &Vmctx, mutex: &Mutex<()>) { + let lock = mutex.lock().unwrap(); + lucet_hostcall_terminate!(ERROR_MESSAGE); + drop(lock); + } + + #[inline] + unsafe fn unwind_outer(vmctx: &Vmctx, mutex: &Mutex<()>, cb_idx: u32) -> u64 { + let lock = mutex.lock().unwrap(); + let func = vmctx + .get_func_from_idx(0, cb_idx) + .expect("can get function by index"); + let func = std::mem::transmute:: u64>( + func.ptr.as_usize(), + ); + let res = (func)(vmctx.as_raw()); + drop(lock); + res + } + + #[lucet_hostcall] #[no_mangle] - pub fn hostcall_test_func_hostcall_error_unwind( - vmctx: &Vmctx, + pub fn hostcall_test_func_hostcall_error(_vmctx: &Vmctx) { + lucet_hostcall_terminate!(ERROR_MESSAGE); + } + + #[lucet_hostcall] + #[no_mangle] + pub fn hostcall_test_func_hello(vmctx: &Vmctx, hello_ptr: u32, hello_len: u32) { + let heap = vmctx.heap(); + let hello = heap.as_ptr() as usize + hello_ptr as usize; + if !vmctx.check_heap(hello as *const c_void, hello_len as usize) { + lucet_hostcall_terminate!("heap access"); + } + let hello = + unsafe { std::slice::from_raw_parts(hello as *const u8, hello_len as usize) }; + if hello.starts_with(b"hello") { + *vmctx.get_embed_ctx_mut::() = true; + } + } + + #[lucet_hostcall] + #[allow(unreachable_code)] + #[no_mangle] + pub fn hostcall_test_func_hostcall_error_unwind(vmctx: &Vmctx) { ) -> () { let lock = HOSTCALL_MUTEX.lock().unwrap(); unsafe { @@ -319,73 +374,6 @@ macro_rules! host_tests { use $crate::helpers::{FunctionPointer, HeapSpec, MockExportBuilder, MockModuleBuilder}; use $TestRegion as TestRegion; - const ERROR_MESSAGE: &'static str = "hostcall_test_func_hostcall_error"; - - lazy_static! { - static ref HOSTCALL_MUTEX: Mutex<()> = Mutex::new(()); - static ref NESTED_OUTER: Mutex<()> = Mutex::new(()); - static ref NESTED_INNER: Mutex<()> = Mutex::new(()); - static ref NESTED_REGS_OUTER: Mutex<()> = Mutex::new(()); - static ref NESTED_REGS_INNER: Mutex<()> = Mutex::new(()); - static ref BAD_ACCESS_UNWIND: Mutex<()> = Mutex::new(()); - static ref STACK_OVERFLOW_UNWIND: Mutex<()> = Mutex::new(()); - } - - #[inline] - unsafe fn unwind_outer(vmctx: &mut Vmctx, mutex: &Mutex<()>, cb_idx: u32) -> u64 { - let lock = mutex.lock().unwrap(); - let func = vmctx - .get_func_from_idx(0, cb_idx) - .expect("can get function by index"); - let func = std::mem::transmute:: u64>( - func.ptr.as_usize(), - ); - let res = (func)(vmctx.as_raw()); - drop(lock); - res - } - - #[allow(unreachable_code)] - #[inline] - unsafe fn unwind_inner(vmctx: &mut Vmctx, mutex: &Mutex<()>) { - let lock = mutex.lock().unwrap(); - lucet_hostcall_terminate!(ERROR_MESSAGE); - drop(lock); - } - - /* - #[lucet_hostcall] - #[no_mangle] - pub fn hostcall_test_func_hostcall_error(_vmctx: &Vmctx) { - lucet_hostcall_terminate!(ERROR_MESSAGE); - } - - #[lucet_hostcall] - #[no_mangle] - pub fn hostcall_test_func_hello(vmctx: &Vmctx, hello_ptr: u32, hello_len: u32) { - let heap = vmctx.heap(); - let hello = heap.as_ptr() as usize + hello_ptr as usize; - if !vmctx.check_heap(hello as *const c_void, hello_len as usize) { - lucet_hostcall_terminate!("heap access"); - } - let hello = - unsafe { std::slice::from_raw_parts(hello as *const u8, hello_len as usize) }; - if hello.starts_with(b"hello") { - *vmctx.get_embed_ctx_mut::() = true; - } - } - - #[lucet_hostcall] - #[allow(unreachable_code)] - #[no_mangle] - pub fn hostcall_test_func_hostcall_error_unwind(vmctx: &Vmctx) { - let lock = vmctx.get_embed_ctx::>>(); - let _mutex_guard = lock.lock().unwrap(); - lucet_hostcall_terminate!(ERROR_MESSAGE); - drop(_mutex_guard); - } - */ - #[test] fn load_module() { let _module = test_module_c("host", "trivial.c").expect("build and load module"); From ff3a4d74ec9acb2cead48af329de02d82051ab9e Mon Sep 17 00:00:00 2001 From: iximeow Date: Thu, 16 Jan 2020 11:38:50 -0800 Subject: [PATCH 26/37] misimplemented some cranelift functions, fix that --- lucet-runtime/lucet-runtime-tests/src/host.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/lucet-runtime/lucet-runtime-tests/src/host.rs b/lucet-runtime/lucet-runtime-tests/src/host.rs index 600a27445..e01f3199b 100644 --- a/lucet-runtime/lucet-runtime-tests/src/host.rs +++ b/lucet-runtime/lucet-runtime-tests/src/host.rs @@ -550,22 +550,6 @@ macro_rules! host_tests { ); } - /// Ensures that callee-saved registers are properly restored following a `catch_unwind` - /// that catches a panic. - #[test] - fn restore_callee_saved() { - let module = - test_module_c("host", "nested_error_unwind.c").expect("build and load module"); - let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - assert_eq!( - u64::from(inst.run("entrypoint_restore", &[]).unwrap()), - 6148914668330025056 - ); - } - /// Ensures that hostcall stack frames get unwound when a fault occurs in guest code. #[test] fn fault_unwind() { From 4af7b7ed53ebd734ec086e8ee86d1ddae2b68eaa Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 17 Jan 2020 16:45:51 -0800 Subject: [PATCH 27/37] adjust cfi expressions to use the correct parent_ctx offset the struct at rbp changed between first authorship and today, to fix a bug where lucet instances were accidentally tied to their starting thread --- .../src/context/context_asm.S | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S b/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S index 36785d992..89452e40c 100644 --- a/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S +++ b/lucet-runtime/lucet-runtime-internals/src/context/context_asm.S @@ -92,8 +92,8 @@ _unwind_stub: // able to figure this out based on the saved rsp value, things go wrong if this is missing. .cfi_escape 0x0f, /* DW_CFA_def_cfa_expression */ \ - 8, /* uleb128 length of expression bytes */ \ - 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 9, /* uleb128 length of expression bytes */ \ + 0x76, 0xf0, 0x01, /* DW_OP_breg5 (put rbp + 240 on the stack; should be a pointer to the parent context's address) */ \ 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ 0x23, 0x08, /* DW_OP_plus_uconst (add 8 to the base of context to point to saved rsp) */ \ 0x06, /* deref */ \ @@ -104,8 +104,8 @@ _unwind_stub: // the extra `lucet_context_swap` call frame we want to skip over .cfi_escape 0x16, 0x07, /* DW_CFA_val_expression(7=rsp) */ \ - 8, /* uleb128 length of expression bytes */ \ - 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 9, /* uleb128 length of expression bytes */ \ + 0x76, 0xf0, 0x01, /* DW_OP_breg5 (put rbp + 240 on the stack; should be a pointer to the parent context's address) */ \ 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ 0x23, 0x08, /* DW_OP_plus_uconst (add 8 to the base of context to point to saved rsp) */ \ 0x06, /* deref */ \ @@ -115,44 +115,44 @@ _unwind_stub: // the corresponding field on the `Context` struct .cfi_escape 0x10, 0x03, /* DW_CFA_expression(3=rbx) */ \ - 3, /* uleb128 length of expression bytes */ \ - 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 4, /* uleb128 length of expression bytes */ \ + 0x76, 0xf0, 0x01, /* DW_OP_breg5 (put rbp + 240 on the stack; should be a pointer to the parent context's address) */ \ 0x06 /* DW_OP_deref should then leave the parent context pointer on the stack */ \ /* rbx is at offset 0 */ .cfi_escape 0x10, 0x06, /* DW_CFA_expression(6=rbp) */ \ - 5, /* uleb128 length of expression bytes */ \ - 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 6, /* uleb128 length of expression bytes */ \ + 0x76, 0xf0, 0x01, /* DW_OP_breg5 (put rbp + 240 on the stack; should be a pointer to the parent context's address) */ \ 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ 0x23, 0x10 /* DW_OP_plus_uconst (add 16 to the base of context to point to saved rbp) */ .cfi_escape 0x10, 0x05, /* DW_CFA_expression(5=rdi) */ \ - 5, /* uleb128 length of expression bytes */ \ - 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 6, /* uleb128 length of expression bytes */ \ + 0x76, 0xf0, 0x01, /* DW_OP_breg5 (put rbp + 240 on the stack; should be a pointer to the parent context's address) */ \ 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ 0x23, 0x18 /* DW_OP_plus_uconst (add 24 to the base of context to point to saved rdi) */ .cfi_escape 0x10, 0x0c, /* DW_CFA_expression(12=r12) */ \ - 5, /* uleb128 length of expression bytes */ \ - 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 6, /* uleb128 length of expression bytes */ \ + 0x76, 0xf0, 0x01, /* DW_OP_breg5 (put rbp + 240 on the stack; should be a pointer to the parent context's address) */ \ 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ 0x23, 0x20 /* DW_OP_plus_uconst (add 32 to the base of context to point to saved r12) */ .cfi_escape 0x10, 0x0d, /* DW_CFA_expression(13=r13) */ \ - 5, /* uleb128 length of expression bytes */ \ - 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 6, /* uleb128 length of expression bytes */ \ + 0x76, 0xf0, 0x01, /* DW_OP_breg5 (put rbp + 240 on the stack; should be a pointer to the parent context's address) */ \ 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ 0x23, 0x28 /* DW_OP_plus_uconst (add 40 to the base of context to point to saved r13) */ .cfi_escape 0x10, 0x0e, /* DW_CFA_expression(14=r14) */ \ - 5, /* uleb128 length of expression bytes */ \ - 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 6, /* uleb128 length of expression bytes */ \ + 0x76, 0xf0, 0x01, /* DW_OP_breg5 (put rbp + 240 on the stack; should be a pointer to the parent context's address) */ \ 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ 0x23, 0x30 /* DW_OP_plus_uconst (add 48 to the base of context to point to saved r14) */ .cfi_escape 0x10, 0x0f, /* DW_CFA_expression(15=r15) */ \ - 5, /* uleb128 length of expression bytes */ \ - 0x76, 0x18, /* DW_OP_breg5 (put rbp + 24 on the stack; should be a pointer to the parent context's address) */ \ + 6, /* uleb128 length of expression bytes */ \ + 0x76, 0xf0, 0x01, /* DW_OP_breg5 (put rbp + 240 on the stack; should be a pointer to the parent context's address) */ \ 0x06, /* DW_OP_deref should then leave the parent context pointer on the stack */ \ 0x23, 0x38 /* DW_OP_plus_uconst (add 56 to the base of context to point to saved r15) */ From ce5b86f499701e7d24993cc1a5d9e64d2a269bdd Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 17 Jan 2020 17:13:54 -0800 Subject: [PATCH 28/37] fix conflict from rebase --- helpers/indent.sh | 2 +- .../src/context/mod.rs | 3 +- .../lucet-runtime-internals/src/instance.rs | 2 +- .../src/region/uffd.rs | 9 ++ .../src/sysdeps/linux.rs | 5 +- lucet-runtime/lucet-runtime-macros/src/lib.rs | 24 +--- lucet-runtime/lucet-runtime-tests/src/host.rs | 134 ++++++++++-------- 7 files changed, 94 insertions(+), 85 deletions(-) diff --git a/helpers/indent.sh b/helpers/indent.sh index 1bc30d3f2..90e0f35dd 100755 --- a/helpers/indent.sh +++ b/helpers/indent.sh @@ -10,7 +10,7 @@ cleanup() { } trap cleanup 1 2 3 6 15 -RUSTFMT_VERSION=1.4.12-nightly +RUSTFMT_VERSION=1.4.15-nightly if ! rustfmt --version | grep -q "rustfmt $RUSTFMT_VERSION"; then echo "indent requires rustfmt $RUSTFMT_VERSION" diff --git a/lucet-runtime/lucet-runtime-internals/src/context/mod.rs b/lucet-runtime/lucet-runtime-internals/src/context/mod.rs index ac35d3ef7..c7a0582ac 100644 --- a/lucet-runtime/lucet-runtime-internals/src/context/mod.rs +++ b/lucet-runtime/lucet-runtime-internals/src/context/mod.rs @@ -5,7 +5,8 @@ mod tests; use crate::instance::Instance; use crate::val::{val_to_reg, val_to_stack, RegVal, UntypedRetVal, Val}; - +use nix; +use nix::sys::signal; use std::arch::x86_64::{__m128, _mm_setzero_ps}; use std::ptr::NonNull; use std::{mem, ptr}; diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index d40ee05ac..6353a6100 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -1265,7 +1265,7 @@ impl Instance { // Force a guest to unwind the stack from the specified guest address fn force_unwind(&mut self) -> Result<(), Error> { - match &self.state { + match &mut self.state { State::Yielded { .. } => { self.pending_termination = Some(TerminationDetails::ForcedUnwind); match self.with_redzone_stack(|inst| inst.swap_and_return()) { diff --git a/lucet-runtime/lucet-runtime-internals/src/region/uffd.rs b/lucet-runtime/lucet-runtime-internals/src/region/uffd.rs index c1ef015df..779aae2b2 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/uffd.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/uffd.rs @@ -318,6 +318,14 @@ impl RegionInternal for UffdRegion { fn as_dyn_internal(&self) -> &dyn RegionInternal { self } + + fn enable_stack_redzone(&self, _slot: &Slot) { + // no-op + } + + fn disable_stack_redzone(&self, _slot: &Slot) { + // no-op + } } impl RegionCreate for UffdRegion { @@ -502,6 +510,7 @@ impl UffdRegion { sigstack: sigstack as *mut c_void, limits: region.limits.clone(), region: Arc::downgrade(region) as Weak, + redzone_stack_enabled: true, }) } } diff --git a/lucet-runtime/lucet-runtime-internals/src/sysdeps/linux.rs b/lucet-runtime/lucet-runtime-internals/src/sysdeps/linux.rs index 8e11263d9..c7273e1b2 100644 --- a/lucet-runtime/lucet-runtime-internals/src/sysdeps/linux.rs +++ b/lucet-runtime/lucet-runtime-internals/src/sysdeps/linux.rs @@ -1,4 +1,3 @@ -use libc::{c_void, ucontext_t, REG_RDI, REG_RIP}; use crate::context::Context; use libc::{ c_void, ucontext_t, REG_R12, REG_R13, REG_R14, REG_R15, REG_RBP, REG_RBX, REG_RDI, REG_RIP, @@ -85,8 +84,8 @@ impl UContext { } } - pub fn as_ptr(&self) -> UContextPtr { - UContextPtr::new(&self.context as *const _ as *const _) + pub fn as_ptr(&mut self) -> UContextPtr { + UContextPtr::new(self.context as *mut _ as *mut _) } } diff --git a/lucet-runtime/lucet-runtime-macros/src/lib.rs b/lucet-runtime/lucet-runtime-macros/src/lib.rs index fd575946f..c0d15a31a 100644 --- a/lucet-runtime/lucet-runtime-macros/src/lib.rs +++ b/lucet-runtime/lucet-runtime-macros/src/lib.rs @@ -90,12 +90,6 @@ pub fn lucet_hostcall(_attr: TokenStream, item: TokenStream) -> TokenStream { }) .collect::>(); - let termination_details = if from_internals { - quote! { lucet_runtime_internals::instance::TerminationDetails } - } else { - quote! { lucet_runtime::TerminationDetails } - }; - let raw_hostcall = quote! { #(#attrs)* #vis @@ -105,20 +99,10 @@ pub fn lucet_hostcall(_attr: TokenStream, item: TokenStream) -> TokenStream { let vmctx = #vmctx_mod::Vmctx::from_raw(vmctx_raw); #vmctx_mod::VmctxInternal::instance_mut(&vmctx).uninterruptable(|| { - let res = std::panic::catch_unwind(move || { - #hostcall_ident(&#vmctx_mod::Vmctx::from_raw(vmctx_raw), #(#impl_args),*) - }); - match res { - Ok(res) => res, - Err(e) => { - match e.downcast::<#termination_details>() { - Ok(details) => { - #vmctx_mod::Vmctx::from_raw(vmctx_raw).terminate_no_unwind(*details) - }, - Err(e) => std::panic::resume_unwind(e), - } - } - } + let vmctx = #vmctx_mod::Vmctx::from_raw(vmctx_raw); + #vmctx_mod::VmctxInternal::instance_mut(&vmctx).in_hostcall(|| { + #hostcall_ident(&mut #vmctx_mod::Vmctx::from_raw(vmctx_raw), #(#impl_args),*) + }) }) } }; diff --git a/lucet-runtime/lucet-runtime-tests/src/host.rs b/lucet-runtime/lucet-runtime-tests/src/host.rs index e01f3199b..71b263df8 100644 --- a/lucet-runtime/lucet-runtime-tests/src/host.rs +++ b/lucet-runtime/lucet-runtime-tests/src/host.rs @@ -51,7 +51,7 @@ macro_rules! host_tests { let func = vmctx .get_func_from_idx(0, cb_idx) .expect("can get function by index"); - let func = std::mem::transmute:: u64>( + let func = std::mem::transmute:: u64>( func.ptr.as_usize(), ); let res = (func)(vmctx.as_raw()); @@ -84,7 +84,6 @@ macro_rules! host_tests { #[allow(unreachable_code)] #[no_mangle] pub fn hostcall_test_func_hostcall_error_unwind(vmctx: &Vmctx) { - ) -> () { let lock = HOSTCALL_MUTEX.lock().unwrap(); unsafe { lucet_hostcall_terminate!(ERROR_MESSAGE); @@ -98,7 +97,9 @@ macro_rules! host_tests { vmctx: &Vmctx, cb_idx: u32, ) -> u64 { - unwind_outer(vmctx, &*NESTED_OUTER, cb_idx) + unsafe { + unwind_outer(vmctx, &*NESTED_OUTER, cb_idx) + } } #[lucet_hostcall] @@ -106,7 +107,9 @@ macro_rules! host_tests { pub fn nested_error_unwind_inner( vmctx: &Vmctx, ) -> () { - unwind_inner(vmctx, &*NESTED_INNER) + unsafe { + unwind_inner(vmctx, &*NESTED_INNER) + } } #[lucet_hostcall] @@ -115,7 +118,9 @@ macro_rules! host_tests { vmctx: &Vmctx, cb_idx: u32, ) -> u64 { - unwind_outer(vmctx, &*NESTED_REGS_OUTER, cb_idx) + unsafe { + unwind_outer(vmctx, &*NESTED_REGS_OUTER, cb_idx) + } } #[lucet_hostcall] @@ -123,7 +128,9 @@ macro_rules! host_tests { pub fn nested_error_unwind_regs_inner( vmctx: &Vmctx, ) -> () { - unwind_inner(vmctx, &*NESTED_REGS_INNER) + unsafe { + unwind_inner(vmctx, &*NESTED_REGS_INNER) + } } #[lucet_hostcall] @@ -169,9 +176,11 @@ macro_rules! host_tests { let func = vmctx .get_func_from_idx(0, cb_idx) .expect("can get function by index"); - let func = std::mem::transmute:: u64>( - func.ptr.as_usize(), - ); + let func = unsafe { + std::mem::transmute:: u64>( + func.ptr.as_usize(), + ) + }; let vmctx_raw = vmctx.as_raw(); let res = std::panic::catch_unwind(|| { (func)(vmctx_raw); @@ -205,9 +214,11 @@ macro_rules! host_tests { let func = vmctx .get_func_from_idx(0, cb_idx) .expect("can get function by index"); - let func = std::mem::transmute::( - func.ptr.as_usize(), - ); + let func = unsafe { + std::mem::transmute::( + func.ptr.as_usize(), + ) + }; let vmctx_raw = vmctx.as_raw(); func(vmctx_raw); @@ -225,9 +236,11 @@ macro_rules! host_tests { let func = vmctx .get_func_from_idx(0, cb_idx) .expect("can get function by index"); - let func = std::mem::transmute::( - func.ptr.as_usize(), - ); + let func = unsafe { + std::mem::transmute::( + func.ptr.as_usize(), + ) + }; let vmctx_raw = vmctx.as_raw(); func(vmctx_raw); @@ -431,14 +444,14 @@ macro_rules! host_tests { .expect("instance can be created"); match inst.run("main", &[0u32.into(), 0i32.into()]) { - Err(Error::RuntimeTerminated(term)) => { + Err(Error::RuntimeTerminated(term)) => { assert_eq!( *term .provided_details() .expect("user provided termination reason") .downcast_ref::<&'static str>() .expect("error was static str"), - ERROR_MESSAGE + super::ERROR_MESSAGE ); } res => panic!("unexpected result: {:?}", res), @@ -452,9 +465,7 @@ macro_rules! host_tests { let region = ::create(1, &Limits::default()).expect("region can be created"); let mut inst = region - .new_instance_builder(module) - .with_embed_ctx(HOSTCALL_MUTEX.clone()) - .build() + .new_instance(module) .expect("instance can be created"); match inst.run("main", &[0u32.into(), 0u32.into()]) { @@ -465,18 +476,24 @@ macro_rules! host_tests { .expect("user provided termination reason") .downcast_ref::<&'static str>() .expect("error was static str"), - ERROR_MESSAGE + super::ERROR_MESSAGE ); } res => panic!("unexpected result: {:?}", res), } - assert!(HOSTCALL_MUTEX.is_poisoned()); + assert!(super::HOSTCALL_MUTEX.is_poisoned()); } /// Check that if two segments of hostcall stack are present when terminating, that they /// both get properly unwound. + /// + /// Currently ignored as we don't allow nested hostcalls - the nested hostcall runs afoul + /// of timeouts' domain-checking logic, which assumes beginning a hostscall will only + /// happen from a guest context, but when initiated from a nested hostcall is actually a + /// hostcall context #[test] + #[ignore] fn nested_error_unwind() { let module = test_module_c("host", "nested_error_unwind.c").expect("build and load module"); @@ -493,19 +510,25 @@ macro_rules! host_tests { .expect("user provided termination reason") .downcast_ref::<&'static str>() .expect("error was static str"), - ERROR_MESSAGE + super::ERROR_MESSAGE ); } res => panic!("unexpected result: {:?}", res), } - assert!(NESTED_OUTER.is_poisoned()); - assert!(NESTED_INNER.is_poisoned()); + assert!(super::NESTED_OUTER.is_poisoned()); + assert!(super::NESTED_INNER.is_poisoned()); } /// Like `nested_error_unwind`, but the guest code callback in between the two segments of /// hostcall stack uses enough locals to require saving callee registers. + /// + /// Currently ignored as we don't allow nested hostcalls - the nested hostcall runs afoul + /// of timeouts' domain-checking logic, which assumes beginning a hostscall will only + /// happen from a guest context, but when initiated from a nested hostcall is actually a + /// hostcall context #[test] + #[ignore] fn nested_error_unwind_regs() { let module = test_module_c("host", "nested_error_unwind.c").expect("build and load module"); @@ -522,19 +545,23 @@ macro_rules! host_tests { .expect("user provided termination reason") .downcast_ref::<&'static str>() .expect("error was static str"), - ERROR_MESSAGE + super::ERROR_MESSAGE ); } res => panic!("unexpected result: {:?}", res), } - assert!(NESTED_REGS_OUTER.is_poisoned()); - assert!(NESTED_REGS_INNER.is_poisoned()); + assert!(super::NESTED_REGS_OUTER.is_poisoned()); + assert!(super::NESTED_REGS_INNER.is_poisoned()); } /// Ensures that callee-saved registers are properly restored following a `catch_unwind` - /// that catches a panic. Currently disabled because it relies on UB until - /// `#[unwind(allowed)]` is stabilized. + /// that catches a panic. + /// + /// Currently ignored as we don't allow nested hostcalls - the nested hostcall runs afoul + /// of timeouts' domain-checking logic, which assumes beginning a hostscall will only + /// happen from a guest context, but when initiated from a nested hostcall is actually a + /// hostcall context #[ignore] #[test] fn restore_callee_saved() { @@ -552,15 +579,29 @@ macro_rules! host_tests { /// Ensures that hostcall stack frames get unwound when a fault occurs in guest code. #[test] - fn fault_unwind() { + fn bad_access_unwind() { let module = test_module_c("host", "fault_unwind.c").expect("build and load module"); - let region = TestRegion::create(1, &Limits::default()).expect("region can be created"); + let region = ::create(1, &Limits::default()).expect("region can be created"); let mut inst = region .new_instance(module) .expect("instance can be created"); - inst.run("entrypoint", &[]).unwrap_err(); + inst.run("bad_access", &[]).unwrap_err(); inst.reset().unwrap(); - assert!(FAULT_UNWIND.is_poisoned()); + assert!(super::BAD_ACCESS_UNWIND.is_poisoned()); + } + + /// Ensures that hostcall stack frames get unwound even when a stack overflow occurs in + /// guest code. + #[test] + fn stack_overflow_unwind() { + let module = test_module_c("host", "fault_unwind.c").expect("build and load module"); + let region = ::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + inst.run("stack_overflow", &[]).unwrap_err(); + inst.reset().unwrap(); + assert!(super::STACK_OVERFLOW_UNWIND.is_poisoned()); } #[test] @@ -581,31 +622,6 @@ macro_rules! host_tests { } } - #[test] - fn run_hostcall_print_host_rsp() { - extern "C" { - fn hostcall_print_host_rsp(vmctx: *mut lucet_vmctx); - } - - unsafe extern "C" fn f(vmctx: *mut lucet_vmctx) { - hostcall_print_host_rsp(vmctx); - } - - let module = MockModuleBuilder::new() - .with_export_func(MockExportBuilder::new( - "f", - FunctionPointer::from_usize(f as usize), - )) - .build(); - - let region = ::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - - inst.run("f", &[]).unwrap(); - } - #[test] fn run_hostcall_bad_borrow() { extern "C" { From 0d9db266007289c8393d6ef381b46e2573e91bef Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 19 Jun 2020 14:00:29 -0700 Subject: [PATCH 29/37] eh_frame writing needs to be upstream --- lucetc/src/compiler.rs | 97 ------------------------------------------ 1 file changed, 97 deletions(-) diff --git a/lucetc/src/compiler.rs b/lucetc/src/compiler.rs index a7d9396d0..0a97ae14e 100644 --- a/lucetc/src/compiler.rs +++ b/lucetc/src/compiler.rs @@ -320,36 +320,6 @@ impl<'a> Compiler<'a> { let size = compiled.size; - println!( - "signature for {} is {:?}", - func.name.symbol(), - func.signature.call_conv - ); - println!("isa: {}, ptr bits: {}", isa.name(), isa.pointer_bits()); - let unwind_info = - clif_context - .create_unwind_info(isa.as_ref()) - .map_err(|compilation_error| Error::FunctionDefinition { - symbol: func.name.symbol().to_string(), - source: ClifModuleError::Compilation(compilation_error), - })?; - if let Some(unwind_info) = unwind_info { - if let UnwindInfo::SystemV(info) = unwind_info { - frame_table.add_fde( - cie_id, - info.to_fde(Address::Symbol { - symbol: idx, // func.name.symbol().to_string(), - addend: 0, - }), - ); - } else { - panic!("non-sysv unwind info in lucet module"); - } - } else { - println!("no info for {}", func.name.symbol()); - // .expect("function yields .eh_frame information"); - } - let trap_data_id = traps.write(&mut self.clif_module, func.name.symbol())?; function_map.insert(func_id, (size, trap_data_id, traps.len())); @@ -426,73 +396,6 @@ impl<'a> Compiler<'a> { self.clif_module .define_data(manifest_data_id, &function_manifest_ctx)?; - let mut eh_frame_ctx = ClifDataContext::new(); - struct EhFrameSink<'a> { - pub data: Vec, - pub data_context: &'a mut ClifDataContext, - pub decls: &'a ModuleDecls<'a>, - } - - impl<'a> Writer for EhFrameSink<'a> { - type Endian = gimli::LittleEndian; - - fn endian(&self) -> Self::Endian { - gimli::LittleEndian - } - fn len(&self) -> usize { - self.data.len() - } - fn write(&mut self, bytes: &[u8]) -> gimli::write::Result<()> { - self.data.extend_from_slice(bytes); - Ok(()) - } - fn write_at(&mut self, offset: usize, bytes: &[u8]) -> gimli::write::Result<()> { - if offset + bytes.len() > self.data.len() { - return Err(gimli::write::Error::LengthOutOfBounds); - } - self.data[offset..][..bytes.len()].copy_from_slice(bytes); - Ok(()) - } - - fn write_address(&mut self, address: Address, size: u8) -> gimli::write::Result<()> { - match address { - Address::Constant(val) => self.write_udata(val, size), - Address::Symbol { symbol, addend } => { - assert_eq!(addend, 0); - - use crate::module::UniqueFuncIndex; - let name = self - .decls - .get_func(UniqueFuncIndex::from_u32(symbol as u32)) - .expect("valid index") - .name - .as_externalname(); - let funcref = self.data_context.import_function(name); - let offset = self.data.len(); - self.data_context - .write_function_addr(offset as u32, funcref); - - self.write_udata(0, size) - } - } - } - } - let mut eh_frame = EhFrame(EhFrameSink { - data: Vec::new(), - data_context: &mut eh_frame_ctx, - decls: &self.decls, - }); - frame_table - .write_eh_frame(&mut eh_frame) - .expect("this works too"); - let eh_frame_bytes = eh_frame.0.data.into_boxed_slice(); - eh_frame_ctx.define(eh_frame_bytes); - let eh_frame_data_id = - self.clif_module - .declare_data(".eh_frame", ClifLinkage::Export, false, false, None)?; - self.clif_module - .define_data(eh_frame_data_id, &eh_frame_ctx)?; - // Write out the structure tying everything together. let mut native_data = Cursor::new(Vec::with_capacity(std::mem::size_of::())); From 6ddb7a359fc55f8ae67fd9d2e3409b645b31f761 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 19 Jun 2020 14:00:43 -0700 Subject: [PATCH 30/37] forced unwinding of guests has had its assumptions challenged when forced unwinding was first envisioned, guests did not run at all from the point they faulted. this mean that the fault address would be a simple `guest_ctx.get_ip()` away. in the mean time, the Lucet signal handler learning how to be crossplatform broke this assumption: it now works by *overwriting* the guest's instruction pointer, swapping to the guest, and letting a function run. consequently, the guest instruction pointer is replaced and when a guest unwind is instigated after a guest faults, the return address before `initiate_unwind` (or `unwind_stub`, if present) will no longer be correct. libgcc_s will then fail to locate an FDE to describe the call frame above runtime-added unwind machinery, fail to unwind, and SIGABRT. the solution is quite simple: since the rip-accessing code is already handling a guest fault, we know the original faulting guest `rip` is preserved in the fault's `details`. insted of `guest_ctx.get_ip()`, get the address from `details.rip_addr`. --- lucet-runtime/lucet-runtime-internals/src/instance.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index 6353a6100..fb7f2f902 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -1277,7 +1277,7 @@ impl Instance { Ok(_) => lucet_bail!("resume with forced unwind returned normally"), } } - State::Faulted { context, .. } => { + State::Faulted { context, details, .. } => { #[unwind(allowed)] extern "C" fn initiate_unwind() { panic!(TerminationDetails::ForcedUnwind); @@ -1287,7 +1287,7 @@ impl Instance { context.as_ptr().save_to_context(&mut self.ctx); // get the instruction pointer when the fault was raised - let guest_addr = context.as_ptr().get_ip(); + let guest_addr = details.rip_addr; // if we should unwind by returning into the guest to cause a fault, do so with the redzone // available in case the guest was at or close to overflowing. From a0aa813505ecd085652d2a0bf41bbef5cd3388a5 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 19 Jun 2020 14:17:45 -0700 Subject: [PATCH 31/37] roll back the last of lucetc eh_frame emission changes. this should all be upstream --- lucetc/src/compiler.rs | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/lucetc/src/compiler.rs b/lucetc/src/compiler.rs index 0a97ae14e..3e76b4cde 100644 --- a/lucetc/src/compiler.rs +++ b/lucetc/src/compiler.rs @@ -14,18 +14,16 @@ use crate::traps::{translate_trapcode, trap_sym_for_func}; use byteorder::{LittleEndian, WriteBytesExt}; use cranelift_codegen::{ binemit, ir, - isa::unwind::UnwindInfo, isa::TargetIsa, settings::{self, Configurable}, Context as ClifContext, }; use cranelift_module::{ Backend as ClifBackend, DataContext as ClifDataContext, DataId, FuncId, FuncOrDataId, - Linkage as ClifLinkage, Module as ClifModule, ModuleError as ClifModuleError, + Linkage as ClifLinkage, Module as ClifModule, }; use cranelift_object::{ObjectBackend, ObjectBuilder}; use cranelift_wasm::{translate_module, FuncTranslator, ModuleTranslationState, WasmError}; -use gimli::write::{Address, EhFrame, Writer}; use lucet_module::bindings::Bindings; use lucet_module::{ ModuleData, ModuleFeatures, SerializedModule, VersionInfo, LUCET_MODULE_SYM, MODULE_DATA_SYM, @@ -273,23 +271,9 @@ impl<'a> Compiler<'a> { let mut func_translator = FuncTranslator::new(); let mut function_manifest_ctx = ClifDataContext::new(); let mut function_manifest_bytes = Cursor::new(Vec::new()); - // let mut eh_frame_table_bytes: Cursor> = Cursor::new(Vec::new()); let mut function_map: HashMap = HashMap::new(); - let mut frame_table = gimli::write::FrameTable::default(); - - let isa = Self::target_isa( - self.target.clone(), - self.opt_level, - &self.cpu_features, - self.canonicalize_nans, - )?; - let cie_id = frame_table.add_cie(match isa.create_systemv_cie() { - Some(cie) => cie, - None => panic!("uh oh"), - }); - - for (idx, (ref func, (code, code_offset))) in self.decls.function_bodies().enumerate() { + for (ref func, (code, code_offset)) in self.decls.function_bodies() { let mut func_info = FuncInfo::new(&self.decls, self.count_instructions); let mut clif_context = ClifContext::new(); clif_context.func.name = func.name.as_externalname(); From bb27ee76a5b56d7dadc98ca5b131dd90ef18ecf6 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 19 Jun 2020 14:31:54 -0700 Subject: [PATCH 32/37] rustfmt --- lucet-runtime/lucet-runtime-internals/src/instance.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/instance.rs b/lucet-runtime/lucet-runtime-internals/src/instance.rs index fb7f2f902..875ef8d46 100644 --- a/lucet-runtime/lucet-runtime-internals/src/instance.rs +++ b/lucet-runtime/lucet-runtime-internals/src/instance.rs @@ -1277,7 +1277,9 @@ impl Instance { Ok(_) => lucet_bail!("resume with forced unwind returned normally"), } } - State::Faulted { context, details, .. } => { + State::Faulted { + context, details, .. + } => { #[unwind(allowed)] extern "C" fn initiate_unwind() { panic!(TerminationDetails::ForcedUnwind); From 6510c4c220e14f274ad0d096b5eb8018114cc7c5 Mon Sep 17 00:00:00 2001 From: iximeow Date: Fri, 19 Jun 2020 15:15:47 -0700 Subject: [PATCH 33/37] tidy a little more --- Cargo.lock | 1 - lucet-runtime/lucet-runtime-internals/src/context/mod.rs | 5 +---- .../lucet-runtime-internals/src/instance/internals.rs | 0 lucetc/Cargo.toml | 3 +-- 4 files changed, 2 insertions(+), 7 deletions(-) delete mode 100644 lucet-runtime/lucet-runtime-internals/src/instance/internals.rs diff --git a/Cargo.lock b/Cargo.lock index 9ca8ca3b6..6898cd611 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1158,7 +1158,6 @@ dependencies = [ "cranelift-object", "cranelift-wasm", "env_logger", - "gimli 0.20.0", "human-size", "log", "lucet-module", diff --git a/lucet-runtime/lucet-runtime-internals/src/context/mod.rs b/lucet-runtime/lucet-runtime-internals/src/context/mod.rs index c7a0582ac..6afcae529 100644 --- a/lucet-runtime/lucet-runtime-internals/src/context/mod.rs +++ b/lucet-runtime/lucet-runtime-internals/src/context/mod.rs @@ -5,8 +5,7 @@ mod tests; use crate::instance::Instance; use crate::val::{val_to_reg, val_to_stack, RegVal, UntypedRetVal, Val}; -use nix; -use nix::sys::signal; + use std::arch::x86_64::{__m128, _mm_setzero_ps}; use std::ptr::NonNull; use std::{mem, ptr}; @@ -123,7 +122,6 @@ pub struct Context { // TODO ACF 2019-10-23: make Instance into a generic parameter? backstop_callback: *const unsafe extern "C" fn(*mut Instance), callback_data: *mut Instance, - sigset: signal::SigSet, } impl Context { @@ -137,7 +135,6 @@ impl Context { parent_ctx: ptr::null_mut(), backstop_callback: Context::default_backstop_callback as *const _, callback_data: ptr::null_mut(), - sigset: signal::SigSet::empty(), } } diff --git a/lucet-runtime/lucet-runtime-internals/src/instance/internals.rs b/lucet-runtime/lucet-runtime-internals/src/instance/internals.rs deleted file mode 100644 index e69de29bb..000000000 diff --git a/lucetc/Cargo.toml b/lucetc/Cargo.toml index e0e656378..59bd9551b 100644 --- a/lucetc/Cargo.toml +++ b/lucetc/Cargo.toml @@ -16,7 +16,7 @@ path = "lucetc/main.rs" [dependencies] anyhow = "1" bincode = "1.1.4" -cranelift-codegen = { path = "../wasmtime/cranelift/codegen", version = "0.64.0", features = ["unwind"] } +cranelift-codegen = { path = "../wasmtime/cranelift/codegen", version = "0.64.0" } cranelift-entity = { path = "../wasmtime/cranelift/entity", version = "0.64.0" } cranelift-native = { path = "../wasmtime/cranelift/native", version = "0.64.0" } cranelift-frontend = { path = "../wasmtime/cranelift/frontend", version = "0.64.0" } @@ -31,7 +31,6 @@ wasmparser = "0.57.0" clap="2.32" log = "0.4" env_logger = "0.6" -gimli = { version = "0.20.0", default-features = false, features = ["write"] } object = { version = "0.18.0", default-features = false, features = ["write"] } byteorder = "1.2" wabt = "0.9.2" From 486ae6a964e6888a2f8448a4207fd17a7561274b Mon Sep 17 00:00:00 2001 From: iximeow Date: Mon, 22 Jun 2020 19:00:08 -0700 Subject: [PATCH 34/37] .eh_frame generation can happen in lucet now --- Cargo.lock | 67 ++++++++++---------- lucet-module/Cargo.toml | 2 +- lucet-validate/Cargo.toml | 2 +- lucet-wasi/Cargo.toml | 2 +- lucet-wasi/generate/Cargo.toml | 4 +- lucet-wiggle/Cargo.toml | 2 +- lucet-wiggle/generate/Cargo.toml | 2 +- lucet-wiggle/macro/Cargo.toml | 2 +- lucetc/Cargo.toml | 17 +++--- lucetc/src/compiler.rs | 101 ++++++++++++++++++++++++++++++- lucetc/src/lib.rs | 1 + lucetc/src/unwind.rs | 46 ++++++++++++++ wasmtime | 2 +- 13 files changed, 196 insertions(+), 54 deletions(-) create mode 100644 lucetc/src/unwind.rs diff --git a/Cargo.lock b/Cargo.lock index 6898cd611..cdd4f30c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6,7 +6,7 @@ version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a49806b9dadc843c61e7c97e72490ad7f7220ae249012fbda9ad0609457c0543" dependencies = [ - "gimli 0.21.0", + "gimli", ] [[package]] @@ -306,21 +306,21 @@ dependencies = [ [[package]] name = "cranelift-bforest" -version = "0.64.0" +version = "0.65.0" dependencies = [ "cranelift-entity", ] [[package]] name = "cranelift-codegen" -version = "0.64.0" +version = "0.65.0" dependencies = [ "byteorder", "cranelift-bforest", "cranelift-codegen-meta", "cranelift-codegen-shared", "cranelift-entity", - "gimli 0.20.0", + "gimli", "log", "regalloc", "smallvec", @@ -330,7 +330,7 @@ dependencies = [ [[package]] name = "cranelift-codegen-meta" -version = "0.64.0" +version = "0.65.0" dependencies = [ "cranelift-codegen-shared", "cranelift-entity", @@ -338,15 +338,15 @@ dependencies = [ [[package]] name = "cranelift-codegen-shared" -version = "0.64.0" +version = "0.65.0" [[package]] name = "cranelift-entity" -version = "0.64.0" +version = "0.65.0" [[package]] name = "cranelift-frontend" -version = "0.64.0" +version = "0.65.0" dependencies = [ "cranelift-codegen", "log", @@ -356,7 +356,7 @@ dependencies = [ [[package]] name = "cranelift-module" -version = "0.64.0" +version = "0.65.0" dependencies = [ "anyhow", "cranelift-codegen", @@ -367,7 +367,7 @@ dependencies = [ [[package]] name = "cranelift-native" -version = "0.64.0" +version = "0.65.0" dependencies = [ "cranelift-codegen", "raw-cpuid 7.0.3", @@ -376,17 +376,18 @@ dependencies = [ [[package]] name = "cranelift-object" -version = "0.64.0" +version = "0.65.0" dependencies = [ + "anyhow", "cranelift-codegen", "cranelift-module", - "object 0.18.0", + "object 0.19.0", "target-lexicon", ] [[package]] name = "cranelift-wasm" -version = "0.64.0" +version = "0.65.0" dependencies = [ "cranelift-codegen", "cranelift-entity", @@ -680,20 +681,13 @@ dependencies = [ [[package]] name = "gimli" -version = "0.20.0" +version = "0.21.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81dd6190aad0f05ddbbf3245c54ed14ca4aa6dd32f22312b70d8f168c3e3e633" +checksum = "bcc8e0c9bce37868955864dbecd2b1ab2bdf967e6f28066d65aaac620444b65c" dependencies = [ - "byteorder", "indexmap", ] -[[package]] -name = "gimli" -version = "0.21.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bcc8e0c9bce37868955864dbecd2b1ab2bdf967e6f28066d65aaac620444b65c" - [[package]] name = "glob" version = "0.2.11" @@ -1158,6 +1152,7 @@ dependencies = [ "cranelift-object", "cranelift-wasm", "env_logger", + "gimli", "human-size", "log", "lucet-module", @@ -1165,7 +1160,7 @@ dependencies = [ "lucet-wiggle-generate", "memoffset", "minisign", - "object 0.18.0", + "object 0.19.0", "raw-cpuid 6.1.0", "serde", "serde_json", @@ -1308,9 +1303,7 @@ version = "0.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e5666bbb90bc4d1e5bdcb26c0afda1822d25928341e9384ab187a9b37ab69e36" dependencies = [ - "crc32fast", "flate2", - "indexmap", "target-lexicon", "wasmparser 0.51.4", ] @@ -1320,6 +1313,10 @@ name = "object" version = "0.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9cbca9424c482ee628fa549d9c812e2cd22f1180b9222c9200fdfa6eb31aecb2" +dependencies = [ + "crc32fast", + "indexmap", +] [[package]] name = "oorandom" @@ -1703,9 +1700,9 @@ checksum = "2439c63f3f6139d1b57529d16bc3b8bb855230c8efcc5d3a896c8bea7c3b1e84" [[package]] name = "regalloc" -version = "0.0.25" +version = "0.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cca5b48c9db66c5ba084e4660b4c0cfe8b551a96074bc04b7c11de86ad0bf1f9" +checksum = "7c03092d79e0fd610932d89ed53895a38c0dd3bcd317a0046e69940de32f1d95" dependencies = [ "log", "rustc-hash", @@ -2285,7 +2282,7 @@ checksum = "cccddf32554fecc6acb585f82a32a72e28b48f8c4c1883ddfeeeaa96f7d8e519" [[package]] name = "wasi-common" -version = "0.17.0" +version = "0.18.0" dependencies = [ "anyhow", "cfg-if", @@ -2396,7 +2393,7 @@ dependencies = [ [[package]] name = "wig" -version = "0.17.0" +version = "0.18.0" dependencies = [ "heck", "proc-macro2 1.0.13", @@ -2406,7 +2403,7 @@ dependencies = [ [[package]] name = "wiggle" -version = "0.17.0" +version = "0.18.0" dependencies = [ "thiserror", "tracing", @@ -2416,7 +2413,7 @@ dependencies = [ [[package]] name = "wiggle-generate" -version = "0.17.0" +version = "0.18.0" dependencies = [ "anyhow", "heck", @@ -2428,7 +2425,7 @@ dependencies = [ [[package]] name = "wiggle-macro" -version = "0.17.0" +version = "0.18.0" dependencies = [ "quote 1.0.6", "syn 1.0.22", @@ -2438,7 +2435,7 @@ dependencies = [ [[package]] name = "wiggle-test" -version = "0.17.0" +version = "0.18.0" dependencies = [ "proptest", "wiggle", @@ -2489,7 +2486,7 @@ checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" [[package]] name = "winx" -version = "0.17.0" +version = "0.18.0" dependencies = [ "bitflags 1.2.1", "cvt", @@ -2514,7 +2511,7 @@ checksum = "da90eac47bf1d7871b75004b9b631d107df15f37669383b23f0b5297bc7516b6" [[package]] name = "yanix" -version = "0.17.0" +version = "0.18.0" dependencies = [ "bitflags 1.2.1", "cfg-if", diff --git a/lucet-module/Cargo.toml b/lucet-module/Cargo.toml index 4c6c719f2..33371a06d 100644 --- a/lucet-module/Cargo.toml +++ b/lucet-module/Cargo.toml @@ -11,7 +11,7 @@ edition = "2018" [dependencies] anyhow = "1.0" -cranelift-entity = { path = "../wasmtime/cranelift/entity", version = "0.64.0" } +cranelift-entity = { path = "../wasmtime/cranelift/entity", version = "0.65.0" } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" bincode = "1.1.4" diff --git a/lucet-validate/Cargo.toml b/lucet-validate/Cargo.toml index e6f7c06ff..502204f64 100644 --- a/lucet-validate/Cargo.toml +++ b/lucet-validate/Cargo.toml @@ -19,7 +19,7 @@ path = "src/main.rs" [dependencies] clap = "2" witx = { path = "../wasmtime/crates/wasi-common/WASI/tools/witx", version = "0.8.5" } -cranelift-entity = { path = "../wasmtime/cranelift/entity", version = "0.64.0" } +cranelift-entity = { path = "../wasmtime/cranelift/entity", version = "0.65.0" } thiserror = "1.0.4" wasmparser = "0.52.0" diff --git a/lucet-wasi/Cargo.toml b/lucet-wasi/Cargo.toml index dd80248c1..74a39791d 100644 --- a/lucet-wasi/Cargo.toml +++ b/lucet-wasi/Cargo.toml @@ -22,7 +22,7 @@ lucet-wiggle = { path = "../lucet-wiggle", version = "=0.7.0-dev" } libc = "0.2.65" nix = "0.17" rand = "0.6" -wasi-common = { path = "../wasmtime/crates/wasi-common", version = "0.17.0", features = ["wiggle_metadata"] } +wasi-common = { path = "../wasmtime/crates/wasi-common", version = "0.18.0", features = ["wiggle_metadata"] } [dev-dependencies] lucet-wasi-sdk = { path = "../lucet-wasi-sdk" } diff --git a/lucet-wasi/generate/Cargo.toml b/lucet-wasi/generate/Cargo.toml index ba4889cab..e36394153 100644 --- a/lucet-wasi/generate/Cargo.toml +++ b/lucet-wasi/generate/Cargo.toml @@ -13,8 +13,8 @@ proc-macro = true [dependencies] lucet-wiggle = { path = "../../lucet-wiggle", version = "0.7.0-dev" } -wasi-common = { path = "../../wasmtime/crates/wasi-common", version = "0.17.0", features = ["wiggle_metadata"] } -wiggle-generate = { path = "../../wasmtime/crates/wiggle/generate", version = "0.17.0" } +wasi-common = { path = "../../wasmtime/crates/wasi-common", version = "0.18.0", features = ["wiggle_metadata"] } +wiggle-generate = { path = "../../wasmtime/crates/wiggle/generate", version = "0.18.0" } syn = { version = "1.0", features = ["full"] } quote = "1.0" proc-macro2 = "1.0" diff --git a/lucet-wiggle/Cargo.toml b/lucet-wiggle/Cargo.toml index 911f2fb14..38a9566a1 100644 --- a/lucet-wiggle/Cargo.toml +++ b/lucet-wiggle/Cargo.toml @@ -14,7 +14,7 @@ edition = "2018" lucet-wiggle-macro = { path = "./macro", version = "0.7.0-dev" } lucet-wiggle-generate = { path = "./generate", version = "0.7.0-dev" } lucet-runtime = { path = "../lucet-runtime", version = "0.7.0-dev" } -wiggle = { path = "../wasmtime/crates/wiggle", version = "0.17.0" } +wiggle = { path = "../wasmtime/crates/wiggle", version = "0.18.0" } [dev-dependencies] wiggle-test = { path = "../wasmtime/crates/wiggle/test-helpers" } diff --git a/lucet-wiggle/generate/Cargo.toml b/lucet-wiggle/generate/Cargo.toml index 2add756f1..fb6e8ed02 100644 --- a/lucet-wiggle/generate/Cargo.toml +++ b/lucet-wiggle/generate/Cargo.toml @@ -9,7 +9,7 @@ authors = ["Lucet team "] edition = "2018" [dependencies] -wiggle-generate = { path = "../../wasmtime/crates/wiggle/generate", version = "0.17.0" } +wiggle-generate = { path = "../../wasmtime/crates/wiggle/generate", version = "0.18.0" } lucet-module = { path = "../../lucet-module", version = "0.7.0-dev" } witx = { path = "../../wasmtime/crates/wasi-common/WASI/tools/witx", version = "0.8.4" } quote = "1.0" diff --git a/lucet-wiggle/macro/Cargo.toml b/lucet-wiggle/macro/Cargo.toml index dcfe9e778..b26e8377e 100644 --- a/lucet-wiggle/macro/Cargo.toml +++ b/lucet-wiggle/macro/Cargo.toml @@ -13,7 +13,7 @@ proc-macro = true [dependencies] lucet-wiggle-generate = { path = "../generate", version = "0.7.0-dev" } -wiggle-generate = { path = "../../wasmtime/crates/wiggle/generate", version = "0.17.0" } +wiggle-generate = { path = "../../wasmtime/crates/wiggle/generate", version = "0.18.0" } witx = { path = "../../wasmtime/crates/wasi-common/WASI/tools/witx", version = "0.8.4" } syn = { version = "1.0", features = ["full"] } quote = "1.0" diff --git a/lucetc/Cargo.toml b/lucetc/Cargo.toml index 59bd9551b..6d3f16518 100644 --- a/lucetc/Cargo.toml +++ b/lucetc/Cargo.toml @@ -16,13 +16,13 @@ path = "lucetc/main.rs" [dependencies] anyhow = "1" bincode = "1.1.4" -cranelift-codegen = { path = "../wasmtime/cranelift/codegen", version = "0.64.0" } -cranelift-entity = { path = "../wasmtime/cranelift/entity", version = "0.64.0" } -cranelift-native = { path = "../wasmtime/cranelift/native", version = "0.64.0" } -cranelift-frontend = { path = "../wasmtime/cranelift/frontend", version = "0.64.0" } -cranelift-module = { path = "../wasmtime/cranelift/module", version = "0.64.0" } -cranelift-object = { path = "../wasmtime/cranelift/object", version = "0.64.0" } -cranelift-wasm = { path = "../wasmtime/cranelift/wasm", version = "0.64.0" } +cranelift-codegen = { path = "../wasmtime/cranelift/codegen", version = "0.65.0", features = ["unwind"] } +cranelift-entity = { path = "../wasmtime/cranelift/entity", version = "0.65.0" } +cranelift-native = { path = "../wasmtime/cranelift/native", version = "0.65.0" } +cranelift-frontend = { path = "../wasmtime/cranelift/frontend", version = "0.65.0" } +cranelift-module = { path = "../wasmtime/cranelift/module", version = "0.65.0" } +cranelift-object = { path = "../wasmtime/cranelift/object", version = "0.65.0" } +cranelift-wasm = { path = "../wasmtime/cranelift/wasm", version = "0.65.0" } target-lexicon = "0.10" lucet-module = { path = "../lucet-module", version = "=0.7.0-dev" } lucet-validate = { path = "../lucet-validate", version = "=0.7.0-dev" } @@ -31,7 +31,8 @@ wasmparser = "0.57.0" clap="2.32" log = "0.4" env_logger = "0.6" -object = { version = "0.18.0", default-features = false, features = ["write"] } +gimli = { version = "0.21.0", default-features = false, features = ["write"] } +object = { version = "0.19.0", default-features = false, features = ["write"] } byteorder = "1.2" wabt = "0.9.2" tempfile = "3.0" diff --git a/lucetc/src/compiler.rs b/lucetc/src/compiler.rs index 3e76b4cde..b1d59c086 100644 --- a/lucetc/src/compiler.rs +++ b/lucetc/src/compiler.rs @@ -11,7 +11,10 @@ use crate::runtime::Runtime; use crate::stack_probe; use crate::table::write_table_data; use crate::traps::{translate_trapcode, trap_sym_for_func}; +use crate::unwind::EhFrameSink; use byteorder::{LittleEndian, WriteBytesExt}; +use cranelift_codegen::isa::unwind::UnwindInfo; +use cranelift_codegen::CodegenError; use cranelift_codegen::{ binemit, ir, isa::TargetIsa, @@ -20,10 +23,12 @@ use cranelift_codegen::{ }; use cranelift_module::{ Backend as ClifBackend, DataContext as ClifDataContext, DataId, FuncId, FuncOrDataId, - Linkage as ClifLinkage, Module as ClifModule, + Linkage as ClifLinkage, Module as ClifModule, ModuleError, }; use cranelift_object::{ObjectBackend, ObjectBuilder}; use cranelift_wasm::{translate_module, FuncTranslator, ModuleTranslationState, WasmError}; +use gimli::write::EhFrame; +use gimli::write::{Address, FrameTable}; use lucet_module::bindings::Bindings; use lucet_module::{ ModuleData, ModuleFeatures, SerializedModule, VersionInfo, LUCET_MODULE_SYM, MODULE_DATA_SYM, @@ -228,7 +233,7 @@ impl<'a> Compiler<'a> { _ => (cranelift_module::default_libcall_names())(libcall), }); - let mut builder = ObjectBuilder::new(isa, "lucet_guest".to_owned(), libcalls); + let mut builder = ObjectBuilder::new(isa, "lucet_guest".to_owned(), libcalls)?; builder.function_alignment(16); let mut clif_module: ClifModule = ClifModule::new(builder); @@ -268,7 +273,20 @@ impl<'a> Compiler<'a> { } pub fn object_file(mut self) -> Result { + let isa = Self::target_isa( + self.target.clone(), + self.opt_level, + &self.cpu_features, + self.canonicalize_nans, + )?; + let target_binary_format = isa.triple().binary_format; + let mut frame_table = FrameTable::default(); let mut func_translator = FuncTranslator::new(); + let cie_id = frame_table.add_cie( + isa.as_ref() + .create_systemv_cie() + .expect("creating a SystemV CIE does not fail"), + ); let mut function_manifest_ctx = ClifDataContext::new(); let mut function_manifest_bytes = Cursor::new(Vec::new()); let mut function_map: HashMap = HashMap::new(); @@ -302,6 +320,48 @@ impl<'a> Compiler<'a> { source, })?; + let unwind_info = clif_context + .create_unwind_info(isa.as_ref()) + .map_err(|err| Error::ClifModuleError(ModuleError::Compilation(err)))?; + if let Some(unwind_info) = unwind_info { + match target_binary_format { + target_lexicon::BinaryFormat::Elf | target_lexicon::BinaryFormat::Macho => { + if let UnwindInfo::SystemV(info) = unwind_info { + frame_table.add_fde( + cie_id, + info.to_fde(Address::Symbol { + symbol: func_id.as_u32() as usize, + addend: 0, + }), + ); + } else { + return Err(Error::ClifModuleError(ModuleError::Compilation( + CodegenError::Unsupported( + "Non-SystemV unwind information in ELF/MachO output." + .to_string(), + ), + ))); + } + } + objfmt => { + return Err(Error::ClifModuleError(ModuleError::Compilation( + CodegenError::Unsupported(format!( + "lucetc does not yet support consolidating \ + {:?} unwind information.", + objfmt + )), + ))); + } + } + } else { + return Err(Error::ClifModuleError(ModuleError::Compilation( + CodegenError::Unsupported(format!( + "lucetc does not yet support consolidated unwind \ + information where some functions have no unwind information." + )), + ))); + } + let size = compiled.size; let trap_data_id = traps.write(&mut self.clif_module, func.name.symbol())?; @@ -438,6 +498,43 @@ impl<'a> Compiler<'a> { self.clif_module .define_data(native_data_id, &native_data_ctx)?; + let mut eh_frame_ctx = cranelift_module::DataContext::new(); + + let mut eh_frame = EhFrame(EhFrameSink { + data: Vec::new(), + data_context: &mut eh_frame_ctx, + }); + frame_table + .write_eh_frame(&mut eh_frame) + .map_err(|e| cranelift_module::ModuleError::Backend(anyhow::anyhow!(e)))?; + let eh_frame_bytes = eh_frame.0.data; + + let eh_section_name = match target_binary_format { + target_lexicon::BinaryFormat::Elf => ".eh_frame", + target_lexicon::BinaryFormat::Macho => "__eh_frame", + objfmt => { + return Err(Error::ClifModuleError(ModuleError::Compilation( + CodegenError::Unsupported(format!( + "lucetc does not yet support consolidating \ + {:?} unwind information.", + objfmt + )), + ))); + } + }; + + eh_frame_ctx.set_segment_section("", eh_section_name); + eh_frame_ctx.define(eh_frame_bytes.into_boxed_slice()); + + let dataid = self.clif_module.declare_data( + eh_section_name, + ClifLinkage::Local, + false, + false, + None, + )?; + self.clif_module.define_data(dataid, &eh_frame_ctx)?; + let obj = ObjectFile::new(self.clif_module.finish())?; Ok(obj) diff --git a/lucetc/src/lib.rs b/lucetc/src/lib.rs index d79ded350..9551a2e90 100644 --- a/lucetc/src/lib.rs +++ b/lucetc/src/lib.rs @@ -17,6 +17,7 @@ mod stack_probe; mod table; mod traps; mod types; +mod unwind; use crate::load::read_bytes; pub use crate::{ diff --git a/lucetc/src/unwind.rs b/lucetc/src/unwind.rs new file mode 100644 index 000000000..f05f9d238 --- /dev/null +++ b/lucetc/src/unwind.rs @@ -0,0 +1,46 @@ +use cranelift_module::{DataContext, FuncId}; +use gimli::write::{Address, Error, Result, Writer}; + +pub(crate) struct EhFrameSink<'a> { + pub data: Vec, + pub data_context: &'a mut DataContext, +} + +impl<'a> Writer for EhFrameSink<'a> { + type Endian = gimli::LittleEndian; + + fn endian(&self) -> Self::Endian { + gimli::LittleEndian + } + fn len(&self) -> usize { + self.data.len() + } + fn write(&mut self, bytes: &[u8]) -> Result<()> { + self.data.extend_from_slice(bytes); + Ok(()) + } + fn write_at(&mut self, offset: usize, bytes: &[u8]) -> Result<()> { + if offset + bytes.len() > self.data.len() { + return Err(Error::LengthOutOfBounds); + } + self.data[offset..][..bytes.len()].copy_from_slice(bytes); + Ok(()) + } + + fn write_address(&mut self, address: Address, size: u8) -> Result<()> { + match address { + Address::Constant(val) => self.write_udata(val, size), + Address::Symbol { symbol, addend } => { + assert_eq!(addend, 0); + + let name = FuncId::from_u32(symbol as u32).into(); + let funcref = self.data_context.import_function(name); + let offset = self.data.len(); + self.data_context + .write_function_addr(offset as u32, funcref); + + self.write_udata(0, size) + } + } + } +} diff --git a/wasmtime b/wasmtime index 3de418630..60ac091af 160000 --- a/wasmtime +++ b/wasmtime @@ -1 +1 @@ -Subproject commit 3de418630a263ca214931d69f796879be50d4f72 +Subproject commit 60ac091afe8df48f843ce8ad06a2bd1a626f6183 From 4d68f0bf09aadb20f411dcb285d16c22595a4353 Mon Sep 17 00:00:00 2001 From: iximeow Date: Mon, 22 Jun 2020 19:21:17 -0700 Subject: [PATCH 35/37] fix macos sysdeps --- .../src/sysdeps/macos.rs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/lucet-runtime/lucet-runtime-internals/src/sysdeps/macos.rs b/lucet-runtime/lucet-runtime-internals/src/sysdeps/macos.rs index ebcae4cdf..b03701f39 100644 --- a/lucet-runtime/lucet-runtime-internals/src/sysdeps/macos.rs +++ b/lucet-runtime/lucet-runtime-internals/src/sysdeps/macos.rs @@ -1,4 +1,6 @@ +use crate::context::Context; use libc::{c_int, c_short, c_void, sigset_t, size_t}; +use std::arch::x86_64::{__m128, _mm_loadu_ps}; #[derive(Clone, Copy, Debug)] #[repr(C)] pub struct sigaltstack { @@ -148,6 +150,41 @@ impl UContextPtr { let mcontext: &mut mcontext64 = unsafe { &mut (*self.0).uc_mcontext.as_mut().unwrap() }; mcontext.ss.rdi = new_rdi; } + + pub fn save_to_context(&self, ctx: &mut Context) { + let mcontext: &mut mcontext64 = unsafe { (*self.0).uc_mcontext.as_mut().unwrap() }; + ctx.gpr.rbx = mcontext.ss.rbx; + ctx.gpr.rsp = mcontext.ss.rsp; + ctx.gpr.rbp = mcontext.ss.rbp; + ctx.gpr.rdi = mcontext.ss.rdi; + ctx.gpr.r12 = mcontext.ss.r12; + ctx.gpr.r13 = mcontext.ss.r13; + ctx.gpr.r14 = mcontext.ss.r14; + ctx.gpr.r15 = mcontext.ss.r15; + + let fpregs = &mcontext.fs; + let xmms = [ + fpregs.fpu_xmm0, + fpregs.fpu_xmm1, + fpregs.fpu_xmm2, + fpregs.fpu_xmm3, + fpregs.fpu_xmm4, + fpregs.fpu_xmm5, + fpregs.fpu_xmm6, + fpregs.fpu_xmm7, + ] + .iter() + .map(|reg| unsafe { _mm_loadu_ps(reg.0.as_ptr() as *const u32 as *const _) }) + .collect::>(); + ctx.fpr.xmm0 = xmms[0]; + ctx.fpr.xmm1 = xmms[1]; + ctx.fpr.xmm2 = xmms[2]; + ctx.fpr.xmm3 = xmms[3]; + ctx.fpr.xmm4 = xmms[4]; + ctx.fpr.xmm5 = xmms[5]; + ctx.fpr.xmm6 = xmms[6]; + ctx.fpr.xmm7 = xmms[7]; + } } #[derive(Clone, Copy)] From caa11523fcbdb968a84f1748a53adcc72c4c4a60 Mon Sep 17 00:00:00 2001 From: iximeow Date: Mon, 22 Jun 2020 20:35:14 -0700 Subject: [PATCH 36/37] get mutex-poisoning tests well-behaved for multiple region impls --- .../src/region/uffd.rs | 3 +- lucet-runtime/lucet-runtime-tests/src/host.rs | 112 +++++++++++------- 2 files changed, 70 insertions(+), 45 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region/uffd.rs b/lucet-runtime/lucet-runtime-internals/src/region/uffd.rs index 779aae2b2..813b09edd 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/uffd.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/uffd.rs @@ -320,7 +320,6 @@ impl RegionInternal for UffdRegion { } fn enable_stack_redzone(&self, _slot: &Slot) { - // no-op } fn disable_stack_redzone(&self, _slot: &Slot) { @@ -510,7 +509,7 @@ impl UffdRegion { sigstack: sigstack as *mut c_void, limits: region.limits.clone(), region: Arc::downgrade(region) as Weak, - redzone_stack_enabled: true, + redzone_stack_enabled: false, }) } } diff --git a/lucet-runtime/lucet-runtime-tests/src/host.rs b/lucet-runtime/lucet-runtime-tests/src/host.rs index 71b263df8..d5d12fef2 100644 --- a/lucet-runtime/lucet-runtime-tests/src/host.rs +++ b/lucet-runtime/lucet-runtime-tests/src/host.rs @@ -28,15 +28,16 @@ macro_rules! host_tests { const ERROR_MESSAGE: &'static str = "hostcall_test_func_hostcall_error"; lazy_static! { - static ref HOSTCALL_MUTEX: Mutex<()> = Mutex::new(()); static ref NESTED_OUTER: Mutex<()> = Mutex::new(()); static ref NESTED_INNER: Mutex<()> = Mutex::new(()); static ref NESTED_REGS_OUTER: Mutex<()> = Mutex::new(()); static ref NESTED_REGS_INNER: Mutex<()> = Mutex::new(()); - static ref BAD_ACCESS_UNWIND: Mutex<()> = Mutex::new(()); - static ref STACK_OVERFLOW_UNWIND: Mutex<()> = Mutex::new(()); } + static mut HOSTCALL_MUTEX: Option> = None; + static mut BAD_ACCESS_UNWIND: Option> = None; + static mut STACK_OVERFLOW_UNWIND: Option> = None; + #[allow(unreachable_code)] #[inline] unsafe fn unwind_inner(vmctx: &Vmctx, mutex: &Mutex<()>) { @@ -84,7 +85,7 @@ macro_rules! host_tests { #[allow(unreachable_code)] #[no_mangle] pub fn hostcall_test_func_hostcall_error_unwind(vmctx: &Vmctx) { - let lock = HOSTCALL_MUTEX.lock().unwrap(); + let lock = unsafe { HOSTCALL_MUTEX.as_ref().unwrap() }.lock().unwrap(); unsafe { lucet_hostcall_terminate!(ERROR_MESSAGE); } @@ -209,7 +210,7 @@ macro_rules! host_tests { vmctx: &Vmctx, cb_idx: u32, ) -> () { - let lock = STACK_OVERFLOW_UNWIND.lock().unwrap(); + let lock = unsafe { STACK_OVERFLOW_UNWIND.as_ref().unwrap() }.lock().unwrap(); let func = vmctx .get_func_from_idx(0, cb_idx) @@ -231,7 +232,7 @@ macro_rules! host_tests { vmctx: &Vmctx, cb_idx: u32, ) -> () { - let lock = BAD_ACCESS_UNWIND.lock().unwrap(); + let lock = unsafe { BAD_ACCESS_UNWIND.as_ref().unwrap() }.lock().unwrap(); let func = vmctx .get_func_from_idx(0, cb_idx) @@ -384,7 +385,7 @@ macro_rules! host_tests { }; use std::sync::{Arc, Mutex}; use $crate::build::test_module_c; - use $crate::helpers::{FunctionPointer, HeapSpec, MockExportBuilder, MockModuleBuilder}; + use $crate::helpers::{FunctionPointer, HeapSpec, MockExportBuilder, MockModuleBuilder, test_ex}; use $TestRegion as TestRegion; #[test] @@ -460,29 +461,42 @@ macro_rules! host_tests { #[test] fn run_hostcall_error_unwind() { - let module = - test_module_c("host", "hostcall_error_unwind.c").expect("build and load module"); - let region = ::create(1, &Limits::default()).expect("region can be created"); - - let mut inst = region - .new_instance(module) - .expect("instance can be created"); + test_ex(|| { + // Since `hostcall_test_func_hostcall_error_unwind` is reused in two + // different modules, meaning two different tests, we need to reset the + // mutex it will (hopefully) poison before running this test. + // + // The contention for this global mutex is why this test must be `test_ex`. + unsafe { + super::HOSTCALL_MUTEX = Some(Mutex::new(())); + } - match inst.run("main", &[0u32.into(), 0u32.into()]) { - Err(Error::RuntimeTerminated(term)) => { - assert_eq!( - *term - .provided_details() - .expect("user provided termination reason") - .downcast_ref::<&'static str>() - .expect("error was static str"), - super::ERROR_MESSAGE - ); + let module = + test_module_c("host", "hostcall_error_unwind.c").expect("build and load module"); + let region = ::create(1, &Limits::default()).expect("region can be created"); + + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + + match inst.run("main", &[0u32.into(), 0u32.into()]) { + Err(Error::RuntimeTerminated(term)) => { + assert_eq!( + *term + .provided_details() + .expect("user provided termination reason") + .downcast_ref::<&'static str>() + .expect("error was static str"), + super::ERROR_MESSAGE + ); + } + res => panic!("unexpected result: {:?}", res), } - res => panic!("unexpected result: {:?}", res), - } - assert!(super::HOSTCALL_MUTEX.is_poisoned()); + unsafe { + assert!(super::HOSTCALL_MUTEX.as_ref().unwrap().is_poisoned()); + } + }) } /// Check that if two segments of hostcall stack are present when terminating, that they @@ -580,28 +594,40 @@ macro_rules! host_tests { /// Ensures that hostcall stack frames get unwound when a fault occurs in guest code. #[test] fn bad_access_unwind() { - let module = test_module_c("host", "fault_unwind.c").expect("build and load module"); - let region = ::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - inst.run("bad_access", &[]).unwrap_err(); - inst.reset().unwrap(); - assert!(super::BAD_ACCESS_UNWIND.is_poisoned()); + test_ex(|| { + unsafe { + super::BAD_ACCESS_UNWIND = Some(Mutex::new(())); + } + let module = test_module_c("host", "fault_unwind.c").expect("build and load module"); + let region = ::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + inst.run("bad_access", &[]).unwrap_err(); + inst.reset().unwrap(); + unsafe { + assert!(unsafe { super::BAD_ACCESS_UNWIND.as_ref().unwrap() }.is_poisoned()); + } + }) } /// Ensures that hostcall stack frames get unwound even when a stack overflow occurs in /// guest code. #[test] fn stack_overflow_unwind() { - let module = test_module_c("host", "fault_unwind.c").expect("build and load module"); - let region = ::create(1, &Limits::default()).expect("region can be created"); - let mut inst = region - .new_instance(module) - .expect("instance can be created"); - inst.run("stack_overflow", &[]).unwrap_err(); - inst.reset().unwrap(); - assert!(super::STACK_OVERFLOW_UNWIND.is_poisoned()); + test_ex(|| { + unsafe { + super::STACK_OVERFLOW_UNWIND = Some(Mutex::new(())); + } + let module = test_module_c("host", "fault_unwind.c").expect("build and load module"); + let region = ::create(1, &Limits::default()).expect("region can be created"); + let mut inst = region + .new_instance(module) + .expect("instance can be created"); + inst.run("stack_overflow", &[]).unwrap_err(); + inst.reset().unwrap(); + assert!(unsafe { super::STACK_OVERFLOW_UNWIND.as_ref().unwrap() }.is_poisoned()); + }) } #[test] From d10bd0a281070bf750e1259d2911e6e9797dba89 Mon Sep 17 00:00:00 2001 From: iximeow Date: Thu, 25 Jun 2020 15:41:10 -0700 Subject: [PATCH 37/37] implement stack redzone for uffd --- .../src/region/uffd.rs | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/lucet-runtime/lucet-runtime-internals/src/region/uffd.rs b/lucet-runtime/lucet-runtime-internals/src/region/uffd.rs index 813b09edd..f69e5907d 100644 --- a/lucet-runtime/lucet-runtime-internals/src/region/uffd.rs +++ b/lucet-runtime/lucet-runtime-internals/src/region/uffd.rs @@ -319,11 +319,26 @@ impl RegionInternal for UffdRegion { self } - fn enable_stack_redzone(&self, _slot: &Slot) { + fn enable_stack_redzone(&self, slot: &Slot) { + // The stack grows downward, `slot.stack` is the lowest address of the stack, meaning the + // guard page is before `slot.stack`. + let stack_guard_start = slot.stack as usize - host_page_size(); + unsafe { + self.uffd + .zeropage(stack_guard_start as *mut _, host_page_size(), true) + .expect("uffd.zeropage succeeds"); + } } - fn disable_stack_redzone(&self, _slot: &Slot) { - // no-op + fn disable_stack_redzone(&self, slot: &Slot) { + let stack_guard_start = slot.stack as usize - host_page_size(); + unsafe { + madvise( + stack_guard_start as *mut _, + host_page_size(), + MmapAdvise::MADV_DONTNEED, + ).expect("disabling stack redzone succeeds"); + } } }