[ruby-dev:39592] infinite recursive call to C function

[Yusuke ENDOH]

遠藤です。

[ruby-core:24794] と似たような問題を探してみましたが、かなりありそうです。

$ ./ruby -e '
  class Symbol
    include Enumerable
    alias to_enum zip
  end
  :each.zip(:each) { }
'
Segmentation fault


$ ruby19 -e '
  class Hash
    alias default []
  end
  {}[1]
'
Segmentation fault


$ ruby19 -e '
  class Object
    alias to_open open
    public :to_open
  end
  o = Object.new
  eval("open(#{ (["o"] * 10000).join(",") })") # open(o, o, o, o, ...)
'
Segmentation fault


$ ruby19 -e '
  class Time
    def to_str
    end
  end
  Time.now <=> ""
'
Segmentation fault


matz ruby では、この手のバグはちまちまと潰していくということで
いいのでしょうか。
最後のは相互再帰するので rb_funcall_no_recursive では救えそうに
ありません。<=> での相互再帰は多分結構ありそうですが、どうしま
しょう。


Index: enum.c
===================================================================
--- enum.c (revision 25576)
+++ enum.c (working copy)
@@ -1745,7 +1745,11 @@
     if (!allary) {
  CONST_ID(conv, "to_enum");
  for (i=0; i<argc; i++) {
-    argv[i] = rb_funcall(argv[i], conv, 1, ID2SYM(id_each));
+    VALUE sym = ID2SYM(id_each);
+    argv[i] = rb_funcall_no_recursive(argv[i], conv, 1, &sym, enum_zip);
+    if (argv[i] == Qundef) {
+ rb_raise(rb_eRuntimeError, "recursive call to Enumerable#zip");
+    }
  }
     }
     if (!rb_block_given_p()) {
Index: io.c
===================================================================
--- io.c (revision 25576)
+++ io.c (working copy)
@@ -5589,7 +5589,10 @@
  }
     }
     if (redirect) {
- VALUE io = rb_funcall2(argv[0], to_open, argc-1, argv+1);
+ VALUE io = rb_funcall_no_recursive(argv[0], to_open, argc-1, argv+1,
rb_f_open);
+ if (io == Qundef) {
+    rb_raise(rb_eRuntimeError, "recursive call to Kernel#open");
+ }

  if (rb_block_given_p()) {
     return rb_ensure(rb_yield, io, io_close, io);
Index: hash.c
===================================================================
--- hash.c (revision 25576)
+++ hash.c (working copy)
@@ -494,7 +494,11 @@
     VALUE val;

     if (!RHASH(hash)->ntbl || !st_lookup(RHASH(hash)->ntbl, key, &val)) {
- return rb_funcall(hash, id_default, 1, key);
+ VALUE ret = rb_funcall_no_recursive(hash, id_default, 1, &key, rb_hash_aref);
+ if (ret == Qundef) {
+    rb_raise(rb_eRuntimeError, "recursive call to Hash#[]");
+ }
+ return ret;
     }
     return val;
 }

--
Yusuke ENDOH <mame@...>

[Yukihiro Matsumoto]

まつもと ゆきひろです

In message "Re: [ruby-dev:39592] infinite recursive call to C function"
    on Fri, 30 Oct 2009 21:46:20 +0900, Yusuke ENDOH <mame@...> writes:

|[ruby-core:24794] と似たような問題を探してみましたが、かなりありそうです。

|matz ruby では、この手のバグはちまちまと潰していくということで
|いいのでしょうか。
|最後のは相互再帰するので rb_funcall_no_recursive では救えそうに
|ありません。<=> での相互再帰は多分結構ありそうですが、どうしま
|しょう。

対処した「これを仕様にしない」と言ったのは、きりがないことが
容易に予想できたためです。ので、対応が簡単なものは対応します
が、全部ちまちまと対応するするつもりはありません。絶対に終わ
らないし。正直、rb_funcall_no_recursive()についても、あくまで
も実験のつもりで、うまくいかないようならrevertするつもりです。

で、いずれにしてもこの無限再帰がSEGVになることの方を先に対処
すべきだと思います。なかださんによると、これ自体がバグのよう
だし。

[Yusuke ENDOH]

遠藤です。

2009年10月31日0:01 Yukihiro Matsumoto <matz@...>:

> |matz ruby では、この手のバグはちまちまと潰していくということで
> |いいのでしょうか。
> |最後のは相互再帰するので rb_funcall_no_recursive では救えそうに
> |ありません。<=> での相互再帰は多分結構ありそうですが、どうしま
> |しょう。
>
> 対処した「これを仕様にしない」と言ったのは、きりがないことが
> 容易に予想できたためです。ので、対応が簡単なものは対応します
> が、全部ちまちまと対応するするつもりはありません。絶対に終わ
> らないし。正直、rb_funcall_no_recursive()についても、あくまで
> も実験のつもりで、うまくいかないようならrevertするつもりです。

了解しました。
では、もし rb_funcall_no_recursive() が落ち着いたら入れます、
と思ったらもう消されてました。活発な開発。


> で、いずれにしてもこの無限再帰がSEGVになることの方を先に対処
> すべきだと思います。なかださんによると、これ自体がバグのよう
> だし。

そういえば sigaltstack を使ってましたね。

--
Yusuke ENDOH <mame@...>

[Nobuyoshi Nakada-2]

なかだです。

At Sat, 31 Oct 2009 00:01:52 +0900,
Yukihiro Matsumoto wrote in [ruby-dev:39596]:
> で、いずれにしてもこの無限再帰がSEGVになることの方を先に対処
> すべきだと思います。なかださんによると、これ自体がバグのよう
> だし。

r23720をrevertすればSEGVしなくなります。rb_longjmp()中の
rb_make_backtrace()でGCが起きて、代替スタックから通常のスタック
までsweepしようとして落ちている感じです。

たとえばrb_make_backtrace()からset_backtrace()までGCを禁止するこ
とでも落ちなくなりますが、そもそもmallocの途中でシグナルを受けて
いることもありうるわけで、無条件に復帰可能な例外を投げるのも危険
な気がします。

--
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦

[Yukihiro Matsumoto]

まつもと ゆきひろです

In message "Re: [ruby-dev:39599] Re: infinite recursive call to C function"
    on Mon, 2 Nov 2009 10:15:30 +0900, Nobuyoshi Nakada <nobu@...> writes:

|r23720をrevertすればSEGVしなくなります。rb_longjmp()中の
|rb_make_backtrace()でGCが起きて、代替スタックから通常のスタック
|までsweepしようとして落ちている感じです。

scan?

|たとえばrb_make_backtrace()からset_backtrace()までGCを禁止するこ
|とでも落ちなくなりますが、そもそもmallocの途中でシグナルを受けて
|いることもありうるわけで、無条件に復帰可能な例外を投げるのも危険
|な気がします。

うーむ、とういうことは、

  * 代替スタック中ではRubyコードを実行しない(r23720をリバー
    トする)
  * 代替スタック使用中はスタック先頭アドレスも書き換える

のいずれかが必要ということですか。

それよりもむしろrb_longjmp でのbacktraceの割り当てをlazyにす
べきなのかなあ。でも、バックとレース情報を記録する場所はいず
れにせよ必要なわけだし...。

[Nobuyoshi Nakada-2]

なかだです。

At Mon, 2 Nov 2009 11:52:04 +0900,
Yukihiro Matsumoto wrote in [ruby-dev:39601]:
> |までsweepしようとして落ちている感じです。
>
> scan?

でした。GCは詳しくないもので。

> |たとえばrb_make_backtrace()からset_backtrace()までGCを禁止するこ
> |とでも落ちなくなりますが、そもそもmallocの途中でシグナルを受けて
> |いることもありうるわけで、無条件に復帰可能な例外を投げるのも危険
> |な気がします。
>
> うーむ、とういうことは、
>
>   * 代替スタック中ではRubyコードを実行しない(r23720をリバー
>     トする)
>   * 代替スタック使用中はスタック先頭アドレスも書き換える
>
> のいずれかが必要ということですか。

前者のほうがいいんじゃないでしょうか。代替スタック使用中のGCでス
タックオーバフローしたりとか考えると。

> それよりもむしろrb_longjmp でのbacktraceの割り当てをlazyにす
> べきなのかなあ。でも、バックとレース情報を記録する場所はいず
> れにせよ必要なわけだし...。

「バックトレース情報を記録する場所」というのは?

とりあえずJUMP_TAGのときにチェックさせるようにしてみました。


Index: eval_error.c
===================================================================
--- eval_error.c (revision 25629)
+++ eval_error.c (working copy)
@@ -273,2 +273,14 @@ error_handle(int ex)
     return status;
 }
+
+void
+ruby_threadptr_handle_critical_exception(rb_thread_t *th)
+{
+    VALUE e = th->errinfo, bt;
+    if (!OBJ_FROZEN(e)) return;
+    if (ruby_threadptr_stack_check(th)) return;
+    bt = rb_threadptr_backtrace(th, -1);
+    th->errinfo = e = rb_obj_dup(e);
+    set_backtrace(e, bt);
+    rb_thread_raised_reset(th, RAISED_STACKOVERFLOW|RAISED_NOMEMORY);
+}
Index: eval_intern.h
===================================================================
--- eval_intern.h (revision 25629)
+++ eval_intern.h (working copy)
@@ -131,5 +131,7 @@ NORETURN(void _longjmp(jmp_buf, int));
 
 #define TH_JUMP_TAG(th, st) do { \
-  ruby_longjmp(th->tag->buf,(st)); \
+    if (rb_thread_raised_p(th, RAISED_STACKOVERFLOW|RAISED_NOMEMORY)) \
+ ruby_threadptr_handle_critical_exception(th); \
+    ruby_longjmp(th->tag->buf,(st)); \
 } while (0)
 
@@ -192,4 +194,6 @@ int rb_threadptr_reset_raised(rb_thread_
 #define rb_thread_raised_p(th, f)     (((th)->raised_flag & (f)) != 0)
 #define rb_thread_raised_clear(th)    ((th)->raised_flag = 0)
+void ruby_threadptr_handle_critical_exception(rb_thread_t *th);
+int ruby_threadptr_stack_check(rb_thread_t *th);
 
 VALUE rb_f_eval(int argc, VALUE *argv, VALUE self);
Index: gc.c
===================================================================
--- gc.c (revision 25629)
+++ gc.c (working copy)
@@ -1205,9 +1205,8 @@ ruby_stack_length(VALUE **p)
 }
 
-static int
-stack_check(void)
+int
+ruby_threadptr_stack_check(rb_thread_t *th)
 {
     int ret;
-    rb_thread_t *th = GET_THREAD();
     SET_STACK_END;
     ret = STACK_LENGTH > STACK_LEVEL_MAX - GC_WATER_MARK;
@@ -1221,4 +1220,10 @@ stack_check(void)
 }
 
+static inline int
+stack_check(void)
+{
+    return ruby_threadptr_stack_check(GET_THREAD());
+}
+
 int
 ruby_stack_check(void)
Index: signal.c
===================================================================
--- signal.c (revision 25629)
+++ signal.c (working copy)
@@ -599,6 +599,4 @@ sigsegv(int sig SIGINFO_ARG)
 {
 #ifdef USE_SIGALTSTACK
-    int ruby_stack_overflowed_p(const rb_thread_t *, const void *);
-    NORETURN(void ruby_thread_stack_overflow(rb_thread_t *th));
     rb_thread_t *th = GET_THREAD();
     if (ruby_stack_overflowed_p(th, info->si_addr)) {
Index: thread.c
===================================================================
--- thread.c (revision 25629)
+++ thread.c (working copy)
@@ -1347,12 +1347,7 @@ void
 ruby_thread_stack_overflow(rb_thread_t *th)
 {
-    th->raised_flag = 0;
-#ifdef USE_SIGALTSTACK
-    th->raised_flag = 0;
-    rb_exc_raise(sysstack_error);
-#else
+    th->raised_flag = RAISED_STACKOVERFLOW;
     th->errinfo = sysstack_error;
-    TH_JUMP_TAG(th, TAG_RAISE);
-#endif
+    ruby_longjmp(th->tag->buf, TAG_RAISE);
 }
 
Index: vm_core.h
===================================================================
--- vm_core.h (revision 25629)
+++ vm_core.h (working copy)
@@ -616,6 +616,8 @@ void *rb_thread_call_with_gvl(void *(*fu
 int ruby_thread_has_gvl_p(void);
 VALUE rb_make_backtrace(void);
+VALUE rb_threadptr_backtrace(rb_thread_t *th, int lev);
 typedef int rb_backtrace_iter_func(void *, VALUE, int, VALUE);
 int rb_backtrace_each(rb_backtrace_iter_func *iter, void *arg);
+int rb_threadptr_backtrace_each(rb_thread_t *th, int lev, rb_backtrace_iter_func *iter, void *arg);
 rb_control_frame_t *rb_vm_get_ruby_level_next_cfp(rb_thread_t *th, rb_control_frame_t *cfp);
 VALUE rb_name_err_mesg_new(VALUE obj, VALUE mesg, VALUE recv, VALUE method);
@@ -654,4 +656,6 @@ void rb_threadptr_signal_raise(rb_thread
 void rb_threadptr_signal_exit(rb_thread_t *th);
 void rb_threadptr_execute_interrupts(rb_thread_t *);
+int ruby_stack_overflowed_p(const rb_thread_t *, const void *);
+void ruby_thread_stack_overflow(rb_thread_t *th);
 
 #define RUBY_VM_CHECK_INTS_TH(th) do { \
Index: vm_eval.c
===================================================================
--- vm_eval.c (revision 25629)
+++ vm_eval.c (working copy)
@@ -192,11 +192,8 @@ rb_call_super(int argc, const VALUE *arg
 
 static inline void
-stack_check(void)
+stack_check(rb_thread_t *th)
 {
-    rb_thread_t *th = GET_THREAD();
-
     if (!rb_thread_raised_p(th, RAISED_STACKOVERFLOW) && ruby_stack_check()) {
- rb_thread_raised_set(th, RAISED_STACKOVERFLOW);
- rb_exc_raise(sysstack_error);
+ ruby_thread_stack_overflow(th);
     }
 }
@@ -231,5 +228,5 @@ rb_call0(VALUE recv, ID mid, int argc, c
  return method_missing(recv, mid, argc, argv, call_status);
     }
-    stack_check();
+    stack_check(th);
     return vm_call0(th, recv, mid, argc, argv, me);
 }
@@ -286,5 +283,5 @@ check_funcall(VALUE recv, ID mid, int ar
  }
     }
-    stack_check();
+    stack_check(th);
     return vm_call0(th, recv, mid, argc, argv, me);
 }
@@ -434,5 +431,5 @@ raise_method_missing(rb_thread_t *th, in
     }
 
-    stack_check();
+    stack_check(th);
 
     id = SYM2ID(argv[0]);
@@ -1524,4 +1521,16 @@ rb_thread_backtrace(VALUE thval)
 
 int
+rb_threadptr_backtrace_each(rb_thread_t *th, int lev, rb_backtrace_iter_func *iter, void *arg)
+{
+    return vm_backtrace_each(th, lev, iter, arg);
+}
+
+VALUE
+rb_threadptr_backtrace(rb_thread_t *th, int lev)
+{
+    return vm_backtrace(th, lev);
+}
+
+int
 rb_backtrace_each(rb_backtrace_iter_func *iter, void *arg)
 {

[Yukihiro Matsumoto]

まつもと ゆきひろです

In message "Re: [ruby-dev:39602] Re: infinite recursive call to C function"
    on Mon, 2 Nov 2009 15:36:51 +0900, Nobuyoshi Nakada <nobu@...> writes:

|>   * 代替スタック中ではRubyコードを実行しない(r23720をリバー
|>     トする)
|>   * 代替スタック使用中はスタック先頭アドレスも書き換える
|>
|> のいずれかが必要ということですか。
|
|前者のほうがいいんじゃないでしょうか。代替スタック使用中のGCでス
|タックオーバフローしたりとか考えると。

とすると、[ruby-core:23813]が再発するんじゃないかが心配なの
ですが。

|> それよりもむしろrb_longjmp でのbacktraceの割り当てをlazyにす
|> べきなのかなあ。でも、バックとレース情報を記録する場所はいず
|> れにせよ必要なわけだし...。
|
|「バックトレース情報を記録する場所」というのは?

現在はbacktrace情報を文字列の配列としてrb_longjmpの中で生成し
ていますが、これはかならずしも必要とは限りません。たとえば、
例外をrecueでハンドルした場合には、このオブジェクトは使われず
に終わります。ので、「どのファイル」、「何行目」、「どのメソッ
ド」の情報(×呼び出しネスト数)だけどこかに保存しておければ、
無駄なオブジェクト割り当てが削れるはずなんですが。「どこに保
存するか」について良いアイディアがないんですよねえ。

|とりあえずJUMP_TAGのときにチェックさせるようにしてみました。

これを当てると、r23720をrevertしなくてもよい、というわけでは
ないんでしょうね。

[Nobuyoshi Nakada-2]

なかだです。

At Mon, 2 Nov 2009 16:00:53 +0900,
Yukihiro Matsumoto wrote in [ruby-dev:39603]:

> |> それよりもむしろrb_longjmp でのbacktraceの割り当てをlazyにす
> |> べきなのかなあ。でも、バックとレース情報を記録する場所はいず
> |> れにせよ必要なわけだし...。
> |
> |「バックトレース情報を記録する場所」というのは?
>
> 現在はbacktrace情報を文字列の配列としてrb_longjmpの中で生成し
> ていますが、これはかならずしも必要とは限りません。たとえば、
> 例外をrecueでハンドルした場合には、このオブジェクトは使われず
> に終わります。ので、「どのファイル」、「何行目」、「どのメソッ
> ド」の情報(×呼び出しネスト数)だけどこかに保存しておければ、
> 無駄なオブジェクト割り当てが削れるはずなんですが。「どこに保
> 存するか」について良いアイディアがないんですよねえ。

専用のスタックにメソッドの呼び出しごとに積んでいくとかですか。

> |とりあえずJUMP_TAGのときにチェックさせるようにしてみました。
>
> これを当てると、r23720をrevertしなくてもよい、というわけでは
> ないんでしょうね。

    if (ruby_threadptr_stack_check(th)) return;

と、ruby_threadptr_handle_critical_exception()でスタックにある程
度余裕ができるまで待っています。単純にr23720をrevertするだけでも、
バックトレースが出なくなるだけでSEGVはしないようですが。

--
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦

[Yukihiro Matsumoto]

まつもと ゆきひろです

In message "Re: [ruby-dev:39605] Re: infinite recursive call to C function"
    on Mon, 2 Nov 2009 17:36:42 +0900, Nobuyoshi Nakada <nobu@...> writes:

|> に終わります。ので、「どのファイル」、「何行目」、「どのメソッ
|> ド」の情報(×呼び出しネスト数)だけどこかに保存しておければ、
|> 無駄なオブジェクト割り当てが削れるはずなんですが。「どこに保
|> 存するか」について良いアイディアがないんですよねえ。
|
|専用のスタックにメソッドの呼び出しごとに積んでいくとかですか。

そんな感じですね。

|> これを当てると、r23720をrevertしなくてもよい、というわけでは
|> ないんでしょうね。
|
|    if (ruby_threadptr_stack_check(th)) return;
|
|と、ruby_threadptr_handle_critical_exception()でスタックにある程
|度余裕ができるまで待っています。単純にr23720をrevertするだけでも、
|バックトレースが出なくなるだけでSEGVはしないようですが。

ちょっと確認させてください。無限再帰でSEGVするのは、
[ruby-dev:39602]のパッチとr23720 のrevertで解決するんですか？
であれば、[ruby-dev:39602]のコミットに賛成します。解決しなく
ても、そんなに悪いパッチではなさそうですが。

元々の「sizeをcountのalias」では発生しなくなってしまったので、
遠藤さんが[ruby-dev:39592]で紹介した

  class Symbol
    include Enumerable
    alias to_enum zip
  end
  :each.zip(:each) { }

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

[Nobuyoshi Nakada-2]

なかだです。

At Mon, 2 Nov 2009 18:27:59 +0900,
Yukihiro Matsumoto wrote in [ruby-dev:39608]:
> ちょっと確認させてください。無限再帰でSEGVするのは、
> [ruby-dev:39602]のパッチとr23720 のrevertで解決するんですか？
> であれば、[ruby-dev:39602]のコミットに賛成します。解決しなく
> ても、そんなに悪いパッチではなさそうですが。

[ruby-dev:39602]のパッチはr23720のrevertを含んでいます。

> 元々の「sizeをcountのalias」では発生しなくなってしまったので、
> 遠藤さんが[ruby-dev:39592]で紹介した
>
>   class Symbol
>     include Enumerable
>     alias to_enum zip
>   end
>   :each.zip(:each) { }
>
> とかが良いのではないかと思います。

この例ではSEGVはしませんが、バックトレースも出ませんでした。
[ruby-dev:39593]の例では出るのですが。ということで少々改善。


Index: eval_error.c
===================================================================
--- eval_error.c (revision 25629)
+++ eval_error.c (working copy)
@@ -273,2 +273,14 @@ error_handle(int ex)
     return status;
 }
+
+void
+ruby_threadptr_handle_critical_exception(rb_thread_t *th)
+{
+    VALUE e = th->errinfo, bt;
+    if (!OBJ_FROZEN(e)) return;
+    if (ruby_threadptr_stack_check(th)) return;
+    bt = rb_threadptr_backtrace(th, -1);
+    th->errinfo = e = rb_obj_dup(e);
+    set_backtrace(e, bt);
+    rb_thread_raised_reset(th, RAISED_STACKOVERFLOW|RAISED_NOMEMORY);
+}
Index: eval_intern.h
===================================================================
--- eval_intern.h (revision 25629)
+++ eval_intern.h (working copy)
@@ -130,5 +130,10 @@ NORETURN(void _longjmp(jmp_buf, int));
   TH_EXEC_TAG()
 
+#define HANDLE_CRITICAL_EXCEPTION(th) \
+    (rb_thread_raised_p(th, RAISED_STACKOVERFLOW|RAISED_NOMEMORY) ? \
+     ruby_threadptr_handle_critical_exception(th) : (void)0)
+
 #define TH_JUMP_TAG(th, st) do { \
+  HANDLE_CRITICAL_EXCEPTION(th); \
   ruby_longjmp(th->tag->buf,(st)); \
 } while (0)
@@ -192,4 +197,6 @@ int rb_threadptr_reset_raised(rb_thread_
 #define rb_thread_raised_p(th, f)     (((th)->raised_flag & (f)) != 0)
 #define rb_thread_raised_clear(th)    ((th)->raised_flag = 0)
+void ruby_threadptr_handle_critical_exception(rb_thread_t *th);
+int ruby_threadptr_stack_check(rb_thread_t *th);
 
 VALUE rb_f_eval(int argc, VALUE *argv, VALUE self);
Index: gc.c
===================================================================
--- gc.c (revision 25629)
+++ gc.c (working copy)
@@ -1205,9 +1205,8 @@ ruby_stack_length(VALUE **p)
 }
 
-static int
-stack_check(void)
+int
+ruby_threadptr_stack_check(rb_thread_t *th)
 {
     int ret;
-    rb_thread_t *th = GET_THREAD();
     SET_STACK_END;
     ret = STACK_LENGTH > STACK_LEVEL_MAX - GC_WATER_MARK;
@@ -1221,4 +1220,10 @@ stack_check(void)
 }
 
+static inline int
+stack_check(void)
+{
+    return ruby_threadptr_stack_check(GET_THREAD());
+}
+
 int
 ruby_stack_check(void)
Index: signal.c
===================================================================
--- signal.c (revision 25629)
+++ signal.c (working copy)
@@ -599,6 +599,4 @@ sigsegv(int sig SIGINFO_ARG)
 {
 #ifdef USE_SIGALTSTACK
-    int ruby_stack_overflowed_p(const rb_thread_t *, const void *);
-    NORETURN(void ruby_thread_stack_overflow(rb_thread_t *th));
     rb_thread_t *th = GET_THREAD();
     if (ruby_stack_overflowed_p(th, info->si_addr)) {
Index: thread.c
===================================================================
--- thread.c (revision 25629)
+++ thread.c (working copy)
@@ -1347,12 +1347,7 @@ void
 ruby_thread_stack_overflow(rb_thread_t *th)
 {
-    th->raised_flag = 0;
-#ifdef USE_SIGALTSTACK
-    th->raised_flag = 0;
-    rb_exc_raise(sysstack_error);
-#else
+    th->raised_flag = RAISED_STACKOVERFLOW;
     th->errinfo = sysstack_error;
-    TH_JUMP_TAG(th, TAG_RAISE);
-#endif
+    ruby_longjmp(th->tag->buf, TAG_RAISE);
 }
 
Index: vm.c
===================================================================
--- vm.c (revision 25629)
+++ vm.c (working copy)
@@ -1132,4 +1132,5 @@ vm_exec(rb_thread_t *th)
  VALUE type;
 
+ HANDLE_CRITICAL_EXCEPTION(th);
  err = th->errinfo;
 
Index: vm_core.h
===================================================================
--- vm_core.h (revision 25629)
+++ vm_core.h (working copy)
@@ -616,6 +616,8 @@ void *rb_thread_call_with_gvl(void *(*fu
 int ruby_thread_has_gvl_p(void);
 VALUE rb_make_backtrace(void);
+VALUE rb_threadptr_backtrace(rb_thread_t *th, int lev);
 typedef int rb_backtrace_iter_func(void *, VALUE, int, VALUE);
 int rb_backtrace_each(rb_backtrace_iter_func *iter, void *arg);
+int rb_threadptr_backtrace_each(rb_thread_t *th, int lev, rb_backtrace_iter_func *iter, void *arg);
 rb_control_frame_t *rb_vm_get_ruby_level_next_cfp(rb_thread_t *th, rb_control_frame_t *cfp);
 VALUE rb_name_err_mesg_new(VALUE obj, VALUE mesg, VALUE recv, VALUE method);
@@ -654,4 +656,6 @@ void rb_threadptr_signal_raise(rb_thread
 void rb_threadptr_signal_exit(rb_thread_t *th);
 void rb_threadptr_execute_interrupts(rb_thread_t *);
+int ruby_stack_overflowed_p(const rb_thread_t *, const void *);
+void ruby_thread_stack_overflow(rb_thread_t *th);
 
 #define RUBY_VM_CHECK_INTS_TH(th) do { \
Index: vm_eval.c
===================================================================
--- vm_eval.c (revision 25629)
+++ vm_eval.c (working copy)
@@ -192,11 +192,8 @@ rb_call_super(int argc, const VALUE *arg
 
 static inline void
-stack_check(void)
+stack_check(rb_thread_t *th)
 {
-    rb_thread_t *th = GET_THREAD();
-
     if (!rb_thread_raised_p(th, RAISED_STACKOVERFLOW) && ruby_stack_check()) {
- rb_thread_raised_set(th, RAISED_STACKOVERFLOW);
- rb_exc_raise(sysstack_error);
+ ruby_thread_stack_overflow(th);
     }
 }
@@ -231,5 +228,5 @@ rb_call0(VALUE recv, ID mid, int argc, c
  return method_missing(recv, mid, argc, argv, call_status);
     }
-    stack_check();
+    stack_check(th);
     return vm_call0(th, recv, mid, argc, argv, me);
 }
@@ -286,5 +283,5 @@ check_funcall(VALUE recv, ID mid, int ar
  }
     }
-    stack_check();
+    stack_check(th);
     return vm_call0(th, recv, mid, argc, argv, me);
 }
@@ -434,5 +431,5 @@ raise_method_missing(rb_thread_t *th, in
     }
 
-    stack_check();
+    stack_check(th);
 
     id = SYM2ID(argv[0]);
@@ -1524,4 +1521,16 @@ rb_thread_backtrace(VALUE thval)
 
 int
+rb_threadptr_backtrace_each(rb_thread_t *th, int lev, rb_backtrace_iter_func *iter, void *arg)
+{
+    return vm_backtrace_each(th, lev, iter, arg);
+}
+
+VALUE
+rb_threadptr_backtrace(rb_thread_t *th, int lev)
+{
+    return vm_backtrace(th, lev);
+}
+
+int
 rb_backtrace_each(rb_backtrace_iter_func *iter, void *arg)
 {


--
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦

[Yukihiro Matsumoto]

まつもと ゆきひろです

In message "Re: [ruby-dev:39609] Re: infinite recursive call to C function"
    on Mon, 2 Nov 2009 23:27:44 +0900, Nobuyoshi Nakada <nobu@...> writes:

|[ruby-dev:39602]のパッチはr23720のrevertを含んでいます。

|>   class Symbol
|>     include Enumerable
|>     alias to_enum zip
|>   end
|>   :each.zip(:each) { }
|>
|> とかが良いのではないかと思います。
|
|この例ではSEGVはしませんが、バックトレースも出ませんでした。
|[ruby-dev:39593]の例では出るのですが。ということで少々改善。

このパッチを当てても、私のところではSEGVしました。なにが違う
のかな。valgrindで実行したところ、vm_push_frameがあらかじめ割
り当てたスタック領域を越えたところまでフレームをpushしている
ようです。

で、CHECK_STACK_OVERFLOW()をあちこち追加してみましたが、現象
は変わらないようです。この辺、よくわからない。

[Nobuyoshi Nakada-2]

なかだです。

At Tue, 3 Nov 2009 00:05:09 +0900,
Yukihiro Matsumoto wrote in [ruby-dev:39610]:
> このパッチを当てても、私のところではSEGVしました。なにが違う
> のかな。valgrindで実行したところ、vm_push_frameがあらかじめ割
> り当てたスタック領域を越えたところまでフレームをpushしている
> ようです。

vm_push_frameということは、マシンスタックじゃなくてVMスタックで
すか?

--
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦

[Yukihiro Matsumoto]

まつもと ゆきひろです

In message "Re: [ruby-dev:39613] Re: infinite recursive call to C function"
    on Tue, 3 Nov 2009 02:18:25 +0900, Nobuyoshi Nakada <nobu@...> writes:

|At Tue, 3 Nov 2009 00:05:09 +0900,
|Yukihiro Matsumoto wrote in [ruby-dev:39610]:
|> このパッチを当てても、私のところではSEGVしました。なにが違う
|> のかな。valgrindで実行したところ、vm_push_frameがあらかじめ割
|> り当てたスタック領域を越えたところまでフレームをpushしている
|> ようです。
|
|vm_push_frameということは、マシンスタックじゃなくてVMスタックで
|すか?

そうです。

[SASADA Koichi]

　ささだです．

Yukihiro Matsumoto wrote::
> |vm_push_frameということは、マシンスタックじゃなくてVMスタックで
> |すか?
>
> そうです。

　ほぼ余談です．

　VM のスタック領域チェックは，数を減らすために色々頑張ってるんで，ミス
があるかもしれないですねぇ．でも，C マシンスタックのオーバーフローと違っ
て，ミスがなければ確実に trap 出来るから，この元々の問題である「C での再
帰が SEGV」を抜本的に（移植性を考えて）解決するためには，徐々に Ruby で
書き換えていく必要があるんですかねぇ．

　ちなみに，VM スタックオーバーフローしたら，VM スタックを伸張する，って
ハックをやりたいんですが，時間が無い．誰かやりませんかね．来年，うちに新
入生が入ってきたら，練習でやってもらおうとは思うんですが，1.9.3 あたりに
入るかなぁ．これが入ると，いろいろと効率的になるんですが．ついでにスタッ
クGCとかも出来るな（これは，tail call optimizationの話だけど．そういえ
ば，その議論もpendingだなぁ）．

--
// SASADA Koichi at atdot dot net

[Yukihiro Matsumoto]

まつもと ゆきひろです

In message "Re: [ruby-dev:39616] Re: infinite recursive call to C function"
    on Tue, 3 Nov 2009 09:53:40 +0900, SASADA Koichi <ko1@...> writes:

|　ちなみに，VM スタックオーバーフローしたら，VM スタックを伸張する，って
|ハックをやりたいんですが，時間が無い．誰かやりませんかね．

cfpとかアドレスが直接入っていたような気がするんで、フレーム
をたどってアドレス書き換える必要があるんですかね。

[SASADA Koichi]

　ささだです．

Yukihiro Matsumoto wrote::
> |　ちなみに，VM スタックオーバーフローしたら，VM スタックを伸張する，って
> |ハックをやりたいんですが，時間が無い．誰かやりませんかね．
>
> cfpとかアドレスが直接入っていたような気がするんで、フレーム
> をたどってアドレス書き換える必要があるんですかね。

　cfp を弄るのは一箇所（rb_thread_tだけ）なんで一瞬ですけど，lfp, dfp を
弄るのが大変そうです．逆に言うと，それだけという気もします．全部 heap に
追い出してしまうってのも手ですが．

　あ，ローカル変数に cfp 保存してる関数がいるので，それは問題だ．どうし
たもんかな．cfp は index にしないと駄目か？　そのオーバヘッドを含めてペ
イするだろうか．

# あと，無限再帰バグが検知出来なくなるのは問題なので，
# 閾値をどこにするか，とか．

--
// SASADA Koichi at atdot dot net

[Yukihiro Matsumoto]

まつもと ゆきひろです

In message "Re: [ruby-dev:39615] Re: infinite recursive call to C function"
    on Tue, 3 Nov 2009 09:32:45 +0900, Yukihiro Matsumoto <matz@...> writes:

||vm_push_frameということは、マシンスタックじゃなくてVMスタックで
||すか?
|
|そうです。

同じLinuxのはずなのに、振る舞いが違うのはおかしいなと思って
いたのですが、中田さんのところではマシンスタックのリミットが
先に来て、私のところではVMスタックリミットが先に来ているとい
うことのようですね。limitとか設定しますか？

こちらではVMスタックのチェックを加える方向で作業しようと思い
ます。マシンスタックの方はおまかせします。