Skip to content

Commit

Permalink
YJIT: Reject keywords hash in -1 arity cfunc splat support
Browse files Browse the repository at this point in the history
`test_keyword.rb` caught this issue. Just need to run with `threshold=1`
  • Loading branch information
XrXr committed Feb 28, 2024
1 parent c5b568b commit 2b19561
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 18 deletions.
16 changes: 16 additions & 0 deletions bootstraptest/test_yjit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4658,3 +4658,19 @@ def test_cfunc_vargs_splat(sub_instance, array_class, empty_kw_hash)
test_cfunc_vargs_splat(Foo.new, Array, Hash.ruby2_keywords_hash({}))
}

# Class#new (arity=-1), splat, and ruby2_keywords
assert_equal '[0, {1=>1}]', %q{
class KwInit
attr_reader :init_args
def initialize(x = 0, **kw)
@init_args = [x, kw]
end
end
def test(klass, args)
klass.new(*args).init_args
end
test(KwInit, [Hash.ruby2_keywords_hash({1 => 1})])
}
22 changes: 10 additions & 12 deletions yjit.c
Original file line number Diff line number Diff line change
Expand Up @@ -903,13 +903,22 @@ rb_yjit_splat_varg_checks(VALUE *sp, VALUE splat_array, rb_control_frame_t *cfp)
// Would we overflow if we put the contents of the array onto the stack?
if (sp + len > (VALUE *)(cfp - 2)) return Qfalse;

// Reject keywords hash since that requires duping it sometimes
if (len > 0) {
VALUE last_hash = RARRAY_AREF(splat_array, len - 1);
if (RB_TYPE_P(last_hash, T_HASH) &&
FL_TEST_RAW(last_hash, RHASH_PASS_AS_KEYWORDS)) {
return Qfalse;
}
}

return Qtrue;
}

// Push array elements to the stack for a C method that has a variable number
// of parameters. Returns the number of arguments the splat array contributes.
int
rb_yjit_splat_varg_cfunc(VALUE *stack_splat_array, bool sole_splat)
rb_yjit_splat_varg_cfunc(VALUE *stack_splat_array)
{
VALUE splat_array = *stack_splat_array;
int len;
Expand All @@ -918,17 +927,6 @@ rb_yjit_splat_varg_cfunc(VALUE *stack_splat_array, bool sole_splat)
RUBY_ASSERT(RB_TYPE_P(splat_array, T_ARRAY));
len = (int)RARRAY_LEN(splat_array);

// If this is a splat call without any keyword arguments, exclude the
// ruby2_keywords hash if it's empty
if (sole_splat && len > 0) {
VALUE last_hash = RARRAY_AREF(splat_array, len - 1);
if (RB_TYPE_P(last_hash, T_HASH) &&
FL_TEST_RAW(last_hash, RHASH_PASS_AS_KEYWORDS) &&
RHASH_EMPTY_P(last_hash)) {
len--;
}
}

// Push the contents of the array onto the stack
MEMCPY(stack_splat_array, RARRAY_CONST_PTR(splat_array), VALUE, len);

Expand Down
3 changes: 1 addition & 2 deletions yjit/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6324,9 +6324,8 @@ fn gen_send_cfunc(
// Push a dynamic number of items from the splat array to the stack when calling a vargs method
let dynamic_splat_size = if variable_splat {
asm_comment!(asm, "variable length splat");
let just_splat = usize::from(!kw_splat && kw_arg.is_null()).into();
let stack_splat_array = asm.lea(asm.stack_opnd(0));
Some(asm.ccall(rb_yjit_splat_varg_cfunc as _, vec![stack_splat_array, just_splat]))
Some(asm.ccall(rb_yjit_splat_varg_cfunc as _, vec![stack_splat_array]))
} else {
None
};
Expand Down
5 changes: 1 addition & 4 deletions yjit/src/cruby_bindings.inc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1199,10 +1199,7 @@ extern "C" {
splat_array: VALUE,
cfp: *mut rb_control_frame_t,
) -> VALUE;
pub fn rb_yjit_splat_varg_cfunc(
stack_splat_array: *mut VALUE,
sole_splat: bool,
) -> ::std::os::raw::c_int;
pub fn rb_yjit_splat_varg_cfunc(stack_splat_array: *mut VALUE) -> ::std::os::raw::c_int;
pub fn rb_yjit_dump_iseq_loc(iseq: *const rb_iseq_t, insn_idx: u32);
pub fn rb_yjit_iseq_inspect(iseq: *const rb_iseq_t) -> *mut ::std::os::raw::c_char;
pub fn rb_FL_TEST(obj: VALUE, flags: VALUE) -> VALUE;
Expand Down

0 comments on commit 2b19561

Please sign in to comment.