From 7a29db270cc8322b6b0d265c55e172abc5b53718 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Mon, 3 Aug 2026 18:07:20 +0900 Subject: [PATCH 1/2] jit: bound float-carrying calldescrs to the dispatch table's covered width `dispatch_classes_body!` enumerates one `extern "C"` signature per ordered argument-class sequence: every sequence up to 5 arguments, plus the all-Int sequences on to MAX_HOST_CALL_ARITY. That float-carrying width was stated nowhere on the descr-build side, so a wider float-bearing signature would reach the table's catch-all panic only at the first deopt that dispatches it. Add MAX_FLOAT_CARRYING_CALL_ARITY = 5 next to BhCallDescr and a debug_assert on its three constructors. Signatures past MAX_HOST_CALL_ARITY are exempt: residual_call.rs declines to emit the call at that width, so no blackhole dispatches them. Add call_stub tests for a float in the last covered slot and for the refusal one argument past it. Assisted-by: Claude --- majit/majit-backend/src/call_stub.rs | 54 +++++++++++++++++++ .../majit-translate/src/codewriter/jitcode.rs | 49 ++++++++++++++++- 2 files changed, 102 insertions(+), 1 deletion(-) diff --git a/majit/majit-backend/src/call_stub.rs b/majit/majit-backend/src/call_stub.rs index 7f73d3f75b7..75cf664cd14 100644 --- a/majit/majit-backend/src/call_stub.rs +++ b/majit/majit-backend/src/call_stub.rs @@ -32,6 +32,14 @@ pub enum ArgClass { /// as `extern "C" fn(f64, i64)` rather than a class-blind /// `extern "C" fn(i64, f64)`. That preserves SysV/AAPCS register-file order /// and the Microsoft x64 positional argument slots alike. +/// +/// Coverage: every ordered sequence up to 5 arguments, plus the all-`Int` +/// sequences on to `MAX_HOST_CALL_ARITY`. The float-carrying bound is mirrored +/// by `majit_translate::codewriter::jitcode::MAX_FLOAT_CARRYING_CALL_ARITY`, +/// which flags such a signature where the calldescr is built instead of at the +/// deopt that first runs it; widening the arms here means raising it there in +/// the same change (`majit-translate` cannot call into `majit-backend`, so the +/// bound is stated on both sides rather than shared). macro_rules! dispatch_classes_body { ($func:ident, $classes:ident, $args:ident, $ret:ty) => {{ type I = i64; @@ -1429,4 +1437,50 @@ mod tests { }; assert_eq!(result, 321.0); } + + extern "C" fn four_ints_then_float(a: i64, b: i64, c: i64, d: i64, e: f64) -> i64 { + a + b * 10 + c * 100 + d * 1000 + e as i64 * 10000 + } + + /// The widest float-carrying sequence the table covers, and the bound + /// `majit_translate::codewriter::jitcode::MAX_FLOAT_CARRYING_CALL_ARITY` + /// states on the descr-build side. + #[test] + fn call_stub_i_dispatches_a_float_in_the_last_covered_slot() { + let result = unsafe { + bh_call_i_dispatch( + four_ints_then_float as *const () as usize, + &[ + ArgClass::Int, + ArgClass::Int, + ArgClass::Int, + ArgClass::Int, + ArgClass::Float, + ], + &[1, 2, 3, 4, 5.0_f64.to_bits() as i64], + ) + }; + assert_eq!(result, 54321); + } + + /// One argument past that bound the table has no arm, so the call is + /// refused instead of being placed against the wrong signature. + #[test] + #[should_panic(expected = "unsupported arg class sequence")] + fn call_stub_i_refuses_a_float_past_the_covered_width() { + unsafe { + bh_call_i_dispatch( + four_ints_then_float as *const () as usize, + &[ + ArgClass::Int, + ArgClass::Int, + ArgClass::Int, + ArgClass::Int, + ArgClass::Int, + ArgClass::Float, + ], + &[1, 2, 3, 4, 5, 6.0_f64.to_bits() as i64], + ); + } + } } diff --git a/majit/majit-translate/src/codewriter/jitcode.rs b/majit/majit-translate/src/codewriter/jitcode.rs index 24f082b4325..0cd26c16f25 100644 --- a/majit/majit-translate/src/codewriter/jitcode.rs +++ b/majit/majit-translate/src/codewriter/jitcode.rs @@ -925,6 +925,49 @@ pub struct BhCallDescr { pub extra_info: majit_ir::descr::EffectInfo, } +/// Widest `arg_classes` the blackhole's residual-call dispatch table can build +/// a signature for once a float argument is present. +/// +/// That table (`majit-backend` `call_stub.rs`, `dispatch_classes_body!`) +/// enumerates one `extern "C"` signature per *ordered* argument-class sequence, +/// so the arm count doubles with each argument a float signature may occupy; +/// integer-only signatures need one arm per length and run to +/// [`MAX_HOST_CALL_ARITY`](super::insns::MAX_HOST_CALL_ARITY). +/// +/// Upstream has no such limit: `descr.py:604-605 create_call_stub` +/// source-generates `FuncType(ARGS, RESULT)` per calldescr at translation time, +/// so every sequence has a stub. Lifting it here means an ABI adapter that can +/// place arguments for a signature only known at run time. +pub const MAX_FLOAT_CARRYING_CALL_ARITY: usize = 5; + +/// Reject, at descr-build time, a signature the blackhole could not dispatch. +/// +/// A compiled trace places arbitrary signatures itself and wasm32 routes through +/// the host trampoline, so a too-wide descr does not fail when it is built or +/// when the trace runs — it fails the first time a guard failure hands the call +/// to the blackhole. Checking here points at whoever widened the callee instead +/// of at that deopt. +/// +/// `debug_assert` rather than a hard check: the dispatch table's own catch-all +/// stays as the release backstop, and this costs nothing on the build path. +fn debug_assert_dispatchable(arg_classes: &str) { + let arity = arg_classes.chars().count(); + if arity > super::insns::MAX_HOST_CALL_ARITY { + // Past that width the residual call is never emitted in the first place + // (`pyre-jit-trace` `residual_call.rs` declines and leaves the call to + // the interpreter), so the descr exists but no blackhole ever dispatches + // it. Flagging it here would turn an orderly decline into a panic. + return; + } + debug_assert!( + !arg_classes.contains('f') || arity <= MAX_FLOAT_CARRYING_CALL_ARITY, + "calldescr arg_classes {arg_classes:?} carries a float argument across \ + {arity} arguments; the residual-call dispatch table enumerates \ + float-bearing signatures only up to {MAX_FLOAT_CARRYING_CALL_ARITY}, so \ + the blackhole would panic on the first deopt that runs this call" + ); +} + impl BhCallDescr { pub fn from_call_descr(cd: &dyn majit_ir::descr::CallDescr) -> Self { // RPython `descr.py:456 CallDescr.result_type` is the char @@ -936,8 +979,10 @@ impl BhCallDescr { let (_, _, result_erased) = result_type_char_layout_key(result_class); let result_signed = cd.is_result_signed(); let result_size = cd.result_size(); + let arg_classes = cd.arg_classes(); + debug_assert_dispatchable(&arg_classes); Self { - arg_classes: cd.arg_classes(), + arg_classes, result_type: result_class, result_signed, result_size, @@ -960,6 +1005,7 @@ impl BhCallDescr { extra_info: majit_ir::descr::EffectInfo, ) -> Self { let (result_signed, result_size, result_erased) = result_type_char_layout_key(result_type); + debug_assert_dispatchable(&arg_classes); Self { arg_classes, result_type, @@ -981,6 +1027,7 @@ impl BhCallDescr { | majit_ir::value::Type::Float => 8, majit_ir::value::Type::Void => 0, }; + debug_assert_dispatchable(&arg_classes); Self { arg_classes, result_type: ir_type_to_result_char(result_type), From 5879f6fd8945fa59e9663d4c8ea72a4026224939 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Mon, 3 Aug 2026 19:11:02 +0900 Subject: [PATCH 2/2] jit: collect residual-call arguments into a stack buffer, assert the typed-call type list covers every slot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `collect_call_args` returned two `Vec`s, allocated on every residual call the blackhole dispatches. Return a fixed-size `CallArgs` buffer sized MAX_HOST_CALL_ARITY — the bound the dispatch table's widest arm already stops at — and update the dynasm, cranelift and shared `*_by_classes` call sites. The wasm host-trampoline path keeps `collect_call_args_positional`. `arg_classes_from_types` read `arg_types` with `.get(i)` and mapped a missing slot to `ArgClass::Int`, which would put an undescribed Float argument in an integer register. Both callers take the positional arguments and the type list from the same calldescr, so debug_assert that the type list is no shorter. Assisted-by: Claude --- majit/majit-backend-cranelift/src/compiler.rs | 39 +++++-- majit/majit-backend-dynasm/src/runner.rs | 39 +++++-- majit/majit-backend/src/call_stub.rs | 101 ++++++++++++++---- .../majit-metainterp/src/pyjitpl/dispatch.rs | 17 ++- 4 files changed, 157 insertions(+), 39 deletions(-) diff --git a/majit/majit-backend-cranelift/src/compiler.rs b/majit/majit-backend-cranelift/src/compiler.rs index b4c7567624e..80a9e392222 100644 --- a/majit/majit-backend-cranelift/src/compiler.rs +++ b/majit/majit-backend-cranelift/src/compiler.rs @@ -16822,13 +16822,19 @@ impl majit_backend::Backend for CraneliftBackend { if func == 0 { return 0; } - let (classes, args) = majit_backend::call_stub::collect_call_args( + let collected = majit_backend::call_stub::collect_call_args( &calldescr.arg_classes, args_i, args_r, args_f, ); - unsafe { majit_backend::call_stub::bh_call_i_dispatch(func as usize, &classes, &args) } + unsafe { + majit_backend::call_stub::bh_call_i_dispatch( + func as usize, + collected.classes(), + collected.args(), + ) + } } /// llmodel.py:818 bh_call_r: GcRef-returning parallel of `bh_call_i`. @@ -16848,14 +16854,19 @@ impl majit_backend::Backend for CraneliftBackend { if func == 0 { return majit_ir::GcRef::NULL; } - let (classes, args) = majit_backend::call_stub::collect_call_args( + let collected = majit_backend::call_stub::collect_call_args( &calldescr.arg_classes, args_i, args_r, args_f, ); - let raw = - unsafe { majit_backend::call_stub::bh_call_i_dispatch(func as usize, &classes, &args) }; + let raw = unsafe { + majit_backend::call_stub::bh_call_i_dispatch( + func as usize, + collected.classes(), + collected.args(), + ) + }; majit_ir::GcRef(raw as usize) } @@ -16876,13 +16887,19 @@ impl majit_backend::Backend for CraneliftBackend { if func == 0 { return 0.0; } - let (classes, args) = majit_backend::call_stub::collect_call_args( + let collected = majit_backend::call_stub::collect_call_args( &calldescr.arg_classes, args_i, args_r, args_f, ); - unsafe { majit_backend::call_stub::bh_call_f_dispatch(func as usize, &classes, &args) } + unsafe { + majit_backend::call_stub::bh_call_f_dispatch( + func as usize, + collected.classes(), + collected.args(), + ) + } } /// llmodel.py:834 bh_call_v / descr.py:590-605 create_call_stub @@ -16907,14 +16924,18 @@ impl majit_backend::Backend for CraneliftBackend { if func == 0 { return; } - let (classes, args) = majit_backend::call_stub::collect_call_args( + let collected = majit_backend::call_stub::collect_call_args( &calldescr.arg_classes, args_i, args_r, args_f, ); unsafe { - majit_backend::call_stub::bh_call_v_dispatch(func as usize, &classes, &args); + majit_backend::call_stub::bh_call_v_dispatch( + func as usize, + collected.classes(), + collected.args(), + ); } } diff --git a/majit/majit-backend-dynasm/src/runner.rs b/majit/majit-backend-dynasm/src/runner.rs index 87ee74a4af3..f3c0ca366c8 100644 --- a/majit/majit-backend-dynasm/src/runner.rs +++ b/majit/majit-backend-dynasm/src/runner.rs @@ -3355,13 +3355,19 @@ impl Backend for DynasmBackend { if func == 0 { return 0; } - let (classes, args) = majit_backend::call_stub::collect_call_args( + let collected = majit_backend::call_stub::collect_call_args( &calldescr.arg_classes, args_i, args_r, args_f, ); - unsafe { majit_backend::call_stub::bh_call_i_dispatch(func as usize, &classes, &args) } + unsafe { + majit_backend::call_stub::bh_call_i_dispatch( + func as usize, + collected.classes(), + collected.args(), + ) + } } /// llmodel.py:818 bh_call_r: GcRef-returning parallel of `bh_call_i`. @@ -3381,14 +3387,19 @@ impl Backend for DynasmBackend { if func == 0 { return majit_ir::GcRef::NULL; } - let (classes, args) = majit_backend::call_stub::collect_call_args( + let collected = majit_backend::call_stub::collect_call_args( &calldescr.arg_classes, args_i, args_r, args_f, ); - let raw = - unsafe { majit_backend::call_stub::bh_call_i_dispatch(func as usize, &classes, &args) }; + let raw = unsafe { + majit_backend::call_stub::bh_call_i_dispatch( + func as usize, + collected.classes(), + collected.args(), + ) + }; majit_ir::GcRef(raw as usize) } @@ -3409,13 +3420,19 @@ impl Backend for DynasmBackend { if func == 0 { return 0.0; } - let (classes, args) = majit_backend::call_stub::collect_call_args( + let collected = majit_backend::call_stub::collect_call_args( &calldescr.arg_classes, args_i, args_r, args_f, ); - unsafe { majit_backend::call_stub::bh_call_f_dispatch(func as usize, &classes, &args) } + unsafe { + majit_backend::call_stub::bh_call_f_dispatch( + func as usize, + collected.classes(), + collected.args(), + ) + } } /// llmodel.py:834 bh_call_v / descr.py:590-605 create_call_stub @@ -3439,14 +3456,18 @@ impl Backend for DynasmBackend { if func == 0 { return; } - let (classes, args) = majit_backend::call_stub::collect_call_args( + let collected = majit_backend::call_stub::collect_call_args( &calldescr.arg_classes, args_i, args_r, args_f, ); unsafe { - majit_backend::call_stub::bh_call_v_dispatch(func as usize, &classes, &args); + majit_backend::call_stub::bh_call_v_dispatch( + func as usize, + collected.classes(), + collected.args(), + ); } } diff --git a/majit/majit-backend/src/call_stub.rs b/majit/majit-backend/src/call_stub.rs index 75cf664cd14..5f2227016d5 100644 --- a/majit/majit-backend/src/call_stub.rs +++ b/majit/majit-backend/src/call_stub.rs @@ -13,6 +13,8 @@ //! Microsoft x64 positional-slot convention and is also correct under SysV and //! AAPCS. +use majit_translate::codewriter::insns::MAX_HOST_CALL_ARITY; + /// `descr.py:556-570 TYPE()` collapsed to the two C-ABI register classes the /// dispatch table can express: `'i'`, `'r'` and `'L'` (`lltype.Signed`, /// `llmemory.GCREF`, `lltype.SignedLongLong`) all pass in an integer register; @@ -1113,6 +1115,47 @@ pub unsafe fn bh_call_f_dispatch(func: usize, classes: &[ArgClass], args: &[i64] unsafe { dispatch_classes_body!(func, classes, args, f64) } } +/// The ordered class sequence and matching positional argument list that +/// [`collect_call_args`] hands to `bh_call_*_dispatch`. +/// +/// Fixed-size so a residual call allocates nothing. `MAX_HOST_CALL_ARITY` is +/// the same bound the dispatch table's all-`Int` arms stop at, so a signature +/// that does not fit here has no arm either. +pub struct CallArgs { + classes: [ArgClass; MAX_HOST_CALL_ARITY], + args: [i64; MAX_HOST_CALL_ARITY], + len: usize, +} + +impl CallArgs { + fn with_arity(arity: usize) -> Self { + assert!( + arity <= MAX_HOST_CALL_ARITY, + "bh_call dispatch: {arity} arguments exceeds MAX_HOST_CALL_ARITY \ + ({MAX_HOST_CALL_ARITY}); the dispatch table has no arm this wide" + ); + Self { + classes: [ArgClass::Int; MAX_HOST_CALL_ARITY], + args: [0; MAX_HOST_CALL_ARITY], + len: 0, + } + } + + fn push(&mut self, class: ArgClass, arg: i64) { + self.classes[self.len] = class; + self.args[self.len] = arg; + self.len += 1; + } + + pub fn classes(&self) -> &[ArgClass] { + &self.classes[..self.len] + } + + pub fn args(&self) -> &[i64] { + &self.args[..self.len] + } +} + /// Build the C-ABI class sequence and positional argument list from /// `args_i` / `args_r` / `args_f`, following `calldescr.arg_classes` order. /// @@ -1147,12 +1190,16 @@ pub unsafe fn bh_call_f_dispatch(func: usize, classes: &[ArgClass], args: &[i64] /// Mirrors `rpython/jit/backend/llsupport/descr.py:616-620 verify_types`: /// the per-class counts in `arg_classes` must match the corresponding list /// length, and any unknown class is a codegen bug. +/// +/// Returns a stack buffer rather than two `Vec`s: this runs on every residual +/// call the blackhole makes, and upstream's generated stub reaches the callee +/// with no intermediate collection at all. pub fn collect_call_args( arg_classes: &str, args_i: Option<&[i64]>, args_r: Option<&[i64]>, args_f: Option<&[i64]>, -) -> (Vec, Vec) { +) -> CallArgs { // descr.py:616-620 verify_types parity: assert per-class counts. let count_i: usize = arg_classes .chars() @@ -1179,26 +1226,31 @@ pub fn collect_call_args( "BhCallDescr.verify_types: arg_classes={arg_classes:?} has {count_f} float slots, args_f has {len_f}" ); - let mut classes: Vec = Vec::with_capacity(arg_classes.len()); - let mut args: Vec = Vec::with_capacity(arg_classes.len()); + let mut out = CallArgs::with_arity(count_i + count_r + count_f); let mut ii = 0usize; let mut ri = 0usize; let mut fi = 0usize; for c in arg_classes.chars() { match c { 'i' => { - classes.push(ArgClass::Int); - args.push(args_i.expect("BhCallDescr.collect_call_args: args_i missing")[ii]); + out.push( + ArgClass::Int, + args_i.expect("BhCallDescr.collect_call_args: args_i missing")[ii], + ); ii += 1; } 'r' => { - classes.push(ArgClass::Int); - args.push(args_r.expect("BhCallDescr.collect_call_args: args_r missing")[ri]); + out.push( + ArgClass::Int, + args_r.expect("BhCallDescr.collect_call_args: args_r missing")[ri], + ); ri += 1; } 'f' => { - classes.push(ArgClass::Float); - args.push(args_f.expect("BhCallDescr.collect_call_args: args_f missing")[fi]); + out.push( + ArgClass::Float, + args_f.expect("BhCallDescr.collect_call_args: args_f missing")[fi], + ); fi += 1; } 'L' => { @@ -1206,8 +1258,10 @@ pub fn collect_call_args( // (PyPy rewrites `c = 'f'` for the lookup); FUNC parameter // type = `lltype.SignedLongLong` -> C `long long` -> // 8-byte int dispatched in an integer register. - classes.push(ArgClass::Int); - args.push(args_f.expect("BhCallDescr.collect_call_args: args_f missing")[fi]); + out.push( + ArgClass::Int, + args_f.expect("BhCallDescr.collect_call_args: args_f missing")[fi], + ); fi += 1; } 'S' => { @@ -1231,7 +1285,7 @@ pub fn collect_call_args( ), } } - (classes, args) + out } /// Bucket `args_i` / `args_r` / `args_f` into a single positional list in @@ -1307,8 +1361,8 @@ pub unsafe fn bh_call_i_by_classes( let args = collect_call_args_positional(arg_classes, args_i, args_r, args_f); return hook(func, &args); } - let (classes, args) = collect_call_args(arg_classes, args_i, args_r, args_f); - unsafe { bh_call_i_dispatch(func, &classes, &args) } + let collected = collect_call_args(arg_classes, args_i, args_r, args_f); + unsafe { bh_call_i_dispatch(func, collected.classes(), collected.args()) } } /// f64-returning parallel of [`bh_call_i_by_classes`]. @@ -1327,8 +1381,8 @@ pub unsafe fn bh_call_f_by_classes( // The trampoline returns an f64 callee result as its raw bits. return f64::from_bits(hook(func, &args) as u64); } - let (classes, args) = collect_call_args(arg_classes, args_i, args_r, args_f); - unsafe { bh_call_f_dispatch(func, &classes, &args) } + let collected = collect_call_args(arg_classes, args_i, args_r, args_f); + unsafe { bh_call_f_dispatch(func, collected.classes(), collected.args()) } } /// Result-discarding parallel of [`bh_call_i_by_classes`]. @@ -1347,8 +1401,8 @@ pub unsafe fn bh_call_v_by_classes( let _ = hook(func, &args); return; } - let (classes, args) = collect_call_args(arg_classes, args_i, args_r, args_f); - unsafe { bh_call_v_dispatch(func, &classes, &args) } + let collected = collect_call_args(arg_classes, args_i, args_r, args_f); + unsafe { bh_call_v_dispatch(func, collected.classes(), collected.args()) } } /// A host-provided trampoline that performs a residual call by reflecting the @@ -1463,6 +1517,17 @@ mod tests { assert_eq!(result, 54321); } + /// `collect_call_args` fills a fixed buffer, so a signature wider than the + /// dispatch table's widest arm is refused while collecting rather than + /// overrunning it. + #[test] + #[should_panic(expected = "exceeds MAX_HOST_CALL_ARITY")] + fn collect_call_args_refuses_more_arguments_than_the_table_covers() { + let too_many = MAX_HOST_CALL_ARITY + 1; + let args_i = vec![0_i64; too_many]; + let _ = collect_call_args(&"i".repeat(too_many), Some(&args_i), None, None); + } + /// One argument past that bound the table has no arm, so the call is /// refused instead of being placed against the wrong signature. #[test] diff --git a/majit/majit-metainterp/src/pyjitpl/dispatch.rs b/majit/majit-metainterp/src/pyjitpl/dispatch.rs index 14ab12db39d..c6a1bc3f2c1 100644 --- a/majit/majit-metainterp/src/pyjitpl/dispatch.rs +++ b/majit/majit-metainterp/src/pyjitpl/dispatch.rs @@ -8480,9 +8480,14 @@ pub fn call_float_function(func_ptr: *const (), args: &[i64], arg_types: &[Type] /// (`descr.py:556-570 TYPE()`). /// /// Returns the fixed-size buffer so the residual-call path stays -/// allocation-free; callers slice it to `args.len()`. A positional slot with -/// no declared type is forwarded as a machine word, matching the untyped -/// [`call_int_function`] / [`call_void_function`] seams. +/// allocation-free; callers slice it to `args.len()`. +/// +/// `arg_types` must cover every positional slot: a slot the descr does not +/// describe is forwarded as a machine word, which is the very mis-placement +/// this projection exists to prevent if the undescribed slot is a `Float`. +/// Both callers (`executor::execute_pure_call` / `execute_residual_call`) take +/// the argument list and the type list from the same calldescr, so the two +/// agree by construction. fn arg_classes_from_types( args_len: usize, arg_types: &[Type], @@ -8491,6 +8496,12 @@ fn arg_classes_from_types( args_len <= MAX_HOST_CALL_ARITY, "unsupported JitCode typed call arity {args_len} (max {MAX_HOST_CALL_ARITY})" ); + debug_assert!( + arg_types.len() >= args_len, + "typed call: calldescr describes {} argument types for {args_len} positional \ + arguments; an undescribed Float slot would travel in an integer register", + arg_types.len() + ); let mut classes = [majit_backend::call_stub::ArgClass::Int; MAX_HOST_CALL_ARITY]; for (i, slot) in classes.iter_mut().enumerate().take(args_len) { *slot = match arg_types.get(i) {