ruby-changes:60781
From: Takashi <ko1@a...>
Date: Wed, 15 Apr 2020 15:42:31 +0900 (JST)
Subject: [ruby-changes:60781] 8355a99883 (master): Invalidate fastpath when calling attr_writer by super
https://git.ruby-lang.org/ruby.git/commit/?id=8355a99883 From 8355a998839f17ff214a89062821a0a4287f6a54 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun <takashikkbn@g...> Date: Tue, 14 Apr 2020 23:24:50 -0700 Subject: Invalidate fastpath when calling attr_writer by super We started to use fastpath on invokesuper when a method is not refinements since 5c27681813, but we shouldn't have used fastpath for attr_writer either. `cc->aux_.attr_index` is for an actual receiver class, while we store its superclass in `cc->klass` and therefore there's no way to properly invalidate attr_writer's inline cache when it's called by super. [Bug #16785] I suspect the same bug also exists in attr_reader. I'll address that in another commit. diff --git a/test/ruby/test_super.rb b/test/ruby/test_super.rb index 3d5af3d..7cbe851 100644 --- a/test/ruby/test_super.rb +++ b/test/ruby/test_super.rb @@ -603,4 +603,34 @@ class TestSuper < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_super.rb#L603 assert_equal :boo, subklass.new.baz assert_equal :boo2, subklass.new.boo end + + def test_super_attr_writer # Bug #16785 + writer_class = Class.new do + attr_writer :test + end + superwriter_class = Class.new(writer_class) do + def initialize + @test = 1 # index: 1 + end + + def test=(test) + super(test) + end + end + inherited_class = Class.new(superwriter_class) do + def initialize + @a = nil + @test = 2 # index: 2 + end + end + + superwriter = superwriter_class.new + superwriter.test = 3 # set ic->index of superwriter_class#test= to 1 + + inherited = inherited_class.new + inherited.test = 4 # it may set 4 to index=1 while it should be index=2 + + assert_equal 3, superwriter.instance_variable_get(:@test) + assert_equal 4, inherited.instance_variable_get(:@test) + end end diff --git a/vm_insnhelper.c b/vm_insnhelper.c index a4edd9a..c4a8dfa 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -3231,9 +3231,18 @@ vm_search_super_method(const rb_control_frame_t *reg_cfp, struct rb_call_data *c https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L3231 const struct rb_callcache *cc = vm_cc_new(klass, cme, vm_call_super_method); RB_OBJ_WRITE(reg_cfp->iseq, &cd->cc, cc); } - else if (cached_cme->def->type == VM_METHOD_TYPE_REFINED) { - // vm_call_refined (search_refined_method) assumes cc->call is vm_call_super_method on invokesuper. - vm_cc_call_set(cd->cc, vm_call_super_method); + else { + switch (cached_cme->def->type) { + // vm_call_refined (search_refined_method) assumes cc->call is vm_call_super_method on invokesuper + case VM_METHOD_TYPE_REFINED: + // cc->klass is superclass of a class of receiver. Checking cc->klass is not enough to invalidate IVC for the receiver class. + case VM_METHOD_TYPE_ATTRSET: + // TODO: case VM_METHOD_TYPE_IVAR: + vm_cc_call_set(cd->cc, vm_call_super_method); // invalidate fastpath + break; + default: + break; // use fastpath + } } } } -- cgit v0.10.2 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/