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

Author: fitzgen

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,

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

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

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

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()?;

tests/smoke.rs

@@ -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)
+                    && 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
+        }
+    }
+}