Fix all runtime IL generation paths from passing GC types to unmanaged function calls...
authorAaron Robinson <arobins@microsoft.com>
Sat, 18 Apr 2020 03:50:39 +0000 (20:50 -0700)
committerGitHub <noreply@github.com>
Sat, 18 Apr 2020 03:50:39 +0000 (03:50 +0000)
* Native COM clients have not been running since we switched over to SDK projects.

* Stop IL generators from passing GC types to unmanaged function calls.
Update crossgen2 IL stub generators.
Update Dynamic runtime stub generator.
Add assert for GC types in unmanaged function calls.

src/coreclr/src/jit/importer.cpp
src/coreclr/src/tools/Common/TypeSystem/Interop/IL/Marshaller.cs
src/coreclr/src/vm/dllimport.cpp
src/coreclr/src/vm/ilmarshalers.cpp
src/coreclr/src/vm/ilmarshalers.h
src/coreclr/tests/src/Interop/COM/NativeClients/DefaultInterfaces.csproj
src/coreclr/tests/src/Interop/COM/NativeClients/Dispatch.csproj
src/coreclr/tests/src/Interop/COM/NativeClients/Licensing.csproj
src/coreclr/tests/src/Interop/COM/NativeClients/Primitives.csproj
src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ComRuntimeHelpers.cs

index ae7a100949909975fd570f7f8aca95e56cf24995..8bf1a7b4b9adf06cf6d3973a19dde8b4da6ea747 100644 (file)
@@ -6741,9 +6741,16 @@ void Compiler::impPopArgsForUnmanagedCall(GenTree* call, CORINFO_SIG_INFO* sig)
         assert(thisPtr->TypeGet() == TYP_I_IMPL || thisPtr->TypeGet() == TYP_BYREF);
     }
 
-    for (GenTreeCall::Use& use : GenTreeCall::UseList(args))
+    for (GenTreeCall::Use& argUse : GenTreeCall::UseList(args))
     {
-        call->gtFlags |= use.GetNode()->gtFlags & GTF_GLOB_EFFECT;
+        // We should not be passing gc typed args to an unmanaged call.
+        GenTree* arg = argUse.GetNode();
+        if (varTypeIsGC(arg->TypeGet()))
+        {
+            assert(!"*** invalid IL: gc type passed to unmanaged call");
+        }
+
+        call->gtFlags |= arg->gtFlags & GTF_GLOB_EFFECT;
     }
 }
 
index db6e71fc11645e612513892bae4d329907cfa815..50bbebb49f08b1922ae5b77953e3e8af907a8820 100644 (file)
@@ -597,9 +597,14 @@ namespace Internal.TypeSystem.Interop
         protected void LoadNativeArg(ILCodeStream stream)
         {
             if (IsNativeByRef)
+            {
                 _nativeHome.LoadAddr(stream);
+                stream.Emit(ILOpcode.conv_i);
+            }
             else
+            {
                 _nativeHome.LoadValue(stream);
+            }
         }
 
         protected void LoadNativeAddr(ILCodeStream stream)
index 0046ed957c3906976f4560dfc49708ca68fe592d..b2f1ee1f94e8d19a5f44e9da9433d149ad391940 100644 (file)
@@ -410,12 +410,14 @@ public:
             locDescInnerPtr.MakeByRef();
             pcsDispatch->SetStubTargetArgType(&locDescInnerPtr, false);
             pcsDispatch->EmitLDLOCA(dwInnerIInspectableLocalNum);
+            pcsDispatch->EmitCONV_I();
         }
 
         // pass pointer to the local to the factory method (last argument)
         locDescFactoryRetVal.MakeByRef();
         pcsDispatch->SetStubTargetArgType(&locDescFactoryRetVal, false);
         pcsDispatch->EmitLDLOCA(dwFactoryRetValLocalNum);
+        pcsDispatch->EmitCONV_I();
 
         /*
         *   UNMARSHAL
index 8ec77a0bd28cc884b6cca9a1eb452d23345e3ea1..a84d6ca21d5f896e6d583657f436569d4010d9f3 100644 (file)
@@ -2907,7 +2907,7 @@ MarshalerOverrideStatus ILSafeHandleMarshaler::ArgumentOverride(NDirectStubLinke
             }
 
             // Leave the address of the native handle local as the argument to the native method.
-            pslILDispatch->EmitLDLOCA(dwNativeHandleLocal);
+            EmitLoadNativeLocalAddrForByRefDispatch(pslILDispatch, dwNativeHandleLocal);
 
             // On the output side we only backpropagate the native handle into the output SafeHandle and the output SafeHandle
             // to the caller if the native handle actually changed (otherwise we can end up with two SafeHandles wrapping the
@@ -3071,7 +3071,7 @@ ILSafeHandleMarshaler::ReturnOverride(
         pslIL->SetStubTargetArgType(&locDescReturnHandle, false);   // extra arg is a byref IntPtr
 
         // 5) [byref] pass address of local as last arg
-        pslILDispatch->EmitLDLOCA(dwReturnNativeHandleLocal);
+        EmitLoadNativeLocalAddrForByRefDispatch(pslILDispatch, dwReturnNativeHandleLocal);
 
         // We will use cleanup stream to avoid leaking the handle on thread abort.
         psl->EmitSetArgMarshalIndex(pslIL, NDirectStubLinker::CLEANUP_INDEX_RETVAL_UNMARSHAL);
@@ -3253,7 +3253,7 @@ MarshalerOverrideStatus ILCriticalHandleMarshaler::ArgumentOverride(NDirectStubL
             }
 
             // Leave the address of the native handle local as the argument to the native method.
-            pslILDispatch->EmitLDLOCA(dwNativeHandleLocal);
+            EmitLoadNativeLocalAddrForByRefDispatch(pslILDispatch, dwNativeHandleLocal);
 
             if (fin)
             {
@@ -3406,7 +3406,7 @@ ILCriticalHandleMarshaler::ReturnOverride(
         pslIL->SetStubTargetArgType(&locDescReturnHandle, false);   // extra arg is a byref IntPtr
 
         // 5) [byref] pass address of local as last arg
-        pslILDispatch->EmitLDLOCA(dwReturnNativeHandleLocal);
+        EmitLoadNativeLocalAddrForByRefDispatch(pslILDispatch, dwReturnNativeHandleLocal);
 
         // We will use cleanup stream to avoid leaking the handle on thread abort.
         psl->EmitSetArgMarshalIndex(pslIL, NDirectStubLinker::CLEANUP_INDEX_RETVAL_UNMARSHAL);
@@ -3512,7 +3512,7 @@ MarshalerOverrideStatus ILBlittableValueClassWithCopyCtorMarshaler::ArgumentOver
         pslILDispatch->EmitLDLOC(dwNewValueTypeLocal);      // we load the local directly
 #else
         pslIL->SetStubTargetArgType(ELEMENT_TYPE_I);        // native type is a pointer
-        pslILDispatch->EmitLDLOCA(dwNewValueTypeLocal);
+        EmitLoadNativeLocalAddrForByRefDispatch(pslILDispatch, dwNewValueTypeLocal);
 #endif
 
         return OVERRIDDEN;
index de661be63f8c75b9704c22244141c21b652ad60b..21e1b2db4478d721f4c1ea50ea376b157d5f7f8d 100644 (file)
@@ -371,6 +371,16 @@ protected:
         return (0 != (dwMarshalFlags & MARSHAL_FLAG_FIELD));
     }
 
+    static void EmitLoadNativeLocalAddrForByRefDispatch(ILCodeStream* pslILEmit, DWORD local)
+    {
+        WRAPPER_NO_CONTRACT;
+        pslILEmit->EmitLDLOCA(local);
+
+        // Convert the loaded local containing a native address
+        // into a non-GC type for the byref case.
+        pslILEmit->EmitCONV_I();
+    }
+
     void EmitLoadManagedValue(ILCodeStream* pslILEmit)
     {
         WRAPPER_NO_CONTRACT;
@@ -395,6 +405,16 @@ protected:
         m_nativeHome.EmitLoadHomeAddr(pslILEmit);
     }
 
+    void EmitLoadNativeHomeAddrForByRefDispatch(ILCodeStream* pslILEmit)
+    {
+        WRAPPER_NO_CONTRACT;
+        EmitLoadNativeHomeAddr(pslILEmit);
+
+        // Convert the loaded value containing a native address
+        // into a non-GC type for the byref case.
+        pslILEmit->EmitCONV_I();
+    }
+
     void EmitStoreManagedValue(ILCodeStream* pslILEmit)
     {
         WRAPPER_NO_CONTRACT;
@@ -421,6 +441,7 @@ protected:
 
     void EmitLogNativeArgument(ILCodeStream* pslILEmit, DWORD dwPinnedLocal)
     {
+        WRAPPER_NO_CONTRACT;
         if (g_pConfig->InteropLogArguments())
         {
             m_pslNDirect->EmitLogNativeArgument(pslILEmit, dwPinnedLocal);
@@ -666,7 +687,7 @@ public:
         {
             if (IsNativePassedByRef())
             {
-                EmitLoadNativeHomeAddr(pslILEmit);
+                EmitLoadNativeHomeAddrForByRefDispatch(pslILEmit);
             }
             else
             {
@@ -807,7 +828,7 @@ public:
             if (IsHresultSwap(dwMarshalFlags) || byrefNativeReturn)
             {
                 EmitReInitNative(m_pcsMarshal);
-                EmitLoadNativeHomeAddr(pcsDispatch);    // load up the byref native type as an extra arg
+                EmitLoadNativeHomeAddrForByRefDispatch(pcsDispatch);    // load up the byref native type as an extra arg
             }
             else
             {
index 75cd4e855269e947363cd27087e910bbe16497b1..fbea00a3521e753ee72913422d4f06b2112a786e 100644 (file)
@@ -1,5 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
+    <OutputType>Exe</OutputType>
     <IgnoreCoreCLRTestLibraryDependency>true</IgnoreCoreCLRTestLibraryDependency>
     <CLRTestScriptLocalCoreShim>true</CLRTestScriptLocalCoreShim>
     <RequiresMockHostPolicy>true</RequiresMockHostPolicy>
index 1c758d92634fb84e90fac8b65b67bdbca989dd55..be2377f98ef28b4a4f5bc2d4c1fce150573246b2 100644 (file)
@@ -1,5 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
+    <OutputType>Exe</OutputType>
     <IgnoreCoreCLRTestLibraryDependency>true</IgnoreCoreCLRTestLibraryDependency>
     <CLRTestScriptLocalCoreShim>true</CLRTestScriptLocalCoreShim>
     <RequiresMockHostPolicy>true</RequiresMockHostPolicy>
index 4b94fda93daa9291b02d55bed14ed61215cea13d..5d71d37966a2da9ee30b0e83027718df8379f1a6 100644 (file)
@@ -1,5 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
+    <OutputType>Exe</OutputType>
     <IgnoreCoreCLRTestLibraryDependency>true</IgnoreCoreCLRTestLibraryDependency>
     <CLRTestScriptLocalCoreShim>true</CLRTestScriptLocalCoreShim>
     <RequiresMockHostPolicy>true</RequiresMockHostPolicy>
index 0419036b66cfbe75bc0756bfba1cb1ea23aa92f5..c27fe1402b6c2e1d8e4ecf8ac06feff36a646275 100644 (file)
@@ -1,5 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
+    <OutputType>Exe</OutputType>
     <IgnoreCoreCLRTestLibraryDependency>true</IgnoreCoreCLRTestLibraryDependency>
     <CLRTestScriptLocalCoreShim>true</CLRTestScriptLocalCoreShim>
     <RequiresMockHostPolicy>true</RequiresMockHostPolicy>
index 7732a965da8967d5a9066f4be136d83886e93693..3c32a0e7f98ef8d0ce238bfab459a3781b648970 100644 (file)
@@ -526,17 +526,21 @@ namespace Microsoft.CSharp.RuntimeBinder.ComInterop
             EmitLoadArg(method, flagsIndex);
 
             EmitLoadArg(method, dispParamsIndex);
+            method.Emit(OpCodes.Conv_I);
 
             if (returnResult)
             {
                 EmitLoadArg(method, resultIndex);
+                method.Emit(OpCodes.Conv_I);
             }
             else
             {
                 method.Emit(OpCodes.Ldsfld, typeof(IntPtr).GetField(nameof(IntPtr.Zero)));
             }
             EmitLoadArg(method, exceptInfoIndex);
+            method.Emit(OpCodes.Conv_I);
             EmitLoadArg(method, argErrIndex);
+            method.Emit(OpCodes.Conv_I);
 
             // functionPtr = *(IntPtr*)(*(dispatchPointer) + VTABLE_OFFSET)
             int idispatchInvokeOffset = ((int)IDispatchMethodIndices.IDispatch_Invoke) * Marshal.SizeOf(typeof(IntPtr));