[前][次][番号順一覧][スレッド一覧]

ruby-changes:66488

From: Eileen <ko1@a...>
Date: Tue, 15 Jun 2021 09:35:15 +0900 (JST)
Subject: [ruby-changes:66488] 2088a45798 (master): [Bug #17880] Set leaf false on opt_setinlinecache (#4565)

https://git.ruby-lang.org/ruby.git/commit/?id=2088a45798

From 2088a457981b0f71a3bfd14871ed5b6f0d090e6a Mon Sep 17 00:00:00 2001
From: "Eileen M. Uchitelle" <eileencodes@u...>
Date: Mon, 14 Jun 2021 20:34:57 -0400
Subject: [Bug #17880] Set leaf false on opt_setinlinecache (#4565)

This change fixes the bug described in https://bugs.ruby-lang.org/issues/17880.

Checking `ractor_shareable_p` will cause the method to call back into
Ruby. Anything calling this method can't be a leaf instruction,
otherwise it could crash. By adding `attr bool leaf = false` we no
longer crash because it marks the function as not a leaf.

Here's a simplified reproduction script:

```ruby
require "set"

class Id
  attr_reader :db_id
  def initialize(db_id)
    @db_id = db_id
  end

  def ==(other)
    other.class == self.class && other.db_id == db_id
  end
  alias_method :eql?, :==

  def hash
    10
  end

  def <=>(other)
    db_id <=> other.db_id if other.is_a?(self.class)
  end
end

class Namespace
  IDS = Set[
    Id.new(1).freeze,
    Id.new(2).freeze,
    Id.new(3).freeze,
    Id.new(4).freeze,
  ].freeze

  class << self
    def test?(id)
      IDS.include?(id)
    end
  end
end

p Namespace.test?(Id.new(1))
p Namespace.test?(Id.new(5))
```

Co-authored-by: Aaron Patterson <tenderlove@r...>

Co-authored-by: Aaron Patterson <tenderlove@r...>
---
 insns.def                    |  1 +
 test/ruby/test_insns_leaf.rb | 46 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)
 create mode 100644 test/ruby/test_insns_leaf.rb

diff --git a/insns.def b/insns.def
index 8d60992..c6ebf74 100644
--- a/insns.def
+++ b/insns.def
@@ -1023,6 +1023,7 @@ opt_setinlinecache https://github.com/ruby/ruby/blob/trunk/insns.def#L1023
 (IC ic)
 (VALUE val)
 (VALUE val)
+// attr bool leaf = false;
 {
     vm_ic_update(GET_ISEQ(), ic, val, GET_EP());
 }
diff --git a/test/ruby/test_insns_leaf.rb b/test/ruby/test_insns_leaf.rb
new file mode 100644
index 0000000..9c9a432
--- /dev/null
+++ b/test/ruby/test_insns_leaf.rb
@@ -0,0 +1,46 @@ https://github.com/ruby/ruby/blob/trunk/test/ruby/test_insns_leaf.rb#L1
+# frozen_string_literal: false
+require 'test/unit'
+
+class TestInsnsLeaf < Test::Unit::TestCase
+  require "set"
+
+  class Id
+    attr_reader :db_id
+    def initialize(db_id)
+      @db_id = db_id
+    end
+
+    def ==(other)
+      other.class == self.class && other.db_id == db_id
+    end
+    alias_method :eql?, :==
+
+    def hash
+      10
+    end
+
+    def <=>(other)
+      db_id <=> other.db_id if other.is_a?(self.class)
+    end
+  end
+
+  class Namespace
+    IDS = Set[
+      Id.new(1).freeze,
+      Id.new(2).freeze,
+      Id.new(3).freeze,
+      Id.new(4).freeze,
+    ].freeze
+
+    class << self
+      def test?(id)
+	IDS.include?(id)
+      end
+    end
+  end
+
+  def test_insns_leaf
+    assert Namespace.test?(Id.new(1)), "IDS should include 1"
+    assert !Namespace.test?(Id.new(5)), "IDS should not include 5"
+  end
+end
-- 
cgit v1.1


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]