Bug #5217: lineno is broken when source code has about 7000 lines

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: -
• ruby -v: -

[mame (Yusuke Endoh)]

遠藤です。

asakusa.rb で出た話だそうですが (なひさんからの伝聞) 、soap4r と
simplecov を組み合わせると [BUG] bug が出るそうです。

いろいろ話を聞かせてもらった結果、以下のようにすると再現できました。

$ ruby -e 'puts "p\n" * 7000; puts "p([1"; puts "])"' > t.rb

$ cat t.rb
p
p
p

... 7000 行ほど p ...

p
p

$ ./ruby -rcoverage -e 'Coverage.start; load "t.rb"'

応急措置ですが、以下のパッチで直ります。
(rb_bug("bug") というアホメッセージは私の所業のようです。スミマセン)
害はないと思うので、これはこれでコミットしておこうと思います。

diff --git a/thread.c b/thread.c
index 6970d8f..57a6962 100644
--- a/thread.c
+++ b/thread.c
@@ -4764,7 +4764,7 @@ update_coverage(rb_event_flag_t event, VALUE proc, VALUE self, ID id, VALUE klas
long line = rb_sourceline() - 1;
long count;
if (RARRAY_PTR(coverage)[line] == Qnil) {

•   rb_bug("bug");
  

•   return;
  

  }
  count = FIX2LONG(RARRAY_PTR(coverage)[line]) + 1;
  if (POSFIXABLE(count)) {

しかしそもそもの原因は、iseq のバイトコードの大きさが unsigned short
で管理されているために、バイトコードの大きさが 65536 を超えると行番号を
見失ってしまうという問題のような気がします。
私も最初勘違いしたのですが、行番号が 16 ビットという問題とは別であること
に注意。65536 行ほど大きくなくても 7000 行くらいで発症します。

これにより、rb_vm_get_sourceline の挙動が怪しくなっています。
これは coverage に限らず、

set_trace_func(proc {|type, file, line,| p line if type == "line" })
p
p
p

... 7000 行ほど p ...

p
p
p

というコードの出力が、

2
3
4
5
...
6550
6551
6552
6553
7001
7001
7001
7001
7001
...

となります。行番号が飛んでいることがわかります。

直そうと思えば、以下のようにすれば直ります。しかしあえて short を選んで
いるのは省メモリ化のためだと思いますので、これは既知の制限ということで
しょうか > ささださん

diff --git a/iseq.h b/iseq.h
index beeacbb..9c19501 100644
--- a/iseq.h
+++ b/iseq.h
@@ -44,9 +44,9 @@ struct rb_compile_option_struct {
};

struct iseq_insn_info_entry {

• unsigned short position;
• unsigned short line_no;
• unsigned short sp;

• unsigned long position;
• unsigned long line_no;
• unsigned long sp;
  };

struct iseq_catch_table_entry {

--
Yuskue Endoh [e-mail]

[mame (Yusuke Endoh)]

• Assignee set to ko1 (Koichi Sasada)

[ko1 (Koichi Sasada)]

• ruby -v changed from ruby 1.9.4dev (2011-08-23 trunk 33019) [i686-linux] to -

　ささだです．

(2011/08/23 7:43), Yusuke Endoh wrote:

直そうと思えば、以下のようにすれば直ります。しかしあえて short を選んで
いるのは省メモリ化のためだと思いますので、これは既知の制限ということで
しょうか > ささださん

　どうもすみません．

　色々考えて，遠藤さんに IRC でアドバイスしてもらった，「改行位置が変
わったところだけ情報を付ける」という方法でやり直しました．

　ある大きな Rails アプリで，ロード後にバイトコード全部のサイズを測った
ところ (*1)，
改善前：24,699,850 B（約 24.7MB）
改善後：21,835,244 B（約 21.8MB）
となりました．32bit 環境です: ruby 1.9.4dev (2011-08-22 trunk 33022)
[i686-linux]．

*1: objspace を使うと，こんな風に測れます．

     require 'objspace'
     p ObjectSpace.memsize_of_all(RubyVM::InstructionSequence)
     exit!

 これを，rack-*/lib/rack/handler/webrick.rb の self.run
 に無理矢理書き加えるというかっこ悪さ．
 もうちょっとなんとかならないかな．

　バイトコードのサイズ自体は，fork した後で共有できるとかあるので，あん
まり削減しても嬉しくないような気もしますが，まぁちょっとは効果があったと
いうことで．

　パッチ自体は結構大きくなりました．
http://www.atdot.net/sp/view/wpheql/readonly?lang=diff

　コミットしてもいいでしょうか．って，trunk ならさくっと入れちゃえばいい
かなぁ．

--
// SASADA Koichi at atdot dot net

[nahi (Hiroshi Nakamura)]

Koichi Sasada wrote:

　色々考えて，遠藤さんに IRC でアドバイスしてもらった，「改行位置が変
わったところだけ情報を付ける」という方法でやり直しました．

ありがとうございました。

　パッチ自体は結構大きくなりました．
http://www.atdot.net/sp/view/wpheql/readonly?lang=diff

　コミットしてもいいでしょうか．って，trunk ならさくっと入れちゃえばいい
かなぁ．

今のままでは評価を依頼しにくいので、入れてくださいな。

[mame (Yusuke Endoh)]

遠藤です。

2011年8月24日10:08 Hiroshi Nakamura [e-mail]:

Koichi Sasada wrote:

　 色々考えて，遠藤さんに IRC でアドバイスしてもらった，「改行位置が変
わったところだけ情報を付ける」という方法でやり直しました．

ありがとうございました。

おお。ささださんが一晩でやってくれました。

　 パッチ自体は結構大きくなりました．
http://www.atdot.net/sp/view/wpheql/readonly?lang=diff

　 コミットしてもいいでしょうか．って，trunk ならさくっと入れちゃえばいい
かなぁ．

trunk ならいいんじゃないですかね。
1.9.3 はいじらなくていいと思います。

今のままでは評価を依頼しにくいので、入れてくださいな。

元問題 ([BUG] bug) の話でしたら、trunk には前述の「応急措置」
パッチをコミット済み (r33030) なので、ささださんのパッチに関わらず、
もう [BUG] にはならなくなってると思います。

Yugui さん、このパッチ (r33030) は 1.9.3 にバックポート希望です。

--
Yusuke Endoh [e-mail]

[ko1 (Koichi Sasada)]

(2011/08/23 20:43), Yusuke ENDOH wrote:

おお。ささださんが一晩でやってくれました。

　こっちは朝だったけどね．コミットします．

--
// SASADA Koichi at atdot dot net

[ko1 (Koichi Sasada)]

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

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

---

• iseq.h, iseq.c, compile.c: Change the line number data structure
  to solve an issue reported at [ruby-dev:44413] [Ruby 1.9 - Bug #5217].
  Before this fix, each instruction has an information including
  line number (iseq::iseq_insn_info_table). Instead of this data
  structure, recording only line number changing places
  (iseq::iseq_line_info_table).
  The order of entries in iseq_line_info_table is ascending order of
  iseq_line_info_table_entry::position. You can get a line number
  by an iseq and a program counter with this data structure.
  This fix reduces memory consumption of iseq (bytecode).
  On my measurement, a rails application consumes 21.8MB for
  iseq with this fix on the 32bit CPU. Without this fix, it
  consumes 24.7MB for iseq [ruby-dev:44415].
• proc.c: ditto.
• vm_insnhelper.c: ditto.
• vm_method.c: ditto.
• vm.c (rb_vm_get_sourceline): change to use rb_iseq_line_no().

[kosaki (Motohiro KOSAKI)]

元問題 ([BUG] bug) の話でしたら、trunk には前述の「応急措置」
パッチをコミット済み (r33030) なので、ささださんのパッチに関わらず、
もう [BUG] にはならなくなってると思います。

Yugui さん、このパッチ (r33030) は 1.9.3 にバックポート希望です。

脱線ですが、そろそろbackport projectに1.9.3を追加してチケットで管理しませんか？
メールでバックポート依頼はあぶなっかしいと思います。

[ursm (Keita Urashima)]

浦嶌と申します。
Asakusa.rb でなひさんに相談に乗っていただきました。

[BUG] が出ていたコードを r33046 で試してみたところ、正常に動いているように見えました。
念のため以下のパッチを当てた状態でも試してみましたが、落ちるようなことはありませんでした。

ご対応ありがとうございました。

diff --git a/thread.c b/thread.c
index 57a6962..880e5f8 100644
--- a/thread.c
+++ b/thread.c
@@ -4764,7 +4764,7 @@ update_coverage(rb_event_flag_t event, VALUE
proc, VALUE self, ID id, VALUE klas
long line = rb_sourceline() - 1;
long count;
if (RARRAY_PTR(coverage)[line] == Qnil) {

•  return;
  

•  rb_bug("mame");
  

  }
  count = FIX2LONG(RARRAY_PTR(coverage)[line]) + 1;
  if (POSFIXABLE(count)) {

[mame (Yusuke Endoh)]

2011年8月24日18:09 KOSAKI Motohiro [e-mail]:

元問題 ([BUG] bug) の話でしたら、trunk には前述の「応急措置」
パッチをコミット済み (r33030) なので、ささださんのパッチに関わらず、
もう [BUG] にはならなくなってると思います。

Yugui さん、このパッチ (r33030) は 1.9.3 にバックポート希望です。

脱線ですが、そろそろbackport projectに1.9.3を追加してチケットで管理しませんか？
メールでバックポート依頼はあぶなっかしいと思います。

やっときました！

--
Yusuke Endoh [e-mail]

[mame (Yusuke Endoh)]

遠藤です。

2011年8月24日19:30 Keita Urashima [e-mail]:

浦嶌と申します。
Asakusa.rb でなひさんに相談に乗っていただきました。

[BUG] が出ていたコードを r33046 で試してみたところ、正常に動いているように見えました。
念のため以下のパッチを当てた状態でも試してみましたが、落ちるようなことはありませんでした。

ご対応ありがとうございました。

こちらこそご協力ありがとうございました。

ささださんのパッチによってこの問題は根本的に直ったのですが、行番号の
オーバーフローが起きるとやはり coverage 測定は変なことになるような
気がする (未確認ですが) ので、rb_bug を return にするパッチはその
ままにしておこうかなと思っております。

レアケースでカバレッジの測定漏れが起きるのと、rb_bug で落ちて測定
不能になるのと、どっちが幸せかという話ですが、なんとなく前者かなあと
思ったので。

--
Yusuke Endoh [e-mail]

[mame (Yusuke Endoh)]

遠藤です。

2011年8月24日10:08 Hiroshi Nakamura [e-mail]:

Koichi Sasada wrote:

　 色々考えて，遠藤さんに IRC でアドバイスしてもらった，「改行位置が変
わったところだけ情報を付ける」という方法でやり直しました．

ありがとうございました。

おお。ささださんが一晩でやってくれました。

　 パッチ自体は結構大きくなりました．
http://www.atdot.net/sp/view/wpheql/readonly?lang=diff

　 コミットしてもいいでしょうか．って，trunk ならさくっと入れちゃえばいい
かなぁ．

trunk ならいいんじゃないですかね。
1.9.3 はいじらなくていいと思います。

今のままでは評価を依頼しにくいので、入れてくださいな。

元問題 ([BUG] bug) の話でしたら、trunk には前述の「応急措置」
パッチをコミット済み (r33030) なので、ささださんのパッチに関わらず、
もう [BUG] にはならなくなってると思います。

Yugui さん、このパッチ (r33030) は 1.9.3 にバックポート希望です。

--
Yusuke Endoh [e-mail]

[yugui (Yuki Sonoda)]

2011/8/24 KOSAKI Motohiro [e-mail]:

脱線ですが、そろそろbackport projectに1.9.3を追加してチケットで管理しませんか？
メールでバックポート依頼はあぶなっかしいと思います。

done

--
Yuki Sonoda (Yugui)
[e-mail]
http://yugui.jp