diff --git a/majit/majit-metainterp/src/blackhole.rs b/majit/majit-metainterp/src/blackhole.rs index ec97787fcad..8a1655fc837 100644 --- a/majit/majit-metainterp/src/blackhole.rs +++ b/majit/majit-metainterp/src/blackhole.rs @@ -307,6 +307,44 @@ pub struct BlackholeInterpreter { // Read by handler dispatch to populate exception_last_value. thread_local! { pub static BH_LAST_EXC_VALUE: std::cell::Cell = const { std::cell::Cell::new(0) }; + + /// llmodel.py:240 `grab_exc_value(deadframe)`: the exception a failing + /// guard carried, parked for the bridge / blackhole handoff. + /// + /// Grabbing the value reads `jf_guard_exc` off the deadframe and drops the + /// jitframe, which was the collector's only handle on the exception + /// (`jitframe_trace`). The handoff then reconstructs the resume state + /// through the blackhole allocator before anything re-roots the value, so + /// in that window the exception — and the young `args` / `__traceback__` + /// reachable only through it — live behind a bare `i64`. RPython's + /// `grab_exc_value` result is a shadowstack-rooted local across the same + /// span; pyre has no GC transform, so the frontend registers a root walker + /// over this cell instead. + pub static GUARD_EXC_VALUE: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Park a grabbed guard exception in [`GUARD_EXC_VALUE`] for the duration of +/// one handoff. +/// +/// Restores the previous value on drop rather than clearing, so a nested +/// handoff (a bridge trace that itself deopts) unwinds to the exception its +/// caller is still carrying. +pub struct GuardExcRoot { + prev: i64, +} + +impl GuardExcRoot { + pub fn park(exc: i64) -> Self { + Self { + prev: GUARD_EXC_VALUE.with(|cell| cell.replace(exc)), + } + } +} + +impl Drop for GuardExcRoot { + fn drop(&mut self) { + GUARD_EXC_VALUE.with(|cell| cell.set(self.prev)); + } } // rvmprof integration lives in the `rpython.rlib.rvmprof.cintf` analog. diff --git a/majit/majit-metainterp/src/jitdriver.rs b/majit/majit-metainterp/src/jitdriver.rs index 9a0be4e6975..efa879b84ab 100644 --- a/majit/majit-metainterp/src/jitdriver.rs +++ b/majit/majit-metainterp/src/jitdriver.rs @@ -3477,6 +3477,11 @@ impl JitDriver { // the no-exception continuation. let guard_exc = result.exception.exc_value; drop(result); + // The deadframe root died with the grab and the reconstruction + // below allocates through the blackhole allocator, so hold the + // exception where the frontend's root walker can reach it until + // `prepare_resume_from_failure` hands it to the blackhole. + let _guard_exc_root = crate::blackhole::GuardExcRoot::park(guard_exc); // must_compile tick for bridge threshold counting. if crate::majit_log_enabled() { diff --git a/majit/majit-metainterp/src/trace_ctx.rs b/majit/majit-metainterp/src/trace_ctx.rs index 02a70fea481..e3d8ccca48b 100644 --- a/majit/majit-metainterp/src/trace_ctx.rs +++ b/majit/majit-metainterp/src/trace_ctx.rs @@ -538,12 +538,10 @@ pub struct TraceCtx { /// grabs BEFORE frame reconstruction). Raw `PyObjectRef as i64`; 0 when the /// guard carried no exception. Class is re-derived from the value's typeptr. /// - /// NOT a traced GC root: it is stored unconditionally but only dereferenced - /// under the default-off `PYRE_CARRIER_EXC_RESUME` gate, so today no live - /// deref can outlive a moving collection. Before that gate is flipped on - /// this must become a real root (or be re-grabbed at read time), since a - /// collection between `set_bridge_guard_exc` and the seed read would leave - /// the raw integer stale — tracked as a pre-flip-on parity gap. + /// Not itself a traced slot: the exception is kept alive for the whole + /// handoff by [`crate::blackhole::GuardExcRoot`], which `handle_fail` parks + /// before it starts the bridge, so the value read back here is still live + /// whether or not a collection ran during the resume decode. pub(crate) bridge_guard_exc: i64, } diff --git a/pyre/bench/synth/getframe_while_caller_locals_across_subwalk.py b/pyre/bench/synth/getframe_while_caller_locals_across_subwalk.py new file mode 100644 index 00000000000..d8f269fb9d5 --- /dev/null +++ b/pyre/bench/synth/getframe_while_caller_locals_across_subwalk.py @@ -0,0 +1,35 @@ +# Caller locals that are live ACROSS the inlined call, exercised at a vable +# escape inside an inline sub-walk. +# +# The multi-frame chain gives each level its own frame as the virtualizable, so +# the walked frame's level writes its `setfield_vable` stores to the LIVE frame, +# while the adopt's resume-state write targets the snapshot and the portal +# epilogue then copies the snapshot's whole locals array back over the live +# frame. Any such store the resume-state write does not cover would revert, and +# `acc` and `tag` below are exactly the kind of caller local that would: both are +# carried across every iteration, so a single reverted slot changes the printed +# totals rather than merely perturbing timing. +import sys + +_gf = sys._getframe + + +def leaf(x): + _gf() + return x + 1 + + +def main(): + total = 0 + acc = 0 + tag = 7 + i = 0 + while i < 30000: + total = leaf(total) + acc = acc + total + tag = tag ^ i + i = i + 1 + return total, acc, tag + + +print(main()) diff --git a/pyre/bench/synth/getframe_while_captured_frame_outlives_call.py b/pyre/bench/synth/getframe_while_captured_frame_outlives_call.py new file mode 100644 index 00000000000..20c728f2077 --- /dev/null +++ b/pyre/bench/synth/getframe_while_captured_frame_outlives_call.py @@ -0,0 +1,35 @@ +# The callee's frame OUTLIVES the call and its f_back is read after the loop. +# +# This is the discriminator for the root `f_backref` operand in the multi-frame +# blackhole adopt. The adopt relinks the resumed chain before running it; the +# walked frame is represented twice, by the live frame the compiled loop runs on +# and by the `snapshot_for_tracing` copy, and the snapshot is freed at the end of +# the walk. Linking the chain root to the snapshot therefore leaves a dangling +# `f_back` that only a reader outliving the walk can observe -- which is what +# `kept.f_back` below does. A run that prints the right names proves nothing +# unless the fixture actually reaches the path, so keep the `while` drive and the +# zero-argument sys._getframe (see getframe_while_inlined_callee_subwalk). +import sys + +_gf = sys._getframe + +kept = None + + +def leaf(x): + global kept + kept = _gf() + return x + 1 + + +def main(): + total = 0 + i = 0 + while i < 30000: + total = leaf(total) + i = i + 1 + return total + + +t = main() +print(t, kept.f_back.f_code.co_name, kept.f_code.co_name) diff --git a/pyre/bench/synth/getframe_while_escaping_read_frame_identity.py b/pyre/bench/synth/getframe_while_escaping_read_frame_identity.py new file mode 100644 index 00000000000..3e6f5f6f1d8 --- /dev/null +++ b/pyre/bench/synth/getframe_while_escaping_read_frame_identity.py @@ -0,0 +1,67 @@ +# The frame-identity read that the multi-frame blackhole adopt gets wrong, and +# the acceptance test for flipping `PYRE_FBW_MULTIFRAME` default-ON. +# +# The walk executes the forcing residual CONCRETELY, and an inline push never +# runs the interpreter's call sequence, so `ec.topframeref` still names the +# CALLER while the inlined callee body runs. A `sys._getframe` that is itself +# the escaping call therefore reads the caller's frame at walk time, and the +# adopt commits that answer instead of discarding it the way the legacy +# escape/replay path does. +# +# Measured 2026-07-26 with the gate forced on -- one wrong iteration per +# multi-frame adopt, 5 adopts and 5 wrong in each part: +# +# part_a `_gf()` names `main`, not `leaf` +# part_b `_gf(1)` names ``, not `main` -- one level too far up, which +# is the same error seen through the argument +# +# A `_gf(1)` reading `f_locals` on that shape raises `KeyError` for any caller +# local, for the same reason and not because outer locals go unmaterialized. +# +# Both are correct with the gate off, which is the default, so this fixture +# passes today. It exists to fail loudly if the gate is flipped before the +# inlined-call push publishes the callee frame on the execution context. Note +# the read has to be the ESCAPING call: once the escape has happened, a +# `sys._getframe(1)` executed inside the blackhole is correct, because the chain +# publishes each level's frame as it runs. +import sys + +_gf = sys._getframe + +wrong_a = [] +wrong_b = [] + + +def leaf_a(x): + name = _gf().f_code.co_name + if name != "leaf_a": + wrong_a.append(name) + return x + 1 + + +def part_a(): + total = 0 + i = 0 + while i < 30000: + total = leaf_a(total) + i = i + 1 + return total + + +def leaf_b(x): + name = _gf(1).f_code.co_name + if name != "part_b": + wrong_b.append(name) + return x + 1 + + +def part_b(): + total = 0 + i = 0 + while i < 30000: + total = leaf_b(total) + i = i + 1 + return total + + +print(part_a(), part_b(), len(wrong_a), len(wrong_b)) diff --git a/pyre/bench/synth/getframe_while_inlined_callee_subwalk.py b/pyre/bench/synth/getframe_while_inlined_callee_subwalk.py new file mode 100644 index 00000000000..b5c70f96b1a --- /dev/null +++ b/pyre/bench/synth/getframe_while_inlined_callee_subwalk.py @@ -0,0 +1,44 @@ +# Coverage guard for the multi-frame blackhole path (PYRE_FBW_MULTIFRAME). +# +# A vable escape inside an INLINE sub-walk is what latches a multi-frame +# blackhole image. The rest of the corpus never produces one: every other +# getframe_* fixture drives with `for`, and with a FOR_ITER item in flight the +# callee's nested residual is declined by fbw_abort_nested_unjournaled_residual +# before execute_residual_call runs, so the force happens outside the sub-walk +# and the single-frame arm takes it. Driving with `while` is what reaches the +# site, and build_multi_frame_miframe then produces a depth-2 image. +# +# The shape below is load-bearing, not incidental: +# - `while`, not `for`, per the decline above; +# - sys._getframe called with NO argument, because the executor declines a +# non-void residual whose arguments are not all concrete and the generic +# LoadConst path is hard-declined as symbolic inside a sub-walk, so a +# literal argument would make this depend on a dedicated fold rather than +# on the loop form; +# - nothing read off the returned frame, which would reintroduce that +# constant-fold dependency. +# Changing any of the three can silently stop exercising the path. +# +# The printed total counts one callee entry per iteration, so a resume that +# replays the region or re-delivers an iteration prints something other than +# 30000. +import sys + +_gf = sys._getframe + + +def leaf(x): + _gf() + return x + 1 + + +def main(): + total = 0 + i = 0 + while i < 30000: + total = leaf(total) + i = i + 1 + return total + + +print(main()) diff --git a/pyre/bench/synth/getframe_while_subwalk_decline_shapes.py b/pyre/bench/synth/getframe_while_subwalk_decline_shapes.py new file mode 100644 index 00000000000..1615dcaa1e5 --- /dev/null +++ b/pyre/bench/synth/getframe_while_subwalk_decline_shapes.py @@ -0,0 +1,62 @@ +# Two sub-walk shapes the multi-frame blackhole build DECLINES, pinned so the +# decline stays a decline rather than silently becoming a wrong answer. +# +# Both reach the vable-escape latch inside an inline sub-walk and both are then +# refused by `capture_inline_parent_blackhole`, because a ref register that is +# live at the caller's post-call coordinate holds the untracked sentinel rather +# than a value. That sentinel is deliberately distinct from a known null (an +# uninitialised local is `Ref(PY_NULL)`), so recording it as null would fabricate +# a parent frame; declining is the correct answer until the caller's concrete +# banks are complete at an inline escape. +# +# part_a -- the caller has an exception handler around the inlined call, so a +# live stack ref at the resume coordinate is untracked. +# part_b -- two nested inlined levels, where the intermediate level's own +# parent capture hits the same sentinel. +# +# The values printed are what a correct legacy replay produces; a build that +# started accepting either shape without completing the banks would diverge here. +import sys + +_gf = sys._getframe + + +def leaf_a(x): + _gf() + if x < 0: + raise ValueError("never") + return x + 1 + + +def part_a(): + total = 0 + caught = 0 + i = 0 + while i < 30000: + try: + total = leaf_a(total) + except ValueError: + caught = caught + 1 + i = i + 1 + return total, caught + + +def inner_b(x): + _gf() + return x + 1 + + +def outer_b(x): + return inner_b(x) + + +def part_b(): + total = 0 + i = 0 + while i < 30000: + total = outer_b(total) + i = i + 1 + return total + + +print(part_a(), part_b()) diff --git a/pyre/gate-triage.md b/pyre/gate-triage.md index 493b16fb065..cc46f520d74 100644 --- a/pyre/gate-triage.md +++ b/pyre/gate-triage.md @@ -83,9 +83,113 @@ parity pass read `direct_assembler_call` and found its ON design is what upstream's `num_red_args` assert forbids. Retired. Still kept: `PYRE_CARRIER_EXC_RESUME` (default-off; threads the guard-failure exception into the bridge sym for the depth-2 carrier exception-resume slice #343/#126 — -inert until validated; the seed's `bridge_guard_exc` GC-rooting and the -unconditional `execute_ll_raised` exception assign are parity gaps to close -before it is enabled by default). +inert until validated). Two parity gaps were listed here as pre-flip work. The +`bridge_guard_exc` GC-rooting is closed by §1e — which also measures the seed +site as **reachable** (170 bridge-route guard failures carry a live exception), +so this gate is a live adoption target rather than an inert one. + +The second is still open, and the row described it as "the unconditional +`execute_ll_raised` exception assign", which is not what the divergence is. + +pyre's standing-exception maintenance for an exception-guard bridge lives in +`seed_bridge_standing_exception_from_current` (`state.rs`), which is **not +gated** and already mirrors upstream's branch: it assigns `last_exc_value` / +`last_exc_box` when it finds an exception, and clears all four exception slots +when it does not (`_prepare_exception_resumption`'s +`else: clear_exception()`). The divergence is the **source**. Upstream takes it +from `cpu.grab_exc_value(deadframe)` — the exception the failing guard carried. +pyre takes it from `sym.current_exc_value`, falling back to +`get_current_exception()` — the *execution context's* current exception, which is +the `sys.exc_info()` mirror, a different slot with different lifetime rules. + +`PYRE_CARRIER_EXC_RESUME` is a **back-channel into that function**: its only +effect is to write `guard_exc` into `current_exc_value` beforehand so the ungated +code picks it up. Hence the `is_null` conjunct — it exists to avoid clobbering a +live `sys.exc_info` value, which also means the injection is suppressed exactly +when the EC already holds an exception. That is why forcing the gate on measures +as a no-op: **dynasm 336/336 with the gate forced on**, correctness results +matching the default run, and the seven live-exception producers of §1e among +them, despite the seed site being entered 170 times. + +So "inert until validated" should read **inert because the guard's exception +reaches `last_exc_value` only through a slot it does not belong in**. A green +corpus under the gate is not evidence about the gate. Two further deltas to +settle before any flip, both in that function: it early-returns when +`last_exc_box` is already set, and it sets `class_of_last_exc_is_const = true`, +whereas the `_prepare_exception_resumption` path reaches `execute_ll_raised` with +the default `constant=False`. + +## §1e — The grabbed guard exception is rooted for the whole handoff (2026-07-27) + +`bridge_guard_exc` was booked as a pre-flip gap for `PYRE_CARRIER_EXC_RESUME`. +It is **not gate-specific**: the same grabbed pointer drives the default +blackhole resume, so the gate never bounded the exposure. + +`grab_exc_value` (`llmodel.py:240`) reads `jf_guard_exc` off the deadframe and +drops the jitframe, which was the collector's only handle on the exception +(`jitframe_trace`). The handoff then decodes resume data and rebuilds virtuals +through the blackhole allocator before anything re-roots the value, so in that +window the exception — and the young `args` / `__traceback__` reachable only +through it — live behind a bare `i64` that a precise collector cannot see. +RPython's `grab_exc_value` result is a shadowstack-rooted local across the same +span. Closed the same way as the six sibling raw-exception carriers +(`walk_jit_exc_value`, `walk_bh_last_exc_value`, …): `GuardExcRoot` parks the +value and a `GUARD_EXC_VALUE` root walker marks the carrier and forwards its +young children. Parked at the three handoff owners — `handle_fail`, +`blackhole_resume_via_rd_numb` (which also covers the CALL_ASSEMBLER caller), +and `back_edge_internal`. + +### Coverage census (339 files, `bench/` + `bench/synth/`) + +Instrumenting `handle_fail` counted **732,660** guard failures: + +| `guard_exc` | `is_guard_exc` | `should_bridge` | count | +|---|---|---|---| +| NULL | false | false | 648,725 | +| NULL | **true** | false | 48,327 | +| **NON-NULL** | **true** | false | **34,620** | +| NULL | false | **true** | 579 | +| NULL | **true** | **true** | 239 | +| **NON-NULL** | **true** | **true** | **170** | + +So the window is entered with a live exception **34,790** times, and the 170 in +the last row are exactly the `bridge_guard_exc` read this section is about — the +`PYRE_CARRIER_EXC_RESUME` seed site is **reachable**, not inert. Seven benches +produce them: `inline_subwalk_property_mutates` and +`inline_subwalk_mutating_residual_abort` (11,482 each), +`type_name_surrogate_reject` (9,462), `named_reraise_sibling_hot` (1,418), +`exc_mixed_classes_bridge_flavor` (410), `handler_reraise_second_exc` (400), +`sre_pattern_methods` (136). + +★ **TRAP** — an earlier revision of this section reported "zero coverage" from a +sweep whose every invocation had silently failed: `timeout` does not exist on +macOS, so each run died with `command not found` and produced no lines. The +control used to validate that sweep did not go through `timeout`, so it did not +catch it. Use `perl -e 'alarm N; exec @ARGV' --` instead. + +### The walker is not load-bearing on any measured workload + +A `gc_stress` build under `MAJIT_GC_STRESS` (full collection at the start of +every allocation, so the window's blackhole-allocator calls all collect) was run +over the producers above with the walker registered and with it suppressed: +`handler_reraise_second_exc`, `exc_mixed_classes_bridge_flavor`, +`named_reraise_sibling_hot`, `sre_pattern_methods` and +`type_name_surrogate_reject` all **pass identically both ways**. + +So this stays a **parity fix at a reachable site**, not a demonstrated bug fix. +The likely reason it cannot be discriminated: on the residual-raise path the +same exception is still parked in `BH_LAST_EXC_VALUE`, which `walk_bh_last_exc_value` +already roots, and nothing drains that cell before the handoff completes. What +the new walker covers is the case where it *is* drained first — untested, +because no workload produces it. + +★ **TRAP** — the first attempt at this A/B forced `try_gc_collect()` at +`handle_fail` entry instead of using `MAJIT_GC_STRESS`. That aborts with +`GC BUG: invalid type_id … site=object_total_size` on ~8/12 runs **with the +walker on as well**, which reads like a second defect but is not: an arbitrary +program point is not a safepoint, and the same bench is clean under the real +allocation-driven stress. Force collections through the GC's own stress hook, +never at a hand-picked instruction. ## §1c — Retired since the 2026-07-05 audit (10): reader already deleted by a closed epic @@ -118,7 +222,7 @@ upstream lines: | gate | orthodox side | outcome | |---|---|---| | PYRE_FBW_VABLE_SCALAR_CA | **OFF** | **RETIRED** — the ON design contradicts upstream | -| PYRE_FBW_MULTIFRAME | **ON** | keep; the ON path is the port, it is unfinished, and §1 measures it as never reached by the corpus | +| PYRE_FBW_MULTIFRAME | **ON** | keep default-OFF; the ON path is the port and the adopt now works, but §1d measures one remaining wrong answer under it — a `sys._getframe` that is itself the escaping residual reads the caller frame | | PYRE_FBW_CALLEE_VSTACK | NEITHER | keep OFF; see §5 | The walker's default-ON `PYRE_FBW_*` cluster was retired separately in #757. @@ -185,15 +289,51 @@ PyFrames heap-authoritative before the chain is built, whereas pyre's walker keeps inlined callee frames unmaterialized (`fbw_strict_fold_frame_reg`, `vable_ops.rs:184-192`). So the remaining work is a materialization step upstream does not have — a consequence of pyre's virtual-callee-frame inlining — -plus per-frame vable binding, since `PyjitplBlackholeFrameConfig` stamps one -shared `virtualizable_ptr` onto every frame in the chain and the adopt writes -only `last_instr`. A third item sits below both: `try_adopt_multi_frame_blackhole` -(`pyre-jit-trace/src/trace.rs`) declines outright when the recovered chain is not -rooted at the walked frame, and names the `jit.virtual_ref` emit at the inline -push as the prerequisite. That emit does not exist — `opimpl_virtual_ref` / -`_finish` are ported in both `majit-metainterp/src/pyjitpl.rs` and -`pyre-jit-trace/src/state.rs`, and **neither has a caller outside a `#[test]`**, -so `virtualref_boxes` is empty and no live trace records a `VIRTUAL_REF`. +and that is the whole of it. The per-frame vable binding this section used to +list beside it is already done: `PyjitplBlackholeFrameConfig` carries a +`per_frame` slice and `convert_and_run_from_pyjitpl` overrides each level's +`virtualizable_ptr` / `virtualizable_stack_base` from it (`blackhole.rs`), so +every level already runs against its own frame instead of a shared pointer. + +**Resolved 2026-07-26 — the adopt's root-mismatch decline.** +`try_adopt_multi_frame_blackhole` (`pyre-jit-trace/src/trace.rs`) declined +whenever the recovered chain's root was not the walked frame, and its comment +attributed that to "a chain rooted at an intermediate frame", naming the +`jit.virtual_ref` emit at the inline push as the prerequisite. Both halves of +that attribution were wrong. The chain *is* rooted at the walked frame; the two +sides of the comparison were two representations of it. `per_frame[0]` is +recovered from the trace's frame register, whose root vable identity +`seed_virtualizable_boxes` bakes against the **live** frame address +(`set_live_vable_frame_addr`, set before `init_symbolic` precisely so it is not +the discarded snapshot's), while `cf_addr` is the **snapshot** copy. Five +events printed `per_frame[0] == live == 0xa4be2db40` against +`cf_addr == 0xa4c0515e8`, so the decline was unconditional and no `VIRTUAL_REF` +emit was involved. (The emit is still absent — `opimpl_virtual_ref` / `_finish` +are ported in both `majit-metainterp/src/pyjitpl.rs` and +`pyre-jit-trace/src/state.rs` and **neither has a caller outside a `#[test]`**, +so `virtualref_boxes` is empty in every live trace. That is a real gap; it was +simply not this one.) + +The fix points the two *identity* uses at the live address, under the same +`!= 0` fallback the identity bake itself uses, and leaves every other use where +it was: + +| use of the walked frame | address | why | +|---|---|---| +| root-mismatch comparison | live | must be asked against the address the identity was baked from | +| root `f_backref` operand | live | the `ptr::eq` skip has to fire for `frames[0]`; the snapshot is freed at walk end, so linking to it would leave a dangling `f_back` for a later `sys._getframe().f_back` | +| `apply_blackhole_crn` | snapshot | the portal epilogue propagates snapshot → live, the same contract the single-frame arm relies on | +| `drive_multi_frame_blackhole` vable root + `stack_base` | dead | `per_frame` is always `Some` here and overrides both, per level | +| `concrete_nlocals`, the `ec` read | either | pycode-derived, and the snapshot copies `execution_context` verbatim | + +Opening the comparison exposes one consequence that could not fire while it was +shut: frame 0's blackhole level runs against `per_frame[0]`, the **live** frame, +so its `setfield_vable` stores land there while `apply_blackhole_crn` writes the +snapshot — and the epilogue then copies the snapshot's *whole* locals array onto +the live frame (`restore_resume_state_from`), reverting every such store the CRN +write does not happen to cover. The adopt therefore folds the live frame's +state into the snapshot before the CRN write, restoring "the snapshot is the +committed image". **Measured 2026-07-25: the multi-frame path has no corpus coverage.** The vable-escape latch site was instrumented and all **318** benchmarks @@ -203,10 +343,87 @@ is reached in **3 benches** (`getframe_escape_flush_writethrough_regression`, 5 events each, and **all 15 have `inline_subwalk=false`** — every one takes the single-frame arm and adopts. `build_multi_frame_miframe` is therefore never called, the image is never latched, and the adopt never sees a candidate. So -flipping `_MULTIFRAME` ON is a no-op across the corpus, none of the three items -above is exercised, and any port of them would be unvalidatable until a -benchmark that reaches `inline_subwalk=true` at a vable escape exists. Building -that benchmark is the prerequisite for the rest. Note the multi-frame latch is +flipping `_MULTIFRAME` ON is a no-op across the corpus and none of the items +above is exercised. Building a benchmark that reaches `inline_subwalk=true` at +a vable escape was the prerequisite, and **that benchmark now exists**: a +`while`-driven loop calling a straight-line inlined callee that calls a +zero-argument `sys._getframe` reaches the site. `for` is what every existing +`getframe_*` bench gets wrong — with a FOR_ITER item in flight the callee's +nested residual is declined by `fbw_abort_nested_unjournaled_residual` before +`execute_residual_call` runs, so the force never happens inside the sub-walk. +Under that shape `build_multi_frame_miframe` **succeeds at depth 2**, so the +build side was never what blocked. It is landed as +`synth/getframe_while_inlined_callee_subwalk`; the three shape choices in its +header are load-bearing and changing any of them silently stops exercising the +path. With the comparison fixed, that fixture under `PYRE_FBW_MULTIFRAME=1` +reports **5 `BUILT multi-frame depth=2` and 5 `adopted multi-frame terminal`, +zero declines** (the other 5 escapes in the run have `inline_subwalk=false` and +take the single-frame arm, as before), and prints the same result as CPython and +PyPy. + +**What the build still declines, and why the decline is right.** Two shapes +reach the latch and are then refused by `capture_inline_parent_blackhole` +(`resume_snapshot.rs`): a caller with an exception handler around the inlined +call, and two nested inlined levels (depth 3). Instrumented 2026-07-26, both +report the same cause — a ref color that is **live at the caller's post-call +coordinate holds `ConcreteValue::Null`**: + +``` +try/except caller: ref color=11 not concrete: Null result_color=Some(5) nlocals=3 depth=3 live_ref=[0,1,2,5,11] +depth 3: ref color=2 not concrete: Null result_color=Some(0) nlocals=1 depth=1 live_ref=[0,1,2] +``` + +Neither is the not-yet-produced result slot, and neither involves the bridge +parent-frame constructors — every `[s2-gate]` event in both runs prints +`not_bridge=true`, and the latch requires `!is_bridge_trace`, so those +constructors are unreachable from here. `ConcreteValue::Null` is the +**untracked** sentinel, deliberately distinct from `Ref(PY_NULL)` = "uninitialised +local" (`state.rs`, `trace_opcode.rs`), so accepting it would fabricate a parent +frame rather than reproduce one. Declining is correct; closing these two shapes +is the outer-locals materialization named above — completing the caller's +concrete banks at an inline escape — not a change to the capture itself. Both +are pinned by `synth/getframe_while_subwalk_decline_shapes` so a decline cannot +silently become a wrong answer. + +**The flip is blocked, and the blocker is a wrong answer, not a decline.** +Measured 2026-07-26. The walker executes residuals **concretely** while an +inline push never runs the interpreter's call sequence, so `ec.topframeref` +still names the CALLER while an inlined callee body runs. A `sys._getframe` +that is *itself* the escaping residual therefore reads the wrong frame at walk +time, and the adopt commits that answer where legacy escape/replay discards it: + +``` +_gf().f_code.co_name -> "main", not "leaf" +_gf(1).f_code.co_name -> "", not "main" # one level too far up +_gf(1).f_locals["k"] -> KeyError # same cause, seen through the argument +``` + +**One wrong iteration per multi-frame adopt** — 5 adopts, 5 wrong, in each part +of `synth/getframe_while_escaping_read_frame_identity`, which is the acceptance +test: it passes today (gate off, the default) and fails loudly if the gate is +flipped first. This is *not* outer-locals staleness. A `sys._getframe` +executed **after** the escape, inside the blackhole, is correct — the chain +publishes each level's frame as it runs — and an in-blackhole read of a caller +local mutated earlier in the same iteration was measured correct against CPython +and PyPy. Closing it needs the inlined-call push to publish the callee frame on +the execution context, which is what the open `walker_ec_enter` / `walker_ec_leave` +work does; the `jit.virtual_ref` emit rides along with it. So the original +decline comment was right that an inline-push `enter` is the prerequisite, and +wrong only about which check it gated. + +One thing the ON path already fixes: with a side-effecting inlined callee under +a `while` loop that returns from inside the loop, the OFF path runs the callee's +side effect ~5.2k extra times (the recorded trace-abort double-run class) while +the adopt gives the exact count. + +Everything else that was thought to block the flip has been measured and does +not: the full corpus is **336/336 with the gate on (dynasm) and 336/336 with it +off (cranelift)**, the blast radius is exactly `inline_subwalk = true` at a +vable escape (the latch is an `if`/`else if` whose single-frame arm requires +`!inline_subwalk`, so with the gate off that condition latches nothing and falls +to legacy escape/replay), and the two build-side declines above are correct. + +Note the multi-frame latch is nested inside `single_frame_blackhole_resume_enabled()`, so it also requires `_BLACKHOLE_RESUME` to stay ON. The pre-existing `[s2-gate]` eprintln (under `PYRE_FBW_DEBUG_ABORT`) already reports `inline_subwalk` at that site. @@ -251,7 +468,7 @@ OFF path is a needed safety net. Retire at the listed trigger (A7). | var | subsystem | retire when | |---|---|---| -| PYRE_FBW_BLACKHOLE_RESUME | single-frame resume-past-escape (#754) | flipped default-ON 2026-07-25; retirement was conditioned on the multi-frame twin (`_MULTIFRAME`) landing, but §1 now measures that twin as having zero corpus coverage, so the condition is unevaluable — keep the gate and re-open the question only once a benchmark reaches `inline_subwalk=true` at a vable escape | +| PYRE_FBW_BLACKHOLE_RESUME | single-frame resume-past-escape (#754) | flipped default-ON 2026-07-25; retirement was conditioned on the multi-frame twin (`_MULTIFRAME`) landing, but §1 now measures that twin as having zero corpus coverage, so the condition is unevaluable — keep the gate and re-open the question once the multi-frame adopt's root-mismatch decline (§1d) is resolved | | PYRE_TWO_PHASE_RTYPE, PYRE_TUPLE_PER_SHAPE_CLASSDEF | rtyper prepass / per-shape tuple classdef | WS2 / #346 rtyper epic | | PYRE_ORIGINAL_BOXES | greens++reds original_boxes index shape | box-identity #202 / resume F1 | | PYRE_MIR_FRAMESTATE | framestate-threaded MIR lowering | MIR front-end #176/#181/#346 | diff --git a/pyre/pyre-jit-trace/src/trace.rs b/pyre/pyre-jit-trace/src/trace.rs index 33c7f4ecc31..fbacd617fa1 100644 --- a/pyre/pyre-jit-trace/src/trace.rs +++ b/pyre/pyre-jit-trace/src/trace.rs @@ -1681,7 +1681,11 @@ fn try_adopt_single_frame_blackhole(ctx: &mut TraceCtx, cf_addr: usize) -> bool adopted } -fn try_adopt_multi_frame_blackhole(ctx: &mut TraceCtx, cf_addr: usize) -> bool { +fn try_adopt_multi_frame_blackhole( + ctx: &mut TraceCtx, + cf_addr: usize, + live_root_addr: usize, +) -> bool { // Every arm below returns to the legacy escape/replay path. Name each one // under `PYRE_FBW_DEBUG_ABORT`, the way `build_multi_frame_miframe`'s // `s2dbg!` names its own: a silent decline is indistinguishable from the @@ -1744,63 +1748,88 @@ fn try_adopt_multi_frame_blackhole(ctx: &mut TraceCtx, cf_addr: usize) -> bool { }; per_frame.push((frame_ptr, frame_stack_base)); } - // A chain rooted at an intermediate frame means the walk descended into a - // residual call and inlined inside it. Two things then do not hold: + // The walked frame has two representations here: `cf_addr` is the + // `snapshot_for_tracing` copy the walk steps concretely, and + // `live_root_addr` is the frame the compiled loop runs on. `per_frame[0]` + // is recovered from the trace's frame register, and + // `seed_virtualizable_boxes` bakes that root vable identity against the + // live address whenever there is one (`state.rs`), so an identity question + // has to be asked against the same address under the same fallback. + let root_addr = if live_root_addr != 0 { + live_root_addr + } else { + cf_addr + }; + // Frame-identity collapse guard. Every level must be a distinct frame and + // only `frames[0]` may be the walked frame: a level whose frame register + // resolved to the root would make the relink below write a `f_backref` + // cycle and run an inner level against the root's own virtualizable. No + // producer is known — an unseeded inline level has no frame object, so its + // register never resolves to a concrete `PyFrame` and the recovery loop + // above declines it — but the failure mode is silent, so decline rather + // than rely on the absence. The chain is two or three levels, so the + // pairwise scan is free. + for i in 0..per_frame.len() { + if i > 0 && per_frame[i].0 == root_addr as i64 { + mfdbg!("frame {i}: is the walked frame {root_addr:#x}"); + return false; + } + if per_frame[..i].iter().any(|&(ptr, _)| ptr == per_frame[i].0) { + mfdbg!("frame {i}: {:#x} repeats an earlier level", per_frame[i].0); + return false; + } + } + // Identity gate: the recovered chain has to be rooted at the frame this + // walk is stepping. A chain rooted at an intermediate frame means the walk + // descended into a residual call and inlined inside it, and then // `resume_py_pc` is a coordinate in `frames[0]`'s code while the restart - // moves `cf_addr`, and — because the walker executes residuals CONCRETELY - // while an inline push never runs the interpreter's call sequence — a - // `sys._getframe()` inside the inlined body already read `ec.topframeref` - // as the CALLER and committed the wrong frame object, which the adopt - // would then publish. Bracketing each inline level's concrete frame with - // an `enter`/`leave` does not fix it: levels that run without a - // materialized frame have nothing to publish, so the chain stays short by - // one and shifts every `_getframe` result up a level. Closing this needs - // the `jit.virtual_ref` emit at the inline push, so decline to legacy - // replay until then. + // moves the frame. There is no upstream counterpart to imitate: + // `convert_and_run_from_pyjitpl` converts the whole framestack + // unconditionally, and its `frames[0]` is always the portal frame, so this + // shape cannot arise there. // - // Measured by driving the chain with this check lifted, over the - // `getframe_inline_subwalk_multiframe` shape: the shift is exactly one - // level and it is silent. A `sys._getframe(2)` meant for the walked frame - // lands on the module frame instead, so the same run returns a wrong - // integer rather than failing — `f_locals["base"]` raises `KeyError`, - // `f_locals.get("base", -1)` scores -1 for 7, `len(f_locals)` scores the - // module globals' 12 for the walked frame's 3, and - // `len(f_code.co_name)` scores ``'s 8 for `main_loop`'s 9. The - // check is therefore load-bearing for EVERY outcome arm, not only the - // `ContinueRunningNormally` one that consumes a `cf_addr` coordinate: - // `DoneWithThisFrame*` and `ExitFrameWithExceptionRef` hand back a value - // the chain computed off the wrong frame. It also has to stay AHEAD of - // the drive — declining afterwards returns to a legacy replay that - // re-executes what the chain already committed. + // Passing this gate is NOT sufficient for the adopt to be right, and the + // remaining gap is why `PYRE_FBW_MULTIFRAME` is still opt-in. The walker + // executes residuals CONCRETELY while an inline push never runs the + // interpreter's call sequence, so `ec.topframeref` still names the CALLER + // while an inlined callee body runs. A `sys._getframe` that is itself the + // escaping residual therefore already read the wrong frame at walk time, + // and adopting commits that answer where the legacy escape/replay path + // discards it — measured as one wrong iteration per adopt + // (`synth/getframe_while_escaping_read_frame_identity`). Closing it needs + // the inlined-call push to publish the callee frame on the execution + // context; a `sys._getframe` executed later, inside the blackhole, is + // already correct because each level is published as it runs. mfdbg!( - "chain cf_addr={cf_addr:#x} levels=[{}]", + "chain root={root_addr:#x} cf_addr={cf_addr:#x} levels=[{}]", per_frame .iter() .map(|&(p, b)| format!("{p:#x}/nl{b}")) .collect::>() .join(", "), ); - if per_frame.first().map(|&(frame_ptr, _)| frame_ptr) != Some(cf_addr as i64) { + if per_frame.first().map(|&(frame_ptr, _)| frame_ptr) != Some(root_addr as i64) { mfdbg!( - "chain rooted at {:#x}, not the walked frame {cf_addr:#x} (needs the \ - virtual_ref emit at the inline push)", + "chain rooted at {:#x}, not the walked frame {root_addr:#x}", per_frame.first().map(|&(p, _)| p).unwrap_or(0), ); return false; } // `ExecutionContext::enter` parity for the resumed chain: - // `frames[0].f_backref = cf_addr`, `frames[i].f_backref = frames[i - 1]`. - // The blackhole re-executes each level's residual `sys._getframe` against - // `ec.topframeref`/`f_backref`, so the chain must be live before the run; - // it also stays live afterwards for a frame the residual captured. When - // `frames[0]` IS the walked frame it was already entered by its own - // caller — linking it would overwrite its `f_backref` with itself and - // orphan the frame above it. + // `frames[i].f_backref = frames[i - 1]`. The blackhole re-executes each + // level's residual `sys._getframe` against `ec.topframeref`/`f_backref`, so + // the chain must be live before the run; it also stays live afterwards for + // a frame the residual captured. `frames[0]` is the walked frame, already + // entered by its own caller, so the `ptr::eq` skip leaves its `f_backref` + // alone — which is exactly why the root operand must be `root_addr` and not + // the snapshot. The snapshot is freed at the end of this walk, so a link + // to it would survive as a dangling `f_back` for any later + // `sys._getframe().f_back` or traceback walk. unsafe { for i in 0..per_frame.len() { let callee = per_frame[i].0 as *mut pyre_interpreter::PyFrame; let f_back = if i == 0 { - cf_addr as i64 + root_addr as i64 } else { per_frame[i - 1].0 } as *mut pyre_interpreter::PyFrame; @@ -1838,6 +1867,12 @@ fn try_adopt_multi_frame_blackhole(ctx: &mut TraceCtx, cf_addr: usize) -> bool { &mut latched.framestack, majit_metainterp::blackhole::StateFieldLayout::default(), virtualizable_info, + // `per_frame` is `Some` just below, and `convert_and_run_from_pyjitpl` + // then overrides every level's `virtualizable_ptr` / + // `virtualizable_stack_base` from it (`blackhole.rs`), so this pair + // never reaches a blackhole level. Frame 0's vable writes land on + // `per_frame[0]` — the live frame — which is also what `on_enter_level` + // publishes as `ec.topframeref`. cf_addr as i64, stack_base, ctx.metainterp_sd().as_ref(), @@ -1850,6 +1885,26 @@ fn try_adopt_multi_frame_blackhole(ctx: &mut TraceCtx, cf_addr: usize) -> bool { unsafe { (*ec).topframeref = saved_topframeref; } + // Frame 0's blackhole level ran against `per_frame[0]`, the LIVE frame, + // because `convert_and_run_from_pyjitpl` overrides each level's + // `virtualizable_ptr` from `per_frame` — so its `setfield_vable` stores + // landed there, not in the snapshot. EVERY adopted arm below sets + // `WALK_END_FLUSH_COMMITTED`, and the portal epilogue then copies the + // SNAPSHOT's whole locals array onto the live frame + // (`restore_resume_state_from`), which would revert every one of those + // stores. Fold them into the snapshot before the arms so it is once again + // the committed image, which is the contract the epilogue and the + // single-frame arm both assume. This has to sit outside the match: a + // `DoneWithThisFrame*` or `ExitFrameWithExceptionRef` terminal writes no + // resume state of its own, and for the exception terminal the traceback + // keeps the root frame reachable, so a stale copy is observable through + // `tb_frame.f_locals` long after the walk. + if root_addr != cf_addr { + unsafe { + (*(cf_addr as *mut pyre_interpreter::PyFrame)) + .restore_resume_state_from(&*(root_addr as *const pyre_interpreter::PyFrame)); + } + } let adopted = match outcome { majit_metainterp::jitexc::JitException::ContinueRunningNormally { ref green_int, .. @@ -1886,6 +1941,9 @@ fn try_adopt_multi_frame_blackhole(ctx: &mut TraceCtx, cf_addr: usize) -> bool { ); return false; }; + // Snapshot, not `root_addr`: with the fold above the match the + // snapshot is the committed image the epilogue propagates, + // matching the single-frame arm. if !apply_blackhole_crn( cf_addr, terminal_jitcode_index, @@ -1943,8 +2001,9 @@ fn try_adopt_multi_frame_blackhole(ctx: &mut TraceCtx, cf_addr: usize) -> bool { adopted } -fn try_adopt_force_blackhole(ctx: &mut TraceCtx, cf_addr: usize) -> bool { - try_adopt_multi_frame_blackhole(ctx, cf_addr) || try_adopt_single_frame_blackhole(ctx, cf_addr) +fn try_adopt_force_blackhole(ctx: &mut TraceCtx, cf_addr: usize, live_root_addr: usize) -> bool { + try_adopt_multi_frame_blackhole(ctx, cf_addr, live_root_addr) + || try_adopt_single_frame_blackhole(ctx, cf_addr) } fn run_perfn_walk( @@ -2658,10 +2717,11 @@ fn run_perfn_walk( // predicate as the CloseLoop end-flush above. A latched inline-callee // forward abort has already distinguished an outside mark from a mark // inside its discarded attempt. + let live_root_addr = sym.live_vable_frame_addr(); let force_blackhole_adopted = matches!( &walk_result, Err(crate::jitcode_dispatch::DispatchError::VableEscapedDuringResidualCall { .. }) - ) && try_adopt_force_blackhole(ctx, cf_addr); + ) && try_adopt_force_blackhole(ctx, cf_addr, live_root_addr); if !force_blackhole_adopted && matches!( &walk_result, diff --git a/pyre/pyre-jit/src/call_jit.rs b/pyre/pyre-jit/src/call_jit.rs index 8357f61fcdf..ed36ee98d74 100644 --- a/pyre/pyre-jit/src/call_jit.rs +++ b/pyre/pyre-jit/src/call_jit.rs @@ -1902,6 +1902,12 @@ pub fn blackhole_resume_via_rd_numb( // `None` for both the vinfo and the per-frame virtualizable handle. novable: bool, ) -> BlackholeResult { + // Same window as `handle_fail`, for every blackhole resume including the + // CALL_ASSEMBLER caller: the decode below rebuilds virtuals through the + // blackhole allocator while the grabbed exception is still only a bare + // pointer with no deadframe root behind it. + let _guard_exc_root = majit_metainterp::blackhole::GuardExcRoot::park(guard_exc); + let nbody_debug = pyre_nbody_debug_enabled(); use majit_metainterp::resume; diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index 663b028d54d..137693520a4 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -3316,6 +3316,26 @@ fn walk_bh_last_exc_value(visitor: &mut dyn FnMut(&mut majit_ir::GcRef)) { }; } +/// Root the exception grabbed off a failing guard's deadframe +/// (`GUARD_EXC_VALUE`): the grab drops `jf_guard_exc`, the collector's only +/// handle on it, and the bridge / blackhole handoff reconstructs resume state +/// through the blackhole allocator before re-rooting the value. Same +/// carrier/children split as [`walk_jit_exc_value`]. +fn walk_guard_exc_value(visitor: &mut dyn FnMut(&mut majit_ir::GcRef)) { + let exc = majit_metainterp::blackhole::GUARD_EXC_VALUE.with(|c| c.get()); + if exc == 0 { + return; + } + let mut gcref = majit_ir::GcRef(exc as usize); + visitor(&mut gcref); + unsafe { + pyre_interpreter::eval::walk_raw_exception_roots( + gcref.0 as pyre_object::PyObjectRef, + visitor, + ) + }; +} + /// Root PyPy's process-global `rbigint._parts_cache`. The object crate exposes /// its raw `_digits` slots without depending on majit-ir; this adapter performs /// the `GcRef` conversion and writes back a forwarded address. @@ -3334,6 +3354,7 @@ fn install_gc_root_walkers() { pyre_interpreter::eval::register_pyframe_root_walker(); majit_gc::shadow_stack::register_extra_root_walker(walk_jit_exc_value); majit_gc::shadow_stack::register_extra_root_walker(walk_bh_last_exc_value); + majit_gc::shadow_stack::register_extra_root_walker(walk_guard_exc_value); majit_gc::shadow_stack::register_extra_root_walker(walk_rbigint_parts_cache); // Stored `PyError` carriers whose GC refs the precise collector cannot // reach through their raw TLS cells: the call-assembler FFI stash and the @@ -7106,6 +7127,11 @@ fn handle_fail( guard_exc: i64, _info: &majit_metainterp::virtualizable::VirtualizableInfo, ) -> HandleFailOutcome { + // The guard exception arrives as a bare pointer whose deadframe root is + // already gone, and bridge setup decodes resume data (allocating) before + // `setup_bridge_sym` copies it onto the sym. Park it for the walker first. + let _guard_exc_root = majit_metainterp::blackhole::GuardExcRoot::park(guard_exc); + // A failure reported through a retired/inlined source descr can belong to // an invalidated JitCellToken even while the outer entry token is still // installed. PyPy cannot make that namespace mistake: the live