From b7ea66bc3228635a87125bea69f01779f75c39de Mon Sep 17 00:00:00 2001
From: Alan Wu <XrXr@users.noreply.github.com>
Date: Mon, 6 Dec 2021 17:09:52 -0500
Subject: YJIT: Fix incomplete invalidation from opt_setinlinecache

As part of YJIT's strategy for promoting Ruby constant expressions into
constants in the output native code, the interpreter calls
rb_yjit_constant_ic_update() from opt_setinlinecache.

The block invalidation loop indirectly calls rb_darray_remove_unordered(),
which does a shuffle remove. Because of this, looping with an
incrementing counter like done previously can miss some elements in the
array. Repeatedly invalidate the first element instead.

The bug this commit resolves does not seem to cause crashes or divergent
behaviors.

Co-authored-by: Jemma Issroff <jemmaissroff@gmail.com>
---
 bootstraptest/test_yjit.rb | 85 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

(limited to 'bootstraptest')

diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb
index 18d1f85b93..140365ee0c 100644
--- a/bootstraptest/test_yjit.rb
+++ b/bootstraptest/test_yjit.rb
@@ -1,3 +1,88 @@
+assert_equal '0', %q{
+  # This is a regression test for incomplete invalidation from
+  # opt_setinlinecache. This test might be brittle, so
+  # feel free to remove it in the future if it's too annoying.
+  # This test assumes --yjit-call-threshold=2.
+  module M
+    Foo = 1
+    def foo
+      Foo
+    end
+
+    def pin_self_type_then_foo
+      _ = @foo
+      foo
+    end
+
+    def only_ints
+      1 + self
+      foo
+    end
+  end
+
+  class Integer
+    include M
+  end
+
+  class Sub
+    include M
+  end
+
+  foo_method = M.instance_method(:foo)
+
+  dbg = ->(message) do
+    return # comment this out to get printouts
+
+    $stderr.puts RubyVM::YJIT.disasm(foo_method)
+    $stderr.puts message
+  end
+
+  2.times { 42.only_ints }
+
+  dbg["There should be two versions of getinlineache"]
+
+  module M
+    remove_const(:Foo)
+  end
+
+  dbg["There should be no getinlinecaches"]
+
+  2.times do
+    42.only_ints
+  rescue NameError => err
+    _ = "caught name error #{err}"
+  end
+
+  dbg["There should be one version of getinlineache"]
+
+  2.times do
+    Sub.new.pin_self_type_then_foo
+  rescue NameError
+    _ = 'second specialization'
+  end
+
+  dbg["There should be two versions of getinlineache"]
+
+  module M
+    Foo = 1
+  end
+
+  dbg["There should still be two versions of getinlineache"]
+
+  42.only_ints
+
+  dbg["There should be no getinlinecaches"]
+
+  # Find name of the first VM instruction in M#foo.
+  insns = RubyVM::InstructionSequence.of(foo_method).to_a
+  if defined?(RubyVM::YJIT.blocks_for) && (insns.last.find { Array === _1 }&.first == :opt_getinlinecache)
+    RubyVM::YJIT.blocks_for(RubyVM::InstructionSequence.of(foo_method))
+      .filter { _1.iseq_start_index == 0 }.count
+  else
+    0 # skip the test
+  end
+}
+
 # Check that frozen objects are respected
 assert_equal 'great', %q{
   class Foo
-- 
cgit v1.2.3