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

diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp
index 834d3cfc8c40bb05e5678895e57a727fad31cfa8..8bdc5f0c2a892ba3932d6033d6764017c47f07b4 100644 (file)
--- 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 4063e50b7b61c71de5ec3d13ae5344903b1c51f6..df712a378d4ecee95249e777d91ac79abafac073 100644 (file)
--- 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 a3adc702cbe3d6f33c9722512694f4bf5b39e381..b1af313a29556bd3a13c552b5b33a6a93a2c3bee 100644 (file)
--- 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 89926ffdaae020485e2d4969f78d4c1208079542..154c642cf40d71c0e4cd4ba326e48a272ed6875b 100644 (file)
--- 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 (file)
index 0000000..e8a1139
--- /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 (file)
index 0000000..b566f02
--- /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