Bug #6460

`unexpected return' occurs when a proc is called in ensure

Added by Kazuki Tsujimoto almost 2 years ago. Updated over 1 year ago.

[ruby-dev:45656]
Status:Closed
Priority:Normal
Assignee:Yui NARUSE
Category:-
Target version:-
ruby -v:trunk Backport:

Description

=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 @@ procnew(VALUE klass, int islambda)
}

  procval = rb_vm_make_proc(th, block, klass);
  • rbvmrewritedfpin_errinfo(th, cfp);

    if (islambda) {
    rb
    proct *proc;
    diff --git a/vm.c b/vm.c
    index 6fbd3ad..abfe993 100644
    --- a/vm.c
    +++ b/vm.c
    @@ -406,8 +406,6 @@ vm
    makeenveach(rbthreadt * const th, rbcontrolframet * const cfp,
    if (!RUBY
    VMNORMALISEQ_P(cfp->iseq)) {
    /* TODO */
    env->block.iseq = 0;

  • } else {

  • rbvmrewritedfpinerrinfo(th, cfp);
    }
    return envval;
    }
    @@ -462,6 +460,7 @@ rb
    vmmakeenvobject(rbthreadt * th, rbcontrolframet *cfp)
    }

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

  • rbvmrewritedfpin_errinfo(th, th->cfp);

    if (PROCDEBUG) {
    checkenvvalue(envval);
    @@ -473,23 +472,26 @@ rbvmmakeenvobject(rbthreadt * th, rbcontrolframet *cfp)
    void
    rb
    vmrewritedfpinerrinfo(rbthreadt *th, rbcontrolframe_t *cfp)
    {

  • /* rewrite dfp in errinfo to point to heap */

  • if (RUBYVMNORMALISEQP(cfp->iseq) &&

  • (cfp->iseq->type == ISEQTYPERESCUE ||

  • cfp->iseq->type == ISEQTYPEENSURE)) {

  • VALUE errinfo = cfp->dfp[-2]; /* #$! */

  • if (RBTYPEP(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 (!RUBYVMCONTROLFRAMESTACKOVERFLOWP(th, cfp)) {

  • /* rewrite dfp in errinfo to point to heap */

  • if (RUBYVMNORMALISEQP(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 = RUBYVMPREVIOUSCONTROLFRAME(cfp);

  • };
    }

    void

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

6460-1_9_3.patch Magnifier (3.6 KB) Kazuki Tsujimoto, 07/01/2012 07:13 PM

Associated revisions

Revision 36259
Added by Kazuki Tsujimoto almost 2 years ago

  • KNOWNBUGS.rb: add tests. [Bug #6460]

Revision 37430
Added by Nobuyoshi Nakada over 1 year ago

vm.c: rewrite all catch points

  • vm.c (rbvmrewriteepin_errinfo): rewrite all catch points in errinfo, not only the topmost frame. based on the patch by ktsj (Kazuki Tsujimoto) in . [Bug #6460]

History

#1 Updated by Koichi Sasada almost 2 years ago

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

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

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

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

#2 Updated by Kazuki Tsujimoto almost 2 years ago

  • 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. [Bug #6460]

#3 Updated by Kazuki Tsujimoto almost 2 years ago

  • Tracker changed from Bug to Backport
  • Project changed from ruby-trunk to Backport93
  • Category deleted (YARV)
  • Status changed from Closed to Assigned
  • Assignee changed from Koichi Sasada to Yui NARUSE

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

#4 Updated by Kazuki Tsujimoto almost 2 years ago

パッチを添付します。

#5 Updated by Yui NARUSE almost 2 years ago

  • 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.  [Bug #6460]

#6 Updated by Nobuyoshi Nakada over 1 year ago

  • Status changed from Closed to Open

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

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

+static int vmrewriteepinerrinfo(rbthreadt th, rbcontrolframet *cfp);
+
void
rb
vmrewriteepinerrinfo(rbthreadt *th, rbcontrolframet *cfp)
{
+ while (!RUBY
VMCONTROLFRAMESTACKOVERFLOWP(th, cfp)) {
+ if (vm
rewriteepinerrinfo(th, cfp)) break;
+ cfp = RUBY
VMPREVIOUSCONTROLFRAME(cfp);
+ }
+}
+
+static int
+vm
rewriteepinerrinfo(rbthreadt *th, rbcontrolframet *cfp)
+{
/
rewrite ep in errinfo to point to heap */
if (RUBYVMNORMALISEQP(cfp->iseq) &&
(cfp->iseq->type == ISEQTYPERESCUE ||
@@ -509,10 +521,12 @@ rbvmrewriteepinerrinfo(rbthreadt *th, rbcontrolframet *cfp)
rbenvt *epenv;
GetEnvPtr(epval, epenv);
SETTHROWOBJCATCHPOINT(errinfo, (VALUE)(epenv->env + epenv->localsize));
+ return 1;
}
}
}
}
+ return 0;
}

void
=end

#7 Updated by Kazuki Tsujimoto over 1 year ago

=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

#8 Updated by Nobuyoshi Nakada over 1 year ago

  • Tracker changed from Backport to Bug
  • Project changed from Backport93 to ruby-trunk

#9 Updated by Nobuyoshi Nakada over 1 year ago

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

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

Also available in: Atom PDF