From bdad4987ae7c0c6976511fed8c4e6fe7443d6594 Mon Sep 17 00:00:00 2001 From: rfm Date: Fri, 15 Nov 2024 15:43:47 +0000 Subject: [PATCH] Fix leaked FFI structure argument/returntype information --- Source/GSFFIInvocation.m | 3 +- Source/GSInvocation.h | 11 +++-- Source/GSPrivate.h | 12 +++-- Source/NSInvocation.m | 4 +- Source/cifframe.h | 8 ++-- Source/cifframe.m | 100 ++++++++++++++++++++++----------------- 6 files changed, 79 insertions(+), 59 deletions(-) diff --git a/Source/GSFFIInvocation.m b/Source/GSFFIInvocation.m index a861754a4..327c344e9 100644 --- a/Source/GSFFIInvocation.m +++ b/Source/GSFFIInvocation.m @@ -295,8 +295,7 @@ exitedThread(void *slot) _sig = RETAIN(aSignature); _numArgs = [aSignature numberOfArguments]; _info = [aSignature methodInfo]; - _frame = cifframe_from_signature(_sig); - [_frame retain]; + [self setupFrameFFI: _sig]; _cframe = [_frame mutableBytes]; /* Make sure we have somewhere to store the return value if needed. diff --git a/Source/GSInvocation.h b/Source/GSInvocation.h index 6c1eef180..60a6934db 100644 --- a/Source/GSInvocation.h +++ b/Source/GSInvocation.h @@ -27,6 +27,7 @@ #import "Foundation/NSInvocation.h" @class NSMutableData; +@class NSPointerArray; typedef struct { int offset; @@ -39,13 +40,17 @@ typedef struct { } NSArgumentInfo; -@interface GSFFIInvocation : NSInvocation +@interface GSFFIInvocation : NSInvocation { @public - uint8_t _retbuf[32]; // Store return values of up to 32 bytes here. - NSMutableData *_frame; + uint8_t _retbuf[32]; // Return values of up to 32 bytes here. + NSMutableData *_frame; // Frame information for invoking. + NSPointerArray *_extra; // Extra FFI data to be released. } @end +@interface GSFFIInvocation (FFI) +- (void) setupFrameFFI: (NSMethodSignature*)sig; +@end @interface GSFFCallInvocation : NSInvocation { diff --git a/Source/GSPrivate.h b/Source/GSPrivate.h index 5ce4fd8a1..5adc3a3b9 100644 --- a/Source/GSPrivate.h +++ b/Source/GSPrivate.h @@ -32,6 +32,7 @@ @class _GSMutableInsensitiveDictionary; @class NSNotification; +@class NSPointerArray; @class NSRecursiveLock; #if ( (__GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 3) ) && HAVE_VISIBILITY_ATTRIBUTE ) @@ -555,17 +556,18 @@ GSPrivateUnloadModule(FILE *errorStream, */ @interface GSCodeBuffer : NSObject { - unsigned size; - void *buffer; - void *executable; - id frame; + unsigned size; + void *buffer; + void *executable; + id frame; + NSPointerArray *extra; } + (GSCodeBuffer*) memoryWithSize: (NSUInteger)_size; - (void*) buffer; - (void*) executable; - (id) initWithSize: (NSUInteger)_size; - (void) protect; -- (void) setFrame: (id)aFrame; +- (void) setFrame: (id)aFrame extra: (NSPointerArray*)pa; @end /* For tuning socket connections diff --git a/Source/NSInvocation.m b/Source/NSInvocation.m index a5af4863c..7c05f3dec 100644 --- a/Source/NSInvocation.m +++ b/Source/NSInvocation.m @@ -70,6 +70,7 @@ - (void) dealloc { DESTROY(frame); + DESTROY(extra); if (size > 0) { #if defined(HAVE_FFI_PREP_CLOSURE_LOC) @@ -173,9 +174,10 @@ #endif } -- (void) setFrame: (id)aFrame +- (void) setFrame: (id)aFrame extra: (NSPointerArray*)pa { ASSIGN(frame, aFrame); + ASSIGN(extra, pa); } @end diff --git a/Source/cifframe.h b/Source/cifframe.h index c20e5155a..1316cff2d 100644 --- a/Source/cifframe.h +++ b/Source/cifframe.h @@ -52,16 +52,14 @@ typedef struct _cifframe_t { @class NSMutableData; -extern NSMutableData *cifframe_from_signature (NSMethodSignature *info); - -extern GSCodeBuffer* cifframe_closure (NSMethodSignature *sig, void (*func)()); +extern GSCodeBuffer* cifframe_closure(NSMethodSignature *sig, void (*func)()); extern void cifframe_set_arg(cifframe_t *cframe, int index, void *buffer, int size); extern void cifframe_get_arg(cifframe_t *cframe, int index, void *buffer, int size); extern void *cifframe_arg_addr(cifframe_t *cframe, int index); -extern BOOL cifframe_decode_arg (const char *type, void* buffer); -extern BOOL cifframe_encode_arg (const char *type, void* buffer); +extern BOOL cifframe_decode_arg(const char *type, void* buffer); +extern BOOL cifframe_encode_arg(const char *type, void* buffer); #endif diff --git a/Source/cifframe.m b/Source/cifframe.m index 928639ddc..2ae37d42f 100644 --- a/Source/cifframe.m +++ b/Source/cifframe.m @@ -39,6 +39,7 @@ #endif #include "cifframe.h" +#import "Foundation/NSPointerArray.h" #import "Foundation/NSException.h" #import "Foundation/NSData.h" #import "GSInvocation.h" @@ -91,7 +92,8 @@ #endif #endif -ffi_type *cifframe_type(const char *typePtr, const char **advance); +static ffi_type * +cifframe_type(const char *typePtr, const char **adv, NSPointerArray **extra); /* Best guess at the space needed for a structure, since we don't know for sure until it's calculated in ffi_prep_cif, which is too late */ @@ -122,30 +124,30 @@ cifframe_guess_struct_size(ffi_type *stype) return size; } - -NSMutableData * -cifframe_from_signature (NSMethodSignature *info) +static void +cifframe_from_signature(NSMethodSignature *info, + NSMutableData **frame, NSPointerArray **extra) { - unsigned size = sizeof(cifframe_t); - unsigned align = __alignof(double); - unsigned type_offset = 0; - unsigned offset = 0; - NSMutableData *result; - void *buf; - int i; - int numargs = [info numberOfArguments]; - ffi_type *rtype; - ffi_type *arg_types[numargs]; - cifframe_t *cframe; + unsigned size = sizeof(cifframe_t); + unsigned align = __alignof(double); + unsigned type_offset = 0; + unsigned offset = 0; + NSMutableData *result; + void *buf; + int i; + int numargs = [info numberOfArguments]; + ffi_type *rtype; + ffi_type *arg_types[numargs]; + cifframe_t *cframe; - /* FIXME: in cifframe_type, return values/arguments that are structures - have custom ffi_types with are allocated separately. We should allocate - them in our cifframe so we don't leak memory. Or maybe we could - cache structure types? */ - rtype = cifframe_type([info methodReturnType], NULL); + /* The extra parameter returns an array of any allocated memory which + * needs freeing at the end of the invocation. + */ + rtype = cifframe_type([info methodReturnType], NULL, extra); for (i = 0; i < numargs; i++) { - arg_types[i] = cifframe_type([info getArgumentTypeAtIndex: i], NULL); + arg_types[i] + = cifframe_type([info getArgumentTypeAtIndex: i], NULL, extra); } if (numargs > 0) @@ -220,9 +222,21 @@ cifframe_from_signature (NSMethodSignature *info) } } } - return result; + *frame = result; } +@implementation GSFFIInvocation (FFI) +- (void) setupFrameFFI: (NSMethodSignature*)info +{ + NSMutableData *f = nil; + NSPointerArray *e = nil; + + cifframe_from_signature(info, &f, &e); + ASSIGN(_frame, f); + ASSIGN(_extra, e); +} +@end + void cifframe_set_arg(cifframe_t *cframe, int index, void *buffer, int size) { @@ -250,8 +264,8 @@ cifframe_arg_addr(cifframe_t *cframe, int index) /* * Get the ffi_type for this type */ -ffi_type * -cifframe_type(const char *typePtr, const char **advance) +static ffi_type * +cifframe_type(const char *typePtr, const char **advance, NSPointerArray **extra) { static ffi_type stypeNSPoint = { 0 }; static ffi_type stypeNSRange = { 0 }; @@ -309,7 +323,7 @@ cifframe_type(const char *typePtr, const char **advance) else { const char *adv; - cifframe_type(typePtr, &adv); + cifframe_type(typePtr, &adv, extra); typePtr = adv; } break; @@ -328,7 +342,7 @@ cifframe_type(const char *typePtr, const char **advance) { typePtr++; } - cifframe_type(typePtr, &adv); + cifframe_type(typePtr, &adv, extra); typePtr = adv; typePtr++; /* Skip end-of-array */ } @@ -433,8 +447,8 @@ cifframe_type(const char *typePtr, const char **advance) /* An NSRect is an NSPoint and an NSSize, but those * two structures are actually identical. */ - elems[0] = cifframe_type(@encode(NSSize), NULL); - elems[1] = cifframe_type(@encode(NSPoint), NULL); + elems[0] = cifframe_type(@encode(NSSize), NULL, extra); + elems[1] = cifframe_type(@encode(NSPoint), NULL, extra); elems[2] = 0; ftype->elements = elems; ftype->type = FFI_TYPE_STRUCT; @@ -471,7 +485,7 @@ cifframe_type(const char *typePtr, const char **advance) */ while (*typePtr != _C_STRUCT_E) { - local = cifframe_type(typePtr, &adv); + local = cifframe_type(typePtr, &adv, extra); typePtr = adv; NSCAssert(typePtr, @"End of signature while parsing"); ftype->elements[types++] = local; @@ -485,6 +499,13 @@ cifframe_type(const char *typePtr, const char **advance) } ftype->elements[types] = NULL; typePtr++; /* Skip end-of-struct */ + if (nil == *extra) + { + *extra = [NSPointerArray pointerArrayWithOptions: + NSPointerFunctionsOpaquePersonality + | NSPointerFunctionsMallocMemory]; + } + [*extra addPointer: ftype]; } break; @@ -508,19 +529,11 @@ cifframe_type(const char *typePtr, const char **advance) { ffi_type *local; int align = objc_alignof_type(typePtr); - local = cifframe_type(typePtr, &adv); + local = cifframe_type(typePtr, &adv, extra); typePtr = adv; NSCAssert(typePtr, @"End of signature while parsing"); if (align > max_align) { - if (ftype && ftype->type == FFI_TYPE_STRUCT - && ftype != &stypeNSPoint - && ftype != &stypeNSRange - && ftype != &stypeNSRect - && ftype != &stypeNSSize) - { - free(ftype); - } ftype = local; max_align = align; } @@ -557,21 +570,22 @@ cifframe_type(const char *typePtr, const char **advance) } GSCodeBuffer* -cifframe_closure (NSMethodSignature *sig, void (*cb)()) +cifframe_closure(NSMethodSignature *sig, void (*cb)()) { - NSMutableData *frame; + NSMutableData *frame = nil; + NSPointerArray *extra = nil; cifframe_t *cframe; ffi_closure *cclosure; void *executable; GSCodeBuffer *memory; - /* Construct the frame (stored in an NSMutableDate object) and sety it + /* Construct the frame (stored in an NSMutableDate object) and set it * in a new closure. */ - frame = cifframe_from_signature(sig); + cifframe_from_signature(sig, &frame, &extra); cframe = [frame mutableBytes]; memory = [GSCodeBuffer memoryWithSize: sizeof(ffi_closure)]; - [memory setFrame: frame]; + [memory setFrame: frame extra: extra]; cclosure = [memory buffer]; executable = [memory executable]; if (cframe == NULL || cclosure == NULL)