Avoid unnecessary type loads during JIT-time access checks

[davidwrighton]

Note

This PR description was generated with AI assistance.

Summary

Fixes #120554

When the JIT triggers access checks (for fields, methods, or types), the runtime previously loaded fully instantiated generic types even when the type arguments were type variables (!0, !!0, etc.). This is unnecessary because access checks only need to verify visibility of the named types (typedefs) involved, not the fully constructed generic instantiation.

This PR changes the access check infrastructure to perform access checks against the signature rather than materializing the full TypeHandle, avoiding loading generic types instantiated over type variables of the enclosing generic method/type.

Approach

The core idea is to introduce abstraction types (TargetTypeForAccessCheck, TargetMethodForAccessCheck, TargetInstantiationForAccessCheck) that can either wrap a fully-loaded TypeHandle/MethodDesc or lazily represent a type via its signature (SigPointer). Access checks operate on these abstractions, only loading types when truly needed (e.g., for error messages).

Components Modified

src/coreclr/vm/siginfo.hpp / src/coreclr/vm/siginfo.cpp

• New classes: TargetInstantiationForAccessCheck, TargetTypeForAccessCheck, TargetMethodForAccessCheck
  • TargetInstantiationForAccessCheck: Holds a SigPointer, SigTypeContext, and Module* representing a deferred instantiation.
  • TargetTypeForAccessCheck: Wraps a TypeHandle with an optional TargetInstantiationForAccessCheck*. Provides the same query surface as MethodTable* for access check purposes (e.g., IsNested, GetProtection, HasSameTypeDefAs, GetModule, GetAssembly, HasTypeParam). Only calls GetTypeHandleThrowing when actually needed (error paths).
  • TargetMethodForAccessCheck: Wraps a MethodDesc* with optional instantiation info; defers FindOrCreateAssociatedMethodDesc to GetMethodDescForThrow().
• New method: SigPointer::AccessCheckType — walks a signature recursively performing access checks on all named types without loading generic instantiations.

src/coreclr/vm/clsload.hpp / src/coreclr/vm/clsload.cpp / src/coreclr/vm/clsload.inl

• Refactored CanAccessClass, CanAccess, CheckAccessMember, CanAccessFamily, CanAccessMethodInstantiation to accept const TargetTypeForAccessCheck& / const TargetMethodForAccessCheck* instead of raw MethodTable* / MethodDesc*.
• Added ClassLoader::CanAccessInstantiation (operates on loaded Instantiation) and ClassLoader::CanAccessInstantiationBySignature (operates on SigPointer).
• AccessCheckOptions updated: stores TargetTypeForAccessCheck* and TargetMethodForAccessCheck*; removed unused kMemberAccess and kRestrictedMemberAccess enum values.
• DemandMemberAccess simplified — always returns TRUE (the remaining access check modes all permit access at this point).

src/coreclr/vm/jitinterface.cpp

• getFieldInfo, canAccessClass, and getCallInfo updated to construct TargetTypeForAccessCheck / TargetMethodForAccessCheck from the resolved token signatures instead of eagerly calling GetTypeHandleThrowing / FindOrCreateAssociatedMethodDesc.

src/coreclr/vm/methodtable.cpp / src/coreclr/vm/methodtable.h

• DoAccessibilityCheck wraps the target in TargetTypeForAccessCheck before calling CanAccessClass.
• InterfaceMapIterator::HasSameTypeDefAs marked const.

src/coreclr/vm/runtimehandles.cpp

• CheckCAVisibilityFromDecoratedType updated to use TargetTypeForAccessCheck; passes NULL for the method instantiation check since custom attribute constructors don't need method-instantiation access validation.

[dotnet-policy-service]

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

[davidwrighton]

NOTE: I'm not convinced this amount of complexity is warranted here. A simpler approach of simply disabling access checks of this form when compiling System.Private.CoreLib would probably produce most of the benefits without all the shenanigans that were needed here.

Also, this might be slower than the existing approach. I have not attempted to measure this.

Overall though, this reduces type loads in a test scenario which is a small application from about 2725 to 2416 when the application is loaded without R2R enabled. Without R2R enabled, we don't do anywhere near as much jitting and the number of types which don't need to be loaded is about 30. I view this change as being more useful in our interpreter scenarios where we are intentionally limiting R2R for size reasons.

[Copilot]

Pull request overview

This PR updates CoreCLR’s JIT-triggered accessibility checking to avoid eagerly loading fully-instantiated generic types (especially when instantiations involve VAR/MVAR type variables). It introduces “target for access check” wrapper types that can represent targets via signature data and only materialize full TypeHandle/MethodDesc when needed (primarily for throw paths), reducing unnecessary type loads during JIT access checks.

Changes:

• Added TargetTypeForAccessCheck / TargetMethodForAccessCheck / TargetInstantiationForAccessCheck and a new SigPointer::AccessCheckType routine to perform access checks by walking signatures.
• Refactored ClassLoader access-check APIs (CanAccessClass, CanAccess, etc.) to accept the new wrapper types and to support signature-based instantiation checks.
• Updated JIT-EE (jitinterface.cpp) and other call sites to construct these wrappers from TypeSpec/MethodSpec signatures instead of forcing full instantiation.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/coreclr/vm/siginfo.hpp	Adds new wrapper types and declares SigPointer::AccessCheckType for signature-walking access checks.
src/coreclr/vm/siginfo.cpp	Implements wrapper behavior, signature-walking access checking, and method/type instantiation access helpers.
src/coreclr/vm/clsload.hpp	Refactors access-check API signatures to use wrapper types; removes unused access-check enum values.
src/coreclr/vm/clsload.inl	Updates AccessCheckOptions constructors/initializers to store wrapper pointers.
src/coreclr/vm/clsload.cpp	Implements signature-based instantiation access checks and updates throw/error plumbing for wrapper targets.
src/coreclr/vm/jitinterface.cpp	Uses signature-backed wrapper targets for access checks in getFieldInfo, canAccessClass, and getCallInfo.
src/coreclr/vm/methodtable.cpp	Wraps target types when calling CanAccessClass in DoAccessibilityCheck.
src/coreclr/vm/methodtable.h	Marks InterfaceMapIterator::HasSameTypeDefAs as const.
src/coreclr/vm/runtimehandles.cpp	Switches custom-attribute visibility check to pass wrapper type and no optional target method.

[Copilot]

Typo in comment: "siganatures" -> "signatures" (also comment mentions "Method Instantiation" but this method is named AccessCheckType and walks type signatures).

[Copilot]

AccessCheckOptions::DemandMemberAccess now unconditionally returns TRUE, leaving canAccessTarget unused. This will likely fail the build under /WX or equivalent; remove the variable (and any now-dead parameters like visibilityCheck if they’re no longer needed).

[Copilot]

CanAccessClass now takes const TargetTypeForAccessCheck&, but the function body’s contract block still contains a CheckPointer(pTargetClass) precondition (left over from the old MethodTable*). That’s either a compile error or a no-op; update it to a meaningful check for this wrapper (or remove it).

[Copilot]

SigPointer::AccessCheckType declares notFoundAction and tdTypes but never uses them. This will likely trigger unused-variable warnings under /WX; remove them (or use them if they’re intended for future logic).

Suggested change

	ClassLoader::NotFoundAction notFoundAction;
	CorInternalStates tdTypes;

[Copilot]

The updated type-parameter access check uses if (!typeParam.IsNull()) to decide whether to call CanAccessClass. For generic variables / fnptr, the type handle is non-null but GetMethodTableOfRootTypeParam() is null, and CanAccessClass will dereference a null MethodTable via TargetTypeForAccessCheck::IsNested/GetProtection. This should preserve the previous behavior (if (pTypeParamMT != NULL)): skip the access check unless the (root) MethodTable exists.

[Copilot]

TargetMethodForAccessCheck::GetMethodDescForThrow reads the method-spec element byte into etype but never uses it. This is likely to trigger an unused-local warning under /WX; either remove etype or validate it (e.g., assert/THROW_BAD_FORMAT for IMAGE_CEE_CS_CALLCONV_GENERICINST).

Suggested change

	IfFailThrow(sp.GetByte(&etype));
	IfFailThrow(sp.GetByte(&etype));
	if (etype != IMAGE_CEE_CS_CALLCONV_GENERICINST)
	{
	COMPlusThrowHR(COR_E_BADIMAGEFORMAT);
	}

[Copilot]

TargetMethodForAccessCheck::CanAccessMethodInstantiation reads the method-spec element byte into etype but never uses it. This is likely to trigger an unused-local warning under /WX; either remove etype or validate it before proceeding.