From 2d86e79fe6a8eaea85edb4b9ab59d12228dbd8b9 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 31 Oct 2022 15:38:51 -0700 Subject: [PATCH] Always lookup IV buffers when iterating 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] --- variable.c | 43 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/variable.c b/variable.c index 0f71b2741816d5..745ace528f1a5b 100644 --- a/variable.c +++ b/variable.c @@ -1566,22 +1566,41 @@ rb_ivar_defined(VALUE obj, ID id) typedef int rb_ivar_foreach_callback_func(ID key, VALUE val, st_data_t arg); st_data_t rb_st_nth_key(st_table *tab, st_index_t index); +struct iv_itr_data { + VALUE obj; + struct gen_ivtbl * ivtbl; + st_data_t arg; +}; + static void -iterate_over_shapes_with_callback(rb_shape_t *shape, VALUE* iv_list, rb_ivar_foreach_callback_func *callback, st_data_t arg) +iterate_over_shapes_with_callback(rb_shape_t *shape, rb_ivar_foreach_callback_func *callback, struct iv_itr_data * itr_data) { switch ((enum shape_type)shape->type) { case SHAPE_ROOT: return; case SHAPE_IVAR: - iterate_over_shapes_with_callback(rb_shape_get_shape_by_id(shape->parent_id), iv_list, callback, arg); + iterate_over_shapes_with_callback(rb_shape_get_shape_by_id(shape->parent_id), callback, itr_data); + VALUE * iv_list; + switch(BUILTIN_TYPE(itr_data->obj)) { + case T_OBJECT: + iv_list = ROBJECT_IVPTR(itr_data->obj); + break; + case T_CLASS: + case T_MODULE: + iv_list = RCLASS_IVPTR(itr_data->obj); + break; + default: + iv_list = itr_data->ivtbl->ivptr; + break; + } VALUE val = iv_list[shape->next_iv_index - 1]; if (val != Qundef) { - callback(shape->edge_name, val, arg); + callback(shape->edge_name, val, itr_data->arg); } return; case SHAPE_IVAR_UNDEF: case SHAPE_FROZEN: - iterate_over_shapes_with_callback(rb_shape_get_shape_by_id(shape->parent_id), iv_list, callback, arg); + iterate_over_shapes_with_callback(rb_shape_get_shape_by_id(shape->parent_id), callback, itr_data); return; } } @@ -1590,7 +1609,10 @@ static void obj_ivar_each(VALUE obj, rb_ivar_foreach_callback_func *func, st_data_t arg) { rb_shape_t* shape = rb_shape_get_shape(obj); - iterate_over_shapes_with_callback(shape, ROBJECT_IVPTR(obj), func, arg); + struct iv_itr_data itr_data; + itr_data.obj = obj; + itr_data.arg = arg; + iterate_over_shapes_with_callback(shape, func, &itr_data); } static void @@ -1600,7 +1622,11 @@ gen_ivar_each(VALUE obj, rb_ivar_foreach_callback_func *func, st_data_t arg) struct gen_ivtbl *ivtbl; if (!rb_gen_ivtbl_get(obj, 0, &ivtbl)) return; - iterate_over_shapes_with_callback(shape, ivtbl->ivptr, func, arg); + struct iv_itr_data itr_data; + itr_data.obj = obj; + itr_data.ivtbl = ivtbl; + itr_data.arg = arg; + iterate_over_shapes_with_callback(shape, func, &itr_data); } static void @@ -1609,7 +1635,10 @@ class_ivar_each(VALUE obj, rb_ivar_foreach_callback_func *func, st_data_t arg) RUBY_ASSERT(RB_TYPE_P(obj, T_CLASS) || RB_TYPE_P(obj, T_MODULE)); rb_shape_t* shape = rb_shape_get_shape(obj); - iterate_over_shapes_with_callback(shape, RCLASS_IVPTR(obj), func, arg); + struct iv_itr_data itr_data; + itr_data.obj = obj; + itr_data.arg = arg; + iterate_over_shapes_with_callback(shape, func, &itr_data); } void