Skip to content

Commit bf73a46

Browse files
committed
gimli: Don't reallocate file/name information
Instead reuse the original storage in the original frame that we're iterating over, avoiding an extra copy/clone per frame we otherwise don't need.
1 parent 74b0c66 commit bf73a46

File tree

1 file changed

+67
-48
lines changed

1 file changed

+67
-48
lines changed

src/symbolize/gimli.rs

Lines changed: 67 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,16 @@
44
//! all platforms, but it's hoped to be developed over time! Long-term this is
55
//! intended to wholesale replace the `libbacktrace.rs` implementation.
66
7+
use self::gimli::read::EndianRcSlice;
8+
use self::gimli::RunTimeEndian;
79
use crate::symbolize::dladdr;
810
use crate::symbolize::ResolveWhat;
911
use crate::types::BytesOrWideString;
1012
use crate::SymbolName;
11-
use addr2line;
13+
use addr2line::gimli;
1214
use addr2line::object::{self, Object};
1315
use core::cell::RefCell;
16+
use core::convert::TryFrom;
1417
use core::mem;
1518
use core::u32;
1619
use findshlibs::{self, Segment, SharedLibrary};
@@ -243,17 +246,15 @@ pub unsafe fn resolve(what: ResolveWhat, cb: &mut FnMut(&super::Symbol)) {
243246
let mut found_sym = false;
244247
if let Ok(mut frames) = dwarf.find_frames(addr.0 as u64) {
245248
while let Ok(Some(frame)) = frames.next() {
246-
let (file, line) = frame
247-
.location
248-
.map(|l| (l.file, l.line))
249-
.unwrap_or((None, None));
250-
let name = frame
251-
.function
252-
.and_then(|f| f.raw_name().ok().map(|f| f.to_string()));
253-
let sym = super::Symbol {
254-
inner: Symbol::new(addr.0 as usize, file, line, name),
255-
};
256-
cb.call(&sym);
249+
let name = frame.function.as_ref().and_then(|f| f.raw_name().ok());
250+
let name = name.as_ref().map(|n| n.as_bytes() as *const _);
251+
cb.call(&super::Symbol {
252+
inner: Symbol::Frame {
253+
addr: addr.0 as *mut c_void,
254+
frame,
255+
name,
256+
},
257+
});
257258
found_sym = true;
258259
}
259260
}
@@ -262,7 +263,10 @@ pub unsafe fn resolve(what: ResolveWhat, cb: &mut FnMut(&super::Symbol)) {
262263
if !found_sym {
263264
if let Some(name) = symbols.get(addr.0 as u64).and_then(|x| x.name()) {
264265
let sym = super::Symbol {
265-
inner: Symbol::new(addr.0 as usize, None, None, Some(name.to_string())),
266+
inner: Symbol::Symbol {
267+
addr: addr.0 as usize as *mut c_void,
268+
name: name.as_bytes(),
269+
},
266270
};
267271
cb.call(&sym);
268272
}
@@ -301,67 +305,82 @@ impl Drop for DladdrFallback<'_, '_> {
301305
}
302306

303307
pub enum Symbol {
304-
Dladdr(dladdr::Symbol),
305-
Gimli {
306-
addr: usize,
307-
file: Option<String>,
308-
line: Option<u64>,
309-
name: Option<String>,
308+
/// We were able to locate frame information for this symbol, and
309+
/// `addr2line`'s frame internally has all the nitty gritty details.
310+
Frame {
311+
addr: *mut c_void,
312+
frame: addr2line::Frame<EndianRcSlice<RunTimeEndian>>,
313+
// Note that this would ideally be `&[u8]`, but we currently can't have
314+
// a lifetime parameter on this type. Eventually in the next breaking
315+
// change of this crate we should add a lifetime parameter to the
316+
// `Symbol` type in the top-level module, and then thread that lifetime
317+
// through to here.
318+
name: Option<*const [u8]>,
319+
},
320+
/// We weren't able to find anything in the debuginfo for this address, but
321+
/// we were able to find an entry into the symbol table for the symbol name.
322+
Symbol {
323+
addr: *mut c_void,
324+
// see note on `name` above
325+
name: *const [u8],
310326
},
327+
/// We weren't able to find anything in the original file, so we had to fall
328+
/// back to using `dladdr` which had a hit.
329+
Dladdr(dladdr::Symbol),
311330
}
312331

313332
impl Symbol {
314-
fn new(addr: usize, file: Option<String>, line: Option<u64>, name: Option<String>) -> Symbol {
315-
Symbol::Gimli {
316-
addr,
317-
file,
318-
line,
319-
name,
320-
}
321-
}
322-
323333
pub fn name(&self) -> Option<SymbolName> {
324334
match self {
325335
Symbol::Dladdr(s) => s.name(),
326-
Symbol::Gimli { name, .. } => name.as_ref().map(|s| SymbolName::new(s.as_bytes())),
336+
Symbol::Frame { name, .. } => {
337+
let name = name.as_ref()?;
338+
unsafe { Some(SymbolName::new(&**name)) }
339+
}
340+
Symbol::Symbol { name, .. } => unsafe { Some(SymbolName::new(&**name)) },
327341
}
328342
}
329343

330344
pub fn addr(&self) -> Option<*mut c_void> {
331345
match self {
332346
Symbol::Dladdr(s) => s.addr(),
333-
Symbol::Gimli { addr, .. } => Some(*addr as *mut c_void),
347+
Symbol::Frame { addr, .. } => Some(*addr),
348+
Symbol::Symbol { addr, .. } => Some(*addr),
334349
}
335350
}
336351

337352
pub fn filename_raw(&self) -> Option<BytesOrWideString> {
338-
let file = match self {
353+
match self {
339354
Symbol::Dladdr(s) => return s.filename_raw(),
340-
Symbol::Gimli { file, .. } => file,
341-
};
342-
file.as_ref()
343-
.map(|f| BytesOrWideString::Bytes(f.as_bytes()))
355+
Symbol::Frame { frame, .. } => {
356+
let location = frame.location.as_ref()?;
357+
let file = location.file.as_ref()?;
358+
Some(BytesOrWideString::Bytes(file.as_bytes()))
359+
}
360+
Symbol::Symbol { .. } => None,
361+
}
344362
}
345363

346364
pub fn filename(&self) -> Option<&Path> {
347-
let file = match self {
365+
match self {
348366
Symbol::Dladdr(s) => return s.filename(),
349-
Symbol::Gimli { file, .. } => file,
350-
};
351-
file.as_ref().map(Path::new)
367+
Symbol::Frame { frame, .. } => {
368+
let location = frame.location.as_ref()?;
369+
let file = location.file.as_ref()?;
370+
Some(Path::new(file))
371+
}
372+
Symbol::Symbol { .. } => None,
373+
}
352374
}
353375

354376
pub fn lineno(&self) -> Option<u32> {
355-
let line = match self {
377+
match self {
356378
Symbol::Dladdr(s) => return s.lineno(),
357-
Symbol::Gimli { line, .. } => line,
358-
};
359-
line.and_then(|l| {
360-
if l > (u32::MAX as u64) {
361-
None
362-
} else {
363-
Some(l as u32)
379+
Symbol::Frame { frame, .. } => {
380+
let location = frame.location.as_ref()?;
381+
location.line.and_then(|l| u32::try_from(l).ok())
364382
}
365-
})
383+
Symbol::Symbol { .. } => None,
384+
}
366385
}
367386
}

0 commit comments

Comments
 (0)