Skip to content

Conversation

@chrisseaton
Copy link
Collaborator

No description provided.

Copy link

@wks wks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wks wks merged commit fa5a095 into mmtk:mmtk Aug 4, 2022
chrisseaton pushed a commit that referenced this pull request Nov 6, 2022
Always look up instance variable buffers when iterating.  It is possible
for the instance variable buffer to change out from under the object
during iteration, so we cannot cache the buffer on the stack.

In the case of Bug #19095, the transient heap moved the buffer during
iteration:

```
Watchpoint 1 hit:
old value: 0x0000000107c00df8
new value: 0x00000001032743c0
Process 31720 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = watchpoint 1
    frame #0: 0x00000001006e5178 miniruby`rb_obj_transient_heap_evacuate(obj=0x000000010d6b94b0, promote=1) at variable.c:1361:5
   1358	        }
   1359	        MEMCPY(new_ptr, old_ptr, VALUE, len);
   1360	        ROBJECT(obj)->as.heap.ivptr = new_ptr;
-> 1361	    }
   1362	}
   1363	#endif
   1364
miniruby`rb_obj_transient_heap_evacuate:
->  0x1006e5178 <+328>: b      0x1006e517c               ; <+332> at variable.c:1362:1
    0x1006e517c <+332>: ldp    x29, x30, [sp, #0x50]
    0x1006e5180 <+336>: add    sp, sp, #0x60
    0x1006e5184 <+340>: ret
Target 0: (miniruby) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = watchpoint 1
  * frame #0: 0x00000001006e5178 miniruby`rb_obj_transient_heap_evacuate(obj=0x000000010d6b94b0, promote=1) at variable.c:1361:5
    frame #1: 0x00000001006cb150 miniruby`transient_heap_block_evacuate(theap=0x0000000100b196c0, block=0x0000000107c00000) at transient_heap.c:734:17
    frame #2: 0x00000001006c854c miniruby`transient_heap_evacuate(dmy=0x0000000000000000) at transient_heap.c:808:17
    frame #3: 0x00000001007fe6c0 miniruby`rb_postponed_job_flush(vm=0x0000000104402900) at vm_trace.c:1773:21
    frame #4: 0x0000000100637a84 miniruby`rb_threadptr_execute_interrupts(th=0x0000000103803bc0, blocking_timing=0) at thread.c:2316:13
    frame #5: 0x000000010078b730 miniruby`rb_vm_check_ints(ec=0x00000001048038d0) at vm_core.h:2025:9
    frame #6: 0x00000001006fbd10 miniruby`vm_pop_frame(ec=0x00000001048038d0, cfp=0x0000000104a04440, ep=0x0000000104904a28) at vm_insnhelper.c:422:5
    frame #7: 0x00000001006fbca0 miniruby`rb_vm_pop_frame(ec=0x00000001048038d0) at vm_insnhelper.c:431:5
    frame #8: 0x00000001007d6420 miniruby`vm_call0_cfunc_with_frame(ec=0x00000001048038d0, calling=0x000000016fdcc6a0, argv=0x0000000000000000) at vm_eval.c:153:9
    frame #9: 0x00000001007d44cc miniruby`vm_call0_cfunc(ec=0x00000001048038d0, calling=0x000000016fdcc6a0, argv=0x0000000000000000) at vm_eval.c:164:12
    frame #10: 0x0000000100766e80 miniruby`vm_call0_body(ec=0x00000001048038d0, calling=0x000000016fdcc6a0, argv=0x0000000000000000) at vm_eval.c:210:15
    frame #11: 0x00000001007d76f0 miniruby`vm_call0_cc(ec=0x00000001048038d0, recv=0x000000010d6b49d8, id=2769, argc=0, argv=0x0000000000000000, cc=0x000000010d6b2e58, kw_splat=0) at vm_eval.c:87:12
    frame #12: 0x0000000100769e48 miniruby`rb_funcallv_scope(recv=0x000000010d6b49d8, mid=2769, argc=0, argv=0x0000000000000000, scope=CALL_FCALL) at vm_eval.c:1051:16
    frame #13: 0x0000000100760a54 miniruby`rb_funcallv(recv=0x000000010d6b49d8, mid=2769, argc=0, argv=0x0000000000000000) at vm_eval.c:1066:12
    frame #14: 0x000000010037513c miniruby`rb_inspect(obj=0x000000010d6b49d8) at object.c:633:34
    frame #15: 0x000000010002c950 miniruby`inspect_ary(ary=0x000000010d6b4938, dummy=0x0000000000000000, recur=0) at array.c:3091:13
    frame #16: 0x0000000100642020 miniruby`exec_recursive(func=(miniruby`inspect_ary at array.c:3084), obj=0x000000010d6b4938, pairid=0x0000000000000000, arg=0x0000000000000000, outer=0, mid=2769) at thread.c:5177:23
    frame #17: 0x00000001006412fc miniruby`rb_exec_recursive(func=(miniruby`inspect_ary at array.c:3084), obj=0x000000010d6b4938, arg=0x0000000000000000) at thread.c:5205:12
    frame #18: 0x00000001000127f0 miniruby`rb_ary_inspect(ary=0x000000010d6b4938) at array.c:3117:12
```

In general though, any calls back out to the interpreter could change
the IV buffer, so it's not safe to cache.

[Bug #19095]
wks pushed a commit that referenced this pull request Dec 5, 2024
[Bug #20921]

When we create a cache entry for a constant, the following sequence of
events could happen:

- vm_track_constant_cache is called to insert a constant cache.
- In vm_track_constant_cache, we first look up the ST table for the ID
  of the constant. Assume the ST table exists because another iseq also
  holds a cache entry for this ID.
- We then insert into this ST table with the iseq_inline_constant_cache.
- However, while inserting into this ST table, it allocates memory, which
  could trigger a GC. Assume that it does trigger a GC.
- The GC frees the one and only other iseq that holds a cache entry for
  this ID.
- In remove_from_constant_cache, it will appear that the ST table is now
  empty because there are no more iseq with cache entries for this ID, so
  we free the ST table.
- We complete GC and continue our st_insert. However, this ST table has
  been freed so we now have a use-after-free.

This issue is very hard to reproduce, because it requires that the GC runs
at a very specific time. However, we can make it show up by applying this
patch which runs GC right before the st_insert to mimic the st_insert
triggering a GC:

    diff --git a/vm_insnhelper.c b/vm_insnhelper.c
    index 3cb23f0..a93998136a 100644
    --- a/vm_insnhelper.c
    +++ b/vm_insnhelper.c
    @@ -6338,6 +6338,10 @@ vm_track_constant_cache(ID id, void *ic)
            rb_id_table_insert(const_cache, id, (VALUE)ics);
        }

    +    if (id == rb_intern("MyConstant")) rb_gc();
    +
        st_insert(ics, (st_data_t) ic, (st_data_t) Qtrue);
    }

And if we run this script:

    Object.const_set("MyConstant", "Hello!")

    my_proc = eval("-> { MyConstant }")
    my_proc.call

    my_proc = eval("-> { MyConstant }")
    my_proc.call

We can see that ASAN outputs a use-after-free error:

    ==36540==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000049528 at pc 0x000102f3ceac bp 0x00016d607a70 sp 0x00016d607a68
    READ of size 8 at 0x606000049528 thread T0
        #0 0x102f3cea8 in do_hash st.c:321
        #1 0x102f3ddd0 in rb_st_insert st.c:1132
        #2 0x103140700 in vm_track_constant_cache vm_insnhelper.c:6345
        #3 0x1030b91d8 in vm_ic_track_const_chain vm_insnhelper.c:6356
        #4 0x1030b8cf8 in rb_vm_opt_getconstant_path vm_insnhelper.c:6424
        #5 0x1030bc1e0 in vm_exec_core insns.def:263
        #6 0x1030b55fc in rb_vm_exec vm.c:2585
        #7 0x1030fe0ac in rb_iseq_eval_main vm.c:2851
        #8 0x102a82588 in rb_ec_exec_node eval.c:281
        #9 0x102a81fe0 in ruby_run_node eval.c:319
        #10 0x1027f3db4 in rb_main main.c:43
        #11 0x1027f3bd4 in main main.c:68
        #12 0x183900270  (<unknown module>)

    0x606000049528 is located 8 bytes inside of 56-byte region [0x606000049520,0x606000049558)
    freed by thread T0 here:
        #0 0x104174d40 in free+0x98 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x54d40)
        #1 0x102ada89c in rb_gc_impl_free default.c:8183
        #2 0x102ada7dc in ruby_sized_xfree gc.c:4507
        #3 0x102ac4d34 in ruby_xfree gc.c:4518
        #4 0x102f3cb34 in rb_st_free_table st.c:663
        #5 0x102bd52d8 in remove_from_constant_cache iseq.c:119
        #6 0x102bbe2cc in iseq_clear_ic_references iseq.c:153
        #7 0x102bbd2a0 in rb_iseq_free iseq.c:166
        #8 0x102b32ed0 in rb_imemo_free imemo.c:564
        #9 0x102ac4b44 in rb_gc_obj_free gc.c:1407
        #10 0x102af4290 in gc_sweep_plane default.c:3546
        #11 0x102af3bdc in gc_sweep_page default.c:3634
        #12 0x102aeb140 in gc_sweep_step default.c:3906
        #13 0x102aeadf0 in gc_sweep_rest default.c:3978
        #14 0x102ae4714 in gc_sweep default.c:4155
        #15 0x102af8474 in gc_start default.c:6484
        #16 0x102afbe30 in garbage_collect default.c:6363
        #17 0x102ad37f0 in rb_gc_impl_start default.c:6816
        #18 0x102ad3634 in rb_gc gc.c:3624
        #19 0x1031406ec in vm_track_constant_cache vm_insnhelper.c:6342
        #20 0x1030b91d8 in vm_ic_track_const_chain vm_insnhelper.c:6356
        #21 0x1030b8cf8 in rb_vm_opt_getconstant_path vm_insnhelper.c:6424
        #22 0x1030bc1e0 in vm_exec_core insns.def:263
        #23 0x1030b55fc in rb_vm_exec vm.c:2585
        #24 0x1030fe0ac in rb_iseq_eval_main vm.c:2851
        #25 0x102a82588 in rb_ec_exec_node eval.c:281
        #26 0x102a81fe0 in ruby_run_node eval.c:319
        #27 0x1027f3db4 in rb_main main.c:43
        #28 0x1027f3bd4 in main main.c:68
        #29 0x183900270  (<unknown module>)

    previously allocated by thread T0 here:
        #0 0x104174c04 in malloc+0x94 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x54c04)
        #1 0x102ada0ec in rb_gc_impl_malloc default.c:8198
        #2 0x102acee44 in ruby_xmalloc gc.c:4438
        #3 0x102f3c85c in rb_st_init_table_with_size st.c:571
        #4 0x102f3c900 in rb_st_init_table st.c:600
        #5 0x102f3c920 in rb_st_init_numtable st.c:608
        #6 0x103140698 in vm_track_constant_cache vm_insnhelper.c:6337
        #7 0x1030b91d8 in vm_ic_track_const_chain vm_insnhelper.c:6356
        #8 0x1030b8cf8 in rb_vm_opt_getconstant_path vm_insnhelper.c:6424
        #9 0x1030bc1e0 in vm_exec_core insns.def:263
        #10 0x1030b55fc in rb_vm_exec vm.c:2585
        #11 0x1030fe0ac in rb_iseq_eval_main vm.c:2851
        #12 0x102a82588 in rb_ec_exec_node eval.c:281
        #13 0x102a81fe0 in ruby_run_node eval.c:319
        #14 0x1027f3db4 in rb_main main.c:43
        #15 0x1027f3bd4 in main main.c:68
        #16 0x183900270  (<unknown module>)

This commit fixes this bug by adding a inserting_constant_cache_id field
to the VM, which stores the ID that is currently being inserted and, in
remove_from_constant_cache, we don't free the ST table for ID equal to
this one.

Co-Authored-By: Alan Wu <alanwu@ruby-lang.org>
wks pushed a commit that referenced this pull request Sep 10, 2025
If we malloc when the current Ractor is locked, we can deadlock because
GC requires VM lock and Ractor barrier. If another Ractor is waiting on
this Ractor lock, then it will deadlock because the other Ractor will
never join the barrier.

For example, this script deadlocks:

    r = Ractor.new do
      loop do
        Ractor::Port.new
      end
    end

    100000.times do |i|
      r.send(nil)
      puts i
    end

On debug builds, it fails with this assertion error:

    vm_sync.c:75: Assertion Failed: vm_lock_enter:cr->sync.locked_by != rb_ractor_self(cr)

On non-debug builds, we can see that it deadlocks in the debugger:

    Main Ractor:
    frame #3: 0x000000010021fdc4 miniruby`rb_native_mutex_lock(lock=<unavailable>) at thread_pthread.c:115:14
    frame #4: 0x0000000100193eb8 miniruby`ractor_send0 [inlined] ractor_lock(r=<unavailable>, file=<unavailable>, line=1180) at ractor.c:73:5
    frame #5: 0x0000000100193eb0 miniruby`ractor_send0 [inlined] ractor_send_basket(ec=<unavailable>, rp=0x0000000131092840, b=0x000000011c63de80, raise_on_error=true) at ractor_sync.c:1180:5
    frame #6: 0x0000000100193eac miniruby`ractor_send0(ec=<unavailable>, rp=0x0000000131092840, obj=4, move=<unavailable>, raise_on_error=true) at ractor_sync.c:1211:5

    Second Ractor:
    frame #2: 0x00000001002208d0 miniruby`rb_ractor_sched_barrier_start [inlined] rb_native_cond_wait(cond=<unavailable>, mutex=<unavailable>) at thread_pthread.c:221:13
    frame #3: 0x00000001002208cc miniruby`rb_ractor_sched_barrier_start(vm=0x000000013180d600, cr=0x0000000131093460) at thread_pthread.c:1438:13
    frame #4: 0x000000010028a328 miniruby`rb_vm_barrier at vm_sync.c:262:13 [artificial]
    frame #5: 0x00000001000dfa6c miniruby`gc_start [inlined] rb_gc_vm_barrier at gc.c:179:5
    frame #6: 0x00000001000dfa68 miniruby`gc_start [inlined] gc_enter(objspace=0x000000013180fc00, event=gc_enter_event_start, lock_lev=<unavailable>) at default.c:6636:9
    frame #7: 0x00000001000dfa48 miniruby`gc_start(objspace=0x000000013180fc00, reason=<unavailable>) at default.c:6361:5
    frame #8: 0x00000001000e3fd8 miniruby`objspace_malloc_increase_body [inlined] garbage_collect(objspace=0x000000013180fc00, reason=512) at default.c:6341:15
    frame #9: 0x00000001000e3fa4 miniruby`objspace_malloc_increase_body [inlined] garbage_collect_with_gvl(objspace=0x000000013180fc00, reason=512) at default.c:6741:16
    frame #10: 0x00000001000e3f88 miniruby`objspace_malloc_increase_body(objspace=0x000000013180fc00, mem=<unavailable>, new_size=<unavailable>, old_size=<unavailable>, type=<unavailable>) at default.c:8007:13
    frame #11: 0x00000001000e3c44 miniruby`rb_gc_impl_malloc [inlined] objspace_malloc_fixup(objspace=0x000000013180fc00, mem=0x000000011c700000, size=12582912) at default.c:8085:5
    frame #12: 0x00000001000e3c30 miniruby`rb_gc_impl_malloc(objspace_ptr=0x000000013180fc00, size=12582912) at default.c:8182:12
    frame #13: 0x00000001000d4584 miniruby`ruby_xmalloc [inlined] ruby_xmalloc_body(size=<unavailable>) at gc.c:5128:12
    frame #14: 0x00000001000d4568 miniruby`ruby_xmalloc(size=<unavailable>) at gc.c:5118:34
    frame #15: 0x00000001001eb184 miniruby`rb_st_init_existing_table_with_size(tab=0x000000011c2b4b40, type=<unavailable>, size=<unavailable>) at st.c:559:39
    frame #16: 0x00000001001ebc74 miniruby`rebuild_table_if_necessary [inlined] rb_st_init_table_with_size(type=0x00000001004f4a78, size=524287) at st.c:585:5
    frame #17: 0x00000001001ebc5c miniruby`rebuild_table_if_necessary [inlined] rebuild_table(tab=0x000000013108e2f0) at st.c:753:19
    frame #18: 0x00000001001ebbfc miniruby`rebuild_table_if_necessary(tab=0x000000013108e2f0) at st.c:1125:9
    frame #19: 0x00000001001eba08 miniruby`rb_st_insert(tab=0x000000013108e2f0, key=262144, value=4767566624) at st.c:1143:5
    frame #20: 0x0000000100194b84 miniruby`ractor_port_initialzie [inlined] ractor_add_port(r=0x0000000131093460, id=262144) at ractor_sync.c:399:9
    frame #21: 0x0000000100194b58 miniruby`ractor_port_initialzie [inlined] ractor_port_init(rpv=4750065560, r=0x0000000131093460) at ractor_sync.c:87:5
    frame #22: 0x0000000100194b34 miniruby`ractor_port_initialzie(self=4750065560) at ractor_sync.c:103:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants