Skip to content

Commit 15ff723

Browse files
hez2010jkotas
hez2010
and
jkotas
authored
Support generic arguments for calli in CoreCLR (#97079)
* Add generic support for calli * Consider classInst as well * Block runtime marshaling for calli * Fix the error mode * Adding tests * Compare instantiations as well when lookup * Adding more test cases * Avoid getting export multiple times * Improve tests * Use standard boolean * Handle collectible assemblies * Handle inlined P/Invoke transitions * Reimplement unloadability handling * Fix native library resolving in tests * Use AllocMemTracker properly * Skip tests on windows x86 * Skip negative tests on Windows x86 and NativeAOT * Use better exception message * Exclude tests for mono * Use correct loader elsewhere * Put VASigCookieBlock on loader module * Move VASigCookie creation to a new method * Update issues.targets * Allocate StubMD on loader module * Add an error mode for generic varargs * Apply suggestions from code review Co-authored-by: Jan Kotas <jkotas@microsoft.com> * Tweak the tests * Add link to an GH issue --------- Co-authored-by: Jan Kotas <jkotas@microsoft.com>
1 parent c86ab13 commit 15ff723

File tree

11 files changed

+278
-46
lines changed

11 files changed

+278
-46
lines changed

‎src/coreclr/vm/ceeload.cpp

Lines changed: 93 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4680,7 +4680,7 @@ PTR_VOID ReflectionModule::GetRvaField(RVA field) // virtual
46804680
//==========================================================================
46814681
// Enregisters a VASig.
46824682
//==========================================================================
4683-
VASigCookie *Module::GetVASigCookie(Signature vaSignature)
4683+
VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* typeContext)
46844684
{
46854685
CONTRACT(VASigCookie*)
46864686
{
@@ -4693,79 +4693,153 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature)
46934693
}
46944694
CONTRACT_END;
46954695

4696+
Module* pLoaderModule = ClassLoader::ComputeLoaderModuleWorker(this, mdTokenNil, typeContext->m_classInst, typeContext->m_methodInst);
4697+
VASigCookie *pCookie = GetVASigCookieWorker(this, pLoaderModule, vaSignature, typeContext);
4698+
4699+
RETURN pCookie;
4700+
}
4701+
4702+
VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoaderModule, Signature vaSignature, const SigTypeContext* typeContext)
4703+
{
4704+
CONTRACT(VASigCookie*)
4705+
{
4706+
THROWS;
4707+
GC_TRIGGERS;
4708+
MODE_ANY;
4709+
POSTCONDITION(CheckPointer(RETVAL));
4710+
INJECT_FAULT(COMPlusThrowOM());
4711+
}
4712+
CONTRACT_END;
4713+
46964714
VASigCookieBlock *pBlock;
46974715
VASigCookie *pCookie;
46984716

46994717
pCookie = NULL;
47004718

47014719
// First, see if we already enregistered this sig.
47024720
// Note that we're outside the lock here, so be a bit careful with our logic
4703-
for (pBlock = m_pVASigCookieBlock; pBlock != NULL; pBlock = pBlock->m_Next)
4721+
for (pBlock = pLoaderModule->m_pVASigCookieBlock; pBlock != NULL; pBlock = pBlock->m_Next)
47044722
{
47054723
for (UINT i = 0; i < pBlock->m_numcookies; i++)
47064724
{
47074725
if (pBlock->m_cookies[i].signature.GetRawSig() == vaSignature.GetRawSig())
47084726
{
4709-
pCookie = &(pBlock->m_cookies[i]);
4710-
break;
4727+
_ASSERTE(pBlock->m_cookies[i].classInst.GetNumArgs() == typeContext->m_classInst.GetNumArgs());
4728+
_ASSERTE(pBlock->m_cookies[i].methodInst.GetNumArgs() == typeContext->m_methodInst.GetNumArgs());
4729+
4730+
bool instMatch = true;
4731+
4732+
for (DWORD j = 0; j < pBlock->m_cookies[i].classInst.GetNumArgs(); j++)
4733+
{
4734+
if (pBlock->m_cookies[i].classInst[j] != typeContext->m_classInst[j])
4735+
{
4736+
instMatch = false;
4737+
break;
4738+
}
4739+
}
4740+
4741+
if (instMatch)
4742+
{
4743+
for (DWORD j = 0; j < pBlock->m_cookies[i].methodInst.GetNumArgs(); j++)
4744+
{
4745+
if (pBlock->m_cookies[i].methodInst[j] != typeContext->m_methodInst[j])
4746+
{
4747+
instMatch = false;
4748+
break;
4749+
}
4750+
}
4751+
}
4752+
4753+
if (instMatch)
4754+
{
4755+
pCookie = &(pBlock->m_cookies[i]);
4756+
break;
4757+
}
47114758
}
47124759
}
47134760
}
4714-
4761+
47154762
if (!pCookie)
47164763
{
47174764
// If not, time to make a new one.
47184765

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

4721-
// @TODO GENERICS: We may be calling a varargs method from a
4722-
// generic type/method. Using an empty context will make such a
4723-
// case cause an unexpected exception. To make this work,
4724-
// we need to create a specialized signature for every instantiation
4725-
SigTypeContext typeContext;
4726-
4727-
MetaSig metasig(vaSignature, this, &typeContext);
4768+
MetaSig metasig(vaSignature, pDefiningModule, typeContext);
47284769
ArgIterator argit(&metasig);
47294770

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

4774+
// Prepare instantiation
4775+
LoaderAllocator *pLoaderAllocator = pLoaderModule->GetLoaderAllocator();
4776+
4777+
DWORD classInstCount = typeContext->m_classInst.GetNumArgs();
4778+
DWORD methodInstCount = typeContext->m_methodInst.GetNumArgs();
4779+
pLoaderAllocator->EnsureInstantiation(pDefiningModule, typeContext->m_classInst);
4780+
pLoaderAllocator->EnsureInstantiation(pDefiningModule, typeContext->m_methodInst);
4781+
47334782
// enable gc before taking lock
47344783
{
4735-
CrstHolder ch(&m_Crst);
4784+
CrstHolder ch(&pLoaderModule->m_Crst);
47364785

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

47424791
// Is the first block in the list full?
4743-
if (m_pVASigCookieBlock && m_pVASigCookieBlock->m_numcookies
4792+
if (pLoaderModule->m_pVASigCookieBlock && pLoaderModule->m_pVASigCookieBlock->m_numcookies
47444793
< VASigCookieBlock::kVASigCookieBlockSize)
47454794
{
47464795
// Nope, reserve a new slot in the existing block.
4747-
pCookie = &(m_pVASigCookieBlock->m_cookies[m_pVASigCookieBlock->m_numcookies]);
4796+
pCookie = &(pLoaderModule->m_pVASigCookieBlock->m_cookies[pLoaderModule->m_pVASigCookieBlock->m_numcookies]);
47484797
}
47494798
else
47504799
{
47514800
// Yes, create a new block.
47524801
VASigCookieBlock *pNewBlock = new VASigCookieBlock();
47534802

4754-
pNewBlock->m_Next = m_pVASigCookieBlock;
4803+
pNewBlock->m_Next = pLoaderModule->m_pVASigCookieBlock;
47554804
pNewBlock->m_numcookies = 0;
4756-
m_pVASigCookieBlock = pNewBlock;
4805+
pLoaderModule->m_pVASigCookieBlock = pNewBlock;
47574806
pCookie = &(pNewBlock->m_cookies[0]);
47584807
}
47594808

47604809
// Now, fill in the new cookie (assuming we had enough memory to create one.)
4761-
pCookie->pModule = this;
4810+
pCookie->pModule = pDefiningModule;
47624811
pCookie->pNDirectILStub = NULL;
47634812
pCookie->sizeOfArgs = sizeOfArgs;
47644813
pCookie->signature = vaSignature;
4814+
pCookie->pLoaderModule = pLoaderModule;
4815+
4816+
AllocMemTracker amt;
4817+
4818+
if (classInstCount != 0)
4819+
{
4820+
TypeHandle* pClassInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(classInstCount) * S_SIZE_T(sizeof(TypeHandle))));
4821+
for (DWORD i = 0; i < classInstCount; i++)
4822+
{
4823+
pClassInst[i] = typeContext->m_classInst[i];
4824+
}
4825+
pCookie->classInst = Instantiation(pClassInst, classInstCount);
4826+
}
4827+
4828+
if (methodInstCount != 0)
4829+
{
4830+
TypeHandle* pMethodInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(methodInstCount) * S_SIZE_T(sizeof(TypeHandle))));
4831+
for (DWORD i = 0; i < methodInstCount; i++)
4832+
{
4833+
pMethodInst[i] = typeContext->m_methodInst[i];
4834+
}
4835+
pCookie->methodInst = Instantiation(pMethodInst, methodInstCount);
4836+
}
4837+
4838+
amt.SuppressRelease();
47654839

47664840
// Finally, now that it's safe for asynchronous readers to see it,
47674841
// update the count.
4768-
m_pVASigCookieBlock->m_numcookies++;
4842+
pLoaderModule->m_pVASigCookieBlock->m_numcookies++;
47694843
}
47704844
}
47714845

‎src/coreclr/vm/ceeload.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,10 @@ struct VASigCookie
338338
unsigned sizeOfArgs; // size of argument list
339339
Volatile<PCODE> pNDirectILStub; // will be use if target is NDirect (tag == 0)
340340
PTR_Module pModule;
341+
PTR_Module pLoaderModule;
341342
Signature signature;
343+
Instantiation classInst;
344+
Instantiation methodInst;
342345
};
343346

344347
//
@@ -1360,7 +1363,9 @@ class Module : public ModuleBase
13601363
void NotifyEtwLoadFinished(HRESULT hr);
13611364

13621365
// Enregisters a VASig.
1363-
VASigCookie *GetVASigCookie(Signature vaSignature);
1366+
VASigCookie *GetVASigCookie(Signature vaSignature, const SigTypeContext* typeContext);
1367+
private:
1368+
static VASigCookie *GetVASigCookieWorker(Module* pDefiningModule, Module* pLoaderModule, Signature vaSignature, const SigTypeContext* typeContext);
13641369

13651370
public:
13661371
#ifndef DACCESS_COMPILE

‎src/coreclr/vm/comdelegate.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2019,7 +2019,7 @@ void COMDelegate::ThrowIfInvalidUnmanagedCallersOnlyUsage(MethodDesc* pMD)
20192019

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

‎src/coreclr/vm/dllimport.cpp

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ StubSigDesc::StubSigDesc(MethodDesc *pMD)
113113
INDEBUG(InitDebugNames());
114114
}
115115

116-
StubSigDesc::StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* pModule)
116+
StubSigDesc::StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* pModule, Module* pLoaderModule)
117117
{
118118
CONTRACTL
119119
{
@@ -135,13 +135,13 @@ StubSigDesc::StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* pModule)
135135
m_tkMethodDef = pMD->GetMemberDef();
136136
SigTypeContext::InitTypeContext(pMD, &m_typeContext);
137137
m_pMetadataModule = pMD->GetModule();
138-
m_pLoaderModule = pMD->GetLoaderModule(); // Used for ILStubCache selection and MethodTable creation.
138+
m_pLoaderModule = pLoaderModule == NULL ? pMD->GetLoaderModule() : pLoaderModule; // Used for ILStubCache selection and MethodTable creation.
139139
}
140140
else
141141
{
142142
m_tkMethodDef = mdMethodDefNil;
143143
m_pMetadataModule = m_pModule;
144-
m_pLoaderModule = m_pModule;
144+
m_pLoaderModule = pLoaderModule == NULL ? m_pModule : pLoaderModule;
145145
}
146146

147147
INDEBUG(InitDebugNames());
@@ -3180,6 +3180,7 @@ BOOL NDirect::MarshalingRequired(
31803180
_In_opt_ MethodDesc* pMD,
31813181
_In_opt_ PCCOR_SIGNATURE pSig,
31823182
_In_opt_ Module* pModule,
3183+
_In_opt_ SigTypeContext* pTypeContext,
31833184
_In_ bool unmanagedCallersOnlyRequiresMarshalling)
31843185
{
31853186
CONTRACTL
@@ -3260,8 +3261,6 @@ BOOL NDirect::MarshalingRequired(
32603261
mdParamDef *pParamTokenArray = (mdParamDef *)_alloca(numArgs * sizeof(mdParamDef));
32613262
IMDInternalImport *pMDImport = pModule->GetMDImport();
32623263

3263-
SigTypeContext emptyTypeContext;
3264-
32653264
mdMethodDef methodToken = mdMethodDefNil;
32663265
if (pMD != NULL)
32673266
{
@@ -3321,7 +3320,7 @@ BOOL NDirect::MarshalingRequired(
33213320
case ELEMENT_TYPE_VALUETYPE:
33223321
case ELEMENT_TYPE_GENERICINST:
33233322
{
3324-
TypeHandle hndArgType = arg.GetTypeHandleThrowing(pModule, &emptyTypeContext);
3323+
TypeHandle hndArgType = arg.GetTypeHandleThrowing(pModule, pTypeContext);
33253324
bool isValidGeneric = IsValidForGenericMarshalling(hndArgType.GetMethodTable(), false, runtimeMarshallingEnabled);
33263325
if(!hndArgType.IsValueType() || !isValidGeneric)
33273326
return true;
@@ -4192,8 +4191,10 @@ namespace
41924191
pHashParams,
41934192
pParams->m_dwStubFlags,
41944193
pParams->m_pModule,
4194+
pParams->m_pLoaderModule,
41954195
pParams->m_sig.GetRawSig(),
41964196
pParams->m_sig.GetRawSigLen(),
4197+
pParams->m_pTypeContext,
41974198
pamTracker,
41984199
bILStubCreator,
41994200
pLastMD);
@@ -5041,6 +5042,21 @@ namespace
50415042
}
50425043
else
50435044
{
5045+
if (!pSigDesc->m_typeContext.IsEmpty())
5046+
{
5047+
// For generic calli, we only support blittable types
5048+
if (SF_IsCALLIStub(dwStubFlags)
5049+
&& NDirect::MarshalingRequired(NULL, pStubMD->GetSig(), pSigDesc->m_pModule, &pSigDesc->m_typeContext))
5050+
{
5051+
COMPlusThrow(kMarshalDirectiveException, IDS_EE_BADMARSHAL_GENERICS_RESTRICTION);
5052+
}
5053+
// We don't want to support generic varargs, so block it
5054+
else if (SF_IsVarArgStub(dwStubFlags))
5055+
{
5056+
COMPlusThrow(kNotSupportedException, BFA_GENCODE_NOT_BE_VARARG);
5057+
}
5058+
}
5059+
50445060
CreateNDirectStubWorker(pss,
50455061
pSigDesc,
50465062
nlType,
@@ -6027,7 +6043,7 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD)
60276043
}
60286044
}
60296045

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

@@ -6065,7 +6081,8 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD)
60656081
nlType = nltAnsi;
60666082
}
60676083

6068-
StubSigDesc sigDesc(pMD, signature, pVASigCookie->pModule);
6084+
StubSigDesc sigDesc(pMD, signature, pVASigCookie->pModule, pVASigCookie->pLoaderModule);
6085+
sigDesc.InitTypeContext(pVASigCookie->classInst, pVASigCookie->methodInst);
60696086

60706087
MethodDesc* pStubMD = NDirect::CreateCLRToNativeILStub(&sigDesc,
60716088
nlType,

‎src/coreclr/vm/dllimport.h

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ struct StubSigDesc
1616
{
1717
public:
1818
StubSigDesc(MethodDesc* pMD);
19-
StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* m_pModule);
20-
StubSigDesc(MethodTable* pMT, const Signature& sig, Module* m_pModule);
21-
StubSigDesc(const Signature& sig, Module* m_pModule);
19+
StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* pModule, Module* pLoaderModule = NULL);
20+
StubSigDesc(MethodTable* pMT, const Signature& sig, Module* pModule);
21+
StubSigDesc(const Signature& sig, Module* pModule);
2222

2323
MethodDesc *m_pMD;
2424
MethodTable *m_pMT;
@@ -56,6 +56,17 @@ struct StubSigDesc
5656
}
5757
}
5858
#endif // _DEBUG
59+
60+
#ifndef DACCESS_COMPILE
61+
void InitTypeContext(Instantiation classInst, Instantiation methodInst)
62+
{
63+
LIMITED_METHOD_CONTRACT;
64+
65+
_ASSERTE(m_typeContext.IsEmpty());
66+
67+
m_typeContext = SigTypeContext(classInst, methodInst);
68+
}
69+
#endif
5970
};
6071

6172
//=======================================================================
@@ -92,6 +103,7 @@ class NDirect
92103
_In_opt_ MethodDesc* pMD,
93104
_In_opt_ PCCOR_SIGNATURE pSig = NULL,
94105
_In_opt_ Module* pModule = NULL,
106+
_In_opt_ SigTypeContext* pTypeContext = NULL,
95107
_In_ bool unmanagedCallersOnlyRequiresMarshalling = true);
96108

97109
static void PopulateNDirectMethodDesc(_Inout_ NDirectMethodDesc* pNMD);

‎src/coreclr/vm/ilstubcache.cpp

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -500,8 +500,10 @@ MethodDesc* ILStubCache::GetStubMethodDesc(
500500
ILStubHashBlob* pHashBlob,
501501
DWORD dwStubFlags,
502502
Module* pSigModule,
503+
Module* pSigLoaderModule,
503504
PCCOR_SIGNATURE pSig,
504505
DWORD cbSig,
506+
SigTypeContext* pTypeContext,
505507
AllocMemTracker* pamTracker,
506508
bool& bILStubCreator,
507509
MethodDesc *pLastMD)
@@ -538,22 +540,23 @@ MethodDesc* ILStubCache::GetStubMethodDesc(
538540
// Couldn't find it, let's make a new one.
539541
//
540542

541-
Module *pContainingModule = pSigModule;
542-
if (pTargetMD != NULL)
543+
if (pSigLoaderModule == NULL)
543544
{
544-
// loader module may be different from signature module for generic targets
545-
pContainingModule = pTargetMD->GetLoaderModule();
545+
pSigLoaderModule = (pTargetMD != NULL) ? pTargetMD->GetLoaderModule() : pSigModule;
546546
}
547547

548-
MethodTable *pStubMT = GetOrCreateStubMethodTable(pContainingModule);
549-
550548
SigTypeContext typeContext;
551-
if (pTargetMD != NULL)
549+
if (pTypeContext == NULL)
552550
{
553-
SigTypeContext::InitTypeContext(pTargetMD, &typeContext);
551+
if (pTargetMD != NULL)
552+
{
553+
SigTypeContext::InitTypeContext(pTargetMD, &typeContext);
554+
}
555+
pTypeContext = &typeContext;
554556
}
555557

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

558561
if (SF_IsSharedStub(dwStubFlags))
559562
{

0 commit comments

Comments
 (0)