ruby-changes:57281
From: usa <ko1@a...>
Date: Tue, 27 Aug 2019 00:59:11 +0900 (JST)
Subject: [ruby-changes:57281] usa: 689a6a0a76 (ruby_2_5): merge revision(s) 3f9562015e651735bfc2fdd14e8f6963b673e22a,c06ddfee878524168e4af07443217ed2f8d0954b,3b3b4a44e5: [Backport #15792]
https://git.ruby-lang.org/ruby.git/commit/?id=689a6a0a76 From 689a6a0a763517e5fa1fc078b2f8130e0af7c4c0 Mon Sep 17 00:00:00 2001 From: usa <usa@b2dd03c8-39d4-4d8f-98ff-823fe69b080e> Date: Mon, 26 Aug 2019 15:58:57 +0000 Subject: merge revision(s) 3f9562015e651735bfc2fdd14e8f6963b673e22a,c06ddfee878524168e4af07443217ed2f8d0954b,3b3b4a44e5: [Backport #15792] Get rid of indirect sharing * string.c (str_duplicate): share the root shared string if the original string is already sharing, so that all shared strings refer the root shared string directly. indirect sharing can cause a dangling pointer. [Bug #15792] str_duplicate: Don't share with a frozen shared string This is a follow up for 3f9562015e651735bfc2fdd14e8f6963b673e22a. Before this commit, it was possible to create a shared string which shares with another shared string by passing a frozen shared string to `str_duplicate`. Such string looks like: ``` -------- ----------------- | root | ------ owns -----> | root's buffer | -------- ----------------- ^ ^ ^ ----------- | | | shared1 | ------ references ----- | ----------- | ^ | ----------- | | shared2 | ------ references --------- ----------- ``` This is bad news because `rb_fstring(shared2)` can make `shared1` independent, which severs the reference from `shared1` to `root`: ```c /* from fstr_update_callback() */ str = str_new_frozen(rb_cString, shared2); /* can return shared1 */ if (STR_SHARED_P(str)) { /* shared1 is also a shared string */ str_make_independent(str); /* no frozen check */ } ``` If `shared1` was the only reference to `root`, then `root` can be reclaimed by the GC, leaving `shared2` in a corrupted state: ``` ----------- -------------------- | shared1 | -------- owns --------> | shared1's buffer | ----------- -------------------- ^ | ----------- ------------------------- | shared2 | ------ references ----> | root's buffer (freed) | ----------- ------------------------- ``` Here is a reproduction script for the situation this commit fixes. ```ruby a = ('a' * 24).strip.freeze.strip -a p a 4.times { GC.start } p a ``` - string.c (str_duplicate): always share with the root string when the original is a shared string. - test_rb_str_dup.rb: specifically test `rb_str_dup` to make sure it does not try to share with a shared string. [Bug #15792] Closes: https://github.com/ruby/ruby/pull/2159 Update dependencies git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_5@67766 b2dd03c8-39d4-4d8f-98ff-823fe69b080e diff --git a/ext/-test-/string/depend b/ext/-test-/string/depend index 8e7ee2a..71e995a 100644 --- a/ext/-test-/string/depend +++ b/ext/-test-/string/depend @@ -173,6 +173,17 @@ qsort.o: $(hdrdir)/ruby/subst.h https://github.com/ruby/ruby/blob/trunk/ext/-test-/string/depend#L173 qsort.o: $(hdrdir)/ruby/util.h qsort.o: $(top_srcdir)/include/ruby.h qsort.o: qsort.c +rb_str_dup.o: $(RUBY_EXTCONF_H) +rb_str_dup.o: $(arch_hdrdir)/ruby/config.h +rb_str_dup.o: $(hdrdir)/ruby.h +rb_str_dup.o: $(hdrdir)/ruby/backward.h +rb_str_dup.o: $(hdrdir)/ruby/defines.h +rb_str_dup.o: $(hdrdir)/ruby/intern.h +rb_str_dup.o: $(hdrdir)/ruby/missing.h +rb_str_dup.o: $(hdrdir)/ruby/ruby.h +rb_str_dup.o: $(hdrdir)/ruby/st.h +rb_str_dup.o: $(hdrdir)/ruby/subst.h +rb_str_dup.o: rb_str_dup.c set_len.o: $(RUBY_EXTCONF_H) set_len.o: $(arch_hdrdir)/ruby/config.h set_len.o: $(hdrdir)/ruby/backward.h diff --git a/ext/-test-/string/rb_str_dup.c b/ext/-test-/string/rb_str_dup.c new file mode 100644 index 0000000..a0bd658 --- /dev/null +++ b/ext/-test-/string/rb_str_dup.c @@ -0,0 +1,35 @@ https://github.com/ruby/ruby/blob/trunk/ext/-test-/string/rb_str_dup.c#L1 +#include "ruby.h" + +VALUE rb_str_dup(VALUE str); + +static VALUE +bug_rb_str_dup(VALUE self, VALUE str) +{ + rb_check_type(str, T_STRING); + return rb_str_dup(str); +} + +static VALUE +bug_shared_string_p(VALUE self, VALUE str) +{ + rb_check_type(str, T_STRING); + return RB_FL_TEST(str, RUBY_ELTS_SHARED) && RB_FL_TEST(str, RSTRING_NOEMBED) ? Qtrue : Qfalse; +} + +static VALUE +bug_sharing_with_shared_p(VALUE self, VALUE str) +{ + rb_check_type(str, T_STRING); + if (bug_shared_string_p(self, str)) { + return bug_shared_string_p(self, RSTRING(str)->as.heap.aux.shared); + } + return Qfalse; +} + +void +Init_string_rb_str_dup(VALUE klass) +{ + rb_define_singleton_method(klass, "rb_str_dup", bug_rb_str_dup, 1); + rb_define_singleton_method(klass, "shared_string?", bug_shared_string_p, 1); + rb_define_singleton_method(klass, "sharing_with_shared?", bug_sharing_with_shared_p, 1); +} diff --git a/string.c b/string.c index 2bc1d25..09fe2a3 100644 --- a/string.c +++ b/string.c @@ -1472,10 +1472,13 @@ str_duplicate(VALUE klass, VALUE str) https://github.com/ruby/ruby/blob/trunk/string.c#L1472 MEMCPY(RSTRING(dup)->as.ary, RSTRING(str)->as.ary, char, embed_size); if (flags & STR_NOEMBED) { - if (UNLIKELY(!(flags & FL_FREEZE))) { - str = str_new_frozen(klass, str); - FL_SET_RAW(str, flags & FL_TAINT); - flags = FL_TEST_RAW(str, flag_mask); + if (FL_TEST_RAW(str, STR_SHARED)) { + str = RSTRING(str)->as.heap.aux.shared; + } + else if (UNLIKELY(!(flags & FL_FREEZE))) { + str = str_new_frozen(klass, str); + FL_SET_RAW(str, flags & FL_TAINT); + flags = FL_TEST_RAW(str, flag_mask); } if (flags & STR_NOEMBED) { RB_OBJ_WRITE(dup, &RSTRING(dup)->as.heap.aux.shared, str); diff --git a/test/-ext-/string/test_rb_str_dup.rb b/test/-ext-/string/test_rb_str_dup.rb new file mode 100644 index 0000000..49b6af9 --- /dev/null +++ b/test/-ext-/string/test_rb_str_dup.rb @@ -0,0 +1,16 @@ https://github.com/ruby/ruby/blob/trunk/test/-ext-/string/test_rb_str_dup.rb#L1 +require 'test/unit' +require '-test-/string' + +class Test_RbStrDup < Test::Unit::TestCase + def test_nested_shared_non_frozen + str = Bug::String.rb_str_dup(Bug::String.rb_str_dup("a" * 50)) + assert_send([Bug::String, :shared_string?, str]) + assert_not_send([Bug::String, :sharing_with_shared?, str], '[Bug #15792]') + end + + def test_nested_shared_frozen + str = Bug::String.rb_str_dup(Bug::String.rb_str_dup("a" * 50).freeze) + assert_send([Bug::String, :shared_string?, str]) + assert_not_send([Bug::String, :sharing_with_shared?, str], '[Bug #15792]') + end +end diff --git a/test/ruby/test_string.rb b/test/ruby/test_string.rb index c0c4fe4..d4303e8 100644 --- a/test/ruby/test_string.rb +++ b/test/ruby/test_string.rb @@ -2921,6 +2921,15 @@ CODE https://github.com/ruby/ruby/blob/trunk/test/ruby/test_string.rb#L2921 end =end + def test_nesting_shared + a = ('a' * 24).encode(Encoding::ASCII).gsub('x', '') + hash = {} + hash[a] = true + assert_equal(('a' * 24), a) + 4.times { GC.start } + assert_equal(('a' * 24), a, '[Bug #15792]') + end + def test_shared_force_encoding s = "\u{3066}\u{3059}\u{3068}".gsub(//, '') h = {} diff --git a/version.h b/version.h index c4b3f14..8d09ff8 100644 --- a/version.h +++ b/version.h @@ -1,6 +1,6 @@ https://github.com/ruby/ruby/blob/trunk/version.h#L1 #define RUBY_VERSION "2.5.6" #define RUBY_RELEASE_DATE "2019-08-27" -#define RUBY_PATCHLEVEL 184 +#define RUBY_PATCHLEVEL 185 #define RUBY_RELEASE_YEAR 2019 #define RUBY_RELEASE_MONTH 8 -- cgit v0.10.2 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/