Add MacOS thread names

CHANGELOG.md

@@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- next-header -->
 ## [Unreleased] - ReleaseDate
+### Added
+-[PR#32](https://github.com/rust-minidump/minidump-writer/pull/32) resolved [#23](https://github.com/rust-minidump/minidump-writer/issues/23) by adding support for the thread names stream on MacOS.
+
 ## [0.2.0] - 2022-05-23
 ### Added
 - [PR#21](https://github.com/rust-minidump/minidump-writer/pull/21) added an initial implementation for `x86_64-apple-darwin` and `aarch64-apple-darwin`

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,15 @@ 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]
+# PR: https://github.com/mozilla/dump_syms/pull/376
+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

@@ -14,6 +14,8 @@ pub struct MinidumpWriter {
     /// List of raw blocks of memory we've written into the stream. These are
     /// referenced by other streams (eg thread list)
     pub(crate) memory_blocks: Vec<MDMemoryDescriptor>,
+    /// List of threads, minus the handler thread
+    threads: Vec<u32>,
 }
 
 impl MinidumpWriter {
@@ -22,6 +24,7 @@ impl MinidumpWriter {
         Self {
             crash_context,
             memory_blocks: Vec::new(),
+            threads: Vec::new(),
         }
     }
 
@@ -37,6 +40,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
@@ -85,4 +89,28 @@ impl MinidumpWriter {
 
         Ok(buffer.into())
     }
+
+    /// Retrieves the list of active threads in the target process, but removes
+    /// the handler thread if it is known to simplify dump analysis
+    #[inline]
+    pub(crate) fn threads(&mut self, dumper: &TaskDumper) -> &[u32] {
+        if self.threads.is_empty() {
+            if let Ok(threads) = dumper.read_threads() {
+                self.threads = threads.into();
+                // Ignore the thread that handled the exception
+                if self.crash_context.handler_thread != mach2::port::MACH_PORT_NULL {
+                    if let Some(ind) = self
+                        .threads
+                        .iter()
+                        .position(|tid| *tid == self.crash_context.handler_thread)
+                    {
+                        // Preserves order, but not sure if that is actually relevant in most cases
+                        self.threads.remove(ind);
+                    }
+                }
+            }
+        }
+
+        &self.threads
+    }
 }

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_list.rs

@@ -9,31 +9,19 @@ impl MinidumpWriter {
         buffer: &mut DumpBuf,
         dumper: &TaskDumper,
     ) -> Result<MDRawDirectory, WriterError> {
-        let threads = dumper.read_threads()?;
+        let threads = self.threads(dumper);
 
-        // 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 list_header = MemoryWriter::<u32>::alloc_with_val(buffer, threads.len() as u32)?;
 
         let mut dirent = MDRawDirectory {
             stream_type: MDStreamType::ThreadListStream as u32,
             location: list_header.location(),
         };
 
-        let mut thread_list = MemoryArrayWriter::<MDRawThread>::alloc_array(buffer, thread_count)?;
+        let mut thread_list = MemoryArrayWriter::<MDRawThread>::alloc_array(buffer, threads.len())?;
         dirent.location.data_size += thread_list.location().data_size;
 
-        let handler_thread = self.crash_context.handler_thread;
-        for (i, tid) in threads
-            .iter()
-            .filter(|tid| **tid != handler_thread)
-            .enumerate()
-        {
+        for (i, tid) in threads.iter().enumerate() {
             let thread = self.write_thread(*tid, buffer, dumper)?;
             thread_list.set_value_at(buffer, thread, i)?;
         }

src/mac/streams/thread_names.rs

@@ -0,0 +1,79 @@
+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 = self.threads(dumper);
+
+        let list_header = MemoryWriter::<u32>::alloc_with_val(buffer, threads.len() as u32)?;
+
+        let mut dirent = MDRawDirectory {
+            stream_type: MDStreamType::ThreadNamesStream as u32,
+            location: list_header.location(),
+        };
+
+        let mut names = MemoryArrayWriter::<MDRawThreadName>::alloc_array(buffer, threads.len())?;
+        dirent.location.data_size += names.location().data_size;
+
+        for (i, tid) in threads.iter().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.