gc: put the per-thread GC box behind a gc_box feature and gate its remaining dynasm probes

majit/majit-backend-cranelift/Cargo.toml

@@ -33,6 +33,7 @@ dynasm = ["majit-metainterp/dynasm"]
 cranelift = ["majit-metainterp/cranelift"]
 
 [dev-dependencies]
+majit-gc = { workspace = true, features = ["gc_box"] }
 majit-ir = { workspace = true, features = ["test-support"] }
 majit-metainterp = { workspace = true }
 smallvec = { workspace = true }

majit/majit-backend-dynasm/Cargo.toml

@@ -21,5 +21,6 @@ indexmap = { workspace = true }
 rustc-hash = { workspace = true }
 
 [dev-dependencies]
+majit-gc = { workspace = true, features = ["gc_box"] }
 majit-ir = { workspace = true, features = ["test-support"] }
 majit-trace = { workspace = true }

majit/majit-backend-wasm/Cargo.toml

@@ -31,6 +31,7 @@ wasm-encoder = { workspace = true }
 wasm-bindgen = { workspace = true, optional = true }
 
 [dev-dependencies]
+majit-gc = { workspace = true, features = ["gc_box"] }
 majit-ir = { workspace = true, features = ["test-support"] }
 wasmparser = { workspace = true }
 smallvec = { workspace = true }

majit/majit-backend-wasm/src/lib.rs

@@ -288,40 +288,96 @@ pub fn jit_call_area_addr() -> usize {
     &JIT_CALL_AREA as *const _ as usize
 }
 
-thread_local! {
-    /// llmodel.py self.gc_ll_descr — owned by the active wasm
-    /// backend on this thread. Stored as a thread-local so the
-    /// backend-agnostic `majit_gc::ActiveGcGuardHooks` shims can
-    /// reach the live allocator without taking a wasm dependency.
-    /// Mirrors `cranelift::compiler::CRANELIFT_ACTIVE_GC` and
-    /// `dynasm::runner::DYNASM_ACTIVE_GC` — RPython's
-    /// `cpu.gc_ll_descr` parity, single-slot per thread.
-    static WASM_ACTIVE_GC: RefCell<Option<Box<dyn GcAllocator>>> = const { RefCell::new(None) };
-    /// Raw mirror of the boxed allocator, read by `wasm_gc_owns_object`'s
-    /// reentrant fallback: the interpreter-safepoint major holds the
-    /// `WASM_ACTIVE_GC` mutable borrow while extra-root walkers ask whether a
-    /// slot is GC-managed, so that query routes through the raw pointer instead
-    /// of a second borrow. Mirrors `dynasm::runner::DYNASM_ACTIVE_GC_RAW`.
-    static WASM_ACTIVE_GC_RAW: std::cell::Cell<Option<*mut dyn GcAllocator>> =
-        const { std::cell::Cell::new(None) };
+/// The per-thread GC box, and the accessors every trampoline reaches it through.
+///
+/// `gc.py:30` `GcLLDescription.__init__` holds `self.gcdescr` as a plain field
+/// on the backend descriptor — there is no per-thread allocator upstream — so
+/// this cell is scaffolding, not a ported structure. Only `install_gc_box`
+/// fills it and only tests reach that; the production build goes through
+/// `install_gc_standalone` and allocates from the `gc_sync` singleton.
+///
+/// Every accessor opens with `majit_gc::gc_box_installed()`, which without
+/// `majit-gc/gc_box` is a constant `false` — so in a production build each one
+/// folds to `None`, the thread-local becomes unreachable, and the trampolines
+/// call `gc_sync` directly. The gate lives in `majit-gc` because a Cargo
+/// feature is per-crate: this crate cannot `#[cfg]` on a feature of its
+/// dependency, so the box is eliminated by the optimizer rather than by
+/// conditional compilation. Mirrors `majit-backend-dynasm/src/runner.rs`'s
+/// `gc_box`.
+mod gc_box {
+    use super::{GcAllocator, RefCell};
+
+    thread_local! {
+        /// llmodel.py self.gc_ll_descr — owned by the active wasm backend on
+        /// this thread. Stored as a thread-local so the backend-agnostic
+        /// `majit_gc::ActiveGcGuardHooks` shims can reach the live allocator
+        /// without taking a wasm dependency. RPython's `cpu.gc_ll_descr`
+        /// parity, single-slot per thread.
+        static WASM_ACTIVE_GC: RefCell<Option<Box<dyn GcAllocator>>> =
+            const { RefCell::new(None) };
+        /// Read-only mirror of the box address: the interpreter-safepoint major
+        /// holds the mutable borrow while extra-root walkers ask whether a slot
+        /// is GC-managed, so that query routes through the raw pointer instead
+        /// of taking a second borrow.
+        static WASM_ACTIVE_GC_RAW: std::cell::Cell<Option<*mut dyn GcAllocator>> =
+            const { std::cell::Cell::new(None) };
+    }
+
+    /// `&mut` access to this thread's GC box, for allocation, write barriers
+    /// and collection. `None` means there is no box, and the caller runs its
+    /// `gc_sync` path instead.
+    pub(super) fn with_mut<R>(f: impl FnOnce(&mut dyn GcAllocator) -> R) -> Option<R> {
+        if !majit_gc::gc_box_installed() {
+            return None;
+        }
+        WASM_ACTIVE_GC.with(|cell| {
+            let mut guard = cell.borrow_mut();
+            let raw: *mut dyn GcAllocator = guard.as_deref_mut()?;
+            // SAFETY: `guard` holds the borrow for the whole `f` call and
+            // these are non-reentrant top-level trampolines, so the reborrow
+            // is exclusive and outlives `f`.
+            Some(f(unsafe { &mut *raw }))
+        })
+    }
+
+    /// Read-only access that tolerates being reached from inside a collection:
+    /// when an in-progress mutation already holds the mutable borrow, read the
+    /// same allocator through the raw mirror rather than taking a second one.
+    pub(super) fn with_reentrant_ref<R>(f: impl FnOnce(&dyn GcAllocator) -> R) -> Option<R> {
+        if !majit_gc::gc_box_installed() {
+            return None;
+        }
+        WASM_ACTIVE_GC.with(|cell| match cell.try_borrow() {
+            Ok(guard) => guard.as_deref().map(f),
+            // SAFETY: the mirror is published and cleared under the same
+            // borrow as the box itself, so a non-null value points at the
+            // live allocator, and this query only reads it.
+            Err(_) => WASM_ACTIVE_GC_RAW.with(|raw| raw.get().map(|p| f(unsafe { &*p }))),
+        })
+    }
+
+    /// Whether this thread holds a box at all.
+    pub(super) fn present() -> bool {
+        majit_gc::gc_box_installed() && WASM_ACTIVE_GC.with(|cell| cell.borrow().is_some())
+    }
+
+    /// Store `gc` as this thread's box, publishing the raw mirror with it.
+    pub(super) fn store(gc: Box<dyn majit_gc::GcAllocator>) {
+        WASM_ACTIVE_GC.with(|cell| {
+            let mut guard = cell.borrow_mut();
+            *guard = Some(gc);
+            let raw = guard.as_deref_mut().map(|gc| gc as *mut dyn GcAllocator);
+            WASM_ACTIVE_GC_RAW.with(|raw_cell| raw_cell.set(raw));
+        });
+    }
 }
 
+/// Read-only GC query for the guard hooks and codegen helpers. The box arm is
+/// reentrancy-tolerant because these can fire during a collection's extra-root
+/// walk, which is also why the singleton arm is the reentrant read.
 fn with_wasm_active_gc<R>(f: impl Fn(&dyn GcAllocator) -> R) -> Option<R> {
-    if !majit_gc::gc_box_installed() {
-        return majit_gc::gc_sync::is_initialized()
-            .then(|| majit_gc::gc_sync::gc_query_reentrant(|gc| f(gc)));
-    }
-    match WASM_ACTIVE_GC.with(|cell| cell.try_borrow().map(|g| g.as_deref().map(|gc| f(gc)))) {
-        Ok(Some(r)) => return Some(r),
-        Ok(None) => {}
-        Err(_) => {
-            if let Some(ptr) = WASM_ACTIVE_GC_RAW.with(|raw| raw.get()) {
-                // SAFETY: the raw mirror points at the same live test box whose
-                // mutable borrow is held by the in-progress mutation. This is a
-                // read-only reentrant query, so it does not create a second &mut.
-                return Some(f(unsafe { &*ptr }));
-            }
-        }
+    if let Some(r) = gc_box::with_reentrant_ref(&f) {
+        return Some(r);
     }
     if majit_gc::gc_sync::is_initialized() {
         return Some(majit_gc::gc_sync::gc_query_reentrant(|gc| f(gc)));
@@ -335,15 +391,8 @@ fn with_wasm_active_gc<R>(f: impl Fn(&dyn GcAllocator) -> R) -> Option<R> {
 /// `None` so callers keep their non-GC fallback. Top-level mutator/
 /// blackhole trampolines, never inside a collection, so `gc_op` is correct.
 fn with_wasm_active_gc_mut<R>(f: impl FnOnce(&mut dyn GcAllocator) -> R) -> Option<R> {
-    if majit_gc::gc_box_installed() && WASM_ACTIVE_GC.with(|cell| cell.borrow().is_some()) {
-        return WASM_ACTIVE_GC.with(|cell| {
-            let mut guard = cell.borrow_mut();
-            let raw: *mut dyn GcAllocator = guard.as_deref_mut()?;
-            // SAFETY: `guard` holds the borrow for the whole `f` call and
-            // these are non-reentrant top-level trampolines, so the
-            // reborrow is exclusive and outlives `f`.
-            Some(f(unsafe { &mut *raw }))
-        });
+    if gc_box::present() {
+        return gc_box::with_mut(f);
     }
     if majit_gc::gc_sync::is_initialized() {
         return Some(majit_gc::gc_sync::gc_op(|gc| f(gc)));
@@ -409,12 +458,7 @@ fn install_gc_box(gc: Box<dyn majit_gc::GcAllocator>) {
     majit_gc::disarm_published_nursery();
     majit_gc::note_gc_box_installed();
     let supports_guard_gc_type = gc.supports_guard_gc_type();
-    WASM_ACTIVE_GC.with(|cell| {
-        let mut guard = cell.borrow_mut();
-        *guard = Some(gc);
-        let raw = guard.as_deref_mut().map(|gc| gc as *mut dyn GcAllocator);
-        WASM_ACTIVE_GC_RAW.with(|raw_cell| raw_cell.set(raw));
-    });
+    gc_box::store(gc);
     register_active_hooks(supports_guard_gc_type);
 }
 
@@ -1015,43 +1059,15 @@ fn wasm_active_gc_write_barrier(obj: GcRef) {
 
 /// Host-side `is_managed_heap_object` trampoline.
 ///
-/// This read-only ownership query can fire reentrantly from an extra-root
-/// walker mid-collection (the interpreter-safepoint major holds the
-/// `WASM_ACTIVE_GC` mutable borrow while asking whether a slot is
-/// GC-managed), so:
-///   - `Ok(Some)`: a test box is present and free-borrowable → use it.
-///   - `Ok(None)`: no box (production) → route to `gc_sync` reentrantly.
-///   - `Err`: the test box's mutable borrow is held by an in-progress
-///     mutation → reach it through the raw mirror (read-only), or fall
-///     back to `gc_sync` if there is no box at all.
+/// This query can fire reentrantly from an extra-root walker mid-collection
+/// (the interpreter-safepoint major holds the box's mutable borrow while
+/// asking whether a slot is GC-managed), so both arms are read-only.
 fn wasm_gc_owns_object(addr: usize) -> bool {
-    if !majit_gc::gc_box_installed() {
-        return majit_gc::gc_sync::is_initialized()
-            && majit_gc::gc_sync::gc_query_reentrant(|g| g.is_managed_heap_object(addr));
-    }
-    match WASM_ACTIVE_GC.with(|cell| {
-        cell.try_borrow()
-            .map(|g| g.as_deref().map(|gc| gc.is_managed_heap_object(addr)))
-    }) {
-        Ok(Some(r)) => r,
-        Ok(None) => {
-            if majit_gc::gc_sync::is_initialized() {
-                majit_gc::gc_sync::gc_query_reentrant(|g| g.is_managed_heap_object(addr))
-            } else {
-                false
-            }
-        }
-        Err(_) => WASM_ACTIVE_GC_RAW.with(|raw| match raw.get() {
-            Some(ptr) => unsafe { (*ptr).is_managed_heap_object(addr) },
-            None => {
-                if majit_gc::gc_sync::is_initialized() {
-                    majit_gc::gc_sync::gc_query_reentrant(|g| g.is_managed_heap_object(addr))
-                } else {
-                    false
-                }
-            }
-        }),
+    if let Some(r) = gc_box::with_reentrant_ref(|gc| gc.is_managed_heap_object(addr)) {
+        return r;
     }
+    majit_gc::gc_sync::is_initialized()
+        && majit_gc::gc_sync::gc_query_reentrant(|g| g.is_managed_heap_object(addr))
 }
 
 pub struct WasmBackend {

majit/majit-gc/Cargo.toml

@@ -27,6 +27,16 @@ gc_stress = []
 # by default so ordinary wasm builds do not abort.
 bh_probe = []
 
+# Per-thread GC box. `install_gc_box` stores a caller-supplied `GcAllocator` in
+# a backend thread-local and every allocation, write-barrier and heap query
+# checks for it first. Only tests install one — production goes through
+# `install_gc_standalone` and allocates from the `gc_sync` singleton — so
+# without this feature `gc_box_installed()` is a constant `false` and the
+# backends' box branches fold away entirely. A crate whose tests install a box
+# must carry `majit-gc = { ..., features = ["gc_box"] }` in `dev-dependencies`;
+# installing one without it panics rather than silently missing every probe.
+gc_box = []
+
 [dependencies]
 indexmap = { workspace = true }
 majit-ir = { workspace = true }

majit/majit-gc/src/lib.rs

@@ -1626,20 +1626,48 @@ pub fn supports_guard_gc_type() -> bool {
 /// `malloc` against the single translation-time `gcdata`
 /// (`gctransform/framework.py`) — so the box is pyre-side test scaffolding, and
 /// the queries below let the allocation path it does not serve skip it.
+///
+/// Only compiled when a box can exist at all, i.e. under `gc_box`; see
+/// [`gc_box_installed`] for what the other build spells instead.
+#[cfg(any(test, feature = "gc_box"))]
 static GC_BOX_INSTALLED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false);
 
 /// Record that a backend has installed a per-thread GC box. Called from each
 /// backend's box-installing entry point.
+#[cfg(any(test, feature = "gc_box"))]
 pub fn note_gc_box_installed() {
     GC_BOX_INSTALLED.store(true, std::sync::atomic::Ordering::Release);
 }
 
+/// A build without `gc_box` answers every [`gc_box_installed`] query with a
+/// constant `false`, so a box installed here would be invisible to all of them
+/// and its owner would silently allocate out of the singleton instead. Refuse
+/// loudly rather than let that read as a pass.
+#[cfg(not(any(test, feature = "gc_box")))]
+pub fn note_gc_box_installed() {
+    panic!(
+        "a per-thread GC box was installed in a build without the `gc_box` feature; \
+         production installs the backend standalone (`install_gc_standalone`), and a \
+         test that needs a box must depend on `majit-gc` with `features = [\"gc_box\"]`"
+    );
+}
+
 /// Whether any thread may own a per-thread GC box. See [`GC_BOX_INSTALLED`].
+#[cfg(any(test, feature = "gc_box"))]
 #[inline]
 pub fn gc_box_installed() -> bool {
     GC_BOX_INSTALLED.load(std::sync::atomic::Ordering::Acquire)
 }
 
+/// No backend can hold a box in this build, so every `gc_box_installed() && …`
+/// probe folds away and the allocation, write-barrier and query trampolines
+/// call `gc_sync` directly — which is the whole shape RPython has.
+#[cfg(not(any(test, feature = "gc_box")))]
+#[inline(always)]
+pub fn gc_box_installed() -> bool {
+    false
+}
+
 // ── Host-side nursery allocation hook ───────────────────────────────
 //
 // Separate from `ActiveGcGuardHooks` because allocation is not a

pyre/bench/synth/getframe_method_call_residual_body_once.cranelift.jitstats

@@ -0,0 +1,14 @@
+bridges_compiled=0
+descr_set_absent=0
+descr_set_ambiguous=0
+descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=0
+fbw_blackhole_adopted_single_frame=10
+fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
+field_pos_attached_misplaced=0
+field_pos_spec_misplaced=0
+guard_failures=0
+internal_compile_panics=0
+loops_aborted=10
+loops_compiled=0

pyre/bench/synth/getframe_method_call_residual_body_once.dynasm.jitstats

@@ -0,0 +1,14 @@
+bridges_compiled=0
+descr_set_absent=0
+descr_set_ambiguous=0
+descr_set_stale_absent=0
+fbw_blackhole_adopted_multi_frame=0
+fbw_blackhole_adopted_single_frame=10
+fbw_rolled_back_with_effects=0
+fbw_store_journal_rollback_failed=0
+field_pos_attached_misplaced=0
+field_pos_spec_misplaced=0
+guard_failures=0
+internal_compile_panics=0
+loops_aborted=10
+loops_compiled=0

pyre/bench/synth/getframe_method_call_residual_body_once.py

@@ -0,0 +1,57 @@
+# A method-form `LOAD_ATTR` + `CALL` whose callee forces the calling frame and
+# then raises, caught in the loop body. The callee's own body is outside every
+# journal, so a walk that cannot commit makes the legacy entry replay run it a
+# second time per trace attempt: `Force.hits` climbs past the iteration count
+# while `total` and `len(seen)` — both journaled — stay correct. That asymmetry
+# is the whole oracle; a bench that only checked the loop result saw nothing.
+#
+# The shape matters, not the effect. `f.raiser()` lowers to `LOAD_ATTR` (method
+# form) followed by `CALL`, and the CALL's lowering re-enters the LOAD_ATTR
+# source segment, so the walk's py_pc floor reports the earlier opcode again
+# after the later one. That backward step is no CFG successor, so it arms the
+# out-of-order region and every following boundary reseeds the operand mirror
+# from the virtualizable shadow — which mid-expression holds the NULLs the
+# in-flight opcode already popped, and so cannot return the callable. The
+# mirror came back one slot short with the callable in the `self_or_null` slot,
+# the escape flush declined on the hole, and the portal replayed the frame from
+# its entry.
+#
+# N is sized to reach the tracing thresholds on its own rather than for
+# throughput: at 200 the loop never gets hot and the bench passes with the
+# defect in place. Measured against a binary without the fix, N=2000 reports
+# 2001 hits and N=20000 reports 20005 — the overshoot is the trace-attempt
+# count, so only the fixed value (exactly N) is stable enough to pin. No
+# `max-pypy-ratio` for the same reason `exception_reused_object_tb_not_doubled`
+# has none — this is a shape oracle, not a workload.
+#
+# Expected output: (20000, 20000, 20000)
+import sys
+
+N = 20000
+
+
+class Force:
+    hits = 0
+
+    def raiser(self):
+        _ = sys._getframe(1).f_back
+        Force.hits += 1
+        raise ValueError(1)
+
+
+f = Force()
+
+
+def main():
+    seen = []
+    total = 0
+    for i in list(range(N)):
+        try:
+            total += f.raiser()
+        except ValueError:
+            total += 1
+        seen.append(i)
+    return total, len(seen), Force.hits
+
+
+print(main())