Fix synthetic JIT parity failures and improve CI checks - #70
Conversation
Bring the synthetic parity suite back to green without filename-based gating. Normalize canraise exit ordering before flattening so the graph shape matches the RPython guessexception layout, and tolerate pyre-local non-payload canraise links by flattening them as ordinary links. Handle backend GC ownership queries during root walking reentrantly for dynasm and cranelift. RPython's GC descriptor can answer ownership checks while collection is active; pyre's RefCell-backed active GC needs a raw immutable ownership-query escape hatch for that structural adaptation. Limit unsupported bytecode fallback by structural shape. WITH_EXCEPT_START remains a structural-region interpreter fallback because codewriter still lowers it through abort_permanent. FOR_ITER is only a current-frame fallback, so module-level driver loops do not suppress JIT in hot callees such as fannkuch. Adjust the two synthetic workloads that exceeded the default synthetic timeout after correctness was restored, keeping the same feature coverage. Verification: python3 pyre/check.py --synthetic-only passes dynasm 25/25 and cranelift 25/25. python3 pyre/check.py --backend dynasm passes 14/14, with fannkuch at cpython 1.21s and dynasm 0.76s.
Make pyre/check.py include the synthetic feature-parity suite in the default pre-merge run, so CI picks it up without changing the workflow invocation. Remove the opt-in --synthetic flag and add --no-synthetic for local opt-out runs. Keep --synthetic-only for focused synthetic runs and reject the contradictory --synthetic-only --no-synthetic combination. Disable argparse option abbreviation so the removed --synthetic flag does not continue to resolve ambiguously as a prefix of the remaining synthetic options. Verification: python3 pyre/check.py --help; python3 pyre/check.py --synthetic --no-synthetic errors as unrecognized; python3 pyre/check.py --synthetic-only --no-synthetic errors with the explicit conflict message.
WalkthroughThis PR adds raw-pointer GC allocator mirrors and fallback ownership checks to two JIT backends, introduces JIT suppression for unsupported bytecode shapes, refines JIT codegen liveness and exception-edge ordering, and reduces several synthetic benchmark iteration counts. ChangesGC Reentrant Access Fallback
JIT Unsupported Bytecode Suppression
JIT Code Generation Safety
Benchmark Harness and Constants
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@majit/majit-backend-cranelift/src/compiler.rs`:
- Around line 1358-1380: The current raw-pointer fallback in
CRANELIFT_ACTIVE_GC_RAW creates an aliased Rust reference (&*ptr) while a
RefCell borrow exists, causing UB; instead, avoid constructing a shared Rust
reference across borrow scopes by moving the heap-membership query to
separately-owned interior-mutable state or a synchronized accessor. Replace the
deref in the try_borrow_mut() Err branch so it does not produce &*ptr: either
read a dedicated Cell/Atomic/Cell<bool> (e.g., introduce
CRANELIFT_ACTIVE_GC_IS_MANAGED and check that for addr membership) or expose a
synchronized API on the allocator (e.g., wrap the allocator in a Mutex/RwLock
and call is_managed_heap_object via a locked accessor), and update uses of
CRANELIFT_ACTIVE_GC_RAW and the is_managed_heap_object call to use that safe
path rather than constructing a Rust reference from the raw pointer.
- Around line 1186-1190: The code replaces the boxed allocator in
CRANELIFT_ACTIVE_GC then updates CRANELIFT_ACTIVE_GC_RAW, which can drop the old
allocator and leave CRANELIFT_ACTIVE_GC_RAW briefly pointing at freed memory;
instead, compute the new raw pointer from the new allocator value and set
CRANELIFT_ACTIVE_GC_RAW before replacing/dropping the boxed GC. Specifically,
inside the CRANELIFT_ACTIVE_GC.with closure compute raw =
guard.as_deref_mut().map(|gc| gc as *mut dyn GcAllocator) (or the equivalent
from the incoming gc value), call CRANELIFT_ACTIVE_GC_RAW.with(|raw_cell|
raw_cell.set(raw)) first, and only then assign *guard = gc (or perform the boxed
swap), so gc_owns_object_via_active_runtime never observes a stale pointer.
In `@majit/majit-backend-dynasm/src/runner.rs`:
- Around line 1057-1064: Add a public clearing helper that clears both
thread-locals to avoid a dangling raw pointer: implement a function (e.g., pub
fn clear_gc_allocator()) which sets DYNASM_ACTIVE_GC to None and then sets
DYNASM_ACTIVE_GC_RAW to None; update docs to instruct callers to call
clear_gc_allocator() instead of setting DYNASM_ACTIVE_GC directly (or
alternatively make DYNASM_ACTIVE_GC non-pub and expose only the setter/clearer)
so the invariant between DYNASM_ACTIVE_GC and DYNASM_ACTIVE_GC_RAW is preserved.
In `@pyre/pyre-jit/src/eval.rs`:
- Around line 2257-2264: The structural-region suppression guard is only
installed in eval_with_jit_inner's match on unsupported_jit_shape, but
recursive/portal paths (portal_runner_result / portal_runner_dispatch) can start
executing frames later without that TLS guard, allowing nested helpers to enter
the JIT; locate the code paths that call portal_runner_result and
portal_runner_dispatch (the recursive portal runner code later in this file that
can be reached instead of eval_with_jit_inner) and wrap those execution calls in
a JitSuppressionGuard when UnsupportedJitShape::StructuralRegion applies (mirror
the pattern used in eval_with_jit_inner), ensuring the guard is created before
entering portal_runner_result/portal_runner_dispatch so nested frames see the
suppression (also reference WITH_EXCEPT_START where relevant to ensure frames
reached via that sentinel are covered).
In `@pyre/pyre-jit/src/jit/codewriter.rs`:
- Around line 2101-2147: Add a unit test that constructs a BlockRef and
populates block.exits with the problematic shape (first a normal link, then an
exception link, then a duplicate normal fallthrough with no extravars), call
restore_canraise_exit_order(&block) and assert the resulting block.exits has
been pruned/ordered to exactly [normal, exception] (check both length and that
the first exit has no exception fields and the second has
last_exception/last_exc_value set); place the test alongside other tests for
codewriter/flow behavior and reference restore_canraise_exit_order, BlockRef,
and Link fields (last_exception/last_exc_value/extravars) so the regression is
pinned.
In `@pyre/pyre-jit/src/jit/flatten.rs`:
- Around line 1848-1868: The current branch treats any missing exception payload
as a normal link, hiding mixed/partial states; change the condition so only the
case where both last_exception and last_exc_value are None is flattened via
self.make_link(link, false), while the mixed case (one Some and the other None)
should fail loudly (e.g., panic or return an Err) with a clear diagnostic
referencing the link and which field is missing; keep the existing path that
uses make_exception_link when both are Some. Ensure you update the check around
last_exception / last_exc_value and calls to self.make_link /
make_exception_link accordingly.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 80785aac-b9f3-48fd-aad7-ce74f31dcfbb
📒 Files selected for processing (8)
majit/majit-backend-cranelift/src/compiler.rsmajit/majit-backend-dynasm/src/runner.rspyre/bench/synth/comprehensions.pypyre/bench/synth/context_manager.pypyre/check.pypyre/pyre-jit/src/eval.rspyre/pyre-jit/src/jit/codewriter.rspyre/pyre-jit/src/jit/flatten.rs
| fn unsupported_jit_shape(code: &pyre_interpreter::CodeObject) -> UnsupportedJitShape { | ||
| // Structural adaptation: RPython/PyPy traces these bytecodes with | ||
| // fully translated support. Pyre's codewriter still lowers | ||
| // `WITH_EXCEPT_START` through a pyre-local `abort_permanent` | ||
| // path. A frame containing this unsupported shape must run in the | ||
| // interpreter. While that frame is active, nested helper calls are | ||
| // also kept out of the JIT by `JitSuppressionGuard`; this mirrors | ||
| // the structural unsupported region instead of keying on a | ||
| // benchmark filename. | ||
| // | ||
| // `FOR_ITER` is narrower: pyre currently emits `abort_permanent` | ||
| // for iterator protocol opcodes in codewriter.rs, so the current | ||
| // code object must run in the interpreter to preserve the Python | ||
| // loop result. Unlike `WITH_EXCEPT_START`, this is not a structural | ||
| // region boundary; callees are allowed to enter the JIT. This keeps | ||
| // module-level driver loops such as fannkuch's `for range(3, 10)` | ||
| // from disabling the hot function they call. | ||
| let mut arg_state = pyre_interpreter::OpArgState::default(); | ||
| let mut has_for_iter = false; | ||
| for unit in code.instructions.iter().copied() { | ||
| match arg_state.get(unit).0 { | ||
| pyre_interpreter::Instruction::WithExceptStart => { | ||
| return UnsupportedJitShape::StructuralRegion; | ||
| } | ||
| pyre_interpreter::Instruction::ForIter { .. } => has_for_iter = true, | ||
| _ => {} | ||
| } | ||
| } | ||
| if has_for_iter { | ||
| UnsupportedJitShape::CurrentFrameOnly | ||
| } else { | ||
| UnsupportedJitShape::None | ||
| } | ||
| } |
There was a problem hiding this comment.
Cache unsupported-shape classification instead of rescanning the whole bytecode.
unsupported_jit_shape is now queried from try_function_entry_jit, maybe_compile_and_run, and jit_merge_point_hook; the last one runs at every merge point while tracing. Re-walking code.instructions for supported frames turns these hot probes into O(code_len) work and can easily dominate tracing time on larger functions. Please compute this once per CodeObject and reuse it.
| fn restore_canraise_exit_order(block: &super::flow::BlockRef) { | ||
| let mut block_mut = block.borrow_mut(); | ||
| if block_mut.exits.len() < 2 { | ||
| return; | ||
| } | ||
|
|
||
| let first_normal = block_mut.exits.iter().position(|link| { | ||
| let link = link.borrow(); | ||
| link.exitcase.is_none() | ||
| && link.llexitcase.is_none() | ||
| && link.last_exception.is_none() | ||
| && link.last_exc_value.is_none() | ||
| }); | ||
| let Some(first_normal) = first_normal else { | ||
| return; | ||
| }; | ||
|
|
||
| let mut ordered = Vec::with_capacity(block_mut.exits.len()); | ||
| ordered.push(block_mut.exits[first_normal].clone()); | ||
| for (index, link) in block_mut.exits.iter().enumerate() { | ||
| if index == first_normal { | ||
| continue; | ||
| } | ||
| let is_exception_edge = { | ||
| let link = link.borrow(); | ||
| link.last_exception.is_some() || link.last_exc_value.is_some() | ||
| }; | ||
| if is_exception_edge { | ||
| ordered.push(link.clone()); | ||
| } | ||
| } | ||
|
|
||
| // Structural adaptation for pyre's PC-sequential walker: | ||
| // RPython `flowcontext.py:130-156 guessexception` closes a | ||
| // canraise block with exactly one normal edge followed by | ||
| // exception edges. Pyre may transiently append an extra normal | ||
| // fallthrough while forcing the next-PC boundary after | ||
| // `emit_catch_exception!`. That duplicate has no | ||
| // `Link.extravars`, so `flatten.py:223-238` would treat it as an | ||
| // exception link and trip `make_exception_link`'s | ||
| // `last_exception` assertion. Keep the upstream shape at the | ||
| // graph boundary: first normal edge, then only seeded exception | ||
| // edges. | ||
| if ordered.len() >= 2 { | ||
| block_mut.exits = ordered; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add a regression test for the exit-pruning behavior.
This helper now does more than reorder: it also drops extra non-exception exits. A small unit test that pins a shape like [normal, exception, duplicate_normal] -> [normal, exception] would make this change much safer to evolve.
🤖 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 `@pyre/pyre-jit/src/jit/codewriter.rs` around lines 2101 - 2147, Add a unit
test that constructs a BlockRef and populates block.exits with the problematic
shape (first a normal link, then an exception link, then a duplicate normal
fallthrough with no extravars), call restore_canraise_exit_order(&block) and
assert the resulting block.exits has been pruned/ordered to exactly [normal,
exception] (check both length and that the first exit has no exception fields
and the second has last_exception/last_exc_value set); place the test alongside
other tests for codewriter/flow behavior and reference
restore_canraise_exit_order, BlockRef, and Link fields
(last_exception/last_exc_value/extravars) so the regression is pinned.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c74062a9ae
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return CRANELIFT_ACTIVE_GC_RAW.with(|raw| match raw.get() { | ||
| Some(ptr) => unsafe { (&*ptr).is_managed_heap_object(addr) }, | ||
| None => false, |
There was a problem hiding this comment.
Avoid aliasing active GC via raw-pointer fallback
This fallback runs exactly when CRANELIFT_ACTIVE_GC.try_borrow_mut() failed (the reentrant path where a mutable borrow is already live), but unsafe { (&*ptr).is_managed_heap_object(addr) } creates a shared reference to that same allocator anyway; that aliases an outstanding &mut and is undefined behavior under Rust’s aliasing rules, so GC ownership checks can miscompile or crash in optimized builds during reentrant collection/root-walk scenarios. Please avoid materializing a new &dyn GcAllocator in this branch (and mirror the fix in the equivalent dynasm fallback).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b829fa3790
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ssarepr.insns[insn_idx].is_live() { | ||
| if live_idx.is_none() { | ||
| live_idx = Some(insn_idx); | ||
| } | ||
| live_idx = Some(insn_idx); |
There was a problem hiding this comment.
Preserve first live marker for duplicate PC anchors
When the walker emits duplicate PcAnchors for the same py_pc before the next distinct PC (the supersede/sibling-joinpoint case documented in pc_anchor_positions), those later duplicate anchors are not included as scan boundaries because positions keeps only the first anchor. With this last-wins assignment, the live table for the runtime pcN label can now be taken from the duplicate block even though dispatch still jumps to the first anchor, so guard-failure/blackhole resume can read a liveness set for code it is not resuming at. The new jit-merge-point case needs to distinguish the second live within the same canonical block without letting ignored duplicate anchors override the first block's liveness.
Useful? React with 👍 / 👎.
…nly (slice D of Send/Sync epic)
After slices B (_nr -> Arc<AtomicI64>) and C (concretetype ->
Arc<RwLock<…>>), `Variable`'s only non-Sync field is `annotation`.
Replace the blanket "all cells are !Send + !Sync" preamble with an
audit-trail comment that:
- Names the single remaining field that still relies on the
`unsafe impl` (`annotation`).
- Enumerates the nine `Rc<…>` payloads inside `SomeValue` that
block its `Send + Sync` (`DictDefInner`, `ListDefInner`, nested
`SomeValue`, and the six `RefCell<…>`-wrapped `Desc` family
members) so a future reader can see the precise blast radius.
- Cross-links task #70 / `project_variable_sync_p1_full_scope.md`
so the cascade rationale stays discoverable.
No behavioural change. The `unsafe impl Send`/`Sync` stays in place
because the SomeValue cascade is still pending; this slice only
updates the documentation to match the post-B/C reality.
- cranelift set_cranelift_active_gc: publish new raw pointer before dropping the previous allocator so a reentrant gc_owns_object_via_active_runtime cannot read freed memory during drop(old). - cranelift CraneliftBackend::drop: clear CRANELIFT_ACTIVE_GC_RAW before the boxed allocator for the same reason. - dynasm: same publish-before-drop reordering in set_gc_allocator, plus a clear_gc_allocator helper that clears DYNASM_ACTIVE_GC_RAW and DYNASM_ACTIVE_GC together so external callers cannot leave the raw mirror dangling. - portal_runner_result: install JitSuppressionGuard when the recursive portal frame's code matches UnsupportedJitShape::StructuralRegion, mirroring eval_with_jit_inner so nested helpers reached via bhimpl_recursive_call_* paths also see the TLS suppression. - flatten.rs canraise catch loop: match payload_shape and panic on partially-seeded links (only one of last_exception / last_exc_value set) instead of silently downgrading to make_link.
b829fa3 to
c6a7375
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6a7375003
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let raw = next.as_deref_mut().map(|gc| gc as *mut dyn GcAllocator); | ||
| CRANELIFT_ACTIVE_GC_RAW.with(|raw_cell| raw_cell.set(raw)); | ||
| let old = std::mem::replace(&mut *guard, next); |
There was a problem hiding this comment.
Keep old GC published while dropping it
When replacing an active allocator, this publishes the raw mirror for the new allocator before drop(old), even though the comment notes that drop(old) can reenter ownership checks via root walkers. In that reentrant window the RefCell borrow is still held, so gc_owns_object_via_active_runtime uses CRANELIFT_ACTIVE_GC_RAW and queries the new allocator for addresses that belong to the old heap, causing old managed objects to be reported as unmanaged during teardown/replacement. Keep the old allocator available to the fallback until its drop completes, and apply the same ordering fix in the dynasm setter.
Useful? React with 👍 / 👎.
|
A. Aliasing UB (CodeRabbit 2 / Codex 1) — cranelift compiler.rs:1372, dynasm Core problem: The try_borrow_mut() Err branch fires precisely when a &mut borrow Realistic options (smallest → largest):
When to fix: Immediately if Miri (cargo +nightly miri test) flags it. Otherwise B. unsupported_jit_shape O(n) rescan (CodeRabbit 5) — eval.rs:2201 Core problem: The function is queried from try_function_entry_jit, Options:
When to fix: Only when profiling shows unsupported_jit_shape is hot. Small/medium C. restore_canraise_exit_order regression test (CodeRabbit 7, nitpick) — Core problem: The helper now both reorders and prunes exits, but there's no test
When to fix: Fold into the next PR that touches codewriter. Cost is trivial — no |
Reverts the publish-before-drop ordering. While the drop body is running, the raw mirror must keep pointing at the old allocator so reentrant is_managed_heap_object queries still resolve old-heap addresses; routing them to the new allocator would report old-heap objects as unmanaged. After the drop returns no further reentry is possible on the same thread before the raw mirror is republished synchronously. Applied to cranelift set_cranelift_active_gc, CraneliftBackend::drop, dynasm set_gc_allocator, and dynasm clear_gc_allocator.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22b71165b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| UnsupportedJitShape::StructuralRegion => { | ||
| let _guard = JitSuppressionGuard::new(); | ||
| return frame.execute_frame(None, None); |
There was a problem hiding this comment.
Limit suppression to actual unsupported region
When a code object merely contains WITH_EXCEPT_START, unsupported_jit_shape scans the whole bytecode and this guard stays installed for the entire execute_frame call. Any nested Python call made while that wrapper/module frame is active then hits jit_suppressed_by_unsupported_frame() in try_function_entry_jit/maybe_compile_and_run and can never warm up, even if the callee has no unsupported bytecode and is called after an unrelated with block. Please scope this to the current frame or the dynamic unsupported region rather than suppressing all callees for the full lifetime of any frame containing a with.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyre/pyre-jit/src/eval.rs (1)
2242-2264:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRegister the lazy eval hooks before these interpreter fallbacks.
These early returns now run before the lazy
register_eval_override/register_set_jit_param_hooksafety net. If the first frame after startup hitsUnsupportedJitShape::CurrentFrameOnly, it falls back toexecute_frame(None, None)with no override installed, so helper calls inside that frame never get a chance to enter the JIT even though this mode is supposed to suppress only the current frame.Suggested fix
fn eval_with_jit_inner(frame: &mut PyFrame) -> PyResult { // PYRE_JIT=0 disables JIT entirely, falling back to plain interpreter. static PYRE_JIT_DISABLED: std::sync::OnceLock<bool> = std::sync::OnceLock::new(); if *PYRE_JIT_DISABLED.get_or_init(|| std::env::var("PYRE_JIT").as_deref() == Ok("0")) { return frame.execute_frame(None, None); } + pyre_interpreter::call::register_eval_override(eval_with_jit); + pyre_interpreter::call::register_set_jit_param_hook(set_jit_param_via_warmstate); if jit_suppressed_by_unsupported_frame() { return frame.execute_frame(None, None); } let code = unsafe { &*pyre_interpreter::pyframe_get_pycode(frame) }; - pyre_interpreter::call::register_eval_override(eval_with_jit); - pyre_interpreter::call::register_set_jit_param_hook(set_jit_param_via_warmstate);🤖 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 `@pyre/pyre-jit/src/eval.rs` around lines 2242 - 2264, Move the lazy eval hooks registration earlier so the safety net is always installed before any early fallbacks: call pyre_interpreter::call::register_eval_override(eval_with_jit) and pyre_interpreter::call::register_set_jit_param_hook(set_jit_param_via_warmstate) (and related init like install_jit_call_bridge()/init_callbacks()/cranelift registrations) before checking jit_suppressed_by_unsupported_frame() and before matching unsupported_jit_shape(code); this ensures that when unsupported_jit_shape returns CurrentFrameOnly or code paths use JitSuppressionGuard and return frame.execute_frame(None, None), the eval override and param hook are already registered and helper calls inside that frame can still enter the JIT.
♻️ Duplicate comments (1)
majit/majit-backend-cranelift/src/compiler.rs (1)
1369-1384:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftAvoid materializing
&dyn GcAllocatorfrom the raw mirror while theRefCellborrow is live.The
Err(_)path still does(&*ptr).is_managed_heap_object(addr). That creates a shared reference to the same allocator while the originalborrow_mut()owner is still live, which is aliasing UB in Rust even for a read-only call. Keep the fallback on separately owned snapshot state (for example cached heap extents) or remove the reentrant borrow window instead of dereferencing the trait-object pointer here.In Rust, does creating `&*ptr` from a `*mut dyn Trait` and calling a read-only method remain undefined behavior if the same value is currently protected by an active `RefCell::borrow_mut()` / `RefMut` borrow elsewhere?🤖 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 `@majit/majit-backend-cranelift/src/compiler.rs` around lines 1369 - 1384, The Err(_) branch currently dereferences CRANELIFT_ACTIVE_GC_RAW (creating &*ptr) while a RefCell borrow_mut() owner may still be live, causing aliasing UB; fix by avoiding materializing a shared reference to the trait object while the RefMut exists — for example, snapshot the minimal immutable state needed (e.g., cached heap extents or a small copy) from the allocator into separately owned data and query that (instead of calling (&*ptr).is_managed_heap_object), or else ensure the RefMut `guard` is dropped before accessing CRANELIFT_ACTIVE_GC_RAW so no simultaneous borrow exists; update the code paths around cell.try_borrow_mut(), CRANELIFT_ACTIVE_GC_RAW, and is_managed_heap_object accordingly.
🤖 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 `@pyre/pyre-jit/src/jit/flatten.rs`:
- Around line 1848-1878: Add unit tests that exercise the payload_shape triage
in the flattening logic: (1) a test that constructs a Link with both
last_exception and last_exc_value set to None so the code path that calls
self.make_link(link, false) is taken and assert that the fallback
(non-exception) path executes (e.g., by observing that make_link was invoked or
that the resulting flattened graph contains the expected normal exit), and (2) a
test that constructs a Link with exactly one of last_exception or last_exc_value
set (the mixed case) and assert that the code panics with the message containing
"canraise catch link payload partially seeded" (or use should_panic to catch the
panic). Locate the triage around payload_shape in flatten.rs (the match on
payload_shape, and the calls to self.make_link and make_exception_link) to
create and feed appropriate Link instances into the flattening function so each
branch is deterministically covered; name tests to reflect the cases (e.g.,
payload_shape_all_absent_fallback and payload_shape_mixed_panic).
---
Outside diff comments:
In `@pyre/pyre-jit/src/eval.rs`:
- Around line 2242-2264: Move the lazy eval hooks registration earlier so the
safety net is always installed before any early fallbacks: call
pyre_interpreter::call::register_eval_override(eval_with_jit) and
pyre_interpreter::call::register_set_jit_param_hook(set_jit_param_via_warmstate)
(and related init like install_jit_call_bridge()/init_callbacks()/cranelift
registrations) before checking jit_suppressed_by_unsupported_frame() and before
matching unsupported_jit_shape(code); this ensures that when
unsupported_jit_shape returns CurrentFrameOnly or code paths use
JitSuppressionGuard and return frame.execute_frame(None, None), the eval
override and param hook are already registered and helper calls inside that
frame can still enter the JIT.
---
Duplicate comments:
In `@majit/majit-backend-cranelift/src/compiler.rs`:
- Around line 1369-1384: The Err(_) branch currently dereferences
CRANELIFT_ACTIVE_GC_RAW (creating &*ptr) while a RefCell borrow_mut() owner may
still be live, causing aliasing UB; fix by avoiding materializing a shared
reference to the trait object while the RefMut exists — for example, snapshot
the minimal immutable state needed (e.g., cached heap extents or a small copy)
from the allocator into separately owned data and query that (instead of calling
(&*ptr).is_managed_heap_object), or else ensure the RefMut `guard` is dropped
before accessing CRANELIFT_ACTIVE_GC_RAW so no simultaneous borrow exists;
update the code paths around cell.try_borrow_mut(), CRANELIFT_ACTIVE_GC_RAW, and
is_managed_heap_object accordingly.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 945d2ed7-5329-4ce9-87f5-22544aa34344
📒 Files selected for processing (6)
majit/majit-backend-cranelift/src/compiler.rsmajit/majit-backend-dynasm/src/runner.rspyre/bench/synth/context_manager.pypyre/bench/synth/iteration_protocol.pypyre/pyre-jit/src/eval.rspyre/pyre-jit/src/jit/flatten.rs
| let guard = match cell.try_borrow() { | ||
| Ok(guard) => guard, | ||
| Err(_) => { | ||
| // Structural adaptation: RPython's GC descriptor is a | ||
| // normal object reference and `gc_current_object_address` | ||
| // can query ownership while a collection is already in | ||
| // progress. Pyre stores the active dynasm GC behind a | ||
| // Rust `RefCell`; during `alloc_nursery` the mutable | ||
| // borrow is held while extra-root walkers may ask whether | ||
| // a mapdict key is GC-managed. Use the raw pointer only | ||
| // for this immutable ownership query, matching the | ||
| // read-only nature of RPython's descriptor call, instead | ||
| // of panicking across the extern "C" slowpath. | ||
| return DYNASM_ACTIVE_GC_RAW.with(|raw| match raw.get() { | ||
| Some(ptr) => unsafe { (&*ptr).is_managed_heap_object(addr) }, | ||
| None => false, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Rust, is unsafe { (&*ptr).method() }undefined behavior ifptraliases a value currently mutably borrowed viaRefCellor&mut, even when method() is read-only? Please cite stacked-borrows or Miri guidance.
💡 Result:
Yes—under Miri’s aliasing models (Stacked Borrows / Tree Borrows), using unsafe { (&*ptr).method() } can be Undefined Behavior if ptr aliases memory that is still protected by an active mutable borrow derived from RefCell (e.g., a RefMut) or from &mut, even if method() only performs reads. Reason (Stacked Borrows): In Stacked Borrows, references carry permission “tags” on a borrow stack; later borrows can invalidate earlier ones for certain kinds of access. If a later &mut-derived borrow supersedes earlier access, any dereference through the old reference/raw pointer that requires those permissions is forbidden and can be UB. Dmitriev’s Miri walkthrough explicitly states that creating a new &mut invalidates earlier mutable borrows (including raw pointers derived from them), and that later reads/writes through that invalidated pointer are UB, even when the only access is through the pointer itself (i.e., “unsafe pointer dereference” is checked against the current stack state) [1]. How this relates to RefCell: RefCell::borrow_mut returns a RefMut guard whose dynamic borrow is active until the RefMut (and derived RefMuts) is dropped [2]. While that guard is alive, the interior is considered mutably borrowed by Rust’s model of aliasing; accesses that violate the “don’t alias an active mutable borrow” rule are treated as UB by Miri’s aliasing models. The Stacked Borrows unsafe-code-guidelines discussions give concrete examples where an active shared protector becomes invalidated and later mutation through another reference becomes UB-like (i.e., the model tracks read/write permissions rather than syntactic read/write intent) [3]. Also note: RefCell enforces borrow rules at runtime for safe APIs (panic on conflicting borrows), but once you go into unsafe and create/consume references from aliased raw pointers, you’re outside that runtime checking; Miri then checks aliasing permissions via Stacked Borrows / Tree Borrows rather than letting “it’s only read-only” save you [4][2]. Tree Borrows: The Tree Borrows model similarly treats access permissions as time-varying based on the borrow tree; Miri uses this model too for checking reads/writes via pointers against the permission state [5][6]. Conclusion: Even if method() is read-only, dereferencing a raw pointer into a value that is currently covered by an active mutable borrow (from RefCell::borrow_mut or &mut) can be UB, because Miri’s models gate even reads/writes on whether the pointer’s permission is currently enabled. The correct way to avoid this is to not create/read through ptr while the conflicting RefMut / &mut is alive, or to ensure the raw pointer access obeys the aliasing permissions the model expects (e.g., by using appropriate interior mutability patterns or restructuring so borrows do not overlap) [2][1].
Citations:
- 1: https://blog.dmitriev.de/rust/0011-miri/
- 2: https://doc.rust-lang.org/std/cell/struct.RefCell.html
- 3: Stacked Borrows: Retag (private) fields of ADTs? rust-lang/unsafe-code-guidelines#125
- 4: https://doc.rust-lang.org/book/ch15-05-interior-mutability.html
- 5: https://doc.rust-lang.org/beta/nightly-rustc/miri/struct.Stack.html
- 6: https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/tree-borrows.md
Avoid creating a shared reference from the raw pointer while a RefMut is active.
The Err(_) fallback branch executes only when cell.try_borrow() fails—meaning a mutable borrow is still held. At that moment, unsafe { (&*ptr).is_managed_heap_object(addr) } creates a shared reference aliasing the protected memory, which violates Rust's aliasing rules and is undefined behavior under Stacked Borrows / Tree Borrows, even for read-only operations. Replace this with a borrow-free approach (e.g., caching heap extents or using atomic operations) rather than materializing &dyn majit_gc::GcAllocator while the RefMut is alive.
| let payload_shape = { | ||
| let link_borrow = link.borrow(); | ||
| ( | ||
| link_borrow.last_exception.is_some(), | ||
| link_borrow.last_exc_value.is_some(), | ||
| ) | ||
| }; | ||
| match payload_shape { | ||
| (false, false) => { | ||
| // Structural adaptation for pyre's bytecode-level | ||
| // graph builder. RPython `flowcontext.py:130-156 | ||
| // guessexception` closes a canraise block with | ||
| // `exits[0]` as the sole normal link and all | ||
| // following links seeded via `Link.extravars`. | ||
| // Pyre's PC-sequential walker can leave an | ||
| // additional normal/explicit-raise edge after | ||
| // the first slot. Such a link is not an | ||
| // exception match arm; flatten it with ordinary | ||
| // `make_link` so final exceptblock targets still | ||
| // lower through `make_return`, and keep | ||
| // `make_exception_link` reserved for seeded | ||
| // exception links as upstream expects. | ||
| self.make_link(link, false); | ||
| continue; | ||
| } | ||
| (true, true) => {} | ||
| (last_exception, last_exc_value) => panic!( | ||
| "canraise catch link payload partially seeded: \ | ||
| last_exception={last_exception}, last_exc_value={last_exc_value}", | ||
| ), | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add targeted regression tests for the new payload-shape triage.
This branch now encodes three behaviors ((false,false) fallback, (true,true) exception path, mixed panic), but there isn’t a direct test here that pins all three outcomes. Please add focused tests for the mixed-case panic and the fully-absent fallback path.
🤖 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 `@pyre/pyre-jit/src/jit/flatten.rs` around lines 1848 - 1878, Add unit tests
that exercise the payload_shape triage in the flattening logic: (1) a test that
constructs a Link with both last_exception and last_exc_value set to None so the
code path that calls self.make_link(link, false) is taken and assert that the
fallback (non-exception) path executes (e.g., by observing that make_link was
invoked or that the resulting flattened graph contains the expected normal
exit), and (2) a test that constructs a Link with exactly one of last_exception
or last_exc_value set (the mixed case) and assert that the code panics with the
message containing "canraise catch link payload partially seeded" (or use
should_panic to catch the panic). Locate the triage around payload_shape in
flatten.rs (the match on payload_shape, and the calls to self.make_link and
make_exception_link) to create and feed appropriate Link instances into the
flattening function so each branch is deterministically covered; name tests to
reflect the cases (e.g., payload_shape_all_absent_fallback and
payload_shape_mixed_panic).
…t write Add `ResumeGuardDescr.overlay_state: OnceLock<OverlayState>` carrying the CALL_ASSEMBLER overlay's `callee_descr` + `caller_layout` (Path 1 task #70 Slice 2). `overlay_deadframe_fail_descr` (cranelift compiler.rs:548) no longer pre-composes via `prefixed_by` and no longer calls `fail_descr_set_recovery_layout` on the synthetic descr; the reader (`fail_descr_recovery_layout`) consults `overlay_state` first, composing on-demand and memoising in `OverlayState.composed`. Performance: raise_catch went 5.15s → 0.07s (74x speedup) because the memo amortises the per-deopt `prefixed_by` clone cost that the prior cell-driven path was paying repeatedly. Why Slice 2 needed before Slice 3: the meta-side `recovery_layout` cell on `ResumeGuardDescr` has two remaining writers — codegen-time (compiler.rs:13223) and `patch_fail_descr_recovery_layout` (compiler.rs:6445) — both for production guards. Slice 3 routes those through `recovery_layout_via_callback` (already plumbed in Slice 1) and deletes the cell. Overlay guards now have a slot-independent storage path so Slice 3's cell deletion is safe. Production guards leave overlay_state unset → unchanged behaviour (meta cell read).
…t write Add `ResumeGuardDescr.overlay_state: OnceLock<OverlayState>` carrying the CALL_ASSEMBLER overlay's `callee_descr` + `caller_layout` (Path 1 task #70 Slice 2). `overlay_deadframe_fail_descr` (cranelift compiler.rs:548) no longer pre-composes via `prefixed_by` and no longer calls `fail_descr_set_recovery_layout` on the synthetic descr; the reader (`fail_descr_recovery_layout`) consults `overlay_state` first, composing on-demand and memoising in `OverlayState.composed`. Performance: raise_catch went 5.15s → 0.07s (74x speedup) because the memo amortises the per-deopt `prefixed_by` clone cost that the prior cell-driven path was paying repeatedly. Why Slice 2 needed before Slice 3: the meta-side `recovery_layout` cell on `ResumeGuardDescr` has two remaining writers — codegen-time (compiler.rs:13223) and `patch_fail_descr_recovery_layout` (compiler.rs:6445) — both for production guards. Slice 3 routes those through `recovery_layout_via_callback` (already plumbed in Slice 1) and deletes the cell. Overlay guards now have a slot-independent storage path so Slice 3's cell deletion is safe. Production guards leave overlay_state unset → unchanged behaviour (meta cell read).
…ch (#68) * Slice 7-Tβ7: migrate force_token_slots_cell to ResumeGuardDescr `ResumeGuardDescr::force_token_slots: UnsafeCell<Vec<usize>>` is the single physical home (Slice 7-Tβ7). `CraneliftFailDescr::set_force_token_slots` and `force_token_slots_view` forward through `meta_descr.as_any() .downcast_ref::<ResumeGuardDescr>()`; `is_force_token_slot` and the on-demand `gc_map()` consume the forwarded view. Constructor signature change: `new_with_trace_and_kind_and_force_tokens` renamed to `new_with_trace_and_kind` and drops the `force_token_slots` parameter; callers invoke `descr.set_force_token_slots(...)` after `meta_descr` is assigned and `Arc::new` materialises the descr (same post-Arc lifecycle as `set_source_op_index` / `set_recovery_layout` / `set_trace_info`). External-JUMP path now synthesizes a `ResumeGuardDescr` meta_descr (via `make_resume_guard_descr_typed`) instead of leaving meta None, so the meta-side `force_token_slots` slot is reachable. The `external_jump_target_cell` and `is_external_jump()` cell-membership predicate remain independent of `meta_descr`; `is_resume_guard()` still short-circuits to false for external JUMPs via the cell check before consulting meta_descr. * Slice 7-Tβ9: migrate fail_count to ResumeGuardDescr `ResumeGuardDescr::fail_count: AtomicU32` is the single physical home (Slice 7-Tβ9). `CraneliftFailDescr::increment_fail_count` and `get_fail_count` forward through `meta_descr.as_any().downcast_ref:: <ResumeGuardDescr>()`; non-`ResumeGuardDescr` meta (Done* / Exit* singletons) return 0 on read and silently no-op on increment, which is correct — those descrs never carry bridges and their counter never feeds threshold logic. Drops the `fail_count` field from `CraneliftFailDescr` and removes `AtomicU32` from the cranelift guard.rs imports. * Slice 7-Tβ10: migrate trace_info_cell to ResumeGuardDescr `ResumeGuardDescr::trace_info: AtomicPtr<CompiledTraceInfo>` is the single physical home (Slice 7-Tβ10). `CraneliftFailDescr::set_trace_info` and `trace_info_ref` forward through `meta_descr.as_any().downcast_ref:: <ResumeGuardDescr>()`; non-`ResumeGuardDescr` meta (Done* / Exit* singletons) silently drop writes and return `None` on read — those descrs do not carry per-trace metadata. Adds `impl Drop for ResumeGuardDescr` to reclaim the published `Arc<CompiledTraceInfo>` (swap-to-null + Arc::from_raw + drop), mirroring the lifecycle that previously lived on `CraneliftFailDescr`. * Slice 7-Tβ8: migrate external_jump_target_cell to ResumeGuardDescr `ResumeGuardDescr::external_jump_target: OnceLock<DescrRef>` is the single physical home (Slice 7-Tβ8). CraneliftFailDescr accessors `set_external_jump_target` / `external_jump_target_ref` and the `is_external_jump()` / `target_descr()` predicates forward through `meta_descr.as_any().downcast_ref::<ResumeGuardDescr>()`. External-JUMP descrs already synthesise a `ResumeGuardDescr` meta (Slice 7-Tβ7); `set_external_jump_target` expects this meta and panics if absent, matching the `assembler.py:2456-2462 closing_jump` codegen-time finality. `is_resume_guard()` keeps the cell-membership short-circuit so a synthetic `ResumeGuardDescr` carrying only the external_jump_target slot does not flip the backend role-reading to "ResumeGuard". Drops the `external_jump_target_cell` field and the `OnceLock` import from the cranelift guard.rs (now only `Mutex` remains in `std::sync::*`). This slot is a cranelift-only NEW DEVIATION (PyPy's `AbstractFailDescr._attrs_` carries no equivalent — upstream emits raw inter-function JMPs). Documented as such on the meta-side field. * Slice 7-Tβ11: migrate bridge_code_ptr_cache / bridge_frame_depth_cache to ResumeGuardDescr Move the two JIT-baked `Box<AtomicUsize>` cells from `CraneliftFailDescr` to `ResumeGuardDescr` so the meta Arc owns the canonical bridge-cache slots. The cell addresses (heap-pinned via Box inside the Arc) are still embedded as immediates by `emit_attached_bridge_dispatch` at codegen. Cranelift `bridge_cache_addrs` / `bridge_code_ptr` / `has_bridge` / `attach_bridge` now forward through `meta_descr` via a new `meta_resume_guard_descr()` `as_any` downcast helper. Production guards (real `op.descr` ResumeGuardDescr or the test-scaffold synthesis at compiler.rs:12884) all carry a `ResumeGuardDescr` meta, so the downcast succeeds. Non-Resume `op.descr` (PropagateExceptionDescr backing GUARD_NO_EXCEPTION in `compile_tmp_callback`) used to bypass meta-side cells; extend the meta synthesis at compiler.rs:12853 to mint an empty ResumeGuardDescr meta in that case so the JIT-baked load resolves to a heap-pinned cell that always reads 0. No bridge is ever attached to these guards (PyPy `_assemble_op` skips bridge logic for non-Resume descrs). Add `Box<AtomicUsize>` initialisers at the 5 `ResumeGuardDescr` sites in `majit-metainterp/src/compile.rs` (clone_descr, make_resume_guard_descr_typed, plus the three `payload`-tagged sites) and the cranelift-side `Descr::clone_descr` + `make_resume_guard_descr_typed` in `majit-backend/src/resume_guard_descr.rs`. * Slice 7-Tβ12: migrate bridge_dispatch_cell to ResumeGuardDescr via type-erasure Move the last JIT-baked bridge cell — `bridge_dispatch_cell: Box<AtomicPtr<BridgeData>>` — from `CraneliftFailDescr` to `ResumeGuardDescr`. `BridgeData` lives in `majit-backend-cranelift` (downstream crate) and majit-backend cannot depend on it, so the cell is type-erased to `AtomicPtr<()>` with a backend-registered cleanup function (`bridge_dispatch_drop_fn: OnceLock<fn(*mut ())>`). Cranelift registers `drop_bridge_payload` as the cleanup on first `attach_bridge`; the matching `ResumeGuardDescr::drop` invokes it on any payload still in the cell at descr teardown so the published `Arc<BridgeData>` is reclaimed by the owning crate without majit-backend knowing its concrete type. `bridge_ref` / `attach_bridge` on `CraneliftFailDescr` forward through `meta_resume_guard_descr()`. Cell address is not JIT-baked (runtime reads only) so `AtomicPtr<()>` is plain (no Box) — `Arc::new(self)` pins the surrounding `ResumeGuardDescr` and the field address stays stable across `Arc::clone`. Add the 2 new fields at the 5 metainterp `ResumeGuardDescr` initialiser sites (clone_descr, make_resume_guard_descr_typed plus 3 `payload`-tagged sites) and the 2 cranelift-side initialisers in `Descr::clone_descr` + `make_resume_guard_descr_typed`. Drop the now- unused `AtomicPtr` / `Ordering` / `Mutex` imports from cranelift's `guard.rs`. * Slice 7-Tβ4: drop per-descr position invariant assertion (NEW DEVIATION) Remove the `fail_descrs position must equal fail_index` assertion in cranelift `build_guard_metadata`. PyPy does not carry per-emission `fail_index` on the descr itself (`llsupport/assembler.py` keys on `_allgcrefs` index); pyre's runtime dispatch is position-based via `find_fail_descr_in_fail_descrs` after Slice 7-Tβ1. The assertion would otherwise reject any singleton FINISH push (singletons answer the trait-default `0` for `fail_index_per_trace()` regardless of Vec position). Parallels dynasm Slice 7-Tα4 commit cf05a9e99c. * Own source_op_index per-emission on ResumeGuardCopiedDescr source_op_index was migrated to ResumeGuardDescr in Slice 7-Tβ6. cranelift guard.rs reached it by chasing prev_descr through ResumeGuardCopiedDescr; two copies sharing one donor (optimizer.py:691 / optimizeopt/mod.rs:4438-4470) clobbered each other's op indices. Per-emission classification per history.py:132 AbstractFailDescr._attrs_ rd_locs / adr_jump_offset (assembler.py:279 writes onto each emitted faildescr directly, never chases prev). Add source_op_index slot to ResumeGuardCopiedDescr, surface FailDescr trait accessors with default None / no-op, override on both ResumeGuardDescr and ResumeGuardCopiedDescr (+ ResumeGuardCopiedExcDescr delegation). cranelift set_source_op_index / source_op_index_ref now route through the trait; no prev chase. * Own force_token_slots per-emission on ResumeGuardCopiedDescr force_token_slots was migrated to ResumeGuardDescr in Slice 7-Tβ7. cranelift guard.rs reached it by downcasting meta_descr to ResumeGuardDescr only; when meta_descr was a ResumeGuardCopiedDescr (common after optimizeopt copied-guard sharing), the set_force_token_slots write was silently dropped, leaving the copied descr's GC classification empty at deopt. Per-emission classification matches PyPy's inline GC map encoded per emission in assembler.py:write_failure_recovery_description. Add force_token_slots slot to ResumeGuardCopiedDescr, surface FailDescr trait setter with default no-op, override on both descr classes (+ ResumeGuardCopiedExcDescr delegation). cranelift set_force_token_slots / force_token_slots_view now route through the trait; force_token_slots_view returns owned Vec<usize> since the trait API is owned. * Own fail_count per-emission on ResumeGuardCopiedDescr fail_count was migrated to ResumeGuardDescr in Slice 7-Tβ9. cranelift get_fail_count / increment_fail_count downcast meta_descr to ResumeGuardDescr only; copied guards' increments silently returned 0, so their bridge-compilation threshold never tripped. Per-emission classification matches compile.py:683 AbstractResumeGuardDescr._attrs_ ('status',) where each copied descr retraces independently of the donor (jitcounter hash lives in per-descr status, not chased through prev). Add fail_count slot to ResumeGuardCopiedDescr, surface FailDescr trait accessors with default 0 / no-op, override on both descr classes (+ ResumeGuardCopiedExcDescr delegation). cranelift counter helpers now route through the trait. * Own trace_info per-emission on ResumeGuardCopiedDescr trace_info was migrated to ResumeGuardDescr in Slice 7-Tβ10. cranelift set_trace_info / trace_info_ref downcast meta_descr to ResumeGuardDescr only; copied guards never received the publish and their trace_info_ref returned None, blanking FailDescrLayout.trace_info for copied exits. Per-emission classification matches record_loop_or_bridge (compile.py:185-186) which stamps loop-token-equivalent metadata on each emitted descr; copied descrs in the same trace carry the same logical value but write into their own cell. Type-erased trait methods on FailDescr (trace_info_any / set_trace_info_any) take Arc<dyn Any + Send + Sync> to avoid pulling CompiledTraceInfo into majit-ir. Same pattern as rd_loop_token_clt. ResumeGuardCopiedDescr gets its own AtomicPtr<CompiledTraceInfo> slot + Drop reclaim. * Own bridge cells per-emission on ResumeGuardCopiedDescr bridge_code_ptr_cache / bridge_frame_depth_cache (Slice 7-Tβ11) and bridge_dispatch_cell (Slice 7-Tβ12) were migrated to ResumeGuardDescr. cranelift has_bridge / bridge_code_ptr / bridge_cache_addrs / attach_bridge / bridge_ref reached them via meta_resume_guard_descr, which downcasts to ResumeGuardDescr only. For copied guards, bridge_cache_addrs panicked and attach_bridge could not publish into the right slot. PyPy parity: compile.py:701-717 handle_fail and compile_and_attach apply to both ResumeGuardDescr and ResumeGuardCopiedDescr; each emitted descr can have its own attached bridge. Add bridge cell slots + Drop reclaim to ResumeGuardCopiedDescr. Surface FailDescr trait methods (bridge_cache_addrs / bridge_code_ptr / store_bridge_caches / bridge_dispatch_load / bridge_dispatch_swap) with conservative defaults. Override on both descr classes (+ ResumeGuardCopiedExcDescr delegation). cranelift bridge helpers now route through trait via new meta_fail_descr() helper. * Document why recovery_layout chases prev (only legitimate chase) After Slices 1-5 removed the prev_descr chase from per-emission slots, only set_recovery_layout / recovery_layout_ref still chase. This is PyPy-orthodox: recovery_layout is a derived view of the resume payload (rd_numb / rd_consts / rd_virtuals / rd_pendingfields) which compile.py:849 ResumeGuardCopiedDescr.get_resumestorage() explicitly delegates to prev. Add a doc comment to both functions citing compile.py:849 + assembler.py:279 to lock in the rule for future contributors: resume-derived chases, per-emission does not. * Slice 7-Tβ3: cranelift FINISH push the singleton CraneliftFailDescr Parallel of dynasm Slice 7-Tα3 commit 3f282e5b19. PyPy's compile.py:626-656 _DoneWithThisFrameDescr* are module-level singletons; every FINISH exit of a given result type resolves to the SAME Arc::ptr_eq identity (make_and_attach_done_descrs parity). The per-trace CraneliftFailDescr wrapper for FINISH was a pyre NEW DEVIATION whose per-trace state (recovery_layout, force_token_slots, source_op_index) was already silently no-op'd on the singleton's non-Resume meta_descr (DoneWithThisFrameDescr* has no ResumeGuardDescr-shaped slots); the canonical per-FINISH state lives on TerminalExitLayout in CompiledLoop::terminal_exit_layouts instead. is_exit_frame_with_exception vs DoneWithThisFrame routing mirrors the dispatch table at compiler.rs:12992-13001 — exception exits use EXIT_FRAME_WITH_EXCEPTION_DESCR_REF_CL; ordinary FINISH uses the result-type-specific DONE_WITH_THIS_FRAME_DESCR_* singleton. Singletons carry self.fail_index = u32::MAX and self.trace_id = 0, which do not carry per-trace identity. compiled_*_fail_descr_layouts overlay the per-trace fail_index (position in fail_descrs) and trace_id when constructing the FailDescrLayout view via the new build_per_trace_layouts helper, matching PyPy's assembler.py:227 self.faildescr.index = i convention. * Slice 7-Tβ14a: extract fail_descr_layout / fail_descr_gc_map as free fns Lift composite methods off CraneliftFailDescr to free functions in compiler.rs that take &DescrRef + (fail_index, trace_id). Parallel of dynasm Slice 7-Tα7's layout_for_fail_descr signature (guard.rs:146). build_per_trace_layouts and the two run_compiled_code FINISH/guard exit paths now go through fail_descr_layout, threading the runtime per-trace fail_index/trace_id into the layout instead of reading the singleton's self.fail_index = u32::MAX placeholder. Add FailDescr trait overrides on CraneliftFailDescr for source_op_index and trace_info_any — both forward to meta_descr. Without these, the free fn (which calls the trait methods on &dyn FailDescr) would hit the trait defaults and mask the meta-side per-emission slot, breaking test_compiled_fail_descr_layouts_include_backend_recovery_layout. The inherent CraneliftFailDescr::layout / gc_map / source_op_index_ref / trace_info_ref methods stay for now (a few call sites + the Debug impl still depend on them); the next sub-slice (14b) migrates those callers and deletes the inherent forms. * Slice 7-Tβ14b: remove inherent CraneliftFailDescr::layout and gc_map Last two callers of the inherent composite methods migrate to the free function counterparts (Slice 7-Tβ14a): - deadframe_layout: uses fail_descr_layout, threading the descr- internal fail_index/trace_id (preserves pre-14a singleton u32::MAX/0 semantics for deadframe-only lookups). - Debug impl: calls fail_descr_gc_map(self). - test_compiled_fail_descr_layouts_include_backend_recovery_layout sibling (test_setarrayitem_gc_marks_ref_slots_only_in_gc_map): calls fail_descr_gc_map(descr.as_ref()). Then delete: inherent layout() / gc_map() / gc_map_for_types() helper and the meta_resume_guard_descr() downcast helper that only gc_map_for_types depended on. All composite operations now go through the free functions in compiler.rs, so the struct is one step closer to being purely a data wrapper around fail_index / trace_id / fail_arg_types / meta_descr. * Slice 7-Tβ14c-1: add FailDescr trait overrides on CraneliftFailDescr Add the 10 missing setter/getter overrides that forward through meta_descr so trait-method dispatch on a `&dyn FailDescr` observes the same values the existing inherent `*_ref` / `set_*` forwarders publish. Without these overrides the trait defaults (no-op / 0 / None / null) would mask the meta-side slots whenever a CraneliftFailDescr is reached via the `FailDescr` trait object (e.g. from `fail_descr_layout` or the extension-trait migration in upcoming 14c-2/14c-3 slices). Overrides added: set_source_op_index, set_force_token_slots, fail_count, increment_fail_count, set_trace_info_any, bridge_cache_addrs, bridge_code_ptr, store_bridge_caches, bridge_dispatch_load, bridge_dispatch_swap. Plus rewires the existing force_token_slots override to forward via meta_descr's FailDescr trait method (was reading the inherent force_token_slots_view helper which now becomes redundant alongside the rest of the inherent forwarders). * Slice 7-Tβ14c-2: add fail_descr_* free fns; migrate compiler.rs callsites Lift the cranelift-specific bridge / per-emission helpers off `CraneliftFailDescr`'s inherent impl into free functions taking `&dyn FailDescr` (or `&DescrRef` for prev_descr-chase paths) in compiler.rs: - fail_descr_bridge_ref -> Option<Arc<BridgeData>> - fail_descr_attach_bridge -> publishes bridge + caches - fail_descr_has_bridge -> bool - fail_descr_bridge_cache_addrs -> (usize, usize), panicking - fail_descr_set_trace_info -> wraps Arc<dyn Any> - fail_descr_set_recovery_layout -> chases prev_descr (compile.py:849) - fail_descr_external_jump_target -> downcast ResumeGuardDescr - fail_descr_set_external_jump_target -> downcast ResumeGuardDescr drop_bridge_payload is now `pub(crate)` so the cranelift-side `fail_descr_attach_bridge` can hand it to `bridge_dispatch_swap` — the cell knows nothing about `Arc<BridgeData>` so the drop closure travels with the writer. Migrate the 55+ compiler.rs callsites that previously invoked the inherent `bridge_ref` / `attach_bridge` / `has_bridge` / `bridge_cache_addrs` / `set_trace_info` / `set_recovery_layout` / `set_external_jump_target` to the new free functions. Also rename test- side `get_fail_count()` to the FailDescr trait method `fail_count()` — the inherent forwarder will be deleted in 14c-3, but the trait method already returns the same value via 14c-1's override. No behaviour change — the inherent forwarders on CraneliftFailDescr remain in place for guard.rs's Debug impl and the FailDescr trait impl's internal use; those go away in 14c-3 alongside the inherent methods themselves. * Slice 7-Tβ14c-3: delete CraneliftFailDescr inherent forwarders Removes the 17 inherent forwarder methods on `CraneliftFailDescr` that 14c-1 / 14c-2 made redundant: bridge_ref, attach_bridge, has_bridge, bridge_cache_addrs, bridge_code_ptr, recovery_layout_ref, trace_info_ref, source_op_index_ref, force_token_slots_view, set_source_op_index, set_force_token_slots, set_recovery_layout, set_trace_info, set_external_jump_target, external_jump_target_ref, increment_fail_count, get_fail_count, is_force_token_slot, meta_fail_descr. All callers in compiler.rs already route through the free `fail_descr_*` helpers (14c-2) or via the `FailDescr` trait method overrides added in 14c-1. The Debug impl + `is_resume_guard` / `is_external_jump` / `target_descr` / `is_gc_ref_slot` trait overrides now inline the meta_descr downcasts directly via the new private helper `meta_external_jump_target` and through the `FailDescr` trait methods on `self`, removing the last internal consumers of the deleted inherent forwarders. CraneliftFailDescr's inherent API now consists only of the two constructors (new_with_trace_and_kind / new_external_jump) plus two private downcast helpers (meta_resume_fd / meta_external_jump_target). This unblocks 14c-4 (extension trait or further consolidation) and ultimately 14d/14e/14f (storage type swap, JIT-baked Arc handling, struct deletion). * Slice 7-Tβ14d: migrate cranelift fail_descrs storage to Box<[DescrRef]> Storage change: CompiledLoop.fail_descrs, BridgeData.fail_descrs, RegisteredLoopTarget.fail_descrs now Box<[DescrRef]> instead of Box<[Arc<CraneliftFailDescr>]>. Mutable temporaries in compile/bridge paths and run_compiled_code signatures (&[DescrRef]) match dynasm shape (majit-backend-dynasm runner.rs:381). JitFrameDeadFrame.fail_descr also migrates to DescrRef (was Arc<CraneliftFailDescr>); deadframe_from_jitframe + JitFrameDeadFrame::new accept DescrRef. overlay_deadframe_fail_descr's Arc<CraneliftFailDescr> return coerces via the unsized-coercion at the field assignment. force_token_to_dead_frame still recovers Arc<CraneliftFailDescr> from the JIT-baked thin pointer in jf_force_descr, then upcasts to DescrRef at the boundary (the JIT-baked-pointer side is the 14e concern). New Descr trait method _backend_wrapper_meta_descr() returns Option<DescrRef> (default None, overridden on CraneliftFailDescr to clone meta_descr). Used by: * register_fail_descrs dual addr-keying (meta-keyed entry for downstream consumers that compute descr_addr from the meta Arc). * get_latest_descr_arc_from_deadframe meta navigation (history.py:125 identity). Deleted in 7-Tβ14f. match_metainterp_finish_descr return type changed from Option<(DescrRef, Arc<CraneliftFailDescr>)> to Option<(DescrRef, DescrRef)> so the if/else expression types align with the singleton-or-found path. RawExecResult.descr_arc is already Arc<dyn Descr>; the bogus `as Arc<dyn FailDescr>` cast in the raw-exec exit paths is replaced by direct fail_descr_arc.clone() — the field type accepts DescrRef unchanged. Method calls on &DescrRef now go through the as_fd() helper introduced in 14d-start; new sites added at ~55 callsites covering is_finish, fail_arg_types, trace_id, fail_index, increment_fail_count, get_status, store_hash, is_external_jump, target_descr, force_token_slots, is_exit_frame_with_exception, is_resume_guard, is_resume_guard_copied, set_rd_loop_token_clt, fail_index_per_trace. Pointer casts through `*const () as usize` thin-pointer intermediate. All 119 lib + 35 integration cranelift tests green. * Slice 7-Tβ14e: flip cranelift JIT-baked jf_descr to metainterp Arc Non-FINISH guards now bake `Arc::as_ptr(meta) as *const () as i64` (the metainterp AbstractFailDescr Arc's data pointer) instead of `Arc::as_ptr(&CraneliftFailDescr_wrapper) as i64`. This matches PyPy's assembler.py:2126 `get_gcref_from_faildescr` identity: the pointer the backend writes to jf_descr is the same Arc the metainterp stamped onto op.descr (history.py:125). The wrapper address was a pyre NEW DEVIATION dating from the split-descr era. Producer (compiler.rs:13340): non-FINISH branch reads `meta_descr` (always populated post-14d) and bakes the meta data pointer. Consumers updated to look up via the dual-keyed `CRANELIFT_ACTIVE_FAIL_DESCR_REGISTRY`: - run_compiled_code result path (compiler.rs:6306 area) - force_token_to_dead_frame (jf_force_descr deref) - call_assembler_guard_failure_inner `emit_attached_bridge_dispatch` was previously deriving the bridge cache cell addresses from `*const CraneliftFailDescr` derived from `fail_descr_ptr` — that backwards-cast is gone. GuardInfo carries `bridge_cache_addrs: Option<(usize, usize)>` pre-computed at codegen time via `fail_descr_bridge_cache_addrs(descr.as_ref())`, populated when `can_have_bridge=true`. The dispatch emitter takes the pair directly. `register_fail_descrs` dual-keys the registry on both the backend wrapper address (for legacy code paths still casting `i64` back through the wrapper type) and the meta Arc data pointer (the new JIT-baked address). Both lookups return the same DescrRef so trait dispatch lands on `CraneliftFailDescr` impl as before. Test status: 119 lib + 35 integration cranelift tests green. Next slice (14f) deletes the CraneliftFailDescr struct entirely. * Slice 7-Tβ14f-α: singleton-direct DescrRef + ResumeGuardDescr external-JUMP overrides Singleton FINISH descrs are now `LazyLock<DescrRef>` holding the metainterp `Arc<DoneWithThisFrameDescr*>` / `Arc<ExitFrameWithExceptionDescrRef>` directly, no CraneliftFailDescr wrapper. Production codegen `let descr: DescrRef = if is_finish { ... } else { ... }` produces the metainterp Arc directly: `op.descr` when it is a ResumeGuardDescr, otherwise a freshly synthesised ResumeGuardDescr. Per-emission setters (`set_source_op_index`/`set_force_token_slots`) and the JIT-baked `jf_descr` immediate all route off this single Arc. fail_descr_ptr fat-pointer addresses derived via `*const () as i64` consistently. Add `is_external_jump` / `target_descr` overrides to `ResumeGuardDescr`'s `FailDescr` trait impl. Pre-deletion the wrapper forwarded these through `meta_external_jump_target()`; with the wrapper gone the trait default `false` / `None` was returned, breaking the bridge external-JUMP re-entry loop in `execute_registered_loop_target`. The structural answer is on the metainterp side: `external_jump_target` slot membership (`history.py:470 TargetToken._ll_loop_code`, populated by Slice 7-Tβ8) IS the predicate. * Slice 7-Tβ14f-β: overlay path emits DescrRef directly + Weak<dyn Descr> registry `overlay_deadframe_fail_descr` now returns `DescrRef` (synthesized `Arc<ResumeGuardDescr>`) instead of wrapping a `CraneliftFailDescr` around a synthesized meta. All per-emission setters (`set_trace_id` / `set_source_op_index` / `set_force_token_slots` / `set_recovery_layout` / `set_trace_info`) publish directly into the `ResumeGuardDescr` slots through trait dispatch. `overlay_fail_descr_registry`, the matching TLS handle, and `register_overlay_in_active_registry` switch from `Weak<CraneliftFailDescr>` to `Weak<dyn Descr>` keyed on the data pointer half of the `Arc<dyn Descr>` fat pointer (`*const () as usize`) — the same encoding used by `jf_descr` stamps (`call_jit.rs` / `wrap_call_assembler_deadframe_with_caller_prefix`) and the C-ABI `descr_addr` lookup in `fail_descr_arc_from_addr`. `is_finish=false` is the trait default for `ResumeGuardDescr`, consistent with the prior wrapper behavior (`CraneliftFailDescr` forwarded `is_finish` through `meta_descr` which was the same synthesized `ResumeGuardDescr`). Finish inner descrs cannot reach this constructor because `deadframe_recovery_layout_for_call_assembler` early-returns when no recovery layout is present, and finish singletons carry none. * Slice 7-Tβ14f-δ: delete CraneliftFailDescr struct + impls entirely Final endpoint of the Unified-Descr Port Epic on the cranelift side: `CraneliftFailDescr` (the per-emission backend wrapper) is gone. Cranelift now matches PyPy `assembler.py` structurally — every guard op's `op.descr` is the same `AbstractFailDescr` Arc the metainterp stamps via `compile.py:870 store_final_boxes_in_guard`, and every FINISH emission writes the cpu-attached class-distinct singleton (`DoneWithThisFrameDescr*` / `ExitFrameWithExceptionDescrRef`) directly into the deadframe's `jf_descr` slot. Removed from `guard.rs` (~742 lines): - `struct CraneliftFailDescr` - `impl Drop` / `impl Debug` / `unsafe impl Send/Sync` - `impl CraneliftFailDescr` (inherent — `new_with_trace_and_kind`, `new_external_jump`, `meta_resume_fd`, `meta_external_jump_target`) - `impl majit_ir::Descr for CraneliftFailDescr` - `impl FailDescr for CraneliftFailDescr` - Stale historical comment blocks for the long-removed SOURCE_OP_INDEX_TABLE / RECOVERY_LAYOUT_TABLE / TRACE_INFO_TABLE / EXTERNAL_JUMP_TARGETS / FAIL_COUNT_TABLE / BRIDGE_CACHES_TABLE / FORCE_TOKEN_SLOTS_TABLE side tables (replaced with a single consolidated header note). Removed from `descr.rs`: - `FailDescr::_backend_wrapper_meta_descr` trait method (was the last transitional escape hatch; with no wrapper there is nothing to peek past). Cleaned up in `compiler.rs`: - Import of `CraneliftFailDescr`. - `get_latest_descr_arc_from_deadframe`: drop dead wrapper-meta branch — `jf.fail_descr` IS the metainterp Arc. - `find_fail_descr_by_ptr`: drop the wrapper-meta dual-lookup; a single addr comparison resolves identity (`history.py:125`). - `register_fail_descrs`: drop dual-keying — only one Arc to key on. Test sites migrated: - 4 sites in `compiler.rs::tests` to a new `mk_test_resume_guard_descr` helper that synthesises a `ResumeGuardDescr` and stamps `trace_id` via the `FailDescr::set_trace_id` setter. - 2 sites in `tests/integration.rs` to direct `majit_backend::make_resume_guard_descr_typed` calls. All 119 cranelift lib + 35 integration tests green. Workspace check clean. The pre-existing `guard_exit_getters_fall_back_to_previous_token_backend_layouts` failure (dynasm-only) is unrelated — reproduces on origin/main. * Remove bridge_addr gate from dynasm compiled_bridge_descr_arc The trait contract (majit-backend/lib.rs:1797-1816) is identity recovery for `bridge_source_descr` when the metainterp side cannot produce the descr (synthetic / cut-tentative ops, post-retrace stale traces). Whether a bridge is currently attached is irrelevant — the bridge-compilation path needs the source descr identity regardless. Matches cranelift's `compiled_bridge_descr_arc` (compiler.rs:14290), which does not gate on bridge attachment. Fixes pre-existing dynasm-only test failures: - guard_exit_getters_fall_back_to_previous_token_backend_layouts - test_start_retrace_from_guard_uses_previous_token_backend_resume_data * Add bridge cache / per-emission method forwarders to ResumeGuard wrappers Slice 7-Tβ migration moved the per-emission JIT-baked cells (source_op_index, force_token_slots, fail_count, trace_info, bridge_code_ptr_cache, bridge_frame_depth_cache, bridge_dispatch_cell) onto ResumeGuardDescr's FailDescr impl. The three wrapper subtype descrs — ResumeAtPositionDescr, ResumeGuardForcedDescr, ResumeGuardExcDescr — never gained the matching forwarders, so any guard whose `op.descr` was one of these wrappers fell through to the trait default (None / 0 / no-op). cranelift's `collect_guards` panics on `fail_descr_bridge_cache_addrs` when the descr returns None for `bridge_cache_addrs`, exposing the gap during benchmarks (inline_helper / fib_recursive / raise_catch / nbody). Unit tests passed because no test exercises a guard whose op.descr is ResumeGuardExcDescr (minted by optimizer at optimizeopt/mod.rs:4857) reaching cranelift codegen. Forward all 13 methods through `self.inner` via fully-qualified trait syntax to disambiguate from the inherent shadowing on majit_backend::ResumeGuardDescr. Matches the existing ResumeGuardCopiedExcDescr forwarder shape (compile.rs:4255-4293). * Path 1 Slice 2: overlay descrs store (callee, caller) — drop meta-slot write Add `ResumeGuardDescr.overlay_state: OnceLock<OverlayState>` carrying the CALL_ASSEMBLER overlay's `callee_descr` + `caller_layout` (Path 1 task #70 Slice 2). `overlay_deadframe_fail_descr` (cranelift compiler.rs:548) no longer pre-composes via `prefixed_by` and no longer calls `fail_descr_set_recovery_layout` on the synthetic descr; the reader (`fail_descr_recovery_layout`) consults `overlay_state` first, composing on-demand and memoising in `OverlayState.composed`. Performance: raise_catch went 5.15s → 0.07s (74x speedup) because the memo amortises the per-deopt `prefixed_by` clone cost that the prior cell-driven path was paying repeatedly. Why Slice 2 needed before Slice 3: the meta-side `recovery_layout` cell on `ResumeGuardDescr` has two remaining writers — codegen-time (compiler.rs:13223) and `patch_fail_descr_recovery_layout` (compiler.rs:6445) — both for production guards. Slice 3 routes those through `recovery_layout_via_callback` (already plumbed in Slice 1) and deletes the cell. Overlay guards now have a slot-independent storage path so Slice 3's cell deletion is safe. Production guards leave overlay_state unset → unchanged behaviour (meta cell read). * Revert "Path 1 Slice 2: overlay descrs store (callee, caller) — drop meta-slot write" This reverts commit 81804011842de50879276411a8da8bf1e3705fa5. * Set jf_frame.length in cranelift's Vec-backed jitframe alloc run_compiled_code_inner's GC-alloc branch writes frame_depth to JF_FRAME_LENGTH_OFS (compiler.rs:6282); the Vec-backed branch did not, leaving the length word as 0. emit_attached_bridge_dispatch's frame_fits gate compares this length against required_frame_len, so the Vec-backed branch always reported frame_fits=false and fell through to the host-loop bridge fallback. Mirror the GC-alloc store so frame-size checks behave identically in nursery-backed and Vec-backed modes. * Add cranelift_realloc_frame helper Standalone scaffold mirroring PyPy _build_frame_realloc_slowpath (assembler.py:118 setup + aarch64/assembler.py:434 body) and dynasm's runner::dynasm_realloc_frame. Allocates a wider libc-backed JitFrame, copies the fixed header and live slots, writes jf_forward so jitframe_resolve follows the forwarding chain, and registers the new payload with the shadow-stack tracer. Currently unwired — emit_attached_bridge_dispatch still falls through to the host-loop bridge fallback when the frame is too small. A future slice replaces the fallback by routing the frame-fits-false branch through this helper, mirroring PyPy's _check_frame_depth flow. * Switch cranelift_realloc_frame to GC nursery allocation llmodel.py:140 `new_frame = jitframe.JITFRAME.allocate(...)` uses the same nursery allocator as the initial JITFRAME.allocate(frame_info); the libc::alloc_zeroed path leaked the new frame on every realloc. Match `run_compiled_code_inner` at compiler.rs:6235 — call `alloc_nursery_no_collect_typed(JITFRAME_TID, payload_bytes)` so the new frame is reclaimed through the regular GC cycle. Host-test path (no GC configured) keeps the libc fallback + shadow-stack registration. * Add _check_frame_depth prologue to cranelift bridges Port aarch64/assembler.py:927 `_check_frame_depth` line-by-line: every bridge prologue compares jf_frame.length against the bridge's required frame depth and calls cranelift_realloc_frame (= _frame_realloc_slowpath, llmodel.py:127-154 / assembler.py:118) when the parent guard's frame is too small. Source-guard presence (Option<(u64, u32)>) discriminates bridges from main loops in do_compile, mirroring compile_bridge as the sole supplier of that parameter. precompute_max_output_slots mirrors collect_guards' accumulation (compiler.rs ~12821) as a pure pre-pass so the prologue knows the required depth before collect_guards runs. The cranelift dispatch loop's host-side bridge frame_fits fallback (emit_attached_bridge_dispatch at compiler.rs:5506) remains in place for now; the next slice removes it once this prologue is verified in production traces. * Drop frame_fits check from cranelift bridge dispatch site x86/assembler.py:987 patch_jump_for_descr emits an unconditional jump to the attached bridge — PyPy never frame-checks at the source-guard dispatch site because the bridge's own prologue (_check_frame_depth, aarch64/assembler.py:927) reallocates the JITFRAME on entry when it's too small. Mirror that here: emit_attached_bridge_dispatch now reads only the bridge code cell and dispatches on non-null alone. The frame_depth cell is left in place (still written by fail_descr_attach_bridge for the host-loop bridge fallback) but no longer read at the dispatch site; the bridge's prologue bakes the required size as an iconst at compile time via precompute_max_output_slots. * Reorder fail_descr_attach_bridge cache publish before Arc swap Writing the bridge dispatch cache (store_bridge_caches) before the BridgeData Arc swap ensures every JIT guard-exit that observes a non-null bridge_dispatch cell also sees a populated code_ptr cache, so emit_attached_bridge_dispatch reaches the bridge in-code. The previous Arc-first ordering left a window where fail_descr_bridge_ref returned Some but bridge_code_ptr was still 0, forcing the host-loop fallback at compiler.rs:7287 to dispatch and reset every loop iteration. raise_catch dropped from 3.82s to 0.07s (≈50x) on this change alone. Add a one-shot probe on the host-loop fallback to confirm it is unreachable; Slice X1-delete follows. * Delete cranelift host-loop bridge dispatch fallback After Slice X1-realloc (bridge prologue handles JITFRAME growth) and the fail_descr_attach_bridge reorder (cache published before BridgeData Arc swap), every guard exit observing a non-null bridge code cache reaches the in-code dispatch in emit_attached_bridge_dispatch (compiler.rs:5464). A counter probe confirmed zero host-loop fires on raise_catch_loop.py (1M iterations, exception every 7th). The execute_with_inputs loop case that handled "bridge attached but in-code dispatch missed" is now structurally unreachable; delete the branch. Control reaching the deadframe return means cache was null at guard exit time, so the descr really has no bridge attached — the normal deopt exit. * Collapse cranelift execute_token_ints_raw dispatch loop to single call execute_token_ints_raw previously wrapped run_compiled_code in a `loop {}` mirroring execute_with_inputs's host-side dispatch for cross-loop JUMP and bridge-follow. Both branches are dead here: an X2 counter probe across raise_catch_loop / fannkuch / fib_recursive / fib_recursive_small / nbody / int_loop / nested_loop / float_loop / list_pop_append / list_insert / spectral_norm shows zero is_external_jump observations on the raw entry, and the bridge-follow branch is covered in-code by emit_attached_bridge_dispatch (Slice X1-delete confirmed dead on the deadframe entry). Convert the loop body to a single-pass block and replace both continue branches with asserts that fail loudly if a future caller violates the execute_token single-call shape. Slice X2-probe-2 (compiler.rs::slice_x2_probe): retains a global counter on execute_with_inputs's external_jump branch (still hot — 142629 on raise_catch_loop, 32M on fannkuch, 4M on nbody) to drive subsequent in-code closing_jump work. * Migrate BasicLoopTargetDescr.ll_loop_code from Mutex<usize> to AtomicUsize assembler.py:2456-2462 closing_jump reads `target_token._ll_loop_code` directly as an integer attribute (Python attribute, GIL-atomic). Pyre previously stored it inside `state: Mutex<BasicLoopTargetDescrState>` together with `target_arglocs` and `original_jitcell_token_number`, which forced every read to acquire the lock and prevented JIT'd code from inlining a load against the slot. Split `ll_loop_code` out of the Mutex into a sibling `std::sync::atomic::AtomicUsize` field on `BasicLoopTargetDescr`. The remaining Mutex still covers the arglocs / token-number pair (set once at compile time, infrequently read). Add `ll_loop_code_ptr()` on the `LoopTargetDescr` trait so the cranelift backend can bake the field's heap-stable address into JIT code for the upcoming in-code `closing_jump` dispatch. * Add closing_jump in-code dispatch scaffolding (no call site yet) Carries `BasicLoopTargetDescr.ll_loop_code` and the duplicate `optimizeopt::unroll::LoopTargetDescr.ll_loop_code` slots into the shape needed for `assembler.py:2456-2462 closing_jump`: an `AtomicUsize` field whose address is exposed via `LoopTargetDescr::ll_loop_code_ptr` so cranelift can bake the slot's heap-stable address into JIT'd code. GuardInfo gains `external_jump_ll_loop_code_addr` capturing that address at codegen time. `emit_attached_loop_dispatch` mirrors `emit_attached_bridge_dispatch` but issues a `return_call_indirect` (tail call) so the source loop's frame is replaced by the target's instead of pushing a return address — a regular `call_indirect + return` would grow the stack one frame per iteration and exhaust the OS thread stack on fannkuch-class traces within seconds. The dispatch invocation in `emit_guard_exit` is intentionally gated off (`let _ = info.external_jump_ll_loop_code_addr;`) until two follow-up slices land: - JIT functions migrate to `isa::CallConv::Tail` so the return-call-indirect signature is valid (cranelift rejects tail-call ABI mismatches at codegen / lowering). - target_argloc remapping: pyre's `ll_loop_code` points at the function entry (whose preamble decodes inputs at positions 0..preamble-arity), not at the body label PyPy targets directly. Without remapping, peeled-loop traces re-enter the preamble with body-argloc-shaped inputs and corrupt loop induction state — an experimental in-code dispatch attempt hung fannkuch on this mismatch. * Gate emit_attached_loop_dispatch off; revert to call_indirect+return The previous scaffolding wired return_call_indirect at the external- JUMP exit but cranelift requires both caller and callee to use isa::CallConv::Tail for tail-call ABI compatibility. Switching the outer JIT signature to Tail breaks the AppleAarch64 host caller contract (Tail clobbers callee-saves x19-x28/x29), producing wrong results on raise_catch_loop / fib_recursive / nbody / fannkuch. Reverting the dispatch helper to call_indirect + return keeps the helper compilable, but enabling the dispatch site then fails empirically: raise_catch_loop hits RecursionError after ~hundreds of self-jump frames (call+return grows the host stack), and fannkuch/nbody spin allocating ~26MB/s (re-entered loop reads stale input slots). Gate the dispatch site off via `let _ = info.external_jump_ll_loop_code_addr;` and document the two remaining prerequisites in the comment block: (a) wrapper+body call_conv split so the body can use CallConv::Tail while the wrapper stays host-ABI-compatible; (b) target argloc remapping for peeled-loop JUMPs whose body_arity differs from the target's preamble_arity. * Split cranelift JIT into wrapper(host) + body(Tail) functions compile_loop now declares two cranelift functions per trace: trace_N_body with CallConv::Tail (carries all existing JIT codegen) and trace_N_entry with default_call_conv (a thin shim that call_indirects the body and returns its result). After finalize_definitions: - CompiledLoop.code_ptr / LoopTargetEntry.code_ptr = wrapper entry address — what host (run_compiled_code, iconst-loaded bridge pointers) calls. - LoopTargetDescr.ll_loop_code = body entry address — what in-code closing_jump must tail-call. assembler.py:2456-2462 closing_jump emits raw JMP imm(target); cranelift exposes the same via return_call_indirect, which requires both caller and callee to use CallConv::Tail. The wrapper bridges the host AppleAarch64/SystemV ABI so the host caller's callee-saves (x19-x28/x29) remain preserved across a JIT call. emit_attached_loop_dispatch is still gated off at the GuardInfo external_jump_ll_loop_code_addr site; this slice only establishes the wrapper+body machinery. All 11 production benches pass with the new split. * Promote emit_attached_loop_dispatch to return_call_indirect; dispatch site stays gated emit_attached_loop_dispatch now emits cranelift's return_call_indirect (the analogue of assembler.py:2456-2462 closing_jump's raw JMP) into the Tail-conv target body. The previous call_indirect + return form would push one host frame per JUMP, exhausting the 768 KiB pyre stack budget on tight loops (raise_catch_loop, fannkuch); the wrapper+body split in 0fb27edc79 ("Split cranelift JIT into wrapper(host) + body(Tail) functions") makes the Tail call ABI legally available. The call site in emit_guard_exit stays gated off via `let _ = info.external_jump_ll_loop_code_addr;`. Enabling it empirically still leaks ~12 bytes of OS stack per fired tail call on aarch64 — raise_catch_loop surfaces RecursionError after ~61 k tail calls (61 k * 12 ≈ 730 KiB ≈ MAX_STACK_SIZE) and nbody's result drifts by a few ULPs (-0.03513379910298962 vs -0.035117363568587606), so something subtler than pure stack accumulation is in play too — the output corruption suggests the wrapper/body cross-conv interaction is not just leaking bytes but also losing register state. Slice X2-step4b will diagnose the leak + corruption. Until then, the host-loop dispatch in execute_with_inputs handles external JUMPs the legacy way; the dispatch helper compiles cleanly but is unreached. * Document X2-step4b diagnosis: cranelift Tail return_call_indirect leaks 12.5 bytes/take on aarch64 MAJIT_X2_PROBE re-run on 2026-05-18: raise_catch_loop crashes at exactly 61315 dispatch_takes for >=600k iterations (768 KiB / 61315 = 12.5 bytes per take). call_indirect+return variant leaks ~35 bytes/take (full body frame). Both shapes also corrupt nbody (-0.03513379910298962 vs dynasm reference -0.035132020348426815) and silently empty fannkuch. cranelift-codegen-0.130.2 aarch64 gen_clobber_save/_restore + emit_return_call_common_sequence look symmetric per source inspection. Dispatch site stays gated; comment updated to reflect findings. * Add CallAssemblerCallerContext scaffolding (X3-A) Declares CallAssemblerCallerContext { trace_id, header_pc, source_guard, input_types, inputs, caller_prefix_layout } and from_compiled_loop / from_bridge_data / from_registered_loop_target constructors mirroring the six values wrap_call_assembler_deadframe_with_caller_prefix already takes one at a time. Adds a thread-local CALL_ASSEMBLER_CALLER_STACK + push/pop/current_caller helpers. No call sites push, pop, or consume the stack in this slice; the helpers are gated with #[allow(dead_code)] and wired up by Slice X3-B. * Wire CallAssemblerCallerContext push/pop + dual-verify (X3-B) Push a CallAssemblerCallerContextGuard at each of the four wrap_call_assembler_deadframe_with_caller_prefix call sites (execute_with_inputs, execute_bridge, execute_registered_loop_target, execute_token_ints_raw); the RAII guard pops on scope exit. Inside wrap, reconstruct the caller_layout from the stack-top CallAssemblerCallerContext and assert_eq! it against the parameter-derived caller_layout. The wrap consumer expect()s a non-empty stack at every call so any future wrap site without a guard panics loudly. X3-C will drop the explicit caller-data parameters and use the stack as the sole source. Bench-verified on raise_catch_loop, nbody_50k, fannkuch, fib_recursive(_small), inline_helper, fib_loop, int_loop, nested_loop, float_loop (cranelift + dynasm), where every CALL_ASSEMBLER deadframe interception exercises the dual-verify. * Drop wrap explicit params; CallerContext stack is sole source (X3-C) wrap_call_assembler_deadframe_with_caller_prefix loses trace_id / header_pc / source_guard / input_types / inputs / caller_prefix_layout parameters. The caller-prefix layout is now reconstructed from the top of CALL_ASSEMBLER_CALLER_STACK exclusively — X3-B's dual-verify ran the two paths side-by-side and confirmed identity on every production CALL_ASSEMBLER deadframe interception (raise_catch_loop, nbody_50k, fannkuch, fib_recursive(_small), inline_helper). The four call sites (execute_with_inputs, execute_bridge, execute_registered_loop_target, execute_token_ints_raw) collapse to `wrap(frame)` after pushing their CallAssemblerCallerContextGuard. Overlay descr synthesis remains in place; X3-D will replace it with direct CallerContext routing through the deopt callback, and X3-E will then delete the ResumeGuardDescr.recovery_layout cell. * Delete overlay descr synthesis machinery; side-channel layout on deadframe (X3-D) Wrap no longer synthesises a transient ResumeGuardDescr to carry the caller-prefixed recovery layout. The caller-prefix layout lives in a new JitFrameDeadFrame.call_assembler_caller_layout side-channel field; deadframe_layout reads it and prepends to the callee descr's own recovery layout on demand, matching the previous combined shape. The callee descr's Arc identity now flows through the deadframe and the raw jf_descr stamp unchanged, so addr->Arc lookups resolve through the regular fail_descr_registry — overlay_fail_descr_registry, CRANELIFT_ACTIVE_OVERLAY_FAIL_DESCR_REGISTRY, set_cranelift_active_ overlay_fail_descr_registry, register_overlay_in_active_registry, overlay_deadframe_fail_descr, and sweep_stale_overlay_fail_descrs all deleted. fail_descr_arc_from_addr loses its overlay-fallback branch; the regular fail_descr_registry is the sole source. Bench-verified on raise_catch_loop, nbody_50k, fannkuch, fib_recursive, inline_helper. * Delete ResumeGuardDescr.recovery_layout cell + writer chain (X3-E) cranelift fail_descr_recovery_layout now consults the on-demand RECOVERY_LAYOUT_FN callback (pyre-jit's compute_recovery_layout_for_descr). The metainterp's StoredExitLayout.recovery_layout is the sole canonical store; describe_deadframe consumers fall back to trace_layout_ref.recovery_layout at pyjitpl/mod.rs:6431, matching the dynasm contract. - Drop ResumeGuardDescr.recovery_layout UnsafeCell + recovery_layout() / set_recovery_layout() accessors + 5 initializer sites. - Drop fail_descr_set_recovery_layout + patch_fail_descr_recovery_layout helpers + the codegen-time stamp at compile_loop's guard emit. - Drop Backend::update_fail_descr_recovery_layout trait method, cranelift impl, dynasm impl, and the metainterp call in both patch_backend_guard_recovery_layouts_for_trace and the bridge-path cell write at pyjitpl/mod.rs:6905. - patch_backend_guard_recovery_layouts_for_trace now only updates the metainterp's StoredExitLayout.recovery_layout (its only remaining job). - Adjust cranelift tests that asserted backend-cached recovery_layout / frame_stack to the new metainterp-owned contract. * cranelift X2-step4b: align test fixtures + dispatch helper signature Test fixture: make_label_descr now uses BasicLoopTargetDescr (real LoopTargetDescr impl with ll_loop_code slot) instead of the bare TestLabelDescr stub that masked external_jump_ll_loop_code_addr in emit_guard_exit. The unused TestLabelDescr struct is removed. emit_attached_loop_dispatch: drop the call_conv parameter and hard-code CallConv::Tail. Caller emit_guard_exit receives the host call_conv for its non-tail helper calls (write barrier, bridge dispatch); the in-code tail-call requires Tail conv to match the target body's signature regardless of caller context. Update the gating comment in emit_guard_exit to document the re-verification of both failure modes (nbody jf_frame-slot corruption and raise_catch/fannkuch cranelift 0.130/0.131 aarch64 Tail-conv stack leak) after the cranelift 0.131.1 upgrade attempt. Mark test_execute_bridge_follows_external_jump_to_compiled_target as ignored with explicit reference to the gating site; un-ignore once the dispatch enables. * Add per-LABEL label_block_id slot on LoopTargetDescr (X2-step4b plumbing) `assembler.py:990-993 TargetToken._ll_loop_code` parity gate plumbing. PyPy stores each LABEL's address directly so JMP lands at the LABEL's first instruction. Cranelift can't expose internal block addresses, so the body function shares one entry across all LABELs and the function preamble decodes the FIRST LABEL's inputarg layout. A cross-loop JUMP to a non-first LABEL via the function entry corrupts state when inputarg count/layout differs (verified for nbody: trace_id=1 has labels with 24-input and 15-input layouts). Adds an AtomicU32 `label_block_id` slot to `LoopTargetDescr`: - 0 = first LABEL of the body (function entry's preamble matches) - 1, 2, ... = subsequent LABELs (preamble layout differs) `compile_loop` assigns label_block_id positionally during LABEL registration. `collect_guards` captures `(ll_loop_code_ptr, label_block_id_ptr)` for external-JUMP exits. `emit_attached_loop_dispatch` now takes both addresses and runtime-gates `return_call_indirect` on `label_block_id == 0`; non-first-LABEL targets fall through to the deadframe exit so the host loop re-enters via `execute_token`. Dispatch site at `emit_guard_exit` stays gated (the cranelift aarch64 Tail-conv ~12.5 bytes/take stack leak documented in project_cranelift_wrapper_body_tail_leak.md remains the upstream blocker, independent of this per-LABEL gate). Plumbing landed so that when the upstream leak is fixed, only the gate flip remains. Test fixture `make_label_descr` in compiler.rs:tests now allocates a real `BasicLoopTargetDescr` via `majit_ir::make_loop_target_descr` instead of the test-only `TestLabelDescr` (removed); the test `test_execute_bridge_follows_external_jump_to_compiled_target` is marked `#[ignore]` until the dispatch gate flips on. * Document gate-flip probe finding: per-LABEL gate alone doesn't fix nbody 2026-05-18 probe: temporarily flipped the `emit_guard_exit` dispatch gate on with the per-LABEL `label_block_id` check active. nbody still produced the documented corrupted output `-0.03513379910298962` (vs. dynasm reference `-0.035132020348426815`), identical to the pre-gate `return_call_indirect`-on result. Conclusion: the corruption is NOT inputarg-count mismatch (which the per-LABEL gate addresses by falling through to deadframe exit for non-first LABELs). The remaining mismatch is output/input slot remap between source's exit slots and target's input slots — orthodox fix is porting PyPy's regalloc.py `_consider_jump` argloc remap (option ii on task #114). Gate reverted; updated comment in `emit_guard_exit` records the probe outcome. Per-LABEL plumbing committed in fb3674d513 stays; it's a necessary precondition for cranelift `br_table` per-LABEL entry blocks if option (i) is revisited later, and the gate-flip probe itself is rerunnable. * Document jf_gcmap/jf_descr clear hypothesis test (negative result) 2026-05-18 second probe: with the dispatch gate on plus jf_gcmap + jf_descr + jf_guard_exc cleared to 0 in `emit_attached_loop_dispatch` before `return_call_indirect`, nbody_50k still produced `-0.03513214049650899` (vs. dynasm reference `-0.035132020348426815`). Hypothesis was: target body inherits stale source-side gcmap from source's last `emit_guard_exit`, so any GC during target body re-walks the wrong jf_frame slots and overwrites floats with relocated "pointers". Mirroring `run_compiled_code`'s header-zero step (compiler.rs:6418) should have eliminated this if it were the cause. Negative result: the corruption is not from stale header state. Reverted to gate-off; updated in-helper comment to record the probe trace. raise_catch and fannkuch continue to crash/hang via the cranelift 0.130/0.131 aarch64 Tail-conv stack leak independently. Remaining diagnosis candidates (untested): - cranelift stack-slot interference between source/target bodies sharing the same SP on `return_call_indirect` - shadowstack imbalance from emit_call_footer_shadowstack + target body's emit_call_header_shadowstack sequence - target body's compile-time-baked `max_output_slots` / `ref_root_base_ofs` differing from source's, causing source's fail-arg writes to land in the wrong jf_frame area for target * Document ref_root_base_ofs zero hypothesis test (negative result) 2026-05-18 third probe: with the dispatch gate on plus source body's ref_root area zeroed (`jf_frame[ref_root_base_ofs_src + slot * 8] = 0`) before `return_call_indirect`, nbody_50k still produced `-0.03513214049650899` (vs. dynasm `-0.035132020348426815`). Hypothesis was: each cranelift body picks its own `max_output_slots` (per-trace constant), so source's `ref_root_base_ofs = ITEM0_OFS + max_output_slots_src * 8` differs from target's. Source's `sync_ref_root_var` writes from the entry would leave stale ref values at jf_frame positions that target's post-GC ref reload would misinterpret as its own roots. Negative result: zeroing source's ref_root area didn't change the corruption value at all. Mechanism is not source's leftover ref_root writes. Three corruption-source hypotheses now ruled out (per-LABEL inputarg count, stale jf_gcmap/jf_descr, stale ref_root area). Reverted gate to off; consolidated probe history in the dispatch helper comment. Next diagnosis step is empirical: byte-by-byte jf_frame compare between wrapper-direct vs tail-call entry, plus disassembly inspection of cranelift's `return_call_indirect` stack-slot reuse. * Document main-loop frame-realloc hypothesis test (negative result) 2026-05-18 fourth probe: enabled the frame-realloc check at every body prologue (removed `if source_guard.is_some()` so main loops also check `jf_frame.length >= expected_size`). With the dispatch gate on, nbody_50k still produces `-0.03513214049650899` (vs. dynasm `-0.035132020348426815`). Hypothesis was: in-code tail-call from source body to target body reuses source's jitframe, sized for source's `max_output_slots + num_ref_roots`. If target's expected size exceeds source's, target's prologue writes overflow into adjacent nursery memory, producing ULP-level corruption in nearby float allocations. Adding the realloc check at main-loop prologue should have detected this and called `cranelift_realloc_frame` to grow the jitframe. Negative result: target's frame_depth must already fit in source's jitframe — overflow is not the corruption mechanism. Four corruption hypotheses ruled out (per-LABEL inputarg count, header zero, ref_root zero, frame realloc). Diagnosis remains open. * Document jf_frame tail-zero hypothesis test (probe 5, negative result) 2026-05-18 fifth probe: with the dispatch gate on plus wholesale-zero of source's `jf_frame[fail_arg_count..length]` before `return_call_indirect`, nbody_50k still produced `-0.03513214049650899` (vs. dynasm reference `-0.035132020348426815`). Probe 3 (zero source's compile-time-baked `ref_root_base_ofs` area) only covered slots at source's `ITEM0 + source_max_output_slots * 8`; target's ref_root area starts at `ITEM0 + target_max_output_slots * 8` which may differ. Probe 5 zeros EVERY non-input slot in jf_frame through the `length` word — covering target's ref_root area and any other location target might read regardless of source's layout. Negative result: the corruption is NOT in jf_frame heap state. Five hypotheses now ruled out (per-LABEL inputarg count, stale jf_gcmap/jf_descr, stale ref_root area, frame-realloc, wholesale jf_frame tail). Both `return_call_indirect` and `call_indirect + return` shapes corrupt, both transit cranelift's aarch64 Tail-conv function entry/exit lowering, both produce deterministic ULP drift. This converges on the documented ~12.5 bytes/take SP leak in `emit_return_call_common_sequence` as the same root cause — pyre-side workarounds cannot address an upstream stack-frame accounting defect. * cranelift: address Codex P1+P2 review on PR #68 P1 (compiler.rs:execute_token_ints_raw): host-loop external_jump fallback restored. Cranelift's in-code `closing_jump` (`emit_attached_loop_dispatch`) stays gated off pending the upstream aarch64 Tail-conv blocker, so guard exits may legitimately surface `is_external_jump` descrs on the raw path. Replace the assertion with the same target-switch + continue loop `execute_with_inputs` uses (compiler.rs:7357), mirroring `llgraph/runner.py:1130-1140`. P2 (compiler.rs:Backend::force): set `CRANELIFT_ACTIVE_FAIL_DESCR_REGISTRY` before calling `force_token_to_dead_frame`. `force()` is reachable outside an `execute_token*` guard scope (async-forcing / virtualref paths), and the helper needs the registry handle to resolve `jf_force_descr`. Install the same `FailDescrRegistryGuard` `execute_token_ints*` uses so the JIT-baked descr address round-trips through the active backend's registry regardless of caller. * metainterp: wrap CompileLoopVersionDescr around ResumeGuardDescr Reshape CompileLoopVersionDescr as a newtype wrapping an inner ResumeGuardDescr, matching the pattern ResumeAtPositionDescr already uses for compile.py:892's sibling subclass. The wrapper inherits the full _attrs_ slot set (source_op_index, force_token_slots, fail_count, trace_info, external_jump_target, bridge_code_ptr_cache, bridge_frame_depth_cache, bridge_dispatch_cell, bridge_dispatch_drop_fn) that cranelift's collect_guards / fail_descr_bridge_cache_addrs read off every guard descr. Document the single-JIT-thread invariant on ResumeGuardDescr::trace_info and FailDescr::trace_info_any so the atomic Arc protocol's safety is explicit at the load -> increment_strong_count window. * metainterp: add call_assembler_*_arc_typed Slice X-D entry points Mirror the four call_assembler_*_by_number_typed wrappers with Arc-taking variants that route through make_call_assembler_descr (real-Arc factory) instead of make_call_assembler_descr_by_number (synth-Arc factory). Slice X-D dispatch-side wire-up will resolve the production Arc via warmstate.find_token_by_number at the four BC_CALL_ASSEMBLER_* sites and call these in place of the by-number variants; the by-number paths stay available for test fixtures and the dispatcher does not yet have warmstate access. No behavioral change: the new methods are not yet called. * dispatch: wire BC_CALL_ASSEMBLER_* sites through Arc resolver Extend `JitCodeRuntime` with `jitcell_token_arc_for_number` (default returns `None`). Add `ClosureRuntimeWithResolver` so trace entry points carrying warmstate access can pass a resolver closure. At each of the four `BC_CALL_ASSEMBLER_*` dispatch sites, when the resolver yields the production `Arc<JitCellToken>`, record via the `call_assembler_*_arc_typed` history methods (Slice X-D); otherwise fall back to the existing `_by_number_typed` path. Tests using the existing `ClosureRuntime::new(label_at)` keep the trait-default `None` resolver and continue to take the by-number path; the existing keepalive walker fallback covers them. * metainterp: convert CompiledEntry.previous_tokens to Weak (Slice X-G) memmgr.py:73 parity step: `MemoryManager.alive_loops` is the sole strong owner of compiled JitCellToken Arcs. Previously `CompiledEntry.previous_tokens` held a parallel strong owner, so evicted bridge tokens stayed pinned until both `alive_loops` and the `compiled_loops` entry dropped them. Convert the field to `Vec<Weak<JitCellToken>>`. Readers upgrade on demand and filter out dropped references: - `retire_compiled_entry` returns `Vec<Weak<JitCellToken>>`. - `fail_descr_layout_lookup` / `terminal_exit_layout_lookup` upgrade per element before passing to backend. - `release_evicted_loop_token` retains only non-evicted live tokens. - `jitcell_token_by_number` upgrades each entry, skipping dead ones. - `get_guard_status`, `find_source_fail_descr`, `has_bridge_fail_descr` upgrade-and-pass for backend calls. - `compile_bridge` materialises a `Vec<Arc<JitCellToken>>` from the upgrades before invoking the backend (backend signature unchanged). `CompiledEntry.token` remains `Arc<JitCellToken>` for now; downgrading it requires every reader on the hot path (~30+ sites) to opt into upgrade-on-demand and is its own follow-up slice. * metainterp: convert CompiledEntry.token to Weak (Slice X-G second cut) Change `CompiledEntry.token` from `Arc<JitCellToken>` to `Weak<JitCellToken>` and route all readers through a new `live_token()` helper that upgrades-or-panics. Eviction-aware paths (jitcell_token_by_number, ptr_eq scans) keep using `token.upgrade()` directly so they tolerate dropped entries. Writer sites switch `Arc::clone(&token)` → `Arc::downgrade(&token)`. Aligns with memmgr.py:73: `MemoryManager.alive_loops` is now the sole strong owner of `JitCellToken`; `compiled_loops`, `previous_tokens` (first cut, 53d84cdc14) and `.token` (this cut) hold weak refs only. Test fixtures that previously wrapped a fresh `JitCellToken` in `Arc::new` and stored it directly into `entry.token` now bind the Arc to a `_fresh_token_keepalive` local that outlives the assertion block, mirroring how `alive_loops` keeps the token reachable in production. The dynasm-only `patch_dynasm_fail_descr_resume_data` helper accepts `&Weak<JitCellToken>` and upgrades internally. * metainterp+macros: wire macro-generated __trace_* through warmstate token resolver (Slice X-D production wire-up) Add `MetaInterp::with_trace_ctx_and_token_resolver` split-borrow helper that hands the caller a `&mut TraceCtx` plus a `&dyn Fn(u64) -> Option<Arc<JitCellToken>>` resolver closure backed by `compiled_loops` + `warm_state.find_token_by_number` (the existing `jitcell_token_by_number` body, inlined to satisfy the borrow split). Add `trace_jitcode_{,observer_}with_args_and_runtime` siblings to the existing `_with_args` entry points; they take a pre-built `&impl JitCodeRuntime` instead of constructing `ClosureRuntime::new` internally. Export `ClosureRuntimeWithResolver` alongside. Change the `#[jit_interp]` macro's `__trace_*` generated function to accept a `&__R: JitCodeRuntime` parameter and call the `_and_runtime` variant. The matching `__merge_*` wrapper now drives the new split-borrow helper, constructs a `ClosureRuntimeWithResolver` that wraps the warmstate resolver, and forwards it to `__trace_*`. The `is_too_long` check moves to a fresh `trace_ctx()` re-borrow after the helper returns. Net effect: `BC_CALL_ASSEMBLER_*` dispatch sites that previously fell back to `_by_number_typed` (synth-Arc + warmstate fallback) now route through the real warmstate-backed `Arc<JitCellToken>` for every trace recorded by macro-generated `__trace_*` functions — closing the remaining downstream consumer gap of Slice X-D. * cranelift precompute_max_output_slots: key label-arity by descr_identity Replace HashMap<u32, usize> keyed by d.index() with Vec<(usize, usize)> keyed by descr_identity (Arc allocation address). d.index() is not unique across distinct TargetTokens in the same trace, so two Label descrs sharing an index could collide and misclassify a JUMP as internal vs external. * cranelift collect_guards: preserve op.descr identity for non-Resume FailDescrs Previousl…
…nly (slice D of Send/Sync epic)
After slices B (_nr -> Arc<AtomicI64>) and C (concretetype ->
Arc<RwLock<…>>), `Variable`'s only non-Sync field is `annotation`.
Replace the blanket "all cells are !Send + !Sync" preamble with an
audit-trail comment that:
- Names the single remaining field that still relies on the
`unsafe impl` (`annotation`).
- Enumerates the nine `Rc<…>` payloads inside `SomeValue` that
block its `Send + Sync` (`DictDefInner`, `ListDefInner`, nested
`SomeValue`, and the six `RefCell<…>`-wrapped `Desc` family
members) so a future reader can see the precise blast radius.
- Cross-links task #70 / `project_variable_sync_p1_full_scope.md`
so the cascade rationale stays discoverable.
No behavioural change. The `unsafe impl Send`/`Sync` stays in place
because the SomeValue cascade is still pending; this slice only
updates the documentation to match the post-B/C reality.
…"the arms" #70 measured the instruction this diagnostic gives and found it makes at least one subject strictly worse. The sentence read "declare `greens = [pc, program]` once this interpreter's dispatch arms lower". Nine of dualtape's ten arms lower, so it satisfies that precondition as written; declaring greens there takes it from one compiled loop of five ops to zero compiled traces, because `b']'` is still a degraded stub and `b']'` is the back edge, so every traced pass reaches it. Measured both arms at c6737bbeece in archive trees with private CARGO_TARGET_DIRs. The remedy now names the condition that actually governs -- every arm on the loop's back edge lowers -- and states what following it early costs. Two words dropped from the consequence clause: "zero-iteration segmented trace" -> "segmented trace whose compiled loop runs zero iterations", because the shorter form was read as "nothing compiles". It does compile: dualtape reads COMPILES=1, ops_before=4805, ops_after=5, body [GuardValue, IntIsZero, GuardFalse, GuardAlwaysFails, Finish]. "forever" is gone. It read as non-termination, which holds for braininterp's binary at TAPE_SIZE=30000 and not for a bounded fixture; "every trace attempt" already carries the repetition. The consequence clause itself is unchanged in substance -- it always said the shipped trace runs zero iterations, and that is what was measured. Subject identification unaffected: 20 distinct (crate, fn) pairs before and after. majit-macros + majit-metainterp 27 targets / 1761 passed / 0 failed. fmt clean. Assisted-by: Claude
Summary
This pull request introduces several improvements and bug fixes across the JIT backends, the benchmarking suite, and the command-line interface. The most significant changes address GC ownership queries under reentrant conditions, improve structural compatibility with upstream RPython/PyPy, and refine synthetic benchmark controls and JIT suppression for unsupported bytecode shapes.
Garbage Collector (GC) Ownership and Reentrancy Handling:
*_ACTIVE_GC_RAW) for both Cranelift and Dynasm backends, enabling safe GC ownership checks (is_managed_heap_object) even when the main GC lock is already held. This prevents panics during reentrant queries from root walkers and matches the RPython/PyPy GC descriptor behavior.JIT Suppression for Unsupported Bytecode Shapes:
WITH_EXCEPT_START, certainFOR_ITERcases). When such patterns are present, the JIT is disabled for the affected frame or region, falling back to the interpreter to match upstream semantics and avoid incorrect execution.Structural Compatibility and SSA/Liveness Fixes:
-live-marker per anchor pair, ensuring correct resume behavior after guard failures and matching upstream RPython/PyPy expectations.Benchmarking and CLI Improvements:
--syntheticto--no-synthetic(defaulting to running synthetic benchmarks unless explicitly skipped), clarified help text, and enforced mutual exclusion with--synthetic-only.pyre/bench/synth/comprehensions.pyandpyre/bench/synth/context_manager.pyto speed up test runs.Other Minor Fixes:
These changes improve robustness, correctness, and upstream compatibility, particularly in GC and JIT edge cases, and streamline synthetic benchmark management.
Self-review
This PR is intentionally not reviewed because its primary purpose is to restore synthetic test parity and enforce it hereafter in the CI. Some tests, notably synth/context_manager and synth/for_range_loop are excluded from JIT in order to restore parity. It's advised to ignore auto-reviews as well because their suggestions often break otherwise working tests.
Summary by CodeRabbit
New Features
--syntheticwith--no-syntheticand tighter arg validationBug Fixes
Chores