[ruby-dev:44461] [Ruby 1.9 - Bug #5274][Assigned] [ruby-dev:44460] Class の clone と Class の生成を組み合わせたときのバグ

Issue #5274 has been reported by Kenta Murata.

----------------------------------------
Bug #5274: [ruby-dev:44460] Class の clone と Class の生成を組み合わせたときのバグ
http://redmine.ruby-lang.org/issues/5274

Author: Kenta Murata
Status: Assigned
Priority: High
Assignee: Yukihiro Matsumoto
Category: core
Target version: 1.9.3
ruby -v: ruby 1.9.3dev (2011-09-04 revision 33179)


[ruby-dev:44460] をチケット化します。
----

芝と申します。

次のサンプルコードを実行すると segv を吐きます。
確認した Ruby 処理系は ruby 1.9.3dev (2011-09-04 revision 33179)
[i686-linux] です。

#サンプルコード(#
StrClone = String.clone
Class.new(StrClone)
Marshal.dump(StrClone.new("a"))



segv を吐く原因は、StrClone.new("a") で壊れたオブジェクト(インスタンス変
数の数がえらいことになってるオブジェクト)を生成しているためです。
Class.new(StrClone) を実行した時点で StrClone の特異クラス(以降
<StrClone> と書きます)が変更され、メソッドの検索結果が変わってしまってい
ます。このため、StrClone インスタンスのアロケータが T_STRING のものか
ら、T_OBJECT のものとなり、StrClone.new で T_OBJECT のオブジェクトが割り
当てられます。

しかし、StrClone のメソッドテーブルは変更されていないため、割り当てたオ
ブジェクトに対しては文字列のイニシャライザが呼び出されます。このせいで、
T_OBJECT のオブジェクトに対して文字列の初期化処理が走ってしまい、壊れた
オブジェクトが作られます。サンプルコードでは、この、壊れたオブジェクトを
Marshal.dump で吐こうとしたときに、インスタンス変数のダンプをしようとし
て、segv を吐きます。

class オブジェクトの clone や、特異クラスの処理は複雑なので理解しきれて
いませんが、今回の場合、StrClone の特異クラスが変更されてしまうのが問題
に見えます。StrClone の特異メソッドが変更されてしまうことの原因は、以下
のように、make_metaclass(class.c) にあります。



#define ENSURE_EIGENCLASS(klass) \
 (rb_ivar_get(METACLASS_OF(klass), id_attached) == (klass) ?
METACLASS_OF(klass) : make_metaclass(klass))

static inline VALUE
make_metaclass(VALUE klass)
{
   ....

   super = RCLASS_SUPER(klass);
   while (RB_TYPE_P(super, T_ICLASS)) super = RCLASS_SUPER(super);
   RCLASS_SUPER(metaclass) = super ? ENSURE_EIGENCLASS(super) : rb_cClass;

   ....

   return metaclass;
}



ここの ENSURE_EIGENCLASS というマクロは、klass が StrClone だった場合に
rb_ivar_get(METACLASS_OF(StrClone), id_attached) == (StrClone)
の判定が偽になり、make_metaclass(StrClone) が実行されます。
この make_metaclass(StrClone) で、StrClone の特異クラスが変更されます。

これが何故起こるかというと、<StrClone> の "__attached__" というインスタ
ンス変数が、StrClone を指していないためです。<StrClone> の
"__attached__" は、rb_singleton_class_clone(class.c)の以下の処理によっ
て、自分自身を指しています。この部分を少しかえて、<StrClone> の
"__attached__" が StrClone を指すようにすれば、StrClone の特異クラスが変
更されてしまうという点については解決できます。



VALUE
rb_singleton_class_clone(VALUE obj)
{
   VALUE klass = RBASIC(obj)->klass;

   ...

       VALUE clone = class_alloc((RBASIC(klass)->flags & ~(FL_MARK)), 0);

       if (BUILTIN_TYPE(obj) == T_CLASS) {
           RBASIC(clone)->klass = (VALUE)clone; /* <= ココを通る(1) */
       }
       else {
           RBASIC(clone)->klass = rb_singleton_class_clone(klass);
       }

       rb_singleton_class_attached(RBASIC(clone)->klass, (VALUE)clone);
       /* (1) を通った場合、RBASIC(clone)->klass = clone */
       /* clone の "__attached__" は clone 自身を指す */

   ...
}



本メールの末尾に、rb_singleton_class_clone などの "__attached__" の処理
に対するパッチを添付します。特異クラスが常に "__attached__" に何かしらを
指している必要があるのなら、class.c に対するパッチは無視してください。

特異クラスの "__attached__" が自分自身を指していないとまずいケースがある
かどうかが分からないので、このパッチが有効かどうかは分かりませんが、再現
環境でサンプルコードを無事実行し、test-all にも通過することは確認できま
した。

参考にしていただければ幸いです。
よろしくお願いいたします。



# パッチ #
Index: object.c
===================================================================
--- object.c    (revision 33182)
+++ object.c    (working copy)
@@ -272,12 +272,17 @@
 rb_obj_clone(VALUE obj)
 {
    VALUE clone;
+    VALUE singleton;

    if (rb_special_const_p(obj)) {
        rb_raise(rb_eTypeError, "can't clone %s", rb_obj_classname(obj));
    }
    clone = rb_obj_alloc(rb_obj_class(obj));
-    RBASIC(clone)->klass = rb_singleton_class_clone(obj);
+    singleton = rb_singleton_class_clone(obj);
+    RBASIC(clone)->klass = singleton;
+    if (FL_TEST(singleton, FL_SINGLETON)) {
+      rb_singleton_class_attached(singleton, clone);
+    }
    RBASIC(clone)->flags = (RBASIC(obj)->flags | FL_TEST(clone,
FL_TAINT) | FL_TEST(clone, FL_UNTRUSTED)) &
~(FL_FREEZE|FL_FINALIZE|FL_MARK);
    init_copy(clone, obj);
    rb_funcall(clone, id_init_clone, 1, obj);
Index: class.c
===================================================================
--- class.c     (revision 33182)
+++ class.c     (working copy)
@@ -230,12 +230,15 @@
       struct clone_method_data data;
       /* copy singleton(unnamed) class */
       VALUE clone = class_alloc((RBASIC(klass)->flags & ~(FL_MARK)), 0);
+       int attach;

       if (BUILTIN_TYPE(obj) == T_CLASS) {
           RBASIC(clone)->klass = (VALUE)clone;
+           attach = 0;
       }
       else {
           RBASIC(clone)->klass = rb_singleton_class_clone(klass);
+           attach = 1;
       }

       RCLASS_SUPER(clone) = RCLASS_SUPER(klass);
@@ -251,7 +254,9 @@
       data.klass = (VALUE)clone;
       st_foreach(RCLASS_M_TBL(klass), clone_method,
                  (st_data_t)&data);
-       rb_singleton_class_attached(RBASIC(clone)->klass, (VALUE)clone);
+       if (attach) {
+         rb_singleton_class_attached(RBASIC(clone)->klass, (VALUE)clone);
+       }
       FL_SET(clone, FL_SINGLETON);
       return (VALUE)clone;
    }



-- 
http://redmine.ruby-lang.org