Skip to content

Commit

Permalink
Enable feature unwind by default
Browse files Browse the repository at this point in the history
Enable feature `unwind` by default, user can disable it by using:
`cargo build --no-default-features`

Signed-off-by: Jiang Liu <[email protected]>
  • Loading branch information
jiangliu committed Oct 15, 2024
1 parent 7c33089 commit 89fc82b
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 31 deletions.
9 changes: 5 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
[features]
unwind = []

[package]
name = "py-spy"
version = "0.3.14"
Expand Down Expand Up @@ -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]
Expand All @@ -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"]
2 changes: 1 addition & 1 deletion src/binary_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
14 changes: 9 additions & 5 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand Down
4 changes: 1 addition & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 1 addition & 3 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion src/native_stack_trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
31 changes: 18 additions & 13 deletions src/python_spy.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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>,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -433,6 +434,10 @@ impl PythonSpy {
let processed_os_threads: HashSet<Tid> =
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
Expand Down Expand Up @@ -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<u64>,
threadids: &std::collections::HashSet<u64>,
) -> Result<u64, Error> {
let mut pthread_id = 0;

Expand Down
2 changes: 1 addition & 1 deletion src/utils.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down

0 comments on commit 89fc82b

Please sign in to comment.