Skip to content

jit(bridge): read the traced frame's vable pointer when patching a retraced loop - #889

Closed
fregataa wants to merge 1 commit into
youknowone:mainfrom
fregataa:fix/bridge-vable-ptr
Closed

jit(bridge): read the traced frame's vable pointer when patching a retraced loop#889
fregataa wants to merge 1 commit into
youknowone:mainfrom
fregataa:fix/bridge-vable-ptr

Conversation

@fregataa

@fregataa fregataa commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

python3 pyre/check.py --backend dynasm failed on synth/str_search_index_bounds with:

rust panic: compile.py:458 assert i == len(inputargs) failed (16 != 26)

A loop compiled from a bridge (trace_and_compile_from_bridge) reads the virtualizable's array lengths through orig_vable_ptr_from_trace_ctx, which preferred the MetaInterp's cached vable_ptr. That cache is clobbered when a residual call re-enters compiled code and deopts nested frames, and the residual-call epilogue only restores TraceCtx::virtualizable_heap_ptr — so the compile-time length read hit a nested callee frame whose locals_cells_stack_w (8 slots) is shorter than the traced frame's (18 slots), tripping the field-load preamble's inputargs accounting (16 = 2 reds + 6 statics + 8 vs the trace's 26 expanded inputargs).

Fix

Prefer the trace-local TraceCtx::virtualizable_heap_ptr in the bridge fallback — RPython's orig_inpargs is the trace's own history, so the trace-local pointer is the parity-correct source — and keep the MetaInterp cache only as a last resort.

Testing

  • synth/str_search_index_bounds completes and its output matches CPython exactly
  • cargo test -p majit-metainterp --release: 1510 passed
  • python3 pyre/check.py --backend dynasm: 342/342 ALL PASSED

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved bridge-trace handling during re-entrant deoptimization.
    • Corrected virtualizable state selection to use the most accurate available runtime pointer, improving stability in affected execution paths.

…traced loop

A loop compiled from a bridge reads the virtualizable's array lengths
through orig_vable_ptr_from_trace_ctx, which preferred the MetaInterp's
cached vable_ptr. That cache is clobbered when a residual call re-enters
compiled code and deopts nested frames, and the residual-call epilogue
only restores TraceCtx::virtualizable_heap_ptr — so the compile-time
length read hit a nested callee frame whose locals_cells_stack_w is
shorter than the traced frame's, tripping the field-load preamble's
`assert i == len(inputargs)` (16 != 26) on
synth/str_search_index_bounds under the dynasm backend.

Prefer the trace-local TraceCtx pointer (RPython's orig_inpargs is the
trace's own history) and keep the MetaInterp cache as last resort.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 27607024-9374-45f3-9b7f-3ff9b3a52d66

📥 Commits

Reviewing files that changed from the base of the PR and between 83fe239 and 00cbd33.

📒 Files selected for processing (1)
  • majit/majit-metainterp/src/pyjitpl.rs

Walkthrough

Bridge-trace virtualizable heap pointer selection now checks the context-provided pointer first, ignoring null values, and falls back to self.vable_ptr when unavailable.

Changes

Bridge pointer selection

Layer / File(s) Summary
Update bridge pointer fallback
majit/majit-metainterp/src/pyjitpl.rs
Bridge tracing prioritizes a non-null ctx.virtualizable_heap_ptr() and otherwise returns self.vable_ptr; comments describe the updated pointer sources during re-entrant deoptimization.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • youknowone/pyre#501: Reworks virtualizable identity and resume decoding used during deoptimization.
  • youknowone/pyre#834: Updates virtualizable identity, box, and bridge pointer initialization logic.

Suggested reviewers: youknowone

Poem

A bunny hops through bridge-trace air,
Checking context pointers with care.
If nullness blocks the way,
The frame pointer saves the day—
Deoptimization finds its hare!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main fix: preferring the trace-local vable pointer when patching a retraced bridge loop.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@fregataa

Copy link
Copy Markdown
Contributor Author

Closed since #876 has already resolved the same issue

@fregataa fregataa closed this Jul 30, 2026
@youknowone

Copy link
Copy Markdown
Owner

sorry! I missed this patch

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.

2 participants