From 854b3d4bd38dae94e27f05648da0a0ae4cbd88c5 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Thu, 17 Nov 2022 14:15:19 +0100 Subject: [PATCH 1/5] Use replacement for RtlCaptureContext This uses the RtlCaptureContext replacement from crash-context, which also has correct bindings for CONTEXT. This also changes to use winapi for most bindings, other than some minidump related types/functions that are not provided by winapi. --- Cargo.toml | 8 +- src/windows/ffi.rs | 525 ++------------------------------- src/windows/minidump_writer.rs | 34 ++- 3 files changed, 46 insertions(+), 521 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 94ba7268..c387a9f7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ license = "MIT" [dependencies] byteorder = "1.3.2" cfg-if = "1.0" -crash-context = "0.4" +crash-context = "0.5" memoffset = "0.7" minidump-common = "0.14" scroll = "0.11" @@ -35,6 +35,12 @@ nix = { version = "0.25", default-features = false, features = [ # Binds some additional mac specifics not in libc mach2 = "0.4" +# Additional bindings to Windows specific APIs. Note we don't use windows-sys +# due to massive version churn +[target.'cfg(target_os = "windows")'.dependencies.winapi] +version = "0.3" +features = ["minwindef", "processthreadsapi", "winnt"] + [dev-dependencies] # Minidump-processor is async so we need an executor futures = { version = "0.3", features = ["executor"] } diff --git a/src/windows/ffi.rs b/src/windows/ffi.rs index 8a9dcca7..18e9e938 100644 --- a/src/windows/ffi.rs +++ b/src/windows/ffi.rs @@ -1,440 +1,21 @@ -pub type BOOL = i32; -pub const FALSE: BOOL = 0; +//! Contains bindings for [`MiniDumpWriteDump`](https://learn.microsoft.com/en-us/windows/win32/api/minidumpapiset/nf-minidumpapiset-minidumpwritedump) +//! and related structures, as they are not present in `winapi` and we don't want +//! to depend on `windows-sys` due to version churn. +//! +//! Also has a binding for [`GetThreadContext`](https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getthreadcontext) +//! as the `CONTEXT` structures in `winapi` are not correctly aligned which can +//! cause crashes or bad data, so the [`crash_context::ffi::CONTEXT`] is used +//! instead. See [#63](https://github.com/rust-minidump/minidump-writer/issues/63) -pub type HANDLE = isize; -pub type NTSTATUS = i32; +#![allow(non_snake_case, non_camel_case_types, non_upper_case_globals)] -pub const STATUS_NONCONTINUABLE_EXCEPTION: NTSTATUS = -1073741787i32; +pub use crash_context::ffi::{capture_context, CONTEXT, EXCEPTION_POINTERS}; +pub use winapi::{shared::minwindef::BOOL, um::winnt::HANDLE}; -extern "system" { - pub fn CloseHandle(hobject: HANDLE) -> BOOL; -} - -// threading - -#[allow(non_camel_case_types)] -pub type PROCESS_ACCESS_RIGHTS = u32; - -pub const PROCESS_ALL_ACCESS: PROCESS_ACCESS_RIGHTS = 2097151u32; - -#[allow(non_camel_case_types)] -pub type THREAD_ACCESS_RIGHTS = u32; - -pub const THREAD_SUSPEND_RESUME: THREAD_ACCESS_RIGHTS = 2u32; -pub const THREAD_GET_CONTEXT: THREAD_ACCESS_RIGHTS = 8u32; -pub const THREAD_QUERY_INFORMATION: THREAD_ACCESS_RIGHTS = 64u32; - -extern "system" { - pub fn GetCurrentProcess() -> HANDLE; - pub fn GetCurrentProcessId() -> u32; - pub fn GetCurrentThread() -> HANDLE; - pub fn GetCurrentThreadId() -> u32; - pub fn OpenProcess( - dwdesiredaccess: PROCESS_ACCESS_RIGHTS, - binherithandle: BOOL, - dwprocessid: u32, - ) -> HANDLE; - pub fn OpenThread( - dwdesiredaccess: THREAD_ACCESS_RIGHTS, - binherithandle: BOOL, - dwthreadid: u32, - ) -> HANDLE; - pub fn ResumeThread(hthread: HANDLE) -> u32; - pub fn SuspendThread(hthread: HANDLE) -> u32; -} - -// context - -#[allow(non_snake_case)] -#[repr(C)] -pub union ARM64_NT_NEON128 { - pub Anonymous: ARM64_NT_NEON128_0, - pub D: [f64; 2], - pub S: [f32; 4], - pub H: [u16; 8], - pub B: [u8; 16], -} - -impl ::core::marker::Copy for ARM64_NT_NEON128 {} -impl ::core::clone::Clone for ARM64_NT_NEON128 { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] -#[repr(C)] -pub struct ARM64_NT_NEON128_0 { - pub Low: u64, - pub High: i64, -} - -impl ::core::marker::Copy for ARM64_NT_NEON128_0 {} -impl ::core::clone::Clone for ARM64_NT_NEON128_0 { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] -#[repr(C)] -#[cfg(target_arch = "aarch64")] -pub struct CONTEXT { - pub ContextFlags: u32, - pub Cpsr: u32, - pub Anonymous: CONTEXT_0, - pub Sp: u64, - pub Pc: u64, - pub V: [ARM64_NT_NEON128; 32], - pub Fpcr: u32, - pub Fpsr: u32, - pub Bcr: [u32; 8], - pub Bvr: [u64; 8], - pub Wcr: [u32; 2], - pub Wvr: [u64; 2], -} - -#[cfg(target_arch = "aarch64")] -impl ::core::marker::Copy for CONTEXT {} -#[cfg(target_arch = "aarch64")] -impl ::core::clone::Clone for CONTEXT { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] -#[repr(C)] -#[cfg(target_arch = "aarch64")] -pub union CONTEXT_0 { - pub Anonymous: CONTEXT_0_0, - pub X: [u64; 31], -} - -#[cfg(target_arch = "aarch64")] -impl ::core::marker::Copy for CONTEXT_0 {} -#[cfg(target_arch = "aarch64")] -impl ::core::clone::Clone for CONTEXT_0 { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] -#[repr(C)] -#[cfg(target_arch = "aarch64")] -pub struct CONTEXT_0_0 { - pub X0: u64, - pub X1: u64, - pub X2: u64, - pub X3: u64, - pub X4: u64, - pub X5: u64, - pub X6: u64, - pub X7: u64, - pub X8: u64, - pub X9: u64, - pub X10: u64, - pub X11: u64, - pub X12: u64, - pub X13: u64, - pub X14: u64, - pub X15: u64, - pub X16: u64, - pub X17: u64, - pub X18: u64, - pub X19: u64, - pub X20: u64, - pub X21: u64, - pub X22: u64, - pub X23: u64, - pub X24: u64, - pub X25: u64, - pub X26: u64, - pub X27: u64, - pub X28: u64, - pub Fp: u64, - pub Lr: u64, -} - -#[cfg(target_arch = "aarch64")] -impl ::core::marker::Copy for CONTEXT_0_0 {} -#[cfg(target_arch = "aarch64")] -impl ::core::clone::Clone for CONTEXT_0_0 { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] -#[repr(C)] -#[cfg(target_arch = "x86_64")] -pub struct CONTEXT { - pub P1Home: u64, - pub P2Home: u64, - pub P3Home: u64, - pub P4Home: u64, - pub P5Home: u64, - pub P6Home: u64, - pub ContextFlags: u32, - pub MxCsr: u32, - pub SegCs: u16, - pub SegDs: u16, - pub SegEs: u16, - pub SegFs: u16, - pub SegGs: u16, - pub SegSs: u16, - pub EFlags: u32, - pub Dr0: u64, - pub Dr1: u64, - pub Dr2: u64, - pub Dr3: u64, - pub Dr6: u64, - pub Dr7: u64, - pub Rax: u64, - pub Rcx: u64, - pub Rdx: u64, - pub Rbx: u64, - pub Rsp: u64, - pub Rbp: u64, - pub Rsi: u64, - pub Rdi: u64, - pub R8: u64, - pub R9: u64, - pub R10: u64, - pub R11: u64, - pub R12: u64, - pub R13: u64, - pub R14: u64, - pub R15: u64, - pub Rip: u64, - pub Anonymous: CONTEXT_0, - pub VectorRegister: [M128A; 26], - pub VectorControl: u64, - pub DebugControl: u64, - pub LastBranchToRip: u64, - pub LastBranchFromRip: u64, - pub LastExceptionToRip: u64, - pub LastExceptionFromRip: u64, -} - -#[cfg(target_arch = "x86_64")] -impl ::core::marker::Copy for CONTEXT {} -#[cfg(target_arch = "x86_64")] -impl ::core::clone::Clone for CONTEXT { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] -#[repr(C)] -#[cfg(target_arch = "x86_64")] -pub union CONTEXT_0 { - pub FltSave: XSAVE_FORMAT, - pub Anonymous: CONTEXT_0_0, -} - -#[cfg(target_arch = "x86_64")] -impl ::core::marker::Copy for CONTEXT_0 {} -#[cfg(target_arch = "x86_64")] -impl ::core::clone::Clone for CONTEXT_0 { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] -#[repr(C)] -#[cfg(target_arch = "x86_64")] -pub struct CONTEXT_0_0 { - pub Header: [M128A; 2], - pub Legacy: [M128A; 8], - pub Xmm0: M128A, - pub Xmm1: M128A, - pub Xmm2: M128A, - pub Xmm3: M128A, - pub Xmm4: M128A, - pub Xmm5: M128A, - pub Xmm6: M128A, - pub Xmm7: M128A, - pub Xmm8: M128A, - pub Xmm9: M128A, - pub Xmm10: M128A, - pub Xmm11: M128A, - pub Xmm12: M128A, - pub Xmm13: M128A, - pub Xmm14: M128A, - pub Xmm15: M128A, -} -#[cfg(target_arch = "x86_64")] -impl ::core::marker::Copy for CONTEXT_0_0 {} -#[cfg(target_arch = "x86_64")] -impl ::core::clone::Clone for CONTEXT_0_0 { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] -#[repr(C)] -#[cfg(target_arch = "x86")] -pub struct CONTEXT { - pub ContextFlags: u32, - pub Dr0: u32, - pub Dr1: u32, - pub Dr2: u32, - pub Dr3: u32, - pub Dr6: u32, - pub Dr7: u32, - pub FloatSave: FLOATING_SAVE_AREA, - pub SegGs: u32, - pub SegFs: u32, - pub SegEs: u32, - pub SegDs: u32, - pub Edi: u32, - pub Esi: u32, - pub Ebx: u32, - pub Edx: u32, - pub Ecx: u32, - pub Eax: u32, - pub Ebp: u32, - pub Eip: u32, - pub SegCs: u32, - pub EFlags: u32, - pub Esp: u32, - pub SegSs: u32, - pub ExtendedRegisters: [u8; 512], -} - -#[cfg(target_arch = "x86")] -impl ::core::marker::Copy for CONTEXT {} -#[cfg(target_arch = "x86")] -impl ::core::clone::Clone for CONTEXT { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] -#[repr(C)] -#[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))] -pub struct FLOATING_SAVE_AREA { - pub ControlWord: u32, - pub StatusWord: u32, - pub TagWord: u32, - pub ErrorOffset: u32, - pub ErrorSelector: u32, - pub DataOffset: u32, - pub DataSelector: u32, - pub RegisterArea: [u8; 80], - pub Cr0NpxState: u32, -} - -#[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))] -impl ::core::marker::Copy for FLOATING_SAVE_AREA {} -#[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))] -impl ::core::clone::Clone for FLOATING_SAVE_AREA { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] -#[repr(C)] -#[cfg(target_arch = "x86")] -pub struct FLOATING_SAVE_AREA { - pub ControlWord: u32, - pub StatusWord: u32, - pub TagWord: u32, - pub ErrorOffset: u32, - pub ErrorSelector: u32, - pub DataOffset: u32, - pub DataSelector: u32, - pub RegisterArea: [u8; 80], - pub Spare0: u32, -} - -#[cfg(target_arch = "x86")] -impl ::core::marker::Copy for FLOATING_SAVE_AREA {} -#[cfg(target_arch = "x86")] -impl ::core::clone::Clone for FLOATING_SAVE_AREA { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] -#[repr(C)] -#[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))] -pub struct XSAVE_FORMAT { - pub ControlWord: u16, - pub StatusWord: u16, - pub TagWord: u8, - pub Reserved1: u8, - pub ErrorOpcode: u16, - pub ErrorOffset: u32, - pub ErrorSelector: u16, - pub Reserved2: u16, - pub DataOffset: u32, - pub DataSelector: u16, - pub Reserved3: u16, - pub MxCsr: u32, - pub MxCsr_Mask: u32, - pub FloatRegisters: [M128A; 8], - pub XmmRegisters: [M128A; 16], - pub Reserved4: [u8; 96], -} - -#[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))] -impl ::core::marker::Copy for XSAVE_FORMAT {} -#[cfg(any(target_arch = "aarch64", target_arch = "x86_64"))] -impl ::core::clone::Clone for XSAVE_FORMAT { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] -#[repr(C)] -#[cfg(target_arch = "x86")] -pub struct XSAVE_FORMAT { - pub ControlWord: u16, - pub StatusWord: u16, - pub TagWord: u8, - pub Reserved1: u8, - pub ErrorOpcode: u16, - pub ErrorOffset: u32, - pub ErrorSelector: u16, - pub Reserved2: u16, - pub DataOffset: u32, - pub DataSelector: u16, - pub Reserved3: u16, - pub MxCsr: u32, - pub MxCsr_Mask: u32, - pub FloatRegisters: [M128A; 8], - pub XmmRegisters: [M128A; 8], - pub Reserved4: [u8; 224], -} - -#[cfg(target_arch = "x86")] -impl ::core::marker::Copy for XSAVE_FORMAT {} -#[cfg(target_arch = "x86")] -impl ::core::clone::Clone for XSAVE_FORMAT { - fn clone(&self) -> Self { - *self - } -} - -// minidump - -#[allow(non_camel_case_types)] pub type MINIDUMP_TYPE = u32; -#[allow(non_upper_case_globals)] pub const MiniDumpNormal: MINIDUMP_TYPE = 0u32; -#[allow(non_camel_case_types)] pub type MINIDUMP_CALLBACK_ROUTINE = Option< unsafe extern "system" fn( callbackparam: *mut ::core::ffi::c_void, @@ -443,59 +24,25 @@ pub type MINIDUMP_CALLBACK_ROUTINE = Option< ) -> BOOL, >; +#[derive(Copy, Clone)] #[repr(C, packed(4))] pub struct MINIDUMP_CALLBACK_INPUT { dummy: u32, } -impl ::core::marker::Copy for MINIDUMP_CALLBACK_INPUT {} -impl ::core::clone::Clone for MINIDUMP_CALLBACK_INPUT { - fn clone(&self) -> Self { - *self - } -} - +#[derive(Copy, Clone)] #[repr(C, packed(4))] pub struct MINIDUMP_CALLBACK_OUTPUT { dummy: u32, } - -impl ::core::marker::Copy for MINIDUMP_CALLBACK_OUTPUT {} -impl ::core::clone::Clone for MINIDUMP_CALLBACK_OUTPUT { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] -#[repr(C)] -pub struct M128A { - pub Low: u64, - pub High: i64, -} - -impl ::core::marker::Copy for M128A {} -impl ::core::clone::Clone for M128A { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] +#[derive(Copy, Clone)] #[repr(C, packed(4))] pub struct MINIDUMP_CALLBACK_INFORMATION { pub CallbackRoutine: MINIDUMP_CALLBACK_ROUTINE, pub CallbackParam: *mut ::core::ffi::c_void, } -impl ::core::marker::Copy for MINIDUMP_CALLBACK_INFORMATION {} -impl ::core::clone::Clone for MINIDUMP_CALLBACK_INFORMATION { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] +#[derive(Copy, Clone)] #[repr(C, packed(4))] pub struct MINIDUMP_EXCEPTION_INFORMATION { pub ThreadId: u32, @@ -503,57 +50,20 @@ pub struct MINIDUMP_EXCEPTION_INFORMATION { pub ClientPointers: BOOL, } -#[allow(non_snake_case)] -#[repr(C)] -pub struct EXCEPTION_POINTERS { - pub ExceptionRecord: *mut EXCEPTION_RECORD, - pub ContextRecord: *mut CONTEXT, -} - -#[allow(non_snake_case)] -#[repr(C)] -pub struct EXCEPTION_RECORD { - pub ExceptionCode: NTSTATUS, - pub ExceptionFlags: u32, - pub ExceptionRecord: *mut EXCEPTION_RECORD, - pub ExceptionAddress: *mut ::core::ffi::c_void, - pub NumberParameters: u32, - pub ExceptionInformation: [usize; 15], -} - -impl ::core::marker::Copy for EXCEPTION_RECORD {} -impl ::core::clone::Clone for EXCEPTION_RECORD { - fn clone(&self) -> Self { - *self - } -} - -#[allow(non_snake_case)] +#[derive(Copy, Clone)] #[repr(C, packed(4))] pub struct MINIDUMP_USER_STREAM { pub Type: u32, pub BufferSize: u32, pub Buffer: *mut ::core::ffi::c_void, } -impl ::core::marker::Copy for MINIDUMP_USER_STREAM {} -impl ::core::clone::Clone for MINIDUMP_USER_STREAM { - fn clone(&self) -> Self { - *self - } -} -#[allow(non_snake_case)] +#[derive(Copy, Clone)] #[repr(C, packed(4))] pub struct MINIDUMP_USER_STREAM_INFORMATION { pub UserStreamCount: u32, pub UserStreamArray: *mut MINIDUMP_USER_STREAM, } -impl ::core::marker::Copy for MINIDUMP_USER_STREAM_INFORMATION {} -impl ::core::clone::Clone for MINIDUMP_USER_STREAM_INFORMATION { - fn clone(&self) -> Self { - *self - } -} extern "system" { pub fn GetThreadContext(hthread: HANDLE, lpcontext: *mut CONTEXT) -> BOOL; @@ -566,5 +76,4 @@ extern "system" { userstreamparam: *const MINIDUMP_USER_STREAM_INFORMATION, callbackparam: *const MINIDUMP_CALLBACK_INFORMATION, ) -> BOOL; - pub fn RtlCaptureContext(contextrecord: *mut CONTEXT); } diff --git a/src/windows/minidump_writer.rs b/src/windows/minidump_writer.rs index 8218beb5..8a1ef5c2 100644 --- a/src/windows/minidump_writer.rs +++ b/src/windows/minidump_writer.rs @@ -1,15 +1,25 @@ use crate::windows::errors::Error; use crate::windows::ffi::{ - CloseHandle, GetCurrentProcess, GetCurrentThreadId, GetThreadContext, MiniDumpNormal, - MiniDumpWriteDump, OpenProcess, OpenThread, ResumeThread, RtlCaptureContext, SuspendThread, - EXCEPTION_POINTERS, EXCEPTION_RECORD, FALSE, HANDLE, MINIDUMP_EXCEPTION_INFORMATION, - MINIDUMP_USER_STREAM, MINIDUMP_USER_STREAM_INFORMATION, PROCESS_ALL_ACCESS, - STATUS_NONCONTINUABLE_EXCEPTION, THREAD_GET_CONTEXT, THREAD_QUERY_INFORMATION, - THREAD_SUSPEND_RESUME, + capture_context, GetThreadContext, MiniDumpNormal, MiniDumpWriteDump, EXCEPTION_POINTERS, + HANDLE, MINIDUMP_EXCEPTION_INFORMATION, MINIDUMP_USER_STREAM, MINIDUMP_USER_STREAM_INFORMATION, }; use minidump_common::format::{BreakpadInfoValid, MINIDUMP_BREAKPAD_INFO, MINIDUMP_STREAM_TYPE}; use scroll::Pwrite; use std::os::windows::io::AsRawHandle; +use winapi::{ + shared::minwindef::FALSE, + um::{ + handleapi::CloseHandle, + processthreadsapi::{ + GetCurrentProcess, GetCurrentThreadId, OpenProcess, OpenThread, ResumeThread, + SuspendThread, + }, + winnt::{ + EXCEPTION_RECORD, PROCESS_ALL_ACCESS, STATUS_NONCONTINUABLE_EXCEPTION, + THREAD_GET_CONTEXT, THREAD_QUERY_INFORMATION, THREAD_SUSPEND_RESUME, + }, + }, +}; pub struct MinidumpWriter { /// Optional exception information @@ -22,7 +32,7 @@ pub struct MinidumpWriter { tid: u32, /// The exception code for the dump #[allow(dead_code)] - exception_code: i32, + exception_code: u32, /// Whether we are dumping the current process or not is_external_process: bool, } @@ -42,7 +52,7 @@ impl MinidumpWriter { /// function can also fail if `thread_id` is specified and we are unable to /// acquire the thread's context pub fn dump_local_context( - exception_code: Option, + exception_code: Option, thread_id: Option, destination: &mut std::fs::File, ) -> Result<(), Error> { @@ -57,7 +67,7 @@ impl MinidumpWriter { // We need to suspend the thread to get its context, which would be bad // if it's the current thread, so we check it early before regrets happen if tid == GetCurrentThreadId() { - RtlCaptureContext(ec.as_mut_ptr()); + capture_context(ec.as_mut_ptr()); } else { // We _could_ just fallback to the current thread if we can't get the // thread handle, but probably better for this to fail with a specific @@ -69,7 +79,7 @@ impl MinidumpWriter { tid, // thread id ); - if thread_handle == 0 { + if thread_handle.is_null() { return Err(Error::ThreadOpen(std::io::Error::last_os_error())); } @@ -107,7 +117,7 @@ impl MinidumpWriter { ec.assume_init() } else { let mut ec = std::mem::MaybeUninit::uninit(); - RtlCaptureContext(ec.as_mut_ptr()); + capture_context(ec.as_mut_ptr()); ec.assume_init() }; @@ -160,7 +170,7 @@ impl MinidumpWriter { pid, // pid ); - if proc == 0 { + if proc.is_null() { return Err(std::io::Error::last_os_error().into()); } From 8028866942c8cc7ebc8710da625111c245bc64c1 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Thu, 17 Nov 2022 14:16:18 +0100 Subject: [PATCH 2/5] Revert testing in release --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2ef37c00..b007bd23 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,9 +46,9 @@ jobs: - name: Fetch run: cargo fetch --target ${{ matrix.job.target }} - name: Build - run: cargo test --target ${{ matrix.job.target }} --release --no-run + run: cargo test --target ${{ matrix.job.target }} --no-run - name: Test - run: cargo test --target ${{ matrix.job.target }} --release + run: cargo test --target ${{ matrix.job.target }} # This job builds non-tier1 targets that aren't already tested build_lower_tier: From c3c3a43c308af1887e1a910c0511191abc549cca Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Thu, 17 Nov 2022 14:39:16 +0100 Subject: [PATCH 3/5] Fix tests --- src/bin/test.rs | 12 ++++++------ src/windows/ffi.rs | 13 +++++++++++-- src/windows/minidump_writer.rs | 9 +++------ tests/windows_minidump_writer.rs | 7 ++----- 4 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/bin/test.rs b/src/bin/test.rs index 239e97ee..195640fc 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -296,7 +296,7 @@ mod linux { #[cfg(target_os = "windows")] mod windows { use minidump_writer::ffi::{ - GetCurrentProcessId, GetCurrentThread, GetCurrentThreadId, GetThreadContext, CONTEXT, + GetCurrentProcessId, GetCurrentThread, GetCurrentThreadId, GetThreadContext, EXCEPTION_POINTERS, EXCEPTION_RECORD, }; @@ -305,20 +305,20 @@ mod windows { #[inline(never)] pub(super) fn real_main(args: Vec) -> Result<()> { - let exception_code = u32::from_str_radix(&args[0], 16).unwrap() as i32; + let exception_code = u32::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: CONTEXT = mem::zeroed(); - exception_context.ContextFlags = - minidump_common::format::ContextFlagsAmd64::CONTEXT_AMD64_ALL.bits(); + let mut exception_context = std::mem::MaybeUninit::uninit(); let pid = GetCurrentProcessId(); let tid = GetCurrentThreadId(); - GetThreadContext(GetCurrentThread(), &mut exception_context); + GetThreadContext(GetCurrentThread(), exception_context.as_mut_ptr()); + + let mut exception_context = exception_context.assume_init(); let exception_ptrs = EXCEPTION_POINTERS { ExceptionRecord: &mut exception_record, diff --git a/src/windows/ffi.rs b/src/windows/ffi.rs index 18e9e938..9841075c 100644 --- a/src/windows/ffi.rs +++ b/src/windows/ffi.rs @@ -9,8 +9,17 @@ #![allow(non_snake_case, non_camel_case_types, non_upper_case_globals)] -pub use crash_context::ffi::{capture_context, CONTEXT, EXCEPTION_POINTERS}; -pub use winapi::{shared::minwindef::BOOL, um::winnt::HANDLE}; +pub use crash_context::ffi::{capture_context, CONTEXT, EXCEPTION_POINTERS, EXCEPTION_RECORD}; +pub use winapi::{ + shared::minwindef::BOOL, + um::{ + processthreadsapi::{ + GetCurrentProcess, GetCurrentProcessId, GetCurrentThread, GetCurrentThreadId, + OpenProcess, OpenThread, ResumeThread, SuspendThread, + }, + winnt::HANDLE, + }, +}; pub type MINIDUMP_TYPE = u32; diff --git a/src/windows/minidump_writer.rs b/src/windows/minidump_writer.rs index 8a1ef5c2..e538907b 100644 --- a/src/windows/minidump_writer.rs +++ b/src/windows/minidump_writer.rs @@ -1,6 +1,7 @@ use crate::windows::errors::Error; use crate::windows::ffi::{ - capture_context, GetThreadContext, MiniDumpNormal, MiniDumpWriteDump, EXCEPTION_POINTERS, + capture_context, GetCurrentProcess, GetCurrentThreadId, GetThreadContext, MiniDumpNormal, + MiniDumpWriteDump, OpenProcess, OpenThread, ResumeThread, SuspendThread, EXCEPTION_POINTERS, HANDLE, MINIDUMP_EXCEPTION_INFORMATION, MINIDUMP_USER_STREAM, MINIDUMP_USER_STREAM_INFORMATION, }; use minidump_common::format::{BreakpadInfoValid, MINIDUMP_BREAKPAD_INFO, MINIDUMP_STREAM_TYPE}; @@ -10,10 +11,6 @@ use winapi::{ shared::minwindef::FALSE, um::{ handleapi::CloseHandle, - processthreadsapi::{ - GetCurrentProcess, GetCurrentThreadId, OpenProcess, OpenThread, ResumeThread, - SuspendThread, - }, winnt::{ EXCEPTION_RECORD, PROCESS_ALL_ACCESS, STATUS_NONCONTINUABLE_EXCEPTION, THREAD_GET_CONTEXT, THREAD_QUERY_INFORMATION, THREAD_SUSPEND_RESUME, @@ -198,7 +195,7 @@ impl MinidumpWriter { /// `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 - ClientPointers: if is_external_process { 1 } else { 0 }, + ClientPointers: i32::from(is_external_process), }, ); diff --git a/tests/windows_minidump_writer.rs b/tests/windows_minidump_writer.rs index 1ea04710..9a6efa65 100644 --- a/tests/windows_minidump_writer.rs +++ b/tests/windows_minidump_writer.rs @@ -8,10 +8,7 @@ use minidump_writer::minidump_writer::MinidumpWriter; mod common; use common::start_child_and_return; -type NTSTATUS = i32; - -const EXCEPTION_ILLEGAL_INSTRUCTION: NTSTATUS = -1073741795i32; -const STATUS_INVALID_PARAMETER: NTSTATUS = -1073741811i32; +use winapi::um::{minwinbase::EXCEPTION_ILLEGAL_INSTRUCTION, winnt::STATUS_INVALID_PARAMETER}; extern "system" { pub(crate) fn GetCurrentThreadId() -> u32; @@ -138,7 +135,7 @@ fn dump_external_process() { 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; + let exception_code = u32::from_str_radix(biter.next().unwrap(), 16).unwrap(); (process_id, exception_pointers, thread_id, exception_code) }; From dcd9ae8c512e4686f1a27bb5b7d546b4bae40fbd Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Thu, 17 Nov 2022 14:47:56 +0100 Subject: [PATCH 4/5] Mark `i686-pc-windows-msvc` not implemented Crash-context needs to add capture_context impl if needed Also just cleans up the markdown table to be easier to edit --- README.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index dcdc4d6d..20962c91 100644 --- a/README.md +++ b/README.md @@ -143,13 +143,13 @@ fn write_minidump(crash_context: crash_context::CrashContext) { - ⭕️ Unimplemented, but could be implemented in the future - ❌ Unimplemented, and unlikely to ever be implemented -| Arch | unknown-linux-gnu | unknown-linux-musl | linux-android | pc-windows-msvc | apple-darwin | apple-ios ---- | --- | --- | --- | --- | --- | --- | -`x86_64` | ✅ | ✅ | ⚠️ | ✅ | ✅ | ⭕️ | -`i686` | ✅ | ✅ | ❌ | ⚠️ | ❌ | ❌ | ⭕️ | -`arm` | ⚠️ | ⚠️ | ⚠️ | ⭕️ | ❌ | ❌ | -`aarch64` | ⚠️ | ⚠️ | ⚠️ | ⭕️ | ✅ | ⭕️ | -`mips` | ⭕️ | ⭕️ | ❌ | ❌ | ❌ | ❌ | -`mips64` | ⭕️ | ⭕️ | ❌ | ❌ | ❌ | ❌ | -`powerpc` | ⭕️ | ⭕️ | ❌ | ❌ | ❌ | ❌ | -`powerpc64` | ⭕️ | ⭕️ | ❌ | ❌ | ❌ | ❌ | +| Arch | unknown-linux-gnu | unknown-linux-musl | linux-android | pc-windows-msvc | apple-darwin | apple-ios | +----------- | ----------------- | ------------------ | ------------- | --------------- | ------------ | --------- | +`x86_64` | ✅ | ✅ | ⚠️ | ✅ | ✅ | ⭕️ | +`i686` | ✅ | ✅ | ❌ | ⭕️ | ❌ | ❌ | +`arm` | ⚠️ | ⚠️ | ⚠️ | ⭕️ | ❌ | ❌ | +`aarch64` | ⚠️ | ⚠️ | ⚠️ | ⭕️ | ✅ | ⭕️ | +`mips` | ⭕️ | ⭕️ | ❌ | ❌ | ❌ | ❌ | +`mips64` | ⭕️ | ⭕️ | ❌ | ❌ | ❌ | ❌ | +`powerpc` | ⭕️ | ⭕️ | ❌ | ❌ | ❌ | ❌ | +`powerpc64` | ⭕️ | ⭕️ | ❌ | ❌ | ❌ | ❌ | From 93529ede7c67a6f9325ef1f1810598fdda26cae1 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Thu, 17 Nov 2022 14:50:23 +0100 Subject: [PATCH 5/5] Update CHANGELOG --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5f90664..eb438a1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] - ReleaseDate +### Changed +- [PR#65](https://github.com/rust-minidump/minidump-writer/pull/65) updated `crash-context` to 0.5, which has support for a custom `capture_context` to replace `RtlCaptureContext` on Windows, due to improper bindings and deficiencies, resolving [#63](https://github.com/rust-minidump/minidump-writer/issues/63). + ## [0.6.0] - 2022-11-15 ### Changed - [PR#60](https://github.com/rust-minidump/minidump-writer/pull/60) removed the dependency on `windows-sys` due the massive version churn, resolving [#58](https://github.com/rust-minidump/minidump-writer/issues/58). This should allow projects to more easily integrate this crate into their project without introducing multiple versions of transitive dependencies.