diff --git a/pyre/bench/synth/blackhole_inlined_callee_local_after_escape.py b/pyre/bench/synth/blackhole_inlined_callee_local_after_escape.py new file mode 100644 index 00000000000..ef0216bb9e4 --- /dev/null +++ b/pyre/bench/synth/blackhole_inlined_callee_local_after_escape.py @@ -0,0 +1,47 @@ +# Guard for what an adopted multi-frame blackhole chain owes its inner levels. +# +# An inlined callee assigns a local, the frame then escapes through a residual +# `sys._getframe()`, and an attribute read POSITIONED AFTER that escape reads the +# local back. The read is executed by the blackhole, not by the walk, so the +# shape holds the adopt to two separate obligations and fails differently on +# each: +# +# * every LOAD_FAST lowers to `getarrayitem_vable_r` on the level's own frame +# array, so a level whose locals were left unpublished resumes `tb` as null +# and the attribute read faults in `object_getattr_miss` -- a hard SIGSEGV +# (rc=139) with no output at all; +# * the traceback the callee stored has to name the frame the callee runs on, +# so a walk-time node anchored on any other object prints `False` here while +# still exiting 0. +# +# The second is the quieter one and the reason the assertion is an identity +# rather than a liveness check. Both need the escape to happen inside an +# INLINED callee: the same shape through the single-frame arm was always +# correct. +# +# Deliberately carries no `# pyre-check: max-pypy-ratio=` header: this guards an +# output, and the forcing read makes it a poor perf subject. +import sys + +N = 20000 + + +def catches_here(i): + try: + raise ValueError(i) + except ValueError as e: + tb = e.__traceback__ + f = sys._getframe() + return (tb.tb_frame is f, tb.tb_lineno - f.f_code.co_firstlineno) + + +def drive(): + seen = set() + k = 0 + while k < N: + seen.add(catches_here(k)) + k += 1 + return sorted(seen) + + +print(drive()) diff --git a/pyre/bench/synth/getframe_inline_subwalk_multiframe.py b/pyre/bench/synth/getframe_inline_subwalk_multiframe.py index 88652f8a699..cdd10f62b4c 100644 --- a/pyre/bench/synth/getframe_inline_subwalk_multiframe.py +++ b/pyre/bench/synth/getframe_inline_subwalk_multiframe.py @@ -9,9 +9,10 @@ # residual frame rather than the walked frame, and # `try_adopt_multi_frame_blackhole`'s chain-root identity gate declined it. What # that decline wanted was the `jit.virtual_ref` emit at the inline push -# (`executioncontext.py:89`); `walker_ec_enter` / `walker_ec_leave` landed it. -# The gate no longer fires here: the shape now adopts once per build and returns -# the same result as before. This fixture pins that result. +# (`executioncontext.py:89`); `walker_ec_enter` / `walker_ec_leave` landed it, so +# the chain-root gate no longer fires here, and with every level resumable from +# the concrete frame it owns, the chain is adopted rather than replayed. This fixture pins the +# result across that adopt. # # One `sys._getframe(1)` level does NOT reach the build: the chain needs a # residual level under the walked frame and an inlined level under that, so @@ -21,10 +22,10 @@ # repro at all. # # The multi-frame image is built unconditionally when the latch conditions hold, -# so this is both an output guard and build-path coverage: 5 builds, and now 5 -# adopts with zero chain-root declines (`PYRE_FBW_DEBUG_ABORT=1` prints both -# tallies; the other 5 escapes in the run have `inline_subwalk=false` and take -# the single-frame arm). +# so this is both an output guard and build-path coverage: 5 builds, 5 adopts, +# and zero declines of any kind (`PYRE_FBW_DEBUG_ABORT=1` prints every tally; +# the other 5 escapes in the run have `inline_subwalk=false` and take the +# single-frame arm, which adopts too). # # What the decline used to hold back, measured by lifting it before the # execution-context push landed: the resumed chain shifted every diff --git a/pyre/bench/synth/getframe_while_escaping_read_frame_identity.py b/pyre/bench/synth/getframe_while_escaping_read_frame_identity.py index 3a510303325..99ea0eefadb 100644 --- a/pyre/bench/synth/getframe_while_escaping_read_frame_identity.py +++ b/pyre/bench/synth/getframe_while_escaping_read_frame_identity.py @@ -1,6 +1,6 @@ # pyre-check: max-pypy-ratio=159 # The frame-identity read the multi-frame blackhole adopt commits, and the -# regression guard for making that path unconditional. +# regression guard for the path that commits it. # # The walk executes the forcing residual CONCRETELY, and an inline push never # runs the interpreter's call sequence. Before `walker_ec_enter` / @@ -18,12 +18,15 @@ # A `_gf(1)` reading `f_locals` on that shape raised `KeyError` for any caller # local, for the same reason and not because outer locals go unmaterialized. # -# Both answers are correct now, with the adopt committing rather than declining -# (`PYRE_FBW_DEBUG_ABORT=1` prints one `adopted multi-frame terminal` per -# iteration that latches, and no `chain rooted at` decline). Note the read has -# to be the ESCAPING call: once the escape has happened, a `sys._getframe(1)` -# executed inside the blackhole was always correct, because the chain publishes -# each level's frame as it runs. +# Both answers are correct now, and they come from the adopt: with every level +# resumable from the concrete frame it owns, the chain is adopted rather than +# replayed +# (`PYRE_FBW_DEBUG_ABORT=1` prints 10 `BUILT multi-frame` and 10 `adopted +# multi-frame terminal`, with no decline of any kind), so this fixture is once +# again the discriminator for the identity answer rather than a guard on the +# replay that stood in for it. Note the read has to be the ESCAPING call: once +# the escape has happened, a `sys._getframe(1)` executed inside the blackhole was +# always correct, because the chain publishes each level's frame as it runs. import sys _gf = sys._getframe diff --git a/pyre/bench/synth/getframe_while_inlined_callee_subwalk.py b/pyre/bench/synth/getframe_while_inlined_callee_subwalk.py index 60b3905c08c..6a15493f6e8 100644 --- a/pyre/bench/synth/getframe_while_inlined_callee_subwalk.py +++ b/pyre/bench/synth/getframe_while_inlined_callee_subwalk.py @@ -7,7 +7,8 @@ # 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. +# site, and build_multi_frame_miframe then produces a depth-2 image the adopt +# takes. # # The shape below is load-bearing, not incidental: # - `while`, not `for`, per the decline above; diff --git a/pyre/gate-triage.md b/pyre/gate-triage.md index 7a1596152da..a3f95eb1f02 100644 --- a/pyre/gate-triage.md +++ b/pyre/gate-triage.md @@ -411,21 +411,99 @@ context, which is what `walker_ec_enter` / `walker_ec_leave` do; the right that an inline-push `enter` is the prerequisite, and wrong only about which check it gated. -**Resolved 2026-07-30.** With that push landed, the same fixture reports -`30000 30000 0 0` — zero wrong frames — under CPython and under pyre, with 10 -`adopted multi-frame terminal` events and no chain-root decline, so the adopt -commits rather than declining. The gate was flipped default-ON and then retired -outright (§3); the acceptance test became the regression guard for the -unconditional path. `synth/getframe_inline_subwalk_multiframe`, whose header had -recorded that the chain-root identity gate declined its shape, now measures 5 -builds, 5 adopts, zero declines, and the same output as before. +**Identity half resolved 2026-07-30.** With that push landed, the same fixture +reports `30000 30000 0 0` — zero wrong frames — under CPython and under pyre, and +the chain-root identity gate no longer declines these shapes. The gate was +flipped default-ON and then retired outright (§3). + +**But the flip was not sound.** A SECOND blocker was recorded in +`try_adopt_multi_frame_blackhole`'s own comment and missed when the flip was +verified: only frame 0's locals were published, so an inlined callee's frame array +kept its pre-sub-walk contents while every LOAD_FAST reads that array. With the +flip live, an inlined callee that stores `e.__traceback__` and then reads an +attribute off it resumed the local as null and faulted in `object_getattr_miss` +— a hard SIGSEGV, reproduced at `0f9c371b63`. Two PR reviewers flagged it +independently, and `synth/blackhole_inlined_callee_local_after_escape` pins the +crashing shape. It is closed by mirroring an inlined MIFrame's standard-vable +writes onto that level's own concrete red frame at the time they are made +(`current_inline_concrete_frame`, `store_live_frame_static_int`), so a level is +resumable from the frame it already owns rather than from a publication step +that runs at the adopt — one red frame per MIFrame, which is the shape the +codewriter's `getarrayitem_vable_r` lowering assumes. + +**A THIRD blocker sits under the second.** With the levels resumable and no +crash, the fixture still returned a silently wrong `[(False, 2), (True, 2)]` +against `[(True, 2)]`, at exactly one wrong iteration per adopt. At every +mismatch the escaping `sys._getframe()` returned the chain's level-1 frame +(`per_frame[1]`, the frame the seed built and the sub-walk runs on) while the +traceback named a *different* frame object for the same invocation. The producer +is `record_inline_traceback_for_recording`: the walk-time concrete traceback node +for an inlined level was anchored on a frame the hook `createframe_obj`s from the +promoted code and globals. That hook predates the seed, and the level now has a +real frame — the same object the EMITTED node already names, since +`traceback_node_site` resolves its frame operand from the level's frame register. +So the walk and the compiled run disagreed, and only the walk's answer is +committed by an adopt, which is why the corpus saw it exactly once per adopt and +never in steady state. `record_inline_application_traceback` now anchors the +concrete node on that frame and falls back to the fabricating hook only for a +level inlined without one. + +Anchoring on the real frame moves one obligation along with the node. The +fabricated frame carried the raise coordinate because the hook stamped it, while +the level's own frame carries the entry sentinel: the recording walk does not +make `dispatch_bytecode`'s per-opcode `last_instr` store, and a frame that leaves +by the exception never reaches an exit that would publish one either, so +`f_lineno` answers the `def` line. `synth/exception_traceback_frame_lineno` +reads exactly that, as a second `('raises_out', 1, 0)` shape beside the correct +`('raises_out', 1, 1)`. The anchor therefore makes the same store the blackhole +already makes for its replay in `publish_last_instr_at_live_marker`. + +**All three are closed, and the arm adopts.** +`synth/getframe_inline_subwalk_multiframe` measures 5 builds / 5 adopts / 0 +declines, `..._while_escaping_read_frame_identity` 10 / 10 / 0 and +`..._while_inlined_callee_subwalk` 5 / 5 / 0, all with unchanged output, and +`synth/blackhole_inlined_callee_local_after_escape` matches the reference. + +**The RUNTIME half of that anchor was investigated and declined.** Only the +walk-time record was moved onto the level's own frame; the `emit_runtime` arm of +`record_inline_application_traceback` still emits the frame-fabricating hook. +Two things came out of measuring it, and both are worth keeping: + +*It is unreachable.* An lldb breakpoint on that arm's own call-descr +construction counts zero hits across the 43 corpus exception fixtures and 15 +hand-built probes, corroborated by a `MAJIT_LOG` scan finding no call with the +hook's `[Ref, Ref, Ref, Int, Int]` signature in any dumped trace. The mechanism +is that `record_prepend_application_traceback` never declines: `emit_runtime` is +its negation, and the `exc.is_constant()` arm it would decline on is suppressed +because every raising residual assigns `class_of_last_exc_is_const = false` +immediately before `walker_record_guard_exception` reads it. So the fabricating +hook reaches no compiled traceback today — it is still called, but only from the +walk's own no-frame fallback inside a bridge sub-walk. + +*Porting it would break a documented allocation contract.* The obvious port — +emit the pointer-taking hook with the level's frame operand, the shape the +top-level sibling already uses — cannot be applied here. Every frame that +reaches `record_application_traceback` today is a non-moving oldgen block, which +is exactly what `w_pytraceback_new` relies on when it roots `w_next` and `w_code` +but deliberately not `frame`. The top-level sibling passes the standard +virtualizable, the walk passes a `FrameBox`, and the fabricating hook passes its +own `createframe_obj` frame — all oldgen. A compiled trace's inlined callee +frame is not: it is the trace's own `NewWithVtable`, which the GC rewriter lowers +to a nursery allocation. Handing that to the recorder would hold a movable +pointer across the parking allocation inside `w_pytraceback_new` and store a +pre-move address into `PyTraceback.frame`. The port therefore needs the +root-and-reload shape on the recorder first, which also covers the same +pre-existing exposure on its `w_next` argument. 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 was measured and did not: the +Everything else that was thought to block the flip was measured and did not — +except the two gaps above, which the sweeps below did not cover because no corpus +fixture assigned a local in an inlined callee and read it back after the escape, +the one shape that reaches both: the full corpus was **336/336 with the gate on (dynasm) and 336/336 with it off (cranelift)** at the time, the blast radius is exactly `inline_subwalk = true` at a vable escape (the latch is an `if`/`else if` whose single-frame arm requires diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index 331cf0cf410..3252621bb80 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -597,13 +597,69 @@ fn record_inline_application_traceback( return; } if execute_concrete { - majit_metainterp::record_inline_application_traceback_for_recording( - exc_ptr as usize as i64, - consts.w_code as i64, - consts.w_globals as i64, + // `pytraceback.py:104 record_application_traceback(space, operror, + // frame, last_instruction)` anchors the node on the frame that is + // executing. This level has one whenever it was seeded: the sub-walk + // runs the callee on it, and the emitted node names the very same + // object, since [`traceback_node_site`] resolves its frame operand from + // this register and the seed stamps the concrete frame onto that + // operand's box (`inline_call.rs`). + // + // Only an unseeded level falls through to the frame-fabricating hook. + // It has no frame at all, so a node built from the promoted code / + // globals is the only one available — but it names an object nothing + // else can reach, and a `sys._getframe()` in the same handler answers + // the seeded frame, so `tb.tb_frame is sys._getframe()` reads False. + // That stays invisible only while the walk's concrete effects are + // discarded and the iteration replayed; a level whose effects are + // committed, as the multi-frame blackhole adopt commits them, shows it. + // + // Resolving the python pc first keeps this a choice of FRAME and + // nothing else: the fabricating hook drops the node outright when the + // coordinate does not map, while the pointer-taking recorder would + // substitute `frame.last_instr`. Whether an unmappable coordinate + // should still contribute a node is a separate question from which + // frame the node names. + let node_frame = crate::state::python_pc_for_jitcode_pc_public( consts.jitcode_index, opcode_position as i32, - ); + ) + .and_then(|py_pc| { + concrete_portal_frame(ctx, consts.jitcode_index).map(|frame| (frame, py_pc)) + }); + match node_frame { + Some((frame_ptr, py_pc)) => { + // `dispatch_bytecode` (pyopcode.py) writes `self.last_instr` + // before every opcode so a frame read while it runs answers for + // the instruction executing. The recording walk does not make + // that store, so this level's frame still holds the entry + // sentinel, and a frame that leaves by the exception never + // reaches an exit that would publish one either: `f_lineno` + // would answer the code object's first line for the node + // anchored here. The blackhole closes the same gap for its + // replay in `publish_last_instr_at_live_marker`, and the + // fabricating hook stamps its fabricated frame for this reason. + // + // SAFETY: the portal red of this level's own register bank + // holds the concrete frame the sub-walk runs the callee on. + unsafe { + (*frame_ptr).last_instr = py_pc as isize; + } + majit_metainterp::record_application_traceback_for_recording( + exc_ptr as usize as i64, + frame_ptr as i64, + consts.jitcode_index, + opcode_position as i32, + ) + } + None => majit_metainterp::record_inline_application_traceback_for_recording( + exc_ptr as usize as i64, + consts.w_code as i64, + consts.w_globals as i64, + consts.jitcode_index, + opcode_position as i32, + ), + } } let hook = majit_metainterp::record_inline_application_traceback_hook_address(); if emit_runtime && !hook.is_null() && !exc.is_none() { @@ -620,6 +676,25 @@ fn record_inline_application_traceback( } } +/// The concrete `PyFrame` this level's portal frame register holds, or `None` +/// when the level was inlined without a materialized frame — a branchless leaf +/// leaves the register unseeded, and the walk then has no frame identity for +/// it at all. +fn concrete_portal_frame( + ctx: &WalkContext<'_, '_, Sym>, + jitcode_index: i32, +) -> Option<*mut pyre_interpreter::PyFrame> { + let jitcode = crate::state::pyjitcode_for_jitcode_index(jitcode_index)?; + let ConcreteValue::Ref(frame) = ctx + .concrete_registers_r + .get(jitcode.metadata.portal_frame_reg as usize) + .copied()? + else { + return None; + }; + (!frame.is_null()).then_some(frame as *mut pyre_interpreter::PyFrame) +} + fn recording_instruction_is_bare_reraise( ctx: &WalkContext<'_, '_, Sym>, opcode_position: usize,