Pin embedded shared strings

Embedded shared strings cannot be moved because strings point into the
slot of the shared string. There may be code using the RSTRING_PTR on
the stack, which would pin the string but not pin the shared string,
causing it to move.

3 files changed, 10 insertions, 23 deletions

diff --git a/gc.c b/gc.c
index dc9974be22..9282f8a136 100644
--- a/gc.c
+++ b/gc.c
@@ -7346,7 +7346,16 @@ gc_mark_children(rb_objspace_t *objspace, VALUE obj)
 
       case T_STRING:
         if (STR_SHARED_P(obj)) {
-            gc_mark(objspace, any->as.string.as.heap.aux.shared);
+            if (STR_EMBED_P(any->as.string.as.heap.aux.shared)) {
+                /* Embedded shared strings cannot be moved because this string
+                 * points into the slot of the shared string. There may be code
+                 * using the RSTRING_PTR on the stack, which would pin this
+                 * string but not pin the shared string, causing it to move. */
+                gc_mark_and_pin(objspace, any->as.string.as.heap.aux.shared);
+            }
+            else {
+                gc_mark(objspace, any->as.string.as.heap.aux.shared);
+            }
         }
         break;
 
@@ -10701,7 +10710,6 @@ gc_update_object_references(rb_objspace_t *objspace, VALUE obj)
                 VALUE old_root = any->as.string.as.heap.aux.shared;
                 UPDATE_IF_MOVED(objspace, any->as.string.as.heap.aux.shared);
                 VALUE new_root = any->as.string.as.heap.aux.shared;
-                rb_str_update_shared_ary(obj, old_root, new_root);
             }
 
             /* If, after move the string is not embedded, and can fit in the
diff --git a/internal/string.h b/internal/string.h
index abb0a536ad..8c481f979e 100644
--- a/internal/string.h
+++ b/internal/string.h
@@ -64,7 +64,6 @@ VALUE rb_str_upto_endless_each(VALUE, int (*each)(VALUE, VALUE), VALUE);
 void rb_str_make_embedded(VALUE);
 size_t rb_str_size_as_embedded(VALUE);
 bool rb_str_reembeddable_p(VALUE);
-void rb_str_update_shared_ary(VALUE str, VALUE old_root, VALUE new_root);
 RUBY_SYMBOL_EXPORT_END
 
 VALUE rb_fstring_new(const char *ptr, long len);
diff --git a/string.c b/string.c
index 662987431e..8af608047a 100644
--- a/string.c
+++ b/string.c
@@ -280,26 +280,6 @@ rb_str_make_embedded(VALUE str)
 }
 
 void
-rb_str_update_shared_ary(VALUE str, VALUE old_root, VALUE new_root)
-{
-    // if the root location hasn't changed, we don't need to update
-    if (new_root == old_root) {
-        return;
-    }
-
-    // if the root string isn't embedded, we don't need to touch the pointer.
-    // it already points to the shame shared buffer
-    if (!STR_EMBED_P(new_root)) {
-        return;
-    }
-
-    size_t offset = (size_t)((uintptr_t)RSTRING(str)->as.heap.ptr - (uintptr_t)RSTRING(old_root)->as.embed.ary);
-
-    RUBY_ASSERT(RSTRING(str)->as.heap.ptr >= RSTRING(old_root)->as.embed.ary);
-    RSTRING(str)->as.heap.ptr = RSTRING(new_root)->as.embed.ary + offset;
-}
-
-void
 rb_debug_rstring_null_ptr(const char *func)
 {
     fprintf(stderr, "%s is returning NULL!! "