marshal.c: no overwriting ivars

nobu · nobu · commit 5e79c1a54313 · 2013-05-02T08:32:48.000Z
* marshal.c (copy_ivar_i): get rid of overwriting already copied instance variales. c.f. [Bug ruby#8276] git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@40559 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,8 @@
+Thu May  2 17:32:45 2013  Nobuyoshi Nakada  <nobu@ruby-lang.org>
+
+	* marshal.c (copy_ivar_i): get rid of overwriting already copied
+	  instance variales.  c.f. [Bug #8276]
+
 Thu May  2 16:55:43 2013  Nobuyoshi Nakada  <nobu@ruby-lang.org>
 
 	* thread.c (id_locals): use cached ID.
diff --git a/marshal.c b/marshal.c
@@ -1373,7 +1373,11 @@ r_leave(VALUE v, struct load_arg *arg)
 static int
 copy_ivar_i(st_data_t key, st_data_t val, st_data_t arg)
 {
-    rb_ivar_set((VALUE)arg, (ID)key, (VALUE)val);
+    VALUE obj = (VALUE)arg, value = (VALUE)val;
+    ID vid = (ID)key;
+
+    if (!rb_ivar_defined(obj, vid))
+	rb_ivar_set(obj, vid, value);
     return ST_CONTINUE;
 }
 
diff --git a/test/ruby/test_marshal.rb b/test/ruby/test_marshal.rb
@@ -555,6 +555,16 @@ def initialize(*)
     alias marshal_load initialize
   end
 
+  class FrozenData < LoadData
+    def marshal_load(data)
+      super
+      data.instance_variables.each do |iv|
+        instance_variable_set(iv, data.instance_variable_get(iv))
+      end
+      freeze
+    end
+  end
+
   def test_marshal_dump_excess_encoding
     bug8276 = '[ruby-core:54334] [Bug #8276]'
     t = Bug8276.new(bug8276)
@@ -574,18 +584,20 @@ def test_marshal_dump_ivar
   def test_marshal_load_ivar
     s = "data with ivar"
     s.instance_variable_set(:@t, 42)
-    t = LoadData.new(s)
-    s = Marshal.dump(t)
     hook = ->(v) {
       if LoadData === v
-        assert_send([v, :instance_variable_defined?, :@t])
-        assert_equal(42, v.instance_variable_get(:@t))
+        assert_send([v, :instance_variable_defined?, :@t], v.class.name)
+        assert_equal(42, v.instance_variable_get(:@t), v.class.name)
       end
       v
     }
-    v = Marshal.load(s, hook)
-    assert_send([v, :instance_variable_defined?, :@t])
-    assert_equal(42, v.instance_variable_get(:@t))
+    [LoadData, FrozenData].each do |klass|
+      t = klass.new(s)
+      d = Marshal.dump(t)
+      v = assert_nothing_raised(RuntimeError) {break Marshal.load(d, hook)}
+      assert_send([v, :instance_variable_defined?, :@t], klass.name)
+      assert_equal(42, v.instance_variable_get(:@t), klass.name)
+    end
   end
 
   def test_class_ivar

Original file line number	Diff line number	Diff line change
`@@ -1373,7 +1373,11 @@ r_leave(VALUE v, struct load_arg *arg)`
`1373`	`1373`	`static int`
`1374`	`1374`	`copy_ivar_i(st_data_t key, st_data_t val, st_data_t arg)`
`1375`	`1375`	`{`
`1376`		`- rb_ivar_set((VALUE)arg, (ID)key, (VALUE)val);`
	`1376`	`+ VALUE obj = (VALUE)arg, value = (VALUE)val;`
	`1377`	`+ ID vid = (ID)key;`
	`1378`	`+`
	`1379`	`+ if (!rb_ivar_defined(obj, vid))`
	`1380`	`+ rb_ivar_set(obj, vid, value);`
`1377`	`1381`	`return ST_CONTINUE;`
`1378`	`1382`	`}`
`1379`	`1383`