From adb7acc716e7daadd32c8280017c307de29043ba Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Thu, 24 Mar 2022 14:47:28 +0100 Subject: [PATCH 01/36] Add first pass at windows minidump writer --- Cargo.toml | 15 ++ src/lib.rs | 4 + src/windows.rs | 2 + src/windows/errors.rs | 7 + src/windows/minidump_writer.rs | 332 +++++++++++++++++++++++++++++++++ 5 files changed, 360 insertions(+) create mode 100644 src/windows.rs create mode 100644 src/windows/errors.rs create mode 100644 src/windows/minidump_writer.rs diff --git a/Cargo.toml b/Cargo.toml index 6beb65e0..66026a2f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,9 +23,24 @@ memmap2 = "0.5" [target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies] nix = "0.23" +[target.'cfg(target_os = "windows")'.dependencies.windows-sys] +version = "0.34" +features = [ + # MiniDumpWriteDump requires...a lot of features + "Win32_Foundation", + "Win32_Storage_FileSystem", + "Win32_System_Diagnostics_Debug", + "Win32_System_Kernel", + "Win32_System_Memory", + # VerifierEnumerateResource and friends + "Win32_System_ApplicationVerifier", +] + [dev-dependencies] minidump = "0.10" [patch.crates-io] minidump = { git = "https://github.com/rust-minidump/rust-minidump" } minidump-common = { git = "https://github.com/rust-minidump/rust-minidump" } +#crash-context = { git = "https://github.com/EmbarkStudios/crash-handling", branch = "main" } +crash-context = { path = "../crash-handling/crash-context" } diff --git a/src/lib.rs b/src/lib.rs index 8a5ea122..ab803757 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,6 +3,10 @@ cfg_if::cfg_if! { mod linux; pub use linux::*; + } else if #[cfg(target_os = "windows")] { + mod windows; + + pub use windows::*; } } diff --git a/src/windows.rs b/src/windows.rs new file mode 100644 index 00000000..1ae7a276 --- /dev/null +++ b/src/windows.rs @@ -0,0 +1,2 @@ +pub mod errors; +pub mod minidump_writer; diff --git a/src/windows/errors.rs b/src/windows/errors.rs new file mode 100644 index 00000000..f85e9752 --- /dev/null +++ b/src/windows/errors.rs @@ -0,0 +1,7 @@ +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error(transparent)] + Io(#[from] std::io::Error), + #[error(transparent)] + Scroll(#[from] scroll::Error), +} diff --git a/src/windows/minidump_writer.rs b/src/windows/minidump_writer.rs new file mode 100644 index 00000000..ecdf4f67 --- /dev/null +++ b/src/windows/minidump_writer.rs @@ -0,0 +1,332 @@ +use crate::windows::errors::Error; +use minidump_common::format::{BreakpadInfoValid, MINIDUMP_BREAKPAD_INFO, MINIDUMP_STREAM_TYPE}; +use scroll::Pwrite; +use std::{ffi::c_void, os::windows::io::AsRawHandle}; +pub use windows_sys::Win32::Foundation::HANDLE; +use windows_sys::Win32::{ + Foundation::{CloseHandle, ERROR_SUCCESS, STATUS_INVALID_HANDLE}, + System::{ + ApplicationVerifier as av, + Diagnostics::Debug as md, + Threading::{GetCurrentThreadId, OpenProcess}, + }, +}; + +pub struct MinidumpWriter { + /// The crash context as captured by an exception handler + crash_context: crash_context::CrashContext, + /// Handle to the crashing process, which could be ourselves + crashing_process: HANDLE, + /// The pid of the crashing process. + crashing_pid: u32, + /// The `EXCEPTION_POINTERS` contained in crash context is a pointer into the + /// memory of the process that crashed, as it contains an `EXCEPTION_RECORD` + /// record which is an internally linked list, so in the case that we are + /// dumping a process other than the current one, we need to tell + /// MiniDumpWriteDump that the pointers come from an external process so that + /// it can use eg ReadProcessMemory to get the contextual information from + /// the crash, rather than from the current process + is_external_process: bool, +} + +impl MinidumpWriter { + /// Creates a minidump writer for a crash that occurred in an external process. + pub fn external_process( + crash_context: crash_context::CrashContext, + pid: u32, + proc_handle: HANDLE, + ) -> Self { + Self { + crash_context, + crashing_process: proc_handle, + crashing_pid: pid, + is_external_process: true, + } + } + + /// Creates a minidump writer for a crash that occurred in the current process. + /// + /// # Errors + /// + /// Fails if we are unable to open a `HANDLE` to the current process + pub fn current_process(crash_context: crash_context::CrashContext) -> Result { + let crashing_pid = std::process::id(); + + // SAFETY: syscall + let crashing_process = unsafe { + OpenProcess( + 268435456, // desired access - GENERIC_ALL - for some reason this is defined in SystemServices which is massive and not worth bringing in for ONE constant + 0, // inherit handles + crashing_pid, + ) + }; + + if crashing_process == 0 { + Err(std::io::Error::last_os_error().into()) + } else { + Ok(Self { + crash_context, + crashing_process, + crashing_pid, + is_external_process: false, + }) + } + } + + /// Writes a minidump to the specified file + pub fn dump(&self, destination: &mut std::fs::File) -> Result<(), Error> { + let exc_info = if !self.crash_context.exception_pointers.is_null() { + // https://docs.microsoft.com/en-us/windows/win32/api/minidumpapiset/ns-minidumpapiset-minidump_exception_information + Some(md::MINIDUMP_EXCEPTION_INFORMATION { + ThreadId: self.crash_context.thread_id, + // This is a mut pointer for some reason...I don't _think_ it is + // actually mut in practice...? + ExceptionPointers: self.crash_context.exception_pointers as *mut _, + ClientPointers: if self.is_external_process { 1 } else { 0 }, + }) + } else { + None + }; + + // This is a bit dangerous if doing in-process dumping, but that's not + // (currently) a real target of this crate, so this allocation is fine + let mut user_streams = Vec::with_capacity(3); + + // Add an MDRawBreakpadInfo stream to the minidump, to provide additional + // information about the exception handler to the Breakpad processor. + // The information will help the processor determine which threads are + // relevant. The Breakpad processor does not require this information but + // can function better with Breakpad-generated dumps when it is present. + // The native debugger is not harmed by the presence of this information. + // + // This info is only relevant for in-process dumping + let mut breakpad_info = [0u8; 12]; + if !self.is_external_process { + let bp_info = MINIDUMP_BREAKPAD_INFO { + validity: BreakpadInfoValid::DumpThreadId.bits() + | BreakpadInfoValid::RequestingThreadId.bits(), + dump_thread_id: self.crash_context.thread_id, + // Safety: syscall + requesting_thread_id: unsafe { GetCurrentThreadId() }, + }; + + // TODO: derive Pwrite for MINIDUMP_BREAKPAD_INFO + let mut offset = 0; + offset += breakpad_info.pwrite(bp_info.validity, offset)?; + offset += breakpad_info.pwrite(bp_info.dump_thread_id, offset)?; + breakpad_info.pwrite(bp_info.requesting_thread_id, offset)?; + + user_streams.push(md::MINIDUMP_USER_STREAM { + Type: MINIDUMP_STREAM_TYPE::BreakpadInfoStream as u32, + BufferSize: breakpad_info.len() as u32, + // Again with the mut pointer + Buffer: breakpad_info.as_mut_ptr().cast(), + }); + } + + // When dumping an external process we want to retrieve the actual contents + // of the assertion info and add it as a user stream, but we need to + // keep the memory alive for the duration of the write + // SAFETY: POD + let mut assertion_info: crash_context::RawAssertionInfo = unsafe { std::mem::zeroed() }; + + if let Some(ai) = self.crash_context.assertion_info { + let ai_ptr = if self.is_external_process { + // Even though this information is useful for non-exceptional dumps + // (purecall, invalid parameter), we don't treat it is a critical + // failure if we can't read it (unlike Breakpad) since we will still + // have the synthetic exception context that was generated which + // indicates the kind (again, purecall or invalid parameter), and + // realistically, the assertion information is going to fairly pointless + // anyways (at least for invalid parameters) since the information + // supplied to the handler is only going to be filled in if using + // the debug MSVCRT, which you can only realistically do in dev + // environments since the debug MSVCRT is not redistributable. + let mut assert_info = + std::mem::MaybeUninit::::uninit(); + let mut bytes_read = 0; + + // SAFETY: syscall + if unsafe { + md::ReadProcessMemory( + self.crashing_process, // client process handle to read the memory from + ai.cast(), // the pointer to read from the client process + assert_info.as_mut_ptr().cast(), // The buffer we're filling with the memory + std::mem::size_of::(), + &mut bytes_read, + ) + } == 0 + { + // log::error!( + // "failed to read assertion information from client: {}", + // last_os_error() + // ); + std::ptr::null() + } else if bytes_read != std::mem::size_of::() { + // log::error!( + // "read invalid number of bytes: expected {} != received {}", + // std::mem::size_of::(), + // bytes_read + // ); + std::ptr::null() + } else { + // SAFETY: this is fine as lone as Windows didn't lie to us + assertion_info = unsafe { assert_info.assume_init() }; + + &assertion_info + } + } else { + ai + }; + + if !ai.is_null() { + user_streams.push(md::MINIDUMP_USER_STREAM { + Type: MINIDUMP_STREAM_TYPE::AssertionInfoStream as u32, + BufferSize: std::mem::size_of::() as u32, + // Again with the mut pointer + Buffer: (ai_ptr as *mut crash_context::RawAssertionInfo).cast(), + }); + } + } + + let handle_stream = self.fill_handle_stream(); + + // Note that we do this by ref, as the buffer inside the option needs + // to stay alive for as long as we're writing the minidump since + // the user stream has a pointer to it + if let Some((_buf, handle_stream)) = &handle_stream { + user_streams.push(*handle_stream); + } + + let user_stream = md::MINIDUMP_USER_STREAM_INFORMATION { + UserStreamCount: user_streams.len() as u32, + UserStreamArray: user_streams.as_mut_ptr(), + }; + + // Write the actual minidump + // https://docs.microsoft.com/en-us/windows/win32/api/minidumpapiset/nf-minidumpapiset-minidumpwritedump + // SAFETY: syscall + let ret = unsafe { + md::MiniDumpWriteDump( + self.crashing_process, // HANDLE to the process with the crash we want to capture + self.crashing_pid, // process id + destination.as_raw_handle() as HANDLE, // file to write the minidump to + md::MiniDumpNormal, // MINIDUMP_TYPE - we _might_ want to make this configurable + exc_info + .as_ref() + .map_or(std::ptr::null(), |ei| ei as *const _), // exceptionparam - the actual exception information + &user_stream, // user streams + std::ptr::null(), // callback, unused + ) + }; + + if ret == 0 { + Err(std::io::Error::last_os_error().into()) + } else { + Ok(()) + } + } + + /// In the case of a `STATUS_INVALID_HANDLE` exception, this function + /// enumerates all of the handle operations that occurred within the crashing + /// process and fills out a minidump user stream with the ops pertaining to + /// the last invalid handle that is enumerated, which is, presumably + /// (hopfully?), the one that led to the exception + fn fill_handle_stream(&self) -> Option<(Vec, md::MINIDUMP_USER_STREAM)> { + if self.crash_context.exception_code != STATUS_INVALID_HANDLE { + return None; + } + + // State object we pass to the enumeration + struct HandleState { + ops: Vec, + last_invalid: u64, + } + + unsafe extern "system" fn enum_callback( + resource_description: *mut c_void, + enumeration_context: *mut c_void, + enumeration_level: *mut u32, + ) -> u32 { + let description = &*resource_description.cast::(); + let mut hs = &mut *enumeration_context.cast::(); + + // Remember the last invalid handle operation. + if description.OperationType == av::OperationDbBADREF as u32 { + hs.last_invalid = description.Handle; + } + + // Record all handle operations. + hs.ops.push(*description); + *enumeration_level = av::HeapEnumerationEverything as u32; + ERROR_SUCCESS + } + + let mut hs = HandleState { + ops: Vec::new(), + last_invalid: 0, + }; + + // https://docs.microsoft.com/en-us/windows/win32/api/avrfsdk/nf-avrfsdk-verifierenumerateresource + // SAFETY: syscall + if unsafe { + av::VerifierEnumerateResource( + self.crashing_process, // process to enumerate the handles for + 0, // flags + av::AvrfResourceHandleTrace, // resource typea, we want to trace handles + Some(enum_callback), // enumeration callback + (&mut hs as *mut HandleState).cast(), // enumeration context + ) + } != 0 + { + let mut stream_buf = Vec::new(); + + // https://docs.microsoft.com/en-us/windows/win32/api/minidumpapiset/ns-minidumpapiset-minidump_handle_operation_list + let mut md_list = md::MINIDUMP_HANDLE_OPERATION_LIST { + SizeOfHeader: std::mem::size_of::() as u32, + SizeOfEntry: std::mem::size_of::() as u32, + NumberOfEntries: 0, + Reserved: 0, + }; + + stream_buf.resize(md_list.SizeOfHeader as usize, 0); + + #[inline] + fn to_bytes(v: &T) -> &[u8] { + let s = std::slice::from_ref(v); + + unsafe { std::slice::from_raw_parts(s.as_ptr().cast(), std::mem::size_of::()) } + } + + for op in hs.ops.into_iter().filter(|op| op.Handle == hs.last_invalid) { + stream_buf.extend_from_slice(to_bytes(&op)); + md_list.NumberOfEntries += 1; + } + + stream_buf[..md_list.SizeOfHeader as usize].copy_from_slice(to_bytes(&md_list)); + + let handle_stream = md::MINIDUMP_USER_STREAM { + Type: MINIDUMP_STREAM_TYPE::HandleOperationListStream as u32, + BufferSize: stream_buf.len() as u32, + // Still not getting over the mut pointers here + Buffer: stream_buf.as_mut_ptr().cast(), + }; + + Some((stream_buf, handle_stream)) + } else { + // We don't _particularly_ care if this fails, it's better if we had + // the info, but not critical + None + } + } +} + +impl Drop for MinidumpWriter { + fn drop(&mut self) { + // If we're the current process we created the handle ourselves, so we need to close it + if !self.is_external_process { + // SAFETY: syscall + unsafe { CloseHandle(self.crashing_process) }; + } + } +} From f4253be8565bbe203aab1d8401e1bb75b946a773 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Fri, 25 Mar 2022 08:26:33 +0100 Subject: [PATCH 02/36] Remove assertion info, it is functionally useless --- src/windows/minidump_writer.rs | 67 +--------------------------------- 1 file changed, 1 insertion(+), 66 deletions(-) diff --git a/src/windows/minidump_writer.rs b/src/windows/minidump_writer.rs index ecdf4f67..aafa2b1c 100644 --- a/src/windows/minidump_writer.rs +++ b/src/windows/minidump_writer.rs @@ -90,7 +90,7 @@ impl MinidumpWriter { // This is a bit dangerous if doing in-process dumping, but that's not // (currently) a real target of this crate, so this allocation is fine - let mut user_streams = Vec::with_capacity(3); + let mut user_streams = Vec::with_capacity(2); // Add an MDRawBreakpadInfo stream to the minidump, to provide additional // information about the exception handler to the Breakpad processor. @@ -124,71 +124,6 @@ impl MinidumpWriter { }); } - // When dumping an external process we want to retrieve the actual contents - // of the assertion info and add it as a user stream, but we need to - // keep the memory alive for the duration of the write - // SAFETY: POD - let mut assertion_info: crash_context::RawAssertionInfo = unsafe { std::mem::zeroed() }; - - if let Some(ai) = self.crash_context.assertion_info { - let ai_ptr = if self.is_external_process { - // Even though this information is useful for non-exceptional dumps - // (purecall, invalid parameter), we don't treat it is a critical - // failure if we can't read it (unlike Breakpad) since we will still - // have the synthetic exception context that was generated which - // indicates the kind (again, purecall or invalid parameter), and - // realistically, the assertion information is going to fairly pointless - // anyways (at least for invalid parameters) since the information - // supplied to the handler is only going to be filled in if using - // the debug MSVCRT, which you can only realistically do in dev - // environments since the debug MSVCRT is not redistributable. - let mut assert_info = - std::mem::MaybeUninit::::uninit(); - let mut bytes_read = 0; - - // SAFETY: syscall - if unsafe { - md::ReadProcessMemory( - self.crashing_process, // client process handle to read the memory from - ai.cast(), // the pointer to read from the client process - assert_info.as_mut_ptr().cast(), // The buffer we're filling with the memory - std::mem::size_of::(), - &mut bytes_read, - ) - } == 0 - { - // log::error!( - // "failed to read assertion information from client: {}", - // last_os_error() - // ); - std::ptr::null() - } else if bytes_read != std::mem::size_of::() { - // log::error!( - // "read invalid number of bytes: expected {} != received {}", - // std::mem::size_of::(), - // bytes_read - // ); - std::ptr::null() - } else { - // SAFETY: this is fine as lone as Windows didn't lie to us - assertion_info = unsafe { assert_info.assume_init() }; - - &assertion_info - } - } else { - ai - }; - - if !ai.is_null() { - user_streams.push(md::MINIDUMP_USER_STREAM { - Type: MINIDUMP_STREAM_TYPE::AssertionInfoStream as u32, - BufferSize: std::mem::size_of::() as u32, - // Again with the mut pointer - Buffer: (ai_ptr as *mut crash_context::RawAssertionInfo).cast(), - }); - } - } - let handle_stream = self.fill_handle_stream(); // Note that we do this by ref, as the buffer inside the option needs From 6e3333f0cfaac99c51cb45eae39332bb95430e68 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Tue, 29 Mar 2022 09:46:55 +0200 Subject: [PATCH 03/36] Always open process handle internally --- src/windows/minidump_writer.rs | 39 ++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/src/windows/minidump_writer.rs b/src/windows/minidump_writer.rs index aafa2b1c..2bf30165 100644 --- a/src/windows/minidump_writer.rs +++ b/src/windows/minidump_writer.rs @@ -12,6 +12,9 @@ use windows_sys::Win32::{ }, }; +// For some reason this is defined in SystemServices which is massive and not worth bringing in for ONE constant +const GENERIC_ALL: u32 = 268435456; + pub struct MinidumpWriter { /// The crash context as captured by an exception handler crash_context: crash_context::CrashContext, @@ -34,13 +37,24 @@ impl MinidumpWriter { pub fn external_process( crash_context: crash_context::CrashContext, pid: u32, - proc_handle: HANDLE, - ) -> Self { - Self { - crash_context, - crashing_process: proc_handle, - crashing_pid: pid, - is_external_process: true, + ) -> Result { + let crashing_process = unsafe { + OpenProcess( + GENERIC_ALL, // desired access + 0, // inherit handles + pid, // pid + ) + }; + + if crashing_process == 0 { + Err(std::io::Error::last_os_error().into()) + } else { + Ok(Self { + crash_context, + crashing_process, + crashing_pid: pid, + is_external_process: true, + }) } } @@ -55,8 +69,8 @@ impl MinidumpWriter { // SAFETY: syscall let crashing_process = unsafe { OpenProcess( - 268435456, // desired access - GENERIC_ALL - for some reason this is defined in SystemServices which is massive and not worth bringing in for ONE constant - 0, // inherit handles + GENERIC_ALL, // desired access + 0, // inherit handles crashing_pid, ) }; @@ -258,10 +272,7 @@ impl MinidumpWriter { impl Drop for MinidumpWriter { fn drop(&mut self) { - // If we're the current process we created the handle ourselves, so we need to close it - if !self.is_external_process { - // SAFETY: syscall - unsafe { CloseHandle(self.crashing_process) }; - } + // SAFETY: syscall + unsafe { CloseHandle(self.crashing_process) }; } } From 612d1d495c3d0ba7744ec66f66153aa29731fac0 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Tue, 29 Mar 2022 18:24:40 +0200 Subject: [PATCH 04/36] Use published crash-context --- Cargo.toml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 66026a2f..1a0f07e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ license = "MIT" [dependencies] byteorder = "1.3.2" cfg-if = "1.0" -crash-context = "0.0.2" +crash-context = "0.0.3" memoffset = "0.6" minidump-common = "0.10" scroll = "0.11" @@ -42,5 +42,3 @@ minidump = "0.10" [patch.crates-io] minidump = { git = "https://github.com/rust-minidump/rust-minidump" } minidump-common = { git = "https://github.com/rust-minidump/rust-minidump" } -#crash-context = { git = "https://github.com/EmbarkStudios/crash-handling", branch = "main" } -crash-context = { path = "../crash-handling/crash-context" } From f917d932118cfdeab2211c07e6c36bd90376f94a Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 08:06:19 +0200 Subject: [PATCH 05/36] Fix tests --- Cargo.toml | 4 ---- tests/minidump_writer.rs | 14 +++++++------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1a0f07e5..961c6682 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,3 @@ features = [ [dev-dependencies] minidump = "0.10" - -[patch.crates-io] -minidump = { git = "https://github.com/rust-minidump/rust-minidump" } -minidump-common = { git = "https://github.com/rust-minidump/rust-minidump" } diff --git a/tests/minidump_writer.rs b/tests/minidump_writer.rs index 27c56f9e..400c6e27 100644 --- a/tests/minidump_writer.rs +++ b/tests/minidump_writer.rs @@ -183,25 +183,25 @@ fn test_write_and_read_dump_from_parent_helper(context: Context) { let _: MinidumpException = dump.get_stream().expect("Couldn't find MinidumpException"); let _: MinidumpSystemInfo = dump.get_stream().expect("Couldn't find MinidumpSystemInfo"); let _ = dump - .get_raw_stream(LinuxCpuInfo) + .get_raw_stream(LinuxCpuInfo as u32) .expect("Couldn't find LinuxCpuInfo"); let _ = dump - .get_raw_stream(LinuxProcStatus) + .get_raw_stream(LinuxProcStatus as u32) .expect("Couldn't find LinuxProcStatus"); let _ = dump - .get_raw_stream(LinuxCmdLine) + .get_raw_stream(LinuxCmdLine as u32) .expect("Couldn't find LinuxCmdLine"); let _ = dump - .get_raw_stream(LinuxEnviron) + .get_raw_stream(LinuxEnviron as u32) .expect("Couldn't find LinuxEnviron"); let _ = dump - .get_raw_stream(LinuxAuxv) + .get_raw_stream(LinuxAuxv as u32) .expect("Couldn't find LinuxAuxv"); let _ = dump - .get_raw_stream(LinuxMaps) + .get_raw_stream(LinuxMaps as u32) .expect("Couldn't find LinuxMaps"); let _ = dump - .get_raw_stream(LinuxDsoDebug) + .get_raw_stream(LinuxDsoDebug as u32) .expect("Couldn't find LinuxDsoDebug"); } #[test] From 3522e4297fb9d3210341f0f3a691de011c981716 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 09:37:40 +0200 Subject: [PATCH 06/36] Rename linux integration test --- tests/{minidump_writer.rs => linux_minidump_writer.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{minidump_writer.rs => linux_minidump_writer.rs} (100%) diff --git a/tests/minidump_writer.rs b/tests/linux_minidump_writer.rs similarity index 100% rename from tests/minidump_writer.rs rename to tests/linux_minidump_writer.rs From c99359be7248359c43d3c3b61c149b1c21690552 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 09:37:56 +0200 Subject: [PATCH 07/36] Gate ptrace integration test --- tests/ptrace_dumper.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/ptrace_dumper.rs b/tests/ptrace_dumper.rs index ffe2fb79..520a3eaa 100644 --- a/tests/ptrace_dumper.rs +++ b/tests/ptrace_dumper.rs @@ -1,3 +1,6 @@ +//! All of these tests are specific to ptrace +#![cfg(any(target_os = "linux", target_os = "android"))] + use minidump_writer::ptrace_dumper::PtraceDumper; use nix::sys::mman::{mmap, MapFlags, ProtFlags}; use nix::sys::signal::Signal; From d92e80782cff35ed883cb06d3ca2c55640358cdd Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 09:38:25 +0200 Subject: [PATCH 08/36] Remove duplicate stream test --- tests/linux_minidump_writer.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/linux_minidump_writer.rs b/tests/linux_minidump_writer.rs index 400c6e27..3901f08e 100644 --- a/tests/linux_minidump_writer.rs +++ b/tests/linux_minidump_writer.rs @@ -180,7 +180,6 @@ fn test_write_and_read_dump_from_parent_helper(context: Context) { let _: MinidumpException = dump.get_stream().expect("Couldn't find MinidumpException"); let _: MinidumpThreadList = dump.get_stream().expect("Couldn't find MinidumpThreadList"); let _: MinidumpMemoryList = dump.get_stream().expect("Couldn't find MinidumpMemoryList"); - let _: MinidumpException = dump.get_stream().expect("Couldn't find MinidumpException"); let _: MinidumpSystemInfo = dump.get_stream().expect("Couldn't find MinidumpSystemInfo"); let _ = dump .get_raw_stream(LinuxCpuInfo as u32) From 332241ffd35e8d2832f3ab21f7b9b273001dfb8f Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 09:38:58 +0200 Subject: [PATCH 09/36] Move linux specific code to submodule --- src/bin/test.rs | 504 +++++++++++++++++++++++++----------------------- 1 file changed, 259 insertions(+), 245 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 0146db7a..87511e8c 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -1,292 +1,306 @@ -#![cfg(any(target_os = "linux", target_os = "android"))] - // This binary shouldn't be under /src, but under /tests, but that is // currently not possible (https://github.com/rust-lang/cargo/issues/4356) -use minidump_writer::{ - ptrace_dumper::{PtraceDumper, AT_SYSINFO_EHDR}, - LINUX_GATE_LIBRARY_NAME, -}; -use nix::{ - sys::mman::{mmap, MapFlags, ProtFlags}, - unistd::getppid, -}; -use std::{env, error, result}; -type Error = Box; -pub type Result = result::Result; +type Error = Box; +pub type Result = std::result::Result; -macro_rules! test { - ($x:expr, $errmsg:expr) => { - if $x { - Ok(()) - } else { - Err($errmsg) - } +#[cfg(any(target_os = "linux", target_os = "android"))] +mod linux { + use super::*; + use minidump_writer::{ + ptrace_dumper::{PtraceDumper, AT_SYSINFO_EHDR}, + LINUX_GATE_LIBRARY_NAME, + }; + use nix::{ + sys::mman::{mmap, MapFlags, ProtFlags}, + unistd::getppid, }; -} -fn test_setup() -> Result<()> { - let ppid = getppid(); - PtraceDumper::new(ppid.as_raw())?; - Ok(()) -} + macro_rules! test { + ($x:expr, $errmsg:expr) => { + if $x { + Ok(()) + } else { + Err($errmsg) + } + }; + } -fn test_thread_list() -> Result<()> { - let ppid = getppid(); - let dumper = PtraceDumper::new(ppid.as_raw())?; - test!(!dumper.threads.is_empty(), "No threads")?; - test!( - dumper - .threads - .iter() - .filter(|x| x.tid == ppid.as_raw()) - .count() - == 1, - "Thread found multiple times" - )?; - Ok(()) -} + fn test_setup() -> Result<()> { + let ppid = getppid(); + PtraceDumper::new(ppid.as_raw())?; + Ok(()) + } -fn test_copy_from_process(stack_var: usize, heap_var: usize) -> Result<()> { - let ppid = getppid().as_raw(); - let mut dumper = PtraceDumper::new(ppid)?; - dumper.suspend_threads()?; - let stack_res = PtraceDumper::copy_from_process(ppid, stack_var as *mut libc::c_void, 1)?; + fn test_thread_list() -> Result<()> { + let ppid = getppid(); + let dumper = PtraceDumper::new(ppid.as_raw())?; + test!(!dumper.threads.is_empty(), "No threads")?; + test!( + dumper + .threads + .iter() + .filter(|x| x.tid == ppid.as_raw()) + .count() + == 1, + "Thread found multiple times" + )?; + Ok(()) + } - let expected_stack: libc::c_long = 0x11223344; - test!( - stack_res == expected_stack.to_ne_bytes(), - "stack var not correct" - )?; + fn test_copy_from_process(stack_var: usize, heap_var: usize) -> Result<()> { + let ppid = getppid().as_raw(); + let mut dumper = PtraceDumper::new(ppid)?; + dumper.suspend_threads()?; + let stack_res = PtraceDumper::copy_from_process(ppid, stack_var as *mut libc::c_void, 1)?; - let heap_res = PtraceDumper::copy_from_process(ppid, heap_var as *mut libc::c_void, 1)?; - let expected_heap: libc::c_long = 0x55667788; - test!( - heap_res == expected_heap.to_ne_bytes(), - "heap var not correct" - )?; - dumper.resume_threads()?; - Ok(()) -} + let expected_stack: libc::c_long = 0x11223344; + test!( + stack_res == expected_stack.to_ne_bytes(), + "stack var not correct" + )?; -fn test_find_mappings(addr1: usize, addr2: usize) -> Result<()> { - let ppid = getppid(); - let dumper = PtraceDumper::new(ppid.as_raw())?; - dumper - .find_mapping(addr1) - .ok_or("No mapping for addr1 found")?; + let heap_res = PtraceDumper::copy_from_process(ppid, heap_var as *mut libc::c_void, 1)?; + let expected_heap: libc::c_long = 0x55667788; + test!( + heap_res == expected_heap.to_ne_bytes(), + "heap var not correct" + )?; + dumper.resume_threads()?; + Ok(()) + } - dumper - .find_mapping(addr2) - .ok_or("No mapping for addr2 found")?; + fn test_find_mappings(addr1: usize, addr2: usize) -> Result<()> { + let ppid = getppid(); + let dumper = PtraceDumper::new(ppid.as_raw())?; + dumper + .find_mapping(addr1) + .ok_or("No mapping for addr1 found")?; - test!(dumper.find_mapping(0).is_none(), "NULL found")?; - Ok(()) -} + dumper + .find_mapping(addr2) + .ok_or("No mapping for addr2 found")?; -fn test_file_id() -> Result<()> { - let ppid = getppid().as_raw(); - let exe_link = format!("/proc/{}/exe", ppid); - let exe_name = std::fs::read_link(&exe_link)?.into_os_string(); - let mut dumper = PtraceDumper::new(getppid().as_raw())?; - let mut found_exe = None; - for (idx, mapping) in dumper.mappings.iter().enumerate() { - if mapping.name.as_ref().map(|x| x.into()).as_ref() == Some(&exe_name) { - found_exe = Some(idx); - break; - } + test!(dumper.find_mapping(0).is_none(), "NULL found")?; + Ok(()) } - let idx = found_exe.unwrap(); - let id = dumper.elf_identifier_for_mapping_index(idx)?; - assert!(!id.is_empty()); - assert!(id.iter().any(|&x| x > 0)); - Ok(()) -} -fn test_merged_mappings(path: String, mapped_mem: usize, mem_size: usize) -> Result<()> { - // Now check that PtraceDumper interpreted the mappings properly. - let dumper = PtraceDumper::new(getppid().as_raw())?; - let mut mapping_count = 0; - for map in &dumper.mappings { - if map.name == Some(path.clone()) { - mapping_count += 1; - // This mapping should encompass the entire original mapped - // range. - assert_eq!(map.start_address, mapped_mem); - assert_eq!(map.size, mem_size); - assert_eq!(0, map.offset); + fn test_file_id() -> Result<()> { + let ppid = getppid().as_raw(); + let exe_link = format!("/proc/{}/exe", ppid); + let exe_name = std::fs::read_link(&exe_link)?.into_os_string(); + let mut dumper = PtraceDumper::new(getppid().as_raw())?; + let mut found_exe = None; + for (idx, mapping) in dumper.mappings.iter().enumerate() { + if mapping.name.as_ref().map(|x| x.into()).as_ref() == Some(&exe_name) { + found_exe = Some(idx); + break; + } } + let idx = found_exe.unwrap(); + let id = dumper.elf_identifier_for_mapping_index(idx)?; + assert!(!id.is_empty()); + assert!(id.iter().any(|&x| x > 0)); + Ok(()) } - assert_eq!(1, mapping_count); - Ok(()) -} -fn test_linux_gate_mapping_id() -> Result<()> { - let ppid = getppid().as_raw(); - let mut dumper = PtraceDumper::new(ppid)?; - let mut found_linux_gate = false; - for mut mapping in dumper.mappings.clone() { - if mapping.name.as_deref() == Some(LINUX_GATE_LIBRARY_NAME) { - found_linux_gate = true; - dumper.suspend_threads()?; - let id = PtraceDumper::elf_identifier_for_mapping(&mut mapping, ppid)?; - test!(!id.is_empty(), "id-vec is empty")?; - test!(id.iter().any(|&x| x > 0), "all id elements are 0")?; - dumper.resume_threads()?; - break; + fn test_merged_mappings(path: String, mapped_mem: usize, mem_size: usize) -> Result<()> { + // Now check that PtraceDumper interpreted the mappings properly. + let dumper = PtraceDumper::new(getppid().as_raw())?; + let mut mapping_count = 0; + for map in &dumper.mappings { + if map.name == Some(path.clone()) { + mapping_count += 1; + // This mapping should encompass the entire original mapped + // range. + assert_eq!(map.start_address, mapped_mem); + assert_eq!(map.size, mem_size); + assert_eq!(0, map.offset); + } } + assert_eq!(1, mapping_count); + Ok(()) } - test!(found_linux_gate, "found no linux_gate")?; - Ok(()) -} - -fn test_mappings_include_linux_gate() -> Result<()> { - let ppid = getppid().as_raw(); - let dumper = PtraceDumper::new(ppid)?; - let linux_gate_loc = dumper.auxv[&AT_SYSINFO_EHDR]; - test!(linux_gate_loc != 0, "linux_gate_loc == 0")?; - let mut found_linux_gate = false; - for mapping in &dumper.mappings { - if mapping.name.as_deref() == Some(LINUX_GATE_LIBRARY_NAME) { - found_linux_gate = true; - test!( - linux_gate_loc == mapping.start_address.try_into()?, - "linux_gate_loc != start_address" - )?; - // This doesn't work here, as we do not test via "fork()", so the addresses are different - // let ll = mapping.start_address as *const u8; - // for idx in 0..header::SELFMAG { - // let mag = unsafe { std::ptr::read(ll.offset(idx as isize)) == header::ELFMAG[idx] }; - // test!( - // mag, - // format!("ll: {} != ELFMAG: {} at {}", mag, header::ELFMAG[idx], idx) - // )?; - // } - break; + fn test_linux_gate_mapping_id() -> Result<()> { + let ppid = getppid().as_raw(); + let mut dumper = PtraceDumper::new(ppid)?; + let mut found_linux_gate = false; + for mut mapping in dumper.mappings.clone() { + if mapping.name.as_deref() == Some(LINUX_GATE_LIBRARY_NAME) { + found_linux_gate = true; + dumper.suspend_threads()?; + let id = PtraceDumper::elf_identifier_for_mapping(&mut mapping, ppid)?; + test!(!id.is_empty(), "id-vec is empty")?; + test!(id.iter().any(|&x| x > 0), "all id elements are 0")?; + dumper.resume_threads()?; + break; + } } + test!(found_linux_gate, "found no linux_gate")?; + Ok(()) } - test!(found_linux_gate, "found no linux_gate")?; - Ok(()) -} -fn spawn_and_wait(num: usize) -> Result<()> { - // One less than the requested amount, as the main thread counts as well - for _ in 1..num { - std::thread::spawn(|| { - println!("1"); - loop { - std::thread::park(); + fn test_mappings_include_linux_gate() -> Result<()> { + let ppid = getppid().as_raw(); + let dumper = PtraceDumper::new(ppid)?; + let linux_gate_loc = dumper.auxv[&AT_SYSINFO_EHDR]; + test!(linux_gate_loc != 0, "linux_gate_loc == 0")?; + let mut found_linux_gate = false; + for mapping in &dumper.mappings { + if mapping.name.as_deref() == Some(LINUX_GATE_LIBRARY_NAME) { + found_linux_gate = true; + test!( + linux_gate_loc == mapping.start_address.try_into()?, + "linux_gate_loc != start_address" + )?; + + // This doesn't work here, as we do not test via "fork()", so the addresses are different + // let ll = mapping.start_address as *const u8; + // for idx in 0..header::SELFMAG { + // let mag = unsafe { std::ptr::read(ll.offset(idx as isize)) == header::ELFMAG[idx] }; + // test!( + // mag, + // format!("ll: {} != ELFMAG: {} at {}", mag, header::ELFMAG[idx], idx) + // )?; + // } + break; } - }); - } - println!("1"); - loop { - std::thread::park(); + } + test!(found_linux_gate, "found no linux_gate")?; + Ok(()) } -} -fn spawn_name_wait(num: usize) -> Result<()> { - // One less than the requested amount, as the main thread counts as well - for id in 1..num { - std::thread::Builder::new() - .name(format!("thread_{}", id)) - .spawn(|| { + fn spawn_and_wait(num: usize) -> Result<()> { + // One less than the requested amount, as the main thread counts as well + for _ in 1..num { + std::thread::spawn(|| { println!("1"); loop { std::thread::park(); } - })?; - } - println!("1"); - loop { - std::thread::park(); + }); + } + println!("1"); + loop { + std::thread::park(); + } } -} -fn spawn_mmap_wait() -> Result<()> { - let page_size = nix::unistd::sysconf(nix::unistd::SysconfVar::PAGE_SIZE).unwrap(); - let memory_size = page_size.unwrap() as usize; - // Get some memory to be mapped by the child-process - let mapped_mem = unsafe { - mmap( - std::ptr::null_mut(), - memory_size, - ProtFlags::PROT_READ | ProtFlags::PROT_WRITE, - MapFlags::MAP_PRIVATE | MapFlags::MAP_ANON, - -1, - 0, - ) - .unwrap() - }; - println!("{} {}", mapped_mem as usize, memory_size); - loop { - std::thread::park(); + fn spawn_name_wait(num: usize) -> Result<()> { + // One less than the requested amount, as the main thread counts as well + for id in 1..num { + std::thread::Builder::new() + .name(format!("thread_{}", id)) + .spawn(|| { + println!("1"); + loop { + std::thread::park(); + } + })?; + } + println!("1"); + loop { + std::thread::park(); + } } -} -fn spawn_alloc_wait() -> Result<()> { - let page_size = nix::unistd::sysconf(nix::unistd::SysconfVar::PAGE_SIZE).unwrap(); - let memory_size = page_size.unwrap() as usize; + fn spawn_mmap_wait() -> Result<()> { + let page_size = nix::unistd::sysconf(nix::unistd::SysconfVar::PAGE_SIZE).unwrap(); + let memory_size = page_size.unwrap() as usize; + // Get some memory to be mapped by the child-process + let mapped_mem = unsafe { + mmap( + std::ptr::null_mut(), + memory_size, + ProtFlags::PROT_READ | ProtFlags::PROT_WRITE, + MapFlags::MAP_PRIVATE | MapFlags::MAP_ANON, + -1, + 0, + ) + .unwrap() + }; - let mut values = Vec::::with_capacity(memory_size); - for idx in 0..memory_size { - values.push((idx % 255) as u8); + println!("{} {}", mapped_mem as usize, memory_size); + loop { + std::thread::park(); + } } - println!("{:p} {}", values.as_ptr(), memory_size); - loop { - std::thread::park(); + fn spawn_alloc_wait() -> Result<()> { + let page_size = nix::unistd::sysconf(nix::unistd::SysconfVar::PAGE_SIZE).unwrap(); + let memory_size = page_size.unwrap() as usize; + + let mut values = Vec::::with_capacity(memory_size); + for idx in 0..memory_size { + values.push((idx % 255) as u8); + } + + println!("{:p} {}", values.as_ptr(), memory_size); + loop { + std::thread::park(); + } } -} -fn main() -> Result<()> { - let args: Vec<_> = env::args().skip(1).collect(); - match args.len() { - 1 => match args[0].as_ref() { - "file_id" => test_file_id(), - "setup" => test_setup(), - "thread_list" => test_thread_list(), - "mappings_include_linux_gate" => test_mappings_include_linux_gate(), - "linux_gate_mapping_id" => test_linux_gate_mapping_id(), - "spawn_mmap_wait" => spawn_mmap_wait(), - "spawn_alloc_wait" => spawn_alloc_wait(), - _ => Err("Len 1: Unknown test option".into()), - }, - 2 => match args[0].as_ref() { - "spawn_and_wait" => { - let num_of_threads: usize = args[1].parse().unwrap(); - spawn_and_wait(num_of_threads) - } - "spawn_name_wait" => { - let num_of_threads: usize = args[1].parse().unwrap(); - spawn_name_wait(num_of_threads) + pub(super) fn real_main(args: Vec) -> Result<()> { + match args.len() { + 1 => match args[0].as_ref() { + "file_id" => test_file_id(), + "setup" => test_setup(), + "thread_list" => test_thread_list(), + "mappings_include_linux_gate" => test_mappings_include_linux_gate(), + "linux_gate_mapping_id" => test_linux_gate_mapping_id(), + "spawn_mmap_wait" => spawn_mmap_wait(), + "spawn_alloc_wait" => spawn_alloc_wait(), + _ => Err("Len 1: Unknown test option".into()), + }, + 2 => match args[0].as_ref() { + "spawn_and_wait" => { + let num_of_threads: usize = args[1].parse().unwrap(); + spawn_and_wait(num_of_threads) + } + "spawn_name_wait" => { + let num_of_threads: usize = args[1].parse().unwrap(); + spawn_name_wait(num_of_threads) + } + _ => Err(format!("Len 2: Unknown test option: {}", args[0]).into()), + }, + 3 => { + if args[0] == "find_mappings" { + let addr1: usize = args[1].parse().unwrap(); + let addr2: usize = args[2].parse().unwrap(); + test_find_mappings(addr1, addr2) + } else if args[0] == "copy_from_process" { + let stack_var: usize = args[1].parse().unwrap(); + let heap_var: usize = args[2].parse().unwrap(); + test_copy_from_process(stack_var, heap_var) + } else { + Err(format!("Len 3: Unknown test option: {}", args[0]).into()) + } } - _ => Err(format!("Len 2: Unknown test option: {}", args[0]).into()), - }, - 3 => { - if args[0] == "find_mappings" { - let addr1: usize = args[1].parse().unwrap(); - let addr2: usize = args[2].parse().unwrap(); - test_find_mappings(addr1, addr2) - } else if args[0] == "copy_from_process" { - let stack_var: usize = args[1].parse().unwrap(); - let heap_var: usize = args[2].parse().unwrap(); - test_copy_from_process(stack_var, heap_var) - } else { - Err(format!("Len 3: Unknown test option: {}", args[0]).into()) + 4 => { + if args[0] == "merged_mappings" { + let path = &args[1]; + let mapped_mem: usize = args[2].parse().unwrap(); + let mem_size: usize = args[3].parse().unwrap(); + test_merged_mappings(path.to_string(), mapped_mem, mem_size) + } else { + Err(format!("Len 4: Unknown test option: {}", args[0]).into()) + } } + _ => Err("Unknown test option".into()), } - 4 => { - if args[0] == "merged_mappings" { - let path = &args[1]; - let mapped_mem: usize = args[2].parse().unwrap(); - let mem_size: usize = args[3].parse().unwrap(); - test_merged_mappings(path.to_string(), mapped_mem, mem_size) - } else { - Err(format!("Len 4: Unknown test option: {}", args[0]).into()) - } + } +} + +fn main() -> Result<()> { + let args: Vec<_> = std::env::args().skip(1).collect(); + + cfg_if::cfg_if! { + if #[cfg(any(target_os = "linux", target_os = "android"))] { + linux::real_main(args) + } else { + compile_error!("test binary is not implemented for this target"); } - _ => Err("Unknown test option".into()), } } From 9cae5c9b79b4c3df99e90279974feeb70edaff0b Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 09:39:14 +0200 Subject: [PATCH 10/36] Add missing feature --- Cargo.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 961c6682..0533eecd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,8 @@ features = [ "Win32_System_Memory", # VerifierEnumerateResource and friends "Win32_System_ApplicationVerifier", + # GetCurrentThreadId & OpenProcess + "Win32_System_Threading", ] [dev-dependencies] From a21f94fe8e15c43d5190827c5cb462ae40f25515 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 10:02:09 +0200 Subject: [PATCH 11/36] Add inproc dump test --- src/bin/test.rs | 2 +- tests/windows_minidump_writer.rs | 79 ++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 tests/windows_minidump_writer.rs diff --git a/src/bin/test.rs b/src/bin/test.rs index 87511e8c..7b2c2549 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -300,7 +300,7 @@ fn main() -> Result<()> { if #[cfg(any(target_os = "linux", target_os = "android"))] { linux::real_main(args) } else { - compile_error!("test binary is not implemented for this target"); + panic!("not implemented"); } } } diff --git a/tests/windows_minidump_writer.rs b/tests/windows_minidump_writer.rs new file mode 100644 index 00000000..d631b8c2 --- /dev/null +++ b/tests/windows_minidump_writer.rs @@ -0,0 +1,79 @@ +#![cfg(all(target_os = "windows", target_arch = "x86_64"))] + +use minidump::{CrashReason, Minidump, MinidumpMemoryList, MinidumpSystemInfo, MinidumpThreadList}; +use minidump_writer::minidump_writer::MinidumpWriter; +use std::mem; +use windows_sys::Win32::{ + Foundation::{ + DBG_PRINTEXCEPTION_C, DBG_PRINTEXCEPTION_WIDE_C, EXCEPTION_BREAKPOINT, + EXCEPTION_SINGLE_STEP, STATUS_INVALID_PARAMETER, STATUS_NONCONTINUABLE_EXCEPTION, + }, + System::{ + Diagnostics::Debug::{RtlCaptureContext, EXCEPTION_POINTERS, EXCEPTION_RECORD}, + Threading::GetCurrentThreadId, + }, +}; + +fn get_crash_reason<'a, T: std::ops::Deref + 'a>( + md: &Minidump<'a, T>, +) -> CrashReason { + let exc: minidump::MinidumpException<'_> = + md.get_stream().expect("unable to find exception stream"); + + exc.get_crash_reason( + minidump::system_info::Os::Windows, + minidump::system_info::Cpu::X86_64, + ) +} + +/// Ensures that we can write minidumps for the current process, even if this is +/// not necessarily the primary intended use case of out-of-process dumping +#[test] +fn dump_current_process() { + let mut tmpfile = tempfile::Builder::new() + .prefix("windows_current_process") + .tempfile() + .unwrap(); + + unsafe { + let mut exception_record: EXCEPTION_RECORD = mem::zeroed(); + let mut exception_context = mem::MaybeUninit::uninit(); + + RtlCaptureContext(exception_context.as_mut_ptr()); + + let mut exception_context = exception_context.assume_init(); + + let exception_ptrs = EXCEPTION_POINTERS { + ExceptionRecord: &mut exception_record, + ContextRecord: &mut exception_context, + }; + + exception_record.ExceptionCode = STATUS_INVALID_PARAMETER; + + let crash_context = crash_context::CrashContext { + exception_pointers: &exception_ptrs, + thread_id: GetCurrentThreadId(), + exception_code: STATUS_INVALID_PARAMETER, + }; + + let dumper = MinidumpWriter::current_process(crash_context) + .expect("failed to create MinidumpWriter"); + + dumper + .dump(tmpfile.as_file_mut()) + .expect("failed to write minidump"); + } + + let md = Minidump::read_path(tmpfile.path()).expect("failed to read minidump"); + + let _: MinidumpThreadList = md.get_stream().expect("Couldn't find MinidumpThreadList"); + let _: MinidumpMemoryList = md.get_stream().expect("Couldn't find MinidumpMemoryList"); + let _: MinidumpSystemInfo = md.get_stream().expect("Couldn't find MinidumpSystemInfo"); + + let crash_reason = get_crash_reason(&md); + + assert_eq!( + crash_reason, + CrashReason::from_windows_error(STATUS_INVALID_PARAMETER as u32) + ); +} From ffc5bdb423006a6e240633ffbfc375accd4f1117 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 10:42:21 +0200 Subject: [PATCH 12/36] Add external process dump test --- src/bin/test.rs | 44 +++++++++++++++++++++- tests/windows_minidump_writer.rs | 64 ++++++++++++++++++++++++++++++-- 2 files changed, 103 insertions(+), 5 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 7b2c2549..6a43592c 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -293,6 +293,48 @@ mod linux { } } +#[cfg(target_os = "windows")] +mod windows { + use super::*; + use std::mem; + use windows_sys::Win32::System::{ + Diagnostics::Debug::{RtlCaptureContext, EXCEPTION_POINTERS, EXCEPTION_RECORD}, + Threading::GetCurrentThreadId, + }; + + pub(super) fn real_main(args: Vec) -> Result<()> { + let exception_code = i32::from_str_radix(&args[0], 16).unwrap(); + + // Generate the exception and communicate back where the exception pointers + // are + unsafe { + let mut exception_record: EXCEPTION_RECORD = mem::zeroed(); + let mut exception_context = mem::MaybeUninit::uninit(); + + RtlCaptureContext(exception_context.as_mut_ptr()); + + let mut exception_context = exception_context.assume_init(); + + let exception_ptrs = EXCEPTION_POINTERS { + ExceptionRecord: &mut exception_record, + ContextRecord: &mut exception_context, + }; + + exception_record.ExceptionCode = exception_code; + + let exc_ptr_addr = &exception_ptrs as *const _ as usize; + let tid = GetCurrentThreadId(); + + println!("{exc_ptr_addr} {tid} {exception_code:x}"); + + // Wait until we're killed + loop { + std::thread::park(); + } + } + } +} + fn main() -> Result<()> { let args: Vec<_> = std::env::args().skip(1).collect(); @@ -300,7 +342,7 @@ fn main() -> Result<()> { if #[cfg(any(target_os = "linux", target_os = "android"))] { linux::real_main(args) } else { - panic!("not implemented"); + windows::real_main(args) } } } diff --git a/tests/windows_minidump_writer.rs b/tests/windows_minidump_writer.rs index d631b8c2..6fbf4b3b 100644 --- a/tests/windows_minidump_writer.rs +++ b/tests/windows_minidump_writer.rs @@ -4,15 +4,14 @@ use minidump::{CrashReason, Minidump, MinidumpMemoryList, MinidumpSystemInfo, Mi use minidump_writer::minidump_writer::MinidumpWriter; use std::mem; use windows_sys::Win32::{ - Foundation::{ - DBG_PRINTEXCEPTION_C, DBG_PRINTEXCEPTION_WIDE_C, EXCEPTION_BREAKPOINT, - EXCEPTION_SINGLE_STEP, STATUS_INVALID_PARAMETER, STATUS_NONCONTINUABLE_EXCEPTION, - }, + Foundation::{EXCEPTION_ACCESS_VIOLATION, STATUS_INVALID_PARAMETER}, System::{ Diagnostics::Debug::{RtlCaptureContext, EXCEPTION_POINTERS, EXCEPTION_RECORD}, Threading::GetCurrentThreadId, }, }; +mod common; +use common::start_child_and_return; fn get_crash_reason<'a, T: std::ops::Deref + 'a>( md: &Minidump<'a, T>, @@ -77,3 +76,60 @@ fn dump_current_process() { CrashReason::from_windows_error(STATUS_INVALID_PARAMETER as u32) ); } + +/// Ensures that we can write minidumps for an external process. Unfortunately +/// this requires us to know the actual pointer in the client process for the +/// exception, as the `MiniDumpWriteDump` syscall directly reads points from +/// the process memory, so we communicate that back from the client process +/// via stdout +#[test] +fn dump_external_process() { + use std::io::BufRead; + + let mut child = start_child_and_return(&format!("{:x}", EXCEPTION_ACCESS_VIOLATION)); + + let mut f = std::io::BufReader::new(child.stdout.as_mut().expect("Can't open stdout")); + let mut buf = String::new(); + f.read_line(&mut buf).expect("failed to read stdout"); + + let mut biter = buf.split(' '); + + let exception_pointers: usize = biter.next().unwrap().parse().unwrap(); + let thread_id: u32 = biter.next().unwrap().parse().unwrap(); + let exception_code = i32::from_str_radix(biter.next().unwrap(), 16).unwrap(); + + assert_eq!(exception_code, EXCEPTION_ACCESS_VIOLATION); + + let crash_context = crash_context::CrashContext { + exception_pointers: exception_pointers as _, + thread_id, + exception_code, + }; + + let mut tmpfile = tempfile::Builder::new() + .prefix("windows_external_process") + .tempfile() + .unwrap(); + + let dumper = MinidumpWriter::external_process(crash_context, child.id()) + .expect("failed to create MinidumpWriter"); + + dumper + .dump(tmpfile.as_file_mut()) + .expect("failed to write minidump"); + + child.kill().expect("failed to kill child"); + + let md = Minidump::read_path(tmpfile.path()).expect("failed to read minidump"); + + let _: MinidumpThreadList = md.get_stream().expect("Couldn't find MinidumpThreadList"); + let _: MinidumpMemoryList = md.get_stream().expect("Couldn't find MinidumpMemoryList"); + let _: MinidumpSystemInfo = md.get_stream().expect("Couldn't find MinidumpSystemInfo"); + + let crash_reason = get_crash_reason(&md); + + assert_eq!( + crash_reason, + CrashReason::from_windows_error(EXCEPTION_ACCESS_VIOLATION as u32) + ); +} From 01fe7f47dac83974dfec378a5925936124433a8c Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 10:48:00 +0200 Subject: [PATCH 13/36] Well that's a bit annoying --- src/bin/test.rs | 2 +- tests/windows_minidump_writer.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 6a43592c..15b8a3d8 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -303,7 +303,7 @@ mod windows { }; pub(super) fn real_main(args: Vec) -> Result<()> { - let exception_code = i32::from_str_radix(&args[0], 16).unwrap(); + let exception_code = u32::from_str_radix(&args[0], 16).unwrap() as i32; // Generate the exception and communicate back where the exception pointers // are diff --git a/tests/windows_minidump_writer.rs b/tests/windows_minidump_writer.rs index 6fbf4b3b..39815716 100644 --- a/tests/windows_minidump_writer.rs +++ b/tests/windows_minidump_writer.rs @@ -96,7 +96,7 @@ fn dump_external_process() { let exception_pointers: usize = biter.next().unwrap().parse().unwrap(); let thread_id: u32 = biter.next().unwrap().parse().unwrap(); - let exception_code = i32::from_str_radix(biter.next().unwrap(), 16).unwrap(); + let exception_code = u32::from_str_radix(biter.next().unwrap(), 16).unwrap() as i32; assert_eq!(exception_code, EXCEPTION_ACCESS_VIOLATION); From 2cecfe9d85ccf103c57d66693285440eb220b6f0 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 10:55:51 +0200 Subject: [PATCH 14/36] Minor cleanup --- src/bin/test.rs | 10 ++++++---- tests/windows_minidump_writer.rs | 18 +++++++++++------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 15b8a3d8..1cfbd906 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -328,10 +328,10 @@ mod windows { println!("{exc_ptr_addr} {tid} {exception_code:x}"); // Wait until we're killed - loop { - std::thread::park(); - } + std::thread::sleep(std::time::Duration::from_secs_f32(60.0)); } + + panic!("we should be killed"); } } @@ -341,8 +341,10 @@ fn main() -> Result<()> { cfg_if::cfg_if! { if #[cfg(any(target_os = "linux", target_os = "android"))] { linux::real_main(args) - } else { + } else if #[cfg(target_os = "windows")] { windows::real_main(args) + } else { + unimplemented!(); } } } diff --git a/tests/windows_minidump_writer.rs b/tests/windows_minidump_writer.rs index 39815716..47fa3a9e 100644 --- a/tests/windows_minidump_writer.rs +++ b/tests/windows_minidump_writer.rs @@ -88,15 +88,19 @@ fn dump_external_process() { let mut child = start_child_and_return(&format!("{:x}", EXCEPTION_ACCESS_VIOLATION)); - let mut f = std::io::BufReader::new(child.stdout.as_mut().expect("Can't open stdout")); - let mut buf = String::new(); - f.read_line(&mut buf).expect("failed to read stdout"); + let (exception_pointers, thread_id, exception_code) = { + let mut f = std::io::BufReader::new(child.stdout.as_mut().expect("Can't open stdout")); + let mut buf = String::new(); + f.read_line(&mut buf).expect("failed to read stdout"); - let mut biter = buf.split(' '); + let mut biter = buf.split(' '); - let exception_pointers: usize = biter.next().unwrap().parse().unwrap(); - let thread_id: u32 = biter.next().unwrap().parse().unwrap(); - let exception_code = u32::from_str_radix(biter.next().unwrap(), 16).unwrap() as i32; + let exception_pointers: usize = biter.next().unwrap().parse().unwrap(); + let thread_id: u32 = biter.next().unwrap().parse().unwrap(); + let exception_code = u32::from_str_radix(biter.next().unwrap(), 16).unwrap() as i32; + + (exception_pointers, thread_id, exception_code) + }; assert_eq!(exception_code, EXCEPTION_ACCESS_VIOLATION); From 7fcb05a265e30832c2a43a61bcd2adce60d3d137 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 10:57:38 +0200 Subject: [PATCH 15/36] Buffer should not be empty --- tests/windows_minidump_writer.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/windows_minidump_writer.rs b/tests/windows_minidump_writer.rs index 47fa3a9e..3e80fdba 100644 --- a/tests/windows_minidump_writer.rs +++ b/tests/windows_minidump_writer.rs @@ -92,6 +92,7 @@ fn dump_external_process() { let mut f = std::io::BufReader::new(child.stdout.as_mut().expect("Can't open stdout")); let mut buf = String::new(); f.read_line(&mut buf).expect("failed to read stdout"); + assert!(!buf.is_empty()); let mut biter = buf.split(' '); From fbe23743e575745211cc89d3ea8f988669eb5be1 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 11:09:56 +0200 Subject: [PATCH 16/36] Get CONTEXT on another thread --- src/bin/test.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 1cfbd906..20f0ae77 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -309,11 +309,14 @@ mod windows { // are unsafe { let mut exception_record: EXCEPTION_RECORD = mem::zeroed(); - let mut exception_context = mem::MaybeUninit::uninit(); - RtlCaptureContext(exception_context.as_mut_ptr()); - - let mut exception_context = exception_context.assume_init(); + let mut exception_context = std::thread::spawn(move || { + let mut exception_context = mem::MaybeUninit::uninit(); + RtlCaptureContext(exception_context.as_mut_ptr()); + exception_context.assume_init() + }) + .join() + .unwrap(); let exception_ptrs = EXCEPTION_POINTERS { ExceptionRecord: &mut exception_record, From 61341d773f5e37b688e8285389bbb03bf961b1b4 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 11:52:28 +0200 Subject: [PATCH 17/36] Try to keep RtlCaptureContext from crashing --- src/bin/test.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 20f0ae77..3cedd462 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -302,6 +302,7 @@ mod windows { Threading::GetCurrentThreadId, }; + #[inline(never)] pub(super) fn real_main(args: Vec) -> Result<()> { let exception_code = u32::from_str_radix(&args[0], 16).unwrap() as i32; @@ -309,14 +310,10 @@ mod windows { // are unsafe { let mut exception_record: EXCEPTION_RECORD = mem::zeroed(); + let mut exception_context = mem::MaybeUninit::uninit(); - let mut exception_context = std::thread::spawn(move || { - let mut exception_context = mem::MaybeUninit::uninit(); - RtlCaptureContext(exception_context.as_mut_ptr()); - exception_context.assume_init() - }) - .join() - .unwrap(); + RtlCaptureContext(exception_context.as_mut_ptr()); + let mut exception_context = exception_context.assume_init(); let exception_ptrs = EXCEPTION_POINTERS { ExceptionRecord: &mut exception_record, From d48f8a9601061acb3330a53ba578463d38564a0f Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 12:08:30 +0200 Subject: [PATCH 18/36] Grasping at straws here --- src/bin/test.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 3cedd462..44429609 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -298,7 +298,7 @@ mod windows { use super::*; use std::mem; use windows_sys::Win32::System::{ - Diagnostics::Debug::{RtlCaptureContext, EXCEPTION_POINTERS, EXCEPTION_RECORD}, + Diagnostics::Debug::{RtlCaptureContext, CONTEXT, EXCEPTION_POINTERS, EXCEPTION_RECORD}, Threading::GetCurrentThreadId, }; @@ -312,7 +312,12 @@ mod windows { let mut exception_record: EXCEPTION_RECORD = mem::zeroed(); let mut exception_context = mem::MaybeUninit::uninit(); - RtlCaptureContext(exception_context.as_mut_ptr()); + #[inline(never)] + unsafe fn another_function(ctx: *mut CONTEXT) { + RtlCaptureContext(ctx); + } + + another_function(exception_context.as_mut_ptr()); let mut exception_context = exception_context.assume_init(); let exception_ptrs = EXCEPTION_POINTERS { From d9b2492b3d0dc5ad1c7190fb93dd661fc3860f34 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 12:11:31 +0200 Subject: [PATCH 19/36] Just for shits and giggles --- src/bin/test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 44429609..5eec0736 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -310,7 +310,7 @@ mod windows { // are unsafe { let mut exception_record: EXCEPTION_RECORD = mem::zeroed(); - let mut exception_context = mem::MaybeUninit::uninit(); + let mut exception_context = mem::MaybeUninit::zeroed(); #[inline(never)] unsafe fn another_function(ctx: *mut CONTEXT) { From f7587bebb0b37e5bc90c5be8c582169f655bcccf Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 12:14:28 +0200 Subject: [PATCH 20/36] Ugh --- src/bin/test.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 5eec0736..3d59c811 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -310,15 +310,7 @@ mod windows { // are unsafe { let mut exception_record: EXCEPTION_RECORD = mem::zeroed(); - let mut exception_context = mem::MaybeUninit::zeroed(); - - #[inline(never)] - unsafe fn another_function(ctx: *mut CONTEXT) { - RtlCaptureContext(ctx); - } - - another_function(exception_context.as_mut_ptr()); - let mut exception_context = exception_context.assume_init(); + let mut exception_context: CONTEXT = mem::zeroed(); let exception_ptrs = EXCEPTION_POINTERS { ExceptionRecord: &mut exception_record, From 597e8b733be3288f5f65ea5c6e79b7a013e212c4 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 12:17:38 +0200 Subject: [PATCH 21/36] Use loop --- src/bin/test.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 3d59c811..18366b45 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -298,7 +298,7 @@ mod windows { use super::*; use std::mem; use windows_sys::Win32::System::{ - Diagnostics::Debug::{RtlCaptureContext, CONTEXT, EXCEPTION_POINTERS, EXCEPTION_RECORD}, + Diagnostics::Debug::{CONTEXT, EXCEPTION_POINTERS, EXCEPTION_RECORD}, Threading::GetCurrentThreadId, }; @@ -325,10 +325,10 @@ mod windows { println!("{exc_ptr_addr} {tid} {exception_code:x}"); // Wait until we're killed - std::thread::sleep(std::time::Duration::from_secs_f32(60.0)); + loop { + std::thread::park(); + } } - - panic!("we should be killed"); } } From 1807a63237b0508e245f5f2c89cd39134e7f6cd2 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 12:19:36 +0200 Subject: [PATCH 22/36] Trim string --- tests/windows_minidump_writer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/windows_minidump_writer.rs b/tests/windows_minidump_writer.rs index 3e80fdba..7a56d192 100644 --- a/tests/windows_minidump_writer.rs +++ b/tests/windows_minidump_writer.rs @@ -94,7 +94,7 @@ fn dump_external_process() { f.read_line(&mut buf).expect("failed to read stdout"); assert!(!buf.is_empty()); - let mut biter = buf.split(' '); + let mut biter = buf.trim().split(' '); let exception_pointers: usize = biter.next().unwrap().parse().unwrap(); let thread_id: u32 = biter.next().unwrap().parse().unwrap(); From df973a33ffd9130681dfae62b3b10e1865fc206f Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 12:21:12 +0200 Subject: [PATCH 23/36] Use illegal instead, access violation is special --- tests/windows_minidump_writer.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/windows_minidump_writer.rs b/tests/windows_minidump_writer.rs index 7a56d192..68d8325e 100644 --- a/tests/windows_minidump_writer.rs +++ b/tests/windows_minidump_writer.rs @@ -4,7 +4,7 @@ use minidump::{CrashReason, Minidump, MinidumpMemoryList, MinidumpSystemInfo, Mi use minidump_writer::minidump_writer::MinidumpWriter; use std::mem; use windows_sys::Win32::{ - Foundation::{EXCEPTION_ACCESS_VIOLATION, STATUS_INVALID_PARAMETER}, + Foundation::{EXCEPTION_ILLEGAL_INSTRUCTION, STATUS_INVALID_PARAMETER}, System::{ Diagnostics::Debug::{RtlCaptureContext, EXCEPTION_POINTERS, EXCEPTION_RECORD}, Threading::GetCurrentThreadId, @@ -86,7 +86,7 @@ fn dump_current_process() { fn dump_external_process() { use std::io::BufRead; - let mut child = start_child_and_return(&format!("{:x}", EXCEPTION_ACCESS_VIOLATION)); + let mut child = start_child_and_return(&format!("{:x}", EXCEPTION_ILLEGAL_INSTRUCTION)); let (exception_pointers, thread_id, exception_code) = { let mut f = std::io::BufReader::new(child.stdout.as_mut().expect("Can't open stdout")); @@ -103,7 +103,7 @@ fn dump_external_process() { (exception_pointers, thread_id, exception_code) }; - assert_eq!(exception_code, EXCEPTION_ACCESS_VIOLATION); + assert_eq!(exception_code, EXCEPTION_ILLEGAL_INSTRUCTION); let crash_context = crash_context::CrashContext { exception_pointers: exception_pointers as _, @@ -135,6 +135,6 @@ fn dump_external_process() { assert_eq!( crash_reason, - CrashReason::from_windows_error(EXCEPTION_ACCESS_VIOLATION as u32) + CrashReason::from_windows_error(EXCEPTION_ILLEGAL_INSTRUCTION as u32) ); } From 110e33370b973b5d21398a2be84b5eede8018e30 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 12:24:03 +0200 Subject: [PATCH 24/36] Don't pretend we have a CONTEXT record --- src/bin/test.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 18366b45..142cbfc0 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -310,11 +310,10 @@ mod windows { // are unsafe { let mut exception_record: EXCEPTION_RECORD = mem::zeroed(); - let mut exception_context: CONTEXT = mem::zeroed(); let exception_ptrs = EXCEPTION_POINTERS { ExceptionRecord: &mut exception_record, - ContextRecord: &mut exception_context, + ContextRecord: std::ptr::null_mut(), }; exception_record.ExceptionCode = exception_code; From 97b20c4afd0ff2f1f8d9a36e45ab0578222f046d Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 12:43:46 +0200 Subject: [PATCH 25/36] Try GetThreadContext instead --- src/bin/test.rs | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 142cbfc0..dd01a10d 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -297,9 +297,12 @@ mod linux { mod windows { use super::*; use std::mem; - use windows_sys::Win32::System::{ - Diagnostics::Debug::{CONTEXT, EXCEPTION_POINTERS, EXCEPTION_RECORD}, - Threading::GetCurrentThreadId, + use windows_sys::Win32::{ + Foundation::CloseHandle, + System::{ + Diagnostics::Debug::{GetThreadContext, CONTEXT, EXCEPTION_POINTERS, EXCEPTION_RECORD}, + Threading::{GetCurrentThreadId, OpenThread, THREAD_ALL_ACCESS}, + }, }; #[inline(never)] @@ -310,16 +313,24 @@ mod windows { // are unsafe { let mut exception_record: EXCEPTION_RECORD = mem::zeroed(); + let mut exception_context: CONTEXT = mem::zeroed(); + exception_context.ContextFlags = + minidump_common::format::ContextFlagsAmd64::CONTEXT_AMD64_ALL.bits(); + + let tid = GetCurrentThreadId(); + + let thread_handle = OpenThread(THREAD_ALL_ACCESS, 0, tid); + GetThreadContext(thread_handle, &mut exception_context); + CloseHandle(thread_handle); let exception_ptrs = EXCEPTION_POINTERS { ExceptionRecord: &mut exception_record, - ContextRecord: std::ptr::null_mut(), + ContextRecord: &mut exception_context, }; exception_record.ExceptionCode = exception_code; let exc_ptr_addr = &exception_ptrs as *const _ as usize; - let tid = GetCurrentThreadId(); println!("{exc_ptr_addr} {tid} {exception_code:x}"); From b9a85921a8b7c104453a3a6b86610e8e6bba22d6 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 12:53:59 +0200 Subject: [PATCH 26/36] Add windows test to GHA --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 31b9abd1..c7dcbd0b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,8 +25,8 @@ jobs: - { os: ubuntu-latest, target: arm-unknown-linux-musleabi, use-cross: true } - { os: ubuntu-latest, target: arm-linux-androideabi, use-cross: true } - { os: ubuntu-latest, target: arm-unknown-linux-gnueabihf, use-cross: true } + - { os: windows-2022, target: x86_64-pc-windows-msvc, use-cross: false } #- { os: macos-latest, target: x86_64-apple-darwin, use-cross: false } - #- { os: windows-latest, target: x86_64-pc-windows-msvc, use-cross: false } steps: - name: Checkout repository uses: actions/checkout@v2 From 7f5b7d79dbd77bd2034f67fa2eec173497ef1582 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 30 Mar 2022 13:07:15 +0200 Subject: [PATCH 27/36] Cancel in flight actions when new commits are pushed to PRs --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c7dcbd0b..f6c734ba 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,6 +7,10 @@ on: - github-actions pull_request: +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + jobs: unit_tests: name: Build sources From 30fce5768cc79c1601002804e7e7d341e93c9dc6 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Thu, 21 Apr 2022 13:52:09 +0200 Subject: [PATCH 28/36] Note PR that implements pWrite for MINIDUMP_BREAKPAD_INFO --- src/windows/minidump_writer.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/windows/minidump_writer.rs b/src/windows/minidump_writer.rs index 2bf30165..b080e80d 100644 --- a/src/windows/minidump_writer.rs +++ b/src/windows/minidump_writer.rs @@ -125,6 +125,7 @@ impl MinidumpWriter { }; // TODO: derive Pwrite for MINIDUMP_BREAKPAD_INFO + // https://github.com/rust-minidump/rust-minidump/pull/534 let mut offset = 0; offset += breakpad_info.pwrite(bp_info.validity, offset)?; offset += breakpad_info.pwrite(bp_info.dump_thread_id, offset)?; From aa2e999de11e73dfdd57b52673f988bdab105b5b Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Thu, 21 Apr 2022 14:10:34 +0200 Subject: [PATCH 29/36] Update crash-context to 0.1 --- Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0533eecd..8b839010 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ license = "MIT" [dependencies] byteorder = "1.3.2" cfg-if = "1.0" -crash-context = "0.0.3" +crash-context = "0.1" memoffset = "0.6" minidump-common = "0.10" scroll = "0.11" @@ -24,7 +24,7 @@ memmap2 = "0.5" nix = "0.23" [target.'cfg(target_os = "windows")'.dependencies.windows-sys] -version = "0.34" +version = "0.35" features = [ # MiniDumpWriteDump requires...a lot of features "Win32_Foundation", From d163ae510ea0dcb5416a396e2710b44db4585733 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Thu, 21 Apr 2022 16:59:54 +0200 Subject: [PATCH 30/36] Fix bump to 0.1 --- tests/windows_minidump_writer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/windows_minidump_writer.rs b/tests/windows_minidump_writer.rs index 68d8325e..2744af47 100644 --- a/tests/windows_minidump_writer.rs +++ b/tests/windows_minidump_writer.rs @@ -50,7 +50,7 @@ fn dump_current_process() { exception_record.ExceptionCode = STATUS_INVALID_PARAMETER; let crash_context = crash_context::CrashContext { - exception_pointers: &exception_ptrs, + exception_pointers: (&exception_ptrs as *const EXCEPTION_POINTERS).cast(), thread_id: GetCurrentThreadId(), exception_code: STATUS_INVALID_PARAMETER, }; From c60c57b399284c1bd9466e80a7376690b6b02def Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Fri, 22 Apr 2022 07:07:01 +0200 Subject: [PATCH 31/36] Address feedback --- src/windows/minidump_writer.rs | 102 ++++++++++++++----------------- tests/windows_minidump_writer.rs | 3 +- 2 files changed, 47 insertions(+), 58 deletions(-) diff --git a/src/windows/minidump_writer.rs b/src/windows/minidump_writer.rs index b080e80d..6a4dede6 100644 --- a/src/windows/minidump_writer.rs +++ b/src/windows/minidump_writer.rs @@ -5,16 +5,9 @@ use std::{ffi::c_void, os::windows::io::AsRawHandle}; pub use windows_sys::Win32::Foundation::HANDLE; use windows_sys::Win32::{ Foundation::{CloseHandle, ERROR_SUCCESS, STATUS_INVALID_HANDLE}, - System::{ - ApplicationVerifier as av, - Diagnostics::Debug as md, - Threading::{GetCurrentThreadId, OpenProcess}, - }, + System::{ApplicationVerifier as av, Diagnostics::Debug as md, Threading as threading}, }; -// For some reason this is defined in SystemServices which is massive and not worth bringing in for ONE constant -const GENERIC_ALL: u32 = 268435456; - pub struct MinidumpWriter { /// The crash context as captured by an exception handler crash_context: crash_context::CrashContext, @@ -34,15 +27,20 @@ pub struct MinidumpWriter { impl MinidumpWriter { /// Creates a minidump writer for a crash that occurred in an external process. + /// + /// # Errors + /// + /// Fails if we are unable to open the external process for some reason pub fn external_process( crash_context: crash_context::CrashContext, pid: u32, ) -> Result { + // SAFETY: syscall let crashing_process = unsafe { - OpenProcess( - GENERIC_ALL, // desired access - 0, // inherit handles - pid, // pid + threading::OpenProcess( + threading::PROCESS_ALL_ACCESS, // desired access + 0, // inherit handles + pid, // pid ) }; @@ -60,30 +58,20 @@ impl MinidumpWriter { /// Creates a minidump writer for a crash that occurred in the current process. /// - /// # Errors - /// - /// Fails if we are unable to open a `HANDLE` to the current process - pub fn current_process(crash_context: crash_context::CrashContext) -> Result { + /// Note that in-process dumping is inherently unreliable, it is recommended + /// to use the [`Self::external_process`] in a different process than the + /// one that crashed when possible. + pub fn current_process(crash_context: crash_context::CrashContext) -> Self { let crashing_pid = std::process::id(); // SAFETY: syscall - let crashing_process = unsafe { - OpenProcess( - GENERIC_ALL, // desired access - 0, // inherit handles - crashing_pid, - ) - }; + let crashing_process = unsafe { threading::GetCurrentProcess() }; - if crashing_process == 0 { - Err(std::io::Error::last_os_error().into()) - } else { - Ok(Self { - crash_context, - crashing_process, - crashing_pid, - is_external_process: false, - }) + Self { + crash_context, + crashing_process, + crashing_pid, + is_external_process: false, } } @@ -121,15 +109,15 @@ impl MinidumpWriter { | BreakpadInfoValid::RequestingThreadId.bits(), dump_thread_id: self.crash_context.thread_id, // Safety: syscall - requesting_thread_id: unsafe { GetCurrentThreadId() }, + requesting_thread_id: unsafe { threading::GetCurrentThreadId() }, }; // TODO: derive Pwrite for MINIDUMP_BREAKPAD_INFO // https://github.com/rust-minidump/rust-minidump/pull/534 let mut offset = 0; - offset += breakpad_info.pwrite(bp_info.validity, offset)?; - offset += breakpad_info.pwrite(bp_info.dump_thread_id, offset)?; - breakpad_info.pwrite(bp_info.requesting_thread_id, offset)?; + breakpad_info.gwrite(bp_info.validity, &mut offset)?; + breakpad_info.gwrite(bp_info.dump_thread_id, &mut offset)?; + breakpad_info.gwrite(bp_info.requesting_thread_id, &mut offset)?; user_streams.push(md::MINIDUMP_USER_STREAM { Type: MINIDUMP_STREAM_TYPE::BreakpadInfoStream as u32, @@ -139,16 +127,23 @@ impl MinidumpWriter { }); } - let handle_stream = self.fill_handle_stream(); + let mut handle_stream_buffer = self.fill_handle_stream(); // Note that we do this by ref, as the buffer inside the option needs // to stay alive for as long as we're writing the minidump since // the user stream has a pointer to it - if let Some((_buf, handle_stream)) = &handle_stream { - user_streams.push(*handle_stream); + if let Some(buf) = &mut handle_stream_buffer { + let handle_stream = md::MINIDUMP_USER_STREAM { + Type: MINIDUMP_STREAM_TYPE::HandleOperationListStream as u32, + BufferSize: buf.len() as u32, + // Still not getting over the mut pointers here + Buffer: buf.as_mut_ptr().cast(), + }; + + user_streams.push(handle_stream); } - let user_stream = md::MINIDUMP_USER_STREAM_INFORMATION { + let user_stream_infos = md::MINIDUMP_USER_STREAM_INFORMATION { UserStreamCount: user_streams.len() as u32, UserStreamArray: user_streams.as_mut_ptr(), }; @@ -165,8 +160,8 @@ impl MinidumpWriter { exc_info .as_ref() .map_or(std::ptr::null(), |ei| ei as *const _), // exceptionparam - the actual exception information - &user_stream, // user streams - std::ptr::null(), // callback, unused + &user_stream_infos, // user streams + std::ptr::null(), // callback, unused ) }; @@ -181,8 +176,8 @@ impl MinidumpWriter { /// enumerates all of the handle operations that occurred within the crashing /// process and fills out a minidump user stream with the ops pertaining to /// the last invalid handle that is enumerated, which is, presumably - /// (hopfully?), the one that led to the exception - fn fill_handle_stream(&self) -> Option<(Vec, md::MINIDUMP_USER_STREAM)> { + /// (hopefully?), the one that led to the exception + fn fill_handle_stream(&self) -> Option> { if self.crash_context.exception_code != STATUS_INVALID_HANDLE { return None; } @@ -227,7 +222,7 @@ impl MinidumpWriter { Some(enum_callback), // enumeration callback (&mut hs as *mut HandleState).cast(), // enumeration context ) - } != 0 + } == 0 { let mut stream_buf = Vec::new(); @@ -243,9 +238,11 @@ impl MinidumpWriter { #[inline] fn to_bytes(v: &T) -> &[u8] { - let s = std::slice::from_ref(v); - - unsafe { std::slice::from_raw_parts(s.as_ptr().cast(), std::mem::size_of::()) } + // SAFETY: both AVRF_HANDLE_OPERATION and MINIDUMP_HANDLE_OPERATION_LIST + // are POD types + unsafe { + std::slice::from_raw_parts((v as *const T).cast(), std::mem::size_of::()) + } } for op in hs.ops.into_iter().filter(|op| op.Handle == hs.last_invalid) { @@ -255,14 +252,7 @@ impl MinidumpWriter { stream_buf[..md_list.SizeOfHeader as usize].copy_from_slice(to_bytes(&md_list)); - let handle_stream = md::MINIDUMP_USER_STREAM { - Type: MINIDUMP_STREAM_TYPE::HandleOperationListStream as u32, - BufferSize: stream_buf.len() as u32, - // Still not getting over the mut pointers here - Buffer: stream_buf.as_mut_ptr().cast(), - }; - - Some((stream_buf, handle_stream)) + Some(stream_buf) } else { // We don't _particularly_ care if this fails, it's better if we had // the info, but not critical diff --git a/tests/windows_minidump_writer.rs b/tests/windows_minidump_writer.rs index 2744af47..bfcae31c 100644 --- a/tests/windows_minidump_writer.rs +++ b/tests/windows_minidump_writer.rs @@ -55,8 +55,7 @@ fn dump_current_process() { exception_code: STATUS_INVALID_PARAMETER, }; - let dumper = MinidumpWriter::current_process(crash_context) - .expect("failed to create MinidumpWriter"); + let dumper = MinidumpWriter::current_process(crash_context); dumper .dump(tmpfile.as_file_mut()) From 7d927df3133ad1d7fb592680f39b37d435bf7583 Mon Sep 17 00:00:00 2001 From: Chris Martin Date: Fri, 22 Apr 2022 12:39:57 -0400 Subject: [PATCH 32/36] Tests: Pass PID of child process to parent This fixes the "ReadProcessMemory" failure in the MiniDumpWriteDump() call. The root issue is that Cargo is being used to run the test executable, and so `child.id()` returns Cargo's PID and not the PID of the test.exe executable. This has the test.exe pass its actual PID back to the parent so it can be debugged properly. Note that this doesn't fully fix the test -- There is still another error --- src/bin/test.rs | 5 +++-- tests/windows_minidump_writer.rs | 7 ++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index dd01a10d..41d608db 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -301,7 +301,7 @@ mod windows { Foundation::CloseHandle, System::{ Diagnostics::Debug::{GetThreadContext, CONTEXT, EXCEPTION_POINTERS, EXCEPTION_RECORD}, - Threading::{GetCurrentThreadId, OpenThread, THREAD_ALL_ACCESS}, + Threading::{GetCurrentProcessId, GetCurrentThreadId, OpenThread, THREAD_ALL_ACCESS}, }, }; @@ -317,6 +317,7 @@ mod windows { exception_context.ContextFlags = minidump_common::format::ContextFlagsAmd64::CONTEXT_AMD64_ALL.bits(); + let pid = GetCurrentProcessId(); let tid = GetCurrentThreadId(); let thread_handle = OpenThread(THREAD_ALL_ACCESS, 0, tid); @@ -332,7 +333,7 @@ mod windows { let exc_ptr_addr = &exception_ptrs as *const _ as usize; - println!("{exc_ptr_addr} {tid} {exception_code:x}"); + println!("{pid} {exc_ptr_addr} {tid} {exception_code:x}"); // Wait until we're killed loop { diff --git a/tests/windows_minidump_writer.rs b/tests/windows_minidump_writer.rs index bfcae31c..61779875 100644 --- a/tests/windows_minidump_writer.rs +++ b/tests/windows_minidump_writer.rs @@ -87,7 +87,7 @@ fn dump_external_process() { let mut child = start_child_and_return(&format!("{:x}", EXCEPTION_ILLEGAL_INSTRUCTION)); - let (exception_pointers, thread_id, exception_code) = { + let (process_id, exception_pointers, thread_id, exception_code) = { let mut f = std::io::BufReader::new(child.stdout.as_mut().expect("Can't open stdout")); let mut buf = String::new(); f.read_line(&mut buf).expect("failed to read stdout"); @@ -95,11 +95,12 @@ fn dump_external_process() { let mut biter = buf.trim().split(' '); + let process_id: u32 = biter.next().unwrap().parse().unwrap(); let exception_pointers: usize = biter.next().unwrap().parse().unwrap(); let thread_id: u32 = biter.next().unwrap().parse().unwrap(); let exception_code = u32::from_str_radix(biter.next().unwrap(), 16).unwrap() as i32; - (exception_pointers, thread_id, exception_code) + (process_id, exception_pointers, thread_id, exception_code) }; assert_eq!(exception_code, EXCEPTION_ILLEGAL_INSTRUCTION); @@ -115,7 +116,7 @@ fn dump_external_process() { .tempfile() .unwrap(); - let dumper = MinidumpWriter::external_process(crash_context, child.id()) + let dumper = MinidumpWriter::external_process(crash_context, process_id) .expect("failed to create MinidumpWriter"); dumper From 07526b5b9cb0d546899d6d36f3466e5ffe2f9886 Mon Sep 17 00:00:00 2001 From: Chris Martin Date: Fri, 22 Apr 2022 12:55:26 -0400 Subject: [PATCH 33/36] Tests: Use CrashReason::from_windows_code for exception --- tests/windows_minidump_writer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/windows_minidump_writer.rs b/tests/windows_minidump_writer.rs index 61779875..7415e370 100644 --- a/tests/windows_minidump_writer.rs +++ b/tests/windows_minidump_writer.rs @@ -135,6 +135,6 @@ fn dump_external_process() { assert_eq!( crash_reason, - CrashReason::from_windows_error(EXCEPTION_ILLEGAL_INSTRUCTION as u32) + CrashReason::from_windows_code(EXCEPTION_ILLEGAL_INSTRUCTION as u32) ); } From 79493f9d38544f79665a134c8d0e28f13c1b6e21 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Sat, 23 Apr 2022 08:36:57 +0200 Subject: [PATCH 34/36] Address feedback --- src/bin/test.rs | 4 +- src/windows/minidump_writer.rs | 69 +++++++++++++++++++++------------- 2 files changed, 44 insertions(+), 29 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 41d608db..91422890 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -301,7 +301,7 @@ mod windows { Foundation::CloseHandle, System::{ Diagnostics::Debug::{GetThreadContext, CONTEXT, EXCEPTION_POINTERS, EXCEPTION_RECORD}, - Threading::{GetCurrentProcessId, GetCurrentThreadId, OpenThread, THREAD_ALL_ACCESS}, + Threading::{GetCurrentProcessId, GetCurrentThread, GetCurrentThreadId}, }, }; @@ -320,7 +320,7 @@ mod windows { let pid = GetCurrentProcessId(); let tid = GetCurrentThreadId(); - let thread_handle = OpenThread(THREAD_ALL_ACCESS, 0, tid); + let thread_handle = GetCurrentThread(); GetThreadContext(thread_handle, &mut exception_context); CloseHandle(thread_handle); diff --git a/src/windows/minidump_writer.rs b/src/windows/minidump_writer.rs index 6a4dede6..ce6bc2f8 100644 --- a/src/windows/minidump_writer.rs +++ b/src/windows/minidump_writer.rs @@ -94,36 +94,14 @@ impl MinidumpWriter { // (currently) a real target of this crate, so this allocation is fine let mut user_streams = Vec::with_capacity(2); - // Add an MDRawBreakpadInfo stream to the minidump, to provide additional - // information about the exception handler to the Breakpad processor. - // The information will help the processor determine which threads are - // relevant. The Breakpad processor does not require this information but - // can function better with Breakpad-generated dumps when it is present. - // The native debugger is not harmed by the presence of this information. - // - // This info is only relevant for in-process dumping - let mut breakpad_info = [0u8; 12]; - if !self.is_external_process { - let bp_info = MINIDUMP_BREAKPAD_INFO { - validity: BreakpadInfoValid::DumpThreadId.bits() - | BreakpadInfoValid::RequestingThreadId.bits(), - dump_thread_id: self.crash_context.thread_id, - // Safety: syscall - requesting_thread_id: unsafe { threading::GetCurrentThreadId() }, - }; - - // TODO: derive Pwrite for MINIDUMP_BREAKPAD_INFO - // https://github.com/rust-minidump/rust-minidump/pull/534 - let mut offset = 0; - breakpad_info.gwrite(bp_info.validity, &mut offset)?; - breakpad_info.gwrite(bp_info.dump_thread_id, &mut offset)?; - breakpad_info.gwrite(bp_info.requesting_thread_id, &mut offset)?; + let mut breakpad_info = self.fill_breakpad_stream(); + if let Some(bp_info) = &mut breakpad_info { user_streams.push(md::MINIDUMP_USER_STREAM { Type: MINIDUMP_STREAM_TYPE::BreakpadInfoStream as u32, - BufferSize: breakpad_info.len() as u32, + BufferSize: bp_info.len() as u32, // Again with the mut pointer - Buffer: breakpad_info.as_mut_ptr().cast(), + Buffer: bp_info.as_mut_ptr().cast(), }); } @@ -172,6 +150,43 @@ impl MinidumpWriter { } } + /// Create an MDRawBreakpadInfo stream to the minidump, to provide additional + /// information about the exception handler to the Breakpad processor. + /// The information will help the processor determine which threads are + /// relevant. The Breakpad processor does not require this information but + /// can function better with Breakpad-generated dumps when it is present. + /// The native debugger is not harmed by the presence of this information. + /// + /// This info is only relevant for in-process dumping + fn fill_breakpad_stream(&self) -> Option<[u8; 12]> { + if self.is_external_process { + return None; + } + + let mut breakpad_info = [0u8; 12]; + + let bp_info = MINIDUMP_BREAKPAD_INFO { + validity: BreakpadInfoValid::DumpThreadId.bits() + | BreakpadInfoValid::RequestingThreadId.bits(), + dump_thread_id: self.crash_context.thread_id, + // Safety: syscall + requesting_thread_id: unsafe { threading::GetCurrentThreadId() }, + }; + + // TODO: derive Pwrite for MINIDUMP_BREAKPAD_INFO + // https://github.com/rust-minidump/rust-minidump/pull/534 + let mut offset = 0; + breakpad_info.gwrite(bp_info.validity, &mut offset).ok()?; + breakpad_info + .gwrite(bp_info.dump_thread_id, &mut offset) + .ok()?; + breakpad_info + .gwrite(bp_info.requesting_thread_id, &mut offset) + .ok()?; + + Some(breakpad_info) + } + /// In the case of a `STATUS_INVALID_HANDLE` exception, this function /// enumerates all of the handle operations that occurred within the crashing /// process and fills out a minidump user stream with the ops pertaining to @@ -222,7 +237,7 @@ impl MinidumpWriter { Some(enum_callback), // enumeration callback (&mut hs as *mut HandleState).cast(), // enumeration context ) - } == 0 + } == ERROR_SUCCESS { let mut stream_buf = Vec::new(); From 233322bdc6deb14e8a7521d0344050e2deb38704 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Mon, 25 Apr 2022 09:45:38 +0200 Subject: [PATCH 35/36] Downgrade to 0.34 to better align with ecosystem --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 8b839010..f402185c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ memmap2 = "0.5" nix = "0.23" [target.'cfg(target_os = "windows")'.dependencies.windows-sys] -version = "0.35" +version = "0.34" # Latest is 0.35 (for now), but a vast majority of the ecosystem uses 0.34 features = [ # MiniDumpWriteDump requires...a lot of features "Win32_Foundation", From 0083d50662ae298093e56029e42b9cdb9b08b55b Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Mon, 25 Apr 2022 23:44:46 +0200 Subject: [PATCH 36/36] Cleanup --- src/bin/test.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 91422890..8a8f4095 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -320,9 +320,7 @@ mod windows { let pid = GetCurrentProcessId(); let tid = GetCurrentThreadId(); - let thread_handle = GetCurrentThread(); - GetThreadContext(thread_handle, &mut exception_context); - CloseHandle(thread_handle); + GetThreadContext(GetCurrentThread(), &mut exception_context); let exception_ptrs = EXCEPTION_POINTERS { ExceptionRecord: &mut exception_record,