From b2170647583461f42dc2d9f201211fcafda2429f Mon Sep 17 00:00:00 2001
From: Anthony Green <green@moxielogic.com>
Date: Mon, 27 Jul 2026 20:08:15 -0400
Subject: [PATCH] powerpc: fix Darwin closure returns broken by #951

PR #951 (840add3b) changed the shared PowerPC closure helper,
ffi_closure_helper_common, to return a small PPC_LD_* jump-table index
instead of the ffi_type*, and rewrote aix_closure.S to consume it -- but
left darwin_closure.S expecting the old ffi_type* and dereferencing it.
With the helper now returning a small integer, ffi_closure_ASM
dereferenced e.g. 0 (PPC_LD_NONE, a void return) as a pointer, faulting
on a load from address 0.  This crashed essentially every closure call
-- including every gobject-introspection signal handler -- on 32- and
64-bit PowerPC Darwin (SIGBUS at ffi_closure_ASM, dar=0; issue #1002).

Convert darwin_closure.S to the PPC_LD_* convention, mirroring
aix_closure.S: drop the ffi_type* dereference, use the returned index
directly, and reorder the return-value jump table into PPC_LD_* order
(NONE, R3, R3R4, F32, F64, F128, U8, S8, U16, S16, and on ppc64 U32, S32).

Darwin, unlike AIX, returns small structs by value in registers, which
the existing assembly handles (Lsmallstruct/Lfour/Lstructend).  The
helper's return code is a single small integer with no room for
cif->rtype, which that assembly needs, so for a by-value struct return
the helper now stashes cif->rtype in the first parameter-save slot (dead
by return time) and returns a new PPC_LD_STRUCT code; the PPC_LD_STRUCT
fragment recovers it and drives the unchanged struct machinery.  By-
reference struct returns still return PPC_LD_NONE.

Based on the approach in a patch by Sergey Fedorov (@barracuda156); the
jump table here is reordered to the PPC_LD_* layout so that float,
double, long double, sub-word and 64-bit returns also dispatch correctly.

I have no PowerPC Darwin hardware; the jump-table fragment offsets were
checked by assembling for powerpc and powerpc64, but runtime
confirmation on 10.5/10.6 is still needed.

Fixes #1002.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/powerpc/darwin_closure.S | 131 +++++++++++++++--------------------
 src/powerpc/ffi_darwin.c     |  23 ++++++
 2 files changed, 79 insertions(+), 75 deletions(-)

diff --git a/src/powerpc/darwin_closure.S b/src/powerpc/darwin_closure.S
index 3121e6ac2..08cbe4bc1 100644
--- src/powerpc/darwin_closure.S
+++ src/powerpc/darwin_closure.S
@@ -186,19 +186,17 @@ LCFI1:
 	/* Make the call.  */
 	bl	BLCLS_HELP
 
-	/* r3 contains the rtype pointer... save it since we will need
-	   it later.  */
-	sg	r3,LINKAGE_SIZE(r1)	; ffi_type * result_type
-	lg	r0,0(r3)		; size => r0
-	lhz	r3,FFI_TYPE_TYPE(r3)	; type => r3
-
-	/* The helper will have intercepted structure returns and inserted
-	   the caller`s destination address for structs returned by ref.  */
-
-	/* r3 contains the return type  so use it to look up in a table
-	   so we know how to deal with each type.  */
-
-	addi	r5,r1,(SAVE_SIZE-RESULT_BYTES) /* Otherwise, our return is here.  */
+	/* r3 now holds a small PPC_LD_* jump-table index (see the PPC_LD_*
+	   defines in ffi_darwin.c), not an ffi_type* as this file previously
+	   assumed: ffi_closure_helper_common cannot return both an ffi_type*
+	   and the dispatch index through r3, so it returns the index.  The
+	   helper has already intercepted by-reference struct returns (writing
+	   the result to the caller`s buffer and returning PPC_LD_NONE); for a
+	   by-value struct return it returns PPC_LD_STRUCT and stashes cif->rtype
+	   in the first parameter-save slot, which the PPC_LD_STRUCT fragment
+	   below recovers.  */
+
+	addi	r5,r1,(SAVE_SIZE-RESULT_BYTES) /* Our return value is here.  */
 	bl	Lget_ret_type0_addr	/* Get pointer to Lret_type0 into LR.  */
 	mflr	r4			/* Move to r4.  */
 	slwi	r3,r3,4			/* Now multiply return type by 16.  */
@@ -218,43 +216,60 @@ LFE1:
 Lget_ret_type0_addr:
 	blrl
 
-/* case FFI_TYPE_VOID  */
+/* The fragments below are indexed by the PPC_LD_* return code that
+   ffi_closure_helper_common handed back in r3, so their order must match the
+   PPC_LD_* values in ffi_darwin.c.  Each is exactly 16 bytes (four
+   instructions), except the final PPC_LD_STRUCT fragment.  */
+
+/* case PPC_LD_NONE (void, or a struct returned by reference)  */
 Lret_type0:
 	b	Lfinish
 	nop
 	nop
 	nop
 
-/* case FFI_TYPE_INT  */
+/* case PPC_LD_R3 (one GPR: int, pointer, and on ppc64 also 64-bit ints)  */
 Lret_type1:
 	lg	r3,0(r5)
 	b	Lfinish
 	nop
 	nop
 
-/* case FFI_TYPE_FLOAT  */
+/* case PPC_LD_R3R4 (two GPRs: the 32-bit ABI`s 64-bit integer)  */
 Lret_type2:
+#if defined(__ppc64__)
+	lg	r3,0(r5)
+	lg	r4,8(r5)
+#else
+	lwz	r3,0(r5)
+	lwz	r4,4(r5)
+#endif
+	b	Lfinish
+	nop
+
+/* case PPC_LD_F32  */
+Lret_type3:
 	lfs	f1,0(r5)
 	b	Lfinish
 	nop
 	nop
 
-/* case FFI_TYPE_DOUBLE  */
-Lret_type3:
+/* case PPC_LD_F64  */
+Lret_type4:
 	lfd	f1,0(r5)
 	b	Lfinish
 	nop
 	nop
 
-/* case FFI_TYPE_LONGDOUBLE  */
-Lret_type4:
+/* case PPC_LD_F128 (128-bit long double: two doubles)  */
+Lret_type5:
 	lfd	f1,0(r5)
 	lfd	f2,8(r5)
 	b	Lfinish
 	nop
 
-/* case FFI_TYPE_UINT8  */
-Lret_type5:
+/* case PPC_LD_U8  */
+Lret_type6:
 #if defined(__ppc64__)
 	lbz	r3,7(r5)
 #else
@@ -264,8 +279,8 @@ Lret_type5:
 	nop
 	nop
 
-/* case FFI_TYPE_SINT8  */
-Lret_type6:
+/* case PPC_LD_S8  */
+Lret_type7:
 #if defined(__ppc64__)
 	lbz	r3,7(r5)
 #else
@@ -275,8 +290,8 @@ Lret_type6:
 	b	Lfinish
 	nop
 
-/* case FFI_TYPE_UINT16  */
-Lret_type7:
+/* case PPC_LD_U16  */
+Lret_type8:
 #if defined(__ppc64__)
 	lhz	r3,6(r5)
 #else
@@ -286,8 +301,8 @@ Lret_type7:
 	nop
 	nop
 
-/* case FFI_TYPE_SINT16  */
-Lret_type8:
+/* case PPC_LD_S16  */
+Lret_type9:
 #if defined(__ppc64__)
 	lha	r3,6(r5)
 #else
@@ -297,77 +312,43 @@ Lret_type8:
 	nop
 	nop
 
-/* case FFI_TYPE_UINT32  */
-Lret_type9:
 #if defined(__ppc64__)
-	lwz	r3,4(r5)
-#else
-	lwz	r3,0(r5)
-#endif
-	b	Lfinish
-	nop
-	nop
-
-/* case FFI_TYPE_SINT32  */
+/* case PPC_LD_U32 (ppc64 only; the 32-bit ABI aliases U32 to PPC_LD_R3)  */
 Lret_type10:
-#if defined(__ppc64__)
 	lwz	r3,4(r5)
-#else
-	lwz	r3,0(r5)
-#endif
 	b	Lfinish
 	nop
 	nop
 
-/* case FFI_TYPE_UINT64  */
+/* case PPC_LD_S32 (ppc64 only; the 32-bit ABI aliases S32 to PPC_LD_R3)  */
 Lret_type11:
-#if defined(__ppc64__)
-	lg	r3,0(r5)
-	b	Lfinish
-	nop
-#else
-	lwz	r3,0(r5)
-	lwz	r4,4(r5)
+	lwa	r3,4(r5)
 	b	Lfinish
-#endif
 	nop
-
-/* case FFI_TYPE_SINT64  */
-Lret_type12:
-#if defined(__ppc64__)
-	lg	r3,0(r5)
-	b	Lfinish
 	nop
-#else
-	lwz	r3,0(r5)
-	lwz	r4,4(r5)
-	b	Lfinish
 #endif
-	nop
 
-/* case FFI_TYPE_STRUCT  */
-Lret_type13:
+/* case PPC_LD_STRUCT (a by-value struct return).  This is the final,
+   variable-length fragment, so it need not be padded to 16 bytes.  The helper
+   stashed cif->rtype in the first parameter-save slot (see ffi_darwin.c),
+   because the small dispatch index in r3 left no room for it.  */
+Lret_type_struct:
+	lg	r6,PARENT_PARM_BASE(r1)	; cif->rtype
+	sg	r6,LINKAGE_SIZE(r1)	; where the struct code below expects it
+	lg	r0,0(r6)		; size => r0
 #if defined(__ppc64__)
 	lg	r3,0(r5)		; we need at least this...
 	cmpi	0,r0,4
 	bgt	Lstructend		; not a special small case
 	b	Lsmallstruct		; see if we need more.
 #else
-	cmpwi	0,r0,4
-	bgt	Lfinish		; not by value
-	lg	r3,0(r5)
+	lg	r3,0(r5)		; a <=4-byte struct, returned in r3
 	b	Lfinish
 #endif
-/* case FFI_TYPE_POINTER  */
-Lret_type14:
-	lg	r3,0(r5)
-	b	Lfinish
-	nop
-	nop
 
 #if defined(__ppc64__)
 Lsmallstruct:
-	beq	Lfour			; continuation of Lret13.
+	beq	Lfour			; continuation of Lret_type_struct.
 	cmpi	0,r0,3
 	beq	Lfinish			; don`t adjust this - can`t be any floats here...
 	srdi	r3,r3,48
diff --git a/src/powerpc/ffi_darwin.c b/src/powerpc/ffi_darwin.c
index 01e2a4370..64449c38e 100644
--- src/powerpc/ffi_darwin.c
+++ src/powerpc/ffi_darwin.c
@@ -60,11 +60,13 @@ struct ffi_aix_trampoline_struct {
 # define PPC_LD_S32		PPC_LD_R3
 # define PPC_LD_PTR		PPC_LD_R3
 # define PPC_LD_I64		PPC_LD_R3R4
+# define PPC_LD_STRUCT		10
 #else
 # define PPC_LD_U32		10
 # define PPC_LD_S32		11
 # define PPC_LD_PTR		PPC_LD_R3
 # define PPC_LD_I64		PPC_LD_R3
+# define PPC_LD_STRUCT		12
 #endif
 
 extern void ffi_closure_ASM (void);
@@ -1260,6 +1262,13 @@ ffi_closure_helper_common (ffi_cif* cif,
   long             i, avn;
   ffi_dblfl *      end_pfr = pfr + NUM_FPR_ARG_REGISTERS;
   unsigned         size_al;
+  int              struct_ret_by_value = 0;
+  /* When a struct is returned by value, ffi_closure_ASM's jump-table
+     dispatch carries only a small integer return code (see PPC_LD_* above),
+     with no room for cif->rtype.  We hand cif->rtype back in the first
+     parameter-save slot -- which is dead by the time we return -- for the
+     PPC_LD_STRUCT fragment in darwin_closure.S to recover.  */
+  unsigned long *  pgr0 = pgr;
 #if defined(POWERPC_DARWIN64)
   unsigned 	   fpsused = 0;
 #endif
@@ -1275,12 +1284,16 @@ ffi_closure_helper_common (ffi_cif* cif,
 	  rvalue = (void *) *pgr;
 	  pgr++;
 	}
+      else
+	struct_ret_by_value = 1;
 #elif defined(DARWIN_PPC)
       if (cif->rtype->size > 4)
 	{
 	  rvalue = (void *) *pgr;
 	  pgr++;
 	}
+      else
+	struct_ret_by_value = 1;
 #else /* assume we return by ref.  */
       rvalue = (void *) *pgr;
       pgr++;
@@ -1480,7 +1493,17 @@ ffi_closure_helper_common (ffi_cif* cif,
   switch (cif->rtype->type)
     {
     case FFI_TYPE_VOID:
+      return PPC_LD_NONE;
     case FFI_TYPE_STRUCT:
+      /* A by-reference struct return needs nothing further here: the result
+	 was written straight to the caller's buffer.  A by-value struct
+	 return is loaded into registers by darwin_closure.S, which needs
+	 cif->rtype -- hand it back in the first parameter-save slot.  */
+      if (struct_ret_by_value)
+	{
+	  *pgr0 = (unsigned long) cif->rtype;
+	  return PPC_LD_STRUCT;
+	}
       return PPC_LD_NONE;
     case FFI_TYPE_FLOAT:
       return PPC_LD_F32;
