Fix FnPtrTypeDesc::GetManagedClassObject (#82325)

* Fix FnPtrTypeDesc::GetManagedClassObject The code was missing support for allocating RuntimeType objects on frozen heap Fixes #82252 * Revert "Disable System.Reflection.Tests.TypeDelegatorTests.FunctionPointers (#82321)" This reverts commit 473e278.
dotnet · Feb 18, 2023 · 2bd2ed9 · 2bd2ed9
1 parent 5ec100a
commit 2bd2ed9
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 32 deletions.
diff --git a/src/coreclr/vm/typedesc.cpp b/src/coreclr/vm/typedesc.cpp
@@ -522,26 +522,9 @@ OBJECTREF FnPtrTypeDesc::GetManagedClassObject()
     }
     CONTRACTL_END;
 
-    if (m_hExposedClassObject == NULL) {
-        REFLECTCLASSBASEREF  refClass = NULL;
-        GCPROTECT_BEGIN(refClass);
-        refClass = (REFLECTCLASSBASEREF) AllocateObject(g_pRuntimeTypeClass);
-
-        LoaderAllocator *pLoaderAllocator = GetLoaderAllocator();
-        TypeHandle th = TypeHandle(this);
-        ((ReflectClassBaseObject*)OBJECTREFToObject(refClass))->SetType(th);
-        ((ReflectClassBaseObject*)OBJECTREFToObject(refClass))->SetKeepAlive(pLoaderAllocator->GetExposedObject());
-
-        // Let all threads fight over who wins using InterlockedCompareExchange.
-        // Only the winner can set m_hExposedClassObject from NULL.
-        LOADERHANDLE hExposedClassObject = pLoaderAllocator->AllocateHandle(refClass);
-
-        if (InterlockedCompareExchangeT(&m_hExposedClassObject, hExposedClassObject, static_cast<LOADERHANDLE>(NULL)))
-        {
-            pLoaderAllocator->FreeHandle(hExposedClassObject);
-        }
-
-        GCPROTECT_END();
+    if (m_hExposedClassObject == NULL)
+    {
+        TypeHandle(this).AllocateManagedClassObject(&m_hExposedClassObject);
     }
     return GetManagedClassObjectIfExists();
 }

diff --git a/src/coreclr/vm/typedesc.h b/src/coreclr/vm/typedesc.h
@@ -540,6 +540,7 @@ class FnPtrTypeDesc : public TypeDesc
     void EnumMemoryRegions(CLRDataEnumMemoryFlags flags);
 #endif //DACCESS_COMPILE
 
+#ifndef DACCESS_COMPILE
     OBJECTREF GetManagedClassObject();
 
     OBJECTREF GetManagedClassObjectIfExists()
@@ -552,23 +553,37 @@ class FnPtrTypeDesc : public TypeDesc
         }
         CONTRACTL_END;
 
-        OBJECTREF objRet = NULL;
-        GET_LOADERHANDLE_VALUE_FAST(GetLoaderAllocator(), m_hExposedClassObject, &objRet);
-        return objRet;
+        const RUNTIMETYPEHANDLE handle = m_hExposedClassObject;
+
+        OBJECTREF retVal;
+        if (!TypeHandle::GetManagedClassObjectFromHandleFast(handle, &retVal) &&
+            !GetLoaderAllocator()->GetHandleValueFastPhase2(handle, &retVal))
+        {
+            return NULL;
+        }
+
+        COMPILER_ASSUME(retVal != NULL);
+        return retVal;
     }
 
     OBJECTREF GetManagedClassObjectFast()
     {
         LIMITED_METHOD_CONTRACT;
 
-        OBJECTREF objRet = NULL;
-        LoaderAllocator::GetHandleValueFast(m_hExposedClassObject, &objRet);
-        return objRet;
-    }    
+        OBJECTREF objRef;
+        if (!TypeHandle::GetManagedClassObjectFromHandleFast(m_hExposedClassObject, &objRef))
+        {
+            return FALSE;
+        }
+        COMPILER_ASSUME(objRef != NULL);
+        return objRef;
+    }
+#endif
 
 protected:
-    // Handle back to the internal reflection Type object
-    LOADERHANDLE m_hExposedClassObject;
+    // Non-unloadable context: internal RuntimeType object handle
+    // Unloadable context: slot index in LoaderAllocator's pinned table
+    RUNTIMETYPEHANDLE m_hExposedClassObject;
 
     // Number of arguments
     DWORD m_NumArgs;

diff --git a/src/coreclr/vm/typehandle.cpp b/src/coreclr/vm/typehandle.cpp
@@ -349,8 +349,7 @@ bool TypeHandle::IsManagedClassObjectPinned() const
 {
     LIMITED_METHOD_DAC_CONTRACT;
 
-    // Function pointers are always mapped to typeof(IntPtr)
-    return !GetLoaderAllocator()->CanUnload() || IsFnPtrType();
+    return !GetLoaderAllocator()->CanUnload();
 }
 
 void TypeHandle::AllocateManagedClassObject(RUNTIMETYPEHANDLE* pDest)

diff --git a/src/libraries/System.Runtime/tests/System/Reflection/TypeDelegatorTests.cs b/src/libraries/System.Runtime/tests/System/Reflection/TypeDelegatorTests.cs
@@ -59,7 +59,6 @@ public void Properties()
 
         [Fact]
         [ActiveIssue("https://github.com/dotnet/runtime/issues/71095", TestRuntimes.Mono)]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/82252", TestRuntimes.CoreCLR)]
         [ActiveIssue("https://github.com/dotnet/runtime/issues/71883", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))]          
         public void FunctionPointers()
         {