ruby-changes:71592
From: Jeremy <ko1@a...>
Date: Fri, 1 Apr 2022 23:23:51 +0900 (JST)
Subject: [ruby-changes:71592] d1d48cb690 (master): Revert "Raise RuntimeError if Kernel#binding is called from a non-Ruby frame"
https://git.ruby-lang.org/ruby.git/commit/?id=d1d48cb690 From d1d48cb690fdad855da94b2a2d11721428bc06ba Mon Sep 17 00:00:00 2001 From: Jeremy Evans <code@j...> Date: Fri, 1 Apr 2022 07:22:49 -0700 Subject: Revert "Raise RuntimeError if Kernel#binding is called from a non-Ruby frame" This reverts commit 343ea9967e4a6b279eed6bd8e81ad0bdc747f254. This causes an assertion failure with -DRUBY_DEBUG=1 -DRGENGC_CHECK_MODE=2 --- bootstraptest/test_yjit.rb | 31 +++++++++++++++++++ spec/ruby/optional/capi/binding_spec.rb | 21 ++++--------- test/ruby/test_proc.rb | 5 --- test/ruby/test_settracefunc.rb | 54 ++++++++++++++++----------------- vm.c | 11 ++++--- vm_trace.c | 9 +----- 6 files changed, 72 insertions(+), 59 deletions(-) diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb index a84a9e035a..d124d180d1 100644 --- a/bootstraptest/test_yjit.rb +++ b/bootstraptest/test_yjit.rb @@ -1,3 +1,34 @@ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_yjit.rb#L1 +assert_equal '2022', %q{ + def contrivance(hash, key) + # Expect this to compile to an `opt_aref`. + hash[key] + + # The [] call above tracks that the `hash` local has a VALUE that + # is a heap pointer and the guard for the Kernel#itself call below + # doesn't check that it's a heap pointer VALUE. + # + # As you can see from the crash, the call to rb_hash_aref() can set the + # `hash` local, making eliding the heap object guard unsound. + hash.itself + end + + # This is similar to ->(recv, mid) { send(recv, mid).local_variable_set(...) }. + # By composing we avoid creating new Ruby frames and so sending :binding + # captures the environment of the frame that does the missing key lookup. + # We use it to capture the environment inside of `contrivance`. + cap_then_set = + Kernel.instance_method(:send).method(:bind_call).to_proc >> + ->(binding) { binding.local_variable_set(:hash, 2022) } + special_missing = Hash.new(&cap_then_set) + + # Make YJIT speculate that it's a hash and generate code + # that calls rb_hash_aref(). + contrivance({}, :warmup) + contrivance({}, :warmup) + + contrivance(special_missing, :binding) +} + assert_equal '18374962167983112447', %q{ # regression test for incorrectly discarding 32 bits of a pointer when it # comes to default values. diff --git a/spec/ruby/optional/capi/binding_spec.rb b/spec/ruby/optional/capi/binding_spec.rb index 2165705457..966d650c46 100644 --- a/spec/ruby/optional/capi/binding_spec.rb +++ b/spec/ruby/optional/capi/binding_spec.rb @@ -8,21 +8,12 @@ describe "CApiBindingSpecs" do https://github.com/ruby/ruby/blob/trunk/spec/ruby/optional/capi/binding_spec.rb#L8 end describe "Kernel#binding" do - ruby_version_is '3.2' do - it "raises when called from C" do - foo = 14 - -> { @b.get_binding }.should raise_error(RuntimeError) - end - end - - ruby_version_is ''...'3.2' do - it "gives the top-most Ruby binding when called from C" do - foo = 14 - b = @b.get_binding - b.local_variable_get(:foo).should == 14 - b.local_variable_set :foo, 12 - foo.should == 12 - end + it "gives the top-most Ruby binding when called from C" do + foo = 14 + b = @b.get_binding + b.local_variable_get(:foo).should == 14 + b.local_variable_set :foo, 12 + foo.should == 12 end end end diff --git a/test/ruby/test_proc.rb b/test/ruby/test_proc.rb index 05c6e03aac..63c258744a 100644 --- a/test/ruby/test_proc.rb +++ b/test/ruby/test_proc.rb @@ -445,11 +445,6 @@ class TestProc < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_proc.rb#L445 assert_equal(@@line_of_source_location_test, lineno, 'Bug #2427') end - def test_binding_error_unless_ruby_frame - define_singleton_method :binding_from_c!, method(:binding).to_proc >> ->(bndg) {bndg} - assert_raise(RuntimeError) { binding_from_c! } - end - def test_proc_lambda assert_raise(ArgumentError) { proc } assert_raise(ArgumentError) { assert_warn(/deprecated/) {lambda} } diff --git a/test/ruby/test_settracefunc.rb b/test/ruby/test_settracefunc.rb index dcf821f563..f10272f2ee 100644 --- a/test/ruby/test_settracefunc.rb +++ b/test/ruby/test_settracefunc.rb @@ -594,7 +594,7 @@ CODE https://github.com/ruby/ruby/blob/trunk/test/ruby/test_settracefunc.rb#L594 eval <<-EOF.gsub(/^.*?: /, ""), nil, 'xyzzy' 1: set_trace_func(lambda{|event, file, line, id, binding, klass| - 2: events << [event, line, file, klass, id, binding&.eval('self'), binding&.eval("_local_var")] if file == 'xyzzy' + 2: events << [event, line, file, klass, id, binding.eval('self'), binding.eval("_local_var")] if file == 'xyzzy' 3: }) 4: 1.times{|;_local_var| _local_var = :inner 5: tap{} @@ -619,31 +619,31 @@ CODE https://github.com/ruby/ruby/blob/trunk/test/ruby/test_settracefunc.rb#L619 answer_events = [ # - [:c_return, 1, "xyzzy", TracePoint, :trace, TracePoint, nil, nil], + [:c_return, 1, "xyzzy", TracePoint, :trace, TracePoint, :outer, trace], [:line, 4, 'xyzzy', self.class, method, self, :outer, :nothing], - [:c_call, 4, 'xyzzy', Integer, :times, 1, nil, nil], + [:c_call, 4, 'xyzzy', Integer, :times, 1, :outer, :nothing], [:line, 4, 'xyzzy', self.class, method, self, nil, :nothing], [:line, 5, 'xyzzy', self.class, method, self, :inner, :nothing], - [:c_return, 4, "xyzzy", Integer, :times, 1, nil, nil], + [:c_return, 4, "xyzzy", Integer, :times, 1, :outer, 1], [:line, 7, 'xyzzy', self.class, method, self, :outer, :nothing], - [:c_call, 7, "xyzzy", Class, :inherited, Object, nil, nil], - [:c_return, 7, "xyzzy", Class, :inherited, Object, nil, nil], - [:c_call, 7, "xyzzy", Class, :const_added, Object, nil, nil], - [:c_return, 7, "xyzzy", Class, :const_added, Object, nil, nil], + [:c_call, 7, "xyzzy", Class, :inherited, Object, :outer, :nothing], + [:c_return, 7, "xyzzy", Class, :inherited, Object, :outer, nil], + [:c_call, 7, "xyzzy", Class, :const_added, Object, :outer, :nothing], + [:c_return, 7, "xyzzy", Class, :const_added, Object, :outer, nil], [:class, 7, "xyzzy", nil, nil, xyzzy.class, nil, :nothing], [:line, 8, "xyzzy", nil, nil, xyzzy.class, nil, :nothing], [:line, 9, "xyzzy", nil, nil, xyzzy.class, :XYZZY_outer, :nothing], - [:c_call, 9, "xyzzy", Module, :method_added, xyzzy.class, nil, nil], - [:c_return, 9, "xyzzy", Module, :method_added, xyzzy.class, nil, nil], + [:c_call, 9, "xyzzy", Module, :method_added, xyzzy.class, :XYZZY_outer, :nothing], + [:c_return, 9, "xyzzy", Module, :method_added, xyzzy.class, :XYZZY_outer, nil], [:line, 13, "xyzzy", nil, nil, xyzzy.class, :XYZZY_outer, :nothing], - [:c_call, 13, "xyzzy", Module, :method_added, xyzzy.class, nil, nil], - [:c_return,13, "xyzzy", Module, :method_added, xyzzy.class, nil, nil], + [:c_call, 13, "xyzzy", Module, :method_added, xyzzy.class, :XYZZY_outer, :nothing], + [:c_return,13, "xyzzy", Module, :method_added, xyzzy.class, :XYZZY_outer, nil], [:end, 17, "xyzzy", nil, nil, xyzzy.class, :XYZZY_outer, :nothing], [:line, 18, "xyzzy", TestSetTraceFunc, method, self, :outer, :nothing], - [:c_call, 18, "xyzzy", Class, :new, xyzzy.class, nil, nil], - [:c_call, 18, "xyzzy", BasicObject, :initialize, xyzzy, nil, nil], - [:c_return,18, "xyzzy", BasicObject, :initialize, xyzzy, nil, nil], - [:c_return,18, "xyzzy", Class, :new, xyzzy.class, nil, nil], + [:c_call, 18, "xyzzy", Class, :new, xyzzy.class, :outer, :nothing], + [:c_call, 18, "xyzzy", BasicObject, :initialize, xyzzy, :outer, :nothing], + [:c_return,18, "xyzzy", BasicObject, :initialize, xyzzy, :outer, nil], + [:c_return,18, "xyzzy", Class, :new, xyzzy.class, :outer, xyzzy], [:line, 19, "xyzzy", TestSetTraceFunc, method, self, :outer, :nothing], [:call, 9, "xyzzy", xyzzy.class, :foo, xyzzy, nil, :nothing], [:line, 10, "xyzzy", xyzzy.class, :foo, xyzzy, nil, :nothing], @@ -654,19 +654,19 @@ CODE https://github.com/ruby/ruby/blob/trunk/test/ruby/test_settracefunc.rb#L654 [:return, 16, "xyzzy", xyzzy.class, :bar, xyzzy, :XYZZY_bar, xyzzy], [:return, 12, "xyzzy", xyzzy.class, :foo, xyzzy, :XYZZY_foo, xyzzy], [:line, 20, "xyzzy", TestSetTraceFunc, method, self, :outer, :nothing], - [:c_call, 20, "xyzzy", Kernel, :raise, self, nil, nil], - [:c_call, 20, "xyzzy", Exception, :exception, RuntimeError, nil, nil], - [:c_call, 20, "xyzzy", Exception, :initialize, raised_exc, nil, nil], - [:c_return,20, "xyzzy", Exception, :initialize, raised_exc, nil, nil], - [:c_return,20, "xyzzy", Exception, :exception, RuntimeError, nil, nil], - [:c_return,20, "xyzzy", Kernel, :raise, self, nil, nil], - [:c_call, 20, "xyzzy", Exception, :backtrace, raised_exc, nil, nil], - [:c_return,20, "xyzzy", Exception, :backtrace, raised_exc, nil, nil], + [:c_call, 20, "xyzzy", K (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/