Test that a coerced TYPE_CONST_STRING argument stays in place (#213)

## Problem

`TYPE_CONST_STRING` stores a pointer into a Ruby String:

```c
case TYPE_CONST_STRING:
    ...
    else { dst->pointer = rb_string_value_cstr(src); }
```

A String argument is safe. `argv` holds it and keeps it in place.

A `to_str` object is not. `rb_string_value_cstr` writes the coerced
String back into `src`. `argv` still holds the original object, not the
String. The next loop turn overwrites `src`. After that, only
`converted_args` holds the String.

The pointer must stay valid until `ffi_call` returns. `ffi_call` runs
under `rb_thread_call_without_gvl`, so another thread can compact the
heap during the call. `converted_args` must keep the String **in
place**, not only alive.

Master does this. `converted_args` sits in the `ALLOCV` buffer. The GC
scans that buffer conservatively, and a conservative scan pins the
object.

## Gap

No test covers this.

Before #205, `converted_args` was a Ruby Array. The GC may move an
Array's elements, so the String moved while the pointer stayed, and the
C function read the vacated slot. #205 replaced the Array with the
buffer and fixed the fault as a side effect. Nothing records that the
buffer is required.

## Change

1. `test_call_const_string_from_to_str_after_compaction` in
`test/fiddle/test_function.rb`.
2. A comment at `converted_args` in `ext/fiddle/function.c` stating the
requirement.

The test calls `strcspn` as `[TYPE_CONST_STRING, TYPE_VOIDP] ->
TYPE_SIZE_T`. Arg 0 is a `to_str` object returning a fresh 100-byte
String. Arg 1 is a `to_ptr` object, which makes Fiddle call
`Pointer.[]`; that allocation lets the GC run after the pointer is
stored.

The subject holds no `"Z"`; the reject set is `"Z"`. A correct call
returns 100. A stale pointer returns 0, because a vacated slot holds
zero bytes.

Three details are load-bearing. Remove any one and the test passes on a
faulty build; each has a comment:

- Churn is the subject's byte size, half of it dropped. The compactor
only evacuates non-full pages.
- `to_ptr` returns a **new** `Pointer` each time. An existing `Pointer`
gives Fiddle nothing to allocate.
- The intact-case call runs **after** the loop. A call before it
prepares the CIF, and that preparation is part of what opens the window.

## Result

| tree | arm64-darwin, ruby 3.4.10 | aarch64-linux, ruby 3.4.7 |
|---|---|---|
| `d389bbf` (before #205) | fail | fail |
| master | pass | pass |

Three runs per tree on darwin, same result each time. Full suite on
master with this change: 242 tests, 668 assertions, 0 failures, 1 error,
3 omissions. The error is `test_nogvl_poll` needing `envutil`; it fails
the same way without this change.

Environment: ruby 3.4.10 [arm64-darwin23]; ruby 3.4.7 [aarch64-linux],
glibc 2.36, libffi 3.4.4.

## Note

Released versions still have the fault. I measured 1.1.6 and 1.1.8 on
both platforms: a two-argument call of this shape returns the wrong
result on every iteration under `GC.stress` with `GC.auto_compact`.
Master is correct. You may want a release.

I report this as latent. The trigger is the declared argument type plus
a `to_str` object in place of a String. In one application that loads
fiddle at boot, all 227 bundled gems put every `Fiddle::Function.new`
behind a Windows-only branch.

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,81 @@ 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
+      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 }
+
+        begin
+          GC.auto_compact = true
+          GC.stress = true
+          results << strcspn.call(subject_class.new, opener_class.new)
+        ensure
+          GC.stress = false
+          GC.auto_compact = auto_compact
+        end
+      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)