Skip to content

Commit

Permalink
Fix clippy warnings (benfred#679)
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewjcg authored Oct 9, 2024
1 parent 8dd5492 commit 5820bf6
Show file tree
Hide file tree
Showing 13 changed files with 62 additions and 112 deletions.
78 changes: 25 additions & 53 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,10 @@ jobs:
steps:
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
with:
python-version: 3.11
- uses: pre-commit/[email protected]

build-linux-armv7:
runs-on: [self-hosted, linux, arm]
needs: [lint]
steps:
- name: Setup python
run: |
pyenv global system
python --version
- uses: actions/checkout@v3
- name: Build
run: cargo build --verbose
- name: Run tests
run: cargo test

build:
runs-on: ${{ matrix.os }}
needs: [lint]
Expand All @@ -52,6 +40,9 @@ jobs:
id: test
continue-on-error: true
run: cargo test --release
- uses: actions/setup-python@v4
with:
python-version: 3.11
- name: Test (retry#1)
id: test1
run: cargo test --release
Expand All @@ -60,9 +51,6 @@ jobs:
- name: Test (retry#2)
run: cargo test --release
if: steps.test1.outcome=='failure'
- uses: actions/setup-python@v4
with:
python-version: 3.9
- name: Build Wheel
run: |
pip install --upgrade maturin
Expand All @@ -75,6 +63,7 @@ jobs:
MACOSX_DEPLOYMENT_TARGET: 10.9
run: |
rustup target add aarch64-apple-darwin
rustup target add x86_64-apple-darwin
pip install --upgrade maturin
maturin build --release -o dist --target universal2-apple-darwin
if: matrix.os == 'macos-latest'
Expand Down Expand Up @@ -181,23 +170,33 @@ jobs:
strategy:
fail-fast: false
matrix:
python-version: [3.5.4, 3.5.9, 3.5.10, 3.6.7, 3.6.8, 3.6.9, 3.6.10, 3.6.11, 3.6.12, 3.6.13, 3.6.14, 3.6.15, 3.7.1, 3.7.2, 3.7.3, 3.7.4, 3.7.5, 3.7.6, 3.7.7, 3.7.8, 3.7.9, 3.7.10, 3.7.11, 3.7.12, 3.7.13, 3.7.14, 3.7.15, 3.7.16, 3.7.17, 3.8.0, 3.8.1, 3.8.2, 3.8.3, 3.8.4, 3.8.5, 3.8.6, 3.8.7, 3.8.8, 3.8.9, 3.8.10, 3.8.11, 3.8.12, 3.8.13, 3.8.14, 3.8.15, 3.8.16, 3.8.17, 3.8.18, 3.9.0, 3.9.1, 3.9.2, 3.9.3, 3.9.4, 3.9.5, 3.9.6, 3.9.7, 3.9.8, 3.9.9, 3.9.10, 3.9.11, 3.9.12, 3.9.13, 3.9.14, 3.9.15, 3.9.16, 3.9.17, 3.9.18, 3.10.0, 3.10.1, 3.10.2, 3.10.3, 3.10.4, 3.10.5, 3.10.6, 3.10.7, 3.10.8, 3.10.9, 3.10.10, 3.10.11, 3.10.12, 3.10.13, 3.11.0, 3.11.1, 3.11.2, 3.11.3, 3.11.4, 3.11.5]
python-version: [3.6.7, 3.6.8, 3.6.9, 3.6.10, 3.6.11, 3.6.12, 3.6.13, 3.6.14, 3.6.15, 3.7.1, 3.7.2, 3.7.3, 3.7.4, 3.7.5, 3.7.6, 3.7.7, 3.7.8, 3.7.9, 3.7.10, 3.7.11, 3.7.12, 3.7.13, 3.7.14, 3.7.15, 3.7.16, 3.7.17, 3.8.0, 3.8.1, 3.8.2, 3.8.3, 3.8.4, 3.8.5, 3.8.6, 3.8.7, 3.8.8, 3.8.9, 3.8.10, 3.8.11, 3.8.12, 3.8.13, 3.8.14, 3.8.15, 3.8.16, 3.8.17, 3.8.18, 3.9.0, 3.9.1, 3.9.2, 3.9.3, 3.9.4, 3.9.5, 3.9.6, 3.9.7, 3.9.8, 3.9.9, 3.9.10, 3.9.11, 3.9.12, 3.9.13, 3.9.14, 3.9.15, 3.9.16, 3.9.17, 3.9.18, 3.9.19, 3.9.20, 3.10.0, 3.10.1, 3.10.2, 3.10.3, 3.10.4, 3.10.5, 3.10.6, 3.10.7, 3.10.8, 3.10.9, 3.10.10, 3.10.11, 3.10.12, 3.10.13, 3.10.14, 3.10.15, 3.11.0, 3.11.1, 3.11.2, 3.11.3, 3.11.4, 3.11.5, 3.11.6, 3.11.7, 3.11.8, 3.11.9, 3.11.10]
# TODO: also test windows
os: [ubuntu-20.04, macos-latest]
os: [ubuntu-20.04, macos-13]
# some versions of python can't be tested on GHA with osx because of SIP:
exclude:
- os: macos-latest
- os: macos-13
python-version: 3.11.0
- os: macos-latest
- os: macos-13
python-version: 3.11.1
- os: macos-latest
- os: macos-13
python-version: 3.11.2
- os: macos-latest
- os: macos-13
python-version: 3.11.3
- os: macos-latest
- os: macos-13
python-version: 3.11.4
- os: macos-latest
- os: macos-13
python-version: 3.11.5
- os: macos-13
python-version: 3.11.6
- os: macos-13
python-version: 3.11.7
- os: macos-13
python-version: 3.11.8
- os: macos-13
python-version: 3.11.9
- os: macos-13
python-version: 3.11.10

steps:
- uses: actions/checkout@v2
Expand Down Expand Up @@ -239,38 +238,11 @@ jobs:
run: sudo "PATH=$PATH" python tests/integration_test.py
if: steps.osx_test1.outcome=='failure'

test-wheel-linux-armv7:
name: Test ARMv7 Wheel
needs: [build-linux-cross]
runs-on: [self-hosted, linux, arm]
strategy:
matrix:
# we're installing the manylinux2014 wheel, so can
# only test out relatively recent versions of python
pyenv-python-version: [3.7.10, 3.8.9, 3.9.4]
steps:
- uses: actions/checkout@v3
- uses: actions/download-artifact@v3
with:
name: wheels
- name: Setup pyenv
run: |
# build the version of python if not installed already
# (note this relies on pyenv being setup already)
pyenv install -s ${{ matrix.pyenv-python-version }}
pyenv global ${{ matrix.pyenv-python-version }}
python --version
- name: Install wheel
run: |
pip install --force-reinstall --no-index --find-links . py-spy
- name: Test Wheel
run: python tests/integration_test.py

release:
name: Release
runs-on: ubuntu-latest
if: "startsWith(github.ref, 'refs/tags/')"
needs: [test-wheels, test-wheel-linux-armv7]
needs: [test-wheels]
steps:
- uses: actions/download-artifact@v3
with:
Expand Down
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
[features]
unwind = []

[package]
name = "py-spy"
version = "0.3.14"
Expand Down
4 changes: 3 additions & 1 deletion ci/test_freebsd.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ if [ -f build-artifacts.tar ]; then
fi

cargo build --release --workspace --all-targets
cargo test --release

# TODO: re-enable integration tests
# cargo test --release

set +e
tar cf build-artifacts.tar target
Expand Down
4 changes: 2 additions & 2 deletions ci/update_python_test_versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ def get_github_python_versions():
continue

major, minor, patch = parse_version(version_str)
if major == 3 and minor < 5:
# we don't support python 3.0/3.1/3.2 , and don't bother testing 3.3/3.4
if major == 3 and minor < 6:
# we don't support python 3.0/3.1/3.2 , and don't bother testing 3.3/3.4/3.5
continue

elif major == 2 and minor < 7:
Expand Down
20 changes: 2 additions & 18 deletions src/binary_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,20 @@ use goblin::Object;
use memmap::Mmap;

pub struct BinaryInfo {
pub filename: std::path::PathBuf,
pub symbols: HashMap<String, u64>,
pub bss_addr: u64,
pub bss_size: u64,
pub offset: u64,
pub addr: u64,
pub size: u64,
}

impl BinaryInfo {
#[cfg(unwind)]
#[cfg(feature = "unwind")]
pub fn contains(&self, addr: u64) -> bool {
addr >= self.addr && addr < (self.addr + self.size)
}
}

/// Uses goblin to parse a binary file, returns information on symbols/bss/adjusted offset etc
pub fn parse_binary(filename: &Path, addr: u64, size: u64) -> Result<BinaryInfo, Error> {
pub fn parse_binary(filename: &Path, addr: u64) -> Result<BinaryInfo, Error> {
let offset = addr;

let mut symbols = HashMap::new();
Expand Down Expand Up @@ -79,13 +75,9 @@ pub fn parse_binary(filename: &Path, addr: u64, size: u64) -> Result<BinaryInfo,
}
}
Ok(BinaryInfo {
filename: filename.to_owned(),
symbols,
bss_addr,
bss_size,
offset,
addr,
size,
})
}

Expand Down Expand Up @@ -137,13 +129,9 @@ pub fn parse_binary(filename: &Path, addr: u64, size: u64) -> Result<BinaryInfo,
symbols.insert(name, dynsym.st_value + offset);
}
Ok(BinaryInfo {
filename: filename.to_owned(),
symbols,
bss_addr: bss_header.sh_addr + offset,
bss_size: bss_header.sh_size,
offset,
addr,
size,
})
}
Object::PE(pe) => {
Expand All @@ -169,13 +157,9 @@ pub fn parse_binary(filename: &Path, addr: u64, size: u64) -> Result<BinaryInfo,
let bss_size = u64::from(data_section.virtual_size);

BinaryInfo {
filename: filename.to_owned(),
symbols,
bss_addr,
bss_size,
offset,
addr,
size,
}
})
}
Expand Down
10 changes: 5 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(unwind)]
#[cfg(feature = "unwind")]
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(unwind)]
#[cfg(feature = "unwind")]
let record = record.arg(native.clone());
#[cfg(unwind)]
#[cfg(feature = "unwind")]
let top = top.arg(native.clone());
#[cfg(unwind)]
#[cfg(feature = "unwind")]
let dump = dump.arg(native.clone());

// Nonblocking isn't an option for freebsd, remove
Expand Down Expand Up @@ -429,7 +429,7 @@ impl Config {
.value_of("pid")
.map(|p| p.parse().expect("invalid pid"));
config.full_filenames = matches.occurrences_of("full_filenames") > 0;
if cfg!(unwind) {
if cfg!(feature = "unwind") {
config.native = matches.occurrences_of("native") > 0;
}

Expand Down
5 changes: 2 additions & 3 deletions src/coredump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ impl PythonCoreDump {
.find(|m| m.filename().is_some() & m.is_exec())
.ok_or_else(|| format_err!("Failed to get binary from coredump"))?;
let python_filename = map.filename().unwrap();
let python_binary = parse_binary(python_filename, map.start() as _, map.size() as _);
let python_binary = parse_binary(python_filename, map.start() as _);
info!("Found python binary @ {}", python_filename.display());
(python_filename.to_owned(), python_binary)
};
Expand All @@ -211,8 +211,7 @@ impl PythonCoreDump {
if let Some(libpython) = libmap {
if let Some(filename) = &libpython.filename() {
info!("Found libpython binary @ {}", filename.display());
let parsed =
parse_binary(filename, libpython.start() as u64, libpython.size() as u64)?;
let parsed = parse_binary(filename, libpython.start() as u64)?;
libpython_binary = Some(parsed);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ pub mod binary_parser;
pub mod config;
#[cfg(target_os = "linux")]
pub mod coredump;
#[cfg(unwind)]
#[cfg(feature = "unwind")]
mod cython;
pub mod dump;
#[cfg(unwind)]
#[cfg(feature = "unwind")]
mod native_stack_trace;
mod python_bindings;
mod python_data_access;
Expand Down
4 changes: 2 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ mod config;
mod console_viewer;
#[cfg(target_os = "linux")]
mod coredump;
#[cfg(unwind)]
#[cfg(feature = "unwind")]
mod cython;
mod dump;
mod flamegraph;
#[cfg(unwind)]
#[cfg(feature = "unwind")]
mod native_stack_trace;
mod python_bindings;
mod python_data_access;
Expand Down
12 changes: 4 additions & 8 deletions src/python_process_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl PythonProcessInfo {
let filename = std::path::PathBuf::from(format!("/proc/{}/exe", process.pid));

// TODO: consistent types? u64 -> usize? for map.start etc
let python_binary = parse_binary(&filename, map.start() as u64, map.size() as u64);
let python_binary = parse_binary(&filename, map.start() as u64);

// windows symbols are stored in separate files (.pdb), load
#[cfg(windows)]
Expand Down Expand Up @@ -156,8 +156,7 @@ impl PythonProcessInfo {
));

#[allow(unused_mut)]
let mut parsed =
parse_binary(filename, libpython.start() as u64, libpython.size() as u64)?;
let mut parsed = parse_binary(filename, libpython.start() as u64)?;
#[cfg(windows)]
parsed.symbols.extend(get_windows_python_symbols(
process.pid,
Expand Down Expand Up @@ -203,11 +202,8 @@ impl PythonProcessInfo {
libpython.filename.display()
);

let mut binary = parse_binary(
&libpython.filename,
libpython.segment.vmaddr,
libpython.segment.vmsize,
)?;
let mut binary =
parse_binary(&libpython.filename, libpython.segment.vmaddr)?;

// TODO: bss addr offsets returned from parsing binary are wrong
// (assumes data section isn't split from text section like done here).
Expand Down
Loading

0 comments on commit 5820bf6

Please sign in to comment.