From a93d97702054d7eedf8e4db0144612f47282cb07 Mon Sep 17 00:00:00 2001 From: CarePackage17 <5157010+CarePackage17@users.noreply.github.com> Date: Fri, 27 Jun 2025 14:20:10 +0200 Subject: [PATCH] Replace unwind-sys with backtrace crate on Linux (#37728) This PR removes `unwind-sys` usage from background_hang_monitor on Linux, replacing it with the [backtrace crate](https://crates.io/crates/backtrace). Testing: `hang_monitor-tests.rs` still pass after the change (on Ubuntu 24.04). Fixes: https://github.com/servo/servo/issues/35063 --------- Signed-off-by: CarePackage17 <5157010+CarePackage17@users.noreply.github.com> --- Cargo.lock | 11 --- components/background_hang_monitor/Cargo.toml | 3 +- .../background_hang_monitor/sampler_linux.rs | 84 +++---------------- 3 files changed, 13 insertions(+), 85 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 14a38dc8a33..4800d2042bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -704,7 +704,6 @@ dependencies = [ "mach2", "nix", "serde_json", - "unwind-sys", ] [[package]] @@ -8956,16 +8955,6 @@ version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ecb6da28b8a351d773b68d5825ac39017e680750f980f3a1a85cd8dd28a47c1" -[[package]] -name = "unwind-sys" -version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e7a81ba64bc45243d442e9bb2a362f303df152b5078c56ce4a0dc7d813c8df91" -dependencies = [ - "libc", - "pkg-config", -] - [[package]] name = "url" version = "2.5.3" diff --git a/components/background_hang_monitor/Cargo.toml b/components/background_hang_monitor/Cargo.toml index 20216ec78b7..9b389dba174 100644 --- a/components/background_hang_monitor/Cargo.toml +++ b/components/background_hang_monitor/Cargo.toml @@ -28,7 +28,6 @@ mach2 = { version = "0.4", optional = true } [target.'cfg(all(target_os = "linux", not(any(target_arch = "arm", target_arch = "aarch64", target_env = "ohos", target_env = "musl"))))'.dependencies] nix = { workspace = true, features = ["signal"], optional = true } -unwind-sys = { version = "0.1.4", optional = true } [features] -sampler = ["unwind-sys", "mach2", "nix"] +sampler = ["mach2", "nix"] diff --git a/components/background_hang_monitor/sampler_linux.rs b/components/background_hang_monitor/sampler_linux.rs index 5026b42ec63..8d80616378b 100644 --- a/components/background_hang_monitor/sampler_linux.rs +++ b/components/background_hang_monitor/sampler_linux.rs @@ -5,13 +5,9 @@ #![allow(unsafe_code)] use std::cell::UnsafeCell; -use std::sync::atomic::{AtomicPtr, Ordering}; -use std::{cmp, io, mem, process, ptr, thread}; +use std::{io, mem, process, thread}; use nix::sys::signal::{SaFlags, SigAction, SigHandler, SigSet, Signal, sigaction}; -use unwind_sys::{ - UNW_ESUCCESS, UNW_REG_IP, UNW_REG_SP, unw_cursor_t, unw_get_reg, unw_init_local, unw_step, -}; use crate::sampler::{NativeStack, Sampler}; @@ -27,8 +23,6 @@ static SHARED_STATE: UncheckedSyncUnsafeCell = msg4: None, })); -static CONTEXT: AtomicPtr = AtomicPtr::new(ptr::null_mut()); - type MonitoredThreadId = libc::pid_t; struct SharedState { @@ -48,7 +42,6 @@ fn clear_shared_state() { shared_state.msg3 = None; shared_state.msg4 = None; } - CONTEXT.store(ptr::null_mut(), Ordering::SeqCst); } fn reset_shared_state() { @@ -59,7 +52,6 @@ fn reset_shared_state() { shared_state.msg3 = Some(PosixSemaphore::new(0).expect("valid semaphore")); shared_state.msg4 = Some(PosixSemaphore::new(0).expect("valid semaphore")); } - CONTEXT.store(ptr::null_mut(), Ordering::SeqCst); } struct PosixSemaphore { @@ -149,40 +141,6 @@ impl LinuxSampler { } } -enum RegNum { - Ip = UNW_REG_IP as isize, - Sp = UNW_REG_SP as isize, -} - -fn get_register(cursor: *mut unw_cursor_t, num: RegNum) -> Result { - unsafe { - let mut val = 0; - let ret = unw_get_reg(cursor, num as i32, &mut val); - if ret == UNW_ESUCCESS { - Ok(val) - } else { - Err(ret) - } - } -} - -fn step(cursor: *mut unw_cursor_t) -> Result { - unsafe { - // libunwind 1.1 seems to get confused and walks off the end of the stack. The last IP - // it reports is 0, so we'll stop if we're there. - if get_register(cursor, RegNum::Ip).unwrap_or(1) == 0 { - return Ok(false); - } - - let ret = unw_step(cursor); - match ret.cmp(&0) { - cmp::Ordering::Less => Err(ret), - cmp::Ordering::Greater => Ok(true), - cmp::Ordering::Equal => Ok(false), - } - } -} - impl Sampler for LinuxSampler { #[allow(unsafe_code)] fn suspend_and_sample_thread(&self) -> Result { @@ -209,33 +167,17 @@ impl Sampler for LinuxSampler { .wait_through_intr() .expect("msg2 failed"); - let context = CONTEXT.load(Ordering::SeqCst); - let mut cursor = mem::MaybeUninit::uninit(); - let ret = unsafe { unw_init_local(cursor.as_mut_ptr(), context) }; - result = if ret == UNW_ESUCCESS { - let mut native_stack = NativeStack::new(); - #[allow(clippy::while_let_loop)] // False positive - loop { - let ip = match get_register(cursor.as_mut_ptr(), RegNum::Ip) { - Ok(ip) => ip, - Err(_) => break, - }; - let sp = match get_register(cursor.as_mut_ptr(), RegNum::Sp) { - Ok(sp) => sp, - Err(_) => break, - }; - if native_stack - .process_register(ip as *mut _, sp as *mut _) - .is_err() || - !step(cursor.as_mut_ptr()).unwrap_or(false) - { - break; - } - } - Ok(native_stack) - } else { - Err(()) + let mut native_stack = NativeStack::new(); + unsafe { + backtrace::trace_unsynchronized(|frame| { + let ip = frame.ip(); + let sp = frame.sp(); + + //This return value here determines whether we proceed to the next stack frame or not. + native_stack.process_register(ip, sp).is_ok() + }) }; + result = Ok(native_stack); // signal the thread to continue. shared_state @@ -272,11 +214,9 @@ impl Drop for LinuxSampler { extern "C" fn sigprof_handler( sig: libc::c_int, _info: *mut libc::siginfo_t, - ctx: *mut libc::c_void, + _ctx: *mut libc::c_void, ) { assert_eq!(sig, libc::SIGPROF); - // copy the context. - CONTEXT.store(ctx as *mut libc::ucontext_t, Ordering::SeqCst); // Safety: non-exclusive reference only // since the sampling thread is accessing this concurrently