summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Zhu <peter@peterzhu.ca>2022-12-23 09:33:51 -0500
committerPeter Zhu <peter@peterzhu.ca>2022-12-23 11:21:14 -0500
commit7891f9407184ca6b4636092555d2546e3830c2ff (patch)
tree0f49dff240166db2b96fd5b81fecadf7cbc1e1d0
parentd61a4cec618b8f5554398fff2ee4656763aeec4e (diff)
downloadruby-7891f9407184ca6b4636092555d2546e3830c2ff.tar.gz
Don't allow re-embedding frozen arrays
Frozen arrays should not move from heap allocated to embedded because frozen arrays could be shared roots for other (shared) arrays. If the frozen array moves from heap allocated to embedded it would cause issues since the shared array would no longer know where to set the pointer in the shared root.
-rw-r--r--array.c11
-rw-r--r--test/ruby/test_gc_compact.rb26
2 files changed, 35 insertions, 2 deletions
diff --git a/array.c b/array.c
index 4d7d60ba49..654c79a44d 100644
--- a/array.c
+++ b/array.c
@@ -226,8 +226,15 @@ ary_embeddable_p(long capa)
bool
rb_ary_embeddable_p(VALUE ary)
{
- // if the array is shared or a shared root then it's not moveable
- return !(ARY_SHARED_P(ary) || ARY_SHARED_ROOT_P(ary));
+ /* An array cannot be turned embeddable when the array is:
+ * - Shared root: other objects may point to the buffer of this array
+ * so we cannot make it embedded.
+ * - Frozen: this array may also be a shared root without the shared root
+ * flag.
+ * - Shared: we don't want to re-embed an array that points to a shared
+ * root (to save memory).
+ */
+ return !(ARY_SHARED_ROOT_P(ary) || OBJ_FROZEN(ary) || ARY_SHARED_P(ary));
}
size_t
diff --git a/test/ruby/test_gc_compact.rb b/test/ruby/test_gc_compact.rb
index c5fac64ca4..b82ffe13ff 100644
--- a/test/ruby/test_gc_compact.rb
+++ b/test/ruby/test_gc_compact.rb
@@ -209,6 +209,32 @@ class TestGCCompact < Test::Unit::TestCase
assert_equal([:call, :line], results)
end
+ def test_updating_references_for_heap_allocated_frozen_shared_arrays
+ assert_separately(%w[-robjspace], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 10, signal: :SEGV)
+ begin;
+ ary = []
+ 50.times { |i| ary << i }
+ # Frozen arrays can become shared root without RARRAY_SHARED_ROOT_FLAG
+ ary.freeze
+
+ slice = ary[10..40]
+
+ # Check that slice is pointing to buffer of ary
+ assert_include(ObjectSpace.dump(slice), '"shared":true')
+
+ # Run compaction to re-embed ary
+ GC.verify_compaction_references(expand_heap: true, toward: :empty)
+
+ # Assert that slice is pointer to updated buffer in ary
+ assert_equal(10, slice[0])
+ # Check that slice is still pointing to buffer of ary
+ assert_include(ObjectSpace.dump(slice), '"shared":true')
+ end;
+ end
+
+ end;
+ end
+
def test_moving_arrays_down_size_pools
omit if GC::INTERNAL_CONSTANTS[:SIZE_POOL_COUNT] == 1
assert_separately([], "#{<<~"begin;"}\n#{<<~"end;"}", timeout: 10, signal: :SEGV)