ruby-changes:37185
From: ktsj <ko1@a...>
Date: Fri, 16 Jan 2015 11:54:29 +0900 (JST)
Subject: [ruby-changes:37185] ktsj:r49266 (trunk): * eval_intern.h, vm.c, vm_eval.c, vm_insnhelper.c:
ktsj 2015-01-16 11:54:22 +0900 (Fri, 16 Jan 2015) New Revision: 49266 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=49266 Log: * eval_intern.h, vm.c, vm_eval.c, vm_insnhelper.c: change throw mechanism (not save target ep, but save target cfp). It fixes `unexpected break' bug that occurs when TracePoint#binding is called. [ruby-dev:48797] [Bug #10689] * test/ruby/test_settracefunc.rb: add a test. Modified files: trunk/ChangeLog trunk/eval_intern.h trunk/test/ruby/test_settracefunc.rb trunk/vm.c trunk/vm_eval.c trunk/vm_insnhelper.c Index: eval_intern.h =================================================================== --- eval_intern.h (revision 49265) +++ eval_intern.h (revision 49266) @@ -207,7 +207,7 @@ enum ruby_tag_type { https://github.com/ruby/ruby/blob/trunk/eval_intern.h#L207 (RNODE((obj))->u3.value = (val)) #define GET_THROWOBJ_VAL(obj) ((VALUE)RNODE((obj))->u1.value) -#define GET_THROWOBJ_CATCH_POINT(obj) ((VALUE*)RNODE((obj))->u2.value) +#define GET_THROWOBJ_CATCH_POINT(obj) ((rb_control_frame_t*)RNODE((obj))->u2.value) #define GET_THROWOBJ_STATE(obj) ((int)RNODE((obj))->u3.value) #define SCOPE_TEST(f) (rb_vm_cref()->nd_visi & (f)) Index: ChangeLog =================================================================== --- ChangeLog (revision 49265) +++ ChangeLog (revision 49266) @@ -1,3 +1,13 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1 +Fri Jan 16 11:44:44 2015 Kazuki Tsujimoto <kazuki@c...> + + * eval_intern.h, vm.c, vm_eval.c, vm_insnhelper.c: + change throw mechanism (not save target ep, but save target cfp). + It fixes `unexpected break' bug that occurs when + TracePoint#binding is called. + [ruby-dev:48797] [Bug #10689] + + * test/ruby/test_settracefunc.rb: add a test. + Thu Jan 15 23:55:15 2015 Tanaka Akira <akr@f...> * io.c (rb_io_close_m): Don't raise when the IO object is closed. Index: vm_eval.c =================================================================== --- vm_eval.c (revision 49265) +++ vm_eval.c (revision 49266) @@ -1100,10 +1100,9 @@ rb_iterate(VALUE (* it_proc) (VALUE), VA https://github.com/ruby/ruby/blob/trunk/vm_eval.c#L1100 else { VALUE err = th->errinfo; if (state == TAG_BREAK) { - VALUE *escape_ep = GET_THROWOBJ_CATCH_POINT(err); - VALUE *cep = cfp->ep; + rb_control_frame_t *escape_cfp = GET_THROWOBJ_CATCH_POINT(err); - if (cep == escape_ep) { + if (cfp == escape_cfp) { state = 0; th->state = 0; th->errinfo = Qnil; @@ -1116,10 +1115,9 @@ rb_iterate(VALUE (* it_proc) (VALUE), VA https://github.com/ruby/ruby/blob/trunk/vm_eval.c#L1115 } } else if (state == TAG_RETRY) { - VALUE *escape_ep = GET_THROWOBJ_CATCH_POINT(err); - VALUE *cep = cfp->ep; + rb_control_frame_t *escape_cfp = GET_THROWOBJ_CATCH_POINT(err); - if (cep == escape_ep) { + if (cfp == escape_cfp) { rb_vm_rewind_cfp(th, cfp); state = 0; Index: vm.c =================================================================== --- vm.c (revision 49265) +++ vm.c (revision 49266) @@ -28,6 +28,26 @@ VM_EP_LEP(VALUE *ep) https://github.com/ruby/ruby/blob/trunk/vm.c#L28 return ep; } +static inline rb_control_frame_t * +rb_vm_search_cf_from_ep(const rb_thread_t * const th, rb_control_frame_t *cfp, const VALUE * const ep) +{ + if (!ep) { + return NULL; + } + else { + const rb_control_frame_t * const eocfp = RUBY_VM_END_CONTROL_FRAME(th); /* end of control frame pointer */ + + while (cfp < eocfp) { + if (cfp->ep == ep) { + return cfp; + } + cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp); + } + + rb_bug("rb_vm_search_cf_from_ep: no corresponding cfp"); + } +} + VALUE * rb_vm_ep_local_ep(VALUE *ep) { @@ -550,7 +570,6 @@ rb_vm_env_local_variables(VALUE envval) https://github.com/ruby/ruby/blob/trunk/vm.c#L570 return local_var_list_finish(&vars); } -static void vm_rewrite_ep_in_errinfo(rb_thread_t *th); static VALUE vm_make_proc_from_block(rb_thread_t *th, rb_block_t *block); static VALUE vm_make_env_object(rb_thread_t * th, rb_control_frame_t *cfp, VALUE *blockprocptr); @@ -577,7 +596,6 @@ vm_make_env_object(rb_thread_t *th, rb_c https://github.com/ruby/ruby/blob/trunk/vm.c#L596 } envval = vm_make_env_each(th, cfp, cfp->ep, lep); - vm_rewrite_ep_in_errinfo(th); if (PROCDEBUG) { check_env_value(envval); @@ -586,32 +604,6 @@ vm_make_env_object(rb_thread_t *th, rb_c https://github.com/ruby/ruby/blob/trunk/vm.c#L604 return envval; } -static void -vm_rewrite_ep_in_errinfo(rb_thread_t *th) -{ - rb_control_frame_t *cfp = th->cfp; - while (!RUBY_VM_CONTROL_FRAME_STACK_OVERFLOW_P(th, cfp)) { - /* rewrite ep in errinfo to point to heap */ - if (RUBY_VM_NORMAL_ISEQ_P(cfp->iseq) && - (cfp->iseq->type == ISEQ_TYPE_RESCUE || - cfp->iseq->type == ISEQ_TYPE_ENSURE)) { - VALUE errinfo = cfp->ep[-2]; /* #$! */ - if (RB_TYPE_P(errinfo, T_NODE)) { - VALUE *escape_ep = GET_THROWOBJ_CATCH_POINT(errinfo); - if (! ENV_IN_HEAP_P(th, escape_ep)) { - VALUE epval = *escape_ep; - if (!SPECIAL_CONST_P(epval) && RBASIC(epval)->klass == rb_cEnv) { - rb_env_t *epenv; - GetEnvPtr(epval, epenv); - SET_THROWOBJ_CATCH_POINT(errinfo, (VALUE)(epenv->env + epenv->local_size)); - } - } - } - } - cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp); - } -} - void rb_vm_stack_to_heap(rb_thread_t *th) { @@ -1152,9 +1144,10 @@ vm_iter_break(rb_thread_t *th, VALUE val https://github.com/ruby/ruby/blob/trunk/vm.c#L1144 { rb_control_frame_t *cfp = th->cfp; VALUE *ep = VM_CF_PREV_EP(cfp); + rb_control_frame_t *target_cfp = rb_vm_search_cf_from_ep(th, cfp, ep); th->state = TAG_BREAK; - th->errinfo = (VALUE)NEW_THROW_OBJECT(val, (VALUE)ep, TAG_BREAK); + th->errinfo = (VALUE)NEW_THROW_OBJECT(val, (VALUE)target_cfp, TAG_BREAK); TH_JUMP_TAG(th, TAG_BREAK); } @@ -1419,7 +1412,7 @@ vm_exec(rb_thread_t *th) https://github.com/ruby/ruby/blob/trunk/vm.c#L1412 VALUE catch_iseqval; rb_control_frame_t *cfp; VALUE type; - VALUE *escape_ep; + rb_control_frame_t *escape_cfp; err = th->errinfo; @@ -1438,14 +1431,14 @@ vm_exec(rb_thread_t *th) https://github.com/ruby/ruby/blob/trunk/vm.c#L1431 cfp = th->cfp; epc = cfp->pc - cfp->iseq->iseq_encoded; - escape_ep = NULL; + escape_cfp = NULL; if (state == TAG_BREAK || state == TAG_RETURN) { - escape_ep = GET_THROWOBJ_CATCH_POINT(err); + escape_cfp = GET_THROWOBJ_CATCH_POINT(err); - if (cfp->ep == escape_ep) { + if (cfp == escape_cfp) { if (state == TAG_RETURN) { if (!VM_FRAME_TYPE_FINISH_P(cfp)) { - SET_THROWOBJ_CATCH_POINT(err, (VALUE)(cfp + 1)->ep); + SET_THROWOBJ_CATCH_POINT(err, (VALUE)(cfp + 1)); SET_THROWOBJ_STATE(err, state = TAG_BREAK); } else { @@ -1519,9 +1512,9 @@ vm_exec(rb_thread_t *th) https://github.com/ruby/ruby/blob/trunk/vm.c#L1512 break; } else if (entry->type == CATCH_TYPE_RETRY) { - VALUE *escape_ep; - escape_ep = GET_THROWOBJ_CATCH_POINT(err); - if (cfp->ep == escape_ep) { + rb_control_frame_t *escape_cfp; + escape_cfp = GET_THROWOBJ_CATCH_POINT(err); + if (cfp == escape_cfp) { cfp->pc = cfp->iseq->iseq_encoded + entry->cont; th->errinfo = Qnil; goto vm_loop_start; @@ -1530,7 +1523,7 @@ vm_exec(rb_thread_t *th) https://github.com/ruby/ruby/blob/trunk/vm.c#L1523 } } } - else if (state == TAG_BREAK && ((VALUE)escape_ep & ~0x03) == 0) { + else if (state == TAG_BREAK && !escape_cfp) { type = CATCH_TYPE_BREAK; search_restart_point: Index: vm_insnhelper.c =================================================================== --- vm_insnhelper.c (revision 49265) +++ vm_insnhelper.c (revision 49266) @@ -573,174 +573,175 @@ vm_setinstancevariable(VALUE obj, ID id, https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L573 } static VALUE -vm_throw(rb_thread_t *th, rb_control_frame_t *reg_cfp, - rb_num_t throw_state, VALUE throwobj) +vm_throw_continue(rb_thread_t *th, VALUE throwobj) { - int state = (int)(throw_state & 0xff); - int flag = (int)(throw_state & 0x8000); - rb_num_t level = throw_state >> 16; + /* continue throw */ + VALUE err = throwobj; - if (state != 0) { - VALUE *pt = 0; - if (flag != 0) { - pt = (void *) 1; - } - else { - if (state == TAG_BREAK) { - rb_control_frame_t *cfp = GET_CFP(); - VALUE *ep = GET_EP(); - int is_orphan = 1; - rb_iseq_t *base_iseq = GET_ISEQ(); - - search_parent: - if (cfp->iseq->type != ISEQ_TYPE_BLOCK) { - if (cfp->iseq->type == ISEQ_TYPE_CLASS) { - cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp); - ep = cfp->ep; - goto search_parent; - } - ep = VM_EP_PREV_EP(ep); - base_iseq = base_iseq->parent_iseq; + if (FIXNUM_P(err)) { + th->state = FIX2INT(err); + } + else if (SYMBOL_P(err)) { + th->state = TAG_THROW; + } + else if (BUILTIN_TYPE(err) == T_NODE) { + th->state = GET_THROWOBJ_STATE(err); + } + else { + th->state = TAG_RAISE; + /*th->state = FIX2INT(rb_ivar_get(err, idThrowState));*/ + } + return err; +} - while ((VALUE *) cfp < th->stack + th->stack_size) { - if (cfp->ep == ep) { - goto search_parent; - } - cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp); - } - rb_bug("VM (throw): can't find break base."); - } +static VALUE +vm_throw_start(rb_thread_t * const th, rb_control_frame_t * const reg_cfp, int state, const int flag, const rb_num_t level, const VALUE throwobj) +{ + rb_control_frame_t *escape_cfp = NULL; + const rb_control_frame_t * const eocfp = RUBY_VM_END_CONTROL_FRAME(th); /* end of control frame pointer */ - if (VM_FRAME_TYPE(cfp) == VM_FRAME_MAGIC_LAMBDA) { - /* lambda{... break ...} */ - is_orphan = 0; - pt = cfp->ep; - state = TAG_RETURN; - } - else { - ep = VM_EP_PREV_EP(ep); + if (flag != 0) { + /* do nothing */ + } + else if (state == TAG_BREAK) { + int is_orphan = 1; + VALUE *ep = GET_EP(); + rb_iseq_t *base_iseq = GET_ISEQ(); + escape_cfp = reg_cfp; + + search_parent: + if (base_iseq->type != ISEQ_TYPE_BLOCK) { + if (escape_cfp->iseq->type == ISEQ_TYPE_CLASS) { + escape_cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(escape_cfp); + ep = escape_cfp->ep; + base_iseq = escape_cfp->iseq; + goto search_parent; + } + else { + ep = VM_EP_PREV_EP(ep); + base_iseq = base_iseq->parent_iseq; + escape_cfp = rb_vm_search_cf_from_ep(th, escape_cfp, ep); + assert(escape_cfp->iseq == base_iseq); + } + } - while ((VALUE *)cfp < th->stack + th->stack_size) { - if (cfp->ep == ep) { - VALUE epc = cfp->pc - cfp->iseq->iseq_encoded; - rb_iseq_t *iseq = cfp->iseq; - struct iseq_catch_table *ct = iseq->catch_table; - struct iseq_catch_table_entry *entry; - int i; - - for (i=0; i<ct->size; i++) { - entry = &ct->entries[i]; - - if (entry->type == CATCH_TYPE_BREAK && - entry->start < epc && entry->end >= epc) { - if (entry->cont == epc) { - goto found; - } - else { - break; - } - } - } - break; + if (VM_FRAME_TYPE(escape_cfp) == VM_FRAME_MAGIC_LAMBDA) { + /* lambda{... break ...} */ + is_orphan = 0; + state = TAG_RETURN; + } + else { + ep = VM_EP_PREV_EP(ep); - found: - pt = ep; - is_orphan = 0; + while (escape_cfp < eocfp) { + if (escape_cfp->ep == ep) { + const VALUE epc = escape_cfp->pc - escape_cfp->iseq->iseq_encoded; + const rb_iseq_t * const iseq = escape_cfp->iseq; + const struct iseq_catch_table * const ct = iseq->catch_table; + const int ct_size = ct->size; + int i; + + for (i=0; i<ct_size; i++) { + const struct iseq_catch_table_entry * const entry = &ct->entries[i];; + + if (entry->type == CATCH_TYPE_BREAK && entry->start < epc && entry->end >= epc) { + if (entry->cont == epc) { /* found! */ + is_orphan = 0; + } break; } - cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp); } + break; } - if (is_orphan) { - rb_vm_localjump_error("break from proc-closure", throwobj, TAG_BREAK); - } + escape_cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(escape_cfp); } - else if (state == TAG_RETRY) { - rb_num_t i; - pt = VM_EP_PREV_EP(GET_EP()); - for (i = 0; i < level; i++) { - pt = GC_GUARDED_PTR_REF((VALUE *) * pt); - } + } + + if (is_orphan) { + rb_vm_localjump_error("break from proc-closure", throwobj, TAG_BREAK); + } + } + else if (state == TAG_RETRY) { + rb_num_t i; + VALUE *ep = VM_EP_PREV_EP(GET_EP()); + + for (i = 0; i < level; i++) { + ep = VM_EP_PREV_EP(ep); + } + + escape_cfp = rb_vm_search_cf_from_ep(th, reg_cfp, ep); + } + else if (state == TAG_RETURN) { + VALUE *current_ep = GET_EP(); + VALUE *target_lep = VM_EP_LEP(current_ep); + int in_class_frame = 0; + escape_cfp = reg_cfp; + + while (escape_cfp < eocfp) { + VALUE *lep = VM_CF_LEP(escape_cfp); + + if (!target_lep) { + target_lep = lep; } - else if (state == TAG_RETURN) { - rb_control_frame_t *cfp = GET_CFP(); - VALUE *ep = GET_EP(); - VALUE *target_lep = VM_CF_LEP(cfp); - int in_class_frame = 0; - - /* check orphan and get dfp */ - while ((VALUE *) cfp < th->stack + th->stack_size) { - VALUE *lep = VM_CF_LEP(cfp); - if (!target_lep) { - target_lep = lep; - } + if (lep == target_lep && escape_cfp->iseq->type == ISEQ_TYPE_CLASS) { + in_class_frame = 1; + target_lep = 0; + } - if (lep == target_lep && cfp->iseq->type == ISEQ_TYPE_CLASS) { - in_class_frame = 1; - target_lep = 0; + if (lep == target_lep) { + if (VM_FRAME_TYPE(escape_cfp) == VM_FRAME_MAGIC_LAMBDA) { + if (in_class_frame) { + /* lambda {class A; ... return ...; end} */ + goto valid_return; } + else { + VALUE *tep = current_ep; - if (lep == target_lep) { - if (VM_FRAME_TYPE(cfp) == VM_FRAME_MAGIC_LAMBDA) { - VALUE *tep = ep; - - if (in_class_frame) { - /* lambda {class A; ... return ...; end} */ - ep = cfp->ep; + while (target_lep != tep) { + if (escape_cfp->ep == tep) { + /* in lambda */ goto valid_return; } - - while (target_lep != tep) { - if (cfp->ep == tep) { - /* in lambda */ - ep = cfp->ep; - goto valid_return; - } - tep = VM_EP_PREV_EP(tep); - } + tep = VM_EP_PREV_EP(tep); } } - - if (cfp->ep == target_lep && cfp->iseq->type == ISEQ_TYPE_METHOD) { - ep = target_lep; - goto valid_return; - } - - cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp); } - - rb_vm_localjump_error("unexpected return", throwobj, TAG_RETURN); - - valid_return: - pt = ep; } - else { - rb_bug("isns(throw): unsupport throw type"); + + if (escape_cfp->ep == target_lep && escape_cfp->iseq->type == ISEQ_TYPE_METHOD) { + goto valid_return; } + + escape_cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(escape_cfp); } - th->state = state; - return (VALUE)NEW_THROW_OBJECT(throwobj, (VALUE) pt, state); + rb_vm_localjump_error("unexpected return", throwobj, TAG_RETURN); + + valid_return:; + /* do nothing */ } else { - /* continue throw */ - VALUE err = throwobj; + rb_bug("isns(throw): unsupport throw type"); + } - if (FIXNUM_P(err)) { - th->state = FIX2INT(err); - } - else if (SYMBOL_P(err)) { - th->state = TAG_THROW; - } - else if (BUILTIN_TYPE(err) == T_NODE) { - th->state = GET_THROWOBJ_STATE(err); - } - else { - th->state = TAG_RAISE; - /*th->state = FIX2INT(rb_ivar_get(err, idThrowState));*/ - } - return err; + th->state = state; + return (VALUE)NEW_THROW_OBJECT(throwobj, (VALUE)escape_cfp, state); +} + +static VALUE +vm_throw(rb_thread_t *th, rb_control_frame_t *reg_cfp, + rb_num_t throw_state, VALUE throwobj) +{ + const int state = (int)(throw_state & 0xff); + const int flag = (int)(throw_state & 0x8000); + const rb_num_t level = throw_state >> 16; + + if (state != 0) { + return vm_throw_start(th, reg_cfp, state, flag, level, throwobj); + } + else { + return vm_throw_continue(th, throwobj); } } Index: test/ruby/test_settracefunc.rb =================================================================== --- test/ruby/test_settracefunc.rb (revision 49265) +++ test/ruby/test_settracefunc.rb (revision 49266) @@ -977,6 +977,27 @@ class TestSetTraceFunc < Test::Unit::Tes https://github.com/ruby/ruby/blob/trunk/test/ruby/test_settracefunc.rb#L977 end end + def test_trace_point_binding_after_break + bug10689 = '[ruby-dev:48797]' + assert_in_out_err([], <<-INPUT, [], [], bug10689) + class Bug + include Enumerable + + def each + [0].each do + yield + end + end + end + + TracePoint.trace(:c_return) do |tp| + tp.binding + end + + Bug.new.all? { false } + INPUT + end + def test_tracepoint_b_return_with_next n = 0 TracePoint.new(:b_return){ -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/