From 56b3c007f428d93b7f230d58744393ad69e4ca63 Mon Sep 17 00:00:00 2001 From: vkargov Date: Fri, 6 Oct 2017 13:07:33 -0700 Subject: [PATCH] [runtime] Don't insta-fail when a faulty COM type is encountered. (#5616) * [jit] Don't insta-fail on faulty cominterop invocations. * [runtime] New test for bug 8477. * [jit] Implement mono_error_get_exception_name() and mono_mb_emit_exception_for_error() * [jit] Throw an exception when a static method on a ComImport class is called. --- mono/metadata/cominterop.c | 51 +++++++++++++++++++++++++------ mono/metadata/method-builder.c | 14 +++++++++ mono/metadata/method-builder.h | 3 ++ mono/tests/bug-8477.cs | 42 +++++++++++++++++++++++++ mono/utils/mono-error-internals.h | 2 ++ mono/utils/mono-error.c | 11 +++++++ 6 files changed, 114 insertions(+), 9 deletions(-) create mode 100644 mono/tests/bug-8477.cs diff --git a/mono/metadata/cominterop.c b/mono/metadata/cominterop.c index b00ab355d6e..c4bc5b03781 100644 --- a/mono/metadata/cominterop.c +++ b/mono/metadata/cominterop.c @@ -378,30 +378,39 @@ cominterop_get_method_interface (MonoMethod* method) } } - if (!ic) - g_assert (ic); - g_assert (MONO_CLASS_IS_INTERFACE (ic)); - return ic; } +static void +mono_cominterop_get_interface_missing_error (MonoError* error, MonoMethod* method) +{ + mono_error_set_invalid_operation (error, "Method '%s' in ComImport class '%s' must implement an interface method.", method->name, method->klass->name); +} + /** * cominterop_get_com_slot_for_method: * @method: a method + * @error: set on error * * Returns: the method's slot in the COM interface vtable */ static int -cominterop_get_com_slot_for_method (MonoMethod* method) +cominterop_get_com_slot_for_method (MonoMethod* method, MonoError* error) { guint32 slot = method->slot; MonoClass *ic = method->klass; + error_init (error); + /* if method is on a class, we need to look up interface method exists on */ if (!MONO_CLASS_IS_INTERFACE(ic)) { int offset = 0; int i = 0; ic = cominterop_get_method_interface (method); + if (!ic || !MONO_CLASS_IS_INTERFACE (ic)) { + mono_cominterop_get_interface_missing_error (error, method); + return -1; + } offset = mono_class_interface_offset (method->klass, ic); g_assert(offset >= 0); int mcount = mono_class_get_method_count (ic); @@ -643,11 +652,20 @@ void mono_mb_emit_cominterop_get_function_pointer (MonoMethodBuilder *mb, MonoMethod *method) { #ifndef DISABLE_JIT + int slot; + MonoError error; // get function pointer from 1st arg, the COM interface pointer mono_mb_emit_ldarg (mb, 0); - mono_mb_emit_icon (mb, cominterop_get_com_slot_for_method (method)); - mono_mb_emit_icall (mb, cominterop_get_function_pointer); - /* Leaves the function pointer on top of the stack */ + slot = cominterop_get_com_slot_for_method (method, &error); + if (is_ok (&error)) { + mono_mb_emit_icon (mb, slot); + mono_mb_emit_icall (mb, cominterop_get_function_pointer); + /* Leaves the function pointer on top of the stack */ + } + else { + mono_mb_emit_exception_for_error (mb, &error); + } + mono_error_cleanup (&error); #endif } @@ -995,6 +1013,18 @@ mono_cominterop_get_native_wrapper (MonoMethod *method) mono_mb_emit_managed_call (mb, ctor, NULL); mono_mb_emit_byte (mb, CEE_RET); } + else if (method->flags & METHOD_ATTRIBUTE_STATIC) { + /* + * The method's class must implement an interface. + * However, no interfaces are allowed to have static methods. + * Thus, calling it should invariably lead to an exception. + */ + MonoError error; + error_init (&error); + mono_cominterop_get_interface_missing_error (&error, method); + mono_mb_emit_exception_for_error (mb, &error); + mono_error_cleanup (&error); + } else { static MonoMethod * ThrowExceptionForHR = NULL; MonoMethod *adjusted_method; @@ -1706,7 +1736,10 @@ guint32 ves_icall_System_Runtime_InteropServices_Marshal_GetComSlotForMethodInfoInternal (MonoReflectionMethod *m) { #ifndef DISABLE_COM - return cominterop_get_com_slot_for_method (m->method); + MonoError error; + int slot = cominterop_get_com_slot_for_method (m->method, &error); + mono_error_assert_ok (&error); + return slot; #else g_assert_not_reached (); #endif diff --git a/mono/metadata/method-builder.c b/mono/metadata/method-builder.c index a02312ab472..ccff8c91a0c 100644 --- a/mono/metadata/method-builder.c +++ b/mono/metadata/method-builder.c @@ -653,6 +653,20 @@ mono_mb_emit_exception (MonoMethodBuilder *mb, const char *exc_name, const char mono_mb_emit_exception_full (mb, "System", exc_name, msg); } +/** + * mono_mb_emit_exception_for_error: + */ +void +mono_mb_emit_exception_for_error (MonoMethodBuilder *mb, MonoError *error) +{ + /* + * If at some point there is need to support other types of errors, + * the behaviour should conform with mono_error_prepare_exception(). + */ + g_assert (mono_error_get_error_code (error) == MONO_ERROR_GENERIC && "Unsupported error code."); + mono_mb_emit_exception_full (mb, "System", mono_error_get_exception_name (error), mono_error_get_message (error)); +} + /** * mono_mb_emit_add_to_local: */ diff --git a/mono/metadata/method-builder.h b/mono/metadata/method-builder.h index 26a53e1c34d..92d71dd4ead 100644 --- a/mono/metadata/method-builder.h +++ b/mono/metadata/method-builder.h @@ -111,6 +111,9 @@ mono_mb_emit_exception (MonoMethodBuilder *mb, const char *exc_name, const char void mono_mb_emit_exception_full (MonoMethodBuilder *mb, const char *exc_nspace, const char *exc_name, const char *msg); +void +mono_mb_emit_exception_for_error (MonoMethodBuilder *mb, MonoError *error); + void mono_mb_emit_icon (MonoMethodBuilder *mb, gint32 value); diff --git a/mono/tests/bug-8477.cs b/mono/tests/bug-8477.cs new file mode 100644 index 00000000000..b806a6ee13b --- /dev/null +++ b/mono/tests/bug-8477.cs @@ -0,0 +1,42 @@ +// This test is meant to make sure Mono doesn't fail when invalid COM invocations are present but never reached. +// See https://bugzilla.xamarin.com/show_bug.cgi?id=8477 for details. + +using System; +using System.Runtime.InteropServices; +using System.Runtime.CompilerServices; + +[ComImport, Guid("06A82D35-8946-4E2E-AE71-DADDE8341F5D")] +class COMponent +{ + [MethodImpl(MethodImplOptions.InternalCall)] public static extern void InCOMplete1 (); + [MethodImpl(MethodImplOptions.InternalCall)] public extern void InCOMplete2 (); +} + +class Test +{ + static void COMmunicate (COMponent c) + { + if (c != null) + c.InCOMplete2 (); + } + + static int Main() + { + // Check #1: An invocation of a ComImport class method w/o a corresponding interface method must lead to an exception. + try + { + COMponent.InCOMplete1(); + // No exception has been thrown, something is wrong. + return 1; + } + catch (InvalidOperationException) + { + // An exception has been thrown and caught correctly. + } + + // Check #2: Same as #1, but the method is not executed (i.e. it's located in a "cold" basic block). No exception should be thrown. + COMmunicate (null); + + return 0; + } +} diff --git a/mono/utils/mono-error-internals.h b/mono/utils/mono-error-internals.h index e3402353ff5..d181c3b88b1 100644 --- a/mono/utils/mono-error-internals.h +++ b/mono/utils/mono-error-internals.h @@ -153,5 +153,7 @@ mono_error_box (const MonoError *error, MonoImage *image); gboolean mono_error_set_from_boxed (MonoError *error, const MonoErrorBoxed *from); +const char* +mono_error_get_exception_name (MonoError *oerror); #endif diff --git a/mono/utils/mono-error.c b/mono/utils/mono-error.c index e193291a569..27f2b62fb24 100644 --- a/mono/utils/mono-error.c +++ b/mono/utils/mono-error.c @@ -176,6 +176,17 @@ mono_error_get_error_code (MonoError *error) return error->error_code; } +const char* +mono_error_get_exception_name (MonoError *oerror) +{ + MonoErrorInternal *error = (MonoErrorInternal*)oerror; + + if (error->error_code == MONO_ERROR_NONE) + return NULL; + + return error->exception_name; +} + /*Return a pointer to the internal error message, might be NULL. Caller should not release it.*/ const char* -- 2.25.1