From b2170647583461f42dc2d9f201211fcafda2429f Mon Sep 17 00:00:00 2001 From: Anthony Green 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) --- 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;