Move TypeReference.ToObject to managed (CoreCLR) (#70055)

* Initial partial TypedReference.ToObject managed port

* Port rest of mamaged TypeDesc method table logic

* Add RuntimeTypeHandle.GetElementTypeMethodTable FCall

* Remove TypedReference.InternalToObject FCall

* Remove throwing path, add an assert

* Remove unnecessary TypeDesc paths

* Remove ParamTypeDesc::m_TemplateMT and related code

* Fix TypeDesc::GetMethodTable

* Reuse RuntimeTypeHandle.GetValueInternal to drop an FCall

* Add unit tests for TypedReference.ToObject and type desc

* Move ifdef methods to TypedReference.Mono file

* Skip new TypedReference tests on Mono

* Use [ActiveIssue] instead of [SkipOnMono]

* Commented tests out to skip Mono AOT failure

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs

@@ -621,6 +621,11 @@ public TypeHandle(void* tAddr)
             m_asTAddr = tAddr;
         }
 
+        /// <summary>
+        /// Gets whether the current instance wraps a <see langword="null"/> pointer.
+        /// </summary>
+        public bool IsNull => m_asTAddr is null;
+
         /// <summary>
         /// Gets whether or not this <see cref="TypeHandle"/> wraps a <c>TypeDesc</c> pointer.
         /// Only if this returns <see langword="false"/> it is safe to call <see cref="AsMethodTable"/>.

src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs

@@ -2,7 +2,10 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 // TypedReference is basically only ever seen on the call stack, and in param arrays.
-//  These are blob that must be dealt with by the compiler.
+// These are blob that must be dealt with by the compiler.
+
+using System.Diagnostics;
+using System.Runtime.CompilerServices;
 
 namespace System
 {
@@ -11,5 +14,38 @@ public ref partial struct TypedReference
     {
         private readonly ByReference<byte> _value;
         private readonly IntPtr _type;
+
+        public static unsafe object? ToObject(TypedReference value)
+        {
+            TypeHandle typeHandle = new((void*)value._type);
+
+            if (typeHandle.IsNull)
+            {
+                ThrowHelper.ThrowArgumentException_ArgumentNull_TypedRefType();
+            }
+
+            // The only case where a type handle here might be a type desc is when the type is either a
+            // pointer or a function pointer. In those cases, just always return the method table pointer
+            // for System.UIntPtr without inspecting the type desc any further. Otherwise, the type handle
+            // is just wrapping a method table pointer, so return that directly with a reinterpret cast.
+            MethodTable* pMethodTable = typeHandle.IsTypeDesc
+                ? (MethodTable*)RuntimeTypeHandle.GetValueInternal(typeof(UIntPtr).TypeHandle)
+                : typeHandle.AsMethodTable();
+
+            Debug.Assert(pMethodTable is not null);
+
+            object? result;
+
+            if (pMethodTable->IsValueType)
+            {
+                result = RuntimeHelpers.Box(pMethodTable, ref value._value.Value);
+            }
+            else
+            {
+                result = Unsafe.As<byte, object>(ref value._value.Value);
+            }
+
+            return result;
+        }
     }
 }

src/coreclr/vm/clsload.cpp

@@ -2940,7 +2940,6 @@ TypeHandle ClassLoader::CreateTypeHandleForTypeKey(TypeKey* pKey, AllocMemTracke
 
         CorElementType kind = pKey->GetKind();
         TypeHandle paramType = pKey->GetElementType();
-        MethodTable *templateMT;
 
         // Create a new type descriptor and insert into constructed type table
         if (CorTypeInfo::IsArray(kind))
@@ -2970,7 +2969,8 @@ TypeHandle ClassLoader::CreateTypeHandleForTypeKey(TypeKey* pKey, AllocMemTracke
                 ThrowTypeLoadException(pKey, IDS_CLASSLOAD_RANK_TOOLARGE);
             }
 
-            templateMT = pLoaderModule->CreateArrayMethodTable(paramType, kind, rank, pamTracker);
+            MethodTable *templateMT = pLoaderModule->CreateArrayMethodTable(paramType, kind, rank, pamTracker);
+
             typeHnd = TypeHandle(templateMT);
         }
         else
@@ -2984,15 +2984,8 @@ TypeHandle ClassLoader::CreateTypeHandleForTypeKey(TypeKey* pKey, AllocMemTracke
             // We do allow parametrized types of ByRefLike types. Languages may restrict them to produce safe or verifiable code,
             // but there is not a good reason for restricting them in the runtime.
 
-            // let <Type>* type have a method table
-            // System.UIntPtr's method table is used for types like int*, void *, string * etc.
-            if (kind == ELEMENT_TYPE_PTR)
-                templateMT = CoreLibBinder::GetElementType(ELEMENT_TYPE_U);
-            else
-                templateMT = NULL;
-
             BYTE* mem = (BYTE*) pamTracker->Track(pLoaderModule->GetAssembly()->GetLowFrequencyHeap()->AllocMem(S_SIZE_T(sizeof(ParamTypeDesc))));
-            typeHnd = TypeHandle(new(mem)  ParamTypeDesc(kind, templateMT, paramType));
+            typeHnd = TypeHandle(new(mem)  ParamTypeDesc(kind, paramType));
         }
     }
 

src/coreclr/vm/ecalllist.h

@@ -120,7 +120,6 @@ FCFuncStart(gExceptionFuncs)
 FCFuncEnd()
 
 FCFuncStart(gTypedReferenceFuncs)
-    FCFuncElement("InternalToObject", ReflectionInvocation::TypedReferenceToObject)
     FCFuncElement("InternalMakeTypedReference", ReflectionInvocation::MakeTypedReference)
 FCFuncEnd()
 

src/coreclr/vm/reflectioninvocation.cpp

@@ -1470,38 +1470,6 @@ FCIMPL4(void, ReflectionInvocation::MakeTypedReference, TypedByRef * value, Obje
 }
 FCIMPLEND
 
-// This is an internal helper function to TypedReference class.
-// It extracts the object from the typed reference.
-FCIMPL1(Object*, ReflectionInvocation::TypedReferenceToObject, TypedByRef * value) {
-    FCALL_CONTRACT;
-
-    OBJECTREF       Obj = NULL;
-
-    TypeHandle th(value->type);
-
-    if (th.IsNull())
-        FCThrowRes(kArgumentNullException, W("ArgumentNull_TypedRefType"));
-
-    MethodTable* pMT = th.GetMethodTable();
-    PREFIX_ASSUME(NULL != pMT);
-
-    if (pMT->IsValueType())
-    {
-        // value->data is protected by the caller
-        HELPER_METHOD_FRAME_BEGIN_RET_1(Obj);
-
-        Obj = pMT->Box(value->data);
-
-        HELPER_METHOD_FRAME_END();
-    }
-    else {
-        Obj = ObjectToOBJECTREF(*((Object**)value->data));
-    }
-
-    return OBJECTREFToObject(Obj);
-}
-FCIMPLEND
-
 FCIMPL2_IV(Object*, ReflectionInvocation::CreateEnum, ReflectClassBaseObject *pTypeUNSAFE, INT64 value) {
     FCALL_CONTRACT;
 

src/coreclr/vm/reflectioninvocation.h

@@ -52,7 +52,6 @@ class ReflectionInvocation {
 
     // TypedReference functions, should go somewhere else
     static FCDECL4(void, MakeTypedReference, TypedByRef * value, Object* targetUNSAFE, ArrayBase* fldsUNSAFE, ReflectClassBaseObject *pFieldType);
-    static FCDECL1(Object*, TypedReferenceToObject, TypedByRef * value);
 
 #ifdef FEATURE_COMINTEROP
     static FCDECL8(Object*, InvokeDispMethod, ReflectClassBaseObject* refThisUNSAFE, StringObject* nameUNSAFE, INT32 invokeAttr, Object* targetUNSAFE, PTRArray* argsUNSAFE, PTRArray* byrefModifiersUNSAFE, LCID lcid, PTRArray* namedParametersUNSAFE);

src/coreclr/vm/runtimehandles.h

@@ -176,8 +176,7 @@ class RuntimeTypeHandle {
     static FCDECL1(MethodDesc *, GetFirstIntroducedMethod, ReflectClassBaseObject* pType);
     static FCDECL1(void, GetNextIntroducedMethod, MethodDesc **ppMethod);
 
-    static
-    FCDECL1(IMDInternalImport*, GetMetadataImport, ReflectClassBaseObject * pModuleUNSAFE);
+    static FCDECL1(IMDInternalImport*, GetMetadataImport, ReflectClassBaseObject * pModuleUNSAFE);
 
     // Helper methods not called by managed code
 

src/coreclr/vm/typedesc.cpp

@@ -30,7 +30,6 @@ BOOL ParamTypeDesc::Verify() {
     STATIC_CONTRACT_DEBUG_ONLY;
     STATIC_CONTRACT_SUPPORTS_DAC;
 
-    _ASSERTE((m_TemplateMT == NULL) || GetTemplateMethodTableInternal()->SanityCheck());
     _ASSERTE(!GetTypeParam().IsNull());
     _ASSERTE(CorTypeInfo::IsModifier_NoThrow(GetInternalCorElementType()) ||
                               GetInternalCorElementType() == ELEMENT_TYPE_VALUETYPE);
@@ -133,26 +132,6 @@ PTR_Module TypeDesc::GetModule() {
     return GetLoaderModule();
 }
 
-BOOL ParamTypeDesc::OwnsTemplateMethodTable()
-{
-    CONTRACTL
-    {
-        NOTHROW;
-        GC_NOTRIGGER;
-    }
-    CONTRACTL_END;
-
-    CorElementType kind = GetInternalCorElementType();
-
-    // The m_TemplateMT for pointer types is UIntPtr
-    if (!CorTypeInfo::IsArray_NoThrow(kind))
-    {
-        return FALSE;
-    }
-
-    return TRUE;
-}
-
 Assembly* TypeDesc::GetAssembly() {
     STATIC_CONTRACT_NOTHROW;
     STATIC_CONTRACT_GC_NOTRIGGER;
@@ -532,12 +511,6 @@ OBJECTREF ParamTypeDesc::GetManagedClassObject()
             pLoaderAllocator->FreeHandle(hExposedClassObject);
         }
 
-        if (OwnsTemplateMethodTable())
-        {
-            // Set the handle on template methodtable as well to make Object.GetType for arrays take the fast path
-            GetTemplateMethodTableInternal()->GetWriteableDataForWrite()->m_hExposedClassObject = m_hExposedClassObject;
-        }
-
         GCPROTECT_END();
     }
     return GetManagedClassObjectIfExists();
@@ -690,14 +663,6 @@ void TypeDesc::DoFullyLoad(Generics::RecursionGraph *pVisited, ClassLoadLevel le
 
         // Fully load the type parameter
         GetTypeParam().DoFullyLoad(&newVisited, level, pPending, &fBailed, pInstContext);
-
-        ParamTypeDesc* pPTD = (ParamTypeDesc*) this;
-
-        // Fully load the template method table
-        if (pPTD->m_TemplateMT != NULL)
-        {
-            pPTD->GetTemplateMethodTableInternal()->DoFullyLoad(&newVisited, level, pPending, &fBailed, pInstContext);
-        }
     }
 
     switch (level)
@@ -1722,12 +1687,6 @@ ParamTypeDesc::EnumMemoryRegions(CLRDataEnumMemoryFlags flags)
     SUPPORTS_DAC;
     DAC_ENUM_DTHIS();
 
-    PTR_MethodTable pTemplateMT = GetTemplateMethodTableInternal();
-    if (pTemplateMT.IsValid())
-    {
-        pTemplateMT->EnumMemoryRegions(flags);
-    }
-
     m_Arg.EnumMemoryRegions(flags);
 }
 

src/coreclr/vm/typedesc.h

@@ -220,13 +220,11 @@ class ParamTypeDesc : public TypeDesc {
 
 public:
 #ifndef DACCESS_COMPILE
-    ParamTypeDesc(CorElementType type, MethodTable* pMT, TypeHandle arg)
+    ParamTypeDesc(CorElementType type, TypeHandle arg)
         : TypeDesc(type), m_Arg(arg), m_hExposedClassObject(0) {
 
         LIMITED_METHOD_CONTRACT;
 
-        m_TemplateMT = pMT;
-
         // ParamTypeDescs start out life not fully loaded
         m_typeAndFlags |= TypeDesc::enum_flag_IsNotFullyLoaded;
 
@@ -276,8 +274,6 @@ class ParamTypeDesc : public TypeDesc {
 
     TypeHandle GetTypeParam();
 
-    BOOL OwnsTemplateMethodTable();
-
 #ifdef DACCESS_COMPILE
     void EnumMemoryRegions(CLRDataEnumMemoryFlags flags);
 #endif
@@ -288,13 +284,8 @@ class ParamTypeDesc : public TypeDesc {
     friend class ArrayOpLinker;
 #endif
 protected:
-    PTR_MethodTable GetTemplateMethodTableInternal() {
-        WRAPPER_NO_CONTRACT;
-        return m_TemplateMT;
-    }
 
     // the m_typeAndFlags field in TypeDesc tell what kind of parameterized type we have
-    PTR_MethodTable m_TemplateMT; // The shared method table, some variants do not use this field (it is null)
     TypeHandle      m_Arg;              // The type that is being modified
     LOADERHANDLE    m_hExposedClassObject;  // handle back to the internal reflection Type object
 };

src/coreclr/vm/typedesc.inl

@@ -18,19 +18,18 @@ inline PTR_MethodTable  TypeDesc::GetMethodTable() {
 
     LIMITED_METHOD_DAC_CONTRACT;
 
-    if (IsGenericVariable())
-        return NULL;
-
-    if (GetInternalCorElementType() == ELEMENT_TYPE_FNPTR)
+    switch (GetInternalCorElementType())
+    {
+    case ELEMENT_TYPE_PTR:
+    case ELEMENT_TYPE_FNPTR:
         return CoreLibBinder::GetElementType(ELEMENT_TYPE_U);
 
-    _ASSERTE(HasTypeParam());
-    ParamTypeDesc* asParam = dac_cast<PTR_ParamTypeDesc>(this);
+    case ELEMENT_TYPE_VALUETYPE:
+        return dac_cast<PTR_MethodTable>(dac_cast<PTR_ParamTypeDesc>(this)->m_Arg.AsMethodTable());
 
-    if (GetInternalCorElementType() == ELEMENT_TYPE_VALUETYPE)
-        return dac_cast<PTR_MethodTable>(asParam->m_Arg.AsMethodTable());
-    else
-        return(asParam->GetTemplateMethodTableInternal());
+    default:
+        return NULL;
+    }
 }
 
 inline TypeHandle TypeDesc::GetTypeParam() {

src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs

@@ -86,6 +86,12 @@ internal static void ThrowArgumentException_OverlapAlignmentMismatch()
             throw new ArgumentException(SR.Argument_OverlapAlignmentMismatch);
         }
 
+        [DoesNotReturn]
+        internal static void ThrowArgumentException_ArgumentNull_TypedRefType()
+        {
+            throw new ArgumentNullException("value", SR.ArgumentNull_TypedRefType);
+        }
+
         [DoesNotReturn]
         internal static void ThrowArgumentException_CannotExtractScalar(ExceptionArgument argument)
         {

src/libraries/System.Private.CoreLib/src/System/TypedReference.cs

@@ -75,14 +75,6 @@ public override bool Equals(object? o)
             throw new NotSupportedException(SR.NotSupported_NYI);
         }
 
-        public static unsafe object ToObject(TypedReference value)
-        {
-            return InternalToObject(&value);
-        }
-
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        internal static extern unsafe object InternalToObject(void* value);
-
         internal bool IsNull => Unsafe.IsNullRef(ref _value.Value) && _type == IntPtr.Zero;
 
         public static Type GetTargetType(TypedReference value)

src/libraries/System.Runtime/tests/System/TypedReferenceTests.cs

@@ -78,6 +78,35 @@ public static void MakeTypedReference_ToObjectTests()
             Assert.Equal(structObj, TypedReference.ToObject(reference));
         }
 
+        // These tests are currently crashing the Mono AOT compiler, so commenting them out (skipping them isn't enough)
+        //[Fact]
+        //[ActiveIssue("https://github.com/dotnet/runtime/issues/70091", TestRuntimes.Mono)]
+        //public static unsafe void MakeTypedReference_ToObjectTests_WithPointer()
+        //{
+        //    float* pointer = (float*)(nuint)0x12345678;
+        //    TypedReference reference = __makeref(pointer);
+        //    object obj = TypedReference.ToObject(reference);
+        //
+        //    // TypedReference-s over pointers use the UIntPtr type when boxing
+        //    Assert.NotNull(obj);
+        //    Assert.IsType<UIntPtr>(obj);
+        //    Assert.Equal((nuint)0x12345678, (nuint)obj);
+        //}
+        //
+        //[Fact]
+        //[ActiveIssue("https://github.com/dotnet/runtime/issues/70091", TestRuntimes.Mono)]
+        //public static unsafe void MakeTypedReference_ToObjectTests_WithFunctionPointer()
+        //{
+        //    delegate*<int, float, string> pointer = (delegate*<int, float, string>)(void*)(nuint)0x12345678;
+        //    TypedReference reference = __makeref(pointer);
+        //    object obj = TypedReference.ToObject(reference);
+        //
+        //    // TypedReference-s over function pointers use the UIntPtr type when boxing
+        //    Assert.NotNull(obj);
+        //    Assert.IsType<UIntPtr>(obj);
+        //    Assert.Equal((nuint)0x12345678, (nuint)obj);
+        //}
+
         [Fact]
         public static void MakeTypedReference_ReadOnlyField_Succeeds()
         {

src/mono/System.Private.CoreLib/src/System/TypedReference.Mono.cs

@@ -15,5 +15,13 @@ public ref partial struct TypedReference
         private readonly IntPtr _type;
         #pragma warning restore CA1823
         #endregion
+
+        public static unsafe object? ToObject(TypedReference value)
+        {
+            return InternalToObject(&value);
+        }
+
+        [MethodImpl(MethodImplOptions.InternalCall)]
+        internal static extern unsafe object InternalToObject(void* value);
     }
 }