[release/5.0] Fix covariant returns with generic return types

src/coreclr/src/vm/class.cpp

@@ -1109,20 +1109,18 @@ bool ClassLoader::IsCompatibleWith(TypeHandle hType1, TypeHandle hType2)
         return false;
     }
 
-    _ASSERTE(hType1.GetMethodTable() != NULL);
-    _ASSERTE(hType2.GetMethodTable() != NULL);
-
-    // Nullable<T> can be cast to T, but this is not compatible according to ECMA I.8.7.1
-    bool isCastFromNullableOfTtoT = hType1.GetMethodTable()->IsNullable() && hType2.IsEquivalentTo(hType1.GetMethodTable()->GetInstantiation()[0]);
-    if (isCastFromNullableOfTtoT)
+    MethodTable* pMT1 = hType1.GetMethodTable();
+    if (pMT1 != NULL)
     {
-        return false;
+        // Nullable<T> can be cast to T, but this is not compatible according to ECMA I.8.7.1
+        bool isCastFromNullableOfTtoT = pMT1->IsNullable() && hType2.IsEquivalentTo(pMT1->GetInstantiation()[0]);
+        if (isCastFromNullableOfTtoT)
+        {
+            return false;
+        }
     }
 
-    {
-        GCX_COOP();
-        return hType2.GetMethodTable()->CanCastTo(hType1.GetMethodTable(), NULL);
-    }
+    return hType2.CanCastTo(hType1, NULL);
 }
 
 /*static*/
@@ -1171,16 +1169,21 @@ void ClassLoader::ValidateMethodsWithCovariantReturnTypes(MethodTable* pMT)
             if (!pMD->RequiresCovariantReturnTypeChecking() && !pParentMD->RequiresCovariantReturnTypeChecking())
                 continue;
 
-            Instantiation classInst = pParentMD->GetClassInstantiation();
-            if (ClassLoader::IsTypicalSharedInstantiation(classInst))
+            Instantiation parentClassInst = pParentMD->GetClassInstantiation();
+            if (ClassLoader::IsTypicalSharedInstantiation(parentClassInst))
             {
-                classInst = pParentMT->GetInstantiation();
+                parentClassInst = pParentMT->GetInstantiation();
             }
-            SigTypeContext context1(classInst, pMD->GetMethodInstantiation());
+            SigTypeContext context1(parentClassInst, pMD->GetMethodInstantiation());
             MetaSig methodSig1(pParentMD);
             TypeHandle hType1 = methodSig1.GetReturnProps().GetTypeHandleThrowing(pParentMD->GetModule(), &context1, ClassLoader::LoadTypesFlag::LoadTypes, CLASS_LOAD_EXACTPARENTS);
 
-            SigTypeContext context2(pMD);
+            Instantiation classInst = pMD->GetClassInstantiation();
+            if (ClassLoader::IsTypicalSharedInstantiation(classInst))
+            {
+                classInst = pMT->GetInstantiation();
+            }
+            SigTypeContext context2(classInst, pMD->GetMethodInstantiation());
             MetaSig methodSig2(pMD);
             TypeHandle hType2 = methodSig2.GetReturnProps().GetTypeHandleThrowing(pMD->GetModule(), &context2, ClassLoader::LoadTypesFlag::LoadTypes, CLASS_LOAD_EXACTPARENTS);
 

src/coreclr/src/vm/clsload.cpp

@@ -3531,17 +3531,11 @@ static void PushFinalLevels(TypeHandle typeHnd, ClassLoadLevel targetLevel, cons
     // and on its transitive dependencies.
     if (targetLevel == CLASS_LOADED)
     {
-        if (!typeHnd.IsTypeDesc())
-        {
-            ClassLoader::ValidateMethodsWithCovariantReturnTypes(typeHnd.AsMethodTable());
-        }
-
         DFLPendingList pendingList;
         BOOL           fBailed = FALSE;
 
         typeHnd.DoFullyLoad(NULL, CLASS_LOADED, &pendingList, &fBailed, pInstContext);
 
-
         // In the case of a circular dependency, one or more types will have
         // had their promotions deferred.
         //

src/coreclr/src/vm/methodtable.cpp

@@ -5282,6 +5282,12 @@ void MethodTable::DoFullyLoad(Generics::RecursionGraph * const pVisited,  const
     CONSISTENCY_CHECK(IsRestored_NoLogging());
     CONSISTENCY_CHECK(!HasApproxParent());
 
+    if ((level == CLASS_LOADED) && !IsSharedByGenericInstantiations())
+    {
+        _ASSERTE(GetLoadLevel() >= CLASS_DEPENDENCIES_LOADED);
+        ClassLoader::ValidateMethodsWithCovariantReturnTypes(this);
+    }
+
     if (IsArray())
     {
         Generics::RecursionGraph newVisited(pVisited, TypeHandle(this));

src/coreclr/src/vm/typehandle.cpp

@@ -560,7 +560,7 @@ BOOL TypeHandle::IsBoxedAndCanCastTo(TypeHandle type, TypeHandlePairList *pPairL
         GC_TRIGGERS;
         INJECT_FAULT(COMPlusThrowOM());
 
-        LOADS_TYPE(CLASS_LOAD_EXACTPARENTS);
+        LOADS_TYPE(CLASS_DEPENDENCIES_LOADED);
 
         // The caller should check for an exact match.
         // That will cover the cast of a (unboxed) valuetype to itself.
@@ -607,7 +607,7 @@ BOOL TypeHandle::CanCastTo(TypeHandle type, TypeHandlePairList *pVisited)  const
         MODE_ANY;
         INJECT_FAULT(COMPlusThrowOM());
 
-        LOADS_TYPE(CLASS_LOAD_EXACTPARENTS);
+        LOADS_TYPE(CLASS_DEPENDENCIES_LOADED);
     }
     CONTRACTL_END
 

src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/CompatibleWithTest.il

@@ -124,6 +124,17 @@
     }
 }
 
+.class public auto ansi beforefieldinit D2 extends C2
+{
+    .method public hidebysig specialname rtspecialname instance void  .ctor() cil managed { ret }
+
+    .method public hidebysig newslot virtual instance int32 MD2()
+    {
+        ldc.i4.0
+        ret
+    }
+}
+
 .class public auto ansi beforefieldinit C3 extends C1
 {
     .method public hidebysig specialname rtspecialname instance void  .ctor() cil managed { ret } 
@@ -491,6 +502,16 @@
         ret
     }
 
+    .method public static void RunTestD2() noinlining
+    {
+        newobj instance void class D2::.ctor()
+        callvirt instance int32 class D2::MD2()
+        pop
+        ldstr "Unexpectedly succeeded"
+        call void [System.Console]System.Console::WriteLine(string)
+        ret
+    }
+
     .method public static void RunTestC3() noinlining 
     {
         newobj instance void class C3::.ctor()
@@ -796,6 +817,24 @@ CC2:
             call void Main::RunTestC2()
             ldc.i4.0
             stloc.0
+            leave.s CD2
+        }
+        catch [mscorlib]System.TypeLoadException
+        {
+            ldstr "Caught expected TypeLoadException:"
+            call       void [System.Console]System.Console::WriteLine(string)
+            call       void [System.Console]System.Console::WriteLine(object)
+            leave.s CD2
+        }
+CD2:
+        ldstr "D2: call non-overriding method MD2 when base class of D2 has invalid covariant override"
+        call void [System.Console]System.Console::WriteLine(string)
+
+        .try
+        {
+            call void Main::RunTestD2()
+            ldc.i4.0
+            stloc.0
             leave.s CC3
         }
         catch [mscorlib]System.TypeLoadException
@@ -804,7 +843,7 @@ CC2:
             call       void [System.Console]System.Console::WriteLine(string)
             call       void [System.Console]System.Console::WriteLine(object)        
             leave.s CC3
-        }  
+        }
 CC3:
         ldstr "C3: override IList<int32> by int32[]"
         call void [System.Console]System.Console::WriteLine(string)

src/tests/Regressions/coreclr/GitHub_45037/test45037.cs

@@ -0,0 +1,54 @@
+using System;
+using System.Collections.Generic;
+using System.Reflection;
+
+public abstract class Base<T>
+{
+   public virtual T Get() => throw new NotImplementedException();
+}
+
+public sealed class CovariantReturn : Base<object>
+{
+   public override string Get() => throw new NotImplementedException();
+}
+
+public abstract class ABase
+{
+    public abstract object this[int index] { get; }
+}
+
+public sealed class Concrete<T> : ABase
+    where T : class
+{
+    public override T this[int index]
+    {
+        get
+        {
+            throw null;
+        }
+    }
+}
+
+class Parent
+{
+    public virtual object Value { get; }
+}
+
+class Child<T> : Parent where T : class
+{
+    public override T Value { get => (T)base.Value; }
+}
+
+class Foo { }
+
+class Program
+{
+    static int Main()
+    {
+        Type[] t = Assembly.GetExecutingAssembly().GetTypes();
+        new Child<Foo>();
+        new CovariantReturn();
+
+        return 100;
+    }
+}

src/tests/Regressions/coreclr/GitHub_45037/test45037.csproj

@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <CLRTestKind>BuildAndRun</CLRTestKind>
+    <CLRTestPriority>1</CLRTestPriority>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="test45037.cs" />
+  </ItemGroup>
+</Project>

src/tests/Regressions/coreclr/GitHub_45082/test45082.cs

@@ -0,0 +1,26 @@
+using System;
+using System.Collections.Generic;
+
+public abstract class AComponent { }
+public class Component : AComponent { }
+
+public abstract class Abstract
+{
+    public abstract IReadOnlyList<AComponent> New { get; }
+}
+
+public sealed class Concrete<T> : Abstract
+    where T : AComponent
+{
+    public override IReadOnlyList<T> New => throw null;
+}
+
+class Program
+{
+    static int Main()
+    {
+        new Concrete<Component>();
+
+        return 100;
+    }
+}

src/tests/Regressions/coreclr/GitHub_45082/test45082.csproj

@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <CLRTestKind>BuildAndRun</CLRTestKind>
+    <CLRTestPriority>1</CLRTestPriority>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="test45082.cs" />
+  </ItemGroup>
+</Project>