[metadata] Fix dynamic delegate (#12762)

* [metadata] Decouple dynamic method signature from its delegate invoke wrapper

The invoke wrapper should not be referencing the signature because it will be freed once with the dynamic method. The wrapper can outlive the dynamic method, it is cached based on the signature.

Fixes crash on old IronJS benchmark with interpreter (which uses a lot of delegate wrappers, compared to jit)

* [interp] Add debug method for printing transformed code

mono/metadata/marshal.c

@@ -2184,6 +2184,13 @@ mono_marshal_get_delegate_invoke_internal (MonoMethod *method, gboolean callvirt
 		subtype = WRAPPER_SUBTYPE_DELEGATE_INVOKE_BOUND;
 		g_assert (!callvirt);
 		invoke_sig = mono_method_signature_internal (target_method);
+		/*
+		 * The wrapper has a different lifetime from the method to be invoked.
+		 * If the method is dynamic we don't want to be using its signature
+		 * in the wrapper since it could get freed early.
+		 */
+		if (method_is_dynamic (target_method))
+			invoke_sig = mono_metadata_signature_dup_full (get_method_image (target_method), invoke_sig);
 	}
 
 	/*

mono/mini/interp/interp-internals.h

@@ -166,6 +166,9 @@ mono_interp_transform_init (void);
 InterpMethod *
 mono_interp_get_imethod (MonoDomain *domain, MonoMethod *method, MonoError *error);
 
+void
+mono_interp_print_code (InterpMethod *imethod);
+
 static inline int
 mint_type(MonoType *type_)
 {

mono/mini/interp/transform.c

@@ -876,17 +876,36 @@ create_interp_local (TransformData *td, MonoType *type)
 }
 
 static void
-dump_mint_code (TransformData *td)
+dump_mint_code (const guint16 *start, const guint16* end)
 {
-	const guint16 *p = td->new_code;
-	while (p < td->new_ip) {
-		char *ins = mono_interp_dis_mintop (td->new_code, p);
+	const guint16 *p = start;
+	while (p < end) {
+		char *ins = mono_interp_dis_mintop (start, p);
 		g_print ("%s\n", ins);
 		g_free (ins);
 		p = mono_interp_dis_mintop_len (p);
 	}
 }
 
+/* For debug use */
+void
+mono_interp_print_code (InterpMethod *imethod)
+{
+	MonoJitInfo *jinfo = imethod->jinfo;
+	const guint16 *start;
+
+	if (!jinfo)
+		return;
+
+	char *name = mono_method_full_name (imethod->method, 1);
+	g_print ("Method : %s\n", name);
+	g_free (name);
+
+	start = (guint16*) jinfo->code_start;
+	dump_mint_code (start, start + jinfo->code_size);
+}
+
+
 static MonoMethodHeader*
 interp_method_get_header (MonoMethod* method, MonoError *error)
 {
@@ -5417,7 +5436,7 @@ generate (MonoMethod *method, MonoMethodHeader *header, InterpMethod *rtm, MonoG
 	if (td->verbose_level) {
 		g_print ("Runtime method: %s %p, VT stack size: %d\n", mono_method_full_name (method, TRUE), rtm, td->max_vt_sp);
 		g_print ("Calculated stack size: %d, stated size: %d\n", td->max_stack_height, header->max_stack);
-		dump_mint_code (td);
+		dump_mint_code (td->new_code, td->new_ip);
 	}
 
 	/* Check if we use excessive stack space */

mono/tests/Makefile.am

@@ -706,6 +706,7 @@ TESTS_CS_SRC=		\
 	bug-60843.cs	\
 	nested_type_visibility.cs	\
 	dynamic-method-churn.cs	\
+	dynamic-method-delegate.cs \
 	verbose.cs \
 	generic-unmanaged-constraint.cs \
 	bug-10834.cs \
@@ -1384,6 +1385,7 @@ PROFILE_DISABLED_TESTS += \
 	dynamic-method-resurrection.exe	\
 	assembly_append_ordering.exe \
 	assemblyresolve_event5.exe	\
+	dynamic-method-delegate.exe \
 	dynamic-method-churn.exe \
 	bug-666008.exe \
 	bug-685908.exe

mono/tests/dynamic-method-delegate.cs

@@ -0,0 +1,73 @@
+using System;
+using System.Reflection;
+using System.Reflection.Emit;
+
+class Program
+{
+	static int Main (string[] args) {
+		Func<long?,long?> del_global = null;
+		bool caught_ex;
+
+		for (int i = 1; i < 100; ++i) {
+			// Random method whose delegate invoke is not optimized away by jit
+			//
+			//    .method public static hidebysig
+			//           default valuetype [mscorlib]System.Nullable`1<int64> StaticMethodToBeClosedOverNull (object o, valuetype [mscorlib]System.Nullable`1<int64> bar)  cil managed
+			//    {
+			//       IL_0000:  newobj instance void class [mscorlib]System.Exception::'.ctor'()
+			//       IL_0005:  throw
+			//    }
+			//
+			//    public static long? StaticMethodToBeClosedOverNull (object o, long? bar)
+			//    {
+			//         throw new Exception ();
+			//    }
+			DynamicMethod dm = new DynamicMethod(
+				$"dm_{i}",
+				typeof (Nullable<long>),
+				new Type[2] { typeof (object), typeof (Nullable<long>) },
+				typeof(Program).Module);
+
+			ConstructorInfo ctorInfo = typeof (Exception).GetConstructor(Type.EmptyTypes);
+
+			ILGenerator il = dm.GetILGenerator();
+			il.Emit(OpCodes.Newobj, ctorInfo);
+			il.Emit(OpCodes.Throw);
+
+			var del = (Func<long?,long?>)dm.CreateDelegate (typeof (Func<long?,long?>));
+			caught_ex = false;
+			try {
+				del (5);
+			} catch (Exception) {
+				caught_ex = true;
+			}
+			if (!caught_ex)
+				Environment.Exit (1);
+			// Make sure the finalizer thread has work to do, so it will also free dynamic methods
+			new Program ();
+			if (i % 50 == 0) {
+				del_global = del;
+				GC.Collect ();
+				GC.WaitForPendingFinalizers ();
+			}
+		}
+
+		GC.Collect ();
+		// The delegate invoke wrapper of del_global was created for another dynamic method (that should have
+		// been freed) than the one associated with this delegate. Does the delegate invocation/EH still work ?
+		caught_ex = false;
+		try {
+			del_global (5);
+		} catch (Exception) {
+			caught_ex = true;
+		}
+		if (!caught_ex)
+			Environment.Exit (2);
+		return 0;
+	}
+
+	~Program ()
+	{
+
+	}
+}