Teach ILStubLinker to use the managed signature for the target stack delta when the stub is a reverse stub.

Make ILStubLinker understand changing calling conventions from instance to non-instance calling conventions.

Author: jkoritzinsky

src/vm/dllimport.cpp

@@ -270,13 +270,6 @@ class ILStubState : public StubState
         STANDARD_VM_CONTRACT;
 
         pInfo->MarshalHiddenLengthArgument(&m_slIL, SF_IsForwardStub(m_dwStubFlags), isForReturnArray);
-
-        if (SF_IsReverseStub(m_dwStubFlags))
-        {
-            // Hidden length arguments appear explicitly in the native signature
-            // however, they are not in the managed signature.
-            m_slIL.AdjustTargetStackDeltaForExtraParam();
-        }
     }
 
     void MarshalFactoryReturn()
@@ -515,15 +508,6 @@ class ILStubState : public StubState
         // add the extra arg to the unmanaged signature
         LocalDesc locDescNative(ELEMENT_TYPE_I4);
         pcs->SetStubTargetArgType(&locDescNative, false);
-
-        if (SF_IsReverseStub(m_dwStubFlags))
-        {
-            // reverse the effect of SetStubTargetArgType on the stack delta
-            // (the LCID argument is explicitly passed from unmanaged but does not
-            // show up in the managed signature in any way)
-            m_slIL.AdjustTargetStackDeltaForExtraParam();
-        }
-
     }
 
     void SwapStubSignatures(MethodDesc* pStubMD)
@@ -774,17 +758,6 @@ class ILStubState : public StubState
         }
 #endif // FEATURE_COMINTEROP
 
-        if (SF_IsHRESULTSwapping(m_dwStubFlags) && SF_IsReverseStub(m_dwStubFlags))
-        {
-            m_slIL.AdjustTargetStackDeltaForReverseInteropHRESULTSwapping();
-        }
-
-        if (SF_IsForwardCOMStub(m_dwStubFlags))
-        {
-            // Compensate for the 'this' parameter.
-            m_slIL.AdjustTargetStackDeltaForExtraParam();
-        }
-
         // Don't touch target signatures from this point on otherwise it messes up the
         // cache in ILStubState::GetStubTargetMethodSig.
 
@@ -1624,13 +1597,9 @@ class COMToCLR_ILStubState : public ILStubState
 
         ILStubState::BeginEmit(dwStubFlags);
 
-        if (SF_IsWinRTStaticStub(dwStubFlags))
+        if (!SF_IsWinRTStaticStub(dwStubFlags))
         {
             // we are not loading 'this' because the target is static
-            m_slIL.AdjustTargetStackDeltaForExtraParam();
-        }
-        else
-        {
             // load this
             m_slIL.GetDispatchCodeStream()->EmitLoadThis();
         }
@@ -1659,7 +1628,6 @@ class COMToCLR_ILStubState : public ILStubState
 
         // expect one additional argument - pointer to a location that receives the created instance
         DWORD dwRetValArgNum = pcsDispatch->SetStubTargetArgType(&locDescFactoryRetVal, false);
-        m_slIL.AdjustTargetStackDeltaForExtraParam();
 
         // convert 'this' to an interface pointer corresponding to the default interface of this class
         pcsUnmarshal->EmitLoadThis();
@@ -1932,50 +1900,6 @@ ILCodeStream* NDirectStubLinker::GetExceptionCleanupCodeStream()
     return m_pcsExceptionCleanup;
 }
 
-void NDirectStubLinker::AdjustTargetStackDeltaForExtraParam()
-{
-    LIMITED_METHOD_CONTRACT;
-    //
-    // Compensate for the extra parameter.
-    //
-    m_iTargetStackDelta++;
-}
-
-void NDirectStubLinker::AdjustTargetStackDeltaForReverseInteropHRESULTSwapping()
-{
-    WRAPPER_NO_CONTRACT;
-    //
-    // In the case of reverse pinvoke, we build up the 'target'
-    // signature as if it were normal forward pinvoke and then
-    // switch that signature (representing the native sig) with
-    // the stub's sig (representing the managed sig).  However,
-    // as a side-effect, our calcualted target stack delta is 
-    // wrong.  
-    //
-    // The only way that we support a different stack delta is
-    // through hresult swapping.  So this code "undoes" the 
-    // deltas that would have been applied in that case.  
-    //
-
-    if (StubHasVoidReturnType())
-    {
-        //
-        // If the managed return type is void, undo the HRESULT 
-        // return type added to our target sig for HRESULT swapping.
-        // No extra argument will have been added because it makes
-        // no sense to add an extry byref void argument.
-        //
-        m_iTargetStackDelta--;
-    }
-    else
-    {
-        //
-        // no longer pop the extra byref argument from the stack
-        //
-        m_iTargetStackDelta++;
-    }
-}
-
 void NDirectStubLinker::SetInteropParamExceptionInfo(UINT resID, UINT paramIdx)
 {
     LIMITED_METHOD_CONTRACT;

src/vm/dllimport.h

@@ -489,8 +489,6 @@ class NDirectStubLinker : public ILStubLinker
     void    SetExceptionCleanupNeeded();
     BOOL    IsCleanupWorkListSetup();
     void    GetCleanupFinallyOffsets(ILStubEHClause * pClause);
-    void    AdjustTargetStackDeltaForReverseInteropHRESULTSwapping();
-    void    AdjustTargetStackDeltaForExtraParam();
 
     void    SetInteropParamExceptionInfo(UINT resID, UINT paramIdx);
     bool    HasInteropParamExceptionInfo();

src/vm/ilmarshalers.h

@@ -641,17 +641,6 @@ class ILMarshaler
             extraParamType.MakeByRef();
 
             m_pcsMarshal->SetStubTargetArgType(&extraParamType, false);
-            if (byrefNativeReturn && !IsCLRToNative(m_dwMarshalFlags))
-            {
-                // If doing a native->managed call and returning a structure by-ref,
-                // the native signature has an extra param for the struct return
-                // than the managed signature. Adjust the target stack delta to account this extra
-                // parameter.
-                m_pslNDirect->AdjustTargetStackDeltaForExtraParam();
-                // We also need to account for the lack of a return value in the native signature.
-                // To do this, we adjust the stack delta again for the return parameter.
-                m_pslNDirect->AdjustTargetStackDeltaForExtraParam();
-            }
 
             if (IsHresultSwap(dwMarshalFlags))
             {

src/vm/stubgen.cpp

@@ -2242,7 +2242,7 @@ ILStubLinker::ILStubLinker(Module* pStubSigModule, const Signature &signature, S
             uNativeCallingConv |= IMAGE_CEE_CS_CALLCONV_HASTHIS;
         }
 
-        if (fTargetHasThis)
+        if (fTargetHasThis && !fIsReverseStub)
         {
             m_iTargetStackDelta--;
         }
@@ -2252,7 +2252,22 @@ ILStubLinker::ILStubLinker(Module* pStubSigModule, const Signature &signature, S
         if (uStubCallingConvInfo & IMAGE_CEE_CS_CALLCONV_GENERIC)
             IfFailThrow(m_managedSigPtr.GetData(NULL));    // skip number of type parameters
 
-        IfFailThrow(m_managedSigPtr.GetData(NULL));        // skip number of parameters
+        if (fIsReverseStub)
+        {
+            ULONG numParams = 0;
+            IfFailThrow(m_managedSigPtr.GetData(&numParams));
+            // As per ECMA 335, the max number of parameters is 0x1FFFFFFF (see section 11.23.2), which fits in a 32-bit signed integer 
+            // So this cast is safe.
+            m_iTargetStackDelta -= (INT)numParams;
+            if (!m_StubHasVoidReturnType)
+            {
+                m_iTargetStackDelta++;
+            }
+        }
+        else
+        {
+            IfFailThrow(m_managedSigPtr.GetData(NULL));        // skip number of parameters
+        }
         IfFailThrow(m_managedSigPtr.SkipExactlyOne()); // skip return type
     }
 }
@@ -2475,13 +2490,15 @@ void ILStubLinker::SetStubTargetReturnType(LocalDesc* pLoc)
 
     m_nativeFnSigBuilder.SetReturnType(pLoc);
 
-    // Update check for if a stub has a void return type based on the provided return type.
-    m_StubTargetHasVoidReturnType = ((1 == pLoc->cbType) && (ELEMENT_TYPE_VOID == pLoc->ElementType[0])) ? TRUE : FALSE;
-    if (!m_StubTargetHasVoidReturnType)
+    if (!m_fIsReverseStub)
     {
-        m_iTargetStackDelta++;
+        // Update check for if a stub has a void return type based on the provided return type.
+        m_StubTargetHasVoidReturnType = ((1 == pLoc->cbType) && (ELEMENT_TYPE_VOID == pLoc->ElementType[0])) ? TRUE : FALSE;
+        if (!m_StubTargetHasVoidReturnType)
+        {
+            m_iTargetStackDelta++;
+        }
     }
-
 }
 
 DWORD ILStubLinker::SetStubTargetArgType(CorElementType typ, bool fConsumeStubArg /*= true*/)
@@ -2495,7 +2512,17 @@ DWORD ILStubLinker::SetStubTargetArgType(CorElementType typ, bool fConsumeStubAr
 void ILStubLinker::SetStubTargetCallingConv(CorCallingConvention uNativeCallingConv)
 {
     LIMITED_METHOD_CONTRACT;
+    CorCallingConvention originalCallingConvention = m_nativeFnSigBuilder.GetCallingConv();
     m_nativeFnSigBuilder.SetCallingConv(uNativeCallingConv);
+    if (!m_fIsReverseStub)
+    {
+        if ( (originalCallingConvention & CORINFO_CALLCONV_HASTHIS) && !(uNativeCallingConv & CORINFO_CALLCONV_HASTHIS))
+        {
+            // Our calling convention had an implied-this beforehand and now it doesnt.
+            // Account for this in the target stack delta.
+            m_iTargetStackDelta++;
+        }
+    }
 }
 
 static size_t GetManagedTypeForMDArray(LocalDesc* pLoc, Module* pModule, PCCOR_SIGNATURE psigManagedArg, SigTypeContext *pTypeContext)
@@ -2770,7 +2797,11 @@ ILStubLinker::SetStubTargetArgType(
     TransformArgForJIT(pLoc);
 
     DWORD dwArgNum = m_nativeFnSigBuilder.NewArg(pLoc);
-    m_iTargetStackDelta--;
+
+    if (!m_fIsReverseStub)
+    {
+        m_iTargetStackDelta--;
+    }
 
     return dwArgNum;
 } // ILStubLinker::SetStubTargetArgType