PyFrame drops execution_context and w_globals; the #1372 review follow-ups - #1388

`JitDriver::decode_descriptor_values`, `decode_exit_layout_values`,
`resume_layout_with_descriptor_slot_types`,
`compile::terminal_exit_layout_for_trace` and
`decode_values_with_layout` have no caller in either the lib build or
the test build.

The driver returns raw guard-failure data by design — "State restoration
and bridge/blackhole decision happen in the caller's handle_fail()" — so
the decode step these performed belongs to the caller. `pyre-jit`'s
`eval.rs` carries its own `decode_exit_layout_values` with three callers;
the copy here was a byte-identical twin of it.

None of the five carries a doc comment or an upstream citation, and no
`rpython/`/`pypy/` identifier matches their names under case- and
underscore-insensitive comparison.

The `exit_types` comment that named `decode_values_with_layout` now
names the property instead, and records that the gcmap is not a consumer
of that typing: `compute_gcmap` drops a `None` failarg before reading
its type.

Assisted-by: Claude

`Nursery::reset_range` is a safe `pub fn` that writes raw bytes at
caller-supplied addresses, and its three bounds checks were
`debug_assert!`. A release build performed none of them, so an
out-of-range `end` wrote outside the arena and a `start` greater than
`end` wrapped `end - start` into a fill of nearly `usize::MAX` bytes. It
now intersects the requested range with the arena and returns when the
result is empty; the assertions stay for debug builds.

`reserve_nursery_gap` derives `next_free` from `size_for_typeid`, which
decodes the pinned object's extent from its header. An extent that
overstates the object pushed `free` past the barrier being published in
the same block, and `Nursery::alloc` then compared against that bound
and returned memory beyond the gap. `next_free` is now clamped to
`next_top`.

`Nursery::set_free_ptr` and `set_top_ptr` keep their `debug_assert`s:
both are `unsafe fn` whose documented contract puts the bounds on the
caller.

Reported by CodeRabbit on #1372.

Assisted-by: Claude

None has a caller in either the lib build or the test build.

- `GuardOpt::set_guard` wraps a single `IndexMap::insert`.
- `visit_value`, a nested helper in the const-ptr walk, lost its caller
  while its siblings `visit_operands` / `visit_op` / `visit_op_info` keep
  theirs.
- `is_virtual_concat` and `is_virtual_slice` are `.is_some()` wrappers
  over `get_concat_info` / `get_slice_info`, which are called directly.
- `ResumeStorage::rd_consts_mut_for_gc` names
  `MetaInterp::walk_rd_consts_refs` as its only caller. That walker
  exists and is wired, but reaches the pool through
  `SharedConstPool::as_mut_vec_for_gc` directly, so the accessor sits
  beside the path rather than on it.

`Opencoder::refresh_from_gc` is left in place. It is also unreached, but
its doc calls it the required write-back point for the `_refs`
shadow-stack adaptation, and `rooted_ref_indices` is populated by
`_encode_ptr` and otherwise read only by `release_roots`. Deleting it
would remove the record of missing wiring rather than dead code.

Assisted-by: Claude

…t minor

`do_collect_nursery` sampled `any_pinned_object_kept` into a discarded
local and always announced `ExtraRootWalkKind::Minor`. The walkers that
read the kind — `walk_rd_consts_refs`, the front `TargetToken` walk, the
method cache and mapdict scans — each skip a clean area on a minor.

`collect_roots_in_nursery` derives `use_jit_frame_stoppers` from that
same flag and passes it as `walk_roots(is_minor=...)`: a pinned object
created before the previous minor is still in the nursery and was never
promoted, so the skip drops the only edge reaching it. The sampled flag
now selects the announced kind.

Assisted-by: Claude

…test items

`finish_trace_for_parity_preserves_captured_snapshots` carries a full
`assert_eq!` body but no `#[test]`, so `#[allow(dead_code)]` was silencing
the fact that it never ran. It runs and passes.

`may_force_test_lock` in `pyjitpl` has no caller in any cfg; the cranelift
backend has its own copy, which is the one every `may_force` test takes.
The `TID` constant in `a_named_field_resolves_by_name_through_an_ambiguous_offset`
is unused — that test calls `field_specs_from_layout` directly.

Assisted-by: Claude

`interp_jit.py PyFrame._virtualizable_` lists `pycode`, `valuestackdepth` and
`debugdata` as the virtualizable scalars; `ec` is a red in
`PyPyJitDriver.reds`, not a frame field. `PyFrame.execution_context` is
removed and the sites that read it now take the current activation's context,
as `space.getexecutioncontext()` does. `normalize_raise_varargs_fn` no longer
needs the frame pointer for it.

`PyFrame.w_globals` is likewise removed. Upstream `bcd8653e5ec` dropped it
when `PyCode.frame_stores_global` landed, and `InstanceRepr._parse_field_list`
skips a `_virtualizable_` name with no concrete field, so the entry produced
no scalar. `get_w_globals` reads the code object, and `FrameDebugData` carries
the snapshot the debug path needs. `PYFRAME_W_GLOBALS_OFFSET` is replaced by
`FRAME_DEBUG_DATA_W_GLOBALS_OFFSET`; the vable scalar table loses its fourth
entry and `failed_attr_cleanup` narrows to `u8`.

`majit_metainterp::backend_runtime` collects `set_jitframe_gc_type_id` and
`install_gc_standalone` behind the same feature selection that constructs
`BackendImpl`, so a frontend does not repeat the test and disagree under
workspace feature unification. `WarmRunnerDesc.make_cpu` constructs one CPU
and `MetaInterpStaticData.cpu` owns it.

Assisted-by: Claude

Records the kind each registered walker sees across two minors with a
pinned root alive. Without the preceding commit the pair reads
`[Minor, Minor]`; the second walk now announces `Major`.

Assisted-by: Claude

`pyframe.py get_w_globals` ends in `jit.promote(self.pycode).w_globals`.
The port read `self.pycode` unpromoted while the doc comment claimed the
promoted read, which was harmless only while `w_globals` was still a
virtualizable scalar on `PyFrame`. With that field gone the code object is
the sole source and nothing pins it to a trace constant, so the globals read
becomes a load the optimizer cannot fold.

The promote binds a local: `lower_promote_stmt` takes a single-ident LHS, so
a field assignment would drop it silently.

Assisted-by: Claude

`quasiimmut.py QuasiImmut` registers the owning `JitCellToken`, and
`invalidate` sets `looptoken.invalidated` and asks the CPU to activate every
still-unpatched `GUARD_NOT_INVALIDATED` that token owns. The registry held one
`Arc<AtomicBool>` per compiled artifact instead, so an invalidation reached
only the fragments registered by then. It now holds
`Arc<dyn QuasiImmutLoopToken>` and calls `invalidate_for_quasi_immut`, which
the backend implements on its own token.

`can_inline_callable` and `disable_noninlinable_function` are reached through
their typed green-key forms, so a recursive inline decision is looked up under
the key shape the warm state stores.

`Trace` gains `recorded_ops_total`, which `cut` does not rewind, and the bridge
driver's setup-abort test reads it beside `num_ops`. A body walk that recorded
operations and had them cut back to the setup position no longer answers that
test as though nothing had been recorded, so a trace-too-long abort stops
reading as a deterministic setup failure that permanently declines its source
guard.

The 80 jit-stats baseline files this moves, over 27 fixtures, are re-recorded.

Assisted-by: Claude

`collect_outer_active_boxes` reported a branch guard's kept operand-stack slot
as unsourced when the walk mirror held `OpRef::NONE` and no edge-move recovery
covered it, without consulting the virtualizable shadow. The resolution below
that report already reads the shadow for an operand-stack slot the guard pc's
color map does not claim, so the report declined a case its own resolver
handles. The decline raises `BranchGuardKeptSlotUnsourced` at a capture point a
residual has already run past, which the walk counts as
`fbw_rolled_back_with_effects`.

Report the hole only when the shadow also holds no live Ref for the semantic
slot. On `raise_reg_unbound_jitstress` the walk mirror stands at depth 1 while
the branch guard resumes at depth 3: both kept slots read NONE from the mirror
and a live `RefOp` from the shadow.

The `raise_reg_unbound_jitstress` baselines re-recorded in "jit: align
quasi-immutable ownership and recursive bridge state" are restored to their
previous values — fbw_rolled_back_with_effects 1 -> 0, loops_aborted 2 -> 1,
loops_compiled 8 -> 9.

Assisted-by: Claude

… unseeded

`replace_movable_load_global_namespace_with_frame_globals` substitutes the
codewriter's null LOAD_GLOBAL namespace placeholder with the frame's
`get_w_globals()`. For an inline sub-walk it reads that off the callee's
`portal_frame_reg`, and where that register is unseeded it returned, leaving the
placeholder standing. `try_walker_load_global_cell_fold` then declines on the
null namespace before reaching its builtins leg, the residual survives, and
`ensure_residual_call_args_bound` aborts the trace on the same unseeded frame
register as the call's third argument.

The unseeded frame does not by itself block the fold: its builtins leg already
handles `frame_ptr == 0` by deriving the builtin module from the namespace's
`__builtins__` cell. Only the null namespace does. The callee's own
`__globals__` is recorded in `inline_callee_consts`, so name it directly rather
than return. Immovable namespaces only, matching
`guard_current_frame_globals_identity` where both fold legs end: a movable one
declines there anyway, so substituting it would bake a pointer the GC may
forward into the surviving residual. The root frame is still never borrowed.

`polymorphic_binary_receiver` returns to the values it carried before "jit:
align quasi-immutable ownership and recursive bridge state": bridges_compiled
2 -> 3, guard_failures 845 -> 788, loops_aborted 1 -> 0. The
`retraces_compiled` entry those baselines gained there is kept.

Assisted-by: Claude

Twelve upstream citations this branch adds name a line number.
`scripts/check-new-line-citations.py` reports them against `origin/main`, which
now carries the check (#1408).

`warmstate.py:446/458/473/483/491/511` all fall inside `maybe_compile_and_run`;
`assembler.py:46` is `compute_gcmap`; `pyframe.py:132` is `get_w_globals`;
`interp_jit.py:25-30` is `PyFrame._virtualizable_` and `:67` is
`PyPyJitDriver.reds`.

Assisted-by: Claude

`PyFrame` no longer carries the execution context, and this launcher never
installed the one it constructs in the OS-thread locals, so
`space.getexecutioncontext()` answers null for every program it runs.
`eval_frame_plain_with_resume` then takes its context-free arm: no
`ExecutionContext::enter`/`leave`, no `call_trace`/`return_trace`, so
`sys.settrace` and the rest of the state reached through the context are
inert.

`pyrex/src/lib.rs`, `pyre-wasm/src/lib.rs` and `PyFrame::new` each install it
themselves; the comment in `PyFrame::new` named the first two as the whole set
and now names this one too.

Reported by Codex on #1388.

Assisted-by: Claude

`pycode.py frame_stores_global` reads `w_globals` and stores into it under the
GIL, so the pair is indivisible. The port did the same read and store
unsynchronized. Two threads first running one code object in different globals
can both read the null and both store, and both then answer `false` — which is
the answer that makes a frame take its globals from the code object. With
`PyFrame.w_globals` removed the loser's frame reads the winner's namespace
instead of its own, and the unsynchronized store is a data race in its own
right.

The publication is now a compare-exchange, and the answer is computed against
whichever pointer won. `w_code_get_w_globals` and `w_code_set_w_globals` reach
the slot through the same atomic view. `walk_w_globals_stamped_code_roots`
keeps its plain `&mut`: it runs with every mutator at a safepoint.

Reported by Codex on #1388.

Assisted-by: Claude

… is empty

`interp_jit.py PyPyJitDriver.reds = ['frame', 'ec']` makes the execution
context a red that the `-live-` markers force-keep at every guard, and
`setup_bridge_sym` decodes it out of the failing guard's frame-register
section. Two shapes leave it empty: a skeleton jitcode carries `u16::MAX` for
both portal red colors, and a resumed register can name no value.
`unpack_drain_exact_kind.py` reaches the second one — color 10 absent from a
guard whose live refs are `[3, 9]` — and aborted on the assertion this branch
put in its place, because dropping `PyFrame.execution_context` also dropped
the field the recovery read.

`emit_current_execution_context` records a residual `getexecutioncontext()`
instead, which is where the interpreter reads the context from. The call is
`EF_CANNOT_RAISE`, not elidable: it takes no arguments, so an elidable descr
would let the pure pass fold it to the recording thread's ExecutionContext and
every other thread entering the compiled trace would read that thread's
exception state. `jit_getexecutioncontext` wraps the accessor for the
`(i64xn) -> i64` residual ABI, as `jit_force_vref` does for `force_vref`.

The three recoveries — walk entry, `MIFrame::ensure_execution_context` and the
reconstructed bridge callee — each name themselves in the decline census, so
the count that the live red carries no call is separable from the counts that
do. The fixture records `ExecutionContext::WalkEntry: 1` and passes on dynasm,
cranelift and wasm.

Assisted-by: Claude

`short_circuit_side_effects` reads `bridges_compiled` 7 and `guard_failures`
1972 (wasm 1938) against 8 and 2104 (2092). Scaling the fixture's N by 4 and
16 reads 8/1998 and 8/2058 on both dynasm and cranelift: the eighth bridge is
acquired past this fixture's own N, so each counter moves with warm-up rather
than per iteration. 16x iterations of a per-iteration deopt would be near
31000.

Assisted-by: Claude

`frame_builtin_obj` and `FrameBox::new` can both collect, and this branch
removed `PyFrame.w_globals`, so between them the namespace argument is named
only by this function's own local: the code object retains the FIRST globals
dictionary (`pycode.py frame_stores_global`), not necessarily this one.

Push `code`, `w_globals` and the builtins module onto the root stack and read
each back after the last allocation before its use.

Assisted-by: Claude

…onstant namespace

`guard_current_frame_globals_identity` returned a plain constant comparison
whenever the sym's `frame_w_globals` was constant, emitting no guard.  On this
branch every seeding point publishes it as a constant read out of the recording
frame (`setup_call`, the vable-static republication in `trace_opcode.rs`,
`setup_reconstructed_callee_frame`), so the module-cell fold behind LOAD_GLOBAL
had nothing pinning the namespace at runtime.

`pycode.py frame_stores_global` publishes only the FIRST namespace a code object
runs under, so the constant does not distinguish a frame running that same code
object under a second `exec(code, ns)`: that frame carries its own namespace in
`debugdata` (`pyframe.py get_w_globals`) and enters the loop under the same
`(code, pc)` warm key.  `m2.hot()` returned `m1`'s total on dynasm and cranelift.

Emit what `get_w_globals` expresses.  `debugdata` is a virtualizable field, so
the walker reads it off `virtualizable_boxes` and records no op; the recorded
direction picks the arm and each arm pins it, `GuardIsnull` for the frame that
takes `promote(pycode).w_globals` and `GuardNonnull` plus a
`GETFIELD_GC_R`/`GuardValue` pair for the frame that carries an override.  The
payload address itself is per-frame, so only the value it holds is guarded.
`pyjitpl.py _establish_nullity` is the precedent for consulting the heapcache
first: every LOAD_GLOBAL in the frame reaches this fold, and re-recording the
same guard per site spends `trace_limit` on ops the optimizer then removes.

`FRAME_DEBUG_DATA_DESCR_GROUP` becomes headerless.  `StructPtrInfo::make_guards`
certifies a loaded pointer's runtime header against the descr's type id, and
this group declared none, so the GcCache minted one from the structural key --
a number naming nothing in the collector's type table, which failed every check
and cost a bridge per fold site.  `getorcreate_debug_data` also falls back to
`malloc_raw` when the owning frame is not collector-owned, so the pointer may
carry no header at all; `headerless` is the flag that keeps GUARD_GC_TYPE off
it.  The `locals()` fold reads the neighbouring `w_locals` through the same
group.

`shared_code_object_globals.py` execs one code object into two module
namespaces and reads both hot.

Assisted-by: Claude

`ITEMS_BLOCK_DESCR_GROUP` and `RBIGINT_PAIR_DESCR_GROUP` were gc-managed with
`type_id: 0`. That does not keep `StructPtrInfo::make_guards`
(`optimizeopt/info.rs`) from emitting `GUARD_GC_TYPE`: it emits one for every
gc-managed, non-headerless descr and takes the id from whatever the `GcCache`
slot for the key holds, which is how `FrameDebugData` came to be guarded
against 272 while its runtime header reads 103.

Both structs have an allocation path that returns a block with no `GcHeader`:
`alloc_items_block` (`std::alloc`, the `PYRE_GC_ITEMSBLOCK=0` and no-hook
fallback) and `alloc_rbigint_pair_nursery_collecting` /
`alloc_rbigint_pair_no_collect` (`malloc_raw`). `GUARD_GC_TYPE` reads at
`ref - GcHeader::SIZE`, so neither pointer may reach it. `ITEMS_BLOCK` has the
second reason it already carried: one descr fronting the three list strategies'
tids can name none of them.

`a_gc_managed_group_that_names_no_type_id_declares_itself_headerless` asserts
the rule over every `DECLARED_GROUPS` row.

check.py on this machine: dynasm 456 passed, cranelift 456 passed, wasm 448
passed, and the only jit-stats row that moves is `fib_recursive`, which reads
1601/7/1 here with or without this change.

Assisted-by: Claude

…answer

`emit_new_pyframe_inline_with_params` and its self-recursive twin took a
`w_globals` and discarded it. The store it used to feed was
`PyFrame.w_globals`, which this branch removed; a frame the JIT synthesizes
carries no `debugdata`, so `get_w_globals` on it answers with the code object's
published first namespace.

Both emission sites already decline a global-storing callee
(`pycode.py frame_stores_global`) before they build the frame, so no frame with
the wrong namespace is built today. What was missing is the frame shape saying
what it can represent: the parameter now feeds a check that the namespace is
the one the code object published, and the builders return `None` when it is
not. The Branch A site is past its emission boundary and aborts the walk; the
seed site declines to the residual; the reconstructed-bridge callee propagates
`None`, where the arm is unreachable because it derives the namespace from the
same published slot.

check.py on this machine: dynasm 456 passed, `shared_code_object_globals.py`
prints OK, and no jit-stats row moves.

Assisted-by: Claude

`resolve_gc_tid`'s doc said the structural `cache_key` "still resolves to the
dense tid" a `GUARD_GC_TYPE` needs. That holds for a struct the JIT allocates,
whose header the same GcCache id is written into. A struct the host allocates
under an id of its own resolves to the cache's instead, and the guard then
fails on every object — `FrameDebugData` guarded against 272 against a runtime
header of 103.

Name the precondition and point at `headerless`, which is the declaration such
a descr owes and which this function's caller checks first.

Assisted-by: Claude

The two `TopFrameRefGuard::new` calls this branch rebased onto read
`PyFrame.execution_context`, which the branch removes. They merged without a
conflict because they sit in regions the branch does not touch, and the tree
stopped compiling.

`build_jit_state` runs immediately before each of them and carries the same
context the removed field held.

Assisted-by: Claude

fib_recursive reads bridges_compiled=7, loops_aborted=1, guard_failures=1601
and fbw_blackhole_adopted_multi_frame=1 on all three backends here. The
committed 8/0/1600/0 reproduces on none of them, nor on the three CI hosts.

The added abort is `blackhole_if_trace_too_long` on the bridge for trace 3
guard 6. `find_biggest_function` names fib's own function-entry green key,
`disable_noninlinable_function_for_key` marks it, and the retry compiles that
bridge at 919 recorded ops. origin/main, built from the same base with the
same LLBC artefacts, compiles that bridge at 4091 ops and records a further
trace whose bridge adds 1195: 10125 recorded bridge ops against this branch's
6777.

Wall clock, 15 interleaved runs of each binary on this box:
origin/main min=0.310 median=0.340 mean=0.347;
this branch min=0.290 median=0.310 mean=0.324.

Assisted-by: Claude

`pycode_green_keys_preserve_wrapper_identity` calls `green_key_from_pycode`,
`green_key_typed_from_pycode` and `make_green_key`, all of which now take
`is_being_profiled` between the pc and the code operand.

Assisted-by: Claude

`green_key_from_pycode`'s `make_green_key` call and the
`note_inline_subwalk_start` typed-key argument each exceed the line width once
`is_being_profiled` is in the argument list.

Assisted-by: Claude

`assemble_peeled_trace_with_jump_args` appends a body-live, preamble-defined
box to the loop LABEL without a matching `used_boxes` entry, so
`inline_short_preamble` cannot produce it and a bridge closing onto that LABEL
arrives one arg short of `target_arglocs`. `compile_bridge` gives such a bridge
up.

The appended box is reachable again when it is a virtualizable static scalar:
the vable input layout is `[frame, static scalars.., array items..]`, so the
slot is `GetfieldGc*(frame, descr)` -- the load the preamble performed before
the optimizer folded it onto the seeded slot. `TargetToken` records that
`(opcode, descr)` pair per appended arg and `jump_to_existing_trace_impl`
emits the loads from JUMP arg 0. The list is all-or-nothing, since the recipes
rebuild a contiguous LABEL tail and a partial list makes the close overshoot,
and it is rewritten on every assembly so a recompiled token does not inherit a
stale one. An append with no recipe still reaches the giveup.

Reached here because `PyFrame.debugdata`'s `index_in_parent` is 4 on this
branch and 5 on main while `OptVirtualize` seeds that slot at 4 on both, so
the merge-point `GetfieldGcR(frame, debugdata)` recorded by
`record_portal_debugdata_guard` folds onto the seeded slot here and passes
through unfolded there. 68 dynasm and 60 cranelift fixtures failed on the
arity giveup; `store_global_hot` read `bridges_compiled=0 loops_aborted=494
guard_failures=98985` against a `bridges_compiled=2 loops_aborted=0
guard_failures=401` baseline, and reads the baseline again with this change.

check.py dynasm 470/470, cranelift 470/470, wasm 462/462, parity all pass,
`cargo test --workspace --features dynasm` green. No `.jitstats` baseline
moved.

Assisted-by: Claude

`FRAME_DEBUG_DATA_DESCR_GROUP`'s accessors name a field by its position in the
group's field list, so a field inserted ahead of one re-points it at its
neighbour and still compiles. `frame_debug_data_w_globals_descr` carried an
offset assert; `w_locals` and `w_extra_locals` did not.

Assisted-by: Claude