From f376163194686079452ddfd0af61ab505172c07c Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Mon, 20 Nov 2023 14:55:50 -0500 Subject: [PATCH] Fix crash when evacuating generic ivar When transitioning generic instance variable objects to too complex, we set the shape first before performing inserting the new gen_ivtbl. The st_insert for the new gen_ivtbl could allocate and cause a GC. If that happens, then it will crash because the object will have a too complex shape but not yet be backed by a st_table. This commit changes the order so that the insert happens first before the new shape is set. The following script reproduces the issue: ``` o = [] o.instance_variable_set(:@a, 1) i = 0 o = Object.new while RubyVM::Shape.shapes_available > 0 o.instance_variable_set(:"@i#{i}", 1) i += 1 end ary = 1_000.times.map { [] } GC.stress = true ary.each do |o| o.instance_variable_set(:@a, 1) o.instance_variable_set(:@b, 1) end ``` --- test/ruby/test_shapes.rb | 22 ++++++++++++++++++++++ variable.c | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/test/ruby/test_shapes.rb b/test/ruby/test_shapes.rb index bcb6791a05..6efcbbf101 100644 --- a/test/ruby/test_shapes.rb +++ b/test/ruby/test_shapes.rb @@ -277,6 +277,28 @@ class TestShapes < Test::Unit::TestCase end; end + def test_gc_stress_during_evacuate_generic_ivar + assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}") + begin; + [].instance_variable_set(:@a, 1) + + i = 0 + o = Object.new + while RubyVM::Shape.shapes_available > 0 + o.instance_variable_set(:"@i#{i}", 1) + i += 1 + end + + ary = 10.times.map { [] } + + GC.stress = true + ary.each do |o| + o.instance_variable_set(:@a, 1) + o.instance_variable_set(:@b, 1) + end + end; + end + def test_run_out_of_shape_for_module_ivar assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}") begin; diff --git a/variable.c b/variable.c index 3d70d7b808..511d5c7d54 100644 --- a/variable.c +++ b/variable.c @@ -1399,12 +1399,12 @@ rb_obj_convert_to_too_complex(VALUE obj, st_table *table) struct gen_ivtbl *ivtbl = xmalloc(sizeof(struct gen_ivtbl)); ivtbl->as.complex.table = table; + st_insert(gen_ivs, (st_data_t)obj, (st_data_t)ivtbl); #if SHAPE_IN_BASIC_FLAGS rb_shape_set_shape_id(obj, OBJ_TOO_COMPLEX_SHAPE_ID); #else ivtbl->shape_id = OBJ_TOO_COMPLEX_SHAPE_ID; #endif - st_insert(gen_ivs, (st_data_t)obj, (st_data_t)ivtbl); } RB_VM_LOCK_LEAVE(); }