diff --git a/majit/majit-backend-cranelift/src/compiler.rs b/majit/majit-backend-cranelift/src/compiler.rs index a86a5efbd73..cdacfb0b791 100644 --- a/majit/majit-backend-cranelift/src/compiler.rs +++ b/majit/majit-backend-cranelift/src/compiler.rs @@ -7690,6 +7690,9 @@ pub struct CraneliftBackend { trace_counter: u64, next_trace_id: Option, next_header_pc: Option, + /// `Backend::set_next_frame_value_count_fn` — the compiling driver's + /// `-live-` decoder for the `rd_numb` reads below. + next_frame_value_count_fn: Option usize>, registered_call_assembler_tokens: IndexSet, registered_call_assembler_bridge_traces: IndexSet, /// llmodel.py: self.vtable_offset — byte offset for vtable in objects. @@ -7917,6 +7920,7 @@ impl CraneliftBackend { trace_counter: 1, next_trace_id: None, next_header_pc: None, + next_frame_value_count_fn: None, registered_call_assembler_tokens: IndexSet::new(), registered_call_assembler_bridge_traces: IndexSet::new(), // llmodel.py:64-69: vtable_offset is None when gcremovetypeptr is @@ -8487,6 +8491,7 @@ impl CraneliftBackend { caller_layout, &constants_i64, attached_descrs, + self.next_frame_value_count_fn, )?; // RPython jitframe layout parity: ref_root slots start AFTER all // output slots. max_output_slots must be >= inputs.len() so that @@ -14404,6 +14409,10 @@ fn collect_guards( caller_layout: Option<&ExitRecoveryLayout>, _constants: &indexmap::IndexMap, attached_descrs: majit_backend::AttachedDescrPtrs, + // `Backend::set_next_frame_value_count_fn` — the compiling driver's + // `-live-` decoder for the `rd_numb` reads below. `None` falls back to the + // process-global callback. + frame_value_count_fn: Option usize>, ) -> Result<(), BackendError> { let type_index = OpTypeIndex::new(inputargs, ops); let (type_overrides, op_def_positions) = build_type_overrides(ops, &type_index); @@ -14541,7 +14550,7 @@ fn collect_guards( (op.resolved_rd_numb(), op.resolved_rd_consts()) { use majit_ir::resumedata::{get_frame_value_count_fn, rebuild_from_numbering}; - let fvc = get_frame_value_count_fn(); + let fvc = frame_value_count_fn.or_else(get_frame_value_count_fn); let fvc_ref: Option<&dyn Fn(i32, i32) -> usize> = fvc.as_ref().map(|f| f as &dyn Fn(i32, i32) -> usize); let num_virtuals = op.resolved_rd_virtuals().map_or(0, |v| v.len()); @@ -14588,7 +14597,7 @@ fn collect_guards( let rd_vi = op.resolved_rd_virtuals(); use majit_ir::resumedata::{self, RebuiltValue, rebuild_from_numbering}; let rd_consts_ref: &[majit_ir::Const] = &rd_consts_data; - let fvc = majit_ir::resumedata::get_frame_value_count_fn(); + let fvc = frame_value_count_fn.or_else(majit_ir::resumedata::get_frame_value_count_fn); let fvc_ref: Option<&dyn Fn(i32, i32) -> usize> = fvc.as_ref().map(|f| f as &dyn Fn(i32, i32) -> usize); let num_virtuals = rd_vi.as_ref().map_or(0, |v| v.len()); @@ -15386,6 +15395,10 @@ impl majit_backend::Backend for CraneliftBackend { self.next_header_pc = Some(header_pc); } + fn set_next_frame_value_count_fn(&mut self, fvc: Option usize>) { + self.next_frame_value_count_fn = fvc; + } + fn set_done_with_this_frame_descr_void(&mut self, descr: majit_ir::DescrRef) { self.descr_attachments .write() diff --git a/majit/majit-backend/src/call_stub.rs b/majit/majit-backend/src/call_stub.rs index 5ac4202c505..741c581d031 100644 --- a/majit/majit-backend/src/call_stub.rs +++ b/majit/majit-backend/src/call_stub.rs @@ -550,10 +550,87 @@ pub fn collect_call_args_positional( out } +/// Dispatch a residual call described by `arg_classes`, picking the signature +/// strategy the active backend supports. +/// +/// `bh_call_*_dispatch` transmutes the funcptr to an `extern "C" fn` guessed +/// from the *bucketed* `(int, float)` arity. That is sound only on a C ABI that +/// tolerates a signature mismatch: SysV/AAPCS pass the surplus in registers the +/// callee ignores, and a `usize` parameter is register-width either way. wasm32 +/// has neither property — `call_indirect` type-checks the callee's declared +/// type on every call, and a pointer parameter is `i32` where the transmute +/// says `i64` — so a mistyped guess traps with `indirect call type mismatch` +/// instead of silently working. +/// +/// Where a host trampoline is installed (`set_residual_host_call`, wasm32) the +/// call must therefore go through it with the *positional* argument list. +/// [`collect_call_args`] discards the interleaving `arg_classes` encodes, so +/// the choice cannot be recovered downstream: it belongs here, at the last +/// point that still holds `arg_classes`. +/// +/// # Safety +/// On the transmute path, `func` must match the ABI [`collect_call_args`] +/// derives from `arg_classes` — see [`bh_call_i_dispatch`]. +pub unsafe fn bh_call_i_by_classes( + func: usize, + arg_classes: &str, + args_i: Option<&[i64]>, + args_r: Option<&[i64]>, + args_f: Option<&[i64]>, +) -> i64 { + if let Some(hook) = residual_host_call() { + let args = collect_call_args_positional(arg_classes, args_i, args_r, args_f); + return hook(func, &args); + } + let (int_args, float_args) = collect_call_args(arg_classes, args_i, args_r, args_f); + unsafe { bh_call_i_dispatch(func, &int_args, &float_args) } +} + +/// f64-returning parallel of [`bh_call_i_by_classes`]. +/// +/// # Safety +/// See [`bh_call_i_by_classes`]. +pub unsafe fn bh_call_f_by_classes( + func: usize, + arg_classes: &str, + args_i: Option<&[i64]>, + args_r: Option<&[i64]>, + args_f: Option<&[i64]>, +) -> f64 { + if let Some(hook) = residual_host_call() { + let args = collect_call_args_positional(arg_classes, args_i, args_r, args_f); + // The trampoline returns an f64 callee result as its raw bits. + return f64::from_bits(hook(func, &args) as u64); + } + let (int_args, float_args) = collect_call_args(arg_classes, args_i, args_r, args_f); + unsafe { bh_call_f_dispatch(func, &int_args, &float_args) } +} + +/// Result-discarding parallel of [`bh_call_i_by_classes`]. +/// +/// # Safety +/// See [`bh_call_i_by_classes`]. +pub unsafe fn bh_call_v_by_classes( + func: usize, + arg_classes: &str, + args_i: Option<&[i64]>, + args_r: Option<&[i64]>, + args_f: Option<&[i64]>, +) { + if let Some(hook) = residual_host_call() { + let args = collect_call_args_positional(arg_classes, args_i, args_r, args_f); + let _ = hook(func, &args); + return; + } + let (int_args, float_args) = collect_call_args(arg_classes, args_i, args_r, args_f); + unsafe { bh_call_v_dispatch(func, &int_args, &float_args) } +} + /// A host-provided trampoline that performs a residual call by reflecting the /// callee's real signature, rather than transmuting the raw funcptr to a /// statically-guessed `extern "C" fn`. /// + /// `func_ptr` is the raw callee address (a table index on wasm32); `args` is /// the positional argument list (floats as raw bits). The return value is the /// callee result as a 64-bit pattern (Void callees return 0; Ref returns the diff --git a/majit/majit-backend/src/lib.rs b/majit/majit-backend/src/lib.rs index 3eb4877215a..5426b46cf90 100644 --- a/majit/majit-backend/src/lib.rs +++ b/majit/majit-backend/src/lib.rs @@ -1766,6 +1766,18 @@ pub trait Backend: Send { /// this header PC to synthesised exit recovery layouts. fn set_next_header_pc(&mut self, _header_pc: u64) {} + /// The compiling driver's override for the `jitcode.py:147 enumerate_vars` + /// frame box count, when a backend decodes the guards' `rd_numb` itself to + /// build exit layouts. + /// + /// Carried on `JitDriverStaticData::frame_value_count_fn`: a driver whose + /// frames are numbered outside the process-global liveness pool must not + /// decode against it, and the wrong pool decodes *successfully* with a + /// mistyped count rather than failing. `None` (the default) leaves the + /// backend on the global callback + /// (`majit_ir::resumedata::set_frame_value_count_fn`). + fn set_next_frame_value_count_fn(&mut self, _fvc: Option usize>) {} + /// `compile.py:665-674` `make_and_attach_done_descrs([self, cpu])` — /// per-result-type `DoneWithThisFrame*` singleton shared with /// `MetaInterpStaticData`. Attached once per CPU instance, matching @@ -2692,20 +2704,17 @@ pub trait Backend: Send { if func == 0 { return 0; } - if let Some(hook) = crate::call_stub::residual_host_call() { - let args = crate::call_stub::collect_call_args_positional( + // SAFETY: `func` is a valid funcptr matching the ABI recovered from + // `calldescr.arg_classes`. + unsafe { + crate::call_stub::bh_call_i_by_classes( + func as usize, &calldescr.arg_classes, args_i, args_r, args_f, - ); - return hook(func as usize, &args); + ) } - let (int_args, float_args) = - crate::call_stub::collect_call_args(&calldescr.arg_classes, args_i, args_r, args_f); - // SAFETY: `func` is a valid funcptr matching the (ints, floats) arity - // recovered from `calldescr.arg_classes`. - unsafe { crate::call_stub::bh_call_i_dispatch(func as usize, &int_args, &float_args) } } /// model.py:268 bh_call_r(func, args_i, args_r, args_f, calldescr). /// `llmodel.py:818 bh_call_r`: GCREF-returning parallel — a host pointer @@ -2721,20 +2730,16 @@ pub trait Backend: Send { if func == 0 { return GcRef::NULL; } - if let Some(hook) = crate::call_stub::residual_host_call() { - let args = crate::call_stub::collect_call_args_positional( + // SAFETY: see `bh_call_i`. + let raw = unsafe { + crate::call_stub::bh_call_i_by_classes( + func as usize, &calldescr.arg_classes, args_i, args_r, args_f, - ); - return GcRef(hook(func as usize, &args) as usize); - } - let (int_args, float_args) = - crate::call_stub::collect_call_args(&calldescr.arg_classes, args_i, args_r, args_f); - // SAFETY: see `bh_call_i`. - let raw = - unsafe { crate::call_stub::bh_call_i_dispatch(func as usize, &int_args, &float_args) }; + ) + }; GcRef(raw as usize) } /// model.py:270 bh_call_f(func, args_i, args_r, args_f, calldescr). @@ -2751,20 +2756,16 @@ pub trait Backend: Send { if func == 0 { return 0.0; } - if let Some(hook) = crate::call_stub::residual_host_call() { - let args = crate::call_stub::collect_call_args_positional( + // SAFETY: see `bh_call_i`. + unsafe { + crate::call_stub::bh_call_f_by_classes( + func as usize, &calldescr.arg_classes, args_i, args_r, args_f, - ); - // The trampoline returns an f64 callee result as its raw bits. - return f64::from_bits(hook(func as usize, &args) as u64); + ) } - let (int_args, float_args) = - crate::call_stub::collect_call_args(&calldescr.arg_classes, args_i, args_r, args_f); - // SAFETY: see `bh_call_i`. - unsafe { crate::call_stub::bh_call_f_dispatch(func as usize, &int_args, &float_args) } } /// model.py:272 bh_call_v(func, args_i, args_r, args_f, calldescr). /// `llmodel.py:834 bh_call_v`: void-typed dispatch so a genuinely void @@ -2780,20 +2781,16 @@ pub trait Backend: Send { if func == 0 { return; } - if let Some(hook) = crate::call_stub::residual_host_call() { - let args = crate::call_stub::collect_call_args_positional( + // SAFETY: see `bh_call_i`. + unsafe { + crate::call_stub::bh_call_v_by_classes( + func as usize, &calldescr.arg_classes, args_i, args_r, args_f, - ); - let _ = hook(func as usize, &args); - return; + ) } - let (int_args, float_args) = - crate::call_stub::collect_call_args(&calldescr.arg_classes, args_i, args_r, args_f); - // SAFETY: see `bh_call_i`. - unsafe { crate::call_stub::bh_call_v_dispatch(func as usize, &int_args, &float_args) } } // ── model.py: additional bh_* helpers ── diff --git a/majit/majit-metainterp/src/blackhole.rs b/majit/majit-metainterp/src/blackhole.rs index 2ff4be3c03c..0f831a62bf7 100644 --- a/majit/majit-metainterp/src/blackhole.rs +++ b/majit/majit-metainterp/src/blackhole.rs @@ -3435,6 +3435,65 @@ mod tests { assert_eq!(builder.op_rvmprof_code, 91); } + /// Every canonical `inline_call_*` byte must reach its handler in the + /// production builder. + /// + /// `wire_handler` resolves an opname through `_insns` and returns + /// `false` for a key `setup_insns` never registered; both call sites + /// discard the bool. So the ten `wire_handler("inline_call_*", …)` calls + /// in `wire_bhimpl_handlers` were silent no-ops for as long as the + /// curated `build_inline_call_only_bh_builder` map omitted the keys, and + /// a build-time (LLBC-extracted) jitcode's `inline_call_*` reached + /// `dispatch_step`'s unwired panic instead. `unwired_opnames()` cannot + /// catch that — an absent key has no table slot to be a placeholder in. + #[test] + fn production_bh_builder_wires_every_canonical_inline_call_byte() { + use majit_translate::insns; + let builder = super::build_inline_call_only_bh_builder(); + let placeholder = super::unwired_handler_placeholder as super::BhOpcodeHandler; + for (opname, byte) in [ + ("inline_call_r_i/dR>i", insns::BC_INLINE_CALL_R_I), + ("inline_call_r_r/dR>r", insns::BC_INLINE_CALL_R_R), + ("inline_call_r_v/dR", insns::BC_INLINE_CALL_R_V), + ("inline_call_ir_i/dIR>i", insns::BC_INLINE_CALL_IR_I), + ("inline_call_ir_r/dIR>r", insns::BC_INLINE_CALL_IR_R), + ("inline_call_ir_v/dIR", insns::BC_INLINE_CALL_IR_V), + ("inline_call_irf_i/dIRF>i", insns::BC_INLINE_CALL_IRF_I), + ("inline_call_irf_r/dIRF>r", insns::BC_INLINE_CALL_IRF_R), + ("inline_call_irf_f/dIRF>f", insns::BC_INLINE_CALL_IRF_F), + ("inline_call_irf_v/dIRF", insns::BC_INLINE_CALL_IRF_V), + ] { + let slot = builder.dispatch_table[byte as usize]; + assert_ne!( + slot as usize, placeholder as usize, + "`{opname}` (byte {byte}) is unwired in the production builder", + ); + } + } + + /// The two `fnaddr` classifiers agree with the walker's gate. + /// + /// `residual_call.rs:1117` declines a funcptr with any bit ≥ 47 set; + /// `0` is upstream's "no address" spelling (`jitcode.py:14`) and is a + /// no-op — not a decline — for `residual_call`, whose backend + /// `bh_call_*` returns `0`/null for it. + #[test] + fn fnaddr_classifiers_match_the_walker_symbolic_gate() { + let real = fnaddr_classifiers_match_the_walker_symbolic_gate as *const () as i64; + assert!(!super::is_symbolic_fnaddr(real)); + assert!(super::is_callable_fnaddr(real)); + + // `symbolic_fnaddr_for_path` is a `DefaultHasher` finish() cast to + // i64, so its high bits are set with overwhelming probability; the + // gate's contract is the bit-47 test, not the hash function. + let symbolic = 0x1234_5678_9abc_def0_u64 as i64; + assert!(super::is_symbolic_fnaddr(symbolic)); + assert!(!super::is_callable_fnaddr(symbolic)); + + assert!(!super::is_symbolic_fnaddr(0)); + assert!(!super::is_callable_fnaddr(0)); + } + #[test] fn test_bh_interp_inline_call() { // Build sub-jitcode: r0 = arg, result = r0 + r0, return r1. @@ -5818,7 +5877,12 @@ fn handler_unreachable( #[inline] fn read_descr<'a>(bh: &'a BlackholeInterpreter, code: &[u8], pos: usize) -> (&'a BhDescr, usize) { let descr_idx = (code[pos] as usize) | ((code[pos + 1] as usize) << 8); - if let Some(entry) = bh.jitcode.exec.descrs.get(descr_idx) { + // `descr_at` resolves the per-jitcode `exec.descrs` first, then the + // process-global build-time descr pool — a build-time jitcode (jd1's drain + // and its inlined callees) carries an empty `exec.descrs` and names its + // `d`-arg descrs by their index in that shared pool, matching the trace + // walker's own `descr_at` resolution. + if let Some(entry) = bh.jitcode.descr_at(descr_idx) { let descr = entry.as_bh_descr().unwrap_or_else(|| { panic!("d-arg descrs[{descr_idx}] is not a BhDescr entry: {entry:?}") }); @@ -6544,6 +6608,29 @@ fn check_residual_call_exception_after( Err(DispatchError::LeaveFrame) } +/// Refuse to jump to a `residual_call_*` funcptr that is a +/// `symbolic_fnaddr_for_path` hash (see `is_symbolic_fnaddr`). +/// +/// `bh_call_*` takes the funcptr straight to an indirect branch, so a hash +/// there is a wild call. The walker's residual path already declines this exact +/// case (`ResidualDecline::Symbolic`); the blackhole has no decline channel, so +/// it aborts the frame, which hands the continuation back to the interpreter. +/// +/// `func == 0` is deliberately *not* rejected: the backends' `bh_call_*` treat +/// it as a no-op returning `0`/null, a convention pyre relies on for host calls +/// left unbound on purpose. +#[inline] +fn reject_symbolic_residual_call(bh: &mut BlackholeInterpreter, func: i64) -> DispatchError { + if crate::majit_log_enabled() { + eprintln!( + "[bh] residual_call declined: funcptr {func:#x} is a symbolic path hash, not a code \ + address; register the callee's path in the host's fnaddr bindings" + ); + } + bh.aborted = true; + DispatchError::LeaveFrame +} + // residual_call_irf_* fn handler_residual_call_irf_i( bh: &mut BlackholeInterpreter, @@ -6551,6 +6638,9 @@ fn handler_residual_call_irf_i( position: usize, ) -> Result { let func = bh.registers_i[code[position] as usize]; + if is_symbolic_fnaddr(func) { + return Err(reject_symbolic_residual_call(bh, func)); + } let (ai, p) = read_list_i(bh, code, position + 1); let (ar, p) = read_list_r(bh, code, p); let (af, p) = read_list_f(bh, code, p); @@ -6572,6 +6662,9 @@ fn handler_residual_call_irf_r( position: usize, ) -> Result { let func = bh.registers_i[code[position] as usize]; + if is_symbolic_fnaddr(func) { + return Err(reject_symbolic_residual_call(bh, func)); + } let (ai, p) = read_list_i(bh, code, position + 1); let (ar, p) = read_list_r(bh, code, p); let (af, p) = read_list_f(bh, code, p); @@ -6593,6 +6686,9 @@ fn handler_residual_call_irf_f( position: usize, ) -> Result { let func = bh.registers_i[code[position] as usize]; + if is_symbolic_fnaddr(func) { + return Err(reject_symbolic_residual_call(bh, func)); + } let (ai, p) = read_list_i(bh, code, position + 1); let (ar, p) = read_list_r(bh, code, p); let (af, p) = read_list_f(bh, code, p); @@ -6614,6 +6710,9 @@ fn handler_residual_call_irf_v( position: usize, ) -> Result { let func = bh.registers_i[code[position] as usize]; + if is_symbolic_fnaddr(func) { + return Err(reject_symbolic_residual_call(bh, func)); + } let (ai, p) = read_list_i(bh, code, position + 1); let (ar, p) = read_list_r(bh, code, p); let (af, p) = read_list_f(bh, code, p); @@ -6635,6 +6734,9 @@ fn handler_residual_call_ir_i( position: usize, ) -> Result { let func = bh.registers_i[code[position] as usize]; + if is_symbolic_fnaddr(func) { + return Err(reject_symbolic_residual_call(bh, func)); + } let (ai, p) = read_list_i(bh, code, position + 1); let (ar, p) = read_list_r(bh, code, p); let (calldescr, p) = read_descr(bh, code, p); @@ -6655,6 +6757,9 @@ fn handler_residual_call_ir_r( position: usize, ) -> Result { let func = bh.registers_i[code[position] as usize]; + if is_symbolic_fnaddr(func) { + return Err(reject_symbolic_residual_call(bh, func)); + } let (ai, p) = read_list_i(bh, code, position + 1); let (ar, p) = read_list_r(bh, code, p); let (calldescr, p) = read_descr(bh, code, p); @@ -6675,6 +6780,9 @@ fn handler_residual_call_ir_v( position: usize, ) -> Result { let func = bh.registers_i[code[position] as usize]; + if is_symbolic_fnaddr(func) { + return Err(reject_symbolic_residual_call(bh, func)); + } let (ai, p) = read_list_i(bh, code, position + 1); let (ar, p) = read_list_r(bh, code, p); let (calldescr, p) = read_descr(bh, code, p); @@ -6694,6 +6802,9 @@ fn handler_residual_call_r_i( position: usize, ) -> Result { let func = bh.registers_i[code[position] as usize]; + if is_symbolic_fnaddr(func) { + return Err(reject_symbolic_residual_call(bh, func)); + } let (ar, p) = read_list_r(bh, code, position + 1); let (calldescr, p) = read_descr(bh, code, p); let calldescr = calldescr.as_calldescr().clone(); @@ -6713,6 +6824,9 @@ fn handler_residual_call_r_r( position: usize, ) -> Result { let func = bh.registers_i[code[position] as usize]; + if is_symbolic_fnaddr(func) { + return Err(reject_symbolic_residual_call(bh, func)); + } let (ar, p) = read_list_r(bh, code, position + 1); let (calldescr, p) = read_descr(bh, code, p); let calldescr = calldescr.as_calldescr().clone(); @@ -6732,6 +6846,9 @@ fn handler_residual_call_r_v( position: usize, ) -> Result { let func = bh.registers_i[code[position] as usize]; + if is_symbolic_fnaddr(func) { + return Err(reject_symbolic_residual_call(bh, func)); + } let (ar, p) = read_list_r(bh, code, position + 1); let (calldescr, p) = read_descr(bh, code, p); let calldescr = calldescr.as_calldescr().clone(); @@ -7024,6 +7141,14 @@ pub fn build_inline_call_only_bh_builder() -> BlackholeInterpBuilder { for (key, byte) in [ ("int_neg/i>i", majit_translate::insns::BC_INT_NEG), ("int_invert/i>i", majit_translate::insns::BC_INT_INVERT), + // `int_is_true/i>i` — `@arguments("i", returns="i")` unary + // (`blackhole.py:559 bhimpl_int_is_true`). Emitted as the + // loop-condition test of any int-typed `while`, including the + // `_unpackiterable_unknown_length` drain's back-edge guard; its + // handler is wired in `wire_bhimpl_handlers` but the byte was + // absent from this curated set, so a blackhole-executed drain hit + // the unwired-opcode panic at the first back-edge test. + ("int_is_true/i>i", majit_translate::insns::BC_INT_IS_TRUE), ("float_add/ff>f", majit_translate::insns::BC_FLOAT_ADD), ("float_sub/ff>f", majit_translate::insns::BC_FLOAT_SUB), ("float_mul/ff>f", majit_translate::insns::BC_FLOAT_MUL), @@ -7328,6 +7453,62 @@ pub fn build_inline_call_only_bh_builder() -> BlackholeInterpBuilder { "residual_call_irf_f/iIRFd>f".to_string(), majit_translate::insns::BC_RESIDUAL_CALL_IRF_F, ); + // Canonical `inline_call_*` family (blackhole.py:1278-1319). The ten + // handlers and their `wire_bhimpl_handlers` calls already exist, but + // `wire_handler` is a no-op for a key `setup_insns` never registered, so + // without these entries the bytes reach `dispatch_step`'s unwired panic + // instead of their handler. Build-time (LLBC-extracted) jitcodes are the + // only producer — the runtime `JitCodeBuilder` emits the pyre-only + // `inline_call_pyre_nested/P` byte instead — and any of them a + // guard-failure resume forward-executes can reach one. + // + // A target whose path the host never published has no runtime address; + // `read_inline_call_jitcode` + `is_callable_fnaddr` decline it rather than + // branch to a `symbolic_fnaddr_for_path` hash. + for (key, byte) in [ + ( + "inline_call_r_i/dR>i", + majit_translate::insns::BC_INLINE_CALL_R_I, + ), + ( + "inline_call_r_r/dR>r", + majit_translate::insns::BC_INLINE_CALL_R_R, + ), + ( + "inline_call_r_v/dR", + majit_translate::insns::BC_INLINE_CALL_R_V, + ), + ( + "inline_call_ir_i/dIR>i", + majit_translate::insns::BC_INLINE_CALL_IR_I, + ), + ( + "inline_call_ir_r/dIR>r", + majit_translate::insns::BC_INLINE_CALL_IR_R, + ), + ( + "inline_call_ir_v/dIR", + majit_translate::insns::BC_INLINE_CALL_IR_V, + ), + ( + "inline_call_irf_i/dIRF>i", + majit_translate::insns::BC_INLINE_CALL_IRF_I, + ), + ( + "inline_call_irf_r/dIRF>r", + majit_translate::insns::BC_INLINE_CALL_IRF_R, + ), + ( + "inline_call_irf_f/dIRF>f", + majit_translate::insns::BC_INLINE_CALL_IRF_F, + ), + ( + "inline_call_irf_v/dIRF", + majit_translate::insns::BC_INLINE_CALL_IRF_V, + ), + ] { + insns.insert(key.to_string(), byte); + } // Sub-slice C.2.1 + Path 2/3 (`subslice_c2_attempt_failure_cpu_prereq_2026_05_07.md`): // vable family (full 14-key coverage). Path 3 (single-indirection // `getfield_vable_*` / `setfield_vable_*` / `hint_force_virtualizable`) @@ -9426,30 +9607,120 @@ fn handler_setlistitem_gc_f( } // inline_call — RPython blackhole.py:1278-1319 // RPython: cpu.bh_call_*(adr2int(jitcode.fnaddr), args_i, args_r, args_f, jitcode.calldescr) -// The 'j' argcode reads a JitCode descriptor carrying fnaddr + calldescr. -// pyre: fnaddr is stored in BhDescr::JitCode; calldescr not yet modeled. -// TODO: Full implementation should use jitcode_index for frame-chain push/pop. +/// Read the `j` argcode of an `inline_call_*` op. +/// +/// `blackhole.py:150-157` resolves `'d'` and `'j'` out of the *same* `descrs` +/// table and differs only in the trailing check — `if argtype == 'j': assert +/// isinstance(value, JitCode)` — because upstream's `JitCode` is itself an +/// `AbstractDescr` (`jitcode.py:9`). pyre's runtime pool models that with +/// `RuntimeBhDescr::JitCode(Arc)`, so this reads the entry as a +/// jitcode and takes `fnaddr` / `calldescr` off the object, exactly as +/// `bhimpl_inline_call_*` does (`blackhole.py:1280`). +/// +/// Reading them off the object also matters for correctness: the flattened +/// `BhDescr::JitCode.fnaddr` reached through `read_descr` lives in +/// `ALL_DESCRS`, which no `runtime_fnaddr_patch` pass ever rewrites, so it is +/// always the build-script process's value. The `Arc` the pool wraps +/// comes from the patched jitcode table and carries the runtime address. +/// +/// `descr_at` (not `exec.descrs`) is the resolver: an LLBC-extracted jitcode +/// carries an empty per-jitcode pool and names its descrs by index into the +/// process-global build-time pool. fn read_inline_call_jitcode( bh: &BlackholeInterpreter, code: &[u8], p: usize, ) -> (usize, i64, BhCallDescr, usize) { - let (jc_descr, p) = read_descr(bh, code, p); - match jc_descr { - BhDescr::JitCode { + let idx = (code[p] as usize) | ((code[p + 1] as usize) << 8); + let entry = bh + .jitcode + .descr_at(idx) + .unwrap_or_else(|| panic!("inline_call: descrs[{idx}] is absent from both descr pools")); + if let Some(jitcode) = entry.as_jitcode() { + return ( + jitcode.try_index().unwrap_or(0), + jitcode.fnaddr, + jitcode.calldescr().clone(), + p + 2, + ); + } + // A runtime-built pool may still hold the flattened form. + match entry.as_bh_descr() { + Some(BhDescr::JitCode { jitcode_index, fnaddr, calldescr, - } => (*jitcode_index, *fnaddr, calldescr.clone(), p), - _ => panic!("expected JitCode descriptor"), + }) => (*jitcode_index, *fnaddr, calldescr.clone(), p + 2), + _ => panic!("inline_call: descrs[{idx}] is not a JitCode entry: {entry:?}"), } } + +/// Whether `fnaddr` is a `symbolic_fnaddr_for_path` hash rather than a real +/// code address. +/// +/// Upstream never has to ask: `CallControl.get_jitcode` binds +/// `llmemory.cast_ptr_to_adr(getfunctionptr(graph))` (`call.py:181-183`) for +/// every jitcode it mints, so `jitcode.fnaddr` is always a linker-resolved +/// address in the same binary as the metainterp. pyre runs its codewriter in +/// `pyre-jit-trace/build.rs`, a different process, and substitutes a +/// `symbolic_fnaddr_for_path` hash for any callee whose real address the host +/// did not publish through `jit_trace_fnaddrs()`; `patch_constants_i_fnaddrs` +/// is keyed on the build-time address, so a hash is never rebound. +/// +/// User-space code addresses occupy the canonical low half on every target pyre +/// builds for, so the top bits separate the two. This is the same discriminator +/// the walker's residual-call path already uses to decline a symbolic funcptr — +/// `(func_ptr as u64) >> 47 != 0` → `ResidualDecline::Symbolic` +/// (`jitcode_dispatch/residual_call.rs:1117`). +#[inline] +pub(crate) fn is_symbolic_fnaddr(fnaddr: i64) -> bool { + (fnaddr as u64) >> 47 != 0 +} + +/// Whether a jitcode's `fnaddr` can be called as a function pointer. +/// +/// Stricter than `!is_symbolic_fnaddr`: `0` is upstream's own "no address" +/// spelling (`jitcode.py:14 fnaddr=None`) and must not be called either. The +/// `func == 0` arm of the backends' `bh_call_*` returns `0`/null instead, which +/// is a fine no-op convention for a `residual_call` whose funcptr the host +/// deliberately left unbound, but for an `inline_call` it would fabricate a +/// return value for a callee that never ran. +#[inline] +pub(crate) fn is_callable_fnaddr(fnaddr: i64) -> bool { + fnaddr != 0 && !is_symbolic_fnaddr(fnaddr) +} + +/// Refuse to call an unresolved `inline_call_*` target. +/// +/// The walker declines the equivalent residual (`ResidualDecline::Symbolic`); +/// the blackhole has no decline channel, so it aborts the frame — a +/// guard-failure resume that cannot finish hands the continuation back to the +/// interpreter, which is always correct, where an indirect branch to a hash is +/// not. +#[inline] +fn reject_unresolved_inline_call( + bh: &mut BlackholeInterpreter, + jitcode_index: usize, + fnaddr: i64, +) -> DispatchError { + if crate::majit_log_enabled() { + eprintln!( + "[bh] inline_call declined: jitcodes[{jitcode_index}] has no runtime address \ + (fnaddr={fnaddr:#x}); register the callee's path in the host's fnaddr bindings" + ); + } + bh.aborted = true; + DispatchError::LeaveFrame +} fn handler_inline_call_irf_i( bh: &mut BlackholeInterpreter, code: &[u8], p: usize, ) -> Result { - let (_jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + let (jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + if !is_callable_fnaddr(fnaddr) { + return Err(reject_unresolved_inline_call(bh, jitcode_index, fnaddr)); + } let (ai, p) = read_list_i(bh, code, p); let (ar, p) = read_list_r(bh, code, p); let (af, p) = read_list_f(bh, code, p); @@ -9463,7 +9734,10 @@ fn handler_inline_call_irf_r( code: &[u8], p: usize, ) -> Result { - let (_jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + let (jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + if !is_callable_fnaddr(fnaddr) { + return Err(reject_unresolved_inline_call(bh, jitcode_index, fnaddr)); + } let (ai, p) = read_list_i(bh, code, p); let (ar, p) = read_list_r(bh, code, p); let (af, p) = read_list_f(bh, code, p); @@ -9478,7 +9752,10 @@ fn handler_inline_call_irf_f( code: &[u8], p: usize, ) -> Result { - let (_jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + let (jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + if !is_callable_fnaddr(fnaddr) { + return Err(reject_unresolved_inline_call(bh, jitcode_index, fnaddr)); + } let (ai, p) = read_list_i(bh, code, p); let (ar, p) = read_list_r(bh, code, p); let (af, p) = read_list_f(bh, code, p); @@ -9493,7 +9770,10 @@ fn handler_inline_call_irf_v( code: &[u8], p: usize, ) -> Result { - let (_jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + let (jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + if !is_callable_fnaddr(fnaddr) { + return Err(reject_unresolved_inline_call(bh, jitcode_index, fnaddr)); + } let (ai, p) = read_list_i(bh, code, p); let (ar, p) = read_list_r(bh, code, p); let (af, p) = read_list_f(bh, code, p); @@ -9506,7 +9786,10 @@ fn handler_inline_call_ir_i( code: &[u8], p: usize, ) -> Result { - let (_jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + let (jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + if !is_callable_fnaddr(fnaddr) { + return Err(reject_unresolved_inline_call(bh, jitcode_index, fnaddr)); + } let (ai, p) = read_list_i(bh, code, p); let (ar, p) = read_list_r(bh, code, p); // blackhole.py:1291-1294 → bhimpl_inline_call_ir_i. @@ -9518,7 +9801,10 @@ fn handler_inline_call_ir_r( code: &[u8], p: usize, ) -> Result { - let (_jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + let (jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + if !is_callable_fnaddr(fnaddr) { + return Err(reject_unresolved_inline_call(bh, jitcode_index, fnaddr)); + } let (ai, p) = read_list_i(bh, code, p); let (ar, p) = read_list_r(bh, code, p); // blackhole.py:1295-1298 → bhimpl_inline_call_ir_r. @@ -9531,7 +9817,10 @@ fn handler_inline_call_ir_v( code: &[u8], p: usize, ) -> Result { - let (_jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + let (jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + if !is_callable_fnaddr(fnaddr) { + return Err(reject_unresolved_inline_call(bh, jitcode_index, fnaddr)); + } let (ai, p) = read_list_i(bh, code, p); let (ar, p) = read_list_r(bh, code, p); // blackhole.py:1299-1302 → bhimpl_inline_call_ir_v. @@ -9543,7 +9832,10 @@ fn handler_inline_call_r_i( code: &[u8], p: usize, ) -> Result { - let (_jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + let (jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + if !is_callable_fnaddr(fnaddr) { + return Err(reject_unresolved_inline_call(bh, jitcode_index, fnaddr)); + } let (ar, p) = read_list_r(bh, code, p); // blackhole.py:1279-1281 → bhimpl_inline_call_r_i. bh.registers_i[code[p] as usize] = bh.bhimpl_inline_call_r_i(fnaddr, &ar, &calldescr); @@ -9554,7 +9846,10 @@ fn handler_inline_call_r_r( code: &[u8], p: usize, ) -> Result { - let (_jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + let (jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + if !is_callable_fnaddr(fnaddr) { + return Err(reject_unresolved_inline_call(bh, jitcode_index, fnaddr)); + } let (ar, p) = read_list_r(bh, code, p); // blackhole.py:1282-1285 → bhimpl_inline_call_r_r. bh.registers_r[code[p] as usize] = bh.bhimpl_inline_call_r_r(fnaddr, &ar, &calldescr).0 as i64; @@ -9565,7 +9860,10 @@ fn handler_inline_call_r_v( code: &[u8], p: usize, ) -> Result { - let (_jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + let (jitcode_index, fnaddr, calldescr, p) = read_inline_call_jitcode(bh, code, p); + if !is_callable_fnaddr(fnaddr) { + return Err(reject_unresolved_inline_call(bh, jitcode_index, fnaddr)); + } let (ar, p) = read_list_r(bh, code, p); // blackhole.py:1286-1289 → bhimpl_inline_call_r_v. bh.bhimpl_inline_call_r_v(fnaddr, &ar, &calldescr); diff --git a/majit/majit-metainterp/src/compile.rs b/majit/majit-metainterp/src/compile.rs index c8389b1943d..eff9e8313a2 100644 --- a/majit/majit-metainterp/src/compile.rs +++ b/majit/majit-metainterp/src/compile.rs @@ -370,10 +370,16 @@ impl<'a> UnrolledLoopData<'a> { /// The backend numbers every guard and finish in a single exit table, so this /// helper mirrors that numbering and records only the guard entries that need /// resume data plus the corresponding op index for blackhole fallback. +/// +/// `frame_value_count_fn` is the compiling driver's +/// [`crate::jitdriver::JitDriverStaticData::frame_value_count_fn`] override for +/// the `jitcode.py:147 enumerate_vars` frame box count; `None` falls back to the +/// process-global callback the host registered. pub(crate) fn build_guard_metadata>( inputargs: &[InputArg], ops: &[T], pc: u64, + frame_value_count_fn: Option usize>, ) -> ( indexmap::IndexMap, indexmap::IndexMap, @@ -383,6 +389,12 @@ pub(crate) fn build_guard_metadata>( let mut exit_layouts: indexmap::IndexMap = indexmap::IndexMap::new(); let mut fail_index = 0u32; let mut resume_memo = ResumeDataLoopMemo::new(); + // The driver-scoped override wins: a driver whose frames are numbered + // outside the process-global liveness pool must not decode against it (see + // `JitDriverStaticData::frame_value_count_fn`). + let fvc = frame_value_count_fn.or_else(majit_ir::resumedata::get_frame_value_count_fn); + let fvc_ref: Option<&dyn Fn(i32, i32) -> usize> = + fvc.as_ref().map(|f| f as &dyn Fn(i32, i32) -> usize); // history.py:220/261/307 — each fail-arg's type is intrinsic on the Box // (the OpRef variant tag, `ty()`); a fail_arg carries its own type // regardless of trace position, so no position-keyed side table is needed. @@ -540,9 +552,6 @@ pub(crate) fn build_guard_metadata>( (op.resolved_rd_numb(), op.resolved_rd_consts()) { use majit_ir::resumedata::{RebuiltValue, rebuild_from_numbering}; - let fvc = majit_ir::resumedata::get_frame_value_count_fn(); - let fvc_ref: Option<&dyn Fn(i32, i32) -> usize> = - fvc.as_ref().map(|f| f as &dyn Fn(i32, i32) -> usize); let num_virtuals = op.resolved_rd_virtuals().map_or(0, |v| v.len()); let (_num_failargs, vable_values, _vref_values, frames) = rebuild_from_numbering( &rd_numb_bytes, @@ -657,9 +666,6 @@ pub(crate) fn build_guard_metadata>( (op.resolved_rd_numb(), op.resolved_rd_consts()) { use majit_ir::resumedata::{RebuiltValue, rebuild_from_numbering}; - let fvc = majit_ir::resumedata::get_frame_value_count_fn(); - let fvc_ref: Option<&dyn Fn(i32, i32) -> usize> = - fvc.as_ref().map(|f| f as &dyn Fn(i32, i32) -> usize); let num_virtuals = op.resolved_rd_virtuals().map_or(0, |v| v.len()); let (num_failargs, vable_values, vref_values, frames) = rebuild_from_numbering( &rd_numb_bytes, @@ -2648,7 +2654,7 @@ mod tests { ]); guard.set_fail_arg_types(vec![Type::Ref, Type::Int]); - let (_resume_data, exit_layouts) = build_guard_metadata(&inputargs, &[guard], 8); + let (_resume_data, exit_layouts) = build_guard_metadata(&inputargs, &[guard], 8, None); let exit = exit_layouts.get(&0).expect("guard exit layout"); let resume_layout = exit.resume_layout.as_ref().expect("resume_layout"); @@ -2698,7 +2704,7 @@ mod tests { ]); guard.set_fail_arg_types(fail_arg_types); - let (_resume_data, exit_layouts) = build_guard_metadata(&inputargs, &[guard], 0); + let (_resume_data, exit_layouts) = build_guard_metadata(&inputargs, &[guard], 0, None); let exit = exit_layouts.get(&0).expect("guard exit layout"); assert_eq!( diff --git a/majit/majit-metainterp/src/jitdriver.rs b/majit/majit-metainterp/src/jitdriver.rs index 3a602893f87..77ea2b1b07c 100644 --- a/majit/majit-metainterp/src/jitdriver.rs +++ b/majit/majit-metainterp/src/jitdriver.rs @@ -781,6 +781,30 @@ pub struct JitDriverStaticData { /// populated alongside `virtualizable_info` once the host runtime /// supplies the field descriptor. pub vable_token_descr: Option, + /// Per-driver override for the `jitcode.py:147 enumerate_vars` frame box + /// count (`length_i + length_r + length_f`) that `rebuild_from_numbering` + /// asks for while building guard metadata. + /// + /// Upstream needs no such thing: one codewriter run numbers every jitcode + /// and interns every `-live-` triple into the single + /// `metainterp_sd.liveness_info` pool (`pyjitpl.py:2264`), so the global + /// decode is unambiguous. pyre has **two** numbering spaces — the runtime + /// CodeObject-keyed store jd0 grows via `intern_liveness`, and the + /// build-time `jitcode_runtime` artifacts a driver over an extracted + /// interpreter body (jd1 `unpackiterable_driver`) numbers against. A frame + /// numbered in one space and decoded against the other reads an unrelated + /// frame's liveness triple. + /// + /// Selecting the store therefore has to be keyed on the driver, which is + /// what this field does: `None` keeps the process-global callback + /// (`majit_ir::resumedata::set_frame_value_count_fn`), and a driver whose + /// frames are numbered elsewhere installs its own decoder here. + /// + /// ⚠️ A "try the global store, fall back on failure" scheme cannot replace + /// this. The wrong store frequently *succeeds*: its low indices hold + /// unrelated jitcodes that decode at the same pc and silently return a + /// mistyped count. A successful decode is not evidence of the right store. + pub frame_value_count_fn: Option usize>, } impl JitDriverStaticData { @@ -860,6 +884,7 @@ impl JitDriverStaticData { no_loop_header: false, assembler_helper_adr: 0, vable_token_descr: None, + frame_value_count_fn: None, }; // warmspot.py:529/538 — keep `index_of_virtualizable` in sync // with the `virtualizable_arg_index()` derived from `reds`. diff --git a/majit/majit-metainterp/src/pyjitpl.rs b/majit/majit-metainterp/src/pyjitpl.rs index 1210421dacc..eb1102f60a8 100644 --- a/majit/majit-metainterp/src/pyjitpl.rs +++ b/majit/majit-metainterp/src/pyjitpl.rs @@ -3463,12 +3463,22 @@ impl MetaInterp { /// as `self.jitdriver_sd` (pyjitpl.py:3291) for callers that need /// `virtualizable_info` and the related metadata together. /// - /// Prefers the trace-bound `active_jitdriver_sd` and validates it - /// carries `virtualizable_info`; otherwise scans for the first - /// registered slot with `virtualizable_info` populated. Returns - /// `None` when no driver carries `virtualizable_info` yet (the - /// caller should bail out — RPython would have early-returned - /// from the `vinfo is None` branch in `initialize_virtualizable`). + /// pyjitpl.py:3291 `vinfo = self.jitdriver_sd.virtualizable_info` — the + /// vinfo is STRICTLY that of the elected active driver, so a novable + /// active driver (jd1 `unpackiterable_driver`, `virtualizable_info=None`) + /// never borrows a sibling driver's vinfo; borrowing would capture a + /// phantom jd0-shaped vable section in the novable trace's resume data. + /// Returning `None` makes `initialize_virtualizable` bail, matching + /// RPython's `vinfo is None` early return. + /// + /// One adaptation preserves the documented [`Self::ensure_default_driver_sd`] + /// contract: a host that calls `set_virtualizable_info` BEFORE + /// `register_jitdriver_sd` lands its vinfo on the empty placeholder slot + /// (`index == None`), so its real driver is elected without one. That vinfo + /// still belongs to the host's single logical driver, so it is adopted via + /// the linear scan — but only while no *registered* driver owns a vinfo. + /// Once one does (pyre: jd0 at slot 1), a later registered driver that + /// carries none is genuinely novable (pyre: jd1 at slot 2) and gets `None`. fn resolve_active_jitdriver_sd_with_vinfo(&self) -> Option { if let Some(idx) = self.active_jitdriver_sd { if let Some(jd) = self.staticdata.jitdrivers_sd.get(idx) { @@ -3476,6 +3486,17 @@ impl MetaInterp { return Some(idx); } } + // `jd.index` is stamped only by `register_jitdriver_sd` + // (call.py:46-47), so `index.is_some()` marks a host-registered + // driver as opposed to the pre-registration placeholder. + if self + .staticdata + .jitdrivers_sd + .iter() + .any(|jd| jd.index.is_some() && jd.virtualizable_info.is_some()) + { + return None; + } } self.staticdata .jitdrivers_sd @@ -3483,6 +3504,22 @@ impl MetaInterp { .position(|jd| jd.virtualizable_info.is_some()) } + /// The active driver's + /// [`crate::jitdriver::JitDriverStaticData::frame_value_count_fn`] override, + /// for `build_guard_metadata`'s `-live-` decode. + /// + /// Read STRICTLY off `active_jitdriver_sd` — the driver whose trace is being + /// compiled. There is deliberately no fallback scan over the other slots: + /// the override names the jitcode/liveness store the frames were numbered + /// against, and borrowing a sibling driver's store decodes *successfully* + /// against unrelated frames rather than failing (see the field's doc). + fn active_frame_value_count_fn(&self) -> Option usize> { + self.staticdata + .jitdrivers_sd + .get(self.active_jitdriver_sd?)? + .frame_value_count_fn + } + /// warmspot.py:519-525 `jd.greenfield_info = GreenFieldInfo(cpu, jd)`. /// /// Hosts that declare green fields (greens containing `.`) call @@ -6055,6 +6092,8 @@ impl MetaInterp { let trace_id = self.alloc_trace_id(); self.backend.set_next_trace_id(trace_id); self.backend.set_next_header_pc(green_key); + self.backend + .set_next_frame_value_count_fn(self.active_frame_value_count_fn()); let front_target_tokens = if retried_without_unroll { let target_token = crate::history::TargetToken::new_loop(token_num); @@ -6238,8 +6277,12 @@ impl MetaInterp { ); } // Build resume data and exit layouts for all guards in the optimized trace. - let (mut resume_data, mut exit_layouts) = - compile::build_guard_metadata(&inputargs, &compiled_ops, green_key); + let (mut resume_data, mut exit_layouts) = compile::build_guard_metadata( + &inputargs, + &compiled_ops, + green_key, + self.active_frame_value_count_fn(), + ); let mut terminal_exit_layouts = compile::build_terminal_exit_layouts(&inputargs, &compiled_ops); if let Some(backend_layouts) = @@ -7102,6 +7145,8 @@ impl MetaInterp { let trace_id = self.alloc_trace_id(); self.backend.set_next_trace_id(trace_id); self.backend.set_next_header_pc(green_key); + self.backend + .set_next_frame_value_count_fn(self.active_frame_value_count_fn()); // compile.py:532-546 `debug_start("jit-backend") + // profiler.start_backend() ... try: do_compile_loop ... finally: @@ -7165,8 +7210,12 @@ impl MetaInterp { inputargs.len() ); } - let (mut resume_data, mut exit_layouts) = - compile::build_guard_metadata(&inputargs, &combined_ops, green_key); + let (mut resume_data, mut exit_layouts) = compile::build_guard_metadata( + &inputargs, + &combined_ops, + green_key, + self.active_frame_value_count_fn(), + ); let mut terminal_exit_layouts = compile::build_terminal_exit_layouts(&inputargs, &combined_ops); if let Some(backend_layouts) = @@ -7624,6 +7673,8 @@ impl MetaInterp { let trace_id = self.alloc_trace_id(); self.backend.set_next_trace_id(trace_id); self.backend.set_next_header_pc(green_key); + self.backend + .set_next_frame_value_count_fn(self.active_frame_value_count_fn()); // compile.py:233 `loop.inputargs = loop_info.inputargs`. let mut inputargs: Vec = trace.inputargs_cloned(); @@ -7702,8 +7753,12 @@ impl MetaInterp { self.warm_state.memory_manager.keep_loop_alive(&token); // compile.py:213 record_loop_or_bridge. self.record_loop_or_bridge(&token, &optimized_ops, trace_id); - let (mut resume_data, mut exit_layouts) = - compile::build_guard_metadata(&inputargs, &optimized_ops, green_key); + let (mut resume_data, mut exit_layouts) = compile::build_guard_metadata( + &inputargs, + &optimized_ops, + green_key, + self.active_frame_value_count_fn(), + ); let mut terminal_exit_layouts = compile::build_terminal_exit_layouts(&inputargs, &optimized_ops); if let Some(backend_layouts) = @@ -7988,6 +8043,8 @@ impl MetaInterp { let trace_id = self.alloc_trace_id(); self.backend.set_next_trace_id(trace_id); self.backend.set_next_header_pc(green_key); + self.backend + .set_next_frame_value_count_fn(self.active_frame_value_count_fn()); // compile.py:233 `loop.inputargs = loop_info.inputargs`. let mut inputargs: Vec = trace.inputargs_cloned(); @@ -8062,8 +8119,12 @@ impl MetaInterp { self.warm_state.memory_manager.keep_loop_alive(&token); // compile.py:213 record_loop_or_bridge. self.record_loop_or_bridge(&token, &compiled_ops, trace_id); - let (mut resume_data, mut exit_layouts) = - compile::build_guard_metadata(&inputargs, &compiled_ops, green_key); + let (mut resume_data, mut exit_layouts) = compile::build_guard_metadata( + &inputargs, + &compiled_ops, + green_key, + self.active_frame_value_count_fn(), + ); let mut terminal_exit_layouts = compile::build_terminal_exit_layouts(&inputargs, &compiled_ops); if let Some(backend_layouts) = @@ -10291,6 +10352,8 @@ impl MetaInterp { .set_callinfocollection(self.callinfocollection.clone()); self.backend.set_next_trace_id(trace_id); self.backend.set_next_header_pc(original_green_key); + self.backend + .set_next_frame_value_count_fn(self.active_frame_value_count_fn()); let token = make_jitcell_token(self.warm_state.alloc_token_number(), None); // `green_key` is interior-mutable, so it is written through the @@ -10325,6 +10388,7 @@ impl MetaInterp { bridge_inputargs, &optimized_ops, original_green_key, + self.active_frame_value_count_fn(), ); let mut terminal_exit_layouts = compile::build_terminal_exit_layouts(bridge_inputargs, &optimized_ops); @@ -10937,6 +11001,8 @@ impl MetaInterp { .set_callinfocollection(self.callinfocollection.clone()); self.backend.set_next_trace_id(bridge_trace_id); self.backend.set_next_header_pc(green_key); + self.backend + .set_next_frame_value_count_fn(self.active_frame_value_count_fn()); let result = { let compiled = self.compiled_loops.get(&green_key).unwrap(); @@ -11049,6 +11115,8 @@ impl MetaInterp { self.last_quasi_immutable_deps = std::mem::take(&mut optimizer.quasi_immutable_deps); self.record_loop_or_bridge(&source_jct, &mut optimized_ops, bridge_trace_id); + // Read before the `compiled_loops` mutable borrow below. + let fvc = self.active_frame_value_count_fn(); // Mark the bridge as compiled if let Some(compiled) = self.compiled_loops.get_mut(&green_key) { // pyjitpl.py:1049 — `fail_descr.trace_id()` is the @@ -11056,8 +11124,12 @@ impl MetaInterp { // starts at 1). No `0 → root_trace_id` sentinel; // RPython resolves the source via descr identity. let source_trace_id = fail_descr.trace_id(); - let (mut resume_data, mut exit_layouts) = - compile::build_guard_metadata(bridge_inputargs, &optimized_ops, green_key); + let (mut resume_data, mut exit_layouts) = compile::build_guard_metadata( + bridge_inputargs, + &optimized_ops, + green_key, + fvc, + ); let mut terminal_exit_layouts = compile::build_terminal_exit_layouts(bridge_inputargs, &optimized_ops); if let Some(backend_layouts) = self.backend.compiled_bridge_fail_descr_layouts( @@ -16615,6 +16687,7 @@ mod metainterp_static_data_tests { no_loop_header: false, assembler_helper_adr: 0, vable_token_descr: None, + frame_value_count_fn: None, }; { let MetaInterp { @@ -20149,12 +20222,18 @@ mod tests { let mut token = JitCellToken::new(green_key + 1000); let trace_id = meta.alloc_trace_id(); meta.backend.set_next_trace_id(trace_id); + meta.backend + .set_next_frame_value_count_fn(meta.active_frame_value_count_fn()); let ops_rc: Vec = ops.iter().cloned().map(std::rc::Rc::new).collect(); meta.backend .compile_loop(inputargs, &ops_rc, &mut token) .expect("loop should compile"); - let (mut resume_data, mut exit_layouts) = - compile::build_guard_metadata(inputargs, &ops, green_key); + let (mut resume_data, mut exit_layouts) = compile::build_guard_metadata( + inputargs, + &ops, + green_key, + meta.active_frame_value_count_fn(), + ); let mut terminal_exit_layouts = compile::build_terminal_exit_layouts(inputargs, &ops); if let Some(backend_layouts) = meta.backend.compiled_fail_descr_layouts(&token) { compile::merge_backend_exit_layouts(&mut exit_layouts, &backend_layouts, &ops); diff --git a/majit/majit-metainterp/src/pyjitpl/dispatch.rs b/majit/majit-metainterp/src/pyjitpl/dispatch.rs index 9d20887f813..a7dfcb55d24 100644 --- a/majit/majit-metainterp/src/pyjitpl/dispatch.rs +++ b/majit/majit-metainterp/src/pyjitpl/dispatch.rs @@ -3598,11 +3598,19 @@ where jitcode::insns::BC_SETARRAYITEM_GC_F => self.read_float_reg(value_reg), _ => self.read_int_reg(value_reg), }; + let descr_index = descr.index(); ctx.record_op_with_descr( OpCode::SetarrayitemGc, &[array_opref, index_opref, value_opref], descr, ); + // execute_setarrayitem_gc (pyjitpl.py:2744): update the trace + // heap cache after the store so a later getarrayitem of the same + // (array, const index) reads the stored value, not a stale + // cached element. Key on descr.index() (the canonical resolved + // descr index the getarrayitem read path uses), not the raw + // bytecode operand descr_idx. + ctx.heapcache_setarrayitem(array_opref, index_opref, descr_index, value_opref); if array_addr != 0 { let item_addr = (array_addr as usize) .wrapping_add(base_size) @@ -5370,17 +5378,13 @@ where if effectinfo.oopspecindex == majit_ir::descr::OopSpecIndex::NotInTrace { self.clear_exception(); if !concrete_ptr.is_null() { - let (int_args, float_args) = majit_backend::call_stub::collect_call_args( - &calldescr.arg_classes, - Some(&raw_i), - Some(&raw_r), - Some(&raw_f), - ); unsafe { - majit_backend::call_stub::bh_call_v_dispatch( + majit_backend::call_stub::bh_call_v_by_classes( concrete_ptr as usize, - &int_args, - &float_args, + &calldescr.arg_classes, + Some(&raw_i), + Some(&raw_r), + Some(&raw_f), ); } } @@ -5465,17 +5469,13 @@ where // `extern "C" fn(...) -> i64` and reads garbage from // rax/x0). if !concrete_ptr.is_null() { - let (int_args, float_args) = majit_backend::call_stub::collect_call_args( - &calldescr.arg_classes, - Some(&raw_i), - Some(&raw_r), - Some(&raw_f), - ); unsafe { - majit_backend::call_stub::bh_call_v_dispatch( + majit_backend::call_stub::bh_call_v_by_classes( concrete_ptr as usize, - &int_args, - &float_args, + &calldescr.arg_classes, + Some(&raw_i), + Some(&raw_r), + Some(&raw_f), ); } } @@ -5685,17 +5685,13 @@ where // int destination register, and abort on exception. self.clear_exception(); if !concrete_ptr.is_null() { - let (int_args, float_args) = majit_backend::call_stub::collect_call_args( - &calldescr.arg_classes, - Some(&raw_i), - Some(&raw_r), - Some(&raw_f), - ); unsafe { - majit_backend::call_stub::bh_call_v_dispatch( + majit_backend::call_stub::bh_call_v_by_classes( concrete_ptr as usize, - &int_args, - &float_args, + &calldescr.arg_classes, + Some(&raw_i), + Some(&raw_r), + Some(&raw_f), ); } } @@ -5761,17 +5757,13 @@ where let concrete = if concrete_ptr.is_null() { 0 } else { - let (int_args, float_args) = majit_backend::call_stub::collect_call_args( - &calldescr.arg_classes, - Some(&raw_i), - Some(&raw_r), - Some(&raw_f), - ); unsafe { - majit_backend::call_stub::bh_call_i_dispatch( + majit_backend::call_stub::bh_call_i_by_classes( concrete_ptr as usize, - &int_args, - &float_args, + &calldescr.arg_classes, + Some(&raw_i), + Some(&raw_r), + Some(&raw_f), ) } }; @@ -5975,17 +5967,13 @@ where // branch for the full citation. self.clear_exception(); if !concrete_ptr.is_null() { - let (int_args, float_args) = majit_backend::call_stub::collect_call_args( - &calldescr.arg_classes, - Some(&raw_i), - Some(&raw_r), - Some(&raw_f), - ); unsafe { - majit_backend::call_stub::bh_call_v_dispatch( + majit_backend::call_stub::bh_call_v_by_classes( concrete_ptr as usize, - &int_args, - &float_args, + &calldescr.arg_classes, + Some(&raw_i), + Some(&raw_r), + Some(&raw_f), ); } } @@ -6048,17 +6036,13 @@ where let concrete = if concrete_ptr.is_null() { 0 } else { - let (int_args, float_args) = majit_backend::call_stub::collect_call_args( - &calldescr.arg_classes, - Some(&raw_i), - Some(&raw_r), - Some(&raw_f), - ); unsafe { - majit_backend::call_stub::bh_call_i_dispatch( + majit_backend::call_stub::bh_call_i_by_classes( concrete_ptr as usize, - &int_args, - &float_args, + &calldescr.arg_classes, + Some(&raw_i), + Some(&raw_r), + Some(&raw_f), ) } }; @@ -6224,17 +6208,13 @@ where // branch for the full citation. self.clear_exception(); if !concrete_ptr.is_null() { - let (int_args, float_args) = majit_backend::call_stub::collect_call_args( - &calldescr.arg_classes, - Some(&raw_i), - Some(&raw_r), - Some(&raw_f), - ); unsafe { - majit_backend::call_stub::bh_call_v_dispatch( + majit_backend::call_stub::bh_call_v_by_classes( concrete_ptr as usize, - &int_args, - &float_args, + &calldescr.arg_classes, + Some(&raw_i), + Some(&raw_r), + Some(&raw_f), ); } } @@ -6291,17 +6271,13 @@ where let concrete = if concrete_ptr.is_null() { 0.0f64 } else { - let (int_args, float_args) = majit_backend::call_stub::collect_call_args( - &calldescr.arg_classes, - Some(&raw_i), - Some(&raw_r), - Some(&raw_f), - ); unsafe { - majit_backend::call_stub::bh_call_f_dispatch( + majit_backend::call_stub::bh_call_f_by_classes( concrete_ptr as usize, - &int_args, - &float_args, + &calldescr.arg_classes, + Some(&raw_i), + Some(&raw_r), + Some(&raw_f), ) } }; diff --git a/majit/majit-translate/src/front/exc_from_raise.rs b/majit/majit-translate/src/front/exc_from_raise.rs index eb6c0c1c1f7..c0a412f5880 100644 --- a/majit/majit-translate/src/front/exc_from_raise.rs +++ b/majit/majit-translate/src/front/exc_from_raise.rs @@ -41,10 +41,21 @@ //! //! ```text //! evalue = op.simple_call(const(exc_class), *message_args) -//! etype = op.type(evalue) -//! graph.set_raise_values(block, etype, evalue) +//! graph.set_raise_values(block, evalue, evalue) //! ``` //! +//! ### The `etype` link arg +//! +//! Upstream's tail line is `w_type = op.type(w_value)`, but that value never +//! reaches a jitcode. `make_bytecode_block` hands the `exceptblock`'s +//! `inputargs` to `make_return` (`flatten.py:106-108`), whose 2-arg arm emits +//! `-live-` + `raise self.getcolor(args[1])` and never touches `args[0]` +//! (`flatten.py:139-143`, mirrored at `flatten.rs:781-793`). So the slot the +//! `etype` link arg feeds has no consumer in any emitted bytecode. +//! `set_raise_values` therefore receives `evalue` for both slots instead of a +//! synthesised `type(evalue)` call, which would be a residual with a dead +//! result register on every raise tail. +//! //! ### TODO: `Constant` SSA carrier shape //! //! Upstream RPython encodes the exception class operand as a @@ -118,9 +129,9 @@ //! //! ## What this helper is not //! -//! - It is **not** a synthetic helper. The call targets emitted here -//! are the RPython op names themselves (`simple_call`, `type`), so -//! any downstream reader sees the same op namespace upstream uses. +//! - It is **not** a synthetic helper. The call target emitted here +//! is the RPython op name itself (`simple_call`), so any downstream +//! reader sees the same op namespace upstream uses. //! - There are no `__pyre_exc_from_raise__` / `__pyre_exception_type_of__` //! opaque Call targets any more — that earlier deviation is removed //! by the same change that introduced this module. @@ -129,9 +140,9 @@ use crate::flowspace::model::Variable; use crate::model::{BlockId, CallTarget, FunctionGraph, OpKind, ValueType}; /// Close `block` with an `(etype, evalue)` Link to `exceptblock` -/// whose values come from the canonical RPython `exc_from_raise` -/// op sequence (`op.simple_call(const(exc_class), *args)` followed -/// by `op.type(evalue)`). +/// whose value comes from the canonical RPython `exc_from_raise` op +/// sequence (`op.simple_call(const(exc_class), *args)`). The `etype` +/// slot reuses `evalue` — see the module-level "etype link arg" note. /// /// `exc_class_name` is the Python-layer exception class name /// (`"AssertionError"`, `"PanicError"`, …) carried as the second @@ -175,22 +186,21 @@ pub fn lower_exc_from_raise( true, ) .expect("op.simple_call(exc_class, ...) must produce a Ref exception instance"); - // `op.type(evalue)` — upstream `flowcontext.py:600` tail line - // (`w_type = op.type(w_value).eval(self)`). - let type_target = CallTarget::function_path(["type"]); - let etype_var = graph - .push_op_var( - block, - OpKind::Call { - target: type_target, - args: vec![evalue_var.clone()], - result_ty: ValueType::Ref(None), - }, - true, - ) - .expect("op.type(evalue) must produce a Ref type value"); - // `flowspace/flowcontext.py:1253 Raise.nomoreblocks` — close - // the block with the `(etype, evalue)` Link to the graph's - // `exceptblock`. - graph.set_raise_values(block, etype_var, evalue_var); + // `flowspace/flowcontext.py:1253 Raise.nomoreblocks` — close the block + // with the `(etype, evalue)` Link to the graph's `exceptblock`. + // + // Upstream's `w_type = op.type(w_value)` (`flowcontext.py:634`) lives at + // flow-space level only — see the module-level "etype link arg" note: the + // 2-arg `make_return` arm emits `raise ` and never reads + // `args[0]`, and `make_exception_link` drops both for a direct `reraise`. + // So the `etype` link arg is write-only in every emitted jitcode. The + // emitted drain tail is `-live-` + `raise ` and nothing else. + // Materialising it as a `type(evalue)` call — which pyre + // did, because `set_raise_values` takes `Variable`s and every pyre SSA + // value needs a producing op (see the `Constant` carrier TODO above) — + // left a residual whose result register is dead by construction on every + // raise tail in the corpus, and made the raise arm unwalkable in the + // blackhole (canonical `inline_call_*` on a callee with no runtime + // address). Reuse `evalue`: same ref kind, no new op. + graph.set_raise_values(block, evalue_var.clone(), evalue_var); } diff --git a/majit/majit-translate/src/front/result_exc.rs b/majit/majit-translate/src/front/result_exc.rs index f27c8627319..00215265569 100644 --- a/majit/majit-translate/src/front/result_exc.rs +++ b/majit/majit-translate/src/front/result_exc.rs @@ -33,8 +33,9 @@ //! (`PyError::to_exc_object` — the trace-level exception value //! domain is the `W_BaseException` ref, the same value //! `BH_LAST_EXC_VALUE` carries) and closes the block towards -//! `exceptblock` with `(op.type(exc), exc)`, exactly the -//! `lower_exc_from_raise` tail shape (`flowcontext.py:600`). +//! `exceptblock` with `(exc, exc)`, exactly the +//! `lower_exc_from_raise` tail shape (`flowcontext.py:600`) — whose +//! `etype` slot is write-only, see that module's "etype link arg" note. //! //! - **Caller rule** ([`rewire_result_exc_call_sites`]): a `?` on a //! call to a scoped callee lowers in MIR as a @@ -533,20 +534,12 @@ pub(crate) fn lower_result_exc_returns( true, ) .expect("to_exc_object call must produce a value"); - // `op.type(evalue)` — the `lower_exc_from_raise` tail - // (`flowcontext.py:600` `w_type = op.type(w_value)`). - let v_type = graph - .push_op_var( - block_id, - OpKind::Call { - target: CallTarget::function_path(["type"]), - args: vec![v_exc.clone()], - result_ty: ValueType::Ref(None), - }, - true, - ) - .expect("op.type(evalue) must produce a value"); - graph.set_raise_values(block_id, v_type, v_exc); + // `graph.set_raise_values(block, etype, evalue)`. The `etype` + // link arg is write-only: `make_return`'s 2-arg arm emits + // `raise ` and never reads `args[0]` + // (`flatten.rs:781-793`, `flatten.py:139-143`). Pass the evalue + // for it — see `front::exc_from_raise`'s "etype link arg" note. + graph.set_raise_values(block_id, v_exc.clone(), v_exc); } else { // `return Ok(v)` → forward the payload itself. for link in &mut graph.blocks[bi].exits { @@ -1620,16 +1613,41 @@ fn try_fuse_drain_match(graph: &mut FunctionGraph, a: usize, r: &Variable) -> Re let r_err = forward_alias(graph, &r_b, &err_link) .ok_or_else(|| format!("{name}: drain fuse: Err link drops the Result value"))?; let err_ops = &graph.blocks[err_target].operations; - let ctor_idx = err_ops + // `PyErrorKind::StopIteration` reaches the `eq` below in either of two + // lowered forms: as a niladic `SyntheticTransparentCtor` while the + // fieldless variant is still carried as an ADT constructor, or as a plain + // `ConstInt` once the fieldless enum lowers to its discriminant. Accept + // both. The constant form is value-checked, so a comparison against a + // different kind (`e.kind == PyErrorKind::ValueError`) can never be fused + // into a StopIteration test; the operand is additionally pinned by the + // `PyErrorKind::eq` argument check below. + // + // 9 == `PyErrorKind::StopIteration` (pyre-interpreter error.rs); like the + // `ExcKind::StopIteration` 10 used by the synthesised handler, the two are + // coupled — renumbering that variant makes this recognizer decline, which + // `unpackiterable_drain_match_fuses_to_kind_test` reports as a failure. + const PYERRORKIND_STOP_ITERATION: i64 = 9; + let (ctor_idx, sc) = err_ops .iter() - .position(|op| matches!(&op.kind, - OpKind::Call { target: CallTarget::SyntheticTransparentCtor { name: n, owner_path }, .. } - if n == "StopIteration" && owner_path.last().is_some_and(|s| s == "PyErrorKind"))) - .ok_or_else(|| format!("{name}: drain fuse: Err arm lacks the StopIteration ctor"))?; - let sc = err_ops[ctor_idx] - .result - .clone() - .ok_or_else(|| format!("{name}: drain fuse: StopIteration ctor without result"))?; + .enumerate() + .find_map(|(i, op)| { + let is_stop_iteration = match &op.kind { + OpKind::Call { + target: + CallTarget::SyntheticTransparentCtor { + name: n, + owner_path, + }, + .. + } => n == "StopIteration" && owner_path.last().is_some_and(|s| s == "PyErrorKind"), + OpKind::ConstInt(v) => *v == PYERRORKIND_STOP_ITERATION, + _ => false, + }; + is_stop_iteration.then(|| op.result.clone().map(|s| (i, s)))? + }) + .ok_or_else(|| { + format!("{name}: drain fuse: Err arm lacks the StopIteration ctor or discriminant") + })?; let (errpay_idx, err_payload) = err_ops .iter() .enumerate() @@ -2073,21 +2091,13 @@ fn try_fuse_drain_match(graph: &mut FunctionGraph, a: usize, r: &Variable) -> Re "drain fuse: H references its unused etype inputarg" ); - // R: `v_type = type(vb)`; `goto exceptblock [v_type, vb]`. DO NOT reuse - // `va` (the int-kinded etype); `type(evalue)` recomputes the ref-kind - // class the raise tail needs. - let v_type = graph - .push_op_var( - r_id, - OpKind::Call { - target: CallTarget::function_path(["type"]), - args: vec![r_vb.clone()], - result_ty: ValueType::Ref(None), - }, - true, - ) - .expect("type(evalue) produces a value"); - graph.set_raise_values(r_id, v_type, r_vb); + // R: `goto exceptblock [etype, vb]`, i.e. the drain's `return Err(e)`. + // The `etype` slot is write-only (`make_return` emits `raise ` + // and never reads `args[0]`, `flatten.rs:781-793`), so pass `vb` rather + // than the int-kinded `va` — the raise operand must be the ref-kinded + // exception value, and a second ref-kinded producer would only add a dead + // residual to the arm a guard-failure resume walks. + graph.set_raise_values(r_id, r_vb.clone(), r_vb); // Break edge args in H scope (all forwarded Variables; the dead threads // were pruned, so no const rides the surviving edge). diff --git a/majit/majit-translate/tests/test_result_exc_lowering.rs b/majit/majit-translate/tests/test_result_exc_lowering.rs index 0dfdacdcc18..a8c2f47dff2 100644 --- a/majit/majit-translate/tests/test_result_exc_lowering.rs +++ b/majit/majit-translate/tests/test_result_exc_lowering.rs @@ -176,6 +176,95 @@ fn execute_wrapper_family_lowers_to_raise_links() { assert!(lastexc_blocks >= 1, "wrapper `?` gets LastException exits"); } +/// Facet A firing guard — the jd1 drain-loop `match next()` fusion. +/// +/// `_unpackiterable_unknown_length`'s StopIteration drain loop is a +/// hand-written `match next() { Ok(w) => append, Err(e) if e.kind == +/// StopIteration => break, Err(e) => return Err(e) }`. Lowered naively it +/// materialises a `Result` shell whose Err arm holds a `StopIteration` +/// `SyntheticTransparentCtor` + a `PyErrorKind::eq` — residuals with no host +/// funcptr that SIGBUS the jd1 walk. `try_fuse_drain_match` +/// (`front::result_exc`) replaces that shell with a `LastException` +/// exception-edge whose handler is the exact-kind test +/// `exc_kind_discriminant(evalue) == 10` (`ExcKind::StopIteration`). +/// +/// The fusion is FAIL-SAFE: on any shape it does not recognise it silently +/// falls back to `catch_and_rewrap`, which leaves the `StopIteration` ctor in +/// place. That silent decline is invisible to the correctness suite — the +/// default (non-jd1) run never executes the fusion, and the +/// `unpack_drain_exact_kind` parity test only guards the default path — yet it +/// reintroduces the unwalkable ctors and reopens the jd1 SIGBUS. A drain +/// rework that perturbs the recognised shape (or a recognizer regression) is +/// exactly such a silent decline. This lowers the REAL drain and asserts the +/// fused signature is present and the ctor is gone, so a decline fails loud. +#[test] +fn unpackiterable_drain_match_fuses_to_kind_test() { + let llbc = interp(); + let graph = lower_function( + llbc, + "pyre_interpreter::baseobjspace::_unpackiterable_unknown_length", + ) + .expect("lower _unpackiterable_unknown_length"); + + // Positive firing signal: the fusion synthesises the exc_kind_discriminant + // kind-test call — the only site in the tree that emits this FunctionPath, + // so its presence proves `try_fuse_drain_match` fired (not declined). + let exc_kind_calls = graph + .blocks + .iter() + .flat_map(|b| b.operations.iter()) + .filter(|op| { + matches!( + &op.kind, + OpKind::Call { target: CallTarget::FunctionPath { segments }, .. } + if segments.last().map(String::as_str) == Some("exc_kind_discriminant") + ) + }) + .count(); + assert!( + exc_kind_calls >= 1, + "drain fusion must synthesise the exc_kind_discriminant kind-test \ + (0 = recognizer silently declined to catch_and_rewrap → the \ + StopIteration ctor/eq residuals remain and SIGBUS the jd1 walk)" + ); + + // Elimination signal: the StopIteration guard ctor survives ONLY on the + // decline (catch_and_rewrap) path, so a fired fusion leaves none. + let stopiteration_ctors = graph + .blocks + .iter() + .flat_map(|b| b.operations.iter()) + .filter(|op| { + matches!( + &op.kind, + OpKind::Call { + target: CallTarget::SyntheticTransparentCtor { name, .. }, + .. + } if name == "StopIteration" + ) + }) + .count(); + assert_eq!( + stopiteration_ctors, 0, + "the StopIteration guard ctor must be gone after the drain fusion" + ); + + // The fused next() call site carries a LastException exit. + let lastexc_blocks = graph + .blocks + .iter() + .filter(|b| matches!(b.exitswitch, Some(ExitSwitch::LastException))) + .count(); + assert!( + lastexc_blocks >= 1, + "the drain next() site must become a LastException exception-edge" + ); + eprintln!( + "drain fusion: exc_kind_discriminant={exc_kind_calls} \ + stopiteration_ctors={stopiteration_ctors} lastexc_blocks={lastexc_blocks}" + ); +} + #[test] fn eval_loop_custom_match_gets_catch_and_rewrap() { let llbc = interp(); diff --git a/pyre/bench/synth/unpack_drain_star_raise.py b/pyre/bench/synth/unpack_drain_star_raise.py new file mode 100644 index 00000000000..1ef39a075f9 --- /dev/null +++ b/pyre/bench/synth/unpack_drain_star_raise.py @@ -0,0 +1,99 @@ +# Star-unpack of an iterator of unknown length: `f(*it)` is the consumer that +# routes through `_unpackiterable_unknown_length`, the drain loop jd1 +# (`unpackiterable_driver`) traces and compiles. `a, b, c = it` does NOT reach +# it — the exact-arity form has its own `unpack_sequence_exact` loop — so this +# is the shape that exercises the second jit driver end to end. +# +# Three cases, each driven past the jd1 trace threshold: +# 1. plain drain to exhaustion, +# 2. an iterator that raises a non-StopIteration exception mid-drain and +# re-raises it on every later `next()`, +# 3. the same but NOT exhaustion-stable — it raises once and reports +# exhausted afterwards. A drain that loses the in-flight exception and +# lets the interpreter re-derive it by calling `next()` again turns this +# into a silent early return, so the case pins the exception actually +# travelling out of the compiled drain / blackhole resume rather than +# being re-discovered. +N = 4000 +ROUNDS = 12 + + +class Counting: + def __init__(self, n): + self.i = 0 + self.n = n + + def __iter__(self): + return self + + def __next__(self): + if self.i >= self.n: + raise StopIteration + self.i += 1 + return self.i + + +class Raising: + """Raises at `at` and keeps raising — exhaustion-stable.""" + + def __init__(self, n, at): + self.i = 0 + self.n = n + self.at = at + + def __iter__(self): + return self + + def __next__(self): + if self.i == self.at: + raise ValueError("raising") + if self.i >= self.n: + raise StopIteration + self.i += 1 + return self.i + + +class RaisingOnce: + """Raises at `at` exactly once, then reports exhausted.""" + + def __init__(self, n, at): + self.i = 0 + self.n = n + self.at = at + self.fired = False + + def __iter__(self): + return self + + def __next__(self): + if self.i == self.at and not self.fired: + self.fired = True + raise ValueError("raising-once") + if self.fired or self.i >= self.n: + raise StopIteration + self.i += 1 + return self.i + + +def count(*args): + return len(args) + + +def main(): + drained = 0 + stable = 0 + once = 0 + for _ in range(ROUNDS): + drained += count(*Counting(N)) + try: + count(*Raising(N, N // 2)) + except ValueError as e: + stable += len(str(e)) + try: + count(*RaisingOnce(N, N // 2)) + except ValueError as e: + once += len(str(e)) + print(drained, stable, once) + + +main() diff --git a/pyre/pyre-interpreter/src/baseobjspace.rs b/pyre/pyre-interpreter/src/baseobjspace.rs index 93f80e7b0cb..ec01670b3c4 100644 --- a/pyre/pyre-interpreter/src/baseobjspace.rs +++ b/pyre/pyre-interpreter/src/baseobjspace.rs @@ -10201,7 +10201,11 @@ pub fn unpackiterable( generator_unpack_into(w_iterator, &mut lst_w)?; return Ok(lst_w); } - _unpackiterable_unknown_length(w_iterator, w_iterable) + // The drain returns the grown `W_List` ref (`Type::Ref`, RPython's + // `return items`); read it back into the `Vec` the Rust + // signature promises here, outside the traced/blackholed drain body. + let w_list = _unpackiterable_unknown_length(w_iterator, w_iterable)?; + Ok(drain_collect_items(w_list)) } else { // baseobjspace.py:996-998 — known-length path with shape validation. _unpackiterable_known_length_jitlook(w_iterator, expected_length as usize) @@ -10383,11 +10387,11 @@ fn generator_unpack_into( fn _unpackiterable_unknown_length( w_iterator: PyObjectRef, w_iterable: PyObjectRef, -) -> Result, crate::PyError> { +) -> Result { // baseobjspace.py:1005-1008 — `try: items = newlist_hint(length_hint(...)) // except MemoryError: items = []`. let _ = length_hint(w_iterable, 0)?; - let items = pyre_object::listobject::w_list_new_object(Vec::new()); + let items = pyre_object::listobject::w_list_new_empty(); let _roots = pyre_object::gc_roots::push_roots(); pyre_object::gc_roots::pin_root(items); // baseobjspace.py:1010 `greenkey = self.iterator_greenkey(w_iterator)`. @@ -10397,7 +10401,7 @@ fn _unpackiterable_unknown_length( // `unpackiterable_driver.jit_merge_point(greenkey=greenkey)`. unpackiterable_driver.jit_merge_point(greenkey, w_iterator, items); match next(w_iterator) { - Ok(w_item) => unsafe { pyre_object::listobject::w_list_append(items, w_item) }, + Ok(w_item) => unsafe { pyre_object::listobject::drain_list_append(items, w_item) }, // `except OperationError as e: if not e.match(space, // w_StopIteration): raise; break` — the StopIteration test rides // inside the handler (`e` is bound once, consumed only on the @@ -10410,12 +10414,38 @@ fn _unpackiterable_unknown_length( } } } + // `return items` — hand the grown `W_List` back as a single ref, matching + // the driver's `Type::Ref` result and RPython's `return items`. The + // `W_List` → `Vec` readback is caller-side host glue + // (`drain_collect_items`), kept out of the traced/blackholed drain body so + // the blackhole epilogue is a plain `ref_return` rather than a + // multi-word (`Vec`, sret-ABI) residual the single-register residual-call + // handlers cannot invoke. + Ok(items) +} + +/// Copy the drained `W_List` back into a `Vec` for the Rust +/// return type — the caller-side host glue for `unpackiterable`'s `Vec` +/// contract (RPython returns the list object directly; the `Vec` growth, +/// `Range`-indexed readback and `Option::unwrap` are pyre-only). Called +/// AFTER the drain returns the `W_List` ref, outside the traced/blackholed +/// drain body, so it never surfaces as a residual funcptr in the drain +/// jitcode. `#[dont_look_inside]` keeps the host plumbing opaque should a +/// caller ever be traced. +/// +/// Pins `items` across the readback: an Integer/Float-strategy `getitem` +/// boxes each element through the moving collector (`w_int_new` / +/// `w_float_new`), which can relocate `items`. +#[majit_macros::dont_look_inside] +pub(crate) fn drain_collect_items(items: PyObjectRef) -> Vec { + let _roots = pyre_object::gc_roots::push_roots(); + pyre_object::gc_roots::pin_root(items); let n = unsafe { pyre_object::listobject::w_list_len(items) }; let mut out: Vec = Vec::with_capacity(n); for i in 0..n as i64 { out.push(unsafe { pyre_object::listobject::w_list_getitem(items, i).unwrap() }); } - Ok(out) + out } /// pypy/interpreter/baseobjspace.py:1080-1108 `length_hint`. diff --git a/pyre/pyre-interpreter/src/jit_fnaddr.rs b/pyre/pyre-interpreter/src/jit_fnaddr.rs index 992769a574b..412b4d93534 100644 --- a/pyre/pyre-interpreter/src/jit_fnaddr.rs +++ b/pyre/pyre-interpreter/src/jit_fnaddr.rs @@ -366,13 +366,15 @@ pub fn jit_trace_fnaddrs() -> Vec<(&'static str, i64)> { // `unpackiterable_driver` (jd1) portal callees. Its extracted body // (`_unpackiterable_unknown_length`) residual-calls `next(w_iterator)` and - // `w_list_append(items, w_item)` directly in source, so the codewriter + // `drain_list_append(items, w_item)` directly in source, so the codewriter // records the bare source paths; without a runtime binding the funcptr // constants fall back to a `symbolic_fnaddr_for_path` hash the residual // handler cannot resolve. `next` returns `Result` // and rides the Ref-returning `bh_next` bridge (publishes StopIteration, - // unlike the FOR_ITER `jit_next`); `w_list_append` is a `-> ()` residual - // and binds its Rust `fn` directly. + // unlike the FOR_ITER `jit_next`); `drain_list_append` is a `-> ()` + // `dont_look_inside` seam over `w_list_append` that collapses append's + // strategy/grow helper subtree to one registered residual (the global + // `list.append` stays traced), and binds its Rust `fn` directly. push_alias_pair( &mut entries, "pyre_interpreter::baseobjspace::next", @@ -384,15 +386,55 @@ pub fn jit_trace_fnaddrs() -> Vec<(&'static str, i64)> { "next", crate::runtime_ops::bh_next as *const (), ); - let w_list_append: unsafe fn(pyre_object::PyObjectRef, pyre_object::PyObjectRef) = - pyre_object::listobject::w_list_append; + let drain_list_append: unsafe fn(pyre_object::PyObjectRef, pyre_object::PyObjectRef) = + pyre_object::listobject::drain_list_append; push_alias_pair( &mut entries, - "pyre_object::listobject::w_list_append", - "pyre_object::w_list_append", - w_list_append as *const (), + "pyre_object::listobject::drain_list_append", + "pyre_object::drain_list_append", + drain_list_append as *const (), + ); + push_fnaddr( + &mut entries, + "drain_list_append", + drain_list_append as *const (), + ); + + // The drain's prologue (`w_list_new_empty`) and epilogue + // (`drain_collect_items`) wrap the `Vec`/`Range`/`Option` host plumbing + // that RPython does not have; both are `#[dont_look_inside]` residuals and + // bind their `fn` directly (`-> PyObjectRef` / `-> Vec`). + // `w_list_new_object` is residualized (`#[dont_look_inside]`) but was + // unregistered; bind it too so any direct residual site resolves. + let w_list_new_empty: fn() -> pyre_object::PyObjectRef = + pyre_object::listobject::w_list_new_empty; + push_alias_pair( + &mut entries, + "pyre_object::listobject::w_list_new_empty", + "pyre_object::w_list_new_empty", + w_list_new_empty as *const (), + ); + push_fnaddr( + &mut entries, + "w_list_new_empty", + w_list_new_empty as *const (), + ); + let w_list_new_object: fn(Vec) -> pyre_object::PyObjectRef = + pyre_object::listobject::w_list_new_object; + push_alias_pair( + &mut entries, + "pyre_object::listobject::w_list_new_object", + "pyre_object::w_list_new_object", + w_list_new_object as *const (), + ); + let drain_collect_items: fn(pyre_object::PyObjectRef) -> Vec = + crate::baseobjspace::drain_collect_items; + push_alias_pair( + &mut entries, + "pyre_interpreter::baseobjspace::drain_collect_items", + "pyre_interpreter::drain_collect_items", + drain_collect_items as *const (), ); - push_fnaddr(&mut entries, "w_list_append", w_list_append as *const ()); push_alias_pair( &mut entries, diff --git a/pyre/pyre-interpreter/src/stack_check.rs b/pyre/pyre-interpreter/src/stack_check.rs index f60c582dd4e..896c57f436a 100644 --- a/pyre/pyre-interpreter/src/stack_check.rs +++ b/pyre/pyre-interpreter/src/stack_check.rs @@ -428,6 +428,45 @@ pub fn is_jit_overflow_pending() -> bool { TL_JIT_PENDING_EXCEPTION.with(|slot| slot.get() != 0) } +/// Park `err` in this thread's pending slot so the next interpreter call +/// boundary re-raises it through [`drain_jit_pending_exception`]. +/// +/// The prologue stack check is one producer; the other is a JIT driver whose +/// compiled loop raised while the Rust caller loop, not a portal runner, owns +/// the continuation. `warmspot.py:998-1005` re-raises an +/// `ExitFrameWithExceptionRef` out of `ll_portal_runner` straight into the +/// portal's caller; pyre's second driver (`unpackiterable_driver`) is entered +/// from a merge-point hook returning `()`, so the error travels through this +/// slot instead of a return value. Delivery is at the caller loop's next +/// dispatch, which for the drain is the `next(w_iterator)` that immediately +/// follows the hook — before `__next__` re-runs, so no side effect repeats. +/// +/// Overwrites any slot content: a stack overflow raised while a driver error +/// is parked (or vice versa) is one thread unwinding for two reasons, and the +/// caller sees whichever was stored last, as it would with `pos_exception()`. +pub fn park_jit_pending_error(mut err: PyError) { + let obj = err.to_exc_object(); + if !obj.is_null() { + set_jit_pending_exception(obj); + } +} + +/// Shadow-stack root walker for the pending-exception slot: a parked +/// `W_BaseException` is reachable from nothing else until the next call +/// boundary drains it, and the drain crosses collecting code +/// (`next()`/`__next__`). Registered once at JIT init next to the other +/// `register_extra_root_walker` slots. +pub fn walk_jit_pending_exception(visitor: &mut dyn FnMut(&mut majit_ir::GcRef)) { + TL_JIT_PENDING_EXCEPTION.with(|slot| { + let obj = slot.get(); + if obj != 0 { + let mut r = majit_ir::GcRef(obj as usize); + visitor(&mut r); + slot.set(r.0 as i64); + } + }); +} + /// rpython/translator/c/src/stack.h:42 `LL_stack_criticalcode_start`. /// Clears this thread's `report_error` mirror so its slowpath will not signal /// a stack overflow during short critical-code sections. diff --git a/pyre/pyre-jit-trace/src/state.rs b/pyre/pyre-jit-trace/src/state.rs index ed3dcddacdf..809af426fae 100644 --- a/pyre/pyre-jit-trace/src/state.rs +++ b/pyre/pyre-jit-trace/src/state.rs @@ -775,7 +775,15 @@ pub fn raw_code_for_jitcode_index(jitcode_index: i32) -> Option<*const CodeObjec METAINTERP_SD.with(|r| { let sd = r.borrow(); let idx = jitcode_index as usize; - sd.jitcodes.get(idx).map(|jc| unsafe { jc.raw_code() }) + // A novable drain portal (the jd1 unpackiterable driver) is a native + // function with no Python `CodeObject`; its degenerate PyJitCode carries + // a null `code_ptr`. Report that as "no raw code" so the instruction- + // decoding consumers (bare-reraise probe, traceback lineno) skip it + // instead of dereferencing null. + sd.jitcodes.get(idx).and_then(|jc| { + let raw = unsafe { jc.raw_code() }; + (!raw.is_null()).then_some(raw) + }) }) } @@ -1146,6 +1154,66 @@ pub fn frame_value_count_at(jitcode_index: i32, pc: i32) -> usize { }) } +/// [`frame_value_count_at`] for a driver whose frames are numbered in the +/// build-time `jitcode_runtime` tables instead of the runtime +/// `MetaInterpStaticData` store. +/// +/// pyre has two jitcode numbering spaces. jd0 (`pyframe_driver`) numbers +/// Python-bytecode jitcodes into `MetaInterpStaticData.jitcodes`, keyed by +/// CodeObject, and interns their `-live-` triples into +/// `metainterp_sd.liveness_info` (`pyjitpl.py:2264`) as tracing discovers them. +/// A novable driver over an extracted interpreter body — jd1 +/// `unpackiterable_driver`, whose jitcode is the +/// `_unpackiterable_unknown_length` graph plus its inlined build-time callees — +/// numbers against `jitcode_runtime::all_jitcodes()`, with `-live-` offsets +/// baked at extraction into `jitcode_runtime::all_liveness()`. +/// +/// Decoding one space's coordinate against the other's tables does not fail +/// loudly, which is why the store has to be picked per driver rather than tried +/// and retried: the runtime store's low indices hold unrelated PyCode jitcodes +/// that decode at the same pc and hand back a mistyped count (the drain's 2 refs +/// read as ints → `Const::getint on Ref`). Same split, and same reasoning, as +/// the `novable` arms of `call_jit.rs`'s `blackhole_resume_via_rd_numb`. +/// +/// Installed on jd1's `JitDriverStaticData::frame_value_count_fn`, so only that +/// driver's guard metadata decodes here. +pub fn build_time_frame_value_count_at(jitcode_index: i32, pc: i32) -> usize { + // `pyjitpl.py:2236` `self.op_live = self.opcode_implementations...` — the + // `live/` byte of the table the jitcode's bytes were assembled with. Read it + // off the build-time table rather than `blackhole_control_opcodes()`, whose + // value comes from the runtime assembler's `insns`; the two agree + // (`jitcode_runtime.rs` `build_default_bh_builder_matches_insns_table`), but + // the build-time decode should not depend on the runtime store being set up. + let Some(op_live) = crate::jitcode_runtime::insns_opname_to_byte() + .get("live/") + .copied() + else { + return 0; + }; + let jitcode = match crate::jitcode_runtime::get_jitcode_by_index(jitcode_index as usize) { + Some(jc) => jc, + None => return 0, + }; + let all_liveness = crate::jitcode_runtime::all_liveness(); + if pc >= 0 && jitcode.can_decode_live_vars(pc as usize, op_live) { + let off = jitcode.get_live_vars_info(pc as usize, op_live); + if off + 2 < all_liveness.len() { + let length_i = all_liveness[off] as usize; + let length_r = all_liveness[off + 1] as usize; + let length_f = all_liveness[off + 2] as usize; + return length_i + length_r + length_f; + } + } + // Same fail-loud contract as `frame_value_count_at`: a published resume + // coordinate that does not decode violates the capture contract. + panic!( + "build_time_frame_value_count_at: no decodable `-live-` for \ + jitcode_index={jitcode_index} pc={pc} (code_len={}, all_liveness.len={})", + jitcode.code.len(), + all_liveness.len(), + ); +} + /// Resolve the JitCode byte offset the full-body walk should RESUME at for a /// bridge whose guard carried `carried_jitcode_pc`. Agrees with where the /// blackhole resumes (`call_jit.rs` `resolve_jitcode`): a kept-stack branch diff --git a/pyre/pyre-jit-trace/src/unpack_state.rs b/pyre/pyre-jit-trace/src/unpack_state.rs index ab1419382bf..53cea231d5a 100644 --- a/pyre/pyre-jit-trace/src/unpack_state.rs +++ b/pyre/pyre-jit-trace/src/unpack_state.rs @@ -97,6 +97,10 @@ impl UnpackJitState { // first trace closes the loop on the `goto` back-edge instead of // unrolling the whole drain to the StopIteration finish. sd.no_loop_header = true; + // The drain's frames are numbered in the build-time `jitcode_runtime` + // tables, not the CodeObject-keyed runtime store jd0 grows, so guard + // metadata must decode `-live-` there. + sd.frame_value_count_fn = Some(crate::state::build_time_frame_value_count_at); sd } } diff --git a/pyre/pyre-jit/src/call_jit.rs b/pyre/pyre-jit/src/call_jit.rs index 1ef6b8208f1..d21411c6415 100644 --- a/pyre/pyre-jit/src/call_jit.rs +++ b/pyre/pyre-jit/src/call_jit.rs @@ -1817,6 +1817,7 @@ fn jit_blackhole_resume_from_guard( Some(&storage.rd_virtuals), deadframe_types.as_deref(), guard_exc, + false, // CALL_ASSEMBLER portal is jd0 (virtualizable) ); return handle_blackhole_result(result, actual_green_key); } @@ -1992,6 +1993,12 @@ pub fn blackhole_resume_via_rd_numb( rd_virtuals: Option<&[std::rc::Rc]>, deadframe_types: Option<&[majit_ir::Type]>, guard_exc: i64, + // compile.py:990 `vinfo = self.jitdriver_sd.virtualizable_info`: a novable + // jitdriver (jd1 `unpackiterable_driver`) has `vinfo=None`, so its resume + // data carries no vable section. Decoding with the (jd0) vinfo would try to + // consume a phantom vable and dereference garbage; a novable resume passes + // `None` for both the vinfo and the per-frame virtualizable handle. + novable: bool, ) -> BlackholeResult { let nbody_debug = pyre_nbody_debug_enabled(); use majit_metainterp::resume; @@ -2014,16 +2021,39 @@ pub fn blackhole_resume_via_rd_numb( }; // resume.py:1339 jitcodes[jitcode_pos]: resolve jitcode_index + pc - // through the trace-side MetaInterpStaticData.jitcodes store. + // through the store the frame numbered against. let resolve_jitcode = |jitcode_index: i32, pc: i32| -> Option { if pc < 0 { return None; } + let op_live = pyre_jit_trace::state::blackhole_control_opcodes().0 as u8; + // A novable jitdriver (jd1 `unpackiterable_driver`) numbers its drain + // frames' `jitcode_index` in the build-time `jitcode_runtime` table + // (the extracted `_unpackiterable_unknown_length` body plus its inlined + // build-time callees), NOT the runtime `MetaInterpStaticData.jitcodes` + // store. The runtime store is a different index space keyed by Python + // CodeObject, so its low indices hold unrelated jd0 PyCode jitcodes + // whose liveness at the same pc mistypes the drain's 2 refs as ints + // (`Const::getint on Ref`). Resolve against the same table the frame + // numbered against — mirroring the driver's own bridge resume + // (`run_compiled_detailed_with_bridge_keyed`, jitdriver.rs), which + // resolves through the flat build-time `jitcode_registry`. + if novable { + let canonical = + pyre_jit_trace::jitcode_runtime::get_jitcode_by_index(jitcode_index as usize)?; + if !canonical.can_decode_live_vars(pc as usize, op_live) { + return None; + } + let core = majit_metainterp::JitCode::from_canonical((*canonical).clone()); + return Some(resume::ResolvedJitCode::new( + std::sync::Arc::new(core), + pc as usize, + )); + } let pyjitcode = pyre_jit_trace::state::pyjitcode_for_jitcode_index(jitcode_index)?; if pyjitcode.has_abort_opcode() { return None; } - let op_live = pyre_jit_trace::state::blackhole_control_opcodes().0 as u8; // A published resume frame carries a decodable JitCode `-live-` // coordinate. An unrepresentable frame declines this blackhole path. let resolved_pc = if pyjitcode.jitcode.can_decode_live_vars(pc as usize, op_live) { @@ -2031,13 +2061,6 @@ pub fn blackhole_resume_via_rd_numb( } else { return None; }; - // resume.py:1339 reads from one `jitcodes[]` store. pyre's - // `state::code_for_jitcode_index` indices name the runtime - // `MetaInterpStaticData.jitcodes` table keyed by CodeObject; they - // are not the same index space as `jitcode_runtime::ALL_JITCODES` - // (build-time opcode-dispatch artifacts). Do not cross-lookup the - // canonical store by `jitcode_index` until pyre actually shares a - // single JitCode object graph end-to-end. Some( resume::ResolvedJitCode::new(pyjitcode.jitcode.clone(), resolved_pc) .with_virtualizable_stack_base(pyjitcode.metadata.stack_base), @@ -2079,11 +2102,27 @@ pub fn blackhole_resume_via_rd_numb( // unused in pyre (no greenfield_info installed on the driver). let (driver, driver_vinfo) = crate::eval::driver_pair(); let vinfo_dyn: &dyn resume::VirtualizableInfo = driver_vinfo.as_ref(); + // A novable driver's resume data has no vable section; pass no vinfo so the + // decoder skips `consume_vable_info` entirely. + let vinfo_arg: Option<&dyn resume::VirtualizableInfo> = + if novable { None } else { Some(vinfo_dyn) }; let vrefinfo_dyn: &dyn resume::VRefInfo = driver.meta_interp().virtualref_info(); let allocator = crate::eval::PyreBlackholeAllocator; // pyjitpl.py:2264: metainterp_sd.liveness_info — single shared pool. // Snapshot once per call so the slice outlives ResumeDataDirectReader. - let all_liveness = pyre_jit_trace::state::liveness_info_snapshot(); + let runtime_liveness = pyre_jit_trace::state::liveness_info_snapshot(); + // A novable drain frame's `-live-` markers carry 2-byte offsets baked at + // extraction into the build-time `ALL_LIVENESS` byte stream, NOT the + // runtime-accumulated `metainterp_sd.liveness_info` that jd0 tracing grows + // via `intern_liveness`. Decoding a baked offset against the runtime buffer + // reads an unrelated jd0 frame's liveness triple (mistyping the drain's 2 + // refs as ints → `Const::getint on Ref`). Decode against the same build-time + // table the drain jitcode was resolved from. + let all_liveness: &[u8] = if novable { + pyre_jit_trace::jitcode_runtime::all_liveness() + } else { + &runtime_liveness + }; // Scope the &mut to chain construction; the run() loop below uses // release_bh_rd to drop and re-acquire the borrow. let bh = BH_BUILDER_RD.with(|cell| unsafe { @@ -2094,13 +2133,13 @@ pub fn blackhole_resume_via_rd_numb( &resolve_jitcode, rd_numb, rd_consts, - &all_liveness, + all_liveness, deadframe, deadframe_types, // deadframe_types: decode_ref boxes TAGBOX ints rd_virtuals_slice, // rd_virtuals rd_guard_pendingfields, // rd_guard_pendingfields Some(vrefinfo_dyn), // resume.py:1314 metainterp_sd.virtualref_info - Some(vinfo_dyn), // resume.py:1312 self.jitdriver_sd.virtualizable_info + vinfo_arg, // resume.py:1312 self.jitdriver_sd.virtualizable_info None, // resume.py:1316 greenfield_info unused in pyre None, // heap PyFrame identity remains the live TAGBOX &allocator, @@ -2116,13 +2155,17 @@ pub fn blackhole_resume_via_rd_numb( // resume.py:1404: virtualizable_ptr was read by consume_vable_info // from the vable section. Set on the blackhole for vable bytecodes. - if virtualizable_ptr != 0 { - bh.virtualizable_ptr = virtualizable_ptr; - } else if !deadframe.is_empty() { - // Fallback for guards without vable section. - bh.virtualizable_ptr = deadframe[0]; + // A novable resume runs no vable opcodes, so leave both the pointer and + // the vinfo handle unset (null) instead of borrowing jd0's. + if !novable { + if virtualizable_ptr != 0 { + bh.virtualizable_ptr = virtualizable_ptr; + } else if !deadframe.is_empty() { + // Fallback for guards without vable section. + bh.virtualizable_ptr = deadframe[0]; + } + bh.virtualizable_info = crate::eval::get_virtualizable_info(); } - bh.virtualizable_info = crate::eval::get_virtualizable_info(); // resume.py:1332-1343 builds the caller chain (`nextblackholeinterp`) // but does not set the virtualizable-info handle on each frame. pyre // stores the vinfo per-`BlackholeInterpreter` (RPython reads it from @@ -3704,6 +3747,7 @@ pub extern "C" fn wasm_ca_resume_deopt(frame_ptr: i64, compiled_ptr: i64) -> i64 &raw_values, &exit_layout, guard_exc, + false, ); handle_blackhole_result(bh, green_key).unwrap_or(0) } diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index 200fa681746..cd6c6a2f65b 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -3303,6 +3303,12 @@ fn install_gc_root_walkers() { // reach through their raw TLS cells: the call-assembler FFI stash and the // no-handler trace→portal stash. Mirrors `walk_pending_call_error`. majit_gc::shadow_stack::register_extra_root_walker(crate::call_jit::walk_last_ca_exception); + // Exception parked for the next interpreter call boundary (JIT prologue + // overflow, or a jd1 drain error handed back to the caller loop): live in + // a raw TLS cell across the collecting code that runs before the drain. + majit_gc::shadow_stack::register_extra_root_walker( + pyre_interpreter::stack_check::walk_jit_pending_exception, + ); majit_gc::shadow_stack::register_extra_root_walker( pyre_jit_trace::trace::walk_walk_end_propagated_exception, ); @@ -4810,15 +4816,45 @@ fn set_jit_param_string_via_warmstate(text: &str) -> Result<(), ()> { apply_jit_param_string(ws, text) } -/// WIP gate for jd1 (`unpackiterable_driver`) live-path residual execution. -/// OFF by default: the merge-point hook stays inert so the second driver does -/// not perturb jd0 until the full activation slice (blackhole entry + -/// compiled-loop reuse) lands. `PYRE_JD1=1` opts into driving the -/// `JitCodeMachine` trace of `_unpackiterable_unknown_length` on the live -/// unpack path. +/// Gate for jd1 (`unpackiterable_driver`): the merge-point hook drives a +/// `JitCodeMachine` trace of `_unpackiterable_unknown_length` on hot unpack +/// sites, closing and compiling the drain loop. ON by default, alongside the +/// main JIT. Opt out with `PYRE_NO_JD1` (or `PYRE_JD1=0`); it also follows the +/// master JIT off-switches (`PYRE_NO_JIT`, `PYRE_JIT=0`) so "no JIT" means no +/// jd1. +/// +/// The walk executes the drain's residuals concretely, so it depends on the +/// drain lowering to the fused exception-edge shape: when +/// `front::result_exc::try_fuse_drain_match` declines, the `StopIteration` +/// ctor/eq residuals survive with `symbolic_fnaddr_for_path` addresses and the +/// walk calls a hash (`EXC_BAD_ACCESS`). `unpackiterable_drain_match_fuses_to_kind_test` +/// guards that the fusion still fires. fn jd1_experiment_enabled() -> bool { static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); - *ENABLED.get_or_init(|| std::env::var_os("PYRE_JD1").is_some()) + *ENABLED.get_or_init(|| { + if std::env::var_os("PYRE_NO_JD1").is_some() + || std::env::var("PYRE_JD1").as_deref() == Ok("0") + { + return false; + } + // No JIT at all → no jd1. + if std::env::var_os("PYRE_NO_JIT").is_some() + || std::env::var("PYRE_JIT").as_deref() == Ok("0") + { + return false; + } + true + }) +} + +/// Whether jd1 enters the compiled drain loop live on `RunCompiled` (the JIT +/// speedup) versus only compiling and registering it (the cooperative-drain +/// fallback, where the interpreter caller keeps draining). ON by default; +/// `PYRE_JD1_NO_ENTER` keeps the compiled loop registered but leaves the drain +/// to the interpreter caller. +fn jd1_enter_enabled() -> bool { + static E: std::sync::OnceLock = std::sync::OnceLock::new(); + *E.get_or_init(|| std::env::var_os("PYRE_JD1_NO_ENTER").is_none()) } thread_local! { @@ -4867,7 +4903,8 @@ fn jd1_counter_tick(green_key: u64) -> bool { /// jd1 (`unpackiterable_driver`) merge-point hook body. On the hot iterator /// type, drives one `JitCodeMachine` trace of the extracted /// `_unpackiterable_unknown_length` loop with `w_iterator`/`items` as the two -/// `reds='auto'` values. Inert unless `PYRE_JD1=1`. +/// `reds='auto'` values. Inert when jd1 is disabled (see +/// [`jd1_experiment_enabled`]). fn unpack_merge_point_jit( greenkey: pyre_object::PyObjectRef, w_iterator: pyre_object::PyObjectRef, @@ -4910,8 +4947,10 @@ fn unpack_merge_point_jit( /// each residual (`self.next`, `items.append`) concretely on the shared reds, /// so this advances the live iterator and grows the live list in place; the /// Rust caller loop then resumes from the advanced state (cooperative drain). -/// The recorded trace is discarded — compiled-loop reuse and blackhole -/// entry are not wired. +/// On a hot green key the recorded trace is closed into a compiled drain loop +/// (`CloseLoop` → `compile_loop`); subsequent hits re-enter it live when +/// [`jd1_enter_enabled`], draining `items` in compiled code with a blackhole +/// resume on guard failure. fn drive_unpack_iterable_trace( green_key: u64, greenkey_raw: pyre_object::PyObjectRef, @@ -5013,6 +5052,121 @@ fn drive_unpack_iterable_trace( }; eprintln!("[jd1] force_start_tracing -> {name}"); } + // Live-path enter (on by default; see `jd1_enter_enabled`): on RunCompiled, + // run the compiled drain loop with the shared `(w_iterator, items)` reds so + // it drains `items` in compiled code (residual `next`/`append` executed on + // the live path). `items` is a shared heap list, so the drain lands in + // place. A guard failure is resumed in the blackhole interpreter + // (compile.py:710-716) so the in-flight iteration completes instead of being + // dropped; `ContinueRunningNormally` re-enters the compiled loop, and the + // drain ends at the StopIteration guard exit. The interp `ln` loop then + // exits on its own next `next()` (StopIteration). Uses the values-based + // runner + the frameless `resume_in_blackhole_from_exit_layout` because jd1 + // is novable with raw `(w_iterator, items)` reds, not a PyFrame the + // frame-projecting `execute_assembler`/`handle_fail` path expects. + if matches!(action, BackEdgeAction::RunCompiled) && jd1_enter_enabled() { + // Root the shared reds across the compiled run (it may collect). + // `items` is already pinned by `ln`; re-pinning is a harmless dup that + // pops with `ln`'s root scope. `w_iterator` is a bare `ln` local. + pyre_object::gc_roots::pin_root(w_iterator); + pyre_object::gc_roots::pin_root(items); + let before = unsafe { pyre_object::listobject::w_list_len(items) }; + // A drain-time error that is not the loop-exit StopIteration has to + // travel out of the unpack; `ln` cannot re-derive it, because calling + // `next()` a second time re-enters an iterator that has already raised + // (a generator is closed by then, and a plain `__next__` need not be + // exhaustion-stable — both report StopIteration and the real error is + // lost). Parked here and re-raised by the `drain_jit_pending_exception` + // at `ln`'s very next call dispatch, before `__next__` re-runs. + // `warmspot.py:998-1005` propagates the same `ExitFrameWithExceptionRef` + // by re-raising out of `ll_portal_runner`; jd1 is entered from a + // merge-point hook with no return value, so the slot carries it. + let mut pending_err: Option = None; + loop { + // Extract owned copies so the `&mut meta` borrow held by the + // `CompileResult` is released before the blackhole resume (which + // re-acquires `driver_pair()`). + let Some((is_finish, fail_index, has_storage, values, exit_layout, guard_exc)) = meta + .run_compiled_detailed_with_values(green_key, &live_values) + .map(|r| { + ( + r.is_finish, + r.fail_index, + r.exit_layout.storage.is_some(), + r.values.clone(), + r.exit_layout.clone(), + r.exception.exc_value, + ) + }) + else { + // No compiled loop / backend refused — leave the rest to `ln`. + break; + }; + if is_finish { + break; + } + // A normal back-edge JUMP (`fail_index == u32::MAX`) or a guard exit + // that carries no resume storage cannot be blackhole-resumed; hand + // the rest to `ln` rather than panic in the resume decoder. + if fail_index == u32::MAX || !has_storage { + break; + } + // A guard exit already carrying a non-StopIteration exception is + // the drain's `return Err(e)` arm. Resuming it would only walk the + // drain's re-raise tail to re-derive the same error, so take it + // here; the iteration is over either way. + if guard_exc != 0 { + let err = unsafe { + pyre_interpreter::error::PyError::from_exc_object( + guard_exc as pyre_object::PyObjectRef, + ) + }; + if err.kind != pyre_interpreter::PyErrorKind::StopIteration { + pending_err = Some(err); + break; + } + } + // compile.py:710-716 resume_in_blackhole: complete the in-flight + // `next()`/`append` and run forward to the next merge point. + let bh = resume_in_blackhole_from_exit_layout(&values, &exit_layout, guard_exc, true); + match bh { + // Merge point reached: re-enter the compiled drain. + crate::call_jit::BlackholeResult::ContinueRunningNormally { .. } => continue, + crate::call_jit::BlackholeResult::ExitFrameWithExceptionRef(err) => { + // StopIteration = drain complete; `ln` re-derives its own + // loop-exit StopIteration on its next `next()`. + if err.kind != pyre_interpreter::PyErrorKind::StopIteration { + pending_err = Some(err); + } + break; + } + crate::call_jit::BlackholeResult::DoneWithThisFrameVoid + | crate::call_jit::BlackholeResult::DoneWithThisFrameInt(_) + | crate::call_jit::BlackholeResult::DoneWithThisFrameRef(_) + | crate::call_jit::BlackholeResult::DoneWithThisFrameFloat(_) => break, + // Blackhole could not resume; leave the rest to `ln`. + crate::call_jit::BlackholeResult::Failed => break, + } + } + if dbg { + let after = unsafe { pyre_object::listobject::w_list_len(items) }; + eprintln!( + "[jd1] enter: drained {} items ({}→{})", + after - before, + before, + after + ); + } + // Discard any pending compiled-side StopIteration; `ln` re-derives its + // own loop exit. + let _ = pyre_interpreter::stack_check::drain_jit_pending_exception(); + let _ = crate::call_jit::take_ca_exception(); + // Parked last, so the clears above cannot swallow it. + if let Some(err) = pending_err { + pyre_interpreter::stack_check::park_jit_pending_error(err); + } + return; + } if !matches!(action, BackEdgeAction::StartedTracing) { return; } @@ -6939,6 +7093,10 @@ pub(crate) fn resume_in_blackhole_from_exit_layout( raw_values: &[i64], exit_layout: &CompiledExitLayout, guard_exc: i64, + // True when the failing guard belongs to a novable jitdriver (jd1 + // `unpackiterable_driver`): its resume data has no vable section, so the + // decode must not consume one. jd0 guards pass `false`. + novable: bool, ) -> crate::call_jit::BlackholeResult { if majit_metainterp::majit_log_enabled() { eprintln!( @@ -6975,6 +7133,7 @@ pub(crate) fn resume_in_blackhole_from_exit_layout( Some(&storage.rd_virtuals), deadframe_types.as_deref(), guard_exc, + novable, ); if majit_metainterp::majit_log_enabled() { eprintln!( @@ -7247,8 +7406,12 @@ fn execute_assembler( HandleFailOutcome::BridgeRaised(err) => Some(LoopResult::Done(Err(err))), HandleFailOutcome::ResumeInBlackhole => { // compile.py:710-716 / pyjitpl.py:2906 SwitchToBlackhole - let bh_result = - resume_in_blackhole_from_exit_layout(raw_values, exit_layout, guard_exc); + let bh_result = resume_in_blackhole_from_exit_layout( + raw_values, + exit_layout, + guard_exc, + false, + ); match &bh_result { crate::call_jit::BlackholeResult::ContinueRunningNormally { green_int, @@ -7570,8 +7733,12 @@ fn bound_reached( return Some(LoopResult::Done(Err(err))); } HandleFailOutcome::ResumeInBlackhole => { - let bh_result = - resume_in_blackhole_from_exit_layout(raw_values, exit_layout, guard_exc); + let bh_result = resume_in_blackhole_from_exit_layout( + raw_values, + exit_layout, + guard_exc, + false, + ); match &bh_result { crate::call_jit::BlackholeResult::ContinueRunningNormally { green_int, @@ -7762,8 +7929,12 @@ pub fn try_function_entry_jit(frame: &mut PyFrame) -> Option { return Some(Err(err)); } HandleFailOutcome::ResumeInBlackhole => { - let bh_result = - resume_in_blackhole_from_exit_layout(raw_values, exit_layout, guard_exc); + let bh_result = resume_in_blackhole_from_exit_layout( + raw_values, + exit_layout, + guard_exc, + false, + ); match &bh_result { crate::call_jit::BlackholeResult::ContinueRunningNormally { green_int, diff --git a/pyre/pyre-object/src/listobject.rs b/pyre/pyre-object/src/listobject.rs index e36c6bd484c..e134c561e24 100644 --- a/pyre/pyre-object/src/listobject.rs +++ b/pyre/pyre-object/src/listobject.rs @@ -515,6 +515,14 @@ pub fn w_list_new_object(items: Vec) -> PyObjectRef { w_list_new_with_strategy(items, ListStrategy::Object) } +/// Construct an empty list. Residualized so the `Vec::new()` backing-store +/// construction stays inside the opaque call rather than surfacing as a +/// separate residual funcptr at the caller's trace/blackhole level. +#[majit_macros::dont_look_inside] +pub fn w_list_new_empty() -> PyObjectRef { + w_list_new_object(Vec::new()) +} + fn w_list_new_with_strategy(items: Vec, strategy: ListStrategy) -> PyObjectRef { // `gct_fv_gc_malloc` bracket pattern (`framework.py:853-856`): // pin every PyObjectRef in `items` before the GC malloc paths @@ -921,6 +929,26 @@ pub unsafe fn w_list_append(obj: PyObjectRef, value: PyObjectRef) { } } +/// Drain-only `dont_look_inside` seam over [`w_list_append`]. +/// +/// The jd1 `_unpackiterable_unknown_length` driver compiles its drain loop and +/// blackhole-executes it on a guard failure. An *inlined* append would surface +/// each of its strategy/grow helpers (`object_push`, +/// `switch_to_correct_strategy`, typed-array grow, …) as a separate residual +/// funcptr the blackhole must resolve; wrapping the drain's append in one +/// `dont_look_inside` residual collapses that whole subtree to a single +/// registered address (`jit_fnaddr.rs`) — the same seam the drain already draws +/// around its `w_list_new_empty` prologue and `drain_collect_items` epilogue. +/// The global `list.append` path keeps calling [`w_list_append`] directly and +/// stays traced, so the append fold and the escape-flush replay are unaffected. +/// +/// # Safety +/// `obj` must point to a valid `W_ListObject`. +#[majit_macros::dont_look_inside] +pub unsafe fn drain_list_append(obj: PyObjectRef, value: PyObjectRef) { + w_list_append(obj, value) +} + /// Set the live length of an Integer-strategy list without reallocating /// or boxing — the undo of a spare-capacity append (`_ll_list_resize_ge`'s /// `l.length = newsize` run in reverse). The backing array already has