Bug #3136: reuse of singleton method definition causes SEGV

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: 2.0.0
• ruby -v: ruby 1.9.2dev (2010-04-12 trunk 27317) [i686-linux]

[mame (Yusuke Endoh)]

=begin
遠藤です。

以下で 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

一連の流れを説明すると:

• (1) によって、reverse の iseq が "123" のメソッドとなる。
  (reverse の iseq の klass に「String を継承した特異クラス」
  がセットされる)

• (2) によって、reverse の iseq が [1,2,3] のメソッドとなる
  (reverse の iseq の klass に「Array を継承した特異クラス」
  が 上書き される)

• (3) によって、String のインスタンスを self として reverse
  の iseq が実行される。その中の super で現在実行中の iseq
  の klass (= Array を継承した特異クラス) を元に呼び出す
  メソッドを決める。すると Array#reverse が選ばれてしまう。

• String のインスタンスを self として rb_ary_reverse が実行
  されてしまう。

iseq の klass に super のコンテキスト情報を持たせている設計が
バグだと思いました。

解決方法としては、

1. rb_iseq_t 以外で super のコンテキスト情報を管理する
   (Ruby レベルのメソッドでも rb_method_entry_t の klass を
   使うようにする？)

2. メソッドを定義するたびに rb_iseq_t を複製する

を思いつきましたが、どちらも結構タフそうな変更です。

安定させるのに時間がかかることが予想されるため、がんばって
早く直してください > ささださん。

また、[ruby-dev:40458] の修正も急ぎ目でお願いします。

--
Yusuke Endoh [e-mail]
=end

[mame (Yusuke Endoh)]

• Assignee set to ko1 (Koichi Sasada)

=begin

=end

[mame (Yusuke Endoh)]

=begin
遠藤です。

以下でも 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]
=end

[mame (Yusuke Endoh)]

=begin
遠藤です。

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

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

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

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

--
Yusuke Endoh [e-mail]
=end

[mame (Yusuke Endoh)]

=begin
遠藤です。

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]
=end

[mame (Yusuke Endoh)]

• Target version changed from 1.9.2 to 2.0.0

=begin

=end

[wanabe (_ wanabe)]

=begin
ワナベと申します。
解決方法 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");
  }
  =end

[ko1 (Koichi Sasada)]

=begin
　ささだです．

(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

=end

[wanabe (_ wanabe)]

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

=begin
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.

=end