Bug #6460: `unexpected return' occurs when a proc is called in ensure

• Status: Closed
• Assignee: naruse (Yui NARUSE)
• Target version: -
• ruby -v: trunk

[ktsj (Kazuki Tsujimoto)]

=begin
辻本です。

Bug #2729, #5234 の続きのような話になりますが
以下のコードでunexpected returnとなります。

class C
def each
begin
yield :foo
ensure
1.times { Proc.new }
end
end

def detect
each{|e|
r = yield(e)
return true if r
}
false
end
end

p C.new.detect{|e|
true
}

結局のところmake_envの際にはすべてのcontrol frameの
dfpの値をチェックしないといけないようです。

diff --git a/proc.c b/proc.c
index d44e8d8..07a904b 100644
--- a/proc.c
+++ b/proc.c
@@ -418,7 +418,6 @@ proc_new(VALUE klass, int is_lambda)
}

  procval = rb_vm_make_proc(th, block, klass);

• rb_vm_rewrite_dfp_in_errinfo(th, cfp);

  if (is_lambda) {
  rb_proc_t proc;
  diff --git a/vm.c b/vm.c
  index 6fbd3ad..abfe993 100644
  --- a/vm.c
  +++ b/vm.c
  @@ -406,8 +406,6 @@ vm_make_env_each(rb_thread_t * const th, rb_control_frame_t * const cfp,
  if (!RUBY_VM_NORMAL_ISEQ_P(cfp->iseq)) {
  / TODO */
  env->block.iseq = 0;

• } else {

• rb_vm_rewrite_dfp_in_errinfo(th, cfp);
  }
  return envval;
  }
  @@ -462,6 +460,7 @@ rb_vm_make_env_object(rb_thread_t * th, rb_control_frame_t *cfp)
  }

  envval = vm_make_env_each(th, cfp, cfp->dfp, cfp->lfp);

• rb_vm_rewrite_dfp_in_errinfo(th, th->cfp);

  if (PROCDEBUG) {
  check_env_value(envval);
  @@ -473,23 +472,26 @@ rb_vm_make_env_object(rb_thread_t * th, rb_control_frame_t *cfp)
  void
  rb_vm_rewrite_dfp_in_errinfo(rb_thread_t *th, rb_control_frame_t *cfp)
  {

• /* rewrite dfp 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->dfp[-2]; /* #$! */
• if (RB_TYPE_P(errinfo, T_NODE)) {

•  VALUE *escape_dfp = GET_THROWOBJ_CATCH_POINT(errinfo);
  

•  if (! ENV_IN_HEAP_P(th, escape_dfp)) {
  

•  VALUE dfpval = *escape_dfp;
  

•  if (CLASS_OF(dfpval) == rb_cEnv) {
  

•      rb_env_t *dfpenv;
  

•      GetEnvPtr(dfpval, dfpenv);
  

•      SET_THROWOBJ_CATCH_POINT(errinfo, (VALUE)(dfpenv->env + dfpenv->local_size));
  

• while (!RUBY_VM_CONTROL_FRAME_STACK_OVERFLOW_P(th, cfp)) {
• /* rewrite dfp 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->dfp[-2]; /* #$! */
  

•  if (RB_TYPE_P(errinfo, T_NODE)) {
  

•  VALUE *escape_dfp = GET_THROWOBJ_CATCH_POINT(errinfo);
  

•  if (! ENV_IN_HEAP_P(th, escape_dfp)) {
  

•      VALUE dfpval = *escape_dfp;
  

•      if (CLASS_OF(dfpval) == rb_cEnv) {
  

•  	rb_env_t *dfpenv;
  

•  	GetEnvPtr(dfpval, dfpenv);
  

•  	SET_THROWOBJ_CATCH_POINT(errinfo, (VALUE)(dfpenv->env + dfpenv->local_size));
  

•      }
   }
   }
  

  }

• }

• cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp);
• };
  }

void

近々この辺の構造が大きく変わるようなので
trunkにこのパッチを取り込む意義はあまり感じませんが、
1.9.3へのバックポートはしておいてもいいのではと思います。
どうでしょうか。
=end

[ko1 (Koichi Sasada)]

こちらも遅くなって済みません．

テストを trunk に，
パッチを 1.9.3 にバックポート，

が良いのではないかと思います．

テストは btest の KNOWNBUGS.rb かなぁ．

[ktsj (Kazuki Tsujimoto)]

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r36259.
Kazuki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• KNOWNBUGS.rb: add tests. [ruby-dev:45656] [Bug #6460]

[ktsj (Kazuki Tsujimoto)]

• Tracker changed from Bug to Backport
• Project changed from Ruby master to Backport193
• Category deleted (YARV)
• Status changed from Closed to Assigned
• Assignee changed from ko1 (Koichi Sasada) to naruse (Yui NARUSE)

バックポートチケット化しておきます。

[ktsj (Kazuki Tsujimoto)]

• File 6460-1_9_3.patch 6460-1_9_3.patch added

パッチを添付します。

[naruse (Yui NARUSE)]

• Status changed from Assigned to Closed

This issue was solved with changeset r36286.
Kazuki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

merge revision(s) 36259:

* KNOWNBUGS.rb: add tests. [ruby-dev:45656] [Bug #6460]

[nobu (Nobuyoshi Nakada)]

• Status changed from Closed to Open

=begin
これどうしましょうか。
最低限のパッチはこんな感じみたいです。

index d4d4ebb..3f31558 100644
--- i/vm.c
+++ w/vm.c
@@ -493,9 +494,20 @@ rb_vm_make_env_object(rb_thread_t * th, rb_control_frame_t *cfp)
return envval;
}

+static int vm_rewrite_ep_in_errinfo(rb_thread_t *th, rb_control_frame_t *cfp);
+
void
rb_vm_rewrite_ep_in_errinfo(rb_thread_t *th, rb_control_frame_t *cfp)
{

• while (!RUBY_VM_CONTROL_FRAME_STACK_OVERFLOW_P(th, cfp)) {
• if (vm_rewrite_ep_in_errinfo(th, cfp)) break;
• cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp);
• }
  +}

+static int
+vm_rewrite_ep_in_errinfo(rb_thread_t *th, rb_control_frame_t cfp)
+{
/ rewrite ep in errinfo to point to heap */
if (RUBY_VM_NORMAL_ISEQ_P(cfp->iseq) &&
(cfp->iseq->type == ISEQ_TYPE_RESCUE ||
@@ -509,10 +521,12 @@ rb_vm_rewrite_ep_in_errinfo(rb_thread_t *th, rb_control_frame_t *cfp)
rb_env_t *epenv;
GetEnvPtr(epval, epenv);
SET_THROWOBJ_CATCH_POINT(errinfo, (VALUE)(epenv->env + epenv->local_size));

•      return 1;
   }
   }
  

  }
  }
• return 0;
  }

void
=end

[ktsj (Kazuki Tsujimoto)]

=begin
辻本です。

なかださんのパッチだと、例えば以下のようなコードでunexpected returnとなってしまいます。

class C
def m1
m2{|e|
return
}
end

def m2
begin
yield :foo
ensure
begin
begin
yield :foo
ensure
Proc.new
raise ''
end
rescue
end
end
end
end

C.new.m1
=end

[nobu (Nobuyoshi Nakada)]

• Tracker changed from Backport to Bug
• Project changed from Backport193 to Ruby master

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed
• ruby -v set to trunk

やっぱり全部辿らないとまずいですね。
r37430でコミットしました。