Fix Enumerator#with_index for GC compaction

peterzhu2118 · peterzhu2118 · commit 5af64ff7db3d · 2023-12-24T16:22:34.000-05:00
enumerator_block_call was not safe for compaction because the Array backing the argv was not pinned, so it could get moved during compaction which would make argv point to somewhere else. The test crashes when RGENGC_CHECK_MODE is turned on: TestEnumerator#test_with_index_under_gc_compact_stress check_rvalue_consistency: 0x55db0b399450 is not a Ruby object. test/ruby/test_enumerator.rb:133: [BUG] check_rvalue_consistency_force: there is 1 errors. ruby 3.3.0dev (2023-12-23T23:00:27Z master 50bf437) [x86_64-linux] -- Control frame information ----------------------------------------------- c:0034 p:---- s:0192 e:000187 CFUNC :with_index c:0033 p:---- s:0185 e:000184 CFUNC :each c:0032 p:---- s:0182 e:000181 CFUNC :to_a c:0031 p:0055 s:0178 e:000175 BLOCK test/ruby/test_enumerator.rb:133 c:0030 p:0024 s:0172 e:000171 METHOD tool/lib/envutil.rb:242 c:0029 p:0024 s:0167 e:000166 METHOD tool/lib/envutil.rb:251 c:0028 p:0005 s:0160 e:000159 METHOD test/ruby/test_enumerator.rb:131 ... -- C level backtrace information ------------------------------------------- build/ruby(rb_print_backtrace+0x14) [0x55db0b1deb21] vm_dump.c:820 build/ruby(rb_vm_bugreport) vm_dump.c:1151 build/ruby(bug_report_end+0x0) [0x55db0b3a53a6] error.c:1042 build/ruby(rb_bug_without_die) error.c:1042 build/ruby(die+0x0) [0x55db0afc77c2] error.c:1050 build/ruby(rb_bug) error.c:1052 build/ruby(gc_move+0x0) [0x55db0afbada0] gc.c:1714 build/ruby(check_rvalue_consistency+0xa) [0x55db0afef0c3] gc.c:1729 build/ruby(is_markable_object) gc.c:4769 build/ruby(gc_mark_stack_values) gc.c:6595 build/ruby(rb_gc_mark_vm_stack_values) gc.c:6605 build/ruby(rb_execution_context_mark+0x39) [0x55db0b1d8589] vm.c:3309 build/ruby(thread_mark+0x15) [0x55db0b1a9805] vm.c:3381 build/ruby(gc_mark_stacked_objects+0x6d) [0x55db0aff2c3d] gc.c:7564 build/ruby(gc_mark_stacked_objects_all) gc.c:7602 build/ruby(gc_marks_rest) gc.c:8797 build/ruby(gc_marks+0xd) [0x55db0aff43d5] gc.c:8855 build/ruby(gc_start) gc.c:9608 build/ruby(rb_multi_ractor_p+0x0) [0x55db0aff5463] gc.c:9489 build/ruby(rb_vm_lock_leave) vm_sync.h:92 build/ruby(garbage_collect) gc.c:9491 build/ruby(newobj_slowpath+0xcb) [0x55db0aff57ab] gc.c:2871 build/ruby(newobj_slowpath_wb_protected) gc.c:2895 build/ruby(newobj_of0+0x24) [0x55db0aff59e4] gc.c:2937 build/ruby(newobj_of) gc.c:2947 build/ruby(rb_wb_protected_newobj_of) gc.c:2962 build/ruby(ary_alloc_embed+0x10) [0x55db0b2f3e40] array.c:668 build/ruby(ary_new) array.c:709 build/ruby(rb_ary_tmp_new_from_values) array.c:759 build/ruby(rb_ary_new_from_values) array.c:771 build/ruby(args_copy+0x18) [0x55db0b1bbb88] vm_args.c:158
diff --git a/enumerator.c b/enumerator.c
@@ -559,11 +559,17 @@ enumerator_block_call(VALUE obj, rb_block_call_func *func, VALUE arg)
     const struct enumerator *e = enumerator_ptr(obj);
     ID meth = e->meth;
 
-    if (e->args) {
-        argc = RARRAY_LENINT(e->args);
-        argv = RARRAY_CONST_PTR(e->args);
+    VALUE args = e->args;
+    if (args) {
+        argc = RARRAY_LENINT(args);
+        argv = RARRAY_CONST_PTR(args);
     }
-    return rb_block_call_kw(e->obj, meth, argc, argv, func, arg, e->kw_splat);
+
+    VALUE ret = rb_block_call_kw(e->obj, meth, argc, argv, func, arg, e->kw_splat);
+
+    RB_GC_GUARD(args);
+
+    return ret;
 }
 
 /*
diff --git a/test/ruby/test_enumerator.rb b/test/ruby/test_enumerator.rb
@@ -127,6 +127,16 @@ def test_with_index
     assert_equal([[1,5],[2,6],[3,7]], @obj.to_enum(:foo, 1, 2, 3).with_index(5).to_a)
   end
 
+  def test_with_index_under_gc_compact_stress
+    EnvUtil.under_gc_compact_stress do
+      assert_equal([[1, 0], [2, 1], [3, 2]], @obj.to_enum(:foo, 1, 2, 3).with_index.to_a)
+      assert_equal([[1, 5], [2, 6], [3, 7]], @obj.to_enum(:foo, 1, 2, 3).with_index(5).to_a)
+
+      s = 1 << (8 * 1.size - 2)
+      assert_equal([[1, s], [2, s + 1], [3, s + 2]], @obj.to_enum(:foo, 1, 2, 3).with_index(s).to_a)
+    end
+  end
+
   def test_with_index_large_offset
     bug8010 = '[ruby-dev:47131] [Bug #8010]'
     s = 1 << (8*1.size-2)
