Include source column numbers, where available (#367)

* Provide access to source column numbers of symbol locations, in
addition to filename and line numbers (where available - currently
only gimli).

* Print column numbers in backtraces, if available.

* cargo fmt

* Fixed breaking-change to public API, by delegating to new print_raw_with_column (as suggested by @alexcrichton in review #367 (comment))

* Added tests for column numbers

* Reflect actual conditional compilation logic
in smoke test guards

Author: eggyal

src/capture.rs

@@ -94,6 +94,7 @@ pub struct BacktraceSymbol {
     addr: Option<usize>,
     filename: Option<PathBuf>,
     lineno: Option<u32>,
+    colno: Option<u32>,
 }
 
 impl Backtrace {
@@ -213,6 +214,7 @@ impl Backtrace {
                         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 {
@@ -322,6 +324,16 @@ impl BacktraceSymbol {
     pub fn lineno(&self) -> Option<u32> {
         self.lineno
     }
+
+    /// Same as `Symbol::colno`
+    ///
+    /// # 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 colno(&self) -> Option<u32> {
+        self.colno
+    }
 }
 
 impl fmt::Debug for Backtrace {
@@ -383,6 +395,7 @@ impl fmt::Debug for BacktraceSymbol {
             .field("addr", &self.addr())
             .field("filename", &self.filename())
             .field("lineno", &self.lineno())
+            .field("colno", &self.colno())
             .finish()
     }
 }

src/print.rs

@@ -130,7 +130,7 @@ impl BacktraceFrameFmt<'_, '_, '_> {
         frame: &BacktraceFrame,
         symbol: &BacktraceSymbol,
     ) -> fmt::Result {
-        self.print_raw(
+        self.print_raw_with_column(
             frame.ip(),
             symbol.name(),
             // TODO: this isn't great that we don't end up printing anything
@@ -140,18 +140,20 @@ impl BacktraceFrameFmt<'_, '_, '_> {
                 .filename()
                 .and_then(|p| Some(BytesOrWideString::Bytes(p.to_str()?.as_bytes()))),
             symbol.lineno(),
+            symbol.colno(),
         )?;
         Ok(())
     }
 
     /// Prints a raw traced `Frame` and `Symbol`, typically from within the raw
     /// callbacks of this crate.
     pub fn symbol(&mut self, frame: &Frame, symbol: &super::Symbol) -> fmt::Result {
-        self.print_raw(
+        self.print_raw_with_column(
             frame.ip(),
             symbol.name(),
             symbol.filename_raw(),
             symbol.lineno(),
+            symbol.colno(),
         )?;
         Ok(())
     }
@@ -174,7 +176,32 @@ impl BacktraceFrameFmt<'_, '_, '_> {
         if cfg!(target_os = "fuchsia") {
             self.print_raw_fuchsia(frame_ip)?;
         } else {
-            self.print_raw_generic(frame_ip, symbol_name, filename, lineno)?;
+            self.print_raw_generic(frame_ip, symbol_name, filename, lineno, None)?;
+        }
+        self.symbol_index += 1;
+        Ok(())
+    }
+
+    /// Adds a raw frame to the backtrace output, including column information.
+    ///
+    /// This method, like the previous, takes the raw arguments in case
+    /// they're being source from different locations. Note that this may be
+    /// called multiple times for one frame.
+    pub fn print_raw_with_column(
+        &mut self,
+        frame_ip: *mut c_void,
+        symbol_name: Option<SymbolName<'_>>,
+        filename: Option<BytesOrWideString<'_>>,
+        lineno: Option<u32>,
+        colno: Option<u32>,
+    ) -> fmt::Result {
+        // Fuchsia is unable to symbolize within a process so it has a special
+        // format which can be used to symbolize later. Print that instead of
+        // printing addresses in our own format here.
+        if cfg!(target_os = "fuchsia") {
+            self.print_raw_fuchsia(frame_ip)?;
+        } else {
+            self.print_raw_generic(frame_ip, symbol_name, filename, lineno, colno)?;
         }
         self.symbol_index += 1;
         Ok(())
@@ -187,6 +214,7 @@ impl BacktraceFrameFmt<'_, '_, '_> {
         symbol_name: Option<SymbolName<'_>>,
         filename: Option<BytesOrWideString<'_>>,
         lineno: Option<u32>,
+        colno: Option<u32>,
     ) -> fmt::Result {
         // No need to print "null" frames, it basically just means that the
         // system backtrace was a bit eager to trace back super far.
@@ -232,13 +260,18 @@ impl BacktraceFrameFmt<'_, '_, '_> {
 
         // And last up, print out the filename/line number if they're available.
         if let (Some(file), Some(line)) = (filename, lineno) {
-            self.print_fileline(file, line)?;
+            self.print_fileline(file, line, colno)?;
         }
 
         Ok(())
     }
 
-    fn print_fileline(&mut self, file: BytesOrWideString<'_>, line: u32) -> fmt::Result {
+    fn print_fileline(
+        &mut self,
+        file: BytesOrWideString<'_>,
+        line: u32,
+        colno: Option<u32>,
+    ) -> fmt::Result {
         // Filename/line are printed on lines under the symbol name, so print
         // some appropriate whitespace to sort of right-align ourselves.
         if let PrintFmt::Full = self.fmt.format {
@@ -249,7 +282,14 @@ impl BacktraceFrameFmt<'_, '_, '_> {
         // Delegate to our internal callback to print the filename and then
         // print out the line number.
         (self.fmt.print_path)(self.fmt.fmt, file)?;
-        write!(self.fmt.fmt, ":{}\n", line)?;
+        write!(self.fmt.fmt, ":{}", line)?;
+
+        // Add column number, if available.
+        if let Some(colno) = colno {
+            write!(self.fmt.fmt, ":{}", colno)?;
+        }
+
+        write!(self.fmt.fmt, "\n")?;
         Ok(())
     }
 

src/symbolize/dbghelp.rs

@@ -62,6 +62,10 @@ impl Symbol<'_> {
             .map(|slice| unsafe { BytesOrWideString::Wide(&*slice) })
     }
 
+    pub fn colno(&self) -> Option<u32> {
+        None
+    }
+
     pub fn lineno(&self) -> Option<u32> {
         self.line
     }

src/symbolize/gimli.rs

@@ -682,4 +682,11 @@ impl Symbol<'_> {
             Symbol::Symtab { .. } => None,
         }
     }
+
+    pub fn colno(&self) -> Option<u32> {
+        match self {
+            Symbol::Frame { location, .. } => location.as_ref()?.column,
+            Symbol::Symtab { .. } => None,
+        }
+    }
 }

src/symbolize/libbacktrace.rs

@@ -149,6 +149,10 @@ impl Symbol<'_> {
             Symbol::Pcinfo { lineno, .. } => Some(lineno as u32),
         }
     }
+
+    pub fn colno(&self) -> Option<u32> {
+        None
+    }
 }
 
 extern "C" fn error_cb(_data: *mut c_void, _msg: *const c_char, _errnum: c_int) {

src/symbolize/mod.rs

@@ -219,6 +219,14 @@ impl Symbol {
         self.inner.filename_raw()
     }
 
+    /// Returns the column number for where this symbol is currently executing.
+    ///
+    /// Only gimli currently provides a value here and even then only if `filename`
+    /// returns `Some`, and so it is then consequently subject to similar caveats.
+    pub fn colno(&self) -> Option<u32> {
+        self.inner.colno()
+    }
+
     /// Returns the line number for where this symbol is currently executing.
     ///
     /// This return value is typically `Some` if `filename` returns `Some`, and
@@ -229,8 +237,8 @@ impl Symbol {
 
     /// Returns the file name where this function was defined.
     ///
-    /// This is currently only available when libbacktrace is being used (e.g.
-    /// unix platforms other than OSX) and when a binary is compiled with
+    /// This is currently only available when libbacktrace or gimli is being
+    /// used (e.g. unix platforms other) and when a binary is compiled with
     /// debuginfo. If neither of these conditions is met then this will likely
     /// return `None`.
     ///

src/symbolize/noop.rs

@@ -32,4 +32,8 @@ impl Symbol<'_> {
     pub fn lineno(&self) -> Option<u32> {
         None
     }
+
+    pub fn colno(&self) -> Option<u32> {
+        None
+    }
 }

tests/smoke.rs

@@ -1,8 +1,36 @@
 use backtrace::Frame;
 use std::thread;
 
-static LIBBACKTRACE: bool = cfg!(feature = "libbacktrace") && !cfg!(target_os = "fuchsia");
-static GIMLI_SYMBOLIZE: bool = cfg!(all(feature = "gimli-symbolize", unix, target_os = "linux"));
+// Reflects the conditional compilation logic at end of src/symbolize/mod.rs
+static NOOP: bool = cfg!(miri);
+static DBGHELP: bool = !NOOP
+    && cfg!(all(
+        windows,
+        target_env = "msvc",
+        not(target_vendor = "uwp")
+    ));
+static LIBBACKTRACE: bool = !NOOP
+    && !DBGHELP
+    && cfg!(all(
+        feature = "libbacktrace",
+        any(
+            unix,
+            all(windows, not(target_vendor = "uwp"), target_env = "gnu")
+        ),
+        not(target_os = "fuchsia"),
+        not(target_os = "emscripten"),
+        not(target_env = "uclibc"),
+        not(target_env = "libnx"),
+    ));
+static GIMLI_SYMBOLIZE: bool = !NOOP
+    && !DBGHELP
+    && !LIBBACKTRACE
+    && cfg!(all(
+        feature = "gimli-symbolize",
+        any(unix, windows),
+        not(target_vendor = "uwp"),
+        not(target_os = "emscripten"),
+    ));
 
 #[test]
 // FIXME: shouldn't ignore this test on i686-msvc, unsure why it's failing
@@ -46,34 +74,39 @@ fn smoke_test_frames() {
             "frame_4",
             "tests/smoke.rs",
             start_line + 6,
+            9,
         );
         assert_frame(
             frames.next().unwrap(),
             frame_3 as usize,
             "frame_3",
             "tests/smoke.rs",
             start_line + 3,
+            52,
         );
         assert_frame(
             frames.next().unwrap(),
             frame_2 as usize,
             "frame_2",
             "tests/smoke.rs",
             start_line + 2,
+            52,
         );
         assert_frame(
             frames.next().unwrap(),
             frame_1 as usize,
             "frame_1",
             "tests/smoke.rs",
             start_line + 1,
+            52,
         );
         assert_frame(
             frames.next().unwrap(),
             smoke_test_frames as usize,
             "smoke_test_frames",
             "",
             0,
+            0,
         );
     }
 
@@ -83,6 +116,7 @@ fn smoke_test_frames() {
         expected_name: &str,
         expected_file: &str,
         expected_line: u32,
+        expected_col: u32,
     ) {
         backtrace::resolve_frame(frame, |sym| {
             print!("symbol  ip:{:?} address:{:?} ", frame.ip(), frame.symbol_address());
@@ -95,6 +129,9 @@ fn smoke_test_frames() {
             if let Some(lineno) = sym.lineno() {
                 print!("lineno:{} ", lineno);
             }
+            if let Some(colno) = sym.colno() {
+                print!("colno:{} ", colno);
+            }
             println!();
         });
 
@@ -103,12 +140,13 @@ fn smoke_test_frames() {
         assert!(ip >= sym);
         assert!(
             sym >= actual_fn_pointer,
-            "{:?} < {:?} ({} {}:{})",
+            "{:?} < {:?} ({} {}:{}:{})",
             sym as *const usize,
             actual_fn_pointer as *const usize,
             expected_name,
             expected_file,
             expected_line,
+            expected_col,
         );
 
         // windows dbghelp is *quite* liberal (and wrong) in many of its reports
@@ -121,15 +159,18 @@ fn smoke_test_frames() {
 
         let mut resolved = 0;
         let can_resolve = LIBBACKTRACE || GIMLI_SYMBOLIZE;
+        let can_resolve_cols = GIMLI_SYMBOLIZE;
 
         let mut name = None;
         let mut addr = None;
+        let mut col  = None;
         let mut line = None;
         let mut file = None;
         backtrace::resolve_frame(frame, |sym| {
             resolved += 1;
             name = sym.name().map(|v| v.to_string());
             addr = sym.addr();
+            col  = sym.colno();
             line = sym.lineno();
             file = sym.filename().map(|v| v.to_path_buf());
         });
@@ -179,6 +220,18 @@ fn smoke_test_frames() {
                     expected_line
                 );
             }
+            if can_resolve_cols {
+                let col = col.expect("didn't find a column number");
+                if expected_col != 0 {
+                    assert!(
+                        col == expected_col,
+                        "bad column number on frame for `{}`: {} != {}",
+                        expected_name,
+                        col,
+                        expected_col
+                    );
+                }
+            }
         }
     }
 }