ruby-changes:47250
From: normal <ko1@a...>
Date: Wed, 19 Jul 2017 10:35:12 +0900 (JST)
Subject: [ruby-changes:47250] normal:r59364 (trunk): revert r59359, r59356, r59355, r59354
normal 2017-07-19 10:35:04 +0900 (Wed, 19 Jul 2017) New Revision: 59364 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=59364 Log: revert r59359, r59356, r59355, r59354 These caused numerous CI failures I haven't been able to reproduce [ruby-core:82102] Modified files: trunk/hash.c trunk/internal.h trunk/st.c trunk/string.c trunk/test/ruby/test_optimization.rb trunk/test/ruby/test_string.rb trunk/vm.c trunk/vm_core.h Index: string.c =================================================================== --- string.c (revision 59363) +++ string.c (revision 59364) @@ -301,79 +301,27 @@ fstr_update_callback(st_data_t *key, st_ https://github.com/ruby/ruby/blob/trunk/string.c#L301 } } -static int -tainted_fstr_update(st_data_t *key, st_data_t *val, st_data_t arg, int existing) -{ - VALUE *fstr = (VALUE *)arg; - VALUE str = (VALUE)*key; - - if (existing) { - /* because of lazy sweep, str may be unmarked already and swept - * at next time */ - if (rb_objspace_garbage_object_p(str)) { - *fstr = Qundef; - return ST_DELETE; - } - - *fstr = str; - return ST_STOP; - } - else { - str = rb_str_resurrect(str); - RB_OBJ_TAINT_RAW(str); - RB_FL_SET_RAW(str, RSTRING_FSTR); - RB_OBJ_FREEZE_RAW(str); - - *key = *val = *fstr = str; - return ST_CONTINUE; - } -} - -static VALUE -register_fstring_tainted(VALUE str, st_table *tfstrings) -{ - st_data_t fstr; - - do { - fstr = (st_data_t)str; - st_update(tfstrings, fstr, tainted_fstr_update, (st_data_t)&fstr); - } while ((VALUE)fstr == Qundef); - - str = (VALUE)fstr; - assert(OBJ_FROZEN_RAW(str)); - assert(!FL_TEST_RAW(str, STR_FAKESTR)); - assert(!FL_TEST_RAW(str, FL_EXIVAR)); - assert(FL_TEST_RAW(str, RSTRING_FSTR)); - assert(FL_TEST_RAW(str, FL_TAINT)); - assert(RBASIC_CLASS(str) == rb_cString); - - return str; -} - RUBY_FUNC_EXPORTED VALUE rb_fstring(VALUE str) { VALUE fstr; - int bare_ish; + int bare; Check_Type(str, T_STRING); if (FL_TEST(str, RSTRING_FSTR)) return str; - bare_ish = !FL_TEST_RAW(str, FL_EXIVAR) && RBASIC_CLASS(str) == rb_cString; - if (STR_EMBED_P(str) && !bare_ish) { + bare = BARE_STRING_P(str); + if (STR_EMBED_P(str) && !bare) { OBJ_FREEZE_RAW(str); return str; } - if (!FL_TEST_RAW(str, FL_TAINT)) { - fstr = register_fstring(str); - } - else { - fstr = register_fstring_tainted(str, rb_vm_tfstring_table()); - } - if (!bare_ish) { + + fstr = register_fstring(str); + + if (!bare) { str_replace_shared_without_enc(str, fstr); OBJ_FREEZE_RAW(str); return str; @@ -402,59 +350,6 @@ register_fstring(VALUE str) https://github.com/ruby/ruby/blob/trunk/string.c#L350 } static VALUE -rb_fstring_existing0(VALUE str) -{ - st_table *frozen_strings = rb_vm_fstring_table(); - st_data_t fstr; - - if (st_lookup(frozen_strings, str, &fstr)) { - if (rb_objspace_garbage_object_p(fstr)) { - return register_fstring(str); - } - else { - return (VALUE)fstr; - } - } - return Qnil; -} - -static VALUE -rb_tainted_fstring_existing(VALUE str) -{ - VALUE ret; - st_data_t fstr; - st_table *tfstrings = rb_vm_tfstring_table(); - - if (st_lookup(tfstrings, str, &fstr)) { - ret = (VALUE)fstr; - if (!rb_objspace_garbage_object_p(ret)) { - return ret; - } - } - ret = rb_fstring_existing0(str); - if (NIL_P(ret)) { - return Qnil; - } - if (!RB_FL_TEST_RAW(ret, RSTRING_FSTR)) { - return Qnil; - } - - return register_fstring_tainted(str, tfstrings); -} - -VALUE -rb_fstring_existing(VALUE str) -{ - if (FL_TEST_RAW(str, RSTRING_FSTR)) - return str; - - if (!RB_OBJ_TAINTED_RAW(str)) - return rb_fstring_existing0(str); - - return rb_tainted_fstring_existing(str); -} - -static VALUE setup_fake_str(struct RString *fake_str, const char *name, long len, int encidx) { fake_str->basic.flags = T_STRING|RSTRING_NOEMBED|STR_NOFREE|STR_FAKESTR; @@ -1416,7 +1311,6 @@ rb_str_free(VALUE str) https://github.com/ruby/ruby/blob/trunk/string.c#L1311 if (FL_TEST(str, RSTRING_FSTR)) { st_data_t fstr = (st_data_t)str; st_delete(rb_vm_fstring_table(), &fstr, NULL); - st_delete(rb_vm_tfstring_table(), &fstr, NULL); RB_DEBUG_COUNTER_INC(obj_str_fstr); } Index: st.c =================================================================== --- st.c (revision 59363) +++ st.c (revision 59364) @@ -2092,23 +2092,6 @@ st_rehash(st_table *tab) https://github.com/ruby/ruby/blob/trunk/st.c#L2092 } #ifdef RUBY - -static VALUE -str_key(VALUE key) -{ - VALUE k; - - if (RB_OBJ_FROZEN(key)) { - return key; - } - if ((k = rb_fstring_existing(key)) != Qnil) { - return k; - } - else { - return rb_str_new_frozen(key); - } -} - /* Mimics ruby's { foo => bar } syntax. This function is placed here because it touches table internals and write barriers at once. */ void @@ -2131,7 +2114,8 @@ rb_hash_bulk_insert(long argc, const VAL https://github.com/ruby/ruby/blob/trunk/st.c#L2114 for (i = 0; i < argc; /* */) { VALUE key = argv[i++]; VALUE val = argv[i++]; - st_data_t k = (rb_obj_class(key) == rb_cString) ? str_key(key) : key; + st_data_t k = (rb_obj_class(key) == rb_cString) ? + rb_str_new_frozen(key) : key; st_table_entry e; e.hash = do_hash(k, tab); e.key = k; Index: hash.c =================================================================== --- hash.c (revision 59363) +++ hash.c (revision 59364) @@ -18,6 +18,7 @@ https://github.com/ruby/ruby/blob/trunk/hash.c#L18 #include "probes.h" #include "id.h" #include "symbol.h" +#include "gc.h" #ifdef __APPLE__ # ifdef HAVE_CRT_EXTERNS_H @@ -1515,13 +1516,33 @@ hash_aset(st_data_t *key, st_data_t *val https://github.com/ruby/ruby/blob/trunk/hash.c#L1516 return ST_CONTINUE; } +static VALUE +fstring_existing_str(VALUE str) +{ + st_data_t fstr; + st_table *tbl = rb_vm_fstring_table(); + + if (st_lookup(tbl, str, &fstr)) { + if (rb_objspace_garbage_object_p(fstr)) { + return rb_fstring(str); + } + else { + return (VALUE)fstr; + } + } + else { + return Qnil; + } +} + static int hash_aset_str(st_data_t *key, st_data_t *val, struct update_arg *arg, int existing) { if (!existing && !RB_OBJ_FROZEN(*key)) { VALUE k; - if ((k = rb_fstring_existing(*key)) != Qnil) { + if (!RB_OBJ_TAINTED(*key) && + (k = fstring_existing_str(*key)) != Qnil) { *key = k; } else { Index: internal.h =================================================================== --- internal.h (revision 59363) +++ internal.h (revision 59364) @@ -1590,7 +1590,6 @@ VALUE rb_strftime(const char *format, si https://github.com/ruby/ruby/blob/trunk/internal.h#L1590 /* string.c */ VALUE rb_fstring(VALUE); VALUE rb_fstring_new(const char *ptr, long len); -VALUE rb_fstring_existing(VALUE); #define rb_fstring_lit(str) rb_fstring_new((str), rb_strlen_lit(str)) #define rb_fstring_literal(str) rb_fstring_lit(str) VALUE rb_fstring_cstr(const char *str); @@ -1743,7 +1742,7 @@ void rb_vm_check_redefinition_by_prepend https://github.com/ruby/ruby/blob/trunk/internal.h#L1742 VALUE rb_yield_refine_block(VALUE refinement, VALUE refinements); VALUE ruby_vm_special_exception_copy(VALUE); PUREFUNC(st_table *rb_vm_fstring_table(void)); -PUREFUNC(st_table *rb_vm_tfstring_table(void)); + /* vm_dump.c */ void rb_print_backtrace(void); Index: vm.c =================================================================== --- vm.c (revision 59363) +++ vm.c (revision 59364) @@ -2206,10 +2206,6 @@ ruby_vm_destruct(rb_vm_t *vm) https://github.com/ruby/ruby/blob/trunk/vm.c#L2206 st_free_table(vm->frozen_strings); vm->frozen_strings = 0; } - if (vm->tainted_frozen_strings) { - st_free_table(vm->tainted_frozen_strings); - vm->tainted_frozen_strings = 0; - } rb_vm_gvl_destroy(vm); if (objspace) { rb_objspace_free(objspace); @@ -3146,8 +3142,6 @@ Init_vm_objects(void) https://github.com/ruby/ruby/blob/trunk/vm.c#L3142 vm->mark_object_ary = rb_ary_tmp_new(128); vm->loading_table = st_init_strtable(); vm->frozen_strings = st_init_table_with_size(&rb_fstring_hash_type, 1000); - vm->tainted_frozen_strings = - st_init_table_with_size(&rb_fstring_hash_type, 1000); } /* top self */ @@ -3209,12 +3203,6 @@ rb_vm_fstring_table(void) https://github.com/ruby/ruby/blob/trunk/vm.c#L3203 return GET_VM()->frozen_strings; } -st_table * -rb_vm_tfstring_table(void) -{ - return GET_VM()->tainted_frozen_strings; -} - #if VM_COLLECT_USAGE_DETAILS #define HASH_ASET(h, k, v) rb_hash_aset((h), (st_data_t)(k), (st_data_t)(v)) Index: test/ruby/test_string.rb =================================================================== --- test/ruby/test_string.rb (revision 59363) +++ test/ruby/test_string.rb (revision 59364) @@ -2750,14 +2750,8 @@ CODE https://github.com/ruby/ruby/blob/trunk/test/ruby/test_string.rb#L2750 assert_not_same(str, +str) assert_same(str, -str) - return unless @cls == String bar = %w(b a r).join('') - assert_not_predicate bar, :tainted? - assert_not_predicate str, :tainted? assert_same(str, -bar, "uminus deduplicates [Feature #13077]") - bar = %w(b a r).taint.join('') - tstr = str.dup.taint - assert_same -tstr, -bar end def test_ord Index: test/ruby/test_optimization.rb =================================================================== --- test/ruby/test_optimization.rb (revision 59363) +++ test/ruby/test_optimization.rb (revision 59364) @@ -181,44 +181,6 @@ class TestRubyOptimization < Test::Unit: https://github.com/ruby/ruby/blob/trunk/test/ruby/test_optimization.rb#L181 assert_redefine_method('Hash', 'empty?', 'assert_nil({}.empty?); assert_nil({1=>1}.empty?)') end - def test_hash_reuse_fstring - embed = -'h e l l o' - shared = -'this string is too long to be embedded and should be shared' - assert_not_equal GC::INTERNAL_CONSTANTS[:RVALUE_SIZE], - ObjectSpace.memsize_of(shared), - 'update this test if string embedding changed' - - [ embed, shared ].each do |exp| - key = exp.split(' ').join(' ') - h = {} - h[key] = true - assert_not_same key, h.keys[0] - assert_predicate h.keys[0], :frozen? - assert_same exp, h.keys[0] - - h = { key => 1 } - assert_same exp, h.keys[0], 'newhash insn should reuse strings, too' - - h1 = {} - h2 = {} - key.taint - h1[key] = true - h2[key] = true - k1 = h1.keys[0] - k2 = h2.keys[0] - assert_same k1, k2 - assert_predicate k1, :tainted? - - h = { key => 1 } - assert_not_predicate key, :frozen? - assert_same k1, h.keys[0], 'newhash insn should reuse tainted strings' - - assert_equal GC::INTERNAL_CONSTANTS[:RVALUE_SIZE], - ObjectSpace.memsize_of(k1), - 'tainted string should share with untainted fstring' - end - end - def test_hash_aref_with h = { "foo" => 1 } assert_equal 1, h["foo"] Index: vm_core.h =================================================================== --- vm_core.h (revision 59363) +++ vm_core.h (revision 59364) @@ -574,7 +574,6 @@ typedef struct rb_vm_struct { https://github.com/ruby/ruby/blob/trunk/vm_core.h#L574 VALUE *defined_strings; st_table *frozen_strings; - st_table *tainted_frozen_strings; /* params */ struct { /* size in byte */ -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/