From 89fc82bc7fa2bd438d1a243a1c29b2f05bcd1cb8 Mon Sep 17 00:00:00 2001 From: Jiang Liu Date: Tue, 15 Oct 2024 00:41:55 +0800 Subject: [PATCH] Enable feature `unwind` by default Enable feature `unwind` by default, user can disable it by using: `cargo build --no-default-features` Signed-off-by: Jiang Liu --- Cargo.toml | 9 +++++---- src/binary_parser.rs | 2 +- src/config.rs | 14 +++++++++----- src/lib.rs | 4 +--- src/main.rs | 4 +--- src/native_stack_trace.rs | 4 +++- src/python_spy.rs | 31 ++++++++++++++++++------------- src/utils.rs | 2 +- 8 files changed, 39 insertions(+), 31 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 044e182b..37f8a6a3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,3 @@ -[features] -unwind = [] - [package] name = "py-spy" version = "0.3.14" @@ -37,7 +34,7 @@ serde_derive = "1.0" serde_json = "1.0" rand = "0.8" rand_distr = "0.4" -remoteprocess = {version="0.4.12", features=["unwind"]} +remoteprocess = "0.4.12" chrono = "0.4.26" [dev-dependencies] @@ -48,3 +45,7 @@ termios = "0.3.3" [target.'cfg(windows)'.dependencies] winapi = {version = "0.3", features = ["errhandlingapi", "winbase", "consoleapi", "wincon", "handleapi", "timeapi", "processenv" ]} + +[features] +default = ["unwind"] +unwind = ["remoteprocess/unwind"] diff --git a/src/binary_parser.rs b/src/binary_parser.rs index ace9ab45..c62cacbf 100644 --- a/src/binary_parser.rs +++ b/src/binary_parser.rs @@ -17,7 +17,7 @@ pub struct BinaryInfo { } impl BinaryInfo { - #[cfg(feature = "unwind")] + #[cfg(all(feature = "unwind", target_os = "linux", target_arch = "x86_64"))] pub fn contains(&self, addr: u64) -> bool { addr >= self.addr && addr < (self.addr + self.size) } diff --git a/src/config.rs b/src/config.rs index 709516eb..3b900516 100644 --- a/src/config.rs +++ b/src/config.rs @@ -160,7 +160,7 @@ impl Config { .help("PID of a running python program to spy on") .takes_value(true); - #[cfg(feature = "unwind")] + #[cfg(all(feature = "unwind", target_os = "linux", target_arch = "x86_64"))] let native = Arg::new("native") .short('n') .long("native") @@ -328,11 +328,11 @@ impl Config { ); // add native unwinding if appropriate - #[cfg(feature = "unwind")] + #[cfg(all(feature = "unwind", target_os = "linux", target_arch = "x86_64"))] let record = record.arg(native.clone()); - #[cfg(feature = "unwind")] + #[cfg(all(feature = "unwind", target_os = "linux", target_arch = "x86_64"))] let top = top.arg(native.clone()); - #[cfg(feature = "unwind")] + #[cfg(all(feature = "unwind", target_os = "linux", target_arch = "x86_64"))] let dump = dump.arg(native.clone()); // Nonblocking isn't an option for freebsd, remove @@ -429,7 +429,11 @@ impl Config { .value_of("pid") .map(|p| p.parse().expect("invalid pid")); config.full_filenames = matches.occurrences_of("full_filenames") > 0; - if cfg!(feature = "unwind") { + if cfg!(all( + feature = "unwind", + target_os = "linux", + target_arch = "x86_64" + )) { config.native = matches.occurrences_of("native") > 0; } diff --git a/src/lib.rs b/src/lib.rs index b7bb259b..943087b6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -33,10 +33,8 @@ pub mod binary_parser; pub mod config; #[cfg(target_os = "linux")] pub mod coredump; -#[cfg(feature = "unwind")] -mod cython; pub mod dump; -#[cfg(feature = "unwind")] +#[cfg(all(feature = "unwind", target_os = "linux", target_arch = "x86_64"))] mod native_stack_trace; mod python_bindings; mod python_data_access; diff --git a/src/main.rs b/src/main.rs index df0d602c..e0193da1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9,11 +9,9 @@ mod config; mod console_viewer; #[cfg(target_os = "linux")] mod coredump; -#[cfg(feature = "unwind")] -mod cython; mod dump; mod flamegraph; -#[cfg(feature = "unwind")] +#[cfg(all(feature = "unwind", target_os = "linux", target_arch = "x86_64"))] mod native_stack_trace; mod python_bindings; mod python_data_access; diff --git a/src/native_stack_trace.rs b/src/native_stack_trace.rs index ba7e6c64..1e1c20c3 100644 --- a/src/native_stack_trace.rs +++ b/src/native_stack_trace.rs @@ -8,10 +8,12 @@ use lru::LruCache; use remoteprocess::{self, Pid}; use crate::binary_parser::BinaryInfo; -use crate::cython; use crate::stack_trace::Frame; use crate::utils::resolve_filename; +#[path = "cython.rs"] +mod cython; + pub struct NativeStack { should_reload: bool, python: Option, diff --git a/src/python_spy.rs b/src/python_spy.rs index 531616ab..e286a241 100644 --- a/src/python_spy.rs +++ b/src/python_spy.rs @@ -1,15 +1,11 @@ use std::collections::HashMap; -#[cfg(all(target_os = "linux", feature = "unwind"))] -use std::collections::HashSet; -#[cfg(all(target_os = "linux", feature = "unwind"))] -use std::iter::FromIterator; use std::path::Path; use anyhow::{Context, Error, Result}; use remoteprocess::{Pid, Process, ProcessMemory, Tid}; use crate::config::{Config, LockingStrategy}; -#[cfg(feature = "unwind")] +#[cfg(all(feature = "unwind", target_os = "linux", target_arch = "x86_64"))] use crate::native_stack_trace::NativeStack; use crate::python_bindings::{ v2_7_15, v3_10_0, v3_11_0, v3_12_0, v3_3_7, v3_5_5, v3_6_6, v3_7_0, v3_8_0, v3_9_5, @@ -31,7 +27,7 @@ pub struct PythonSpy { pub interpreter_address: usize, pub threadstate_address: usize, pub config: Config, - #[cfg(feature = "unwind")] + #[cfg(all(feature = "unwind", target_os = "linux", target_arch = "x86_64"))] pub native: Option, pub short_filenames: HashMap>, pub python_thread_ids: HashMap, @@ -64,7 +60,7 @@ impl PythonSpy { // lets us figure out which thread has the GIL let threadstate_address = get_threadstate_address(&python_info, &version, config)?; - #[cfg(feature = "unwind")] + #[cfg(all(feature = "unwind", target_os = "linux", target_arch = "x86_64"))] let native = if config.native { Some(NativeStack::new( pid, @@ -81,7 +77,7 @@ impl PythonSpy { version, interpreter_address, threadstate_address, - #[cfg(feature = "unwind")] + #[cfg(all(feature = "unwind", target_os = "linux", target_arch = "x86_64"))] native, #[cfg(target_os = "linux")] dockerized: python_info.dockerized, @@ -288,7 +284,7 @@ impl PythonSpy { } // Merge in the native stack frames if necessary - #[cfg(feature = "unwind")] + #[cfg(all(feature = "unwind", target_os = "linux", target_arch = "x86_64"))] { if self.config.native { if let Some(native) = self.native.as_mut() { @@ -386,7 +382,10 @@ impl PythonSpy { Ok(None) } - #[cfg(all(target_os = "linux", not(feature = "unwind")))] + #[cfg(all( + target_os = "linux", + not(all(feature = "unwind", target_arch = "x86_64")) + ))] fn _get_os_thread_id( &mut self, _python_thread_id: u64, @@ -395,12 +394,14 @@ impl PythonSpy { Ok(None) } - #[cfg(all(target_os = "linux", feature = "unwind"))] + #[cfg(all(target_os = "linux", feature = "unwind", target_arch = "x86_64"))] fn _get_os_thread_id( &mut self, python_thread_id: u64, interp: &I, ) -> Result, Error> { + use std::collections::HashSet; + // in nonblocking mode, we can't get the threadid reliably (method here requires reading the RBX // register which requires a ptrace attach). fallback to heuristic thread activity here if self.config.blocking == LockingStrategy::NonBlocking { @@ -433,6 +434,10 @@ impl PythonSpy { let processed_os_threads: HashSet = HashSet::from_iter(self.python_thread_ids.values().copied()); + if self.config.native { + panic!("Native stack traces not supported on this platform"); + } + let unwinder = self.process.unwinder()?; // Try getting the pthread_id from the native stack registers for threads we haven't looked up yet @@ -480,12 +485,12 @@ impl PythonSpy { Ok(None) } - #[cfg(all(target_os = "linux", feature = "unwind"))] + #[cfg(all(target_os = "linux", feature = "unwind", target_arch = "x86_64"))] pub fn _get_pthread_id( &self, unwinder: &remoteprocess::Unwinder, thread: &remoteprocess::Thread, - threadids: &HashSet, + threadids: &std::collections::HashSet, ) -> Result { let mut pthread_id = 0; diff --git a/src/utils.rs b/src/utils.rs index 2ca1374f..ee58e4de 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,4 +1,4 @@ -#[cfg(feature = "unwind")] +#[cfg(all(feature = "unwind", target_os = "linux", target_arch = "x86_64"))] pub fn resolve_filename(filename: &str, modulename: &str) -> Option { // check the filename first, if it exists use it use std::path::Path;