From a3e7ef919e8d2cedaac1ec27067e5700a7c1f7d8 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 24 Apr 2026 12:58:21 -0700 Subject: [PATCH 1/7] Fix argument size calculation for x86 stack --- src/coreclr/vm/dllimport.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index f9d5924c94a517..8c6b9d37475581 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -1299,6 +1299,7 @@ class PInvoke_StackArgumentSize_ILStubState final : public ILStubState void MarshalArgument(MarshalInfo* pInfo, int argOffset) { LIMITED_METHOD_CONTRACT; + pInfo->SetupArgumentSizes(); } void MarshalLCID(int argIdx) From 6843ceaeb5e444f5379bc8e3e54c885fbcb07767 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 24 Apr 2026 12:58:58 -0700 Subject: [PATCH 2/7] SuppressGCTransition methods can hit the PInvokeImportWorker depending on inlining decisions. Update handling to understand this case. --- src/coreclr/vm/dllimport.cpp | 40 +++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 8c6b9d37475581..cbfb3c9cf8f3ab 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -6009,6 +6009,8 @@ VOID PInvokeMethodDesc::SetPInvokeTarget(LPVOID pTarget) // - 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. +// - Non-inlined P/Invokes in jitted code at invoke time. +// In these cases the target is guaranteed to be resolved in the prestub. //========================================================================== EXTERN_C void* PInvokeImportWorker(PInvokeMethodDesc* pMD) { @@ -6020,22 +6022,40 @@ EXTERN_C void* PInvokeImportWorker(PInvokeMethodDesc* pMD) { THROWS; GC_TRIGGERS; - MODE_PREEMPTIVE; + if (pMD->ShouldSuppressGCTransition()) + { + MODE_COOPERATIVE; + } + else + { + MODE_PREEMPTIVE; + } } CONTRACTL_END; - INSTALL_MANAGED_EXCEPTION_DISPATCHER; - // this function is called by CLR to native assembly stubs which are called by - // managed code as a result, we need an unwind and continue handler to translate - // any of our internal exceptions into managed exceptions. - INSTALL_UNWIND_AND_CONTINUE_HANDLER; + if (!pMD->PInvokeTargetIsImportThunk()) + { + ret = pMD->GetPInvokeTarget(); + } + else + { + // Inlined SuppressGCTransition P/Invokes should + // be resolved during jitting, not here. + _ASSERTE(!pMD->ShouldSuppressGCTransition()); - PInvoke::ResolvePInvokeTarget(pMD); + INSTALL_MANAGED_EXCEPTION_DISPATCHER; + // this function is called by CLR to native assembly stubs which are called by + // managed code as a result, we need an unwind and continue handler to translate + // any of our internal exceptions into managed exceptions. + INSTALL_UNWIND_AND_CONTINUE_HANDLER; - ret = pMD->GetPInvokeTarget(); + PInvoke::ResolvePInvokeTarget(pMD); - UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; - UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; + ret = pMD->GetPInvokeTarget(); + + UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; + UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; + } return ret; } From 67b2952ab5fe616190e77f6a261f89a3e7a32d50 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 24 Apr 2026 13:09:47 -0700 Subject: [PATCH 3/7] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- 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 cbfb3c9cf8f3ab..ac787be2fe2ce7 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -1298,7 +1298,7 @@ class PInvoke_StackArgumentSize_ILStubState final : public ILStubState void MarshalArgument(MarshalInfo* pInfo, int argOffset) { - LIMITED_METHOD_CONTRACT; + STANDARD_VM_CONTRACT; pInfo->SetupArgumentSizes(); } From c6b6d6fbb0cb29d9d10b2a46db0f7e022ec73c6e Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 24 Apr 2026 13:12:55 -0700 Subject: [PATCH 4/7] Apply suggestions from code review Co-authored-by: Jan Kotas --- src/coreclr/vm/dllimport.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index ac787be2fe2ce7..8a4548b8748406 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -6033,11 +6033,7 @@ EXTERN_C void* PInvokeImportWorker(PInvokeMethodDesc* pMD) } CONTRACTL_END; - if (!pMD->PInvokeTargetIsImportThunk()) - { - ret = pMD->GetPInvokeTarget(); - } - else + if (pMD->PInvokeTargetIsImportThunk()) { // Inlined SuppressGCTransition P/Invokes should // be resolved during jitting, not here. @@ -6051,13 +6047,11 @@ EXTERN_C void* PInvokeImportWorker(PInvokeMethodDesc* pMD) PInvoke::ResolvePInvokeTarget(pMD); - ret = pMD->GetPInvokeTarget(); - UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; } - return ret; + return pMD->GetPInvokeTarget(); } //=========================================================================== From 7bd2b43fb288f684ae47972497f23c68b8fc23b4 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 24 Apr 2026 14:47:07 -0700 Subject: [PATCH 5/7] Use a volatile store to avoid store reordering --- src/coreclr/vm/dllimport.cpp | 42 ++++++++++-------------------------- 1 file changed, 11 insertions(+), 31 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index cbfb3c9cf8f3ab..e7c7591616b618 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -5998,7 +5998,7 @@ VOID PInvokeMethodDesc::SetPInvokeTarget(LPVOID pTarget) } CONTRACTL_END; - m_pPInvokeTarget = pTarget; + VolatileStore(&m_pPInvokeTarget, pTarget); } //========================================================================== @@ -6009,8 +6009,6 @@ VOID PInvokeMethodDesc::SetPInvokeTarget(LPVOID pTarget) // - 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. -// - Non-inlined P/Invokes in jitted code at invoke time. -// In these cases the target is guaranteed to be resolved in the prestub. //========================================================================== EXTERN_C void* PInvokeImportWorker(PInvokeMethodDesc* pMD) { @@ -6022,40 +6020,22 @@ EXTERN_C void* PInvokeImportWorker(PInvokeMethodDesc* pMD) { THROWS; GC_TRIGGERS; - if (pMD->ShouldSuppressGCTransition()) - { - MODE_COOPERATIVE; - } - else - { - MODE_PREEMPTIVE; - } + MODE_PREEMPTIVE; } CONTRACTL_END; - if (!pMD->PInvokeTargetIsImportThunk()) - { - ret = pMD->GetPInvokeTarget(); - } - else - { - // Inlined SuppressGCTransition P/Invokes should - // be resolved during jitting, not here. - _ASSERTE(!pMD->ShouldSuppressGCTransition()); - - INSTALL_MANAGED_EXCEPTION_DISPATCHER; - // this function is called by CLR to native assembly stubs which are called by - // managed code as a result, we need an unwind and continue handler to translate - // any of our internal exceptions into managed exceptions. - INSTALL_UNWIND_AND_CONTINUE_HANDLER; + INSTALL_MANAGED_EXCEPTION_DISPATCHER; + // this function is called by CLR to native assembly stubs which are called by + // managed code as a result, we need an unwind and continue handler to translate + // any of our internal exceptions into managed exceptions. + INSTALL_UNWIND_AND_CONTINUE_HANDLER; - PInvoke::ResolvePInvokeTarget(pMD); + PInvoke::ResolvePInvokeTarget(pMD); - ret = pMD->GetPInvokeTarget(); + ret = pMD->GetPInvokeTarget(); - UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; - UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; - } + UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; + UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; return ret; } From d4984b8ce92967ce1914e576167bd80e22e6251d Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 27 Apr 2026 13:11:53 -0700 Subject: [PATCH 6/7] Set P/Invoke target after IL generation (so MarshalingDirectiveExceptions get thrown first) but before jitting (so we don't cache the code pointer before resolving the target for SuppressGCTransition P/Invokes. --- src/coreclr/vm/dllimport.cpp | 14 +++++++++++++- src/coreclr/vm/prestub.cpp | 12 ------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 82523225ec84c7..29c023d271005e 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -4760,6 +4760,18 @@ COR_ILMETHOD_DECODER* PInvoke::CreatePInvokeMethodIL(PInvokeMethodDesc* pMD, Dyn pResolver->SetStubMethodDesc(pMD); COR_ILMETHOD_DECODER* pIL = CreatePInvokeStubWorker(&stubState, pResolver, &sigDesc, sigInfo.GetCharSet(), sigInfo.GetLinkFlags(), sigInfo.GetCallConv(), stubState.GetFlags(), pMD, pParamTokenArray, iLCIDArg); + + // By now, we've generated the marshaling IL and we will have thrown any exception for invalid marshaling. + // We can now safely resolve the P/Invoke target. + // We need to resolve the P/Invoke target here 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. + PInvoke::ResolvePInvokeTarget(pMD); + *ppResolver = pResolver.Extract(); return pIL; } @@ -5998,7 +6010,7 @@ VOID PInvokeMethodDesc::SetPInvokeTarget(LPVOID pTarget) } CONTRACTL_END; - VolatileStore(&m_pPInvokeTarget, pTarget); + m_pPInvokeTarget = pTarget; } //========================================================================== diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index 73ae13ab6e5ac9..532c285d4fab46 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -2402,18 +2402,6 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT, CallerGCMode callerGCMo } #endif // !FEATURE_PORTABLE_ENTRYPOINTS pCode = PrepareInitialCode(callerGCMode); - - // 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)); - } } // end else if (IsIL() || IsNoMetadata() || (IsPInvoke() && !IsVarArg())) else if (IsPInvoke()) { From e94add11368ebbf3513e3e0d60636111703ef015 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 27 Apr 2026 14:09:14 -0700 Subject: [PATCH 7/7] Use TryGetResolvedPInvokeTarget and move handling for no-pinvoke-import-thunk into a more shared location --- src/coreclr/vm/dllimport.cpp | 29 +++++++++++++++-------------- src/coreclr/vm/method.cpp | 12 +++++++++--- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 29c023d271005e..0c5e85018240c5 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -2392,9 +2392,21 @@ void PInvokeStubLinker::DoPInvoke(ILCodeStream *pcsEmit, DWORD dwStubFlags, Meth { _ASSERTE(pMD->IsPInvoke()); PInvokeMethodDesc* pTargetMD = (PInvokeMethodDesc*)pMD; - pcsEmit->EmitLDC((DWORD_PTR)&pTargetMD->m_pPInvokeTarget); - pcsEmit->EmitCONV_I(); - pcsEmit->EmitLDIND_I(); + + // Resolve the P/Invoke target now if our P/Invoke is one that must be resolved eagerly + // (ie SuppressGCTransition) + void* pInvokeTarget = nullptr; + if (PInvokeMethodDesc::TryGetResolvedPInvokeTarget(pTargetMD, &pInvokeTarget)) + { + pcsEmit->EmitLDC((DWORD_PTR)pInvokeTarget); + pcsEmit->EmitCONV_I(); + } + else + { + pcsEmit->EmitLDC((DWORD_PTR)&pTargetMD->m_pPInvokeTarget); + pcsEmit->EmitCONV_I(); + pcsEmit->EmitLDIND_I(); + } } } else // native-to-managed @@ -4761,17 +4773,6 @@ COR_ILMETHOD_DECODER* PInvoke::CreatePInvokeMethodIL(PInvokeMethodDesc* pMD, Dyn COR_ILMETHOD_DECODER* pIL = CreatePInvokeStubWorker(&stubState, pResolver, &sigDesc, sigInfo.GetCharSet(), sigInfo.GetLinkFlags(), sigInfo.GetCallConv(), stubState.GetFlags(), pMD, pParamTokenArray, iLCIDArg); - // By now, we've generated the marshaling IL and we will have thrown any exception for invalid marshaling. - // We can now safely resolve the P/Invoke target. - // We need to resolve the P/Invoke target here 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. - PInvoke::ResolvePInvokeTarget(pMD); - *ppResolver = pResolver.Extract(); return pIL; } diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 79040c69c9e08b..9f5dd3d17e6a8e 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -3543,11 +3543,17 @@ 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 (this behavior is documented). + // We only resolve P/Invoke targets early in two cases: + // - 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 (this behavior is documented). + // - Platforms with no P/Invoke import precode: + // On these platforms, there is no mechanism to lazily resolve the target of a P/Invoke, + // so we must resolve it eagerly in order for the call to succeed. +#ifdef HAS_PINVOKE_IMPORT_PRECODE if (!pMD->ShouldSuppressGCTransition()) return FALSE; +#endif PInvoke::ResolvePInvokeTarget(pMD); *ndirectTarget = pMD->GetPInvokeTarget();