1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2022-11-09 12:17:21 -05:00

[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>
This commit is contained in:
Eileen M. Uchitelle 2021-06-14 20:34:57 -04:00 committed by GitHub
parent a09ddfc420
commit 2088a45798
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
Notes: git 2021-06-15 09:35:19 +09:00
Merged-By: k0kubun <takashikkbn@gmail.com>
2 changed files with 47 additions and 0 deletions

View file

@ -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());
}

View file

@ -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