From 39f7eddec4c55711d56f05b085992a83bf23159e Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Thu, 20 Oct 2022 16:45:24 -0400 Subject: [PATCH] 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. --- yjit/src/codegen.rs | 23 ++++++++++++----------- 1 file 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