summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEileen M. Uchitelle <eileencodes@users.noreply.github.com>2021-06-14 20:34:57 -0400
committerGitHub <noreply@github.com>2021-06-14 17:34:57 -0700
commit2088a457981b0f71a3bfd14871ed5b6f0d090e6a (patch)
tree3ba3959e6b8b8debb4d18e5ba310595e651d90d9
parenta09ddfc4207cce58693f2226ebbbc4b8f009fb23 (diff)
[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@ruby-lang.org> Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
Notes
Notes: Merged-By: k0kubun <takashikkbn@gmail.com>
-rw-r--r--insns.def1
-rw-r--r--test/ruby/test_insns_leaf.rb46
2 files changed, 47 insertions, 0 deletions
diff --git a/insns.def b/insns.def
index 8d609927b5..c6ebf7401b 100644
--- a/insns.def
+++ b/insns.def
@@ -1023,6 +1023,7 @@ opt_setinlinecache
(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 0000000000..9c9a4324cb
--- /dev/null
+++ b/test/ruby/test_insns_leaf.rb
@@ -0,0 +1,46 @@
+# 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