Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YJIT: 2 __send__ miscompilations #7377

Merged
merged 2 commits into from
Feb 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions bootstraptest/test_yjit.rb
Expand Up @@ -3579,3 +3579,8 @@ def a.test = :test
def a.test = :test
a.is_a?(a.singleton_class)
}

# Test send with splat to a cfunc
assert_equal 'true', %q{
1.send(:==, 1, *[])
}
20 changes: 20 additions & 0 deletions test/ruby/test_yjit.rb
Expand Up @@ -1196,6 +1196,26 @@ def freeze = :ugokanai
RUBY
end

def test_nested_send
#[Bug #19464]
assert_compiles(<<~RUBY, result: [:ok, :ok])
klass = Class.new do
class << self
alias_method :my_send, :send

def bar = :ok

def foo = bar
end
end

with_break = -> { break klass.send(:my_send, :foo) }
wo_break = -> { klass.send(:my_send, :foo) }

[with_break[], wo_break[]]
RUBY
end

private

def code_gc_helpers
Expand Down
31 changes: 16 additions & 15 deletions yjit/src/codegen.rs
Expand Up @@ -4929,6 +4929,12 @@ fn gen_send_cfunc(

// push_splat_args does stack manipulation so we can no longer side exit
if flags & VM_CALL_ARGS_SPLAT != 0 {
if flags & VM_CALL_OPT_SEND != 0 {
// FIXME: This combination is buggy.
// For example `1.send(:==, 1, *[])` fails to adjust the stack properly
gen_counter_incr!(asm, send_cfunc_splat_send);
return CantCompile;
}
let required_args : u32 = (cfunc_argc as u32).saturating_sub(argc as u32 - 1);
// + 1 because we pass self
if required_args + 1 >= C_ARG_OPNDS.len() as u32 {
Expand Down Expand Up @@ -6318,14 +6324,23 @@ fn gen_send_general(
let opt_type = unsafe { get_cme_def_body_optimized_type(cme) };
match opt_type {
OPTIMIZED_METHOD_TYPE_SEND => {

// This is for method calls like `foo.send(:bar)`
// The `send` method does not get its own stack frame.
// instead we look up the method and call it,
// doing some stack shifting based on the VM_CALL_OPT_SEND flag

let starting_context = ctx.clone();

// Reject nested cases such as `send(:send, :alias_for_send, :foo))`.
// We would need to do some stack manipulation here or keep track of how
// many levels deep we need to stack manipulate. Because of how exits
// currently work, we can't do stack manipulation until we will no longer
// side exit.
if flags & VM_CALL_OPT_SEND != 0 {
gen_counter_incr!(asm, send_send_nested);
return CantCompile;
}

if argc == 0 {
gen_counter_incr!(asm, send_send_wrong_args);
return CantCompile;
Expand All @@ -6352,20 +6367,6 @@ fn gen_send_general(
return CantCompile;
}

// We aren't going to handle `send(send(:foo))`. We would need to
// do some stack manipulation here or keep track of how many levels
// deep we need to stack manipulate
// Because of how exits currently work, we can't do stack manipulation
// until we will no longer side exit.
let def_type = unsafe { get_cme_def_type(cme) };
if let VM_METHOD_TYPE_OPTIMIZED = def_type {
let opt_type = unsafe { get_cme_def_body_optimized_type(cme) };
if let OPTIMIZED_METHOD_TYPE_SEND = opt_type {
gen_counter_incr!(asm, send_send_nested);
return CantCompile;
}
}

flags |= VM_CALL_FCALL | VM_CALL_OPT_SEND;

assume_method_lookup_stable(jit, ocb, cme);
Expand Down
1 change: 1 addition & 0 deletions yjit/src/stats.rs
Expand Up @@ -192,6 +192,7 @@ make_counters! {
send_cfunc_tracing,
send_cfunc_kwargs,
send_cfunc_splat_with_kw,
send_cfunc_splat_send,
send_attrset_kwargs,
send_iseq_tailcall,
send_iseq_arity_error,
Expand Down