Skip to content

Commit

Permalink
Fix Enumerator#with_index for GC compaction
Browse files Browse the repository at this point in the history
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
  • Loading branch information
peterzhu2118 committed Dec 24, 2023
1 parent 86ef381 commit 5af64ff
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 4 deletions.
14 changes: 10 additions & 4 deletions enumerator.c
Expand Up @@ -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;
}

/*
Expand Down
10 changes: 10 additions & 0 deletions test/ruby/test_enumerator.rb
Expand Up @@ -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)
Expand Down

0 comments on commit 5af64ff

Please sign in to comment.