Bug #858

r20625 dumps core and many strings associated with wrong encoding

Added by yugui (Yuki Sonoda) over 3 years ago. Updated about 1 year ago.

[ruby-dev:37390]
Status:Closed Start date:
Priority:High Due date:
Assignee:nobu (Nobuyoshi Nakada) % Done:

100%

Category:M17N
Target version:1.9.1 Release Candidate
ruby -v:

Description

Yuguiです。

r20625で、default_externalもdefault_internal同様に未設定時に
(rb_encoding_t*)NULL を返すようになった結果、trunkでruby -e 1とするとコ
アを吐くようになりました。

とりあえず、直すのはしてみました。結果として、process_options中に幾つか
default_externalに依存するオブジェクトを生成していることが分かりました。

* ruby_process_options()内。ruby_script()経由rb_progname:
 どうせあとで再設定しているので、ASCII-8BITで仮構築

* process_options()内。ruby_script()経由rb_progname:
 直後にlocale encodingをassociateしているので、構築時はASCII-8BIT

* rb_init_load_path_safeが設定するLOAD_PATH
* -Iで設定されるLOAD_PATH
  一応、default_externalを意図しているらしいので、default_externalの決定
後にassociateするようにしてみました。



* gem_prelude.rb:291で開いたIO
 特に何もしていないもののあまり問題はなさそう?

* これにより設定されるLOAD_PATH
 一緒にdefault_external決定後にそれをassociateしてしまいましたが、良いの
かな?

-- 
Yugui <yugui@yugui.jp>
http://yugui.jp
私は私をDumpする

Associated revisions

Revision 20656
Added by yugui (Yuki Sonoda) over 3 years ago

* encoding.c (enc_get_default_encoding): removed. Generalizing rb_default_{external,internal}_encoding seems to be difficult. default_external cannot be NULL even before detected. [ruby-dev:37390] * encoding.c (rb_default_external_encoding): has its own implementation again. * encoding.c (rb_default_internal_encoding): ditto. * gem_prelude.rb: added notice. * ruby.c (rubylib_mangled_path, rubylib_mangled_path2): uses locale encoding but not ASCII-8BIT. * ruby.c (process_options): refers less to default_external.

History

Updated by yugui (Yuki Sonoda) over 3 years ago

Yugui (Yuki Sonoda) さんは書きました:
> とりあえず、直すのはしてみました。結果として、process_options中に幾つか
> default_externalに依存するオブジェクトを生成していることが分かりました。

パッチを添付し忘れました。

修正方針ってこれで良いものでしょうか?
* LOAD_PATHのエンコーディングはどうするべき?
  -E を考慮したdefault_externalか、それともlocale encodingを強制するか

* prelude内でのIOはどれほど許される?

* なぜこんなに早いタイミングでpreludeが実行されてるんでしょうか?

* rb_prognameをrb_gc_register_mark_objectしていましたが、rb_prognameはあ
とから上書きされているのでこちらもrb_gc_register_mark_objectしないとまずい?

-- 
Yugui <yugui@yugui.jp>
http://yugui.jp
私は私をDumpする

diff --git a/encoding.c b/encoding.c
index 3253bfd..cd84af4 100644
--- a/encoding.c
+++ b/encoding.c
@@ -1072,7 +1072,7 @@ enc_set_default_encoding(struct default_encoding *def, VALUE encoding,
     return overridden;
 }

-static struct default_encoding default_external = {-2};
+static struct default_encoding default_external = {0};

 rb_encoding *
 rb_default_external_encoding(void)
diff --git a/gem_prelude.rb b/gem_prelude.rb
index ddc56c8..336dfac 100644
--- a/gem_prelude.rb
+++ b/gem_prelude.rb
@@ -2,6 +2,10 @@
 # vim: filetype=ruby
 # THIS FILE WAS AUTOGENERATED, DO NOT EDIT

+# NOTICE: Ruby is during initialization here. 
+#   * Encoding.default_external is always ASCII-8BIT.
+#   * Should not expect Encoding.default_internal.
+#   * Locale encoding is available.
 if defined?(Gem) then

   module Kernel
diff --git a/ruby.c b/ruby.c
index 913b454..0897c69 100644
--- a/ruby.c
+++ b/ruby.c
@@ -1097,6 +1097,8 @@ true_value(void)
 #define rb_define_readonly_boolean(name, val) \
     rb_define_virtual_variable((name), (val) ? true_value : false_value, 0)

+static void fix_objects_associated_with_default_external(void);
+
 static VALUE
 process_options(VALUE arg)
 {
@@ -1178,7 +1180,7 @@ process_options(VALUE arg)
 	}
     }

-    ruby_script(opt->script);
+    rb_progname = rb_obj_freeze(rb_str_new_cstr(opt->script));
 #if defined DOSISH || defined __CYGWIN__
     translate_char(RSTRING_PTR(rb_progname), '\\', '/');
 #endif
@@ -1211,6 +1213,7 @@ process_options(VALUE arg)
 	enc = lenc;
     }
     rb_enc_set_default_external(rb_enc_from_encoding(enc));
+    fix_objects_associated_with_default_external();
     if (opt->intern.enc.index >= 0) {
 	enc = rb_enc_from_index(opt->intern.enc.index);
 	rb_enc_set_default_internal(rb_enc_from_encoding(enc));
@@ -1287,6 +1290,18 @@ process_options(VALUE arg)

     return iseq;
 }
+static void 
+fix_objects_associated_with_default_external(void)
+{
+    int i;
+    VALUE load_path = GET_VM()->load_path;
+
+    for (i = 0; i < RARRAY_LEN(load_path); ++i) {
+        VALUE path = rb_ary_entry(load_path, i);
+        rb_enc_associate(path, rb_default_external_encoding());
+        rb_ary_store(load_path, i, path);
+    }
+}

 struct load_file_arg {
     VALUE parser;
@@ -1669,10 +1684,10 @@ ruby_process_options(int argc, char **argv)
     struct cmdline_arguments args;
     struct cmdline_options opt;
     NODE *tree;
+    VALUE tmp_progname; /* rb_progname will set with right encoding later */

-    ruby_script(argv[0]);	/* for the time being */
-    rb_argv0 = rb_str_new4(rb_progname);
-    rb_gc_register_mark_object(rb_argv0);
+    tmp_progname = rb_obj_freeze(rb_str_new_cstr(argv[0]));
+    rb_obj_freeze(tmp_progname);
     args.argc = argc;
     args.argv = argv;
     args.opt = cmdline_options_init(&opt);
@@ -1680,7 +1695,7 @@ ruby_process_options(int argc, char **argv)
     opt.intern.enc.index = -1;
     tree = (NODE *)rb_vm_call_cfunc(rb_vm_top_self(),
 				    process_options, (VALUE)&args,
-				    0, rb_progname);
+				    0, tmp_progname);
     return tree;
 }

Updated by yugui (Yuki Sonoda) over 3 years ago

  • Category set to M17N
  • Assignee set to nobu (Nobuyoshi Nakada)
  • Priority changed from Low to High
  • Target version set to 1.9.1 Release Candidate

Updated by matz (Yukihiro Matsumoto) over 3 years ago

まつもと ゆきひろです

In message "Re: [ruby-dev:37390] [BUG:trunk] r20625 dumps core and many strings associated with wrong encoding"
    on Thu, 11 Dec 2008 23:10:52 +0900, "Yugui (Yuki Sonoda)" <yugui@yugui.jp> writes:

|r20625で、default_externalもdefault_internal同様に未設定時に
|(rb_encoding_t*)NULL を返すようになった結果、trunkでruby -e 1とするとコ
|アを吐くようになりました。

あらら。default_externalについては未設定時はlocale_encoding
を返せばいいんじゃないですかね。って、そういう問題じゃない?

Updated by matz (Yukihiro Matsumoto) over 3 years ago

まつもと ゆきひろです

In message "Re: [ruby-dev:37391] Re: [BUG:trunk] r20625 dumps core and many strings associated with wrong encoding"
    on Thu, 11 Dec 2008 23:18:43 +0900, "Yugui (Yuki Sonoda)" <yugui@yugui.jp> writes:

|修正方針ってこれで良いものでしょうか?
|* LOAD_PATHのエンコーディングはどうするべき?
|  -E を考慮したdefault_externalか、それともlocale encodingを強制するか

-Eを考慮すべきですが、オプションの処理をさかのぼる必要はない
のではないかと思います。つまり、PATHの中身と-Eより前に指定さ
れた-Iについてはlocaleと見なすとか。

|* prelude内でのIOはどれほど許される?

許されるんじゃないかと。あまりpreludeが重いのは推奨しがたい
のですが(だから本当はgem_preludeはかなりイヤ)。

|* なぜこんなに早いタイミングでpreludeが実行されてるんでしょうか?

これはよくわかりません。プログラム実行直前が望ましい気がしま
すが。

|* rb_prognameをrb_gc_register_mark_objectしていましたが、rb_prognameはあ
|とから上書きされているのでこちらもrb_gc_register_mark_objectしないとまずい?

rb_gc_register_mark_object()されているのはrb_argv0ですよね。
rb_progname自身はVMから参照されているのでmarkは不要ではない
かと。

                                まつもと ゆきひろ /:|)

Updated by yugui (Yuki Sonoda) over 3 years ago

Yuguiです。

Yukihiro Matsumoto さんは書きました:
> まつもと ゆきひろです
> あらら。default_externalについては未設定時はlocale_encoding
> を返せばいいんじゃないですかね。って、そういう問題じゃない?

何らかのエンコーディングを返さざるを得ませんが、私のパッチのASCII-8BIT決
め打ちよりはlocale encodingを返しておいたほうが良さそうですね。

-- 
Yugui <yugui@yugui.jp>
http://yugui.jp
私は私をDumpする

Updated by yugui (Yuki Sonoda) over 3 years ago

Yukihiro Matsumoto さんは書きました:
> まつもと ゆきひろです
> |修正方針ってこれで良いものでしょうか?
> |* LOAD_PATHのエンコーディングはどうするべき?
> |  -E を考慮したdefault_externalか、それともlocale encodingを強制するか
> 
> -Eを考慮すべきですが、オプションの処理をさかのぼる必要はない
> のではないかと思います。つまり、PATHの中身と-Eより前に指定さ
> れた-Iについてはlocaleと見なすとか。

あー、rb_file_systemencodingの選択肢もありますよね。パス名のエンコーディ
ングをどう持つかは色々悩ましかったと思います。どうしたもんでしょう。
とりあえずlocaleにしておきますね。(r20656)

before:
ANG=ja_JP.UTF-8 ./ruby -Ecp932 -I tmp -e 'p $:.map{|x| [x, x.encoding]};
require "date"; p $LOADED_FEATURES.map{|x| [x, x.encoding]}'
[["/Users/yugui/src/ruby/mri/build/O0/tmp", #<Encoding:UTF-8>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/gems/1.9.1/gems/evil-ruby-0.1.0/lib",
#<Encoding:US-ASCII>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/site_ruby/1.9.1",
#<Encoding:ASCII-8BIT>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/site_ruby/1.9.1/i386-darwin9.5.0",
#<Encoding:ASCII-8BIT>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/site_ruby",
#<Encoding:ASCII-8BIT>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/vendor_ruby/1.9.1",
#<Encoding:ASCII-8BIT>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/vendor_ruby/1.9.1/i386-darwin9.5.0",
#<Encoding:ASCII-8BIT>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/vendor_ruby",
#<Encoding:ASCII-8BIT>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1",
#<Encoding:ASCII-8BIT>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1/i386-darwin9.5.0",
#<Encoding:ASCII-8BIT>], [".", #<Encoding:ASCII-8BIT>]]

[["enumerator.so", #<Encoding:ASCII-8BIT>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1/i386-darwin9.5.0/enc/encdb.bundle",
#<Encoding:US-ASCII>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1/i386-darwin9.5.0/enc/trans/transdb.bundle",
#<Encoding:US-ASCII>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1/rubygems.rb",
#<Encoding:US-ASCII>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1/i386-darwin9.5.0/enc/shift_jis.bundle",
#<Encoding:US-ASCII>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1/date/format.rb",
#<Encoding:US-ASCII>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1/date.rb",
#<Encoding:US-ASCII>]]

after:
[["/Users/yugui/src/ruby/mri/build/O0/tmp", #<Encoding:UTF-8>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/gems/1.9.1/gems/evil-ruby-0.1.0/lib",
#<Encoding:UTF-8>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/site_ruby/1.9.1",
#<Encoding:UTF-8>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/site_ruby/1.9.1/i386-darwin9.5.0",
#<Encoding:UTF-8>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/site_ruby",
#<Encoding:UTF-8>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/vendor_ruby/1.9.1",
#<Encoding:UTF-8>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/vendor_ruby/1.9.1/i386-darwin9.5.0",
#<Encoding:UTF-8>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/vendor_ruby",
#<Encoding:UTF-8>], ["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1",
#<Encoding:UTF-8>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1/i386-darwin9.5.0",
#<Encoding:UTF-8>], [".", #<Encoding:UTF-8>]]

[["enumerator.so", #<Encoding:ASCII-8BIT>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1/i386-darwin9.5.0/enc/encdb.bundle",
#<Encoding:US-ASCII>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1/i386-darwin9.5.0/enc/trans/transdb.bundle",
#<Encoding:US-ASCII>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1/rubygems.rb",
#<Encoding:UTF-8>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1/i386-darwin9.5.0/enc/shift_jis.bundle",
#<Encoding:US-ASCII>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1/date/format.rb",
#<Encoding:US-ASCII>],
["/Users/yugui/varyrubies/trunk-O0/lib/ruby/1.9.1/date.rb",
#<Encoding:US-ASCII>]]

-- 
Yugui <yugui@yugui.jp>
http://yugui.jp
私は私をDumpする

Updated by yugui (Yuki Sonoda) over 3 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100
Applied in changeset r20656.

Also available in: Atom PDF