From 1f4613aa1acacbea90a8aa82daeea4a449cf4df7 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Aleksey=20Kliger=20=28=CE=BBgeek=29?= Date: Wed, 18 Oct 2017 08:09:39 -0400 Subject: [PATCH] [sre] Register a canonical reflected method for a methodspec token. (Fixes #60233) (#5814) * [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. --- .../Test/System.Reflection.Emit/ILGeneratorTest.cs | 45 ++++++++++++++++++++-- mono/metadata/dynamic-image.c | 7 +++- mono/metadata/sre.c | 12 ++++-- 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 { public int x; + + public void M () { } } - public class Derived : Base { + public class Derived : Base { } [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).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).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)) { -- 2.11.4.GIT