Skip to content

Commit ac45e72

Browse files
committed
Favor debuginfo function names instead of symbol table
This commit fixes an accidental regression with #185 where libbacktrace was updated to match the standard library by always returning the symbol table symbol. Turns out though this library was one-upping the standard library by consulting debuginfo (what it was previously doing) because debuginfo can have more accurate symbol information for inline functions, for example. The fix here is to always prefer the debuginfo function name, if present, and otherwise fall back to the symbol table symbol if necessary. Closes #186
1 parent 0c124d6 commit ac45e72

File tree

1 file changed

+31
-23
lines changed

1 file changed

+31
-23
lines changed

src/symbolize/libbacktrace.rs

+31-23
Original file line numberDiff line numberDiff line change
@@ -31,24 +31,41 @@ pub enum Symbol {
3131
filename: *const c_char,
3232
lineno: c_int,
3333
function: *const c_char,
34+
symname: *const c_char,
3435
},
3536
}
3637

3738
impl Symbol {
3839
pub fn name(&self) -> Option<SymbolName> {
39-
let ptr = match *self {
40-
Symbol::Syminfo { symname, .. } => symname,
41-
Symbol::Pcinfo { function, .. } => function,
42-
};
43-
if ptr.is_null() {
44-
None
45-
} else {
40+
let symbol = |ptr: *const c_char| {
4641
unsafe {
47-
let len = libc::strlen(ptr);
48-
Some(SymbolName::new(slice::from_raw_parts(
49-
ptr as *const u8,
50-
len,
51-
)))
42+
if ptr.is_null() {
43+
None
44+
} else {
45+
let len = libc::strlen(ptr);
46+
Some(SymbolName::new(slice::from_raw_parts(
47+
ptr as *const u8,
48+
len,
49+
)))
50+
}
51+
}
52+
};
53+
match *self {
54+
Symbol::Syminfo { symname, .. } => symbol(symname),
55+
Symbol::Pcinfo { function, symname, .. } => {
56+
// If possible prefer the `function` name which comes from
57+
// debuginfo and can typically be more accurate for inline
58+
// frames for example. If that's not present though fall back to
59+
// the symbol table name specified in `symname`.
60+
//
61+
// Note that sometimes `function` can feel somewhat less
62+
// accurate, for example being listed as `try<i32,closure>`
63+
// isntead of `std::panicking::try::do_call`. It's not really
64+
// clear why, but overall the `function` name seems more accurate.
65+
if let Some(sym) = symbol(function) {
66+
return Some(sym)
67+
}
68+
symbol(symname)
5269
}
5370
}
5471
}
@@ -187,17 +204,8 @@ extern "C" fn pcinfo_cb(
187204
pc: pc,
188205
filename: filename,
189206
lineno: lineno,
190-
191-
// Favor the function name going into the `syminfo_cb`. That's
192-
// typically from the symbol table and is the full symbol name
193-
// whereas the `function` above is coming from debuginfo which
194-
// isn't always as descriptive when considering the whole
195-
// program.
196-
function: if !state.symname.is_null() {
197-
state.symname
198-
} else {
199-
function
200-
},
207+
symname: state.symname,
208+
function,
201209
},
202210
});
203211
bomb.enabled = false;

0 commit comments

Comments
 (0)