Skip to content

Commit 81b7fd6

Browse files
byrootbyroot
committed
Fix rb_copy_generic_ivar to handle object_id in shapes
1 parent 9dbe473 commit 81b7fd6

4 files changed

Lines changed: 248 additions & 27 deletions

File tree

‎gc.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1835,14 +1835,17 @@ object_id(VALUE obj)
18351835

18361836
if (rb_shape_too_complex_p(shape)) {
18371837
st_table *table = ROBJECT_IV_HASH(obj);
1838-
if (st_lookup(table, (st_data_t)internal_object_id, (st_data_t *)&id)) {
1838+
if (rb_shape_has_object_id(shape)) {
1839+
st_lookup(table, (st_data_t)internal_object_id, (st_data_t *)&id);
18391840
rb_gc_vm_unlock(lock_lev);
18401841
return id;
18411842
}
18421843

18431844
id = ULL2NUM(next_object_id);
18441845
next_object_id += OBJ_ID_INCREMENT;
1846+
rb_shape_t *object_id_shape = rb_shape_object_id_shape(obj);
18451847
st_insert(table, (st_data_t)internal_object_id, (st_data_t)id);
1848+
rb_shape_set_shape(obj, object_id_shape);
18461849
if (RB_UNLIKELY(id_to_obj_tbl)) {
18471850
st_insert(id_to_obj_tbl, (st_data_t)id, (st_data_t)obj);
18481851
}

‎shape.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1072,7 +1072,7 @@ rb_shape_rebuild_shape(rb_shape_t *initial_shape, rb_shape_t *dest_shape)
10721072

10731073
rb_shape_t *midway_shape;
10741074

1075-
RUBY_ASSERT(initial_shape->type == SHAPE_T_OBJECT);
1075+
RUBY_ASSERT(initial_shape->type == SHAPE_T_OBJECT || initial_shape->type == SHAPE_ROOT);
10761076

10771077
if (dest_shape->type != initial_shape->type) {
10781078
midway_shape = rb_shape_rebuild_shape(initial_shape, rb_shape_get_parent(dest_shape));

‎test/ruby/test_object_id.rb‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
require 'test/unit'
2+
3+
class TestObjectId < Test::Unit::TestCase
4+
def setup
5+
@obj = Object.new
6+
end
7+
8+
def test_dup_new_id
9+
id = @obj.object_id
10+
refute_equal id, @obj.dup.object_id
11+
end
12+
13+
def test_dup_with_ivar_and_id
14+
id = @obj.object_id
15+
@obj.instance_variable_set(:@foo, 42)
16+
17+
copy = @obj.dup
18+
refute_equal id, copy.object_id
19+
assert_equal 42, copy.instance_variable_get(:@foo)
20+
end
21+
22+
def test_dup_with_id_and_ivar
23+
@obj.instance_variable_set(:@foo, 42)
24+
id = @obj.object_id
25+
26+
copy = @obj.dup
27+
refute_equal id, copy.object_id
28+
assert_equal 42, copy.instance_variable_get(:@foo)
29+
end
30+
31+
def test_dup_with_id_and_ivar_and_frozen
32+
@obj.instance_variable_set(:@foo, 42)
33+
@obj.freeze
34+
id = @obj.object_id
35+
36+
copy = @obj.dup
37+
refute_equal id, copy.object_id
38+
assert_equal 42, copy.instance_variable_get(:@foo)
39+
refute_predicate copy, :frozen?
40+
end
41+
42+
def test_clone_new_id
43+
id = @obj.object_id
44+
refute_equal id, @obj.clone.object_id
45+
end
46+
47+
def test_clone_with_ivar_and_id
48+
id = @obj.object_id
49+
@obj.instance_variable_set(:@foo, 42)
50+
51+
copy = @obj.clone
52+
refute_equal id, copy.object_id
53+
assert_equal 42, copy.instance_variable_get(:@foo)
54+
end
55+
56+
def test_clone_with_id_and_ivar
57+
@obj.instance_variable_set(:@foo, 42)
58+
id = @obj.object_id
59+
60+
copy = @obj.clone
61+
refute_equal id, copy.object_id
62+
assert_equal 42, copy.instance_variable_get(:@foo)
63+
end
64+
65+
def test_clone_with_id_and_ivar_and_frozen
66+
@obj.instance_variable_set(:@foo, 42)
67+
@obj.freeze
68+
id = @obj.object_id
69+
70+
copy = @obj.clone
71+
refute_equal id, copy.object_id
72+
assert_equal 42, copy.instance_variable_get(:@foo)
73+
assert_predicate copy, :frozen?
74+
end
75+
76+
def test_marshal_new_id
77+
return pass if @obj.is_a?(Module)
78+
79+
id = @obj.object_id
80+
refute_equal id, Marshal.load(Marshal.dump(@obj)).object_id
81+
end
82+
83+
def test_marshal_with_ivar_and_id
84+
return pass if @obj.is_a?(Module)
85+
86+
id = @obj.object_id
87+
@obj.instance_variable_set(:@foo, 42)
88+
89+
copy = Marshal.load(Marshal.dump(@obj))
90+
refute_equal id, copy.object_id
91+
assert_equal 42, copy.instance_variable_get(:@foo)
92+
end
93+
94+
def test_marshal_with_id_and_ivar
95+
return pass if @obj.is_a?(Module)
96+
97+
@obj.instance_variable_set(:@foo, 42)
98+
id = @obj.object_id
99+
100+
copy = Marshal.load(Marshal.dump(@obj))
101+
refute_equal id, copy.object_id
102+
assert_equal 42, copy.instance_variable_get(:@foo)
103+
end
104+
105+
def test_marshal_with_id_and_ivar_and_frozen
106+
return pass if @obj.is_a?(Module)
107+
108+
@obj.instance_variable_set(:@foo, 42)
109+
@obj.freeze
110+
id = @obj.object_id
111+
112+
copy = Marshal.load(Marshal.dump(@obj))
113+
refute_equal id, copy.object_id
114+
assert_equal 42, copy.instance_variable_get(:@foo)
115+
refute_predicate copy, :frozen?
116+
end
117+
end
118+
119+
class TestObjectIdClass < TestObjectId
120+
def setup
121+
@obj = Class.new
122+
end
123+
end
124+
125+
class TestObjectIdGeneric < TestObjectId
126+
def setup
127+
@obj = Array.new
128+
end
129+
end
130+
131+
class TestObjectIdTooComplex < TestObjectId
132+
class TooComplex
133+
end
134+
135+
def setup
136+
if defined?(RubyVM::Shape::SHAPE_MAX_VARIATIONS)
137+
assert_equal 8, RubyVM::Shape::SHAPE_MAX_VARIATIONS
138+
end
139+
8.times do |i|
140+
TooComplex.new.instance_variable_set("@a#{i}", 1)
141+
end
142+
@obj = TooComplex.new
143+
@obj.instance_variable_set(:@test, 1)
144+
end
145+
end
146+
147+
class TestObjectIdTooComplexClass < TestObjectId
148+
class TooComplex < Module
149+
end
150+
151+
def setup
152+
if defined?(RubyVM::Shape::SHAPE_MAX_VARIATIONS)
153+
assert_equal 8, RubyVM::Shape::SHAPE_MAX_VARIATIONS
154+
end
155+
8.times do |i|
156+
TooComplex.new.instance_variable_set("@a#{i}", 1)
157+
end
158+
@obj = TooComplex.new
159+
@obj.instance_variable_set(:@test, 1)
160+
end
161+
end
162+
163+
class TestObjectIdTooComplexGeneric < TestObjectId
164+
class TooComplex < Array
165+
end
166+
167+
def setup
168+
if defined?(RubyVM::Shape::SHAPE_MAX_VARIATIONS)
169+
assert_equal 8, RubyVM::Shape::SHAPE_MAX_VARIATIONS
170+
end
171+
8.times do |i|
172+
TooComplex.new.instance_variable_set("@a#{i}", 1)
173+
end
174+
@obj = TooComplex.new
175+
@obj.instance_variable_set(:@test, 1)
176+
end
177+
end

‎variable.c‎

Lines changed: 66 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2276,35 +2276,83 @@ class_fields_each(VALUE obj, rb_ivar_foreach_callback_func *func, st_data_t arg,
22762276
}
22772277

22782278
void
2279-
rb_copy_generic_ivar(VALUE clone, VALUE obj)
2279+
rb_copy_generic_ivar(VALUE dest, VALUE obj)
22802280
{
22812281
struct gen_ivtbl *obj_ivtbl;
22822282
struct gen_ivtbl *new_ivtbl;
22832283

2284-
rb_check_frozen(clone);
2284+
rb_check_frozen(dest);
22852285

22862286
if (!FL_TEST(obj, FL_EXIVAR)) {
22872287
goto clear;
22882288
}
22892289

2290+
unsigned long src_num_ivs = rb_ivar_count(obj);
2291+
if (!src_num_ivs) {
2292+
goto clear;
2293+
}
2294+
2295+
rb_shape_t *src_shape = rb_shape_get_shape(obj);
2296+
22902297
if (rb_gen_ivtbl_get(obj, 0, &obj_ivtbl)) {
22912298
if (gen_ivtbl_count(obj, obj_ivtbl) == 0)
22922299
goto clear;
22932300

2294-
FL_SET(clone, FL_EXIVAR);
2301+
FL_SET(dest, FL_EXIVAR);
22952302

2296-
if (rb_shape_obj_too_complex(obj)) {
2297-
new_ivtbl = xmalloc(sizeof(struct gen_ivtbl));
2298-
#if !SHAPE_IN_BASIC_FLAGS
2299-
new_ivtbl->shape_id = SHAPE_OBJ_TOO_COMPLEX;
2300-
#endif
2301-
new_ivtbl->as.complex.table = st_copy(obj_ivtbl->as.complex.table);
2303+
if (rb_shape_too_complex_p(src_shape)) {
2304+
// obj is TOO_COMPLEX so we can copy its iv_hash
2305+
st_table *table = st_copy(ROBJECT_IV_HASH(obj));
2306+
if (rb_shape_has_object_id(src_shape)) {
2307+
st_data_t id = (st_data_t)internal_object_id;
2308+
st_delete(table, &id, NULL);
2309+
}
2310+
rb_obj_init_too_complex(dest, table);
2311+
2312+
return;
2313+
}
2314+
2315+
rb_shape_t *shape_to_set_on_dest = src_shape;
2316+
rb_shape_t *initial_shape = rb_shape_get_shape(dest);
2317+
2318+
if (initial_shape->heap_index != src_shape->heap_index || !rb_shape_canonical_p(src_shape)) {
2319+
RUBY_ASSERT(initial_shape->type == SHAPE_ROOT);
2320+
2321+
shape_to_set_on_dest = rb_shape_rebuild_shape(initial_shape, src_shape);
2322+
if (UNLIKELY(rb_shape_too_complex_p(shape_to_set_on_dest))) {
2323+
st_table *table = rb_st_init_numtable_with_size(src_num_ivs);
2324+
rb_obj_copy_ivs_to_hash_table(obj, table);
2325+
rb_obj_init_too_complex(dest, table);
2326+
2327+
return;
2328+
}
2329+
}
2330+
2331+
new_ivtbl = gen_ivtbl_resize(0, shape_to_set_on_dest->next_field_index);
2332+
2333+
VALUE *src_buf = obj_ivtbl->as.shape.ivptr;
2334+
VALUE *dest_buf = new_ivtbl->as.shape.ivptr;
2335+
2336+
if (src_shape->next_field_index == shape_to_set_on_dest->next_field_index) {
2337+
// Happy path, we can just memcpy the ivptr content
2338+
MEMCPY(dest_buf, src_buf, VALUE, src_num_ivs);
2339+
2340+
// Fire write barriers
2341+
for (uint32_t i = 0; i < src_num_ivs; i++) {
2342+
RB_OBJ_WRITTEN(dest, Qundef, dest_buf[i]);
2343+
}
23022344
}
23032345
else {
2304-
new_ivtbl = gen_ivtbl_resize(0, obj_ivtbl->as.shape.numiv);
2346+
rb_shape_t *dest_shape = shape_to_set_on_dest;
2347+
while (src_shape->parent_id != INVALID_SHAPE_ID) {
2348+
if (src_shape->type == SHAPE_IVAR) {
2349+
while (dest_shape->edge_name != src_shape->edge_name) {
2350+
dest_shape = rb_shape_get_shape_by_id(dest_shape->parent_id);
2351+
}
23052352

2306-
for (uint32_t i=0; i<obj_ivtbl->as.shape.numiv; i++) {
2307-
RB_OBJ_WRITE(clone, &new_ivtbl->as.shape.ivptr[i], obj_ivtbl->as.shape.ivptr[i]);
2353+
RB_OBJ_WRITE(dest, &dest_buf[dest_shape->next_field_index - 1], src_buf[src_shape->next_field_index - 1]);
2354+
}
2355+
src_shape = rb_shape_get_shape_by_id(src_shape->parent_id);
23082356
}
23092357
}
23102358

@@ -2314,25 +2362,18 @@ rb_copy_generic_ivar(VALUE clone, VALUE obj)
23142362
*/
23152363
RB_VM_LOCK_ENTER();
23162364
{
2317-
generic_ivtbl_no_ractor_check(clone);
2318-
st_insert(generic_ivtbl_no_ractor_check(obj), (st_data_t)clone, (st_data_t)new_ivtbl);
2365+
generic_ivtbl_no_ractor_check(dest);
2366+
st_insert(generic_ivtbl_no_ractor_check(obj), (st_data_t)dest, (st_data_t)new_ivtbl);
23192367
}
23202368
RB_VM_LOCK_LEAVE();
2321-
2322-
rb_shape_t * obj_shape = rb_shape_get_shape(obj);
2323-
if (rb_shape_frozen_shape_p(obj_shape)) {
2324-
rb_shape_set_shape_id(clone, obj_shape->parent_id);
2325-
}
2326-
else {
2327-
rb_shape_set_shape(clone, obj_shape);
2328-
}
2369+
rb_shape_set_shape(dest, shape_to_set_on_dest);
23292370
}
23302371
return;
23312372

23322373
clear:
2333-
if (FL_TEST(clone, FL_EXIVAR)) {
2334-
rb_free_generic_ivar(clone);
2335-
FL_UNSET(clone, FL_EXIVAR);
2374+
if (FL_TEST(dest, FL_EXIVAR)) {
2375+
rb_free_generic_ivar(dest);
2376+
FL_UNSET(dest, FL_EXIVAR);
23362377
}
23372378
}
23382379

0 commit comments

Comments
 (0)