Skip to content

Commit

Permalink
Update MERP event type to MonoAppCrash
Browse files Browse the repository at this point in the history
* Update parameters in MERP WerReportMetadata xml

To enable backend analysis, we need to update our parameters to match our back end event type, which is MonoAppCrash.

It doesn't make sense to me that the previous implementation constructed the XML here in Mono, but depended on the user (VSMac in this case) to add the "correct" event type.  The parameters in the XML, must match exactly what is expected for the given EventType, and so we should ensure they are always in-sync by defining them here.  Thus, I removed the usage of `eventType` in `mono_merp_enable` and hard coded the expected eventType.

* [merp] Don't pass eventType from managed

The eventType must me MONO_MERP_EVENT_TYPE_STR ("MonoAppCrash") - the backend
determines the format of the other fields based on the event type, so it
doesn't make sense to pass other values from managed since the runtime controls
the other fields.

For compatability Mono.Runtime.EnableMicrosoftTelemetry will still take a
string parameter (now called "unused" instead of "eventType_str") but it will
not pass it down to the internal call.

* Bump corlib version

* [merp] Revert paramerters in WerReportMetadata.xml

Still need event type to be `MonoAppCrash` but the parameters will be the same as before.
  • Loading branch information
kdubau authored and marek-safar committed Oct 15, 2019
1 parent 6184ff0 commit b601371
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 15 deletions.
2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ MONO_VERSION_BUILD=`echo $VERSION | cut -d . -f 3`
# This line is parsed by tools besides autoconf, such as msvc/mono.winconfig.targets.
# It should remain in the format they expect.
#
MONO_CORLIB_VERSION=657116DD-B24B-4615-8680-6C56BA42F86D
MONO_CORLIB_VERSION=A144A63D-652C-4CCF-A9EE-8E5A091547F1

#
# Put a quoted #define in config.h.
Expand Down
7 changes: 3 additions & 4 deletions mcs/class/corlib/Mono/Runtime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ static Tuple<String, ulong, ulong>
static extern void DisableMicrosoftTelemetry ();

[MethodImplAttribute (MethodImplOptions.InternalCall)]
static extern void EnableMicrosoftTelemetry_internal (IntPtr appBundleID, IntPtr appSignature, IntPtr appVersion, IntPtr merpGUIPath, IntPtr eventType, IntPtr appPath, IntPtr configDir);
static extern void EnableMicrosoftTelemetry_internal (IntPtr appBundleID, IntPtr appSignature, IntPtr appVersion, IntPtr merpGUIPath, IntPtr appPath, IntPtr configDir);

[MethodImplAttribute (MethodImplOptions.InternalCall)]
static extern void SendMicrosoftTelemetry_internal (IntPtr payload, ulong portable_hash, ulong unportable_hash);
Expand Down Expand Up @@ -148,18 +148,17 @@ static void SendExceptionToTelemetry (Exception exc)
}

// All must be set except for configDir_str
static void EnableMicrosoftTelemetry (string appBundleID_str, string appSignature_str, string appVersion_str, string merpGUIPath_str, string eventType_str, string appPath_str, string configDir_str)
static void EnableMicrosoftTelemetry (string appBundleID_str, string appSignature_str, string appVersion_str, string merpGUIPath_str, string unused /* eventType_str */, string appPath_str, string configDir_str)
{
if (RuntimeInformation.IsOSPlatform (OSPlatform.OSX)) {
using (var appBundleID_chars = RuntimeMarshal.MarshalString (appBundleID_str))
using (var appSignature_chars = RuntimeMarshal.MarshalString (appSignature_str))
using (var appVersion_chars = RuntimeMarshal.MarshalString (appVersion_str))
using (var merpGUIPath_chars = RuntimeMarshal.MarshalString (merpGUIPath_str))
using (var eventType_chars = RuntimeMarshal.MarshalString (eventType_str))
using (var appPath_chars = RuntimeMarshal.MarshalString (appPath_str))
using (var configDir_chars = RuntimeMarshal.MarshalString (configDir_str))
{
EnableMicrosoftTelemetry_internal (appBundleID_chars.Value, appSignature_chars.Value, appVersion_chars.Value, merpGUIPath_chars.Value, eventType_chars.Value, appPath_chars.Value, configDir_chars.Value);
EnableMicrosoftTelemetry_internal (appBundleID_chars.Value, appSignature_chars.Value, appVersion_chars.Value, merpGUIPath_chars.Value, appPath_chars.Value, configDir_chars.Value);
}
} else {
throw new PlatformNotSupportedException("Merp support is currently only supported on OSX.");
Expand Down
2 changes: 1 addition & 1 deletion mono/metadata/icall-def-netcore.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ HANDLES(RUNTIME_1, "DisableMicrosoftTelemetry", ves_icall_Mono_Runtime_DisableMi
HANDLES(RUNTIME_15, "DumpStateSingle_internal", ves_icall_Mono_Runtime_DumpStateSingle, MonoString, 2, (guint64_ref, guint64_ref))
HANDLES(RUNTIME_16, "DumpStateTotal_internal", ves_icall_Mono_Runtime_DumpStateTotal, MonoString, 2, (guint64_ref, guint64_ref))
HANDLES(RUNTIME_18, "EnableCrashReportLog_internal", ves_icall_Mono_Runtime_EnableCrashReportingLog, void, 1, (const_char_ptr))
HANDLES(RUNTIME_2, "EnableMicrosoftTelemetry_internal", ves_icall_Mono_Runtime_EnableMicrosoftTelemetry, void, 7, (const_char_ptr, const_char_ptr, const_char_ptr, const_char_ptr, const_char_ptr, const_char_ptr, const_char_ptr))
HANDLES(RUNTIME_2, "EnableMicrosoftTelemetry_internal", ves_icall_Mono_Runtime_EnableMicrosoftTelemetry, void, 6, (const_char_ptr, const_char_ptr, const_char_ptr, const_char_ptr, const_char_ptr, const_char_ptr))
HANDLES(RUNTIME_3, "ExceptionToState_internal", ves_icall_Mono_Runtime_ExceptionToState, MonoString, 3, (MonoException, guint64_ref, guint64_ref))
HANDLES(RUNTIME_4, "GetDisplayName", ves_icall_Mono_Runtime_GetDisplayName, MonoString, 0, ())
HANDLES(RUNTIME_12, "GetNativeStackTrace", ves_icall_Mono_Runtime_GetNativeStackTrace, MonoString, 1, (MonoException))
Expand Down
2 changes: 1 addition & 1 deletion mono/metadata/icall-def.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ HANDLES(RUNTIME_1, "DisableMicrosoftTelemetry", ves_icall_Mono_Runtime_DisableMi
HANDLES(RUNTIME_15, "DumpStateSingle_internal", ves_icall_Mono_Runtime_DumpStateSingle, MonoString, 2, (guint64_ref, guint64_ref))
HANDLES(RUNTIME_16, "DumpStateTotal_internal", ves_icall_Mono_Runtime_DumpStateTotal, MonoString, 2, (guint64_ref, guint64_ref))
HANDLES(RUNTIME_18, "EnableCrashReportLog_internal", ves_icall_Mono_Runtime_EnableCrashReportingLog, void, 1, (const_char_ptr))
HANDLES(RUNTIME_2, "EnableMicrosoftTelemetry_internal", ves_icall_Mono_Runtime_EnableMicrosoftTelemetry, void, 7, (const_char_ptr, const_char_ptr, const_char_ptr, const_char_ptr, const_char_ptr, const_char_ptr, const_char_ptr))
HANDLES(RUNTIME_2, "EnableMicrosoftTelemetry_internal", ves_icall_Mono_Runtime_EnableMicrosoftTelemetry, void, 6, (const_char_ptr, const_char_ptr, const_char_ptr, const_char_ptr, const_char_ptr, const_char_ptr))
HANDLES(RUNTIME_3, "ExceptionToState_internal", ves_icall_Mono_Runtime_ExceptionToState, MonoString, 3, (MonoException, guint64_ref, guint64_ref))
HANDLES(RUNTIME_4, "GetDisplayName", ves_icall_Mono_Runtime_GetDisplayName, MonoString, 0, ())
HANDLES(RUNTIME_12, "GetNativeStackTrace", ves_icall_Mono_Runtime_GetNativeStackTrace, MonoString, 1, (MonoException))
Expand Down
4 changes: 2 additions & 2 deletions mono/metadata/icall.c
Original file line number Diff line number Diff line change
Expand Up @@ -6236,10 +6236,10 @@ ves_icall_Mono_Runtime_AnnotateMicrosoftTelemetry (const char *key, const char *
}

void
ves_icall_Mono_Runtime_EnableMicrosoftTelemetry (const char *appBundleID, const char *appSignature, const char *appVersion, const char *merpGUIPath, const char *eventType, const char *appPath, const char *configDir, MonoError *error)
ves_icall_Mono_Runtime_EnableMicrosoftTelemetry (const char *appBundleID, const char *appSignature, const char *appVersion, const char *merpGUIPath, const char *appPath, const char *configDir, MonoError *error)
{
#if defined(TARGET_OSX) && !defined(DISABLE_CRASH_REPORTING)
mono_merp_enable (appBundleID, appSignature, appVersion, merpGUIPath, eventType, appPath, configDir);
mono_merp_enable (appBundleID, appSignature, appVersion, merpGUIPath, appPath, configDir);

// Why does this install the sigterm handler so early?
// mono_get_runtime_callbacks ()->install_state_summarizer ();
Expand Down
14 changes: 9 additions & 5 deletions mono/utils/mono-merp.c
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,18 @@ typedef struct {
char systemModel [100];
const char *systemManufacturer;

const char *eventType;
const char *eventType; /* Must be MONO_MERP_EVENT_TYPE_STR */

MonoStackHash hashes;
GSList *annotations;
} MERPStruct;

/* The event type determines the format of the fields that are reported. It
* must be MonoAppCrash for the rest of our report to make sense.
*/
#define MONO_MERP_EVENT_TYPE_STR "MonoAppCrash"


typedef struct {
gboolean enable_merp;

Expand Down Expand Up @@ -413,7 +419,7 @@ mono_init_merp (const intptr_t crashed_pid, const char *signal, MonoStackHash *h
merp->systemManufacturer = "apple";
get_apple_model ((char *) merp->systemModel, sizeof (merp->systemModel));

merp->eventType = config.eventType;
merp->eventType = MONO_MERP_EVENT_TYPE_STR;

merp->hashes = *hashes;

Expand Down Expand Up @@ -582,7 +588,6 @@ mono_merp_disable (void)
g_free ((char*)config.appSignature);
g_free ((char*)config.appVersion);
g_free ((char*)config.merpGUIPath);
g_free ((char*)config.eventType);
g_free ((char*)config.appPath);
g_free ((char*)config.moduleVersion);
g_slist_free (config.annotations);
Expand All @@ -592,7 +597,7 @@ mono_merp_disable (void)
}

void
mono_merp_enable (const char *appBundleID, const char *appSignature, const char *appVersion, const char *merpGUIPath, const char *eventType, const char *appPath, const char *configDir)
mono_merp_enable (const char *appBundleID, const char *appSignature, const char *appVersion, const char *merpGUIPath, const char *appPath, const char *configDir)
{
mono_memory_barrier ();

Expand All @@ -617,7 +622,6 @@ mono_merp_enable (const char *appBundleID, const char *appSignature, const char
config.appSignature = g_strdup (appSignature);
config.appVersion = g_strdup (appVersion);
config.merpGUIPath = g_strdup (merpGUIPath);
config.eventType = g_strdup (eventType);
config.appPath = g_strdup (appPath);

config.log = g_getenv ("MONO_MERP_VERBOSE") != NULL;
Expand Down
2 changes: 1 addition & 1 deletion mono/utils/mono-merp.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ void mono_merp_disable (void);
* See MERP documentation for information on the bundle ID, signature, and version fields
*/
void
mono_merp_enable (const char *appBundleID, const char *appSignature, const char *appVersion, const char *merpGUIPath, const char *eventType, const char *appPath, const char *configDir);
mono_merp_enable (const char *appBundleID, const char *appSignature, const char *appVersion, const char *merpGUIPath, const char *appPath, const char *configDir);

/**
* Whether the MERP-based handler is registered
Expand Down

0 comments on commit b601371

Please sign in to comment.