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 <[email protected]>
benfred · Oct 16, 2024 · fbd38cf · fbd38cf
1 parent e8c8c97
commit fbd38cf
Show file tree

Hide file tree

Showing 9 changed files with 37 additions and 31 deletions.
diff --git 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
@@ -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
@@ -166,7 +166,11 @@ impl Config {
             .help("Collect stack traces from native extensions written in Cython, C or C++");
 
         // Only show `--native` on platforms where it's supported
-        if !cfg!(feature = "unwind") {
+        if !cfg!(all(
+            feature = "unwind",
+            target_os = "linux",
+            target_arch = "x86_64"
+        )) {
             native = native.hide(true);
         }
 
@@ -362,7 +366,7 @@ impl Config {
         let (subcommand, matches) = matches.subcommand().unwrap();
 
         // Check if `--native` was used on an unsupported platform
-        if !cfg!(feature = "unwind") && matches.contains_id("native") {
+        if native.is_hide_set() && matches.contains_id("native") {
             eprintln!(
                 "Collecting stack traces from native extensions (`--native`) is not supported on your platform."
             );
@@ -437,9 +441,7 @@ 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") {
-            config.native = matches.occurrences_of("native") > 0;
-        }
+        config.native = matches.occurrences_of("native") > 0;
 
         config.capture_output = config.command != "record" || matches.occurrences_of("capture") > 0;
         if !config.capture_output {

diff --git 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
@@ -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
@@ -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<BinaryInfo>,

diff --git 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<NativeStack>,
     pub short_filenames: HashMap<String, Option<String>>,
     pub python_thread_ids: HashMap<u64, Tid>,
@@ -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<I: InterpreterState>(
         &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<I: InterpreterState>(
         &mut self,
         python_thread_id: u64,
         interp: &I,
     ) -> Result<Option<Tid>, 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 {
@@ -446,6 +447,8 @@ impl PythonSpy {
                 Ok(pthread_id) => {
                     if pthread_id != 0 {
                         self.python_thread_ids.insert(pthread_id, threadid);
+                    } else if self.config.native {
+                        panic!("Native stack traces not supported on this platform");
                     }
                 }
                 Err(e) => {
@@ -480,12 +483,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<u64>,
+        threadids: &std::collections::HashSet<u64>,
     ) -> Result<u64, Error> {
         let mut pthread_id = 0;
 

diff --git 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<String> {
     // check the filename first, if it exists use it
     use std::path::Path;

diff --git a/tests/integration_test.rs b/tests/integration_test.rs
@@ -62,6 +62,8 @@ fn test_busy_loop() {
     assert!(traces[0].active);
 }
 
+// TODO: fix https://github.com/benfred/py-spy/issues/701
+#[ignore]
 #[cfg(feature = "unwind")]
 #[test]
 fn test_thread_reuse() {