Fix value type unboxing bug (#69815)

dotnet · May 26, 2022 · 5734a23 · 5734a23
1 parent cb1fd54
commit 5734a23
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 8 deletions.
diff --git a/src/coreclr/vm/comdelegate.cpp b/src/coreclr/vm/comdelegate.cpp
@@ -2158,6 +2158,7 @@ FCIMPL1(ReflectMethodObject *, COMDelegate::FindMethodHandle, Object* refThisIn)
     HELPER_METHOD_FRAME_BEGIN_RET_1(refThis);
 
     pMD = GetMethodDesc(refThis);
+    pMD = MethodDesc::FindOrCreateAssociatedMethodDescForReflection(pMD, TypeHandle(pMD->GetMethodTable()), pMD->GetMethodInstantiation());
     pRet = pMD->GetStubMethodInfo();
     HELPER_METHOD_FRAME_END();
 

diff --git a/src/coreclr/vm/genmeth.cpp b/src/coreclr/vm/genmeth.cpp
@@ -1236,12 +1236,11 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD,
         if (methodInst.GetNumArgs() != pMethod->GetNumGenericMethodArgs())
             COMPlusThrow(kArgumentException);
 
-        // we base the creation of an unboxing stub on whether the original method was one already
-        // that keeps the reflection logic the same for value types
+        // we need unboxing stubs for virtual methods on value types
         pInstMD = MethodDesc::FindOrCreateAssociatedMethodDesc(
             pMethod,
             pMT,
-            pMethod->IsUnboxingStub(),
+            instType.IsValueType() && pMethod->IsVirtual(),
             methodInst,
             FALSE,      /* no allowInstParam */
             TRUE   /* force remotable method (i.e. inst wrappers for non-generic methods on generic interfaces) */);
@@ -1263,12 +1262,8 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD,
         // - non generic method on a generic interface
         //
 
-        // we base the creation of an unboxing stub on whether the original method was one already
-        // that keeps the reflection logic the same for value types
-
         // we need unboxing stubs for virtual methods on value types unless the method is generic
-        BOOL fNeedUnboxingStub = pMethod->IsUnboxingStub() ||
-            ( instType.IsValueType() && pMethod->IsVirtual() );
+        BOOL fNeedUnboxingStub = instType.IsValueType() && pMethod->IsVirtual();
 
         pInstMD = MethodDesc::FindOrCreateAssociatedMethodDesc(
             pMethod,            /* the original MD          */

diff --git a/src/libraries/System.Runtime/tests/System/DelegateTests.cs b/src/libraries/System.Runtime/tests/System/DelegateTests.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.IO;
+using System.Linq.Expressions;
 using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
@@ -435,6 +436,50 @@ private static void IntIntMethod(int expected, int actual)
             Assert.Equal(expected, actual);
         }
 
+        [Fact]
+        public static void SameMethodObtainedViaDelegateAndReflectionAreSameForClass()
+        {
+            var m1 = ((MethodCallExpression)((Expression<Action>)(() => new Class().M())).Body).Method;
+            var m2 = new Action(new Class().M).Method;
+            Assert.True(m1.Equals(m2));
+        }
+
+        [Fact]
+        public static void SameMethodObtainedViaDelegateAndReflectionAreSameForStruct()
+        {
+            var m1 = ((MethodCallExpression)((Expression<Action>)(() => new Struct().M())).Body).Method;
+            var m2 = new Action(new Struct().M).Method;
+            Assert.True(m1.Equals(m2));
+        }
+
+        [Fact]
+        public static void SameGenericMethodObtainedViaDelegateAndReflectionAreSameForClass()
+        {
+            var m1 = ((MethodCallExpression)((Expression<Action>)(() => new ClassG().M<string, object>())).Body).Method;
+            var m2 = new Action(new ClassG().M<string, object>).Method;
+            Assert.True(m1.Equals(m2));
+            Assert.True(m1.GetHashCode().Equals(m2.GetHashCode()));
+            Assert.Equal(m1.MethodHandle.Value, m2.MethodHandle.Value);
+        }
+
+        [Fact]
+        public static void SameGenericMethodObtainedViaDelegateAndReflectionAreSameForStruct()
+        {
+            var m1 = ((MethodCallExpression)((Expression<Action>)(() => new StructG().M<string, object>())).Body).Method;
+            var m2 = new Action(new StructG().M<string, object>).Method;
+            Assert.True(m1.Equals(m2));
+            Assert.True(m1.GetHashCode().Equals(m2.GetHashCode()));
+            Assert.Equal(m1.MethodHandle.Value, m2.MethodHandle.Value);
+        }
+
+        class Class { internal void M() { } }
+
+        struct Struct { internal void M() { } }
+
+        class ClassG { internal void M<Key, Value>() { } }
+
+        struct StructG { internal void M<Key, Value>() { } }
+
         private delegate void IntIntDelegate(int expected, int actual);
         private delegate void IntIntDelegateWithDefault(int expected, int actual = 7);