Simplify MonoError and managed Exception creation (mono#6123)

MonoError is simplified for MME by having the error message eagerly created, which simplifies things A LOT.
Managed Exception creation is simplified by only setting Message, as it's done by other runtimes.

* [mini] Fix mini_emit_memory_copy_internal with ref types and remove workaround from mono_method_to_ir

* [mini] Remove unused argument and rename function to better reflect its use.

* [utils] Fix swapped member and signature fields in error message.

* [runtime] Simplify MME by only setting the message field since ClassName/MemberName don't need to be set.

* [runtime] Add mono_error_set_method_missing to exceptions.c and collapse setting MME to a single function.

This implements the cleanup discussed in the PR.

* [runtime] Cleanup MonoError for BadImageFormatException.

Move mono_error_set_bad_image & mono_error_set_bad_image_name to exception.c.
Rename mono_error_set_bad_image_name to mono_error_set_bad_image_by_name.

Introduce mono_corlib_exception_new_with_args to be used when creating exception objects.
It's easier to consume than mono_exception_from_name_two_strings_checked as it takes string arguments as char*.

Fix usage of mono_error_set_bad_image_by_name to never dup the supplied image name as the callee does it.
Fix some empty error messages.

Finally, reduce the cruft in mono_error_prepare_exception by using the new exception creation function from above.

* [runtime] Cleanup MonoError of FileNotFoundException.

Move it to use the generic first_argument and make it a simple error - IE, get_message doesn't format anything.

* [runtime] Rework MonoError for MissingFieldException.

* Fix build issues

* [runtime] Ensure memory is freed using new MonoError set functions.

Author: kumpera

mcs/class/corlib/LinkerDescriptor/mscorlib.xml

@@ -248,10 +248,6 @@
 		<type fullname="System.MissingMethodException">
 			<!-- mini.c (mono_jit_compiler_method_inner) mono_exception_from_name_msg -->
 			<method signature="System.Void .ctor(System.String)" />
-			<!-- exception.c (mono_get_exception_type_load) mono_exception_from_name_two_strings -->
-			<method signature="System.Void .ctor(System.String,System.String)" />
-			<!-- exception.c  mono_exception_from_name_four_strings -->
-			<method signature="System.Void .ctor(System.String,System.String,System.String,System.String)" />
 		</type>
 
 		<!-- threadpool.c: mono_thread_pool_init (assert) -->

mcs/class/referencesource/mscorlib/system/missingmethodexception.cs

@@ -48,19 +48,10 @@ public override String Message
                 if (ClassName == null) {
                     return base.Message;
                 } else {
-#if MONO
-                    string res = ClassName + "." + MemberName;
-                    if (!string.IsNullOrEmpty(signature))
-                        res = string.Format (CultureInfo.InvariantCulture, signature, res);
-                    if (!string.IsNullOrEmpty(_message))
-                        res += " Due to: " + _message;
-                    return res;
-#else
                     // do any desired fixups to classname here.
                     return Environment.GetResourceString("MissingMethod_Name",
                                                                        ClassName + "." + MemberName +
                                                                        (Signature != null ? " " + FormatSignature(Signature) : ""));
-#endif
                 }
             }
         }
@@ -82,17 +73,5 @@ public MissingMethodException(String className, String methodName)
         // If ClassName != null, Message will construct on the fly using it
         // and the other variables. This allows customization of the
         // format depending on the language environment.
-#if MONO
-        // Called from the EE
-        private MissingMethodException(String className, String methodName, String signature, String message) : base (message)
-        {
-            ClassName   = className;
-            MemberName  = methodName;
-            this.signature = signature;
-        }
-
-		[NonSerialized]
-        string signature;
-#endif
     }
 }

mono/metadata/appdomain.c

@@ -1230,7 +1230,7 @@ mono_try_assembly_resolve_handle (MonoDomain *domain, MonoStringHandle fname, Mo
 
 	if (ret && !refonly && ret->ref_only) {
 		/* .NET Framework throws System.IO.FileNotFoundException in this case */
-		mono_error_set_file_not_found (error, "AssemblyResolveEvent handlers cannot return Assemblies loaded for reflection only");
+		mono_error_set_file_not_found (error, NULL, "AssemblyResolveEvent handlers cannot return Assemblies loaded for reflection only");
 		ret = NULL;
 		goto leave;
 	}
@@ -2172,9 +2172,9 @@ ves_icall_System_Reflection_Assembly_LoadFrom (MonoStringHandle fname, MonoBoole
 
 	if (!ass) {
 		if (status == MONO_IMAGE_IMAGE_INVALID)
-			mono_error_set_bad_image_name (error, g_strdup (name), "");
+			mono_error_set_bad_image_by_name (error, name, "Invalid Image");
 		else
-			mono_error_set_assembly_load (error, g_strdup (name), "%s", "");
+			mono_error_set_file_not_found (error, name, "Invalid Image");
 		goto leave;
 	}
 
@@ -2214,7 +2214,7 @@ ves_icall_System_AppDomain_LoadAssemblyRaw (MonoAppDomainHandle ad,
 	MonoImage *image = mono_image_open_from_data_full (assembly_data, raw_assembly_len, FALSE, NULL, refonly);
 
 	if (!image) {
-		mono_error_set_bad_image_name (error, g_strdup (""), "%s", "");
+		mono_error_set_bad_image_by_name (error, "In memory assembly", "0x%p", raw_data);
 		return refass;
 	}
 
@@ -2231,7 +2231,7 @@ ves_icall_System_AppDomain_LoadAssemblyRaw (MonoAppDomainHandle ad,
 
 	if (!ass) {
 		mono_image_close (image);
-		mono_error_set_bad_image_name (error, g_strdup (""), "%s", "");
+		mono_error_set_bad_image_by_name (error, "In Memory assembly", "0x%p", assembly_data);
 		return refass; 
 	}
 

mono/metadata/class-init.c

@@ -12,6 +12,7 @@
 #include <mono/metadata/class-internals.h>
 #include <mono/metadata/custom-attrs-internals.h>
 #include <mono/metadata/debug-helpers.h>
+#include <mono/metadata/exception-internals.h>
 #include <mono/metadata/gc-internals.h>
 #include <mono/metadata/object-internals.h>
 #include <mono/metadata/profiler-private.h>

mono/metadata/class.c

@@ -22,6 +22,7 @@
 #include <mono/metadata/image-internals.h>
 #include <mono/metadata/assembly.h>
 #include <mono/metadata/assembly-internals.h>
+#include <mono/metadata/exception-internals.h>
 #include <mono/metadata/metadata.h>
 #include <mono/metadata/metadata-internals.h>
 #include <mono/metadata/profiler-private.h>
@@ -191,7 +192,8 @@ mono_class_from_typeref_checked (MonoImage *image, guint32 type_token, MonoError
 
 		mono_assembly_get_assemblyref (image, idx - 1, &aname);
 		human_name = mono_stringify_assembly_name (&aname);
-		mono_error_set_assembly_load_simple (error, human_name, image->assembly ? image->assembly->ref_only : FALSE);
+		mono_error_set_simple_file_not_found (error, human_name, image->assembly ? image->assembly->ref_only : FALSE);
+		g_free (human_name);
 		return NULL;
 	}
 

mono/metadata/debug-helpers.c

@@ -287,49 +287,6 @@ mono_signature_full_name (MonoMethodSignature *sig)
 	return result;
 }
 
-/*
- * Returns a string ready to be consumed by managed code when formating a string to include class + method name.
- * IE, say you have void Foo:Bar(int). It will return "void {0}(int)".
- * The reason for this is that managed exception constructors for missing members require a both class and member names to be provided independently of the signature.
- */
-char*
-mono_signature_get_managed_fmt_string (MonoMethodSignature *sig)
-{
-	int i;
-	char *result;
-	GString *res;
-
-	if (!sig)
-		return g_strdup ("<invalid signature>");
-
-	res = g_string_new ("");
-
-	mono_type_get_desc (res, sig->ret, TRUE);
-
-	g_string_append (res, " {0}");
-
-	if (sig->generic_param_count) {
-		g_string_append_c (res, '<');
-		for (i = 0; i < sig->generic_param_count; ++i) {
-			if (i > 0)
-				g_string_append (res, ",");
-			g_string_append_printf (res, "!%d", i);
-		}
-		g_string_append_c (res, '>');
-	}
-
-	g_string_append_c (res, '(');
-	for (i = 0; i < sig->param_count; ++i) {
-		if (i > 0)
-			g_string_append_c (res, ',');
-		mono_type_get_desc (res, sig->params [i], TRUE);
-	}
-	g_string_append_c (res, ')');
-	result = res->str;
-	g_string_free (res, FALSE);
-	return result;
-}
-
 void
 mono_ginst_get_desc (GString *str, MonoGenericInst *ginst)
 {

mono/metadata/exception-internals.h

@@ -30,15 +30,30 @@ mono_exception_from_token_two_strings_checked (MonoImage *image, uint32_t token,
 					       MonoString *a1, MonoString *a2,
 					       MonoError *error);
 
-MonoException *
-mono_exception_from_name_four_strings_checked (MonoImage *image, const char *name_space,
-				      const char *name, MonoString *a1, MonoString *a2, MonoString *a3, MonoString *a4,
-				      MonoError *error);
-
-
 typedef int (*MonoGetSeqPointFunc) (MonoDomain *domain, MonoMethod *method, gint32 native_offset);
 
 void
 mono_install_get_seq_point (MonoGetSeqPointFunc func);
 
+void
+mono_error_set_method_missing (MonoError *error, MonoClass *klass, const char *method_name, MonoMethodSignature *sig, const char *reason, ...) MONO_ATTR_FORMAT_PRINTF(5,6);
+
+void
+mono_error_set_field_missing (MonoError *oerror, MonoClass *klass, const char *field_name, MonoType *sig, const char *reason, ...) MONO_ATTR_FORMAT_PRINTF(5,6);
+
+void
+mono_error_set_bad_image (MonoError *error, MonoImage *image, const char *msg_format, ...) MONO_ATTR_FORMAT_PRINTF(3,4);
+
+void
+mono_error_set_bad_image_by_name (MonoError *error, const char *image_name, const char *msg_format, ...) MONO_ATTR_FORMAT_PRINTF(3,4);
+
+void
+mono_error_set_file_not_found (MonoError *oerror, const char *file_name, const char *msg_format, ...) MONO_ATTR_FORMAT_PRINTF(3,4);
+
+void
+mono_error_set_simple_file_not_found (MonoError *oerror, const char *assembly_name, gboolean refection_only);
+
+MonoException *
+mono_corlib_exception_new_with_args (const char *name_space, const char *name, const char *arg_0, const char *arg_1, MonoError *error);
+
 #endif