[sre] Register a canonical reflected method for a methodspec token. (Fixes #60233) (#5814)mono-5.4.1.6

* [sre] mono_dynamic_image_register_token: Change asserts to warnings

This partly relaxes the changes in 792b5367cd3a6ffa1a192c4d2d7ace3509cbb646
which added the assertions.

In practice the assertions introduced a crash in heavy uses of
System.Reflection.Emit such as unit testing frameworks and dynamic languages.
We still want to know if there are surprising uses of
mono_dynamic_image_register_token, but we don't need a crash.  SRE used to work
before 792b5367cd3a6ffa1a192c4d2d7ace3509cbb646, so no need to break it.

* [test] ILGenerator test for duplicate methodspec tokens

Regression test for [Bugzilla #60233](https://bugzilla.xamarin.com/show_bug.cgi?id=60233)

* [sre] Register a canonical reflected method for a methodspec token (Fixes #60233)

This is like the fix for [bugzilla
59364](https://bugzilla.xamarin.com/show_bug.cgi?id=59364) but this time it's a
methodspec token, not a memberref token.

3 files changed, 56 insertions, 8 deletions

diff --git a/mcs/class/corlib/Test/System.Reflection.Emit/ILGeneratorTest.cs b/mcs/class/corlib/Test/System.Reflection.Emit/ILGeneratorTest.cs
index c943d6859ca..b31ed2e117e 100644
--- a/mcs/class/corlib/Test/System.Reflection.Emit/ILGeneratorTest.cs
+++ b/mcs/class/corlib/Test/System.Reflection.Emit/ILGeneratorTest.cs
@@ -524,11 +524,13 @@ namespace MonoTests.System.Reflection.Emit
 			
 		}
 
-		public class Base {
+		public class Base<T> {
 			public int x;
+
+			public void M () { }
 		}
 
-		public class Derived : Base {
+		public class Derived : Base<string> {
 		}
 
 		[Test]
@@ -540,7 +542,7 @@ namespace MonoTests.System.Reflection.Emit
 			//
 			// Regression test for bugzilla #59364
 
-			var f1 = typeof (Base).GetField ("x");
+			var f1 = typeof (Base<string>).GetField ("x");
 			var f2 = typeof (Derived).GetField ("x");
 
 			var value_getter = typeof (RuntimeFieldHandle).GetProperty("Value").GetMethod;
@@ -572,5 +574,42 @@ namespace MonoTests.System.Reflection.Emit
 			Assert.AreEqual ("1", s);
 		}
 
+		[Test]
+		public void MethodSpecTokenSame () {
+			DefineBasicMethod ();
+			// Get the same method from a base generic instance and
+			// a type derived from it so that the
+			// MemberInfo:DeclaredType differs but the tokens are
+			// the same.
+			//
+			// Regression test for bugzilla #60233
+
+			var m1 = typeof (Base<string>).GetMethod ("M");
+			var m2 = typeof (Derived).GetMethod ("M");
+
+			var il = il_gen;
+
+			var loc = il.DeclareLocal (typeof (Derived));
+
+			il.Emit (OpCodes.Newobj, typeof (Derived).GetConstructor(new Type[]{}));
+			il.Emit (OpCodes.Stloc, loc);
+			il.Emit (OpCodes.Ldloc, loc);
+			il.Emit (OpCodes.Call, m1);
+			il.Emit (OpCodes.Ldloc, loc);
+			il.Emit (OpCodes.Call, m2);
+			il.Emit (OpCodes.Ldstr, "1");
+			il.Emit (OpCodes.Ret);
+
+			var baked = tb.CreateType ();
+
+			var x = Activator.CreateInstance (baked);
+			var m = baked.GetMethod ("F");
+
+			var s = m.Invoke (x, null);
+
+			Assert.AreEqual ("1", s);
+
+		}
+
 	}
 }
diff --git a/mono/metadata/dynamic-image.c b/mono/metadata/dynamic-image.c
index 4cfeaa41db8..b8874198b02 100644
--- a/mono/metadata/dynamic-image.c
+++ b/mono/metadata/dynamic-image.c
@@ -204,9 +204,12 @@ mono_dynamic_image_register_token (MonoDynamicImage *assembly, guint32 token, Mo
 	if (prev) {
 		switch (how_collide) {
 		case MONO_DYN_IMAGE_TOK_NEW:
-			g_assert_not_reached ();
+			g_warning ("%s: Unexpected previous object when called with MONO_DYN_IMAGE_TOK_NEW", __func__);
+			break;
 		case MONO_DYN_IMAGE_TOK_SAME_OK:
-			g_assert (prev == MONO_HANDLE_RAW (obj));
+			if (prev != MONO_HANDLE_RAW (obj)) {
+				g_warning ("%s: condition `prev == MONO_HANDLE_RAW (obj)' not met", __func__);
+			}
 			break;
 		case MONO_DYN_IMAGE_TOK_REPLACE:
 			break;
diff --git a/mono/metadata/sre.c b/mono/metadata/sre.c
index a009f41ecd8..cf4715e61e5 100644
--- a/mono/metadata/sre.c
+++ b/mono/metadata/sre.c
@@ -1149,9 +1149,15 @@ mono_image_create_token (MonoDynamicImage *assembly, MonoObjectHandle obj,
 		MonoReflectionMethodHandle m = MONO_HANDLE_CAST (MonoReflectionMethod, obj);
 		MonoMethod *method = MONO_HANDLE_GETVAL (m, method);
 		if (method->is_inflated) {
-			if (create_open_instance)
-				token = mono_image_get_methodspec_token (assembly, method);
-			else
+			if (create_open_instance) {
+				guint32 methodspec_token = mono_image_get_methodspec_token (assembly, method);
+				MonoReflectionMethodHandle canonical_obj =
+					mono_method_get_object_handle (MONO_HANDLE_DOMAIN (obj), method, NULL, error);
+				if (!is_ok (error))
+					goto leave;
+				MONO_HANDLE_ASSIGN (register_obj, canonical_obj);
+				token = methodspec_token;
+			} else
 				token = mono_image_get_inflated_method_token (assembly, method);
 		} else if ((method->klass->image == &assembly->image) &&
 			 !mono_class_is_ginst (method->klass)) {