Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Runtime support for static virtual interface methods #52173

Merged
merged 39 commits into from
May 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
c4d983f
Autogenerated tests for context hopping in Virtual static interface m…
davidwrighton Mar 25, 2021
a6ca485
Adjust line endings
davidwrighton Mar 25, 2021
1fc73ef
Refactor so that C# code is expressed as C# not as hand copied around IL
davidwrighton Mar 25, 2021
2eee539
Fix new lines
davidwrighton Mar 25, 2021
41811ca
Closer
davidwrighton Mar 25, 2021
635172f
Fixup expected strings
davidwrighton Mar 25, 2021
d0a2a13
Move reporting to C# code
davidwrighton Mar 25, 2021
a42a5fd
First pass at type hierarchy test
davidwrighton Mar 26, 2021
0815e4c
Fix the build
davidwrighton Mar 26, 2021
e5a9878
Add minimal covariant testing
davidwrighton Mar 29, 2021
3ea2dca
Add minimal test for initial bringup
davidwrighton Mar 29, 2021
26e949b
Fix issues discovered by Tomas
davidwrighton Apr 16, 2021
b7db7d2
Adjust test autogeneration based on spec tweaks
davidwrighton Apr 20, 2021
8589818
WIP: runtime support for static virtual interface methods (#2)
trylek Apr 20, 2021
93c9fb7
First version of method resolution - Scenario1-5 passing
trylek Apr 21, 2021
9e8be24
Desired change
davidwrighton Apr 22, 2021
4b0a75f
Add try and catch to TypeHierarchyTest
davidwrighton Apr 22, 2021
df39e61
First generic context tests pass
davidwrighton Apr 22, 2021
b045092
Handle generic methods (#4)
davidwrighton Apr 22, 2021
ddff311
60 TypeHierarchy tests passing via broader use of associated method d…
trylek Apr 23, 2021
9042354
Adjust generic dictionary to be able to resolve to a code pointer the…
davidwrighton Apr 26, 2021
b7d0917
Fix manipulation of signature type context in SVM resolution (#8)
trylek Apr 26, 2021
7d615d1
Fix GenericsContextTest test bugs (#10)
davidwrighton Apr 27, 2021
bb534b4
Fix type hierarchy tests (#11)
davidwrighton Apr 27, 2021
8f6f55e
- Pass thru allowInstParam flag from jit interface to TryResolveConst…
davidwrighton Apr 28, 2021
dc25f38
Add a selection of new handwritten IL tests (#14)
davidwrighton May 1, 2021
2360951
Fix remaining failing test issues in current testbed (#13)
davidwrighton May 1, 2021
4c460fa
Exclude SVM test generators from test build (#15)
trylek May 4, 2021
aa86d0a
Fix default interface method tests (#16)
trylek May 10, 2021
0caa786
Add support for interface variance (#18)
davidwrighton May 11, 2021
36f310f
Fix variance safety check (#19)
davidwrighton May 11, 2021
ab57767
Partial fix for the negative virtual static method tests (#17)
trylek May 12, 2021
00426d6
Fix bad merge
davidwrighton May 12, 2021
8eb54e6
Constraint checking (#20)
davidwrighton May 12, 2021
7cdf214
Fix issue where variance checking uses an unsafe contract by being mo…
davidwrighton May 12, 2021
43b0a96
Fix constraint check when a type variable is used to instantiate a ge…
davidwrighton May 13, 2021
28be21e
fix build break (#23)
davidwrighton May 13, 2021
dbea3da
Exempt static virtual method callers from Crossgen1/2 compilation (#24)
trylek May 14, 2021
211495a
Exclude static virtual method tests on Mono
trylek May 14, 2021
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 @@ -8,11 +8,13 @@
using System.Runtime.InteropServices;
using System.Runtime.Loader;
using System.Runtime.Serialization;
using System.Runtime.Versioning;
using System.Threading;
using Internal.Runtime.CompilerServices;

namespace System
{
[NonVersionable]
public unsafe struct RuntimeTypeHandle : ISerializable
{
// Returns handle for interop with EE. The handle is guaranteed to be non-null.
Expand Down Expand Up @@ -798,6 +800,7 @@ RuntimeMethodHandleInternal Value
}
}

[NonVersionable]
public unsafe struct RuntimeMethodHandle : ISerializable
{
// Returns handle for interop with EE. The handle is guaranteed to be non-null.
Expand Down Expand Up @@ -1122,6 +1125,7 @@ internal sealed class RuntimeFieldInfoStub : IRuntimeFieldInfo
RuntimeFieldHandleInternal IRuntimeFieldInfo.Value => m_fieldHandle;
}

[NonVersionable]
public unsafe struct RuntimeFieldHandle : ISerializable
{
// Returns handle for interop with EE. The handle is guaranteed to be non-null.
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/dlls/mscorrc/mscorrc.rc
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ BEGIN
IDS_CLASSLOAD_MI_MISSING_SIG_BODY "Signature for the body in a method implementation cannot be found. Type: '%1'. Assembly: '%2'."
IDS_CLASSLOAD_MI_MISSING_SIG_DECL "Signature for the declaration in a method implementation cannot be found. Type: '%1'. Assembly: '%2'."
IDS_CLASSLOAD_MI_BADRETURNTYPE "Return type in method '%1' on type '%2' from assembly '%3' is not compatible with base type method '%4'."
IDS_CLASSLOAD_STATICVIRTUAL_NOTIMPL "Virtual static method '%3' is not implemented on type '%1' from assembly '%2'."

IDS_CLASSLOAD_EQUIVALENTSTRUCTMETHODS "Could not load the structure '%1' from assembly '%2'. The structure is marked as eligible for type equivalence, but it has a method."
IDS_CLASSLOAD_EQUIVALENTSTRUCTFIELDS "Could not load the structure '%1' from assembly '%2'. The structure is marked as eligible for type equivalence, but it has a static or non-public field."
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/dlls/mscorrc/resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@
#define IDS_CLASSLOAD_MI_MISSING_SIG_BODY 0x17a6
#define IDS_CLASSLOAD_MI_MISSING_SIG_DECL 0x17a7
#define IDS_CLASSLOAD_MI_BADRETURNTYPE 0x17a8
#define IDS_CLASSLOAD_STATICVIRTUAL_NOTIMPL 0x17a9

#define IDS_CLASSLOAD_TOOMANYGENERICARGS 0x17ab
#define IDS_ERROR 0x17b0
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1143,9 +1143,9 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
{
OPCODE actualOpcode = impGetNonPrefixOpcode(codeAddr, codeEndp);

if (actualOpcode != CEE_CALLVIRT)
if (actualOpcode != CEE_CALLVIRT && actualOpcode != CEE_CALL && actualOpcode != CEE_LDFTN)
{
BADCODE("constrained. has to be followed by callvirt");
BADCODE("constrained. has to be followed by callvirt, call or ldftn");
}
}
goto OBSERVE_OPCODE;
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13903,7 +13903,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)

JITDUMP(" %08X", resolvedToken.token);

eeGetCallInfo(&resolvedToken, nullptr /* constraint typeRef*/,
eeGetCallInfo(&resolvedToken, (prefixFlags & PREFIX_CONSTRAINED) ? &constrainedResolvedToken : nullptr,
addVerifyFlag(combine(CORINFO_CALLINFO_SECURITYCHECKS, CORINFO_CALLINFO_LDFTN)),
&callInfo);

Expand Down Expand Up @@ -14074,9 +14074,9 @@ void Compiler::impImportBlockCode(BasicBlock* block)

{
OPCODE actualOpcode = impGetNonPrefixOpcode(codeAddr, codeEndp);
if (actualOpcode != CEE_CALLVIRT)
if (actualOpcode != CEE_CALLVIRT && actualOpcode != CEE_CALL && actualOpcode != CEE_LDFTN)
{
BADCODE("constrained. has to be followed by callvirt");
BADCODE("constrained. has to be followed by callvirt, call or ldftn");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1818,6 +1818,13 @@ private void VerifyMethodSignatureIsStable(MethodSignature methodSig)

private void getCallInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_RESOLVED_TOKEN* pConstrainedResolvedToken, CORINFO_METHOD_STRUCT_* callerHandle, CORINFO_CALLINFO_FLAGS flags, CORINFO_CALL_INFO* pResult)
{
if ((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_CALLVIRT) == 0 && pConstrainedResolvedToken != null)
{
// Defer constrained call / ldftn instructions used for static virtual methods
// to runtime resolution.
throw new RequiresRuntimeJitException("SVM");
}

MethodDesc methodToCall;
MethodDesc targetMethod;
TypeDesc constrainedType;
Expand Down
5 changes: 4 additions & 1 deletion src/coreclr/vm/genmeth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1180,7 +1180,10 @@ MethodDesc::FindOrCreateAssociatedMethodDesc(MethodDesc* pDefMD,
pExactMT->GetCanonicalMethodTable(),
FALSE,
Instantiation(repInst, methodInst.GetNumArgs()),
TRUE);
/* allowInstParam */ TRUE,
/* forceRemotableMethod */ FALSE,
/* allowCreate */ TRUE,
/* level */ level);

_ASSERTE(pWrappedMD->IsSharedByGenericInstantiations());
_ASSERTE(!methodInst.IsEmpty() || !pWrappedMD->IsSharedByGenericMethodInstantiations());
Expand Down
64 changes: 56 additions & 8 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5124,7 +5124,7 @@ void CEEInfo::getCallInfo(
TypeHandle exactType = TypeHandle(pResolvedToken->hClass);

TypeHandle constrainedType;
if ((flags & CORINFO_CALLINFO_CALLVIRT) && (pConstrainedResolvedToken != NULL))
if (pConstrainedResolvedToken != NULL)
{
constrainedType = TypeHandle(pConstrainedResolvedToken->hClass);
}
Expand Down Expand Up @@ -5352,13 +5352,37 @@ void CEEInfo::getCallInfo(
// Direct calls to abstract methods are not allowed
if (IsMdAbstract(dwTargetMethodAttrs) &&
// Compensate for always treating delegates as direct calls above
!(((flags & CORINFO_CALLINFO_LDFTN) && (flags & CORINFO_CALLINFO_CALLVIRT) && !resolvedCallVirt)))
!(((flags & CORINFO_CALLINFO_LDFTN) && (flags & CORINFO_CALLINFO_CALLVIRT) && !resolvedCallVirt))
&& !(IsMdStatic(dwTargetMethodAttrs) && fForceUseRuntimeLookup))
{
COMPlusThrowHR(COR_E_BADIMAGEFORMAT, BFA_BAD_IL);
}

bool allowInstParam = (flags & CORINFO_CALLINFO_ALLOWINSTPARAM);

// If the target method is resolved via constrained static virtual dispatch
// And it requires an instParam, we do not have the generic dictionary infrastructure
// to load the correct generic context arg via EmbedGenericHandle.
// Instead, force the call to go down the CORINFO_CALL_CODE_POINTER code path
// which should have somewhat inferior performance. This should only actually happen in the case
// of shared generic code calling a shared generic implementation method, which should be rare.
//
// An alternative design would be to add a new generic dictionary entry kind to hold the MethodDesc
// of the constrained target instead, and use that in some circumstances; however, implementation of
// that design requires refactoring variuos parts of the JIT interface as well as
// TryResolveConstraintMethodApprox. In particular we would need to be abled to embed a constrained lookup
// via EmbedGenericHandle, as well as decide in TryResolveConstraintMethodApprox if the call can be made
// via a single use of CORINFO_CALL_CODE_POINTER, or would be better done with a CORINFO_CALL + embedded
// constrained generic handle, or if there is a case where we would want to use both a CORINFO_CALL and
// embedded constrained generic handle. Given the current expected high performance use case of this feature
// which is generic numerics which will always resolve to exact valuetypes, it is not expected that
// the complexity involved would be worth the risk. Other scenarios are not expected to be as performance
// sensitive.
if (IsMdStatic(dwTargetMethodAttrs) && constrainedType != NULL && pResult->exactContextNeedsRuntimeLookup)
{
allowInstParam = FALSE;
}

// Create instantiating stub if necesary
if (!allowInstParam && pTargetMD->RequiresInstArg())
{
Expand Down Expand Up @@ -5401,9 +5425,20 @@ void CEEInfo::getCallInfo(
{
pResult->kind = CORINFO_CALL_CODE_POINTER;

// For reference types, the constrained type does not affect method resolution
DictionaryEntryKind entryKind = (!constrainedType.IsNull() && constrainedType.IsValueType()) ? ConstrainedMethodEntrySlot : MethodEntrySlot;

DictionaryEntryKind entryKind;
if (constrainedType.IsNull() || ((flags & CORINFO_CALLINFO_CALLVIRT) && !constrainedType.IsValueType()))
{
// For reference types, the constrained type does not affect method resolution on a callvirt, and if there is no
// constraint, it doesn't effect it either
entryKind = MethodEntrySlot;
}
else
{
// constrained. callvirt case where the constraint type is a valuetype
// OR
// constrained. call or constrained. ldftn case
entryKind = ConstrainedMethodEntrySlot;
}
ComputeRuntimeLookupForSharedGenericToken(entryKind,
pResolvedToken,
pConstrainedResolvedToken,
Expand Down Expand Up @@ -5706,7 +5741,19 @@ void CEEInfo::getCallInfo(

pResult->methodFlags = getMethodAttribsInternal(pResult->hMethod);

SignatureKind signatureKind = flags & CORINFO_CALLINFO_CALLVIRT && !(pResult->kind == CORINFO_CALL) ? SK_VIRTUAL_CALLSITE : SK_CALLSITE;
SignatureKind signatureKind;
if (flags & CORINFO_CALLINFO_CALLVIRT && !(pResult->kind == CORINFO_CALL))
{
signatureKind = SK_VIRTUAL_CALLSITE;
}
else if ((pResult->kind == CORINFO_CALL_CODE_POINTER) && IsMdVirtual(dwTargetMethodAttrs) && IsMdStatic(dwTargetMethodAttrs))
{
signatureKind = SK_STATIC_VIRTUAL_CODEPOINTER_CALLSITE;
}
else
{
signatureKind = SK_CALLSITE;
}
getMethodSigInternal(pResult->hMethod, &pResult->sig, (pResult->hMethod == pResolvedToken->hMethod) ? pResolvedToken->hClass : NULL, signatureKind);

if (flags & CORINFO_CALLINFO_VERIFICATION)
Expand Down Expand Up @@ -8639,9 +8686,10 @@ CEEInfo::getMethodSigInternal(
// Otherwise we would end up with two secret generic dictionary arguments (since the stub also provides one).
//
BOOL isCallSiteThatGoesThroughInstantiatingStub =
signatureKind == SK_VIRTUAL_CALLSITE &&
(signatureKind == SK_VIRTUAL_CALLSITE &&
!ftn->IsStatic() &&
ftn->GetMethodTable()->IsInterface();
ftn->GetMethodTable()->IsInterface()) ||
signatureKind == SK_STATIC_VIRTUAL_CODEPOINTER_CALLSITE;
if (!isCallSiteThatGoesThroughInstantiatingStub)
sigRet->callConv = (CorInfoCallConv) (sigRet->callConv | CORINFO_CALLCONV_PARAMTYPE);
}
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/jitinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ enum SignatureKind
SK_NOT_CALLSITE,
SK_CALLSITE,
SK_VIRTUAL_CALLSITE,
SK_STATIC_VIRTUAL_CODEPOINTER_CALLSITE,
};

class Stub;
Expand Down
15 changes: 10 additions & 5 deletions src/coreclr/vm/memberload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,8 @@ FieldDesc * MemberLoader::GetFieldDescFromMemberRefAndType(Module * pModule,
//
MethodDesc* MemberLoader::GetMethodDescFromMethodDef(Module *pModule,
mdToken MethodDef,
BOOL strictMetadataChecks)
BOOL strictMetadataChecks,
ClassLoadLevel owningTypeLoadLevel)
{
CONTRACTL
{
Expand Down Expand Up @@ -623,7 +624,7 @@ MethodDesc* MemberLoader::GetMethodDescFromMethodDef(Module *pModule,
}
}

pMD->CheckRestore();
pMD->CheckRestore(owningTypeLoadLevel);

#if 0
// <TODO> Generics: enable this check after the findMethod call in the Zapper which passes
Expand Down Expand Up @@ -713,7 +714,8 @@ MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec(
BOOL strictMetadataChecks,
// Normally true - the zapper is one exception. Throw an exception if no generic method args
// given for a generic method, otherwise return the 'generic' instantiation
BOOL allowInstParam)
BOOL allowInstParam,
ClassLoadLevel owningTypeLoadLevel)
{
CONTRACTL
{
Expand All @@ -738,7 +740,7 @@ MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec(
switch (TypeFromToken(MemberRef))
{
case mdtMethodDef:
pMD = GetMethodDescFromMethodDef(pModule, MemberRef, strictMetadataChecks);
pMD = GetMethodDescFromMethodDef(pModule, MemberRef, strictMetadataChecks, owningTypeLoadLevel);
th = pMD->GetMethodTable();
break;

Expand Down Expand Up @@ -770,7 +772,10 @@ MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec(
th.GetMethodTable(),
FALSE /* don't get unboxing entry point */,
strictMetadataChecks ? Instantiation() : pMD->LoadMethodInstantiation(),
allowInstParam);
allowInstParam,
/* forceRemotableMethod */ FALSE,
/* allowCreate */ TRUE,
/* level */ owningTypeLoadLevel);
} // MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec

//---------------------------------------------------------------------------------------
Expand Down
6 changes: 4 additions & 2 deletions src/coreclr/vm/memberload.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ class MemberLoader
mdToken MemberRefOrDefOrSpec,
const SigTypeContext *pTypeContext, // Context for type parameters in any parent TypeSpec and in the instantiation in a MethodSpec
BOOL strictMetadataChecks, // Normally true - the zapper is one exception. Throw an exception if no generic method args given for a generic method, otherwise return the 'generic' instantiation
BOOL allowInstParam);
BOOL allowInstParam,
ClassLoadLevel owningTypeLoadLevel = CLASS_LOADED);

static FieldDesc* GetFieldDescFromMemberDefOrRef(Module *pModule,
mdMemberRef MemberDefOrRef,
Expand All @@ -92,7 +93,8 @@ class MemberLoader

static MethodDesc* GetMethodDescFromMethodDef(Module *pModule,
mdToken MethodDef,
BOOL strictMetadataChecks);
BOOL strictMetadataChecks,
ClassLoadLevel owningTypeLoadLevel = CLASS_LOADED);

static FieldDesc* GetFieldDescFromFieldDef(Module *pModule,
mdToken FieldDef,
Expand Down
Loading