Skip to content

jit: guard-snapshot capture side channels TLS → explicit GuardCaptureScope parameter#515

Merged
youknowone merged 1 commit into
mainfrom
nbody
Jul 13, 2026
Merged

jit: guard-snapshot capture side channels TLS → explicit GuardCaptureScope parameter#515
youknowone merged 1 commit into
mainfrom
nbody

Conversation

@youknowone

Copy link
Copy Markdown
Owner

Follow-up to #500/#509 (FBW walker per-walk TLS → explicit ownership). This slice absorbs the three remaining call-scoped TLS side channels in jitcode_dispatch.rs, which were set immediately before exactly one walker_capture_snapshot_for_last_guard call and cleared right after:

Change

They become fields of a GuardCaptureScope<'_> parameter — the explicit-parameter shape capture_resumedata's keyword arguments (pyjitpl.py:2599-2603) already use for after_residual_call, which pyre threads explicitly today.

  • The ~42 ordinary capture call sites keep the two-argument walker_capture_snapshot_for_last_guard(ctx, pc) signature, forwarding the default (empty) scope — identical to the empty TLS those captures observed.
  • The two writer sites call a new _scoped variant; the TLS set/clear brackets (and their unwind hazard) are deleted.
  • The scope threads through walker_capture_snapshot_for_last_guard_impl and walker_capture_multi_frame_inline_snapshot; collect_outer_active_boxes takes the recovery pairs as a slice parameter. Its five callers outside the capture path pass &[], matching the empty TLS they previously observed.
  • Option<usize> replaces the usize::MAX sentinel for the guard pc.

No behavior change intended; every read observes exactly the value the TLS version observed on the same path.

Verification

  • cargo test -p pyre-jit-trace --release --no-default-features --features dynasm: 282 passed / 0 failed
  • python3 pyre/check.py: 162/162 on dynasm, cranelift, wasm
  • rg "FBW_FORITER_NEXT_CATCH_RESUME|BRANCH_GUARD_JITCODE_PC|BRANCH_GUARD_KEPT_RECOVERED": zero hits

🤖 Generated with Claude Code

…Scope parameter

Replace the three call-scoped thread-locals bracketed around
walker_capture_snapshot_for_last_guard (FBW_FORITER_NEXT_CATCH_RESUME,
BRANCH_GUARD_JITCODE_PC, BRANCH_GUARD_KEPT_RECOVERED) with a
GuardCaptureScope parameter threaded through the capture impl,
walker_capture_multi_frame_inline_snapshot, and
collect_outer_active_boxes — the explicit-parameter shape
capture_resumedata's keyword arguments (pyjitpl.py:2599-2603) already
use for after_residual_call.

The two writer sites (FOR_ITER-next residual GUARD_NO_EXCEPTION,
kept-stack branch guard) call a new _scoped variant; their TLS
set/clear brackets are deleted. The ordinary capture call sites keep
the two-argument signature and forward the default (empty) scope.
collect_outer_active_boxes callers outside the capture path pass an
empty slice, matching the empty TLS they previously observed.

Assisted-by: Claude
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@youknowone, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 45 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: cb9e8365-f967-4fbc-b004-3b9240a124e1

📥 Commits

Reviewing files that changed from the base of the PR and between cef60cd and 9588e8e.

📒 Files selected for processing (1)
  • pyre/pyre-jit-trace/src/jitcode_dispatch.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch nbody

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

🤖 Codex parity review

Static analysis of this diff vs the local RPython/PyPy sources (commit 303ebc1).

1. Regressions to PyPy parity introduced by this patch

None.

2. Other mismatches introduced by this patch

None.

3. Pre-existing mismatches (already present before this patch)

  • pyre/pyre-jit-trace/src/jitcode_dispatch.rs:10467 ↔ rpython/jit/metainterp/pyjitpl.py:2610 and rpython/jit/metainterp/resume.py:1049: Pyre explicitly says helper-walker guards “must resume to the outer Python opcode boundary” and collapses the frame stack; PyPy captures self.framestack and rebuilds one MIFrame per encoded JitCode frame. This is the prohibited frame-identity collapse and can conflate caller/callee globals, locals, and pycode.

  • pyre/pyre-jit-trace/src/jitcode_dispatch.rs:6985 ↔ rpython/jit/metainterp/pyjitpl.py:215: Pyre builds HashMap<u32, OpRef> for recovered branch values, whereas PyPy’s _get_list_of_active_boxes directly reads each live self.registers_r[index]. This pre-existing side table diverges from the per-frame/register representation and violates the repository’s RPython container-parity rule.

4. Structural adaptations

  • pyre/pyre-jit-trace/src/jitcode_dispatch.rs:7191 ↔ rpython/jit/metainterp/pyjitpl.py:2599: GuardCaptureScope replaces Rust thread-local transport with explicit call-scoped inputs. PyPy needs only resumepc and after_residual_call; Pyre additionally carries compiled-edge recovery metadata. The patch improves isolation: the old FOR_ITER flag could leak after an error, while the scoped value cannot.

  • pyre/pyre-jit-trace/src/jitcode_dispatch.rs:14967 ↔ rpython/jit/metainterp/pyjitpl.py:1941: Pyre’s foriter_next_catch_resume selects a CPython-compiled FOR_ITER post-call catch location. PyPy’s generic residual-call execution/capture path has no corresponding per-FOR_ITER marker; this is an opcode/compiler-layout adaptation.

@youknowone youknowone merged commit 48ce5fb into main Jul 13, 2026
30 checks passed
@youknowone youknowone deleted the nbody branch July 13, 2026 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant