From 558b58d330783f663d1cfb37c4af7dc330d490b3 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Wed, 28 Feb 2024 14:27:16 -0500 Subject: [PATCH] YJIT: Reject keywords hash in -1 arity cfunc splat support `test_keyword.rb` caught this issue. Just need to run with `threshold=1` --- bootstraptest/test_yjit.rb | 16 ++++++++++++++++ yjit.c | 22 ++++++++++------------ yjit/src/codegen.rs | 3 +-- yjit/src/cruby_bindings.inc.rs | 5 +---- 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb index 1d98b92d3a..90f1c0b5f1 100644 --- a/bootstraptest/test_yjit.rb +++ b/bootstraptest/test_yjit.rb @@ -4658,3 +4658,19 @@ assert_equal '[[1, 2, 3], [0, 2, 3], [1, 2, 3], [2, 2, 3], [], [], [{}]]', %q{ 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})]) +} diff --git a/yjit.c b/yjit.c index 8632361699..13707900c0 100644 --- a/yjit.c +++ b/yjit.c @@ -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; @@ -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); diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 2e946441f1..9419f52894 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -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 }; diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index b131b62bfd..e1cfeec22b 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -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;