fix(ptrace): handle duplicated syscall exit stops with restarted sysc…#296
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe tracer now reads fresh syscall metadata to distinguish entry and exit stops, including seccomp stops, and adds test-only support for attaching to an existing tracee. A regression test covers ptrace restart behavior during ChangesPtrace syscall-stop handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Test
participant Tracer
participant TracerInner
participant Tracee
participant ptrace_kernel
Test->>Tracee: launch nanosleep helper
Tracee->>Test: publish tracee TID
Test->>Tracer: attach_for_test(pid)
Tracer->>TracerInner: run_attached(pid)
TracerInner->>ptrace_kernel: seize tracee
ptrace_kernel-->>TracerInner: initial syscall stop
TracerInner->>ptrace_kernel: read syscall_info
ptrace_kernel-->>TracerInner: entry or exit metadata
TracerInner->>Tracee: process syscall stop
Tracee-->>Test: successful exit event
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the reliability of the ptrace tracer when handling restarted syscalls. Instead of relying on a local boolean state (presyscall) to track whether a syscall stop is an entry or exit, the tracer now queries the kernel-reported stop kind authoritatively using SyscallInfo::is_entry(). Additionally, a regression test has been added to verify that the tracer correctly handles duplicated syscall exit stops after attaching to a blocked process. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/tracexec-backend-ptrace/src/ptrace/tracer/test.rs`:
- Around line 607-617: Replace the fixed delay in the ready_thread closure with
synchronization that waits until tracee_tid is confirmed blocked in nanosleep,
then write its TID to ready_path. Ensure the readiness signal is emitted only
after the target syscall is active, while preserving the existing nanosleep
assertion and ptrace-restart test flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: e57bb21f-94ee-4922-81dd-250f5f4d90ae
📒 Files selected for processing (4)
crates/tracexec-backend-ptrace/src/ptrace/syscall.rscrates/tracexec-backend-ptrace/src/ptrace/tracer.rscrates/tracexec-backend-ptrace/src/ptrace/tracer/inner.rscrates/tracexec-backend-ptrace/src/ptrace/tracer/test.rs
| let ready_thread = std::thread::spawn(move || { | ||
| std::thread::sleep(Duration::from_millis(20)); | ||
| std::fs::write(ready_path, tracee_tid.as_raw().to_string()).unwrap(); | ||
| }); | ||
|
|
||
| let request = nix::libc::timespec { | ||
| tv_sec: 0, | ||
| tv_nsec: 500_000_000, | ||
| }; | ||
| let result = unsafe { nix::libc::nanosleep(&request, std::ptr::null_mut()) }; | ||
| assert_eq!(result, 0, "nanosleep should complete after ptrace restart"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Make readiness prove that nanosleep is active.
The fixed 20 ms delay does not establish this contract. Under scheduling delays, the 500 ms sleep can finish before the TID is published, allowing the regression to pass without exercising restart handling—or fail intermittently. Publish readiness only after confirming the target thread is blocked in the intended syscall.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/tracexec-backend-ptrace/src/ptrace/tracer/test.rs` around lines 607 -
617, Replace the fixed delay in the ready_thread closure with synchronization
that waits until tracee_tid is confirmed blocked in nanosleep, then write its
TID to ready_path. Ensure the readiness signal is emitted only after the target
syscall is active, while preserving the existing nanosleep assertion and
ptrace-restart test flow.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #296 +/- ##
==========================================
+ Coverage 82.22% 82.25% +0.03%
==========================================
Files 84 84
Lines 20960 21026 +66
==========================================
+ Hits 17234 17295 +61
- Misses 3726 3731 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
…alls
Summary by CodeRabbit
Bug Fixes
Tests