From 635e159d1f2b71c4f667c18d07d1660c5703d262 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 9 Jun 2020 14:19:45 -0700 Subject: [PATCH] Add the `Frame::sp` method to get the frame's stack pointer (#341) * Add the `Frame::sp` method to get the frame's stack pointer * rustfmt --- src/backtrace/dbghelp.rs | 11 +++++ src/backtrace/libunwind.rs | 96 ++++++++++++++++++++++++++++---------- src/backtrace/mod.rs | 8 ++++ src/backtrace/noop.rs | 4 ++ src/capture.rs | 19 ++++---- tests/smoke.rs | 81 ++++++++++++++++++++++++++++++++ 6 files changed, 185 insertions(+), 34 deletions(-) diff --git a/src/backtrace/dbghelp.rs b/src/backtrace/dbghelp.rs index a6b1cde36..cbfa4ec12 100644 --- a/src/backtrace/dbghelp.rs +++ b/src/backtrace/dbghelp.rs @@ -42,6 +42,10 @@ impl Frame { self.addr_pc().Offset as *mut _ } + pub fn sp(&self) -> *mut c_void { + self.addr_stack().Offset as *mut _ + } + pub fn symbol_address(&self) -> *mut c_void { self.ip() } @@ -67,6 +71,13 @@ impl Frame { } } + fn addr_stack(&self) -> &ADDRESS64 { + match self { + Frame::New(new) => &new.AddrStack, + Frame::Old(old) => &old.AddrStack, + } + } + fn addr_stack_mut(&mut self) -> &mut ADDRESS64 { match self { Frame::New(new) => &mut new.AddrStack, diff --git a/src/backtrace/libunwind.rs b/src/backtrace/libunwind.rs index cb2c0fd29..c5ddc76ed 100644 --- a/src/backtrace/libunwind.rs +++ b/src/backtrace/libunwind.rs @@ -31,6 +31,7 @@ pub enum Frame { Raw(*mut uw::_Unwind_Context), Cloned { ip: *mut c_void, + sp: *mut c_void, symbol_address: *mut c_void, }, } @@ -51,6 +52,13 @@ impl Frame { unsafe { uw::_Unwind_GetIP(ctx) as *mut c_void } } + pub fn sp(&self) -> *mut c_void { + match *self { + Frame::Raw(ctx) => unsafe { uw::get_sp(ctx) as *mut c_void }, + Frame::Cloned { sp, .. } => sp, + } + } + pub fn symbol_address(&self) -> *mut c_void { if let Frame::Cloned { symbol_address, .. } = *self { return symbol_address; @@ -76,6 +84,7 @@ impl Clone for Frame { fn clone(&self) -> Frame { Frame::Cloned { ip: self.ip(), + sp: self.sp(), symbol_address: self.symbol_address(), } } @@ -155,22 +164,46 @@ mod uw { pub fn _Unwind_GetIP(ctx: *mut _Unwind_Context) -> libc::uintptr_t; #[cfg(all( - not(target_os = "android"), + not(all(target_os = "android", target_arch = "arm")), not(all(target_os = "freebsd", target_arch = "arm")), not(all(target_os = "linux", target_arch = "arm")) ))] pub fn _Unwind_FindEnclosingFunction(pc: *mut c_void) -> *mut c_void; + + #[cfg(all( + not(all(target_os = "android", target_arch = "arm")), + not(all(target_os = "freebsd", target_arch = "arm")), + not(all(target_os = "linux", target_arch = "arm")) + ))] + // This function is a misnomer: rather than getting this frame's + // Canonical Frame Address (aka the caller frame's SP) it + // returns this frame's SP. + // + // https://github.com/libunwind/libunwind/blob/d32956507cf29d9b1a98a8bce53c78623908f4fe/src/unwind/GetCFA.c#L28-L35 + #[link_name = "_Unwind_GetCFA"] + pub fn get_sp(ctx: *mut _Unwind_Context) -> libc::uintptr_t; } - // On android, the function _Unwind_GetIP is a macro, and this is the - // expansion of the macro. This is all copy/pasted directly from the - // header file with the definition of _Unwind_GetIP. + // On android and arm, the function `_Unwind_GetIP` and a bunch of others + // are macros, so we define functions containing the expansion of the + // macros. + // + // TODO: link to the header file that defines these macros, if you can find + // it. (I, fitzgen, cannot find the header file that some of these macro + // expansions were originally borrowed from.) + #[cfg(any( + all(target_os = "android", target_arch = "arm"), + all(target_os = "freebsd", target_arch = "arm"), + all(target_os = "linux", target_arch = "arm") + ))] + pub use self::arm::*; #[cfg(any( all(target_os = "android", target_arch = "arm"), all(target_os = "freebsd", target_arch = "arm"), all(target_os = "linux", target_arch = "arm") ))] - pub unsafe fn _Unwind_GetIP(ctx: *mut _Unwind_Context) -> libc::uintptr_t { + mod arm { + pub use super::*; #[repr(C)] enum _Unwind_VRS_Result { _UVRSR_OK = 0, @@ -206,26 +239,39 @@ mod uw { ) -> _Unwind_VRS_Result; } - let mut val: _Unwind_Word = 0; - let ptr = &mut val as *mut _Unwind_Word; - let _ = _Unwind_VRS_Get( - ctx, - _Unwind_VRS_RegClass::_UVRSC_CORE, - 15, - _Unwind_VRS_DataRepresentation::_UVRSD_UINT32, - ptr as *mut c_void, - ); - (val & !1) as libc::uintptr_t - } + pub unsafe fn _Unwind_GetIP(ctx: *mut _Unwind_Context) -> libc::uintptr_t { + let mut val: _Unwind_Word = 0; + let ptr = &mut val as *mut _Unwind_Word; + let _ = _Unwind_VRS_Get( + ctx, + _Unwind_VRS_RegClass::_UVRSC_CORE, + 15, + _Unwind_VRS_DataRepresentation::_UVRSD_UINT32, + ptr as *mut c_void, + ); + (val & !1) as libc::uintptr_t + } - // This function also doesn't exist on Android or ARM/Linux, so make it - // a no-op - #[cfg(any( - target_os = "android", - all(target_os = "freebsd", target_arch = "arm"), - all(target_os = "linux", target_arch = "arm") - ))] - pub unsafe fn _Unwind_FindEnclosingFunction(pc: *mut c_void) -> *mut c_void { - pc + // R13 is the stack pointer on arm. + const SP: _Unwind_Word = 13; + + pub unsafe fn get_sp(ctx: *mut _Unwind_Context) -> libc::uintptr_t { + let mut val: _Unwind_Word = 0; + let ptr = &mut val as *mut _Unwind_Word; + let _ = _Unwind_VRS_Get( + ctx, + _Unwind_VRS_RegClass::_UVRSC_CORE, + SP, + _Unwind_VRS_DataRepresentation::_UVRSD_UINT32, + ptr as *mut c_void, + ); + val as libc::uintptr_t + } + + // 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 + } } } diff --git a/src/backtrace/mod.rs b/src/backtrace/mod.rs index febacf3bb..a765acb8c 100644 --- a/src/backtrace/mod.rs +++ b/src/backtrace/mod.rs @@ -90,6 +90,14 @@ impl Frame { self.inner.ip() } + /// Returns the current stack pointer of this frame. + /// + /// In the case that a backend cannot recover the stack pointer for this + /// frame, a null pointer is returned. + pub fn sp(&self) -> *mut c_void { + self.inner.sp() + } + /// Returns the starting symbol address of the frame of this function. /// /// This will attempt to rewind the instruction pointer returned by `ip` to diff --git a/src/backtrace/noop.rs b/src/backtrace/noop.rs index 25e2062b3..78788b268 100644 --- a/src/backtrace/noop.rs +++ b/src/backtrace/noop.rs @@ -14,6 +14,10 @@ impl Frame { 0 as *mut _ } + pub fn sp(&self) -> *mut c_void { + 0 as *mut _ + } + pub fn symbol_address(&self) -> *mut c_void { 0 as *mut _ } diff --git a/src/capture.rs b/src/capture.rs index ba7644418..dba43bea0 100644 --- a/src/capture.rs +++ b/src/capture.rs @@ -338,17 +338,18 @@ impl fmt::Debug for Backtrace { // short format, because if it's full we presumably want to print // everything. let cwd = std::env::current_dir(); - let mut print_path = move |fmt: &mut fmt::Formatter<'_>, path: crate::BytesOrWideString<'_>| { - let path = path.into_path_buf(); - if !full { - if let Ok(cwd) = &cwd { - if let Ok(suffix) = path.strip_prefix(cwd) { - return fmt::Display::fmt(&suffix.display(), fmt); + let mut print_path = + move |fmt: &mut fmt::Formatter<'_>, path: crate::BytesOrWideString<'_>| { + let path = path.into_path_buf(); + if !full { + if let Ok(cwd) = &cwd { + if let Ok(suffix) = path.strip_prefix(cwd) { + return fmt::Display::fmt(&suffix.display(), fmt); + } } } - } - fmt::Display::fmt(&path.display(), fmt) - }; + fmt::Display::fmt(&path.display(), fmt) + }; let mut f = BacktraceFmt::new(fmt, style, &mut print_path); f.add_context()?; diff --git a/tests/smoke.rs b/tests/smoke.rs index 243b15844..bb7fa75e2 100644 --- a/tests/smoke.rs +++ b/tests/smoke.rs @@ -228,3 +228,84 @@ fn is_serde() { is_serialize::(); is_deserialize::(); } + +#[test] +fn sp_smoke_test() { + let mut refs = vec![]; + recursive_stack_references(&mut refs); + return; + + #[inline(never)] + fn recursive_stack_references(refs: &mut Vec) { + assert!(refs.len() < 5); + + let x = refs.len(); + refs.push(&x as *const _ as usize); + + if refs.len() < 5 { + recursive_stack_references(refs); + eprintln!("exiting: {}", x); + return; + } + + backtrace::trace(make_trace_closure(refs)); + eprintln!("exiting: {}", x); + } + + // NB: the following `make_*` functions are pulled out of line, rather than + // defining their results as inline closures at their call sites, so that + // the resulting closures don't have "recursive_stack_references" in their + // mangled names. + + fn make_trace_closure<'a>( + refs: &'a mut Vec, + ) -> impl FnMut(&backtrace::Frame) -> bool + 'a { + let mut child_sp = None; + let mut child_ref = None; + move |frame| { + eprintln!("\n=== frame ==================================="); + + let mut is_recursive_stack_references = false; + backtrace::resolve(frame.ip(), |sym| { + is_recursive_stack_references |= (LIBBACKTRACE || GIMLI_SYMBOLIZE) + && sym + .name() + .and_then(|name| name.as_str()) + .map_or(false, |name| { + eprintln!("name = {}", name); + name.contains("recursive_stack_references") + }) + }); + + let sp = frame.sp() as usize; + eprintln!("sp = {:p}", sp as *const u8); + if sp == 0 { + // If the SP is null, then we don't have an implementation for + // getting the SP on this target. Just keep walking the stack, + // but don't make our assertions about the on-stack pointers and + // SP values. + return true; + } + + // The stack grows down. + if let Some(child_sp) = child_sp { + assert!(child_sp <= sp); + } + + if is_recursive_stack_references { + let r = refs.pop().unwrap(); + eprintln!("ref = {:p}", r as *const u8); + if sp != 0 { + assert!(r > sp); + if let Some(child_ref) = child_ref { + assert!(sp >= child_ref); + } + } + child_ref = Some(r); + } + + child_sp = Some(sp); + true + } + } +}