Project

General

Profile

Bug #14847

`clone` can generate strange objects

Added by ko1 (Koichi Sasada) 4 months ago. Updated 4 months ago.

Status:
Closed
Priority:
Normal
Target version:
[ruby-dev:50575]

Description

Object#clone(obj) を呼び出すと、

  • (1) rb_obj_alloc(rb_obj_class(obj)); で空の obj を作り、
  • (2) RBasic(orig)->flags を(できるだけ)引き継ぎ
  • (3) 特異クラス情報を引き継ぎ
  • (4) インスタンス変数を引き継ぎ
  • (5) taint 情報を引き継ぎ
  • (6) initialize_copy を呼んで初期化

という感じで処理をするのですが、(2) の箇所で FL_USER* がそのまま引き継がれます。

そして、initialize_copy が呼ばれたとき、適切に初期化をしないと、FL_USER* がおかしなことになります。

下記はどれも遠藤さんが見つけてくれた例です。

class Array; def initialize_copy(*); end; end # 何もしない
x = [1,2,3].clone; p x #=> [false, false, false]

EMBED_LEN だけがコピーされ、配列の中身はコピーされない(0 初期化のまま)なので false が充填された配列になる。

class Array; def initialize_copy(*); end; end
x = [1,2,3,4,5,6,7][1..-2].clone
x.push(1,1,1,1,1)
#=> [BUG] Segmentation fault

ELTS_SHARED がコピーされるが、shared root への参照がコピーされないので SEGV する

class Hash; def initialize_copy(*); end; end
h = {}; h.default_proc = proc { }
h = h.clone
h[1] #=> undefined method `default_proc=' for {}:Hash (NoMethodError)

HASH_PROC_DEFAULT (== FL_USER2) だけがコピーされ、default_proc は nil のまま。

initialize_copy が責務を果たしていない、というのはその通りですが、そもそも FL_USER* をここでコピーするのは必要なんでしょうか。
initialize_copy 側で、FL_USER* を適切に設定するべきではないでしょうか。

少なくとも、SEGV は回避しなければいけないかと思います。

歴史的経緯を知らないので、要るんだって話かもしれませんが、とりあえず現状はなんかやばそうな気がします。

Associated revisions

Revision 38e05ff3
Added by ko1 (Koichi Sasada) 4 months ago

Don't copy FL_USER* on Kernel#clone. [Bug #14847]

  • object.c (mutable_obj_clone): Kernel#clone should not copy FL_USER* flags because they are copied unexpectedly. Unexpected copy will break internal data consistency.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@63912 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 63912
Added by ko1 (Koichi Sasada) 4 months ago

Don't copy FL_USER* on Kernel#clone. [Bug #14847]

  • object.c (mutable_obj_clone): Kernel#clone should not copy FL_USER* flags because they are copied unexpectedly. Unexpected copy will break internal data consistency.

History

#1 [ruby-dev:50576] Updated by matz (Yukihiro Matsumoto) 4 months ago

まつもと ゆきひろです

おそらくは FL_FREEZE などいくつかのフラグだけ特別にコピーするべきなんだと思います。
最近 mruby でも同様のレポートが来て、あちらでは FREEZE だけコピーすることにしました。
CRuby ではもうちょっとコピーする必要がありそうです。

#2 Updated by ko1 (Koichi Sasada) 4 months ago

  • Status changed from Open to Closed

Applied in changeset trunk|r63912.


Don't copy FL_USER* on Kernel#clone. [Bug #14847]

  • object.c (mutable_obj_clone): Kernel#clone should not copy FL_USER* flags because they are copied unexpectedly. Unexpected copy will break internal data consistency.

Also available in: Atom PDF