Skip to content

Commit 187cb0d

Browse files
committed
ZJIT: Exit with the block arg on the stack from SendDirect args
When a direct send drops a profiled-nil `&block` argument, it lays out the callee frame from a snapshot with the block arg stripped from the VM stack. emit_send_direct_args() also used that snapshot for instructions that materialize the callee's arguments before the call, such as the rest array of a `*args` callee. Those can side-exit, e.g. at the NoNewObjHook patch point of the inline allocation once a NEWOBJ hook is enabled, and the exit re-executes the send in the interpreter at the same PC. With the block arg missing from the stack, the interpreter took the last positional argument as the block: TypeError: no implicit conversion of Net::HTTP::Post into Proc This happened in Shopify's Sonic test suite after a test enabled ObjectSpace.trace_object_allocations: Net::HTTP#request passes `&block` to Semian's `transport_request(*)`, and `request` had reached the version limit, so its invalidated code kept exiting there. Use the send's original frame state for those instructions.
1 parent 6b563b8 commit 187cb0d

2 files changed

Lines changed: 41 additions & 5 deletions

File tree

‎zjit/src/hir.rs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3984,11 +3984,16 @@ impl Function {
39843984
}
39853985

39863986
/// Materialize a validated SendDirect call in the selected runtime path.
3987-
fn emit_send_direct_args(&mut self, block: BlockId, call: SendDirectCall, original_args: &[InsnId], state: InsnId) -> SendDirectArgs {
3987+
///
3988+
/// `state` is the frame state that the callee frame is laid out from, which may have a nil
3989+
/// block arg stripped from the stack. `exit_state` is the frame state of the send itself, used
3990+
/// by instructions that materialize arguments before the call (e.g. the rest array), since a
3991+
/// side exit from them re-executes the send in the interpreter with the original VM stack.
3992+
fn emit_send_direct_args(&mut self, block: BlockId, call: SendDirectCall, original_args: &[InsnId], state: InsnId, exit_state: InsnId) -> SendDirectArgs {
39883993
let args: Vec<_> = call
39893994
.args
39903995
.into_iter()
3991-
.map(|arg| self.emit_send_direct_arg(block, arg, state))
3996+
.map(|arg| self.emit_send_direct_arg(block, arg, exit_state))
39923997
.collect();
39933998

39943999
// If args were reordered or synthesized, create a new snapshot with the updated stack.
@@ -4882,7 +4887,7 @@ impl Function {
48824887
}
48834888

48844889
let SendDirectArgs { state: send_state, args: send_args, kw_bits, jit_entry_idx } =
4885-
self.emit_send_direct_args(block, call, &args, send_frame_state);
4890+
self.emit_send_direct_args(block, call, &args, send_frame_state, state);
48864891
let replacement = self.try_inline_send_direct(block, Insn::SendDirect(Box::new(SendDirectData { recv, cd, cme, iseq, args: send_args, kw_bits, jit_entry_idx, state: send_state, block: send_block })));
48874892
self.make_equal_to(insn_id, replacement);
48884893
} else if !has_block && def_type == VM_METHOD_TYPE_BMETHOD {
@@ -4923,7 +4928,7 @@ impl Function {
49234928
}
49244929

49254930
let SendDirectArgs { state: send_state, args: send_args, kw_bits, jit_entry_idx } =
4926-
self.emit_send_direct_args(block, call, &args, send_frame_state);
4931+
self.emit_send_direct_args(block, call, &args, send_frame_state, state);
49274932
let replacement = self.try_inline_send_direct(block, Insn::SendDirect(Box::new(SendDirectData { recv, cd, cme, iseq, args: send_args, kw_bits, jit_entry_idx, state: send_state, block: None })));
49284933
self.make_equal_to(insn_id, replacement);
49294934
} else if !has_block && def_type == VM_METHOD_TYPE_IVAR && args.is_empty() {
@@ -5456,7 +5461,7 @@ impl Function {
54565461
emit_super_call_guards(self, block, super_cme, current_cme, mid, state, frame_state_iseq);
54575462

54585463
let SendDirectArgs { state: send_state, args: send_args, kw_bits, jit_entry_idx } =
5459-
self.emit_send_direct_args(block, call, &args, state);
5464+
self.emit_send_direct_args(block, call, &args, state, state);
54605465
// Use SendDirect with the super method's CME and ISEQ.
54615466
let replacement = self.try_inline_send_direct(block, Insn::SendDirect(Box::new(SendDirectData {
54625467
recv,

‎zjit/src/hir/opt_tests.rs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,37 @@ mod hir_opt_tests {
2626
hir_string_proc(&format!("{}.method(:{})", "self", method))
2727
}
2828

29+
// A direct send strips a profiled-nil `&block` argument from the stack that the callee
30+
// frame is laid out from, but instructions that materialize the callee's arguments, like
31+
// the rest array, can side-exit and re-execute the send in the interpreter. Their frame
32+
// state must keep the block argument on the stack, or the interpreter takes the last
33+
// positional argument as the block.
34+
#[test]
35+
fn test_send_direct_rest_array_keeps_stripped_block_arg_in_exit_state() {
36+
eval("
37+
def rest_callee(*args) = args
38+
def test(obj, &block) = rest_callee(obj, &block)
39+
test(1)
40+
test(1)
41+
");
42+
let iseq = crate::cruby::with_rubyvm(|| get_method_iseq("self", "test"));
43+
unsafe { crate::cruby::rb_zjit_profile_disable(iseq) };
44+
let mut function = iseq_to_hir(iseq).unwrap();
45+
function.optimize();
46+
function.validate().unwrap();
47+
48+
let new_array_stack_sizes: Vec<usize> = (0..function.num_insns())
49+
.map(InsnId::from)
50+
.filter_map(|insn_id| match function.find(insn_id) {
51+
Insn::NewArray { state, .. } => Some(function.frame_state(state).stack().len()),
52+
_ => None,
53+
})
54+
.collect();
55+
// [self, obj, block]
56+
assert!(!new_array_stack_sizes.is_empty(), "{}", hir_string_function(&function));
57+
assert!(new_array_stack_sizes.iter().all(|&size| size == 3), "{new_array_stack_sizes:?}");
58+
}
59+
2960
#[test]
3061
fn test_fold_iftrue_away() {
3162
eval("

0 commit comments

Comments
 (0)