From 25726598eb3e52788e2a2c83467b72d6126aedb8 Mon Sep 17 00:00:00 2001 From: Glauber Costa Date: Tue, 20 Apr 2021 20:52:57 -0400 Subject: [PATCH 1/4] implement From for BacktraceFrame There are situations where we can only capture raw `Frame` (example: capturing from a signal handler), but it could still be that we can process them later and are not fully nostd in the environment. With a conversion from Frame to BacktraceFrame, this is trivial. But without it, we can't really use the pretty printers as they are reliant on BacktraceFrame, not Frame. Fixes: #419 --- src/capture.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/capture.rs b/src/capture.rs index 9bd6ce907..26f3c0af0 100644 --- a/src/capture.rs +++ b/src/capture.rs @@ -249,6 +249,15 @@ impl From> for Backtrace { } } +impl From for BacktraceFrame { + fn from(frame: crate::Frame) -> BacktraceFrame { + BacktraceFrame { + frame: Frame::Raw(frame), + symbols: None, + } + } +} + impl Into> for Backtrace { fn into(self) -> Vec { self.frames From bfe591b934fe6c3363c6f78ef8a569acb53ee316 Mon Sep 17 00:00:00 2001 From: Glauber Costa Date: Fri, 23 Apr 2021 09:14:16 -0400 Subject: [PATCH 2/4] add test --- src/capture.rs | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/capture.rs b/src/capture.rs index 26f3c0af0..bf1255ab5 100644 --- a/src/capture.rs +++ b/src/capture.rs @@ -527,3 +527,42 @@ mod serde_impls { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_frame_conversion() { + // captures an original backtrace, and makes sure that the manual conversion + // to frames yields the same results. + let bt = Backtrace::new(); + let original_frames = bt.frames(); + + let mut frames = vec![]; + crate::trace(|frame| { + let converted = BacktraceFrame::from(frame.clone()); + frames.push(converted); + true + }); + + // the first frames can be different because we call from slightly different places, + // and the `trace` version has an extra capture. But because of inlining the number of + // frames that differ may be different between release and debug versions. Plus who knows + // what the compiler will do in the future. So we just take 4 frames from the end and make + // sure they match + for (converted, og) in frames + .iter() + .rev() + .take(4) + .zip(original_frames.iter().rev().take(4)) + { + println!( + "converted {:?}, og {:?}", + converted.symbol_address(), + og.symbol_address() + ); + assert_eq!(converted.symbol_address(), og.symbol_address()); + } + } +} From 455fa4e8b0fab57611dac512a9de1dba079ce8f0 Mon Sep 17 00:00:00 2001 From: Glauber Costa Date: Fri, 23 Apr 2021 15:03:26 -0400 Subject: [PATCH 3/4] fixes for miri Every backtrace that miri generates - conversion or no conversion, has totally different addresses. They all resolve to the same thing, though, so test with that. --- src/capture.rs | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/src/capture.rs b/src/capture.rs index bf1255ab5..93ec2af86 100644 --- a/src/capture.rs +++ b/src/capture.rs @@ -536,7 +536,8 @@ mod tests { fn test_frame_conversion() { // captures an original backtrace, and makes sure that the manual conversion // to frames yields the same results. - let bt = Backtrace::new(); + let mut bt = Backtrace::new(); + bt.resolve(); let original_frames = bt.frames(); let mut frames = vec![]; @@ -546,23 +547,42 @@ mod tests { true }); + let mut manual = Backtrace::from(frames); + manual.resolve(); + let frames = manual.frames(); + + let mut total_symbols = 0; // the first frames can be different because we call from slightly different places, // and the `trace` version has an extra capture. But because of inlining the number of // frames that differ may be different between release and debug versions. Plus who knows // what the compiler will do in the future. So we just take 4 frames from the end and make // sure they match + // + // For miri, every backtrace that is taken yields different addresses and different + // instruction pointers. That is irrespective of conversion and happens even if + // Backtrace::new() is invoked in a row. They all resolve to the same names, though, so to + // make sure this works on miri we resolve the symbols and compare the results. It is okay + // if some addresses don't have symbols, but if we scan enough frames at least some will do for (converted, og) in frames .iter() .rev() .take(4) .zip(original_frames.iter().rev().take(4)) { - println!( - "converted {:?}, og {:?}", - converted.symbol_address(), - og.symbol_address() - ); - assert_eq!(converted.symbol_address(), og.symbol_address()); + let converted_symbols = converted.symbols(); + let og_symbols = og.symbols(); + + assert_eq!(og_symbols.len(), converted_symbols.len()); + for (os, cs) in og_symbols.iter().zip(converted_symbols.iter()) { + total_symbols += 1; + assert_eq!( + os.name().map(|x| x.as_bytes()), + cs.name().map(|x| x.as_bytes()) + ); + assert_eq!(os.filename(), cs.filename()); + assert_eq!(os.lineno(), cs.lineno()); + } } + assert_ne!(total_symbols, 0); } } From 48b0141e9b55f09e30ec6867a6bb65d74755c49e Mon Sep 17 00:00:00 2001 From: Glauber Costa Date: Fri, 23 Apr 2021 15:44:27 -0400 Subject: [PATCH 4/4] fix another broken test `cargo test --no-default-features --features std` fails because frames are not resolved to symbols in that case. We'll just remove the assert, rather than disabling the whole test on that case. At least the code paths are stressed. --- src/capture.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/capture.rs b/src/capture.rs index 93ec2af86..713667dea 100644 --- a/src/capture.rs +++ b/src/capture.rs @@ -551,7 +551,6 @@ mod tests { manual.resolve(); let frames = manual.frames(); - let mut total_symbols = 0; // the first frames can be different because we call from slightly different places, // and the `trace` version has an extra capture. But because of inlining the number of // frames that differ may be different between release and debug versions. Plus who knows @@ -574,7 +573,6 @@ mod tests { assert_eq!(og_symbols.len(), converted_symbols.len()); for (os, cs) in og_symbols.iter().zip(converted_symbols.iter()) { - total_symbols += 1; assert_eq!( os.name().map(|x| x.as_bytes()), cs.name().map(|x| x.as_bytes()) @@ -583,6 +581,5 @@ mod tests { assert_eq!(os.lineno(), cs.lineno()); } } - assert_ne!(total_symbols, 0); } }