diff --git a/src/coreclr/src/vm/dllimport.cpp b/src/coreclr/src/vm/dllimport.cpp index 924a50fe9d5dd..93d0f79ca0984 100644 --- a/src/coreclr/src/vm/dllimport.cpp +++ b/src/coreclr/src/vm/dllimport.cpp @@ -3956,7 +3956,10 @@ static void CreateNDirectStubWorker(StubState* pss, BOOL fMarshalReturnValueFirst = FALSE; BOOL fReverseWithReturnBufferArg = FALSE; - bool isInstanceMethod = fStubNeedsCOM || fThisCall; + // Only consider ThisCall methods to be instance methods. + // Techinically COM methods are also instance methods, but we don't want to change the behavior of the built-in + // COM abi work because there are many users that rely on the current behavior (for example WPF). + bool isInstanceMethod = fThisCall; // 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. diff --git a/src/coreclr/src/vm/ilmarshalers.h b/src/coreclr/src/vm/ilmarshalers.h index 866b3ec63cb7a..750ac3de202de 100644 --- a/src/coreclr/src/vm/ilmarshalers.h +++ b/src/coreclr/src/vm/ilmarshalers.h @@ -583,9 +583,7 @@ class ILMarshaler 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); + bool nativeMethodIsMemberFunction = (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 diff --git a/src/coreclr/tests/src/Interop/COM/NETClients/Primitives/ErrorTests.cs b/src/coreclr/tests/src/Interop/COM/NETClients/Primitives/ErrorTests.cs index 93eba8ef72e5a..ed2fa713c307f 100644 --- a/src/coreclr/tests/src/Interop/COM/NETClients/Primitives/ErrorTests.cs +++ b/src/coreclr/tests/src/Interop/COM/NETClients/Primitives/ErrorTests.cs @@ -54,6 +54,7 @@ private void VerifyReturnHResult() foreach (var hr in hrs) { Assert.AreEqual(hr, this.server.Return_As_HResult(hr)); + Assert.AreEqual(hr, this.server.Return_As_HResult_Struct(hr).hr); } } } diff --git a/src/coreclr/tests/src/Interop/COM/NETClients/Primitives/NumericTests.cs b/src/coreclr/tests/src/Interop/COM/NETClients/Primitives/NumericTests.cs index 1eaa2fdde309c..19d0574063a0f 100644 --- a/src/coreclr/tests/src/Interop/COM/NETClients/Primitives/NumericTests.cs +++ b/src/coreclr/tests/src/Interop/COM/NETClients/Primitives/NumericTests.cs @@ -38,7 +38,6 @@ public void Run() 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) @@ -201,39 +200,5 @@ private void Marshal_ManyInts() 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/src/coreclr/tests/src/Interop/COM/NETServer/ErrorMarshalTesting.cs b/src/coreclr/tests/src/Interop/COM/NETServer/ErrorMarshalTesting.cs index 6bd104f5d4948..e0fe38e2149b6 100644 --- a/src/coreclr/tests/src/Interop/COM/NETServer/ErrorMarshalTesting.cs +++ b/src/coreclr/tests/src/Interop/COM/NETServer/ErrorMarshalTesting.cs @@ -25,4 +25,10 @@ public int Return_As_HResult(int hresultToReturn) { return hresultToReturn; } -} \ No newline at end of file + + [PreserveSig] + public Server.Contract.HResult Return_As_HResult_Struct(int hresultToReturn) + { + return new Server.Contract.HResult { hr = hresultToReturn }; + } +} diff --git a/src/coreclr/tests/src/Interop/COM/NETServer/NumericTesting.cs b/src/coreclr/tests/src/Interop/COM/NETServer/NumericTesting.cs index 64ca47d318912..36edab46f44f8 100644 --- a/src/coreclr/tests/src/Interop/COM/NETServer/NumericTesting.cs +++ b/src/coreclr/tests/src/Interop/COM/NETServer/NumericTesting.cs @@ -200,19 +200,4 @@ public int Add_ManyInts12(int i1, int i2, int i3, int i4, int i5, int i6, int i7 { return i1 + i2 + i3 + i4 + i5 + i6 + i7 + i8 + i9 + i10 + i11 + i12; } - - 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/src/coreclr/tests/src/Interop/COM/NativeClients/Primitives/ErrorTests.cpp b/src/coreclr/tests/src/Interop/COM/NativeClients/Primitives/ErrorTests.cpp index 1708aca2e1edc..18606a6280857 100644 --- a/src/coreclr/tests/src/Interop/COM/NativeClients/Primitives/ErrorTests.cpp +++ b/src/coreclr/tests/src/Interop/COM/NativeClients/Primitives/ErrorTests.cpp @@ -52,6 +52,30 @@ namespace THROW_FAIL_IF_FALSE(hr == hrMaybe); } } + + void VerifyReturnHResultStruct(_In_ IErrorMarshalTesting *et) + { + ::printf("Verify preserved function signature\n"); + + HRESULT hrs[] = + { + E_NOTIMPL, + E_POINTER, + E_ACCESSDENIED, + E_INVALIDARG, + E_UNEXPECTED, + HRESULT{-1}, + S_FALSE, + HRESULT{2} + }; + + for (int i = 0; i < ARRAYSIZE(hrs); ++i) + { + HRESULT hr = hrs[i]; + HRESULT hrMaybe = et->Return_As_HResult_Struct(hr); + THROW_FAIL_IF_FALSE(hr == hrMaybe); + } + } } void Run_ErrorTests() @@ -65,4 +89,5 @@ void Run_ErrorTests() VerifyExpectedException(errorMarshal); VerifyReturnHResult(errorMarshal); + VerifyReturnHResultStruct(errorMarshal); } diff --git a/src/coreclr/tests/src/Interop/COM/NativeClients/Primitives/NumericTests.cpp b/src/coreclr/tests/src/Interop/COM/NativeClients/Primitives/NumericTests.cpp index a11bb77020df8..c951c8c52765c 100644 --- a/src/coreclr/tests/src/Interop/COM/NativeClients/Primitives/NumericTests.cpp +++ b/src/coreclr/tests/src/Interop/COM/NativeClients/Primitives/NumericTests.cpp @@ -216,26 +216,6 @@ 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() @@ -265,5 +245,4 @@ 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/src/coreclr/tests/src/Interop/COM/NativeServer/ErrorMarshalTesting.h b/src/coreclr/tests/src/Interop/COM/NativeServer/ErrorMarshalTesting.h index c28fa0f64582e..6675044101e90 100644 --- a/src/coreclr/tests/src/Interop/COM/NativeServer/ErrorMarshalTesting.h +++ b/src/coreclr/tests/src/Interop/COM/NativeServer/ErrorMarshalTesting.h @@ -21,6 +21,12 @@ class ErrorMarshalTesting : public UnknownImpl, public IErrorMarshalTesting return hresultToReturn; } + int STDMETHODCALLTYPE Return_As_HResult_Struct( + /*[in]*/ int hresultToReturn) + { + return hresultToReturn; + } + public: // IUnknown STDMETHOD(QueryInterface)( /* [in] */ REFIID riid, diff --git a/src/coreclr/tests/src/Interop/COM/NativeServer/NumericTesting.h b/src/coreclr/tests/src/Interop/COM/NativeServer/NumericTesting.h index 95ec2dbc185af..d30427aa3d9a3 100644 --- a/src/coreclr/tests/src/Interop/COM/NativeServer/NumericTesting.h +++ b/src/coreclr/tests/src/Interop/COM/NativeServer/NumericTesting.h @@ -283,30 +283,6 @@ class NumericTesting : public UnknownImpl, public INumericTesting 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/src/coreclr/tests/src/Interop/COM/ServerContracts/Server.Contracts.cs b/src/coreclr/tests/src/Interop/COM/ServerContracts/Server.Contracts.cs index f32518485cf41..f16007f38e040 100644 --- a/src/coreclr/tests/src/Interop/COM/ServerContracts/Server.Contracts.cs +++ b/src/coreclr/tests/src/Interop/COM/ServerContracts/Server.Contracts.cs @@ -11,18 +11,6 @@ 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)] @@ -60,14 +48,6 @@ public interface INumericTesting 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)] @@ -198,6 +178,11 @@ string Add_BStr( void Reverse_BStr_OutAttr([MarshalAs(UnmanagedType.BStr)] string a, [Out][MarshalAs(UnmanagedType.BStr)] string b); } + public struct HResult + { + public int hr; + } + [ComVisible(true)] [Guid("592386A5-6837-444D-9DE3-250815D18556")] [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] @@ -207,6 +192,9 @@ public interface IErrorMarshalTesting [PreserveSig] int Return_As_HResult(int hresultToReturn); + + [PreserveSig] + HResult Return_As_HResult_Struct(int hresultToReturn); } public enum IDispatchTesting_Exception diff --git a/src/coreclr/tests/src/Interop/COM/ServerContracts/Server.Contracts.h b/src/coreclr/tests/src/Interop/COM/ServerContracts/Server.Contracts.h index a3629c140cdc1..86e68ed3af311 100644 --- a/src/coreclr/tests/src/Interop/COM/ServerContracts/Server.Contracts.h +++ b/src/coreclr/tests/src/Interop/COM/ServerContracts/Server.Contracts.h @@ -5,18 +5,6 @@ #include -struct SizeF -{ - float width; - float height; -}; - -struct Size -{ - BYTE width; - BYTE height; -}; - struct HFA_4 { float x; @@ -163,17 +151,6 @@ 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")) @@ -388,6 +365,8 @@ IErrorMarshalTesting : IUnknown /*[in]*/ int hresultToReturn ) = 0; virtual int STDMETHODCALLTYPE Return_As_HResult ( /*[in]*/ int hresultToReturn ) = 0; + virtual int STDMETHODCALLTYPE Return_As_HResult_Struct ( + /*[in]*/ int hresultToReturn ) = 0; }; enum IDispatchTesting_Exception