string.c: pool only bare strings in fstring

* string.c (fstr_update_callback): pool bare strings only.
* string.c (rb_fstring): return the original string with sharing a
  fstring if it has extra attributes, not the fstring itself.
  [ruby-dev:49188] [Bug #11386]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@51360 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

ChangeLog

@@ -1,3 +1,11 @@
+Fri Jul 24 16:35:55 2015  Nobuyoshi Nakada  <nobu@ruby-lang.org>
+
+	* string.c (fstr_update_callback): pool bare strings only.
+
+	* string.c (rb_fstring): return the original string with sharing a
+	  fstring if it has extra attributes, not the fstring itself.
+	  [ruby-dev:49188] [Bug #11386]
+
 Fri Jul 24 16:35:34 2015  yui-knk  <spiketeika@gmail.com>
 
 	* file.c (rb_file_s_extname): [DOC] add an example.

ext/-test-/string/fstring.c

@@ -0,0 +1,15 @@
+#include "ruby.h"
+
+VALUE rb_fstring(VALUE str);
+
+VALUE
+bug_s_fstring(VALUE self, VALUE str)
+{
+    return rb_fstring(str);
+}
+
+void
+Init_fstring(VALUE klass)
+{
+    rb_define_singleton_method(klass, "fstring", bug_s_fstring, 1);
+}

string.c

@@ -155,6 +155,9 @@ VALUE rb_cSymbol;
 #define SHARABLE_SUBSTRING_P(beg, len, end) 1
 #endif
 
+static VALUE str_replace_shared_without_enc(VALUE str2, VALUE str);
+static VALUE str_new_shared(VALUE klass, VALUE str);
+static VALUE str_new_frozen(VALUE klass, VALUE orig);
 static VALUE str_new_static(VALUE klass, const char *ptr, long len, int encindex);
 static void str_make_independent_expand(VALUE str, long expand);
 
@@ -228,6 +231,8 @@ static const struct st_hash_type fstring_hash_type = {
     rb_str_hash,
 };
 
+#define BARE_STRING_P(str) (!FL_ANY_RAW(str, FL_TAINT|FL_EXIVAR) && RBASIC_CLASS(str) == rb_cString)
+
 static int
 fstr_update_callback(st_data_t *key, st_data_t *value, st_data_t arg, int existing)
 {
@@ -254,10 +259,15 @@ fstr_update_callback(st_data_t *key, st_data_t *value, st_data_t arg, int existi
 	    OBJ_FREEZE_RAW(str);
 	}
 	else {
-	    str = rb_str_new_frozen(str);
+	    str = str_new_frozen(rb_cString, str);
 	    if (STR_SHARED_P(str)) { /* str should not be shared */
 		/* shared substring  */
 		str_make_independent_expand(str, 0L);
+		assert(OBJ_FROZEN(str));
+	    }
+	    if (!BARE_STRING_P(str)) {
+		str = str_new_shared(rb_cString, str);
+		OBJ_FREEZE_RAW(str);
 	    }
 	}
 	RBASIC(str)->flags |= RSTRING_FSTR;
@@ -267,15 +277,32 @@ fstr_update_callback(st_data_t *key, st_data_t *value, st_data_t arg, int existi
     }
 }
 
+RUBY_FUNC_EXPORTED
 VALUE
 rb_fstring(VALUE str)
 {
+    VALUE fstr;
+    int bare;
+
     Check_Type(str, T_STRING);
 
     if (FL_TEST(str, RSTRING_FSTR))
 	return str;
 
-    return register_fstring(str);
+    bare = BARE_STRING_P(str);
+    if (STR_EMBED_P(str) && !bare) {
+	OBJ_FREEZE_RAW(str);
+	return str;
+    }
+
+    fstr = register_fstring(str);
+
+    if (!bare) {
+	str_replace_shared_without_enc(str, fstr);
+	OBJ_FREEZE_RAW(str);
+	return str;
+    }
+    return fstr;
 }
 
 static VALUE
@@ -286,11 +313,14 @@ register_fstring(VALUE str)
     do {
 	ret = str;
 	st_update(rb_vm_fstring_table(), (st_data_t)str,
-		    fstr_update_callback, (st_data_t)&ret);
+		  fstr_update_callback, (st_data_t)&ret);
     } while (ret == Qundef);
 
     assert(OBJ_FROZEN(ret));
     assert(!FL_TEST_RAW(ret, STR_FAKESTR));
+    assert(!FL_TEST_RAW(ret, FL_EXIVAR));
+    assert(!FL_TEST_RAW(ret, FL_TAINT));
+    assert(RBASIC_CLASS(ret) == rb_cString);
     return ret;
 }
 
@@ -972,11 +1002,15 @@ rb_str_new_shared(VALUE str)
 VALUE
 rb_str_new_frozen(VALUE orig)
 {
-    VALUE klass, str;
-
     if (OBJ_FROZEN(orig)) return orig;
 
-    klass = rb_obj_class(orig);
+    return str_new_frozen(rb_obj_class(orig), orig);
+}
+
+static VALUE
+str_new_frozen(VALUE klass, VALUE orig)
+{
+    VALUE str;
 
     if (STR_EMBED_P(orig)) {
 	str = str_new(klass, RSTRING_PTR(orig), RSTRING_LEN(orig));

test/-ext-/string/test_fstring.rb

@@ -0,0 +1,64 @@
+require 'test/unit'
+require '-test-/string'
+
+class Test_String_Fstring < Test::Unit::TestCase
+  def assert_fstring(str)
+    fstr = Bug::String.fstring(str)
+    yield str
+    yield fstr
+  end
+
+  def test_taint_shared_string
+    str = __method__.to_s.dup
+    str.taint
+    assert_fstring(str) {|s| assert_predicate(s, :tainted?)}
+  end
+
+  def test_taint_normal_string
+    str = __method__.to_s * 3
+    str.taint
+    assert_fstring(str) {|s| assert_predicate(s, :tainted?)}
+  end
+
+  def test_taint_registered_tainted
+    str = __method__.to_s * 3
+    str.taint
+    assert_fstring(str) {|s| assert_predicate(s, :tainted?)}
+
+    str = __method__.to_s * 3
+    assert_fstring(str) {|s| assert_not_predicate(s, :tainted?)}
+  end
+
+  def test_taint_registered_untainted
+    str = __method__.to_s * 3
+    assert_fstring(str) {|s| assert_not_predicate(s, :tainted?)}
+
+    str = __method__.to_s * 3
+    str.taint
+    assert_fstring(str) {|s| assert_predicate(s, :tainted?)}
+  end
+
+  def test_instance_variable
+    str = __method__.to_s * 3
+    str.instance_variable_set(:@test, 42)
+    str.freeze
+    assert_fstring(str) {|s| assert_send([s, :instance_variable_defined?, :@test])}
+  end
+
+  def test_singleton_method
+    str = __method__.to_s * 3
+    def str.foo
+    end
+    str.freeze
+    assert_fstring(str) {|s| assert_send([s, :respond_to?, :foo])}
+  end
+
+  class S < String
+  end
+
+  def test_subclass
+    str = S.new(__method__.to_s * 3)
+    str.freeze
+    assert_fstring(str) {|s| assert_instance_of(S, s)}
+  end
+end