Reorder stack arguments on reverse P/Invoke (dotnet/coreclr#10217)

Commit migrated from https://github.com/dotnet/coreclr/commit/e55c985fb2c833b332581680f050bf7ebaa98f5f

diff --git a/src/coreclr/src/vm/dllimportcallback.cpp b/src/coreclr/src/vm/dllimportcallback.cpp
index 11bad8b..fb96875 100644 (file)
--- 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<PCODE>(&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 (file)
--- 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 (file)
--- 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 (file)
--- 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