Open types can exist as entries in interface map (#55372)

* Open types can exist as entries in interface map
- While invalid via the ECMA spec, the runtime currently represents a type explicitly instantiated over its own generic type parameters via the open type MethodTable
- This is not strictly correct, as per the spec, these should be represented via an instantiated type, but changing that detail at this time is considered highly risky
- This conflicts with the perf optimization around partialy interface loading which uses the open type of an interface to represent a type instantiated in the curiously recurring fashion.
- The fix is to detect types instantiated over generic variables, and make them ineligible for the optimization, and to detect those cases where the optimization is ineligible, and revert back to the non-optimized behavior

Fixes #55323

6 files changed, 48 insertions, 7 deletions

diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp
index 834d3cfc8c4..8bdc5f0c2a8 100644
--- a/src/coreclr/vm/methodtable.cpp
+++ b/src/coreclr/vm/methodtable.cpp
@@ -1577,6 +1577,7 @@ BOOL MethodTable::CanCastByVarianceToInterfaceOrDelegate(MethodTable *pTargetMT,
 
     // Shortcut for generic approx type scenario
     if (pMTInterfaceMapOwner != NULL &&
+        !pMTInterfaceMapOwner->ContainsGenericVariables() &&
         IsSpecialMarkerTypeForGenericCasting() &&
         GetTypeDefRid() == pTargetMT->GetTypeDefRid() &&
         GetModule() == pTargetMT->GetModule() &&
@@ -1603,7 +1604,7 @@ BOOL MethodTable::CanCastByVarianceToInterfaceOrDelegate(MethodTable *pTargetMT,
         for (DWORD i = 0; i < inst.GetNumArgs(); i++)
         {
             TypeHandle thArg = inst[i];
-            if (IsSpecialMarkerTypeForGenericCasting() && pMTInterfaceMapOwner)
+            if (IsSpecialMarkerTypeForGenericCasting() && pMTInterfaceMapOwner && !pMTInterfaceMapOwner->ContainsGenericVariables())
             {
                 thArg = pMTInterfaceMapOwner;
             }
@@ -9820,7 +9821,7 @@ PTR_MethodTable MethodTable::InterfaceMapIterator::GetInterface(MethodTable* pMT
     CONTRACT_END;
 
     MethodTable *pResult = m_pMap->GetMethodTable();
-    if (pResult->IsSpecialMarkerTypeForGenericCasting())
+    if (pResult->IsSpecialMarkerTypeForGenericCasting() && !pMTOwner->ContainsGenericVariables())
     {
         TypeHandle ownerAsInst[MaxGenericParametersForSpecialMarkerType];
         for (DWORD i = 0; i < MaxGenericParametersForSpecialMarkerType; i++)
diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h
index 4063e50b7b6..df712a378d4 100644
--- a/src/coreclr/vm/methodtable.h
+++ b/src/coreclr/vm/methodtable.h
@@ -2245,7 +2245,8 @@ public:
             {
                 if (pCurrentMethodTable->HasSameTypeDefAs(pMT) && 
                     pMT->HasInstantiation() && 
-                    pCurrentMethodTable->IsSpecialMarkerTypeForGenericCasting() && 
+                    pCurrentMethodTable->IsSpecialMarkerTypeForGenericCasting() &&
+                    !pMTOwner->ContainsGenericVariables() &&
                     pMT->GetInstantiation().ContainsAllOneType(pMTOwner))
                 {
                     exactMatch = true;
diff --git a/src/coreclr/vm/methodtable.inl b/src/coreclr/vm/methodtable.inl
index a3adc702cbe..b1af313a295 100644
--- a/src/coreclr/vm/methodtable.inl
+++ b/src/coreclr/vm/methodtable.inl
@@ -1571,7 +1571,7 @@ FORCEINLINE BOOL MethodTable::ImplementsInterfaceInline(MethodTable *pInterface)
     while (--numInterfaces);
 
     // Second scan, looking for the curiously recurring generic scenario
-    if (pInterface->HasInstantiation() && pInterface->GetInstantiation().ContainsAllOneType(this))
+    if (pInterface->HasInstantiation() && !ContainsGenericVariables() && pInterface->GetInstantiation().ContainsAllOneType(this))
     {
         numInterfaces = GetNumInterfaces();
         pInfo = GetInterfaceMap();
diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp
index 89926ffdaae..154c642cf40 100644
--- a/src/coreclr/vm/methodtablebuilder.cpp
+++ b/src/coreclr/vm/methodtablebuilder.cpp
@@ -9101,8 +9101,13 @@ MethodTableBuilder::LoadExactInterfaceMap(MethodTable *pMT)
     MethodTable **pExactMTs = (MethodTable**) _alloca(sizeof(MethodTable *) * nInterfacesCount);
     BOOL duplicates;
     bool retry = false;
-    bool retryWithExactInterfaces = !pMT->IsValueType() || pMT->IsSharedByGenericInstantiations(); // Always use exact loading behavior with classes or shared generics, as they have to deal with inheritance, and the
-                                                                                          // inexact matching logic for classes would be more complex to write.
+
+    // Always use exact loading behavior with classes or shared generics, as they have to deal with inheritance, and the
+    // inexact matching logic for classes would be more complex to write.
+    // Also always use the exact loading behavior with any generic that contains generic variables, as the open type is used
+    // to represent a type instantiated over its own generic variables, and the special marker type is currently the open type
+    // and we make this case distinguishable by simply disallowing the optimization in those cases.
+    bool retryWithExactInterfaces = !pMT->IsValueType() || pMT->IsSharedByGenericInstantiations() || pMT->ContainsGenericVariables();
     
     DWORD nAssigned = 0;
     do
@@ -9132,7 +9137,7 @@ MethodTableBuilder::LoadExactInterfaceMap(MethodTable *pMT)
                                                                                 (const Substitution*)0,
                                                                                 retryWithExactInterfaces ? NULL : pMT).GetMethodTable();
 
-            bool uninstGenericCase = pNewIntfMT->IsSpecialMarkerTypeForGenericCasting();
+            bool uninstGenericCase = !retryWithExactInterfaces && pNewIntfMT->IsSpecialMarkerTypeForGenericCasting();
 
             duplicates |= InsertMethodTable(pNewIntfMT, pExactMTs, nInterfacesCount, &nAssigned);
 
diff --git a/src/tests/Loader/classloader/generics/Instantiation/Positive/CuriouslyRecurringThroughInterface.cs b/src/tests/Loader/classloader/generics/Instantiation/Positive/CuriouslyRecurringThroughInterface.cs
new file mode 100644
index 00000000000..e8a1139f53b
--- /dev/null
+++ b/src/tests/Loader/classloader/generics/Instantiation/Positive/CuriouslyRecurringThroughInterface.cs
@@ -0,0 +1,23 @@
+namespace CuriouslyRecurringPatternThroughInterface
+{
+    interface IGeneric<T_IGeneric>
+    {
+    }
+    interface ICuriouslyRecurring<T_ICuriouslyRecurring> : IGeneric<CuriouslyRecurringThroughInterface<T_ICuriouslyRecurring>>
+    {
+    }
+    class CuriouslyRecurringThroughInterface<T_CuriouslyRecurringThroughInterface> : ICuriouslyRecurring<T_CuriouslyRecurringThroughInterface>
+    {
+    }
+
+    class Program
+    {
+        static object _o;
+        static int Main(string[] args)
+        {
+            // Test that the a generic using a variant of the curiously recurring pattern involving an interface can be loaded.
+            _o = typeof(CuriouslyRecurringThroughInterface<int>);
+            return 100;
+        }
+    }
+}
\ No newline at end of file
diff --git a/src/tests/Loader/classloader/generics/Instantiation/Positive/CuriouslyRecurringThroughInterface.csproj b/src/tests/Loader/classloader/generics/Instantiation/Positive/CuriouslyRecurringThroughInterface.csproj
new file mode 100644
index 00000000000..b566f023697
--- /dev/null
+++ b/src/tests/Loader/classloader/generics/Instantiation/Positive/CuriouslyRecurringThroughInterface.csproj
@@ -0,0 +1,11 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <OutputType>Exe</OutputType>
+    <CLRTestKind>BuildAndRun</CLRTestKind>
+    <CLRTestPriority>1</CLRTestPriority>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="CuriouslyRecurringThroughInterface.cs" />
+  </ItemGroup>
+</Project>
\ No newline at end of file