make it easy to resolve symbols by frame

Author: fraillt

src/capture.rs

@@ -26,9 +26,6 @@ use serde::{Deserialize, Serialize};
 pub struct Backtrace {
     // Frames here are listed from top-to-bottom of the stack
     frames: Vec<BacktraceFrame>,
-    // The index we believe is the actual start of the backtrace, omitting
-    // frames like `Backtrace::new` and `backtrace::trace`.
-    actual_start_index: usize,
 }
 
 fn _assert_send_sync() {
@@ -86,6 +83,27 @@ impl Frame {
             } => module_base_address.map(|addr| addr as *mut c_void),
         }
     }
+
+    /// Resolve all addresses in the frame to their symbolic names.
+    fn resolve_symbols(&self) -> Vec<BacktraceSymbol> {
+        let mut symbols = Vec::new();
+        let sym = |symbol: &Symbol| {
+            symbols.push(BacktraceSymbol {
+                name: symbol.name().map(|m| m.as_bytes().to_vec()),
+                addr: symbol.addr().map(|a| a as usize),
+                filename: symbol.filename().map(|m| m.to_owned()),
+                lineno: symbol.lineno(),
+                colno: symbol.colno(),
+            });
+        };
+        match *self {
+            Frame::Raw(ref f) => resolve_frame(f, sym),
+            Frame::Deserialized { ip, .. } => {
+                resolve(ip as *mut c_void, sym);
+            }
+        }
+        symbols
+    }
 }
 
 /// Captured version of a symbol in a backtrace.
@@ -172,23 +190,22 @@ impl Backtrace {
 
     fn create(ip: usize) -> Backtrace {
         let mut frames = Vec::new();
-        let mut actual_start_index = None;
         trace(|frame| {
             frames.push(BacktraceFrame {
                 frame: Frame::Raw(frame.clone()),
                 symbols: None,
             });
 
-            if frame.symbol_address() as usize == ip && actual_start_index.is_none() {
-                actual_start_index = Some(frames.len());
+            // clear inner frames, and start with call site.
+            if frame.symbol_address() as usize == ip {
+                frames.clear();
             }
+
             true
         });
+        frames.shrink_to_fit();
 
-        Backtrace {
-            frames,
-            actual_start_index: actual_start_index.unwrap_or(0),
-        }
+        Backtrace { frames }
     }
 
     /// Returns the frames from when this backtrace was captured.
@@ -202,7 +219,7 @@ impl Backtrace {
     /// This function requires the `std` feature of the `backtrace` crate to be
     /// enabled, and the `std` feature is enabled by default.
     pub fn frames(&self) -> &[BacktraceFrame] {
-        &self.frames[self.actual_start_index..]
+        self.frames.as_slice()
     }
 
     /// If this backtrace was created from `new_unresolved` then this function
@@ -216,51 +233,28 @@ impl Backtrace {
     /// This function requires the `std` feature of the `backtrace` crate to be
     /// enabled, and the `std` feature is enabled by default.
     pub fn resolve(&mut self) {
-        for frame in self.frames.iter_mut().filter(|f| f.symbols.is_none()) {
-            let mut symbols = Vec::new();
-            {
-                let sym = |symbol: &Symbol| {
-                    symbols.push(BacktraceSymbol {
-                        name: symbol.name().map(|m| m.as_bytes().to_vec()),
-                        addr: symbol.addr().map(|a| a as usize),
-                        filename: symbol.filename().map(|m| m.to_owned()),
-                        lineno: symbol.lineno(),
-                        colno: symbol.colno(),
-                    });
-                };
-                match frame.frame {
-                    Frame::Raw(ref f) => resolve_frame(f, sym),
-                    Frame::Deserialized { ip, .. } => {
-                        resolve(ip as *mut c_void, sym);
-                    }
-                }
-            }
-            frame.symbols = Some(symbols);
-        }
+        self.frames.iter_mut().for_each(BacktraceFrame::resolve);
     }
 }
 
 impl From<Vec<BacktraceFrame>> for Backtrace {
     fn from(frames: Vec<BacktraceFrame>) -> Self {
-        Backtrace {
-            frames,
-            actual_start_index: 0,
-        }
+        Backtrace { frames }
     }
 }
 
 impl From<crate::Frame> for BacktraceFrame {
-    fn from(frame: crate::Frame) -> BacktraceFrame {
+    fn from(frame: crate::Frame) -> Self {
         BacktraceFrame {
             frame: Frame::Raw(frame),
             symbols: None,
         }
     }
 }
 
-impl Into<Vec<BacktraceFrame>> for Backtrace {
-    fn into(self) -> Vec<BacktraceFrame> {
-        self.frames
+impl From<Backtrace> for Vec<BacktraceFrame> {
+    fn from(bt: Backtrace) -> Self {
+        bt.frames
     }
 }
 
@@ -314,6 +308,20 @@ impl BacktraceFrame {
     pub fn symbols(&self) -> &[BacktraceSymbol] {
         self.symbols.as_ref().map(|s| &s[..]).unwrap_or(&[])
     }
+
+    /// Resolve all addresses in this frame to their symbolic names.
+    ///
+    /// If this frame has been previously resolved, this function does nothing.
+    ///
+    /// # Required features
+    ///
+    /// This function requires the `std` feature of the `backtrace` crate to be
+    /// enabled, and the `std` feature is enabled by default.
+    pub fn resolve(&mut self) {
+        if self.symbols.is_none() {
+            self.symbols = Some(self.frame.resolve_symbols());
+        }
+    }
 }
 
 impl BacktraceSymbol {
@@ -370,11 +378,10 @@ impl BacktraceSymbol {
 
 impl fmt::Debug for Backtrace {
     fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let full = fmt.alternate();
-        let (frames, style) = if full {
-            (&self.frames[..], PrintFmt::Full)
+        let style = if fmt.alternate() {
+            PrintFmt::Full
         } else {
-            (&self.frames[self.actual_start_index..], PrintFmt::Short)
+            PrintFmt::Short
         };
 
         // When printing paths we try to strip the cwd if it exists, otherwise
@@ -385,7 +392,7 @@ impl fmt::Debug for Backtrace {
         let mut print_path =
             move |fmt: &mut fmt::Formatter<'_>, path: crate::BytesOrWideString<'_>| {
                 let path = path.into_path_buf();
-                if !full {
+                if style == PrintFmt::Full {
                     if let Ok(cwd) = &cwd {
                         if let Ok(suffix) = path.strip_prefix(cwd) {
                             return fmt::Display::fmt(&suffix.display(), fmt);
@@ -397,7 +404,7 @@ impl fmt::Debug for Backtrace {
 
         let mut f = BacktraceFmt::new(fmt, style, &mut print_path);
         f.add_context()?;
-        for frame in frames {
+        for frame in &self.frames {
             f.frame().backtrace_frame(frame)?;
         }
         f.finish()?;

src/symbolize/gimli.rs

@@ -7,7 +7,6 @@ use self::gimli::NativeEndian as Endian;
 use self::mmap::Mmap;
 use self::stash::Stash;
 use super::BytesOrWideString;
-use super::ResolveWhat;
 use super::SymbolName;
 use addr2line::gimli;
 use core::convert::TryInto;
@@ -382,8 +381,7 @@ impl Cache {
     }
 }
 
-pub unsafe fn resolve(what: ResolveWhat<'_>, cb: &mut dyn FnMut(&super::Symbol)) {
-    let addr = what.address_or_ip();
+pub unsafe fn resolve(addr: *mut c_void, cb: &mut dyn FnMut(&super::Symbol)) {
     let mut call = |sym: Symbol<'_>| {
         // Extend the lifetime of `sym` to `'static` since we are unfortunately
         // required to here, but it's only ever going out as a reference so no

src/symbolize/mod.rs

@@ -105,21 +105,6 @@ pub fn resolve_frame<F: FnMut(&Symbol)>(frame: &Frame, cb: F) {
     unsafe { resolve_frame_unsynchronized(frame, cb) }
 }
 
-pub enum ResolveWhat<'a> {
-    Address(*mut c_void),
-    Frame(&'a Frame),
-}
-
-impl<'a> ResolveWhat<'a> {
-    #[allow(dead_code)]
-    fn address_or_ip(&self) -> *mut c_void {
-        match self {
-            ResolveWhat::Address(a) => adjust_ip(*a),
-            ResolveWhat::Frame(f) => adjust_ip(f.ip()),
-        }
-    }
-}
-
 // IP values from stack frames are typically (always?) the instruction
 // *after* the call that's the actual stack trace. Symbolizing this on
 // causes the filename/line number to be one ahead and perhaps into
@@ -159,7 +144,7 @@ pub unsafe fn resolve_unsynchronized<F>(addr: *mut c_void, mut cb: F)
 where
     F: FnMut(&Symbol),
 {
-    imp::resolve(ResolveWhat::Address(addr), &mut cb)
+    imp::resolve(adjust_ip(addr), &mut cb)
 }
 
 /// Same as `resolve_frame`, only unsafe as it's unsynchronized.
@@ -175,7 +160,7 @@ pub unsafe fn resolve_frame_unsynchronized<F>(frame: &Frame, mut cb: F)
 where
     F: FnMut(&Symbol),
 {
-    imp::resolve(ResolveWhat::Frame(frame), &mut cb)
+    imp::resolve(adjust_ip(frame.ip()), &mut cb)
 }
 
 /// A trait representing the resolution of a symbol in a file.