https://git.ruby-lang.org/ruby.git/commit/?id=c32218de1b

From c32218de1ba094223420a4ea017707f48d0009c5 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <tenderlove@r...>
Date: Wed, 25 Nov 2020 09:24:50 -0800
Subject: Disable auto compaction on platforms that can't support it

Both explicit compaction routines (gc_compact and the verify references form)
need to clear the heap before executing compaction.  Otherwise some
objects may not be alive, and we'll need the read barrier.  The heap
must only contain *live* objects if we want to disable the read barrier
during explicit compaction.

The previous commit was missing the "clear the heap" phase from the
"verify references" explicit compaction function.

Fixes [Bug #17306]

diff --git a/gc.c b/gc.c
index 5573ee5..195a753 100644
--- a/gc.c
+++ b/gc.c
@@ -682,7 +682,7 @@ typedef struct rb_objspace { https://github.com/ruby/ruby/blob/trunk/gc.c#L682
 	unsigned int dont_gc : 1;
 	unsigned int dont_incremental : 1;
 	unsigned int during_gc : 1;
-        unsigned int during_compacting : 1;
+        unsigned int during_compacting : 2;
 	unsigned int gc_stressful: 1;
 	unsigned int has_hook: 1;
 	unsigned int during_minor_gc : 1;
@@ -3090,6 +3090,17 @@ Init_heap(void) https://github.com/ruby/ruby/blob/trunk/gc.c#L3090
 {
     rb_objspace_t *objspace = &rb_objspace;
 
+#if defined(HAVE_SYSCONF) && defined(_SC_PAGE_SIZE)
+    /* If Ruby's heap pages are not a multiple of the system page size, we
+     * cannot use mprotect for the read barrier, so we must disable automatic
+     * compaction. */
+    int pagesize;
+    pagesize = (int)sysconf(_SC_PAGE_SIZE);
+    if ((HEAP_PAGE_SIZE % pagesize) != 0) {
+        ruby_enable_autocompact = 0;
+    }
+#endif
+
     objspace->next_object_id = INT2FIX(OBJ_ID_INITIAL);
     objspace->id_to_obj_tbl = st_init_table(&object_id_hash_type);
     objspace->obj_to_id_tbl = st_init_numtable();
@@ -4389,6 +4400,11 @@ static VALUE gc_move(rb_objspace_t *objspace, VALUE scan, VALUE free); https://github.com/ruby/ruby/blob/trunk/gc.c#L4400
 static void
 lock_page_body(rb_objspace_t *objspace, struct heap_page_body *body)
 {
+    /* If this is an explicit compaction (GC.compact), we don't need a read
+     * barrier, so just return early. */
+    if (objspace->flags.during_compacting >> 1) {
+        return;
+    }
 #if defined(_WIN32)
     DWORD old_protect;
 
@@ -4405,6 +4421,11 @@ lock_page_body(rb_objspace_t *objspace, struct heap_page_body *body) https://github.com/ruby/ruby/blob/trunk/gc.c#L4421
 static void
 unlock_page_body(rb_objspace_t *objspace, struct heap_page_body *body)
 {
+    /* If this is an explicit compaction (GC.compact), we don't need a read
+     * barrier, so just return early. */
+    if (objspace->flags.during_compacting >> 1) {
+        return;
+    }
 #if defined(_WIN32)
     DWORD old_protect;
 
@@ -4624,8 +4645,12 @@ gc_compact_finish(rb_objspace_t *objspace, rb_heap_t *heap) https://github.com/ruby/ruby/blob/trunk/gc.c#L4645
 {
     GC_ASSERT(heap->sweeping_page == heap->compact_cursor);
 
-    gc_unprotect_pages(objspace, heap);
-    uninstall_handlers();
+    /* If this is an explicit compaction (GC.compact), no read barrier was set
+     * so we don't need to unprotect pages or uninstall the SEGV handler */
+    if (!(objspace->flags.during_compacting >> 1)) {
+        gc_unprotect_pages(objspace, heap);
+        uninstall_handlers();
+    }
 
     /* The mutator is allowed to run during incremental sweeping. T_MOVED
      * objects can get pushed on the stack and when the compaction process
@@ -5134,6 +5159,12 @@ gc_compact_start(rb_objspace_t *objspace, rb_heap_t *heap) https://github.com/ruby/ruby/blob/trunk/gc.c#L5159
     memset(objspace->rcompactor.considered_count_table, 0, T_MASK * sizeof(size_t));
     memset(objspace->rcompactor.moved_count_table, 0, T_MASK * sizeof(size_t));
 
+    /* If this is an explicit compaction (GC.compact), we don't need a read
+     * barrier, so just return early. */
+    if (objspace->flags.during_compacting >> 1) {
+        return;
+    }
+
     /* Set up read barrier for pages containing MOVED objects */
     install_handlers();
 }
@@ -7030,7 +7061,7 @@ gc_marks_start(rb_objspace_t *objspace, int full_mark) https://github.com/ruby/ruby/blob/trunk/gc.c#L7061
 #endif
 	objspace->flags.during_minor_gc = FALSE;
         if (ruby_enable_autocompact) {
-            objspace->flags.during_compacting = TRUE;
+            objspace->flags.during_compacting |= TRUE;
         }
 	objspace->profile.major_gc_count++;
 	objspace->rgengc.uncollectible_wb_unprotected_objects = 0;
@@ -8057,7 +8088,9 @@ gc_start(rb_objspace_t *objspace, int reason) https://github.com/ruby/ruby/blob/trunk/gc.c#L8088
 
     /* reason may be clobbered, later, so keep set immediate_sweep here */
     objspace->flags.immediate_sweep = !!((unsigned)reason & GPR_FLAG_IMMEDIATE_SWEEP);
-    objspace->flags.during_compacting = !!((unsigned)reason & GPR_FLAG_COMPACT);
+
+    /* Explicitly enable compaction (GC.compact) */
+    objspace->flags.during_compacting = (!!((unsigned)reason & GPR_FLAG_COMPACT) << 1);
 
     if (!heap_allocated_pages) return FALSE; /* heap is not ready */
     if (!(reason & GPR_FLAG_METHOD) && !ready_to_gc(objspace)) return TRUE; /* GC is not allowed */
@@ -9248,10 +9281,26 @@ heap_check_moved_i(void *vstart, void *vend, size_t stride, void *data) https://github.com/ruby/ruby/blob/trunk/gc.c#L9281
 }
 
 static VALUE
+gc_compact(rb_execution_context_t *ec, VALUE self)
+{
+    /* Clear the heap. */
+    gc_start_internal(ec, self, Qtrue, Qtrue, Qtrue, Qfalse);
+
+    /* At this point, all references are live and the mutator is not allowed
+     * to run, so we don't need a read barrier. */
+    gc_start_internal(ec, self, Qtrue, Qtrue, Qtrue, Qtrue);
+
+    return gc_compact_stats(ec, self);
+}
+
+static VALUE
 gc_verify_compaction_references(rb_execution_context_t *ec, VALUE self, VALUE double_heap, VALUE toward_empty)
 {
     rb_objspace_t *objspace = &rb_objspace;
 
+    /* Clear the heap. */
+    gc_start_internal(ec, self, Qtrue, Qtrue, Qtrue, Qfalse);
+
     RB_VM_LOCK_ENTER();
     {
         gc_rest(objspace);
@@ -9865,6 +9914,16 @@ gc_disable(rb_execution_context_t *ec, VALUE _) https://github.com/ruby/ruby/blob/trunk/gc.c#L9914
 static VALUE
 gc_set_auto_compact(rb_execution_context_t *ec, VALUE _, VALUE v)
 {
+#if defined(HAVE_SYSCONF) && defined(_SC_PAGE_SIZE)
+    /* If Ruby's heap pages are not a multiple of the system page size, we
+     * cannot use mprotect for the read barrier, so we must disable automatic
+     * compaction. */
+    int pagesize;
+    pagesize = (int)sysconf(_SC_PAGE_SIZE);
+    if ((HEAP_PAGE_SIZE % pagesize) != 0) {
+        rb_raise(rb_eNotImpError, "Automatic compaction isn't available on this platform");
+    }
+#endif
     ruby_enable_autocompact = RTEST(v);
     return v;
 }
diff --git a/gc.rb b/gc.rb
index d2b0d8e..4e0faaf 100644
--- a/gc.rb
+++ b/gc.rb
@@ -199,8 +199,7 @@ module GC https://github.com/ruby/ruby/blob/trunk/gc.rb#L199
   end
 
   def self.compact
-    Primitive.gc_start_internal true, true, true, true
-    Primitive.gc_compact_stats
+    Primitive.gc_compact
   end
 
   # call-seq:
diff --git a/test/ruby/test_gc_compact.rb b/test/ruby/test_gc_compact.rb
index 3aad9e6..4a8cff3 100644
--- a/test/ruby/test_gc_compact.rb
+++ b/test/ruby/test_gc_compact.rb
@@ -1,56 +1,82 @@ https://github.com/ruby/ruby/blob/trunk/test/ruby/test_gc_compact.rb#L1
 # frozen_string_literal: true
 require 'test/unit'
 require 'fiddle'
+require 'etc'
 
 class TestGCCompact < Test::Unit::TestCase
-  def test_enable_autocompact
-    before = GC.auto_compact
-    GC.auto_compact = true
-    assert GC.auto_compact
-  ensure
-    GC.auto_compact = before
-  end
+  class AutoCompact < Test::Unit::TestCase
+    def setup
+      skip "autocompact not supported on this platform" unless supports_auto_compact?
+      super
+    end
 
-  def test_disable_autocompact
-    before = GC.auto_compact
-    GC.auto_compact = false
-    refute GC.auto_compact
-  ensure
-    GC.auto_compact = before
-  end
+    def test_enable_autocompact
+      before = GC.auto_compact
+      GC.auto_compact = true
+      assert GC.auto_compact
+    ensure
+      GC.auto_compact = before
+    end
 
-  def test_major_compacts
-    before = GC.auto_compact
-    GC.auto_compact = true
-    compact = GC.stat :compact_count
-    GC.start
-    assert_operator GC.stat(:compact_count), :>, compact
-  ensure
-    GC.auto_compact = before
-  end
+    def test_disable_autocompact
+      before = GC.auto_compact
+      GC.auto_compact = false
+      refute GC.auto_compact
+    ensure
+      GC.auto_compact = before
+    end
 
-  def test_implicit_compaction_does_something
-    before = GC.auto_compact
-    list = []
-    list2 = []
+    def test_major_compacts
+      before = GC.auto_compact
+      GC.auto_compact = true
+      compact = GC.stat :compact_count
+      GC.start
+      assert_operator GC.stat(:compact_count), :>, compact
+    ensure
+      GC.auto_compact = before
+    end
 
-    # Try to make some fragmentation
-    500.times {
-      list << Object.new
-      Object.new
-      Object.new
-    }
-    count = GC.stat :compact_count
-    GC.auto_compact = true
-    loop do
-      break if count < GC.stat(:compact_count)
-      list2 << Object.new
+    def test_implicit_compaction_does_something
+      before = GC.auto_compact
+      list = []
+      list2 = []
+
+      # Try to make some fragmentation
+      500.times {
+        list << Object.new
+        Object.new
+        Object.new
+      }
+      count = GC.stat :compact_count
+      GC.auto_compact = true
+      loop do
+        break if count < GC.stat(:compact_count)
+        list2 << Object.new
+      end
+      compact_stats = GC.latest_compact_info
+      refute_predicate compact_stats[:considered], :empty?
+      refute_predicate compact_stats[:moved], :empty?
+    ensure
+      GC.auto_compact = before
     end
-    compact_stats = GC.latest_compact_info
-    refute_predicate co (... truncated)

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/