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/