From f7f561703a50ab1c5fbe3cf2392265c007b35b34 Mon Sep 17 00:00:00 2001 From: monojenkins Date: Wed, 19 Feb 2020 14:16:29 -0500 Subject: [2019-12] [merp] Capture Environment.FailFast message in crash report (#18921) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [merp] Capture Environment.FailFast message in crash report * [test] merp-crash-test refactor crasher data; add optional validator Switch from a Tuple to a struct for the crash items. Add a third optional validator field which is an action to run on the json blob that we get back. Not updating existing tests to do any json validation, yet. * [test] Add new MERP crash test for Environment.FailFast Check that the resulting json blob includes the failfast message. Co-authored-by: Aleksey Kliger (λgeek) --- mono/metadata/icall.c | 5 +++ mono/tests/merp-crash-test.cs | 84 +++++++++++++++++++++++++++++++++++-------- mono/utils/mono-state.c | 58 ++++++++++++++++++++++-------- mono/utils/mono-state.h | 6 ++++ 4 files changed, 124 insertions(+), 29 deletions(-) diff --git a/mono/metadata/icall.c b/mono/metadata/icall.c index 19001eae024..4ee6e9958e7 100644 --- a/mono/metadata/icall.c +++ b/mono/metadata/icall.c @@ -7735,7 +7735,12 @@ ves_icall_System_Environment_FailFast (MonoStringHandle message, MonoExceptionHa } else { char *msg = mono_string_handle_to_utf8 (message, error); g_warning ("CLR: Managed code called FailFast, saying \"%s\"", msg); +#ifndef DISABLE_CRASH_REPORTING + char *old_msg = mono_crash_save_failfast_msg (msg); + g_free (old_msg); +#else g_free (msg); +#endif } if (!MONO_HANDLE_IS_NULL (exception)) { diff --git a/mono/tests/merp-crash-test.cs b/mono/tests/merp-crash-test.cs index d04f814e318..b5e255b46ec 100644 --- a/mono/tests/merp-crash-test.cs +++ b/mono/tests/merp-crash-test.cs @@ -11,15 +11,35 @@ class C { class CrasherClass { - public static List> Crashers; + public struct Crasher { + public string Name {get;} + public Action Action {get; } + + public Action Validator {get; } + + public Crasher (string name, Action action, Action validator = null) + { + Name = name; + Action = action; + Validator = validator; + } + } + + public class ValidationException : Exception { + public ValidationException () : base () {} + public ValidationException (string msg) : base (msg) {} + public ValidationException (string msg, Exception inner) : base (msg, inner) {} + } + + public static List Crashers; public static int StresserIndex; static CrasherClass () { - Crashers = new List> (); + Crashers = new List (); // Basic functionality - Crashers.Add(new Tuple ("MerpCrashManaged", MerpCrashManaged)); + Crashers.Add(new Crasher ("MerpCrashManaged", MerpCrashManaged)); // Run this test for stress tests // // I've ran a burn-in with all of them of @@ -28,16 +48,17 @@ class C // Feel free to change by moving this line. StresserIndex = Crashers.Count - 1; - Crashers.Add(new Tuple ("MerpCrashMalloc", MerpCrashMalloc)); + Crashers.Add(new Crasher ("MerpCrashMalloc", MerpCrashMalloc)); + Crashers.Add(new Crasher ("MerpCrashFailFast", MerpCrashFailFast, ValidateFailFastMsg)); - Crashers.Add(new Tuple ("MerpCrashNullFp", MerpCrashNullFp)); - Crashers.Add(new Tuple ("MerpCrashExceptionHook", MerpCrashUnhandledExceptionHook)); + Crashers.Add(new Crasher ("MerpCrashNullFp", MerpCrashNullFp)); + Crashers.Add(new Crasher ("MerpCrashExceptionHook", MerpCrashUnhandledExceptionHook)); // Specific Edge Cases - Crashers.Add(new Tuple ("MerpCrashDladdr", MerpCrashDladdr)); - Crashers.Add(new Tuple ("MerpCrashSnprintf", MerpCrashSnprintf)); - Crashers.Add(new Tuple ("MerpCrashDomainUnload", MerpCrashDomainUnload)); - Crashers.Add(new Tuple ("MerpCrashUnbalancedGCSafe", MerpCrashUnbalancedGCSafe)); + Crashers.Add(new Crasher ("MerpCrashDladdr", MerpCrashDladdr)); + Crashers.Add(new Crasher ("MerpCrashSnprintf", MerpCrashSnprintf)); + Crashers.Add(new Crasher ("MerpCrashDomainUnload", MerpCrashDomainUnload)); + Crashers.Add(new Crasher ("MerpCrashUnbalancedGCSafe", MerpCrashUnbalancedGCSafe)); } public static void @@ -46,6 +67,21 @@ class C unsafe { Console.WriteLine("{0}", *(int*) -1); } } + const string failfastMsg = "abcd efgh"; + + public static void + MerpCrashFailFast () + { + Environment.FailFast (failfastMsg); + } + + public static void ValidateFailFastMsg (object json) + { + string s = jsonGetKeys (json, "payload", "failfast_message") as string; + if (s != failfastMsg) + throw new ValidationException (String.Format ("incorrect fail fast message (expected: {0}, got: {1})", failfastMsg, s)); + } + [DllImport("libtest")] public static extern void mono_test_MerpCrashSnprintf (); @@ -125,6 +161,20 @@ class C Console.WriteLine ("And now to crash inside the hook"); mono_test_MerpCrashUnhandledExceptionHook (); } + + + private static object jsonGetKey (object o, string key) => (o as Dictionary)[key]; + private static object jsonGetKeys (object o, params string[] keys) { + try { + foreach (var key in keys) { + o = jsonGetKey (o, key); + } + return o; + } catch (KeyNotFoundException e) { + throw new ValidationException (String.Format ("{0}, key not found, looking for key path [{1}]", e.ToString(), String.Join (", ", keys))); + } + } + } static string configDir = "./merp-crash-test/"; @@ -133,7 +183,7 @@ class C CrashWithMerp (int testNum) { SetupCrash (configDir); - CrasherClass.Crashers [Convert.ToInt32 (testNum)].Item2 (); + CrasherClass.Crashers [Convert.ToInt32 (testNum)].Action (); } public static string env = Environment.GetEnvironmentVariable ("MONO_PATH"); @@ -161,7 +211,7 @@ class C } public static void - TestValidate (string configDir, bool silent) + TestValidate (string configDir, bool silent, Action validator = null) { DumpLogCheck (); @@ -204,6 +254,10 @@ class C Console.WriteLine("Validating: {0}", crashFile); try { var obj = checker.DeserializeObject (crashFile); + if (validator is object) + validator (obj); + } catch (CrasherClass.ValidationException e) { + throw new Exception (String.Format ("Validation failed '{0}', json: {1}", e.Message, crashFile)); } catch (Exception e) { throw new Exception (String.Format ("Invalid json: {0}", crashFile)); } @@ -271,7 +325,7 @@ class C pi.Environment ["MONO_PATH"] = env; if (!silent) { - Console.WriteLine ("Running {0}", CrasherClass.Crashers [testNum].Item1); + Console.WriteLine ("Running {0}", CrasherClass.Crashers [testNum].Name); Console.WriteLine ("MONO_PATH={0} {1} {2} {3}", env, runtime, asm, testNum); } @@ -286,7 +340,7 @@ class C var process = Diag.Process.Start (pi); process.WaitForExit (); - TestValidate (configDir, silent); + TestValidate (configDir, silent, CrasherClass.Crashers [testNum].Validator); } finally { Cleanup (configDir); } @@ -323,7 +377,7 @@ class C if (failure_count > 0) { for (int i=0; i < CrasherClass.Crashers.Count; i++) { if (failures [i] != null) { - Console.WriteLine ("Crash reporter failed test {0}", CrasherClass.Crashers [i].Item1); + Console.WriteLine ("Crash reporter failed test {0}", CrasherClass.Crashers [i].Name); Console.WriteLine ("Cause: {0}\n{1}\n", failures [i].Message, failures [i].StackTrace); } } diff --git a/mono/utils/mono-state.c b/mono/utils/mono-state.c index f56239b5e0f..1ed7e5c6be5 100644 --- a/mono/utils/mono-state.c +++ b/mono/utils/mono-state.c @@ -1001,6 +1001,26 @@ mono_native_state_add_process_map (MonoStateWriter *writer) #endif +static void +mono_native_state_add_logged_message (MonoStateWriter *writer, const char *object_key, const char *msg) +{ + if (msg != NULL) { + assert_has_space (writer); + mono_state_writer_indent (writer); + mono_state_writer_object_key (writer, object_key); + + size_t length; + const char *pos; + if ((pos = strchr (msg, '\n')) != NULL) + length = (size_t)(pos - msg); + else + length = strlen (msg); + length = MIN (length, INT_MAX); + + mono_state_writer_printf(writer, "\"%.*s\",\n", (int)length, msg); + } +} + static void mono_native_state_add_prologue (MonoStateWriter *writer) { @@ -1019,21 +1039,11 @@ mono_native_state_add_prologue (MonoStateWriter *writer) mono_native_state_add_memory (writer); const char *assertion_msg = g_get_assertion_message (); - if (assertion_msg != NULL) { - assert_has_space (writer); - mono_state_writer_indent (writer); - mono_state_writer_object_key (writer, "assertion_message"); + mono_native_state_add_logged_message (writer, "assertion_message", assertion_msg); - size_t length; - const char *pos; - if ((pos = strchr (assertion_msg, '\n')) != NULL) - length = (size_t)(pos - assertion_msg); - else - length = strlen (assertion_msg); - length = MIN (length, INT_MAX); - - mono_state_writer_printf(writer, "\"%.*s\",\n", (int)length, assertion_msg); - } + const char *failfast_msg = mono_crash_get_failfast_msg (); + mono_native_state_add_logged_message (writer, "failfast_message", failfast_msg); + #ifndef MONO_PRIVATE_CRASHES mono_native_state_add_process_map (writer); @@ -1168,3 +1178,23 @@ mono_dump_complete (void) { return (mono_atomic_xchg_i32(&dump_status, 0) == 1); // return true if we completed the dump } + +static char *saved_failfast_msg; + +/** + * mono_crash_save_failfast_msg: + * \param msg the message to save. Takes ownership, caller shouldn't free + * + * \returns the previous message - caller is responsible for freeing. + */ +char* +mono_crash_save_failfast_msg (char *msg) +{ + return (char*) mono_atomic_xchg_ptr ((gpointer*)&saved_failfast_msg, (void*)msg); +} + +const char* +mono_crash_get_failfast_msg (void) +{ + return saved_failfast_msg; +} diff --git a/mono/utils/mono-state.h b/mono/utils/mono-state.h index e1f87f37b5b..994755a8e9b 100644 --- a/mono/utils/mono-state.h +++ b/mono/utils/mono-state.h @@ -117,6 +117,12 @@ mono_state_alloc_mem (MonoStateMem *mem, long tag, size_t size); void mono_state_free_mem (MonoStateMem *mem); +char* +mono_crash_save_failfast_msg (char *msg); + +const char* +mono_crash_get_failfast_msg (void); + #endif // DISABLE_CRASH_REPORTING // Dump context functions (enter/leave) -- cgit v1.2.3