From 6e012a3bce42aae98fc4662e54dba89370c52157 Mon Sep 17 00:00:00 2001 From: Simon Chopin Date: Thu, 23 Jul 2026 11:22:52 +0200 Subject: [PATCH 1/4] tests: use a synthetic crash context Building a crash context from the current process and passing it to a writer that dumps *another* process is bound to cause issues. Using stable data, even if invalid, make the tests more predictable -> less flaky. --- tests/common/mod.rs | 22 +++++++++++++++++++++- tests/linux_minidump_writer.rs | 30 +----------------------------- 2 files changed, 22 insertions(+), 30 deletions(-) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index bf6368b1..fbeb0a19 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -152,7 +152,10 @@ pub use linux::*; #[allow(unused)] mod linux { use { - minidump_writer::module_reader::{self, ModuleMemoryReadError, ReadModuleMemory}, + minidump_writer::{ + CrashContextExt, Pid, + module_reader::{self, ModuleMemoryReadError, ReadModuleMemory}, + }, std::borrow::Cow, }; pub struct SliceModuleMemoryReader<'a>(pub &'a [u8]); @@ -192,4 +195,21 @@ mod linux { false } } + + pub fn get_dummy_crash_context(tid: Pid) -> CrashContextExt { + let siginfo: libc::signalfd_siginfo = unsafe { std::mem::zeroed() }; + let context = unsafe { std::mem::zeroed() }; + #[cfg(not(target_arch = "arm"))] + let float_state = unsafe { std::mem::zeroed() }; + CrashContextExt { + inner: crash_context::CrashContext { + siginfo, + pid: std::process::id() as _, + tid, + context, + #[cfg(not(target_arch = "arm"))] + float_state, + }, + } + } } diff --git a/tests/linux_minidump_writer.rs b/tests/linux_minidump_writer.rs index 576c3a42..4ce892b5 100644 --- a/tests/linux_minidump_writer.rs +++ b/tests/linux_minidump_writer.rs @@ -34,41 +34,13 @@ impl Context { pub fn minidump_writer(&self, pid: Pid) -> MinidumpWriterConfig { let mut mw = MinidumpWriterConfig::new(pid, pid); if self == &Context::With { - let crash_context = get_crash_context(pid); + let crash_context = get_dummy_crash_context(pid); mw.set_crash_context(crash_context); } mw } } -fn get_ucontext() -> Result { - let mut context = std::mem::MaybeUninit::uninit(); - unsafe { - let res = crash_context::crash_context_getcontext(context.as_mut_ptr()); - if res == -1 { - Err(std::io::Error::last_os_error())?; - } - Ok(context.assume_init()) - } -} - -fn get_crash_context(tid: Pid) -> CrashContextExt { - let siginfo: libc::signalfd_siginfo = unsafe { std::mem::zeroed() }; - let context = get_ucontext().expect("Failed to get ucontext"); - #[cfg(not(target_arch = "arm"))] - let float_state = unsafe { std::mem::zeroed() }; - CrashContextExt { - inner: crash_context::CrashContext { - siginfo, - pid: std::process::id() as _, - tid, - context, - #[cfg(not(target_arch = "arm"))] - float_state, - }, - } -} - macro_rules! contextual_test { ( $(#[$attr:meta])? fn $name:ident ($ctx:ident : Context) $body:block ) => { mod $name { From a5cfc2a27a4fe05cb22d4e14c7aeb4d43dab09fa Mon Sep 17 00:00:00 2001 From: Simon Chopin Date: Thu, 23 Jul 2026 12:01:38 +0200 Subject: [PATCH 2/4] threadlist: Record an unmapped thread stack pointer as a soft error Up until now, getting a stack pointer to unmapped memory would just silently be ignored and result in an empty stack. This should probably be recorded instead. --- src/lib.rs | 1 + src/linux/minidump_writer/errors.rs | 2 + src/linux/minidump_writer/mod.rs | 5 +- .../minidump_writer/thread_list_stream.rs | 93 +++++++++++-------- tests/common/mod.rs | 28 ++++++ tests/linux_minidump_writer_soft_error.rs | 23 +++++ 6 files changed, 113 insertions(+), 39 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index d20f1ed3..0851ba1e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,6 +9,7 @@ cfg_if::cfg_if! { ThreadName, SuspendThreads, CpuInfoFileOpen, + StackPointerMapping, } } diff --git a/src/linux/minidump_writer/errors.rs b/src/linux/minidump_writer/errors.rs index 722703ea..0b8f49bc 100644 --- a/src/linux/minidump_writer/errors.rs +++ b/src/linux/minidump_writer/errors.rs @@ -69,6 +69,8 @@ pub enum WriterError { ResumeThreadsErrors(#[source] ErrorList), #[error("Errors occurred while writing system info")] WriteSystemInfoErrors(#[source] ErrorList), + #[error("Errors occurred while writing the thread list")] + WriteThreadListErrors(#[source] ErrorList), #[error("Failed writing cpuinfo")] WriteCpuInfoFailed(#[source] MemoryWriterError), #[error("Failed writing thread proc status")] diff --git a/src/linux/minidump_writer/mod.rs b/src/linux/minidump_writer/mod.rs index e66ede2a..56a5a2bd 100644 --- a/src/linux/minidump_writer/mod.rs +++ b/src/linux/minidump_writer/mod.rs @@ -345,7 +345,10 @@ impl MinidumpWriter { // we should have a mostly-intact dump dir_section.write_to_file(buffer, None)?; - let dirent = self.write_thread_list_stream(buffer)?; + let dirent = self.write_thread_list_stream( + buffer, + soft_errors.subwriter(WriterError::WriteThreadListErrors), + )?; dir_section.write_to_file(buffer, Some(dirent))?; let dirent = self.write_mappings(buffer)?; diff --git a/src/linux/minidump_writer/thread_list_stream.rs b/src/linux/minidump_writer/thread_list_stream.rs index f0160af9..6f724d37 100644 --- a/src/linux/minidump_writer/thread_list_stream.rs +++ b/src/linux/minidump_writer/thread_list_stream.rs @@ -1,4 +1,7 @@ -use {super::*, crate::minidump_cpu::RawContextCPU, std::cmp::min}; +use { + super::*, crate::minidump_cpu::RawContextCPU, error_graph::WriteErrorList, failspot::failspot, + std::cmp::min, +}; // The following kLimit* constants are for when minidump_size_limit_ is set // and the minidump size might exceed it. @@ -34,6 +37,8 @@ pub enum SectionThreadListError { ), #[error("Failed to copy memory from process")] CopyFromProcessError(#[source] CopyFromProcessError), + #[error("Failed to locate the stack pointer's memory mapping")] + GetStackInfoFailed(#[source] Box), #[error("Failed to get thread info")] ThreadInfoError(#[from] ThreadInfoError), #[error("Failed to write to memory buffer")] @@ -50,6 +55,7 @@ impl MinidumpWriter { pub fn write_thread_list_stream( &mut self, buffer: &mut DumpBuf, + mut soft_errors: impl WriteErrorList, ) -> Result { let num_threads = self.threads.len(); // Memory looks like this: @@ -107,6 +113,7 @@ impl MinidumpWriter { instruction_ptr, stack_pointer, MaxStackLen::None, + &mut soft_errors, )?; // Copy 256 bytes around crashing instruction pointer to minidump. let ip_memory_size: usize = 256; @@ -172,6 +179,7 @@ impl MinidumpWriter { instruction_ptr, info.stack_pointer, max_stack_len, + &mut soft_errors, )?; let mut cpu = RawContextCPU::default(); @@ -199,54 +207,63 @@ impl MinidumpWriter { instruction_ptr: usize, stack_ptr: usize, max_stack_len: MaxStackLen, + mut soft_errors: impl WriteErrorList, ) -> Result<(), SectionThreadListError> { thread.stack.start_of_memory_range = stack_ptr.try_into()?; thread.stack.memory.data_size = 0; thread.stack.memory.rva = buffer.position() as u32; - if let Ok((valid_stack_ptr, stack_len)) = self.get_stack_info(stack_ptr) { - let stack_len = if let MaxStackLen::Len(max_stack_len) = max_stack_len { - min(stack_len, max_stack_len) - } else { - stack_len - }; + let stack_info = failspot!(if StackPointerMapping { + Err(WriterError::NoStackPointerMapping) + } else { + self.get_stack_info(stack_ptr) + }); + let (valid_stack_ptr, stack_len) = match stack_info { + Ok(x) => x, + Err(e) => { + soft_errors.push(SectionThreadListError::GetStackInfoFailed(Box::new(e))); + return Ok(()); + } + }; - let mut stack_bytes = MinidumpWriter::copy_from_process( - &self.process_inspector, - valid_stack_ptr, - stack_len, - ) - .map_err(SectionThreadListError::CopyFromProcessError)?; - let stack_pointer_offset = stack_ptr.saturating_sub(valid_stack_ptr); - if self.skip_stacks_if_mapping_unreferenced { - if let Some(principal_mapping) = &self.principal_mapping { - let low_addr = principal_mapping.system_mapping_info.start_address; - let high_addr = principal_mapping.system_mapping_info.end_address; - if (instruction_ptr < low_addr || instruction_ptr > high_addr) - && !principal_mapping - .stack_has_pointer_to_mapping(&stack_bytes, stack_pointer_offset) - { - return Ok(()); - } - } else { + let stack_len = if let MaxStackLen::Len(max_stack_len) = max_stack_len { + min(stack_len, max_stack_len) + } else { + stack_len + }; + + let mut stack_bytes = + MinidumpWriter::copy_from_process(&self.process_inspector, valid_stack_ptr, stack_len) + .map_err(SectionThreadListError::CopyFromProcessError)?; + let stack_pointer_offset = stack_ptr.saturating_sub(valid_stack_ptr); + if self.skip_stacks_if_mapping_unreferenced { + if let Some(principal_mapping) = &self.principal_mapping { + let low_addr = principal_mapping.system_mapping_info.start_address; + let high_addr = principal_mapping.system_mapping_info.end_address; + if (instruction_ptr < low_addr || instruction_ptr > high_addr) + && !principal_mapping + .stack_has_pointer_to_mapping(&stack_bytes, stack_pointer_offset) + { return Ok(()); } + } else { + return Ok(()); } + } - if self.sanitize_stack { - self.sanitize_stack_copy(&mut stack_bytes, stack_ptr, stack_pointer_offset) - .map_err(|e| SectionThreadListError::SanitizeStackCopyFailed(Box::new(e)))?; - } - - let stack_location = MDLocationDescriptor { - data_size: stack_bytes.len() as u32, - rva: buffer.position() as u32, - }; - buffer.write_all(&stack_bytes); - thread.stack.start_of_memory_range = valid_stack_ptr as u64; - thread.stack.memory = stack_location; - self.memory_blocks.push(thread.stack); + if self.sanitize_stack { + self.sanitize_stack_copy(&mut stack_bytes, stack_ptr, stack_pointer_offset) + .map_err(|e| SectionThreadListError::SanitizeStackCopyFailed(Box::new(e)))?; } + + let stack_location = MDLocationDescriptor { + data_size: stack_bytes.len() as u32, + rva: buffer.position() as u32, + }; + buffer.write_all(&stack_bytes); + thread.stack.start_of_memory_range = valid_stack_ptr as u64; + thread.stack.memory = stack_location; + self.memory_blocks.push(thread.stack); Ok(()) } } diff --git a/tests/common/mod.rs b/tests/common/mod.rs index fbeb0a19..1bfa4cd0 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -144,6 +144,34 @@ pub fn assert_soft_errors_in_minidump<'a, 'b, T, I>( } } +/// Asserts that the soft-error stream contains an error named `variant` anywhere +/// in its (possibly nested) structure. Data-carrying variants appear as object +/// keys and unit variants as strings, so both forms are matched. +#[allow(unused)] +pub fn assert_minidump_contains_soft_error<'a, T>(dump: &minidump::Minidump<'a, T>, variant: &str) +where + T: std::ops::Deref + 'a, +{ + let errors = read_minidump_soft_errors_or_panic(dump); + assert!( + soft_errors_contain(&errors, variant), + "soft error list missing expected error `{variant}`\nError_list: {errors:#?}" + ); +} + +fn soft_errors_contain(value: &serde_json::Value, variant: &str) -> bool { + match value { + serde_json::Value::String(s) => s == variant, + serde_json::Value::Array(items) => { + items.iter().any(|item| soft_errors_contain(item, variant)) + } + serde_json::Value::Object(map) => { + map.contains_key(variant) || map.values().any(|v| soft_errors_contain(v, variant)) + } + _ => false, + } +} + #[cfg(any(target_os = "linux", target_os = "android"))] #[allow(unused)] pub use linux::*; diff --git a/tests/linux_minidump_writer_soft_error.rs b/tests/linux_minidump_writer_soft_error.rs index 0b2bd1a0..53546360 100644 --- a/tests/linux_minidump_writer_soft_error.rs +++ b/tests/linux_minidump_writer_soft_error.rs @@ -101,3 +101,26 @@ fn soft_error_stream_content() { let dump = Minidump::read_path(tmpfile.path()).expect("failed to read minidump"); assert_soft_errors_in_minidump(&dump, &expected_errors); } + +#[test] +fn thread_stack_pointer_unmapped_soft_error() { + let mut child = start_child_and_wait_for_threads(1); + let pid = child.id() as i32; + + let mut tmpfile = tempfile::Builder::new() + .prefix("stack_pointer_unmapped") + .tempfile() + .unwrap(); + + let mut fail_client = FailSpotName::testing_client(); + fail_client.set_enabled(FailSpotName::StackPointerMapping, true); + + MinidumpWriterConfig::new(pid, pid) + .write(&mut tmpfile) + .expect("could not write minidump"); + child.kill().expect("Failed to kill process"); + child.wait().expect("Failed to wait on killed process"); + + let dump = Minidump::read_path(tmpfile.path()).expect("failed to read minidump"); + assert_minidump_contains_soft_error(&dump, "GetStackInfoFailed"); +} From 939925221786d3f42dafb30d2f5f93fcfde148fa Mon Sep 17 00:00:00 2001 From: Simon Chopin Date: Thu, 23 Jul 2026 12:02:52 +0200 Subject: [PATCH 3/4] threadlist: change unreadable thread stack errors to soft If the stack pointer is garbage, it could point to a valid-but-unreadable mapping. Since we already ignore and move on if it points to unmapped memory, let's do the same in this case. --- src/lib.rs | 1 + .../minidump_writer/thread_list_stream.rs | 13 +++++++++-- tests/linux_minidump_writer_soft_error.rs | 23 +++++++++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 0851ba1e..fae3aa9d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,6 +10,7 @@ cfg_if::cfg_if! { SuspendThreads, CpuInfoFileOpen, StackPointerMapping, + ThreadStackCopy, } } diff --git a/src/linux/minidump_writer/thread_list_stream.rs b/src/linux/minidump_writer/thread_list_stream.rs index 6f724d37..e0432c56 100644 --- a/src/linux/minidump_writer/thread_list_stream.rs +++ b/src/linux/minidump_writer/thread_list_stream.rs @@ -232,9 +232,18 @@ impl MinidumpWriter { stack_len }; - let mut stack_bytes = + let stack_copy = failspot!(if ThreadStackCopy { + Err(CopyFromProcessError::InvalidArgument) + } else { MinidumpWriter::copy_from_process(&self.process_inspector, valid_stack_ptr, stack_len) - .map_err(SectionThreadListError::CopyFromProcessError)?; + }); + let mut stack_bytes = match stack_copy { + Ok(x) => x, + Err(e) => { + soft_errors.push(SectionThreadListError::CopyFromProcessError(e)); + return Ok(()); + } + }; let stack_pointer_offset = stack_ptr.saturating_sub(valid_stack_ptr); if self.skip_stacks_if_mapping_unreferenced { if let Some(principal_mapping) = &self.principal_mapping { diff --git a/tests/linux_minidump_writer_soft_error.rs b/tests/linux_minidump_writer_soft_error.rs index 53546360..5cf8aaf4 100644 --- a/tests/linux_minidump_writer_soft_error.rs +++ b/tests/linux_minidump_writer_soft_error.rs @@ -124,3 +124,26 @@ fn thread_stack_pointer_unmapped_soft_error() { let dump = Minidump::read_path(tmpfile.path()).expect("failed to read minidump"); assert_minidump_contains_soft_error(&dump, "GetStackInfoFailed"); } + +#[test] +fn thread_stack_unreadable_soft_error() { + let mut child = start_child_and_wait_for_threads(1); + let pid = child.id() as i32; + + let mut tmpfile = tempfile::Builder::new() + .prefix("stack_unreadable") + .tempfile() + .unwrap(); + + let mut fail_client = FailSpotName::testing_client(); + fail_client.set_enabled(FailSpotName::ThreadStackCopy, true); + + MinidumpWriterConfig::new(pid, pid) + .write(&mut tmpfile) + .expect("could not write minidump"); + child.kill().expect("Failed to kill process"); + child.wait().expect("Failed to wait on killed process"); + + let dump = Minidump::read_path(tmpfile.path()).expect("failed to read minidump"); + assert_minidump_contains_soft_error(&dump, "CopyFromProcessError"); +} From 2a4382b991c7241f847f43cc041d11935505035c Mon Sep 17 00:00:00 2001 From: Simon Chopin Date: Thu, 23 Jul 2026 12:04:00 +0200 Subject: [PATCH 4/4] threadlist: gracefully handle IP pointing to garbage Similarly to stack pointers, garbage IP pointer could potentially point to an unreadable memory mapping rather than just unmapped memory. In that case, let's log and move on. --- src/lib.rs | 1 + .../minidump_writer/thread_list_stream.rs | 28 +++++++++++++++---- tests/linux_minidump_writer_soft_error.rs | 25 +++++++++++++++++ 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index fae3aa9d..9337b4cb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,6 +11,7 @@ cfg_if::cfg_if! { CpuInfoFileOpen, StackPointerMapping, ThreadStackCopy, + CrashingThreadIpCopy, } } diff --git a/src/linux/minidump_writer/thread_list_stream.rs b/src/linux/minidump_writer/thread_list_stream.rs index e0432c56..e764e3a0 100644 --- a/src/linux/minidump_writer/thread_list_stream.rs +++ b/src/linux/minidump_writer/thread_list_stream.rs @@ -121,6 +121,12 @@ impl MinidumpWriter { // it's contained within. If it's not in mapped memory, // don't bother trying to write it. for mapping in &self.mappings { + let instruction_ptr = failspot!(if CrashingThreadIpCopy { + // Just use the first mapping since we'll error out anyway + mapping.start_address + mapping.size / 2 + } else { + instruction_ptr + }); if instruction_ptr < mapping.start_address || instruction_ptr >= mapping.start_address + mapping.size { @@ -143,12 +149,22 @@ impl MinidumpWriter { ip_memory_d.memory.data_size = (end_of_range - ip_memory_d.start_of_memory_range) as u32; - let memory_copy = MinidumpWriter::copy_from_process( - &self.process_inspector, - ip_memory_d.start_of_memory_range as _, - ip_memory_d.memory.data_size as usize, - ) - .map_err(SectionThreadListError::CopyFromProcessError)?; + let ip_copy = failspot!(if CrashingThreadIpCopy { + Err(CopyFromProcessError::InvalidArgument) + } else { + MinidumpWriter::copy_from_process( + &self.process_inspector, + ip_memory_d.start_of_memory_range as _, + ip_memory_d.memory.data_size as usize, + ) + }); + let memory_copy = match ip_copy { + Ok(x) => x, + Err(e) => { + soft_errors.push(SectionThreadListError::CopyFromProcessError(e)); + break; + } + }; let mem_section = MemoryArrayWriter::alloc_from_array(buffer, &memory_copy)?; ip_memory_d.memory = mem_section.location(); diff --git a/tests/linux_minidump_writer_soft_error.rs b/tests/linux_minidump_writer_soft_error.rs index 5cf8aaf4..a0881726 100644 --- a/tests/linux_minidump_writer_soft_error.rs +++ b/tests/linux_minidump_writer_soft_error.rs @@ -147,3 +147,28 @@ fn thread_stack_unreadable_soft_error() { let dump = Minidump::read_path(tmpfile.path()).expect("failed to read minidump"); assert_minidump_contains_soft_error(&dump, "CopyFromProcessError"); } + +#[test] +fn crashing_thread_ip_memory_unreadable_soft_error() { + let mut child = start_child_and_wait_for_threads(1); + let pid = child.id() as i32; + + let mut tmpfile = tempfile::Builder::new() + .prefix("ip_memory_unreadable") + .tempfile() + .unwrap(); + + let mut fail_client = FailSpotName::testing_client(); + fail_client.set_enabled(FailSpotName::CrashingThreadIpCopy, true); + + let mut config = MinidumpWriterConfig::new(pid, pid); + config.set_crash_context(get_dummy_crash_context(pid)); + config + .write(&mut tmpfile) + .expect("could not write minidump"); + child.kill().expect("Failed to kill process"); + child.wait().expect("Failed to wait on killed process"); + + let dump = Minidump::read_path(tmpfile.path()).expect("failed to read minidump"); + assert_minidump_contains_soft_error(&dump, "CopyFromProcessError"); +}