Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
1d77e2f
Reduce constraint loading by not forcing constraints to be loaded eag…
davidwrighton Oct 6, 2025
bb4a3d2
Clear up the issues around GetDeclaringMethodTableFromTypeVarTypeDesc
davidwrighton Oct 6, 2025
db83d47
Tweak, and add comments...
davidwrighton Oct 7, 2025
c0ae70d
Fix build errors
davidwrighton Oct 7, 2025
1f742a3
Fix bugs making it not work properly
davidwrighton Oct 7, 2025
4e4a395
Rework the special instantiation type logic to be more effective
davidwrighton Dec 1, 2023
bef91a8
Fix interface matching bug found in testing
davidwrighton Oct 7, 2025
2bdfc3c
Optimize virtual static and default interface method resolution logic
davidwrighton Oct 8, 2025
3b937fd
Update loader/classloader/generics/ByRefLike/ValidateNegative test
davidwrighton Oct 8, 2025
64d10ff
Pass around the type I want passed around
davidwrighton Oct 8, 2025
9eef0b3
Remove WhichConstraintsToLoad::TypeOrMethodVarsOnly
davidwrighton Oct 8, 2025
4a6a81c
Fix out of date comment
davidwrighton Oct 8, 2025
18214f8
Merge branch 'main' into reduce_constraint_load
davidwrighton Oct 9, 2025
533e8ca
- Add support for doing circularity checks on type and method generic…
davidwrighton Oct 10, 2025
ebca7a6
Code review feedback
davidwrighton Oct 10, 2025
9d8b167
- Flesh out implementation of sig parser for constraint accessibility
davidwrighton Oct 10, 2025
d5fad7d
- Add crossgen2 validation of interface impl variance rules
davidwrighton Oct 13, 2025
12511ff
- Adjust contracts as suggested
davidwrighton Oct 13, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}
}
}
6 changes: 6 additions & 0 deletions src/coreclr/vm/ceeload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
8 changes: 8 additions & 0 deletions src/coreclr/vm/ceeload.h
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,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;
Expand Down Expand Up @@ -1500,6 +1502,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
{
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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++)
{
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/vm/comdelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2403,16 +2403,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.
//
Expand Down
49 changes: 11 additions & 38 deletions src/coreclr/vm/genmeth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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;
Expand Down Expand Up @@ -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))
Expand Down
Loading
Loading