Skip to content

Commit

Permalink
Support generic arguments for calli in CoreCLR (dotnet#97079)
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@hez2010 @jkotas
hez2010 and jkotas authored Feb 22, 2024
1 parent c86ab13 commit 15ff723
Show file tree
Hide file tree
Showing 11 changed files with 278 additions and 46 deletions.
112 changes: 93 additions & 19 deletions src/coreclr/vm/ceeload.cpp
View file
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;
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
View file
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);

public:
#ifndef DACCESS_COMPILE
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/comdelegate.cpp
View file
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
View file
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
View file
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
View file
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

0 comments on commit 15ff723

Please sign in to comment.