Merge pull request #4380 from alexanderkyte/conflicting_attrs

[runtime] Fail gracefully when method has conflicting attributes

4 files changed, 19 insertions, 9 deletions

diff --git a/mono/metadata/marshal.c b/mono/metadata/marshal.c
index 6020bf50eb4..76f38e399d8 100644
--- a/mono/metadata/marshal.c
+++ b/mono/metadata/marshal.c
@@ -426,7 +426,9 @@ mono_delegate_to_ftnptr (MonoDelegate *delegate)
 		target_handle = mono_gchandle_new_weakref (delegate->target, FALSE);
 	}
 
-	wrapper = mono_marshal_get_managed_wrapper (method, klass, target_handle);
+	wrapper = mono_marshal_get_managed_wrapper (method, klass, target_handle, &error);
+	if (!is_ok (&error))
+		goto fail;
 
 	delegate->delegate_trampoline = mono_compile_method_checked (wrapper, &error);
 	if (!is_ok (&error))
@@ -8659,9 +8661,8 @@ mono_marshal_set_callconv_from_modopt (MonoMethod *method, MonoMethodSignature *
  * If target_handle==0, the wrapper info will be a WrapperInfo structure.
  */
 MonoMethod *
-mono_marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass, uint32_t target_handle)
+mono_marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass, uint32_t target_handle, MonoError *error)
 {
-	MonoError error;
 	MonoMethodSignature *sig, *csig, *invoke_sig;
 	MonoMethodBuilder *mb;
 	MonoMethod *res, *invoke;
@@ -8672,7 +8673,12 @@ mono_marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass,
 	EmitMarshalContext m;
 
 	g_assert (method != NULL);
-	g_assert (!(method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL));
+	mono_error_init (error);
+
+	if (method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL) {
+		mono_error_set_invalid_program (error, "Failed because method (%s) marked PInvokeCallback (managed method) and extern (unmanaged) simultaneously.", mono_method_full_name (method, TRUE));
+		return NULL;
+	}
 
 	/* 
 	 * FIXME: Should cache the method+delegate type pair, since the same method
@@ -8726,8 +8732,8 @@ mono_marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass,
 		 * contents of the attribute without constructing it, as that might not be
 		 * possible when running in cross-compiling mode.
 		 */
-		cinfo = mono_custom_attrs_from_class_checked (delegate_klass, &error);
-		mono_error_assert_ok (&error);
+		cinfo = mono_custom_attrs_from_class_checked (delegate_klass, error);
+		mono_error_assert_ok (error);
 		attr = NULL;
 		if (cinfo) {
 			for (i = 0; i < cinfo->num_attrs; ++i) {
diff --git a/mono/metadata/marshal.h b/mono/metadata/marshal.h
index e00661b1b59..4b1c185f876 100644
--- a/mono/metadata/marshal.h
+++ b/mono/metadata/marshal.h
@@ -330,7 +330,7 @@ MonoMethodSignature*
 mono_marshal_get_string_ctor_signature (MonoMethod *method);
 
 MonoMethod *
-mono_marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass, uint32_t this_loc);
+mono_marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass, uint32_t this_loc, MonoError *exernal_error);
 
 gpointer
 mono_marshal_get_vtfixup_ftnptr (MonoImage *image, guint32 token, guint16 type);
diff --git a/mono/mini/aot-compiler.c b/mono/mini/aot-compiler.c
index 6e91638d881..6d4f50a4238 100644
--- a/mono/mini/aot-compiler.c
+++ b/mono/mini/aot-compiler.c
@@ -4330,7 +4330,9 @@ add_wrappers (MonoAotCompile *acfg)
 					named += slen;
 				}
 
-				wrapper = mono_marshal_get_managed_wrapper (method, klass, 0);
+				wrapper = mono_marshal_get_managed_wrapper (method, klass, 0, &error);
+				mono_error_assert_ok (&error);
+
 				add_method (acfg, wrapper);
 				if (export_name)
 					g_hash_table_insert (acfg->export_names, wrapper, export_name);
diff --git a/mono/mini/aot-runtime.c b/mono/mini/aot-runtime.c
index ed2b485a626..85990d353ab 100644
--- a/mono/mini/aot-runtime.c
+++ b/mono/mini/aot-runtime.c
@@ -1214,7 +1214,9 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod
 			klass = decode_klass_ref (module, p, &p, error);
 			if (!klass)
 				return FALSE;
-			ref->method = mono_marshal_get_managed_wrapper (m, klass, 0);
+			ref->method = mono_marshal_get_managed_wrapper (m, klass, 0, error);
+			if (!mono_error_ok (error))
+				return FALSE;
 			break;
 		}
 		default: