Delete suspicious code in delegate construction (dotnet/coreclr#22830)

Delegate construction is trying to do some weird special casing around closed delegates to interface methods that is breaking non-virtual delegates to default interface methods. The special casing has suspicious comments indicating people didn't know why it's needed in the first place, so I'm going with a theory that it's a cargo cult (e.g. why the reasons specified in the comments don't apply to normal virtual methods?).

Fixes dotnet/coreclr#22728.

Commit migrated from https://github.com/dotnet/coreclr/commit/674bdcbc2ac824d005b5179cee3c5826b582b9a6

diff --git a/src/coreclr/src/vm/comdelegate.cpp b/src/coreclr/src/vm/comdelegate.cpp
index 42f457f..9a131fa 100644 (file)
--- a/src/coreclr/src/vm/comdelegate.cpp
+++ b/src/coreclr/src/vm/comdelegate.cpp
@@ -1626,35 +1626,9 @@ 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())
@@ -1798,53 +1772,6 @@ 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
@@ -3230,10 +3157,9 @@ MethodDesc* COMDelegate::GetDelegateCtor(TypeHandle delegateType, MethodDesc *pT
 #endif
 
         // under the conditions below the delegate ctor needs to perform some heavy operation
-        // to either resolve the interface call to the real target or to get the unboxing stub (or both)
+        // to get the unboxing stub
         BOOL needsRuntimeInfo = !pTargetMethod->IsStatic() && 
-                    (pTargetMethod->IsInterface() ||
-                    (pTargetMethod->GetMethodTable()->IsValueType() && !pTargetMethod->IsUnboxingStub()));
+                    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
new file mode 100644 (file)
index 0000000..2aa0a61
--- /dev/null
+++ b/src/coreclr/tests/src/Regressions/coreclr/22728/createdelegate.il
@@ -0,0 +1,103 @@
+// 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
new file mode 100644 (file)
index 0000000..d218608
--- /dev/null
+++ b/src/coreclr/tests/src/Regressions/coreclr/22728/createdelegate.ilproj
@@ -0,0 +1,35 @@
+<?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
new file mode 100644 (file)
index 0000000..a545c9b
--- /dev/null
+++ b/src/coreclr/tests/src/Regressions/coreclr/22728/debug15.il
@@ -0,0 +1,133 @@
+// 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
new file mode 100644 (file)
index 0000000..442a59c
--- /dev/null
+++ b/src/coreclr/tests/src/Regressions/coreclr/22728/debug15.ilproj
@@ -0,0 +1,35 @@
+<?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>