Skip to content

Commit

Permalink
Prevent modification of splat array inside setup_parameters_complex
Browse files Browse the repository at this point in the history
For the following:

```
def f(*a); a end
p f(*a, kw: 3)
```

`setup_parameters_complex` pushes `{kw: 3}` onto `a`.  This worked fine
back when `concatarray true` was used and `a` was already a copy. It
does not work correctly with the optimization to switch to
`concatarray false`.  This duplicates the array on the callee side
in such a case.

This affects cases when passing a regular splat and a keyword splat
(or literal keywords) in a method call, where the method does not
accept keywords.

This allocation could probably be avoided, but doing so would
make `setup_parameters_complex` more complicated.
  • Loading branch information
jeremyevans committed Dec 7, 2023
1 parent aa80820 commit 13cd963
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 0 deletions.
6 changes: 6 additions & 0 deletions test/ruby/test_call.rb
Expand Up @@ -100,6 +100,12 @@ def test_invalid_safe_call
}
end

def test_frozen_splat_and_keywords
a = [1, 2].freeze
def self.f(*a); a end
assert_equal([1, 2, {kw: 3}], f(*a, kw: 3))
end

def test_call_bmethod_proc
pr = proc{|sym| sym}
define_singleton_method(:a, &pr)
Expand Down
2 changes: 2 additions & 0 deletions vm_args.c
Expand Up @@ -542,11 +542,13 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
else if (UNLIKELY(ISEQ_BODY(iseq)->param.flags.ruby2_keywords)) {
converted_keyword_hash = check_kwrestarg(converted_keyword_hash, &kw_flag);
flag_keyword_hash = converted_keyword_hash;
arg_rest_dup(args);
rb_ary_push(args->rest, converted_keyword_hash);
keyword_hash = Qnil;
}
else if (!ISEQ_BODY(iseq)->param.flags.has_kwrest && !ISEQ_BODY(iseq)->param.flags.has_kw) {
converted_keyword_hash = check_kwrestarg(converted_keyword_hash, &kw_flag);
arg_rest_dup(args);
rb_ary_push(args->rest, converted_keyword_hash);
keyword_hash = Qnil;
} else {
Expand Down

0 comments on commit 13cd963

Please sign in to comment.