Skip to content

Commit

Permalink
YJIT: Fix off-by-one in Kernel#send type handling
Browse files Browse the repository at this point in the history
Previously, if the method ID argument happens to be on one below the top
of the stack, we didn't overwrite the type of the stack slot, which
leaves an incorrect type for the stack slot. The included script tripped
asserts both with and without --yjit-verify-ctx.
  • Loading branch information
XrXr committed Dec 12, 2023
1 parent 278ce27 commit df2e108
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 2 deletions.
13 changes: 13 additions & 0 deletions bootstraptest/test_yjit.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
# regression test for send stack shifting
assert_normal_exit %q{
def foo(a, b)
a.singleton_methods(b)
end
def call_foo
[1, 1, 1, 1, 1, 1, send(:foo, 1, 1)]
end
call_foo
}

# regression test for arity check with splat
assert_equal '[:ae, :ae]', %q{
def req_one(a_, b_ = 1) = raise
Expand Down
29 changes: 27 additions & 2 deletions yjit/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2180,8 +2180,13 @@ impl Assembler {
let method_name_index = (self.ctx.stack_size as usize) - argc - 1;

for i in method_name_index..(self.ctx.stack_size - 1) as usize {
if i + 1 < MAX_TEMP_TYPES {
self.ctx.set_temp_mapping(i, self.ctx.get_temp_mapping(i + 1));
if i < MAX_TEMP_TYPES {
let next_arg_mapping = if i + 1 < MAX_TEMP_TYPES {
self.ctx.get_temp_mapping(i + 1)
} else {
TempMapping::map_to_stack(Type::Unknown)
};
self.ctx.set_temp_mapping(i, next_arg_mapping);
}
}
self.stack_pop(1);
Expand Down Expand Up @@ -3500,6 +3505,26 @@ mod tests {
// TODO: write more tests for Context type diff
}

#[test]
fn shift_stack_for_send() {
let mut asm = Assembler::new();

// Push values to simulate send(:name, arg) with 6 items already on-stack
for _ in 0..6 {
asm.stack_push(Type::Fixnum);
}
asm.stack_push(Type::Unknown);
asm.stack_push(Type::ImmSymbol);
asm.stack_push(Type::Unknown);

// This method takes argc of the sendee, not argc of send
asm.shift_stack(1);

// The symbol should be gone
assert_eq!(Type::Unknown, asm.ctx.get_opnd_type(StackOpnd(0)));
assert_eq!(Type::Unknown, asm.ctx.get_opnd_type(StackOpnd(1)));
}

#[test]
fn test_miri_ref_unchecked() {
let blockid = BlockId {
Expand Down

0 comments on commit df2e108

Please sign in to comment.