From 1e6827c70ab1e86e47c65ec715249e4b455c3046 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 1 Apr 2026 13:35:27 -0700 Subject: [PATCH 01/18] Use ILCodeStream EH APIs in PInvokeStubLinker instead of manual EH clause construction PInvokeStubLinker previously managed try-catch-finally blocks by manually creating labels, computing offsets, and constructing ILStubEHClause records. ILCodeStream already provides BeginTryBlock/EndTryBlock/BeginCatchBlock/EndCatchBlock/ BeginFinallyBlock/EndFinallyBlock APIs that handle this automatically. Changes: - Move m_buildingEHClauses and m_finishedEHClauses from ILCodeStream to ILStubLinker so EH clauses can span multiple code streams (the PInvoke stub's try-finally begins on the Marshal stream and ends on Cleanup). - Relax BeginHandler assertion to allow calling before EndTryBlock, since the finally handler may begin on a different stream than the try body. - Convert PInvokeStubLinker::Begin to use BeginTryBlock. - Convert SetCleanupNeeded to use BeginFinallyBlock instead of manually creating the cleanup finally begin label. - Convert PInvokeStubLinker::End to use EndTryBlock/EndFinallyBlock instead of manually creating try-end and finally-end labels. - Convert EmitExceptionHandler to use EndTryBlock/BeginCatchBlock/ EndCatchBlock instead of manual label management. - Replace manual EH clause construction (AppendEHClause, PopulateEHSect, GetCleanupFinallyOffsets) with ILStubLinker::GetNumEHClauses and WriteEHClauses. - Add ILStubLinker::GetEHClause for extracting resolved clause info for logging and ETW. - Remove PInvokeStubLinker::ClearCode override and manual EH label fields (m_pCleanupTryBeginLabel, m_pCleanupTryEndLabel, m_pCleanupFinallyBeginLabel, m_pCleanupFinallyEndLabel). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/dllimport.cpp | 223 ++++++++++++----------------------- src/coreclr/vm/dllimport.h | 8 -- src/coreclr/vm/stubgen.cpp | 106 +++++++++-------- src/coreclr/vm/stubgen.h | 11 +- 4 files changed, 141 insertions(+), 207 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 386c1033ea9218..73ff644618b02a 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -54,46 +54,6 @@ #include "interpexec.h" #endif // FEATURE_INTERPRETER -namespace -{ - void AppendEHClause(int nClauses, COR_ILMETHOD_SECT_EH * pEHSect, ILStubEHClause * pClause, int * pCurIdx) - { - LIMITED_METHOD_CONTRACT; - if (pClause->kind == ILStubEHClause::kNone) - return; - - int idx = *pCurIdx; - *pCurIdx = idx + 1; - - CorExceptionFlag flags; - switch (pClause->kind) - { - case ILStubEHClause::kFinally: flags = COR_ILEXCEPTION_CLAUSE_FINALLY; break; - case ILStubEHClause::kTypedCatch: flags = COR_ILEXCEPTION_CLAUSE_NONE; break; - default: - UNREACHABLE_MSG("unexpected ILStubEHClause kind"); - } - _ASSERTE(idx < nClauses); - pEHSect->Fat.Clauses[idx].Flags = flags; - pEHSect->Fat.Clauses[idx].TryOffset = pClause->dwTryBeginOffset; - pEHSect->Fat.Clauses[idx].TryLength = pClause->cbTryLength; - pEHSect->Fat.Clauses[idx].HandlerOffset = pClause->dwHandlerBeginOffset; - pEHSect->Fat.Clauses[idx].HandlerLength = pClause->cbHandlerLength; - pEHSect->Fat.Clauses[idx].ClassToken = pClause->dwTypeToken; - } - - VOID PopulateEHSect(COR_ILMETHOD_SECT_EH * pEHSect, int nClauses, ILStubEHClause * pOne, ILStubEHClause * pTwo) - { - LIMITED_METHOD_CONTRACT; - pEHSect->Fat.Kind = (CorILMethod_Sect_EHTable | CorILMethod_Sect_FatFormat); - pEHSect->Fat.DataSize = COR_ILMETHOD_SECT_EH_FAT::Size(nClauses); - - int curIdx = 0; - AppendEHClause(nClauses, pEHSect, pOne, &curIdx); - AppendEHClause(nClauses, pEHSect, pTwo, &curIdx); - } -} - StubSigDesc::StubSigDesc(MethodDesc *pMD) { CONTRACTL @@ -506,18 +466,20 @@ class ILStubState m_slIL.DoPInvoke(m_slIL.GetDispatchCodeStream(), m_dwStubFlags, pTargetMD); } - virtual void EmitExceptionHandler(LocalDesc* pNativeReturnType, LocalDesc* pManagedReturnType, - ILCodeLabel** ppTryBeginLabel, ILCodeLabel** ppTryEndAndCatchBeginLabel, ILCodeLabel** ppCatchEndAndReturnLabel) + virtual void EmitExceptionHandler(LocalDesc* pNativeReturnType, LocalDesc* pManagedReturnType) { #ifdef FEATURE_COMINTEROP STANDARD_VM_CONTRACT; ILCodeStream* pcsExceptionHandler = m_slIL.NewCodeStream(ILStubLinker::kExceptionHandler); - *ppTryEndAndCatchBeginLabel = pcsExceptionHandler->NewCodeLabel(); // try ends at the same place the catch begins - *ppCatchEndAndReturnLabel = pcsExceptionHandler->NewCodeLabel(); // catch ends at the same place we resume afterwards + ILCodeLabel* pReturnLabel = pcsExceptionHandler->NewCodeLabel(); - pcsExceptionHandler->EmitLEAVE(*ppCatchEndAndReturnLabel); - pcsExceptionHandler->EmitLabel(*ppTryEndAndCatchBeginLabel); + // End the outer try body with a leave to after the catch. + pcsExceptionHandler->EmitLEAVE(pReturnLabel); + + // Close the outer try block and open the catch handler. + pcsExceptionHandler->EndTryBlock(); + pcsExceptionHandler->BeginCatchBlock(pcsExceptionHandler->GetToken(g_pObjectClass)); BYTE nativeReturnElemType = pNativeReturnType->ElementType[0]; // return type of the stub BYTE managedReturnElemType = pManagedReturnType->ElementType[0]; // return type of the mananged target @@ -604,8 +566,9 @@ class ILStubState break; } - pcsExceptionHandler->EmitLEAVE(*ppCatchEndAndReturnLabel); - pcsExceptionHandler->EmitLabel(*ppCatchEndAndReturnLabel); + pcsExceptionHandler->EmitLEAVE(pReturnLabel); + pcsExceptionHandler->EndCatchBlock(); + pcsExceptionHandler->EmitLabel(pReturnLabel); if (nativeReturnElemType != ELEMENT_TYPE_VOID) { _ASSERTE(retvalLocalNum != (DWORD)-1); @@ -844,12 +807,9 @@ class ILStubState ConvertMethodDescSigToModuleIndependentSig(pStubMD); } - ILCodeLabel* pTryBeginLabel = nullptr; - ILCodeLabel* pTryEndAndCatchBeginLabel = nullptr; - ILCodeLabel* pCatchEndLabel = nullptr; if (hasTryCatchExceptionHandler) { - EmitExceptionHandler(&nativeReturnType, &managedReturnType, &pTryBeginLabel, &pTryEndAndCatchBeginLabel, &pCatchEndLabel); + EmitExceptionHandler(&nativeReturnType, &managedReturnType); } UINT maxStack; @@ -867,32 +827,11 @@ class ILStubState pbLocalSig = (BYTE *)pILHeader->LocalVarSig; _ASSERTE(cbSig == pILHeader->cbLocalVarSig); - ILStubEHClause cleanupTryFinally{}; - m_slIL.GetCleanupFinallyOffsets(&cleanupTryFinally); - - ILStubEHClause tryCatchClause{}; - if (hasTryCatchExceptionHandler) - { - tryCatchClause.kind = ILStubEHClause::kTypedCatch; - tryCatchClause.dwTryBeginOffset = pTryBeginLabel != nullptr ? (DWORD)pTryBeginLabel->GetCodeOffset() : 0; - tryCatchClause.dwHandlerBeginOffset = ((DWORD)pTryEndAndCatchBeginLabel->GetCodeOffset()); - tryCatchClause.cbTryLength = tryCatchClause.dwHandlerBeginOffset - tryCatchClause.dwTryBeginOffset; - tryCatchClause.cbHandlerLength = ((DWORD)pCatchEndLabel->GetCodeOffset()) - tryCatchClause.dwHandlerBeginOffset; - tryCatchClause.dwTypeToken = pcsMarshal->GetToken(g_pObjectClass); - } - - int nEHClauses = 0; - - if (tryCatchClause.cbHandlerLength != 0) - nEHClauses++; - - if (cleanupTryFinally.cbHandlerLength != 0) - nEHClauses++; - + size_t nEHClauses = m_slIL.GetNumEHClauses(); if (nEHClauses > 0) { COR_ILMETHOD_SECT_EH* pEHSect = pResolver->AllocEHSect(nEHClauses); - PopulateEHSect(pEHSect, nEHClauses, &cleanupTryFinally, &tryCatchClause); + m_slIL.WriteEHClauses(pEHSect); } m_slIL.GenerateCode(pbBuffer, cbCode); @@ -900,6 +839,19 @@ class ILStubState pResolver->SetJitFlags(jitFlags); + // Extract resolved EH clause info for logging and ETW + ILStubEHClause cleanupTryFinally{}; + ILStubEHClause tryCatchClause{}; + for (size_t i = 0; i < nEHClauses; i++) + { + ILStubEHClause clause{}; + m_slIL.GetEHClause(i, &clause); + if (clause.kind == ILStubEHClause::kFinally) + cleanupTryFinally = clause; + else if (clause.kind == ILStubEHClause::kTypedCatch) + tryCatchClause = clause; + } + #ifdef LOGGING LOG((LF_STUBS, LL_INFO1000, "---------------------------------------------------------------------\n")); LOG((LF_STUBS, LL_INFO1000, "PInvoke IL stub dump: %s::%s\n", pStubMD->m_pszDebugClassName, pStubMD->m_pszDebugMethodName)); @@ -1878,8 +1830,6 @@ PInvokeStubLinker::PInvokeStubLinker( MethodDesc* pTargetMD, int iLCIDParamIdx) : ILStubLinker(pModule, signature, pTypeContext, pTargetMD, GetILStubLinkerFlagsForPInvokeStubFlags((PInvokeStubFlags)dwStubFlags)), - m_pCleanupFinallyBeginLabel(NULL), - m_pCleanupFinallyEndLabel(NULL), m_pSkipExceptionCleanupLabel(NULL), m_fHasCleanupCode(FALSE), m_fHasExceptionCleanupCode(FALSE), @@ -2132,17 +2082,15 @@ BOOL PInvokeStubLinker::IsExceptionCleanupNeeded() return m_fHasExceptionCleanupCode; } -void PInvokeStubLinker::InitCleanupCode() +void PInvokeStubLinker::SetCleanupNeeded() { - CONTRACTL + WRAPPER_NO_CONTRACT; + + if (!m_fHasCleanupCode) { - STANDARD_VM_CHECK; - PRECONDITION(NULL == m_pCleanupFinallyBeginLabel); + m_fHasCleanupCode = TRUE; + m_pcsExceptionCleanup->BeginFinallyBlock(); } - CONTRACTL_END; - - m_pCleanupFinallyBeginLabel = NewCodeLabel(); - m_pcsExceptionCleanup->EmitLabel(m_pCleanupFinallyBeginLabel); } void PInvokeStubLinker::InitExceptionCleanupCode() @@ -2161,17 +2109,6 @@ void PInvokeStubLinker::InitExceptionCleanupCode() EmitCheckForArgCleanup(m_pcsExceptionCleanup, CLEANUP_INDEX_ALL_DONE, BranchIfMarshaled, m_pSkipExceptionCleanupLabel); } -void PInvokeStubLinker::SetCleanupNeeded() -{ - WRAPPER_NO_CONTRACT; - - if (!m_fHasCleanupCode) - { - m_fHasCleanupCode = TRUE; - InitCleanupCode(); - } -} - void PInvokeStubLinker::SetExceptionCleanupNeeded() { WRAPPER_NO_CONTRACT; @@ -2249,8 +2186,19 @@ void PInvokeStubLinker::Begin(DWORD dwStubFlags) } } - m_pCleanupTryBeginLabel = NewCodeLabel(); - m_pcsMarshal->EmitLabel(m_pCleanupTryBeginLabel); + bool hasTryCatchForHRESULT = SF_IsReverseCOMStub(dwStubFlags) + && !SF_IsFieldGetterStub(dwStubFlags) + && !SF_IsFieldSetterStub(dwStubFlags); + + // If we have an outer try-catch for HRESULT conversion, begin the + // outer try first. The catch handler will be emitted later in + // EmitExceptionHandler. + if (hasTryCatchForHRESULT) + { + m_pcsSetup->BeginTryBlock(); + } + + m_pcsMarshal->BeginTryBlock(); } void PInvokeStubLinker::End(DWORD dwStubFlags) @@ -2305,8 +2253,8 @@ void PInvokeStubLinker::End(DWORD dwStubFlags) // if (IsCleanupNeeded()) { - m_pCleanupFinallyEndLabel = NewCodeLabel(); - m_pCleanupTryEndLabel = NewCodeLabel(); + // Create a leave target label positioned outside the try block. + ILCodeLabel* pLeaveTarget = NewCodeLabel(); if (IsExceptionCleanupNeeded()) { @@ -2314,19 +2262,11 @@ void PInvokeStubLinker::End(DWORD dwStubFlags) EmitSetArgMarshalIndex(m_pcsUnmarshal, CLEANUP_INDEX_ALL_DONE); } - // Emit a leave at the end of the try block. If we have an outer try/catch, we need - // to leave to the beginning of the ExceptionHandler code stream, which follows the - // Cleanup code stream. If we don't, we can just leave to the tail end of the - // Unmarshal code stream where we'll emit our RET. - - ILCodeLabel* pLeaveTarget = m_pCleanupTryEndLabel; - if (hasTryCatchForHRESULT) - { - pLeaveTarget = m_pCleanupFinallyEndLabel; - } - + // Emit a leave at the end of the try block. m_pcsUnmarshal->EmitLEAVE(pLeaveTarget); - m_pcsUnmarshal->EmitLabel(m_pCleanupTryEndLabel); + + // End the try block. + m_pcsUnmarshal->EndTryBlock(); // Emit a call to destroy the clean-up list if needed. if (IsCleanupWorkListSetup()) @@ -2335,9 +2275,28 @@ void PInvokeStubLinker::End(DWORD dwStubFlags) m_pcsCleanup->EmitCALL(METHOD__STUBHELPERS__DESTROY_CLEANUP_LIST, 1, 0); } - // Emit the endfinally. + // Emit the endfinally and close the finally block. m_pcsCleanup->EmitENDFINALLY(); - m_pcsCleanup->EmitLabel(m_pCleanupFinallyEndLabel); + m_pcsCleanup->EndFinallyBlock(); + + // Position the leave target after the try-finally construct. + // If we have an outer try/catch, we leave to after the finally handler + // (where the exception handler code will follow). + // Otherwise, we leave to right after the try block where we'll emit our RET. + if (hasTryCatchForHRESULT) + { + m_pcsCleanup->EmitLabel(pLeaveTarget); + } + else + { + m_pcsUnmarshal->EmitLabel(pLeaveTarget); + } + } + else + { + // No cleanup needed - cancel the try block started in Begin(). + _ASSERTE(m_buildingEHClauses.GetCount() > 0); + m_buildingEHClauses.SetCount(m_buildingEHClauses.GetCount() - 1); } if (IsExceptionCleanupNeeded()) @@ -2458,42 +2417,6 @@ void PInvokeStubLinker::EmitLogNativeArgument(ILCodeStream* pslILEmit, DWORD dwP pslILEmit->EmitCALL(METHOD__STUBHELPERS__LOG_PINNED_ARGUMENT, 2, 0); } -#ifndef DACCESS_COMPILE -void PInvokeStubLinker::GetCleanupFinallyOffsets(ILStubEHClause * pClause) -{ - CONTRACTL - { - STANDARD_VM_CHECK; - PRECONDITION(CheckPointer(pClause)); - } - CONTRACTL_END; - - if (m_pCleanupFinallyEndLabel) - { - _ASSERTE(m_pCleanupFinallyBeginLabel); - _ASSERTE(m_pCleanupTryBeginLabel); - _ASSERTE(m_pCleanupTryEndLabel); - - pClause->kind = ILStubEHClause::kFinally; - pClause->dwTryBeginOffset = (DWORD)m_pCleanupTryBeginLabel->GetCodeOffset(); - pClause->cbTryLength = (DWORD)m_pCleanupTryEndLabel->GetCodeOffset() - pClause->dwTryBeginOffset; - pClause->dwHandlerBeginOffset = (DWORD)m_pCleanupFinallyBeginLabel->GetCodeOffset(); - pClause->cbHandlerLength = (DWORD)m_pCleanupFinallyEndLabel->GetCodeOffset() - pClause->dwHandlerBeginOffset; - } -} -#endif // DACCESS_COMPILE - -void PInvokeStubLinker::ClearCode() -{ - WRAPPER_NO_CONTRACT; - ILStubLinker::ClearCode(); - - m_pCleanupTryBeginLabel = 0; - m_pCleanupTryEndLabel = 0; - m_pCleanupFinallyBeginLabel = 0; - m_pCleanupFinallyEndLabel = 0; -} - #ifdef PROFILING_SUPPORTED DWORD PInvokeStubLinker::EmitProfilerBeginTransitionCallback(ILCodeStream* pcsEmit, DWORD dwStubFlags) { diff --git a/src/coreclr/vm/dllimport.h b/src/coreclr/vm/dllimport.h index 5913af5dccd612..b8b8340b9c3836 100644 --- a/src/coreclr/vm/dllimport.h +++ b/src/coreclr/vm/dllimport.h @@ -483,7 +483,6 @@ class PInvokeStubLinker : public ILStubLinker void SetCleanupNeeded(); void SetExceptionCleanupNeeded(); BOOL IsCleanupWorkListSetup(); - void GetCleanupFinallyOffsets(ILStubEHClause * pClause); void SetInteropParamExceptionInfo(UINT resID, UINT paramIdx); bool HasInteropParamExceptionInfo(); @@ -492,8 +491,6 @@ class PInvokeStubLinker : public ILStubLinker return m_targetHasThis == TRUE; } - void ClearCode(); - enum { CLEANUP_INDEX_ARG0_MARSHAL = 0x00000000, // cleanup index of the first argument (marshal and retval unmarshal stream) @@ -524,7 +521,6 @@ class PInvokeStubLinker : public ILStubLinker protected: BOOL IsCleanupNeeded(); BOOL IsExceptionCleanupNeeded(); - void InitCleanupCode(); void InitExceptionCleanupCode(); @@ -538,10 +534,6 @@ class PInvokeStubLinker : public ILStubLinker ILCodeStream* m_pcsCleanup; - ILCodeLabel* m_pCleanupTryBeginLabel; - ILCodeLabel* m_pCleanupTryEndLabel; - ILCodeLabel* m_pCleanupFinallyBeginLabel; - ILCodeLabel* m_pCleanupFinallyEndLabel; ILCodeLabel* m_pSkipExceptionCleanupLabel; #ifdef FEATURE_COMINTEROP diff --git a/src/coreclr/vm/stubgen.cpp b/src/coreclr/vm/stubgen.cpp index 62e3cd0c5c6c34..9ac36366c1b9d8 100644 --- a/src/coreclr/vm/stubgen.cpp +++ b/src/coreclr/vm/stubgen.cpp @@ -175,14 +175,15 @@ void ILCodeStream::Emit(ILInstrEnum instr, INT16 iStackDelta, UINT_PTR uArg) pInstrBuffer[idxCurInstr].iStackDelta = iStackDelta; pInstrBuffer[idxCurInstr].uArg = uArg; - if(m_buildingEHClauses.GetCount() > 0) + if(m_pOwner->m_buildingEHClauses.GetCount() > 0) { - ILStubEHClauseBuilder& clause = m_buildingEHClauses[m_buildingEHClauses.GetCount() - 1]; + ILStubEHClauseBuilder& clause = m_pOwner->m_buildingEHClauses[m_pOwner->m_buildingEHClauses.GetCount() - 1]; if (clause.tryBeginLabel != NULL && clause.tryEndLabel != NULL && clause.handlerBeginLabel != NULL && clause.kind == ILStubEHClause::kTypedCatch) { - if (clause.handlerBeginLabel->m_idxLabeledInstruction == idxCurInstr) + if (clause.handlerBeginLabel->m_pCodeStreamOfLabel == this && + clause.handlerBeginLabel->m_idxLabeledInstruction == idxCurInstr) { // Catch clauses start with an exception on the stack pInstrBuffer[idxCurInstr].iStackDelta++; @@ -803,10 +804,10 @@ size_t ILStubLinker::Link(UINT* puMaxStack) UINT uMaxStack = 0; DEBUG_STMT(bool fStackUnderflow = false); + _ASSERTE(m_buildingEHClauses.GetCount() == 0); + while (pCurrentStream) { - _ASSERTE(pCurrentStream->m_buildingEHClauses.GetCount() == 0); - if (pCurrentStream->m_pqbILInstructions) { ILInstruction* pInstrBuffer = (ILInstruction*)pCurrentStream->m_pqbILInstructions->Ptr(); @@ -848,54 +849,60 @@ size_t ILStubLinker::Link(UINT* puMaxStack) size_t ILStubLinker::GetNumEHClauses() { - size_t result = 0; - for (ILCodeStream* stream = m_pCodeStreamList; stream; stream = stream->m_pNextStream) - { - result += stream->m_finishedEHClauses.GetCount(); - } - - return result; + return m_finishedEHClauses.GetCount(); } void ILStubLinker::WriteEHClauses(COR_ILMETHOD_SECT_EH* pSect) { unsigned int clauseIndex = 0; - for (ILCodeStream* stream = m_pCodeStreamList; stream; stream = stream->m_pNextStream) + const SArray& clauses = m_finishedEHClauses; + for (COUNT_T i = 0; i < clauses.GetCount(); i++) { - const SArray& clauses = stream->m_finishedEHClauses; - for (COUNT_T i = 0; i < clauses.GetCount(); i++) - { - const ILStubEHClauseBuilder& builder = clauses[i]; + const ILStubEHClauseBuilder& builder = clauses[i]; - CorExceptionFlag flags; - switch (builder.kind) - { - case ILStubEHClause::kTypedCatch: flags = COR_ILEXCEPTION_CLAUSE_NONE; break; - case ILStubEHClause::kFinally: flags = COR_ILEXCEPTION_CLAUSE_FINALLY; break; - default: UNREACHABLE_MSG("unexpected EH clause kind"); - } + CorExceptionFlag flags; + switch (builder.kind) + { + case ILStubEHClause::kTypedCatch: flags = COR_ILEXCEPTION_CLAUSE_NONE; break; + case ILStubEHClause::kFinally: flags = COR_ILEXCEPTION_CLAUSE_FINALLY; break; + default: UNREACHABLE_MSG("unexpected EH clause kind"); + } - size_t tryBegin = builder.tryBeginLabel->GetCodeOffset(); - size_t tryEnd = builder.tryEndLabel->GetCodeOffset(); - size_t handlerBegin = builder.handlerBeginLabel->GetCodeOffset(); - size_t handlerEnd = builder.handlerEndLabel->GetCodeOffset(); + size_t tryBegin = builder.tryBeginLabel->GetCodeOffset(); + size_t tryEnd = builder.tryEndLabel->GetCodeOffset(); + size_t handlerBegin = builder.handlerBeginLabel->GetCodeOffset(); + size_t handlerEnd = builder.handlerEndLabel->GetCodeOffset(); - IMAGE_COR_ILMETHOD_SECT_EH_CLAUSE_FAT& clause = pSect->Fat.Clauses[clauseIndex]; - clause.Flags = flags; - clause.TryOffset = static_cast(tryBegin); - clause.TryLength = static_cast(tryEnd - tryBegin); - clause.HandlerOffset = static_cast(handlerBegin); - clause.HandlerLength = static_cast(handlerEnd - handlerBegin); - clause.ClassToken = builder.typeToken; + IMAGE_COR_ILMETHOD_SECT_EH_CLAUSE_FAT& clause = pSect->Fat.Clauses[clauseIndex]; + clause.Flags = flags; + clause.TryOffset = static_cast(tryBegin); + clause.TryLength = static_cast(tryEnd - tryBegin); + clause.HandlerOffset = static_cast(handlerBegin); + clause.HandlerLength = static_cast(handlerEnd - handlerBegin); + clause.ClassToken = builder.typeToken; - clauseIndex++; - } + clauseIndex++; } pSect->Fat.Kind = CorILMethod_Sect_EHTable | CorILMethod_Sect_FatFormat; pSect->Fat.DataSize = COR_ILMETHOD_SECT_EH_FAT::Size(clauseIndex); } +void ILStubLinker::GetEHClause(size_t index, ILStubEHClause* pClause) +{ + _ASSERTE(index < m_finishedEHClauses.GetCount()); + const ILStubEHClauseBuilder& builder = m_finishedEHClauses[static_cast(index)]; + + pClause->kind = builder.kind; + pClause->dwTryBeginOffset = static_cast(builder.tryBeginLabel->GetCodeOffset()); + DWORD tryEnd = static_cast(builder.tryEndLabel->GetCodeOffset()); + pClause->cbTryLength = tryEnd - pClause->dwTryBeginOffset; + pClause->dwHandlerBeginOffset = static_cast(builder.handlerBeginLabel->GetCodeOffset()); + DWORD handlerEnd = static_cast(builder.handlerEndLabel->GetCodeOffset()); + pClause->cbHandlerLength = handlerEnd - pClause->dwHandlerBeginOffset; + pClause->dwTypeToken = builder.typeToken; +} + #ifdef _DEBUG static const PCSTR s_rgOpNames[] = @@ -1072,7 +1079,7 @@ LPCSTR ILCodeStream::GetStreamDescription(ILStubLinker::CodeStreamType streamTyp void ILCodeStream::BeginTryBlock() { - ILStubEHClauseBuilder& clause = *m_buildingEHClauses.Append(); + ILStubEHClauseBuilder& clause = *m_pOwner->m_buildingEHClauses.Append(); memset(&clause, 0, sizeof(ILStubEHClauseBuilder)); clause.kind = ILStubEHClause::kNone; clause.tryBeginLabel = NewCodeLabel(); @@ -1081,8 +1088,8 @@ void ILCodeStream::BeginTryBlock() void ILCodeStream::EndTryBlock() { - _ASSERTE(m_buildingEHClauses.GetCount() > 0); - ILStubEHClauseBuilder& clause = m_buildingEHClauses[m_buildingEHClauses.GetCount() - 1]; + _ASSERTE(m_pOwner->m_buildingEHClauses.GetCount() > 0); + ILStubEHClauseBuilder& clause = m_pOwner->m_buildingEHClauses[m_pOwner->m_buildingEHClauses.GetCount() - 1]; _ASSERTE(clause.tryBeginLabel != NULL && clause.tryEndLabel == NULL); clause.tryEndLabel = NewCodeLabel(); @@ -1091,10 +1098,13 @@ void ILCodeStream::EndTryBlock() void ILCodeStream::BeginHandler(DWORD kind, DWORD typeToken) { - _ASSERTE(m_buildingEHClauses.GetCount() > 0); - ILStubEHClauseBuilder& clause = m_buildingEHClauses[m_buildingEHClauses.GetCount() - 1]; + _ASSERTE(m_pOwner->m_buildingEHClauses.GetCount() > 0); + ILStubEHClauseBuilder& clause = m_pOwner->m_buildingEHClauses[m_pOwner->m_buildingEHClauses.GetCount() - 1]; - _ASSERTE(clause.tryBeginLabel != NULL && clause.tryEndLabel != NULL && + // tryEndLabel may not be set yet when the handler begins on a different + // code stream than the try body (e.g. PInvoke cleanup finally). + // It must be set before the handler ends. + _ASSERTE(clause.tryBeginLabel != NULL && clause.handlerBeginLabel == NULL && clause.kind == ILStubEHClause::kNone); clause.kind = kind; @@ -1105,8 +1115,8 @@ void ILCodeStream::BeginHandler(DWORD kind, DWORD typeToken) void ILCodeStream::EndHandler(DWORD kind) { - _ASSERTE(m_buildingEHClauses.GetCount() > 0); - ILStubEHClauseBuilder& clause = m_buildingEHClauses[m_buildingEHClauses.GetCount() - 1]; + _ASSERTE(m_pOwner->m_buildingEHClauses.GetCount() > 0); + ILStubEHClauseBuilder& clause = m_pOwner->m_buildingEHClauses[m_pOwner->m_buildingEHClauses.GetCount() - 1]; _ASSERTE(clause.tryBeginLabel != NULL && clause.tryEndLabel != NULL && clause.handlerBeginLabel != NULL && clause.handlerEndLabel == NULL && @@ -1115,8 +1125,8 @@ void ILCodeStream::EndHandler(DWORD kind) clause.handlerEndLabel = NewCodeLabel(); EmitLabel(clause.handlerEndLabel); - m_finishedEHClauses.Append(clause); - m_buildingEHClauses.SetCount(m_buildingEHClauses.GetCount() - 1); + m_pOwner->m_finishedEHClauses.Append(clause); + m_pOwner->m_buildingEHClauses.SetCount(m_pOwner->m_buildingEHClauses.GetCount() - 1); } void ILCodeStream::BeginCatchBlock(int token) @@ -3338,6 +3348,8 @@ void ILStubLinker::ClearCode() DeleteCodeLabels(); ClearCodeStreams(); + m_buildingEHClauses.Clear(); + m_finishedEHClauses.Clear(); } void ILStubLinker::SetStubMethodDesc(MethodDesc* pMD) diff --git a/src/coreclr/vm/stubgen.h b/src/coreclr/vm/stubgen.h index 6d1c64c59f4154..846d7ee6e0a418 100644 --- a/src/coreclr/vm/stubgen.h +++ b/src/coreclr/vm/stubgen.h @@ -775,6 +775,8 @@ class ILStubLinker size_t GetNumEHClauses(); // Write out EH clauses. Number of items written out will be GetNumEHCLauses(). void WriteEHClauses(COR_ILMETHOD_SECT_EH* sect); + // Get resolved EH clause info (must be called after Link()). + void GetEHClause(size_t index, ILStubEHClause* pClause); TokenLookupMap* GetTokenLookupMap() { LIMITED_METHOD_CONTRACT; return &m_tokenMap; } @@ -878,6 +880,13 @@ class ILStubLinker // We need this MethodDesc so we can reconstruct the generics // SigTypeContext info, if needed. MethodDesc * m_pMD; + + // EH clause tracking is owned by the linker so that Begin/End calls + // can span different ILCodeStream instances (the PInvoke stub's + // try-finally, for example, begins on the Marshal stream and the + // handler ends on the Cleanup stream). + SArray m_buildingEHClauses; + SArray m_finishedEHClauses; }; // class ILStubLinker @@ -1142,8 +1151,6 @@ class ILCodeStream ILCodeStreamBuffer* m_pqbILInstructions; UINT m_uCurInstrIdx; ILStubLinker::CodeStreamType m_codeStreamType; // Type of the ILCodeStream - SArray m_buildingEHClauses; - SArray m_finishedEHClauses; #ifndef TARGET_64BIT const static UINT32 SPECIAL_VALUE_NAN_64_ON_32 = 0xFFFFFFFF; From 1abf45e80d349e43fc22782e4b70d450d1984375 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 1 Apr 2026 13:45:27 -0700 Subject: [PATCH 02/18] Use FinalizeILStub in the interop IL stub logic --- src/coreclr/vm/dllimport.cpp | 44 +++++++------------------------ src/coreclr/vm/ilstubresolver.cpp | 4 +-- src/coreclr/vm/ilstubresolver.h | 2 +- 3 files changed, 13 insertions(+), 37 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 73ff644618b02a..cd343f916bc749 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -812,37 +812,13 @@ class ILStubState EmitExceptionHandler(&nativeReturnType, &managedReturnType); } - UINT maxStack; - size_t cbCode; - DWORD cbSig; - BYTE * pbBuffer; - BYTE * pbLocalSig; - - cbCode = m_slIL.Link(&maxStack); - cbSig = m_slIL.GetLocalSigSize(); - - ILStubResolver * pResolver = pStubMD->AsDynamicMethodDesc()->GetILStubResolver(); - COR_ILMETHOD_DECODER * pILHeader = pResolver->AllocGeneratedIL(cbCode, cbSig, maxStack); - pbBuffer = (BYTE *)pILHeader->Code; - pbLocalSig = (BYTE *)pILHeader->LocalVarSig; - _ASSERTE(cbSig == pILHeader->cbLocalVarSig); - - size_t nEHClauses = m_slIL.GetNumEHClauses(); - if (nEHClauses > 0) - { - COR_ILMETHOD_SECT_EH* pEHSect = pResolver->AllocEHSect(nEHClauses); - m_slIL.WriteEHClauses(pEHSect); - } - - m_slIL.GenerateCode(pbBuffer, cbCode); - m_slIL.GetLocalSig(pbLocalSig, cbSig); - - pResolver->SetJitFlags(jitFlags); + ILStubResolver* pResolver = pStubMD->AsDynamicMethodDesc()->GetILStubResolver(); + COR_ILMETHOD_DECODER* pILHeader = pResolver->FinalizeILStub(&m_slIL, jitFlags); // Extract resolved EH clause info for logging and ETW ILStubEHClause cleanupTryFinally{}; ILStubEHClause tryCatchClause{}; - for (size_t i = 0; i < nEHClauses; i++) + for (size_t i = 0; i < pILHeader->EHCount(); i++) { ILStubEHClause clause{}; m_slIL.GetEHClause(i, &clause); @@ -867,11 +843,11 @@ class ILStubState pStubMD->GetSig(&pManagedSig, &cManagedSig); - PrettyPrintSig(pManagedSig, cManagedSig, "*", &qbManaged, pStubMD->GetMDImport(), NULL); - PrettyPrintSig(pbLocalSig, cbSig, NULL, &qbLocal, pIMDI, NULL); + PrettyPrintSig(pManagedSig, cManagedSig, "*", &qbManaged, pStubMD->GetMDImport(), NULL); + PrettyPrintSig(pILHeader->LocalVarSig, pILHeader->cbLocalVarSig, NULL, &qbLocal, pIMDI, NULL); LOG((LF_STUBS, LL_INFO1000, "incoming managed sig: %p: %s\n", pManagedSig, qbManaged.Ptr())); - LOG((LF_STUBS, LL_INFO1000, "locals sig: %p: %s\n", pbLocalSig+1, qbLocal.Ptr())); + LOG((LF_STUBS, LL_INFO1000, "locals sig: %p: %s\n", pILHeader->LocalVarSig, qbLocal.Ptr())); if (cleanupTryFinally.cbHandlerLength != 0) { @@ -903,13 +879,13 @@ class ILStubState { EtwOnILStubGenerated( pStubMD, - pbLocalSig, - cbSig, + pILHeader->LocalVarSig, + pILHeader->cbLocalVarSig, jitFlags, &tryCatchClause, &cleanupTryFinally, - maxStack, - (DWORD)cbCode + pILHeader->GetMaxStack(), + (DWORD)pILHeader->GetCodeSize() ); } diff --git a/src/coreclr/vm/ilstubresolver.cpp b/src/coreclr/vm/ilstubresolver.cpp index adc49c75040695..104c871e0e9e23 100644 --- a/src/coreclr/vm/ilstubresolver.cpp +++ b/src/coreclr/vm/ilstubresolver.cpp @@ -453,7 +453,7 @@ COR_ILMETHOD_DECODER* ILStubResolver::AllocGeneratedIL( } // ILStubResolver::AllocGeneratedIL -COR_ILMETHOD_DECODER* ILStubResolver::FinalizeILStub(ILStubLinker* sl) +COR_ILMETHOD_DECODER* ILStubResolver::FinalizeILStub(ILStubLinker* sl, CORJIT_FLAGS corJitFlags) { STANDARD_VM_CONTRACT; _ASSERTE(!IsILGenerated()); @@ -479,7 +479,7 @@ COR_ILMETHOD_DECODER* ILStubResolver::FinalizeILStub(ILStubLinker* sl) // Store the token lookup map SetTokenLookupMap(sl->GetTokenLookupMap()); - SetJitFlags(CORJIT_FLAGS(CORJIT_FLAGS::CORJIT_FLAG_IL_STUB)); + SetJitFlags(corJitFlags); return pILHeader; } diff --git a/src/coreclr/vm/ilstubresolver.h b/src/coreclr/vm/ilstubresolver.h index 7ead8002d49743..ba9a013df15b95 100644 --- a/src/coreclr/vm/ilstubresolver.h +++ b/src/coreclr/vm/ilstubresolver.h @@ -66,7 +66,7 @@ class ILStubResolver : public DynamicResolver COR_ILMETHOD_DECODER* AllocGeneratedIL(size_t cbCode, DWORD cbLocalSig, UINT maxStack); COR_ILMETHOD_SECT_EH* AllocEHSect(size_t nClauses); - COR_ILMETHOD_DECODER* FinalizeILStub(ILStubLinker* sl); + COR_ILMETHOD_DECODER* FinalizeILStub(ILStubLinker* sl, CORJIT_FLAGS corJitFlags = CORJIT_FLAGS(CORJIT_FLAGS::CORJIT_FLAG_IL_STUB)); #endif // !DACCESS_COMPILE static void StubGenFailed(ILStubResolver* pResolver); From 5a476d5cc9e301dd3e22ea43e10974046799c533 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 1 Apr 2026 14:14:44 -0700 Subject: [PATCH 03/18] Refactor stub generation APIs to be shaped more like how the transient IL APIs are shaped --- src/coreclr/vm/dllimport.cpp | 45 ++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index cd343f916bc749..df18290aa27302 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -579,7 +579,7 @@ class ILStubState } #ifndef DACCESS_COMPILE - virtual void FinishEmit(MethodDesc* pStubMD) + virtual COR_ILMETHOD_DECODER* FinishEmit(MethodDesc* pStubMD, ILStubResolver* pResolver) { STANDARD_VM_CONTRACT; @@ -807,12 +807,15 @@ class ILStubState ConvertMethodDescSigToModuleIndependentSig(pStubMD); } + pResolver->SetStubTargetMethodSig( + GetStubTargetMethodSig(), + GetStubTargetMethodSigLength()); + if (hasTryCatchExceptionHandler) { EmitExceptionHandler(&nativeReturnType, &managedReturnType); } - ILStubResolver* pResolver = pStubMD->AsDynamicMethodDesc()->GetILStubResolver(); COR_ILMETHOD_DECODER* pILHeader = pResolver->FinalizeILStub(&m_slIL, jitFlags); // Extract resolved EH clause info for logging and ETW @@ -889,6 +892,7 @@ class ILStubState ); } + return pILHeader; } // @@ -1167,8 +1171,6 @@ class ILStubState m_qbNativeFnSigBuffer.Shrink(0); } - TokenLookupMap* GetTokenLookupMap() { WRAPPER_NO_CONTRACT; return m_slIL.GetTokenLookupMap(); } - DWORD GetFlags() const { return m_dwStubFlags; } protected: @@ -1530,7 +1532,7 @@ class LateBoundCLRToCOM_ILStubState final : public ILStubState } } - void FinishEmit(MethodDesc* pStubMD) + COR_ILMETHOD_DECODER* FinishEmit(MethodDesc* pStubMD, ILStubResolver* pResolver) { if (m_wrapperTypesNeeded) { @@ -1555,7 +1557,7 @@ class LateBoundCLRToCOM_ILStubState final : public ILStubState } } - ILStubState::FinishEmit(pStubMD); + return ILStubState::FinishEmit(pStubMD, pResolver); } void EmitInvokeTarget(MethodDesc* pTargetMD, MethodDesc* pStubMD) @@ -3651,16 +3653,18 @@ static MarshalInfo::MarshalType DoMarshalReturnValue(MetaSig& msig, // Note that this function may now throw if it fails to create // a stub. //--------------------------------------------------------- -static void CreatePInvokeStubWorker(ILStubState* pss, - StubSigDesc* pSigDesc, - CorNativeLinkType nlType, - CorNativeLinkFlags nlFlags, - CorInfoCallConvExtension unmgdCallConv, - DWORD dwStubFlags, - MethodDesc *pMD, - mdParamDef* pParamTokenArray, - int iLCIDArg - ) +static COR_ILMETHOD_DECODER* CreatePInvokeStubWorker( + ILStubState* pss, + ILStubResolver* pResolver, + StubSigDesc* pSigDesc, + CorNativeLinkType nlType, + CorNativeLinkFlags nlFlags, + CorInfoCallConvExtension unmgdCallConv, + DWORD dwStubFlags, + MethodDesc* pMD, + mdParamDef* pParamTokenArray, + int iLCIDArg + ) { CONTRACTL { @@ -3922,7 +3926,7 @@ static void CreatePInvokeStubWorker(ILStubState* pss, // FinishEmit needs to know the native stack arg size so we call it after the number // has been set in the stub MD (code:DynamicMethodDesc.SetNativeStackArgSize) - pss->FinishEmit(pMD); + return pss->FinishEmit(pMD, pResolver); } static void CreateStructMarshalIL(MethodTable* pMT, PInvokeStubLinker* stubLinker, DWORD dwMarshalOperationFlags) @@ -5240,6 +5244,7 @@ namespace } CreatePInvokeStubWorker(pss, + pResolver, pSigDesc, nlType, nlFlags, @@ -5249,12 +5254,6 @@ namespace pParamTokenArray, iLCIDArg); - pResolver->SetTokenLookupMap(pss->GetTokenLookupMap()); - - pResolver->SetStubTargetMethodSig( - pss->GetStubTargetMethodSig(), - pss->GetStubTargetMethodSigLength()); - // we successfully generated the IL stub sgh.SuppressRelease(); } From 34263725b1360dd6fc9259ac167f32894c062795 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 1 Apr 2026 14:55:07 -0700 Subject: [PATCH 04/18] Send non-vararg PInvokes down the transient IL path instead of the IL stub path --- src/coreclr/vm/dllimport.cpp | 87 ++++++++++++++++++++++-------------- src/coreclr/vm/dllimport.h | 4 ++ src/coreclr/vm/prestub.cpp | 36 +++++++++------ 3 files changed, 79 insertions(+), 48 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index df18290aa27302..18b154a05ba800 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -800,10 +800,11 @@ class ILStubState // is the target signature. We need to swap them. SwapStubSignatures(pStubMD); } - else + else if (pStubMD->IsDynamicMethod()) { // If we're not in a Reverse stub, the signatures are correct, - // but we need to convert the signature into a module-independent form. + // but we need to convert the signature into a module-independent form + // if our signature is not backed by metadata. ConvertMethodDescSigToModuleIndependentSig(pStubMD); } @@ -4086,6 +4087,43 @@ bool StructMarshalStubs::TryGenerateStructMarshallingMethod(MethodDesc* pMD, Dyn return true; } +COR_ILMETHOD_DECODER* PInvoke::CreatePInvokeMethodIL(PInvokeMethodDesc* pMD, DynamicResolver** ppResolver) +{ + STANDARD_VM_CONTRACT; + + _ASSERTE(pMD != nullptr); + + PInvokeStaticSigInfo sigInfo; + PInvoke::InitializeSigInfoAndPopulatePInvokeMethodDesc(pMD, &sigInfo); + + StubSigDesc sigDesc(pMD, sigInfo.GetSignature(), sigInfo.GetModule()); + + DWORD dwStubFlags = sigInfo.GetStubFlags(); + int iLCIDArg = 0; + int numArgs = 0; + int numParamTokens = 0; + mdParamDef* pParamTokenArray = NULL; + + CreatePInvokeStubAccessMetadata(&sigDesc, + sigInfo.GetCallConv(), + &dwStubFlags, + &iLCIDArg, + &numArgs); + + Module *pModule = sigDesc.m_pModule; + numParamTokens = numArgs + 1; + pParamTokenArray = (mdParamDef*)_alloca(numParamTokens * sizeof(mdParamDef)); + CollateParamTokens(pModule->GetMDImport(), sigDesc.m_tkMethodDef, numArgs, pParamTokenArray); + + MethodDesc *pMD = sigDesc.m_pMD; + + NewHolder pStubState = new PInvoke_ILStubState(pModule, sigDesc.m_sig, &sigDesc.m_typeContext, dwStubFlags, sigInfo.GetCallConv(), iLCIDArg, pMD); + NewHolder pResolver = new ILStubResolver(); + pResolver->SetStubMethodDesc(pMD); + + return CreatePInvokeStubWorker(pStubState, pResolver, &sigDesc, sigInfo.GetCharSet(), sigInfo.GetLinkFlags(), sigInfo.GetCallConv(), dwStubFlags, pMD, pParamTokenArray, iLCIDArg); +} + static void CreateLayoutClassStub(MethodTable* pMT, PInvokeStubLinker* linker, MarshalOperation op) @@ -5698,38 +5736,23 @@ PCODE PInvoke::GetStubForILStub(PInvokeMethodDesc* pNMD, MethodDesc** ppStubMD, { STANDARD_VM_CONTRACT; - PCODE pStub = (PCODE)NULL; - - CONSISTENCY_CHECK(*ppStubMD == NULL); - - PInvokeStaticSigInfo sigInfo; - PInvoke::InitializeSigInfoAndPopulatePInvokeMethodDesc(pNMD, &sigInfo); - - *ppStubMD = GetILStubMethodDesc(pNMD, &sigInfo, dwStubFlags); - if (SF_IsForNumParamBytes(dwStubFlags)) - return (PCODE)NULL; - - if (*ppStubMD) { - pStub = JitILStub(*ppStubMD); + PInvokeStaticSigInfo sigInfo; + PInvoke::InitializeSigInfoAndPopulatePInvokeMethodDesc(pNMD, &sigInfo); + GetILStubMethodDesc(pNMD, &sigInfo, dwStubFlags); + return (PCODE)NULL; } - else - { - CONSISTENCY_CHECK(pNMD->IsVarArgs()); + + CONSISTENCY_CHECK(pNMD->IsVarArgs()); #ifdef FEATURE_PORTABLE_ENTRYPOINTS - COMPlusThrow(kInvalidProgramException, IDS_EE_VARARG_NOT_SUPPORTED); + COMPlusThrow(kInvalidProgramException, IDS_EE_VARARG_NOT_SUPPORTED); #else // !FEATURE_PORTABLE_ENTRYPOINTS - // - // varargs goes through vararg PInvoke stub - // - pStub = TheVarargPInvokeStub(pNMD->HasRetBuffArg()); - // Only vararg P/Invoke use shared stubs, they need a precode to push the hidden argument. - (void)pNMD->GetOrCreatePrecode(); + // Only vararg P/Invoke use shared stubs, they need a precode to push the hidden argument. + (void)pNMD->GetOrCreatePrecode(); #endif // FEATURE_PORTABLE_ENTRYPOINTS - } if (pNMD->IsEarlyBound()) { @@ -5740,14 +5763,10 @@ PCODE PInvoke::GetStubForILStub(PInvokeMethodDesc* pNMD, MethodDesc** ppStubMD, PInvokeLink(pNMD); } - // NOTE: For IL-stub-backed P/Invoke stubs (i.e., when *ppStubMD is non-null - // and the stub is obtained via JitILStub/ILStubCache), racing threads will - // resolve to the same DynamicMethodDesc via the ILStubCache (keyed by the - // target MethodDesc). The locking around the JIT operation prevents the - // code from being jitted more than once, and all threads get back the same - // PCODE. See CreateHashBlob() for the hashing logic. - - return pStub; + // + // varargs goes through vararg PInvoke stub + // + return TheVarargPInvokeStub(pNMD->HasRetBuffArg()); } PCODE JitILStub(MethodDesc* pStubMD) diff --git a/src/coreclr/vm/dllimport.h b/src/coreclr/vm/dllimport.h index b8b8340b9c3836..e9a771b0722be1 100644 --- a/src/coreclr/vm/dllimport.h +++ b/src/coreclr/vm/dllimport.h @@ -123,6 +123,10 @@ class PInvoke CorInfoCallConvExtension unmgdCallConv, DWORD dwStubFlags); // PInvokeStubFlags + static COR_ILMETHOD_DECODER* CreatePInvokeMethodIL( + PInvokeMethodDesc* pMD, + DynamicResolver** ppResolver); + #ifdef FEATURE_COMINTEROP static MethodDesc* CreateFieldAccessILStub( PCCOR_SIGNATURE szMetaSig, diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 1d9ccb7a191e04..e6640c99e209f4 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -373,8 +373,9 @@ PCODE MethodDesc::PrepareILBasedCode(PrepareCodeConfig* pConfig) } #endif // FEATURE_CODE_VERSIONING - if (pConfig->MayUsePrecompiledCode()) + if (pConfig->MayUsePrecompiledCode() && (!IsPInvoke() || MayUsePrecompiledILStub())) { + _ASSERTE(!IsPInvoke() || GetModule()->GetReadyToRunInfo()->HasNonShareablePInvokeStubs()); if (pCode == (PCODE)NULL) { pCode = GetPrecompiledCode(pConfig, shouldTier); @@ -1055,6 +1056,12 @@ bool MethodDesc::TryGenerateTransientILImplementation(DynamicResolver** resolver // When adding new methods implemented by Transient IL, consider if MethodDesc::IsDiagnosticsHidden() needs to be // updated as well. + if (IsPInvoke()) + { + *methodILDecoder = PInvoke::CreatePInvokeMethodIL(static_cast(this), resolver); + return true; + } + if (TryGenerateAsyncThunk(resolver, methodILDecoder)) { return true; @@ -2279,7 +2286,7 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT, CallerGCMode callerGCMo pStub = MakeInstantiatingStubWorker(this); } #endif // defined(FEATURE_SHARE_GENERIC_CODE) - else if (IsIL() || IsNoMetadata()) + else if (IsIL() || IsNoMetadata() || (IsPInvoke() && !IsVarArg())) { #ifndef FEATURE_PORTABLE_ENTRYPOINTS if (!IsNativeCodeStableAfterInit()) @@ -2288,22 +2295,23 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT, CallerGCMode callerGCMo } #endif // !FEATURE_PORTABLE_ENTRYPOINTS pCode = PrepareInitialCode(callerGCMode); - } // end else if (IsIL() || IsNoMetadata()) - else if (IsPInvoke()) - { - if (GetModule()->IsReadyToRun() && MayUsePrecompiledILStub()) + + if (IsPInvoke()) { - _ASSERTE(GetModule()->GetReadyToRunInfo()->HasNonShareablePInvokeStubs()); - // In crossgen2, we compile non-shareable IL stubs for pinvokes. If we can find code for such - // a stub, we'll use it directly instead and avoid emitting an IL stub. - PrepareCodeConfig config(NativeCodeVersion(this), TRUE, TRUE); - pCode = GetPrecompiledR2RCode(&config); - if (pCode != (PCODE)NULL) + PInvokeMethodDesc* pNMD = static_cast(this); + if (pNMD->IsEarlyBound()) { - LOG_USING_R2R_CODE(this); + pNMD->InitEarlyBoundPInvokeTarget(); + } + else + { + PInvokeLink(pNMD); } } - + } // end else if (IsIL() || IsNoMetadata() || (IsPInvoke() && !IsVarArg())) + else if (IsPInvoke()) + { + _ASSERTE(static_cast(this)->IsVarArgs()); if (pCode == (PCODE)NULL) { pCode = GetStubForInteropMethod(this); From cc1e281a291cd99eab4b2031d4bfe5e030986c0e Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Wed, 1 Apr 2026 15:00:41 -0700 Subject: [PATCH 05/18] Revert "Fix P/Invoke IL stub race in DoPrestub (#124579)" This is no longer necessary as non-vararg forward P/Invokes are now truly non-shared and don't go through the ILStubCache infrastructure at all. --- src/coreclr/inc/CrstTypes.def | 2 +- src/coreclr/vm/dllimport.cpp | 62 ++++++++++------------------------- 2 files changed, 19 insertions(+), 45 deletions(-) diff --git a/src/coreclr/inc/CrstTypes.def b/src/coreclr/inc/CrstTypes.def index 99dd2b4624e8f8..0dc22f646ff0e9 100644 --- a/src/coreclr/inc/CrstTypes.def +++ b/src/coreclr/inc/CrstTypes.def @@ -240,7 +240,7 @@ Crst IJWHash End Crst ILStubGen - AcquiredBefore DeadlockDetection UniqueStack UnresolvedClassLock FusionAppCtx AssemblyLoader + AcquiredBefore DeadlockDetection UniqueStack End Crst InstMethodHashTable diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 18b154a05ba800..7456c3b25c170f 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -191,10 +191,10 @@ static bool IsSharedStubScenario(DWORD dwStubFlags) return false; } - // Forward P/Invoke stubs (non-CALLI, non-vararg) are not shared across different - // target methods, but they still use the ILStubCache for de-duplication when - // multiple threads race to create a stub for the same target. The hash blob for - // these stubs is keyed by target MethodDesc rather than by signature. + if (SF_IsForwardPInvokeStub(dwStubFlags) && !SF_IsCALLIStub(dwStubFlags) && !SF_IsVarArgStub(dwStubFlags)) + { + return false; + } return true; } @@ -4237,37 +4237,10 @@ namespace PCCOR_SIGNATURE pvNativeType; }; - ILStubHashBlob* CreateHashBlob(PInvokeStubParameters* pParams, MethodDesc* pTargetMD) + ILStubHashBlob* CreateHashBlob(PInvokeStubParameters* pParams) { STANDARD_VM_CONTRACT; - // The hash blob for a forward P/Invoke stub is just the target MethodDesc pointer. - // In order to help avoid collisions, ensure various blob sizes are different. - // See asserts below. - constexpr size_t forwardPInvokeHashBlobSize = sizeof(ILStubHashBlobBase) + sizeof(MethodDesc*); - - DWORD dwStubFlags = pParams->m_dwStubFlags; - - // The target MethodDesc may be NULL for field marshalling. - // Forward P/Invoke stubs (non-CALLI, non-vararg) each target a specific method, - // so the hash blob is just the target MethodDesc pointer. This ensures racing - // threads for the same P/Invoke converge on the same DynamicMethodDesc in the - // ILStubCache, while different P/Invoke methods get distinct cache entries. - if (pTargetMD != NULL - && SF_IsForwardPInvokeStub(dwStubFlags) - && !SF_IsCALLIStub(dwStubFlags) - && !SF_IsVarArgStub(dwStubFlags)) - { - const size_t blobSize = forwardPInvokeHashBlobSize; - NewArrayHolder pBytes = new BYTE[blobSize]; - ZeroMemory(pBytes, blobSize); - ILStubHashBlob* pBlob = (ILStubHashBlob*)(BYTE*)pBytes; - pBlob->m_cbSizeOfBlob = blobSize; - memcpy(pBlob->m_rgbBlobData, &pTargetMD, sizeof(pTargetMD)); - pBytes.SuppressRelease(); - return pBlob; - } - PInvokeStubHashBlob* pBlob; IMDInternalImport* pInternalImport = pParams->m_pModule->GetMDImport(); @@ -4322,8 +4295,6 @@ namespace if (cbSizeOfBlob.IsOverflow()) COMPlusThrowHR(COR_E_OVERFLOW); - _ASSERTE(cbSizeOfBlob.Value() != forwardPInvokeHashBlobSize); - static_assert(nltMaxValue <= 0xFF); static_assert(nlfMaxValue <= 0xFF); static_assert(pmMaxValue <= 0xFFFF); @@ -4980,14 +4951,16 @@ namespace class ILStubCreatorHelper { public: - ILStubCreatorHelper(PInvokeStubParameters* pParams, MethodDesc *pTargetMD) - : m_pParams(pParams) - , m_pTargetMD(pTargetMD) - , m_pStubMD(NULL) - , m_bILStubCreator(false) + ILStubCreatorHelper(MethodDesc *pTargetMD, + PInvokeStubParameters* pParams + ) : + m_pTargetMD(pTargetMD), + m_pParams(pParams), + m_pStubMD(NULL), + m_bILStubCreator(false) { STANDARD_VM_CONTRACT; - m_pHashParams = CreateHashBlob(m_pParams, m_pTargetMD); + m_pHashParams = CreateHashBlob(m_pParams); } ~ILStubCreatorHelper() @@ -5049,10 +5022,11 @@ namespace } private: - PInvokeStubParameters* m_pParams; MethodDesc* m_pTargetMD; - MethodDesc* m_pStubMD; + PInvokeStubParameters* m_pParams; NewArrayHolder m_pHashParams; + AllocMemTracker* m_pAmTracker; + MethodDesc* m_pStubMD; AllocMemTracker m_amTracker; bool m_bILStubCreator; // Only the creator can remove the ILStub from the Cache }; //ILStubCreatorHelper @@ -5152,7 +5126,7 @@ namespace // remove it from cache if OOM occurs { - ILStubCreatorHelper ilStubCreatorHelper(¶ms, pTargetMD); + ILStubCreatorHelper ilStubCreatorHelper(pTargetMD, ¶ms); // take the domain level lock ListLockHolder pILStubLock(AppDomain::GetCurrentDomain()->GetILStubGenLock()); @@ -5750,7 +5724,7 @@ PCODE PInvoke::GetStubForILStub(PInvokeMethodDesc* pNMD, MethodDesc** ppStubMD, COMPlusThrow(kInvalidProgramException, IDS_EE_VARARG_NOT_SUPPORTED); #else // !FEATURE_PORTABLE_ENTRYPOINTS - // Only vararg P/Invoke use shared stubs, they need a precode to push the hidden argument. + // Vararg P/Invoke use shared stubs, they need a precode to push the hidden argument. (void)pNMD->GetOrCreatePrecode(); #endif // FEATURE_PORTABLE_ENTRYPOINTS From 4555bb322ba6d041755111b6c221a28e2a53b943 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 2 Apr 2026 17:23:03 -0700 Subject: [PATCH 06/18] Various fixes to get the interop test tree passing locally. --- src/coreclr/vm/dllimport.cpp | 250 ++++++++++++++++++++++---------- src/coreclr/vm/dllimport.h | 11 +- src/coreclr/vm/jitinterface.cpp | 2 +- src/coreclr/vm/method.cpp | 16 +- src/coreclr/vm/prestub.cpp | 48 +----- 5 files changed, 197 insertions(+), 130 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 7456c3b25c170f..1cdd88a540bf9a 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -808,10 +808,6 @@ class ILStubState ConvertMethodDescSigToModuleIndependentSig(pStubMD); } - pResolver->SetStubTargetMethodSig( - GetStubTargetMethodSig(), - GetStubTargetMethodSigLength()); - if (hasTryCatchExceptionHandler) { EmitExceptionHandler(&nativeReturnType, &managedReturnType); @@ -819,6 +815,10 @@ class ILStubState COR_ILMETHOD_DECODER* pILHeader = pResolver->FinalizeILStub(&m_slIL, jitFlags); + pResolver->SetStubTargetMethodSig( + GetStubTargetMethodSig(), + GetStubTargetMethodSigLength()); + // Extract resolved EH clause info for logging and ETW ILStubEHClause cleanupTryFinally{}; ILStubEHClause tryCatchClause{}; @@ -1181,7 +1181,7 @@ class ILStubState DWORD m_dwStubFlags; }; -class PInvoke_ILStubState : public ILStubState +class PInvoke_ILStubState final : public ILStubState { public: @@ -1259,6 +1259,58 @@ class PInvoke_ILStubState : public ILStubState } }; +// A stub state that doeson't actually generate IL but provides enough implementation +// that we can use CreatePInvokeStubWorker to compute the argument stack size for a PInvoke call. +class PInvoke_StackArgumentSize_ILStubState final : public ILStubState +{ +public: + PInvoke_StackArgumentSize_ILStubState(Module* pStubModule, const Signature &signature, SigTypeContext *pTypeContext, DWORD dwStubFlags, + CorInfoCallConvExtension unmgdCallConv, int iLCIDParamIdx, MethodDesc* pTargetMD) + : ILStubState( + pStubModule, + signature, + pTypeContext, + dwStubFlags, + iLCIDParamIdx, + pTargetMD) + { + STANDARD_VM_CONTRACT; + m_slIL.SetCallingConvention(unmgdCallConv, SF_IsVarArgStub(dwStubFlags)); + } + + void BeginEmit(DWORD dwStubFlags) // PInvoke with argument stack size computation IL + { + STANDARD_VM_CONTRACT; + } + + void EmitInvokeTarget(MethodDesc* pTargetMD, MethodDesc* pStubMD) + { + STANDARD_VM_CONTRACT; + } + + void MarshalReturn(MarshalInfo* pInfo, int argOffset) + { + STANDARD_VM_CONTRACT; + } + + void MarshalArgument(MarshalInfo* pInfo, int argOffset) + { + STANDARD_VM_CONTRACT; + } + + void MarshalLCID(int argIdx) + { + LIMITED_METHOD_CONTRACT; + } + + COR_ILMETHOD_DECODER* FinishEmit(MethodDesc* pStubMD, ILStubResolver* pResolver) + { + STANDARD_VM_CONTRACT; + + return nullptr; + } +}; + #ifdef FEATURE_COMINTEROP class CLRToCOM_ILStubState : public ILStubState { @@ -3922,8 +3974,22 @@ static COR_ILMETHOD_DECODER* CreatePInvokeStubWorker( pDMD->SetNativeStackArgSize(static_cast(nativeStackSize)); if (fStubNeedsCOM) pDMD->SetFlags(DynamicMethodDesc::FlagRequiresCOM); -#endif +#endif // FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE } +#ifdef TARGET_X86 + else if (pMD->IsPInvoke()) + { + PInvokeMethodDesc *pNMD = (PInvokeMethodDesc *)pMD; + pNMD->SetStackArgumentSize(static_cast(nativeStackSize), unmgdCallConv); + } +#ifdef FEATURE_COMINTEROP + else if (pMD->IsCLRToCOMCall()) + { + CLRToCOMCallMethodDesc *pCMD = (CLRToCOMCallMethodDesc *)pMD; + pCMD->SetStackArgumentSize(static_cast(nativeStackSize)); + } +#endif // FEATURE_COMINTEROP +#endif // TARGET_X86 // FinishEmit needs to know the native stack arg size so we call it after the number // has been set in the stub MD (code:DynamicMethodDesc.SetNativeStackArgSize) @@ -4087,43 +4153,6 @@ bool StructMarshalStubs::TryGenerateStructMarshallingMethod(MethodDesc* pMD, Dyn return true; } -COR_ILMETHOD_DECODER* PInvoke::CreatePInvokeMethodIL(PInvokeMethodDesc* pMD, DynamicResolver** ppResolver) -{ - STANDARD_VM_CONTRACT; - - _ASSERTE(pMD != nullptr); - - PInvokeStaticSigInfo sigInfo; - PInvoke::InitializeSigInfoAndPopulatePInvokeMethodDesc(pMD, &sigInfo); - - StubSigDesc sigDesc(pMD, sigInfo.GetSignature(), sigInfo.GetModule()); - - DWORD dwStubFlags = sigInfo.GetStubFlags(); - int iLCIDArg = 0; - int numArgs = 0; - int numParamTokens = 0; - mdParamDef* pParamTokenArray = NULL; - - CreatePInvokeStubAccessMetadata(&sigDesc, - sigInfo.GetCallConv(), - &dwStubFlags, - &iLCIDArg, - &numArgs); - - Module *pModule = sigDesc.m_pModule; - numParamTokens = numArgs + 1; - pParamTokenArray = (mdParamDef*)_alloca(numParamTokens * sizeof(mdParamDef)); - CollateParamTokens(pModule->GetMDImport(), sigDesc.m_tkMethodDef, numArgs, pParamTokenArray); - - MethodDesc *pMD = sigDesc.m_pMD; - - NewHolder pStubState = new PInvoke_ILStubState(pModule, sigDesc.m_sig, &sigDesc.m_typeContext, dwStubFlags, sigInfo.GetCallConv(), iLCIDArg, pMD); - NewHolder pResolver = new ILStubResolver(); - pResolver->SetStubMethodDesc(pMD); - - return CreatePInvokeStubWorker(pStubState, pResolver, &sigDesc, sigInfo.GetCharSet(), sigInfo.GetLinkFlags(), sigInfo.GetCallConv(), dwStubFlags, pMD, pParamTokenArray, iLCIDArg); -} - static void CreateLayoutClassStub(MethodTable* pMT, PInvokeStubLinker* linker, MarshalOperation op) @@ -4661,6 +4690,83 @@ void PInvoke::InitializeSigInfoAndPopulatePInvokeMethodDesc(_Inout_ PInvokeMetho PopulatePInvokeMethodDescImpl(pNMD, *pSigInfo, szLibName, szEntryPointName); } + +COR_ILMETHOD_DECODER* PInvoke::CreatePInvokeMethodIL(PInvokeMethodDesc* pMD, DynamicResolver** ppResolver) +{ + STANDARD_VM_CONTRACT; + + _ASSERTE(pMD != nullptr); + + PInvokeStaticSigInfo sigInfo; + PInvoke::InitializeSigInfoAndPopulatePInvokeMethodDesc(pMD, &sigInfo); + + StubSigDesc sigDesc(pMD, sigInfo.GetSignature(), sigInfo.GetModule()); + + DWORD dwStubFlags = sigInfo.GetStubFlags(); + int iLCIDArg = 0; + int numArgs = 0; + int numParamTokens = 0; + mdParamDef* pParamTokenArray = NULL; + + CreatePInvokeStubAccessMetadata(&sigDesc, + sigInfo.GetCallConv(), + &dwStubFlags, + &iLCIDArg, + &numArgs); + + Module *pModule = sigDesc.m_pModule; + numParamTokens = numArgs + 1; + pParamTokenArray = (mdParamDef*)_alloca(numParamTokens * sizeof(mdParamDef)); + CollateParamTokens(pModule->GetMDImport(), sigDesc.m_tkMethodDef, numArgs, pParamTokenArray); + + PInvoke_ILStubState stubState(pModule, sigDesc.m_sig, &sigDesc.m_typeContext, dwStubFlags, sigInfo.GetCallConv(), iLCIDArg, pMD); + NewHolder pResolver = new ILStubResolver(); + pResolver->SetStubMethodDesc(pMD); + + COR_ILMETHOD_DECODER* pIL = CreatePInvokeStubWorker(&stubState, pResolver, &sigDesc, sigInfo.GetCharSet(), sigInfo.GetLinkFlags(), sigInfo.GetCallConv(), dwStubFlags, pMD, pParamTokenArray, iLCIDArg); + *ppResolver = pResolver.Extract(); + return pIL; +} + +#ifdef TARGET_X86 +void PInvoke::CalculateStackArgumentSize(PInvokeMethodDesc* pMD) +{ + STANDARD_VM_CONTRACT; + + _ASSERTE(pMD != nullptr); + + PInvokeStaticSigInfo sigInfo; + PInvoke::InitializeSigInfoAndPopulatePInvokeMethodDesc(pMD, &sigInfo); + + _ASSERTE(sigInfo.GetCallConv() == CorInfoCallConvExtension::Stdcall); + + StubSigDesc sigDesc(pMD, sigInfo.GetSignature(), sigInfo.GetModule()); + + DWORD dwStubFlags = sigInfo.GetStubFlags(); + int iLCIDArg = 0; + int numArgs = 0; + int numParamTokens = 0; + mdParamDef* pParamTokenArray = NULL; + + CreatePInvokeStubAccessMetadata(&sigDesc, + sigInfo.GetCallConv(), + &dwStubFlags, + &iLCIDArg, + &numArgs); + + Module *pModule = sigDesc.m_pModule; + numParamTokens = numArgs + 1; + pParamTokenArray = (mdParamDef*)_alloca(numParamTokens * sizeof(mdParamDef)); + CollateParamTokens(pModule->GetMDImport(), sigDesc.m_tkMethodDef, numArgs, pParamTokenArray); + + PInvoke_StackArgumentSize_ILStubState stubState(pModule, sigDesc.m_sig, &sigDesc.m_typeContext, dwStubFlags, sigInfo.GetCallConv(), iLCIDArg, pMD); + NewHolder pResolver = new ILStubResolver(); + pResolver->SetStubMethodDesc(pMD); + + CreatePInvokeStubWorker(&stubState, pResolver, &sigDesc, sigInfo.GetCharSet(), sigInfo.GetLinkFlags(), sigInfo.GetCallConv(), dwStubFlags, pMD, pParamTokenArray, iLCIDArg); +} +#endif // TARGET_X86 + #ifdef FEATURE_COMINTEROP // Find the MethodDesc of the predefined IL stub method by either // 1) looking at redirected adapter interfaces, OR @@ -5579,28 +5685,6 @@ MethodDesc* PInvoke::CreateCLRToNativeILStub(PInvokeStaticSigInfo* pSigInfo, namespace { - MethodDesc* GetILStubMethodDesc(PInvokeMethodDesc* pNMD, PInvokeStaticSigInfo* pSigInfo, DWORD dwStubFlags) - { - CONTRACTL - { - STANDARD_VM_CHECK; - PRECONDITION(pNMD != NULL); - } - CONTRACTL_END; - - MethodDesc* pStubMD = NULL; - - if (!pNMD->IsVarArgs() || SF_IsForNumParamBytes(dwStubFlags)) - { - pStubMD = PInvoke::CreateCLRToNativeILStub( - pSigInfo, - dwStubFlags & ~PINVOKESTUB_FL_FOR_NUMPARAMBYTES, - pNMD); - } - - return pStubMD; - } - LPVOID PInvokeGetEntryPoint(PInvokeMethodDesc *pMD, NATIVE_LIBRARY_HANDLE hMod) { // GetProcAddress cannot be called while preemptive GC is disabled. @@ -5710,14 +5794,6 @@ PCODE PInvoke::GetStubForILStub(PInvokeMethodDesc* pNMD, MethodDesc** ppStubMD, { STANDARD_VM_CONTRACT; - if (SF_IsForNumParamBytes(dwStubFlags)) - { - PInvokeStaticSigInfo sigInfo; - PInvoke::InitializeSigInfoAndPopulatePInvokeMethodDesc(pNMD, &sigInfo); - GetILStubMethodDesc(pNMD, &sigInfo, dwStubFlags); - return (PCODE)NULL; - } - CONSISTENCY_CHECK(pNMD->IsVarArgs()); #ifdef FEATURE_PORTABLE_ENTRYPOINTS @@ -5728,6 +5804,28 @@ PCODE PInvoke::GetStubForILStub(PInvokeMethodDesc* pNMD, MethodDesc** ppStubMD, (void)pNMD->GetOrCreatePrecode(); #endif // FEATURE_PORTABLE_ENTRYPOINTS + ResolvePInvokeTarget(pNMD); + + // + // varargs goes through vararg PInvoke stub + // + return TheVarargPInvokeStub(pNMD->HasRetBuffArg()); +} + +void PInvoke::ResolvePInvokeTarget(PInvokeMethodDesc* pNMD) +{ + CONTRACTL + { + THROWS; + GC_TRIGGERS; + MODE_PREEMPTIVE; + + PRECONDITION(CheckPointer(pNMD)); + } + CONTRACTL_END; + + PopulatePInvokeMethodDesc(pNMD); + if (pNMD->IsEarlyBound()) { pNMD->InitEarlyBoundPInvokeTarget(); @@ -5736,11 +5834,6 @@ PCODE PInvoke::GetStubForILStub(PInvokeMethodDesc* pNMD, MethodDesc** ppStubMD, { PInvokeLink(pNMD); } - - // - // varargs goes through vararg PInvoke stub - // - return TheVarargPInvokeStub(pNMD->HasRetBuffArg()); } PCODE JitILStub(MethodDesc* pStubMD) @@ -5817,6 +5910,7 @@ PCODE GetStubForInteropMethod(MethodDesc* pMD, DWORD dwStubFlags) if (pMD->IsPInvoke()) { PInvokeMethodDesc* pNMD = (PInvokeMethodDesc*)pMD; + CONSISTENCY_CHECK(pNMD->IsVarArgs()); // Non-varargs shouldn't be using stubs. pStub = PInvoke::GetStubForILStub(pNMD, &pStubMD, dwStubFlags); } #ifdef FEATURE_COMINTEROP diff --git a/src/coreclr/vm/dllimport.h b/src/coreclr/vm/dllimport.h index e9a771b0722be1..121e4cb8597a58 100644 --- a/src/coreclr/vm/dllimport.h +++ b/src/coreclr/vm/dllimport.h @@ -113,6 +113,10 @@ class PInvoke _In_opt_ SigTypeContext* pTypeContext = NULL, _In_ bool unmanagedCallersOnlyRequiresMarshalling = true); +#ifdef TARGET_X86 + static void CalculateStackArgumentSize(PInvokeMethodDesc* pMD); +#endif + static void PopulatePInvokeMethodDesc(_Inout_ PInvokeMethodDesc* pNMD); static void InitializeSigInfoAndPopulatePInvokeMethodDesc(_Inout_ PInvokeMethodDesc* pNMD, _Inout_ PInvokeStaticSigInfo* pSigInfo); @@ -146,6 +150,8 @@ class PInvoke static PCODE GetStubForILStub(PInvokeMethodDesc* pNMD, MethodDesc** ppStubMD, DWORD dwStubFlags); static PCODE GetStubForILStub(MethodDesc* pMD, MethodDesc** ppStubMD, DWORD dwStubFlags); + static void ResolvePInvokeTarget(PInvokeMethodDesc* pNMD); + private: PInvoke() {LIMITED_METHOD_CONTRACT;}; // prevent "new"'s on this class }; @@ -183,9 +189,7 @@ enum PInvokeStubFlags // unused = 0x00200000, // unused = 0x00400000, // unused = 0x00800000, - - // internal flags -- these won't ever show up in an PInvokeStubHashBlob - PINVOKESTUB_FL_FOR_NUMPARAMBYTES = 0x10000000, // do just enough to return the right value from Marshal.NumParamBytes + // unused = 0x10000000, #ifdef FEATURE_COMINTEROP PINVOKESTUB_FL_COMLATEBOUND = 0x20000000, // we use a generic stub for late bound calls @@ -224,7 +228,6 @@ inline bool SF_IsHRESULTSwapping (DWORD dwStubFlags) { LIMITED_METHOD_CONT inline bool SF_IsReverseStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags < PINVOKESTUB_FL_INVALID && 0 != (dwStubFlags & PINVOKESTUB_FL_REVERSE_INTEROP)); } inline bool SF_IsDebuggableStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags < PINVOKESTUB_FL_INVALID && 0 != (dwStubFlags & PINVOKESTUB_FL_GENERATEDEBUGGABLEIL)); } inline bool SF_IsCALLIStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags < PINVOKESTUB_FL_INVALID && 0 != (dwStubFlags & PINVOKESTUB_FL_UNMANAGED_CALLI)); } -inline bool SF_IsForNumParamBytes (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags < PINVOKESTUB_FL_INVALID && 0 != (dwStubFlags & PINVOKESTUB_FL_FOR_NUMPARAMBYTES)); } inline bool SF_IsStructMarshalStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags < PINVOKESTUB_FL_INVALID && 0 != (dwStubFlags & PINVOKESTUB_FL_STRUCT_MARSHAL)); } inline bool SF_IsCheckPendingException (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags < PINVOKESTUB_FL_INVALID && 0 != (dwStubFlags & PINVOKESTUB_FL_CHECK_PENDING_EXCEPTION)); } inline bool SF_IsSharedStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags < PINVOKESTUB_FL_INVALID && 0 != (dwStubFlags & PINVOKESTUB_FL_SHARED_STUB)); } diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 48e6c2ea8c14c0..a6c97eadd71462 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7732,7 +7732,7 @@ CEEInfo::getMethodInfo( getMethodInfoWorker(ftn, &header, methInfo, context); result = true; } - else if (ftn->IsIL() || ftn->IsDynamicMethod()) + else if (ftn->IsIL() || ftn->IsPInvoke() || ftn->IsDynamicMethod()) { // IL methods with no IL header indicate there is no implementation defined in metadata. getMethodInfoWorker(ftn, NULL, methInfo, context); diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 9295fb3d38e534..95061d266e7f52 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -2656,8 +2656,8 @@ BOOL MethodDesc::MayHaveNativeCode() break; case mcFCall: // FCalls do not have real native code. return FALSE; - case mcPInvoke: // PInvoke never have native code (note that the PInvoke method - return FALSE; // does not appear as having a native code even for stubs as IL) + case mcPInvoke: // Non vararg P/Invokes are treated as IL-backed. Vararg P/Invokes go through a stub. + return !IsVarArg(); case mcEEImpl: // Runtime provided implementation. No native code. return FALSE; case mcArray: // Runtime provided implementation. No native code. @@ -3082,7 +3082,10 @@ bool MethodDesc::DetermineAndSetIsEligibleForTieredCompilation() !IsJitOptimizationLevelRequested() && // Tiering the async thunk methods doesn't make sense - !IsAsyncThunkMethod() + !IsAsyncThunkMethod() && + + // Tiering P/Invoke methods doesn't make sense + !IsPInvoke() ) { InterlockedUpdateFlags3(enum_flag3_IsEligibleForTieredCompilation, TRUE); @@ -3657,7 +3660,12 @@ void PInvokeMethodDesc::EnsureStackArgumentSize() if (MarshalingRequired()) { // Generating interop stub sets the stack size as side-effect in all cases - GetStubForInteropMethod(this, PINVOKESTUB_FL_FOR_NUMPARAMBYTES); + PInvokeStaticSigInfo sigInfo; + PInvoke::InitializeSigInfoAndPopulatePInvokeMethodDesc(pNMD, &sigInfo); + PInvoke::CreateCLRToNativeILStub( + &sigInfo, + dwStubFlags, + pNMD); } } } diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index e6640c99e209f4..40b56c33d18704 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -311,7 +311,7 @@ PCODE MethodDesc::PrepareCode(PrepareCodeConfig* pConfig) // If other kinds of code need multi-versioning we could add more cases here, // but for now generation of all other code/stubs occurs in other code paths - _ASSERTE(IsIL() || IsNoMetadata()); + _ASSERTE(IsIL() || IsNoMetadata() || IsPInvoke()); PCODE pCode = PrepareILBasedCode(pConfig); #if defined(FEATURE_GDBJIT) && defined(TARGET_UNIX) @@ -375,7 +375,7 @@ PCODE MethodDesc::PrepareILBasedCode(PrepareCodeConfig* pConfig) if (pConfig->MayUsePrecompiledCode() && (!IsPInvoke() || MayUsePrecompiledILStub())) { - _ASSERTE(!IsPInvoke() || GetModule()->GetReadyToRunInfo()->HasNonShareablePInvokeStubs()); + _ASSERTE(!IsPInvoke() || (!GetModule()->IsReadyToRun() || GetModule()->GetReadyToRunInfo()->HasNonShareablePInvokeStubs())); if (pCode == (PCODE)NULL) { pCode = GetPrecompiledCode(pConfig, shouldTier); @@ -725,7 +725,7 @@ namespace return pResolver->GetILHeader(); } - _ASSERTE(pMD->IsNoMetadata()); + _ASSERTE(pMD->IsNoMetadata() || pMD->IsPInvoke()); return NULL; } } @@ -2298,51 +2298,13 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT, CallerGCMode callerGCMo if (IsPInvoke()) { - PInvokeMethodDesc* pNMD = static_cast(this); - if (pNMD->IsEarlyBound()) - { - pNMD->InitEarlyBoundPInvokeTarget(); - } - else - { - PInvokeLink(pNMD); - } + PInvoke::ResolvePInvokeTarget(static_cast(this)); } } // end else if (IsIL() || IsNoMetadata() || (IsPInvoke() && !IsVarArg())) else if (IsPInvoke()) { _ASSERTE(static_cast(this)->IsVarArgs()); - if (pCode == (PCODE)NULL) - { - pCode = GetStubForInteropMethod(this); - -#ifdef FEATURE_INTERPRETER - // Store the IL stub interpreter data on the P/Invoke MethodDesc so the - // interpreter can run the IL stub as a child frame with a single native - // transition. On WASM this is done for all P/Invokes; on ARM64 Apple it - // is needed specifically for Swift to avoid a stale SwiftError (x21). -#ifdef FEATURE_PORTABLE_ENTRYPOINTS - void* ilStubInterpData = PortableEntryPoint::GetInterpreterData(pCode); - _ASSERTE(ilStubInterpData != NULL); - SetInterpreterCode((InterpByteCodeStart*)ilStubInterpData); -#else // !FEATURE_PORTABLE_ENTRYPOINTS -#if defined(TARGET_APPLE) && defined(TARGET_ARM64) - { - CorInfoCallConvExtension callConv; - PInvoke::GetCallingConvention_IgnoreErrors(this, &callConv, nullptr); - if (callConv == CorInfoCallConvExtension::Swift) - { - TADDR ilStubInterpCode = GetInterpreterCodeFromInterpreterPrecodeIfPresent(pCode); - if (ilStubInterpCode != (TADDR)pCode) - { - SetInterpreterCode(dac_cast(ilStubInterpCode)); - } - } - } -#endif // TARGET_APPLE && TARGET_ARM64 -#endif // FEATURE_PORTABLE_ENTRYPOINTS -#endif // FEATURE_INTERPRETER - } + pCode = GetStubForInteropMethod(this); } else if (IsFCall()) { From f9c441f3409da355f040f02d1d467322233a099f Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 3 Apr 2026 09:41:21 -0700 Subject: [PATCH 07/18] Little cleanups --- src/coreclr/vm/dllimport.cpp | 16 +++++++++------- src/coreclr/vm/method.cpp | 8 +------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 1cdd88a540bf9a..d728862714be24 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -800,12 +800,15 @@ class ILStubState // is the target signature. We need to swap them. SwapStubSignatures(pStubMD); } - else if (pStubMD->IsDynamicMethod()) + else { // If we're not in a Reverse stub, the signatures are correct, // but we need to convert the signature into a module-independent form // if our signature is not backed by metadata. - ConvertMethodDescSigToModuleIndependentSig(pStubMD); + if (pStubMD->IsDynamicMethod()) + { + ConvertMethodDescSigToModuleIndependentSig(pStubMD); + } } if (hasTryCatchExceptionHandler) @@ -2325,7 +2328,7 @@ void PInvokeStubLinker::End(DWORD dwStubFlags) } else { - // No cleanup needed - cancel the try block started in Begin(). + // No cleanup needed - cancel the inner try block started in Begin(). _ASSERTE(m_buildingEHClauses.GetCount() > 0); m_buildingEHClauses.SetCount(m_buildingEHClauses.GetCount() - 1); } @@ -4760,10 +4763,10 @@ void PInvoke::CalculateStackArgumentSize(PInvokeMethodDesc* pMD) CollateParamTokens(pModule->GetMDImport(), sigDesc.m_tkMethodDef, numArgs, pParamTokenArray); PInvoke_StackArgumentSize_ILStubState stubState(pModule, sigDesc.m_sig, &sigDesc.m_typeContext, dwStubFlags, sigInfo.GetCallConv(), iLCIDArg, pMD); - NewHolder pResolver = new ILStubResolver(); - pResolver->SetStubMethodDesc(pMD); + ILStubResolver resolver{}; + resolver.SetStubMethodDesc(pMD); - CreatePInvokeStubWorker(&stubState, pResolver, &sigDesc, sigInfo.GetCharSet(), sigInfo.GetLinkFlags(), sigInfo.GetCallConv(), dwStubFlags, pMD, pParamTokenArray, iLCIDArg); + CreatePInvokeStubWorker(&stubState, &resolver, &sigDesc, sigInfo.GetCharSet(), sigInfo.GetLinkFlags(), sigInfo.GetCallConv(), dwStubFlags, pMD, pParamTokenArray, iLCIDArg); } #endif // TARGET_X86 @@ -5131,7 +5134,6 @@ namespace MethodDesc* m_pTargetMD; PInvokeStubParameters* m_pParams; NewArrayHolder m_pHashParams; - AllocMemTracker* m_pAmTracker; MethodDesc* m_pStubMD; AllocMemTracker m_amTracker; bool m_bILStubCreator; // Only the creator can remove the ILStub from the Cache diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 95061d266e7f52..b4814120c1aa87 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -3659,13 +3659,7 @@ void PInvokeMethodDesc::EnsureStackArgumentSize() // Marshalling required check sets the stack size as side-effect when marshalling is not required. if (MarshalingRequired()) { - // Generating interop stub sets the stack size as side-effect in all cases - PInvokeStaticSigInfo sigInfo; - PInvoke::InitializeSigInfoAndPopulatePInvokeMethodDesc(pNMD, &sigInfo); - PInvoke::CreateCLRToNativeILStub( - &sigInfo, - dwStubFlags, - pNMD); + PInvoke::CalculateStackArgumentSize(this); } } } From 7681f9fd604d17649bf176d366c83d4b0002bfec Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 3 Apr 2026 13:54:43 -0700 Subject: [PATCH 08/18] PR feedback --- src/coreclr/vm/dllimport.cpp | 17 +++++++++-------- src/coreclr/vm/method.cpp | 8 ++++---- src/coreclr/vm/method.hpp | 1 - src/coreclr/vm/prestub.cpp | 32 ++++++++++++++------------------ src/coreclr/vm/stubgen.cpp | 24 ++++++++++++------------ src/coreclr/vm/stubgen.h | 2 +- 6 files changed, 40 insertions(+), 44 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index d728862714be24..e0d82926b2abf1 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -827,8 +827,7 @@ class ILStubState ILStubEHClause tryCatchClause{}; for (size_t i = 0; i < pILHeader->EHCount(); i++) { - ILStubEHClause clause{}; - m_slIL.GetEHClause(i, &clause); + ILStubEHClause clause = m_slIL.GetEHClause(i); if (clause.kind == ILStubEHClause::kFinally) cleanupTryFinally = clause; else if (clause.kind == ILStubEHClause::kTypedCatch) @@ -1262,7 +1261,8 @@ class PInvoke_ILStubState final : public ILStubState } }; -// A stub state that doeson't actually generate IL but provides enough implementation +#ifdef TARGET_X86 +// A stub state that doesn't actually generate IL but provides enough implementation // that we can use CreatePInvokeStubWorker to compute the argument stack size for a PInvoke call. class PInvoke_StackArgumentSize_ILStubState final : public ILStubState { @@ -1283,22 +1283,22 @@ class PInvoke_StackArgumentSize_ILStubState final : public ILStubState void BeginEmit(DWORD dwStubFlags) // PInvoke with argument stack size computation IL { - STANDARD_VM_CONTRACT; + LIMITED_METHOD_CONTRACT; } void EmitInvokeTarget(MethodDesc* pTargetMD, MethodDesc* pStubMD) { - STANDARD_VM_CONTRACT; + LIMITED_METHOD_CONTRACT; } void MarshalReturn(MarshalInfo* pInfo, int argOffset) { - STANDARD_VM_CONTRACT; + LIMITED_METHOD_CONTRACT; } void MarshalArgument(MarshalInfo* pInfo, int argOffset) { - STANDARD_VM_CONTRACT; + LIMITED_METHOD_CONTRACT; } void MarshalLCID(int argIdx) @@ -1308,11 +1308,12 @@ class PInvoke_StackArgumentSize_ILStubState final : public ILStubState COR_ILMETHOD_DECODER* FinishEmit(MethodDesc* pStubMD, ILStubResolver* pResolver) { - STANDARD_VM_CONTRACT; + LIMITED_METHOD_CONTRACT; return nullptr; } }; +#endif // TARGET_X86 #ifdef FEATURE_COMINTEROP class CLRToCOM_ILStubState : public ILStubState diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index b4814120c1aa87..91def7669cdd43 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -2656,8 +2656,8 @@ BOOL MethodDesc::MayHaveNativeCode() break; case mcFCall: // FCalls do not have real native code. return FALSE; - case mcPInvoke: // Non vararg P/Invokes are treated as IL-backed. Vararg P/Invokes go through a stub. - return !IsVarArg(); + case mcPInvoke: // P/Invokes are generally backed by IL. + return TRUE; case mcEEImpl: // Runtime provided implementation. No native code. return FALSE; case mcArray: // Runtime provided implementation. No native code. @@ -3081,10 +3081,10 @@ bool MethodDesc::DetermineAndSetIsEligibleForTieredCompilation() // Functions with NoOptimization or AggressiveOptimization don't participate in tiering !IsJitOptimizationLevelRequested() && - // Tiering the async thunk methods doesn't make sense + // Tiering the async thunk methods is not supported currently !IsAsyncThunkMethod() && - // Tiering P/Invoke methods doesn't make sense + // Tiering P/Invoke methods is not supported currently !IsPInvoke() ) { diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index e196bf575bb9ce..3249659337ced4 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -2213,7 +2213,6 @@ class MethodDesc PCODE PrepareCode(PrepareCodeConfig* pConfig); private: - PCODE PrepareILBasedCode(PrepareCodeConfig* pConfig); PCODE GetPrecompiledCode(PrepareCodeConfig* pConfig, bool shouldTier); PCODE GetPrecompiledR2RCode(PrepareCodeConfig* pConfig); PCODE GetMulticoreJitCode(PrepareCodeConfig* pConfig, bool* pWasTier0); diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 40b56c33d18704..4c415abc59f304 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -305,23 +305,7 @@ PCODE MethodDesc::PrepareInitialCode(CallerGCMode callerGCMode) return PrepareCode(&config); } -PCODE MethodDesc::PrepareCode(PrepareCodeConfig* pConfig) -{ - STANDARD_VM_CONTRACT; - - // If other kinds of code need multi-versioning we could add more cases here, - // but for now generation of all other code/stubs occurs in other code paths - _ASSERTE(IsIL() || IsNoMetadata() || IsPInvoke()); - PCODE pCode = PrepareILBasedCode(pConfig); - -#if defined(FEATURE_GDBJIT) && defined(TARGET_UNIX) - NotifyGdb::MethodPrepared(this); -#endif - - return pCode; -} - -bool MayUsePrecompiledILStub() +static bool MayUsePrecompiledILStub() { if (g_pConfig->InteropValidatePinnedObjects()) return false; @@ -335,9 +319,13 @@ bool MayUsePrecompiledILStub() return true; } -PCODE MethodDesc::PrepareILBasedCode(PrepareCodeConfig* pConfig) +PCODE MethodDesc::PrepareCode(PrepareCodeConfig* pConfig) { STANDARD_VM_CONTRACT; + + // Only IL-backed methods should come through here. + // Other kinds of methods (e.g. FCalls, CLR-to-COM methods, should go down a different path) + _ASSERTE(IsIL() || IsNoMetadata() || IsPInvoke()); PCODE pCode = (PCODE)NULL; bool shouldTier = false; @@ -402,6 +390,10 @@ PCODE MethodDesc::PrepareILBasedCode(PrepareCodeConfig* pConfig) else { DACNotifyCompilationFinished(this, pCode); + +#if defined(FEATURE_GDBJIT) && defined(TARGET_UNIX) + NotifyGdb::MethodPrepared(this); +#endif } return pCode; @@ -884,6 +876,10 @@ PCODE MethodDesc::JitCompileCodeLockedEventWrapper(PrepareCodeConfig* pConfig, J // The notification will only occur if someone has registered for this method. DACNotifyCompilationFinished(this, pCode); +#if defined(FEATURE_GDBJIT) && defined(TARGET_UNIX) + NotifyGdb::MethodPrepared(this); +#endif + return pCode; } diff --git a/src/coreclr/vm/stubgen.cpp b/src/coreclr/vm/stubgen.cpp index 9ac36366c1b9d8..71de97742f5009 100644 --- a/src/coreclr/vm/stubgen.cpp +++ b/src/coreclr/vm/stubgen.cpp @@ -854,7 +854,6 @@ size_t ILStubLinker::GetNumEHClauses() void ILStubLinker::WriteEHClauses(COR_ILMETHOD_SECT_EH* pSect) { - unsigned int clauseIndex = 0; const SArray& clauses = m_finishedEHClauses; for (COUNT_T i = 0; i < clauses.GetCount(); i++) { @@ -873,34 +872,35 @@ void ILStubLinker::WriteEHClauses(COR_ILMETHOD_SECT_EH* pSect) size_t handlerBegin = builder.handlerBeginLabel->GetCodeOffset(); size_t handlerEnd = builder.handlerEndLabel->GetCodeOffset(); - IMAGE_COR_ILMETHOD_SECT_EH_CLAUSE_FAT& clause = pSect->Fat.Clauses[clauseIndex]; + IMAGE_COR_ILMETHOD_SECT_EH_CLAUSE_FAT& clause = pSect->Fat.Clauses[i]; clause.Flags = flags; clause.TryOffset = static_cast(tryBegin); clause.TryLength = static_cast(tryEnd - tryBegin); clause.HandlerOffset = static_cast(handlerBegin); clause.HandlerLength = static_cast(handlerEnd - handlerBegin); clause.ClassToken = builder.typeToken; - - clauseIndex++; } pSect->Fat.Kind = CorILMethod_Sect_EHTable | CorILMethod_Sect_FatFormat; - pSect->Fat.DataSize = COR_ILMETHOD_SECT_EH_FAT::Size(clauseIndex); + pSect->Fat.DataSize = COR_ILMETHOD_SECT_EH_FAT::Size(clauses.GetCount()); } -void ILStubLinker::GetEHClause(size_t index, ILStubEHClause* pClause) +ILStubEHClause ILStubLinker::GetEHClause(size_t index) { _ASSERTE(index < m_finishedEHClauses.GetCount()); const ILStubEHClauseBuilder& builder = m_finishedEHClauses[static_cast(index)]; - pClause->kind = builder.kind; - pClause->dwTryBeginOffset = static_cast(builder.tryBeginLabel->GetCodeOffset()); + ILStubEHClause clause{}; + clause.kind = builder.kind; + clause.dwTryBeginOffset = static_cast(builder.tryBeginLabel->GetCodeOffset()); DWORD tryEnd = static_cast(builder.tryEndLabel->GetCodeOffset()); - pClause->cbTryLength = tryEnd - pClause->dwTryBeginOffset; - pClause->dwHandlerBeginOffset = static_cast(builder.handlerBeginLabel->GetCodeOffset()); + clause.cbTryLength = tryEnd - clause.dwTryBeginOffset; + clause.dwHandlerBeginOffset = static_cast(builder.handlerBeginLabel->GetCodeOffset()); DWORD handlerEnd = static_cast(builder.handlerEndLabel->GetCodeOffset()); - pClause->cbHandlerLength = handlerEnd - pClause->dwHandlerBeginOffset; - pClause->dwTypeToken = builder.typeToken; + clause.cbHandlerLength = handlerEnd - clause.dwHandlerBeginOffset; + clause.dwTypeToken = builder.typeToken; + + return clause; } #ifdef _DEBUG diff --git a/src/coreclr/vm/stubgen.h b/src/coreclr/vm/stubgen.h index 846d7ee6e0a418..529cf95b5e3c6b 100644 --- a/src/coreclr/vm/stubgen.h +++ b/src/coreclr/vm/stubgen.h @@ -776,7 +776,7 @@ class ILStubLinker // Write out EH clauses. Number of items written out will be GetNumEHCLauses(). void WriteEHClauses(COR_ILMETHOD_SECT_EH* sect); // Get resolved EH clause info (must be called after Link()). - void GetEHClause(size_t index, ILStubEHClause* pClause); + ILStubEHClause GetEHClause(size_t index); TokenLookupMap* GetTokenLookupMap() { LIMITED_METHOD_CONTRACT; return &m_tokenMap; } From fcc813993dba07292afed381e0acba8df022dd84 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 3 Apr 2026 13:54:50 -0700 Subject: [PATCH 09/18] Fix CI assert for flag check --- src/coreclr/vm/dllimport.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index e0d82926b2abf1..673c860be971a2 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -4727,7 +4727,7 @@ COR_ILMETHOD_DECODER* PInvoke::CreatePInvokeMethodIL(PInvokeMethodDesc* pMD, Dyn NewHolder pResolver = new ILStubResolver(); pResolver->SetStubMethodDesc(pMD); - COR_ILMETHOD_DECODER* pIL = CreatePInvokeStubWorker(&stubState, pResolver, &sigDesc, sigInfo.GetCharSet(), sigInfo.GetLinkFlags(), sigInfo.GetCallConv(), dwStubFlags, pMD, pParamTokenArray, iLCIDArg); + COR_ILMETHOD_DECODER* pIL = CreatePInvokeStubWorker(&stubState, pResolver, &sigDesc, sigInfo.GetCharSet(), sigInfo.GetLinkFlags(), sigInfo.GetCallConv(), stubState.GetFlags(), pMD, pParamTokenArray, iLCIDArg); *ppResolver = pResolver.Extract(); return pIL; } From 18fc14338b4873bcc6d5f4373561b81d9c9f2599 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 3 Apr 2026 14:39:45 -0700 Subject: [PATCH 10/18] Assert based on sig as PInvoke flags may not be populated yet. --- src/coreclr/vm/dllimport.cpp | 4 ++-- src/coreclr/vm/prestub.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 673c860be971a2..88d9fad973033b 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -5797,7 +5797,7 @@ PCODE PInvoke::GetStubForILStub(PInvokeMethodDesc* pNMD, MethodDesc** ppStubMD, { STANDARD_VM_CONTRACT; - CONSISTENCY_CHECK(pNMD->IsVarArgs()); + CONSISTENCY_CHECK(pNMD->IsVarArg()); #ifdef FEATURE_PORTABLE_ENTRYPOINTS COMPlusThrow(kInvalidProgramException, IDS_EE_VARARG_NOT_SUPPORTED); @@ -5913,7 +5913,7 @@ PCODE GetStubForInteropMethod(MethodDesc* pMD, DWORD dwStubFlags) if (pMD->IsPInvoke()) { PInvokeMethodDesc* pNMD = (PInvokeMethodDesc*)pMD; - CONSISTENCY_CHECK(pNMD->IsVarArgs()); // Non-varargs shouldn't be using stubs. + CONSISTENCY_CHECK(pNMD->IsVarArg()); pStub = PInvoke::GetStubForILStub(pNMD, &pStubMD, dwStubFlags); } #ifdef FEATURE_COMINTEROP diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 4c415abc59f304..6f2b0bbcb938f9 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -2299,8 +2299,8 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT, CallerGCMode callerGCMo } // end else if (IsIL() || IsNoMetadata() || (IsPInvoke() && !IsVarArg())) else if (IsPInvoke()) { - _ASSERTE(static_cast(this)->IsVarArgs()); pCode = GetStubForInteropMethod(this); + _ASSERTE(static_cast(this)->IsVarArgs()); } else if (IsFCall()) { From 06006ce8e3c053c7c6888edeaa2e1e190f6bc901 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 6 Apr 2026 14:36:06 -0700 Subject: [PATCH 11/18] Convert ILStubLinker EH clause tracking from SArray to singly linked lists Replace SArray with intrusive singly linked lists for both the building (stack) and finished (queue) EH clause collections. - m_buildingEHClauses becomes m_pBuildingEHClauseStack (head pointer) - m_finishedEHClauses becomes m_pFinishedEHClauseHead/Tail pointers - EndHandler moves nodes between lists (zero-copy) instead of copying - Add DeleteEHClauses() for cleanup in destructor and ClearCode Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/dllimport.cpp | 6 +- src/coreclr/vm/stubgen.cpp | 113 +++++++++++++++++++++++++++-------- src/coreclr/vm/stubgen.h | 7 ++- 3 files changed, 96 insertions(+), 30 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 88d9fad973033b..b6a765ab9054de 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -2330,8 +2330,10 @@ void PInvokeStubLinker::End(DWORD dwStubFlags) else { // No cleanup needed - cancel the inner try block started in Begin(). - _ASSERTE(m_buildingEHClauses.GetCount() > 0); - m_buildingEHClauses.SetCount(m_buildingEHClauses.GetCount() - 1); + _ASSERTE(m_pBuildingEHClauseStack != NULL); + ILStubEHClauseBuilder* pCancel = m_pBuildingEHClauseStack; + m_pBuildingEHClauseStack = pCancel->next; + delete pCancel; } if (IsExceptionCleanupNeeded()) diff --git a/src/coreclr/vm/stubgen.cpp b/src/coreclr/vm/stubgen.cpp index 71de97742f5009..163cf1a7853bea 100644 --- a/src/coreclr/vm/stubgen.cpp +++ b/src/coreclr/vm/stubgen.cpp @@ -175,9 +175,9 @@ void ILCodeStream::Emit(ILInstrEnum instr, INT16 iStackDelta, UINT_PTR uArg) pInstrBuffer[idxCurInstr].iStackDelta = iStackDelta; pInstrBuffer[idxCurInstr].uArg = uArg; - if(m_pOwner->m_buildingEHClauses.GetCount() > 0) + if(m_pOwner->m_pBuildingEHClauseStack != NULL) { - ILStubEHClauseBuilder& clause = m_pOwner->m_buildingEHClauses[m_pOwner->m_buildingEHClauses.GetCount() - 1]; + ILStubEHClauseBuilder& clause = *m_pOwner->m_pBuildingEHClauseStack; if (clause.tryBeginLabel != NULL && clause.tryEndLabel != NULL && clause.handlerBeginLabel != NULL && clause.kind == ILStubEHClause::kTypedCatch) @@ -804,7 +804,7 @@ size_t ILStubLinker::Link(UINT* puMaxStack) UINT uMaxStack = 0; DEBUG_STMT(bool fStackUnderflow = false); - _ASSERTE(m_buildingEHClauses.GetCount() == 0); + _ASSERTE(m_pBuildingEHClauseStack == NULL); while (pCurrentStream) { @@ -849,15 +849,20 @@ size_t ILStubLinker::Link(UINT* puMaxStack) size_t ILStubLinker::GetNumEHClauses() { - return m_finishedEHClauses.GetCount(); + size_t count = 0; + for (ILStubEHClauseBuilder* pCurrent = m_pFinishedEHClauseHead; pCurrent != NULL; pCurrent = pCurrent->next) + { + count++; + } + return count; } void ILStubLinker::WriteEHClauses(COR_ILMETHOD_SECT_EH* pSect) { - const SArray& clauses = m_finishedEHClauses; - for (COUNT_T i = 0; i < clauses.GetCount(); i++) + COUNT_T i = 0; + for (ILStubEHClauseBuilder* pCurrent = m_pFinishedEHClauseHead; pCurrent != NULL; pCurrent = pCurrent->next, i++) { - const ILStubEHClauseBuilder& builder = clauses[i]; + const ILStubEHClauseBuilder& builder = *pCurrent; CorExceptionFlag flags; switch (builder.kind) @@ -882,13 +887,19 @@ void ILStubLinker::WriteEHClauses(COR_ILMETHOD_SECT_EH* pSect) } pSect->Fat.Kind = CorILMethod_Sect_EHTable | CorILMethod_Sect_FatFormat; - pSect->Fat.DataSize = COR_ILMETHOD_SECT_EH_FAT::Size(clauses.GetCount()); + pSect->Fat.DataSize = COR_ILMETHOD_SECT_EH_FAT::Size(i); } ILStubEHClause ILStubLinker::GetEHClause(size_t index) { - _ASSERTE(index < m_finishedEHClauses.GetCount()); - const ILStubEHClauseBuilder& builder = m_finishedEHClauses[static_cast(index)]; + ILStubEHClauseBuilder* pCurrent = m_pFinishedEHClauseHead; + for (size_t i = 0; i < index; i++) + { + _ASSERTE(pCurrent != NULL); + pCurrent = pCurrent->next; + } + _ASSERTE(pCurrent != NULL); + const ILStubEHClauseBuilder& builder = *pCurrent; ILStubEHClause clause{}; clause.kind = builder.kind; @@ -1079,17 +1090,23 @@ LPCSTR ILCodeStream::GetStreamDescription(ILStubLinker::CodeStreamType streamTyp void ILCodeStream::BeginTryBlock() { - ILStubEHClauseBuilder& clause = *m_pOwner->m_buildingEHClauses.Append(); - memset(&clause, 0, sizeof(ILStubEHClauseBuilder)); - clause.kind = ILStubEHClause::kNone; - clause.tryBeginLabel = NewCodeLabel(); - EmitLabel(clause.tryBeginLabel); + ILStubEHClauseBuilder* pClause = new ILStubEHClauseBuilder(); + pClause->kind = ILStubEHClause::kNone; + pClause->tryBeginLabel = NULL; + pClause->tryEndLabel = NULL; + pClause->handlerBeginLabel = NULL; + pClause->handlerEndLabel = NULL; + pClause->typeToken = 0; + pClause->next = m_pOwner->m_pBuildingEHClauseStack; + m_pOwner->m_pBuildingEHClauseStack = pClause; + pClause->tryBeginLabel = NewCodeLabel(); + EmitLabel(pClause->tryBeginLabel); } void ILCodeStream::EndTryBlock() { - _ASSERTE(m_pOwner->m_buildingEHClauses.GetCount() > 0); - ILStubEHClauseBuilder& clause = m_pOwner->m_buildingEHClauses[m_pOwner->m_buildingEHClauses.GetCount() - 1]; + _ASSERTE(m_pOwner->m_pBuildingEHClauseStack != NULL); + ILStubEHClauseBuilder& clause = *m_pOwner->m_pBuildingEHClauseStack; _ASSERTE(clause.tryBeginLabel != NULL && clause.tryEndLabel == NULL); clause.tryEndLabel = NewCodeLabel(); @@ -1098,8 +1115,8 @@ void ILCodeStream::EndTryBlock() void ILCodeStream::BeginHandler(DWORD kind, DWORD typeToken) { - _ASSERTE(m_pOwner->m_buildingEHClauses.GetCount() > 0); - ILStubEHClauseBuilder& clause = m_pOwner->m_buildingEHClauses[m_pOwner->m_buildingEHClauses.GetCount() - 1]; + _ASSERTE(m_pOwner->m_pBuildingEHClauseStack != NULL); + ILStubEHClauseBuilder& clause = *m_pOwner->m_pBuildingEHClauseStack; // tryEndLabel may not be set yet when the handler begins on a different // code stream than the try body (e.g. PInvoke cleanup finally). @@ -1115,8 +1132,9 @@ void ILCodeStream::BeginHandler(DWORD kind, DWORD typeToken) void ILCodeStream::EndHandler(DWORD kind) { - _ASSERTE(m_pOwner->m_buildingEHClauses.GetCount() > 0); - ILStubEHClauseBuilder& clause = m_pOwner->m_buildingEHClauses[m_pOwner->m_buildingEHClauses.GetCount() - 1]; + _ASSERTE(m_pOwner->m_pBuildingEHClauseStack != NULL); + ILStubEHClauseBuilder* pClause = m_pOwner->m_pBuildingEHClauseStack; + ILStubEHClauseBuilder& clause = *pClause; _ASSERTE(clause.tryBeginLabel != NULL && clause.tryEndLabel != NULL && clause.handlerBeginLabel != NULL && clause.handlerEndLabel == NULL && @@ -1125,8 +1143,18 @@ void ILCodeStream::EndHandler(DWORD kind) clause.handlerEndLabel = NewCodeLabel(); EmitLabel(clause.handlerEndLabel); - m_pOwner->m_finishedEHClauses.Append(clause); - m_pOwner->m_buildingEHClauses.SetCount(m_pOwner->m_buildingEHClauses.GetCount() - 1); + // Move from building stack to finished list + m_pOwner->m_pBuildingEHClauseStack = pClause->next; + pClause->next = NULL; + if (m_pOwner->m_pFinishedEHClauseTail != NULL) + { + m_pOwner->m_pFinishedEHClauseTail->next = pClause; + } + else + { + m_pOwner->m_pFinishedEHClauseHead = pClause; + } + m_pOwner->m_pFinishedEHClauseTail = pClause; } void ILCodeStream::BeginCatchBlock(int token) @@ -2500,7 +2528,10 @@ ILStubLinker::ILStubLinker(Module* pStubSigModule, const Signature &signature, S m_cbCurrentCompressedSigLen(1), m_nLocals(0), m_fHasThis(false), - m_pMD(pMD) + m_pMD(pMD), + m_pBuildingEHClauseStack(NULL), + m_pFinishedEHClauseHead(NULL), + m_pFinishedEHClauseTail(NULL) { CONTRACTL { @@ -2640,6 +2671,7 @@ ILStubLinker::~ILStubLinker() CONTRACTL_END; DeleteCodeLabels(); DeleteCodeStreams(); + DeleteEHClauses(); } void ILStubLinker::DeleteCodeLabels() @@ -2685,6 +2717,36 @@ void ILStubLinker::DeleteCodeStreams() m_pCodeStreamList = NULL; } +void ILStubLinker::DeleteEHClauses() +{ + CONTRACTL + { + NOTHROW; + MODE_ANY; + GC_TRIGGERS; + } + CONTRACTL_END; + + ILStubEHClauseBuilder* pCurrent = m_pBuildingEHClauseStack; + while (pCurrent) + { + ILStubEHClauseBuilder* pDeleteMe = pCurrent; + pCurrent = pCurrent->next; + delete pDeleteMe; + } + m_pBuildingEHClauseStack = NULL; + + pCurrent = m_pFinishedEHClauseHead; + while (pCurrent) + { + ILStubEHClauseBuilder* pDeleteMe = pCurrent; + pCurrent = pCurrent->next; + delete pDeleteMe; + } + m_pFinishedEHClauseHead = NULL; + m_pFinishedEHClauseTail = NULL; +} + void ILStubLinker::ClearCodeStreams() { CONTRACTL @@ -3348,8 +3410,7 @@ void ILStubLinker::ClearCode() DeleteCodeLabels(); ClearCodeStreams(); - m_buildingEHClauses.Clear(); - m_finishedEHClauses.Clear(); + DeleteEHClauses(); } void ILStubLinker::SetStubMethodDesc(MethodDesc* pMD) diff --git a/src/coreclr/vm/stubgen.h b/src/coreclr/vm/stubgen.h index 529cf95b5e3c6b..783be59ccd2a85 100644 --- a/src/coreclr/vm/stubgen.h +++ b/src/coreclr/vm/stubgen.h @@ -692,6 +692,7 @@ struct ILStubEHClauseBuilder ILCodeLabel* handlerBeginLabel; ILCodeLabel* handlerEndLabel; DWORD typeToken; + ILStubEHClauseBuilder* next; }; @@ -725,6 +726,7 @@ class ILStubLinker void DeleteCodeLabels(); void DeleteCodeStreams(); + void DeleteEHClauses(); struct ILInstruction { @@ -885,8 +887,9 @@ class ILStubLinker // can span different ILCodeStream instances (the PInvoke stub's // try-finally, for example, begins on the Marshal stream and the // handler ends on the Cleanup stream). - SArray m_buildingEHClauses; - SArray m_finishedEHClauses; + ILStubEHClauseBuilder* m_pBuildingEHClauseStack; + ILStubEHClauseBuilder* m_pFinishedEHClauseHead; + ILStubEHClauseBuilder* m_pFinishedEHClauseTail; }; // class ILStubLinker From 64c7e0b4380675ea0d32b9e96082413b220c8fb7 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 16 Apr 2026 12:07:31 -0700 Subject: [PATCH 12/18] Disable inlining P/Invoke marshalling IL. --- src/coreclr/vm/jitinterface.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index a6c97eadd71462..3b54b471f6f7cf 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7732,9 +7732,13 @@ CEEInfo::getMethodInfo( getMethodInfoWorker(ftn, &header, methInfo, context); result = true; } - else if (ftn->IsIL() || ftn->IsPInvoke() || ftn->IsDynamicMethod()) + else if (ftn->IsIL() || ftn->IsDynamicMethod()) { // IL methods with no IL header indicate there is no implementation defined in metadata. + // NOTE: P/Invoke methods are also IL methods with no IL header, + // but it is generally not profitable to inline the marshalling IL. + // As a result, we skip inlining the marshalling IL. + // P/Invokes that don't require marshalling can still be inlined directly by the JIT. getMethodInfoWorker(ftn, NULL, methInfo, context); result = true; } From 0d5f13ac568b1b7d2987e55101fbee0a723460a6 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 16 Apr 2026 14:41:11 -0700 Subject: [PATCH 13/18] PR feedback (ifdef early resolve to only be when we won't do it with a precode) --- src/coreclr/vm/prestub.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 6f2b0bbcb938f9..e2a2c8ab24ba1d 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -2292,10 +2292,15 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT, CallerGCMode callerGCMo #endif // !FEATURE_PORTABLE_ENTRYPOINTS pCode = PrepareInitialCode(callerGCMode); + // If we don't have a P/Invoke import precode, we need to resolve the target now. + // We can't wait until we're invoking the P/Invoke as we don't have a precode that + // can do the work then. +#ifndef HAS_PINVOKE_IMPORT_PRECODE if (IsPInvoke()) { PInvoke::ResolvePInvokeTarget(static_cast(this)); } +#endif // !HAS_PINVOKE_IMPORT_PRECODE } // end else if (IsIL() || IsNoMetadata() || (IsPInvoke() && !IsVarArg())) else if (IsPInvoke()) { From fdbf89fbb3ddc7c84c7d7ee746a8ca682a2e5e3a Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Thu, 16 Apr 2026 14:43:15 -0700 Subject: [PATCH 14/18] Apply suggestion from @jkotas --- src/coreclr/vm/prestub.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index e2a2c8ab24ba1d..53d8f3e303721e 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -2292,10 +2292,10 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT, CallerGCMode callerGCMo #endif // !FEATURE_PORTABLE_ENTRYPOINTS pCode = PrepareInitialCode(callerGCMode); +#ifndef HAS_PINVOKE_IMPORT_PRECODE // If we don't have a P/Invoke import precode, we need to resolve the target now. // We can't wait until we're invoking the P/Invoke as we don't have a precode that // can do the work then. -#ifndef HAS_PINVOKE_IMPORT_PRECODE if (IsPInvoke()) { PInvoke::ResolvePInvokeTarget(static_cast(this)); From 909e67fdd99a3a525d22a0fbb14e191805f6a637 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 17 Apr 2026 14:23:14 -0700 Subject: [PATCH 15/18] Fix GC mode checks for later P/Invoke resolution --- src/coreclr/vm/dllimport.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 7d32c1a7a50554..06bdc968758daa 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -6017,7 +6017,18 @@ EXTERN_C void* PInvokeImportWorker(PInvokeMethodDesc* pMD) { THROWS; GC_TRIGGERS; - MODE_PREEMPTIVE; + if (pMD->ShouldSuppressGCTransition()) + { + // SupressGCTransition P/Invokes that are resolved + // at runtime will be in COOP mode. + // SuppressGCTransition P/Invokes that are resolved + // at R2R load time will be in PREEMP mode. + MODE_ANY; + } + else + { + MODE_PREEMPTIVE; + } } CONTRACTL_END; @@ -6046,6 +6057,9 @@ EXTERN_C void* PInvokeImportWorker(PInvokeMethodDesc* pMD) || pMD->ShouldSuppressGCTransition()); CONSISTENCY_CHECK(pMD->IsPInvoke()); + + GCX_PREEMP(); + // // With IL stubs, we don't have to do anything but ensure the DLL is loaded. // From 93ef62577aba7781f668d3a0727e56786b30b335 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 20 Apr 2026 10:48:23 -0700 Subject: [PATCH 16/18] Restore prestub PInvoke target resolution. Add comments to all of the paths that resolve P/Invokes for the cases that they handle. --- src/coreclr/vm/dllimport.cpp | 55 +++++------------------------------- src/coreclr/vm/method.cpp | 33 ++++++---------------- src/coreclr/vm/prestub.cpp | 12 ++++---- 3 files changed, 22 insertions(+), 78 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 06bdc968758daa..876ec67dffdace 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -5833,11 +5833,12 @@ PCODE PInvoke::GetStubForILStub(PInvokeMethodDesc* pNMD, MethodDesc** ppStubMD, #ifdef FEATURE_PORTABLE_ENTRYPOINTS COMPlusThrow(kInvalidProgramException, IDS_EE_VARARG_NOT_SUPPORTED); #else // !FEATURE_PORTABLE_ENTRYPOINTS - // Vararg P/Invoke use shared stubs, they need a precode to push the hidden argument. (void)pNMD->GetOrCreatePrecode(); #endif // FEATURE_PORTABLE_ENTRYPOINTS + // Resolve the target of the P/Invoke method now. + // This way we don't need to try to do this every time that this P/Invoke is called with this signature. ResolvePInvokeTarget(pNMD); // @@ -6003,9 +6004,9 @@ VOID PInvokeMethodDesc::SetPInvokeTarget(LPVOID pTarget) // This function is reached only via PInvokeImportThunk. It's purpose // is to ensure that the target DLL is fully loaded and ready to run. // -// FUN FACTS: Though this function is actually entered in unmanaged mode, -// it can reenter managed mode and throw a CLR exception if the DLL linking -// fails. +// This function is called in the following cases: +// - ReadyToRun fixups for methods that contain inlined P/Invokes (at fixup time) +// - Inlined P/Invokes in jitted code at invoke time. //========================================================================== EXTERN_C void* PInvokeImportWorker(PInvokeMethodDesc* pMD) { @@ -6017,18 +6018,7 @@ EXTERN_C void* PInvokeImportWorker(PInvokeMethodDesc* pMD) { THROWS; GC_TRIGGERS; - if (pMD->ShouldSuppressGCTransition()) - { - // SupressGCTransition P/Invokes that are resolved - // at runtime will be in COOP mode. - // SuppressGCTransition P/Invokes that are resolved - // at R2R load time will be in PREEMP mode. - MODE_ANY; - } - else - { - MODE_PREEMPTIVE; - } + MODE_PREEMPTIVE; } CONTRACTL_END; @@ -6038,38 +6028,7 @@ EXTERN_C void* PInvokeImportWorker(PInvokeMethodDesc* pMD) // any of our internal exceptions into managed exceptions. INSTALL_UNWIND_AND_CONTINUE_HANDLER; - if (pMD->IsEarlyBound()) - { - // we need the MD to be populated in case we decide to build an intercept - // stub to wrap the target in InitEarlyBoundPInvokeTarget - PInvoke::PopulatePInvokeMethodDesc(pMD); - - pMD->InitEarlyBoundPInvokeTarget(); - } - else - { - // - // Otherwise we're in an inlined pinvoke late bound MD - // - INDEBUG(Thread *pThread = GetThread()); - { - _ASSERTE((pThread->GetFrame() != FRAME_TOP && pThread->GetFrame()->GetFrameIdentifier() == FrameIdentifier::InlinedCallFrame) - || pMD->ShouldSuppressGCTransition()); - - CONSISTENCY_CHECK(pMD->IsPInvoke()); - - GCX_PREEMP(); - - // - // With IL stubs, we don't have to do anything but ensure the DLL is loaded. - // - - PInvoke::PopulatePInvokeMethodDesc(pMD); - pMD->CheckRestore(); - - PInvokeLink(pMD); - } - } + PInvoke::ResolvePInvokeTarget(pMD); ret = pMD->GetPInvokeTarget(); diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 3b29475cacbd63..f6c3bd00af5c20 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -3461,29 +3461,6 @@ BOOL PInvokeMethodDesc::ComputeMarshalingRequired() return PInvoke::MarshalingRequired(this); } -/**********************************************************************************/ -// Forward declare the PInvokeImportWorker function - See dllimport.cpp -EXTERN_C LPVOID STDCALL PInvokeImportWorker(PInvokeMethodDesc*); -void *PInvokeMethodDesc::ResolveAndSetPInvokeTarget(_In_ PInvokeMethodDesc* pMD) -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - INJECT_FAULT(COMPlusThrowOM();); - PRECONDITION(CheckPointer(pMD)); - } - CONTRACTL_END - -// This build conditional is here due to dllimport.cpp -// not being relevant during the crossgen build. - LPVOID targetMaybe = PInvokeImportWorker(pMD); - _ASSERTE(targetMaybe != nullptr); - pMD->SetPInvokeTarget(targetMaybe); - return targetMaybe; - -} - BOOL PInvokeMethodDesc::TryGetResolvedPInvokeTarget(_In_ PInvokeMethodDesc* pMD, _Out_ void** ndirectTarget) { CONTRACTL @@ -3503,10 +3480,16 @@ BOOL PInvokeMethodDesc::TryGetResolvedPInvokeTarget(_In_ PInvokeMethodDesc* pMD, return TRUE; } + // We only resolve P/Invoke targets early + // for SuppressGCTransition inlined P/Invokes. + // We do so because we cannot resolve the target of a SuppressGCTransition inlined P/Invoke at the time of the call + // as the resolution logic violates the rules of SuppressGCTransition. if (!pMD->ShouldSuppressGCTransition()) return FALSE; - *ndirectTarget = ResolveAndSetPInvokeTarget(pMD); + PInvoke::ResolvePInvokeTarget(pMD); + *ndirectTarget = pMD->GetPInvokeTarget(); + return TRUE; } @@ -3526,7 +3509,7 @@ void PInvokeMethodDesc::InterlockedSetPInvokeFlags(WORD wFlags) WORD *pFlags = &m_wPInvokeFlags; - // Make sure that m_flags is aligned on a 4 byte boundry + // Make sure that m_flags is aligned on a 4 byte boundary _ASSERTE( ( ((size_t) pFlags) & (sizeof(ULONG)-1) ) == 0); // Ensure we won't be reading or writing outside the bounds of the PInvokeMethodDesc. diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 68ecb2b4ff323e..1fba04ff49d7b3 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -2294,15 +2294,17 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT, CallerGCMode callerGCMo #endif // !FEATURE_PORTABLE_ENTRYPOINTS pCode = PrepareInitialCode(callerGCMode); -#ifndef HAS_PINVOKE_IMPORT_PRECODE - // If we don't have a P/Invoke import precode, we need to resolve the target now. - // We can't wait until we're invoking the P/Invoke as we don't have a precode that - // can do the work then. + // We need to resolve the P/Invoke target in the prestub in the following cases: + // - SuppressGCTransition + // - The logic to resolve the P/Invoke target does not meet the requirements for SuppressGCTransition usage. + // - No P/Invoke import thunk + // - If there's no P/Invoke import thunk, then there's no later time to resolve the P/Invoke target. + // + // For simplicity, we will resolve all P/Invoke targets here for non-inlined P/Invokes. if (IsPInvoke()) { PInvoke::ResolvePInvokeTarget(static_cast(this)); } -#endif // !HAS_PINVOKE_IMPORT_PRECODE } // end else if (IsIL() || IsNoMetadata() || (IsPInvoke() && !IsVarArg())) else if (IsPInvoke()) { From e641566a2b5c8293e71ff5822bd306ed7d26a4c1 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Mon, 20 Apr 2026 10:56:36 -0700 Subject: [PATCH 17/18] Apply suggestion from @jkotas --- src/coreclr/vm/method.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index f6c3bd00af5c20..7bbe6b0560cf8e 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -3480,10 +3480,9 @@ BOOL PInvokeMethodDesc::TryGetResolvedPInvokeTarget(_In_ PInvokeMethodDesc* pMD, return TRUE; } - // We only resolve P/Invoke targets early - // for SuppressGCTransition inlined P/Invokes. + // We only resolve P/Invoke targets early for SuppressGCTransition inlined P/Invokes. // We do so because we cannot resolve the target of a SuppressGCTransition inlined P/Invoke at the time of the call - // as the resolution logic violates the rules of SuppressGCTransition. + // as the resolution logic violates the rules of SuppressGCTransition (this behavior is documented). if (!pMD->ShouldSuppressGCTransition()) return FALSE; From 8acb74b2c0eb3676fbbaacf54f416f170ca5b0ab Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 20 Apr 2026 11:33:14 -0700 Subject: [PATCH 18/18] Remove duplicate accelerator method --- src/coreclr/vm/dllimport.cpp | 5 +++-- src/coreclr/vm/jitinterface.cpp | 3 ++- src/coreclr/vm/method.hpp | 3 --- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 876ec67dffdace..f9d5924c94a517 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -6005,8 +6005,9 @@ VOID PInvokeMethodDesc::SetPInvokeTarget(LPVOID pTarget) // is to ensure that the target DLL is fully loaded and ready to run. // // This function is called in the following cases: -// - ReadyToRun fixups for methods that contain inlined P/Invokes (at fixup time) -// - Inlined P/Invokes in jitted code at invoke time. +// - Inlined non-SuppressGCTransition P/Invokes in jitted code at invoke time +// after we have already transitioned to unmanaged code. +// As far as the jitted code is concerned, we are in the unmanaged call right now. //========================================================================== EXTERN_C void* PInvokeImportWorker(PInvokeMethodDesc* pMD) { diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index fcf2bfa1846416..aa8ef95f926f29 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -14154,7 +14154,8 @@ BOOL LoadDynamicInfoEntry(Module *currentModule, MethodDesc *pMethod = ZapSig::DecodeMethod(currentModule, pInfoModule, pBlob); _ASSERTE(pMethod->IsPInvoke()); - result = (size_t)(LPVOID)PInvokeMethodDesc::ResolveAndSetPInvokeTarget((PInvokeMethodDesc*)pMethod); + PInvoke::ResolvePInvokeTarget((PInvokeMethodDesc*)pMethod); + result = (size_t)(LPVOID)((PInvokeMethodDesc*)pMethod)->GetPInvokeTarget(); } else { diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 1d84b12959b877..c4d3a64db9da70 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -3278,9 +3278,6 @@ class PInvokeMethodDesc : public MethodDesc kPInvokePopulated = 0x8000, // Indicate if the PInvoke has been fully populated. }; - // Resolve the import to the PInvoke target and set it on the PInvokeMethodDesc. - static void* ResolveAndSetPInvokeTarget(_In_ PInvokeMethodDesc* pMD); - // Attempt to get a resolved PInvoke target. This will return true for already resolved // targets and methods that are resolved at JIT time, such as those marked SuppressGCTransition static BOOL TryGetResolvedPInvokeTarget(_In_ PInvokeMethodDesc* pMD, _Out_ void** ndirectTarget);