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().


git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@58262 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

internal.h

@@ -877,6 +877,8 @@ struct vm_svar {
 
 /* THROW_DATA */
 
+#define THROW_DATA_CONSUMED IMEMO_FL_USER0
+
 struct vm_throw_data {
     VALUE flags;
     VALUE reserved;
@@ -885,7 +887,7 @@ struct vm_throw_data {
     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 */
 

test/ruby/test_settracefunc.rb

@@ -1599,4 +1599,146 @@ class TestSetTraceFunc < Test::Unit::TestCase
     assert_equal [[:c_call, :itself, :alias_itself], [:c_return, :itself, :alias_itself]], events
     events.clear
   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
+    raise
+  rescue
+    return :f_raise_defined
+  end
+
+  define_method(:f_break_in_rescue_defined) do
+    raise
+  rescue
+    break :f_break_in_rescue_defined
+  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

vm.c

@@ -1599,29 +1599,69 @@ vm_frametype_name(const rb_control_frame_t *cfp)
 }
 #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, rb_control_frame_t *cfp, int will_finish_vm_exec)
+hook_before_rewind(rb_thread_t *th, const rb_control_frame_t *cfp, int will_finish_vm_exec, struct vm_throw_data *err)
 {
     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, 0, Qnil);
+	EXEC_EVENT_HOOK_AND_POP_FRAME(th, RUBY_EVENT_RETURN, th->cfp->self, 0, 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_BMETHOD_P(th->cfp)) {
-	    EXEC_EVENT_HOOK(th, RUBY_EVENT_B_RETURN, th->cfp->self, 0, 0, 0, Qnil);
+	    EXEC_EVENT_HOOK(th, RUBY_EVENT_B_RETURN, th->cfp->self, 0, 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)->called_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, 0, Qnil);
+	    EXEC_EVENT_HOOK_AND_POP_FRAME(th, RUBY_EVENT_B_RETURN, th->cfp->self, 0, 0, 0, frame_return_value(err));
+	    THROW_DATA_CONSUMED_SET(err);
 	}
 	break;
       case VM_FRAME_MAGIC_CLASS:
@@ -1784,10 +1824,11 @@ vm_exec(rb_thread_t *th)
 				}
 			    }
 			}
-			if (!catch_iseq) {
+			if (catch_iseq == NULL) {
 			    th->errinfo = Qnil;
 			    result = THROW_DATA_VAL(err);
-			    hook_before_rewind(th, th->cfp, TRUE);
+			    THROW_DATA_CATCH_FRAME_SET(err, cfp + 1);
+			    hook_before_rewind(th, th->cfp, TRUE, err);
 			    rb_vm_pop_frame(th);
 			    goto finish_vme;
 			}
@@ -1929,8 +1970,7 @@ vm_exec(rb_thread_t *th)
 	    goto vm_loop_start;
 	}
 	else {
-	    /* skip frame */
+	    hook_before_rewind(th, th->cfp, FALSE, err);
-	    hook_before_rewind(th, th->cfp, FALSE);
 
 	    if (VM_FRAME_FINISHED_P(th->cfp)) {
 		rb_vm_pop_frame(th);

vm_insnhelper.h

@@ -196,18 +196,6 @@ THROW_DATA_NEW(VALUE val, const rb_control_frame_t *cf, VALUE st)
     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)
 {
@@ -220,10 +208,38 @@ THROW_DATA_CATCH_FRAME(const struct vm_throw_data *obj)
     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 */