ZJIT: Don't inline non-parameter locals

Author: rwstauner

test/ruby/test_zjit.rb

@@ -340,6 +340,21 @@ def test(n)
     }
   end
 
+  def test_return_nonparam_local
+    # Use dead code (if false) to create a local without initialization instructions.
+    assert_compiles 'nil', %q{
+      def foo(a)
+        if false
+          x = nil
+        end
+        x
+      end
+      def test = foo(1)
+      test
+      test
+    }, call_threshold: 2
+  end
+
   def test_setlocal_on_eval
     assert_compiles '1', %q{
       @b = binding

zjit/src/hir.rs

@@ -1731,6 +1731,12 @@ fn iseq_get_return_value(iseq: IseqPtr, captured_opnd: Option<InsnId>, ci_flags:
             let ep_offset = unsafe { *rb_iseq_pc_at_idx(iseq, 1) }.as_u32();
             let local_idx = ep_offset_to_local_idx(iseq, ep_offset);
 
+            // Only inline if the local is a parameter (not a method-defined local) as we are indexing args.
+            let param_size = unsafe { rb_get_iseq_body_param_size(iseq) } as usize;
+            if local_idx >= param_size {
+                return None;
+            }
+
             if unsafe { rb_simple_iseq_p(iseq) } {
                 return Some(IseqReturn::LocalVariable(local_idx.try_into().unwrap()));
             }

zjit/src/hir/opt_tests.rs

@@ -761,6 +761,44 @@ mod hir_opt_tests {
         ");
     }
 
+    #[test]
+    fn test_no_inline_nonparam_local_return() {
+        // Methods that return non-parameter local variables should NOT be inlined,
+        // because the local variable index will be out of bounds for args.
+        // The method must have a parameter so param_size > 0, and return a local
+        // that's not a parameter so local_idx >= param_size.
+        // Use dead code (if false) to create a local without initialization instructions,
+        // resulting in just getlocal + leave which enters the inlining code path.
+        eval("
+            def foo(a)
+              if false
+                x = nil
+              end
+              x
+            end
+            def test = foo(1)
+            test; test
+        ");
+        assert_snapshot!(hir_string("test"), @r"
+        fn test@<compiled>:8:
+        bb0():
+          EntryPoint interpreter
+          v1:BasicObject = LoadSelf
+          Jump bb2(v1)
+        bb1(v4:BasicObject):
+          EntryPoint JIT(0)
+          Jump bb2(v4)
+        bb2(v6:BasicObject):
+          v11:Fixnum[1] = Const Value(1)
+          PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010)
+          PatchPoint NoSingletonClass(Object@0x1000)
+          v20:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)]
+          v21:BasicObject = SendWithoutBlockDirect v20, :foo (0x1038), v11
+          CheckInterrupts
+          Return v21
+        ");
+    }
+
     #[test]
     fn test_optimize_send_to_aliased_cfunc() {
         eval("