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

Added tracing of threads other than the calling thread on Windows #473

Closed
wants to merge 13 commits into from
Closed
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
1 change: 1 addition & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ jobs:
name: Test
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
include:
- os: ubuntu-latest
Expand Down
57 changes: 57 additions & 0 deletions examples/other_thread.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#[cfg(all(windows, not(target_vendor = "uwp"), feature = "std"))]
use backtrace::{Backtrace, BacktraceFrame};
#[cfg(all(windows, not(target_vendor = "uwp"), feature = "std"))]
use std::os::windows::prelude::AsRawHandle;

#[cfg(all(windows, not(target_vendor = "uwp"), feature = "std"))]
fn worker() {
foo();
}

#[cfg(all(windows, not(target_vendor = "uwp"), feature = "std"))]
fn foo() {
bar()
}

#[cfg(all(windows, not(target_vendor = "uwp"), feature = "std"))]
fn bar() {
baz()
}

#[cfg(all(windows, not(target_vendor = "uwp"), feature = "std"))]
fn baz() {
println!("Hello from thread!");
// Sleep for simple sync. Can't read thread that has finished running
std::thread::sleep(std::time::Duration::from_millis(1000));
loop {
print!("");
}
}

#[cfg(all(windows, not(target_vendor = "uwp"), feature = "std"))]
fn main() {
let thread = std::thread::spawn(|| {
worker();
});
let os_handle = thread.as_raw_handle();

// Allow the thread to start
std::thread::sleep(std::time::Duration::from_millis(100));

let mut frames = Vec::new();
unsafe {
backtrace::trace_thread_unsynchronized(os_handle, |frame| {
frames.push(BacktraceFrame::from(frame.clone()));
true
});
}

let mut bt = Backtrace::from(frames);
bt.resolve();
println!("{:?}", bt);
}

#[cfg(not(all(windows, not(target_vendor = "uwp"), feature = "std")))]
fn main() {
println!("This example is skipped on non-Windows or no-std platforms");
}
28 changes: 26 additions & 2 deletions src/backtrace/dbghelp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,36 @@ struct MyContext(CONTEXT);

#[inline(always)]
pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) {
let thread = GetCurrentThread();
trace_thread(cb, thread)
}

#[inline(always)]
pub unsafe fn trace_thread(cb: &mut dyn FnMut(&super::Frame) -> bool, thread: HANDLE) {
// Allocate necessary structures for doing the stack walk
let process = GetCurrentProcess();
let thread = GetCurrentThread();

let mut context = mem::zeroed::<MyContext>();
RtlCaptureContext(&mut context.0);
if thread == GetCurrentThread() || thread.is_null() {
RtlCaptureContext(&mut context.0);
} else {
// The suspending and resuming of threads is very risky.
// It can end in a deadlock if the current thread tries to access
// an object thats been locked by the suspended thread.
// That's why we only do as little work as possible while
// the thread is suspended, and resume it quickly after.
// There might be more pitfalls we haven't thought of or encountered
Comment on lines +106 to +111
Copy link
Member

Choose a reason for hiding this comment

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

No speculative "maybe this is bad?" in fresh code, please. I'd like to hear why this is actually okay, despite concerns about corrupting the backtrace.


context.0.ContextFlags = CONTEXT_ALL; // TODO: Narrow down required flags
Copy link
Member

Choose a reason for hiding this comment

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

This TODO would have to be addressed.

if SuspendThread(thread) as i32 == -1 {
ResumeThread(thread);
return;
}
let status = GetThreadContext(thread, &mut context.0);
if ResumeThread(thread) as i32 == -1 || status == 0 {
workingjubilee marked this conversation as resolved.
Show resolved Hide resolved
return;
}
}

// Ensure this process's symbols are initialized
let dbghelp = match dbghelp::init() {
Expand Down
28 changes: 28 additions & 0 deletions src/backtrace/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,33 @@ pub unsafe fn trace_unsynchronized<F: FnMut(&Frame) -> bool>(mut cb: F) {
trace_imp(&mut cb)
}

/// Similar to [trace_unsynchronized], but additionally, it supports tracing a thread other than the current thread.
///
/// It gets the traced thread's handle as its first argument.
///
/// # Safety
///
/// This function is intended for profiling and debugging.
///
/// If the given thread handle is not the current thread,
/// this function suspends the thread for a short amount of time to be able to extract the thread context.
/// This might end in a deadlock if the current thread tries to access an object thats been locked by the suspended thread.
Comment on lines +77 to +79
Copy link
Member

Choose a reason for hiding this comment

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

Are these statements about the briefness and the concern about deadlocking related to each other?

///
/// This function does not have synchronization guarantees but is available
/// when the `std` feature of this crate isn't compiled in. See the `trace`
/// function for more documentation and examples.
///
/// # Panics
///
/// See information on `trace` for caveats on `cb` panicking.
#[cfg(all(windows, not(target_vendor = "uwp")))]
pub unsafe fn trace_thread_unsynchronized<F: FnMut(&Frame) -> bool>(
thread: *mut c_void,
mut cb: F,
) {
trace_thread_imp(&mut cb, thread as super::windows::HANDLE);
}

/// A trait representing one frame of a backtrace, yielded to the `trace`
/// function of this crate.
///
Expand Down Expand Up @@ -151,6 +178,7 @@ cfg_if::cfg_if! {
} else if #[cfg(all(windows, not(target_vendor = "uwp")))] {
mod dbghelp;
use self::dbghelp::trace as trace_imp;
use self::dbghelp::trace_thread as trace_thread_imp;
pub(crate) use self::dbghelp::Frame as FrameImp;
#[cfg(target_env = "msvc")] // only used in dbghelp symbolize
pub(crate) use self::dbghelp::StackFrame;
Expand Down
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ extern crate std;
#[allow(unused_extern_crates)]
extern crate alloc;

#[cfg(all(windows, not(target_vendor = "uwp")))]
pub use self::backtrace::trace_thread_unsynchronized;

pub use self::backtrace::{trace_unsynchronized, Frame};
mod backtrace;

Expand Down
35 changes: 35 additions & 0 deletions src/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,9 @@ ffi! {
pub fn GetCurrentProcess() -> HANDLE;
pub fn GetCurrentThread() -> HANDLE;
pub fn RtlCaptureContext(ContextRecord: PCONTEXT) -> ();
pub fn GetThreadContext(ThreadHandle: HANDLE, ContextRecord: PCONTEXT) -> BOOL;
pub fn SuspendThread(ThreadHandle: HANDLE) -> DWORD;
pub fn ResumeThread(ThreadHandle: HANDLE) -> DWORD;
pub fn LoadLibraryA(a: *const i8) -> HMODULE;
pub fn GetProcAddress(h: HMODULE, name: *const i8) -> FARPROC;
pub fn GetModuleHandleA(name: *const i8) -> HMODULE;
Expand Down Expand Up @@ -518,6 +521,16 @@ ffi! {
pub struct ARM64_NT_NEON128 {
pub D: [f64; 2],
}

pub const CONTEXT_ARM64: DWORD = 0x00400000;
pub const CONTEXT_CONTROL: DWORD = CONTEXT_ARM64 | 0x00000001;
pub const CONTEXT_INTEGER: DWORD = CONTEXT_ARM64 | 0x00000002;
pub const CONTEXT_FLOATING_POINT: DWORD = CONTEXT_ARM64 | 0x00000004;
pub const CONTEXT_DEBUG_REGISTERS: DWORD = CONTEXT_ARM64 | 0x00000008;
pub const CONTEXT_X18: DWORD = CONTEXT_ARM64 | 0x00000010;
pub const CONTEXT_FULL: DWORD = CONTEXT_CONTROL | CONTEXT_INTEGER | CONTEXT_FLOATING_POINT;
pub const CONTEXT_ALL: DWORD = CONTEXT_CONTROL | CONTEXT_INTEGER | CONTEXT_FLOATING_POINT
| CONTEXT_DEBUG_REGISTERS | CONTEXT_X18;
}

#[cfg(target_arch = "x86")]
Expand Down Expand Up @@ -563,6 +576,18 @@ ffi! {
pub RegisterArea: [u8; 80],
pub Spare0: DWORD,
}

pub const CONTEXT_i386: DWORD = 0x00010000;
pub const CONTEXT_i486: DWORD = 0x00010000;
pub const CONTEXT_CONTROL: DWORD = CONTEXT_i386 | 0x00000001;
pub const CONTEXT_INTEGER: DWORD = CONTEXT_i386 | 0x00000002;
pub const CONTEXT_SEGMENTS: DWORD = CONTEXT_i386 | 0x00000004;
pub const CONTEXT_FLOATING_POINT: DWORD = CONTEXT_i386 | 0x00000008;
pub const CONTEXT_DEBUG_REGISTERS: DWORD = CONTEXT_i386 | 0x00000010;
pub const CONTEXT_EXTENDED_REGISTERS: DWORD = CONTEXT_i386 | 0x00000020;
pub const CONTEXT_FULL: DWORD = CONTEXT_CONTROL | CONTEXT_INTEGER | CONTEXT_SEGMENTS;
pub const CONTEXT_ALL: DWORD = CONTEXT_CONTROL | CONTEXT_INTEGER | CONTEXT_SEGMENTS
| CONTEXT_FLOATING_POINT | CONTEXT_DEBUG_REGISTERS | CONTEXT_EXTENDED_REGISTERS;
}

#[cfg(target_arch = "x86_64")]
Expand Down Expand Up @@ -630,6 +655,16 @@ ffi! {
pub Low: u64,
pub High: i64,
}

pub const CONTEXT_AMD64: DWORD = 0x00100000;
pub const CONTEXT_CONTROL: DWORD = CONTEXT_AMD64 | 0x00000001;
pub const CONTEXT_INTEGER: DWORD = CONTEXT_AMD64 | 0x00000002;
pub const CONTEXT_SEGMENTS: DWORD = CONTEXT_AMD64 | 0x00000004;
pub const CONTEXT_FLOATING_POINT: DWORD = CONTEXT_AMD64 | 0x00000008;
pub const CONTEXT_DEBUG_REGISTERS: DWORD = CONTEXT_AMD64 | 0x00000010;
Comment on lines +660 to +664
Copy link
Member

Choose a reason for hiding this comment

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

Can't we avoid repeating the same 5 expressions for 3 different architectures?

pub const CONTEXT_FULL: DWORD = CONTEXT_CONTROL | CONTEXT_INTEGER | CONTEXT_FLOATING_POINT;
pub const CONTEXT_ALL: DWORD = CONTEXT_CONTROL | CONTEXT_INTEGER | CONTEXT_SEGMENTS
| CONTEXT_FLOATING_POINT | CONTEXT_DEBUG_REGISTERS;
}

#[repr(C)]
Expand Down