Test that a coerced TYPE_CONST_STRING argument stays in place

A TYPE_CONST_STRING argument that comes from a to_str object is coerced
inside Fiddle, so the caller's argv holds that object and not the String
that the char * points into. Only converted_args holds the coerced
String, and the conservative scan of the ALLOCV buffer is what keeps it
in place while ffi_call runs without the GVL.

Nothing tested that. Before #205, converted_args was a Ruby Array, whose
elements the GC may move, and the C function read the slot the String
left. #205 replaced the Array with this buffer and corrected the fault as
a side effect. A later change could move these values back to storage the
GC may relocate, and no test would notice.

Add the missing test, and record the requirement next to the buffer.

Author: jeremy

ext/fiddle/function.c

@@ -304,7 +304,18 @@ function_call(int argc, VALUE argv[], VALUE self)
                           (func->is_variadic ? sizeof(int) * n_call_args : 0));
     args.values = (void **)((char *)generic_args +
                             sizeof(fiddle_generic) * n_call_args);
-    /* GC-scanned (conservatively) as part of the ALLOCV buffer */
+    /* GC-scanned (conservatively) as part of the ALLOCV buffer.
+     *
+     * The conservative scan both keeps these values alive and holds them in
+     * place, and both properties are required. generic_args can hold a pointer
+     * into one of them: TYPE_CONST_STRING stores rb_string_value_cstr(&src),
+     * and for a to_str object that is the coerced String, which argv does not
+     * hold. ffi_call then runs without the GVL, where another thread can compact
+     * the heap.
+     *
+     * Storing these values somewhere the GC may move them instead, such as a
+     * Ruby Array, keeps them alive but not in place. The C function then reads
+     * the slot the String left. Keep them in this buffer. */
     converted_args = (VALUE *)((char *)args.values +
                                sizeof(void *) * (n_call_args + 1));
     MEMZERO(converted_args, VALUE, n_call_args);

test/fiddle/test_function.rb

@@ -179,6 +179,86 @@ def test_strcpy
       assert_equal("123", str.to_s)
     end
 
+    # A TYPE_CONST_STRING argument that comes from a to_str object is coerced inside
+    # Fiddle. The caller's argv then holds that object, not the String that the
+    # char * points into, so argv does not keep the String in place. Fiddle keeps the
+    # coerced String in converted_args instead, and the conservative scan of the
+    # ALLOCV buffer pins it there.
+    #
+    # That pin is what this test protects. A converted_args that the garbage
+    # collector can move, such as a Ruby Array, lets compaction move the String after
+    # the char * is stored. The C function then reads the slot the String left.
+    #
+    # The second argument must be neither a String nor a Pointer. Only then does
+    # Fiddle call Pointer.[] on it, and that allocation is what lets the garbage
+    # collector run while the char * is already in place.
+    def test_call_const_string_from_to_str_after_compaction
+      omit("Need CRuby") unless RUBY_ENGINE == "ruby"
+      omit("Need GC.auto_compact=") unless GC.respond_to?(:auto_compact=)
+
+      length = 100
+      subject_class = Class.new do
+        define_method(:to_str) { "Q" * length }
+      end
+      # 4096 bytes keep these bytes in a malloc'd buffer, which compaction does not
+      # move. A short String would hold its bytes in the object itself, and then this
+      # pointer could dangle and report a failure that this test is not about.
+      reject = "Z" + ("\0" * 4095)
+      # to_ptr must build a new Pointer on every call. A Pointer that already exists
+      # gives Fiddle nothing to allocate, the garbage collector then does not run
+      # while the char * is in place, and this test cannot fail.
+      opener_class = Class.new do
+        define_method(:to_ptr) { Pointer[reject] }
+      end
+
+      strcspn = Function.new(@libc["strcspn"],
+                             [TYPE_CONST_STRING, TYPE_VOIDP],
+                             TYPE_SIZE_T)
+      # The subject holds no "Z", so strcspn stops at the terminator and reports the
+      # full length. A stale char * reports 0, because the slot the String left holds
+      # zero bytes, and churn content also reports 0.
+
+      churn = []
+      results = []
+      auto_compact = GC.auto_compact
+      begin
+        10.times do
+          # Churn of the same byte size as the subject, with half of it dropped, so
+          # that the subject's size pool holds sparsely occupied pages. The compactor
+          # only evacuates such pages. Without this the subject never moves, and then
+          # this test cannot fail.
+          churn.clear
+          2000.times { churn << ("Z" * length) }
+          churn.each_index { |i| churn[i] = nil if i.even? }
+          GC.start
+
+          # A short burst of same size allocations that this loop drops at once.
+          # It leaves a partly filled page in the subject's size pool. The
+          # compactor only moves objects out of pages that are not full, so
+          # without this burst the subject can land in a full page. It then never
+          # moves, and this test cannot fail.
+          500.times { "y" * length }
+
+          GC.auto_compact = true
+          GC.stress = true
+          begin
+            results << strcspn.call(subject_class.new, opener_class.new)
+          ensure
+            GC.stress = false
+            GC.auto_compact = auto_compact
+          end
+        end
+      ensure
+        GC.stress = false
+        GC.auto_compact = auto_compact
+      end
+      assert_equal([length] * 10, results)
+      # The intact case, checked after the loop: a call before it would prepare the
+      # CIF, and that preparation is part of what lets the collector run inside the
+      # window on the first measured call.
+      assert_equal(length, strcspn.call("Q" * length, opener_class.new))
+    end
+
     def call_proc(string_to_copy)
       buff = +"000"
       str = yield(buff, string_to_copy)