Repository: gimli-rs/addr2line (rust)

Issue:
addr2line should use .symtab information
In the absence of `.debug_line` information, there is still other file and address information in `.debug_info` that we can use to give vague information about the address.


Pull request description:
Add option to use symbol table
Fixes #3 

This gives results that match binutil addr2line for all addresses except for a couple of oddities (eg zero size symbol).

Patch:
diff --git a/Cargo.toml b/Cargo.toml
index 3aaad63..31852c1 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -20,7 +20,7 @@ fallible-iterator = "0.1.3"
 clap = "2.19.1"
 gimli = "0.14.0"
 memmap = "0.5.0"
-object = "0.4.1"
+object = "0.5.0"
 owning_ref = "0.2.4"
 error-chain = "0.7.1"
 cpp_demangle = { version = "0.2.0", optional = true }
diff --git a/src/bin/addr2line.rs b/src/bin/addr2line.rs
index 1fba28f..8e57c39 100644
--- a/src/bin/addr2line.rs
+++ b/src/bin/addr2line.rs
@@ -68,7 +68,7 @@ fn get_matches() -> clap::ArgMatches<'static> {
 
 #[cfg(any(feature = "rustc-demangle", feature = "cpp_demangle"))]
 fn get_options(matches: &clap::ArgMatches) -> addr2line::Options {
-    let mut opts = addr2line::Options::default();
+    let mut opts = addr2line::Options::default().with_symbol_table();
 
     if matches.is_present("functions") {
         opts = opts.with_functions();
@@ -82,7 +82,7 @@ fn get_options(matches: &clap::ArgMatches) -> addr2line::Options {
 
 #[cfg(not(any(feature = "rustc-demangle", feature = "cpp_demangle")))]
 fn get_options(matches: &clap::ArgMatches) -> addr2line::Options {
-    let mut opts = addr2line::Options::default();
+    let mut opts = addr2line::Options::default().with_symbol_table();
 
     if matches.is_present("functions") {
         opts = opts.with_functions();
@@ -119,16 +119,18 @@ fn main() {
         // TODO: we may want to print an error here. GNU binutils addr2line doesn't though...
         let loc = debug.locate(addr).unwrap_or(None);
         if let Some((file, lineno, func)) = loc {
+            use std::borrow::Cow;
             if show_funcs {
-                use std::borrow::Cow;
                 println!("{}", func.unwrap_or(Cow::Borrowed("??")));
             }
             println!(
                 "{}:{}",
-                file.to_string_lossy(),
+                file.as_ref()
+                    .map(|f| f.to_string_lossy())
+                    .unwrap_or(Cow::Borrowed("??")),
                 lineno
-                    .map(|n| format!("{}", n))
-                    .unwrap_or_else(|| "?".to_owned())
+                    .map(|n| Cow::Owned(format!("{}", n)))
+                    .unwrap_or(Cow::Borrowed("?"))
             );
         } else {
             if show_funcs {
diff --git a/src/lib.rs b/src/lib.rs
index 1f33c63..4ba4ada 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -38,6 +38,7 @@ extern crate error_chain;
 
 use owning_ref::OwningHandle;
 use fallible_iterator::FallibleIterator;
+use object::SymbolKind;
 
 use std::fmt;
 use std::path;
@@ -143,6 +144,7 @@ pub use errors::*;
 #[derive(Clone, Copy, Default)]
 pub struct Options {
     with_functions: bool,
+    with_symbol_table: bool,
     with_demangling: bool,
 }
 
@@ -155,6 +157,14 @@ impl Options {
         self
     }
 
+    /// Make the `Mapping` fallback to using the symbol table if there
+    /// is no debug information for an address. This comes at some parsing
+    /// and lookup cost.
+    pub fn with_symbol_table(mut self) -> Self {
+        self.with_symbol_table = true;
+        self
+    }
+
     /// Make the `Mapping` attempt to demangle Rust and/or C++ symbols. This
     /// option implies `with_functions`.
     #[cfg(any(feature = "cpp_demangle", feature = "rustc-demangle"))]
@@ -215,6 +225,7 @@ where
     Endian: gimli::Endianity,
 {
     units: Vec<Unit<'input, Endian>>,
+    symbols: Vec<Symbol<'input>>,
     opts: Options,
 }
 
@@ -250,7 +261,7 @@ impl Mapping {
     pub fn locate(
         &mut self,
         addr: u64,
-    ) -> Result<Option<(path::PathBuf, Option<u64>, Option<Cow<str>>)>> {
+    ) -> Result<Option<(Option<path::PathBuf>, Option<u64>, Option<Cow<str>>)>> {
         self.inner.locate(addr)
     }
 }
@@ -272,7 +283,7 @@ impl<'input> BufferMapping<'input> {
     pub fn locate(
         &mut self,
         addr: u64,
-    ) -> Result<Option<(path::PathBuf, Option<u64>, Option<Cow<'input, str>>)>> {
+    ) -> Result<Option<(Option<path::PathBuf>, Option<u64>, Option<Cow<'input, str>>)>> {
         self.0.locate(addr)
     }
 }
@@ -294,7 +305,7 @@ impl<'input> EndianDebugInfo<'input> {
     fn locate(
         &mut self,
         addr: u64,
-    ) -> Result<Option<(path::PathBuf, Option<u64>, Option<Cow<'input, str>>)>> {
+    ) -> Result<Option<(Option<path::PathBuf>, Option<u64>, Option<Cow<'input, str>>)>> {
         match *self {
             EndianDebugInfo::LEInfo(ref mut dbg) => dbg.locate(addr),
             EndianDebugInfo::BEInfo(ref mut dbg) => dbg.locate(addr),
@@ -342,16 +353,23 @@ where
             }
         }
 
+        let symbols = if opts.with_symbol_table {
+            parse_symbols(file)
+        } else {
+            Vec::new()
+        };
+
         Ok(DebugInfo {
-            units: units,
-            opts: opts,
+            units,
+            symbols,
+            opts,
         })
     }
 
     pub fn locate(
         &mut self,
         addr: u64,
-    ) -> Result<Option<(path::PathBuf, Option<u64>, Option<Cow<'input, str>>)>> {
+    ) -> Result<Option<(Option<path::PathBuf>, Option<u64>, Option<Cow<'input, str>>)>> {
         // First, find the compilation unit for the given address
         for unit in &mut self.units {
             if !unit.maybe_contains_address(addr) {
@@ -390,7 +408,7 @@ where
 
             unit.read_programs()?;
             if unit.programs.is_empty() {
-                return Ok(Some((path, line, None)));
+                return Ok(Some((Some(path), line, None)));
             }
 
             // The unit also has programs, so let's look for the function wrapping this address.
@@ -462,41 +480,110 @@ where
                     Some(gimli::DW_LANG_C_plus_plus_03) |
                     Some(gimli::DW_LANG_C_plus_plus_11) => demangle_cpp_symbol(u.0.name.buf()),
                     Some(gimli::DW_LANG_Rust) => demangle_rust_symbol(u.0.name.buf()),
-                    _ => u.0.name.to_string_lossy(),
-                }
+                    _ => None,
+                }.unwrap_or_else(|| u.0.name.to_string_lossy())
             });
 
-            return Ok(Some((path, line, func)));
+            return Ok(Some((Some(path), line, func)));
+        }
+
+        // Didn't find in debuginfo, so check symbol table.
+        let idx = self.symbols
+            .binary_search_by(|symbol| if addr < symbol.begin {
+                std::cmp::Ordering::Greater
+            } else if addr < symbol.end {
+                std::cmp::Ordering::Equal
+            } else {
+                std::cmp::Ordering::Less
+            })
+            .ok();
+        if let Some(idx) = idx {
+            let symbol = &self.symbols[idx];
+            if symbol.file.is_some() || self.opts.with_functions {
+                let file = symbol
+                    .file
+                    .map(|file| path::PathBuf::from(&*String::from_utf8_lossy(file)));
+                let name = if self.opts.with_demangling {
+                    demangle_any_symbol(symbol.name)
+                } else {
+                    None
+                };
+                let name = name.or_else(|| Some(String::from_utf8_lossy(symbol.name)));
+                return Ok(Some((file, None, name)));
+            }
         }
+
         Ok(None)
     }
 }
 
+/// Demangle a symbol when we don't know which language it is.
+fn demangle_any_symbol(mangled: &[u8]) -> Option<Cow<str>> {
+    demangle_cpp_symbol(mangled).or_else(|| demangle_rust_symbol(mangled))
+}
+
 #[cfg(feature = "cpp_demangle")]
-fn demangle_cpp_symbol(mangled: &[u8]) -> Cow<str> {
-    if let Ok(sym) = cpp_demangle::Symbol::new(mangled) {
-        Cow::from(format!("{}", sym))
-    } else {
-        String::from_utf8_lossy(mangled)
-    }
+fn demangle_cpp_symbol(mangled: &[u8]) -> Option<Cow<str>> {
+    cpp_demangle::Symbol::new(mangled)
+        .ok()
+        .map(|sym| Cow::from(format!("{}", sym)))
 }
 
 #[cfg(not(feature = "cpp_demangle"))]
-fn demangle_cpp_symbol(mangled: &[u8]) -> Cow<str> {
-    String::from_utf8_lossy(mangled)
+fn demangle_cpp_symbol(mangled: &[u8]) -> Option<Cow<str>> {
+    None
 }
 
 #[cfg(feature = "rustc-demangle")]
-fn demangle_rust_symbol(mangled: &[u8]) -> Cow<str> {
-    Cow::from(format!(
+fn demangle_rust_symbol(mangled: &[u8]) -> Option<Cow<str>> {
+    Some(Cow::from(format!(
         "{}",
         rustc_demangle::demangle(String::from_utf8_lossy(mangled).as_ref())
-    ))
+    )))
 }
 
 #[cfg(not(feature = "rustc-demangle"))]
-fn demangle_rust_symbol(mangled: &[u8]) -> Cow<str> {
-    String::from_utf8_lossy(mangled)
+fn demangle_rust_symbol(mangled: &[u8]) -> Option<Cow<str>> {
+    None
+}
+
+struct Symbol<'input> {
+    name: &'input [u8],
+    file: Option<&'input [u8]>,
+    begin: u64,
+    end: u64,
+}
+
+fn parse_symbols<'input>(file: &object::File<'input>) -> Vec<Symbol<'input>> {
+    let mut symbols = Vec::new();
+    let mut filename = None;
+    for symbol in file.get_symbols() {
+        match symbol.kind() {
+            SymbolKind::Unknown | SymbolKind::Text | SymbolKind::Data => {}
+            SymbolKind::File => {
+                if symbol.name().is_empty() {
+                    filename = None;
+                } else {
+                    filename = Some(symbol.name());
+                }
+                continue;
+            }
+            SymbolKind::Section | SymbolKind::Common | SymbolKind::Tls => continue,
+        }
+        if symbol.is_undefined() || symbol.name().is_empty() || symbol.size() == 0 {
+            continue;
+        }
+        symbols.push(Symbol {
+            name: symbol.name(),
+            file: filename,
+            begin: symbol.address(),
+            end: symbol.address() + symbol.size(),
+        });
+    }
+
+    // Sort so we can binary search.
+    symbols.sort_by_key(|x| x.begin);
+    symbols
 }
 
 struct Unit<'input, Endian>


Test patch:
diff --git a/tests/integration_test.rs b/tests/integration_test.rs
index 612114a..d58eb32 100644
--- a/tests/integration_test.rs
+++ b/tests/integration_test.rs
@@ -1,6 +1,7 @@
 extern crate addr2line;
 extern crate itertools;
 
+use std::borrow::Cow;
 use std::env;
 use std::path::{self, PathBuf};
 use std::process;
@@ -64,7 +65,10 @@ fn identity_map((target, debug): (PathBuf, PathBuf)) {
     let mappings = get_test_addresses(target.as_path());
 
     // Parse the debug symbols using "our" addr2line
-    let mut ours = addr2line::Mapping::new(&debug).unwrap();
+    let mut ours = addr2line::Options::default()
+        .with_symbol_table()
+        .build(&debug)
+        .unwrap();
 
     // Spin up the "real" addr2line
     let theirs = spawn_oracle(target.as_path(), &[], mappings.clone());
@@ -72,7 +76,6 @@ fn identity_map((target, debug): (PathBuf, PathBuf)) {
 
     // Go through addresses one by one, and check that we get the same answer as addr2line does.
     let mut all = 0;
-    let mut excusable = 0;
     let mut got = 0;
     for addr in mappings {
         // Read the oracle ouput
@@ -83,43 +86,40 @@ fn identity_map((target, debug): (PathBuf, PathBuf)) {
         let loc = ours.locate(addr);
         let loc = loc.expect("debug symbols for test binary should be error-free");
         if let Some((file, lineno, _)) = loc {
+            assert!(file.is_some());
+
             // We dared to guess -- did we give the right answer?
-            let mut file = file.to_string_lossy();
+            let mut file = file.as_ref().map(|f| f.to_string_lossy());
             if cfg!(target_os = "macos") {
                 use std::borrow::Cow;
                 // atos doesn't include the full path, just file name
-                let f = path::PathBuf::from(&*file);
-                file = Cow::Owned(f.file_name().unwrap().to_string_lossy().into_owned());
+                file = file.map(|f| {
+                    let f = path::PathBuf::from(&*f);
+                    Cow::Owned(f.file_name().unwrap().to_string_lossy().into_owned())
+                });
             }
+            let file = file.as_ref().map(Cow::as_ref);
+            let test = (file, lineno);
 
-            all += 1;
             if oracle.0.is_none() && lineno.is_none() {
                 // This can happen for the main() wrapper.
-                println!("we found 0x{:08x}: {}:??", addr, file);
-                excusable += 1;
+                println!("we found 0x{:08x}: {}:?", addr, file.unwrap_or("??"));
             } else {
-                assert_eq!((addr, oracle), (addr, (Some(&*file), lineno)));
+                assert_eq!((addr, oracle), (addr, test));
+                all += 1;
                 got += 1;
             }
         } else if oracle.0.is_some() {
             // addr2line found something, and we didn't :(
             println!("we missed 0x{:08x}: {:?}", addr, oracle);
             all += 1;
-            if oracle.1.is_none() {
-                excusable += 1;
-            }
         } else {
             // addr2line did not find a source file, so it's okay that we didn't either.
         }
     }
 
     assert_ne!(got, 0);
-    println!(
-        "resolved {}/{} addresses ({} file-only misses)",
-        got,
-        all,
-        excusable
-    );
+    println!("resolved {}/{} addresses", got, all,);
 }
 
 #[cfg(not(target_os = "macos"))]
@@ -132,6 +132,7 @@ fn with_functions((target, debug): (PathBuf, PathBuf)) {
     // Parse the debug symbols using "our" addr2line
     let mut ours = addr2line::Options::default()
         .with_functions()
+        .with_symbol_table()
         .build(&debug)
         .unwrap();
 
@@ -141,7 +142,6 @@ fn with_functions((target, debug): (PathBuf, PathBuf)) {
 
     // Go through addresses one by one, and check that we get the same answer as addr2line does.
     let mut all = 0;
-    let mut excusable = 0;
     let mut func_hits = 0;
     let mut got = 0;
     for addr in mappings {
@@ -154,32 +154,36 @@ fn with_functions((target, debug): (PathBuf, PathBuf)) {
         let loc = ours.locate(addr);
         let loc = loc.expect("debug symbols for test binary should be error-free");
         if let Some((file, lineno, func)) = loc {
+            assert!(file.is_some() || func.is_some());
+
             // We dared to guess -- did we give the right answer?
-            let f = &*file.to_string_lossy();
-            let test = (Some(f), lineno);
+            let file = file.as_ref().map(|f| f.to_string_lossy());
+            let file = file.as_ref().map(Cow::as_ref);
+            let test = (file, lineno);
 
             all += 1;
-            if oracle.0.is_none() && lineno.is_none() {
+            if oracle.0.is_none() && file.is_some() && lineno.is_none() {
                 // This can happen for the main() wrapper.
-                println!("we found 0x{:08x}: {}", addr, f);
-                excusable += 1;
+                println!(
+                    "we found 0x{:08x}: {}:? {:?}",
+                    addr,
+                    file.unwrap_or("??"),
+                    func
+                );
             } else {
-                assert_eq!(oracle, test);
-
-                if let Some(func) = func {
-                    // We even tried to guess the function!
-                    assert_eq!(function, func);
-                    func_hits += 1;
-                }
+                assert_eq!((addr, oracle), (addr, test));
                 got += 1;
             }
+
+            if let Some(func) = func {
+                // We even tried to guess the function!
+                assert_eq!(function, func);
+                func_hits += 1;
+            }
         } else if oracle.0.is_some() {
             // addr2line found something, and we didn't :(
             println!("we missed 0x{:08x}: {:?}", addr, oracle);
             all += 1;
-            if oracle.1.is_none() {
-                excusable += 1;
-            }
         } else {
             // addr2line did not find a source file, so it's okay that we didn't either.
         }
@@ -188,10 +192,9 @@ fn with_functions((target, debug): (PathBuf, PathBuf)) {
     assert_ne!(got, 0);
     assert_ne!(func_hits, 0);
     println!(
-        "resolved {}/{} addresses ({} file-only misses, {} function hits)",
+        "resolved {}/{} addresses ({} function hits)",
         got,
         all,
-        excusable,
         func_hits
     );
 }