sys: root the audit emit's livevars, and land the two _getframe commits #1112 merged without

pyre/bench/synth/getframe_inline_subwalk_multiframe.py

@@ -54,7 +54,7 @@
 
 
 def leaf(x):
-    sys._getframe(0).f_locals
+    sys._getframe(0).f_locals  # noqa: B018 — the read itself is the force under test
     return sys._getframe(2).f_locals["base"] + x
 
 

pyre/bench/synth/getframe_while_escaping_read_frame_identity.py

@@ -34,8 +34,8 @@
 # Both reads still escape — `_gf()` names `leaf_a`'s published frame and
 # `_gf(1)` names `part_b`'s portal — but only once per call now that
 # `sys._getframe` forces the frame it RETURNS and not also the top of the stack.
-# The multi-frame adopts this file exists for are unmoved at 10; what the
-# duplicate escape carried was the single-frame count, 5 -> 0, alongside
+# The multi-frame adoption count this file exists for is unmoved at 10.  What
+# the duplicate escape carried was the single-frame count, 5 -> 0, alongside
 # `part_a`'s loop compiling.
 import sys
 

pyre/bench/synth/sys_audit_hooks.py

@@ -10,9 +10,18 @@
 #   * `sys.audit` is free when no hook is installed, so the count a hook sees
 #     starts at the first `addaudithook`, not at interpreter start;
 #   * installing a hook emits `sys.addaudithook` to the hooks already there,
-#     and a `RuntimeError` out of that event means those hooks REFUSED the new
-#     one: it is dropped and the refusal does not propagate.  Any other
-#     exception does propagate.
+#     and an `Exception` out of that event means those hooks REFUSED the new
+#     one: it is dropped and the refusal does not propagate.  A `BaseException`
+#     outside `Exception` does propagate -- not exercised here, and it cannot
+#     be: a hook is installed for the life of the interpreter and the first one
+#     that raises masks every hook behind it, so reaching the propagating branch
+#     needs the hook set cleared between cases.  Upstream's own facility for
+#     that (`__pypy__._testing_clear_audithooks`, `interp_magic.py:292`) refuses
+#     to run once translated, so no app-level program on a built interpreter
+#     can get there.  The refusal below raises from app code, so it carries a
+#     real exception object and takes `error_is_exception`'s isinstance arm;
+#     the `PyErrorKind` fallback behind it answers only for an error that never
+#     materialised one, which nothing app-level can hand this event.
 #   * `__cantrace__` on a hook is honoured (the flag exists so a tracing hook
 #     can opt back in); nothing here can observe the tracing state, so only the
 #     attribute lookup path is exercised.

pyre/pyre-interpreter/src/module/sys/vm.rs

@@ -726,7 +726,16 @@ pub fn getframe(depth: i64) -> crate::PyResult {
     // force is what makes those reads see the JIT's live virtualizable fields —
     // upstream gets that ordering from the `hook_access_field` injection at
     // each field read, which pyre has relocated to this one call site.
-    audit("sys._getframe", &[current as PyObjectRef])?;
+    if audit_hooks_armed() {
+        // The wrap of the event name and the hooks themselves are both
+        // collection points, and the frame this is about to return reaches them
+        // only as a copied pointer, which the collector does not rewrite.  Root
+        // it across the emit and read the answer back out of its slot.
+        let _roots = pyre_object::gc_roots::push_roots();
+        let frame_slot = pyre_object::gc_roots::pin_roots(&[current as PyObjectRef]);
+        audit("sys._getframe", &[current as PyObjectRef])?;
+        current = pyre_object::gc_roots::shadow_stack_get(frame_slot) as *mut crate::PyFrame;
+    }
     Ok(current as PyObjectRef)
 }
 
@@ -1287,15 +1296,7 @@ pub fn register_module(ns: pyre_object::PyObjectRef) {
             if ec.is_null() {
                 return Ok(pyre_object::w_none());
             }
-            // Force the frame `topframeref` names before walking.  Kept where
-            // [`getframe`] no longer has it: this walk takes no force on the
-            // frame it ENDS at and then reads that frame's `w_globals`, so
-            // whether the force belongs here at all is a separate question from
-            // the one settled above.
-            let mut current = unsafe {
-                (*ec).gettopframe();
-                (*ec).gettopframe_nohidden()
-            };
+            let mut current = unsafe { (*ec).gettopframe_nohidden() };
             // `while (f && (_PyFrame_IsIncomplete(f) || depth-- > 0))` — the
             // post-decrement test fails immediately for a negative depth, so a
             // negative walks zero frames and reports the current module rather
@@ -1308,6 +1309,13 @@ pub fn register_module(ns: pyre_object::PyObjectRef) {
             if current.is_null() {
                 return Ok(pyre_object::w_none());
             }
+            // `w_globals` is one of the six fields `interp_jit.py:25-30`
+            // declares virtualizable, so the frame it is read off has to be
+            // materialized first.  The force belongs HERE, at the consumer, and
+            // not at the walk that reached the frame — see [`force_frame`]:
+            // forcing a walk escapes the traced virtualizable and
+            // `vable_after_residual_call` aborts the trace with ABORT_ESCAPE.
+            crate::executioncontext::force_frame(current);
             let w_globals = unsafe { (*current).w_globals };
             if w_globals.is_null() {
                 return Ok(pyre_object::w_none());
@@ -2642,32 +2650,60 @@ fn trigger_audit_events(
     w_event: pyre_object::PyObjectRef,
     args_w: &[pyre_object::PyObjectRef],
 ) -> Result<(), crate::PyError> {
-    let _roots = pyre_object::gc_roots::push_roots();
-    pyre_object::gc_roots::pin_root(w_event);
-    // Before the tuple is built, not after: its allocation can collect, and the
-    // caller's arguments are reachable only through the borrowed slice.
-    for &w_arg in args_w {
-        pyre_object::gc_roots::pin_root(w_arg);
-    }
-    let w_args = pyre_object::tupleobject::w_tuple_new(args_w.to_vec());
-    pyre_object::gc_roots::pin_root(w_args);
+    // `pin_root` copies the pointer into a shadow-stack slot and the collector
+    // rewrites THAT slot, never the local it was copied from.  So every value
+    // still needed after one of the app-level calls below is reached through
+    // its slot index, the way `baseobjspace::isinstance` does it.
+    //
+    // The event, the arguments and the hook set are one livevar set spanning
+    // three slices, so they are published together and normalized once
+    // (`gc_roots::pin_roots`): a per-value pin queries the collector after the
+    // first write, which would let a foreign collection run while the values
+    // behind it were still invisible to it.
+    //
     // A hook may install another hook, and upstream's list is replaced rather
     // than appended to, so an in-flight trigger keeps iterating the set it
-    // started with.  Snapshotting into pinned roots reproduces that and keeps
-    // every callable forwarded across the calls below.
-    let hooks_w = holder.hooks_w.clone();
-    for &w_hook in &hooks_w {
-        pyre_object::gc_roots::pin_root(w_hook);
-    }
+    // started with.  The published slots ARE that snapshot — they keep every
+    // callable forwarded across the calls below and are unaffected by a
+    // replacement of `holder.hooks_w`.
+    let _roots = pyre_object::gc_roots::push_roots();
+    let event_slot = pyre_object::gc_roots::publish_roots(&[w_event]);
+    let args_slot = pyre_object::gc_roots::publish_roots(args_w);
+    let hooks_slot = pyre_object::gc_roots::publish_roots(&holder.hooks_w);
+    let hook_count = holder.hooks_w.len();
+    pyre_object::gc_roots::normalize_roots(event_slot, 1 + args_w.len() + hook_count);
+
+    // From the published slots, not from the caller's slice: the tuple's own
+    // allocation can collect, and so can everything reached before it.
+    let items = (0..args_w.len())
+        .map(|i| pyre_object::gc_roots::shadow_stack_get(args_slot + i))
+        .collect();
+    let args_tuple_slot = pyre_object::gc_roots::shadow_stack_len();
+    pyre_object::gc_roots::pin_root(pyre_object::tupleobject::w_tuple_new(items));
 
     let ec = crate::call::getexecutioncontext() as *mut crate::executioncontext::ExecutionContext;
     // don't trace audithooks by default
     if !ec.is_null() {
         unsafe { (*ec).is_tracing += 1 };
     }
     let mut result = Ok(());
-    for &w_hook in &hooks_w {
-        let cantrace = match crate::baseobjspace::findattr(w_hook, "__cantrace__") {
+    for i in 0..hook_count {
+        let w_hook = pyre_object::gc_roots::shadow_stack_get(hooks_slot + i);
+        // `space.findattr` (`baseobjspace.py:881-888`) answers `None` for ANY
+        // non-async error out of the lookup, so a hook whose `__cantrace__`
+        // descriptor raises is simply treated as not having one.  The bare
+        // `findattr` panics on those instead, and this argument is app code, so
+        // it is the async arm alone (`error.py:62-65`; pyre carries SystemExit
+        // of that pair today) that may travel out of here.
+        let w_cantrace = match crate::baseobjspace::findattr_result(w_hook, "__cantrace__") {
+            Ok(found) => found,
+            Err(err) if err.kind == crate::PyErrorKind::SystemExit => {
+                result = Err(err);
+                break;
+            }
+            Err(_) => None,
+        };
+        let cantrace = match w_cantrace {
             None => false,
             Some(w_cantrace) => match crate::baseobjspace::is_true(w_cantrace) {
                 Ok(cantrace) => cantrace,
@@ -2680,7 +2716,13 @@ fn trigger_audit_events(
         if cantrace && !ec.is_null() {
             unsafe { (*ec).is_tracing -= 1 };
         }
-        let w_result = crate::baseobjspace::call_function(w_hook, &[w_event, w_args]);
+        let w_result = crate::baseobjspace::call_function(
+            pyre_object::gc_roots::shadow_stack_get(hooks_slot + i),
+            &[
+                pyre_object::gc_roots::shadow_stack_get(event_slot),
+                pyre_object::gc_roots::shadow_stack_get(args_tuple_slot),
+            ],
+        );
         if cantrace && !ec.is_null() {
             unsafe { (*ec).is_tracing += 1 };
         }
@@ -2716,7 +2758,17 @@ pub fn audit(event: &str, args_w: &[pyre_object::PyObjectRef]) -> Result<(), cra
     if !audit_hooks_armed() {
         return Ok(());
     }
-    audit_w(w_str_new(event), args_w)
+    // The wrap is a collection point and the arguments reach it only as copied
+    // pointers — `call_function_impl_result` reloads its own from the shadow
+    // stack for exactly that reason — so they are rooted in front of it and the
+    // emit runs off the reloaded values.
+    let _roots = pyre_object::gc_roots::push_roots();
+    let args_slot = pyre_object::gc_roots::pin_roots(args_w);
+    let w_event = w_str_new(event);
+    let args_w: Vec<pyre_object::PyObjectRef> = (0..args_w.len())
+        .map(|i| pyre_object::gc_roots::shadow_stack_get(args_slot + i))
+        .collect();
+    audit_w(w_event, &args_w)
 }
 
 /// `vm.py:481 holder.hooks_w is None`, negated.  A JIT fold that answers a call
@@ -2764,9 +2816,18 @@ fn sys_audit(args: &[pyre_object::PyObjectRef]) -> crate::PyResult {
     }
     // The `@unwrap_spec` round trip is observable: the hooks are handed the
     // `str` the unwrapped name is re-wrapped as, so a `str` subclass reaches
-    // them flattened to a plain one.
-    let event = crate::baseobjspace::str_utf8_w(w_event)?;
-    audit_w(w_str_new(event), &positional[1..])?;
+    // them flattened to a plain one.  The owned copy comes first so no borrow
+    // of `w_event` is live across the rooting below, and the arguments behind
+    // the event are rooted in front of the re-wrap because they reach it only
+    // as copied pointers — the same bracket [`audit`] takes around its own.
+    let event = crate::baseobjspace::str_utf8_w(w_event)?.to_string();
+    let _roots = pyre_object::gc_roots::push_roots();
+    let args_slot = pyre_object::gc_roots::pin_roots(&positional[1..]);
+    let w_text = w_str_new(&event);
+    let args_w: Vec<pyre_object::PyObjectRef> = (0..positional.len() - 1)
+        .map(|i| pyre_object::gc_roots::shadow_stack_get(args_slot + i))
+        .collect();
+    audit_w(w_text, &args_w)?;
     Ok(w_none())
 }
 
@@ -2781,31 +2842,41 @@ fn sys_addaudithook(args: &[pyre_object::PyObjectRef]) -> crate::PyResult {
             "addaudithook() missing 1 required positional argument",
         ));
     };
+    // The event below runs the already-installed hooks, which is app code and
+    // can collect, so the pin has to precede it — and the value stored after it
+    // has to come back out of the slot, since a relocation rewrites the slot
+    // and not this local.
+    let _roots = pyre_object::gc_roots::push_roots();
+    let hook_slot = pyre_object::gc_roots::shadow_stack_len();
+    pyre_object::gc_roots::pin_root(w_hook);
     if let Err(err) = audit("sys.addaudithook", &[]) {
         if !error_is_exception(&err) {
             return Err(err);
         }
         return Ok(w_none());
     }
-    let _roots = pyre_object::gc_roots::push_roots();
-    pyre_object::gc_roots::pin_root(w_hook);
     let holder = audit_holder();
     unsafe {
         // `holder.hooks_w = holder.hooks_w + [w_hook]` — a fresh list, so a
         // `trigger_audit_events` already iterating the old one keeps the set it
-        // started with.
-        let old = &(*holder).hooks_w;
-        let mut next = Vec::with_capacity(old.len() + 1);
-        next.extend_from_slice(old);
-        next.push(w_hook);
+        // started with.  The new slice is complete before the notification, so
+        // no borrow of `holder.hooks_w` is live across it.
+        let mut next = {
+            let old: &[pyre_object::PyObjectRef] = &(*holder).hooks_w;
+            let mut next = Vec::with_capacity(old.len() + 1);
+            next.extend_from_slice(old);
+            next
+        };
+        next.push(pyre_object::gc_roots::shadow_stack_get(hook_slot));
+        let next = next.into_boxed_slice();
         // Notification precedes the store, as `rclass.py:1010-1012
         // hook_setfield` emits `jit_force_quasi_immutable` before `setfield`.
         // The `is_installed()` fast path is `quasiimmut.py:38-41 invalidation`'s
         // null test, so an unwatched install stays lock-free.
         if (*holder).hooks_watchers.is_installed() {
             pyre_object::quasiimmut::sweep_quasi_immut_field(&(*holder).hooks_watchers);
         }
-        (*holder).hooks_w = next.into_boxed_slice();
+        (*holder).hooks_w = next;
         (*holder).hooks_armed.set(true);
     }
     Ok(w_none())

pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs

@@ -1359,8 +1359,16 @@ pub fn flush_active_frame_escape(ctx: &TraceCtx, frame: *mut pyre_interpreter::P
             // A declined full flush still escaped the virtualizable, so the
             // locals region is written anyway (`virtualizable.py:101-138
             // write_boxes` has no decline) — otherwise the callee reads an
-            // array of nulls.  That write claims no resume pc, and the undo
-            // stays armed so the legacy replay re-enters the pre-flush frame.
+            // array of nulls.  That write claims no resume pc, and NOTHING
+            // restores the pre-flush frame from here: the committed-pc
+            // walk-end leg is gated on a pc this arm never sets and the
+            // deferred leg on a flag it never arms, so the capture simply
+            // sits until [`capture_escape_flush_undo`] supersedes it or the
+            // walk-start reset drops it.  The deferred arm this arm used to
+            // carry was withdrawn once the `value-stack underflow` it was
+            // added for stopped reproducing on artefacts that pass
+            // `PYRE_LLBC_STRICT=1` — 0/10 on cranelift without it, with the
+            // whole synthetic suite and every recorded counter unmoved.
             //
             // Upstream reports the escape from the vable token state alone,
             // independent of any resume-image write.  See

pyre/pyre-jit-trace/src/state.rs

@@ -3283,10 +3283,14 @@ pub(crate) fn note_inline_subwalk_end(
     jd_no: usize,
     pos: majit_metainterp::recorder::TracePosition,
 ) {
-    majit_metainterp::mc_diag_bump(59);
+    // Below the driver lookup, matching where `note_inline_subwalk_start`
+    // bumps 58: both counters then measure an APPENDED ENTRY, so an unclosed
+    // entry left by a driverless call shows up as `ptp_push != ptp_pop` rather
+    // than hiding behind equal counts.
     let Some((driver, _)) = crate::driver::try_driver_pair() else {
         return;
     };
+    majit_metainterp::mc_diag_bump(59);
     driver
         .meta_interp_mut()
         .push_portal_trace_position(jd_no, None, pos);