From 4ab69ebbd7cef8539f687e1f948845d076461dc6 Mon Sep 17 00:00:00 2001 From: nobu Date: Fri, 24 Jul 2015 07:38:37 +0000 Subject: [PATCH] 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 | 8 ++++ ext/-test-/string/fstring.c | 15 ++++++++ string.c | 46 +++++++++++++++++++--- test/-ext-/string/test_fstring.rb | 64 +++++++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 6 deletions(-) create mode 100644 ext/-test-/string/fstring.c create mode 100644 test/-ext-/string/test_fstring.rb diff --git a/ChangeLog b/ChangeLog index 9a0d44d5aa..cce2c17aee 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +Fri Jul 24 16:35:55 2015 Nobuyoshi Nakada + + * 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 * file.c (rb_file_s_extname): [DOC] add an example. diff --git a/ext/-test-/string/fstring.c b/ext/-test-/string/fstring.c new file mode 100644 index 0000000000..b65c98ce6d --- /dev/null +++ b/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); +} diff --git a/string.c b/string.c index 46dafedc9d..4e02916fda 100644 --- a/string.c +++ b/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)); diff --git a/test/-ext-/string/test_fstring.rb b/test/-ext-/string/test_fstring.rb new file mode 100644 index 0000000000..3ad9a4c312 --- /dev/null +++ b/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