[release/10.0] Reduce types loaded on startup significantly

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Reflection;
 using System.Text;
@@ -133,6 +134,18 @@ Task<bool> ValidateTypeWorkerHelper(TypeDesc typeToCheckForSkipValidation)
                         AddTypeValidationError(type, $"Interface type {interfaceType} failed validation");
                         return false;
                     }
+
+                    if (!interfaceType.IsInterface)
+                    {
+                        AddTypeValidationError(type, $"Runtime interface {interfaceType} is not an interface");
+                        return false;
+                    }
+
+                    if (interfaceType.HasInstantiation)
+                    {
+                        // Interfaces behave covariantly
+                        ValidateVarianceInType(type.Instantiation, interfaceType, Internal.TypeSystem.GenericVariance.Covariant);
+                    }
                 }
 
                 // Validate that each interface type explicitly implemented on this type is accessible to this type -- UNIMPLEMENTED
@@ -293,14 +306,76 @@ Task<bool> ValidateTypeWorkerHelper(TypeDesc typeToCheckForSkipValidation)
                         AddTypeValidationError(type, $"'{method}' is an runtime-impl generic method");
                         return false;
                     }
-                    // Validate that generic variance is properly respected in method signatures -- UNIMPLEMENTED
-                    // Validate that there are no cyclical method constraints -- UNIMPLEMENTED
+
+
+                    // Validate that generic variance is properly respected in method signatures
+                    if (type.IsInterface && method.IsVirtual && !method.Signature.IsStatic && type.HasInstantiation)
+                    {
+                        if (!ValidateVarianceInSignature(type.Instantiation, method.Signature, Internal.TypeSystem.GenericVariance.Covariant, Internal.TypeSystem.GenericVariance.Contravariant))
+                        {
+                            AddTypeValidationError(type, $"'{method}' has invalid variance in signature");
+                            return false;
+                        }
+                    }
+
+                    foreach (var genericParameterType in method.Instantiation)
+                    {
+                        foreach (var constraint in ((GenericParameterDesc)genericParameterType).TypeConstraints)
+                        {
+                            if (!ValidateVarianceInType(type.Instantiation, constraint, Internal.TypeSystem.GenericVariance.Contravariant))
+                            {
+                                AddTypeValidationError(type, $"'{method}' has invalid variance in generic parameter constraint {constraint} on type {genericParameterType}");
+                                return false;
+                            }
+                        }
+                    }
+
+                    if (!BoundedCheckForInstantiation(method.Instantiation))
+                    {
+                        AddTypeValidationError(type, $"'{method}' has cyclical generic parameter constraints");
+                        return false;
+                    }
                     // Validate that constraints are all acccessible to the method using them -- UNIMPLEMENTED
                 }
 
                 // Generic class special rules
                 // Validate that a generic class cannot be a ComImport class, or a ComEventInterface class -- UNIMPLEMENTED
-                // Validate that there are no cyclical class or method constraints, and that constraints are all acccessible to the type using them -- UNIMPLEMENTED
+
+                foreach (var genericParameterType in type.Instantiation)
+                {
+                    foreach (var constraint in ((GenericParameterDesc)genericParameterType).TypeConstraints)
+                    {
+                        if (!ValidateVarianceInType(type.Instantiation, constraint, Internal.TypeSystem.GenericVariance.Contravariant))
+                        {
+                            AddTypeValidationError(type, $"'{genericParameterType}' has invalid variance in generic parameter constraint {constraint}");
+                            return false;
+                        }
+                    }
+                }
+
+                // Validate that generic variance is properly respected in interface signatures
+                if (type.HasInstantiation && type.IsInterface)
+                {
+                    foreach (var interfaceType in type.ExplicitlyImplementedInterfaces)
+                    {
+                        if (interfaceType.HasInstantiation)
+                        {
+                            // Interfaces behave covariantly
+                            if (!ValidateVarianceInType(type.Instantiation, interfaceType, Internal.TypeSystem.GenericVariance.Covariant))
+                            {
+                                AddTypeValidationError(type, $"'{type}' has invalid variance in in interface impl of {interfaceType}");
+                                return false;
+                            }
+                        }
+                    }
+                }
+
+                // Validate that there are no cyclical class or method constraints, and that constraints are all acccessible to the type using them
+                if (!BoundedCheckForInstantiation(type.Instantiation))
+                {
+                    AddTypeValidationError(type, $"'{type}' has cyclical generic parameter constraints");
+                    return false;
+                }
 
                 // Override rules
                 // Validate that each override results does not violate accessibility rules -- UNIMPLEMENTED
@@ -579,6 +654,114 @@ async Task<bool> ValidateTypeHelperFunctionPointerType(FunctionPointerType funct
                 }
                 return true;
             }
+
+            static bool BoundedCheckForInstantiation(Instantiation instantiation)
+            {
+                foreach (var type in instantiation)
+                {
+                    Debug.Assert(type.IsGenericParameter);
+                    GenericParameterDesc genericParameter = (GenericParameterDesc)type;
+
+                    if (!BoundedCheckForType(genericParameter, instantiation.Length))
+                        return false;
+                }
+                return true;
+            }
+
+            static bool BoundedCheckForType(GenericParameterDesc genericParameter, int maxDepth)
+            {
+                if (maxDepth <= 0)
+                    return false;
+                foreach (var type in genericParameter.TypeConstraints)
+                {
+                    if (type is GenericParameterDesc possiblyCircularGenericParameter)
+                    {
+                        if (possiblyCircularGenericParameter.Kind == genericParameter.Kind)
+                        {
+                            if (!BoundedCheckForType(possiblyCircularGenericParameter, maxDepth - 1))
+                                return false;
+                        }
+                    }
+                }
+                return true;
+            }
+
+            static bool ValidateVarianceInSignature(Instantiation associatedTypeInstantiation, MethodSignature signature, Internal.TypeSystem.GenericVariance returnTypePosition, Internal.TypeSystem.GenericVariance argTypePosition)
+            {
+                for (int i = 0; i < signature.Length; i++)
+                {
+                    if (!ValidateVarianceInType(associatedTypeInstantiation, signature[i], argTypePosition))
+                        return false;
+                }
+
+                if (!ValidateVarianceInType(associatedTypeInstantiation, signature.ReturnType, returnTypePosition))
+                    return false;
+
+                return true;
+            }
+
+            static bool ValidateVarianceInType(Instantiation associatedTypeInstantiation, TypeDesc type, Internal.TypeSystem.GenericVariance position)
+            {
+                if (type is SignatureTypeVariable signatureTypeVar)
+                {
+                    GenericParameterDesc gp = (GenericParameterDesc)associatedTypeInstantiation[signatureTypeVar.Index];
+                    if (gp.Variance != Internal.TypeSystem.GenericVariance.None)
+                    {
+                        if (position != gp.Variance)
+                        {
+                            // Covariant and contravariant parameters can *only* appear in resp. covariant and contravariant positions
+                            return false;
+                        }
+                    }
+                }
+                else if (type is InstantiatedType instantiatedType)
+                {
+                    var typeDef = instantiatedType.GetTypeDefinition();
+                    if (typeDef.IsValueType || position == Internal.TypeSystem.GenericVariance.None)
+                    {
+                        // Value types and non-variant positions require all parameters to be non-variant
+                        foreach (TypeDesc instType in instantiatedType.Instantiation)
+                        {
+                            if (!ValidateVarianceInType(associatedTypeInstantiation, instType, Internal.TypeSystem.GenericVariance.None))
+                                return false;
+                        }
+                    }
+                    else
+                    {
+                        int index = 0;
+                        for (index = 0; index < typeDef.Instantiation.Length; index++)
+                        {
+                            Internal.TypeSystem.GenericVariance positionForParameter = ((GenericParameterDesc)typeDef.Instantiation[index]).Variance;
+                            // If the surrounding context is contravariant then we need to flip the variance of this parameter
+                            if (position == Internal.TypeSystem.GenericVariance.Contravariant)
+                            {
+                                if (positionForParameter == Internal.TypeSystem.GenericVariance.Covariant)
+                                    positionForParameter = Internal.TypeSystem.GenericVariance.Contravariant;
+                                else if (positionForParameter == Internal.TypeSystem.GenericVariance.Contravariant)
+                                    positionForParameter = Internal.TypeSystem.GenericVariance.Covariant;
+                                else
+                                    positionForParameter = Internal.TypeSystem.GenericVariance.None;
+                            }
+
+                            if (!ValidateVarianceInType(associatedTypeInstantiation, instantiatedType.Instantiation[index], positionForParameter))
+                                return false;
+                        }
+                    }
+                }
+                else if (type is ParameterizedType parameterizedType)
+                {
+                    // Arrays behave as covariant, byref and pointer types are non-variant
+                    if (!ValidateVarianceInType(associatedTypeInstantiation, parameterizedType.ParameterType, (parameterizedType.IsByRef || parameterizedType.IsPointer) ? Internal.TypeSystem.GenericVariance.None : position))
+                        return false;
+                }
+                else if (type is FunctionPointerType functionPointerType)
+                {
+                    if (!ValidateVarianceInSignature(associatedTypeInstantiation, functionPointerType.Signature, Internal.TypeSystem.GenericVariance.None, Internal.TypeSystem.GenericVariance.None))
+                        return false;
+                }
+
+                return true;
+            }
         }
     }
 }

src/coreclr/vm/ceeload.cpp

@@ -434,10 +434,16 @@ void Module::Initialize(AllocMemTracker *pamTracker, LPCWSTR szName)
     AllocateMaps();
     m_dwTransientFlags &= ~((DWORD)CLASSES_FREED);  // Set flag indicating LookupMaps are now in a consistent and destructable state
 
+    if (IsSystem())
+        m_dwPersistedFlags |= SKIP_TYPE_VALIDATION; // Skip type validation on System
+
 #ifdef FEATURE_READYTORUN
     m_pNativeImage = NULL;
     if ((m_pReadyToRunInfo = ReadyToRunInfo::Initialize(this, pamTracker)) != NULL)
     {
+        if (m_pReadyToRunInfo->SkipTypeValidation())
+            m_dwPersistedFlags |= SKIP_TYPE_VALIDATION; // Skip type validation on System
+
         m_pNativeImage = m_pReadyToRunInfo->GetNativeImage();
         if (m_pNativeImage != NULL)
         {

src/coreclr/vm/ceeload.h

@@ -676,6 +676,8 @@ class Module : public ModuleBase
         RUNTIME_MARSHALLING_ENABLED_IS_CACHED = 0x00008000,
         //If runtime marshalling is enabled for this assembly
         RUNTIME_MARSHALLING_ENABLED = 0x00010000,
+
+        SKIP_TYPE_VALIDATION = 0x00020000,
     };
 
     Volatile<DWORD>          m_dwTransientFlags;
@@ -1490,6 +1492,12 @@ class Module : public ModuleBase
 #endif
     }
 
+    bool SkipTypeValidation() const
+    {
+        LIMITED_METHOD_DAC_CONTRACT;
+
+        return (m_dwPersistedFlags & SKIP_TYPE_VALIDATION) != 0;
+    }
 #ifdef FEATURE_READYTORUN
     PTR_ReadyToRunInfo GetReadyToRunInfo() const
     {

src/coreclr/vm/class.cpp

@@ -941,7 +941,7 @@ EEClass::CheckVarianceInSig(
                 uint32_t cArgs;
                 IfFailThrow(psig.GetData(&cArgs));
 
-                // Conservatively, assume non-variance of function pointer types
+                // Conservatively, assume non-variance of function pointer types, if we ever change this, update the TypeValidationChecker in crossgen2 also
                 if (!CheckVarianceInSig(numGenericArgs, pVarianceInfo, pModule, psig, gpNonVariant))
                     return FALSE;
 
@@ -1359,7 +1359,7 @@ void ClassLoader::ValidateMethodsWithCovariantReturnTypes(MethodTable* pMT)
         return;
 
     // Step 1: Validate compatibility of return types on overriding methods
-    if (pMT->GetClass()->HasCovariantOverride() && (!pMT->GetModule()->IsReadyToRun() || !pMT->GetModule()->GetReadyToRunInfo()->SkipTypeValidation()))
+    if (pMT->GetClass()->HasCovariantOverride() && !pMT->GetModule()->SkipTypeValidation())
     {
         for (WORD i = 0; i < pParentMT->GetNumVirtuals(); i++)
         {

src/coreclr/vm/comdelegate.cpp

@@ -2406,16 +2406,16 @@ static bool IsLocationAssignable(TypeHandle fromHandle, TypeHandle toHandle, boo
                 // We need to check whether constraints of fromHandle have been loaded, because the
                 // CanCastTo operation might have made its decision without enumerating constraints
                 // (e.g. when toHandle is System.Object).
-                if (!fromHandleVar->ConstraintsLoaded())
-                    fromHandleVar->LoadConstraints(CLASS_DEPENDENCIES_LOADED);
+                if (!fromHandleVar->ConstraintsLoaded(WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly))
+                    fromHandleVar->LoadConstraints(CLASS_DEPENDENCIES_LOADED, WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly);
 
                 if (toHandle.IsGenericVariable())
                 {
                     TypeVarTypeDesc *toHandleVar = toHandle.AsGenericVariable();
 
                     // Constraints of toHandleVar were not touched by CanCastTo.
-                    if (!toHandleVar->ConstraintsLoaded())
-                        toHandleVar->LoadConstraints(CLASS_DEPENDENCIES_LOADED);
+                    if (!toHandleVar->ConstraintsLoaded(WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly))
+                        toHandleVar->LoadConstraints(CLASS_DEPENDENCIES_LOADED, WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly);
 
                     // Both handles are type variables. The following table lists all possible combinations.
                     //

src/coreclr/vm/genmeth.cpp

@@ -1536,7 +1536,7 @@ BOOL Bounded(TypeVarTypeDesc *tyvar, DWORD depth) {
     }
 
     DWORD numConstraints;
-    TypeHandle *constraints = tyvar->GetConstraints(&numConstraints, CLASS_DEPENDENCIES_LOADED);
+    TypeHandle *constraints = tyvar->GetConstraints(&numConstraints, CLASS_DEPENDENCIES_LOADED, WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly);
     for (unsigned i = 0; i < numConstraints; i++)
     {
         TypeHandle constraint = constraints[i];
@@ -1554,56 +1554,31 @@ BOOL Bounded(TypeVarTypeDesc *tyvar, DWORD depth) {
     return TRUE;
 }
 
-void MethodDesc::LoadConstraintsForTypicalMethodDefinition(BOOL *pfHasCircularClassConstraints, BOOL *pfHasCircularMethodConstraints, ClassLoadLevel level/* = CLASS_LOADED*/)
+void MethodDesc::CheckConstraintMetadataValidity(BOOL *pfHasCircularMethodConstraints)
 {
     CONTRACTL {
-        THROWS;
-        GC_TRIGGERS;
-        MODE_ANY;
+        STANDARD_VM_CHECK;
         PRECONDITION(IsTypicalMethodDefinition());
-        PRECONDITION(CheckPointer(pfHasCircularClassConstraints));
         PRECONDITION(CheckPointer(pfHasCircularMethodConstraints));
     } CONTRACTL_END;
 
-    *pfHasCircularClassConstraints = FALSE;
+    // In this function we explicitly check for accessibility of method type parameter constraints as
+    // well as explicitly do a check for circularity among method type parameter constraints.
+    //
+    // For checking the variance of the constraints we rely on the fact that both DoAccessibilityCheckForConstraints
+    // and Bounded will call GetConstraints on the type variables, which will in turn call
+    // LoadConstraints, and LoadConstraints will do the variance checking using EEClass::CheckVarianceInSig
     *pfHasCircularMethodConstraints = FALSE;
 
-    // Force a load of the constraints on the type parameters
-    Instantiation classInst = GetClassInstantiation();
-    for (DWORD i = 0; i < classInst.GetNumArgs(); i++)
-    {
-        TypeVarTypeDesc* tyvar = classInst[i].AsGenericVariable();
-        _ASSERTE(tyvar != NULL);
-        tyvar->LoadConstraints(level);
-    }
-
     Instantiation methodInst = GetMethodInstantiation();
-    for (DWORD i = 0; i < methodInst.GetNumArgs(); i++)
-    {
-        TypeVarTypeDesc* tyvar = methodInst[i].AsGenericVariable();
-        _ASSERTE(tyvar != NULL);
-        tyvar->LoadConstraints(level);
-
-        VOID DoAccessibilityCheckForConstraints(MethodTable *pAskingMT, TypeVarTypeDesc *pTyVar, UINT resIDWhy);
-        DoAccessibilityCheckForConstraints(GetMethodTable(), tyvar, E_ACCESSDENIED);
-    }
-
-    // reject circular class constraints
-    for (DWORD i = 0; i < classInst.GetNumArgs(); i++)
-    {
-        TypeVarTypeDesc* tyvar = classInst[i].AsGenericVariable();
-        _ASSERTE(tyvar != NULL);
-        if(!Bounded(tyvar, classInst.GetNumArgs()))
-        {
-            *pfHasCircularClassConstraints = TRUE;
-        }
-    }
 
     // reject circular method constraints
     for (DWORD i = 0; i < methodInst.GetNumArgs(); i++)
     {
         TypeVarTypeDesc* tyvar = methodInst[i].AsGenericVariable();
         _ASSERTE(tyvar != NULL);
+        VOID DoAccessibilityCheckForConstraints(MethodTable *pAskingMT, TypeVarTypeDesc *pTyVar, UINT resIDWhy);
+        DoAccessibilityCheckForConstraints(GetMethodTable(), tyvar, E_ACCESSDENIED);
         if(!Bounded(tyvar, methodInst.GetNumArgs()))
         {
             *pfHasCircularMethodConstraints = TRUE;
@@ -1662,8 +1637,6 @@ BOOL MethodDesc::SatisfiesMethodConstraints(TypeHandle thParent, BOOL fThrowIfNo
         _ASSERTE(tyvar != NULL);
         _ASSERTE(TypeFromToken(tyvar->GetTypeOrMethodDef()) == mdtMethodDef);
 
-        tyvar->LoadConstraints(); //TODO: is this necessary for anything but the typical method?
-
         // Pass in the InstatiationContext so constraints can be correctly evaluated
         // if this is an instantiation where the type variable is in its open position
         if (!tyvar->SatisfiesConstraints(&typeContext,thArg, typicalInstMatchesMethodInst ? &instContext : NULL))