Opt COM methods out of the new Windows instance-method handling. (#23974)
authorJeremy Koritzinsky <jkoritzinsky@gmail.com>
Mon, 15 Apr 2019 22:54:51 +0000 (15:54 -0700)
committerGitHub <noreply@github.com>
Mon, 15 Apr 2019 22:54:51 +0000 (15:54 -0700)
* 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.

12 files changed:
src/vm/dllimport.cpp
src/vm/ilmarshalers.h
tests/src/Interop/COM/NETClients/Primitives/ErrorTests.cs
tests/src/Interop/COM/NETClients/Primitives/NumericTests.cs
tests/src/Interop/COM/NETServer/ErrorMarshalTesting.cs
tests/src/Interop/COM/NETServer/NumericTesting.cs
tests/src/Interop/COM/NativeClients/Primitives/ErrorTests.cpp
tests/src/Interop/COM/NativeClients/Primitives/NumericTests.cpp
tests/src/Interop/COM/NativeServer/ErrorMarshalTesting.h
tests/src/Interop/COM/NativeServer/NumericTesting.h
tests/src/Interop/COM/ServerContracts/Server.Contracts.cs
tests/src/Interop/COM/ServerContracts/Server.Contracts.h

index 924a50f..93d0f79 100644 (file)
@@ -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.
index 866b3ec..750ac3d 100644 (file)
@@ -583,9 +583,7 @@ 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);
+        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
index 93eba8e..ed2fa71 100644 (file)
@@ -54,6 +54,7 @@ namespace NetClient
             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);
             }
         }
     }
index 1eaa2fd..19d0574 100644 (file)
@@ -38,7 +38,6 @@ 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)
@@ -201,39 +200,5 @@ 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);
-            }
-        }
     }
 }
index 6bd104f..e0fe38e 100644 (file)
@@ -25,4 +25,10 @@ public class ErrorMarshalTesting : Server.Contract.IErrorMarshalTesting
     {
         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 };
+    }
+}
index 64ca47d..36edab4 100644 (file)
@@ -200,19 +200,4 @@ public class NumericTesting : Server.Contract.INumericTesting
     {
         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};
-    }
 }
index 1708aca..18606a6 100644 (file)
@@ -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);
 }
index a11bb77..c951c8c 100644 (file)
@@ -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);
 }
index c28fa0f..6675044 100644 (file)
@@ -21,6 +21,12 @@ public: // IErrorMarshalTesting
         return hresultToReturn;
     }
 
+    int STDMETHODCALLTYPE Return_As_HResult_Struct(
+        /*[in]*/ int hresultToReturn)
+    {
+        return hresultToReturn;
+    }
+
 public: // IUnknown
     STDMETHOD(QueryInterface)(
         /* [in] */ REFIID riid,
index 95ec2db..d30427a 100644 (file)
@@ -283,30 +283,6 @@ 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,
index f325184..f16007f 100644 (file)
@@ -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 @@ 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)]
@@ -198,6 +178,11 @@ namespace Server.Contract
         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 @@ namespace Server.Contract
 
         [PreserveSig]
         int Return_As_HResult(int hresultToReturn);
+
+        [PreserveSig]
+        HResult Return_As_HResult_Struct(int hresultToReturn);
     }
 
     public enum IDispatchTesting_Exception
index a3629c1..86e68ed 100644 (file)
@@ -5,18 +5,6 @@
 
 #include <comdef.h>
 
-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