Skip to content
Merged
38 changes: 38 additions & 0 deletions majit/majit-metainterp/src/blackhole.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<i64> = 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<i64> = const { std::cell::Cell::new(0) };
}
Comment on lines +310 to +324

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Do not use TLS for GC-relevant exception state.

GUARD_EXC_VALUE is directly consumed by the GC root walker, so this is GC-relevant interpreter state. Move it to the interpreter-owned root registry/stack or another established shared-root mechanism; otherwise root visibility depends on thread-local storage and can miss the parked exception.

As per coding guidelines, TLS is prohibited for GC-relevant runtime state.

🤖 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-metainterp/src/blackhole.rs` around lines 310 - 324, Replace the
thread-local `GUARD_EXC_VALUE` storage with the interpreter-owned root
registry/stack or another established shared-root mechanism used by the GC root
walker. Update the associated `grab_exc_value` handoff and root-walking access
so the parked exception remains visible across the blackhole transition without
relying on TLS.

Source: Coding guidelines


/// 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)),

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 Keep every nested guard exception rooted

When a bridge handoff deopts while an outer handoff is still active, replace removes the outer exception from the only registered root for the entire nested handoff. Restoring prev on drop is too late if the nested resume decode allocates and triggers a collection, because the outer exception remains only in raw i64 state and can become dangling. Store all active handoff exceptions in a rooted stack rather than a single replaceable cell.

AGENTS.md reference: AGENTS.md:L194-L196

Useful? React with 👍 / 👎.

}
}
}

impl Drop for GuardExcRoot {
fn drop(&mut self) {
GUARD_EXC_VALUE.with(|cell| cell.set(self.prev));
}
Comment on lines +326 to +347

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift

Nested GuardExcRoots can restore a collected pointer.

park replaces the only walked TLS slot and stores prev as an unregistered i64. During a nested handoff, the outer exception is therefore invisible to the GC; if allocation collects it, Drop restores a stale pointer. A nested park(0) has the same failure mode. Use a root stack/registry that walks and updates every active exception root.

🤖 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-metainterp/src/blackhole.rs` around lines 326 - 347, Update
GuardExcRoot::park and its Drop implementation to use a GC-visible root stack or
registry instead of storing the previous exception as an unregistered i64.
Ensure every active nested exception root is walked and updated during
collection, including nested park(0), and restore the GC-updated prior value
when each GuardExcRoot is dropped.

}

// rvmprof integration lives in the `rpython.rlib.rvmprof.cintf` analog.
Expand Down
5 changes: 5 additions & 0 deletions majit/majit-metainterp/src/jitdriver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3477,6 +3477,11 @@ impl<S: JitState> JitDriver<S> {
// 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);
Comment on lines +3480 to +3484

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Root the exception in every guard-failure handoff path.

This protects back_edge_internal, but run_back_edge_generic still drops its result after extracting result_exc and then performs allocation-capable resume work at Lines 5925 and 6013 without a GuardExcRoot. A GC during that work can reclaim or move the exception before it reaches prepare_resume_from_failure. Install the same RAII root immediately after extracting result_exc, keeping it alive through both bridge and blackhole paths.

🤖 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-metainterp/src/jitdriver.rs` around lines 3480 - 3484, Update
run_back_edge_generic to install a GuardExcRoot::park for result_exc immediately
after extracting it, and keep the RAII guard alive through the
allocation-capable bridge and blackhole resume paths, including both
prepare_resume_from_failure call sites. Preserve the existing back_edge_internal
protection and do not drop or move the guard before handoff completes.


// must_compile tick for bridge threshold counting.
if crate::majit_log_enabled() {
Expand Down
10 changes: 4 additions & 6 deletions majit/majit-metainterp/src/trace_ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
35 changes: 35 additions & 0 deletions pyre/bench/synth/getframe_while_caller_locals_across_subwalk.py
Original file line number Diff line number Diff line change
@@ -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())
35 changes: 35 additions & 0 deletions pyre/bench/synth/getframe_while_captured_frame_outlives_call.py
Original file line number Diff line number Diff line change
@@ -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
Comment on lines +16 to +22

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid rebinding the module global from leaf.

global kept triggers Ruff PLW0603. Preserve module-lifetime storage with a mutable holder (for example, kept_box[0] = _gf()) and read that holder after main() so the captured frame remains alive without the warning.

Also applies to: 34-35

🧰 Tools
🪛 Ruff (0.15.21)

[warning] 20-20: Using the global statement to update kept is discouraged

(PLW0603)

🤖 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/bench/synth/getframe_while_captured_frame_outlives_call.py` around lines
16 - 22, Replace the module-level scalar `kept` assignment in `leaf` with a
mutable holder such as `kept_box`, updating its element with `_gf()` and
removing the `global kept` declaration. Update the post-`main()` read to use the
holder so the captured frame remains alive without rebinding a module global.

Source: Linters/SAST tools



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)
67 changes: 67 additions & 0 deletions pyre/bench/synth/getframe_while_escaping_read_frame_identity.py
Original file line number Diff line number Diff line change
@@ -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 `<module>`, 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))
44 changes: 44 additions & 0 deletions pyre/bench/synth/getframe_while_inlined_callee_subwalk.py
Original file line number Diff line number Diff line change
@@ -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())
62 changes: 62 additions & 0 deletions pyre/bench/synth/getframe_while_subwalk_decline_shapes.py
Original file line number Diff line number Diff line change
@@ -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())
Loading
Loading