createdump improvements for better VS4Mac Watson bucketing (#71863)

* Createdump changes for VS4Mac Add target process terminated/alive message. Smaller MacOS dumps. Don't add share_mode == SM_EMPTY regions. Add crashreport success status message for VS4Mac. Launch createdump from SIGTERM handler directly to reduce the time it takes to get the crash report/dump for VS4Mac. * Add more instrumentation to diagnose SIGTERM problem * Remove useless thread state logging
dotnet · Jul 13, 2022 · d7dd98f · d7dd98f
1 parent 3b022db
commit d7dd98f
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 5 deletions.
diff --git a/src/coreclr/debug/createdump/crashinfomac.cpp b/src/coreclr/debug/createdump/crashinfomac.cpp
@@ -3,6 +3,8 @@
 
 #include "createdump.h"
 
+int g_readProcessMemoryResult = KERN_SUCCESS;
+
 bool
 CrashInfo::Initialize()
 {
@@ -127,7 +129,7 @@ CrashInfo::EnumerateMemoryRegions()
         }
         else
         {
-            if ((info.protection & (VM_PROT_READ | VM_PROT_WRITE | VM_PROT_EXECUTE)) != 0)
+            if (info.share_mode != SM_EMPTY && (info.protection & (VM_PROT_READ | VM_PROT_WRITE | VM_PROT_EXECUTE)) != 0)
             {
                 MemoryRegion memoryRegion(ConvertProtectionFlags(info.protection), address, address + size, info.offset);
                 m_allMemoryRegions.insert(memoryRegion);
@@ -362,6 +364,7 @@ CrashInfo::ReadProcessMemory(void* address, void* buffer, size_t size, size_t* r
         kern_return_t result = ::vm_read_overwrite(Task(), addressAligned, PAGE_SIZE, (vm_address_t)data, &bytesRead);
         if (result != KERN_SUCCESS || bytesRead != PAGE_SIZE)
         {
+            g_readProcessMemoryResult = result;
             TRACE_VERBOSE("ReadProcessMemory(%p %d): vm_read_overwrite failed bytesLeft %d bytesRead %d from %p: %x %s\n",
                 address, size, bytesLeft, bytesRead, (void*)addressAligned, result, mach_error_string(result));
             break;

diff --git a/src/coreclr/debug/createdump/crashreportwriter.cpp b/src/coreclr/debug/createdump/crashreportwriter.cpp
@@ -41,6 +41,7 @@ CrashReportWriter::WriteCrashReport(const std::string& dumpFileName)
         }
         WriteCrashReport();
         CloseWriter();
+        printf_status("Crash report successfully written\n");
     }
     catch (const std::exception& e)
     {

diff --git a/src/coreclr/debug/createdump/createdumpunix.cpp b/src/coreclr/debug/createdump/createdumpunix.cpp
@@ -69,6 +69,22 @@ CreateDump(const char* dumpPathTemplate, int pid, const char* dumpType, MINIDUMP
     }
     result = true;
 exit:
+    if (kill(pid, 0) == 0)
+    {
+        printf_status("Target process is alive\n");
+    }
+    else
+    {
+        int err = errno;
+        if (err == ESRCH)
+        {
+            printf_error("Target process terminated\n");
+        }
+        else
+        {
+            printf_error("kill(%d, 0) FAILED %s (%d)\n", pid, strerror(err), err);
+        }
+    }
     crashInfo->CleanupAndResumeProcess();
     return result;
 }
diff --git a/src/coreclr/debug/createdump/dumpwritermacho.cpp b/src/coreclr/debug/createdump/dumpwritermacho.cpp
@@ -4,6 +4,8 @@
 #include "createdump.h"
 #include "specialthreadinfo.h"
 
+extern int g_readProcessMemoryResult;
+
 //
 // Write the core dump file
 //
@@ -264,13 +266,13 @@ DumpWriter::WriteSegments()
                 size_t read = 0;
 
                 if (!m_crashInfo.ReadProcessMemory((void*)address, m_tempBuffer, bytesToRead, &read)) {
-                    printf_error("ReadProcessMemory(%" PRIA PRIx64 ", %08zx) FAILED\n", address, bytesToRead);
+                    printf_error("ReadProcessMemory(%" PRIA PRIx64 ", %08zx) read %d FAILED %s (%x)\n", address, bytesToRead, read, mach_error_string(g_readProcessMemoryResult), g_readProcessMemoryResult);
                     return false;
                 }
 
                 // This can happen if the target process dies before createdump is finished
                 if (read == 0) {
-                    printf_error("ReadProcessMemory(%" PRIA PRIx64 ", %08zx) returned 0 bytes read\n", address, bytesToRead);
+                    printf_error("ReadProcessMemory(%" PRIA PRIx64 ", %08zx) returned 0 bytes read: %s (%x)\n", address, bytesToRead, mach_error_string(g_readProcessMemoryResult), g_readProcessMemoryResult);
                     return false;
                 }
 

diff --git a/src/coreclr/pal/src/exception/signal.cpp b/src/coreclr/pal/src/exception/signal.cpp
@@ -746,6 +746,11 @@ static void sigterm_handler(int code, siginfo_t *siginfo, void *context)
 {
     if (PALIsInitialized())
     {
+        char* enable = getenv("COMPlus_EnableDumpOnSigTerm");
+        if (enable != nullptr && strcmp(enable, "1") == 0)
+        {
+            PROCCreateCrashDumpIfEnabled(code);
+        }
         // g_pSynchronizationManager shouldn't be null if PAL is initialized.
         _ASSERTE(g_pSynchronizationManager != nullptr);
 

diff --git a/src/coreclr/vm/exceptionhandling.cpp b/src/coreclr/vm/exceptionhandling.cpp
@@ -213,8 +213,7 @@ void HandleTerminationRequest(int terminationExitCode)
     {
         SetLatchedExitCode(terminationExitCode);
 
-        DWORD enabled = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_EnableDumpOnSigTerm);
-        ForceEEShutdown(enabled == 1 ? SCA_TerminateProcessWhenShutdownComplete : SCA_ExitProcessWhenShutdownComplete);
+        ForceEEShutdown(SCA_ExitProcessWhenShutdownComplete);
     }
 }
 #endif