[RemoteMirrors] Move FreeBytesFunction from an out-parameter on ReadBytesFunction to a function pointer passed in when creating a reflection context.

Author: mikeash

include/swift/Remote/CMemoryReader.h

@@ -76,13 +76,14 @@ class CMemoryReader final : public MemoryReader {
 
   std::tuple<const void *, std::function<void()>>
     readBytes(RemoteAddress address, uint64_t size) override {
-      FreeBytesFunction freeFunc;
-      void *freeContext;
-      auto ptr = Impl.readBytes(Impl.reader_context, address.getAddressData(), size,
-                                &freeFunc, &freeContext);
-      auto freeLambda = [=]{ freeFunc(ptr, freeContext); };
+      void *FreeContext;
+      auto Ptr = Impl.readBytes(Impl.reader_context, address.getAddressData(), size,
+                                &FreeContext);
+      auto ReaderContext = Impl.reader_context;
+      auto Free = Impl.free;
+      auto freeLambda = [=]{ Free(ReaderContext, Ptr, FreeContext); };
 
-      return std::make_tuple(ptr, freeLambda);
+      return std::make_tuple(Ptr, freeLambda);
   }
 };
 

include/swift/SwiftRemoteMirror/MemoryReaderInterface.h

@@ -32,13 +32,12 @@ extern "C" {
 // in the system library, so we use 'swift_addr_t'.
 typedef uint64_t swift_addr_t;
 
-typedef void (*FreeBytesFunction)(const void *bytes, void *context);
+typedef void (*FreeBytesFunction)(void *reader_context, const void *bytes, void *context);
 
 typedef uint8_t (*PointerSizeFunction)(void *reader_context);
 typedef uint8_t (*SizeSizeFunction)(void *reader_context);
 typedef const void *(*ReadBytesFunction)(void *reader_context, swift_addr_t address,
                                          uint64_t size,
-                                         FreeBytesFunction *outFreeFunction,
                                          void **outFreeContext);
 typedef uint64_t (*GetStringLengthFunction)(void *reader_context,
                                             swift_addr_t address);
@@ -57,6 +56,9 @@ typedef struct MemoryReaderImpl {
   /// Get the size in bytes of the target's size type.
   SizeSizeFunction getSizeSize;
 
+  /// Free memory returned from readBytes.
+  FreeBytesFunction free;
+
   // FIXME: -Wdocumentation complains about \param and \returns on function pointers.
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdocumentation"
@@ -65,14 +67,11 @@ typedef struct MemoryReaderImpl {
   ///
   /// \param address the address in the target address space
   /// \param size the number of bytes to read
-  /// \param outFreeFunction on return, a function that the caller will invoke to free
-  ///                        the returned memory, or NULL if no action needs to be taken
-  ///                        to free the memory
   /// \param outFreeContext on return, an arbitrary context pointer that the caller will
   ///                       pass to the free function
   /// \returns A pointer to the requested memory, or NULL if the memory could not be read.
-  ///          If outFreeFunction is non-NULL, the caller must invoke it on the returned
-  ///          pointer once it's done using it.
+  ///          The caller must invoke the free function on the returned pointer once it's
+  ///          done using the memory.
   ReadBytesFunction readBytes;
 
   /// Get the string length at the given address.

include/swift/SwiftRemoteMirror/SwiftRemoteMirror.h

@@ -45,9 +45,10 @@ SwiftReflectionContextRef
 swift_reflection_createReflectionContext(
     void *ReaderContext,
     uint8_t PointerSize,
-    ReadBytesFunction readBytes,
-    GetStringLengthFunction getStringLength,
-    GetSymbolAddressFunction getSymbolAddress);
+    FreeBytesFunction Free,
+    ReadBytesFunction ReadBytes,
+    GetStringLengthFunction GetStringLength,
+    GetSymbolAddressFunction GetSymbolAddress);
 
 /// Destroys an opaque reflection context.
 void

include/swift/SwiftRemoteMirror/SwiftRemoteMirrorLegacyInterop.h

@@ -59,9 +59,10 @@ struct SwiftReflectionFunctions {
   SwiftReflectionContextRef (*createReflectionContext)(
     void *ReaderContext,
     uint8_t PointerSize,
-    ReadBytesFunction readBytes,
-    GetStringLengthFunction getStringLength,
-    GetSymbolAddressFunction getSymbolAddress);
+    FreeBytesFunction FreeBytes,
+    ReadBytesFunction ReadBytes,
+    GetStringLengthFunction GetStringLength,
+    GetSymbolAddressFunction GetSymbolAddress);
 
   SwiftReflectionContextRef (*createReflectionContextLegacy)(
     void *ReaderContext,
@@ -152,6 +153,7 @@ struct SwiftReflectionInteropContextLibrary {
 
 struct SwiftReflectionInteropContext {
   void *ReaderContext;
+  FreeBytesFunction FreeBytes;
   ReadBytesFunction ReadBytes;
   uint64_t (*GetStringLength)(void *reader_context,
                               swift_addr_t address);
@@ -251,15 +253,14 @@ static int swift_reflection_interop_readBytesAdapter(void *reader_context,
   SwiftReflectionInteropContextRef Context =
     (SwiftReflectionInteropContextRef)reader_context;
 
-  FreeBytesFunction FreeFunction;
   void *FreeContext;
   const void *ptr = Context->ReadBytes(Context->ReaderContext, address, size,
-                                       &FreeFunction, &FreeContext);
+                                       &FreeContext);
   if (ptr == NULL)
     return 0;
 
   memcpy(dest, ptr, size);
-  FreeFunction(ptr, FreeContext);
+  Context->FreeBytes(Context->ReaderContext, ptr, FreeContext);
   return 1;
 }
 
@@ -288,9 +289,10 @@ swift_reflection_interop_createReflectionContext(
     void *LibraryHandle,
     void *LegacyLibraryHandle,
     uint8_t PointerSize,
-    ReadBytesFunction readBytes,
-    GetStringLengthFunction getStringLength,
-    GetSymbolAddressFunction getSymbolAddress) {
+    FreeBytesFunction FreeBytes,
+    ReadBytesFunction ReadBytes,
+    GetStringLengthFunction GetStringLength,
+    GetSymbolAddressFunction GetSymbolAddress) {
 
   SwiftReflectionInteropContextRef ContextRef =
     (SwiftReflectionInteropContextRef)calloc(sizeof(*ContextRef), 1);
@@ -313,14 +315,15 @@ swift_reflection_interop_createReflectionContext(
         swift_reflection_interop_GetSymbolAddressAdapter);
     } else {
       Library->Context = Library->Functions.createReflectionContext(ReaderContext,
-      PointerSize, readBytes, getStringLength, getSymbolAddress);
+      PointerSize, FreeBytes, ReadBytes, GetStringLength, GetSymbolAddress);
     }
   }
 
   ContextRef->ReaderContext = ReaderContext;
-  ContextRef->ReadBytes = readBytes;
-  ContextRef->GetStringLength = getStringLength;
-  ContextRef->GetSymbolAddress = getSymbolAddress;
+  ContextRef->FreeBytes = FreeBytes;
+  ContextRef->ReadBytes = ReadBytes;
+  ContextRef->GetStringLength = GetStringLength;
+  ContextRef->GetSymbolAddress = GetSymbolAddress;
 
   return ContextRef;
 }
@@ -358,31 +361,28 @@ swift_reflection_interop_addImageLegacy(
   SwiftReflectionInteropContextRef ContextRef,
   struct SwiftReflectionInteropContextLibrary *Library,
   swift_addr_t imageStart) {
-  FreeBytesFunction FreeFunction;
   void *FreeContext;
   const void *Buf;
   Buf = ContextRef->ReadBytes(ContextRef->ReaderContext,
                               imageStart,
                               sizeof(MachHeader),
-                              &FreeFunction,
                               &FreeContext);
   if (Buf == NULL)
     return 0;
 
   MachHeader *Header = (MachHeader *)Buf;
 
   if (Header->magic != MH_MAGIC && Header->magic != MH_MAGIC_64) {
-    FreeFunction(Buf, ContextRef->ReaderContext);
+    ContextRef->FreeBytes(ContextRef->ReaderContext, Buf, FreeContext);
     return 0;
   }
 
   uint32_t Length = Header->sizeofcmds;
-  FreeFunction(Buf, ContextRef->ReaderContext);
+  ContextRef->FreeBytes(ContextRef->ReaderContext, Buf, FreeContext);
 
   Buf = ContextRef->ReadBytes(ContextRef->ReaderContext,
                               imageStart,
                               Length,
-                              &FreeFunction,
                               &FreeContext);
   Header = (MachHeader *)Buf;
   // TODO: free the stuff when we're done

stdlib/public/SwiftRemoteMirror/SwiftRemoteMirror.cpp

@@ -54,20 +54,22 @@ swift_reflection_getSupportedMetadataVersion() {
 SwiftReflectionContextRef
 swift_reflection_createReflectionContext(void *ReaderContext,
                                          uint8_t PointerSize,
-                                         ReadBytesFunction readBytes,
-                                         GetStringLengthFunction getStringLength,
-                                         GetSymbolAddressFunction getSymbolAddress) {
+                                         FreeBytesFunction Free,
+                                         ReadBytesFunction ReadBytes,
+                                         GetStringLengthFunction GetStringLength,
+                                         GetSymbolAddressFunction GetSymbolAddress) {
   assert((PointerSize == 4 || PointerSize == 8) && "We only support 32-bit and 64-bit.");
-  auto getSize = PointerSize == 4
+  auto GetSize = PointerSize == 4
     ? [](void *){ return (uint8_t)4; }
     : [](void *){ return (uint8_t)8; };
   MemoryReaderImpl ReaderImpl {
     ReaderContext,
-    getSize,
-    getSize,
-    readBytes,
-    getStringLength,
-    getSymbolAddress
+    GetSize,
+    GetSize,
+    Free,
+    ReadBytes,
+    GetStringLength,
+    GetSymbolAddress
   };
 
   return new SwiftReflectionContext(ReaderImpl);

tools/swift-reflection-test/swift-reflection-test.c

@@ -95,14 +95,14 @@ void PipeMemoryReader_collectBytesFromPipe(const PipeMemoryReader *Reader,
   }
 }
 
-static void PipeMemoryReader_freeBytes(const void *bytes, void *context) {
+static void PipeMemoryReader_freeBytes(void *reader_context, const void *bytes,
+                                       void *context) {
   free((void *)bytes);
 }
 
 static
 const void *PipeMemoryReader_readBytes(void *Context, swift_addr_t Address,
                                        uint64_t Size,
-                                       FreeBytesFunction *outFreeFunction,
                                        void **outFreeContext) {
   const PipeMemoryReader *Reader = (const PipeMemoryReader *)Context;
   uintptr_t TargetAddress = Address;
@@ -115,7 +115,6 @@ const void *PipeMemoryReader_readBytes(void *Context, swift_addr_t Address,
   void *Buf = malloc(Size);
   PipeMemoryReader_collectBytesFromPipe(Reader, Buf, Size);
 
-  *outFreeFunction = PipeMemoryReader_freeBytes;
   *outFreeContext = NULL;
 
   return Buf;
@@ -288,6 +287,7 @@ int doDumpHeapInstance(const char *BinaryFilename) {
       SwiftReflectionContextRef RC = swift_reflection_createReflectionContext(
         (void*)&Pipe,
         sizeof(void *),
+        PipeMemoryReader_freeBytes,
         PipeMemoryReader_readBytes,
         PipeMemoryReader_getStringLength,
         PipeMemoryReader_getSymbolAddress);

unittests/Reflection/RemoteMirrorInterop/test.m

@@ -14,13 +14,15 @@
   return Handle;
 }
 
-void NopFree(const void *bytes, void *context) {}
+void NopFree(void *reader_context, const void *bytes, void *context) {
+  assert(reader_context == (void *)0xdeadbeef);
+  assert(context == (void *)0xfeedface);
+}
 
 const void *ReadBytes(void *context, swift_addr_t address, uint64_t size,
-                      FreeBytesFunction *outFreeFunction, void **outFreeContext) {
+                      void **outFreeContext) {
   assert(context == (void *)0xdeadbeef);
-  *outFreeFunction = NopFree;
-  *outFreeContext = NULL;
+  *outFreeContext = (void *)0xfeedface;
   return (void *)address;
 }
 
@@ -59,6 +61,7 @@ int main(int argc, char **argv) {
       Mirror5Handle,
       Mirror4Handle,
       sizeof(void *),
+      NopFree,
       ReadBytes,
       GetStringLength,
       GetSymbolAddress);