Correctly marshal structure return values in member functions on Win-x64 and Win-x86 (#23145)

* In Windows-x64, if we have a native member function signature with a struct return type, we need to do a by-ref return.

* Implement support for marshalling structure return values via a return buffer argument correctly in instance signatures on AMD64-Windows.

* Change field initialization ordering to satisfy warning.

* Try to narrow down the conditions that trigger these changes to just COM methods.

* Don't bash the return type on AMD64 Windows. Only treat it as a byref return buffer.

* PR feedback.

* Enable returning structs from COM methods via a return buffer on x86 for structs <= 8 bytes.

* Add test for struct returns with ThisCall. Extend the "struct return buffer" fix to functions marked as unmanaged thiscall since they all must be instance methods

* Don't include the return-type-bashing switch on AMD64 platforms.

* Don't do the signature swapping/copy on non-instance functions with struct returns.

* Cast the return type of GetStubTargetCallingConv to the right calling convention enum type.

* If we're doing a thiscall, marshal the "this" parameter before the return buffer (if the return buffer exists) on all platforms.

* Remove temporary logging code I added in for debugging.

* Clean up class naming.

* Try using a vtable instead of a pointer-to-member-function.

* Remove delete of class with non-virtual destructor

1 files changed, 42 insertions, 12 deletions

diff --git a/src/vm/ilmarshalers.h b/src/vm/ilmarshalers.h
index c892ae66a9..87d81504e7 100644
--- a/src/vm/ilmarshalers.h
+++ b/src/vm/ilmarshalers.h
@@ -583,7 +583,10 @@ public:
         bool byrefNativeReturn = false;
         CorElementType typ = ELEMENT_TYPE_VOID;
         UINT32 nativeSize = 0;
-
+        bool nativeMethodIsMemberFunction = (m_pslNDirect->TargetHasThis() && IsCLRToNative(m_dwMarshalFlags))
+            || (m_pslNDirect->HasThis() && !IsCLRToNative(m_dwMarshalFlags))
+            || ((CorInfoCallConv)m_pslNDirect->GetStubTargetCallingConv() == CORINFO_CALLCONV_THISCALL);
+            
         // we need to convert value type return types to primitives as
         // JIT does not inline P/Invoke calls that return structures
         if (nativeType.IsValueClass())
@@ -599,33 +602,56 @@ public:
                 nativeSize = wNativeSize;
             }
 
-#if defined(_TARGET_X86_)
+#if defined(_TARGET_X86_) || (defined(_TARGET_AMD64_) && defined(_WIN32))
             // JIT32 and JIT64 (which is only used on the Windows Desktop CLR) has a problem generating
             // code for the pinvoke ILStubs which do a return using a struct type.  Therefore, we
             // change the signature of calli to return void and make the return buffer as first argument. 
 
-            // for X86 and AMD64-Windows we bash the return type from struct to U1, U2, U4 or U8
+            // For Windows AMD64 and x86, we need to use a return buffer for native member functions returning structures.
+            // for X86 Windows non-member functions we bash the return type from struct to U1, U2, U4 or U8
             // and use byrefNativeReturn for all other structs.
             // for UNIX_X86_ABI, we always need a return buffer argument for any size of structs.
-            switch (nativeSize)
+#if defined(_WIN32)
+            if (nativeMethodIsMemberFunction)
             {
+                byrefNativeReturn = true;
+            }
+            else
+#endif // _WIN32
+            {
+#ifndef _TARGET_AMD64_
+                switch (nativeSize)
+                {
 #ifndef UNIX_X86_ABI
-                case 1: typ = ELEMENT_TYPE_U1; break;
-                case 2: typ = ELEMENT_TYPE_U2; break;
-                case 4: typ = ELEMENT_TYPE_U4; break;
-                case 8: typ = ELEMENT_TYPE_U8; break;
-#endif
-                default: byrefNativeReturn = true; break;
+                    case 1: typ = ELEMENT_TYPE_U1; break;
+                    case 2: typ = ELEMENT_TYPE_U2; break;
+                    case 4: typ = ELEMENT_TYPE_U4; break;
+                    case 8: typ = ELEMENT_TYPE_U8; break;
+#endif // UNIX_X86_ABI
+                    default: byrefNativeReturn = true; break;
+                }
+#endif // _TARGET_AMD64_
             }
-#endif
+#endif // defined(_TARGET_X86_) || (defined(_TARGET_AMD64_) && defined(_WIN32))
         }
 
-        if (IsHresultSwap(dwMarshalFlags) || (byrefNativeReturn && IsCLRToNative(dwMarshalFlags)))
+        if (IsHresultSwap(dwMarshalFlags) || (byrefNativeReturn && (IsCLRToNative(m_dwMarshalFlags) || nativeMethodIsMemberFunction)))
         {
             LocalDesc extraParamType = nativeType;
             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))
             {
@@ -742,6 +768,10 @@ public:
                 m_nativeHome.EmitCopyToByrefArgWithNullCheck(m_pcsUnmarshal, &nativeType, argidx);
                 m_pcsUnmarshal->EmitLDC(S_OK);
             }
+            else if (byrefNativeReturn && nativeMethodIsMemberFunction)
+            {
+                m_nativeHome.EmitCopyToByrefArg(m_pcsUnmarshal, &nativeType, argidx);
+            }
             else
             {
                 if (typ != ELEMENT_TYPE_VOID)