diff options
Diffstat (limited to 'third_party/rust/minidump-writer/src')
12 files changed, 290 insertions, 87 deletions
diff --git a/third_party/rust/minidump-writer/src/bin/test.rs b/third_party/rust/minidump-writer/src/bin/test.rs index 85b6fa6a93..df39b28655 100644 --- a/third_party/rust/minidump-writer/src/bin/test.rs +++ b/third_party/rust/minidump-writer/src/bin/test.rs @@ -8,14 +8,14 @@ pub type Result<T> = std::result::Result<T, Error>; mod linux { use super::*; use minidump_writer::{ + minidump_writer::STOP_TIMEOUT, ptrace_dumper::{PtraceDumper, AT_SYSINFO_EHDR}, LINUX_GATE_LIBRARY_NAME, }; use nix::{ - sys::mman::{mmap, MapFlags, ProtFlags}, + sys::mman::{mmap_anonymous, MapFlags, ProtFlags}, unistd::getppid, }; - use std::os::fd::BorrowedFd; macro_rules! test { ($x:expr, $errmsg:expr) => { @@ -29,13 +29,13 @@ mod linux { fn test_setup() -> Result<()> { let ppid = getppid(); - PtraceDumper::new(ppid.as_raw())?; + PtraceDumper::new(ppid.as_raw(), STOP_TIMEOUT)?; Ok(()) } fn test_thread_list() -> Result<()> { let ppid = getppid(); - let dumper = PtraceDumper::new(ppid.as_raw())?; + let dumper = PtraceDumper::new(ppid.as_raw(), STOP_TIMEOUT)?; test!(!dumper.threads.is_empty(), "No threads")?; test!( dumper @@ -51,7 +51,7 @@ mod linux { fn test_copy_from_process(stack_var: usize, heap_var: usize) -> Result<()> { let ppid = getppid().as_raw(); - let mut dumper = PtraceDumper::new(ppid)?; + let mut dumper = PtraceDumper::new(ppid, STOP_TIMEOUT)?; dumper.suspend_threads()?; let stack_res = PtraceDumper::copy_from_process(ppid, stack_var as *mut libc::c_void, 1)?; @@ -73,7 +73,7 @@ mod linux { fn test_find_mappings(addr1: usize, addr2: usize) -> Result<()> { let ppid = getppid(); - let dumper = PtraceDumper::new(ppid.as_raw())?; + let dumper = PtraceDumper::new(ppid.as_raw(), STOP_TIMEOUT)?; dumper .find_mapping(addr1) .ok_or("No mapping for addr1 found")?; @@ -90,7 +90,7 @@ mod linux { let ppid = getppid().as_raw(); let exe_link = format!("/proc/{}/exe", ppid); let exe_name = std::fs::read_link(exe_link)?.into_os_string(); - let mut dumper = PtraceDumper::new(getppid().as_raw())?; + let mut dumper = PtraceDumper::new(getppid().as_raw(), STOP_TIMEOUT)?; let mut found_exe = None; for (idx, mapping) in dumper.mappings.iter().enumerate() { if mapping.name.as_ref().map(|x| x.into()).as_ref() == Some(&exe_name) { @@ -107,7 +107,7 @@ mod linux { fn test_merged_mappings(path: String, mapped_mem: usize, mem_size: usize) -> Result<()> { // Now check that PtraceDumper interpreted the mappings properly. - let dumper = PtraceDumper::new(getppid().as_raw())?; + let dumper = PtraceDumper::new(getppid().as_raw(), STOP_TIMEOUT)?; let mut mapping_count = 0; for map in &dumper.mappings { if map @@ -129,7 +129,7 @@ mod linux { fn test_linux_gate_mapping_id() -> Result<()> { let ppid = getppid().as_raw(); - let mut dumper = PtraceDumper::new(ppid)?; + let mut dumper = PtraceDumper::new(ppid, STOP_TIMEOUT)?; let mut found_linux_gate = false; for mut mapping in dumper.mappings.clone() { if mapping.name == Some(LINUX_GATE_LIBRARY_NAME.into()) { @@ -148,7 +148,7 @@ mod linux { fn test_mappings_include_linux_gate() -> Result<()> { let ppid = getppid().as_raw(); - let dumper = PtraceDumper::new(ppid)?; + let dumper = PtraceDumper::new(ppid, STOP_TIMEOUT)?; let linux_gate_loc = dumper.auxv[&AT_SYSINFO_EHDR]; test!(linux_gate_loc != 0, "linux_gate_loc == 0")?; let mut found_linux_gate = false; @@ -215,18 +215,16 @@ mod linux { let memory_size = std::num::NonZeroUsize::new(page_size.unwrap() as usize).unwrap(); // Get some memory to be mapped by the child-process let mapped_mem = unsafe { - mmap::<BorrowedFd>( + mmap_anonymous( None, memory_size, ProtFlags::PROT_READ | ProtFlags::PROT_WRITE, MapFlags::MAP_PRIVATE | MapFlags::MAP_ANON, - None, - 0, ) .unwrap() }; - println!("{} {}", mapped_mem as usize, memory_size); + println!("{} {}", mapped_mem.as_ptr() as usize, memory_size); loop { std::thread::park(); } diff --git a/third_party/rust/minidump-writer/src/linux/dso_debug.rs b/third_party/rust/minidump-writer/src/linux/dso_debug.rs index b9f466261f..01c0a73505 100644 --- a/third_party/rust/minidump-writer/src/linux/dso_debug.rs +++ b/third_party/rust/minidump-writer/src/linux/dso_debug.rs @@ -161,11 +161,6 @@ pub fn write_dso_debug_stream( assert!(head.is_empty(), "Data was not aligned"); let dyn_struct = &body[0]; - // #ifdef __mips__ - // const int32_t debug_tag = DT_MIPS_RLD_MAP; - // #else - // const int32_t debug_tag = DT_DEBUG; - // #endif let debug_tag = goblin::elf::dynamic::DT_DEBUG; if dyn_struct.d_tag == debug_tag { r_debug = dyn_struct.d_val as usize; diff --git a/third_party/rust/minidump-writer/src/linux/maps_reader.rs b/third_party/rust/minidump-writer/src/linux/maps_reader.rs index 4d0d3b5aaa..b5b7fb23e6 100644 --- a/third_party/rust/minidump-writer/src/linux/maps_reader.rs +++ b/third_party/rust/minidump-writer/src/linux/maps_reader.rs @@ -289,10 +289,9 @@ impl MappingInfo { true } - fn elf_file_so_name(&self) -> Result<String> { - // Find the shared object name (SONAME) by examining the ELF information - // for |mapping|. If the SONAME is found copy it into the passed buffer - // |soname| and return true. The size of the buffer is |soname_size|. + /// Find the shared object name (SONAME) by examining the ELF information + /// for the mapping. + fn so_name(&self) -> Result<String> { let mapped_file = MappingInfo::get_mmap(&self.name, self.offset)?; let elf_obj = elf::Elf::parse(&mapped_file)?; @@ -303,7 +302,14 @@ impl MappingInfo { Ok(soname.to_string()) } - pub fn get_mapping_effective_path_and_name(&self) -> Result<(PathBuf, String)> { + #[inline] + fn so_version(&self) -> Option<SoVersion> { + SoVersion::parse(self.name.as_deref()?) + } + + pub fn get_mapping_effective_path_name_and_version( + &self, + ) -> Result<(PathBuf, String, Option<SoVersion>)> { let mut file_path = PathBuf::from(self.name.clone().unwrap_or_default()); // Tools such as minidump_stackwalk use the name of the module to look up @@ -312,16 +318,15 @@ impl MappingInfo { // filesystem name of the module. // Just use the filesystem name if no SONAME is present. - let file_name = if let Ok(name) = self.elf_file_so_name() { - name - } else { + let Ok(file_name) = self.so_name() else { // file_path := /path/to/libname.so // file_name := libname.so let file_name = file_path .file_name() .map(|s| s.to_string_lossy().into_owned()) .unwrap_or_default(); - return Ok((file_path, file_name)); + + return Ok((file_path, file_name, self.so_version())); }; if self.is_executable() && self.offset != 0 { @@ -337,7 +342,7 @@ impl MappingInfo { file_path.set_file_name(&file_name); } - Ok((file_path, file_name)) + Ok((file_path, file_name, self.so_version())) } pub fn is_contained_in(&self, user_mapping_list: &MappingList) -> bool { @@ -382,6 +387,97 @@ impl MappingInfo { } } +/// Version metadata retrieved from an .so filename +/// +/// There is no standard for .so version numbers so this implementation just +/// does a best effort to pull as much data as it can based on real .so schemes +/// seen +/// +/// That being said, the [libtool](https://www.gnu.org/software/libtool/manual/html_node/Libtool-versioning.html) +/// versioning scheme is fairly common +#[cfg_attr(test, derive(Debug))] +pub struct SoVersion { + /// Might be non-zero if there is at least one non-zero numeric component after .so. + /// + /// Equivalent to `current` in libtool versions + pub major: u32, + /// The numeric component after the major version, if any + /// + /// Equivalent to `revision` in libtool versions + pub minor: u32, + /// The numeric component after the minor version, if any + /// + /// Equivalent to `age` in libtool versions + pub patch: u32, + /// The patch component may contain additional non-numeric metadata similar + /// to a semver prelease, this is any numeric data that suffixes that prerelease + /// string + pub prerelease: u32, +} + +impl SoVersion { + /// Attempts to retrieve the .so version of the elf path via its filename + fn parse(so_path: &OsStr) -> Option<Self> { + let filename = std::path::Path::new(so_path).file_name()?; + + // Avoid an allocation unless the string contains non-utf8 + let filename = filename.to_string_lossy(); + + let (_, version) = filename.split_once(".so.")?; + + let mut sov = Self { + major: 0, + minor: 0, + patch: 0, + prerelease: 0, + }; + + let comps = [ + &mut sov.major, + &mut sov.minor, + &mut sov.patch, + &mut sov.prerelease, + ]; + + for (i, comp) in version.split('.').enumerate() { + if i <= 1 { + *comps[i] = comp.parse().unwrap_or_default(); + } else if i >= 4 { + break; + } else { + // In some cases the release/patch version is alphanumeric (eg. '2rc5'), + // so try to parse either a single or two numbers + if let Some(pend) = comp.find(|c: char| !c.is_ascii_digit()) { + if let Ok(patch) = comp[..pend].parse() { + *comps[i] = patch; + } + + if i >= comps.len() - 1 { + break; + } + if let Some(pre) = comp.rfind(|c: char| !c.is_ascii_digit()) { + if let Ok(pre) = comp[pre + 1..].parse() { + *comps[i + 1] = pre; + break; + } + } + } else { + *comps[i] = comp.parse().unwrap_or_default(); + } + } + } + + Some(sov) + } +} + +#[cfg(test)] +impl PartialEq<(u32, u32, u32, u32)> for SoVersion { + fn eq(&self, o: &(u32, u32, u32, u32)) -> bool { + self.major == o.0 && self.minor == o.1 && self.patch == o.2 && self.prerelease == o.3 + } +} + #[cfg(test)] #[cfg(target_pointer_width = "64")] // All addresses are 64 bit and I'm currently too lazy to adjust it to work for both mod tests { @@ -628,14 +724,41 @@ a4840000-a4873000 rw-p 09021000 08:12 393449 /data/app/org.mozilla.firefox-1 ); assert_eq!(mappings.len(), 1); - let (file_path, file_name) = mappings[0] - .get_mapping_effective_path_and_name() + let (file_path, file_name, _version) = mappings[0] + .get_mapping_effective_path_name_and_version() .expect("Couldn't get effective name for mapping"); assert_eq!(file_name, "libmozgtk.so"); assert_eq!(file_path, PathBuf::from("/home/martin/Documents/mozilla/devel/mozilla-central/obj/widget/gtk/mozgtk/gtk3/libmozgtk.so")); } #[test] + fn test_elf_file_so_version() { + #[rustfmt::skip] + let test_cases = [ + ("/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.32", (6, 0, 32, 0)), + ("/usr/lib/x86_64-linux-gnu/libcairo-gobject.so.2.11800.0", (2, 11800, 0, 0)), + ("/usr/lib/x86_64-linux-gnu/libm.so.6", (6, 0, 0, 0)), + ("/usr/lib/x86_64-linux-gnu/libpthread.so.0", (0, 0, 0, 0)), + ("/usr/lib/x86_64-linux-gnu/libgmodule-2.0.so.0.7800.0", (0, 7800, 0, 0)), + ("/usr/lib/x86_64-linux-gnu/libabsl_time_zone.so.20220623.0.0", (20220623, 0, 0, 0)), + ("/usr/lib/x86_64-linux-gnu/libdbus-1.so.3.34.2rc5", (3, 34, 2, 5)), + ("/usr/lib/x86_64-linux-gnu/libdbus-1.so.3.34.2rc", (3, 34, 2, 0)), + ("/usr/lib/x86_64-linux-gnu/libdbus-1.so.3.34.rc5", (3, 34, 0, 5)), + ("/usr/lib/x86_64-linux-gnu/libtoto.so.AAA", (0, 0, 0, 0)), + ("/usr/lib/x86_64-linux-gnu/libsemver-1.so.1.2.alpha.1", (1, 2, 0, 1)), + ("/usr/lib/x86_64-linux-gnu/libboop.so.1.2.3.4.5", (1, 2, 3, 4)), + ("/usr/lib/x86_64-linux-gnu/libboop.so.1.2.3pre4.5", (1, 2, 3, 4)), + ]; + + assert!(SoVersion::parse(OsStr::new("/home/alex/bin/firefox/libmozsandbox.so")).is_none()); + + for (path, expected) in test_cases { + let actual = SoVersion::parse(OsStr::new(path)).unwrap(); + assert_eq!(actual, expected); + } + } + + #[test] fn test_whitespaces_in_name() { let mappings = get_mappings_for( "\ diff --git a/third_party/rust/minidump-writer/src/linux/minidump_writer.rs b/third_party/rust/minidump-writer/src/linux/minidump_writer.rs index da395b53f5..c83308f576 100644 --- a/third_party/rust/minidump-writer/src/linux/minidump_writer.rs +++ b/third_party/rust/minidump-writer/src/linux/minidump_writer.rs @@ -13,7 +13,10 @@ use crate::{ mem_writer::{Buffer, MemoryArrayWriter, MemoryWriter, MemoryWriterError}, minidump_format::*, }; -use std::io::{Seek, Write}; +use std::{ + io::{Seek, Write}, + time::Duration, +}; pub enum CrashingThreadContext { None, @@ -21,6 +24,10 @@ pub enum CrashingThreadContext { CrashContextPlusAddress((MDLocationDescriptor, usize)), } +/// The default timeout after a `SIGSTOP` after which minidump writing proceeds +/// regardless of the process state +pub const STOP_TIMEOUT: Duration = Duration::from_millis(100); + pub struct MinidumpWriter { pub process_id: Pid, pub blamed_thread: Pid, @@ -34,6 +41,7 @@ pub struct MinidumpWriter { pub sanitize_stack: bool, pub crash_context: Option<CrashContext>, pub crashing_thread_context: CrashingThreadContext, + pub stop_timeout: Duration, } // This doesn't work yet: @@ -62,6 +70,7 @@ impl MinidumpWriter { sanitize_stack: false, crash_context: None, crashing_thread_context: CrashingThreadContext::None, + stop_timeout: STOP_TIMEOUT, } } @@ -100,10 +109,18 @@ impl MinidumpWriter { self } + /// Sets the timeout after `SIGSTOP` is sent to the process, if the process + /// has not stopped by the time the timeout has reached, we proceed with + /// minidump generation + pub fn stop_timeout(&mut self, duration: Duration) -> &mut Self { + self.stop_timeout = duration; + self + } + /// Generates a minidump and writes to the destination provided. Returns the in-memory /// version of the minidump as well. pub fn dump(&mut self, destination: &mut (impl Write + Seek)) -> Result<Vec<u8>> { - let mut dumper = PtraceDumper::new(self.process_id)?; + let mut dumper = PtraceDumper::new(self.process_id, self.stop_timeout)?; dumper.suspend_threads()?; dumper.late_init()?; @@ -215,31 +232,24 @@ impl MinidumpWriter { dir_section.write_to_file(buffer, None)?; let dirent = thread_list_stream::write(self, buffer, dumper)?; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; let dirent = mappings::write(self, buffer, dumper)?; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; app_memory::write(self, buffer)?; - // Write section to file dir_section.write_to_file(buffer, None)?; let dirent = memory_list_stream::write(self, buffer)?; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; let dirent = exception_stream::write(self, buffer)?; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; let dirent = systeminfo_stream::write(buffer)?; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; let dirent = memory_info_list_stream::write(self, buffer)?; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; let dirent = match self.write_file(buffer, "/proc/cpuinfo") { @@ -249,7 +259,6 @@ impl MinidumpWriter { }, Err(_) => Default::default(), }; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; let dirent = match self.write_file(buffer, &format!("/proc/{}/status", self.blamed_thread)) @@ -260,7 +269,6 @@ impl MinidumpWriter { }, Err(_) => Default::default(), }; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; let dirent = match self @@ -273,7 +281,6 @@ impl MinidumpWriter { }, Err(_) => Default::default(), }; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; let dirent = match self.write_file(buffer, &format!("/proc/{}/cmdline", self.blamed_thread)) @@ -284,7 +291,6 @@ impl MinidumpWriter { }, Err(_) => Default::default(), }; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; let dirent = match self.write_file(buffer, &format!("/proc/{}/environ", self.blamed_thread)) @@ -295,7 +301,6 @@ impl MinidumpWriter { }, Err(_) => Default::default(), }; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; let dirent = match self.write_file(buffer, &format!("/proc/{}/auxv", self.blamed_thread)) { @@ -305,7 +310,6 @@ impl MinidumpWriter { }, Err(_) => Default::default(), }; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; let dirent = match self.write_file(buffer, &format!("/proc/{}/maps", self.blamed_thread)) { @@ -315,12 +319,10 @@ impl MinidumpWriter { }, Err(_) => Default::default(), }; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; let dirent = dso_debug::write_dso_debug_stream(buffer, self.process_id, &dumper.auxv) .unwrap_or_default(); - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; let dirent = match self.write_file(buffer, &format!("/proc/{}/limits", self.blamed_thread)) @@ -331,11 +333,9 @@ impl MinidumpWriter { }, Err(_) => Default::default(), }; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; let dirent = thread_names_stream::write(buffer, dumper)?; - // Write section to file dir_section.write_to_file(buffer, Some(dirent))?; // This section is optional, so we ignore errors when writing it diff --git a/third_party/rust/minidump-writer/src/linux/ptrace_dumper.rs b/third_party/rust/minidump-writer/src/linux/ptrace_dumper.rs index f75499bcdd..0dd0fa2719 100644 --- a/third_party/rust/minidump-writer/src/linux/ptrace_dumper.rs +++ b/third_party/rust/minidump-writer/src/linux/ptrace_dumper.rs @@ -15,10 +15,20 @@ use crate::{ use goblin::elf; use nix::{ errno::Errno, - sys::{ptrace, wait}, + sys::{ptrace, signal, wait}, +}; +use procfs_core::{ + process::{MMPermissions, ProcState, Stat}, + FromRead, ProcError, +}; +use std::{ + collections::HashMap, + ffi::c_void, + io::BufReader, + path, + result::Result, + time::{Duration, Instant}, }; -use procfs_core::process::MMPermissions; -use std::{collections::HashMap, ffi::c_void, io::BufReader, path, result::Result}; #[derive(Debug, Clone)] pub struct Thread { @@ -45,9 +55,27 @@ impl Drop for PtraceDumper { fn drop(&mut self) { // Always try to resume all threads (e.g. in case of error) let _ = self.resume_threads(); + // Always allow the process to continue. + let _ = self.continue_process(); } } +#[derive(Debug, thiserror::Error)] +enum StopProcessError { + #[error("Failed to stop the process")] + Stop(#[from] Errno), + #[error("Failed to get the process state")] + State(#[from] ProcError), + #[error("Timeout waiting for process to stop")] + Timeout, +} + +#[derive(Debug, thiserror::Error)] +enum ContinueProcessError { + #[error("Failed to continue the process")] + Continue(#[from] Errno), +} + /// PTRACE_DETACH the given pid. /// /// This handles special errno cases (ESRCH) which we won't consider errors. @@ -67,7 +95,7 @@ fn ptrace_detach(child: Pid) -> Result<(), DumperError> { impl PtraceDumper { /// Constructs a dumper for extracting information of a given process /// with a process ID of |pid|. - pub fn new(pid: Pid) -> Result<Self, InitError> { + pub fn new(pid: Pid, stop_timeout: Duration) -> Result<Self, InitError> { let mut dumper = PtraceDumper { pid, threads_suspended: false, @@ -76,12 +104,16 @@ impl PtraceDumper { mappings: Vec::new(), page_size: 0, }; - dumper.init()?; + dumper.init(stop_timeout)?; Ok(dumper) } // TODO: late_init for chromeos and android - pub fn init(&mut self) -> Result<(), InitError> { + pub fn init(&mut self, stop_timeout: Duration) -> Result<(), InitError> { + // Stopping the process is best-effort. + if let Err(e) = self.stop_process(stop_timeout) { + log::warn!("failed to stop process {}: {e}", self.pid); + } self.read_auxv()?; self.enumerate_threads()?; self.enumerate_mappings()?; @@ -207,6 +239,38 @@ impl PtraceDumper { result } + /// Send SIGSTOP to the process so that we can get a consistent state. + /// + /// This will block waiting for the process to stop until `timeout` has passed. + fn stop_process(&mut self, timeout: Duration) -> Result<(), StopProcessError> { + signal::kill(nix::unistd::Pid::from_raw(self.pid), Some(signal::SIGSTOP))?; + + // Something like waitpid for non-child processes would be better, but we have no such + // tool, so we poll the status. + const POLL_INTERVAL: Duration = Duration::from_millis(1); + let proc_file = format!("/proc/{}/stat", self.pid); + let end = Instant::now() + timeout; + + loop { + if let Ok(ProcState::Stopped) = Stat::from_file(&proc_file)?.state() { + return Ok(()); + } + + std::thread::sleep(POLL_INTERVAL); + if Instant::now() > end { + return Err(StopProcessError::Timeout); + } + } + } + + /// Send SIGCONT to the process to continue. + /// + /// Unlike `stop_process`, this function does not wait for the process to continue. + fn continue_process(&mut self) -> Result<(), ContinueProcessError> { + signal::kill(nix::unistd::Pid::from_raw(self.pid), Some(signal::SIGCONT))?; + Ok(()) + } + /// Parse /proc/$pid/task to list all the threads of the process identified by /// pid. fn enumerate_threads(&mut self) -> Result<(), InitError> { @@ -334,8 +398,9 @@ impl PtraceDumper { let mut mapping = self.find_mapping(stack_pointer); // The guard page has been 1 MiB in size since kernel 4.12, older - // kernels used a 4 KiB one instead. - let guard_page_max_addr = stack_pointer + (1024 * 1024); + // kernels used a 4 KiB one instead. Note the saturating add, as 32-bit + // processes can have a stack pointer within 1MiB of usize::MAX + let guard_page_max_addr = stack_pointer.saturating_add(1024 * 1024); // If we found no mapping, or the mapping we found has no permissions // then we might have hit a guard page, try looking for a mapping in diff --git a/third_party/rust/minidump-writer/src/linux/sections/mappings.rs b/third_party/rust/minidump-writer/src/linux/sections/mappings.rs index de19c54068..9012ae351b 100644 --- a/third_party/rust/minidump-writer/src/linux/sections/mappings.rs +++ b/third_party/rust/minidump-writer/src/linux/sections/mappings.rs @@ -83,16 +83,29 @@ fn fill_raw_module( sig_section.location() }; - let (file_path, _) = mapping - .get_mapping_effective_path_and_name() + let (file_path, _, so_version) = mapping + .get_mapping_effective_path_name_and_version() .map_err(|e| errors::SectionMappingsError::GetEffectivePathError(mapping.clone(), e))?; let name_header = write_string_to_location(buffer, file_path.to_string_lossy().as_ref())?; - Ok(MDRawModule { + let version_info = so_version.map_or(Default::default(), |sov| format::VS_FIXEDFILEINFO { + signature: format::VS_FFI_SIGNATURE, + struct_version: format::VS_FFI_STRUCVERSION, + file_version_hi: sov.major, + file_version_lo: sov.minor, + product_version_hi: sov.patch, + product_version_lo: sov.prerelease, + ..Default::default() + }); + + let raw_module = MDRawModule { base_of_image: mapping.start_address as u64, size_of_image: mapping.size as u32, cv_record, module_name_rva: name_header.rva, + version_info, ..Default::default() - }) + }; + + Ok(raw_module) } diff --git a/third_party/rust/minidump-writer/src/linux/thread_info.rs b/third_party/rust/minidump-writer/src/linux/thread_info.rs index 5bb1f9e8fb..a3fbed952f 100644 --- a/third_party/rust/minidump-writer/src/linux/thread_info.rs +++ b/third_party/rust/minidump-writer/src/linux/thread_info.rs @@ -38,19 +38,17 @@ enum NT_Elf { } #[inline] -pub fn to_u128(slice: &[u32]) -> &[u128] { - unsafe { std::slice::from_raw_parts(slice.as_ptr().cast(), slice.len().saturating_div(4)) } -} - -#[inline] -pub fn copy_registers(dst: &mut [u128], src: &[u128]) { - let to_copy = std::cmp::min(dst.len(), src.len()); - dst[..to_copy].copy_from_slice(&src[..to_copy]); -} - -#[inline] pub fn copy_u32_registers(dst: &mut [u128], src: &[u32]) { - copy_registers(dst, to_u128(src)); + // SAFETY: We are copying a block of memory from ptrace as u32s to the u128 + // format of minidump-common + unsafe { + let dst: &mut [u8] = + std::slice::from_raw_parts_mut(dst.as_mut_ptr().cast(), dst.len() * 16); + let src: &[u8] = std::slice::from_raw_parts(src.as_ptr().cast(), src.len() * 4); + + let to_copy = std::cmp::min(dst.len(), src.len()); + dst[..to_copy].copy_from_slice(&src[..to_copy]); + } } trait CommonThreadInfo { diff --git a/third_party/rust/minidump-writer/src/mac/mach.rs b/third_party/rust/minidump-writer/src/mac/mach.rs index f95211dc64..9b0179fad4 100644 --- a/third_party/rust/minidump-writer/src/mac/mach.rs +++ b/third_party/rust/minidump-writer/src/mac/mach.rs @@ -590,7 +590,8 @@ pub fn sysctl_by_name<T: Sized + Default>(name: &[u8]) -> T { 0, ) != 0 { - // log? + // TODO convert to ascii characters when logging? + log::warn!("failed to get sysctl for {name:?}"); T::default() } else { out diff --git a/third_party/rust/minidump-writer/src/mac/streams/exception.rs b/third_party/rust/minidump-writer/src/mac/streams/exception.rs index e594dd8d95..7dd7f8fae4 100644 --- a/third_party/rust/minidump-writer/src/mac/streams/exception.rs +++ b/third_party/rust/minidump-writer/src/mac/streams/exception.rs @@ -69,9 +69,11 @@ impl MinidumpWriter { } 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 + // range check here. + if code > u32::MAX.into() { + // TODO: do something more than logging? + log::warn!("exception code {code:#018x} exceeds the expected 32 bits"); + } code as u32 }; diff --git a/third_party/rust/minidump-writer/src/mac/streams/module_list.rs b/third_party/rust/minidump-writer/src/mac/streams/module_list.rs index 2b4d13ea74..d1307c80a8 100644 --- a/third_party/rust/minidump-writer/src/mac/streams/module_list.rs +++ b/third_party/rust/minidump-writer/src/mac/streams/module_list.rs @@ -344,6 +344,14 @@ mod test { /// both the local and intra-process scenarios #[test] fn images_match() { + if std::env::var_os("CI").is_some() && cfg!(target_arch = "aarch64") { + // https://github.com/rust-minidump/minidump-writer/issues/101 + println!( + "this fails on github actions but works on a local aarch64-apple-darwin machine..." + ); + return; + } + let mdw = MinidumpWriter::new(None, None); let td = TaskDumper::new(mdw.task); diff --git a/third_party/rust/minidump-writer/src/mac/streams/thread_names.rs b/third_party/rust/minidump-writer/src/mac/streams/thread_names.rs index 42242a6397..016dd48eb8 100644 --- a/third_party/rust/minidump-writer/src/mac/streams/thread_names.rs +++ b/third_party/rust/minidump-writer/src/mac/streams/thread_names.rs @@ -25,8 +25,8 @@ impl MinidumpWriter { // not a critical failure let name_loc = match Self::write_thread_name(buffer, dumper, tid) { Ok(loc) => loc, - Err(_err) => { - // TODO: log error + Err(err) => { + log::warn!("failed to write thread name for thread {tid}: {err}"); write_string_to_location(buffer, "")? } }; diff --git a/third_party/rust/minidump-writer/src/windows/minidump_writer.rs b/third_party/rust/minidump-writer/src/windows/minidump_writer.rs index 70cc420e57..2175368935 100644 --- a/third_party/rust/minidump-writer/src/windows/minidump_writer.rs +++ b/third_party/rust/minidump-writer/src/windows/minidump_writer.rs @@ -185,13 +185,13 @@ impl MinidumpWriter { // This is a mut pointer for some reason...I don't _think_ it is // actually mut in practice...? ExceptionPointers: crash_context.exception_pointers as *mut _, - /// The `EXCEPTION_POINTERS` contained in crash context is a pointer into the - /// memory of the process that crashed, as it contains an `EXCEPTION_RECORD` - /// record which is an internally linked list, so in the case that we are - /// dumping a process other than the current one, we need to tell - /// `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 + // The `EXCEPTION_POINTERS` contained in crash context is a pointer into the + // memory of the process that crashed, as it contains an `EXCEPTION_RECORD` + // record which is an internally linked list, so in the case that we are + // dumping a process other than the current one, we need to tell + // `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: i32::from(is_external_process), }, ); |