mirror of
https://github.com/ruby/ruby.git
synced 2022-11-09 12:17:21 -05:00
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 3f9562015e
.
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
This commit is contained in:
parent
470dae33ea
commit
689a6a0a76
6 changed files with 79 additions and 5 deletions
|
@ -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
|
||||
|
|
35
ext/-test-/string/rb_str_dup.c
Normal file
35
ext/-test-/string/rb_str_dup.c
Normal file
|
@ -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);
|
||||
}
|
11
string.c
11
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);
|
||||
|
|
16
test/-ext-/string/test_rb_str_dup.rb
Normal file
16
test/-ext-/string/test_rb_str_dup.rb
Normal file
|
@ -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
|
|
@ -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 = {}
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue