Stack unwinding improvements

benchmarks/lucet-benchmarks/src/lib.rs

@@ -1,3 +1,5 @@
+#![feature(unwind_attributes)]
+
 mod compile;
 mod context;
 mod modules;

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.15-nightly
 
 if ! rustfmt --version | grep -q "rustfmt $RUSTFMT_VERSION"; then
 	echo "indent requires rustfmt $RUSTFMT_VERSION"

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"

lucet-runtime/lucet-runtime-internals/src/alloc/mod.rs

@@ -58,6 +58,8 @@ pub struct Slot {
     pub limits: Limits,
 
     pub region: Weak<dyn RegionInternal>,
+
+    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.

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,
     }
 

lucet-runtime/lucet-runtime-internals/src/context/context_asm.S

@@ -55,13 +55,122 @@ _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
+// 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
+
+    // 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 */ \
+                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 */ \
+                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) */ \
+                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 */ \
+                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) */ \
+                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) */ \
+                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) */ \
+                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) */ \
+                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) */ \
+                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) */ \
+                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) */ \
+                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) */
+
+    .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
+    // 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 +203,7 @@ no_backstop_callback:
 #else
     jmp lucet_context_swap
 #endif
+.cfi_endproc
 #ifdef __ELF__
 .size lucet_context_backstop,.-lucet_context_backstop
 #endif

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};
@@ -25,15 +26,15 @@ use thiserror::Error;
 /// `u64`, this should be fine?
 #[repr(C)]
 pub(crate) struct GpRegs {
-    pub(crate) rbx: u64,
-    pub(crate) rsp: u64,
-    rbp: u64,
-    pub(crate) rdi: u64,
-    r12: u64,
-    r13: u64,
-    r14: u64,
-    r15: u64,
-    pub(crate) rsi: u64,
+    pub rbx: u64,
+    pub rsp: 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 +66,15 @@ impl GpRegs {
 /// <https://doc.rust-lang.org/nomicon/other-reprs.html#reprpacked>. 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,13 +116,14 @@ 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,
     // 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 {
@@ -135,6 +137,7 @@ impl Context {
             parent_ctx: ptr::null_mut(),
             backstop_callback: Context::default_backstop_callback as *const _,
             callback_data: ptr::null_mut(),
+            sigset: signal::SigSet::empty(),
         }
     }
 
@@ -729,4 +732,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();
 }