Skip to content

Commit

Permalink
Opt COM methods out of the new Windows instance-method handling. (dot…
Browse files Browse the repository at this point in the history 
…net#23974)

* Opt COM methods out of the new Windows instance-method handling.

* Add test for an HResult "struct" returned from a COM method.

* Update ErrorMarshalTesting.cs

* Update "is member function" check on the ilmarshalers.h side to only consider thiscall.
  • Loading branch information
@jkoritzinsky
jkoritzinsky authored Apr 15, 2019
1 parent 77a5f9c commit 9f9dcdd
Show file tree
Hide file tree
Showing 12 changed files with 54 additions and 143 deletions.
5 changes: 4 additions & 1 deletion src/vm/dllimport.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 1 addition & 3 deletions src/vm/ilmarshalers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions tests/src/Interop/COM/NETClients/Primitives/ErrorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
35 changes: 0 additions & 35 deletions tests/src/Interop/COM/NETClients/Primitives/NumericTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}
}
}
}
8 changes: 7 additions & 1 deletion tests/src/Interop/COM/NETServer/ErrorMarshalTesting.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,10 @@ public int Return_As_HResult(int hresultToReturn)
{
return hresultToReturn;
}
}

[PreserveSig]
public Server.Contract.HResult Return_As_HResult_Struct(int hresultToReturn)
{
return new Server.Contract.HResult { hr = hresultToReturn };
}
}
15 changes: 0 additions & 15 deletions tests/src/Interop/COM/NETServer/NumericTesting.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
}
}
25 changes: 25 additions & 0 deletions tests/src/Interop/COM/NativeClients/Primitives/ErrorTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -65,4 +89,5 @@ void Run_ErrorTests()

VerifyExpectedException(errorMarshal);
VerifyReturnHResult(errorMarshal);
VerifyReturnHResultStruct(errorMarshal);
}
21 changes: 0 additions & 21 deletions tests/src/Interop/COM/NativeClients/Primitives/NumericTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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);
}
6 changes: 6 additions & 0 deletions tests/src/Interop/COM/NativeServer/ErrorMarshalTesting.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
24 changes: 0 additions & 24 deletions tests/src/Interop/COM/NativeServer/NumericTesting.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
28 changes: 8 additions & 20 deletions tests/src/Interop/COM/ServerContracts/Server.Contracts.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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)]
Expand All @@ -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
Expand Down
25 changes: 2 additions & 23 deletions tests/src/Interop/COM/ServerContracts/Server.Contracts.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,6 @@

#include <comdef.h>

struct SizeF
{
float width;
float height;
};

struct Size
{
BYTE width;
BYTE height;
};

struct HFA_4
{
float x;
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 9f9dcdd

Please sign in to comment.