From 69367b540859540ce32f9126e398c3076f9d9a6a Mon Sep 17 00:00:00 2001 From: Jason White Date: Tue, 15 Nov 2022 09:58:51 -0800 Subject: [PATCH] Partially fix symbol resolution Summary: `libunwind` isn't able to resolve symbols from the symbol table. There seems to be a regression preventing this from working. This partially fixes symbol lookup for stack frames by using the `object` crate for looking up symbols in the symbol table. This is a partial fix because symbol lookup does not seem to work yet for executables (only shared libraries). Reviewed By: VladimirMakaev Differential Revision: D41290099 fbshipit-source-id: 5d4ad1173f6ab1ca6c2995369c2dedb4a9f30e86 --- reverie-ptrace/src/task.rs | 25 +++----------- reverie/src/backtrace/mod.rs | 59 +++++++++++++++++++++++++------- reverie/src/backtrace/symbols.rs | 15 ++++++++ tests/backtrace.rs | 15 ++++---- 4 files changed, 71 insertions(+), 43 deletions(-) diff --git a/reverie-ptrace/src/task.rs b/reverie-ptrace/src/task.rs index fe3e950..ca38c05 100644 --- a/reverie-ptrace/src/task.rs +++ b/reverie-ptrace/src/task.rs @@ -47,7 +47,6 @@ use reverie::Pid; #[cfg(target_arch = "x86_64")] use reverie::Rdtsc; use reverie::Subscription; -use reverie::Symbol; use reverie::Tid; use reverie::TimerSchedule; use reverie::Tool; @@ -2195,32 +2194,16 @@ impl Guest for TracedTask { let ip = cursor.register(RegNum::IP).ok()?; let is_signal = cursor.is_signal_frame().ok()?; - // Try to resolve the symbol. - let mut symbol = None; - if let Ok(name) = cursor.procedure_name() { - if let Ok(info) = cursor.procedure_info() { - if info.start_ip() + name.offset() == ip { - symbol = Some(Symbol { - name: name.name().to_string(), - offset: name.offset(), - address: info.start_ip(), - size: info.end_ip() - info.start_ip(), - }); - } - } - } - - frames.push(Frame { - ip, - is_signal, - symbol, - }); + frames.push(Frame { ip, is_signal }); if !cursor.step().ok()? { break; } } + // TODO: Take a snapshot of `/proc/self/maps` so the backtrace can be + // processed offline? + Some(Backtrace::new(self.tid(), frames)) } } diff --git a/reverie/src/backtrace/mod.rs b/reverie/src/backtrace/mod.rs index a8677dd..e90efb2 100644 --- a/reverie/src/backtrace/mod.rs +++ b/reverie/src/backtrace/mod.rs @@ -50,8 +50,6 @@ pub struct Frame { pub ip: u64, /// True if this frame is inside of a signal handler. pub is_signal: bool, - /// The symbol associated with this frame (if known). - pub symbol: Option, } /// A stack frame with debugging information. @@ -59,6 +57,8 @@ pub struct Frame { pub struct PrettyFrame { /// The raw stack frame information. frame: Frame, + /// The symbol as found in the symbol table. + symbol: Option, /// The source file and line where the instruction pointer is located. locations: Vec, } @@ -68,12 +68,10 @@ pub struct PrettyFrame { pub struct Symbol { /// Name of the (mangled) symbol. pub name: String, - /// Offset of the symbol. - pub offset: u64, /// Address of the symbol. pub address: u64, - /// Size of the symbol. - pub size: u64, + /// The offset of the instruction pointer from `address`. + pub offset: u64, } /// The location of a symbol. @@ -98,10 +96,7 @@ impl Symbol { impl fmt::Display for Frame { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match &self.symbol { - Some(symbol) => write!(f, "{:#016x}: {:#}", self.ip, symbol)?, - None => write!(f, "{:#016x}: ???", self.ip)?, - } + write!(f, "{:#016x}: ???", self.ip)?; if self.is_signal { write!(f, " (in signal handler)")?; @@ -147,7 +142,8 @@ impl Backtrace { Self { thread_id, frames } } - /// Generates a pretty backtrace that includes file and line information for each frame. + /// Generates a pretty backtrace that includes file and line information for + /// each frame. pub fn pretty(&self) -> Result { let libraries = Libraries::new(self.thread_id)?; @@ -157,10 +153,19 @@ impl Backtrace { for frame in &self.frames { let ip = frame.ip; let mut locations = Vec::new(); + let mut symbol = None; if let Some((library, addr)) = libraries.ip_to_vaddr(ip) { let symbols = cache.load(library)?; + // Find symbol using the symbol table. + symbol = symbols.find_symbol(addr).map(|sym| Symbol { + name: sym.name().to_string(), + address: sym.address(), + offset: addr - sym.address(), + }); + + // Find the file + line number of the instruction pointer. let mut source_frames = symbols.find_frames(addr)?; while let Some(f) = source_frames.next()? { if let Some(loc) = f.location { @@ -175,6 +180,7 @@ impl Backtrace { frames.push(PrettyFrame { frame: frame.clone(), + symbol, locations, }); } @@ -204,6 +210,11 @@ impl Backtrace { } impl PrettyBacktrace { + /// Returns an iterator over the frames in the backtrace. + pub fn iter(&self) -> impl Iterator { + self.frames.iter() + } + /// Retrieves the name of the thread for this backtrace. This will fail if /// the thread has already exited since the thread ID is used to look up the /// thread name. @@ -212,6 +223,13 @@ impl PrettyBacktrace { } } +impl PrettyFrame { + /// The symbol for this frame, if any. + pub fn symbol(&self) -> Option<&Symbol> { + self.symbol.as_ref() + } +} + impl IntoIterator for Backtrace { type Item = Frame; type IntoIter = std::vec::IntoIter; @@ -221,6 +239,15 @@ impl IntoIterator for Backtrace { } } +impl IntoIterator for PrettyBacktrace { + type Item = PrettyFrame; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.frames.into_iter() + } +} + impl From for Vec { fn from(bt: Backtrace) -> Self { bt.frames @@ -259,10 +286,16 @@ impl fmt::Display for PrettyBacktrace { )?; for (i, frame) in self.frames.iter().enumerate() { + // Frame number + write!(f, "{:>4}: ", i)?; + if frame.locations.is_empty() { - writeln!(f, "{:>4}: {}", i, frame)?; + match &frame.symbol { + Some(symbol) => writeln!(f, "{:#016x}: {:#}", frame.frame.ip, symbol)?, + None => writeln!(f, "{}", frame.frame)?, + } } else { - writeln!(f, "{:>4}: {:#}", i, frame.frame)?; + writeln!(f, "{:#}", frame.frame)?; for location in &frame.locations { writeln!(f, " at {}", location)?; } diff --git a/reverie/src/backtrace/symbols.rs b/reverie/src/backtrace/symbols.rs index 410ea5c..b008326 100644 --- a/reverie/src/backtrace/symbols.rs +++ b/reverie/src/backtrace/symbols.rs @@ -13,10 +13,12 @@ use std::path::Path; use gimli::EndianSlice; use gimli::RunTimeEndian as Endian; use object::Object as _; +use object::SymbolMapName; use typed_arena::Arena; struct Context<'mmap> { dwarf: addr2line::Context>, + symbol_map: object::SymbolMap>, _object: object::File<'mmap>, } @@ -63,6 +65,7 @@ impl Symbols { let context = Context { dwarf: addr2line::Context::from_dwarf(dwarf)?, + symbol_map: object.symbol_map(), _object: object, }; @@ -91,6 +94,18 @@ impl Symbols { self.context.dwarf.find_frames(probe) } + /// Finds a symbol in the symbol table using the given address. If the + /// symbol does not exist, returns `None`. Note that this purely uses the + /// symbol table to find the symbol name and does not depend on the debug + /// info at all. This should be used as a fallback if `find_frames` is + /// unable to locate the symbol name using the debug info. + /// + /// Symbol lookup uses binary search, so it lookup happens in `O(log n)` + /// amortized time. + pub fn find_symbol(&self, probe: u64) -> Option { + self.context.symbol_map.get(probe).copied() + } + /// Returns the number of bytes used to store the debug information. This is /// used to keep track of our memory overhead when loading symbols. pub fn bytes_used(&self) -> usize { diff --git a/tests/backtrace.rs b/tests/backtrace.rs index 492fc7a..cb97acb 100644 --- a/tests/backtrace.rs +++ b/tests/backtrace.rs @@ -29,26 +29,23 @@ impl Tool for TestTool { if let Syscall::Getpid(_) = &syscall { let backtrace = guest .backtrace() - .expect("failed to get backtrace from guest"); + .expect("failed to get backtrace from guest") + .pretty() + .expect("failed to get pretty backtrace from guest"); // There's no guarantee our function is at the top of the stack, so // we simply assert that it is *somewhere* in the stack. assert!( backtrace.iter().any(|frame| { - if let Some(symbol) = &frame.symbol { + if let Some(symbol) = frame.symbol() { // Due to name mangling, there won't be an exact match. symbol.name.contains("funky_function") } else { - // FIXME: The unwind library is currently broken on - // platform010. It is not able to find symbols for any - // stack frames. Change this back to `false` when that - // is fixed. For now, file and line numbers still work. - true - //false + false } }), "guest backtrace did not contain our expected function:\n{}", - backtrace.pretty().unwrap() + backtrace ); }