[mono/mono] [2 commits] 5d6ca293: [runtime] Avoid caching member_ref tokens of dynamic images. Fixes #16096

   Branch: refs/heads/master
     Home: https://github.com/mono/mono
  Compare: https://github.com/mono/mono/compare/0f197300e90a...055eeccb76ea

   Commit: 5d6ca293ec92548fa21bda60a6f9336a728d1e4a
   Author: Rodrigo Kumpera <[email protected]> (kumpera)
     Date: 2013-11-18 19:26:39 GMT
      URL: https://github.com/mono/mono/commit/5d6ca293ec92548fa21bda60a6f9336a728d1e4a

[runtime] Avoid caching member_ref tokens of dynamic images. Fixes #16096

SRE encodes references to self-instantiated generic types in a different way
that regular code.

This is required as there's no clear way to differentiate between an open type
and a self-instantiated type when using System.Type.

Given that restriction, code that deals with loading such methods must take
special case and not cache then as the context under generic sharing will not
be used - so it would be seen as cacheable.

When using regular instantiation the same method lookup will require the context
to be used but will hit the cache before.

This is another problem of passing the instantiation context around. It messes
up with way too much stuff.

Changed paths:
  M mono/metadata/loader.c

Modified: mono/metadata/loader.c
===================================================================
@@ -1812,7 +1812,7 @@ struct _MonoDllMap {
 mono_get_method_full (MonoImage *image, guint32 token, MonoClass *klass,
 		      MonoGenericContext *context)
 {
-	MonoMethod *result;
+	MonoMethod *result = NULL;
 	gboolean used_context = FALSE;
 
 	/* We do everything inside the lock to prevent creation races */
@@ -1823,7 +1823,7 @@ struct _MonoDllMap {
 		if (!image->method_cache)
 			image->method_cache = g_hash_table_new (NULL, NULL);
 		result = g_hash_table_lookup (image->method_cache, GINT_TO_POINTER (mono_metadata_token_index (token)));
-	} else {
+	} else if (!image->dynamic) {
 		if (!image->methodref_cache)
 			image->methodref_cache = g_hash_table_new (NULL, NULL);
 		result = g_hash_table_lookup (image->methodref_cache, GINT_TO_POINTER (token));
@@ -1833,16 +1833,18 @@ struct _MonoDllMap {
 	if (result)
 		return result;
 
+
 	result = mono_get_method_from_token (image, token, klass, context, &used_context);
 	if (!result)
 		return NULL;
 
 	mono_image_lock (image);
 	if (!used_context && !result->is_inflated) {
-		MonoMethod *result2;
+		MonoMethod *result2 = NULL;
+
 		if (mono_metadata_token_table (token) == MONO_TABLE_METHOD)
 			result2 = g_hash_table_lookup (image->method_cache, GINT_TO_POINTER (mono_metadata_token_index (token)));
-		else
+		else if (!image->dynamic)
 			result2 = g_hash_table_lookup (image->methodref_cache, GINT_TO_POINTER (token));
 
 		if (result2) {
@@ -1852,7 +1854,7 @@ struct _MonoDllMap {
 
 		if (mono_metadata_token_table (token) == MONO_TABLE_METHOD)
 			g_hash_table_insert (image->method_cache, GINT_TO_POINTER (mono_metadata_token_index (token)), result);
-		else
+		else if (!image->dynamic)
 			g_hash_table_insert (image->methodref_cache, GINT_TO_POINTER (token), result);
 	}
 

   Commit: 055eeccb76eaed6b33e83cf62806dcc7c4d7b0d5
   Author: Rodrigo Kumpera <[email protected]> (kumpera)
     Date: 2013-11-18 19:26:40 GMT
      URL: https://github.com/mono/mono/commit/055eeccb76eaed6b33e83cf62806dcc7c4d7b0d5

[bcl] Add regression test for bxc #16096.

Changed paths:
  M mcs/class/corlib/Test/System.Reflection.Emit/TypeBuilderTest.cs

Modified: mcs/class/corlib/Test/System.Reflection.Emit/TypeBuilderTest.cs
===================================================================
@@ -11074,5 +11074,53 @@ public class Example<T> {
 			Activator.CreateInstance (t, new object[] { "string"});
 		}
 
+		public interface IFace16096 {
+			object Bar ();
+		}
+
+		[Test]
+		public void MemberRef_Caching_16096 () {
+			var outer_class = module.DefineType(
+				"container",
+				TypeAttributes.Class | TypeAttributes.Public,
+				typeof(object));
+
+			var builder = outer_class.DefineNestedType(
+				"bind@32-1",
+				TypeAttributes.Class | TypeAttributes.Public,
+				typeof(object));
+
+			builder.AddInterfaceImplementation (typeof (IFace16096));
+
+			var ctor = builder.DefineDefaultConstructor (MethodAttributes.Public);
+			var field = builder.DefineField ("Field", typeof (object), FieldAttributes.Public);
+			var g_args = builder.DefineGenericParameters("b","a");
+			var method = builder.DefineMethod ("Bar", MethodAttributes.Public | MethodAttributes.Virtual, typeof (object), new Type [0]);
+
+			var il = method.GetILGenerator();
+			il.Emit (OpCodes.Ldarg_0);
+			il.Emit (OpCodes.Ldfld, TypeBuilder.GetField (builder.MakeGenericType (g_args), field));
+			il.Emit (OpCodes.Pop);
+			il.Emit (OpCodes.Newobj, TypeBuilder.GetConstructor (builder.MakeGenericType (g_args), ctor));
+			il.Emit (OpCodes.Ret);
+
+			var type = builder.CreateType ();
+
+			/*Build a gshared instance. */
+			var ginst = type.MakeGenericType (typeof (List<char>), typeof (object));
+			var ins = (IFace16096)Activator.CreateInstance (ginst);
+
+			/* This will trigger the runtime to cache the MEMBER_REF to the .ctor as it won't have a context. */
+			var ins2 = ins.Bar ();
+			Assert.IsNotNull (ins2);
+
+			/* Build an unsharable version. */
+			var ginst2 = type.MakeGenericType (typeof (List<char>), typeof (char));
+			var ins3 = (IFace16096)Activator.CreateInstance (ginst2);
+
+			/* This will trigger the runtime to use the cached version, which is wrong as it's an open type. */
+			var ins4 = ins3.Bar ();
+			Assert.IsNotNull (ins4);
+		}
 	}
 }


_______________________________________________
Mono-patches maillist  -  [email protected]
http://lists.ximian.com/mailman/listinfo/mono-patches