From bf58357e546cd46cf79cf16ba43b6b1f4b35a9b0 Mon Sep 17 00:00:00 2001 From: Mike Ash Date: Wed, 21 Feb 2018 12:17:28 -0500 Subject: [PATCH] [Reflection] Change ReadBytesFunction to return a pointer rather than copying data, and adjust its users accordingly. rdar://problem/37538580 --- include/swift/Remote/CMemoryReader.h | 26 ++++++++++++----- include/swift/Remote/InProcessMemoryReader.h | 6 ++-- include/swift/Remote/MemoryReader.h | 29 +++++++++++++++---- .../SwiftRemoteMirror/MemoryReaderInterface.h | 9 +++--- .../swift-reflection-dump.cpp | 11 ++++--- .../swift-reflection-test.c | 20 ++++++++++--- 6 files changed, 71 insertions(+), 30 deletions(-) diff --git a/include/swift/Remote/CMemoryReader.h b/include/swift/Remote/CMemoryReader.h index a9303e77324..2d318d631af 100644 --- a/include/swift/Remote/CMemoryReader.h +++ b/include/swift/Remote/CMemoryReader.h @@ -62,17 +62,27 @@ public: if (!length) return false; - auto buffer = std::unique_ptr(new uint8_t[length + 1]); - if (!readBytes(address, buffer.get(), length + 1)) + const void *ptr; + std::function freeFunc; + std::tie(ptr, freeFunc) = readBytes(address, length); + if (ptr != nullptr) { + dest = std::string(reinterpret_cast(ptr), length); + freeFunc(); + return true; + } else { return false; - - dest = std::string(reinterpret_cast(buffer.get())); - return true; + } } - bool readBytes(RemoteAddress address, uint8_t *dest, uint64_t size) override { - return Impl.readBytes(Impl.reader_context, - address.getAddressData(), dest, size) != 0; + std::tuple> + 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); }; + + return std::make_tuple(ptr, freeLambda); } }; diff --git a/include/swift/Remote/InProcessMemoryReader.h b/include/swift/Remote/InProcessMemoryReader.h index c1e01a396fb..b878bc4ca4a 100644 --- a/include/swift/Remote/InProcessMemoryReader.h +++ b/include/swift/Remote/InProcessMemoryReader.h @@ -42,9 +42,9 @@ class InProcessMemoryReader final : public MemoryReader { return true; } - bool readBytes(RemoteAddress address, uint8_t *dest, uint64_t size) override { - std::memcpy(dest, address.getLocalPointer(), (size_t) size); - return true; + std::tuple> + readBytes(RemoteAddress address, uint64_t size) override { + return std::make_tuple(address.getLocalPointer(), []{}); } }; diff --git a/include/swift/Remote/MemoryReader.h b/include/swift/Remote/MemoryReader.h index 53e48cafea6..f3da05fe7c0 100644 --- a/include/swift/Remote/MemoryReader.h +++ b/include/swift/Remote/MemoryReader.h @@ -20,7 +20,9 @@ #include "swift/Remote/RemoteAddress.h" +#include #include +#include namespace swift { namespace remote { @@ -40,12 +42,12 @@ public: /// Look up the given public symbol name in the remote process. virtual RemoteAddress getSymbolAddress(const std::string &name) = 0; - /// Attempts to read 'size' bytes from the given address in the - /// remote process. + /// Attempts to read 'size' bytes from the given address in the remote process. /// - /// Returns false if the operation failed. - virtual bool readBytes(RemoteAddress address, uint8_t *dest, - uint64_t size) = 0; + /// Returns a pointer to the requested data and a function that must be called to + /// free that data when done. The pointer will be NULL if the operation failed. + virtual std::tuple> + readBytes(RemoteAddress address, uint64_t size) = 0; /// Attempts to read a C string from the given address in the remote /// process. @@ -63,6 +65,23 @@ public: sizeof(IntegerType)); } + /// Attempts to read 'size' bytes from the given address in the + /// remote process. + /// + /// Returns false if the operation failed. + bool readBytes(RemoteAddress address, uint8_t *dest, uint64_t size) { + const void *ptr; + std::function freeFunc; + std::tie(ptr, freeFunc) = readBytes(address, size); + if (ptr != nullptr) { + memcpy(dest, ptr, size); + freeFunc(); + return true; + } else { + return false; + } + } + virtual ~MemoryReader() = default; }; diff --git a/include/swift/SwiftRemoteMirror/MemoryReaderInterface.h b/include/swift/SwiftRemoteMirror/MemoryReaderInterface.h index a190eef4637..08189e8a766 100644 --- a/include/swift/SwiftRemoteMirror/MemoryReaderInterface.h +++ b/include/swift/SwiftRemoteMirror/MemoryReaderInterface.h @@ -32,13 +32,14 @@ extern "C" { // in the system library, so we use 'swift_addr_t'. typedef uint64_t swift_addr_t; -typedef void (*FreeBytesFunction)(void *bytes, void *context); +typedef void (*FreeBytesFunction)(const void *bytes, void *context); typedef uint8_t (*PointerSizeFunction)(void *reader_context); typedef uint8_t (*SizeSizeFunction)(void *reader_context); -typedef void *(*ReadBytesFunction)(void *reader_context, swift_addr_t address, - uint64_t size, FreeBytesFunction *outFreeFunction, - void **outFreeContext); +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); typedef swift_addr_t (*GetSymbolAddressFunction)(void *reader_context, diff --git a/tools/swift-reflection-dump/swift-reflection-dump.cpp b/tools/swift-reflection-dump/swift-reflection-dump.cpp index 53e3e4aa9d5..06687ff4968 100644 --- a/tools/swift-reflection-dump/swift-reflection-dump.cpp +++ b/tools/swift-reflection-dump/swift-reflection-dump.cpp @@ -191,15 +191,14 @@ public: return false; } - bool readBytes(RemoteAddress address, uint8_t *dest, - uint64_t size) override { + std::tuple> + readBytes(RemoteAddress address, uint64_t size) override { if (!isAddressValid(address, size)) - return false; + return std::make_tuple(nullptr, []{}); // TODO: Account for offset in ELF binaries - auto src = (const void *)address.getAddressData(); - memcpy(dest, (const void*)src, size); - return true; + auto ptr = (const void *)address.getAddressData(); + return std::make_tuple(ptr, []{}); } bool readString(RemoteAddress address, std::string &dest) override { diff --git a/tools/swift-reflection-test/swift-reflection-test.c b/tools/swift-reflection-test/swift-reflection-test.c index 7880aacd838..549fcfcdd47 100644 --- a/tools/swift-reflection-test/swift-reflection-test.c +++ b/tools/swift-reflection-test/swift-reflection-test.c @@ -95,9 +95,15 @@ void PipeMemoryReader_collectBytesFromPipe(const PipeMemoryReader *Reader, } } +static void PipeMemoryReader_freeBytes(const void *bytes, void *context) { + free((void *)bytes); +} + static -int PipeMemoryReader_readBytes(void *Context, swift_addr_t Address, void *Dest, - uint64_t Size) { +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; size_t TargetSize = (size_t)Size; @@ -105,8 +111,14 @@ int PipeMemoryReader_readBytes(void *Context, swift_addr_t Address, void *Dest, write(WriteFD, REQUEST_READ_BYTES, 2); write(WriteFD, &TargetAddress, sizeof(TargetAddress)); write(WriteFD, &TargetSize, sizeof(size_t)); - PipeMemoryReader_collectBytesFromPipe(Reader, Dest, Size); - return 1; + + void *Buf = malloc(Size); + PipeMemoryReader_collectBytesFromPipe(Reader, Buf, Size); + + *outFreeFunction = PipeMemoryReader_freeBytes; + *outFreeContext = NULL; + + return Buf; } static