Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions pyre/bench/synth/blackhole_inlined_callee_local_after_escape.py
Original file line number Diff line number Diff line change
@@ -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())
15 changes: 8 additions & 7 deletions pyre/bench/synth/getframe_inline_subwalk_multiframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
17 changes: 10 additions & 7 deletions pyre/bench/synth/getframe_while_escaping_read_frame_identity.py
Original file line number Diff line number Diff line change
@@ -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` /
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion pyre/bench/synth/getframe_while_inlined_callee_subwalk.py
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
96 changes: 87 additions & 9 deletions pyre/gate-triage.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
85 changes: 80 additions & 5 deletions pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,13 +597,69 @@ fn record_inline_application_traceback<Sym: WalkSym>(
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))
});
Comment on lines +623 to +629

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the live frame on the emitted traceback path

This selects the seeded callee frame only for execute_concrete; the runtime arm below still emits the frame-fabricating inline hook. That arm is reachable when an inlined callee explicitly raises a promoted constant exception (for example, a module-global exception instance), because record_prepend_application_traceback returns false for exc.is_constant(). The recording/adopted iteration will therefore attach the live frame here, while compiled iterations attach a different fabricated frame, so exc.__traceback__.tb_frame is sys._getframe() changes across iterations. The emitted path must preserve the same per-frame identity, or the affected shape must be declined until it can.

AGENTS.md reference: AGENTS.md:L32-L42

Useful? React with 👍 / 👎.

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() {
Expand All @@ -620,6 +676,25 @@ fn record_inline_application_traceback<Sym: WalkSym>(
}
}

/// 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<Sym: WalkSym>(
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<Sym: WalkSym>(
ctx: &WalkContext<'_, '_, Sym>,
opcode_position: usize,
Expand Down
Loading