From 7891f9407184ca6b4636092555d2546e3830c2ff Mon Sep 17 00:00:00 2001
From: Peter Zhu <peter@peterzhu.ca>
Date: Fri, 23 Dec 2022 09:33:51 -0500
Subject: [PATCH] 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.
---
 array.c                      | 11 +++++++++--
 test/ruby/test_gc_compact.rb | 26 ++++++++++++++++++++++++++
 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)