From 6b889aba05b8e1ccc9cef793a2d1293e30598453 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 29 Mar 2019 09:49:55 -0700 Subject: 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 --- src/vm/dllimport.cpp | 179 ++++++++++++--------- src/vm/dllimport.h | 5 + src/vm/ilmarshalers.h | 54 +++++-- src/vm/stubgen.cpp | 19 +-- src/vm/stubgen.h | 18 ++- tests/src/Interop/CMakeLists.txt | 2 +- .../COM/NETClients/Primitives/NumericTests.cs | 35 ++++ tests/src/Interop/COM/NETServer/NumericTesting.cs | 17 +- .../COM/NativeClients/Primitives/NumericTests.cpp | 21 +++ .../src/Interop/COM/NativeServer/NumericTesting.h | 24 +++ .../COM/ServerContracts/Server.Contracts.cs | 20 +++ .../Interop/COM/ServerContracts/Server.Contracts.h | 41 +++-- .../PInvoke/Miscellaneous/ThisCall/CMakeLists.txt | 9 ++ .../Miscellaneous/ThisCall/ThisCallNative.cpp | 37 +++++ .../PInvoke/Miscellaneous/ThisCall/ThisCallTest.cs | 62 +++++++ .../Miscellaneous/ThisCall/ThisCallTest.csproj | 32 ++++ 16 files changed, 459 insertions(+), 116 deletions(-) create mode 100644 tests/src/Interop/PInvoke/Miscellaneous/ThisCall/CMakeLists.txt create mode 100644 tests/src/Interop/PInvoke/Miscellaneous/ThisCall/ThisCallNative.cpp create mode 100644 tests/src/Interop/PInvoke/Miscellaneous/ThisCall/ThisCallTest.cs create mode 100644 tests/src/Interop/PInvoke/Miscellaneous/ThisCall/ThisCallTest.csproj diff --git a/src/vm/dllimport.cpp b/src/vm/dllimport.cpp index b200a1bccb..c83f05ce05 100644 --- a/src/vm/dllimport.cpp +++ b/src/vm/dllimport.cpp @@ -1737,7 +1737,7 @@ NDirectStubLinker::NDirectStubLinker( int iLCIDParamIdx, BOOL fTargetHasThis, BOOL fStubHasThis) - : ILStubLinker(pModule, signature, pTypeContext, pTargetMD, fTargetHasThis, fStubHasThis, !SF_IsCOMStub(dwStubFlags)), + : ILStubLinker(pModule, signature, pTypeContext, pTargetMD, fTargetHasThis, fStubHasThis, !SF_IsCOMStub(dwStubFlags), SF_IsReverseStub(dwStubFlags)), m_pCleanupFinallyBeginLabel(NULL), m_pCleanupFinallyEndLabel(NULL), m_pSkipExceptionCleanupLabel(NULL), @@ -1747,6 +1747,7 @@ NDirectStubLinker::NDirectStubLinker( m_fHasCleanupCode(FALSE), m_fHasExceptionCleanupCode(FALSE), m_fCleanupWorkListIsSetup(FALSE), + m_targetHasThis(fTargetHasThis), m_dwThreadLocalNum(-1), m_dwCleanupWorkListLocalNum(-1), m_dwRetValLocalNum(-1), @@ -3661,42 +3662,19 @@ static MarshalInfo::MarshalType DoMarshalReturnValue(MetaSig& msig, if (SF_IsCOMStub(dwStubFlags)) { - if (marshalType == MarshalInfo::MARSHAL_TYPE_VALUECLASS || - marshalType == MarshalInfo::MARSHAL_TYPE_BLITTABLEVALUECLASS || + // We don't support native methods that return VARIANTs, non-blittable structs, GUIDs, or DECIMALs directly. + if (marshalType == MarshalInfo::MARSHAL_TYPE_OBJECT || + marshalType == MarshalInfo::MARSHAL_TYPE_VALUECLASS || marshalType == MarshalInfo::MARSHAL_TYPE_GUID || marshalType == MarshalInfo::MARSHAL_TYPE_DECIMAL) { -#ifndef _TARGET_X86_ - // We cannot optimize marshalType to MARSHAL_TYPE_GENERIC_* because the JIT works with exact types - // and would refuse to compile the stub if it implicitly converted between scalars and value types (also see - // code:MarshalInfo.MarhalInfo where we do the optimization on x86). We want to throw only if the structure - // is too big to be returned in registers. - if (marshalType != MarshalInfo::MARSHAL_TYPE_BLITTABLEVALUECLASS || - IsUnmanagedValueTypeReturnedByRef(returnInfo.GetNativeArgSize())) -#endif // _TARGET_X86_ + if (!SF_IsHRESULTSwapping(dwStubFlags) && !SF_IsCOMLateBoundStub(dwStubFlags)) { - if (!SF_IsHRESULTSwapping(dwStubFlags) && !SF_IsCOMLateBoundStub(dwStubFlags)) - { - // Note that this limitation is very likely not needed anymore and could be lifted if we care. - COMPlusThrow(kMarshalDirectiveException, IDS_EE_COM_UNSUPPORTED_SIG); - } + COMPlusThrow(kMarshalDirectiveException, IDS_EE_COM_UNSUPPORTED_SIG); } - - pss->MarshalReturn(&returnInfo, argOffset); } - else - { - // We don't support native methods that return VARIANTs directly. - if (marshalType == MarshalInfo::MARSHAL_TYPE_OBJECT) - { - if (!SF_IsHRESULTSwapping(dwStubFlags) && !SF_IsCOMLateBoundStub(dwStubFlags)) - { - COMPlusThrow(kMarshalDirectiveException, IDS_EE_COM_UNSUPPORTED_SIG); - } - } - pss->MarshalReturn(&returnInfo, argOffset); - } + pss->MarshalReturn(&returnInfo, argOffset); } else #endif // FEATURE_COMINTEROP @@ -3953,14 +3931,21 @@ static void CreateNDirectStubWorker(StubState* pss, // Normally we would like this to be false so that we use the correct signature // in the IL_STUB, (i.e if it returns a value class then the signature will use that) // When this bool is true we change the return type to void and explicitly add a - // return buffer argument as the first argument. - BOOL fMarshalReturnValueFirst = false; + // return buffer argument as the first argument so as to match the native calling convention correctly. + BOOL fMarshalReturnValueFirst = FALSE; + + BOOL fReverseWithReturnBufferArg = FALSE; // We can only change fMarshalReturnValueFirst to true when we are NOT doing HRESULT-swapping! - // + // When we are HRESULT-swapping, the managed return type is actually the type of the last parameter and not the return type. + // The native return type of an HRESULT-swapped function is an HRESULT, which never uses a return-buffer argument. + // Since the managed return type is actually the last parameter, we need to marshal it after the last parameter in the managed signature + // to make sure we match the native signature correctly (when marshalling parameters, we add them to the native stub signature). if (!SF_IsHRESULTSwapping(dwStubFlags)) { - + bool isInstanceMethod = fStubNeedsCOM || fThisCall; + // We cannot just use pSig.GetReturnType() here since it will return ELEMENT_TYPE_VALUETYPE for enums. + bool isReturnTypeValueType = msig.GetRetTypeHandleThrowing().GetVerifierCorElementType() == ELEMENT_TYPE_VALUETYPE; #if defined(_TARGET_X86_) || defined(_TARGET_ARM_) // JIT32 has problems in generating code for pinvoke ILStubs which do a return in return buffer. // Therefore instead we change the signature of calli to return void and make the return buffer as first @@ -3972,55 +3957,18 @@ static void CreateNDirectStubWorker(StubState* pss, #ifdef UNIX_X86_ABI // For functions with value type class, managed and unmanaged calling convention differ fMarshalReturnValueFirst = HasRetBuffArgUnmanagedFixup(&msig); -#else // UNIX_X86_ABI +#elif defined(_TARGET_ARM_) fMarshalReturnValueFirst = HasRetBuffArg(&msig); +#else + // On Windows-X86, the native signature might need a return buffer when the managed doesn't (specifically when the native signature is a member function). + fMarshalReturnValueFirst = HasRetBuffArg(&msig) || (isInstanceMethod && isReturnTypeValueType); #endif // UNIX_X86_ABI - +#elif defined(_TARGET_AMD64_) + fMarshalReturnValueFirst = isInstanceMethod && isReturnTypeValueType; #endif // defined(_TARGET_X86_) || defined(_TARGET_ARM_) - - } - - if (fMarshalReturnValueFirst) - { - marshalType = DoMarshalReturnValue(msig, - pParamTokenArray, - nlType, - nlFlags, - 0, - pss, - fThisCall, - argOffset, - dwStubFlags, - pMD, - nativeStackSize, - fStubNeedsCOM, - 0 - DEBUG_ARG(pSigDesc->m_pDebugName) - DEBUG_ARG(pSigDesc->m_pDebugClassName) - ); - - if (marshalType == MarshalInfo::MARSHAL_TYPE_DATE || - marshalType == MarshalInfo::MARSHAL_TYPE_CURRENCY || - marshalType == MarshalInfo::MARSHAL_TYPE_ARRAYWITHOFFSET || - marshalType == MarshalInfo::MARSHAL_TYPE_HANDLEREF || - marshalType == MarshalInfo::MARSHAL_TYPE_ARGITERATOR -#ifdef FEATURE_COMINTEROP - || marshalType == MarshalInfo::MARSHAL_TYPE_OLECOLOR -#endif // FEATURE_COMINTEROP - ) - { - // These are special non-blittable types returned by-ref in managed, - // but marshaled as primitive values returned by-value in unmanaged. - } - else - { - // This is an ordinary value type - see if it is returned by-ref. - MethodTable *pRetMT = msig.GetRetTypeHandleThrowing().AsMethodTable(); - if (IsUnmanagedValueTypeReturnedByRef(pRetMT->GetNativeSize())) - { - nativeStackSize += sizeof(LPVOID); - } - } +#ifdef _WIN32 + fReverseWithReturnBufferArg = fMarshalReturnValueFirst && SF_IsReverseStub(dwStubFlags); +#endif } // @@ -4088,6 +4036,77 @@ static void CreateNDirectStubWorker(StubState* pss, // Marshal the parameters int argidx = 1; int nativeArgIndex = 0; + + // If we are generating a return buffer on a member function that is marked as thiscall (as opposed to being a COM method) + // then we need to marshal the this parameter first and the return buffer second. + // We don't need to do this for COM methods because the "this" is implied as argument 0 by the signature of the stub. + if (fThisCall && fMarshalReturnValueFirst) + { + msig.NextArg(); + + MarshalInfo &info = pParamMarshalInfo[argidx - 1]; + pss->MarshalArgument(&info, argOffset, GetStackOffsetFromStackSize(nativeStackSize, fThisCall)); + nativeStackSize += info.GetNativeArgSize(); + + fStubNeedsCOM |= info.MarshalerRequiresCOM(); + + // make sure that the first parameter is enregisterable + if (info.GetNativeArgSize() > sizeof(SLOT)) + COMPlusThrow(kMarshalDirectiveException, IDS_EE_NDIRECT_BADNATL_THISCALL); + + argidx++; + } + + // If we're doing a native->managed call and are generating a return buffer, + // we need to move all of the actual arguments over one and have the return value be the first argument (after the this pointer if applicable). + if (fReverseWithReturnBufferArg) + { + ++argOffset; + } + + if (fMarshalReturnValueFirst) + { + marshalType = DoMarshalReturnValue(msig, + pParamTokenArray, + nlType, + nlFlags, + 0, + pss, + fThisCall, + argOffset, + dwStubFlags, + pMD, + nativeStackSize, + fStubNeedsCOM, + 0 + DEBUG_ARG(pSigDesc->m_pDebugName) + DEBUG_ARG(pSigDesc->m_pDebugClassName) + ); + + if (marshalType == MarshalInfo::MARSHAL_TYPE_DATE || + marshalType == MarshalInfo::MARSHAL_TYPE_CURRENCY || + marshalType == MarshalInfo::MARSHAL_TYPE_ARRAYWITHOFFSET || + marshalType == MarshalInfo::MARSHAL_TYPE_HANDLEREF || + marshalType == MarshalInfo::MARSHAL_TYPE_ARGITERATOR +#ifdef FEATURE_COMINTEROP + || marshalType == MarshalInfo::MARSHAL_TYPE_OLECOLOR +#endif // FEATURE_COMINTEROP + ) + { + // These are special non-blittable types returned by-ref in managed, + // but marshaled as primitive values returned by-value in unmanaged. + } + else + { + // This is an ordinary value type - see if it is returned by-ref. + MethodTable *pRetMT = msig.GetRetTypeHandleThrowing().AsMethodTable(); + if (IsUnmanagedValueTypeReturnedByRef(pRetMT->GetNativeSize())) + { + nativeStackSize += sizeof(LPVOID); + } + } + } + while (argidx <= numArgs) { #ifdef FEATURE_COMINTEROP diff --git a/src/vm/dllimport.h b/src/vm/dllimport.h index b88d339a84..a6967ebfb4 100644 --- a/src/vm/dllimport.h +++ b/src/vm/dllimport.h @@ -494,6 +494,10 @@ public: void SetInteropParamExceptionInfo(UINT resID, UINT paramIdx); bool HasInteropParamExceptionInfo(); + bool TargetHasThis() + { + return m_targetHasThis == TRUE; + } void ClearCode(); @@ -556,6 +560,7 @@ protected: BOOL m_fHasCleanupCode; BOOL m_fHasExceptionCleanupCode; BOOL m_fCleanupWorkListIsSetup; + BOOL m_targetHasThis; DWORD m_dwThreadLocalNum; // managed-to-native only DWORD m_dwArgMarshalIndexLocalNum; DWORD m_dwCleanupWorkListLocalNum; 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) diff --git a/src/vm/stubgen.cpp b/src/vm/stubgen.cpp index 67a6ca03e9..78c6831b71 100644 --- a/src/vm/stubgen.cpp +++ b/src/vm/stubgen.cpp @@ -1530,7 +1530,7 @@ void ILCodeStream::EmitPOP() void ILCodeStream::EmitRET() { WRAPPER_NO_CONTRACT; - INT16 iStackDelta = m_pOwner->m_StubHasVoidReturnType ? 0 : -1; + INT16 iStackDelta = m_pOwner->ReturnOpcodePopsStack() ? -1 : 0; Emit(CEE_RET, iStackDelta, 0); } void ILCodeStream::EmitSHR_UN() @@ -1684,11 +1684,6 @@ void ILCodeStream::EmitCALL(BinderMethodID id, int numInArgs, int numRetArgs) EmitCALL(GetToken(MscorlibBinder::GetMethod(id)), numInArgs, numRetArgs); } - - - - - void ILStubLinker::SetHasThis (bool fHasThis) { LIMITED_METHOD_CONTRACT; @@ -2156,14 +2151,15 @@ static BOOL SigHasVoidReturnType(const Signature &signature) ILStubLinker::ILStubLinker(Module* pStubSigModule, const Signature &signature, SigTypeContext *pTypeContext, MethodDesc *pMD, - BOOL fTargetHasThis, BOOL fStubHasThis, BOOL fIsNDirectStub) : + BOOL fTargetHasThis, BOOL fStubHasThis, BOOL fIsNDirectStub, BOOL fIsReverseStub) : m_pCodeStreamList(NULL), m_stubSig(signature), m_pTypeContext(pTypeContext), m_pCode(NULL), m_pStubSigModule(pStubSigModule), m_pLabelList(NULL), - m_StubHasVoidReturnType(0), + m_StubHasVoidReturnType(FALSE), + m_fIsReverseStub(fIsReverseStub), m_iTargetStackDelta(0), m_cbCurrentCompressedSigLen(1), m_nLocals(0), @@ -2181,7 +2177,9 @@ ILStubLinker::ILStubLinker(Module* pStubSigModule, const Signature &signature, S m_managedSigPtr = signature.CreateSigPointer(); if (!signature.IsEmpty()) { + // Until told otherwise, assume that the stub has the same return type as the signature. m_StubHasVoidReturnType = SigHasVoidReturnType(signature); + m_StubTargetHasVoidReturnType = m_StubHasVoidReturnType; // // Get the stub's calling convention. Set m_fHasThis to match @@ -2477,10 +2475,13 @@ void ILStubLinker::SetStubTargetReturnType(LocalDesc* pLoc) m_nativeFnSigBuilder.SetReturnType(pLoc); - if ((1 != pLoc->cbType) || (ELEMENT_TYPE_VOID != pLoc->ElementType[0])) + // 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*/) diff --git a/src/vm/stubgen.h b/src/vm/stubgen.h index 044029b0af..73d72708a7 100644 --- a/src/vm/stubgen.h +++ b/src/vm/stubgen.h @@ -110,11 +110,11 @@ struct LocalDesc bool lastElementTypeIsValueType = false; - if (ElementType[0] == ELEMENT_TYPE_VALUETYPE) + if (ElementType[cbType - 1] == ELEMENT_TYPE_VALUETYPE) { lastElementTypeIsValueType = true; } - else if ((ElementType[0] == ELEMENT_TYPE_INTERNAL) && + else if ((ElementType[cbType - 1] == ELEMENT_TYPE_INTERNAL) && (InternalToken.IsNativeValueType() || InternalToken.GetMethodTable()->IsValueType())) { @@ -378,7 +378,7 @@ class ILStubLinker public: ILStubLinker(Module* pModule, const Signature &signature, SigTypeContext *pTypeContext, MethodDesc *pMD, - BOOL fTargetHasThis, BOOL fStubHasThis, BOOL fIsNDirectStub = FALSE); + BOOL fTargetHasThis, BOOL fStubHasThis, BOOL fIsNDirectStub = FALSE, BOOL fIsReverseStub = FALSE); ~ILStubLinker(); void GenerateCode(BYTE* pbBuffer, size_t cbBufferSize); @@ -424,7 +424,6 @@ public: void GetStubReturnType(LocalDesc * pLoc); void GetStubReturnType(LocalDesc * pLoc, Module * pModule); CorCallingConvention GetStubTargetCallingConv(); - CorElementType GetStubTargetReturnElementType() { WRAPPER_NO_CONTRACT; return m_nativeFnSigBuilder.GetReturnElementType(); } @@ -504,12 +503,23 @@ protected: void SetStubTargetReturnType(LocalDesc* pLoc); void SetStubTargetCallingConv(CorCallingConvention uNativeCallingConv); + bool ReturnOpcodePopsStack() + { + if ((!m_fIsReverseStub && m_StubHasVoidReturnType) || (m_fIsReverseStub && m_StubTargetHasVoidReturnType)) + { + return false; + } + return true; + } + void TransformArgForJIT(LocalDesc *pLoc); Module * GetStubSigModule(); SigTypeContext *GetStubSigTypeContext(); BOOL m_StubHasVoidReturnType; + BOOL m_StubTargetHasVoidReturnType; + BOOL m_fIsReverseStub; INT m_iTargetStackDelta; DWORD m_cbCurrentCompressedSigLen; DWORD m_nLocals; diff --git a/tests/src/Interop/CMakeLists.txt b/tests/src/Interop/CMakeLists.txt index acf4c849db..99bd9937da 100644 --- a/tests/src/Interop/CMakeLists.txt +++ b/tests/src/Interop/CMakeLists.txt @@ -29,6 +29,7 @@ add_subdirectory(PInvoke/SizeParamIndex/ReversePInvoke/PassingByRef) add_subdirectory(PInvoke/Array/MarshalArrayAsField/LPArrayNative) add_subdirectory(PInvoke/Array/MarshalArrayAsParam/LPArrayNative) add_subdirectory(PInvoke/Miscellaneous/HandleRef) +add_subdirectory(PInvoke/Miscellaneous/ThisCall) add_subdirectory(PInvoke/Miscellaneous/MultipleAssembliesWithSamePInvoke) add_subdirectory(PInvoke/ExactSpelling) add_subdirectory(PInvoke/CriticalHandles) @@ -65,7 +66,6 @@ add_subdirectory(DllImportAttribute/Simple) add_subdirectory(ExecInDefAppDom) add_subdirectory(ICustomMarshaler/ConflictingNames) add_subdirectory(LayoutClass) - if(WIN32) add_subdirectory(PInvoke/Attributes/LCID) add_subdirectory(PInvoke/Variant) diff --git a/tests/src/Interop/COM/NETClients/Primitives/NumericTests.cs b/tests/src/Interop/COM/NETClients/Primitives/NumericTests.cs index 19d0574063..1eaa2fdde3 100644 --- a/tests/src/Interop/COM/NETClients/Primitives/NumericTests.cs +++ b/tests/src/Interop/COM/NETClients/Primitives/NumericTests.cs @@ -38,6 +38,7 @@ namespace NetClient this.Marshal_Float(a / 100f, b / 100f); this.Marshal_Double(a / 100.0, b / 100.0); this.Marshal_ManyInts(); + this.Marshal_Struct_Return(); } static private bool EqualByBound(float expected, float actual) @@ -200,5 +201,39 @@ namespace NetClient Console.WriteLine($"{expected.GetType().Name} 12 test invariant: 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 + 11 + 12 = {expected}"); Assert.IsTrue(expected == this.server.Add_ManyInts12(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12)); } + + private void Marshal_Struct_Return() + { + Console.WriteLine("Struct return from member function marshalling with struct > 4 bytes"); + { + var width = 1.0f; + var height = 2.0f; + Server.Contract.SizeF result = this.server.MakeSize(width, height); + + Assert.AreEqual(width, result.width); + Assert.AreEqual(height, result.height); + } + Console.WriteLine("Struct return from member function marshalling with struct <= 4 bytes"); + { + byte width = 1; + byte height = 2; + Server.Contract.Size result = this.server.MakeSizeSmall(width, height); + + Assert.AreEqual(width, result.width); + Assert.AreEqual(height, result.height); + } + Console.WriteLine("Struct return from member function marshalling with struct > 8 bytes"); + { + var x = 1.0f; + var y = 2.0f; + var z = 3.0f; + var w = 4.0f; + Server.Contract.HFA_4 result = this.server.MakeHFA(x, y ,z, w); + Assert.AreEqual(x, result.x); + Assert.AreEqual(y, result.y); + Assert.AreEqual(z, result.z); + Assert.AreEqual(w, result.w); + } + } } } diff --git a/tests/src/Interop/COM/NETServer/NumericTesting.cs b/tests/src/Interop/COM/NETServer/NumericTesting.cs index 4ed713de53..64ca47d318 100644 --- a/tests/src/Interop/COM/NETServer/NumericTesting.cs +++ b/tests/src/Interop/COM/NETServer/NumericTesting.cs @@ -200,4 +200,19 @@ public class NumericTesting : Server.Contract.INumericTesting { return i1 + i2 + i3 + i4 + i5 + i6 + i7 + i8 + i9 + i10 + i11 + i12; } -} \ No newline at end of file + + public Server.Contract.SizeF MakeSize(float width, float height) + { + return new Server.Contract.SizeF { width = width, height = height }; + } + + public Server.Contract.Size MakeSizeSmall(byte width, byte height) + { + return new Server.Contract.Size { width = width, height = height }; + } + + public Server.Contract.HFA_4 MakeHFA(float x, float y, float z, float w) + { + return new Server.Contract.HFA_4 {x = x, y = y, z = z, w = w}; + } +} diff --git a/tests/src/Interop/COM/NativeClients/Primitives/NumericTests.cpp b/tests/src/Interop/COM/NativeClients/Primitives/NumericTests.cpp index c951c8c527..a11bb77020 100644 --- a/tests/src/Interop/COM/NativeClients/Primitives/NumericTests.cpp +++ b/tests/src/Interop/COM/NativeClients/Primitives/NumericTests.cpp @@ -216,6 +216,26 @@ namespace THROW_IF_FAILED(numericTesting->Add_ManyInts12(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, &result)); THROW_FAIL_IF_FALSE(result == expected); } + + void MarshalStructReturn(_In_ INumericTesting* numericTesting) + { + { + ::printf("Marshal struct return type with size > 4 bytes\n"); + float width = 1.0f; + float height = 2.0f; + SizeF size = numericTesting->MakeSize(width, height); + THROW_FAIL_IF_FALSE(width == size.width); + THROW_FAIL_IF_FALSE(height == size.height); + } + { + ::printf("Marshal struct return type with size < 4 bytes\n"); + BYTE width = 1; + BYTE height = 2; + Size size = numericTesting->MakeSizeSmall(width, height); + THROW_FAIL_IF_FALSE(width == size.width); + THROW_FAIL_IF_FALSE(height == size.height); + } + } } void Run_NumericTests() @@ -245,4 +265,5 @@ void Run_NumericTests() MarshalFloat(numericTesting, (float)a / 100.f, (float)b / 100.f); MarshalDouble(numericTesting, (double)a / 100.0, (double)b / 100.0); MarshalManyInts(numericTesting); + MarshalStructReturn(numericTesting); } diff --git a/tests/src/Interop/COM/NativeServer/NumericTesting.h b/tests/src/Interop/COM/NativeServer/NumericTesting.h index d30427aa3d..95ec2dbc18 100644 --- a/tests/src/Interop/COM/NativeServer/NumericTesting.h +++ b/tests/src/Interop/COM/NativeServer/NumericTesting.h @@ -283,6 +283,30 @@ public: return S_OK; } + virtual COM_DECLSPEC_NOTHROW SizeF STDMETHODCALLTYPE MakeSize( + /*[in]*/ float width, + /*[in]*/ float height) + { + return { width, height }; + } + + virtual COM_DECLSPEC_NOTHROW Size STDMETHODCALLTYPE MakeSizeSmall( + /*[in]*/ BYTE width, + /*[in]*/ BYTE height) + { + return { width, height }; + } + + virtual COM_DECLSPEC_NOTHROW HFA_4 STDMETHODCALLTYPE MakeHFA( + /*[in]*/ float x, + /*[in]*/ float y, + /*[in]*/ float z, + /*[in]*/ float w + ) + { + return { x, y, z, w }; + } + public: // IUnknown STDMETHOD(QueryInterface)( /* [in] */ REFIID riid, diff --git a/tests/src/Interop/COM/ServerContracts/Server.Contracts.cs b/tests/src/Interop/COM/ServerContracts/Server.Contracts.cs index 9c2e680ed7..7c198c7138 100644 --- a/tests/src/Interop/COM/ServerContracts/Server.Contracts.cs +++ b/tests/src/Interop/COM/ServerContracts/Server.Contracts.cs @@ -11,6 +11,18 @@ namespace Server.Contract using System.Runtime.InteropServices; using System.Text; + public struct SizeF + { + public float width; + public float height; + } + + public struct Size + { + public byte width; + public byte height; + } + [ComVisible(true)] [Guid("05655A94-A915-4926-815D-A9EA648BAAD9")] [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] @@ -48,6 +60,14 @@ namespace Server.Contract int Add_ManyInts11(int i1, int i2, int i3, int i4, int i5, int i6, int i7, int i8, int i9, int i10, int i11); int Add_ManyInts12(int i1, int i2, int i3, int i4, int i5, int i6, int i7, int i8, int i9, int i10, int i11, int i12); + + [PreserveSig] + SizeF MakeSize(float width, float height); + [PreserveSig] + Size MakeSizeSmall(byte width, byte height); + + [PreserveSig] + HFA_4 MakeHFA(float x, float y, float z, float w); } [ComVisible(true)] diff --git a/tests/src/Interop/COM/ServerContracts/Server.Contracts.h b/tests/src/Interop/COM/ServerContracts/Server.Contracts.h index b985cd3547..a3629c140c 100644 --- a/tests/src/Interop/COM/ServerContracts/Server.Contracts.h +++ b/tests/src/Interop/COM/ServerContracts/Server.Contracts.h @@ -1,10 +1,30 @@ -// Created by Microsoft (R) C/C++ Compiler +// Created by Microsoft (R) C/C++ Compiler #pragma once #pragma pack(push, 8) #include +struct SizeF +{ + float width; + float height; +}; + +struct Size +{ + BYTE width; + BYTE height; +}; + +struct HFA_4 +{ + float x; + float y; + float z; + float w; +}; + struct __declspec(uuid("05655a94-a915-4926-815d-a9ea648baad9")) INumericTesting : IUnknown { @@ -143,6 +163,17 @@ INumericTesting : IUnknown /*[in]*/ int i11, /*[in]*/ int i12, /*[out]*/ int * result ) = 0; + virtual COM_DECLSPEC_NOTHROW SizeF STDMETHODCALLTYPE MakeSize( + /*[in]*/ float width, + /*[in]*/ float height) = 0; + virtual COM_DECLSPEC_NOTHROW Size STDMETHODCALLTYPE MakeSizeSmall( + /*[in]*/ BYTE width, + /*[in]*/ BYTE height) = 0; + virtual COM_DECLSPEC_NOTHROW HFA_4 STDMETHODCALLTYPE MakeHFA( + /*[in]*/ float x, + /*[in]*/ float y, + /*[in]*/ float z, + /*[in]*/ float w) = 0; }; struct __declspec(uuid("7731cb31-e063-4cc8-bcd2-d151d6bc8f43")) @@ -365,14 +396,6 @@ enum IDispatchTesting_Exception IDispatchTesting_Exception_HResult, }; -struct HFA_4 -{ - float x; - float y; - float z; - float w; -}; - struct __declspec(uuid("a5e04c1c-474e-46d2-bbc0-769d04e12b54")) IDispatchTesting : IDispatch { diff --git a/tests/src/Interop/PInvoke/Miscellaneous/ThisCall/CMakeLists.txt b/tests/src/Interop/PInvoke/Miscellaneous/ThisCall/CMakeLists.txt new file mode 100644 index 0000000000..a8908b2143 --- /dev/null +++ b/tests/src/Interop/PInvoke/Miscellaneous/ThisCall/CMakeLists.txt @@ -0,0 +1,9 @@ + +cmake_minimum_required (VERSION 2.6) +project (ThisCallNative) +include ("${CLR_INTEROP_TEST_ROOT}/Interop.cmake") +set(SOURCES + ThisCallNative.cpp +) +add_library (ThisCallNative SHARED ${SOURCES}) +install (TARGETS ThisCallNative DESTINATION bin) diff --git a/tests/src/Interop/PInvoke/Miscellaneous/ThisCall/ThisCallNative.cpp b/tests/src/Interop/PInvoke/Miscellaneous/ThisCall/ThisCallNative.cpp new file mode 100644 index 0000000000..9f1d56ccb9 --- /dev/null +++ b/tests/src/Interop/PInvoke/Miscellaneous/ThisCall/ThisCallNative.cpp @@ -0,0 +1,37 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +#include +#include +#include + +struct SizeF +{ + float width; + float height; +}; + +class C +{ + int dummy = 0xcccccccc; + float width; + float height; + +public: + C(float width, float height) + :width(width), + height(height) + {} + + virtual SizeF GetSize() + { + return {width, height}; + } +}; + + +extern "C" DLL_EXPORT C* STDMETHODCALLTYPE CreateInstanceOfC(float width, float height) +{ + return new C(width, height); +} diff --git a/tests/src/Interop/PInvoke/Miscellaneous/ThisCall/ThisCallTest.cs b/tests/src/Interop/PInvoke/Miscellaneous/ThisCall/ThisCallTest.cs new file mode 100644 index 0000000000..ea7b7dacc3 --- /dev/null +++ b/tests/src/Interop/PInvoke/Miscellaneous/ThisCall/ThisCallTest.cs @@ -0,0 +1,62 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Runtime.InteropServices; +using System; +using System.Reflection; +using System.Text; +using TestLibrary; + +unsafe class ThisCallNative +{ + public struct C + { + public struct VtableLayout + { + public IntPtr getSize; + } + + public VtableLayout* vtable; + private int c; + private float width; + private float height; + } + + public struct SizeF + { + public float width; + public float height; + } + + [UnmanagedFunctionPointer(CallingConvention.ThisCall)] + public delegate SizeF GetSizeFn(C* c); + + [DllImport(nameof(ThisCallNative))] + public static extern C* CreateInstanceOfC(float width, float height); +} + +class ThisCallTest +{ + public unsafe static int Main(string[] args) + { + try + { + float width = 1.0f; + float height = 2.0f; + ThisCallNative.C* instance = ThisCallNative.CreateInstanceOfC(width, height); + ThisCallNative.GetSizeFn callback = Marshal.GetDelegateForFunctionPointer(instance->vtable->getSize); + + ThisCallNative.SizeF result = callback(instance); + + Assert.AreEqual(width, result.width); + Assert.AreEqual(height, result.height); + } + catch (System.Exception ex) + { + Console.WriteLine(ex); + return 101; + } + return 100; + } +} diff --git a/tests/src/Interop/PInvoke/Miscellaneous/ThisCall/ThisCallTest.csproj b/tests/src/Interop/PInvoke/Miscellaneous/ThisCall/ThisCallTest.csproj new file mode 100644 index 0000000000..f457f14c49 --- /dev/null +++ b/tests/src/Interop/PInvoke/Miscellaneous/ThisCall/ThisCallTest.csproj @@ -0,0 +1,32 @@ + + + + + Debug + AnyCPU + ThisCallTest + 2.0 + {F1E66554-8C8E-4141-85CF-D0CD6A0CD0B0} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\..\ + $(DefineConstants);STATIC + true + + + + + + + False + + + + + + + + + + + -- cgit v1.2.3