Revert "Delete suspicious code in delegate construction (dotnet/coreclr#22830)" ...
authorJarret Shook <jashoo@microsoft.com>
Tue, 19 Mar 2019 16:06:56 +0000 (09:06 -0700)
committerGitHub <noreply@github.com>
Tue, 19 Mar 2019 16:06:56 +0000 (09:06 -0700)
This reverts commit dotnet/coreclr@674bdcbc2ac824d005b5179cee3c5826b582b9a6.

Commit migrated from https://github.com/dotnet/coreclr/commit/0786da806b9727c466f3f9393cb3d55cf9df7536

src/coreclr/src/vm/comdelegate.cpp
src/coreclr/tests/src/Regressions/coreclr/22728/createdelegate.il [deleted file]
src/coreclr/tests/src/Regressions/coreclr/22728/createdelegate.ilproj [deleted file]
src/coreclr/tests/src/Regressions/coreclr/22728/debug15.il [deleted file]
src/coreclr/tests/src/Regressions/coreclr/22728/debug15.ilproj [deleted file]

index 9a131fa..42f457f 100644 (file)
@@ -1626,9 +1626,35 @@ FCIMPL3(PCODE, COMDelegate::AdjustTarget, Object* refThisUNSAFE, Object* targetU
     // close delegates
     MethodTable* pMTTarg = target->GetMethodTable();
     MethodTable* pMTMeth = pMeth->GetMethodTable();
+
+    BOOL isComObject = false;
+
+#ifdef FEATURE_COMINTEROP
+    isComObject = pMTTarg->IsComObjectType();
+#endif // FEATURE_COMINTEROP
     
     MethodDesc *pCorrectedMethod = pMeth;
 
+    if (pMTMeth != pMTTarg)
+    {
+        //They cast to an interface before creating the delegate, so we now need 
+        //to figure out where this actually lives before we continue.
+        //<TODO>@perf:  Grovelling with a signature is really slow.  Speed this up.</TODO>
+        if (pCorrectedMethod->IsInterface())
+        {
+            // No need to resolve the interface based method desc to a class based
+            // one for COM objects because we invoke directly thru the interface MT.
+            if (!isComObject)
+            {
+                // <TODO>it looks like we need to pass an ownerType in here.
+                //  Why can we take a delegate to an interface method anyway?  </TODO>
+                // 
+                pCorrectedMethod = pMTTarg->FindDispatchSlotForInterfaceMD(pCorrectedMethod, TRUE /* throwOnConflict */).GetMethodDesc();
+                _ASSERTE(pCorrectedMethod != NULL);
+            }
+        }
+    }
+
     // Use the Unboxing stub for value class methods, since the value
     // class is constructed using the boxed instance.
     if (pCorrectedMethod->GetMethodTable()->IsValueType() && !pCorrectedMethod->IsUnboxingStub())
@@ -1772,6 +1798,53 @@ FCIMPL3(void, COMDelegate::DelegateConstruct, Object* refThisUNSAFE, Object* tar
         {
             if (pMTTarg)
             {
+                // We can skip the demand if SuppressUnmanagedCodePermission is present on the class,
+                //  or in the case where we are setting up a delegate for a COM event sink
+                //   we can skip the check if the source interface is defined in fully trusted code
+                //   we can skip the check if the source interface is a disp-only interface
+                BOOL isComObject = false;
+#ifdef FEATURE_COMINTEROP
+                isComObject = pMTTarg->IsComObjectType();
+#endif // FEATURE_COMINTEROP
+
+                if (pMTMeth != pMTTarg)
+                {
+                    // They cast to an interface before creating the delegate, so we now need 
+                    // to figure out where this actually lives before we continue.
+                    // <TODO>@perf:  We whould never be using this path to invoke on an interface - 
+                    // that should always be resolved when we are creating the delegate </TODO>
+                    if (pMeth->IsInterface())
+                    {
+                        // No need to resolve the interface based method desc to a class based
+                        // one for COM objects because we invoke directly thru the interface MT.
+                        if (!isComObject)
+                        {
+                            // <TODO>it looks like we need to pass an ownerType in here.
+                            //  Why can we take a delegate to an interface method anyway?  </TODO>
+                            // 
+                            MethodDesc * pDispatchSlotMD = pMTTarg->FindDispatchSlotForInterfaceMD(pMeth, TRUE /* throwOnConflict */).GetMethodDesc();
+                            if (pDispatchSlotMD == NULL)
+                            {
+                                COMPlusThrow(kArgumentException, W("Arg_DlgtTargMeth"));
+                            }
+
+                            if (pMeth->HasMethodInstantiation())
+                            {
+                                pMeth = MethodDesc::FindOrCreateAssociatedMethodDesc(
+                                    pDispatchSlotMD,
+                                    pMTTarg,
+                                    (!pDispatchSlotMD->IsStatic() && pMTTarg->IsValueType()),
+                                    pMeth->GetMethodInstantiation(),
+                                    FALSE /* allowInstParam */);
+                            }
+                            else
+                            {
+                                pMeth = pDispatchSlotMD;
+                            }
+                        }
+                    }
+                }
+
                 g_IBCLogger.LogMethodTableAccess(pMTTarg);
 
                 // Use the Unboxing stub for value class methods, since the value
@@ -3157,9 +3230,10 @@ MethodDesc* COMDelegate::GetDelegateCtor(TypeHandle delegateType, MethodDesc *pT
 #endif
 
         // under the conditions below the delegate ctor needs to perform some heavy operation
-        // to get the unboxing stub
+        // to either resolve the interface call to the real target or to get the unboxing stub (or both)
         BOOL needsRuntimeInfo = !pTargetMethod->IsStatic() && 
-                    pTargetMethod->GetMethodTable()->IsValueType() && !pTargetMethod->IsUnboxingStub();
+                    (pTargetMethod->IsInterface() ||
+                    (pTargetMethod->GetMethodTable()->IsValueType() && !pTargetMethod->IsUnboxingStub()));
 
         if (needsRuntimeInfo)
             pRealCtor = MscorlibBinder::GetMethod(METHOD__MULTICAST_DELEGATE__CTOR_RT_CLOSED);
diff --git a/src/coreclr/tests/src/Regressions/coreclr/22728/createdelegate.il b/src/coreclr/tests/src/Regressions/coreclr/22728/createdelegate.il
deleted file mode 100644 (file)
index 2aa0a61..0000000
+++ /dev/null
@@ -1,103 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-.assembly extern System.Runtime { }
-.assembly extern System.Console { }
-.assembly createdelegate { }
-
-.class private auto ansi beforefieldinit A
-       extends [System.Runtime]System.Object
-       implements B,
-                  C,
-                  D
-{
-  .method private hidebysig newslot virtual final 
-          instance char  B.F1(int32 pA) cil managed
-  {
-    .override B::F1
-    ldc.i4.s   65
-    ret
-  }
-
-  .method private hidebysig newslot virtual final 
-          instance char  D.F1(int32 pA) cil managed
-  {
-    .override D::F1
-    ldc.i4.s   65
-    ret
-  }
-
-  .method private hidebysig instance int32
-          Test() cil managed
-  {
-    ldtoken    class [System.Runtime]System.Func`2<int32,char>
-    call       class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
-    ldarg.0
-    ldtoken    method instance char D::F1(int32)
-    call       class [System.Runtime]System.Reflection.MethodBase [System.Runtime]System.Reflection.MethodBase::GetMethodFromHandle(valuetype [System.Runtime]System.RuntimeMethodHandle)
-    castclass  class [System.Runtime]System.Reflection.MethodInfo
-    call       class [System.Runtime]System.Delegate [System.Runtime]System.Delegate::CreateDelegate(class [System.Runtime]System.Type, object, class [System.Runtime]System.Reflection.MethodInfo)
-    castclass  class [System.Runtime]System.Func`2<int32,char>
-
-    ldc.i4.1
-    callvirt   instance !1 class [System.Runtime]System.Func`2<int32,char>::Invoke(!0)
-    dup
-    call       void [System.Console]System.Console::WriteLine(char)
-    ldc.i4.s   65
-    ceq
-    brtrue A_F1_OK
-
-    ldc.i4.3
-    ret
-
-A_F1_OK:
-
-    ldc.i4.s   100
-
-    ret
-  }
-
-  .method private hidebysig static int32 Main() cil managed
-  {
-    .entrypoint
-    newobj     instance void A::.ctor()
-    call       instance int32 A::Test()
-    ret
-  }
-
-  .method public hidebysig specialname rtspecialname 
-          instance void  .ctor() cil managed
-  {
-    ldarg.0
-    call       instance void [System.Runtime]System.Object::.ctor()
-    ret
-  }
-}
-
-.class interface private abstract auto ansi B
-       implements C,
-                  D
-{
-  .method public hidebysig newslot virtual 
-          instance char  F1(int32 pB) cil managed
-  {
-    ldc.i4.s   66
-    ret
-  }
-}
-
-.class interface private abstract auto ansi C
-       implements D
-{
-}
-
-.class interface private abstract auto ansi D
-{
-  .method public hidebysig newslot virtual 
-          instance char  F1(int32 pD) cil managed
-  {
-    ldc.i4.s   68
-    ret
-  }
-}
diff --git a/src/coreclr/tests/src/Regressions/coreclr/22728/createdelegate.ilproj b/src/coreclr/tests/src/Regressions/coreclr/22728/createdelegate.ilproj
deleted file mode 100644 (file)
index d218608..0000000
+++ /dev/null
@@ -1,35 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
-  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
-  <PropertyGroup>
-    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
-    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
-    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
-    <SchemaVersion>2.0</SchemaVersion>
-    <ProjectGuid>{85DFC527-4DB1-595E-A7D7-E94EE1F8140D}</ProjectGuid>
-    <FileAlignment>512</FileAlignment>
-    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
-    <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
-    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
-    <ReferenceLocalMscorlib>true</ReferenceLocalMscorlib>
-    <OutputType>Exe</OutputType>
-    <CLRTestKind>BuildAndRun</CLRTestKind>
-    <CLRTestPriority>0</CLRTestPriority>
-  </PropertyGroup>
-
-  <ItemGroup>
-    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
-      <Visible>False</Visible>
-    </CodeAnalysisDependentAssemblyPaths>
-  </ItemGroup>
-
-  <ItemGroup>
-    <Compile Include="createdelegate.il" />
-  </ItemGroup>
-
-
-  <ItemGroup>
-    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
-  </ItemGroup>
-  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
-</Project>
diff --git a/src/coreclr/tests/src/Regressions/coreclr/22728/debug15.il b/src/coreclr/tests/src/Regressions/coreclr/22728/debug15.il
deleted file mode 100644 (file)
index a545c9b..0000000
+++ /dev/null
@@ -1,133 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-.assembly extern System.Runtime { }
-.assembly extern System.Console { }
-.assembly debug15 { }
-
-.class private auto ansi beforefieldinit A
-       extends [System.Runtime]System.Object
-       implements B,
-                  C,
-                  D
-{
-  .method private hidebysig newslot virtual final 
-          instance char  B.F1(int32 pA) cil managed
-  {
-    .override B::F1
-    ldc.i4.s   65
-    ret
-  }
-
-  .method private hidebysig newslot virtual final 
-          instance char  D.F1(int32 pA) cil managed
-  {
-    .override D::F1
-    ldc.i4.s   65
-    ret
-  }
-
-  .method private hidebysig instance int32
-          Test() cil managed
-  {
-    ldarg.0
-    ldftn      instance char B::F1(int32)
-    newobj     instance void class [System.Runtime]System.Func`2<int32,char>::.ctor(object,
-                                                                                    native int)
-    ldc.i4.1
-    callvirt   instance !1 class [System.Runtime]System.Func`2<int32,char>::Invoke(!0)
-    dup
-    call       void [System.Console]System.Console::WriteLine(char)
-    ldc.i4.s   66
-    ceq
-    brtrue B_F1_OK
-
-    ldc.i4.1
-    ret
-
-B_F1_OK:
-
-    ldarg.0
-    ldftn      instance char D::F1(int32)
-    newobj     instance void class [System.Runtime]System.Func`2<int32,char>::.ctor(object,
-                                                                                   native int)
-    ldc.i4.1
-    callvirt   instance !1 class [System.Runtime]System.Func`2<int32,char>::Invoke(!0)
-    dup
-    call       void [System.Console]System.Console::WriteLine(char)
-    ldc.i4.s   68
-    ceq
-    brtrue D_F1_OK
-
-    ldc.i4.2
-    ret
-
-D_F1_OK:
-
-    ldarg.0
-    dup
-    ldvirtftn  instance char D::F1(int32)
-    newobj     instance void class [System.Runtime]System.Func`2<int32,char>::.ctor(object,
-                                                                                   native int)
-    ldc.i4.1
-    callvirt   instance !1 class [System.Runtime]System.Func`2<int32,char>::Invoke(!0)
-    dup
-    call       void [System.Console]System.Console::WriteLine(char)
-    ldc.i4.s   65
-    ceq
-    brtrue A_F1_OK
-
-    ldc.i4.3
-    ret
-
-A_F1_OK:
-
-    ldc.i4.s   100
-
-    ret
-  }
-
-  .method private hidebysig static int32 Main() cil managed
-  {
-    .entrypoint
-    newobj     instance void A::.ctor()
-    call       instance int32 A::Test()
-    ret
-  }
-
-  .method public hidebysig specialname rtspecialname 
-          instance void  .ctor() cil managed
-  {
-    ldarg.0
-    call       instance void [System.Runtime]System.Object::.ctor()
-    ret
-  }
-}
-
-.class interface private abstract auto ansi B
-       implements C,
-                  D
-{
-  .method public hidebysig newslot virtual 
-          instance char  F1(int32 pB) cil managed
-  {
-    ldc.i4.s   66
-    ret
-  }
-}
-
-.class interface private abstract auto ansi C
-       implements D
-{
-}
-
-.class interface private abstract auto ansi D
-{
-  .method public hidebysig newslot virtual 
-          instance char  F1(int32 pD) cil managed
-  {
-    ldc.i4.s   68
-    ret
-  }
-}
diff --git a/src/coreclr/tests/src/Regressions/coreclr/22728/debug15.ilproj b/src/coreclr/tests/src/Regressions/coreclr/22728/debug15.ilproj
deleted file mode 100644 (file)
index 442a59c..0000000
+++ /dev/null
@@ -1,35 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
-  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
-  <PropertyGroup>
-    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
-    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
-    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
-    <SchemaVersion>2.0</SchemaVersion>
-    <ProjectGuid>{85DFC527-4DB1-595E-A7D7-E94EE1F8140D}</ProjectGuid>
-    <FileAlignment>512</FileAlignment>
-    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
-    <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
-    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
-    <ReferenceLocalMscorlib>true</ReferenceLocalMscorlib>
-    <OutputType>Exe</OutputType>
-    <CLRTestKind>BuildAndRun</CLRTestKind>
-    <CLRTestPriority>0</CLRTestPriority>
-  </PropertyGroup>
-
-  <ItemGroup>
-    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
-      <Visible>False</Visible>
-    </CodeAnalysisDependentAssemblyPaths>
-  </ItemGroup>
-
-  <ItemGroup>
-    <Compile Include="debug15.il" />
-  </ItemGroup>
-
-
-  <ItemGroup>
-    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
-  </ItemGroup>
-  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
-</Project>