Test that a coerced TYPE_CONST_STRING argument stays in place

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)