ruby-changes:47431
From: usa <ko1@a...>
Date: Wed, 9 Aug 2017 19:44:03 +0900 (JST)
Subject: [ruby-changes:47431] usa:r59547 (ruby_2_3): merge revision(s) 58262, 58263: [Backport #13369]
usa 2017-08-09 19:43:57 +0900 (Wed, 09 Aug 2017) New Revision: 59547 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=59547 Log: merge revision(s) 58262,58263: [Backport #13369] fix TracePoint#return_value with non-local exits * vm.c: get return_value from imemo_throw_data object (THROW_DATA_VAL()). imemo_throw_data (TAG_BREAK) contains returned value. However, imemo_throw_data (TAG_BREAK) can skip several frames so that we need to use it only once (at most internal frame). To record it, we introduced THROW_DATA_CONSUMED and check it. * internal.h: define THROW_DATA_CONSUMED flag. * test/ruby/test_settracefunc.rb: add tests for [Bug #13369] * vm_insnhelper.h: add THROW_DATA_CONSUMED_P() and THROW_DATA_CONSUMED_SET(). internal.h: parenthesize macro argument * internal.h (THROW_DATA_P): parenthesize the argument which is casted. Modified directories: branches/ruby_2_3/ Modified files: branches/ruby_2_3/ChangeLog branches/ruby_2_3/internal.h branches/ruby_2_3/test/ruby/test_settracefunc.rb branches/ruby_2_3/version.h branches/ruby_2_3/vm.c branches/ruby_2_3/vm_insnhelper.h Index: ruby_2_3/vm_insnhelper.h =================================================================== --- ruby_2_3/vm_insnhelper.h (revision 59546) +++ ruby_2_3/vm_insnhelper.h (revision 59547) @@ -194,18 +194,6 @@ THROW_DATA_NEW(VALUE val, rb_control_fra https://github.com/ruby/ruby/blob/trunk/ruby_2_3/vm_insnhelper.h#L194 return (struct vm_throw_data *)rb_imemo_new(imemo_throw_data, val, (VALUE)cf, st, 0); } -static inline void -THROW_DATA_CATCH_FRAME_SET(struct vm_throw_data *obj, const rb_control_frame_t *cfp) -{ - obj->catch_frame = cfp; -} - -static inline void -THROW_DATA_STATE_SET(struct vm_throw_data *obj, int st) -{ - obj->throw_state = (VALUE)st; -} - static inline VALUE THROW_DATA_VAL(const struct vm_throw_data *obj) { @@ -218,10 +206,38 @@ THROW_DATA_CATCH_FRAME(const struct vm_t https://github.com/ruby/ruby/blob/trunk/ruby_2_3/vm_insnhelper.h#L206 return obj->catch_frame; } -static int +static inline int THROW_DATA_STATE(const struct vm_throw_data *obj) { return (int)obj->throw_state; } +static inline int +THROW_DATA_CONSUMED_P(const struct vm_throw_data *obj) +{ + VM_ASSERT(THROW_DATA_P(obj)); + return obj->flags & THROW_DATA_CONSUMED; +} + +static inline void +THROW_DATA_CATCH_FRAME_SET(struct vm_throw_data *obj, const rb_control_frame_t *cfp) +{ + obj->catch_frame = cfp; +} + +static inline void +THROW_DATA_STATE_SET(struct vm_throw_data *obj, int st) +{ + obj->throw_state = (VALUE)st; +} + +static inline void +THROW_DATA_CONSUMED_SET(struct vm_throw_data *obj) +{ + if (THROW_DATA_P(obj) && + THROW_DATA_STATE(obj) == TAG_BREAK) { + obj->flags |= THROW_DATA_CONSUMED; + } +} + #endif /* RUBY_INSNHELPER_H */ Index: ruby_2_3/version.h =================================================================== --- ruby_2_3/version.h (revision 59546) +++ ruby_2_3/version.h (revision 59547) @@ -1,6 +1,6 @@ https://github.com/ruby/ruby/blob/trunk/ruby_2_3/version.h#L1 #define RUBY_VERSION "2.3.5" #define RUBY_RELEASE_DATE "2017-08-09" -#define RUBY_PATCHLEVEL 357 +#define RUBY_PATCHLEVEL 358 #define RUBY_RELEASE_YEAR 2017 #define RUBY_RELEASE_MONTH 8 Index: ruby_2_3/internal.h =================================================================== --- ruby_2_3/internal.h (revision 59546) +++ ruby_2_3/internal.h (revision 59547) @@ -583,6 +583,8 @@ struct vm_svar { https://github.com/ruby/ruby/blob/trunk/ruby_2_3/internal.h#L583 /* THROW_DATA */ +#define THROW_DATA_CONSUMED IMEMO_FL_USER0 + struct vm_throw_data { VALUE flags; VALUE reserved; @@ -591,7 +593,7 @@ struct vm_throw_data { https://github.com/ruby/ruby/blob/trunk/ruby_2_3/internal.h#L593 VALUE throw_state; }; -#define THROW_DATA_P(err) RB_TYPE_P((err), T_IMEMO) +#define THROW_DATA_P(err) RB_TYPE_P((VALUE)(err), T_IMEMO) /* IFUNC */ Index: ruby_2_3/vm.c =================================================================== --- ruby_2_3/vm.c (revision 59546) +++ ruby_2_3/vm.c (revision 59547) @@ -1507,6 +1507,42 @@ vm_frametype_name(const rb_control_frame https://github.com/ruby/ruby/blob/trunk/ruby_2_3/vm.c#L1507 } #endif +static VALUE +frame_return_value(const struct vm_throw_data *err) +{ + if (THROW_DATA_P(err) && + THROW_DATA_STATE(err) == TAG_BREAK && + THROW_DATA_CONSUMED_P(err) == FALSE) { + return THROW_DATA_VAL(err); + } + else { + return Qnil; + } +} + +#if 0 +/* for debug */ +static const char * +frame_name(const rb_control_frame_t *cfp) +{ + unsigned long type = VM_FRAME_TYPE(cfp); +#define C(t) if (type == VM_FRAME_MAGIC_##t) return #t + C(METHOD); + C(BLOCK); + C(CLASS); + C(TOP); + C(CFUNC); + C(PROC); + C(IFUNC); + C(EVAL); + C(LAMBDA); + C(RESCUE); + C(DUMMY); +#undef C + return "unknown"; +} +#endif + static void hook_before_rewind(rb_thread_t *th, const rb_control_frame_t *cfp, int will_finish_vm_exec, int state, struct vm_throw_data *err) { @@ -1516,22 +1552,26 @@ hook_before_rewind(rb_thread_t *th, cons https://github.com/ruby/ruby/blob/trunk/ruby_2_3/vm.c#L1552 switch (VM_FRAME_TYPE(th->cfp)) { case VM_FRAME_MAGIC_METHOD: RUBY_DTRACE_METHOD_RETURN_HOOK(th, 0, 0); - EXEC_EVENT_HOOK_AND_POP_FRAME(th, RUBY_EVENT_RETURN, th->cfp->self, 0, 0, Qnil); + EXEC_EVENT_HOOK_AND_POP_FRAME(th, RUBY_EVENT_RETURN, th->cfp->self, 0, 0, frame_return_value(err)); + THROW_DATA_CONSUMED_SET(err); break; case VM_FRAME_MAGIC_BLOCK: case VM_FRAME_MAGIC_LAMBDA: if (VM_FRAME_TYPE_BMETHOD_P(th->cfp)) { - EXEC_EVENT_HOOK(th, RUBY_EVENT_B_RETURN, th->cfp->self, 0, 0, Qnil); + EXEC_EVENT_HOOK(th, RUBY_EVENT_B_RETURN, th->cfp->self, 0, 0, frame_return_value(err)); if (!will_finish_vm_exec) { /* kick RUBY_EVENT_RETURN at invoke_block_from_c() for bmethod */ EXEC_EVENT_HOOK_AND_POP_FRAME(th, RUBY_EVENT_RETURN, th->cfp->self, rb_vm_frame_method_entry(th->cfp)->def->original_id, - rb_vm_frame_method_entry(th->cfp)->owner, Qnil); + rb_vm_frame_method_entry(th->cfp)->owner, + frame_return_value(err)); } + THROW_DATA_CONSUMED_SET(err); } else { - EXEC_EVENT_HOOK_AND_POP_FRAME(th, RUBY_EVENT_B_RETURN, th->cfp->self, 0, 0, Qnil); + EXEC_EVENT_HOOK_AND_POP_FRAME(th, RUBY_EVENT_B_RETURN, th->cfp->self, 0, 0, frame_return_value(err)); + THROW_DATA_CONSUMED_SET(err); } break; case VM_FRAME_MAGIC_CLASS: @@ -1711,7 +1751,7 @@ vm_exec(rb_thread_t *th) https://github.com/ruby/ruby/blob/trunk/ruby_2_3/vm.c#L1751 } } } - if (!catch_iseq) { + if (catch_iseq == NULL) { th->errinfo = Qnil; result = THROW_DATA_VAL(err); THROW_DATA_CATCH_FRAME_SET(err, cfp + 1); Index: ruby_2_3/ChangeLog =================================================================== --- ruby_2_3/ChangeLog (revision 59546) +++ ruby_2_3/ChangeLog (revision 59547) @@ -1,3 +1,23 @@ https://github.com/ruby/ruby/blob/trunk/ruby_2_3/ChangeLog#L1 +Wed Aug 9 19:34:17 2017 Nobuyoshi Nakada <nobu@r...> + + * internal.h (THROW_DATA_P): parenthesize the argument which is + casted. + +Wed Aug 9 19:34:17 2017 Koichi Sasada <ko1@a...> + + * vm.c: get return_value from imemo_throw_data object (THROW_DATA_VAL()). + imemo_throw_data (TAG_BREAK) contains returned value. + However, imemo_throw_data (TAG_BREAK) can skip several frames so that + we need to use it only once (at most internal frame). To record it, + we introduced THROW_DATA_CONSUMED and check it. + + * internal.h: define THROW_DATA_CONSUMED flag. + + * test/ruby/test_settracefunc.rb: add tests for [Bug #13369] + + * vm_insnhelper.h: add THROW_DATA_CONSUMED_P() and + THROW_DATA_CONSUMED_SET(). + Wed Aug 9 19:32:17 2017 Nobuyoshi Nakada <nobu@r...> * random.c (get_rnd, try_get_rnd): ensure initialized to get rid Index: ruby_2_3/test/ruby/test_settracefunc.rb =================================================================== --- ruby_2_3/test/ruby/test_settracefunc.rb (revision 59546) +++ ruby_2_3/test/ruby/test_settracefunc.rb (revision 59547) @@ -1488,4 +1488,150 @@ class TestSetTraceFunc < Test::Unit::Tes https://github.com/ruby/ruby/blob/trunk/ruby_2_3/test/ruby/test_settracefunc.rb#L1488 assert_equal ev, :fiber_switch } end + + # tests for `return_value` with non-local exit [Bug #13369] + + def tp_return_value mid + ary = [] + TracePoint.new(:return, :b_return){|tp| ary << [tp.event, tp.method_id, tp.return_value]}.enable{ + send mid + } + ary.pop # last b_return event is not required. + ary + end + + def f_raise + raise + rescue + return :f_raise_return + end + + def f_iter1 + yield + return :f_iter1_return + end + + def f_iter2 + yield + return :f_iter2_return + end + + def f_return_in_iter + f_iter1 do + f_iter2 do + return :f_return_in_iter_return + end + end + 2 + end + + def f_break_in_iter + f_iter1 do + f_iter2 do + break :f_break_in_iter_break + end + :f_iter1_block_value + end + :f_break_in_iter_return + end + + def test_return_value_with_rescue + assert_equal [[:return, :f_raise, :f_raise_return]], + tp_return_value(:f_raise), + '[Bug #13369]' + + assert_equal [[:b_return, :f_return_in_iter, nil], + [:return, :f_iter2, nil], + [:b_return, :f_return_in_iter, nil], + [:return, :f_iter1, nil], + [:return, :f_return_in_iter, :f_return_in_iter_return]], + tp_return_value(:f_return_in_iter), + '[Bug #13369]' + + assert_equal [[:b_return, :f_break_in_iter, :f_break_in_iter_break], + [:return, :f_iter2, nil], + [:b_return, :f_break_in_iter, :f_iter1_block_value], + [:return, :f_iter1, :f_iter1_return], + [:return, :f_break_in_iter, :f_break_in_iter_return]], + tp_return_value(:f_break_in_iter), + '[Bug #13369]' + end + + define_method(:f_last_defined) do + :f_last_defined + end + + define_method(:f_return_defined) do + return :f_return_defined + end + + define_method(:f_break_defined) do + return :f_break_defined + end + + define_method(:f_raise_defined) do + begin + raise + rescue + return :f_raise_defined + end + end + + define_method(:f_break_in_rescue_defined) do + begin + raise + rescue + break :f_break_in_rescue_defined + end + end + + def test_return_value_with_rescue_and_defined_methods + assert_equal [[:b_return, :f_last_defined, :f_last_defined], + [:return, :f_last_defined, :f_last_defined]], + tp_return_value(:f_last_defined), + '[Bug #13369]' + + assert_equal [[:b_return, :f_return_defined, nil], # current limitation + [:return, :f_return_defined, :f_return_defined]], + tp_return_value(:f_return_defined), + '[Bug #13369]' + + assert_equal [[:b_return, :f_break_defined, nil], + [:return, :f_break_defined, :f_break_defined]], + tp_return_value(:f_break_defined), + '[Bug #13369]' + + assert_equal [[:b_return, :f_raise_defined, nil], + [:return, :f_raise_defined, f_raise_defined]], + tp_return_value(:f_raise_defined), + '[Bug #13369]' + + assert_equal [[:b_return, :f_break_in_rescue_defined, nil], + [:return, :f_break_in_rescue_defined, f_break_in_rescue_defined]], + tp_return_value(:f_break_in_rescue_defined), + '[Bug #13369]' + end + + def f_iter + yield + end + + def f_break_in_rescue + f_iter do + begin + raise + rescue + break :b + end + end + :f_break_in_rescue_return_value + end + + def test_break_with_rescue + assert_equal [[:b_return, :f_break_in_rescue, :b], + [:return, :f_iter, nil], + [:return, :f_break_in_rescue, :f_break_in_rescue_return_value]], + tp_return_value(:f_break_in_rescue), + '[Bug #13369]' + end end Index: ruby_2_3 =================================================================== --- ruby_2_3 (revision 59546) +++ ruby_2_3 (revision 59547) Property changes on: ruby_2_3 ___________________________________________________________________ Modified: svn:mergeinfo ## -0,0 +0,1 ## Merged /trunk:r58262-58263 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/