# A06 — FFI readString returns an un-pinned image-memory pointer

Bug ref      : always.md A.6 ; pharo.md §5.27
Severity     : HIGH (GC between strlen and strcpy in callers invalidates pointer)
File         : ffi/src/utils.c
Lines (HEAD) : 43-50 (`readString`)

## Problem

```c
char *readString(sqInt aString) {
    if(!isKindOfClass(aString, classString())){
        primitiveFail();
        return NULL;
    }
    return (char *)firstIndexableField(aString);
}
```

`readString` returns a raw pointer into Pharo object memory.
`firstIndexableField` resolves to interior bytes of the live string
object; the GC is free to compact that string at any safe-point.

Callers compose `readString` with `strlen` and `strcpy`. The
canonical pattern is in `callbackPrimitives.c:160-161`:

```c
callback->userData = malloc(strlen(readString(debugString)) + 1);
strcpy(callback->userData, readString(debugString));
```

The two `readString` calls return two pointers that may not refer to
the same memory (GC may have moved the string between them). The
`strlen`-derived size and the `strcpy`-resolved source can diverge
into a write that walks past the allocation.

## Fix

Have `readString` return a pinned, heap-owned copy. Update the one
caller chain that already manually does `malloc + strcpy` to consume
the new contract (the new helper takes ownership; the caller is
responsible for `free`-ing it).

```diff
diff --git a/ffi/src/callbacks/callbackPrimitives.c b/ffi/src/callbacks/callbackPrimitives.c
index 08f703ff3..d05f7c12d 100644
--- a/ffi/src/callbacks/callbackPrimitives.c
+++ b/ffi/src/callbacks/callbackPrimitives.c
@@ -157,8 +157,12 @@ PrimitiveWithDepth(primitiveRegisterCallback, 3){
     if(debugString == nilObject()){
     	callback->userData = NULL;
     }else{
-    	callback->userData = malloc(strlen(readString(debugString)) + 1);
-    	strcpy(callback->userData, readString(debugString));
+    	/* readString now returns an owned, NUL-terminated heap copy. */
+    	callback->userData = readString(debugString);
+    	if (callback->userData == NULL) {
+    		/* primitive error already set inside readString */
+    		return;
+    	}
     }
 
     setHandler(receiver, callback->functionAddress);
diff --git a/ffi/src/utils.c b/ffi/src/utils.c
index 647583485..cb3925db5 100644
--- a/ffi/src/utils.c
+++ b/ffi/src/utils.c
@@ -40,13 +40,32 @@ void* getAddressFromExternalAddressOrByteArray(sqInt anExternalAddressOrByteArra
 	return NULL;
 }
 
+/*
+ * Returns a heap-allocated, NUL-terminated copy of the image string.
+ * The caller owns the result and must free() it. Returns NULL on
+ * failure (in which case a primitive error has been set).
+ *
+ * Returning a copy rather than firstIndexableField avoids a GC race:
+ * a moving GC may relocate the string between the caller's strlen
+ * and a subsequent strcpy, leaving the length and the source out of
+ * agreement.
+ */
 char *readString(sqInt aString) {
     if(!isKindOfClass(aString, classString())){
         primitiveFail();
         return NULL;
     }
-    
-    return (char *)firstIndexableField(aString);
+
+    sqInt len = stSizeOf(aString);
+    char *src = (char *)firstIndexableField(aString);
+    char *copy = (char *)malloc((size_t)len + 1);
+    if (!copy) {
+        primitiveFailFor(PrimErrNoMemory);
+        return NULL;
+    }
+    memcpy(copy, src, (size_t)len);
+    copy[len] = '\0';
+    return copy;
 }
 
 // Extras
```



## Test plan

- Run the FFI callback regression tests under a stress-GC build of
  the VM (e.g. with `--gc-stress` if available, or by forcing many
  allocations during callback registration).
- Manually verify that `callback_release` already frees `userData`
  (callbacks.c:75-77 does); no double-free or leak.
- Verify there are no other in-tree callers of `readString` that
  assumed it returned an image-interior pointer:

      $ git grep -n 'readString(' ffi/src/

  Only the two call-sites in callbackPrimitives.c were found and
  both are updated by this patch.

## Risk notes

- API change: `readString` now returns an owned pointer instead of a
  borrowed one. All in-tree callers are updated. External plugins
  built against the public FFI header will need to free the result —
  worth a note in the next release.
- Adds one malloc/free per callback registration; negligible.
- Length-based memcpy + explicit NUL handles strings that contain
  embedded NULs the same way the old code did (i.e. it relied on
  `strlen` and would have stopped at the first NUL; the new code
  copies the full image length, then NUL-terminates).