Bug #3136: reuse of singleton method definition causes SEGV

• Assignee set to ko1 (Koichi Sasada)

遠藤です。

以下でも SEGV または未定義挙動をします。

  Class.new do
    define_method(:foo) { super() }.call
  end

define_method に Proc が登録されることで、Proc のブロックの iseq
の klass が更新されてしまいます。
その Proc を直接呼び出すと、cfp に method_entry が乗らないけれど
super 先の候補になってしまい、落ちるようです。

ブロックの iseq の klass を元に super 先を決めているのが悪いので、
同じ原因と言えます。

違う問題かと悩んだので、登録しておきます。

-- 
Yusuke Endoh <[e-mail]>

遠藤です。

チケット重複情報の補足です。

このチケットで最初に説明した内容は #2502 の重複でした。

補足で説明した内容 (define_method の Proc の直接 call) は
#2420 の重複でした。

また、補足で説明した内容は #2402 のパッチで、SEGV に至る前に
例外になるので、発症しなくなります (本質的に解決するわけでは
ないけれど )。

-- 
Yusuke Endoh <[e-mail]>

遠藤です。

2010年4月12日22:45 Yusuke Endoh <[e-mail]>:
> 以下で SEGV します。
>
>
>  def overlaid(obj)
>    class << obj
>      def reverse
>        super
>      end
>    end
>  end
>
>  overlaid(str = "123")   # (1)
>  overlaid(ary = [1,2,3]) # (2)
>  str.reverse             # (3) => SEGV


この件および以下の関連チケットの件ですが、

  related to Bug #2402  super in instance_eval
  duplicates Bug #2420  super call may use wrong receiver object
  duplicates Bug #2502  strange behavior of anonymous class inside a proc

修正するためには iseq#klass 相当の情報を dfp に載せるようにするなど
しないとダメなようで、ささださんと話した結果、近い将来に直すのが困難
そうということになりました。

1.9.2 では WONTFIX としようと思います。といっても SEGV するのは放置
できないので、SEGV する前に NotImplementedError を投げるパッチを書き
ました。

とりあえずこれをコミットして、当該チケットを Low にしようと思います。


diff --git a/insns.def b/insns.def
index 79b92d8..5ad5948 100644
--- a/insns.def
+++ b/insns.def
@@ -1022,6 +1022,12 @@ invokesuper

     recv = GET_SELF();
     vm_search_superclass(GET_CFP(), GET_ISEQ(), recv, TOPN(num), &id, &klass);
+
+    /* temporary measure for [Bug #2402] [Bug #2502] [Bug #3136] */
+    if (!rb_obj_is_kind_of(recv, klass)) {
+	rb_raise(rb_eNotImpError, "super from singleton method that is
defined to multiple classes is not supported.  This will be fixed in
1.9.3 or later");
+    }
+
     me = rb_method_entry(klass, id);

     CALL_METHOD(num, blockptr, flag, id, me, recv);
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 0bd29b5..3b442c8 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -1402,6 +1402,11 @@ vm_search_superclass(rb_control_frame_t
*reg_cfp, rb_iseq_t *ip,
 	    }
 	}

+	/* temporary measure for [Bug #2420] [Bug #3136] */
+	if (!lcfp->me) {
+	    rb_raise(rb_eNoMethodError, "super called outside of method");
+	}
+
 	id = lcfp->me->def->original_id;
 	klass = vm_search_normal_superclass(lcfp->me->klass, recv);
     }

-- 
Yusuke Endoh <[e-mail]>

• Target version changed from 1.9.2 to 2.0.0

ワナベと申します。
解決方法 1. はきつそうだったので、2. の方向で試してみました。
これで本チケットおよび 2502 は発症しなくなりました。
複製することによる副作用に思い当たらないので、
反対がなければコミットしてしまおうと思っています。

diff --git a/vm.c b/vm.c
index e62c9a4..a381d3c 100644
--- a/vm.c
+++ b/vm.c
@@ -1844,6 +1844,11 @@ vm_define_method(rb_thread_t *th, VALUE obj, ID id, VALUE iseqval,
     rb_iseq_t *miseq;
     GetISeqPtr(iseqval, miseq);

+    if (miseq->klass) {
+       iseqval = rb_iseq_clone(iseqval, 0);
+       GetISeqPtr(iseqval, miseq);
+    }
+
     if (NIL_P(klass)) {
        rb_raise(rb_eTypeError, "no class/module to add method");
     }

　ささだです．

(2010/08/12 18:31), _ wanabe wrote:
> 解決方法 1. はきつそうだったので、2. の方向で試してみました。
> これで本チケットおよび 2502 は発症しなくなりました。
> 複製することによる副作用に思い当たらないので、
> 反対がなければコミットしてしまおうと思っています。

　とくに，他に問題が無いようでしたらお願いします．


----
以下，VM の中身に関する余談．

　ちなみに，この辺の設計を全部やり直そうと考えています．

　この問題（もしくは，別の問題）の根本的な原因は，iseq が klass とかを
持ってる（iseq を用いてメソッドを定義したとき，そのクラスを iseq がくっ
つける）という設計がまずい，ということは数年前から気にしていたんですが，
面倒なので放置していました．

　この問題が面倒なのは，静的な（lexical な）情報と，動的な情報（定義した
クラス，みたいな）を，なんかぐちゃぐちゃにしている Ruby の挙動に問題があ
るんですが，さて，その辺が整理できていない，という話です．

　Ruby 1.8 までは，この辺の静的な情報も動的な情報も，すべて実行時にス
タックに積むことで問題なく実現していました．RHG でいう，「7本のスタッ
ク」に，実行時に積んでいたわけです．

　Ruby 1.9 というか YARV の目標は，高速化ですから，なるべく実行時に何か
積むのは嫌だなぁ，と思って全体を設計しています．なので，iseq->klass のよ
うな実装になっているわけです．

　で，いろいろ考えたんですが，実行時に，毎回制御フレームごとに積んでいる
iseq をやめて，その辺の諸々の情報（iseq とか，klass とか，cref とか．名
前はまだ無い）を含んだ何かを積むことにしようかと．その何かは，メソッド定
義なんかの時に，作ることになります．

　ちょっと面倒なのは，そのメソッド定義で利用される iseq が参照しているブ
ロックなども，その何かを正しく参照できなければならない，ということでし
た．ここが，この実装にするときの面倒な点で，腰が重かった理由でもあります．

　Ruby の挙動がもう少しおとなしければ，制御フレームをうまくたどって，み
たいな実装も考えられると思うのですが，なんとか eval とか，define_method
とかやっちゃうやんちゃな人なので，それもうまくいきません（何度か考えてみ
たんだけど）．

　で，結局メソッド定義時にきちんとその辺の情報を再帰的にたどって整備する
のが早道かなぁ，という結論になりました．ので，そう実装しようと思っています．

# クラス定義時にはどうなるんだろう．
# defineclass 命令とかで情報の伝搬が走るのかな．

　1.9.3 では，この辺の問題が綺麗に整理されているといいなぁ．

-- 
// SASADA Koichi at atdot dot net

• Status changed from Open to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r29063.
Yusuke, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.