Feature #2674: RubyVM::InstructionSequence to accept IOs

• Category set to YARV
• Status changed from Open to Assigned
• Assignee set to ko1 (Koichi Sasada)
• Target version set to 1.9.2

• Priority changed from Low to Normal

ワナベと申します。

せっかく rb_compile_file があるので、IO を受け付けてくれるなら
その方が嬉しいと思います。ですので、このパッチに賛成です。

(2010/03/31 2:28), _ wanabe wrote::
> せっかく rb_compile_file があるので、IO を受け付けてくれるなら
> その方が嬉しいと思います。ですので、このパッチに賛成です。

　すみません，この件忘れていました．IO を受け付けるのはいいんです
が，gets のありなし，で見分けるのが正しいかだけ気になっています．そこだ
け，Ruby 的に OK とか NG とか，誰か判断してもらえれば．

# ここで使うのは gets だけなんだっけ？

-- 
// SASADA Koichi at atdot dot net

ありがとうございます。
NG なケースを見つけてしまったので報告します。これで SEGV します。

a = [nil]
def a.gets
  raise
end
RubyVM::InstructionSequence.compile(a)

中を見ると、lex_io_gets から最終的に rb_io_getline_1 を呼び出して
GetOpenFile で RFILE() を使っているようですので、
src が T_FILE かどうかで判断するのがいいのではないかと思います。

あるいは rb_parser_compile_file 内部で、file が T_FILE かどうかで
lex_gets を変える（rb_io_gets を呼ぶか rb_funcall 経由で gets するか）
というのも考えましたが、ちょっと影響範囲がわかりませんでした。

_ wanabe さんは書きました:

> ありがとうございます。
> NG なケースを見つけてしまったので報告します。これで SEGV します。

それはrb_compile_fileのバグでは...



Signed-off-by: Urabe, Shyuohei <[e-mail]>
---
 iseq.c  |    2 ++
 parse.y |   14 ++++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/iseq.c b/iseq.c
index 77f1497..2cccfcc 100644
--- a/iseq.c
+++ b/iseq.c
@@ -551,6 +551,8 @@ rb_iseq_compile_with_option(VALUE src, VALUE file, VALUE filepath, VALUE line, V
         node = rb_compile_file(fn, src, ln);
     else
         node = rb_compile_string(fn, StringValue(src), ln);
+    if (!node)
+        rb_exc_raise(GET_THREAD()->errinfo);
     if (th->base_block && th->base_block->iseq) {
 	return rb_iseq_new_with_opt(node, th->base_block->iseq->name,
 				    file, filepath, line, th->base_block->iseq->self,
diff --git a/parse.y b/parse.y
index 340a825..438b8f5 100644
--- a/parse.y
+++ b/parse.y
@@ -5138,6 +5138,12 @@ lex_getline(struct parser_params *parser)

 static const rb_data_type_t parser_data_type;

+static VALUE
+lex_get_generic(struct parser_params *parser, VALUE src)
+{
+    return rb_funcall(src, rb_intern("gets"), 0);
+}
+
 #ifndef RIPPER
 static NODE*
 parser_compile_string(volatile VALUE vparser, const char *f, VALUE s, int line)
@@ -5209,7 +5215,7 @@ rb_parser_compile_file(volatile VALUE vparser, const char *f, VALUE file, int st
     NODE *node;

     TypedData_Get_Struct(vparser, struct parser_params, &parser_data_type, parser);
-    lex_gets = lex_io_gets;
+    lex_gets = lex_get_generic;
     lex_input = file;
     lex_pbeg = lex_p = lex_pend = 0;
     compile_for_eval = rb_parse_in_eval();
@@ -10319,11 +10325,7 @@ ripper_warningS(struct parser_params *parser, const char *fmt, const char *str)
                STR_NEW2(fmt), STR_NEW2(str));
 }

-static VALUE
-ripper_lex_get_generic(struct parser_params *parser, VALUE src)
-{
-    return rb_funcall(src, ripper_id_gets, 0);
-}
+#define ripper_lex_get_generic lex_get_generic

 static VALUE
 ripper_s_allocate(VALUE klass)
-- 
1.6.0.4

遠藤です。

2010年3月31日14:38 Urabe Shyouhei <[e-mail]>:
> _ wanabe さんは書きました:
>
>> ありがとうございます。
>> NG なケースを見つけてしまったので報告します。これで SEGV します。
>
> それはrb_compile_fileのバグでは...


rb_compile_file は T_FILE を受け取るという仕様だったのでしょう。

卜部さんのパッチをあてると、メソッド呼び出し分の速度劣化が……
はともかく、IO#gets を再定義することでパース中に任意のコードが
実行できるようになりますが、大丈夫でしょうか。

# ripper がすでにやってるわけですが、ripper は動作実績なさそう
# なので信用できません……


とりあえず continuation を使うと落ちました。

  $ ./ruby -e '
    require "continuation"
    $code = "p :foo"
    class IO
      def gets
        callcc {|c| $c = c }
        code, $code = $code, nil
        code
      end
    end
    load "foo.rb"
    $c.call
  '
  :foo
  -e:11: [BUG] Segmentation fault
  (略)

continuation は自己責任としても、何か妙な影響はないですかね。
load/require 命令を乗っ取れるようになるとか気になります。

  $ cat foo.rb
  p :innocent

  $ ./ruby.new -e '
    $code = "p :evil"
    class IO
      def gets
        code, $code = $code, nil
        code
      end
    end

    load "foo.rb"
  '
  :evil

IO#gets が再定義できる状況ならなんでもやり放題ではあるのですが。
wanabe さんと同じく、T_FILE かどうかで判断するのが無難だと思い
ました。

-- 
Yusuke ENDOH <[e-mail]>

卜部です。

Yusuke ENDOH さんは書きました:
> rb_compile_file は T_FILE を受け取るという仕様だったのでしょう。

getsじゃなきゃヤダという強い動機があるわけではないので、そういう仕様と決まれば
特に異論はありません。ただその場合はripperもT_FILEのほうに倒すべきでしょうね。

• Start date set to 01/28/2010

• Target version changed from 1.9.2 to 2.0.0

遠藤です。

残念ですが、4/1 の時点で「そういう仕様と決まれば特に異論はありません」
という状況で、その後議論が途絶えてしまったようなので、spec freeze まで
には合意ができなかったと考えます。
target を 1.9.x にさせていただきます。

-- 
Yusuke Endoh <[e-mail]>

あっ、はい。

てか1.9.3っていうtarget作りません?