rust-minidump · Jake-Shadle · Jul 21, 2022 · Jul 20, 2022 · Jul 21, 2022 · Jul 21, 2022
@@ -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"
@@ -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,
                             }),

@@ -46,30 +46,21 @@ pub fn write(
     buffer: &mut DumpBuf,
 ) -> Result<MDRawDirectory, errors::SectionExceptionStreamError> {
     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()
         }
     };
 

@@ -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,
                 ));
             }
         }

@@ -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<WrappedException> {
+    is_valid_exc_crash(code).then(|| WrappedException {
+        kind: ((code >> 20) & 0xf) as u32,
+        code: (code & 0xfffff) as u32,
+        _signal: ((code >> 24) & 0xff) as u8,
+    })
+}
@@ -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 =

@@ -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")