Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test cleanup for nightly #641

Merged
merged 5 commits into from
Jul 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 110 additions & 15 deletions src/capture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use crate::resolve;
use crate::PrintFmt;
use crate::{resolve_frame, trace, BacktraceFmt, Symbol, SymbolName};
use std::ffi::c_void;
use core::ffi::c_void;
use std::fmt;
use std::path::{Path, PathBuf};
use std::prelude::v1::*;
Expand All @@ -29,6 +29,101 @@ pub struct Backtrace {
frames: Vec<BacktraceFrame>,
}

#[derive(Clone, Copy)]
struct TracePtr(*mut c_void);
/// SAFETY: These pointers are always valid within a process and are not used for mutation.
unsafe impl Send for TracePtr {}
/// SAFETY: These pointers are always valid within a process and are not used for mutation.
unsafe impl Sync for TracePtr {}

impl TracePtr {
fn into_void(self) -> *mut c_void {
self.0
}
#[cfg(feature = "serde")]
fn from_addr(addr: usize) -> Self {
TracePtr(addr as *mut c_void)
}
}

#[cfg(feature = "serde")]
impl<'de> Deserialize<'de> for TracePtr {
#[inline]
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
struct PrimitiveVisitor;

impl<'de> serde::de::Visitor<'de> for PrimitiveVisitor {
type Value = TracePtr;

fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
formatter.write_str("usize")
}

#[inline]
fn visit_u8<E>(self, v: u8) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
Ok(TracePtr(v as usize as *mut c_void))
}

#[inline]
fn visit_u16<E>(self, v: u16) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
Ok(TracePtr(v as usize as *mut c_void))
}

#[inline]
fn visit_u32<E>(self, v: u32) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
if usize::BITS >= 32 {
Ok(TracePtr(v as usize as *mut c_void))
} else {
Err(E::invalid_type(
serde::de::Unexpected::Unsigned(v as _),
&self,
))
}
}

#[inline]
fn visit_u64<E>(self, v: u64) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
if usize::BITS >= 64 {
Ok(TracePtr(v as usize as *mut c_void))
} else {
Err(E::invalid_type(
serde::de::Unexpected::Unsigned(v as _),
&self,
))
}
}
}

deserializer.deserialize_u64(PrimitiveVisitor)
}
}

#[cfg(feature = "serde")]
impl Serialize for TracePtr {
#[inline]
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::ser::Serializer,
{
serializer.serialize_u64(self.0 as usize as u64)
}
}

fn _assert_send_sync() {
fn _assert<T: Send + Sync>() {}
_assert::<Backtrace>();
Expand All @@ -54,9 +149,9 @@ enum Frame {
Raw(crate::Frame),
#[cfg(feature = "serde")]
Deserialized {
ip: usize,
symbol_address: usize,
module_base_address: Option<usize>,
ip: TracePtr,
symbol_address: TracePtr,
module_base_address: Option<TracePtr>,
},
}

Expand All @@ -65,15 +160,15 @@ impl Frame {
match *self {
Frame::Raw(ref f) => f.ip(),
#[cfg(feature = "serde")]
Frame::Deserialized { ip, .. } => ip as *mut c_void,
Frame::Deserialized { ip, .. } => ip.into_void(),
}
}

fn symbol_address(&self) -> *mut c_void {
match *self {
Frame::Raw(ref f) => f.symbol_address(),
#[cfg(feature = "serde")]
Frame::Deserialized { symbol_address, .. } => symbol_address as *mut c_void,
Frame::Deserialized { symbol_address, .. } => symbol_address.into_void(),
}
}

Expand All @@ -84,7 +179,7 @@ impl Frame {
Frame::Deserialized {
module_base_address,
..
} => module_base_address.map(|addr| addr as *mut c_void),
} => module_base_address.map(|addr| addr.into_void()),
}
}

Expand All @@ -94,7 +189,7 @@ impl Frame {
let sym = |symbol: &Symbol| {
symbols.push(BacktraceSymbol {
name: symbol.name().map(|m| m.as_bytes().to_vec()),
addr: symbol.addr().map(|a| a as usize),
addr: symbol.addr().map(TracePtr),
filename: symbol.filename().map(|m| m.to_owned()),
lineno: symbol.lineno(),
colno: symbol.colno(),
Expand All @@ -104,7 +199,7 @@ impl Frame {
Frame::Raw(ref f) => resolve_frame(f, sym),
#[cfg(feature = "serde")]
Frame::Deserialized { ip, .. } => {
resolve(ip as *mut c_void, sym);
resolve(ip.into_void(), sym);
}
}
symbols
Expand All @@ -124,7 +219,7 @@ impl Frame {
#[cfg_attr(feature = "serde", derive(Deserialize, Serialize))]
pub struct BacktraceSymbol {
name: Option<Vec<u8>>,
addr: Option<usize>,
addr: Option<TracePtr>,
filename: Option<PathBuf>,
lineno: Option<u32>,
colno: Option<u32>,
Expand Down Expand Up @@ -347,7 +442,7 @@ impl BacktraceSymbol {
/// This function requires the `std` feature of the `backtrace` crate to be
/// enabled, and the `std` feature is enabled by default.
pub fn addr(&self) -> Option<*mut c_void> {
self.addr.map(|s| s as *mut c_void)
self.addr.map(|s| s.into_void())
}

/// Same as `Symbol::filename`
Expand Down Expand Up @@ -468,7 +563,7 @@ mod serde_impls {
SerializedFrame {
ip: frame.ip() as usize,
symbol_address: frame.symbol_address() as usize,
module_base_address: frame.module_base_address().map(|addr| addr as usize),
module_base_address: frame.module_base_address().map(|sym_a| sym_a as usize),
symbols: symbols.clone(),
}
.serialize(s)
Expand All @@ -483,9 +578,9 @@ mod serde_impls {
let frame: SerializedFrame = SerializedFrame::deserialize(d)?;
Ok(BacktraceFrame {
frame: Frame::Deserialized {
ip: frame.ip,
symbol_address: frame.symbol_address,
module_base_address: frame.module_base_address,
ip: TracePtr::from_addr(frame.ip),
symbol_address: TracePtr::from_addr(frame.symbol_address),
module_base_address: frame.module_base_address.map(TracePtr::from_addr),
},
symbols: frame.symbols,
})
Expand Down
5 changes: 2 additions & 3 deletions src/dbghelp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,8 @@ macro_rules! dbghelp {
})*

pub fn dbghelp(&self) -> *mut Dbghelp {
unsafe {
ptr::addr_of_mut!(DBGHELP)
}
#[allow(unused_unsafe)]
unsafe { ptr::addr_of_mut!(DBGHELP) }
ChrisDenton marked this conversation as resolved.
Show resolved Hide resolved
}
}
)
Expand Down
6 changes: 3 additions & 3 deletions tests/concurrent-panics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ fn parent() {
fn child() {
let done = Arc::new(AtomicBool::new(false));
let done2 = done.clone();
let a = thread::spawn(move || {
while !done2.load(SeqCst) {
format!("{:?}", backtrace::Backtrace::new());
let a = thread::spawn(move || loop {
if done2.load(SeqCst) {
break format!("{:?}", backtrace::Backtrace::new());
Comment on lines -48 to +50
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new version looks right to me. Or at least I do not understand why the old version was doing that.

}
});

Expand Down
13 changes: 8 additions & 5 deletions tests/skip_inner_frames.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use backtrace::Backtrace;
use core::ffi::c_void;

// This test only works on platforms which have a working `symbol_address`
// function for frames which reports the starting address of a symbol. As a
Expand All @@ -12,6 +13,7 @@ const ENABLED: bool = cfg!(all(
));

#[test]
#[inline(never)]
fn backtrace_new_unresolved_should_start_with_call_site_trace() {
if !ENABLED {
return;
Expand All @@ -22,13 +24,14 @@ fn backtrace_new_unresolved_should_start_with_call_site_trace() {

assert!(!b.frames().is_empty());

let this_ip = backtrace_new_unresolved_should_start_with_call_site_trace as usize;
println!("this_ip: {:?}", this_ip as *const usize);
let frame_ip = b.frames().first().unwrap().symbol_address() as usize;
let this_ip = backtrace_new_unresolved_should_start_with_call_site_trace as *mut c_void;
println!("this_ip: {:p}", this_ip);
let frame_ip = b.frames().first().unwrap().symbol_address();
assert_eq!(this_ip, frame_ip);
}

#[test]
#[inline(never)]
fn backtrace_new_should_start_with_call_site_trace() {
if !ENABLED {
return;
Expand All @@ -38,7 +41,7 @@ fn backtrace_new_should_start_with_call_site_trace() {

assert!(!b.frames().is_empty());

let this_ip = backtrace_new_should_start_with_call_site_trace as usize;
let frame_ip = b.frames().first().unwrap().symbol_address() as usize;
let this_ip = backtrace_new_should_start_with_call_site_trace as *mut c_void;
let frame_ip = b.frames().first().unwrap().symbol_address();
assert_eq!(this_ip, frame_ip);
}
Loading
Loading