diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs index e6ef9cad2c10f9..57dd3bbea81924 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs +++ b/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 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 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 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; + } } } } diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index adfe056bdbe20b..5bd1b6e7ff4cf7 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/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) { diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index 3cea27da8fbcee..c203207e25c1ce 100644 --- a/src/coreclr/vm/ceeload.h +++ b/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 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 { diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index a96bcffae4fe1a..9209e652830c02 100644 --- a/src/coreclr/vm/class.cpp +++ b/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++) { diff --git a/src/coreclr/vm/comdelegate.cpp b/src/coreclr/vm/comdelegate.cpp index cc9535ca13f6ed..30da943a8317ce 100644 --- a/src/coreclr/vm/comdelegate.cpp +++ b/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. // diff --git a/src/coreclr/vm/genmeth.cpp b/src/coreclr/vm/genmeth.cpp index f1d9129f62e13a..492ea84fe7e7f5 100644 --- a/src/coreclr/vm/genmeth.cpp +++ b/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)) diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 7a23d0cd0b6325..4cb42eaa6e23c6 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -479,11 +479,8 @@ class MethodDesc // This method can be called on a non-restored method desc BOOL IsTypicalMethodDefinition() const; - // Force a load of the (typical) constraints on the type parameters of a typical method definition, - // detecting cyclic bounds on class and method type parameters. - void LoadConstraintsForTypicalMethodDefinition(BOOL *pfHasCircularClassConstraints, - BOOL *pfHasCircularMethodConstraints, - ClassLoadLevel level = CLASS_LOADED); + // Validate accessibility and usage of variant generic parameters and check for cycles in the method constraints + void CheckConstraintMetadataValidity(BOOL *pfHasCircularMethodConstraints); DWORD IsClassConstructor() { diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 722383e058dd7f..9729e9e9318e99 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -1265,7 +1265,7 @@ BOOL MethodTable::CanCastByVarianceToInterfaceOrDelegate(MethodTable *pTargetMT, IsSpecialMarkerTypeForGenericCasting() && GetTypeDefRid() == pTargetMT->GetTypeDefRid() && GetModule() == pTargetMT->GetModule() && - pTargetMT->GetInstantiation().ContainsAllOneType(pMTInterfaceMapOwner)) + pTargetMT->GetInstantiation().ContainsAllOneType(pMTInterfaceMapOwner->GetSpecialInstantiationType())) { return TRUE; } @@ -1290,7 +1290,7 @@ BOOL MethodTable::CanCastByVarianceToInterfaceOrDelegate(MethodTable *pTargetMT, TypeHandle thArg = inst[i]; if (IsSpecialMarkerTypeForGenericCasting() && pMTInterfaceMapOwner && !pMTInterfaceMapOwner->GetAuxiliaryData()->MayHaveOpenInterfacesInInterfaceMap()) { - thArg = pMTInterfaceMapOwner; + thArg = pMTInterfaceMapOwner->GetSpecialInstantiationType(); } TypeHandle thTargetArg = targetInst[i]; @@ -1753,6 +1753,63 @@ NOINLINE BOOL MethodTable::ImplementsInterface(MethodTable *pInterface) return ImplementsInterfaceInline(pInterface); } +bool MethodTable::InterfaceMapIterator::CurrentInterfaceEquivalentTo(MethodTable* pMTOwner, MethodTable* pMT) +{ + CONTRACTL + { + THROWS; + GC_TRIGGERS; + PRECONDITION(pMT->IsInterface()); // class we are looking up should be an interface + } + CONTRACTL_END; + + MethodTable *pCurrentMethodTable = m_pMap->GetMethodTable(); + + if (pCurrentMethodTable == pMT) + return true; + + if (pCurrentMethodTable->IsSpecialMarkerTypeForGenericCasting() && !pMTOwner->GetAuxiliaryData()->MayHaveOpenInterfacesInInterfaceMap() && pCurrentMethodTable->HasSameTypeDefAs(pMT)) + { + // Any matches need to use the special marker type logic + if (!pMTOwner->GetAuxiliaryData()->MayHaveOpenInterfacesInInterfaceMap()) + { + TypeHandle pSpecialInstantiationType = pCurrentMethodTable->GetSpecialInstantiationType(); + // If we reach here, we are trying to do a compare with a value in the interface map which is a special marker type + // First check for exact match. + if (pMT->GetInstantiation().ContainsAllOneType(pSpecialInstantiationType)) + { + // We match exactly, and have an actual pMT loaded. Insert + // the searched for interface if it is fully loaded, so that + // future checks are more efficient +#ifndef DACCESS_COMPILE + if (pMT->IsFullyLoaded()) + SetInterface(pMT); +#endif + return true; + } + else + { + // We don't match exactly, but we may still be equivalent + for (DWORD i = 0; i < pMT->GetNumGenericArgs(); i++) + { + TypeHandle arg = pMT->GetInstantiation()[i]; + if (!arg.IsEquivalentTo(pSpecialInstantiationType)) + { + return false; + } + } + return true; + } + } + return false; + } + else + { + // We don't have a special marker type in the interface map, so we need to do the normal equivalence check + return pCurrentMethodTable->IsEquivalentTo(pMT); + } +} + //========================================================================================== BOOL MethodTable::ImplementsEquivalentInterface(MethodTable *pInterface) { @@ -1778,16 +1835,12 @@ BOOL MethodTable::ImplementsEquivalentInterface(MethodTable *pInterface) if (numInterfaces == 0) return FALSE; - InterfaceInfo_t *pInfo = GetInterfaceMap(); - - do + InterfaceMapIterator it = IterateInterfaceMap(); + while (it.Next()) { - if (pInfo->GetMethodTable()->IsEquivalentTo(pInterface)) + if (it.CurrentInterfaceEquivalentTo(this, pInterface)) return TRUE; - - pInfo++; } - while (--numInterfaces); return FALSE; } @@ -4165,12 +4218,7 @@ static VOID DoAccessibilityCheck(MethodTable *pAskingMT, MethodTable *pTargetMT, VOID DoAccessibilityCheckForConstraint(MethodTable *pAskingMT, TypeHandle thConstraint, UINT resIDWhy) { - CONTRACTL - { - THROWS; - GC_TRIGGERS; - } - CONTRACTL_END; + STANDARD_VM_CONTRACT; if (thConstraint.IsArray()) { @@ -4204,22 +4252,181 @@ VOID DoAccessibilityCheckForConstraint(MethodTable *pAskingMT, TypeHandle thCons } -VOID DoAccessibilityCheckForConstraints(MethodTable *pAskingMT, TypeVarTypeDesc *pTyVar, UINT resIDWhy) +VOID DoAccessibilityCheckForConstraintSignature(Module *pModule, SigPointer *pSigPtr, MethodTable *pAskingMT, UINT resIDWhy) { - CONTRACTL + STANDARD_VM_CONTRACT; + + CorElementType elemType; + IfFailThrow(pSigPtr->GetElemType(&elemType)); + switch (elemType) { - THROWS; - GC_TRIGGERS; + case ELEMENT_TYPE_CLASS: + case ELEMENT_TYPE_VALUETYPE: + { + mdToken typeDefOrRef; + IfFailThrow(pSigPtr->GetToken(&typeDefOrRef)); + + TypeHandle th = ClassLoader::LoadTypeDefOrRefThrowing(pModule, typeDefOrRef, ClassLoader::ThrowIfNotFound, ClassLoader::PermitUninstDefOrRef, tdNoTypes, CLASS_DEPENDENCIES_LOADED); + DoAccessibilityCheckForConstraint(pAskingMT, th, resIDWhy); + break; + } + case ELEMENT_TYPE_GENERICINST: + { + IfFailThrow(pSigPtr->GetElemType(&elemType)); + if (!(elemType == ELEMENT_TYPE_CLASS || elemType == ELEMENT_TYPE_VALUETYPE)) + { + COMPlusThrow(kTypeLoadException, IDS_CLASSLOAD_BADFORMAT); + } + mdToken typeDefOrRef; + IfFailThrow(pSigPtr->GetToken(&typeDefOrRef)); + TypeHandle th = ClassLoader::LoadTypeDefOrRefThrowing(pModule, typeDefOrRef, ClassLoader::ThrowIfNotFound, ClassLoader::PermitUninstDefOrRef, tdNoTypes, CLASS_DEPENDENCIES_LOADED); + DoAccessibilityCheckForConstraint(pAskingMT, th, resIDWhy); + uint32_t numArgs; + IfFailThrow(pSigPtr->GetData(&numArgs)); + for (uint32_t i = 0; i < numArgs; i++) + { + DoAccessibilityCheckForConstraintSignature(pModule, pSigPtr, pAskingMT, resIDWhy); + } + break; + } + case ELEMENT_TYPE_SZARRAY: + case ELEMENT_TYPE_BYREF: + case ELEMENT_TYPE_PTR: + DoAccessibilityCheckForConstraintSignature(pModule, pSigPtr, pAskingMT, resIDWhy); + break; + case ELEMENT_TYPE_ARRAY: + { + IfFailThrow(pSigPtr->GetElemType(&elemType)); + DoAccessibilityCheckForConstraintSignature(pModule, pSigPtr, pAskingMT, resIDWhy); + uint32_t rank; + IfFailThrow(pSigPtr->GetData(&rank)); + uint32_t numSizes; + IfFailThrow(pSigPtr->GetData(&numSizes)); + for (ULONG i = 0; i < numSizes; i++) + { + uint32_t size; + IfFailThrow(pSigPtr->GetData(&size)); + } + uint32_t numLoBounds; + IfFailThrow(pSigPtr->GetData(&numLoBounds)); + for (uint32_t i = 0; i < numLoBounds; i++) + { + uint32_t loBound; + IfFailThrow(pSigPtr->GetData(&loBound)); + } + break; + } + case ELEMENT_TYPE_FNPTR: + { + uint32_t uCallConv = 0; + IfFailThrow(pSigPtr->GetData(&uCallConv)); + + if ((uCallConv & IMAGE_CEE_CS_CALLCONV_GENERIC) != 0) + { + // Generic function pointers are not allowed. + ThrowHR(COR_E_BADIMAGEFORMAT); + } + + // Get the arg count. + uint32_t cArgs = 0; + IfFailThrow(pSigPtr->GetData(&cArgs)); + + // Loop for cArgs + 1 to handle the return type and all the args + for (uint32_t i = 0; i <= cArgs; i++) + { + DoAccessibilityCheckForConstraintSignature(pModule, pSigPtr, pAskingMT, resIDWhy); + } + break; + } + case ELEMENT_TYPE_VOID: + case ELEMENT_TYPE_BOOLEAN: + case ELEMENT_TYPE_CHAR: + case ELEMENT_TYPE_I1: + case ELEMENT_TYPE_U1: + case ELEMENT_TYPE_I2: + case ELEMENT_TYPE_U2: + case ELEMENT_TYPE_I4: + case ELEMENT_TYPE_U4: + case ELEMENT_TYPE_I8: + case ELEMENT_TYPE_U8: + case ELEMENT_TYPE_R4: + case ELEMENT_TYPE_R8: + case ELEMENT_TYPE_I: + case ELEMENT_TYPE_U: + case ELEMENT_TYPE_OBJECT: + case ELEMENT_TYPE_STRING: + case ELEMENT_TYPE_TYPEDBYREF: + // Primitive types and such. Nothing to check + break; + + case ELEMENT_TYPE_VAR: + case ELEMENT_TYPE_MVAR: + { + // A generic variable. Its always accessible, but we do need to parse it + uint32_t varNumber; + IfFailThrow(pSigPtr->GetData(&varNumber)); + break; + } + + default: + // Unknown element type, bad format + ThrowHR(COR_E_BADIMAGEFORMAT); + break; } - CONTRACTL_END; +} + + +VOID DoAccessibilityCheckForConstraints(MethodTable *pAskingMT, TypeVarTypeDesc *pTyVar, UINT resIDWhy) +{ + STANDARD_VM_CONTRACT; DWORD numConstraints; - TypeHandle *pthConstraints = pTyVar->GetCachedConstraints(&numConstraints); + TypeHandle *pthConstraints = pTyVar->GetConstraints(&numConstraints, CLASS_DEPENDENCIES_LOADED, WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly); for (DWORD cidx = 0; cidx < numConstraints; cidx++) { TypeHandle thConstraint = pthConstraints[cidx]; - DoAccessibilityCheckForConstraint(pAskingMT, thConstraint, resIDWhy); + if (thConstraint.IsNull()) + { + // This is a constraint which we didn't load above. Instead of doing the full load, just iterate the signature of the constraint to make sure all of the TypeDefs/Refs are accessible + Module *pModule = pTyVar->GetModule(); + + IMDInternalImport* pInternalImport = pModule->GetMDImport(); + HENUMInternalHolder hEnum(pInternalImport); + mdGenericParamConstraint tkConstraint; + hEnum.EnumInit(mdtGenericParamConstraint, pTyVar->GetToken()); + DWORD i = 0; + while (pInternalImport->EnumNext(&hEnum, &tkConstraint)) + { + _ASSERTE(i <= numConstraints); + if (i == cidx) + { + // We've found the constraint we're looking for. + mdToken tkConstraintType, tkParam; + if (FAILED(pInternalImport->GetGenericParamConstraintProps(tkConstraint, &tkParam, &tkConstraintType))) + { + ThrowHR(COR_E_BADIMAGEFORMAT); + } + _ASSERTE(tkParam == pTyVar->GetToken()); + + _ASSERTE(TypeFromToken(tkConstraintType) == mdtTypeSpec); + ULONG cSig; + PCCOR_SIGNATURE pSig; + + if (FAILED(pInternalImport->GetTypeSpecFromToken(tkConstraintType, &pSig, &cSig))) + { + ThrowHR(COR_E_BADIMAGEFORMAT); + } + SigPointer sigPtr(pSig, cSig); + DoAccessibilityCheckForConstraintSignature(pModule, &sigPtr, pAskingMT, resIDWhy); + break; + } + } + } + else + { + DoAccessibilityCheckForConstraint(pAskingMT, thConstraint, resIDWhy); + } } } @@ -4387,13 +4594,11 @@ void MethodTable::DoFullyLoad(Generics::RecursionGraph * const pVisited, const bool fNeedsSanityChecks = true; -#ifdef FEATURE_READYTORUN Module * pModule = GetModule(); // No sanity checks for ready-to-run compiled images if possible - if (pModule->IsSystem() || (pModule->IsReadyToRun() && pModule->GetReadyToRunInfo()->SkipTypeValidation())) + if (pModule->SkipTypeValidation()) fNeedsSanityChecks = false; -#endif bool fNeedAccessChecks = (level == CLASS_LOADED) && fNeedsSanityChecks && @@ -4579,10 +4784,11 @@ void MethodTable::DoFullyLoad(Generics::RecursionGraph * const pVisited, const for (DWORD i = 0; i < formalParams.GetNumArgs(); i++) { + // This call to Bounded/DoAccessibilityCheckForConstraints will also cause constraint Variance rules to be checked + // via the call to GetConstraints which will eventually call EEClass::CheckVarianceInSig BOOL Bounded(TypeVarTypeDesc *tyvar, DWORD depth); TypeVarTypeDesc *pTyVar = formalParams[i].AsGenericVariable(); - pTyVar->LoadConstraints(CLASS_DEPENDENCIES_LOADED); if (!Bounded(pTyVar, formalParams.GetNumArgs())) { COMPlusThrow(kTypeLoadException, VER_E_CIRCULAR_VAR_CONSTRAINTS); @@ -4603,15 +4809,10 @@ void MethodTable::DoFullyLoad(Generics::RecursionGraph * const pVisited, const if (pMD->IsGenericMethodDefinition() && pMD->IsTypicalMethodDefinition()) { - BOOL fHasCircularClassConstraints = TRUE; BOOL fHasCircularMethodConstraints = TRUE; - pMD->LoadConstraintsForTypicalMethodDefinition(&fHasCircularClassConstraints, &fHasCircularMethodConstraints, CLASS_DEPENDENCIES_LOADED); + pMD->CheckConstraintMetadataValidity(&fHasCircularMethodConstraints); - if (fHasCircularClassConstraints) - { - COMPlusThrow(kTypeLoadException, VER_E_CIRCULAR_VAR_CONSTRAINTS); - } if (fHasCircularMethodConstraints) { COMPlusThrow(kTypeLoadException, VER_E_CIRCULAR_MVAR_CONSTRAINTS); @@ -5577,6 +5778,27 @@ namespace } } +// Looking only at the typedef details of pMT, determine if it might have a candidate implementation +bool InterfaceMayHaveCandidateImplementation(MethodTable *pMT, MethodDesc *pInterfaceMD) +{ + LIMITED_METHOD_CONTRACT; + + MethodTable *pInterfaceMT = pInterfaceMD->GetMethodTable(); + + // If a method is defined on pMT and isn't abstract, then it might have a default implementation on that type if it isn't abstract + if (pMT->HasSameTypeDefAs(pInterfaceMT)) + { + return !pInterfaceMD->IsAbstract(); + } + + // If a pMT has MethodImpl records then its possible that it could override the interface method + if (pMT->GetClass()->ContainsMethodImpls()) + return true; + + // Otherwise the type pMT cannot possibly have a candidate implementation for pInterfaceMD + return false; +} + // Find the default interface implementation method for interface dispatch // It is either the interface method with default interface method implementation, // or an most specific interface with an explicit methodimpl overriding the method @@ -5610,7 +5832,7 @@ BOOL MethodTable::FindDefaultInterfaceImplementation( // Check the current method table itself MethodDesc *candidateMaybe = NULL; - if (IsInterface() && TryGetCandidateImplementation(this, pInterfaceMD, pInterfaceMT, findDefaultImplementationFlags, &candidateMaybe, level)) + if (IsInterface() && InterfaceMayHaveCandidateImplementation(this, pInterfaceMD) && TryGetCandidateImplementation(this, pInterfaceMD, pInterfaceMT, findDefaultImplementationFlags, &candidateMaybe, level)) { _ASSERTE(candidateMaybe != NULL); @@ -5647,74 +5869,77 @@ BOOL MethodTable::FindDefaultInterfaceImplementation( MethodTable::InterfaceMapIterator it = pMT->IterateInterfaceMapFrom(dwParentInterfaces); while (!it.Finished()) { - MethodTable *pCurMT = it.GetInterface(pMT, level); - - MethodDesc *pCurMD = NULL; - if (TryGetCandidateImplementation(pCurMT, pInterfaceMD, pInterfaceMT, findDefaultImplementationFlags, &pCurMD, level)) + if (InterfaceMayHaveCandidateImplementation(it.GetInterfaceApprox(), pInterfaceMD)) { - // - // Found a match. But is it a more specific match (we want most specific interfaces) - // - _ASSERTE(pCurMD != NULL); - bool needToInsert = true; - bool seenMoreSpecific = false; + MethodTable *pCurMT = it.GetInterface(pMT, level); - // We need to maintain the invariant that the candidates are always the most specific - // in all path scaned so far. There might be multiple incompatible candidates - for (unsigned i = 0; i < candidatesCount; ++i) + MethodDesc *pCurMD = NULL; + if (TryGetCandidateImplementation(pCurMT, pInterfaceMD, pInterfaceMT, findDefaultImplementationFlags, &pCurMD, level)) { - MethodTable *pCandidateMT = candidates[i].pMT; - if (pCandidateMT == NULL) - continue; - - if (pCandidateMT == pCurMT) + // + // Found a match. But is it a more specific match (we want most specific interfaces) + // + _ASSERTE(pCurMD != NULL); + bool needToInsert = true; + bool seenMoreSpecific = false; + + // We need to maintain the invariant that the candidates are always the most specific + // in all path scaned so far. There might be multiple incompatible candidates + for (unsigned i = 0; i < candidatesCount; ++i) { - // A dup - we are done - needToInsert = false; - break; - } + MethodTable *pCandidateMT = candidates[i].pMT; + if (pCandidateMT == NULL) + continue; - if (allowVariance && pCandidateMT->HasSameTypeDefAs(pCurMT)) - { - // Variant match on the same type - this is a tie - } - else if (pCurMT->CanCastToInterface(pCandidateMT)) - { - // pCurMT is a more specific choice than IFoo/IBar both overrides IBlah : - if (!seenMoreSpecific) + if (pCandidateMT == pCurMT) { - seenMoreSpecific = true; - candidates[i].pMT = pCurMT; - candidates[i].pMD = pCurMD; + // A dup - we are done + needToInsert = false; + break; } - else + + if (allowVariance && pCandidateMT->HasSameTypeDefAs(pCurMT)) { - candidates[i].pMT = NULL; - candidates[i].pMD = NULL; + // Variant match on the same type - this is a tie } + else if (pCurMT->CanCastToInterface(pCandidateMT)) + { + // pCurMT is a more specific choice than IFoo/IBar both overrides IBlah : + if (!seenMoreSpecific) + { + seenMoreSpecific = true; + candidates[i].pMT = pCurMT; + candidates[i].pMD = pCurMD; + } + else + { + candidates[i].pMT = NULL; + candidates[i].pMD = NULL; + } - needToInsert = false; - } - else if (pCandidateMT->CanCastToInterface(pCurMT)) - { - // pCurMT is less specific - we don't need to scan more entries as this entry can - // represent pCurMT (other entries are incompatible with pCurMT) - needToInsert = false; - break; + needToInsert = false; + } + else if (pCandidateMT->CanCastToInterface(pCurMT)) + { + // pCurMT is less specific - we don't need to scan more entries as this entry can + // represent pCurMT (other entries are incompatible with pCurMT) + needToInsert = false; + break; + } + else + { + // pCurMT is incompatible - keep scanning + } } - else + + if (needToInsert) { - // pCurMT is incompatible - keep scanning + ASSERT(candidatesCount < candidates.Size()); + candidates[candidatesCount].pMT = pCurMT; + candidates[candidatesCount].pMD = pCurMD; + candidatesCount++; } } - - if (needToInsert) - { - ASSERT(candidatesCount < candidates.Size()); - candidates[candidatesCount].pMT = pCurMT; - candidates[candidatesCount].pMD = pCurMD; - candidatesCount++; - } } it.Next(); @@ -8150,6 +8375,21 @@ MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType // entries, let's reset the count and just break out. (Should we throw?) break; } + + LPCUTF8 szMember = NULL; + PCCOR_SIGNATURE pSigMember = NULL; + DWORD cSigMember = 0; + if (TypeFromToken(methodDecl) == mdtMemberRef) + { + IfFailThrow(pMDInternalImport->GetNameAndSigOfMemberRef(methodDecl, &pSigMember, &cSigMember, &szMember)); + + // Do a quick name check to avoid excess use of FindMethod and the type load below + if (strcmp(szMember, pInterfaceMD->GetName()) != 0) + { + continue; + } + } + mdToken tkParent; hr = pMDInternalImport->GetParentToken(methodDecl, &tkParent); if (FAILED(hr)) @@ -8196,19 +8436,8 @@ MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType } else if (TypeFromToken(methodDecl) == mdtMemberRef) { - LPCUTF8 szMember; - PCCOR_SIGNATURE pSig; - DWORD cSig; - - IfFailThrow(pMDInternalImport->GetNameAndSigOfMemberRef(methodDecl, &pSig, &cSig, &szMember)); - - // Do a quick name check to avoid excess use of FindMethod - if (strcmp(szMember, pInterfaceMD->GetName()) != 0) - { - continue; - } - - pMethodDecl = MemberLoader::FindMethod(pInterfaceMT, szMember, pSig, cSig, GetModule()); + // We've already gotten the szMember, pSigMember, and cSigMember as a result of the early out check above. + pMethodDecl = MemberLoader::FindMethod(pInterfaceMT, szMember, pSigMember, cSigMember, GetModule()); } else { @@ -8415,6 +8644,12 @@ MethodTable::TryResolveConstraintMethodApprox( DWORD cPotentialMatchingInterfaces = 0; while (it.Next()) { + // If the approx type doesn't match by type handle, then it clearly can't match + // by canonical type. This avoids force loading the interface and breaking the + // special interface map type scenario + if (!it.GetInterfaceApprox()->HasSameTypeDefAs(thInterfaceType.AsMethodTable())) + continue; + TypeHandle thPotentialInterfaceType(it.GetInterface(pCanonMT)); if (thPotentialInterfaceType.AsMethodTable()->GetCanonicalMethodTable() == thInterfaceType.AsMethodTable()->GetCanonicalMethodTable()) @@ -8701,7 +8936,7 @@ PTR_MethodTable MethodTable::InterfaceMapIterator::GetInterface(MethodTable* pMT { TypeHandle ownerAsInst[MaxGenericParametersForSpecialMarkerType]; for (DWORD i = 0; i < MaxGenericParametersForSpecialMarkerType; i++) - ownerAsInst[i] = pMTOwner; + ownerAsInst[i] = pMTOwner->GetSpecialInstantiationType(); _ASSERTE(pResult->GetInstantiation().GetNumArgs() <= MaxGenericParametersForSpecialMarkerType); Instantiation inst(ownerAsInst, pResult->GetInstantiation().GetNumArgs()); diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index 5dd3f85bdd5920..b40e94560e0cf5 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -1432,8 +1432,11 @@ class MethodTable // of the fully loaded type. This is to reduce the amount of type loading // performed at process startup. // + // When placed on a valuetype or a non-generic interface, the special marker will indicate that the interface should be considered instantiated over the valuetype + // When placed on an interface, the special marker will indicate that the interface should be considered instantiated over the first generic parameter of the interface + // // The current rule is that these interfaces can only appear - // on valuetypes that are not shared generic, and that the special + // on valuetypes and interfaces that are not shared generic, and that the special // marker type is the open generic type. // inline bool IsSpecialMarkerTypeForGenericCasting() @@ -1441,6 +1444,19 @@ class MethodTable return IsGenericTypeDefinition(); } + // See comment on IsSpecialMarkerTypeForGenericCasting for details + inline TypeHandle GetSpecialInstantiationType() + { + if (IsInterface() && HasInstantiation()) + { + return GetInstantiation()[0]; + } + else + { + return TypeHandle(this); + } + } + static const DWORD MaxGenericParametersForSpecialMarkerType = 8; static BOOL ComputeContainsGenericVariables(Instantiation inst); @@ -2282,7 +2298,7 @@ class MethodTable pMT->HasInstantiation() && pCurrentMethodTable->IsSpecialMarkerTypeForGenericCasting() && !pMTOwner->GetAuxiliaryData()->MayHaveOpenInterfacesInInterfaceMap() && - pMT->GetInstantiation().ContainsAllOneType(pMTOwner)) + pMT->GetInstantiation().ContainsAllOneType(pMTOwner->GetSpecialInstantiationType())) { exactMatch = true; #ifndef DACCESS_COMPILE @@ -2298,6 +2314,8 @@ class MethodTable RETURN (exactMatch); } + bool CurrentInterfaceEquivalentTo(MethodTable* pMTOwner, MethodTable* pMT); + inline bool HasSameTypeDefAs(MethodTable* pMT) { CONTRACT(bool) diff --git a/src/coreclr/vm/methodtable.inl b/src/coreclr/vm/methodtable.inl index f6aa4746666839..06cd420ccc8d1a 100644 --- a/src/coreclr/vm/methodtable.inl +++ b/src/coreclr/vm/methodtable.inl @@ -1339,7 +1339,7 @@ FORCEINLINE BOOL MethodTable::ImplementsInterfaceInline(MethodTable *pInterface) while (--numInterfaces); // Second scan, looking for the curiously recurring generic scenario - if (pInterface->HasInstantiation() && !GetAuxiliaryData()->MayHaveOpenInterfacesInInterfaceMap() && pInterface->GetInstantiation().ContainsAllOneType(this)) + if (pInterface->HasInstantiation() && !GetAuxiliaryData()->MayHaveOpenInterfacesInInterfaceMap() && pInterface->GetInstantiation().ContainsAllOneType(this->GetSpecialInstantiationType())) { numInterfaces = GetNumInterfaces(); pInfo = GetInterfaceMap(); diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 1e8b5c87893c2c..fa97feb6ea8e87 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -417,7 +417,7 @@ MethodTableBuilder::ExpandApproxInterface( // to have found all of the interfaces that the type implements, and to place them in the interface list itself. Also // we can assume no ambiguous interfaces // Code related to this is marked with #SpecialCorelibInterfaceExpansionAlgorithm - if (!(GetModule()->IsSystem() && IsValueClass())) + if (!(GetModule()->IsSystem() && (IsValueClass() || IsInterface()))) { // Make sure to pass in the substitution from the new itf type created above as // these methods assume that substitutions are allocated in the stacking heap, @@ -1409,11 +1409,8 @@ MethodTableBuilder::BuildMethodTableThrowing( #endif // _DEBUG // If this is CoreLib, then don't perform some sanity checks on the layout - bmtProp->fNoSanityChecks = pModule->IsSystem() || -#ifdef FEATURE_READYTORUN - // No sanity checks for ready-to-run compiled images if possible - (pModule->IsReadyToRun() && pModule->GetReadyToRunInfo()->SkipTypeValidation()) || -#endif + bmtProp->fNoSanityChecks = + pModule->SkipTypeValidation() || // No sanity checks for real generic instantiations !bmtGenerics->IsTypicalTypeDefinition(); @@ -2240,6 +2237,9 @@ MethodTableBuilder::EnumerateMethodImpls() bmtMethod->dwNumberMethodImpls = hEnumMethodImpl.EnumMethodImplGetCount(); bmtMethod->dwNumberInexactMethodImplCandidates = 0; + if (bmtMethod->dwNumberMethodImpls != 0) + GetHalfBakedClass()->SetContainsMethodImpls(); + // This is the first pass. In this we will simply enumerate the token pairs and fill in // the data structures. In addition, we'll sort the list and eliminate duplicates. if (bmtMethod->dwNumberMethodImpls > 0) @@ -9602,12 +9602,9 @@ MethodTableBuilder::LoadExactInterfaceMap(MethodTable *pMT) BOOL duplicates; bool retry = false; - // Always use exact loading behavior with classes or shared generics, as they have to deal with inheritance, and the + // Always use exact loading behavior with normal 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(); + bool retryWithExactInterfaces = !(pMT->IsValueType() || pMT->IsInterface()) || pMT->IsSharedByGenericInstantiations(); if (retryWithExactInterfaces) { pMT->GetAuxiliaryDataForWrite()->SetMayHaveOpenInterfacesInInterfaceMap(); @@ -9641,13 +9638,21 @@ MethodTableBuilder::LoadExactInterfaceMap(MethodTable *pMT) (const Substitution*)0, retryWithExactInterfaces ? NULL : pMT).GetMethodTable(); + // When checking to possibly load the special instantiation type, if we load a type which ISN'T the type instantiatiated over the special instantiation type, but it IS IsSpecialMarkerTypeForGenericCasting + // the ClassLoader::LoadTypeDefOrRefOrSpecThrowing function will return System.Object's MT, and we need to detect that, and abort use of the special path. + if (!pNewIntfMT->IsInterface() && !retry) + { + retry = true; + break; + } + bool uninstGenericCase = !retryWithExactInterfaces && pNewIntfMT->IsSpecialMarkerTypeForGenericCasting(); duplicates |= InsertMethodTable(pNewIntfMT, pExactMTs, nInterfacesCount, &nAssigned); // We have a special algorithm for interface maps in CoreLib, which doesn't expand interfaces, and assumes no ambiguous // duplicates. Code related to this is marked with #SpecialCorelibInterfaceExpansionAlgorithm - if (!(pMT->GetModule()->IsSystem() && pMT->IsValueType())) + if (!(pMT->GetModule()->IsSystem() && (pMT->IsValueType() || pMT->IsInterface()))) { MethodTable::InterfaceMapIterator intIt = pNewIntfMT->IterateInterfaceMap(); while (intIt.Next()) @@ -9702,7 +9707,7 @@ MethodTableBuilder::LoadExactInterfaceMap(MethodTable *pMT) #endif // We have a special algorithm for interface maps in CoreLib, which doesn't expand interfaces, and assumes no ambiguous // duplicates. Code related to this is marked with #SpecialCorelibInterfaceExpansionAlgorithm - _ASSERTE(!duplicates || !(pMT->GetModule()->IsSystem() && pMT->IsValueType())); + _ASSERTE(!duplicates || !(pMT->GetModule()->IsSystem() && (pMT->IsValueType() || pMT->IsInterface()))); CONSISTENCY_CHECK(duplicates || (nAssigned == pMT->GetNumInterfaces())); if (duplicates) @@ -12720,7 +12725,8 @@ ClassLoader::CreateTypeHandleForTypeDefThrowing( } // Check interface for use of variant type parameters - if ((genericsInfo.pVarianceInfo != NULL) && (TypeFromToken(crInterface) == mdtTypeSpec)) + if ((genericsInfo.pVarianceInfo != NULL) && (TypeFromToken(crInterface) == mdtTypeSpec) + && !pModule->SkipTypeValidation()) { ULONG cSig; PCCOR_SIGNATURE pSig; diff --git a/src/coreclr/vm/runtimehandles.cpp b/src/coreclr/vm/runtimehandles.cpp index 9641005ec0aefd..a1f4e41ef9d3b9 100644 --- a/src/coreclr/vm/runtimehandles.cpp +++ b/src/coreclr/vm/runtimehandles.cpp @@ -612,7 +612,7 @@ extern "C" void QCALLTYPE RuntimeTypeHandle_GetConstraints(QCall::TypeHandle pTy TypeVarTypeDesc* pGenericVariable = typeHandle.AsGenericVariable(); DWORD dwCount; - constraints = pGenericVariable->GetConstraints(&dwCount); + constraints = pGenericVariable->GetConstraints(&dwCount, CLASS_LOADED, WhichConstraintsToLoad::All); GCX_COOP(); retTypeArray.Set(CopyRuntimeTypeHandles(constraints, dwCount, CLASS__TYPE)); diff --git a/src/coreclr/vm/siginfo.cpp b/src/coreclr/vm/siginfo.cpp index e23a17418e89f1..023b85030f0e0a 100644 --- a/src/coreclr/vm/siginfo.cpp +++ b/src/coreclr/vm/siginfo.cpp @@ -1103,7 +1103,7 @@ TypeHandle SigPointer::GetTypeHandleThrowing( const Substitution * pSubst/*=NULL*/, // ZapSigContext is only set when decoding zapsigs const ZapSig::Context * pZapSigContext, - MethodTable * pMTInterfaceMapOwner, + MethodTable* pMTInterfaceMapOwner, HandleRecursiveGenericsForFieldLayoutLoad *pRecursiveFieldGenericHandling) const { CONTRACT(TypeHandle) @@ -1616,7 +1616,7 @@ TypeHandle SigPointer::GetTypeHandleThrowing( Instantiation genericLoadInst(thisinst, ntypars); - if (pMTInterfaceMapOwner != NULL && genericLoadInst.ContainsAllOneType(pMTInterfaceMapOwner)) + if (pMTInterfaceMapOwner != NULL && genericLoadInst.ContainsAllOneType(pMTInterfaceMapOwner->GetSpecialInstantiationType())) { thRet = ClassLoader::LoadTypeDefThrowing(pGenericTypeModule, tkGenericType, ClassLoader::ThrowIfNotFound, ClassLoader::PermitUninstDefOrRef, 0, level); } @@ -1637,7 +1637,16 @@ TypeHandle SigPointer::GetTypeHandleThrowing( &instContext, pZapSigContext && pZapSigContext->externalTokens == ZapSig::NormalTokens)); - if (!handlingRecursiveGenericFieldScenario) + if (!thFound.IsNull() && pMTInterfaceMapOwner != NULL && !thFound.IsTypeDesc() && thFound.AsMethodTable()->IsSpecialMarkerTypeForGenericCasting()) + { + // We are trying to load an interface instantiation, and we have a concept of the special marker type enabled, but + // the loaded type is not the expected type we should be looking for to return a special marker type, but the normal load has + // found a type which claims to be a special marker type. In this case return something else (object) to indicate that + // we found an invalid situation and this function should be retried without the special marker type logic enabled. + thRet = TypeHandle(CoreLibBinder::GetElementType(ELEMENT_TYPE_OBJECT)); + break; + } + else if (!handlingRecursiveGenericFieldScenario) { thRet = thFound; break; diff --git a/src/coreclr/vm/siginfo.hpp b/src/coreclr/vm/siginfo.hpp index 7b6dc043d639b9..fa063529d4be10 100644 --- a/src/coreclr/vm/siginfo.hpp +++ b/src/coreclr/vm/siginfo.hpp @@ -253,7 +253,7 @@ class SigPointer : public SigParser BOOL dropGenericArgumentLevel = FALSE, const Substitution *pSubst = NULL, const ZapSig::Context *pZapSigContext = NULL, - MethodTable *pMTInterfaceMapOwner = NULL, + MethodTable* pMTInterfaceMapOwner = NULL, HandleRecursiveGenericsForFieldLayoutLoad *pRecursiveFieldGenericHandling = NULL ) const; diff --git a/src/coreclr/vm/typectxt.cpp b/src/coreclr/vm/typectxt.cpp index 1abaf9441bd674..33ad0f0861a026 100644 --- a/src/coreclr/vm/typectxt.cpp +++ b/src/coreclr/vm/typectxt.cpp @@ -91,36 +91,36 @@ void SigTypeContext::InitTypeContext(MethodDesc *md, TypeHandle declaringType, S #ifndef DACCESS_COMPILE TypeHandle GetDeclaringMethodTableFromTypeVarTypeDesc(TypeVarTypeDesc *pTypeVar, MethodDesc *pMD) { - LIMITED_METHOD_CONTRACT; + CONTRACTL { + THROWS; + GC_TRIGGERS; + } CONTRACTL_END; - // This currently should only happen in cases where we've already loaded the constraints. - // Currently, the only known case where use this code is reflection over methods exposed on a TypeVariable. - _ASSERTE(pTypeVar->ConstraintsLoaded()); + // This can only happen for reflection over type variables. Notably the logic which is used to enumerate the locals + // of a MethodBody which was found by reflection over a type variable. This only needs to be non-null + // in the case where the the type variable is constrained to implement a generic class or struct. - if (pTypeVar->ConstraintsLoaded()) + DWORD cConstraints; + TypeHandle *pTypeHandles = pTypeVar->GetConstraints(&cConstraints, CLASS_DEPENDENCIES_LOADED, WhichConstraintsToLoad::All); + for (DWORD iConstraint = 0; iConstraint < cConstraints; iConstraint++) { - DWORD cConstraints; - TypeHandle *pTypeHandles = pTypeVar->GetCachedConstraints(&cConstraints); - for (DWORD iConstraint = 0; iConstraint < cConstraints; iConstraint++) + if (pTypeHandles[iConstraint].IsGenericVariable()) { - if (pTypeHandles[iConstraint].IsGenericVariable()) - { - TypeHandle th = GetDeclaringMethodTableFromTypeVarTypeDesc(pTypeHandles[iConstraint].AsGenericVariable(), pMD); - if (!th.IsNull()) - return th; - } - else + TypeHandle th = GetDeclaringMethodTableFromTypeVarTypeDesc(pTypeHandles[iConstraint].AsGenericVariable(), pMD); + if (!th.IsNull()) + return th; + } + else + { + MethodTable *pMT = pTypeHandles[iConstraint].GetMethodTable(); + while (pMT != NULL) { - MethodTable *pMT = pTypeHandles[iConstraint].GetMethodTable(); - while (pMT != NULL) + if (pMT == pMD->GetMethodTable()) { - if (pMT == pMD->GetMethodTable()) - { - return TypeHandle(pMT); - } - - pMT = pMT->GetParentMethodTable(); + return TypeHandle(pMT); } + + pMT = pMT->GetParentMethodTable(); } } } @@ -129,10 +129,10 @@ TypeHandle GetDeclaringMethodTableFromTypeVarTypeDesc(TypeVarTypeDesc *pTypeVar, void SigTypeContext::InitTypeContext(MethodDesc *md, TypeHandle declaringType, Instantiation exactMethodInst, SigTypeContext *pRes) { + // This method has an unusual contract for SigTypeContext so that it can be used to load type with a declaringType which is a generic variable. CONTRACTL { - NOTHROW; - GC_NOTRIGGER; - FORBID_FAULT; + THROWS; + GC_TRIGGERS; PRECONDITION(CheckPointer(md)); } CONTRACTL_END; @@ -143,7 +143,6 @@ void SigTypeContext::InitTypeContext(MethodDesc *md, TypeHandle declaringType, I } else { - // factor this with the work above if (declaringType.IsGenericVariable()) { declaringType = GetDeclaringMethodTableFromTypeVarTypeDesc(declaringType.AsGenericVariable(), md); diff --git a/src/coreclr/vm/typedesc.cpp b/src/coreclr/vm/typedesc.cpp index e37d34355bdb16..09598f362c7452 100644 --- a/src/coreclr/vm/typedesc.cpp +++ b/src/coreclr/vm/typedesc.cpp @@ -343,7 +343,7 @@ BOOL TypeDesc::CanCastTo(TypeHandle toTypeHnd, TypeHandlePairList *pVisited) else { DWORD numConstraints; - TypeHandle* constraints = tyvar->GetConstraints(&numConstraints, CLASS_DEPENDENCIES_LOADED); + TypeHandle* constraints = tyvar->GetConstraints(&numConstraints, CLASS_DEPENDENCIES_LOADED, WhichConstraintsToLoad::All); if (constraints != NULL) { @@ -410,8 +410,8 @@ BOOL TypeDesc::CanCastParam(TypeHandle fromParam, TypeHandle toParam, TypeHandle { TypeVarTypeDesc* varFromParam = fromParam.AsGenericVariable(); - if (!varFromParam->ConstraintsLoaded()) - varFromParam->LoadConstraints(CLASS_DEPENDENCIES_LOADED); + if (!varFromParam->ConstraintsLoaded(WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly)) + varFromParam->LoadConstraints(CLASS_DEPENDENCIES_LOADED, WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly); if (!varFromParam->ConstrainedAsObjRef()) return FALSE; @@ -713,34 +713,46 @@ TypeHandle TypeVarTypeDesc::LoadOwnerType() return genericType; } -TypeHandle* TypeVarTypeDesc::GetCachedConstraints(DWORD *pNumConstraints) +TypeHandle* TypeVarTypeDesc::GetCachedConstraints(DWORD *pNumConstraints, WhichConstraintsToLoad which) { LIMITED_METHOD_CONTRACT; PRECONDITION(CheckPointer(pNumConstraints)); - PRECONDITION(m_numConstraints != (DWORD) -1); - *pNumConstraints = m_numConstraints; + DWORD numConstraints = m_numConstraintsWithFlags; + WhichConstraintsToLoad whichCurrent = (WhichConstraintsToLoad)(numConstraints >> WhichConstraintsLoadedShift); + bool prevWhichInsufficient = whichCurrent > which; + _ASSERTE(!prevWhichInsufficient); + + *pNumConstraints = numConstraints & ~WhichConstraintsLoadedMask; return m_constraints; } +TypeHandle* TypeVarTypeDesc::GetConstraints(DWORD *pNumConstraints, ClassLoadLevel level, WhichConstraintsToLoad which) +{ + CONTRACTL + { + THROWS; + GC_TRIGGERS; + MODE_ANY; + } + CONTRACTL_END; + _ASSERTE(CheckPointer(pNumConstraints)); + _ASSERTE(level == CLASS_DEPENDENCIES_LOADED || level == CLASS_LOADED); + DWORD numConstraints = m_numConstraintsWithFlags; + WhichConstraintsToLoad whichCurrent = (WhichConstraintsToLoad)(numConstraints >> WhichConstraintsLoadedShift); + bool prevWhichInsufficient = whichCurrent > which; -TypeHandle* TypeVarTypeDesc::GetConstraints(DWORD *pNumConstraints, ClassLoadLevel level /* = CLASS_LOADED */) -{ - WRAPPER_NO_CONTRACT; - PRECONDITION(CheckPointer(pNumConstraints)); - PRECONDITION(level == CLASS_DEPENDENCIES_LOADED || level == CLASS_LOADED); - - if (m_numConstraints == (DWORD) -1) - LoadConstraints(level); + if (prevWhichInsufficient) + LoadConstraints(level, which); - *pNumConstraints = m_numConstraints; + *pNumConstraints = m_numConstraintsWithFlags & ~WhichConstraintsLoadedMask; return m_constraints; } -void TypeVarTypeDesc::LoadConstraints(ClassLoadLevel level /* = CLASS_LOADED */) +void TypeVarTypeDesc::LoadConstraints(ClassLoadLevel level, WhichConstraintsToLoad which) { CONTRACTL { @@ -755,12 +767,32 @@ void TypeVarTypeDesc::LoadConstraints(ClassLoadLevel level /* = CLASS_LOADED */) CONTRACTL_END; _ASSERTE(((INT_PTR)&m_constraints) % sizeof(m_constraints) == 0); - _ASSERTE(((INT_PTR)&m_numConstraints) % sizeof(m_numConstraints) == 0); + _ASSERTE(((INT_PTR)&m_numConstraintsWithFlags) % sizeof(m_numConstraintsWithFlags) == 0); + + DWORD numConstraints; - DWORD numConstraints = m_numConstraints; + // If we have already loaded the constraints, and the previously loaded constraints are sufficient for the current request, skip the logic to find more constraints. + // Otherwise we need to load more constraints, and then possibly actually force them to be loaded to the appropriate load level. + // NOTE: + // The WhichConstraintsToLoad enum is ordered from most constraints to least constraints, so we can use a simple greater-than comparison to determine + // if the previously loaded constraints are sufficient for the current request. + // There are 3 possible values for WhichConstraintsToLoad: + // All - Load all constraints + // TypeOrMethodVarsAndNonInterfacesOnly - Load all constraints except interface constraints that are not type or method variables. The code MAY load other constraints. This is used when loading constraints for the purpose of type safety checks. + // None - Load no constraints. This is the initial state. - if (numConstraints == (DWORD) -1) + do { + numConstraints = m_numConstraintsWithFlags; + DWORD initialNumConstraints = numConstraints; + WhichConstraintsToLoad whichCurrent = (WhichConstraintsToLoad)(numConstraints >> WhichConstraintsLoadedShift); + bool prevWhichInsufficient = whichCurrent > which; + + if (!prevWhichInsufficient) + { + break; + } + IMDInternalImport* pInternalImport = GetModule()->GetMDImport(); HENUMInternalHolder hEnum(pInternalImport); @@ -787,7 +819,7 @@ void TypeVarTypeDesc::LoadConstraints(ClassLoadLevel level /* = CLASS_LOADED */) bool foundResult = false; if (!GetModule()->m_pTypeGenericInfoMap->HasConstraints(defToken, &foundResult) && foundResult) { - m_numConstraints = 0; + m_numConstraintsWithFlags = 0; return; } @@ -801,11 +833,24 @@ void TypeVarTypeDesc::LoadConstraints(ClassLoadLevel level /* = CLASS_LOADED */) numConstraints = pInternalImport->EnumGetCount(&hEnum); if (numConstraints != 0) { + numConstraints = (numConstraints & ~WhichConstraintsLoadedMask) | (((DWORD)which) << WhichConstraintsLoadedShift); + LoaderAllocator* pAllocator = GetModule()->GetLoaderAllocator(); // If there is a single class constraint we place it at index 0 of the array - AllocMemHolder constraints - (pAllocator->GetLowFrequencyHeap()->AllocMem(S_SIZE_T(numConstraints) * S_SIZE_T(sizeof(TypeHandle)))); + AllocMemHolder constraintAlloc; + TypeHandle *constraints; + + if (whichCurrent == WhichConstraintsToLoad::None) + { + constraintAlloc = (pAllocator->GetLowFrequencyHeap()->AllocMem(S_SIZE_T(numConstraints & ~WhichConstraintsLoadedMask) * S_SIZE_T(sizeof(TypeHandle)))); + constraints = (TypeHandle*)constraintAlloc; + } + else + { + constraints = m_constraints; + } + bool loadedAllConstraints = true; DWORD i = 0; while (pInternalImport->EnumNext(&hEnum, &tkConstraint)) { @@ -816,22 +861,94 @@ void TypeVarTypeDesc::LoadConstraints(ClassLoadLevel level /* = CLASS_LOADED */) GetModule()->GetAssembly()->ThrowTypeLoadException(pInternalImport, pMT->GetCl(), IDS_CLASSLOAD_BADFORMAT); } _ASSERTE(tkParam == GetToken()); - TypeHandle thConstraint = ClassLoader::LoadTypeDefOrRefOrSpecThrowing(GetModule(), tkConstraintType, - &typeContext, - ClassLoader::ThrowIfNotFound, - ClassLoader::FailIfUninstDefOrRef, - ClassLoader::LoadTypes, - level); + TypeHandle thConstraint; + + bool loadConstraint; + if (TypeFromToken(tkConstraintType) == mdtTypeSpec && which != WhichConstraintsToLoad::All) + { + _ASSERTE(which == WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly); + ULONG cSig; + PCCOR_SIGNATURE pSig; - constraints[i++] = thConstraint; + if (FAILED(pInternalImport->GetTypeSpecFromToken(tkConstraintType, &pSig, &cSig))) + { + GetModule()->GetAssembly()->ThrowTypeLoadException(pInternalImport, pMT->GetCl(), IDS_CLASSLOAD_BADFORMAT); + } + + SigPointer investigatePtr(pSig, cSig); + CorElementType elemType; + IfFailThrow(investigatePtr.GetElemType(&elemType)); + if (elemType == ELEMENT_TYPE_VAR || elemType == ELEMENT_TYPE_MVAR) + { + // We can always load variable constraints + loadConstraint = true; + } + else + { + if (elemType != ELEMENT_TYPE_GENERICINST) + { + // We don't know if its a class or interface, but it isn't generic, so finding out is the same as loading + // it, so just allow the load to occur. + loadConstraint = true; + } + else + { + IfFailThrow(investigatePtr.GetElemType(&elemType)); + _ASSERTE(elemType == ELEMENT_TYPE_CLASS + || elemType == ELEMENT_TYPE_VALUETYPE); + mdToken tkInvestigate; + + IfFailThrow(investigatePtr.GetToken(&tkInvestigate)); + + TypeHandle thUninstantiated = ClassLoader::LoadTypeDefOrRefOrSpecThrowing(GetModule(), tkInvestigate, + &typeContext, + ClassLoader::ThrowIfNotFound, + ClassLoader::PermitUninstDefOrRef, + ClassLoader::LoadTypes, + level); + + if (thUninstantiated.IsInterface()) + { + loadConstraint = false; + } + else + { + loadConstraint = true; + } + } + } + } + else + { + loadConstraint = true; + } + + if (loadConstraint) + { + thConstraint = ClassLoader::LoadTypeDefOrRefOrSpecThrowing(GetModule(), tkConstraintType, + &typeContext, + ClassLoader::ThrowIfNotFound, + ClassLoader::FailIfUninstDefOrRef, + ClassLoader::LoadTypes, + level); + } + else + { + loadedAllConstraints = false; + } // Method type constraints behave contravariantly // (cf Bounded polymorphism e.g. see // Cardelli & Wegner, On understanding types, data abstraction and polymorphism, Computing Surveys 17(4), Dec 1985) - if (pMT != NULL && pMT->HasVariance() && TypeFromToken(tkConstraintType) == mdtTypeSpec) + // + // This check is NOT conditional on actually loading the constraint, since we want to run EEClass::CheckVarianceInSig + // even if we didn't load the constraint, to cause the TypeLoadException to happen at a predictable time. + if (pMT != NULL && pMT->HasVariance() && TypeFromToken(tkConstraintType) == mdtTypeSpec + && !GetModule()->SkipTypeValidation()) { ULONG cSig; PCCOR_SIGNATURE pSig; + if (FAILED(pInternalImport->GetTypeSpecFromToken(tkConstraintType, &pSig, &cSig))) { GetModule()->GetAssembly()->ThrowTypeLoadException(pInternalImport, pMT->GetCl(), IDS_CLASSLOAD_BADFORMAT); @@ -845,20 +962,44 @@ void TypeVarTypeDesc::LoadConstraints(ClassLoadLevel level /* = CLASS_LOADED */) GetModule()->GetAssembly()->ThrowTypeLoadException(pInternalImport, pMT->GetCl(), IDS_CLASSLOAD_VARIANCE_IN_CONSTRAINT); } } + + if (!thConstraint.IsNull()) + { + VolatileStore((TADDR*)&constraints[i], thConstraint.AsTAddr()); + } + + i++; } - if (InterlockedCompareExchangeT(&m_constraints, constraints.operator->(), NULL) == NULL) + if (whichCurrent == WhichConstraintsToLoad::None) { - constraints.SuppressRelease(); + if (InterlockedCompareExchangeT(&m_constraints, constraintAlloc.operator->(), NULL) == NULL) + { + constraintAlloc.SuppressRelease(); + } + } + + if (loadedAllConstraints) + { + // We loaded all the constraints, so we can update the number we're going to store to indicate that we're storing all of them + numConstraints = numConstraints & ~WhichConstraintsLoadedMask; } } - m_numConstraints = numConstraints; - } + if (InterlockedCompareExchangeT(&m_numConstraintsWithFlags, numConstraints, initialNumConstraints) != initialNumConstraints) + { + // Retry if another thread set the number of constraints while we were working + continue; + } + break; + } while (true); - for (DWORD i = 0; i < numConstraints; i++) + for (DWORD i = 0; i < (numConstraints & ~WhichConstraintsLoadedMask); i++) { - ClassLoader::EnsureLoaded(m_constraints[i], level); + TypeHandle constraint = m_constraints[i]; + if (constraint.IsNull()) + continue; + ClassLoader::EnsureLoaded(constraint, level); } } @@ -869,7 +1010,7 @@ BOOL TypeVarTypeDesc::ConstrainedAsObjRef() NOTHROW; GC_NOTRIGGER; MODE_ANY; - PRECONDITION(ConstraintsLoaded()); + PRECONDITION(ConstraintsLoaded(WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly)); } CONTRACTL_END; @@ -903,12 +1044,17 @@ BOOL TypeVarTypeDesc::ConstrainedAsObjRefHelper() CONTRACTL_END; DWORD dwNumConstraints = 0; - TypeHandle* constraints = GetCachedConstraints(&dwNumConstraints); + TypeHandle* constraints = GetCachedConstraints(&dwNumConstraints, WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly); for (DWORD i = 0; i < dwNumConstraints; i++) { TypeHandle constraint = constraints[i]; + // This might be null if we didn't load an interface constraint + // Interface constraints do not contribute to this calculation, so we didn't need them + if (constraint.IsNull()) + continue; + if (constraint.IsGenericVariable() && constraint.AsGenericVariable()->ConstrainedAsObjRefHelper()) return TRUE; @@ -936,7 +1082,7 @@ BOOL TypeVarTypeDesc::ConstrainedAsValueType() NOTHROW; GC_NOTRIGGER; MODE_ANY; - PRECONDITION(ConstraintsLoaded()); + PRECONDITION(ConstraintsLoaded(WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly)); } CONTRACTL_END; @@ -953,12 +1099,16 @@ BOOL TypeVarTypeDesc::ConstrainedAsValueType() return TRUE; DWORD dwNumConstraints = 0; - TypeHandle* constraints = GetCachedConstraints(&dwNumConstraints); + TypeHandle* constraints = GetCachedConstraints(&dwNumConstraints, WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly); for (DWORD i = 0; i < dwNumConstraints; i++) { TypeHandle constraint = constraints[i]; + // This might be null if we didn't load an interface constraint + if (constraint.IsNull()) + continue; + if (constraint.IsGenericVariable()) { if (constraint.AsGenericVariable()->ConstrainedAsValueType()) @@ -1669,12 +1819,12 @@ TypeVarTypeDesc::EnumMemoryRegions(CLRDataEnumMemoryFlags flags) GetModule()->EnumMemoryRegions(flags, true); } - if (m_numConstraints != (DWORD)-1) + if (m_numConstraintsWithFlags != (DWORD)-1) { PTR_TypeHandle constraint = m_constraints; - for (DWORD i = 0; i < m_numConstraints; i++) + for (DWORD i = 0; i < (m_numConstraintsWithFlags & ~WhichConstraintsLoadedMask); i++) { - if (constraint.IsValid()) + if (constraint.IsValid() && !constraint->IsNull()) { constraint->EnumMemoryRegions(flags); } diff --git a/src/coreclr/vm/typedesc.h b/src/coreclr/vm/typedesc.h index 8cff26f064e657..08a2074f2e3039 100644 --- a/src/coreclr/vm/typedesc.h +++ b/src/coreclr/vm/typedesc.h @@ -278,6 +278,14 @@ struct cdac_data static constexpr size_t TypeArg = offsetof(ParamTypeDesc, m_Arg); }; +enum class WhichConstraintsToLoad +{ + All = 0, + TypeOrMethodVarsAndNonInterfacesOnly = 1, + Invalid = 2, + None = 3, +}; + /*************************************************************************/ // These are for verification of generic code and reflection over generic code. // Each TypeVarTypeDesc represents a class or method type variable, as specified by a GenericParam entry. @@ -286,6 +294,8 @@ struct cdac_data class TypeVarTypeDesc : public TypeDesc { + static const DWORD WhichConstraintsLoadedMask = 0xC0000000; + static const DWORD WhichConstraintsLoadedShift = 30; public: #ifndef DACCESS_COMPILE @@ -309,7 +319,7 @@ class TypeVarTypeDesc : public TypeDesc m_token = token; m_index = index; m_constraints = NULL; - m_numConstraints = (DWORD)-1; + m_numConstraintsWithFlags = (DWORD)-1; } #endif // #ifndef DACCESS_COMPILE @@ -349,16 +359,16 @@ class TypeVarTypeDesc : public TypeDesc MethodDesc * LoadOwnerMethod(); TypeHandle LoadOwnerType(); - BOOL ConstraintsLoaded() { LIMITED_METHOD_CONTRACT; return m_numConstraints != (DWORD)-1; } + BOOL ConstraintsLoaded(WhichConstraintsToLoad which) { LIMITED_METHOD_CONTRACT; if (((m_numConstraintsWithFlags & WhichConstraintsLoadedMask) >> WhichConstraintsLoadedShift) <= (DWORD)which) return TRUE; return FALSE; } // Return NULL if no constraints are specified // Return an array of type handles if constraints are specified, // with the number of constraints returned in pNumConstraints - TypeHandle* GetCachedConstraints(DWORD *pNumConstraints); - TypeHandle* GetConstraints(DWORD *pNumConstraints, ClassLoadLevel level = CLASS_LOADED); + TypeHandle* GetCachedConstraints(DWORD *pNumConstraints, WhichConstraintsToLoad which); + TypeHandle* GetConstraints(DWORD *pNumConstraints, ClassLoadLevel level, WhichConstraintsToLoad which); // Load the constraints if not already loaded - void LoadConstraints(ClassLoadLevel level = CLASS_LOADED); + void LoadConstraints(ClassLoadLevel level, WhichConstraintsToLoad which); // Check the constraints on this type parameter hold in the supplied context for the supplied type BOOL SatisfiesConstraints(SigTypeContext *pTypeContext, TypeHandle thArg, @@ -386,7 +396,7 @@ class TypeVarTypeDesc : public TypeDesc mdToken m_typeOrMethodDef; // Constraints, determined on first call to GetConstraints - Volatile m_numConstraints; // -1 until number has been determined + Volatile m_numConstraintsWithFlags; // -1 until number has been determined. Bit 31 and 30 bits are WhichConstraintsToLoad PTR_TypeHandle m_constraints; // token for GenericParam entry diff --git a/src/coreclr/vm/typehandle.cpp b/src/coreclr/vm/typehandle.cpp index 1df074a0dc8b4f..7007458071547f 100644 --- a/src/coreclr/vm/typehandle.cpp +++ b/src/coreclr/vm/typehandle.cpp @@ -571,8 +571,8 @@ BOOL TypeHandle::IsBoxedAndCanCastTo(TypeHandle type, TypeHandlePairList *pPairL { TypeVarTypeDesc* varFromParam = AsGenericVariable(); - if (!varFromParam->ConstraintsLoaded()) - varFromParam->LoadConstraints(CLASS_DEPENDENCIES_LOADED); + if (!varFromParam->ConstraintsLoaded(WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly)) + varFromParam->LoadConstraints(CLASS_DEPENDENCIES_LOADED, WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly); // A generic type parameter cannot be compatible with another type // as it could be substitued with a valuetype. However, if it is @@ -1371,8 +1371,6 @@ BOOL TypeHandle::SatisfiesClassConstraints() const _ASSERTE(tyvar != NULL); _ASSERTE(TypeFromToken(tyvar->GetTypeOrMethodDef()) == mdtTypeDef); - tyvar->LoadConstraints(); //TODO: is this necessary for anything but the typical class? - if (!tyvar->SatisfiesConstraints(&typeContext, thArg)) { return FALSE; diff --git a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharpNegative.il b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharpNegative.il index 08767e372ea460..eb5649cf33470e 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharpNegative.il +++ b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharpNegative.il @@ -106,6 +106,7 @@ { ldtoken class InvalidCSharpNegative.GenericDerivedInterface_Invalid`1 call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle) + callvirt instance class [System.Runtime]System.Type[] [System.Runtime]System.Type::GetInterfaces() callvirt instance string [System.Runtime]System.Object::ToString() ret } diff --git a/src/tests/Loader/classloader/generics/Constraints/General/FunctionPointerConstraints.cs b/src/tests/Loader/classloader/generics/Constraints/General/FunctionPointerConstraints.cs new file mode 100644 index 00000000000000..2b5bfc4996421d --- /dev/null +++ b/src/tests/Loader/classloader/generics/Constraints/General/FunctionPointerConstraints.cs @@ -0,0 +1,66 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// this test has a generic type with a constraint that it needs to implement an interface constrained on a function pointer +// we want to make sure we can load such type. + +using System; +using System.Text; +using Xunit; + +public class Test_FunctionPointerConstraints { + [Fact] + public static int TestEntryPoint() + { + MyClass myClass = new MyClass(); + + string result = myClass.MyMethod(new MyClass2()); + + bool pass = result == "Func1 Func2 "; + if (pass) + { + Console.WriteLine("PASS"); + return 100; + } + else + { + Console.WriteLine($"result: {result}"); + Console.WriteLine("FAIL"); + return 101; + } + } + + public unsafe class MyClass where T : I1[]> + { + public string MyMethod(T param) + { + StringBuilder stringBuilder = new StringBuilder(); + foreach (var item in param.Get()) + { + stringBuilder.Append(item()); + } + + return stringBuilder.ToString(); + } + } + + public unsafe class MyClass2 : I1[]> + { + private static string Func1() => "Func1 "; + private static string Func2() => "Func2 "; + + public unsafe delegate*[] Get() + { + return new delegate*[] + { + &Func1, + &Func2 + }; + } + } + public interface I1 + { + U Get(); + } +} + diff --git a/src/tests/Loader/classloader/generics/Constraints/General/FunctionPointerConstraints.csproj b/src/tests/Loader/classloader/generics/Constraints/General/FunctionPointerConstraints.csproj new file mode 100644 index 00000000000000..bc4e8cf32e5ec7 --- /dev/null +++ b/src/tests/Loader/classloader/generics/Constraints/General/FunctionPointerConstraints.csproj @@ -0,0 +1,9 @@ + + + true + 1 + + + + +