[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.

diff --git a/mono/metadata/cominterop.c b/mono/metadata/cominterop.c
index b00ab355d6ecd3c9736d316c0ba167fb3890a521..c4bc5b037810292912e73923e02bd50288feb305 100644 (file)
--- 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 a02312ab4723514308d97c0e0c40e7065a9dc733..ccff8c91a0c1573cf2f0d38174f62fb5ae5a252b 100644 (file)
--- 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 26a53e1c34d46baad794158329fdc9a1b897083f..92d71dd4eadb1ae33dd0eeb67cbc081d726492bf 100644 (file)
--- 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 (file)
index 0000000..b806a6e
--- /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 e3402353ff55b7f7b536c946ddd070bd64078cee..d181c3b88b1f8f39d1992eb4cfdf3cb47b7e0372 100644 (file)
--- 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 e193291a569cbb39a78448fd5a90ceda29dcffbd..27f2b62fb248ff784d2f6dbc9b350f98a7613d07 100644 (file)
--- 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*