Bug #7774

IFUNC上のbinding呼び出しでSEGV

Added by Kazuki Tsujimoto over 2 years ago. Updated over 2 years ago.

[ruby-dev:46908]
Status:Closed
Priority:Normal
Assignee:Koichi Sasada
ruby -v:ruby 2.0.0dev (2013-02-03 trunk 39032) [x86_64-linux] Backport:

Description

=begin
辻本です。

以下のコードでSEGVします。

tp = TracePoint.new(:raise) do |tp|
tp.binding
end
tp.enable

@obj = Object.new
class << @obj
include Enumerable
def each
yield 1
end
end

@obj.zip({}) {}

バックトレースは以下の通り。

#0 0x00007ffff6ef7445 in raise () from /lib/x86_64-linux-gnu/libc.so.6
#1 0x00007ffff6efabab in abort () from /lib/x86_64-linux-gnu/libc.so.6
#2 0x00005555555ad998 in rb_bug (fmt=0x55555573ecd7 "Segmentation fault") at error.c:309
#3 0x000055555567f32f in sigsegv (sig=11, info=0x555555a5aa70, ctx=0x555555a5a940) at signal.c:649
#4
#5 0x00005555556ea894 in VM_EP_LEP (ep=0xc) at vm.c:28
#6 0x00005555556ea8ec in VM_CF_LEP (cfp=0x7ffff6b08e80) at vm.c:44
#7 0x00005555556ea91f in VM_CF_BLOCK_PTR (cfp=0x7ffff6b08e80) at vm.c:56
#8 0x0000555555700f57 in check_block (th=0x5555559f0590) at vm.c:646
#9 0x0000555555700fff in vm_yield (th=0x5555559f0590, argc=1, argv=0x7fffffffba38) at vm.c:666
#10 0x00005555556fd6d3 in rb_yield_0 (argc=1, argv=0x7fffffffba38) at vm_eval.c:897
#11 0x00005555556fd70d in rb_yield (val=93824999078160) at vm_eval.c:907
#12 0x00005555555a63c4 in zip_i (val=3, memo=0x555555bdb960, argc=1, argv=0x7ffff6a09070) at enum.c:2001
...

binding呼び出しによって環境がヒープに移されますが、
IFUNC上のepがそれに追随できていないのが原因です。

以下の拡張ライブラリのコードでも再現させることができるので、
TracePointのバグというよりはVMのバグといえそうです。

static VALUE
segv_i(VALUE i, VALUE ary, int argc, VALUE *argv)
{
rb_binding_new();
rb_yield(Qnil);
return Qnil;
}

VALUE
rb_segv(VALUE obj)
{
rb_block_call(obj, rb_intern("m"), 0, 0, segv_i, 0);
return Qnil;
}
=end

Associated revisions

Revision 39067
Added by Koichi Sasada over 2 years ago

  • proc.c (rb_binding_new_with_cfp): permit to create binding object of IFUNC frame. When rb_binding_new_with_cfp()' is called, VM finds out the first normal (has iseq) frame and create a binding object of this frame and create Env objects.ep's of related frames are updated (ep's point Env object managed spaces). However,ep' of skipped IFUNC frame was not updated and old invalid `ep' was remained. It causes serious problems. To solve this issue, permit IFUNC to create binding. (Maybe there is no problem on it) [ruby-trunk - Bug #7774]
  • test/ruby/test_settracefunc.rb: add a test.
  • vm.c (rb_vm_get_binding_creatable_next_cfp), vm_core.h: added.
  • vm_trace.c: fix to use `rb_vm_get_binding_creatable_next_cfp()'.

Revision 39067
Added by Koichi Sasada over 2 years ago

  • proc.c (rb_binding_new_with_cfp): permit to create binding object of IFUNC frame. When rb_binding_new_with_cfp()' is called, VM finds out the first normal (has iseq) frame and create a binding object of this frame and create Env objects.ep's of related frames are updated (ep's point Env object managed spaces). However,ep' of skipped IFUNC frame was not updated and old invalid `ep' was remained. It causes serious problems. To solve this issue, permit IFUNC to create binding. (Maybe there is no problem on it) [ruby-trunk - Bug #7774]
  • test/ruby/test_settracefunc.rb: add a test.
  • vm.c (rb_vm_get_binding_creatable_next_cfp), vm_core.h: added.
  • vm_trace.c: fix to use `rb_vm_get_binding_creatable_next_cfp()'.

Revision 39073
Added by Kazuki Tsujimoto over 2 years ago

  • vm.c (rb_vm_stack_to_heap): call rb_vm_get_binding_creatable_next_cfp
    instead of rb_vm_get_ruby_level_next_cfp to prevent a segfault by
    calling Kernel#callcc. See r39067 for more details.
    [ruby-trunk - Bug #7774]

  • test/ruby/test_settracefunc.rb: add a test.

Revision 39073
Added by Kazuki Tsujimoto over 2 years ago

  • vm.c (rb_vm_stack_to_heap): call rb_vm_get_binding_creatable_next_cfp
    instead of rb_vm_get_ruby_level_next_cfp to prevent a segfault by
    calling Kernel#callcc. See r39067 for more details.
    [ruby-trunk - Bug #7774]

  • test/ruby/test_settracefunc.rb: add a test.

Revision 39275
Added by Koichi Sasada over 2 years ago

  • proc.c (rb_binding_new_with_cfp): create binding object even if the frame is IFUNC. But return a ruby-level binding to keep compatibility. This patch fix degradation introduced from r39067. [Bug #7774]
  • test/ruby/test_settracefunc.rb: add a test.

Revision 39275
Added by Koichi Sasada over 2 years ago

  • proc.c (rb_binding_new_with_cfp): create binding object even if the frame is IFUNC. But return a ruby-level binding to keep compatibility. This patch fix degradation introduced from r39067. [Bug #7774]
  • test/ruby/test_settracefunc.rb: add a test.

History

#1 Updated by Koichi Sasada over 2 years ago

(2013/02/03 18:14), ktsj (Kazuki Tsujimoto) wrote:

binding呼び出しによって環境がヒープに移されますが、
IFUNC上のepがそれに追随できていないのが原因です。

 あーあーきこえなーい.

 じゃなくて,ええと,IFUNC 回りはてきとーですよね.どうしようかな.

 RC2 っていつ頃出します?

--
// SASADA Koichi at atdot dot net

#2 Updated by Yusuke Endoh over 2 years ago

2013年2月3日 18:35 SASADA Koichi ko1@atdot.net:

 RC2 っていつ頃出します?

連絡してなくてすみません。

rubygems.org の事件に関連して Eric Hodel が何かしら rubygems に
対処が必要かを検討したいと言っていて、その具体的なプランを問い
合わせているところです。(今日中に返事よこせと言った)

明日には何かしらのアナウンスを出したいと思います。

--
Yusuke Endoh mame@tsg.ne.jp

#3 Updated by Koichi Sasada over 2 years ago

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

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


  • proc.c (rb_binding_new_with_cfp): permit to create binding object of IFUNC frame. When rb_binding_new_with_cfp()' is called, VM finds out the first normal (has iseq) frame and create a binding object of this frame and create Env objects.ep's of related frames are updated (ep's point Env object managed spaces). However,ep' of skipped IFUNC frame was not updated and old invalid `ep' was remained. It causes serious problems. To solve this issue, permit IFUNC to create binding. (Maybe there is no problem on it) [ruby-trunk - Bug #7774]
  • test/ruby/test_settracefunc.rb: add a test.
  • vm.c (rb_vm_get_binding_creatable_next_cfp), vm_core.h: added.
  • vm_trace.c: fix to use `rb_vm_get_binding_creatable_next_cfp()'.

#4 Updated by Koichi Sasada over 2 years ago

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


Bug #7774: IFUNC上のbinding呼び出しでSEGV
https://bugs.ruby-lang.org/issues/7774

 ご報告,ありがとうございました.多分,治ったんじゃないかと思います.

binding呼び出しによって環境がヒープに移されますが、
IFUNC上のepがそれに追随できていないのが原因です。

 ご指摘の通りでしたので,IFUNC であっても binding が作れるようにしまし
た.何かまずいことでも起こるかなと試してみたんですが,ちょっと試したとこ
ろ大丈夫そうでした.

 finish frame も無くしたので,この辺全部整理したい.

--
// SASADA Koichi at atdot dot net

#5 Updated by Kazuki Tsujimoto over 2 years ago

ご対応ありがとうございます。
が、次のコードがSEGVするようになってしまいました。

 require 'continuation'

 tp = TracePoint.new(:raise) do |tp|
   tp.binding
 end
 tp.enable do
   obj = Object.new
   class << obj
     include Enumerable
     def each
       yield 1
     end
   end
   c = nil
   obj.sort_by {|x| callcc {|c2| c ||= c2 }; x }
   c.call
 end

#6 Updated by Koichi Sasada over 2 years ago

(2013/02/05 16:47), ktsj (Kazuki Tsujimoto) wrote:

が、次のコードがSEGVするようになってしまいました。

がーん.callcc 使ったコードなんて知らん,とは言えないかなぁ.
これは何が起きている?

--
// SASADA Koichi at atdot dot net

#7 Updated by Kazuki Tsujimoto over 2 years ago

rb_callccから呼ばれるrb_vm_stack_to_heapでも、
同様にIFUNCの対応が必要ということのようです。

以下のパッチで直りましたがどうでしょうか。

diff --git a/vm.c b/vm.c
index ef5dd97..1429cce 100644
--- a/vm.c
+++ b/vm.c
@@ -556,7 +556,7 @@ void
 rb_vm_stack_to_heap(rb_thread_t *th)
 {
     rb_control_frame_t *cfp = th->cfp;
-    while ((cfp = rb_vm_get_ruby_level_next_cfp(th, cfp)) != 0) {
+    while ((cfp = rb_vm_get_binding_creatable_next_cfp(th, cfp)) != 0) {
    rb_vm_make_env_object(th, cfp);
    cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp);
     }

#8 Updated by Kazuki Tsujimoto over 2 years ago

辻本です。

Subject: Re: [ruby-trunk - Bug#7774][Assigned] IFUNC上のbinding呼び出しでSEGV
From: SASADA Koichi ko1@atdot.net
Date: Tue, 5 Feb 2013 15:15:30 +0900

 ご報告,ありがとうございました.多分,治ったんじゃないかと思います.

たびたびすみません。この修正ですが、下記の変更は意図されたものでしょうか。

  • r39067より前では、+メソッド/injectメソッドで例外が起きた場合は両方ともメソッド起動時のコンテキスト(例ではmain)のbindingが返ってきていた。
  • r39067以降は、injectメソッドで例外が起きた場合の動きが変更されレシーバのbindingが返ってくるようになった。(+メソッドは変更なし)

個人的にはこれまでの挙動のほうが便利でした。

  def m1(arg)
    arg + nil
  rescue
  end

  def m2(arg)
    arg.inject(:+)
  rescue
  end

  # TracePoint
  tp = TracePoint.new(:raise) do |tp|
    $b = tp.binding
  end

  tp.enable do
    m1(0)
    p A: eval('self', $b)
    p B: eval('arg rescue nil', $b)

    m2([0, nil])
    p C: eval('self', $b)
    p D: eval('arg rescue nil', $b)
  end

  # set_trace_func
  set_trace_func ->(event, file, line, id, binding, klass) do
    if event == 'raise'
      $b = binding
    end
  end

  m1(0)
  p E: eval('self', $b)
  p F: eval('arg rescue nil', $b)

  m2([0, nil])
  p G: eval('self', $b)
  p H: eval('arg rescue nil', $b)

  # r39066
  # {:A=>main}
  # {:B=>0}
  # {:C=>main}
  # {:D=>[0, nil]}
  # {:E=>main}
  # {:F=>0}
  # {:G=>main}
  # {:H=>[0, nil]}

  # r39067
  # {:A=>main}
  # {:B=>0}
  # {:C=>[0, nil]}
  # {:D=>nil}
  # {:E=>main}
  # {:F=>0}
  # {:G=>[0, nil]}
  # {:H=>nil}

--
Kazuki Tsujimoto

#9 Updated by Koichi Sasada over 2 years ago

(2013/02/09 12:12), Kazuki Tsujimoto wrote:

たびたびすみません。この修正ですが、下記の変更は意図されたものでしょうか。

  • r39067より前では、+メソッド/injectメソッドで例外が起きた場合は両方ともメソッド起動時のコンテキスト(例ではmain)のbindingが返ってきていた。
  • r39067以降は、injectメソッドで例外が起きた場合の動きが変更されレシーバのbindingが返ってくるようになった。(+メソッドは変更なし)

個人的にはこれまでの挙動のほうが便利でした。

 確かに.挙動は変わらないと思っていたんだけど,binding の位置はずれます
な.どうしたものですかねぇ.

 正直,どの binding を返すか,ってきちんとしたコンセンサスが取れてない
と思うのですよ.たまたま,C で実装されたものだった場合には,その上のフ
レームの binding が帰ってきている感じです.

 例えば,Ruby で + を実装した XYZZY というクラスが例外を吐く場合,やは
り呼び出し側のフレームではなく,XYZZY#+ のフレームの Binding オブジェク
トが返ります.

  def m1(arg)
    arg + nil
  rescue
  end

  def m2(arg)
    arg.inject(:+)
  rescue
  end

  class XYZZY
    def +(o)
      raise
    end
  end

  # TracePoint
  tp = TracePoint.new(:raise) do |tp|
    $b = tp.binding
  end

  tp.enable do
    m1(0)
    p A: eval('self', $b)
    p B: eval('arg rescue nil', $b)

    m2([0, nil])
    p C: eval('self', $b)
    p D: eval('arg rescue nil', $b)

    m1(XYZZY.new)
    p X: eval('self', $b)
    p Y: eval('arg rescue nil', $b)
  end

  # set_trace_func
  set_trace_func ->(event, file, line, id, binding, klass) do
    if event == 'raise'
      $b = binding
    end
  end

  m1(0)
  p E: eval('self', $b)
  p F: eval('arg rescue nil', $b)

  m2([0, nil])
  p G: eval('self', $b)
  p H: eval('arg rescue nil', $b)

  m1(XYZZY.new)
  p XX: eval('self', $b)
  p YY: eval('arg rescue nil', $b)
 #=>
 ruby 2.0.0dev (2012-12-21 trunk 38515) [i386-mswin32_100]
 t.rb:19: warning: shadowing outer local variable - tp
 {:A=>main}
 {:B=>0}
 {:C=>main}
 {:D=>[0, nil]}
 {:X=>#<XYZZY:0xb2a0d4>}
 {:Y=>nil}
 {:E=>main}
 {:F=>0}
 {:G=>main}
 {:H=>[0, nil]}
 {:XX=>#<XYZZY:0xb12a60>}
 {:YY=>nil}

 とまぁ,こんな感じで,「呼び出し側のフレームを返す」という仕様ではな
かったわけです.

 もし,以前の挙動に戻すなら,ifuc の binding を作りつつ,ruby-level フ
レームまで遡って,その ruby-level フレームの binding を返す,というのが
思いつきました.これで解決しそうではあります.

 どうしたもんでしょうか?

--
// SASADA Koichi at atdot dot net

#10 Updated by Kazuki Tsujimoto over 2 years ago

辻本です

Subject: Re: [ruby-trunk - Bug #7774][Assigned] IFUNC上のbinding呼び出しでSEGV
From: SASADA Koichi ko1@atdot.net
Date: Sat, 9 Feb 2013 13:12:59 +0900

 とまぁ,こんな感じで,「呼び出し側のフレームを返す」という仕様ではな
かったわけです.

 もし,以前の挙動に戻すなら,ifuc の binding を作りつつ,ruby-level フ
レームまで遡って,その ruby-level フレームの binding を返す,というのが
思いつきました.これで解決しそうではあります.

簡単に仕様をまとめるとこんな感じでしょうか。

1.9.3/2.0.0rc1
Rubyメソッド: 最後のruby-levelフレーム(呼び出し側フレームかどうかは無関係)
Cメソッド: 同上

2.0.0rc2
Rubyメソッド: 最後のruby-levelフレーム(呼び出し側フレームかどうかは無関係)
Cメソッド: 最後のruby-levelまたはC-levelフレーム(メソッドの実装による)

戻すにしても、タイミングが問題ですね。

--
Kazuki Tsujimoto

#11 Updated by Koichi Sasada over 2 years ago

SEGV を直すときに,うっかり意図しない非互換が混入されてしまいました,と
いう理屈で,これは 2_0_0 のほうにバックポートしてもいいでしょうか
>mame さん

Binding はどう取れるべきか,については実はそんなに自明ではないので,今度
誰か議論させて下さい.この辺,今は本当に「実装がそうだったから」以外じゃ
ないような気がしています.

(2013/02/09 16:45), Kazuki Tsujimoto wrote:

辻本です

Subject: Re: [ruby-trunk - Bug #7774][Assigned] IFUNC上のbinding呼び出しでSEGV
From: SASADA Koichi ko1@atdot.net
Date: Sat, 9 Feb 2013 13:12:59 +0900

 とまぁ,こんな感じで,「呼び出し側のフレームを返す」という仕様ではな
かったわけです.

 もし,以前の挙動に戻すなら,ifuc の binding を作りつつ,ruby-level フ
レームまで遡って,その ruby-level フレームの binding を返す,というのが
思いつきました.これで解決しそうではあります.

簡単に仕様をまとめるとこんな感じでしょうか。

1.9.3/2.0.0rc1
Rubyメソッド: 最後のruby-levelフレーム(呼び出し側フレームかどうかは無関係)
Cメソッド: 同上

2.0.0rc2
Rubyメソッド: 最後のruby-levelフレーム(呼び出し側フレームかどうかは無関係)
Cメソッド: 最後のruby-levelまたはC-levelフレーム(メソッドの実装による)

戻すにしても、タイミングが問題ですね。

--
// SASADA Koichi at atdot dot net

#12 Updated by Yusuke Endoh over 2 years ago

どの reversion をバックポートするという話?

rc1 からの regression ならまあいいと思いますが、
修正が大きいと後ろ向きになるかも。

2013年2月9日 17:14 SASADA Koichi ko1@atdot.net:

SEGV を直すときに,うっかり意図しない非互換が混入されてしまいました,と
いう理屈で,これは 2_0_0 のほうにバックポートしてもいいでしょうか
>mame さん

Binding はどう取れるべきか,については実はそんなに自明ではないので,今度
誰か議論させて下さい.この辺,今は本当に「実装がそうだったから」以外じゃ
ないような気がしています.

(2013/02/09 16:45), Kazuki Tsujimoto wrote:

辻本です

Subject: Re: [ruby-trunk - Bug #7774][Assigned] IFUNC上のbinding呼び出しでSEGV
From: SASADA Koichi ko1@atdot.net
Date: Sat, 9 Feb 2013 13:12:59 +0900

  とまぁ,こんな感じで,「呼び出し側のフレームを返す」という仕様ではな
かったわけです.

  もし,以前の挙動に戻すなら,ifuc の binding を作りつつ,ruby-level フ
レームまで遡って,その ruby-level フレームの binding を返す,というのが
思いつきました.これで解決しそうではあります.

簡単に仕様をまとめるとこんな感じでしょうか。

1.9.3/2.0.0rc1
Rubyメソッド: 最後のruby-levelフレーム(呼び出し側フレームかどうかは無関係)
Cメソッド: 同上

2.0.0rc2
Rubyメソッド: 最後のruby-levelフレーム(呼び出し側フレームかどうかは無関係)
Cメソッド: 最後のruby-levelまたはC-levelフレーム(メソッドの実装による)

戻すにしても、タイミングが問題ですね。

// SASADA Koichi at atdot dot net

--
Yusuke Endoh mame@tsg.ne.jp

#13 Updated by Koichi Sasada over 2 years ago

  • Status changed from Closed to Assigned

#14 Updated by Koichi Sasada over 2 years ago

  • Status changed from Assigned to Closed

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


  • proc.c (rb_binding_new_with_cfp): create binding object even if the frame is IFUNC. But return a ruby-level binding to keep compatibility. This patch fix degradation introduced from r39067. [Bug #7774]
  • test/ruby/test_settracefunc.rb: add a test.

#15 Updated by Koichi Sasada over 2 years ago

  • Assignee changed from Koichi Sasada to Yusuke Endoh
  • Status changed from Closed to Assigned

反応が悪くて申し訳ないです。
先ほどコミットした r39275 になります。

SEGV はしませんが、若干の非互換となります。

#16 Updated by Yusuke Endoh over 2 years ago

  • Assignee changed from Yusuke Endoh to Koichi Sasada

了解です。ありがとうございます。バックポートお願いします。

Yusuke Endoh mame@tsg.ne.jp

#17 Updated by Koichi Sasada over 2 years ago

r39305 で backport しました。

が、redmine のほうには backport issue にしていないから
自動的に close されてないんかな。

#18 Updated by Koichi Sasada over 2 years ago

  • Status changed from Assigned to Closed

Also available in: Atom PDF