Project

General

Profile

Actions

Bug #6460

closed

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

Added by ktsj (Kazuki Tsujimoto) almost 12 years ago. Updated over 11 years ago.

Status:
Closed
Target version:
-
ruby -v:
trunk
Backport:
[ruby-dev:45656]

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 @@ 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


Files

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

Updated by ko1 (Koichi Sasada) almost 12 years ago

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

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

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

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

Actions #2

Updated by ktsj (Kazuki Tsujimoto) over 11 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.


Actions #3

Updated by ktsj (Kazuki Tsujimoto) over 11 years ago

  • 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)

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

Updated by ktsj (Kazuki Tsujimoto) over 11 years ago

パッチを添付します。

Actions #5

Updated by naruse (Yui NARUSE) over 11 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. [ruby-dev:45656] [Bug #6460]

Updated by nobu (Nobuyoshi Nakada) over 11 years ago

  • 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

Updated by ktsj (Kazuki Tsujimoto) over 11 years 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

Actions #8

Updated by nobu (Nobuyoshi Nakada) over 11 years ago

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

Updated by nobu (Nobuyoshi Nakada) over 11 years ago

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

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

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0