Include -1 logic on Unix for libbacktrace

After tweaking where all the `-1` annotations are located this is
hopefully the final central location they need to be in... In any case
this also imports the test from libstd which is typically failing
upstream so we can get earlier signals to failures here.

Author: alexcrichton

Cargo.toml

@@ -143,3 +143,8 @@ required-features = ["std"]
 name = "smoke"
 required-features = ["std"]
 edition = '2018'
+
+[[test]]
+name = "accuracy"
+required-features = ["std", "dbghelp", "libbacktrace", "libunwind"]
+edition = '2018'

src/symbolize/libbacktrace.rs

@@ -413,15 +413,20 @@ unsafe fn init_state() -> *mut bt::backtrace_state {
 pub unsafe fn resolve(what: ResolveWhat, cb: &mut FnMut(&super::Symbol)) {
     let mut symaddr = what.address_or_ip() as usize;
 
-    // It's sort of unclear why this is necessary, but it appears that the ip
-    // values from stack traces are typically the instruction *after* the call
-    // that's the actual stack trace. Symbolizing this on Windows causes the
-    // filename/line number to be one ahead and perhaps into the void if it's
-    // near the end of the function. Apparently on Unix though it's roughly fine
-    // in that the filename/line number turn out alright. For now just try to
-    // get good backtraces with this, and hopefully one day we can figure out
-    // why the `-=1` is here.
-    if cfg!(windows) && symaddr > 0 {
+    // 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
+    // the void if it's near the end of the function.
+    //
+    // On Windows it's pretty sure that it's always the case that the IP is one
+    // ahead (except for the final frame but oh well) and for Unix it appears
+    // that we used to use `_Unwind_GetIPInfo` which tells us if the instruction
+    // is the next or not, but it seems that on all platforms it's always the
+    // next instruction so far so let's just always assume that.
+    //
+    // In any case we'll probably have to tweak this over time, but for now this
+    // gives the most accurate backtraces.
+    if symaddr > 0 {
         symaddr -= 1;
     }
 

tests/accuracy/aux.rs

@@ -0,0 +1,11 @@
+#[inline(never)]
+pub fn callback<F>(f: F) where F: FnOnce((&'static str, u32)) {
+    f((file!(), line!()))
+}
+
+// We emit the wrong location for the caller here when inlined on MSVC
+#[cfg_attr(not(target_env = "msvc"), inline(always))]
+#[cfg_attr(target_env = "msvc", inline(never))]
+pub fn callback_inlined<F>(f: F) where F: FnOnce((&'static str, u32)) {
+    f((file!(), line!()))
+}

tests/accuracy/main.rs

@@ -0,0 +1,96 @@
+use std::path::Path;
+
+mod aux;
+
+macro_rules! pos {
+    () => {
+        (file!(), line!())
+    };
+}
+
+macro_rules! check {
+    ($($pos:expr),*) => ({
+        verify(&[$($pos),*]);
+    })
+}
+
+type Pos = (&'static str, u32);
+
+#[test]
+fn doit() {
+    outer(pos!());
+}
+
+#[inline(never)]
+fn outer(main_pos: Pos) {
+    inner(main_pos, pos!());
+    inner_inlined(main_pos, pos!());
+}
+
+#[inline(never)]
+#[rustfmt::skip]
+fn inner(main_pos: Pos, outer_pos: Pos) {
+    check!(main_pos, outer_pos);
+    check!(main_pos, outer_pos);
+    let inner_pos = pos!(); aux::callback(|aux_pos| {
+        check!(main_pos, outer_pos, inner_pos, aux_pos);
+    });
+    let inner_pos = pos!(); aux::callback_inlined(|aux_pos| {
+        check!(main_pos, outer_pos, inner_pos, aux_pos);
+    });
+}
+
+// We emit the wrong location for the caller here when inlined on MSVC
+#[cfg_attr(not(target_env = "msvc"), inline(always))]
+#[cfg_attr(target_env = "msvc", inline(never))]
+#[rustfmt::skip]
+fn inner_inlined(main_pos: Pos, outer_pos: Pos) {
+    check!(main_pos, outer_pos);
+    check!(main_pos, outer_pos);
+
+    // Again, disable inlining for MSVC.
+    #[cfg_attr(not(target_env = "msvc"), inline(always))]
+    #[cfg_attr(target_env = "msvc", inline(never))]
+    fn inner_further_inlined(main_pos: Pos, outer_pos: Pos, inner_pos: Pos) {
+        check!(main_pos, outer_pos, inner_pos);
+    }
+    inner_further_inlined(main_pos, outer_pos, pos!());
+
+    let inner_pos = pos!(); aux::callback(|aux_pos| {
+        check!(main_pos, outer_pos, inner_pos, aux_pos);
+    });
+    let inner_pos = pos!(); aux::callback_inlined(|aux_pos| {
+        check!(main_pos, outer_pos, inner_pos, aux_pos);
+    });
+
+    // this tests a distinction between two independent calls to the inlined function.
+    // (un)fortunately, LLVM somehow merges two consecutive such calls into one node.
+    inner_further_inlined(main_pos, outer_pos, pos!());
+}
+
+fn verify(filelines: &[Pos]) {
+    let trace = backtrace::Backtrace::new();
+    println!("-----------------------------------");
+    println!("looking for:");
+    for (file, line) in filelines.iter().rev() {
+        println!("\t{}:{}", file, line);
+    }
+    println!("found:\n{:?}", trace);
+    let mut symbols = trace.frames().iter().flat_map(|frame| frame.symbols());
+    let mut iter = filelines.iter().rev();
+    while let Some((file, line)) = iter.next() {
+        loop {
+            let sym = match symbols.next() {
+                Some(sym) => sym,
+                None => panic!("failed to find {}:{}", file, line),
+            };
+            if let Some(filename) = sym.filename() {
+                if let Some(lineno) = sym.lineno() {
+                    if filename == Path::new(file) && lineno == *line {
+                        break;
+                    }
+                }
+            }
+        }
+    }
+}