Bug #7774

IFUNC上のbinding呼び出しでSEGV

Added by Kazuki Tsujimoto about 1 year ago. Updated about 1 year ago.

[ruby-dev:46908]
Status:Closed
Priority:High
Assignee:Koichi Sasada
Category:core
Target version:2.0.0
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/x8664-linux-gnu/libc.so.6
#1 0x00007ffff6efabab in abort () from /lib/x86
64-linux-gnu/libc.so.6
#2 0x00005555555ad998 in rbbug (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
EPLEP (ep=0xc) at vm.c:28
#6 0x00005555556ea8ec in VM
CFLEP (cfp=0x7ffff6b08e80) at vm.c:44
#7 0x00005555556ea91f in VM
CFBLOCKPTR (cfp=0x7ffff6b08e80) at vm.c:56
#8 0x0000555555700f57 in checkblock (th=0x5555559f0590) at vm.c:646
#9 0x0000555555700fff in vm
yield (th=0x5555559f0590, argc=1, argv=0x7fffffffba38) at vm.c:666
#10 0x00005555556fd6d3 in rbyield0 (argc=1, argv=0x7fffffffba38) at vmeval.c:897
#11 0x00005555556fd70d in rb
yield (val=93824999078160) at vmeval.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
segvi(VALUE i, VALUE ary, int argc, VALUE *argv)
{
rb
bindingnew();
rb
yield(Qnil);
return Qnil;
}

VALUE
rbsegv(VALUE obj)
{
rb
blockcall(obj, rbintern("m"), 0, 0, segv_i, 0);
return Qnil;
}
=end

Associated revisions

Revision 39067
Added by Koichi Sasada about 1 year ago

  • proc.c (rbbindingnewwithcfp): 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 (rbvmgetbindingcreatablenextcfp), vm_core.h: added.
  • vmtrace.c: fix to use `rbvmgetbindingcreatablenext_cfp()'.

Revision 39073
Added by Kazuki Tsujimoto about 1 year ago

  • vm.c (rbvmstacktoheap): call rbvmgetbindingcreatablenextcfp
    instead of rbvmgetrubylevelnextcfp 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 about 1 year ago

  • proc.c (rbbindingnewwithcfp): 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 about 1 year 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 about 1 year 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 about 1 year 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 (rbbindingnewwithcfp): 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 (rbvmgetbindingcreatablenextcfp), vm_core.h: added.
  • vmtrace.c: fix to use `rbvmgetbindingcreatablenext_cfp()'.

#4 Updated by Koichi Sasada about 1 year 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 about 1 year 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 about 1 year ago

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

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

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

--
// SASADA Koichi at atdot dot net

#7 Updated by Kazuki Tsujimoto about 1 year ago

rbcallccから呼ばれるrbvmstackto_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
rbvmstacktoheap(rbthreadt *th)
{
rbcontrolframet *cfp = th->cfp;
- while ((cfp = rb
vmgetrubylevelnextcfp(th, cfp)) != 0) {
+ while ((cfp = rb
vmgetbindingcreatablenextcfp(th, cfp)) != 0) {
rb
vmmakeenvobject(th, cfp);
cfp = RUBY
VMPREVIOUSCONTROL_FRAME(cfp);
}

#8 Updated by Kazuki Tsujimoto about 1 year 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

    settracefunc

    settracefunc ->(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 about 1 year 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

# settracefunc
settracefunc ->(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 about 1 year 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 about 1 year ago

SEGV を直すときに,うっかり意図しない非互換が混入されてしまいました,と
いう理屈で,これは 200 のほうにバックポートしてもいいでしょうか
>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 about 1 year ago

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

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

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

SEGV を直すときに,うっかり意図しない非互換が混入されてしまいました,と
いう理屈で,これは 200 のほうにバックポートしてもいいでしょうか
>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 about 1 year ago

  • Status changed from Closed to Assigned

#14 Updated by Koichi Sasada about 1 year 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 (rbbindingnewwithcfp): 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 about 1 year ago

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

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

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

#16 Updated by Yusuke Endoh about 1 year ago

  • Assignee changed from Yusuke Endoh to Koichi Sasada

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

Yusuke Endoh mame@tsg.ne.jp

#17 Updated by Koichi Sasada about 1 year ago

r39305 で backport しました。

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

#18 Updated by Koichi Sasada about 1 year ago

  • Status changed from Assigned to Closed

Also available in: Atom PDF