Bug #7774: IFUNC上のbinding呼び出しでSEGV

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: 2.0.0
• ruby -v: ruby 2.0.0dev (2013-02-03 trunk 39032) [x86_64-linux]

[ktsj (Kazuki Tsujimoto)]

=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

[ko1 (Koichi Sasada)]

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

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

　あーあーきこえなーい．

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

　RC2 っていつ頃出します？

--
// SASADA Koichi at atdot dot net

[mame (Yusuke Endoh)]

2013年2月3日 18:35 SASADA Koichi [e-mail]:

　RC2 っていつ頃出します？

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

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

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

--
Yusuke Endoh [e-mail]

[ko1 (Koichi Sasada)]

• 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-dev:46908] [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()'.

[ko1 (Koichi Sasada)]

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

[ktsj (Kazuki Tsujimoto)]

ご対応ありがとうございます。
が、次のコードが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

[ko1 (Koichi Sasada)]

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

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

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

--
// SASADA Koichi at atdot dot net

[ktsj (Kazuki Tsujimoto)]

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);
     }

[ktsj (Kazuki Tsujimoto)]

辻本です。

Subject: [ruby-dev:46921] Re: [ruby-dev:46908] [ruby-trunk - Bug#7774][Assigned] IFUNC上のbinding呼び出しでSEGV
From: SASADA Koichi [e-mail]
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

[ko1 (Koichi Sasada)]

(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

[ktsj (Kazuki Tsujimoto)]

辻本です

Subject: [ruby-dev:46961] Re: [ruby-trunk - Bug #7774][Assigned] IFUNC上のbinding呼び出しでSEGV
From: SASADA Koichi [e-mail]
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

[ko1 (Koichi Sasada)]

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

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

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

辻本です

Subject: [ruby-dev:46961] Re: [ruby-trunk - Bug #7774][Assigned] IFUNC上のbinding呼び出しでSEGV
From: SASADA Koichi [e-mail]
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

[mame (Yusuke Endoh)]

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

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

2013年2月9日 17:14 SASADA Koichi [e-mail]:

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

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

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

辻本です

Subject: [ruby-dev:46961] Re: [ruby-trunk - Bug #7774][Assigned] IFUNC上のbinding呼び出しでSEGV
From: SASADA Koichi [e-mail]
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 [e-mail]

[ko1 (Koichi Sasada)]

• Status changed from Closed to Assigned

[ko1 (Koichi Sasada)]

• 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] [ruby-dev:46960]
• test/ruby/test_settracefunc.rb: add a test.

[ko1 (Koichi Sasada)]

• Status changed from Closed to Assigned
• Assignee changed from ko1 (Koichi Sasada) to mame (Yusuke Endoh)

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

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

[mame (Yusuke Endoh)]

• Assignee changed from mame (Yusuke Endoh) to ko1 (Koichi Sasada)

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

--
Yusuke Endoh [e-mail]

[ko1 (Koichi Sasada)]

r39305 で backport しました。

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

[ko1 (Koichi Sasada)]

• Status changed from Assigned to Closed