diff --git a/samply-mac-preload/binaries/libsamply_mac_preload.dylib b/samply-mac-preload/binaries/libsamply_mac_preload.dylib index 0c1fc394..a0e0f716 100755 Binary files a/samply-mac-preload/binaries/libsamply_mac_preload.dylib and b/samply-mac-preload/binaries/libsamply_mac_preload.dylib differ diff --git a/samply-mac-preload/binaries/libsamply_mac_preload_arm64.dylib b/samply-mac-preload/binaries/libsamply_mac_preload_arm64.dylib index efa29040..c7fa6bce 100755 Binary files a/samply-mac-preload/binaries/libsamply_mac_preload_arm64.dylib and b/samply-mac-preload/binaries/libsamply_mac_preload_arm64.dylib differ diff --git a/samply-mac-preload/binaries/libsamply_mac_preload_arm64e.dylib b/samply-mac-preload/binaries/libsamply_mac_preload_arm64e.dylib index 500af622..fa8bb205 100755 Binary files a/samply-mac-preload/binaries/libsamply_mac_preload_arm64e.dylib and b/samply-mac-preload/binaries/libsamply_mac_preload_arm64e.dylib differ diff --git a/samply-mac-preload/binaries/libsamply_mac_preload_x86_64.dylib b/samply-mac-preload/binaries/libsamply_mac_preload_x86_64.dylib index 646f38df..7397b9aa 100755 Binary files a/samply-mac-preload/binaries/libsamply_mac_preload_x86_64.dylib and b/samply-mac-preload/binaries/libsamply_mac_preload_x86_64.dylib differ diff --git a/samply-mac-preload/src/lib.rs b/samply-mac-preload/src/lib.rs index 894dbe9f..b74385d9 100644 --- a/samply-mac-preload/src/lib.rs +++ b/samply-mac-preload/src/lib.rs @@ -35,7 +35,52 @@ static __SETUP_SAMPLY_CONNECTION: unsafe extern "C" fn() = { __load_samply_lib }; +/// Returns true if the current process is an Apple "platform binary" +/// (`CS_PLATFORM_BINARY`). Such processes are given an *immovable* task-self +/// mach port by the kernel: any attempt to transfer that port to another +/// process — which is exactly what samply's task handoff below does — raises a +/// fatal `EXC_GUARD` (`ILLEGAL_MOVE`) and the kernel SIGKILLs the process. +/// +/// This is how `samply record -- ` would otherwise kill `dsymutil` (and +/// other Apple toolchain binaries) that a build invokes: they inherit samply's +/// `DYLD_INSERT_LIBRARIES`, load this preload, and crash in the handoff. +/// +/// samply cannot profile platform binaries through this mechanism regardless +/// (their task port is protected), so detecting this case and skipping the +/// handoff loses nothing and keeps the process alive. +fn is_platform_binary() -> bool { + // `csops(getpid(), CS_OPS_STATUS, &flags, sizeof(flags))` reports the + // process's code-signing status flags. It works on the calling process + // without any privilege. CS_PLATFORM_BINARY == 0x04000000. + const CS_OPS_STATUS: u32 = 0; + const CS_PLATFORM_BINARY: u32 = 0x0400_0000; + extern "C" { + fn csops( + pid: libc::pid_t, + ops: u32, + useraddr: *mut libc::c_void, + usersize: libc::size_t, + ) -> libc::c_int; + } + let mut flags: u32 = 0; + let r = unsafe { + csops( + libc::getpid(), + CS_OPS_STATUS, + &mut flags as *mut u32 as *mut libc::c_void, + core::mem::size_of::() as libc::size_t, + ) + }; + r == 0 && (flags & CS_PLATFORM_BINARY) != 0 +} + fn set_up_samply_connection() -> Option<()> { + // Don't hand our task port to samply if we're a platform binary: the port + // is immovable and sending it would get us SIGKILLed. See + // `is_platform_binary`. + if is_platform_binary() { + return None; + } let (tx0, rx0) = channel().ok()?; // Safety: // - b"SAMPLY_BOOTSTRAP_SERVER_NAME\0" is a nul-terminated c string diff --git a/samply/resources/libsamply_mac_preload.dylib.gz b/samply/resources/libsamply_mac_preload.dylib.gz index e8085588..ec139f14 100644 Binary files a/samply/resources/libsamply_mac_preload.dylib.gz and b/samply/resources/libsamply_mac_preload.dylib.gz differ diff --git a/samply/tests/dsymutil_sigkill.rs b/samply/tests/dsymutil_sigkill.rs new file mode 100644 index 00000000..c8a5b43a --- /dev/null +++ b/samply/tests/dsymutil_sigkill.rs @@ -0,0 +1,122 @@ +//! Regression test for: `samply record` SIGKILLs `dsymutil`. +//! +//! samply injects `DYLD_INSERT_LIBRARIES` (+ `SAMPLY_BOOTSTRAP_SERVER_NAME`) into +//! the *entire* descendant process tree it launches. Any descendant that loads +//! the preload hands its mach task port to samply and lets it "control us +//! completely" (see `samply-mac-preload`). For `dsymutil` this takeover ends in a +//! deterministic `SIGKILL`, which breaks builds run under `samply record` on +//! macOS (the linker invokes `dsymutil` and reports `running dsymutil failed: +//! signal: killed`). +//! +//! This test launches, under the built `samply` binary, a small locally-built +//! "spawner" that execs `dsymutil` on a Mach-O with DWARF, and asserts that +//! `dsymutil` is NOT killed (the desired behaviour). +//! +//! macOS-only (`cfg(target_os = "macos")`), so it compiles out elsewhere. It +//! needs Xcode's `dsymutil` and a working `cc`, both present on `macos-latest` +//! CI runners. Launch-mode profiling does not need `task_for_pid` entitlements +//! (the child volunteers its task port), so no `samply setup` is required. +//! +//! Run with: +//! cargo test -p samply --test dsymutil_sigkill -- --nocapture +#![cfg(target_os = "macos")] + +use std::path::Path; +use std::process::Command; + +fn cc(args: &[&str]) { + let status = Command::new("cc").args(args).status().expect("failed to run cc"); + assert!(status.success(), "cc {args:?} failed"); +} + +fn xcrun_dsymutil() -> String { + let out = Command::new("xcrun") + .args(["-f", "dsymutil"]) + .output() + .expect("failed to run xcrun"); + assert!(out.status.success(), "xcrun -f dsymutil failed"); + String::from_utf8(out.stdout).unwrap().trim().to_string() +} + +#[test] +fn dsymutil_is_not_killed_under_samply() { + let samply = env!("CARGO_BIN_EXE_samply"); + let dsymutil = xcrun_dsymutil(); + + let tmp = std::env::temp_dir().join(format!("samply_dsym_repro_{}", std::process::id())); + std::fs::create_dir_all(&tmp).unwrap(); + + // A Mach-O with enough DWARF that dsymutil does real work. + let src = tmp.join("big.cpp"); + { + use std::fmt::Write as _; + let mut s = String::from("#include \n"); + for i in 0..1200 { + writeln!(s, "template struct S{i} {{ int v[N%7+1]; int f(int x){{return x*{i}+N;}} }};").unwrap(); + writeln!(s, "int g{i}(int x){{ S{i}<{}> s; return s.f(x)+{i}; }}", i % 9 + 1).unwrap(); + } + s.push_str("int main(){int t=0;"); + for i in 0..1200 { + write!(s, "t+=g{i}(t);").unwrap(); + } + s.push_str("printf(\"%d\\n\",t);return 0;}\n"); + std::fs::write(&src, s).unwrap(); + } + let macho = tmp.join("bigcpp"); + cc(&["-g", "-O0", "-o", macho.to_str().unwrap(), src.to_str().unwrap()]); + + // A locally-built (non-restricted) parent that execs dsymutil and reports + // how the child died via its own exit code: 0 = clean, 1 = killed by signal. + let spawner_src = tmp.join("spawner.c"); + std::fs::write( + &spawner_src, + r#" +#include +#include +#include +#include +int main(int argc, char** argv){ + pid_t pid = fork(); + if(pid==0){ execl(argv[1],"dsymutil","-f",argv[2],"-o",argv[3],(char*)0); _exit(127); } + int st=0; waitpid(pid,&st,0); + if(WIFSIGNALED(st)){ fprintf(stderr,"dsymutil killed by signal %d\n", WTERMSIG(st)); return 1; } + fprintf(stderr,"dsymutil exited code %d\n", WEXITSTATUS(st)); return 0; +} +"#, + ) + .unwrap(); + let spawner = tmp.join("spawner"); + cc(&["-O0", "-o", spawner.to_str().unwrap(), spawner_src.to_str().unwrap()]); + + let out_dwarf = tmp.join("out.dwarf"); + let profile = tmp.join("profile.json.gz"); + + // samply record --save-only -o -- + let status = Command::new(samply) + .args(["record", "--save-only", "-o"]) + .arg(&profile) + .arg("--") + .arg(&spawner) + .arg(&dsymutil) + .arg(&macho) + .arg(&out_dwarf) + .status() + .expect("failed to run samply"); + + // The spawner exits 0 iff dsymutil completed normally. Under the bug it exits + // 1 because dsymutil was SIGKILLed by samply. + let killed = !status.success(); + let produced_output = Path::new(&out_dwarf).exists(); + + // Clean up only after we've inspected the results (out.dwarf lives in `tmp`). + let _ = std::fs::remove_dir_all(&tmp); + + assert!( + !killed, + "dsymutil was killed when run under `samply record` \ + (spawner exit = {:?}). samply must not SIGKILL build subprocesses it \ + injects into.", + status.code() + ); + assert!(produced_output, "dsymutil did not produce its output"); +}