From 3a475b6f2ff7ae4ad4335aa409473bc1051712c8 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 6 Apr 2026 13:04:47 -0700 Subject: [PATCH 1/5] Clean up dyanmic method desc native stack arg tracking It's only needed on x86 now and we don't need to track it quite as heavily as we have been. --- src/coreclr/vm/clrtocomcall.cpp | 8 ------ src/coreclr/vm/comtoclrcall.cpp | 18 +++--------- src/coreclr/vm/comtoclrcall.h | 50 +++++++++++---------------------- src/coreclr/vm/dllimport.cpp | 12 ++------ src/coreclr/vm/method.cpp | 4 +-- src/coreclr/vm/method.hpp | 12 +++----- 6 files changed, 28 insertions(+), 76 deletions(-) diff --git a/src/coreclr/vm/clrtocomcall.cpp b/src/coreclr/vm/clrtocomcall.cpp index 7592ae55f743f4..96d4b9fc506361 100644 --- a/src/coreclr/vm/clrtocomcall.cpp +++ b/src/coreclr/vm/clrtocomcall.cpp @@ -180,14 +180,6 @@ namespace &stubLinker ); -#if defined(FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE) - if (pStubMD->IsDynamicMethod()) - { - DynamicMethodDesc* pDMD = pStubMD->AsDynamicMethodDesc(); - pDMD->SetNativeStackArgSize(2 * TARGET_POINTER_SIZE); // The native stack arg size is constant since the signature for struct stubs is constant. - } -#endif // FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE - szMetaSig.SuppressRelease(); return pStubMD; diff --git a/src/coreclr/vm/comtoclrcall.cpp b/src/coreclr/vm/comtoclrcall.cpp index 5974264fd83688..25535903d3a69c 100644 --- a/src/coreclr/vm/comtoclrcall.cpp +++ b/src/coreclr/vm/comtoclrcall.cpp @@ -480,22 +480,12 @@ PCODE ComCallMethodDesc::CreateCOMToCLRStub(DWORD dwStubFlags, MethodDesc **ppSt *ppStubMD = pStubMD; -#ifdef TARGET_X86 + _ASSERTE(pStubMD->IsILStub()); + +#if defined(TARGET_X86) // make sure our native stack computation in code:ComCallMethodDesc.InitNativeInfo is right _ASSERTE(HasMarshalError() || !pStubMD->IsILStub() || pStubMD->AsDynamicMethodDesc()->GetNativeStackArgSize() == m_StackBytes); -#else // TARGET_X86 - - if (pStubMD->IsILStub()) - { - m_StackBytes = pStubMD->AsDynamicMethodDesc()->GetNativeStackArgSize(); - _ASSERTE(m_StackBytes == pStubMD->SizeOfArgStack()); - } - else - { - UINT size = pStubMD->SizeOfArgStack(); - _ASSERTE(size <= USHRT_MAX); - m_StackBytes = (UINT16)size; - } + m_StackBytes = pStubMD->AsDynamicMethodDesc()->GetNativeStackArgSize(); #endif // TARGET_X86 RETURN JitILStub(pStubMD); diff --git a/src/coreclr/vm/comtoclrcall.h b/src/coreclr/vm/comtoclrcall.h index 630b2d05b809ad..8f31e2e66042ad 100644 --- a/src/coreclr/vm/comtoclrcall.h +++ b/src/coreclr/vm/comtoclrcall.h @@ -229,22 +229,6 @@ class ComCallMethodDesc RETURN m_pMD->GetSlot(); } - // get num stack bytes to pop - UINT16 GetNumStackBytes() - { - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_ANY; - PRECONDITION(m_flags & enum_NativeInfoInitialized); - SUPPORTS_DAC; - } - CONTRACTL_END; - - return m_StackBytes; - } - //get call sig PCCOR_SIGNATURE GetSig(DWORD *pcbSigSize = NULL) { @@ -292,29 +276,27 @@ class ComCallMethodDesc PCODE m_pILStub; // IL stub for COM to CLR call, invokes GetCallMethodDesc() - // Platform specific data needed for efficient IL stub invocation: #ifdef TARGET_X86 - union + // Number of stack bytes pushed by the unmanaged caller. + UINT16 m_StackBytes; + +public: + // get num stack bytes to pop + UINT16 GetNumStackBytes() { - struct + CONTRACTL { - // Index of the stack slot that gets stuffed into EDX when calling the stub. - UINT16 m_wSourceSlotEDX; - - // Number of stack slots expected by the IL stub. - UINT16 m_wStubStackSlotCount; - }; - // Combination of m_wSourceSlotEDX and m_wStubStackSlotCount for atomic updates. - UINT32 m_dwSlotInfo; - }; + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + PRECONDITION(m_flags & enum_NativeInfoInitialized); + SUPPORTS_DAC; + } + CONTRACTL_END; - // This is an array of m_wStubStackSlotCount numbers where each element is the offset - // on the source stack where the particular stub stack slot should be copied from. - UINT16 *m_pwStubStackSlotOffsets; + return m_StackBytes; + } #endif // TARGET_X86 - - // Number of stack bytes pushed by the unmanaged caller. - UINT16 m_StackBytes; }; extern "C" void ComCallPreStub(); diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 386c1033ea9218..320e46f29cab85 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -460,7 +460,7 @@ class ILStubState pStubMD->SetStatic(); } -#if !defined(TARGET_X86) && defined(FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE) +#if defined(TARGET_X86) // we store the real managed argument stack size in the stub MethodDesc on non-X86 UINT stackSize = pStubMD->SizeOfNativeArgStack(); @@ -4012,7 +4012,7 @@ static void CreatePInvokeStubWorker(ILStubState* pss, if (!FitsInU2(nativeStackSize)) COMPlusThrow(kMarshalDirectiveException, IDS_EE_SIGTOOCOMPLEX); -#ifdef FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE +#ifdef TARGET_X86 DynamicMethodDesc *pDMD = pMD->AsDynamicMethodDesc(); pDMD->SetNativeStackArgSize(static_cast(nativeStackSize)); @@ -5632,14 +5632,6 @@ MethodDesc* PInvoke::CreateLayoutClassMarshalILStub(MethodTable* pMT, MarshalOpe &pLinker ); -#if defined(FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE) - if (pStubMD->IsDynamicMethod()) - { - DynamicMethodDesc* pDMD = pStubMD->AsDynamicMethodDesc(); - pDMD->SetNativeStackArgSize(3 * TARGET_POINTER_SIZE); // The native stack arg size is constant since the signature for struct stubs is constant. - } -#endif - szMetaSig.SuppressRelease(); RETURN pStubMD; diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 9295fb3d38e534..9ba8818e0936e2 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -1759,7 +1759,7 @@ UINT MethodDesc::SizeOfArgStack() return argit.SizeOfArgStack(); } -#ifdef FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE +#ifdef TARGET_X86 UINT MethodDesc::SizeOfNativeArgStack() { #ifndef UNIX_AMD64_ABI @@ -1771,7 +1771,7 @@ UINT MethodDesc::SizeOfNativeArgStack() return argit.SizeOfArgStack(); #endif } -#endif // FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE +#endif // TARGET_X86 #ifdef TARGET_X86 //******************************************************************************* diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index e196bf575bb9ce..f046c819dedfff 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -60,10 +60,6 @@ EXTERN_C VOID STDCALL PInvokeImportThunk(); #define METHOD_TOKEN_RANGE_BIT_COUNT (24 - METHOD_TOKEN_REMAINDER_BIT_COUNT) #define METHOD_TOKEN_RANGE_MASK ((1 << METHOD_TOKEN_RANGE_BIT_COUNT) - 1) -#if defined(TARGET_X86) || defined(FEATURE_COMINTEROP) -#define FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE -#endif - enum class AsyncMethodFlags { // Method uses CORINFO_CALLCONV_ASYNCCALL call convention. @@ -868,11 +864,11 @@ class MethodDesc // arguments passed in registers. UINT SizeOfArgStack(); -#ifdef FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE +#ifdef TARGET_X86 // Returns the # of bytes of stack used by arguments in a call from native to this function. // Does not include arguments passed in registers. UINT SizeOfNativeArgStack(); -#endif // FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE +#endif // TARGET_X86 // Returns the # of bytes to pop after a call. Not necessary the // same as SizeOfArgStack()! @@ -2938,7 +2934,7 @@ class DynamicMethodDesc : public StoredSigMethodDesc return asMetadata; } -#if defined(TARGET_X86) || defined(FEATURE_COMINTEROP) +#if defined(TARGET_X86) WORD GetNativeStackArgSize() { LIMITED_METHOD_DAC_CONTRACT; @@ -2955,7 +2951,7 @@ class DynamicMethodDesc : public StoredSigMethodDesc #endif m_dwExtendedFlags = (m_dwExtendedFlags & ~StackArgSizeMask) | ((DWORD)cbArgSize << 16); } -#endif // TARGET_X86 || FEATURE_COMINTEROP +#endif // TARGET_X86 bool IsReversePInvokeStub() const { From daadc0424731fd07c3c58a2788f8b8c1b2655d13 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 6 Apr 2026 13:29:33 -0700 Subject: [PATCH 2/5] Copilot feedback --- src/coreclr/vm/comtoclrcall.cpp | 13 +------------ src/coreclr/vm/dllimport.cpp | 2 +- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/coreclr/vm/comtoclrcall.cpp b/src/coreclr/vm/comtoclrcall.cpp index 25535903d3a69c..c245ee09c17765 100644 --- a/src/coreclr/vm/comtoclrcall.cpp +++ b/src/coreclr/vm/comtoclrcall.cpp @@ -45,11 +45,6 @@ void ComCallMethodDesc::InitMethod(MethodDesc *pMD, MethodDesc *pInterfaceMD) m_pInterfaceMD = PTR_MethodDesc(pInterfaceMD); m_pILStub = NULL; -#ifdef TARGET_X86 - m_dwSlotInfo = 0; - m_pwStubStackSlotOffsets = NULL; -#endif // TARGET_X86 - // Initialize the native type information size of native stack, native retval flags, etc). InitNativeInfo(); } @@ -66,11 +61,6 @@ void ComCallMethodDesc::InitField(FieldDesc* pFD, BOOL isGetter) m_pFD = pFD; m_pILStub = NULL; -#ifdef TARGET_X86 - m_dwSlotInfo = 0; - m_pwStubStackSlotOffsets = NULL; -#endif // TARGET_X86 - m_flags = enum_IsFieldCall; // mark the attribute as a field m_flags |= isGetter ? enum_IsGetter : 0; @@ -92,11 +82,10 @@ void ComCallMethodDesc::InitNativeInfo() } CONTRACT_END; - m_StackBytes = (UINT16)-1; - EX_TRY { #ifdef TARGET_X86 + m_StackBytes = (UINT16)-1; // On x86, this method has to compute size of arguments because we need to know size of the native stack // to be able to return back to unmanaged code UINT16 nativeArgSize; diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 320e46f29cab85..f78776dc35fb45 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -461,7 +461,7 @@ class ILStubState } #if defined(TARGET_X86) - // we store the real managed argument stack size in the stub MethodDesc on non-X86 + // we store the real native argument stack size in the stub MethodDesc UINT stackSize = pStubMD->SizeOfNativeArgStack(); if (!FitsInU2(stackSize)) From b4a0585af5b2144c75607021b084f2883b7495b1 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 6 Apr 2026 13:55:09 -0700 Subject: [PATCH 3/5] PR feedback --- src/coreclr/vm/comtoclrcall.cpp | 2 +- src/coreclr/vm/dllimport.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/comtoclrcall.cpp b/src/coreclr/vm/comtoclrcall.cpp index c245ee09c17765..47a5aab82f1ca9 100644 --- a/src/coreclr/vm/comtoclrcall.cpp +++ b/src/coreclr/vm/comtoclrcall.cpp @@ -471,7 +471,7 @@ PCODE ComCallMethodDesc::CreateCOMToCLRStub(DWORD dwStubFlags, MethodDesc **ppSt _ASSERTE(pStubMD->IsILStub()); -#if defined(TARGET_X86) +#ifdef TARGET_X86 // make sure our native stack computation in code:ComCallMethodDesc.InitNativeInfo is right _ASSERTE(HasMarshalError() || !pStubMD->IsILStub() || pStubMD->AsDynamicMethodDesc()->GetNativeStackArgSize() == m_StackBytes); m_StackBytes = pStubMD->AsDynamicMethodDesc()->GetNativeStackArgSize(); diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index f78776dc35fb45..2efd4933af34ce 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -460,7 +460,7 @@ class ILStubState pStubMD->SetStatic(); } -#if defined(TARGET_X86) +#ifdef TARGET_X86 // we store the real native argument stack size in the stub MethodDesc UINT stackSize = pStubMD->SizeOfNativeArgStack(); From 81a5ce9996e569d19a6c64159968688f799cfd9f Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 23 Apr 2026 12:06:24 -0700 Subject: [PATCH 4/5] Fix stack tracking and bad buffer management. Remove more dead code --- src/coreclr/vm/comcallablewrapper.cpp | 2 +- src/coreclr/vm/dllimport.cpp | 26 ++++---------------------- src/coreclr/vm/method.cpp | 14 -------------- src/coreclr/vm/method.hpp | 6 ------ 4 files changed, 5 insertions(+), 43 deletions(-) diff --git a/src/coreclr/vm/comcallablewrapper.cpp b/src/coreclr/vm/comcallablewrapper.cpp index 6f9a656110b3f2..3c3e8c35c3c124 100644 --- a/src/coreclr/vm/comcallablewrapper.cpp +++ b/src/coreclr/vm/comcallablewrapper.cpp @@ -3223,7 +3223,7 @@ BOOL ComMethodTable::LayOutInterfaceMethodTable(MethodTable* pClsMT) // to access empty slots quickly and, during cleanup, we can tell empty // slots from full ones. if (m_pMT->IsSparseForCOMInterop()) - memset(pUnkVtable + cbExtraSlots, -1, m_cbSlots * sizeof(SLOT)); + memset(((SLOT*)pUnkVtable) + cbExtraSlots, -1, m_cbSlots * sizeof(SLOT)); // Method descs are at the end of the vtable // m_cbSlots interfaces methods + IUnk methods diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 2efd4933af34ce..f63a189ece4297 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -459,16 +459,6 @@ class ILStubState ((PTR_DynamicMethodDesc)pStubMD)->SetFlags(DynamicMethodDesc::FlagStatic); pStubMD->SetStatic(); } - -#ifdef TARGET_X86 - // we store the real native argument stack size in the stub MethodDesc - UINT stackSize = pStubMD->SizeOfNativeArgStack(); - - if (!FitsInU2(stackSize)) - COMPlusThrow(kMarshalDirectiveException, IDS_EE_SIGTOOCOMPLEX); - - pStubMD->AsDynamicMethodDesc()->SetNativeStackArgSize(static_cast(stackSize)); -#endif // TARGET_X86 } DWORD cbTempModuleIndependentSigLength; @@ -3991,38 +3981,30 @@ static void CreatePInvokeStubWorker(ILStubState* pss, nativeStackSize += TARGET_POINTER_SIZE; } +#ifdef TARGET_X86 if (pMD->IsDynamicMethod()) { - // Set the native stack size to the IL stub MD. It is needed for alignment - // thunk generation on the Mac and stdcall name decoration on Windows. - // We do not store it directly in the interop MethodDesc here because due - // to sharing we come here only for the first call with given signature and - // the target MD may even be NULL. - -#ifdef TARGET_X86 + // Set the native stack size to the IL stub MD. It is needed for + // stdcall name decoration and correct stack management in error cases for COM->CLR calls. if (fThisCall) { _ASSERTE(nativeStackSize >= TARGET_POINTER_SIZE); nativeStackSize -= TARGET_POINTER_SIZE; } -#endif // TARGET_X86 nativeStackSize = ALIGN_UP(nativeStackSize, TARGET_POINTER_SIZE); if (!FitsInU2(nativeStackSize)) COMPlusThrow(kMarshalDirectiveException, IDS_EE_SIGTOOCOMPLEX); -#ifdef TARGET_X86 DynamicMethodDesc *pDMD = pMD->AsDynamicMethodDesc(); pDMD->SetNativeStackArgSize(static_cast(nativeStackSize)); if (fStubNeedsCOM) pDMD->SetFlags(DynamicMethodDesc::FlagRequiresCOM); -#endif } +#endif - // 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); } diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp index 9a8d6100ac2efc..f581f2d864f339 100644 --- a/src/coreclr/vm/method.cpp +++ b/src/coreclr/vm/method.cpp @@ -1759,20 +1759,6 @@ UINT MethodDesc::SizeOfArgStack() return argit.SizeOfArgStack(); } -#ifdef TARGET_X86 -UINT MethodDesc::SizeOfNativeArgStack() -{ -#ifndef UNIX_AMD64_ABI - return SizeOfArgStack(); -#else - WRAPPER_NO_CONTRACT; - MetaSig msig(this); - PInvokeArgIterator argit(&msig); - return argit.SizeOfArgStack(); -#endif -} -#endif // TARGET_X86 - #ifdef TARGET_X86 //******************************************************************************* UINT MethodDesc::CbStackPop() diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index dca9ff88e7cf12..1b4688f5e631ea 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -866,12 +866,6 @@ class MethodDesc // arguments passed in registers. UINT SizeOfArgStack(); -#ifdef TARGET_X86 - // Returns the # of bytes of stack used by arguments in a call from native to this function. - // Does not include arguments passed in registers. - UINT SizeOfNativeArgStack(); -#endif // TARGET_X86 - // Returns the # of bytes to pop after a call. Not necessary the // same as SizeOfArgStack()! UINT CbStackPop(); From 9ee2c7aef72ece654c955a75f7184813c4b5f8dc Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 23 Apr 2026 13:01:25 -0700 Subject: [PATCH 5/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/coreclr/vm/dllimport.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index f63a189ece4297..728fcefaa972c8 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -3998,13 +3998,16 @@ static void CreatePInvokeStubWorker(ILStubState* pss, COMPlusThrow(kMarshalDirectiveException, IDS_EE_SIGTOOCOMPLEX); DynamicMethodDesc *pDMD = pMD->AsDynamicMethodDesc(); - pDMD->SetNativeStackArgSize(static_cast(nativeStackSize)); - if (fStubNeedsCOM) - pDMD->SetFlags(DynamicMethodDesc::FlagRequiresCOM); } #endif + if (fStubNeedsCOM && pMD->IsDynamicMethod()) + { + DynamicMethodDesc *pDMD = pMD->AsDynamicMethodDesc(); + pDMD->SetFlags(DynamicMethodDesc::FlagRequiresCOM); + } + pss->FinishEmit(pMD); }