From ab90246c67d5d12ec9fabae5e490a05db2c0be6b Mon Sep 17 00:00:00 2001 From: Ludovic Henry Date: Thu, 17 Aug 2017 19:11:23 +0200 Subject: [PATCH] [marshal] Rethrow in native-to-managed wrapper to keep exception stacktrace (#5384) * [marshal] Rethrow in native-to-managed wrapper to keep exception stacktrace Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=58782 * [interp] disable tests for 58782 as they rely on appdomains --- mono/metadata/marshal.c | 2 +- mono/metadata/object-internals.h | 1 + mono/metadata/object.c | 19 ++++++++ mono/metadata/object.h | 3 ++ mono/mini/mini-exceptions.c | 13 +++++- mono/mini/mini.h | 1 + mono/tests/Makefile.am | 33 +++++++++----- mono/tests/bug-58782-capture-and-throw.cs | 55 +++++++++++++++++++++++ mono/tests/bug-58782-plain-throw.cs | 51 +++++++++++++++++++++ mono/tests/libtest.c | 20 +++++++++ 10 files changed, 185 insertions(+), 13 deletions(-) create mode 100644 mono/tests/bug-58782-capture-and-throw.cs create mode 100644 mono/tests/bug-58782-plain-throw.cs diff --git a/mono/metadata/marshal.c b/mono/metadata/marshal.c index cc682a9eed2..63358bf3baf 100644 --- a/mono/metadata/marshal.c +++ b/mono/metadata/marshal.c @@ -12366,7 +12366,7 @@ ftnptr_eh_callback_default (guint32 gchandle) mono_gchandle_free (gchandle); - mono_raise_exception (exc); + mono_reraise_exception (exc); } /* diff --git a/mono/metadata/object-internals.h b/mono/metadata/object-internals.h index 62c8df717de..07aaee68300 100644 --- a/mono/metadata/object-internals.h +++ b/mono/metadata/object-internals.h @@ -643,6 +643,7 @@ typedef struct { gboolean (*mono_current_thread_has_handle_block_guard) (void); gboolean (*mono_above_abort_threshold) (void); void (*mono_clear_abort_threshold) (void); + void (*mono_reraise_exception) (MonoException *ex); } MonoRuntimeExceptionHandlingCallbacks; MONO_COLD void mono_set_pending_exception (MonoException *exc); diff --git a/mono/metadata/object.c b/mono/metadata/object.c index 6afa95de398..ad2a0e12587 100644 --- a/mono/metadata/object.c +++ b/mono/metadata/object.c @@ -7322,6 +7322,25 @@ mono_raise_exception (MonoException *ex) eh_callbacks.mono_raise_exception (ex); } +/** + * mono_raise_exception: + * \param ex exception object + * Signal the runtime that the exception \p ex has been raised in unmanaged code. + */ +void +mono_reraise_exception (MonoException *ex) +{ + MONO_REQ_GC_UNSAFE_MODE; + + /* + * NOTE: Do NOT annotate this function with G_GNUC_NORETURN, since + * that will cause gcc to omit the function epilog, causing problems when + * the JIT tries to walk the stack, since the return address on the stack + * will point into the next function in the executable, not this one. + */ + eh_callbacks.mono_reraise_exception (ex); +} + void mono_raise_exception_with_context (MonoException *ex, MonoContext *ctx) { diff --git a/mono/metadata/object.h b/mono/metadata/object.h index c439fda2e80..0454f86d454 100644 --- a/mono/metadata/object.h +++ b/mono/metadata/object.h @@ -243,6 +243,9 @@ mono_monitor_exit (MonoObject *obj); MONO_API void mono_raise_exception (MonoException *ex); +MONO_API void +mono_reraise_exception (MonoException *ex); + MONO_RT_EXTERNAL_ONLY MONO_API void mono_runtime_object_init (MonoObject *this_obj); diff --git a/mono/mini/mini-exceptions.c b/mono/mini/mini-exceptions.c index ad70319d744..9f5c87b6a13 100644 --- a/mono/mini/mini-exceptions.c +++ b/mono/mini/mini-exceptions.c @@ -251,10 +251,13 @@ mono_exceptions_init (void) cbs.mono_walk_stack_with_state = mono_walk_stack_with_state; - if (mono_llvm_only) + if (mono_llvm_only) { cbs.mono_raise_exception = mono_llvm_raise_exception; - else + cbs.mono_reraise_exception = mono_llvm_reraise_exception; + } else { cbs.mono_raise_exception = (void (*)(MonoException *))mono_get_throw_exception (); + cbs.mono_reraise_exception = (void (*)(MonoException *))mono_get_rethrow_exception (); + } cbs.mono_raise_exception_with_ctx = mono_raise_exception_with_ctx; cbs.mono_exception_walk_trace = mono_exception_walk_trace; cbs.mono_install_handler_block_guard = mono_install_handler_block_guard; @@ -3239,6 +3242,12 @@ mono_llvm_raise_exception (MonoException *e) mono_llvm_throw_exception ((MonoObject*)e); } +void +mono_llvm_reraise_exception (MonoException *e) +{ + mono_llvm_rethrow_exception ((MonoObject*)e); +} + void mono_llvm_throw_corlib_exception (guint32 ex_token_index) { diff --git a/mono/mini/mini.h b/mono/mini/mini.h index d15e9ec7164..9645a0de7b9 100644 --- a/mono/mini/mini.h +++ b/mono/mini/mini.h @@ -2926,6 +2926,7 @@ void mono_llvm_clear_exception (void); MonoObject *mono_llvm_load_exception (void); void mono_llvm_reset_exception (void); void mono_llvm_raise_exception (MonoException *e); +void mono_llvm_reraise_exception (MonoException *e); gint32 mono_llvm_match_exception (MonoJitInfo *jinfo, guint32 region_start, guint32 region_end, gpointer rgctx, MonoObject *this_obj); gboolean diff --git a/mono/tests/Makefile.am b/mono/tests/Makefile.am index aeec29b075f..fbbc0f7a282 100755 --- a/mono/tests/Makefile.am +++ b/mono/tests/Makefile.am @@ -512,7 +512,9 @@ TESTS_CS_SRC= \ bug-46661.cs \ w32message.cs \ runtime-invoke.gen.cs \ - imt_big_iface_test.cs + imt_big_iface_test.cs \ + bug-58782-plain-throw.cs \ + bug-58782-capture-and-throw.cs if AMD64 TESTS_CS_SRC += async-exc-compilation.cs finally_guard.cs finally_block_ending_in_dead_bb.cs @@ -648,9 +650,15 @@ TESTS_GSHARED_SRC = \ bug-1147.cs \ generic-type-builder.2.cs +PLATFORM_DISABLED_TESTS= + +if HOST_WIN32 +PLATFORM_DISABLED_TESTS += bug-58782-plain-throw.exe bug-58782-capture-and-throw.exe +endif + if AMD64 # #651684 -PLATFORM_DISABLED_TESTS = finally_guard.exe +PLATFORM_DISABLED_TESTS += finally_guard.exe if HOST_WIN32 PLATFORM_DISABLED_TESTS += w32message.exe @@ -661,7 +669,7 @@ endif if X86 if HOST_WIN32 -PLATFORM_DISABLED_TESTS=async-exc-compilation.exe finally_guard.exe finally_block_ending_in_dead_bb.exe \ +PLATFORM_DISABLED_TESTS += async-exc-compilation.exe finally_guard.exe finally_block_ending_in_dead_bb.exe \ bug-18026.exe monitor.exe threadpool-exceptions5.exe process-unref-race.exe w32message.exe \ unhandled-exception-1.exe unhandled-exception-2.exe unhandled-exception-3.exe unhandled-exception-4.exe \ unhandled-exception-5.exe unhandled-exception-6.exe unhandled-exception-7.exe unhandled-exception-8.exe @@ -671,12 +679,12 @@ endif if POWERPC # bug #71274 -PLATFORM_DISABLED_TESTS=finalizer-abort.exe finalizer-exception.exe finalizer-exit.exe +PLATFORM_DISABLED_TESTS += finalizer-abort.exe finalizer-exception.exe finalizer-exit.exe endif if POWERPC64 # FIXME: These tests hang/fail for unknown reasons -PLATFORM_DISABLED_TESTS=monitor.exe threadpool-exceptions5.exe appdomain-thread-abort.exe appdomain-unload.exe \ +PLATFORM_DISABLED_TESTS += monitor.exe threadpool-exceptions5.exe appdomain-thread-abort.exe appdomain-unload.exe \ pinvoke2.exe pinvoke3.exe pinvoke11.exe threadpool-exceptions7.exe winx64structs.exe bug-10127.exe pinvoke_ppcc.exe \ pinvoke_ppcs.exe pinvoke_ppci.exe pinvoke_ppcf.exe pinvoke_ppcd.exe abort-cctor.exe load-exceptions.exe \ sgen-domain-unload-2.exe sgen-weakref-stress.exe sgen-cementing-stress.exe sgen-new-threads-dont-join-stw.exe \ @@ -684,7 +692,7 @@ PLATFORM_DISABLED_TESTS=monitor.exe threadpool-exceptions5.exe appdomain-thread- endif if ARM -PLATFORM_DISABLED_TESTS=filter-stack.exe +PLATFORM_DISABLED_TESTS += filter-stack.exe INTERP_DISABLED_TESTS_PLATFORM=finalizer-exception.exe main-returns-abort-resetabort.exe block_guard_restore_aligment_on_exit.exe \ delegate-exit.exe delegate-exit.exe delegate-delegate-exit.exe delegate-async-exit.exe delegate3.exe delegate1.exe endif @@ -695,11 +703,11 @@ endif if MIPS # monitor.exe is racy -PLATFORM_DISABLED_TESTS=filter-stack.exe monitor.exe +PLATFORM_DISABLED_TESTS += filter-stack.exe monitor.exe endif if S390X -PLATFORM_DISABLED_TESTS=dynamic-method-resurrection.exe +PLATFORM_DISABLED_TESTS += dynamic-method-resurrection.exe #PLATFORM_DISABLED_TESTS=dynamic-method-resurrection.exe exception17.exe PLATFORM_DISABLED_TESTS += \ @@ -758,7 +766,9 @@ PROFILE_DISABLED_TESTS += \ marshal8.exe \ pinvoke-2.2.exe \ pinvoke3.exe \ - thunks.exe + thunks.exe \ + bug-58782-plain-throw.exe \ + bug-58782-capture-and-throw.exe # Tests which load assemblies which are not # in the testing_aot_full profile @@ -959,6 +969,8 @@ INTERP_DISABLED_TESTS = \ bug-46661.exe \ bug-47295.exe \ bug-48015.exe \ + bug-58782-plain-throw.exe \ + bug-58782-capture-and-throw.exe \ bug-544446.exe \ bug-685908.exe \ bug-80307.exe \ @@ -1693,7 +1705,8 @@ test-unhandled-exception: unhandled-exception-test-runner.2.exe safehandle.2.exe winx64structs.exe thunks.exe pinvoke3.exe pinvoke2.exe pinvoke-2.2.exe pinvoke17.exe pinvoke13.exe \ pinvoke11.exe pinvoke_ppcs.exe pinvoke_ppci.exe pinvoke_ppcf.exe pinvoke_ppcd.exe pinvoke_ppcc.exe pinvoke.exe \ - marshalbool.exe marshal9.exe marshal5.exe marshal.exe handleref.exe cominterop.exe bug-Xamarin-5278.exe: libtest.la + marshalbool.exe marshal9.exe marshal5.exe marshal.exe handleref.exe cominterop.exe bug-Xamarin-5278.exe \ + bug-58782-plain-throw.exe bug-58782-capture-and-throw.exe: libtest.la event-get.2.exe$(PLATFORM_AOT_SUFFIX): event-il.exe$(PLATFORM_AOT_SUFFIX) event-get.2.exe: event-il.exe diff --git a/mono/tests/bug-58782-capture-and-throw.cs b/mono/tests/bug-58782-capture-and-throw.cs new file mode 100644 index 00000000000..9a917eea49d --- /dev/null +++ b/mono/tests/bug-58782-capture-and-throw.cs @@ -0,0 +1,55 @@ +using System; +using System.Runtime.InteropServices; + +class Driver +{ + [DllImport ("libtest")] + static extern void mono_test_native_to_managed_exception_rethrow (Action action); + + [DllImport ("libc")] + static extern void _exit (int exitCode); + + static int Main (string[] args) + { + AppDomain.CurrentDomain.UnhandledException += (sender, exception_args) => + { + CustomException exc = exception_args.ExceptionObject as CustomException; + if (exc == null) { + Console.WriteLine ($"FAILED - Unknown exception: {exception_args.ExceptionObject}"); + _exit (1); + } + + Console.WriteLine (exc.StackTrace); + if (string.IsNullOrEmpty (exc.StackTrace)) { + Console.WriteLine ("FAILED - StackTrace is null for unhandled exception."); + _exit (2); + } else { + Console.WriteLine ("SUCCESS - StackTrace is not null for unhandled exception."); + _exit (0); + } + }; + + mono_test_native_to_managed_exception_rethrow (CaptureAndThrow); + Console.WriteLine ("Should have exited in the UnhandledException event handler."); + return 2; + } + + static void CaptureAndThrow () + { + try { + Throw (); + } catch (Exception e) { + System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture (e).Throw (); + } + } + + static void Throw () + { + throw new CustomException ("C"); + } + + class CustomException : Exception + { + public CustomException(string s) : base(s) {} + } +} \ No newline at end of file diff --git a/mono/tests/bug-58782-plain-throw.cs b/mono/tests/bug-58782-plain-throw.cs new file mode 100644 index 00000000000..72ce01cb16e --- /dev/null +++ b/mono/tests/bug-58782-plain-throw.cs @@ -0,0 +1,51 @@ +using System; +using System.Runtime.InteropServices; + +class Driver +{ + [DllImport ("libtest")] + static extern void mono_test_native_to_managed_exception_rethrow (Action action); + + [DllImport ("libc")] + static extern void _exit (int exitCode); + + static int Main (string[] args) + { + AppDomain.CurrentDomain.UnhandledException += (sender, exception_args) => + { + CustomException exc = exception_args.ExceptionObject as CustomException; + if (exc == null) { + Console.WriteLine ($"FAILED - Unknown exception: {exception_args.ExceptionObject}"); + _exit (1); + } + + Console.WriteLine (exc.StackTrace); + if (string.IsNullOrEmpty (exc.StackTrace)) { + Console.WriteLine ("FAILED - StackTrace is null for unhandled exception."); + _exit (2); + } else { + Console.WriteLine ("SUCCESS - StackTrace is not null for unhandled exception."); + _exit (0); + } + }; + + mono_test_native_to_managed_exception_rethrow (PlainThrow); + Console.WriteLine ("Should have exited in the UnhandledException event handler."); + return 3; + } + + static void PlainThrow () + { + Throw (); + } + + static void Throw () + { + throw new CustomException ("C"); + } + + class CustomException : Exception + { + public CustomException(string s) : base(s) {} + } +} \ No newline at end of file diff --git a/mono/tests/libtest.c b/mono/tests/libtest.c index 9950476e4ca..8390e86cb57 100644 --- a/mono/tests/libtest.c +++ b/mono/tests/libtest.c @@ -7482,3 +7482,23 @@ mono_test_marshal_pointer_array (int *arr[]) } return 0; } + +#ifndef WIN32 + +typedef void (*NativeToManagedExceptionRethrowFunc) (); + +void *mono_test_native_to_managed_exception_rethrow_thread (void *arg) +{ + NativeToManagedExceptionRethrowFunc func = (NativeToManagedExceptionRethrowFunc) arg; + func (); + return NULL; +} + +LIBTEST_API void STDCALL +mono_test_native_to_managed_exception_rethrow (NativeToManagedExceptionRethrowFunc func) +{ + pthread_t t; + pthread_create (&t, NULL, mono_test_native_to_managed_exception_rethrow_thread, func); + pthread_join (t, NULL); +} +#endif -- 2.25.1