Skip to content

Commit

Permalink
YJIT: Fix gen_expandarray treating argument as VALUE
Browse files Browse the repository at this point in the history
The expandarray instruction interpreters its arguments as rb_num_t.
YJIT was treating the num argument as a VALUE previously and when
it has a certain bit pattern, it can look like a GC pointer. The
argument is not a pointer, so YJIT crashed when trying to mark those
pointers.

This bug existed previously, but our test suite didn't expose it until
f55212b. TestArgf#test_to_io has a
line like:

    a1, a2, a3, a4, a5, a6, a7, a8 = array

Which maps to an expandarray with an argument of 8. Qnil happened to
be defined as 8, which masked the issue.

Fix it by not using the argument as a VALUE.
  • Loading branch information
XrXr committed Oct 20, 2022
1 parent 2930302 commit 39f7edd
Showing 1 changed file with 12 additions and 11 deletions.
23 changes: 12 additions & 11 deletions yjit/src/codegen.rs
Expand Up @@ -1373,33 +1373,33 @@ fn gen_expandarray(
asm: &mut Assembler,
ocb: &mut OutlinedCb,
) -> CodegenStatus {
let flag = jit_get_arg(jit, 1);
let VALUE(flag_value) = flag;
// Both arguments are rb_num_t which is unsigned
let num = jit_get_arg(jit, 0).as_usize();
let flag = jit_get_arg(jit, 1).as_usize();

// If this instruction has the splat flag, then bail out.
if flag_value & 0x01 != 0 {
if flag & 0x01 != 0 {
gen_counter_incr!(asm, expandarray_splat);
return CantCompile;
}

// If this instruction has the postarg flag, then bail out.
if flag_value & 0x02 != 0 {
if flag & 0x02 != 0 {
gen_counter_incr!(asm, expandarray_postarg);
return CantCompile;
}

let side_exit = get_side_exit(jit, ocb, ctx);

// num is the number of requested values. If there aren't enough in the
// array then we're going to push on nils.
let num = jit_get_arg(jit, 0);
let array_type = ctx.get_opnd_type(StackOpnd(0));
let array_opnd = ctx.stack_pop(1);

// num is the number of requested values. If there aren't enough in the
// array then we're going to push on nils.
if matches!(array_type, Type::Nil) {
// special case for a, b = nil pattern
// push N nils onto the stack
for _i in 0..(num.into()) {
for _ in 0..num {
let push_opnd = ctx.stack_push(Type::Nil);
asm.mov(push_opnd, Qnil.into());
}
Expand All @@ -1420,7 +1420,7 @@ fn gen_expandarray(
);

// If we don't actually want any values, then just return.
if num == VALUE(0) {
if num == 0 {
return KeepCompiling;
}

Expand Down Expand Up @@ -1463,9 +1463,10 @@ fn gen_expandarray(
let ary_opnd = asm.csel_nz(ary_opnd, heap_ptr_opnd);

// Loop backward through the array and push each element onto the stack.
for i in (0..(num.as_i32())).rev() {
for i in (0..num).rev() {
let top = ctx.stack_push(Type::Unknown);
asm.mov(top, Opnd::mem(64, ary_opnd, i * (SIZEOF_VALUE as i32)));
let offset = i32::try_from(i * SIZEOF_VALUE).unwrap();
asm.mov(top, Opnd::mem(64, ary_opnd, offset));
}

KeepCompiling
Expand Down

0 comments on commit 39f7edd

Please sign in to comment.