Skip to content

Commit

Permalink
YJIT: Fix missing arity check for splat calls to methods with optionals
Browse files Browse the repository at this point in the history
Previously, for splat callsites that land in methods with optional
parameters, we didn't reject the case where the caller supplies too many
arguments. Accepting those calls previously caused YJIT to construct
corrupted control frames, which leads to crashes if the callee uses
certain stack walking methods such as Kernel#raise and String#gsub (for
setting up the frame-local `$~`).

Example crash in a debug build:

    Assertion Failed: ../vm_core.h:1375:VM_ENV_FLAGS:FIXNUM_P(flags)
  • Loading branch information
XrXr committed Dec 12, 2023
1 parent 4755309 commit 9cb0ad8
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 3 deletions.
26 changes: 26 additions & 0 deletions bootstraptest/test_yjit.rb
@@ -1,3 +1,29 @@
# regression test for arity check with splat
assert_equal '[:ae, :ae]', %q{
def req_one(a_, b_ = 1) = raise
def test(args)
req_one *args
rescue ArgumentError
:ae
end
[test(Array.new 5), test([])]
}

# regression test for arity check with splat and send
assert_equal '[:ae, :ae]', %q{
def two_reqs(a, b_, _ = 1) = a.gsub(a, a)
def test(name, args)
send(name, *args)
rescue ArgumentError
:ae
end
[test(:two_reqs, ["g", nil, nil, nil]), test(:two_reqs, ["g"])]
}

# regression test for GC marking stubs in invalidated code
assert_normal_exit %q{
garbage = Array.new(10_000) { [] } # create garbage to cause iseq movement
Expand Down
11 changes: 8 additions & 3 deletions yjit/src/codegen.rs
Expand Up @@ -6116,9 +6116,14 @@ fn gen_send_iseq(
unsafe { rb_yjit_array_len(array) as u32}
};

if opt_num == 0 && required_num != array_length as i32 + argc - 1 && !iseq_has_rest {
gen_counter_incr(asm, Counter::send_iseq_splat_arity_error);
return None;
// Arity check accounting for size of the splat. When callee has rest parameters, we insert
// runtime guards later in copy_splat_args_for_rest_callee()
if !iseq_has_rest {
let supplying = argc - 1 + array_length as i32;
if (required_num..=required_num + opt_num).contains(&supplying) == false {
gen_counter_incr(asm, Counter::send_iseq_splat_arity_error);
return None;
}
}

if iseq_has_rest && opt_num > 0 {
Expand Down

0 comments on commit 9cb0ad8

Please sign in to comment.