From 77cf29935aff59ff0a960b93092d16d03157763f Mon Sep 17 00:00:00 2001 From: Jonghyun Park Date: Thu, 16 Mar 2017 13:05:30 +0900 Subject: [PATCH] Reorder stack arguments on reverse P/Invoke (dotnet/coreclr#10217) Commit migrated from https://github.com/dotnet/coreclr/commit/e55c985fb2c833b332581680f050bf7ebaa98f5f --- src/coreclr/src/vm/dllimportcallback.cpp | 82 +++++++++++++++++++++++++++++--- src/coreclr/src/vm/dllimportcallback.h | 13 ++++- src/coreclr/src/vm/i386/asmconstants.h | 6 --- src/coreclr/src/vm/i386/umthunkstub.S | 67 ++++++++++---------------- 4 files changed, 111 insertions(+), 57 deletions(-) diff --git a/src/coreclr/src/vm/dllimportcallback.cpp b/src/coreclr/src/vm/dllimportcallback.cpp index 11bad8b..fb96875 100644 --- a/src/coreclr/src/vm/dllimportcallback.cpp +++ b/src/coreclr/src/vm/dllimportcallback.cpp @@ -1385,8 +1385,11 @@ VOID UMThunkMarshInfo::RunTimeInit() MetaSig sig(pMD); ArgIterator argit(&sig); int numRegistersUsed = 0; - m_ecxArgOffset = -1; - m_edxArgOffset = -1; + + // + // m_cbStackArgSize represents the number of arg bytes for the MANAGED signature + // + m_cbStackArgSize = 0; int offs = 0; for (UINT i = 0 ; i < sig.NumFixedArgs(); i++) @@ -1396,15 +1399,12 @@ VOID UMThunkMarshInfo::RunTimeInit() int cbSize = sig.GetElemSize(type, thValueType); if (ArgIterator::IsArgumentInRegister(&numRegistersUsed, type)) { - if (numRegistersUsed == 1) - m_ecxArgOffset = offs; - else if (numRegistersUsed == 2) - m_edxArgOffset = offs; offs += STACK_ELEM_SIZE; } else { offs += StackElemSize(cbSize); + m_cbStackArgSize += StackElemSize(cbSize); } } PInvokeStaticSigInfo sigInfo; @@ -1430,6 +1430,76 @@ VOID UMThunkMarshInfo::RunTimeInit() InterlockedCompareExchangeT(&m_pILStub, pFinalILStub, (PCODE)1); } +#if defined(_TARGET_X86_) && defined(FEATURE_STUBS_AS_IL) +VOID UMThunkMarshInfo::SetupArguments(char *pSrc, ArgumentRegisters *pArgRegs, char *pDst) +{ + MethodDesc *pMD = GetMethod(); + + _ASSERTE(pMD); + + // + // x86 native uses the following stack layout: + // | saved eip | + // | --------- | <- CFA + // | stkarg 0 | + // | stkarg 1 | + // | ... | + // | stkarg N | + // + // x86 managed, however, uses a bit different stack layout: + // | saved eip | + // | --------- | <- CFA + // | stkarg M | (NATIVE/MANAGE may have different number of stack arguments) + // | ... | + // | stkarg 1 | + // | stkarg 0 | + // + // This stub bridges the gap between them. + // + char *pCurSrc = pSrc; + char *pCurDst = pDst + m_cbStackArgSize; + + MetaSig sig(pMD); + + int numRegistersUsed = 0; + + for (UINT i = 0 ; i < sig.NumFixedArgs(); i++) + { + TypeHandle thValueType; + CorElementType type = sig.NextArgNormalized(&thValueType); + int cbSize = sig.GetElemSize(type, thValueType); + int elemSize = StackElemSize(cbSize); + + if (ArgIterator::IsArgumentInRegister(&numRegistersUsed, type)) + { + _ASSERTE(elemSize == STACK_ELEM_SIZE); + + if (numRegistersUsed == 1) + pArgRegs->Ecx = *((UINT32 *)pCurSrc); + else if (numRegistersUsed == 2) + pArgRegs->Edx = *((UINT32 *)pCurSrc); + } + else + { + pCurDst -= elemSize; + memcpy(pCurDst, pCurSrc, elemSize); + } + + pCurSrc += elemSize; + } + + _ASSERTE(pDst == pCurDst); +} + +EXTERN_C VOID STDCALL UMThunkStubSetupArgumentsWorker(UMThunkMarshInfo *pMarshInfo, + char *pSrc, + UMThunkMarshInfo::ArgumentRegisters *pArgRegs, + char *pDst) +{ + pMarshInfo->SetupArguments(pSrc, pArgRegs, pDst); +} +#endif // _TARGET_X86_ && FEATURE_STUBS_AS_IL + #ifdef _DEBUG void STDCALL LogUMTransition(UMEntryThunk* thunk) { diff --git a/src/coreclr/src/vm/dllimportcallback.h b/src/coreclr/src/vm/dllimportcallback.h index c6f5788..b50b917 100644 --- a/src/coreclr/src/vm/dllimportcallback.h +++ b/src/coreclr/src/vm/dllimportcallback.h @@ -205,6 +205,16 @@ public: Stub *CompileNExportThunk(LoaderHeap *pLoaderHeap, PInvokeStaticSigInfo* pSigInfo, MetaSig *pMetaSig, BOOL fNoStub); #endif // _TARGET_X86_ && !FEATURE_STUBS_AS_IL +#if defined(_TARGET_X86_) && defined(FEATURE_STUBS_AS_IL) + struct ArgumentRegisters + { + UINT32 Ecx; + UINT32 Edx; + }; + + VOID SetupArguments(char *pSrc, ArgumentRegisters *pArgRegs, char *pDst); +#endif // _TARGET_X86_ && FEATURE_STUBS_AS_IL + private: PCODE m_pILStub; // IL stub for marshaling // On x86, NULL for no-marshal signatures @@ -213,8 +223,7 @@ private: #if defined(_TARGET_X86_) UINT16 m_cbRetPop; // stack bytes popped by callee (for UpdateRegDisplay) #if defined(FEATURE_STUBS_AS_IL) - UINT32 m_ecxArgOffset; - UINT32 m_edxArgOffset; + UINT32 m_cbStackArgSize; // stack bytes pushed for managed code #else Stub* m_pExecStub; // UMEntryThunk jumps directly here UINT16 m_callConv; // unmanaged calling convention and flags (CorPinvokeMap) diff --git a/src/coreclr/src/vm/i386/asmconstants.h b/src/coreclr/src/vm/i386/asmconstants.h index 58451e0..0a581cf 100644 --- a/src/coreclr/src/vm/i386/asmconstants.h +++ b/src/coreclr/src/vm/i386/asmconstants.h @@ -431,12 +431,6 @@ ASMCONSTANTS_C_ASSERT(UMThunkMarshInfo__m_cbActualArgSize == offsetof(UMThunkMar #ifdef FEATURE_STUBS_AS_IL #define UMThunkMarshInfo__m_cbRetPop 0x08 ASMCONSTANTS_C_ASSERT(UMThunkMarshInfo__m_cbRetPop == offsetof(UMThunkMarshInfo, m_cbRetPop)) - -#define UMThunkMarshInfo__m_ecxArgOffset 0xc -ASMCONSTANTS_C_ASSERT(UMThunkMarshInfo__m_ecxArgOffset == offsetof(UMThunkMarshInfo, m_ecxArgOffset)) - -#define UMThunkMarshInfo__m_edxArgOffset 0x10 -ASMCONSTANTS_C_ASSERT(UMThunkMarshInfo__m_edxArgOffset == offsetof(UMThunkMarshInfo, m_edxArgOffset)) #endif //FEATURE_STUBS_AS_IL #ifndef CROSSGEN_COMPILE diff --git a/src/coreclr/src/vm/i386/umthunkstub.S b/src/coreclr/src/vm/i386/umthunkstub.S index 6cefda9..7f02503 100644 --- a/src/coreclr/src/vm/i386/umthunkstub.S +++ b/src/coreclr/src/vm/i386/umthunkstub.S @@ -143,54 +143,35 @@ LOCAL_LABEL(DoTrapReturningThreadsTHROW): LOCAL_LABEL(UMThunkStub_CopyStackArgs): - // eax = m_cbActualArgSize, in bytes - // esi = src - // edi = dest - // ebx = scratch - lea esi, [ebp + 0x8] + // eax = m_cbActualArgSize (in bytes) sub esp, eax - and esp, -16 // align with 16 byte - lea edi, [esp] - - // First, we copy arguments to ecx and edx registers (if needed). - mov edx, dword ptr [ebp - UMThunkStub_UMENTRYTHUNK_OFFSET] - mov edx, dword ptr [edx + UMEntryThunk__m_pUMThunkMarshInfo] - mov ebx, dword ptr [edx + UMThunkMarshInfo__m_ecxArgOffset] - cmp ebx, -1 - je LOCAL_LABEL(InitCopyStack) - mov ecx, dword ptr [esi + ebx] - add eax, -4 - jz LOCAL_LABEL(UMThunkStub_ArgumentsSetup) - - mov ebx, dword ptr [edx + UMThunkMarshInfo__m_edxArgOffset] - cmp ebx, -1 - je LOCAL_LABEL(InitCopyStack) - mov edx, dword ptr [esi + ebx] - add eax, -4 - jz LOCAL_LABEL(UMThunkStub_ArgumentsSetup) - -LOCAL_LABEL(InitCopyStack): - push ecx + and esp, -16 // align with 16 byte + lea edi, [esp] // edi = dest + + lea esi, [ebp + 0x8] // esi = src + + // + // EXTERN_C VOID STDCALL UMThunkStubSetupArgumentsWorker(UMThunkMarshInfo *pMarshInfo, + // char *pSrc, + // UMThunkMarshInfo::ArgumentRegisters *pArgRegs, + // char *pDst) push edx - mov edx, dword ptr [ebp - UMThunkStub_UMENTRYTHUNK_OFFSET] - mov edx, dword ptr [edx + UMEntryThunk__m_pUMThunkMarshInfo] - mov ebx, [edx + UMThunkMarshInfo__m_cbActualArgSize] - add ebx, -4 -LOCAL_LABEL(CopyStack): - cmp ebx, dword ptr [edx + UMThunkMarshInfo__m_ecxArgOffset] - je LOCAL_LABEL(IncreaseOffset) - cmp ebx, dword ptr [edx + UMThunkMarshInfo__m_edxArgOffset] - je LOCAL_LABEL(IncreaseOffset) - add eax, -4 - mov ecx, dword ptr [esi + ebx] - mov dword ptr [edi + eax], ecx -LOCAL_LABEL(IncreaseOffset): - add ebx, -4 - jc LOCAL_LABEL(CopyStack) + push ecx + lea ecx, [esp] - pop edx + sub esp, 8 // Pad + push edi // pSrc + push ecx // pArgRegs + push esi // pSrc + mov ecx, dword ptr [ebp - UMThunkStub_UMENTRYTHUNK_OFFSET] + mov ecx, dword ptr [ecx + UMEntryThunk__m_pUMThunkMarshInfo] + push ecx // pMarshInfo + CHECK_STACK_ALIGNMENT + call C_FUNC(UMThunkStubSetupArgumentsWorker) + add esp, 8 pop ecx + pop edx jmp LOCAL_LABEL(UMThunkStub_ArgumentsSetup) #if _DEBUG -- 2.7.4