Avoid issues with refinement zsuper lookup

Without this patch, there are multiple issues when defining a zsuper
method via a refinement. These issues are due to the orig_me entry
for the zsuper method not being updated when the method it refers
to is modified via:

* Method being redefined (in same or closer ancestor)
* Method being removed
* Method being undef-ed (in same or closer ancestor)
* Method being overridden by method in included module
* Method being overridden by method in prepended module
* Method being defined in an included/prepended module at time of
  refinement

This includes a comprehensive test suite for these cases, using both
zsuper methods and iseq methods that call super. Without the changes
to refinement method lookup, the following types of errors occur for
refinement zsuper methods (all of the iseq methods that call super
work correctly with or without these changes):

* Incorrect result (not returning result of expected method, not
  raising NoMethodError if the method was removed or has been undef-ed)
* SystemStackError: stack level too deep
* NotImplementedError: false() function is unimplemented on this machine

This avoids the issues with refinement zsuper lookup by turning the
zsuper into a cfunc that uses rb_call_super_kw. This is not a perfect
solution, for two reasons:

* cfuncs are slower than zsuper
* arity/parameters for the method not as helpful (you

It may possible to avoid these issues by clearing method caches in
more cases. However, I think that would require a lot of extra work,
since you cannot just clear the method cache for the current class.
You would need to clear it for all subclasses that are refined, and
if this is a module, do the same for all classes that include/prepend
the module, as well as any subclasses of those classes.

Considering the need for refinement zsuper methods is very rare, the
performance and arity/parameters issues seem acceptable (to me).

Fixes [Bug #22022]

2 files changed, 596 insertions, 5 deletions

diff --git a/test/ruby/test_refinement.rb b/test/ruby/test_refinement.rb
index f4fe2fc44b..daff07ecb0 100644
--- a/test/ruby/test_refinement.rb
+++ b/test/ruby/test_refinement.rb
@@ -1058,6 +1058,586 @@ class TestRefinement < Test::Unit::TestCase
     end;
   end
 
+  {
+    zsuper: "public :a",
+    super: "def a = super"
+  }.each do |desc, method_def|
+    define_method :"test_modify_#{desc}_refinement_method_in_superclass" do
+      assert_separately([], <<-"end;")
+        class A
+          private def a = :a
+          alias a a
+        end
+
+        class B < A
+        end
+
+        module R
+          refine B do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:a, B.new.a)
+
+        class A
+          def a = :b
+        end
+        assert_equal(:b, B.new.a)
+      end;
+    end
+
+    define_method :"test_modify_#{desc}_refinement_method_in_module_prepended_to_superclass" do
+      assert_separately([], <<-"end;")
+        module M
+          private def a = :a
+          alias a a
+        end
+
+        class A
+          prepend M
+        end
+
+        class B < A
+        end
+
+        module R
+          refine B do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:a, B.new.a)
+
+        module M
+          def a = :b
+        end
+        assert_equal(:b, B.new.a)
+      end;
+    end
+
+    define_method :"test_modify_#{desc}_refinement_method_in_module_included_in_superclass" do
+      assert_separately([], <<-"end;")
+        module M
+          private def a = :a
+          alias a a
+        end
+
+        class A
+          include M
+        end
+
+        class B < A
+        end
+
+        module R
+          refine B do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:a, B.new.a)
+
+        module M
+          def a = :b
+        end
+        assert_equal(:b, B.new.a)
+      end;
+    end
+
+    define_method :"test_remove_#{desc}_refinement_method_from_superclass" do
+      assert_separately([], <<-"end;")
+        class A
+          private def a = :a
+        end
+
+        class B < A
+          private def a = :b
+        end
+
+        class C < B
+        end
+
+        module R
+          refine C do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:b, C.new.a)
+
+        class B
+          remove_method(:a)
+        end
+        assert_equal(:a, C.new.a)
+      end;
+    end
+
+    define_method :"test_remove_#{desc}_refinement_method_from_module_prepended_to_superclass" do
+      assert_separately([], <<-"end;")
+        module M
+          private def a = :b
+        end
+
+        class A
+          prepend M
+          private def a = :a
+        end
+
+        class B < A
+        end
+
+        module R
+          refine B do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:b, B.new.a)
+
+        module M
+          remove_method(:a)
+        end
+        assert_equal(:a, B.new.a)
+      end;
+    end
+
+    define_method :"test_remove_#{desc}_refinement_method_from_module_prepended_to_class" do
+      assert_separately([], <<-"end;")
+        module M
+          private def a = :b
+        end
+
+        class A
+          prepend M
+          private def a = :a
+        end
+
+        module R
+          refine A do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:b, A.new.a)
+
+        module M
+          remove_method(:a)
+        end
+        assert_equal(:a, A.new.a)
+      end;
+    end
+
+    define_method :"test_remove_#{desc}_refinement_method_from_module_included_in_superclass" do
+      assert_separately([], <<-"end;")
+        module M
+          private def a = :b
+        end
+
+        class A
+          private def a = :a
+        end
+
+        class B < A
+          include M
+        end
+
+        class C < B
+        end
+
+        module R
+          refine C do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:b, C.new.a)
+
+        module M
+          remove_method(:a)
+        end
+        assert_equal(:a, C.new.a)
+      end;
+    end
+
+    define_method :"test_remove_#{desc}_refinement_method_from_module_included_in_class" do
+      assert_separately([], <<-"end;")
+        module M
+          private def a = :b
+        end
+
+        class A
+          private def a = :a
+        end
+
+        class B < A
+          include M
+        end
+
+        module R
+          refine B do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:b, B.new.a)
+
+        module M
+          remove_method(:a)
+        end
+        assert_equal(:a, B.new.a)
+      end;
+    end
+
+    define_method :"test_undef_#{desc}_refinement_method_in_superclass" do
+      assert_separately([], <<-"end;")
+        class A
+          private def a = :a
+        end
+
+        class B < A
+          private def a = :b
+        end
+
+        class C < B
+        end
+
+        module R
+          refine C do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:b, C.new.a)
+
+        class B
+          undef_method(:a)
+        end
+        assert_raise(NoMethodError) { C.new.a }
+      end;
+    end
+
+    define_method :"test_undef_#{desc}_refinement_method_in_module_prepended_to_superclass" do
+      assert_separately([], <<-"end;")
+        module M
+          private def a = :b
+        end
+
+        class A
+          prepend M
+          private def a = :a
+        end
+
+        class B < A
+        end
+
+        module R
+          refine B do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:b, B.new.a)
+
+        module M
+          undef_method(:a)
+        end
+        assert_raise(NoMethodError) { B.new.a }
+      end;
+    end
+
+    define_method :"test_undef_#{desc}_refinement_method_in_module_prepended_to_class" do
+      assert_separately([], <<-"end;")
+        module M
+          private def a = :b
+        end
+
+        class A
+          prepend M
+          private def a = :a
+        end
+
+        module R
+          refine A do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:b, A.new.a)
+
+        module M
+          undef_method(:a)
+        end
+        assert_raise(NoMethodError) { A.new.a }
+      end;
+    end
+
+    define_method :"test_undef_#{desc}_refinement_method_in_module_included_in_superclass" do
+      assert_separately([], <<-"end;")
+        module M
+          private def a = :b
+        end
+
+        class A
+          private def a = :a
+        end
+
+        class B < A
+          include M
+        end
+
+        class C < B
+        end
+
+        module R
+          refine C do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:b, C.new.a)
+
+        module M
+          undef_method(:a)
+        end
+        assert_raise(NoMethodError) { C.new.a }
+      end;
+    end
+
+    define_method :"test_undef_#{desc}_refinement_method_in_module_included_in_class" do
+      assert_separately([], <<-"end;")
+        module M
+          private def a = :b
+        end
+
+        class A
+          private def a = :a
+        end
+
+        class B < A
+          include M
+        end
+
+        module R
+          refine B do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:b, B.new.a)
+
+        module M
+          undef_method(:a)
+        end
+        assert_raise(NoMethodError) { B.new.a }
+      end;
+    end
+
+    define_method :"test_override_#{desc}_refinement_method_by_prepending_to_class" do
+      assert_separately([], <<-"end;")
+        class A
+          private def a = :a
+        end
+
+        module R
+          refine A do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:a, A.new.a)
+
+        module M
+          def a = :b
+        end
+        A.prepend M
+        assert_equal(:b, A.new.a)
+      end;
+    end
+
+    define_method :"test_override_#{desc}_refinement_method_by_prepending_to_superclass" do
+      assert_separately([], <<-"end;")
+        class A
+          private def a = :a
+        end
+
+        class B < A
+        end
+
+        module R
+          refine B do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:a, B.new.a)
+
+        module M
+          def a = :b
+        end
+        A.prepend M
+        assert_equal(:b, B.new.a)
+      end;
+    end
+
+    define_method :"test_override_#{desc}_refinement_method_by_including_in_class" do
+      assert_separately([], <<-"end;")
+        class A
+          private def a = :a
+        end
+
+        class B < A
+        end
+
+        module R
+          refine B do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:a, B.new.a)
+
+        module M
+          def a = :b
+        end
+        B.include M
+        assert_equal(:b, B.new.a)
+      end;
+    end
+
+    define_method :"test_override_#{desc}_refinement_method_by_including_in_superclass" do
+      assert_separately([], <<-"end;")
+        class A
+          private def a = :a
+        end
+
+        class B < A
+        end
+
+        class C < B
+        end
+
+        module R
+          refine C do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:a, C.new.a)
+
+        module M
+          def a = :b
+        end
+        B.include M
+        assert_equal(:b, C.new.a)
+      end;
+    end
+
+    define_method :"test_override_#{desc}_refinement_method_by_prepending_undef_to_class" do
+      assert_separately([], <<-"end;")
+        class A
+          private def a = :a
+        end
+
+        module R
+          refine A do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:a, A.new.a)
+
+        module M
+          def a = :b
+          undef_method :a
+        end
+        A.prepend M
+        assert_raise(NoMethodError) { A.new.a }
+      end;
+    end
+
+    define_method :"test_override_#{desc}_refinement_method_by_prepending_undef_to_superclass" do
+      assert_separately([], <<-"end;")
+        class A
+          private def a = :a
+        end
+
+        class B < A
+        end
+
+        module R
+          refine B do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:a, B.new.a)
+
+        module M
+          def a = :b
+          undef_method :a
+        end
+        A.prepend M
+        assert_raise(NoMethodError) { B.new.a }
+      end;
+    end
+
+    define_method :"test_override_#{desc}_refinement_method_by_including_undef_in_class" do
+      assert_separately([], <<-"end;")
+        class A
+          private def a = :a
+        end
+
+        class B < A
+        end
+
+        module R
+          refine B do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:a, B.new.a)
+
+        module M
+          def a = :b
+          undef_method :a
+        end
+        B.include M
+        assert_raise(NoMethodError) { B.new.a }
+      end;
+    end
+
+    define_method :"test_override_#{desc}_refinement_method_by_including_undef_in_superclass" do
+      assert_separately([], <<-"end;")
+        class A
+          private def a = :a
+        end
+
+        class B < A
+        end
+
+        class C < B
+        end
+
+        module R
+          refine C do
+            #{method_def}
+          end
+        end
+        using R
+        assert_equal(:a, C.new.a)
+
+        module M
+          def a = :b
+          undef_method :a
+        end
+        B.include M
+        assert_raise(NoMethodError) { C.new.a }
+      end;
+    end
+  end
+
   def test_instance_methods
     bug8881 = '[ruby-core:57080] [Bug #8881]'
     assert_not_include(Foo.instance_methods(false), :z, bug8881)
diff --git a/vm_method.c b/vm_method.c
index 9a090d1eb7..e9f0d85cae 100644
--- a/vm_method.c
+++ b/vm_method.c
@@ -1370,6 +1370,12 @@ check_override_opt_method(VALUE klass, VALUE mid)
     }
 }
 
+static VALUE
+zsuper_to_super(int argc, VALUE *argv, VALUE self)
+{
+    return rb_call_super_kw(argc, argv, RB_PASS_CALLED_KEYWORDS);
+}
+
 static inline rb_method_entry_t* search_method0(VALUE klass, ID id, VALUE *defined_class_ptr, bool skip_refined);
 /*
  * klass->method_table[mid] = method_entry(defined_class, visi, def)
@@ -1386,6 +1392,7 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil
     st_data_t data;
     int make_refined = 0;
     VALUE orig_klass;
+    bool turn_zsuper_to_super = false;
 
     if (NIL_P(klass)) {
         klass = rb_cObject;
@@ -1411,12 +1418,10 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil
 
     if (RB_TYPE_P(klass, T_MODULE) && FL_TEST(klass, RMODULE_IS_REFINEMENT)) {
         VALUE refined_class = rb_refinement_module_get_refined_class(klass);
-        bool search_superclass = type == VM_METHOD_TYPE_ZSUPER && !lookup_method_table(refined_class, mid);
-        rb_add_refined_method_entry(refined_class, mid);
-        if (search_superclass) {
-            rb_method_entry_t *me = lookup_method_table(refined_class, mid);
-            RB_OBJ_WRITE(me, &me->def->body.refined.orig_me, search_method0(refined_class, mid, NULL, true));
+        if (type == VM_METHOD_TYPE_ZSUPER) {
+            turn_zsuper_to_super = true;
         }
+        rb_add_refined_method_entry(refined_class, mid);
     }
     if (type == VM_METHOD_TYPE_REFINED) {
         rb_method_entry_t *old_me = lookup_method_table(RCLASS_ORIGIN(klass), mid);
@@ -1479,6 +1484,12 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil
     me = rb_method_entry_create(mid, defined_class, visi, NULL);
     if (def == NULL) {
         def = rb_method_definition_create(type, original_id);
+        if (turn_zsuper_to_super) {
+          def->type = VM_METHOD_TYPE_CFUNC;
+          def->body.cfunc.func = (rb_cfunc_t)zsuper_to_super;
+          def->body.cfunc.invoker = ractor_safe_call_cfunc_m1;
+          def->body.cfunc.argc = -1;
+        }
     }
     rb_method_definition_set(me, def, opts);