diff --git a/Cargo.toml b/Cargo.toml index 60a63d2d..a4010a54 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,9 +11,9 @@ license = "MIT" [dependencies] byteorder = "1.3.2" cfg-if = "1.0" -crash-context = "0.3" +crash-context = "0.4" memoffset = "0.6" -minidump-common = "0.11" +minidump-common = "0.12" scroll = "0.11" tempfile = "3.1.0" thiserror = "1.0.21" @@ -24,7 +24,12 @@ goblin = "0.5" memmap2 = "0.5" [target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies] -nix = { version = "0.24", default-features = false, features = ["mman", "process", "ptrace", "user"] } +nix = { version = "0.24", default-features = false, features = [ + "mman", + "process", + "ptrace", + "user", +] } [target.'cfg(target_os = "windows")'.dependencies.windows-sys] version = "0.36" @@ -48,15 +53,13 @@ mach2 = "0.4" [dev-dependencies] # Minidump-processor is async so we need an executor futures = { version = "0.3", features = ["executor"] } -minidump = "0.11" +minidump = "0.12" memmap2 = "0.5" [target.'cfg(target_os = "macos")'.dev-dependencies] # We dump symbols for the `test` executable so that we can validate that minidumps # created by this crate can be processed by minidump-processor dump_syms = { version = "1.0.1", default-features = false } -minidump-processor = { version = "0.11", default-features = false, features = [ - "breakpad-syms", -] } +minidump-processor = { version = "0.12", default-features = false } similar-asserts = "1.2" uuid = "1.0" diff --git a/src/bin/test.rs b/src/bin/test.rs index 6f84c799..c5ad8e0c 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -363,7 +363,7 @@ mod mac { thread: mach2::mach_init::mach_thread_self(), handler_thread: mach2::port::MACH_PORT_NULL, exception: Some(crash_context::ExceptionInfo { - kind: exception as i32, + kind: exception, code: 0, subcode: None, }), diff --git a/src/linux/sections/exception_stream.rs b/src/linux/sections/exception_stream.rs index 665b2b47..832d6734 100644 --- a/src/linux/sections/exception_stream.rs +++ b/src/linux/sections/exception_stream.rs @@ -46,30 +46,21 @@ pub fn write( buffer: &mut DumpBuf, ) -> Result { let exception = if let Some(context) = &config.crash_context { - let sig_addr = context.inner.siginfo.ssi_addr as u64; - MDException { exception_code: context.inner.siginfo.ssi_signo as u32, exception_flags: context.inner.siginfo.ssi_code as u32, - exception_record: 0, - exception_address: sig_addr, - number_parameters: 0, - __align: 0, - exception_information: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], + exception_address: context.inner.siginfo.ssi_addr as u64, + ..Default::default() } } else { - let addr = match config.crashing_thread_context { - CrashingThreadContext::CrashContextPlusAddress((_, addr)) => addr, + let addr = match &config.crashing_thread_context { + CrashingThreadContext::CrashContextPlusAddress((_, addr)) => *addr, _ => 0, }; MDException { exception_code: MDExceptionCodeLinux::MD_EXCEPTION_CODE_LIN_DUMP_REQUESTED as u32, - exception_flags: 0, - exception_record: 0, exception_address: addr as u64, - number_parameters: 0, - __align: 0, - exception_information: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], + ..Default::default() } }; diff --git a/src/linux/sections/thread_list_stream.rs b/src/linux/sections/thread_list_stream.rs index e4607784..0e952e87 100644 --- a/src/linux/sections/thread_list_stream.rs +++ b/src/linux/sections/thread_list_stream.rs @@ -163,7 +163,7 @@ pub fn write( // while the instruction pointer is already here. config.crashing_thread_context = CrashingThreadContext::CrashContextPlusAddress(( cpu_section.location(), - info.get_instruction_pointer(), + instruction_ptr, )); } } diff --git a/src/mac/streams/exception.rs b/src/mac/streams/exception.rs index 6d7c91af..e594dd8d 100644 --- a/src/mac/streams/exception.rs +++ b/src/mac/streams/exception.rs @@ -1,5 +1,7 @@ use super::*; +use mach2::exception_types as et; + impl MinidumpWriter { /// Writes the [`minidump_common::format::MINIDUMP_EXCEPTION_STREAM`] stream. /// @@ -33,21 +35,85 @@ impl MinidumpWriter { .exception .as_ref() .map(|exc| { - let exception_address = if let Some(subcode) = exc.subcode { - subcode as u64 - } else if let Some(ts) = thread_state { - ts.pc() + let code = exc.code as u64; + + // `EXC_CRASH` exceptions wrap other exceptions, so we want to + // retrieve the _actual_ exception + let wrapped_exc = if exc.kind as u32 == et::EXC_CRASH { + recover_exc_crash_wrapped_exception(code) + } else { + None + }; + + // For EXC_RESOURCE and EXC_GUARD crashes Crashpad records the + // uppermost 32 bits of the exception code in the exception flags, + // as they are the most interesting for those exceptions. Neither + // of these exceptions can be wrapped by an `EXC_CRASH` + // + // EXC_GUARD + // code: + // +-------------------+----------------+--------------+ + // |[63:61] guard type | [60:32] flavor | [31:0] target| + // +-------------------+----------------+--------------+ + // + // EXC_RESOURCE + // code: + // +--------------------------------------------------------+ + // |[63:61] resource type | [60:58] flavor | [57:32] unused | + // +--------------------------------------------------------+ + let exception_code = + if exc.kind as u32 == et::EXC_RESOURCE || exc.kind as u32 == et::EXC_GUARD { + (code >> 32) as u32 + } else if let Some(wrapped) = wrapped_exc { + wrapped.code + } else { + // For all other exceptions types, the value in the code + // _should_ never exceed 32 bits, crashpad does an actual + // range check here, but since we don't really log anything + // else at the moment I'll punt that for now + // TODO: log/do something if exc.code > u32::MAX + code as u32 + }; + + let exception_kind = if let Some(wrapped) = wrapped_exc { + wrapped.kind } else { - 0 + exc.kind }; + let exception_address = + if exception_kind == et::EXC_BAD_ACCESS && exc.subcode.is_some() { + exc.subcode.unwrap_or_default() + } else if let Some(ts) = thread_state { + ts.pc() + } else { + 0 + }; + // The naming is confusing here, but it is how it is - MDException { - exception_code: exc.kind as u32, - exception_flags: exc.code as u32, + let mut md_exc = MDException { + exception_code: exception_kind, + exception_flags: exception_code, exception_address, ..Default::default() - } + }; + + // Now append the (mostly) original information to the "ancillary" + // exception_information at the end. This allows a minidump parser + // to recover the full exception information for the crash, rather + // than only using the (potentially) truncated information we + // just set in `exception_code` and `exception_flags` + md_exc.exception_information[0] = exception_kind as u64; + md_exc.exception_information[1] = code; + + md_exc.number_parameters = if let Some(subcode) = exc.subcode { + md_exc.exception_information[2] = subcode; + 3 + } else { + 2 + }; + + md_exc }) .unwrap_or_default(); @@ -66,3 +132,45 @@ impl MinidumpWriter { }) } } + +/// [`et::EXC_CRASH`] is a wrapper exception around another exception, but not +/// all exceptions can be wrapped by it, so this function validates that the +/// `EXC_CRASH` is actually valid +#[inline] +fn is_valid_exc_crash(exc_code: u64) -> bool { + let wrapped = ((exc_code >> 20) & 0xf) as u32; + + !( + wrapped == et::EXC_CRASH // EXC_CRASH can't wrap another one + || wrapped == et::EXC_RESOURCE // EXC_RESOURCE would lose information + || wrapped == et::EXC_GUARD // EXC_GUARD would lose information + || wrapped == et::EXC_CORPSE_NOTIFY + // cannot be wrapped + ) +} + +/// The details for an exception wrapped by an `EXC_CRASH` +#[derive(Copy, Clone)] +struct WrappedException { + /// The `EXC_*` that was wrapped + kind: u32, + /// The code of the wrapped exception, for all exceptions other than + /// `EXC_RESOURCE` and `EXC_GUARD` this _should_ never exceed 32 bits, and + /// is one of the reasons that `EXC_CRASH` cannot wrap those 2 exceptions + code: u32, + /// The Unix signal number that the original exception was converted into + _signal: u8, +} + +/// Unwraps an `EXC_CRASH` exception code to the inner exception it wraps. +/// +/// Will return `None` if the specified code is wrapping an exception that +/// should not be possible to be wrapped in an `EXC_CRASH` +#[inline] +fn recover_exc_crash_wrapped_exception(code: u64) -> Option { + is_valid_exc_crash(code).then(|| WrappedException { + kind: ((code >> 20) & 0xf) as u32, + code: (code & 0xfffff) as u32, + _signal: ((code >> 24) & 0xff) as u8, + }) +} diff --git a/src/mac/streams/module_list.rs b/src/mac/streams/module_list.rs index 5bdafcc9..2b4d13ea 100644 --- a/src/mac/streams/module_list.rs +++ b/src/mac/streams/module_list.rs @@ -370,7 +370,7 @@ mod test { }; let actual_img_details = mdw - .read_image(actual_img.clone(), &td) + .read_image(*actual_img, &td) .expect("failed to get image details"); let expected_image_name = diff --git a/tests/linux_minidump_writer.rs b/tests/linux_minidump_writer.rs index 72f19062..7c280a94 100644 --- a/tests/linux_minidump_writer.rs +++ b/tests/linux_minidump_writer.rs @@ -335,9 +335,9 @@ fn test_minidump_size_limit() { // Second, write a minidump with a size limit big enough to not trigger // anything. { - // Set size limit arbitrarily 1MB larger than the normal file size -- such + // Set size limit arbitrarily 2MiB larger than the normal file size -- such // that the limiting code will not kick in. - let minidump_size_limit = normal_file_size + 1024 * 1024; + let minidump_size_limit = normal_file_size + 2 * 1024 * 1024; let mut tmpfile = tempfile::Builder::new() .prefix("write_dump_pseudolimited")