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

6 files changed, 79 insertions, 5 deletions

diff --git a/ext/-test-/string/depend b/ext/-test-/string/depend
index 8e7ee2a55a..71e995a523 100644
--- a/ext/-test-/string/depend
+++ b/ext/-test-/string/depend
@@ -173,6 +173,17 @@ qsort.o: $(hdrdir)/ruby/subst.h
 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 0000000000..a0bd65820f
--- /dev/null
+++ b/ext/-test-/string/rb_str_dup.c
@@ -0,0 +1,35 @@
+#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 2bc1d25e4d..09fe2a3dbb 100644
--- a/string.c
+++ b/string.c
@@ -1472,10 +1472,13 @@ str_duplicate(VALUE klass, VALUE str)
     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 0000000000..49b6af9598
--- /dev/null
+++ b/test/-ext-/string/test_rb_str_dup.rb
@@ -0,0 +1,16 @@
+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 c0c4fe451a..d4303e87c1 100644
--- a/test/ruby/test_string.rb
+++ b/test/ruby/test_string.rb
@@ -2921,6 +2921,15 @@ CODE
   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 c4b3f14e07..8d09ff8fd8 100644
--- a/version.h
+++ b/version.h
@@ -1,6 +1,6 @@
 #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