jit, jit-trace, metainterp: the portal's activation bracket and frame-chain leave, the recursive-call exception protocol, and the inline sub-walk green key

majit/majit-metainterp/src/blackhole.rs

@@ -1215,6 +1215,24 @@
             self.entry_position,
         );
         let result_type = self.jitdrivers_sd[jdindex].result_type;
+        // The portal runner is published behind an `extern "C"` boundary and
+        // cannot unwind, so a raise inside it arrives in `BH_LAST_EXC_VALUE`
+        // instead of out of the call — where `blackhole.py:351-360`'s blanket
+        // `except Exception` catches upstream's real unwind and hands it to
+        // `handle_exception_in_frame`.  Clear the cell before the call and test
+        // it after, the protocol `check_residual_call_exception_after`
+        // documents and every `residual_call_*` / `inline_call_*` /
+        // `call_assembler_*` / `cond_call_*` family follows.  Without it the
+        // call's value is whatever the runner returned on its error path —
+        // `PY_NULL` — the frame leaves with that NULL installed as the result,
+        // and the exception is left unread in the cell for an unrelated
+        // opcode to clear or for `bhimpl_abort_permanent` to deliver at the
+        // wrong bytecode.
+        BH_LAST_EXC_VALUE.with(|c| c.set(0));
+        // The post-call position, where the codewriter put the can-raise
+        // opcode's `-live-` adjacency.  Read before the call: the arms below
+        // do not advance `position`, but the handler search starts here.
+        let post_call_position = self.position;
         match result_type {
             BhReturnType::Void => {
                 self.bhimpl_recursive_call_v(jdindex, gi, gr, gf, ri, rr, rf);
@@ -1236,6 +1254,11 @@
                 self.return_type = BhReturnType::Float;
             }
         }
+        // Upstream's raise unwinds out of `bhimpl_jit_merge_point`, so the
+        // frame does not leave on this path: the dispatch loop searches THIS
+        // frame's handlers at the post-call position first, and only a frame
+        // without one propagates.
+        check_residual_call_exception_after(self, post_call_position)?;
         Err(DispatchError::LeaveFrame)
     }
 
@@ -11885,9 +11908,12 @@
 ) -> Result<usize, DispatchError> {
     let (jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f, p) =
         read_recursive_call_args(bh, code, p);
-    bh.registers_i[code[p] as usize] = bh.bhimpl_recursive_call_i(
+    BH_LAST_EXC_VALUE.with(|c| c.set(0));
+    let result = bh.bhimpl_recursive_call_i(
         jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f,
     );
+    check_residual_call_exception_after(bh, p + 1)?;
+    bh.registers_i[code[p] as usize] = result;
     Ok(p + 1)
 }
 // blackhole.py bhimpl_recursive_call_r
@@ -11898,11 +11924,14 @@
 ) -> Result<usize, DispatchError> {
     let (jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f, p) =
         read_recursive_call_args(bh, code, p);
-    bh.registers_r[code[p] as usize] = bh
+    BH_LAST_EXC_VALUE.with(|c| c.set(0));
+    let result = bh
         .bhimpl_recursive_call_r(
             jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f,
         )
         .0 as i64;
+    check_residual_call_exception_after(bh, p + 1)?;
+    bh.registers_r[code[p] as usize] = result;
     Ok(p + 1)
 }
 // blackhole.py bhimpl_recursive_call_f
@@ -11913,11 +11942,14 @@
 ) -> Result<usize, DispatchError> {
     let (jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f, p) =
         read_recursive_call_args(bh, code, p);
-    bh.registers_f[code[p] as usize] = bh
+    BH_LAST_EXC_VALUE.with(|c| c.set(0));
+    let result = bh
         .bhimpl_recursive_call_f(
             jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f,
         )
         .to_bits() as i64;
+    check_residual_call_exception_after(bh, p + 1)?;
+    bh.registers_f[code[p] as usize] = result;
     Ok(p + 1)
 }
 // blackhole.py bhimpl_recursive_call_v
@@ -11928,8 +11960,10 @@
 ) -> Result<usize, DispatchError> {
     let (jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f, p) =
         read_recursive_call_args(bh, code, p);
+    BH_LAST_EXC_VALUE.with(|c| c.set(0));
     bh.bhimpl_recursive_call_v(
         jdindex, greens_i, greens_r, greens_f, reds_i, reds_r, reds_f,
     );
+    check_residual_call_exception_after(bh, p)?;
     Ok(p)
 }

majit/majit-metainterp/src/pyjitpl.rs

@@ -22395,6 +22395,71 @@ mod metainterp_static_data_tests {
         assert_eq!(meta.find_biggest_function(), None);
     }
 
+    /// pyjitpl.py:2817-2831 runs the too-long bookkeeping once per abort: the
+    /// reason travels on the `SwitchToBlackhole` instance and the `_interpret`
+    /// catch never re-enters the check that raised it.  This pins what a second
+    /// entry costs, because pyre reaches the same handler through a
+    /// `DispatchError` that carries no reason and so had a path back into it.
+    ///
+    /// The first run names the oversized callee and disables just that callee,
+    /// deliberately leaving the root un-marked so it can retrace without it.
+    /// It also retires the log it read, so a second run can name nothing and
+    /// takes `prepare_trace_segmenting` instead — which stamps the root with
+    /// `JC_FORCE_FINISH` + `JC_DONT_TRACE_HERE`, neither of which is ever
+    /// cleared.  The callee's size is what overflowed the trace; the root pays
+    /// for it permanently.
+    #[test]
+    fn a_second_too_long_run_segments_a_root_the_first_one_spared() {
+        // `start_tracing` opens the loop header this walk is rooted at, and
+        // its green key is the one the segmenting arm would mark.
+        const ROOT: u64 = 0;
+        const CALLEE: u64 = 0xa11;
+
+        let (mut meta, jc) = meta_with_recursive_portal();
+        start_tracing(&mut meta);
+        // One inlined callee, sized by the ops recorded between its entries.
+        meta.perform_call(jc, &[], Some(CALLEE)).unwrap_err();
+        record_ops(&mut meta, 5);
+        meta.popframe(true);
+        meta.tracing
+            .as_mut()
+            .expect("tracing is Some")
+            .set_trace_limit(0);
+
+        assert_eq!(
+            meta.blackhole_if_trace_too_long(),
+            Some(AbortReason::TooLong)
+        );
+        assert!(
+            !meta.warm_state_mut().can_inline_callable(CALLEE),
+            "the named callee is the one that gets disabled"
+        );
+        assert!(
+            !meta.warm_state_mut().should_force_finish_tracing(ROOT),
+            "the root is only asked to retrace, so it must not be force-finished"
+        );
+        assert!(
+            meta.warm_state_mut().can_inline_callable(ROOT),
+            "the root is only asked to retrace, so it must stay inlinable"
+        );
+
+        // Exactly what a second entry sees: the same over-budget trace, and a
+        // log this abort already retired.
+        assert_eq!(meta.find_biggest_function(), None);
+        assert_eq!(
+            meta.blackhole_if_trace_too_long(),
+            Some(AbortReason::TooLong)
+        );
+        assert!(
+            meta.warm_state_mut().should_force_finish_tracing(ROOT),
+            "a second run has no callee to name and segments the root instead"
+        );
+        assert!(
+            !meta.warm_state_mut().can_inline_callable(ROOT),
+            "and stamps it dont-trace-here, which nothing clears"
+        );
+    }
+
     #[test]
     fn portal_trace_positions_are_rearmed_for_each_trace() {
         // pyjitpl.py. Upstream builds a MetaInterp per tracing attempt;

majit/majit-metainterp/src/warmstate.rs

@@ -684,6 +684,17 @@ pub struct WarmEnterState {
     /// warmspot.py:110: memory_manager — generation-based loop aging.
     /// pyjitpl.py: try_to_free_some_loops calls next_generation().
     pub memory_manager: crate::memmgr::MemoryManager,
+    /// Bumped by every mutation that can change what
+    /// [`Self::maybe_compile_decision`] answers for a key it already refused
+    /// at the abort ceiling: installing a cell, which resets `abort_count` to
+    /// zero, and attaching a procedure token, which can make the key runnable
+    /// through a sibling cell in the same chain.
+    ///
+    /// It exists so a caller can cache that refusal rather than re-derive it
+    /// per back edge. The refusal is pure — it bumps a diagnostic slot and
+    /// returns `NotHot` above `decay_all_counters`, deliberately — so a cache
+    /// keyed on this counter changes nothing but the work spent reaching it.
+    cell_generation: u64,
 }
 
 /// Result of checking whether a green key is hot.
@@ -793,6 +804,7 @@ impl WarmEnterState {
                 m.max_unroll_loops = DEFAULT_MAX_UNROLL_LOOPS;
                 m
             },
+            cell_generation: 0,
         }
     }
 
@@ -899,6 +911,34 @@ impl WarmEnterState {
         self.counter.tick(bucket, self.increment_threshold)
     }
 
+    /// The counter [`Self::cell_generation`] documents.
+    pub fn cell_generation(&self) -> u64 {
+        self.cell_generation
+    }
+
+    fn bump_cell_generation(&mut self) {
+        self.cell_generation = self.cell_generation.wrapping_add(1);
+    }
+
+    /// Whether [`Self::maybe_compile_decision`] would refuse `cell_key` at the
+    /// abort ceiling.
+    ///
+    /// The condition is restated here rather than shared with that decision
+    /// because the decision reads the cell once and answers four other
+    /// questions from the same borrow, while a caller asking only this one
+    /// wants it alone. The two must agree, which
+    /// `is_ceiling_latched_agrees_with_the_decision_it_mirrors` asserts.
+    pub fn is_ceiling_latched(&self, cell_key: u64) -> bool {
+        let Some(cell) = self.cell_by_key(cell_key) else {
+            return false;
+        };
+        if cell.is_compiled() || cell.is_tracing() {
+            return false;
+        }
+        let dead_token = cell.has_seen_a_procedure_token() && cell.get_procedure_token().is_none();
+        !dead_token && cell.abort_ceiling_latched()
+    }
+
     /// The `dead_token` gate below is narrower than
     /// `warmstate.py maybe_compile_and_run`'s tokenless arm, and
     /// deliberately so. Upstream drops EVERY tokenless cell there — "it was an
@@ -1344,7 +1384,9 @@ impl WarmEnterState {
         let token = token.into();
         let cell = self.ensure_cell_by_key(cell_key);
         cell.flags &= !jc_flags::TRACING;
-        cell.set_procedure_token(token, false)
+        let previous = cell.set_procedure_token(token, false);
+        self.bump_cell_generation();
+        previous
     }
 
     /// Typed form of [`Self::attach_procedure_to_interp`].
@@ -1367,7 +1409,9 @@ impl WarmEnterState {
             .lookup_chain_with_key_mut(key)
             .expect("ensure_cell_for_key just installed a cell matching this key");
         cell.flags &= !jc_flags::TRACING;
-        cell.set_procedure_token(token, false)
+        let previous = cell.set_procedure_token(token, false);
+        self.bump_cell_generation();
+        previous
     }
 
     /// warmstate.py `cell.set_procedure_token(procedure_token, tmp=True)`.
@@ -1388,6 +1432,7 @@ impl WarmEnterState {
         let token = token.into();
         let cell = self.ensure_cell_by_key(cell_key);
         let _old = cell.set_procedure_token(token, true);
+        self.bump_cell_generation();
     }
 
     /// warmstate.py `finally: cell.flags &= ~JC_TRACING` parity —
@@ -2897,6 +2942,7 @@ impl WarmEnterState {
     /// upstream needs no equivalent because it hands the cell object itself
     /// on (warmstate.py:483/:511) and never re-derives it from a number.
     pub fn install_new_cell(&mut self, hash: u64, newcell: Option<BaseJitCell>) {
+        self.bump_cell_generation();
         let mut keep = newcell.map(Box::new);
         if let Some(cell) = &mut keep
             && cell.cell_key.is_none()
@@ -6736,4 +6782,81 @@ mod tests {
              cleanup_chain and left it in the bucket",
         );
     }
+
+    #[test]
+    fn is_ceiling_latched_agrees_with_the_decision_it_mirrors() {
+        let mut ws = WarmEnterState::new(2);
+        let cell_key = 42u64;
+        ws.ensure_cell_by_key(cell_key);
+        assert!(
+            !ws.is_ceiling_latched(cell_key),
+            "a fresh cell is not latched",
+        );
+
+        for _ in 0..MAX_TRACE_ABORT_COUNT {
+            ws.abort_tracing(cell_key, false);
+        }
+        assert!(ws.is_ceiling_latched(cell_key));
+
+        // The refusal is what this predicate stands in for, and it leaves the
+        // generation alone — otherwise a cache keyed on it would be
+        // invalidated by the very answer it is caching.
+        let generation = ws.cell_generation();
+        assert!(matches!(
+            ws.maybe_compile_decision(cell_key),
+            HotResult::NotHot
+        ));
+        assert_eq!(ws.cell_generation(), generation);
+        assert!(
+            ws.is_ceiling_latched(cell_key),
+            "the refusal did not consume the latch",
+        );
+
+        // `install_new_cell` keeps a cell that is not removable, so this one
+        // stays latched; what matters for a cache is that the generation moves
+        // anyway, because the same call drops removable cells and lets a fresh
+        // one trace in their place.
+        ws.install_new_cell(cell_key, None);
+        assert_ne!(ws.cell_generation(), generation);
+    }
+
+    #[test]
+    fn a_dead_token_is_not_a_ceiling_latch_because_the_decision_cleans_it_up() {
+        let mut ws = WarmEnterState::new(2);
+        let key = GreenKey::new(vec![7, 11]);
+        let token_num = ws.alloc_token_number();
+        ws.attach_procedure_to_interp_for_key(&key, JitCellToken::new(token_num));
+        for _ in 0..MAX_TRACE_ABORT_COUNT {
+            ws.abort_tracing_for_key(&key, false);
+        }
+        let cell = ws
+            .get_cell_for_key(&key)
+            .expect("fixture: the cell is still there");
+        assert!(
+            cell.abort_count >= MAX_TRACE_ABORT_COUNT,
+            "fixture: the ceiling must be latched",
+        );
+        let cell_key = cell.cell_key.expect("fixture: the cell carries its key");
+        assert!(
+            !ws.is_ceiling_latched(cell_key),
+            "a dead token takes the cleanup path, so the decision does NOT \
+             refuse at the ceiling and a cache must not answer for it",
+        );
+    }
+
+    #[test]
+    fn attaching_a_procedure_token_moves_the_generation_a_cache_keys_on() {
+        let mut ws = WarmEnterState::new(2);
+        let cell_key = 7u64;
+        ws.ensure_cell_by_key(cell_key);
+        let generation = ws.cell_generation();
+        let token_num = ws.alloc_token_number();
+        ws.attach_procedure_to_interp(cell_key, JitCellToken::new(token_num));
+        assert_ne!(
+            ws.cell_generation(),
+            generation,
+            "a token can make a latched key runnable through its chain, so a \
+             cache must be told",
+        );
+    }
 }

majit/majit-translate/src/memory/gctransform/framework.rs

@@ -175,6 +175,9 @@ pub const PYTHON_DISPATCH_SEEDS: &[&str] = &[
     "eval::portal_runner",
     "eval::portal_runner_dispatch",
     "eval::portal_runner_result",
+    "eval::portal_activation_result",
+    "eval::portal_activation_bracketed",
+    "eval::enter_portal",
     "eval::eval_loop_jit",
     // The space-level helpers most builtins reach Python through.
     "baseobjspace::call_function",