Fix all runtime IL generation paths from passing GC types to unmanage…

…d function calls. (dotnet#35026) * 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.
wenlong-xyz · Apr 18, 2020 · ddd458e · ddd458e
1 parent 028501b
commit ddd458e
Show file tree

Hide file tree

Showing 10 changed files with 52 additions and 9 deletions.
diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp
@@ -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;
     }
 }
 

diff --git a/src/coreclr/src/tools/Common/TypeSystem/Interop/IL/Marshaller.cs b/src/coreclr/src/tools/Common/TypeSystem/Interop/IL/Marshaller.cs
@@ -597,9 +597,14 @@ protected void LoadNativeValue(ILCodeStream stream)
         protected void LoadNativeArg(ILCodeStream stream)
         {
             if (IsNativeByRef)
+            {
                 _nativeHome.LoadAddr(stream);
+                stream.Emit(ILOpcode.conv_i);
+            }
             else
+            {
                 _nativeHome.LoadValue(stream);
+            }
         }
 
         protected void LoadNativeAddr(ILCodeStream stream)

diff --git a/src/coreclr/src/vm/dllimport.cpp b/src/coreclr/src/vm/dllimport.cpp
@@ -410,12 +410,14 @@ class ILStubState : public StubState
             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

diff --git a/src/coreclr/src/vm/ilmarshalers.cpp b/src/coreclr/src/vm/ilmarshalers.cpp
@@ -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;

diff --git a/src/coreclr/src/vm/ilmarshalers.h b/src/coreclr/src/vm/ilmarshalers.h
@@ -371,6 +371,16 @@ class ILMarshaler
         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 @@ class ILMarshaler
         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 @@ class ILMarshaler
 
     void EmitLogNativeArgument(ILCodeStream* pslILEmit, DWORD dwPinnedLocal)
     {
+        WRAPPER_NO_CONTRACT;
         if (g_pConfig->InteropLogArguments())
         {
             m_pslNDirect->EmitLogNativeArgument(pslILEmit, dwPinnedLocal);
@@ -666,7 +687,7 @@ class ILMarshaler
         {
             if (IsNativePassedByRef())
             {
-                EmitLoadNativeHomeAddr(pslILEmit);
+                EmitLoadNativeHomeAddrForByRefDispatch(pslILEmit);
             }
             else
             {
@@ -807,7 +828,7 @@ class ILMarshaler
             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
             {

diff --git a/src/coreclr/tests/src/Interop/COM/NativeClients/DefaultInterfaces.csproj b/src/coreclr/tests/src/Interop/COM/NativeClients/DefaultInterfaces.csproj
@@ -1,5 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
+    <OutputType>Exe</OutputType>
     <IgnoreCoreCLRTestLibraryDependency>true</IgnoreCoreCLRTestLibraryDependency>
     <CLRTestScriptLocalCoreShim>true</CLRTestScriptLocalCoreShim>
     <RequiresMockHostPolicy>true</RequiresMockHostPolicy>

diff --git a/src/coreclr/tests/src/Interop/COM/NativeClients/Dispatch.csproj b/src/coreclr/tests/src/Interop/COM/NativeClients/Dispatch.csproj
@@ -1,5 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
+    <OutputType>Exe</OutputType>
     <IgnoreCoreCLRTestLibraryDependency>true</IgnoreCoreCLRTestLibraryDependency>
     <CLRTestScriptLocalCoreShim>true</CLRTestScriptLocalCoreShim>
     <RequiresMockHostPolicy>true</RequiresMockHostPolicy>

diff --git a/src/coreclr/tests/src/Interop/COM/NativeClients/Licensing.csproj b/src/coreclr/tests/src/Interop/COM/NativeClients/Licensing.csproj
@@ -1,5 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
+    <OutputType>Exe</OutputType>
     <IgnoreCoreCLRTestLibraryDependency>true</IgnoreCoreCLRTestLibraryDependency>
     <CLRTestScriptLocalCoreShim>true</CLRTestScriptLocalCoreShim>
     <RequiresMockHostPolicy>true</RequiresMockHostPolicy>

diff --git a/src/coreclr/tests/src/Interop/COM/NativeClients/Primitives.csproj b/src/coreclr/tests/src/Interop/COM/NativeClients/Primitives.csproj
@@ -1,5 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
+    <OutputType>Exe</OutputType>
     <IgnoreCoreCLRTestLibraryDependency>true</IgnoreCoreCLRTestLibraryDependency>
     <CLRTestScriptLocalCoreShim>true</CLRTestScriptLocalCoreShim>
     <RequiresMockHostPolicy>true</RequiresMockHostPolicy>

diff --git a/...aries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ComRuntimeHelpers.cs b/...aries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/ComInterop/ComRuntimeHelpers.cs
@@ -526,17 +526,21 @@ private static IDispatchInvokeDelegate Create_IDispatchInvoke(bool returnResult)
             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));