Skip to content

Commit 0aac0a4

Browse files
authored
Merge branch 'main' into ekump/macos-build-path-issue
2 parents cfd13f8 + 277094c commit 0aac0a4

File tree

8 files changed

+256
-50
lines changed

8 files changed

+256
-50
lines changed

.github/actions/changed-crates/action.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ runs:
4848
BASE_REF="origin/$PR_BASE_REF"
4949
# Ensure base branch is fetched
5050
echo "Fetching base branch: $PR_BASE_REF"
51-
git fetch --depth=1 origin "$PR_BASE_REF:refs/remotes/origin/$PR_BASE_REF" || true
51+
git fetch origin "$PR_BASE_REF:refs/remotes/origin/$PR_BASE_REF" || true
5252
else
5353
# For push events, compare with previous commit
5454
BASE_REF="HEAD~1"

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bin_tests/src/artifacts.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ pub fn crashtracker_bin_test(profile: BuildProfile, panic_abort: bool) -> Artifa
2929
}
3030

3131
/// Creates an ArtifactsBuild for the crashing_test_app binary.
32-
#[cfg(not(target_os = "macos"))]
3332
pub fn crashing_app(profile: BuildProfile, panic_abort: bool) -> ArtifactsBuild {
3433
ArtifactsBuild {
3534
name: "crashing_test_app".to_owned(),
@@ -90,15 +89,11 @@ pub fn all_prebuild_artifacts() -> Vec<ArtifactsBuild> {
9089
artifacts.push(crashtracker_receiver(profile));
9190
artifacts.push(test_the_tests(profile));
9291
artifacts.push(profiling_ffi(profile));
93-
94-
#[cfg(not(target_os = "macos"))]
9592
artifacts.push(crashing_app(profile, false));
9693
}
9794

9895
// Panic abort variants (used by panic hook tests)
9996
artifacts.push(crashtracker_bin_test(BuildProfile::Debug, true));
100-
101-
#[cfg(not(target_os = "macos"))]
10297
artifacts.push(crashing_app(BuildProfile::Debug, true));
10398

10499
artifacts

bin_tests/tests/crashtracker_bin_test.rs

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -423,19 +423,16 @@ fn test_crash_tracking_errors_intake_uds_socket() {
423423
/// DD_CRASHTRACK_END_STACKTRACE. Error: Can't set non-existant stack complete\n")
424424
#[test]
425425
#[cfg_attr(miri, ignore)]
426-
#[cfg(not(target_os = "macos"))]
427426
fn test_crash_tracking_bin_panic() {
428427
test_crash_tracking_app("panic");
429428
}
430429

431430
#[test]
432-
#[cfg(not(target_os = "macos"))]
433431
#[cfg_attr(miri, ignore)]
434432
fn test_crash_tracking_bin_segfault() {
435433
test_crash_tracking_app("segfault");
436434
}
437435

438-
#[cfg(not(target_os = "macos"))]
439436
fn test_crash_tracking_app(crash_type: &str) {
440437
use bin_tests::test_runner::run_custom_crash_test;
441438

@@ -484,7 +481,6 @@ fn test_crash_tracking_app(crash_type: &str) {
484481

485482
#[test]
486483
#[cfg_attr(miri, ignore)]
487-
#[cfg(not(target_os = "macos"))] // Same restriction as other panic tests
488484
fn test_crash_tracking_bin_panic_hook_after_fork() {
489485
test_panic_hook_mode(
490486
"panic_hook_after_fork",
@@ -495,14 +491,12 @@ fn test_crash_tracking_bin_panic_hook_after_fork() {
495491

496492
#[test]
497493
#[cfg_attr(miri, ignore)]
498-
#[cfg(not(target_os = "macos"))] // Same restriction as other panic tests
499494
fn test_crash_tracking_bin_panic_hook_string() {
500495
test_panic_hook_mode("panic_hook_string", "message", Some("Panic with value: 42"));
501496
}
502497

503498
#[test]
504499
#[cfg_attr(miri, ignore)]
505-
#[cfg(not(target_os = "macos"))] // Same restriction as other panic tests
506500
fn test_crash_tracking_bin_panic_hook_unknown_type() {
507501
test_panic_hook_mode(
508502
"panic_hook_unknown_type",
@@ -513,7 +507,6 @@ fn test_crash_tracking_bin_panic_hook_unknown_type() {
513507

514508
/// Helper function to run panic hook tests with different payload types.
515509
/// Note: Since tests are built with Debug profile, location is always expected.
516-
#[cfg(not(target_os = "macos"))]
517510
fn test_panic_hook_mode(mode: &str, expected_category: &str, expected_panic_message: Option<&str>) {
518511
use bin_tests::test_runner::run_custom_crash_test;
519512

@@ -580,20 +573,20 @@ fn test_panic_hook_mode(mode: &str, expected_category: &str, expected_panic_mess
580573
// ====================================================================================
581574
// These tests use `run_custom_crash_test` with the crashing_test_app artifact.
582575

583-
// This test is disabled for now on x86_64 musl and macos
576+
// This test is disabled for now on x86_64 musl
584577
// It seems that on aarch64 musl, libc has CFI which allows
585578
// unwinding passed the signal frame.
586579
// Don't forget to update the ignore condition for this and also
587580
// `test_crash_tracking_callstack` when this is revisited.
588581
#[test]
589-
#[cfg(not(any(all(target_arch = "x86_64", target_env = "musl"), target_os = "macos")))]
582+
#[cfg(not(any(all(target_arch = "x86_64", target_env = "musl"))))]
590583
#[cfg_attr(miri, ignore)]
591584
fn test_crasht_tracking_validate_callstack() {
592585
test_crash_tracking_callstack()
593586
}
594587

595-
// This test is disabled for now on x86_64 musl and macos for the reason mentioned above.
596-
#[cfg(not(any(all(target_arch = "x86_64", target_env = "musl"), target_os = "macos")))]
588+
// This test is disabled for now on x86_64 musl for the reason mentioned above.
589+
#[cfg(not(any(all(target_arch = "x86_64", target_env = "musl"))))]
597590
fn test_crash_tracking_callstack() {
598591
use bin_tests::test_runner::run_custom_crash_test;
599592

libdd-crashtracker/src/collector/emitters.rs

Lines changed: 131 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,38 @@ unsafe fn emit_backtrace_by_frames(
5555
w: &mut impl Write,
5656
resolve_frames: StacktraceCollection,
5757
fault_ip: usize,
58+
ucontext: *const ucontext_t,
5859
) -> Result<(), EmitterError> {
5960
// https://docs.rs/backtrace/latest/backtrace/index.html
6061
writeln!(w, "{DD_CRASHTRACK_BEGIN_STACKTRACE}")?;
6162

62-
// Absolute addresses appear to be safer to collect during a crash than debug info.
63+
// On macOS, backtrace::trace_unsynchronized fails in forked children because
64+
// macOS restricts many APIs after fork-without-exec. Walk the frame pointer
65+
// chain directly from the saved ucontext registers instead. The parent's
66+
// stack memory is still readable in the forked child
67+
#[cfg(target_os = "macos")]
68+
{
69+
let _ = (resolve_frames, fault_ip);
70+
emit_backtrace_from_ucontext(w, ucontext)?;
71+
}
72+
73+
#[cfg(not(target_os = "macos"))]
74+
{
75+
let _ = ucontext;
76+
emit_backtrace_via_library(w, resolve_frames, fault_ip)?;
77+
}
78+
79+
writeln!(w, "{DD_CRASHTRACK_END_STACKTRACE}")?;
80+
w.flush()?;
81+
Ok(())
82+
}
83+
84+
#[allow(dead_code)] // used from tests on macOS, from emit_backtrace_by_frames on other platforms
85+
unsafe fn emit_backtrace_via_library(
86+
w: &mut impl Write,
87+
resolve_frames: StacktraceCollection,
88+
fault_ip: usize,
89+
) -> Result<(), EmitterError> {
6390
fn emit_absolute_addresses(w: &mut impl Write, frame: &Frame) -> Result<(), EmitterError> {
6491
write!(w, "\"ip\": \"{:?}\"", frame.ip())?;
6592
if let Some(module_base_address) = frame.module_base_address() {
@@ -132,7 +159,103 @@ unsafe fn emit_backtrace_by_frames(
132159
// emit anything at all, if the crashing frame is not found for some reason
133160
ip_found = true;
134161
}
135-
writeln!(w, "{DD_CRASHTRACK_END_STACKTRACE}")?;
162+
Ok(())
163+
}
164+
165+
/// Walk the frame pointer chain from the ucontext's saved registers.
166+
///
167+
/// After fork(), the child process has a copy-on-write view of the parent's
168+
/// stack memory, so the frame pointer chain from the crashed thread is still
169+
/// readable. This avoids depending on `backtrace::trace_unsynchronized` which
170+
/// uses macOS APIs that don't work in forked-but-not-exec'd children.
171+
///
172+
/// For each IP we call `dladdr` to resolve the symbol name, symbol address,
173+
/// and containing shared-object path. `dladdr` is safe here because it only
174+
/// reads dyld's internal data structures (no allocation, no Mach IPC).
175+
#[cfg(target_os = "macos")]
176+
unsafe fn emit_backtrace_from_ucontext(
177+
w: &mut impl Write,
178+
ucontext: *const ucontext_t,
179+
) -> Result<(), EmitterError> {
180+
if ucontext.is_null() {
181+
return Ok(());
182+
}
183+
let mcontext = (*ucontext).uc_mcontext;
184+
if mcontext.is_null() {
185+
return Ok(());
186+
}
187+
188+
// Get the thread's stack bounds so we only deref frame pointers
189+
// that lie within known stack memory. Both pthread_get_stackaddr_np and
190+
// pthread_get_stacksize_np are async-signal-safe on macOS
191+
let thread = libc::pthread_self();
192+
let stack_top = libc::pthread_get_stackaddr_np(thread) as usize;
193+
let stack_size = libc::pthread_get_stacksize_np(thread);
194+
let stack_bottom = stack_top.saturating_sub(stack_size);
195+
196+
// Returns true when the range [addr, addr+len) falls within the thread stack
197+
let in_stack_bounds = |addr: usize, len: usize| -> bool {
198+
let end = addr.saturating_add(len);
199+
addr >= stack_bottom && end <= stack_top
200+
};
201+
202+
let ss = &(*mcontext).__ss;
203+
#[cfg(target_arch = "aarch64")]
204+
let (pc, mut fp) = (ss.__pc as usize, ss.__fp as usize);
205+
#[cfg(target_arch = "x86_64")]
206+
let (pc, mut fp) = (ss.__rip as usize, ss.__rbp as usize);
207+
208+
emit_frame_with_dladdr(w, pc)?;
209+
210+
const MAX_FRAMES: usize = 512;
211+
for _ in 0..MAX_FRAMES {
212+
if fp == 0 || fp % std::mem::align_of::<usize>() != 0 {
213+
break;
214+
}
215+
// Each frame record is two pointer-sized words: [saved_fp, return_addr]
216+
// Bail out if the record falls outside the thread stack
217+
if !in_stack_bounds(fp, 2 * std::mem::size_of::<usize>()) {
218+
break;
219+
}
220+
let next_fp = *(fp as *const usize);
221+
let return_addr = *((fp + std::mem::size_of::<usize>()) as *const usize);
222+
if return_addr == 0 {
223+
break;
224+
}
225+
emit_frame_with_dladdr(w, return_addr)?;
226+
if next_fp <= fp {
227+
break;
228+
}
229+
fp = next_fp;
230+
}
231+
232+
Ok(())
233+
}
234+
235+
/// Emit a single stack frame, enriched with `dladdr` symbol information.
236+
#[cfg(target_os = "macos")]
237+
unsafe fn emit_frame_with_dladdr(w: &mut impl Write, ip: usize) -> Result<(), EmitterError> {
238+
let mut info: libc::Dl_info = std::mem::zeroed();
239+
let resolved = libc::dladdr(ip as *const libc::c_void, &mut info) != 0;
240+
241+
write!(w, "{{\"ip\": \"0x{ip:x}\"")?;
242+
243+
if resolved {
244+
if !info.dli_fbase.is_null() {
245+
write!(w, ", \"module_base_address\": \"{:?}\"", info.dli_fbase)?;
246+
}
247+
if !info.dli_saddr.is_null() {
248+
write!(w, ", \"symbol_address\": \"{:?}\"", info.dli_saddr)?;
249+
}
250+
if !info.dli_sname.is_null() {
251+
let name = std::ffi::CStr::from_ptr(info.dli_sname);
252+
if let Ok(s) = name.to_str() {
253+
write!(w, ", \"function\": \"{s}\"")?;
254+
}
255+
}
256+
}
257+
258+
writeln!(w, "}}")?;
136259
w.flush()?;
137260
Ok(())
138261
}
@@ -213,7 +336,9 @@ pub(crate) fn emit_crashreport(
213336
// https://doc.rust-lang.org/src/std/backtrace.rs.html#332
214337
// We do this last, so even if it crashes, we still get the other info.
215338
let fault_ip = extract_ip(ucontext);
216-
unsafe { emit_backtrace_by_frames(pipe, config.resolve_frames(), fault_ip)? };
339+
unsafe {
340+
emit_backtrace_by_frames(pipe, config.resolve_frames(), fault_ip, ucontext)?
341+
};
217342
}
218343
if is_runtime_callback_registered() {
219344
emit_runtime_stack(pipe)?;
@@ -501,9 +626,11 @@ mod tests {
501626
})
502627
};
503628
let mut buf = Vec::new();
629+
writeln!(buf, "{DD_CRASHTRACK_BEGIN_STACKTRACE}").unwrap();
504630
unsafe {
505-
emit_backtrace_by_frames(&mut buf, collection, ip_of_test_fn).expect("to work ;-)");
631+
emit_backtrace_via_library(&mut buf, collection, ip_of_test_fn).expect("to work ;-)");
506632
}
633+
writeln!(buf, "{DD_CRASHTRACK_END_STACKTRACE}").unwrap();
507634
buf
508635
}
509636

libdd-crashtracker/src/receiver/entry_points.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22
// Copyright 2023-Present Datadog, Inc. https://www.datadoghq.com/
33

44
use super::receive_report::receive_report_from_stream;
5-
use crate::{crash_info::CrashInfo, CrashtrackerConfiguration, StacktraceCollection};
5+
use crate::crash_info::CrashInfo;
6+
use crate::CrashtrackerConfiguration;
7+
#[cfg(target_os = "linux")]
8+
use crate::StacktraceCollection;
69
use anyhow::Context;
710
use std::time::Duration;
811
use tokio::{
@@ -133,6 +136,10 @@ fn resolve_frames(
133136
config: &CrashtrackerConfiguration,
134137
crash_info: &mut CrashInfo,
135138
) -> anyhow::Result<()> {
139+
// enrich_callstacks uses blazesym's normalize_user_addrs (reads /proc/<pid>/maps)
140+
// and assumes ELF binaries. Both are Linux-specific; macOS has no procfs and
141+
// uses Mach-O binaries.
142+
#[cfg(target_os = "linux")]
136143
if config.resolve_frames() == StacktraceCollection::EnabledWithSymbolsInReceiver {
137144
let pid = crash_info
138145
.proc_info
@@ -141,5 +148,7 @@ fn resolve_frames(
141148
.pid;
142149
crash_info.enrich_callstacks(pid)?;
143150
}
151+
#[cfg(not(target_os = "linux"))]
152+
let _ = (config, crash_info);
144153
Ok(())
145154
}

libdd-library-config/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ rmp-serde = "1.3.0"
2525

2626
[dev-dependencies]
2727
tempfile = { version = "3.3" }
28+
serial_test = "3.2"
2829

2930
[target.'cfg(unix)'.dependencies]
3031
memfd = { version = "0.6" }

0 commit comments

Comments
 (0)