Fix dump generation issues for VS4Mac (#61063)

The VS4Mac team found two issues preventing them from successfully diagnosing VS4Mac failures
on .NET:

1) Multiple "crashed" threads in the crash report json (#60932).
2) No flag or way to generate the crash report for hangs via the diagnostic server IPC commands (#60775).

Add new generate core dump IPC command that allows the generate crash report flag to be passed through to createdump for
VS4Mac. VS4Mac needs to distinguish between WriteDump/no signal and unknown signal ExceptionType. Change unknown signal
exception type to 0.

Issue: https://github.com/dotnet/runtime/issues/60775

Fix how the load bias is calculate for shared modules

Local testing with the SOS tests. VS4Mac team testing and verification.

Low risk because it only affects createdump, dump IPC command and the runtime dump generation path.

diff --git a/src/coreclr/debug/createdump/crashreportwriter.cpp b/src/coreclr/debug/createdump/crashreportwriter.cpp
index 1fb0865..4e9382f 100644 (file)
--- a/src/coreclr/debug/createdump/crashreportwriter.cpp
+++ b/src/coreclr/debug/createdump/crashreportwriter.cpp
@@ -84,18 +84,20 @@ CrashReportWriter::WriteCrashReport()
     {
         OpenObject();
         bool crashed = false;
-        if (thread->ManagedExceptionObject() != 0)
+        if (thread->Tid() == m_crashInfo.CrashThread())
         {
             crashed = true;
-            exceptionType = "0x05000000";   // ManagedException
-        }
-        else
-        {
-            if (thread->Tid() == m_crashInfo.CrashThread())
+            if (thread->ManagedExceptionObject() != 0)
+            {
+                exceptionType = "0x05000000";   // ManagedException
+            }
+            else
             {
-                crashed = true;
                 switch (m_crashInfo.Signal())
                 {
+                case 0:
+                    break;
+
                 case SIGILL:
                     exceptionType = "0x50000000";
                     break;
@@ -121,9 +123,12 @@ CrashReportWriter::WriteCrashReport()
                     break;
 
                 case SIGABRT:
-                default:
                     exceptionType = "0x30000000";
                     break;
+
+                default:
+                    exceptionType = "0x00000000";
+                    break;
                 }
             }
         }

diff --git a/src/coreclr/debug/dbgutil/machoreader.cpp b/src/coreclr/debug/dbgutil/machoreader.cpp
index a19c01b..48fea72 100644 (file)
--- a/src/coreclr/debug/dbgutil/machoreader.cpp
+++ b/src/coreclr/debug/dbgutil/machoreader.cpp
@@ -207,11 +207,11 @@ MachOModule::ReadLoadCommands()
                 m_segments.push_back(segment);
 
                 // Calculate the load bias for the module. This is the value to add to the vmaddr of a
-                // segment to get the actual address. 
-                if (strcmp(segment->segname, SEG_TEXT) == 0)
+                // segment to get the actual address. For shared modules, this is 0 since those segments
+                // are absolute address.
+                if (segment->fileoff == 0 && segment->filesize > 0)
                 {
                     m_loadBias = m_baseAddress - segment->vmaddr;
-                    m_reader.TraceVerbose("CMD: load bias %016llx\n", m_loadBias);
                 }
 
                 m_reader.TraceVerbose("CMD: vmaddr %016llx vmsize %016llx fileoff %016llx filesize %016llx nsects %d max %c%c%c init %c%c%c %02x %s\n",
@@ -245,6 +245,7 @@ MachOModule::ReadLoadCommands()
             // Get next load command
             command = (load_command*)((char*)command + command->cmdsize);
         }
+        m_reader.TraceVerbose("CMD: load bias %016llx\n", m_loadBias);
     }
 
     return true;

diff --git a/src/coreclr/pal/inc/pal.h b/src/coreclr/pal/inc/pal.h
index cd96f57..e59e406 100644 (file)
--- a/src/coreclr/pal/inc/pal.h
+++ b/src/coreclr/pal/inc/pal.h
@@ -432,13 +432,22 @@ PALAPI
 PAL_SetShutdownCallback(
     IN PSHUTDOWN_CALLBACK callback);
 
+// Must be the same as the copy in excep.h and the WriteDumpFlags enum in the diagnostics repo
+enum
+{
+    GenerateDumpFlagsNone = 0x00,
+    GenerateDumpFlagsLoggingEnabled = 0x01,
+    GenerateDumpFlagsVerboseLoggingEnabled = 0x02,
+    GenerateDumpFlagsCrashReportEnabled = 0x04
+};
+
 PALIMPORT
 BOOL
 PALAPI
 PAL_GenerateCoreDump(
     IN LPCSTR dumpName,
     IN INT dumpType,
-    IN BOOL diag);
+    IN ULONG32 flags);
 
 typedef VOID (*PPAL_STARTUP_CALLBACK)(
     char *modulePath,

diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp
index 0ae3f2e..2b349a6 100644 (file)
--- a/src/coreclr/pal/src/thread/process.cpp
+++ b/src/coreclr/pal/src/thread/process.cpp
@@ -3124,10 +3124,9 @@ PROCBuildCreateDumpCommandLine(
     std::vector<const char*>& argv,
     char** pprogram,
     char** ppidarg,
-    char* dumpName,
-    char* dumpType,
-    BOOL diag,
-    BOOL crashReport)
+    const char* dumpName,
+    const char* dumpType,
+    ULONG32 flags)
 {
     if (g_szCoreCLRPath == nullptr)
     {
@@ -3190,12 +3189,17 @@ PROCBuildCreateDumpCommandLine(
         }
     }
 
-    if (diag)
+    if (flags & GenerateDumpFlagsLoggingEnabled)
     {
         argv.push_back("--diag");
     }
 
-    if (crashReport)
+    if (flags & GenerateDumpFlagsVerboseLoggingEnabled)
+    {
+        argv.push_back("--verbose");
+    }
+
+    if (flags & GenerateDumpFlagsCrashReportEnabled)
     {
         argv.push_back("--crashreport");
     }
@@ -3286,10 +3290,18 @@ PROCAbortInitialize()
         BOOL diag = diagStr != nullptr && strcmp(diagStr, "1") == 0;
         char* crashReportStr = getenv("COMPlus_EnableCrashReport");
         BOOL crashReport = crashReportStr != nullptr && strcmp(crashReportStr, "1") == 0;
-
+        ULONG32 flags = GenerateDumpFlagsNone;
+        if (diag)
+        {
+            flags |= GenerateDumpFlagsLoggingEnabled;
+        }
+        if (crashReport)
+        {
+            flags |= GenerateDumpFlagsCrashReportEnabled;
+        }
         char* program = nullptr;
         char* pidarg = nullptr;
-        if (!PROCBuildCreateDumpCommandLine(g_argvCreateDump, &program, &pidarg, dumpName, dumpType, diag, crashReport))
+        if (!PROCBuildCreateDumpCommandLine(g_argvCreateDump, &program, &pidarg, dumpName, dumpType, flags))
         {
             return FALSE;
         }
@@ -3311,8 +3323,8 @@ Parameters:
         WithHeap = 2,
         Triage = 3,
         Full = 4
-    diag
-        true - log createdump diagnostics to console
+    flags
+        See enum
 
 Return:
     TRUE success
@@ -3322,7 +3334,7 @@ BOOL
 PAL_GenerateCoreDump(
     LPCSTR dumpName,
     INT dumpType,
-    BOOL diag)
+    ULONG32 flags)
 {
     std::vector<const char*> argvCreateDump;
     char dumpTypeStr[16];
@@ -3341,7 +3353,7 @@ PAL_GenerateCoreDump(
     }
     char* program = nullptr;
     char* pidarg = nullptr;
-    BOOL result = PROCBuildCreateDumpCommandLine(argvCreateDump, &program, &pidarg, (char*)dumpName, dumpTypeStr, diag, false);
+    BOOL result = PROCBuildCreateDumpCommandLine(argvCreateDump, &program, &pidarg, dumpName, dumpTypeStr, flags);
     if (result)
     {
         result = PROCCreateCrashDump(argvCreateDump);

diff --git a/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h b/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h
index 646b8f9..dd5e683 100644 (file)
--- a/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h
+++ b/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h
@@ -188,16 +188,22 @@ ds_rt_config_value_get_default_port_suspend (void)
 
 static
 ds_ipc_result_t
-ds_rt_generate_core_dump (DiagnosticsGenerateCoreDumpCommandPayload *payload)
+ds_rt_generate_core_dump (DiagnosticsDumpCommandId commandId, DiagnosticsGenerateCoreDumpCommandPayload *payload)
 {
        STATIC_CONTRACT_NOTHROW;
 
        ds_ipc_result_t result = DS_IPC_E_FAIL;
        EX_TRY
        {
+               uint32_t flags = ds_generate_core_dump_command_payload_get_flags(payload);
+               if (commandId == DS_DUMP_COMMANDID_GENERATE_CORE_DUMP)
+               {
+                       // For the old commmand, this payload field is a bool of whether to enable logging
+                       flags = flags != 0 ? GenerateDumpFlagsLoggingEnabled : 0;
+               }
                if (GenerateDump (reinterpret_cast<LPCWSTR>(ds_generate_core_dump_command_payload_get_dump_name (payload)),
                        static_cast<int32_t>(ds_generate_core_dump_command_payload_get_dump_type (payload)),
-                       (ds_generate_core_dump_command_payload_get_diagnostics (payload) != 0) ? true : false))
+                       flags))
                        result = DS_IPC_S_OK;
        }
        EX_CATCH {}

diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp
index a6bf988..1674aa2 100644 (file)
--- a/src/coreclr/vm/excep.cpp
+++ b/src/coreclr/vm/excep.cpp
@@ -4178,8 +4178,8 @@ InitializeCrashDump()
 
 bool GenerateDump(
     LPCWSTR dumpName,
-    int dumpType,
-    bool diag)
+    INT dumpType,
+    ULONG32 flags)
 {
 #ifdef TARGET_UNIX
     MAKE_UTF8PTR_FROMWIDE_NOTHROW (dumpNameUtf8, dumpName);
@@ -4189,10 +4189,10 @@ bool GenerateDump(
     }
     else
     {
-        return PAL_GenerateCoreDump(dumpNameUtf8, dumpType, diag);
+        return PAL_GenerateCoreDump(dumpNameUtf8, dumpType, flags);
     }
 #else // TARGET_UNIX
-    return GenerateCrashDump(dumpName, dumpType, diag);
+    return GenerateCrashDump(dumpName, dumpType, flags & GenerateDumpFlagsLoggingEnabled);
 #endif // TARGET_UNIX
 }
 

diff --git a/src/coreclr/vm/excep.h b/src/coreclr/vm/excep.h
index f338dd7..f95a8b6 100644 (file)
--- a/src/coreclr/vm/excep.h
+++ b/src/coreclr/vm/excep.h
@@ -195,10 +195,20 @@ enum UnhandledExceptionLocation
 };
 
 #ifdef HOST_WINDOWS
+
+// Must be the same as the copy in pal.h and the WriteDumpFlags enum in the diagnostics repo
+enum
+{
+    GenerateDumpFlagsNone = 0x00,
+    GenerateDumpFlagsLoggingEnabled = 0x01,
+    GenerateDumpFlagsVerboseLoggingEnabled = 0x02,
+    GenerateDumpFlagsCrashReportEnabled = 0x04
+};
+
 void InitializeCrashDump();
 void CreateCrashDumpIfEnabled(bool stackoverflow = false);
 #endif
-bool GenerateDump(LPCWSTR dumpName, int dumpType, bool diag);
+bool GenerateDump(LPCWSTR dumpName, INT dumpType, ULONG32 flags);
 
 // Generates crash dumps if enabled for both Windows and Linux
 void CrashDumpAndTerminateProcess(UINT exitCode);

diff --git a/src/coreclr/vm/gcenv.ee.cpp b/src/coreclr/vm/gcenv.ee.cpp
index f62a282..e03bd67 100644 (file)
--- a/src/coreclr/vm/gcenv.ee.cpp
+++ b/src/coreclr/vm/gcenv.ee.cpp
@@ -1656,7 +1656,7 @@ void GCToEEInterface::AnalyzeSurvivorsFinished(size_t gcIndex, int condemnedGene
             {
                 EX_TRY
                 {
-                    GenerateDump (GENAWARE_DUMP_FILE_NAME, 2, false);
+                    GenerateDump (GENAWARE_DUMP_FILE_NAME, 2, GenerateDumpFlagsNone);
                 }
                 EX_CATCH {}
                 EX_END_CATCH(SwallowAllExceptions);
@@ -1736,4 +1736,4 @@ uint32_t GCToEEInterface::GetCurrentProcessCpuCount()
 void GCToEEInterface::DiagAddNewRegion(int generation, uint8_t* rangeStart, uint8_t* rangeEnd, uint8_t* rangeEndReserved)
 {
     ProfilerAddNewRegion(generation, rangeStart, rangeEnd, rangeEndReserved);
-}
\ No newline at end of file
+}

diff --git a/src/mono/mono/eventpipe/ds-rt-mono.h b/src/mono/mono/eventpipe/ds-rt-mono.h
index 1785d6e..d3ed4f0 100644 (file)
--- a/src/mono/mono/eventpipe/ds-rt-mono.h
+++ b/src/mono/mono/eventpipe/ds-rt-mono.h
@@ -172,7 +172,7 @@ ds_rt_config_value_get_default_port_suspend (void)
 static
 inline
 ds_ipc_result_t
-ds_rt_generate_core_dump (DiagnosticsGenerateCoreDumpCommandPayload *payload)
+ds_rt_generate_core_dump (DiagnosticsDumpCommandId commandId, DiagnosticsGenerateCoreDumpCommandPayload *payload)
 {
        // TODO: Implement.
        return DS_IPC_E_NOTSUPPORTED;

diff --git a/src/native/eventpipe/ds-dump-protocol.c b/src/native/eventpipe/ds-dump-protocol.c
index 4e247c3..0a93304 100644 (file)
--- a/src/native/eventpipe/ds-dump-protocol.c
+++ b/src/native/eventpipe/ds-dump-protocol.c
@@ -51,7 +51,7 @@ generate_core_dump_command_try_parse_payload (
 
        if (!ds_ipc_message_try_parse_string_utf16_t (&buffer_cursor, &buffer_cursor_len, &instance->dump_name ) ||
                !ds_ipc_message_try_parse_uint32_t (&buffer_cursor, &buffer_cursor_len, &instance->dump_type) ||
-               !ds_ipc_message_try_parse_uint32_t (&buffer_cursor, &buffer_cursor_len, &instance->diagnostics))
+               !ds_ipc_message_try_parse_uint32_t (&buffer_cursor, &buffer_cursor_len, &instance->flags))
                ep_raise_error ();
 
 ep_on_exit:
@@ -106,6 +106,7 @@ dump_protocol_helper_generate_core_dump (
                return false;
 
        bool result = false;
+       DiagnosticsDumpCommandId commandId = (DiagnosticsDumpCommandId)ds_ipc_header_get_commandid (ds_ipc_message_get_header_ref (message));
        DiagnosticsGenerateCoreDumpCommandPayload *payload;
        payload = (DiagnosticsGenerateCoreDumpCommandPayload *)ds_ipc_message_try_parse_payload (message, generate_core_dump_command_try_parse_payload);
 
@@ -115,8 +116,8 @@ dump_protocol_helper_generate_core_dump (
        }
 
        ds_ipc_result_t ipc_result;
-       ipc_result = ds_rt_generate_core_dump (payload);
-       if (result != DS_IPC_S_OK) {
+       ipc_result = ds_rt_generate_core_dump (commandId, payload);
+       if (ipc_result != DS_IPC_S_OK) {
                ds_ipc_message_send_error (stream, result);
                ep_raise_error ();
        } else {
@@ -147,6 +148,7 @@ ds_dump_protocol_helper_handle_ipc_message (
 
        switch ((DiagnosticsDumpCommandId)ds_ipc_header_get_commandid (ds_ipc_message_get_header_ref (message))) {
        case DS_DUMP_COMMANDID_GENERATE_CORE_DUMP:
+       case DS_DUMP_COMMANDID_GENERATE_CORE_DUMP2:
                result = dump_protocol_helper_generate_core_dump (message, stream);
                break;
        default:

diff --git a/src/native/eventpipe/ds-dump-protocol.h b/src/native/eventpipe/ds-dump-protocol.h
index 22e429a..8065e8c 100644 (file)
--- a/src/native/eventpipe/ds-dump-protocol.h
+++ b/src/native/eventpipe/ds-dump-protocol.h
@@ -27,13 +27,13 @@ struct _DiagnosticsGenerateCoreDumpCommandPayload_Internal {
        // The protocol buffer is defined as:
        //   string - dumpName (UTF16)
        //   int - dumpType
-       //   int - diagnostics
+       //   uint32 - flags
        // returns
        //   ulong - status
 
        const ep_char16_t *dump_name;
        uint32_t dump_type;
-       uint32_t diagnostics;
+       uint32_t flags;
 };
 
 #if !defined(DS_INLINE_GETTER_SETTER) && !defined(DS_IMPL_DUMP_PROTOCOL_GETTER_SETTER)
@@ -44,7 +44,7 @@ struct _DiagnosticsGenerateCoreDumpCommandPayload {
 
 DS_DEFINE_GETTER(DiagnosticsGenerateCoreDumpCommandPayload *, generate_core_dump_command_payload, const ep_char16_t *, dump_name)
 DS_DEFINE_GETTER(DiagnosticsGenerateCoreDumpCommandPayload *, generate_core_dump_command_payload, uint32_t, dump_type)
-DS_DEFINE_GETTER(DiagnosticsGenerateCoreDumpCommandPayload *, generate_core_dump_command_payload, uint32_t, diagnostics)
+DS_DEFINE_GETTER(DiagnosticsGenerateCoreDumpCommandPayload *, generate_core_dump_command_payload, uint32_t, flags)
 
 DiagnosticsGenerateCoreDumpCommandPayload *
 ds_generate_core_dump_command_payload_alloc (void);

diff --git a/src/native/eventpipe/ds-rt.h b/src/native/eventpipe/ds-rt.h
index 81ddd3c..8de7157 100644 (file)
--- a/src/native/eventpipe/ds-rt.h
+++ b/src/native/eventpipe/ds-rt.h
@@ -87,7 +87,7 @@ ds_rt_config_value_get_default_port_suspend (void);
 
 static
 ds_ipc_result_t
-ds_rt_generate_core_dump (DiagnosticsGenerateCoreDumpCommandPayload *payload);
+ds_rt_generate_core_dump (DiagnosticsDumpCommandId commandId, DiagnosticsGenerateCoreDumpCommandPayload *payload);
 
 /*
  * DiagnosticsIpc.

diff --git a/src/native/eventpipe/ds-types.h b/src/native/eventpipe/ds-types.h
index fc5d3e1..fba8ba8 100644 (file)
--- a/src/native/eventpipe/ds-types.h
+++ b/src/native/eventpipe/ds-types.h
@@ -44,6 +44,7 @@ typedef struct _EventPipeStopTracingCommandPayload EventPipeStopTracingCommandPa
 typedef enum {
        DS_DUMP_COMMANDID_RESERVED = 0x00,
        DS_DUMP_COMMANDID_GENERATE_CORE_DUMP = 0x01,
+       DS_DUMP_COMMANDID_GENERATE_CORE_DUMP2 = 0x02,
        // future
 } DiagnosticsDumpCommandId;