ruby-dev

なかだです。

At Mon, 4 Oct 2004 17:50:20 +0900,
Yukihiro Matsumoto wrote in [ruby-dev:24430]:
> |[ruby-dev:24412]だと、read中に例外で抜けたときにfreezeされたま
> |まになってしまうので、こうするのはどうでしょうか。
> 
> rb_io_freadは例外を発生させないので、割り込みのことですかね。
> まあ、基本的なアイディアは有効だと思います。コミットしてくだ
> さい。
> 
> # なるほどshared stringを使うのか。

送った後で気づいたんですが、単純にあれだけではまずそうです。

  buf = rb_str_new4(str);
  ptr = RSTRING(buf)->ptr;
  ...
  return str;

のような場合、

(1) bufが最適化によって消されている場合、途中でstrが変更される
    と、shared stringはGCから保護されない
    →dangling pointer

(2) (each_objectなどによって捜し出されて)bufからさらに共有する
    Stringが作られた場合、rb_io_fread後はそのptrはfreezeされて
    いないStringと共有されてしまう
    →dangling pointer または double free

(3) rb_str_shared_replace()は最初にrb_str_modify()を呼ぶため、
    直後に解放するはずのptrを複製している
    →バッファを引数で指定する意味がない


Index: intern.h
===================================================================
RCS file: /cvs/ruby/src/ruby/intern.h,v
retrieving revision 1.156
diff -U2 -p -d -r1.156 intern.h
--- intern.h	6 Oct 2004 07:40:04 -0000	1.156
+++ intern.h	7 Oct 2004 03:37:00 -0000
@@ -415,4 +415,5 @@ VALUE rb_str_buf_append _((VALUE, VALUE)
 VALUE rb_str_buf_cat _((VALUE, const char*, long));
 VALUE rb_str_buf_cat2 _((VALUE, const char*));
+void rb_str_shared_replace _((VALUE, VALUE));
 VALUE rb_obj_as_string _((VALUE));
 VALUE rb_check_string_type _((VALUE));
Index: io.c
===================================================================
RCS file: /cvs/ruby/src/ruby/io.c,v
retrieving revision 1.299
diff -U2 -p -d -r1.299 io.c
--- io.c	6 Oct 2004 15:10:43 -0000	1.299
+++ io.c	7 Oct 2004 03:30:12 -0000
@@ -1172,5 +1172,5 @@ io_read(argc, argv, io)
     OpenFile *fptr;
     long n, len;
-    VALUE length, str;
+    VALUE length, str, buf;
 
     rb_scan_args(argc, argv, "02", &length, &str);
@@ -1196,10 +1196,10 @@ io_read(argc, argv, io)
 	rb_str_resize(str,len);
     }
-    FL_SET(str, FL_FREEZE);
     if (len == 0) return str;
 
+    buf = rb_str_new4(str);
+    RBASIC(buf)->klass = 0;
     READ_CHECK(fptr->f);
-    n = rb_io_fread(RSTRING(str)->ptr, len, fptr->f);
-    FL_UNSET(str, FL_FREEZE);
+    n = rb_io_fread(RSTRING(buf)->ptr, len, fptr->f);
     if (n == 0) {
 	rb_str_resize(str,0);
@@ -1208,7 +1208,9 @@ io_read(argc, argv, io)
 	if (len > 0) rb_sys_fail(fptr->path);
     }
+    rb_str_shared_replace(str, buf);
     RSTRING(str)->len = n;
     RSTRING(str)->ptr[n] = '\0';
     OBJ_TAINT(str);
+    rb_gc_force_recycle(buf);
 
     return str;
@@ -4155,5 +4157,6 @@ next_argv()
 	if (RARRAY(rb_argv)->len > 0) {
 	    filename = rb_ary_shift(rb_argv);
-	    fn = StringValuePtr(filename);
+	    filename = rb_str_new4(StringValue(filename));
+	    fn = RSTRING(filename)->ptr;
 	    if (strlen(fn) == 1 && fn[0] == '-') {
 		current_file = rb_stdin;
@@ -4168,5 +4171,4 @@ next_argv()
 		if (ruby_inplace_mode) {
 		    struct stat st, st2;
-		    VALUE str;
 		    FILE *fw;
 
@@ -4176,5 +4178,5 @@ next_argv()
 		    fstat(fileno(fr), &st);
 		    if (*ruby_inplace_mode) {
-			str = rb_str_new2(fn);
+			VALUE str = rb_str_new2(fn);
 #ifdef NO_LONG_FNAME
                         ruby_add_suffix(str, ruby_inplace_mode);
@@ -4182,4 +4184,5 @@ next_argv()
 			rb_str_cat2(str, ruby_inplace_mode);
 #endif
+			OBJ_FREEZE(str);
 #ifdef NO_SAFE_RENAME
 			(void)fclose(fr);
Index: string.c
===================================================================
RCS file: /cvs/ruby/src/ruby/string.c,v
retrieving revision 1.204
diff -U2 -p -d -r1.204 string.c
--- string.c	6 Oct 2004 07:40:03 -0000	1.204
+++ string.c	7 Oct 2004 03:40:01 -0000
@@ -37,4 +37,6 @@ VALUE rb_cString;
 VALUE rb_fs;
 
+static int str_independent _((VALUE));
+
 static VALUE str_alloc _((VALUE));
 static VALUE
@@ -232,11 +234,10 @@ rb_str_to_str(str)
 }
 
-static void
+void
 rb_str_shared_replace(str, str2)
     VALUE str, str2;
 {
     if (str == str2) return;
-    rb_str_modify(str);
-    if (!FL_TEST(str, ELTS_SHARED)) free(RSTRING(str)->ptr);
+    if (str_independent(str)) free(RSTRING(str)->ptr);
     if (NIL_P(str2)) {
 	RSTRING(str)->ptr = 0;


-- 
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦