[ruby-trunk - Bug #8964][Assigned] [BUG](%p is T_NONE) occurs while marking VM stack

Issue #8964 has been reported by ktsj (Kazuki Tsujimoto).


Bug #8964: [BUG](%p is T_NONE) occurs while marking VM stack

Author: ktsj (Kazuki Tsujimoto)
Status: Assigned
Priority: Normal
Assignee: ko1 (Koichi Sasada)
Category: YARV
Target version: current: 2.1.0
ruby -v: ruby 2.1.0dev (2013-09-28 trunk 43078) [x86_64-linux]
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN

=begin
以下のコードで (({rb_gc_mark(): 0x00555555a1ff68 is T_NONE})) のような[BUG]となります。

def m(a: [])
end

GC.stress=true
tap { m }
GC.start
tap { m }

フレームを積む際にキーワード引数用のVMスタック領域をCレベルで初期化していないため、
積んだ直後には死んだオブジェクトへの参照がスタックに残ってしまっていることがあります。
その後、Rubyレベルで初期化が行われるまでにマーキング処理が走ると上述の[BUG]となります。

vm_push_frame内で初期化を行うようにするパッチです。

diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 96b624d…6d522b4 100644
— a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -58,6 +58,15 @@ vm_push_frame(rb_thread_t *th,
*sp++ = Qnil;
}

  • /* initialize keyword arguments */
  • if (iseq && BUILTIN_TYPE(iseq) != T_NODE && iseq->arg_keyword !=
    -1) {
  • VALUE *const argv = sp - iseq->local_size;
  • VALUE ptr = argv + iseq->arg_size - 1 - (iseq->arg_block != -1); /
    argv+arg_size-(**1)-(&1) */
  • for (i = 0; i < iseq->arg_keywords; i++) {
  •  *--ptr = Qnil;
    
  • }
  • }
  • /* set special val */
    *sp = specval;
    =end

Issue #8964 has been updated by ktsj (Kazuki Tsujimoto).

File clear-keyword-args.patch added

=begin
不要な分岐を避けるために初期化処理はsetup_argでやったほうがよいのでないかと思い直したので、パッチを修正しました。
=end


Bug #8964: [BUG](%p is T_NONE) occurs while marking VM stack

Author: ktsj (Kazuki Tsujimoto)
Status: Assigned
Priority: Normal
Assignee: ko1 (Koichi Sasada)
Category: YARV
Target version: current: 2.1.0
ruby -v: ruby 2.1.0dev (2013-09-28 trunk 43078) [x86_64-linux]
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN

=begin
以下のコードで (({rb_gc_mark(): 0x00555555a1ff68 is T_NONE})) のような[BUG]となります。

def m(a: [])
end

GC.stress=true
tap { m }
GC.start
tap { m }

フレームを積む際にキーワード引数用のVMスタック領域をCレベルで初期化していないため、
積んだ直後には死んだオブジェクトへの参照がスタックに残ってしまっていることがあります。
その後、Rubyレベルで初期化が行われるまでにマーキング処理が走ると上述の[BUG]となります。

vm_push_frame内で初期化を行うようにするパッチです。

diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 96b624d…6d522b4 100644
— a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -58,6 +58,15 @@ vm_push_frame(rb_thread_t *th,
*sp++ = Qnil;
}

  • /* initialize keyword arguments */
  • if (iseq && BUILTIN_TYPE(iseq) != T_NODE && iseq->arg_keyword !=
    -1) {
  • VALUE *const argv = sp - iseq->local_size;
  • VALUE ptr = argv + iseq->arg_size - 1 - (iseq->arg_block != -1); /
    argv+arg_size-(**1)-(&1) */
  • for (i = 0; i < iseq->arg_keywords; i++) {
  •  *--ptr = Qnil;
    
  • }
  • }
  • /* set special val */
    *sp = specval;
    =end

Issue #8964 has been updated by nobu (Nobuyoshi N.).

これでいいんじゃないでしょうか。
vm_callee_setup_arg_complex()とvm_yield_setup_block_args()は共通化できるところも多そうですが。

Bug #8964: [BUG](%p is T_NONE) occurs while marking VM stack

Author: ktsj (Kazuki Tsujimoto)
Status: Assigned
Priority: Normal
Assignee: ko1 (Koichi Sasada)
Category: YARV
Target version: current: 2.1.0
ruby -v: ruby 2.1.0dev (2013-09-28 trunk 43078) [x86_64-linux]
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN

=begin
以下のコードで (({rb_gc_mark(): 0x00555555a1ff68 is T_NONE})) のような[BUG]となります。

def m(a: [])
end

GC.stress=true
tap { m }
GC.start
tap { m }

フレームを積む際にキーワード引数用のVMスタック領域をCレベルで初期化していないため、
積んだ直後には死んだオブジェクトへの参照がスタックに残ってしまっていることがあります。
その後、Rubyレベルで初期化が行われるまでにマーキング処理が走ると上述の[BUG]となります。

vm_push_frame内で初期化を行うようにするパッチです。

diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 96b624d…6d522b4 100644
— a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -58,6 +58,15 @@ vm_push_frame(rb_thread_t *th,
*sp++ = Qnil;
}

  • /* initialize keyword arguments */
  • if (iseq && BUILTIN_TYPE(iseq) != T_NODE && iseq->arg_keyword !=
    -1) {
  • VALUE *const argv = sp - iseq->local_size;
  • VALUE ptr = argv + iseq->arg_size - 1 - (iseq->arg_block != -1); /
    argv+arg_size-(**1)-(&1) */
  • for (i = 0; i < iseq->arg_keywords; i++) {
  •  *--ptr = Qnil;
    
  • }
  • }
  • /* set special val */
    *sp = specval;
    =end

Issue #8964 has been updated by ktsj (Kazuki Tsujimoto).

=begin
レビューありがとうございます。
とりあえず[ruby-dev:47730]のパッチでコミットしておきました。
=end


Bug #8964: [BUG](%p is T_NONE) occurs while marking VM stack

Author: ktsj (Kazuki Tsujimoto)
Status: Closed
Priority: Normal
Assignee: ko1 (Koichi Sasada)
Category: YARV
Target version: current: 2.1.0
ruby -v: ruby 2.1.0dev (2013-09-28 trunk 43078) [x86_64-linux]
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN

=begin
以下のコードで (({rb_gc_mark(): 0x00555555a1ff68 is T_NONE})) のような[BUG]となります。

def m(a: [])
end

GC.stress=true
tap { m }
GC.start
tap { m }

フレームを積む際にキーワード引数用のVMスタック領域をCレベルで初期化していないため、
積んだ直後には死んだオブジェクトへの参照がスタックに残ってしまっていることがあります。
その後、Rubyレベルで初期化が行われるまでにマーキング処理が走ると上述の[BUG]となります。

vm_push_frame内で初期化を行うようにするパッチです。

diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 96b624d…6d522b4 100644
— a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -58,6 +58,15 @@ vm_push_frame(rb_thread_t *th,
*sp++ = Qnil;
}

  • /* initialize keyword arguments */
  • if (iseq && BUILTIN_TYPE(iseq) != T_NODE && iseq->arg_keyword !=
    -1) {
  • VALUE *const argv = sp - iseq->local_size;
  • VALUE ptr = argv + iseq->arg_size - 1 - (iseq->arg_block != -1); /
    argv+arg_size-(**1)-(&1) */
  • for (i = 0; i < iseq->arg_keywords; i++) {
  •  *--ptr = Qnil;
    
  • }
  • }
  • /* set special val */
    *sp = specval;
    =end

(2013/09/29 9:08), nobu (Nobuyoshi N.) wrote:

これでいいんじゃないでしょうか。

と思います。

vm_callee_setup_arg_complex()とvm_yield_setup_block_args()は共通化できるところも多そうですが。

共通箇所の括りだしはいるかもしれないですね。