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

Support generic arguments for calli in CoreCLR #97079

Merged
merged 47 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
f10af6b
Add generic support for calli
hez2010 Jan 17, 2024
345ef56
Consider classInst as well
hez2010 Jan 17, 2024
6e429cf
Merge branch 'main' into feature/generic-calli
hez2010 Jan 17, 2024
42debae
oops
hez2010 Jan 17, 2024
ab8839b
guarded by !DACCESS_COMPILE
hez2010 Jan 17, 2024
0327254
Block runtime marshaling for calli
hez2010 Jan 21, 2024
21c6202
Fix the error mode
hez2010 Jan 22, 2024
a10142c
Adding tests
hez2010 Jan 23, 2024
6f7eb0e
Minor fixes to tests
hez2010 Jan 23, 2024
68d5a09
Compare instantiations as well when lookup
hez2010 Jan 23, 2024
2ea3dac
Adding more test cases
hez2010 Jan 23, 2024
5ecc888
Avoid getting export multiple times
hez2010 Jan 23, 2024
21d9862
Improve tests
hez2010 Jan 24, 2024
8939e32
Use standard boolean
hez2010 Jan 24, 2024
ab165f1
Avoid casting to prevent ABI mismatch
hez2010 Jan 24, 2024
e0e7478
Handle collectible assemblies
hez2010 Jan 25, 2024
97a8bf6
Reverse the branch
hez2010 Jan 25, 2024
fd43493
Handle inlined P/Invoke transitions
hez2010 Jan 26, 2024
fad2701
Reimplement unloadability handling
hez2010 Jan 26, 2024
4f67d66
Fix native library resolving in tests
hez2010 Jan 27, 2024
b050468
Use AllocMemTracker properly
hez2010 Jan 27, 2024
549c746
Skip tests on windows x86
hez2010 Jan 27, 2024
4fcbbf8
Skip negative tests on Windows x86 and NativeAOT
hez2010 Jan 27, 2024
883f531
Merge branch 'main' into feature/generic-calli
hez2010 Jan 27, 2024
fec111f
Use better exception message
hez2010 Feb 12, 2024
83d9919
Enable Windows x86 tests again
hez2010 Feb 12, 2024
1390455
Exclude tests for mono
hez2010 Feb 13, 2024
e37a0d9
Use correct loader elsewhere
hez2010 Feb 13, 2024
7721626
Put VASigCookieBlock on loader module
hez2010 Feb 14, 2024
d05b1cb
Move VASigCookie creation to a new method
hez2010 Feb 14, 2024
909e7fa
Move more things
hez2010 Feb 14, 2024
f6833b9
oops
hez2010 Feb 14, 2024
35edb8d
Update issues.targets
hez2010 Feb 14, 2024
40d5c35
Allocate StubMD on loader module
hez2010 Feb 14, 2024
0282d1a
Fixes for vararg cases
hez2010 Feb 14, 2024
8fa2fd6
Handle varargs in error mode as well
hez2010 Feb 14, 2024
c2cfc1b
Add an error mode for generic varargs
hez2010 Feb 14, 2024
3c1c25d
Remove useless comments
hez2010 Feb 14, 2024
e503dfa
FB
hez2010 Feb 15, 2024
fc79d98
Update tests
hez2010 Feb 15, 2024
22ecfb2
Fix tests
hez2010 Feb 15, 2024
9518dd9
Apply suggestions from code review
hez2010 Feb 15, 2024
c9c0881
Make xUnit happy
hez2010 Feb 16, 2024
3974945
test mono ci
hez2010 Feb 16, 2024
a8d38b0
Tweak the tests
hez2010 Feb 21, 2024
f1a0fd3
Merge branch 'main' into feature/generic-calli
hez2010 Feb 21, 2024
151982a
Add link to an GH issue
hez2010 Feb 22, 2024
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
112 changes: 93 additions & 19 deletions src/coreclr/vm/ceeload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4680,7 +4680,7 @@ PTR_VOID ReflectionModule::GetRvaField(RVA field) // virtual
//==========================================================================
// Enregisters a VASig.
//==========================================================================
VASigCookie *Module::GetVASigCookie(Signature vaSignature)
VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* typeContext)
{
CONTRACT(VASigCookie*)
{
Expand All @@ -4693,79 +4693,153 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature)
}
CONTRACT_END;

Module* pLoaderModule = ClassLoader::ComputeLoaderModuleWorker(this, mdTokenNil, typeContext->m_classInst, typeContext->m_methodInst);
VASigCookie *pCookie = GetVASigCookieWorker(this, pLoaderModule, vaSignature, typeContext);

RETURN pCookie;
}

VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoaderModule, Signature vaSignature, const SigTypeContext* typeContext)
{
CONTRACT(VASigCookie*)
{
THROWS;
GC_TRIGGERS;
MODE_ANY;
POSTCONDITION(CheckPointer(RETVAL));
INJECT_FAULT(COMPlusThrowOM());
}
CONTRACT_END;

VASigCookieBlock *pBlock;
VASigCookie *pCookie;

pCookie = NULL;

// First, see if we already enregistered this sig.
// Note that we're outside the lock here, so be a bit careful with our logic
for (pBlock = m_pVASigCookieBlock; pBlock != NULL; pBlock = pBlock->m_Next)
for (pBlock = pLoaderModule->m_pVASigCookieBlock; pBlock != NULL; pBlock = pBlock->m_Next)
{
for (UINT i = 0; i < pBlock->m_numcookies; i++)
{
if (pBlock->m_cookies[i].signature.GetRawSig() == vaSignature.GetRawSig())
{
pCookie = &(pBlock->m_cookies[i]);
break;
_ASSERTE(pBlock->m_cookies[i].classInst.GetNumArgs() == typeContext->m_classInst.GetNumArgs());
_ASSERTE(pBlock->m_cookies[i].methodInst.GetNumArgs() == typeContext->m_methodInst.GetNumArgs());

bool instMatch = true;

for (DWORD j = 0; j < pBlock->m_cookies[i].classInst.GetNumArgs(); j++)
{
if (pBlock->m_cookies[i].classInst[j] != typeContext->m_classInst[j])
{
instMatch = false;
break;
}
}

if (instMatch)
{
for (DWORD j = 0; j < pBlock->m_cookies[i].methodInst.GetNumArgs(); j++)
{
if (pBlock->m_cookies[i].methodInst[j] != typeContext->m_methodInst[j])
{
instMatch = false;
break;
}
}
}

if (instMatch)
{
pCookie = &(pBlock->m_cookies[i]);
break;
}
}
}
}

if (!pCookie)
{
// If not, time to make a new one.

// Compute the size of args first, outside of the lock.

// @TODO GENERICS: We may be calling a varargs method from a
// generic type/method. Using an empty context will make such a
// case cause an unexpected exception. To make this work,
// we need to create a specialized signature for every instantiation
SigTypeContext typeContext;

MetaSig metasig(vaSignature, this, &typeContext);
MetaSig metasig(vaSignature, pDefiningModule, typeContext);
ArgIterator argit(&metasig);

// Upper estimate of the vararg size
DWORD sizeOfArgs = argit.SizeOfArgStack();

// Prepare instantiation
LoaderAllocator *pLoaderAllocator = pLoaderModule->GetLoaderAllocator();

DWORD classInstCount = typeContext->m_classInst.GetNumArgs();
DWORD methodInstCount = typeContext->m_methodInst.GetNumArgs();
pLoaderAllocator->EnsureInstantiation(pDefiningModule, typeContext->m_classInst);
pLoaderAllocator->EnsureInstantiation(pDefiningModule, typeContext->m_methodInst);

// enable gc before taking lock
{
CrstHolder ch(&m_Crst);
CrstHolder ch(&pLoaderModule->m_Crst);

// Note that we were possibly racing to create the cookie, and another thread
// may have already created it. We could put another check
// here, but it's probably not worth the effort, so we'll just take an
// occasional duplicate cookie instead.

// Is the first block in the list full?
if (m_pVASigCookieBlock && m_pVASigCookieBlock->m_numcookies
if (pLoaderModule->m_pVASigCookieBlock && pLoaderModule->m_pVASigCookieBlock->m_numcookies
< VASigCookieBlock::kVASigCookieBlockSize)
{
// Nope, reserve a new slot in the existing block.
pCookie = &(m_pVASigCookieBlock->m_cookies[m_pVASigCookieBlock->m_numcookies]);
pCookie = &(pLoaderModule->m_pVASigCookieBlock->m_cookies[pLoaderModule->m_pVASigCookieBlock->m_numcookies]);
}
else
{
// Yes, create a new block.
VASigCookieBlock *pNewBlock = new VASigCookieBlock();

pNewBlock->m_Next = m_pVASigCookieBlock;
pNewBlock->m_Next = pLoaderModule->m_pVASigCookieBlock;
pNewBlock->m_numcookies = 0;
m_pVASigCookieBlock = pNewBlock;
pLoaderModule->m_pVASigCookieBlock = pNewBlock;
pCookie = &(pNewBlock->m_cookies[0]);
}

// Now, fill in the new cookie (assuming we had enough memory to create one.)
pCookie->pModule = this;
pCookie->pModule = pDefiningModule;
pCookie->pNDirectILStub = NULL;
pCookie->sizeOfArgs = sizeOfArgs;
pCookie->signature = vaSignature;
hez2010 marked this conversation as resolved.
Show resolved Hide resolved
pCookie->pLoaderModule = pLoaderModule;

AllocMemTracker amt;

if (classInstCount != 0)
{
TypeHandle* pClassInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(classInstCount) * S_SIZE_T(sizeof(TypeHandle))));
for (DWORD i = 0; i < classInstCount; i++)
{
pClassInst[i] = typeContext->m_classInst[i];
}
pCookie->classInst = Instantiation(pClassInst, classInstCount);
}

if (methodInstCount != 0)
{
TypeHandle* pMethodInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(methodInstCount) * S_SIZE_T(sizeof(TypeHandle))));
for (DWORD i = 0; i < methodInstCount; i++)
{
pMethodInst[i] = typeContext->m_methodInst[i];
}
pCookie->methodInst = Instantiation(pMethodInst, methodInstCount);
}

amt.SuppressRelease();

// Finally, now that it's safe for asynchronous readers to see it,
// update the count.
m_pVASigCookieBlock->m_numcookies++;
pLoaderModule->m_pVASigCookieBlock->m_numcookies++;
}
}

Expand Down
7 changes: 6 additions & 1 deletion src/coreclr/vm/ceeload.h
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,10 @@ struct VASigCookie
unsigned sizeOfArgs; // size of argument list
Volatile<PCODE> pNDirectILStub; // will be use if target is NDirect (tag == 0)
PTR_Module pModule;
PTR_Module pLoaderModule;
Signature signature;
Instantiation classInst;
Instantiation methodInst;
};

//
Expand Down Expand Up @@ -1360,7 +1363,9 @@ class Module : public ModuleBase
void NotifyEtwLoadFinished(HRESULT hr);

// Enregisters a VASig.
VASigCookie *GetVASigCookie(Signature vaSignature);
VASigCookie *GetVASigCookie(Signature vaSignature, const SigTypeContext* typeContext);
private:
static VASigCookie *GetVASigCookieWorker(Module* pDefiningModule, Module* pLoaderModule, Signature vaSignature, const SigTypeContext* typeContext);
hez2010 marked this conversation as resolved.
Show resolved Hide resolved

public:
#ifndef DACCESS_COMPILE
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/comdelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2019,7 +2019,7 @@ void COMDelegate::ThrowIfInvalidUnmanagedCallersOnlyUsage(MethodDesc* pMD)

// Arguments - Scenarios involving UnmanagedCallersOnly are handled during the jit.
bool unmanagedCallersOnlyRequiresMarshalling = false;
if (NDirect::MarshalingRequired(pMD, NULL, NULL, unmanagedCallersOnlyRequiresMarshalling))
if (NDirect::MarshalingRequired(pMD, NULL, NULL, NULL, unmanagedCallersOnlyRequiresMarshalling))
EX_THROW(EEResourceException, (kInvalidProgramException, W("InvalidProgram_NonBlittableTypes")));
}

Expand Down
33 changes: 25 additions & 8 deletions src/coreclr/vm/dllimport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ StubSigDesc::StubSigDesc(MethodDesc *pMD)
INDEBUG(InitDebugNames());
}

StubSigDesc::StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* pModule)
StubSigDesc::StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* pModule, Module* pLoaderModule)
{
CONTRACTL
{
Expand All @@ -135,13 +135,13 @@ StubSigDesc::StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* pModule)
m_tkMethodDef = pMD->GetMemberDef();
SigTypeContext::InitTypeContext(pMD, &m_typeContext);
m_pMetadataModule = pMD->GetModule();
m_pLoaderModule = pMD->GetLoaderModule(); // Used for ILStubCache selection and MethodTable creation.
m_pLoaderModule = pLoaderModule == NULL ? pMD->GetLoaderModule() : pLoaderModule; // Used for ILStubCache selection and MethodTable creation.
}
else
{
m_tkMethodDef = mdMethodDefNil;
m_pMetadataModule = m_pModule;
m_pLoaderModule = m_pModule;
m_pLoaderModule = pLoaderModule == NULL ? m_pModule : pLoaderModule;
}

INDEBUG(InitDebugNames());
Expand Down Expand Up @@ -3180,6 +3180,7 @@ BOOL NDirect::MarshalingRequired(
_In_opt_ MethodDesc* pMD,
_In_opt_ PCCOR_SIGNATURE pSig,
_In_opt_ Module* pModule,
_In_opt_ SigTypeContext* pTypeContext,
_In_ bool unmanagedCallersOnlyRequiresMarshalling)
{
CONTRACTL
Expand Down Expand Up @@ -3260,8 +3261,6 @@ BOOL NDirect::MarshalingRequired(
mdParamDef *pParamTokenArray = (mdParamDef *)_alloca(numArgs * sizeof(mdParamDef));
IMDInternalImport *pMDImport = pModule->GetMDImport();

SigTypeContext emptyTypeContext;

mdMethodDef methodToken = mdMethodDefNil;
if (pMD != NULL)
{
Expand Down Expand Up @@ -3321,7 +3320,7 @@ BOOL NDirect::MarshalingRequired(
case ELEMENT_TYPE_VALUETYPE:
case ELEMENT_TYPE_GENERICINST:
{
TypeHandle hndArgType = arg.GetTypeHandleThrowing(pModule, &emptyTypeContext);
TypeHandle hndArgType = arg.GetTypeHandleThrowing(pModule, pTypeContext);
bool isValidGeneric = IsValidForGenericMarshalling(hndArgType.GetMethodTable(), false, runtimeMarshallingEnabled);
if(!hndArgType.IsValueType() || !isValidGeneric)
return true;
Expand Down Expand Up @@ -4192,8 +4191,10 @@ namespace
pHashParams,
pParams->m_dwStubFlags,
pParams->m_pModule,
pParams->m_pLoaderModule,
pParams->m_sig.GetRawSig(),
pParams->m_sig.GetRawSigLen(),
pParams->m_pTypeContext,
pamTracker,
bILStubCreator,
pLastMD);
Expand Down Expand Up @@ -5041,6 +5042,21 @@ namespace
}
else
{
if (!pSigDesc->m_typeContext.IsEmpty())
{
// For generic calli, we only support blittable types
if (SF_IsCALLIStub(dwStubFlags)
&& NDirect::MarshalingRequired(NULL, pStubMD->GetSig(), pSigDesc->m_pModule, &pSigDesc->m_typeContext))
{
COMPlusThrow(kMarshalDirectiveException, IDS_EE_BADMARSHAL_GENERICS_RESTRICTION);
}
// We don't want to support generic varargs, so block it
else if (SF_IsVarArgStub(dwStubFlags))
{
COMPlusThrow(kNotSupportedException, BFA_GENCODE_NOT_BE_VARARG);
}
}

CreateNDirectStubWorker(pss,
pSigDesc,
nlType,
Expand Down Expand Up @@ -6027,7 +6043,7 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD)
}
}

LoaderHeap *pHeap = pVASigCookie->pModule->GetLoaderAllocator()->GetHighFrequencyHeap();
LoaderHeap *pHeap = pVASigCookie->pLoaderModule->GetLoaderAllocator()->GetHighFrequencyHeap();
PCOR_SIGNATURE new_sig = (PCOR_SIGNATURE)(void *)pHeap->AllocMem(S_SIZE_T(signature.GetRawSigLen()));
CopyMemory(new_sig, signature.GetRawSig(), signature.GetRawSigLen());

Expand Down Expand Up @@ -6065,7 +6081,8 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD)
nlType = nltAnsi;
}

StubSigDesc sigDesc(pMD, signature, pVASigCookie->pModule);
StubSigDesc sigDesc(pMD, signature, pVASigCookie->pModule, pVASigCookie->pLoaderModule);
sigDesc.InitTypeContext(pVASigCookie->classInst, pVASigCookie->methodInst);

MethodDesc* pStubMD = NDirect::CreateCLRToNativeILStub(&sigDesc,
nlType,
Expand Down
18 changes: 15 additions & 3 deletions src/coreclr/vm/dllimport.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ struct StubSigDesc
{
public:
StubSigDesc(MethodDesc* pMD);
StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* m_pModule);
StubSigDesc(MethodTable* pMT, const Signature& sig, Module* m_pModule);
StubSigDesc(const Signature& sig, Module* m_pModule);
StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* pModule, Module* pLoaderModule = NULL);
StubSigDesc(MethodTable* pMT, const Signature& sig, Module* pModule);
StubSigDesc(const Signature& sig, Module* pModule);

MethodDesc *m_pMD;
MethodTable *m_pMT;
Expand Down Expand Up @@ -56,6 +56,17 @@ struct StubSigDesc
}
}
#endif // _DEBUG

#ifndef DACCESS_COMPILE
void InitTypeContext(Instantiation classInst, Instantiation methodInst)
{
LIMITED_METHOD_CONTRACT;

_ASSERTE(m_typeContext.IsEmpty());

m_typeContext = SigTypeContext(classInst, methodInst);
}
#endif
};

//=======================================================================
Expand Down Expand Up @@ -92,6 +103,7 @@ class NDirect
_In_opt_ MethodDesc* pMD,
_In_opt_ PCCOR_SIGNATURE pSig = NULL,
_In_opt_ Module* pModule = NULL,
_In_opt_ SigTypeContext* pTypeContext = NULL,
_In_ bool unmanagedCallersOnlyRequiresMarshalling = true);

static void PopulateNDirectMethodDesc(_Inout_ NDirectMethodDesc* pNMD);
Expand Down
21 changes: 12 additions & 9 deletions src/coreclr/vm/ilstubcache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,10 @@ MethodDesc* ILStubCache::GetStubMethodDesc(
ILStubHashBlob* pHashBlob,
DWORD dwStubFlags,
Module* pSigModule,
Module* pSigLoaderModule,
PCCOR_SIGNATURE pSig,
DWORD cbSig,
SigTypeContext* pTypeContext,
AllocMemTracker* pamTracker,
bool& bILStubCreator,
MethodDesc *pLastMD)
Expand Down Expand Up @@ -538,22 +540,23 @@ MethodDesc* ILStubCache::GetStubMethodDesc(
// Couldn't find it, let's make a new one.
//

Module *pContainingModule = pSigModule;
if (pTargetMD != NULL)
if (pSigLoaderModule == NULL)
{
// loader module may be different from signature module for generic targets
pContainingModule = pTargetMD->GetLoaderModule();
pSigLoaderModule = (pTargetMD != NULL) ? pTargetMD->GetLoaderModule() : pSigModule;
}

MethodTable *pStubMT = GetOrCreateStubMethodTable(pContainingModule);

SigTypeContext typeContext;
if (pTargetMD != NULL)
if (pTypeContext == NULL)
{
SigTypeContext::InitTypeContext(pTargetMD, &typeContext);
if (pTargetMD != NULL)
{
SigTypeContext::InitTypeContext(pTargetMD, &typeContext);
}
pTypeContext = &typeContext;
}

pMD = ILStubCache::CreateNewMethodDesc(m_pAllocator->GetHighFrequencyHeap(), pStubMT, dwStubFlags, pSigModule, pSig, cbSig, &typeContext, pamTracker);
MethodTable *pStubMT = GetOrCreateStubMethodTable(pSigLoaderModule);
pMD = ILStubCache::CreateNewMethodDesc(m_pAllocator->GetHighFrequencyHeap(), pStubMT, dwStubFlags, pSigModule, pSig, cbSig, pTypeContext, pamTracker);

if (SF_IsSharedStub(dwStubFlags))
{
Expand Down
Loading
Loading