MacOS managed debugging perf fix (#39804)
authorMike McLaughlin <mikem@microsoft.com>
Sat, 25 Jul 2020 04:01:54 +0000 (21:01 -0700)
committerGitHub <noreply@github.com>
Sat, 25 Jul 2020 04:01:54 +0000 (21:01 -0700)
* MacOS managed debugging perf fix

This PR adds PAL_OpenProcessMemory, PAL_ReadProcessMemory and PAL_CloseProcessMemory that
encapsulate the remote read memory functions for both MacOS and Linux.

The DBI code that reads memory using the runtime IPC messages to use these new functions
and fallback to the IPC message if the open fails.

On MacOS, this won't have any affect on VSCode performance until vsdbg/vsdbgui are signed
with the "com.apple.security.cs.debugger" entitlement.

Sample entitlements.plist file:

```
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
  <dict>
    <key>com.apple.security.cs.debugger</key>
      <true/>
  </dict>
</plist>
```

Example of locally signing with entitlement command:

codesign -s <local-cert> --entitlements entitlements.plist

Fix FreeBSD build. Remove FEATURE_DATATARGET4 for macOS so managed debugging uses faster OOP unwind for HelpMethodFrames

src/coreclr/clrdefinitions.cmake
src/coreclr/src/debug/createdump/main.cpp
src/coreclr/src/debug/di/shimremotedatatarget.cpp
src/coreclr/src/dlls/mscordac/mscordac_unixexports.src
src/coreclr/src/pal/inc/pal.h
src/coreclr/src/pal/src/debug/debug.cpp
src/coreclr/src/pal/src/map/map.cpp

index 7be0a6915051afaea58dbba556cc416cdd713d70..b717c10d840a40c07da63313b7e7fbc0f64180b1 100644 (file)
@@ -30,7 +30,6 @@ if (CLR_CMAKE_TARGET_UNIX)
 
   if(CLR_CMAKE_TARGET_OSX)
     add_definitions(-D_XOPEN_SOURCE)
-    add_definitions(-DFEATURE_DATATARGET4)
   endif(CLR_CMAKE_TARGET_OSX)
 
   if (CLR_CMAKE_TARGET_ARCH_AMD64)
@@ -173,9 +172,9 @@ set(FEATURE_READYTORUN 1)
 
 add_compile_definitions($<$<NOT:$<BOOL:$<TARGET_PROPERTY:CROSSGEN_COMPONENT>>>:FEATURE_REJIT>)
 
-if (CLR_CMAKE_HOST_UNIX AND CLR_CMAKE_TARGET_UNIX AND NOT CLR_CMAKE_TARGET_OSX)
+if (CLR_CMAKE_HOST_UNIX AND CLR_CMAKE_TARGET_UNIX)
   add_definitions(-DFEATURE_REMOTE_PROC_MEM)
-endif (CLR_CMAKE_HOST_UNIX AND CLR_CMAKE_TARGET_UNIX AND NOT CLR_CMAKE_TARGET_OSX)
+endif (CLR_CMAKE_HOST_UNIX AND CLR_CMAKE_TARGET_UNIX)
 
 if (CLR_CMAKE_TARGET_UNIX OR CLR_CMAKE_TARGET_ARCH_ARM64)
     add_definitions(-DFEATURE_STUBS_AS_IL)
index 626175c2903c09e150d4aa1aef20e0e0b89f82d4..66b1976119104bc01c400ed32beeb62484b8c727 100644 (file)
@@ -35,6 +35,7 @@ int __cdecl main(const int argc, const char* argv[])
                                                  MiniDumpWithFullMemoryInfo |
                                                  MiniDumpWithThreadInfo |
                                                  MiniDumpWithTokenInformation);
+    const char* dumpType = "minidump with heap";
     const char* dumpPathTemplate = nullptr;
     int exitCode = 0;
     int pid = 0;
@@ -60,11 +61,13 @@ int __cdecl main(const int argc, const char* argv[])
             }
             else if ((strcmp(*argv, "-n") == 0) || (strcmp(*argv, "--normal") == 0))
             {
+                dumpType = "minidump";
                 minidumpType = (MINIDUMP_TYPE)(MiniDumpNormal |
                                                MiniDumpWithThreadInfo);
             }
             else if ((strcmp(*argv, "-h") == 0) || (strcmp(*argv, "--withheap") == 0))
             {
+                dumpType = "minidump with heap";
                 minidumpType = (MINIDUMP_TYPE)(MiniDumpWithPrivateReadWriteMemory |
                                                MiniDumpWithDataSegs |
                                                MiniDumpWithHandleData |
@@ -75,11 +78,13 @@ int __cdecl main(const int argc, const char* argv[])
             }
             else if ((strcmp(*argv, "-t") == 0) || (strcmp(*argv, "--triage") == 0))
             {
+                dumpType = "triage minidump";
                 minidumpType = (MINIDUMP_TYPE)(MiniDumpFilterTriage |
                                                MiniDumpWithThreadInfo);
             }
             else if ((strcmp(*argv, "-u") == 0) || (strcmp(*argv, "--full") == 0))
             {
+                dumpType = "full dump";
                 minidumpType = (MINIDUMP_TYPE)(MiniDumpWithFullMemory |
                                                MiniDumpWithDataSegs |
                                                MiniDumpWithHandleData |
@@ -122,24 +127,6 @@ int __cdecl main(const int argc, const char* argv[])
 
         snprintf(dumpPath, MAX_LONGPATH, dumpPathTemplate, pid);
 
-        const char* dumpType = "minidump";
-        switch (minidumpType)
-        {
-            case MiniDumpWithPrivateReadWriteMemory:
-                dumpType = "minidump with heap";
-                break;
-
-            case MiniDumpFilterTriage:
-                dumpType = "triage minidump";
-                break;
-
-            case MiniDumpWithFullMemory:
-                dumpType = "full dump";
-                break;
-
-            default:
-                break;
-        }
         printf("Writing %s to file %s\n", dumpType, (char*)dumpPath);
 
         if (CreateDump(dumpPath, pid, minidumpType))
index 38bf162e430f05183692a92e1115838058fe9766..9a01508e2115852823ca68fbc558678c476d50b4 100644 (file)
@@ -68,7 +68,7 @@ private:
     DbgTransportTarget  * m_pProxy;
     DbgTransportSession * m_pTransport;
 #ifdef FEATURE_REMOTE_PROC_MEM
-    int m_fd;                           // /proc/<pid>/mem handle
+    DWORD m_memoryHandle;                   // PAL_ReadProcessMemory handle or UINT32_MAX if fallback
 #endif
 };
 
@@ -106,9 +106,7 @@ ShimRemoteDataTarget::ShimRemoteDataTarget(DWORD processId,
     m_pContinueStatusChangedUserData = NULL;
 
 #ifdef FEATURE_REMOTE_PROC_MEM
-    char memPath[128];
-    _snprintf_s(memPath, sizeof(memPath), sizeof(memPath), "/proc/%lu/mem", m_processId);
-    m_fd = _open(memPath, 0); // O_RDONLY
+    PAL_OpenProcessMemory(m_processId, &m_memoryHandle);
 #endif
 }
 
@@ -135,11 +133,8 @@ ShimRemoteDataTarget::~ShimRemoteDataTarget()
 void ShimRemoteDataTarget::Dispose()
 {
 #ifdef FEATURE_REMOTE_PROC_MEM
-    if (m_fd != -1)
-    {
-        _close(m_fd);
-        m_fd = -1;
-    }
+    PAL_CloseProcessMemory(m_memoryHandle);
+    m_memoryHandle = UINT32_MAX;
 #endif
     if (m_pTransport != NULL)
     {
@@ -269,10 +264,9 @@ ShimRemoteDataTarget::ReadVirtual(
     HRESULT hr = S_OK;
 
 #ifdef FEATURE_REMOTE_PROC_MEM
-    if (m_fd != -1)
+    if (m_memoryHandle != UINT32_MAX)
     {
-        read = _pread(m_fd, pBuffer, cbRequestSize, (ULONG64)address);
-        if (read == (size_t)-1)
+        if (!PAL_ReadProcessMemory(m_memoryHandle, (ULONG64)address, pBuffer, cbRequestSize, &read))
         {
             hr = E_FAIL;
         }
index 29c010b9e8490963d908ce56a055128427095b1c..31dd9ea4875e992bb97bf5a69e67e1e760894620 100644 (file)
@@ -43,6 +43,9 @@ nativeStringResourceTable_mscorrc
 #PAL_InitializeDLL
 #PAL_TerminateEx
 #PAL_IsDebuggerPresent
+#PAL_OpenProcessMemory
+#PAL_CloseProcessMemory
+#PAL_ReadProcessMemory
 #PAL_ProbeMemory
 #PAL_Random
 #PAL_memcpy
index df658c54690561df949d5433d3677004b09ab1aa..fd56fc90c353579c04a9e1a4a3f1dda93b88cdc7 100644 (file)
@@ -511,6 +511,32 @@ PAL_Random(
     IN OUT LPVOID lpBuffer,
     IN DWORD dwLength);
 
+PALIMPORT
+BOOL
+PALAPI
+PAL_OpenProcessMemory(
+    IN DWORD processId,
+    OUT DWORD* pHandle
+);
+
+PALIMPORT
+VOID
+PALAPI
+PAL_CloseProcessMemory(
+    IN DWORD handle
+);
+
+PALIMPORT
+BOOL
+PALAPI
+PAL_ReadProcessMemory(
+    IN DWORD handle,
+    IN ULONG64 address,
+    IN LPVOID buffer,
+    IN SIZE_T size,
+    OUT SIZE_T* numberOfBytesRead
+);
+
 PALIMPORT
 BOOL
 PALAPI
index b58b2fe587e19d309fedd3f478ede808a0f0b331..7e7e368200587b89cc1b48a684aef25468be9164 100644 (file)
@@ -62,6 +62,11 @@ SET_DEFAULT_DEBUG_CHANNEL(DEBUG); // some headers have code with asserts, so do
 #include <procfs.h>
 #endif // HAVE_PROCFS_H
 
+#ifdef __APPLE__
+#include <mach/mach.h>
+#include <mach/mach_vm.h>
+#endif // __APPLE__
+
 #if HAVE_MACH_EXCEPTIONS
 #include "../exception/machexception.h"
 #endif // HAVE_MACH_EXCEPTIONS
@@ -69,6 +74,7 @@ SET_DEFAULT_DEBUG_CHANNEL(DEBUG); // some headers have code with asserts, so do
 using namespace CorUnix;
 
 extern "C" void DBG_DebugBreak_End();
+extern size_t OffsetWithinPage(off_t addr);
 
 #if HAVE_PROCFS_CTL
 #define CTL_ATTACH      "attach"
@@ -541,6 +547,184 @@ SetThreadContext(
     return ret;
 }
 
+/*++
+Function:
+  PAL_OpenProcessMemory
+
+Abstract
+  Creates the handle for PAL_ReadProcessMemory.
+
+Parameter
+  processId : process id to read memory
+  pHandle : returns a platform specific handle or UINT32_MAX if failed
+
+Return
+  true successful, false invalid process id or not supported.
+--*/
+BOOL
+PALAPI
+PAL_OpenProcessMemory(
+    IN DWORD processId,
+    OUT DWORD* pHandle
+)
+{
+    ENTRY("PAL_OpenProcessMemory(pid=%d)\n", processId);
+    _ASSERTE(pHandle != nullptr);
+    *pHandle = UINT32_MAX;
+#ifdef __APPLE__
+    mach_port_name_t port;
+    kern_return_t result = ::task_for_pid(mach_task_self(), (int)processId, &port);
+    if (result != KERN_SUCCESS)
+    {
+        ERROR("task_for_pid(%d) FAILED %x %s\n", processId, result, mach_error_string(result));
+        LOGEXIT("PAL_OpenProcessMemory FALSE\n");
+        return FALSE;
+    }
+    *pHandle = port;
+#else
+    char memPath[128];
+    _snprintf_s(memPath, sizeof(memPath), sizeof(memPath), "/proc/%lu/mem", processId);
+
+    int fd = open(memPath, O_RDONLY);
+    if (fd == -1)
+    {
+        ERROR("open(%s) FAILED %d (%s)\n", memPath, errno, strerror(errno));
+        LOGEXIT("PAL_OpenProcessMemory FALSE\n");
+        return FALSE;
+    }
+    *pHandle = fd;
+#endif
+    LOGEXIT("PAL_OpenProcessMemory TRUE\n");
+    return TRUE;
+}
+
+/*++
+Function:
+  PAL_CloseProcessMemory
+
+Abstract
+  Closes the PAL_OpenProcessMemory handle.
+
+Parameter
+  handle : from PAL_OpenProcessMemory
+
+Return
+  none
+--*/
+VOID
+PALAPI
+PAL_CloseProcessMemory(
+    IN DWORD handle
+)
+{
+    ENTRY("PAL_CloseProcessMemory(handle=%x)\n", handle);
+    if (handle != UINT32_MAX)
+    {
+#ifdef __APPLE__
+        kern_return_t result = ::mach_port_deallocate(mach_task_self(), (mach_port_name_t)handle);
+        if (result != KERN_SUCCESS)
+        {
+            ERROR("mach_port_deallocate FAILED %x %s\n", result, mach_error_string(result));
+        }
+#else
+        close(handle);
+#endif
+    }
+    LOGEXIT("PAL_CloseProcessMemory\n");
+}
+
+/*++
+Function:
+  PAL_ReadProcessMemory
+
+Abstract
+  Reads process memory. 
+
+Parameter
+  handle : from PAL_OpenProcessMemory
+  address : address of memory to read
+  buffer : buffer to read memory to
+  size : number of bytes to read
+  numberOfBytesRead: number of bytes read (optional)
+
+Return
+  true read memory is successful, false if not.
+--*/
+BOOL
+PALAPI
+PAL_ReadProcessMemory(
+    IN DWORD handle,
+    IN ULONG64 address,
+    IN LPVOID buffer,
+    IN SIZE_T size,
+    OUT SIZE_T* numberOfBytesRead)
+{
+    ENTRY("PAL_ReadProcessMemory(handle=%x, address=%p buffer=%p size=%d)\n", handle, (void*)address, buffer, size);
+    _ASSERTE(handle != 0);
+    _ASSERTE(numberOfBytesRead != nullptr);
+    BOOL result = TRUE;
+    size_t read = 0;
+#ifdef __APPLE__
+    vm_map_t task = (vm_map_t)handle;
+
+    // vm_read_overwrite usually requires that the address be page-aligned
+    // and the size be a multiple of the page size.  We can't differentiate
+    // between the cases in which that's required and those in which it
+    // isn't, so we do it all the time.
+    const size_t pageSize = GetVirtualPageSize();
+    vm_address_t addressAligned = ALIGN_DOWN(address, pageSize);
+    size_t offset = OffsetWithinPage(address);
+    size_t bytesToRead;
+
+    char *data = (char*)malloc(pageSize);
+    if (data == nullptr)
+    {
+        ERROR("malloc(%d) FAILED\n", pageSize);
+        result = FALSE;
+        goto exit;
+    }
+
+    while (size > 0)
+    {
+        vm_size_t bytesRead;
+        
+        bytesToRead = pageSize - offset;
+        if (bytesToRead > size)
+        {
+            bytesToRead = size;
+        }
+        bytesRead = pageSize;
+        kern_return_t result = ::vm_read_overwrite(task, addressAligned, pageSize, (vm_address_t)data, &bytesRead);
+        if (result != KERN_SUCCESS || bytesRead != pageSize)
+        {
+            ERROR("vm_read_overwrite failed for %d bytes from %p: %x %s\n", pageSize, (void*)addressAligned, result, mach_error_string(result));
+            result = FALSE;
+            goto exit;
+        }
+        memcpy((LPSTR)buffer + read , data + offset, bytesToRead);
+        addressAligned = addressAligned + pageSize;
+        read += bytesToRead;
+        size -= bytesToRead;
+        offset = 0;
+    }
+
+exit:
+    if (data != nullptr)
+    {
+        free(data);
+    }
+#else
+    read = pread(handle, buffer, size, address);
+    if (read == (size_t)-1)
+    {
+        result = FALSE;
+    }
+#endif
+    *numberOfBytesRead = read;
+    LOGEXIT("PAL_ReadProcessMemory result=%d bytes read=%d\n", result, read);
+    return result;
+}
+
 /*++
 Function:
   PAL_ProbeMemory
index 75405c7407c8bf0236fc94964df46fd404277158..0cbaef5521d0fe0479c1c3759f85560ad0ca8157 100644 (file)
@@ -2137,7 +2137,7 @@ MAPRecordMapping(
     return palError;
 }
 
-static size_t OffsetWithinPage(off_t addr)
+size_t OffsetWithinPage(off_t addr)
 {
     return addr & (GetVirtualPageSize() - 1);
 }