Skip to content

Remove shared IL stubs for P/Invokes. #117901

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.

Sign up for GitHub

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

Merged
AaronRobinsonMSFT merged 29 commits into from
Aug 21, 2025
Merged

Remove shared IL stubs for P/Invokes. #117901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
753c75b
Remove shared IL stubs for P/Invokes.
AaronRobinsonMSFT Jul 21, 2025
ff7349e
Feedback
AaronRobinsonMSFT Jul 28, 2025
2b6f869
Bring back sharing for COM stubs.
AaronRobinsonMSFT Jul 29, 2025
739ba97
Set the CodeHeader to contain the NDirectMethodDesc
AaronRobinsonMSFT Jul 31, 2025
d91fc12
Clean up the CodeHeapRequestInfo type.
AaronRobinsonMSFT Jul 31, 2025
97ed27a
Feedback.
AaronRobinsonMSFT Jul 31, 2025
a329add
Merge remote-tracking branch 'upstream/main' into remove_shared_ilstubs
AaronRobinsonMSFT Jul 31, 2025
1eca412
Make order checks consistent for stub types.
AaronRobinsonMSFT Jul 31, 2025
689d28d
Copy/paste error.
AaronRobinsonMSFT Jul 31, 2025
e0d7236
Emit MD in profiler scenarios
AaronRobinsonMSFT Jul 31, 2025
f65f9e4
Attempt to avoid GCC assumption on null.
AaronRobinsonMSFT Jul 31, 2025
bffe12f
Try to fix up GCC.
AaronRobinsonMSFT Jul 31, 2025
e6d8b67
Initialize locals.
AaronRobinsonMSFT Aug 1, 2025
ef3089b
Merge remote-tracking branch 'upstream/main' into remove_shared_ilstubs
AaronRobinsonMSFT Aug 1, 2025
b1f10de
Missed edits
AaronRobinsonMSFT Aug 1, 2025
d9e50e0
Missed in merge.
AaronRobinsonMSFT Aug 1, 2025
80316b1
Fix vararg interop
AaronRobinsonMSFT Aug 1, 2025
2c20194
Update JIT interface
AaronRobinsonMSFT Aug 4, 2025
4f0a824
Add MethodDesc to VASigCookie
AaronRobinsonMSFT Aug 5, 2025
186aed3
Fix build breaks.
AaronRobinsonMSFT Aug 5, 2025
44bb068
Remove unused path
AaronRobinsonMSFT Aug 5, 2025
0c53561
Remove invalid assert.
AaronRobinsonMSFT Aug 5, 2025
37a1415
Add cast.
AaronRobinsonMSFT Aug 5, 2025
b56bd92
Apply suggestions from code review
AaronRobinsonMSFT Aug 6, 2025
5665400
Feedback.
AaronRobinsonMSFT Aug 6, 2025
c9131e1
Only embed PInvokeMethodDescs for varargs.
AaronRobinsonMSFT Aug 6, 2025
85c23ed
Merge remote-tracking branch 'upstream/main' into remove_shared_ilstubs
AaronRobinsonMSFT Aug 6, 2025
ccc79c7
Merge remote-tracking branch 'upstream/main' into remove_shared_ilstubs
AaronRobinsonMSFT Aug 7, 2025
88927a4
Merge branch 'main' into remove_shared_ilstubs
AaronRobinsonMSFT Aug 21, 2025
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
3 changes: 1 addition & 2 deletions src/coreclr/vm/amd64/PInvokeStubs.asm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,10 @@ NESTED_ENTRY VarargPInvokeGenILStub, _TEXT
mov r13, PINVOKE_CALLI_SIGTOKEN_REGISTER

;
; VarargPInvokeStubWorker(TransitionBlock * pTransitionBlock, VASigCookie *pVASigCookie, MethodDesc *pMD)
; VarargPInvokeStubWorker(TransitionBlock* pTransitionBlock, VASigCookie* pVASigCookie)
;
lea rcx, [rsp + __PWTB_TransitionBlock] ; pTransitionBlock*
mov rdx, PINVOKE_CALLI_SIGTOKEN_REGISTER ; pVASigCookie
mov r8, METHODDESC_REGISTER ; pMD
call VarargPInvokeStubWorker

;
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/vm/amd64/pinvokestubs.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,10 @@ NESTED_ENTRY VarargPInvokeGenILStub, _TEXT, NoHandler
mov r13, PINVOKE_CALLI_SIGTOKEN_REGISTER

//
// VarargPInvokeStubWorker(TransitionBlock * pTransitionBlock, VASigCookie *pVASigCookie, MethodDesc *pMD)
// VarargPInvokeStubWorker(TransitionBlock* pTransitionBlock, VASigCookie* pVASigCookie)
//
lea rdi, [rsp + __PWTB_TransitionBlock] // pTransitionBlock*
mov rsi, PINVOKE_CALLI_SIGTOKEN_REGISTER // pVASigCookie
mov rdx, METHODDESC_REGISTER // pMD
call C_FUNC(VarargPInvokeStubWorker)

//
Expand Down
74 changes: 36 additions & 38 deletions src/coreclr/vm/ceeload.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,16 +687,6 @@ void Module::Destruct()

ReleaseISymUnmanagedReader();

// Clean up sig cookies
VASigCookieBlock *pVASigCookieBlock = m_pVASigCookieBlock;
while (pVASigCookieBlock)
{
VASigCookieBlock *pNext = pVASigCookieBlock->m_Next;
delete pVASigCookieBlock;

pVASigCookieBlock = pNext;
}

// Clean up the IL stub cache
if (m_pILStubCache != NULL)
{
Expand Down Expand Up @@ -4207,12 +4197,13 @@ static bool MethodSignatureContainsGenericVariables(SigParser& sp)
//==========================================================================
// Enregisters a VASig.
//==========================================================================
VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* typeContext)
VASigCookie *Module::GetVASigCookie(Signature vaSignature, MethodDesc* pMD, const SigTypeContext* typeContext)
{
CONTRACT(VASigCookie*)
{
INSTANCE_CHECK;
STANDARD_VM_CHECK;
PRECONDITION(pMD == NULL || pMD->IsPInvoke()); // Only PInvoke methods are embedded in VASig cookies.
POSTCONDITION(CheckPointer(RETVAL));
INJECT_FAULT(COMPlusThrowOM());
}
Expand Down Expand Up @@ -4245,42 +4236,48 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext*
#endif
}

VASigCookie *pCookie = GetVASigCookieWorker(this, pLoaderModule, vaSignature, typeContext);
VASigCookie *pCookie = GetVASigCookieWorker(this, pLoaderModule, pMD, vaSignature, typeContext);

RETURN pCookie;
}

VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoaderModule, Signature vaSignature, const SigTypeContext* typeContext)
VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoaderModule, MethodDesc* pMD, Signature vaSignature, const SigTypeContext* typeContext)
{
CONTRACT(VASigCookie*)
{
STANDARD_VM_CHECK;
PRECONDITION(pMD == NULL || pMD->IsPInvoke());
POSTCONDITION(CheckPointer(RETVAL));
INJECT_FAULT(COMPlusThrowOM());
}
CONTRACT_END;

VASigCookieBlock *pBlock;
VASigCookie *pCookie;

pCookie = NULL;
VASigCookie *pCookie = NULL;

// First, see if we already enregistered this sig.
// First, see if we already have a match for this signature.
// Note that we're outside the lock here, so be a bit careful with our logic
VASigCookieBlock* pBlock;
for (pBlock = pLoaderModule->m_pVASigCookieBlock; pBlock != NULL; pBlock = pBlock->m_Next)
{
for (UINT i = 0; i < pBlock->m_numcookies; i++)
for (UINT i = 0; i < pBlock->m_numCookies; i++)
{
if (pBlock->m_cookies[i].signature.GetRawSig() == vaSignature.GetRawSig())
VASigCookie* cookieMaybe = &pBlock->m_cookies[i];

// Check if the cookie targets the same MethodDesc.
if (cookieMaybe->pMethodDesc != pMD)
continue;

// Check if the cookie has the same signature.
if (cookieMaybe->signature.GetRawSig() == vaSignature.GetRawSig())
{
_ASSERTE(pBlock->m_cookies[i].classInst.GetNumArgs() == typeContext->m_classInst.GetNumArgs());
_ASSERTE(pBlock->m_cookies[i].methodInst.GetNumArgs() == typeContext->m_methodInst.GetNumArgs());
_ASSERTE(cookieMaybe->classInst.GetNumArgs() == typeContext->m_classInst.GetNumArgs());
_ASSERTE(cookieMaybe->methodInst.GetNumArgs() == typeContext->m_methodInst.GetNumArgs());

bool instMatch = true;

for (DWORD j = 0; j < pBlock->m_cookies[i].classInst.GetNumArgs(); j++)
for (DWORD j = 0; j < cookieMaybe->classInst.GetNumArgs(); j++)
{
if (pBlock->m_cookies[i].classInst[j] != typeContext->m_classInst[j])
if (cookieMaybe->classInst[j] != typeContext->m_classInst[j])
{
instMatch = false;
break;
Expand All @@ -4289,9 +4286,9 @@ VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoad

if (instMatch)
{
for (DWORD j = 0; j < pBlock->m_cookies[i].methodInst.GetNumArgs(); j++)
for (DWORD j = 0; j < cookieMaybe->methodInst.GetNumArgs(); j++)
{
if (pBlock->m_cookies[i].methodInst[j] != typeContext->m_methodInst[j])
if (cookieMaybe->methodInst[j] != typeContext->m_methodInst[j])
{
instMatch = false;
break;
Expand All @@ -4301,7 +4298,7 @@ VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoad

if (instMatch)
{
pCookie = &(pBlock->m_cookies[i]);
pCookie = cookieMaybe;
break;
}
}
Expand All @@ -4321,7 +4318,7 @@ VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoad
DWORD sizeOfArgs = argit.SizeOfArgStack();

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

DWORD classInstCount = typeContext->m_classInst.GetNumArgs();
DWORD methodInstCount = typeContext->m_methodInst.GetNumArgs();
Expand All @@ -4338,26 +4335,27 @@ VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoad
// occasional duplicate cookie instead.

// Is the first block in the list full?
if (pLoaderModule->m_pVASigCookieBlock && pLoaderModule->m_pVASigCookieBlock->m_numcookies
< VASigCookieBlock::kVASigCookieBlockSize)
if (pLoaderModule->m_pVASigCookieBlock
&& pLoaderModule->m_pVASigCookieBlock->m_numCookies < VASigCookieBlock::kVASigCookieBlockSize)
{
// Nope, reserve a new slot in the existing block.
pCookie = &(pLoaderModule->m_pVASigCookieBlock->m_cookies[pLoaderModule->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();

VASigCookieBlock* pNewBlock = (VASigCookieBlock*)(void*)pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(sizeof(VASigCookieBlock)));
pNewBlock->m_Next = pLoaderModule->m_pVASigCookieBlock;
pNewBlock->m_numcookies = 0;
pNewBlock->m_numCookies = 0;
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 = pDefiningModule;
pCookie->pPInvokeILStub = 0;
pCookie->pPInvokeILStub = (PCODE)NULL;
pCookie->pMethodDesc = pMD;
pCookie->sizeOfArgs = sizeOfArgs;
pCookie->signature = vaSignature;
pCookie->pLoaderModule = pLoaderModule;
Expand All @@ -4366,7 +4364,7 @@ VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoad

if (classInstCount != 0)
{
TypeHandle* pClassInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(classInstCount) * S_SIZE_T(sizeof(TypeHandle))));
TypeHandle* pClassInst = (TypeHandle*)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];
Expand All @@ -4376,7 +4374,7 @@ VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoad

if (methodInstCount != 0)
{
TypeHandle* pMethodInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(methodInstCount) * S_SIZE_T(sizeof(TypeHandle))));
TypeHandle* pMethodInst = (TypeHandle*)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];
Expand All @@ -4388,7 +4386,7 @@ VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoad

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

Expand Down
9 changes: 5 additions & 4 deletions src/coreclr/vm/ceeload.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ struct VASigCookie
// so please keep this field first
unsigned sizeOfArgs; // size of argument list
Volatile<PCODE> pPInvokeILStub; // will be use if target is PInvoke (tag == 0)
PTR_MethodDesc pMethodDesc; // Only non-null if this is a PInvoke method
PTR_Module pModule;
PTR_Module pLoaderModule;
Signature signature;
Expand All @@ -357,7 +358,7 @@ struct VASigCookie
// allocation cost and allow proper bookkeeping.
//

struct VASigCookieBlock
struct VASigCookieBlock final
{
enum {
#ifdef _DEBUG
Expand All @@ -368,7 +369,7 @@ struct VASigCookieBlock
};

VASigCookieBlock *m_Next;
UINT m_numcookies;
UINT m_numCookies;
VASigCookie m_cookies[kVASigCookieBlockSize];
};

Expand Down Expand Up @@ -1397,9 +1398,9 @@ class Module : public ModuleBase
void NotifyEtwLoadFinished(HRESULT hr);

// Enregisters a VASig.
VASigCookie *GetVASigCookie(Signature vaSignature, const SigTypeContext* typeContext);
VASigCookie *GetVASigCookie(Signature vaSignature, MethodDesc* pMD, const SigTypeContext* typeContext);
private:
static VASigCookie *GetVASigCookieWorker(Module* pDefiningModule, Module* pLoaderModule, Signature vaSignature, const SigTypeContext* typeContext);
static VASigCookie *GetVASigCookieWorker(Module* pDefiningModule, Module* pLoaderModule, MethodDesc* pMD, Signature vaSignature, const SigTypeContext* typeContext);

public:
#ifndef DACCESS_COMPILE
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/cgensys.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ enum class CallerGCMode
// Non-CPU-specific helper functions called by the CPU-dependent code
extern "C" PCODE STDCALL PreStubWorker(TransitionBlock * pTransitionBlock, MethodDesc * pMD);

extern "C" void STDCALL VarargPInvokeStubWorker(TransitionBlock * pTransitionBlock, VASigCookie * pVASigCookie, MethodDesc * pMD);
extern "C" void STDCALL VarargPInvokeStubWorker(TransitionBlock* pTransitionBlock, VASigCookie* pVASigCookie);
extern "C" void STDCALL VarargPInvokeStub(void);
extern "C" void STDCALL VarargPInvokeStub_RetBuffArg(void);

Expand Down
Loading
Loading