Skip to content

Commit

Permalink
Always lookup IV buffers when iterating
Browse files Browse the repository at this point in the history 
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]
  • Loading branch information
@tenderlove
tenderlove committed Nov 1, 2022
1 parent 02f1554 commit 2d86e79
Showing 1 changed file with 36 additions and 7 deletions.
43 changes: 36 additions & 7 deletions variable.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 2d86e79

Please sign in to comment.