[release/8.0-staging] Call the Copy Constructor for stack arguments i…

…n C++/CLI on x86 (#100221)

* Call the Copy Constructor for stack arguments in C++/CLI on Windows-x86 (#100050)

* Add repro test case

* Directly load the argument address using ldarga to avoid making a copy

* Reimplement the "Copy Constructor Cookie" logic in a more modern and maintainable style to get the test passing again

* Narrow support to Windows only

---------

Co-authored-by: Jeremy Koritzinsky <[email protected]>

src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs

@@ -1111,6 +1111,75 @@ public IntPtr AddRef()
         }
     }  // class CleanupWorkListElement
 
+    internal unsafe struct CopyConstructorCookie
+    {
+        private void* m_source;
+
+        private nuint m_destinationOffset;
+
+        public delegate*<void*, void*, void> m_copyConstructor;
+
+        public delegate*<void*, void> m_destructor;
+
+        public CopyConstructorCookie* m_next;
+
+        [StackTraceHidden]
+        public void ExecuteCopy(void* destinationBase)
+        {
+            if (m_copyConstructor != null)
+            {
+                m_copyConstructor((byte*)destinationBase + m_destinationOffset, m_source);
+            }
+
+            if (m_destructor != null)
+            {
+                m_destructor(m_source);
+            }
+        }
+    }
+
+    internal unsafe struct CopyConstructorChain
+    {
+        public void* m_realTarget;
+        public CopyConstructorCookie* m_head;
+
+        public void Add(CopyConstructorCookie* cookie)
+        {
+            cookie->m_next = m_head;
+            m_head = cookie;
+        }
+
+        [ThreadStatic]
+        private static CopyConstructorChain s_copyConstructorChain;
+
+        public void Install(void* realTarget)
+        {
+            m_realTarget = realTarget;
+            s_copyConstructorChain = this;
+        }
+
+        [StackTraceHidden]
+        private void ExecuteCopies(void* destinationBase)
+        {
+            for (CopyConstructorCookie* current = m_head; current != null; current = current->m_next)
+            {
+                current->ExecuteCopy(destinationBase);
+            }
+        }
+
+        [UnmanagedCallersOnly]
+        [StackTraceHidden]
+        public static void* ExecuteCurrentCopiesAndGetTarget(void* destinationBase)
+        {
+            void* target = s_copyConstructorChain.m_realTarget;
+            s_copyConstructorChain.ExecuteCopies(destinationBase);
+            // Reset this instance to ensure we don't accidentally execute the copies again.
+            // All of the pointers point to the stack, so we don't need to free any memory.
+            s_copyConstructorChain = default;
+            return target;
+        }
+    }
+
     internal static class StubHelpers
     {
         [MethodImpl(MethodImplOptions.InternalCall)]

src/coreclr/vm/corelib.h

@@ -1068,6 +1068,21 @@ DEFINE_METHOD(HANDLE_MARSHALER,          CONVERT_SAFEHANDLE_TO_NATIVE,ConvertSaf
 DEFINE_METHOD(HANDLE_MARSHALER,          THROW_SAFEHANDLE_FIELD_CHANGED, ThrowSafeHandleFieldChanged, SM_RetVoid)
 DEFINE_METHOD(HANDLE_MARSHALER,          THROW_CRITICALHANDLE_FIELD_CHANGED, ThrowCriticalHandleFieldChanged, SM_RetVoid)
 
+#ifdef TARGET_WINDOWS
+#ifdef TARGET_X86
+DEFINE_CLASS(COPY_CONSTRUCTOR_CHAIN, StubHelpers,                 CopyConstructorChain)
+DEFINE_METHOD(COPY_CONSTRUCTOR_CHAIN, EXECUTE_CURRENT_COPIES_AND_GET_TARGET, ExecuteCurrentCopiesAndGetTarget, SM_PtrVoid_RetPtrVoid)
+DEFINE_METHOD(COPY_CONSTRUCTOR_CHAIN, INSTALL,                    Install,               IM_PtrVoid_RetVoid)
+DEFINE_METHOD(COPY_CONSTRUCTOR_CHAIN, ADD,                        Add,                   IM_PtrCopyConstructorCookie_RetVoid)
+
+DEFINE_CLASS(COPY_CONSTRUCTOR_COOKIE, StubHelpers, CopyConstructorCookie)
+DEFINE_FIELD(COPY_CONSTRUCTOR_COOKIE, SOURCE, m_source)
+DEFINE_FIELD(COPY_CONSTRUCTOR_COOKIE, DESTINATION_OFFSET, m_destinationOffset)
+DEFINE_FIELD(COPY_CONSTRUCTOR_COOKIE, COPY_CONSTRUCTOR, m_copyConstructor)
+DEFINE_FIELD(COPY_CONSTRUCTOR_COOKIE, DESTRUCTOR, m_destructor)
+#endif // TARGET_X86
+#endif // TARGET_WINDOWS
+
 DEFINE_CLASS(NATIVEVARIANT,         StubHelpers,            NativeVariant)
 DEFINE_CLASS(NATIVEDECIMAL,         StubHelpers,            NativeDecimal)
 

src/coreclr/vm/dllimport.cpp

@@ -1623,6 +1623,10 @@ NDirectStubLinker::NDirectStubLinker(
         m_pcsSetup->EmitSTLOC(m_dwTargetInterfacePointerLocalNum);
     }
 #endif // FEATURE_COMINTEROP
+
+#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+    m_dwCopyCtorChainLocalNum = (DWORD)-1;
+#endif // defined(TARGET_X86) && defined(TARGET_WINDOWS)
 }
 
 void NDirectStubLinker::SetCallingConvention(CorInfoCallConvExtension unmngCallConv, BOOL fIsVarArg)
@@ -1835,6 +1839,23 @@ DWORD NDirectStubLinker::GetReturnValueLocalNum()
     return m_dwRetValLocalNum;
 }
 
+#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+DWORD NDirectStubLinker::GetCopyCtorChainLocalNum()
+{
+    STANDARD_VM_CONTRACT;
+
+    if (m_dwCopyCtorChainLocalNum == (DWORD)-1)
+    {
+        // The local is created and initialized lazily when first asked.
+        m_dwCopyCtorChainLocalNum = NewLocal(CoreLibBinder::GetClass(CLASS__COPY_CONSTRUCTOR_CHAIN));
+        m_pcsSetup->EmitLDLOCA(m_dwCopyCtorChainLocalNum);
+        m_pcsSetup->EmitINITOBJ(m_pcsSetup->GetToken(CoreLibBinder::GetClass(CLASS__COPY_CONSTRUCTOR_CHAIN)));
+    }
+
+    return m_dwCopyCtorChainLocalNum;
+}
+#endif // defined(TARGET_X86) && defined(TARGET_WINDOWS)
+
 BOOL NDirectStubLinker::IsCleanupNeeded()
 {
     LIMITED_METHOD_CONTRACT;
@@ -2064,6 +2085,10 @@ void NDirectStubLinker::End(DWORD dwStubFlags)
     }
 }
 
+#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+EXTERN_C void STDCALL CopyConstructorCallStub(void);
+#endif // defined(TARGET_X86) && defined(TARGET_WINDOWS)
+
 void NDirectStubLinker::DoNDirect(ILCodeStream *pcsEmit, DWORD dwStubFlags, MethodDesc * pStubMD)
 {
     STANDARD_VM_CONTRACT;
@@ -2150,6 +2175,21 @@ void NDirectStubLinker::DoNDirect(ILCodeStream *pcsEmit, DWORD dwStubFlags, Meth
         }
     }
 
+#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+    if (m_dwCopyCtorChainLocalNum != (DWORD)-1)
+    {
+        // If we have a copy constructor chain local, we need to call the copy constructor stub
+        // to ensure that the chain is called correctly.
+        // Let's install the stub chain here and redirect the call to the stub.
+        DWORD targetLoc = NewLocal(ELEMENT_TYPE_I);
+        pcsEmit->EmitSTLOC(targetLoc);
+        pcsEmit->EmitLDLOCA(m_dwCopyCtorChainLocalNum);
+        pcsEmit->EmitLDLOC(targetLoc);
+        pcsEmit->EmitCALL(METHOD__COPY_CONSTRUCTOR_CHAIN__INSTALL, 2, 0);
+        pcsEmit->EmitLDC((DWORD_PTR)&CopyConstructorCallStub);
+    }
+#endif // defined(TARGET_X86) && defined(TARGET_WINDOWS)
+
     // For managed-to-native calls, the rest of the work is done by the JIT. It will
     // erect InlinedCallFrame, flip GC mode, and use the specified calling convention
     // to call the target. For native-to-managed calls, this is an ordinary managed
@@ -6078,5 +6118,21 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD)
     RETURN pVASigCookie->pNDirectILStub;
 }
 
+#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+// Copy constructor support for C++/CLI
+EXTERN_C void* STDCALL CallCopyConstructorsWorker(void* esp)
+{
+    STATIC_CONTRACT_THROWS;
+    STATIC_CONTRACT_GC_TRIGGERS;
+    STATIC_CONTRACT_MODE_PREEMPTIVE; // we've already switched to preemptive
+
+    using ExecuteCallback = void*(STDMETHODCALLTYPE*)(void*);
+
+    MethodDesc* pMD = CoreLibBinder::GetMethod(METHOD__COPY_CONSTRUCTOR_CHAIN__EXECUTE_CURRENT_COPIES_AND_GET_TARGET);
+    ExecuteCallback pExecute = (ExecuteCallback)pMD->GetMultiCallableAddrOfCode();
+
+    return pExecute(esp);
+}
+#endif // defined(TARGET_X86) && defined(TARGET_WINDOWS)
 
 #endif // #ifndef DACCESS_COMPILE

src/coreclr/vm/dllimport.h

@@ -484,6 +484,9 @@ class NDirectStubLinker : public ILStubLinker
     DWORD   GetCleanupWorkListLocalNum();
     DWORD   GetThreadLocalNum();
     DWORD   GetReturnValueLocalNum();
+#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+    DWORD   GetCopyCtorChainLocalNum();
+#endif // defined(TARGET_X86) && defined(TARGET_WINDOWS)
     void    SetCleanupNeeded();
     void    SetExceptionCleanupNeeded();
     BOOL    IsCleanupWorkListSetup();
@@ -553,6 +556,10 @@ class NDirectStubLinker : public ILStubLinker
     DWORD               m_dwTargetEntryPointLocalNum;
 #endif // FEATURE_COMINTEROP
 
+#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+    DWORD               m_dwCopyCtorChainLocalNum;
+#endif // defined(TARGET_X86) && defined(TARGET_WINDOWS)
+
     BOOL                m_fHasCleanupCode;
     BOOL                m_fHasExceptionCleanupCode;
     BOOL                m_fCleanupWorkListIsSetup;

src/coreclr/vm/i386/asmhelpers.asm

@@ -41,6 +41,7 @@ EXTERN _NDirectImportWorker@4:PROC
 
 EXTERN _VarargPInvokeStubWorker@12:PROC
 EXTERN _GenericPInvokeCalliStubWorker@12:PROC
+EXTERN _CallCopyConstructorsWorker@4:PROC
 
 EXTERN _PreStubWorker@8:PROC
 EXTERN _TheUMEntryPrestubWorker@4:PROC
@@ -1062,6 +1063,29 @@ GoCallCalliWorker:
 
 _GenericPInvokeCalliHelper@0 endp
 
+;==========================================================================
+; This is small stub whose purpose is to record current stack pointer and
+; call CallCopyConstructorsWorker to invoke copy constructors and destructors
+; as appropriate. This stub operates on arguments already pushed to the
+; stack by JITted IL stub and must not create a new frame, i.e. it must tail
+; call to the target for it to see the arguments that copy ctors have been
+; called on.
+;
+_CopyConstructorCallStub@0 proc public
+    ; there may be an argument in ecx - save it
+    push    ecx
+
+    ; push pointer to arguments
+    lea     edx, [esp + 8]
+    push    edx
+
+    call    _CallCopyConstructorsWorker@4
+
+    ; restore ecx and tail call to the target
+    pop     ecx
+    jmp     eax
+_CopyConstructorCallStub@0 endp
+
 ifdef FEATURE_COMINTEROP
 
 ;==========================================================================

src/coreclr/vm/ilmarshalers.cpp

@@ -3459,6 +3459,38 @@ MarshalerOverrideStatus ILBlittableValueClassWithCopyCtorMarshaler::ArgumentOver
 #ifdef TARGET_X86
         pslIL->SetStubTargetArgType(&locDesc);              // native type is the value type
         pslILDispatch->EmitLDLOC(dwNewValueTypeLocal);      // we load the local directly
+
+#if defined(TARGET_WINDOWS)
+        // Record this argument's stack slot in the copy constructor chain so we can correctly invoke the copy constructor.
+        DWORD ctorCookie = pslIL->NewLocal(CoreLibBinder::GetClass(CLASS__COPY_CONSTRUCTOR_COOKIE));
+        pslIL->EmitLDLOCA(ctorCookie);
+        pslIL->EmitINITOBJ(pslIL->GetToken(CoreLibBinder::GetClass(CLASS__COPY_CONSTRUCTOR_COOKIE)));
+        pslIL->EmitLDLOCA(ctorCookie);
+        pslIL->EmitLDLOCA(dwNewValueTypeLocal);
+        pslIL->EmitSTFLD(pslIL->GetToken(CoreLibBinder::GetField(FIELD__COPY_CONSTRUCTOR_COOKIE__SOURCE)));
+        pslIL->EmitLDLOCA(ctorCookie);
+        pslIL->EmitLDC(nativeStackOffset);
+        pslIL->EmitSTFLD(pslIL->GetToken(CoreLibBinder::GetField(FIELD__COPY_CONSTRUCTOR_COOKIE__DESTINATION_OFFSET)));
+
+        if (pargs->mm.m_pCopyCtor)
+        {
+            pslIL->EmitLDLOCA(ctorCookie);
+            pslIL->EmitLDFTN(pslIL->GetToken(pargs->mm.m_pCopyCtor));
+            pslIL->EmitSTFLD(pslIL->GetToken(CoreLibBinder::GetField(FIELD__COPY_CONSTRUCTOR_COOKIE__COPY_CONSTRUCTOR)));
+        }
+
+        if (pargs->mm.m_pDtor)
+        {
+            pslIL->EmitLDLOCA(ctorCookie);
+            pslIL->EmitLDFTN(pslIL->GetToken(pargs->mm.m_pDtor));
+            pslIL->EmitSTFLD(pslIL->GetToken(CoreLibBinder::GetField(FIELD__COPY_CONSTRUCTOR_COOKIE__DESTRUCTOR)));
+        }
+
+        pslIL->EmitLDLOCA(psl->GetCopyCtorChainLocalNum());
+        pslIL->EmitLDLOCA(ctorCookie);
+        pslIL->EmitCALL(METHOD__COPY_CONSTRUCTOR_CHAIN__ADD, 2, 0);
+#endif // defined(TARGET_WINDOWS)
+
 #else
         pslIL->SetStubTargetArgType(ELEMENT_TYPE_I);        // native type is a pointer
         EmitLoadNativeLocalAddrForByRefDispatch(pslILDispatch, dwNewValueTypeLocal);
@@ -3477,9 +3509,13 @@ MarshalerOverrideStatus ILBlittableValueClassWithCopyCtorMarshaler::ArgumentOver
 
         DWORD       dwNewValueTypeLocal;
         dwNewValueTypeLocal = pslIL->NewLocal(locDesc);
+#ifdef TARGET_WINDOWS
+        pslILDispatch->EmitLDARGA(argidx);
+#else // !TARGET_WINDOWS
         pslILDispatch->EmitLDARG(argidx);
         pslILDispatch->EmitSTLOC(dwNewValueTypeLocal);
         pslILDispatch->EmitLDLOCA(dwNewValueTypeLocal);
+#endif // TARGET_WINDOWS
 #else
         LocalDesc   locDesc(pargs->mm.m_pMT);
         locDesc.MakeCopyConstructedPointer();

src/coreclr/vm/metasig.h

@@ -578,6 +578,13 @@ DEFINE_METASIG_T(SM(RefCleanupWorkListElement_RetVoid, r(C(CLEANUP_WORK_LIST_ELE
 DEFINE_METASIG_T(SM(RefCleanupWorkListElement_SafeHandle_RetIntPtr, r(C(CLEANUP_WORK_LIST_ELEMENT)) C(SAFE_HANDLE), I))
 DEFINE_METASIG_T(SM(RefCleanupWorkListElement_Obj_RetVoid, r(C(CLEANUP_WORK_LIST_ELEMENT)) j, v))
 
+DEFINE_METASIG(SM(PtrVoid_RetPtrVoid, P(v), P(v)))
+DEFINE_METASIG(IM(PtrVoid_RetVoid, P(v), v))
+#if defined(TARGET_X86) && defined(TARGET_WINDOWS)
+DEFINE_METASIG_T(IM(PtrCopyConstructorCookie_RetVoid, P(g(COPY_CONSTRUCTOR_COOKIE)), v))
+#endif // defined(TARGET_X86) && defined(TARGET_WINDOWS)
+
+
 #ifdef FEATURE_ICASTABLE
 DEFINE_METASIG_T(SM(ICastable_RtType_RefException_RetBool, C(ICASTABLE) C(CLASS) r(C(EXCEPTION)), F))
 DEFINE_METASIG_T(SM(ICastable_RtType_RetRtType, C(ICASTABLE) C(CLASS), C(CLASS)))

src/tests/Interop/IJW/CopyConstructorMarshaler/CopyConstructorMarshaler.cs

@@ -25,34 +25,62 @@ static int Main()
                 object testInstance = Activator.CreateInstance(testType);
                 MethodInfo testMethod = testType.GetMethod("PInvokeNumCopies");
 
+                // On x86, we have an additional copy on every P/Invoke from the "native" parameter to the actual location on the stack.
+                int platformExtra = 0;
+                if (RuntimeInformation.ProcessArchitecture == Architecture.X86)
+                {
+                    platformExtra = 1;
+                }
+
                 // PInvoke will copy twice. Once from argument to parameter, and once from the managed to native parameter.
-                Assert.Equal(2, (int)testMethod.Invoke(testInstance, null));
+                Assert.Equal(2 + platformExtra, (int)testMethod.Invoke(testInstance, null));
 
                 testMethod = testType.GetMethod("ReversePInvokeNumCopies");
 
                 // Reverse PInvoke will copy 3 times. Two are from the same paths as the PInvoke,
                 // and the third is from the reverse P/Invoke call.
-                Assert.Equal(3, (int)testMethod.Invoke(testInstance, null));
+                Assert.Equal(3 + platformExtra, (int)testMethod.Invoke(testInstance, null));
 
                 testMethod = testType.GetMethod("PInvokeNumCopiesDerivedType");
 
                 // PInvoke will copy twice. Once from argument to parameter, and once from the managed to native parameter.
-                Assert.Equal(2, (int)testMethod.Invoke(testInstance, null));
+                Assert.Equal(2 + platformExtra, (int)testMethod.Invoke(testInstance, null));
 
                 testMethod = testType.GetMethod("ReversePInvokeNumCopiesDerivedType");
 
                 // Reverse PInvoke will copy 3 times. Two are from the same paths as the PInvoke,
                 // and the third is from the reverse P/Invoke call.
-                Assert.Equal(3, (int)testMethod.Invoke(testInstance, null));
+                Assert.Equal(3 + platformExtra, (int)testMethod.Invoke(testInstance, null));
             }
             catch (Exception ex)
             {
                 Console.WriteLine(ex);
                 return 101;
             }
+
+            try
+            {
+                CopyConstructorsInArgumentStackSlots();
+            }
+            catch (Exception ex)
+            {
+                Console.WriteLine(ex);
+                return 101;
+            }
+
             return 100;
         }
 
+        public static void CopyConstructorsInArgumentStackSlots()
+        {
+            Assembly ijwNativeDll = Assembly.Load("IjwCopyConstructorMarshaler");
+            Type testType = ijwNativeDll.GetType("TestClass");
+            object testInstance = Activator.CreateInstance(testType);
+            MethodInfo testMethod = testType.GetMethod("ExposedThisCopyConstructorScenario");
+
+            Assert.Equal(0, (int)testMethod.Invoke(testInstance, null));
+        }
+
         [DllImport("kernel32.dll")]
         static extern IntPtr LoadLibraryEx(string lpFileName, IntPtr hReservedNull, int dwFlags);