diff options
author | Alan Wu <XrXr@users.noreply.github.com> | 2022-10-20 16:45:24 -0400 |
---|---|---|
committer | Alan Wu <XrXr@users.noreply.github.com> | 2022-10-20 18:12:15 -0400 |
commit | 39f7eddec4c55711d56f05b085992a83bf23159e (patch) | |
tree | e671ea20ff3d42a557f8c992e257ac9370ae6e50 /yjit/src | |
parent | 2930302244037b0b08da65176eb725541b3ef800 (diff) | |
download | ruby-39f7eddec4c55711d56f05b085992a83bf23159e.tar.gz |
YJIT: Fix gen_expandarray treating argument as VALUE
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
f55212bce939f736559709a8cd16c409772389c8. 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.
Diffstat (limited to 'yjit/src')
-rw-r--r-- | yjit/src/codegen.rs | 23 |
1 files changed, 12 insertions, 11 deletions
diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 4190c9cc40..2d6d93dcd5 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -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()); } @@ -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; } @@ -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 |