Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the Frame::sp method to get the frame's stack pointer #341

Merged
merged 2 commits into from
Jun 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/backtrace/dbghelp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand All @@ -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,
Expand Down
96 changes: 71 additions & 25 deletions src/backtrace/libunwind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}
Expand All @@ -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;
Expand All @@ -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(),
}
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
}
}
8 changes: 8 additions & 0 deletions src/backtrace/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/backtrace/noop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 _
}
Expand Down
19 changes: 10 additions & 9 deletions src/capture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down
81 changes: 81 additions & 0 deletions tests/smoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,84 @@ fn is_serde() {
is_serialize::<backtrace::Backtrace>();
is_deserialize::<backtrace::Backtrace>();
}

#[test]
fn sp_smoke_test() {
let mut refs = vec![];
recursive_stack_references(&mut refs);
return;

#[inline(never)]
fn recursive_stack_references(refs: &mut Vec<usize>) {
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<usize>,
) -> 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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the LIBBACKTRACE || GIMLI_SYMBOLIZE part here necessary? It looks like this handles a non-present name?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can't rely on symbolication, then we can't determine which stack frames are the ones we are interested in (due to inlining happening or not), and therefore don't know when to pop from our stack of on-stack references.

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