From 58ccce60cf5f3268e7ef27942b75e78fe2d78e75 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Tue, 28 Jan 2025 23:54:38 -0500 Subject: [PATCH] YJIT: Initialize locals in ISeqs defined with `...` (#12660) * YJIT: Fix indentation [ci skip] Fixes: cdf33ed5f37f9649c482c3ba1d245f0d80ac01ce * YJIT: Initialize locals in ISeqs defined with `...` Previously, callers of forwardable ISeqs moved the stack pointer up without writing to the stack. If there happens to be a stale value in the area skipped over, it could crash due to "try to mark T_NONE". Also, the uninitialized local variables were observable through `binding`. Initialize the locals to nil. [Bug #21021] --- bootstraptest/test_yjit.rb | 32 ++++++++++++++++++++++++++++++++ yjit/src/codegen.rs | 17 ++++++++++------- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb index b5bd883599..043f5c64c2 100644 --- a/bootstraptest/test_yjit.rb +++ b/bootstraptest/test_yjit.rb @@ -5355,3 +5355,35 @@ assert_normal_exit %{ new.foo end } + +# This used to trigger a "try to mark T_NONE" +# due to an uninitialized local in foo. +assert_normal_exit %{ + def foo(...) + _local_that_should_nil_on_call = GC.start + end + + def test_bug21021 + puts [], [], [], [], [], [] + foo [] + end + + GC.stress = true + test_bug21021 +} + +assert_equal 'nil', %{ + def foo(...) + _a = _b = _c = binding.local_variable_get(:_c) + + _c + end + + # [Bug #21021] + def test_local_fill_in_forwardable + puts [], [], [], [], [] + foo [] + end + + test_local_fill_in_forwardable.inspect +} diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 41f9fb9209..784e2a8135 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -8069,7 +8069,6 @@ fn gen_send_iseq( } } - // Don't nil fill forwarding iseqs if !forwarding { // Nil-initialize missing optional parameters nil_fill( @@ -8090,13 +8089,13 @@ fn gen_send_iseq( // Nil-initialize non-parameter locals nil_fill( "nil-initialize locals", - { - let begin = -argc + num_params; - let end = -argc + num_locals; + { + let begin = -argc + num_params; + let end = -argc + num_locals; - begin..end - }, - asm + begin..end + }, + asm ); } @@ -8104,9 +8103,13 @@ fn gen_send_iseq( assert_eq!(1, num_params); // Write the CI in to the stack and ensure that it actually gets // flushed to memory + asm_comment!(asm, "put call info for forwarding"); let ci_opnd = asm.stack_opnd(-1); asm.ctx.dealloc_reg(ci_opnd.reg_opnd()); asm.mov(ci_opnd, VALUE(ci as usize).into()); + + // Nil-initialize other locals which are above the CI + nil_fill("nil-initialize locals", 1..num_locals, asm); } // Points to the receiver operand on the stack unless a captured environment is used