Add MacOS thread names

Cargo.toml

@@ -11,7 +11,7 @@ license = "MIT"
 [dependencies]
 byteorder = "1.3.2"
 cfg-if = "1.0"
-crash-context = "0.2"
+crash-context = "0.3"
 memoffset = "0.6"
 minidump-common = "0.11"
 scroll = "0.11"
@@ -52,15 +52,14 @@ minidump = "0.11"
 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 = "0.0.7", default-features = false }
-# minidump-processor = { version = "0.11", default-features = false, features = [
-#     "breakpad-syms",
-# ] }
+# 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 = "0.0.7", default-features = false }
+minidump-processor = { version = "0.11", default-features = false, features = [
+    "breakpad-syms",
+] }
 similar-asserts = "1.2"
 uuid = "1.0"
 
-# [patch.crates-io]
-# # PR https://github.com/mozilla/dump_syms/pull/356, merged, but unreleased
-# dump_syms = { git = "https://github.com/mozilla/dump_syms", rev = "c2743d5" } # branch = master
+[patch.crates-io]
+dump_syms = { git = "https://github.com/EmbarkStudios/dump_syms", rev = "6531018" }

src/mac/mach.rs

@@ -290,6 +290,15 @@ pub trait TaskInfo {
     const FLAVOR: u32;
 }
 
+/// Minimal trait that just pairs a structure that can be filled out by
+/// [`thread_info`] with the "flavor" that tells it the info we
+/// actually want to retrieve
+pub trait ThreadInfo {
+    /// One of the `THREAD_*` integers. I assume it's very bad if you implement
+    /// this trait and provide the wrong flavor for the struct
+    const FLAVOR: u32;
+}
+
 /// <usr/include/mach-o/loader.h>, the file type for the main executable image
 pub const MH_EXECUTE: u32 = 0x2;
 // usr/include/mach-o/loader.h, magic number for MachHeader
@@ -568,4 +577,16 @@ extern "C" {
     /// Apple, there is no mention of a replacement function or when/if it might
     /// eventually disappear.
     pub fn pid_for_task(task: mach_port_name_t, pid: *mut i32) -> kern_return_t;
+
+    /// Fomr <user/include/mach/thread_act.h>, this retrieves thread info for the
+    /// for the specified thread.
+    ///
+    /// Note that the info_size parameter is actually the size of the thread_info / 4
+    /// as it is the number of words in the thread info
+    pub fn thread_info(
+        thread: u32,
+        flavor: u32,
+        thread_info: *mut i32,
+        info_size: *mut u32,
+    ) -> kern_return_t;
 }

src/mac/minidump_writer.rs

@@ -37,6 +37,7 @@ impl MinidumpWriter {
                 Box::new(|mw, buffer, dumper| mw.write_module_list(buffer, dumper)),
                 Box::new(|mw, buffer, dumper| mw.write_misc_info(buffer, dumper)),
                 Box::new(|mw, buffer, dumper| mw.write_breakpad_info(buffer, dumper)),
+                Box::new(|mw, buffer, dumper| mw.write_thread_names(buffer, dumper)),
             ];
 
             // Exception stream needs to be the last entry in this array as it may

src/mac/streams.rs

@@ -5,6 +5,7 @@ mod misc_info;
 mod module_list;
 mod system_info;
 mod thread_list;
+mod thread_names;
 
 use super::{
     errors::WriterError,

src/mac/streams/thread_names.rs

@@ -0,0 +1,91 @@
+use super::*;
+
+impl MinidumpWriter {
+    /// Writes the [`MDStreamType::ThreadNamesStream`] which is an array of
+    /// [`miniduimp_common::format::MINIDUMP_THREAD`]
+    pub(crate) fn write_thread_names(
+        &mut self,
+        buffer: &mut DumpBuf,
+        dumper: &TaskDumper,
+    ) -> Result<MDRawDirectory, WriterError> {
+        let threads = dumper.read_threads()?;
+
+        // Ignore the thread that handled the exception
+        let thread_count = if self.crash_context.handler_thread != mach2::port::MACH_PORT_NULL {
+            threads.len() - 1
+        } else {
+            threads.len()
+        };
+
+        let list_header = MemoryWriter::<u32>::alloc_with_val(buffer, thread_count as u32)?;
+
+        let mut dirent = MDRawDirectory {
+            stream_type: MDStreamType::ThreadNamesStream as u32,
+            location: list_header.location(),
+        };
+
+        let mut names = MemoryArrayWriter::<MDRawThreadName>::alloc_array(buffer, thread_count)?;
+        dirent.location.data_size += names.location().data_size;
+
+        let handler_thread = self.crash_context.handler_thread;
+        for (i, tid) in threads
+            .iter()
+            .filter(|tid| **tid != handler_thread)
+            .enumerate()
+        {
+            // It's unfortunate if we can't grab a thread name, but it's also
+            // not a critical failure
+            let name_loc = match Self::write_thread_name(buffer, dumper, *tid) {
+                Ok(loc) => loc,
+                Err(_err) => {
+                    // TODO: log error
+                    write_string_to_location(buffer, "")?
+                }
+            };
+
+            let thread = MDRawThreadName {
+                thread_id: *tid,
+                thread_name_rva: name_loc.rva.into(),
+            };
+
+            names.set_value_at(buffer, thread, i)?;
+        }
+
+        Ok(dirent)
+    }
+
+    /// Attempts to retrieve and write the threadname, returning the threa names
+    /// location if successful
+    fn write_thread_name(
+        buffer: &mut Buffer,
+        dumper: &TaskDumper,
+        tid: u32,
+    ) -> Result<MDLocationDescriptor, WriterError> {
+        // As noted in usr/include/mach/thread_info.h, the THREAD_EXTENDED_INFO
+        // return is exactly the same as proc_pidinfo(..., proc_threadinfo)
+        impl mach::ThreadInfo for libc::proc_threadinfo {
+            const FLAVOR: u32 = 5; // THREAD_EXTENDED_INFO
+        }
+
+        let thread_info: libc::proc_threadinfo = dumper.thread_info(tid)?;
+
+        let name = std::str::from_utf8(
+            // SAFETY: This is an initialized block of static size
+            unsafe {
+                std::slice::from_raw_parts(
+                    thread_info.pth_name.as_ptr().cast(),
+                    thread_info.pth_name.len(),
+                )
+            },
+        )
+        .unwrap_or_default();
+
+        // Ignore the null terminator
+        let tname = match name.find('\0') {
+            Some(i) => &name[..i],
+            None => name,
+        };
+
+        Ok(write_string_to_location(buffer, tname)?)
+    }
+}

src/mac/task_dumper.rs

@@ -287,6 +287,28 @@ impl TaskDumper {
         unsafe { Ok(info.assume_init()) }
     }
 
+    /// Reads the specified task information.
+    ///
+    /// # Errors
+    ///
+    /// The syscall to receive the task information failed for some reason, eg.
+    /// the specified type and the flavor are mismatched and considered invalid,
+    /// or the thread no longer exists
+    pub fn thread_info<T: mach::ThreadInfo>(&self, tid: u32) -> Result<T, TaskDumpError> {
+        let mut thread_info = std::mem::MaybeUninit::<T>::uninit();
+        let mut count = (std::mem::size_of::<T>() / std::mem::size_of::<u32>()) as u32;
+
+        mach_call!(mach::thread_info(
+            tid,
+            T::FLAVOR,
+            thread_info.as_mut_ptr().cast(),
+            &mut count,
+        ))?;
+
+        // SAFETY: this will be initialized if the call succeeded
+        unsafe { Ok(thread_info.assume_init()) }
+    }
+
     /// Retrieves all of the images loaded in the task.
     ///
     /// Note that there may be multiple images with the same load address.

tests/mac_minidump_writer.rs

@@ -138,71 +138,73 @@ fn dump_external_process() {
     }
 }
 
-// /// Validates we can actually walk the stack for each thread in the minidump,
-// /// this is using minidump-processor, which (currently) depends on breakpad
-// /// symbols, however https://github.com/mozilla/dump_syms is not available as
-// /// a library https://github.com/mozilla/dump_syms/issues/253, so we just require
-// /// that it already be installed, hence the ignore
-// #[test]
-// fn stackwalks() {
-//     println!("generating minidump...");
-//     let md = capture_minidump("stackwalks", mach2::exception_types::EXC_BREAKPOINT);
-
-//     // Generate the breakpad symbols
-//     println!("generating symbols...");
-//     dump_syms::dumper::single_file(
-//         &dump_syms::dumper::Config {
-//             output: dump_syms::dumper::Output::Store(".test-symbols".into()),
-//             symbol_server: None,
-//             debug_id: None,
-//             code_id: None,
-//             arch: "",
-//             file_type: dump_syms::common::FileType::Macho,
-//             num_jobs: 2, // default this
-//             check_cfi: false,
-//             mapping_var: None,
-//             mapping_src: None,
-//             mapping_dest: None,
-//             mapping_file: None,
-//         },
-//         "target/debug/test",
-//     )
-//     .expect("failed to dump symbols");
-
-//     let provider =
-//         minidump_processor::Symbolizer::new(minidump_processor::simple_symbol_supplier(vec![
-//             ".test-symbols".into(),
-//         ]));
-
-//     let state = futures::executor::block_on(async {
-//         minidump_processor::process_minidump(&md.minidump, &provider).await
-//     })
-//     .unwrap();
-
-//     //state.print(&mut std::io::stdout()).map_err(|_| ()).unwrap();
-
-//     // We expect at least 2 threads, one of which is the fake crashing thread
-//     let fake_crash_thread = state
-//         .threads
-//         .iter()
-//         .find(|cs| cs.thread_id == md.thread)
-//         .expect("failed to find crash thread");
-
-//     // The thread is named, however we currently don't retrieve that information
-//     // currently, indeed, it appears that you need to retrieve the pthread that
-//     // corresponds the mach port for a thread, however that API seems to be
-//     // task specific...
-//     // assert_eq!(
-//     //     fake_crash_thread.thread_name.as_deref(),
-//     //     Some("test-thread")
-//     // );
-
-//     assert!(
-//         fake_crash_thread.frames.iter().any(|sf| {
-//             sf.function_name
-//                 .as_ref()
-//                 .map_or(false, |fname| fname.ends_with("wait_until_killed"))
-//         }),
-//         "unable to locate expected function"
-//     );
-// }
+/// Validates we can actually walk the stack for each thread in the minidump,
+/// this is using minidump-processor, which (currently) depends on breakpad
+/// symbols, however https://github.com/mozilla/dump_syms is not available as
+/// a library https://github.com/mozilla/dump_syms/issues/253, so we just require
+/// that it already be installed, hence the ignore
+#[test]
+fn stackwalks() {
+    println!("generating minidump...");
+    let md = capture_minidump("stackwalks", mach2::exception_types::EXC_BREAKPOINT);
+
+    // Generate the breakpad symbols
+    println!("generating symbols...");
+    dump_syms::dumper::single_file(
+        &dump_syms::dumper::Config {
+            output: dump_syms::dumper::Output::Store(".test-symbols".into()),
+            symbol_server: None,
+            debug_id: None,
+            code_id: None,
+            arch: if cfg!(target_arch = "aarch64") {
+                "arm64"
+            } else if cfg!(target_arch = "x86_64") {
+                "x86_64"
+            } else {
+                panic!("invalid MacOS target architecture")
+            },
+            file_type: dump_syms::common::FileType::Macho,
+            num_jobs: 2, // default this
+            check_cfi: false,
+            mapping_var: None,
+            mapping_src: None,
+            mapping_dest: None,
+            mapping_file: None,
+        },
+        "target/debug/test",
+    )
+    .expect("failed to dump symbols");
+
+    let provider =
+        minidump_processor::Symbolizer::new(minidump_processor::simple_symbol_supplier(vec![
+            ".test-symbols".into(),
+        ]));
+
+    let state = futures::executor::block_on(async {
+        minidump_processor::process_minidump(&md.minidump, &provider).await
+    })
+    .unwrap();
+
+    //state.print(&mut std::io::stdout()).map_err(|_| ()).unwrap();
+
+    // We expect at least 2 threads, one of which is the fake crashing thread
+    let fake_crash_thread = state
+        .threads
+        .iter()
+        .find(|cs| cs.thread_id == md.thread)
+        .expect("failed to find crash thread");
+
+    assert_eq!(
+        fake_crash_thread.thread_name.as_deref(),
+        Some("test-thread")
+    );
+
+    assert!(
+        fake_crash_thread.frames.iter().any(|sf| {
+            sf.function_name
+                .as_ref()
+                .map_or(false, |fname| fname.ends_with("wait_until_killed"))
+        }),
+        "unable to locate expected function"
+    );
+}