Skip to content

Commit

Permalink
Pin embedded shared strings
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
peterzhu2118 committed Dec 1, 2023
1 parent 0ed55bf commit 80ea7fb
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 23 deletions.
12 changes: 10 additions & 2 deletions gc.c
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion internal/string.h
Expand Up @@ -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);
Expand Down
20 changes: 0 additions & 20 deletions string.c
Expand Up @@ -279,26 +279,6 @@ rb_str_make_embedded(VALUE str)
TERM_FILL(RSTRING(str)->as.embed.ary + len, TERM_LEN(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)
{
Expand Down

0 comments on commit 80ea7fb

Please sign in to comment.