Implement two pass algorithm for variant interface dispatch (#21355)
authorMichal Strehovský <MichalStrehovsky@users.noreply.github.com>
Fri, 14 Dec 2018 11:21:45 +0000 (12:21 +0100)
committerGitHub <noreply@github.com>
Fri, 14 Dec 2018 11:21:45 +0000 (12:21 +0100)
Fixes #20452.

src/vm/methodtable.cpp
src/vm/methodtable.h
tests/src/Regressions/coreclr/20452/twopassvariance.il [new file with mode: 0644]
tests/src/Regressions/coreclr/20452/twopassvariance.ilproj [new file with mode: 0644]

index fe4a18b..91f6edd 100644 (file)
@@ -6977,12 +6977,28 @@ MethodTable::FindDispatchImpl(
                 //
                 // See if we can find a default method from one of the implemented interfaces 
                 //
+
+                // Try exact match first
                 MethodDesc *pDefaultMethod = NULL;
-                if (FindDefaultInterfaceImplementation(
+                BOOL foundDefaultInterfaceImplementation  = FindDefaultInterfaceImplementation(
                     pIfcMD,     // the interface method being resolved
                     pIfcMT,     // the interface being resolved
                     &pDefaultMethod,
-                    throwOnConflict))
+                    FALSE, // allowVariance
+                    throwOnConflict);
+
+                // If there's no exact match, try a variant match
+                if (!foundDefaultInterfaceImplementation && pIfcMT->HasVariance())
+                {
+                    foundDefaultInterfaceImplementation = FindDefaultInterfaceImplementation(
+                        pIfcMD,     // the interface method being resolved
+                        pIfcMT,     // the interface being resolved
+                        &pDefaultMethod,
+                        TRUE, // allowVariance
+                        throwOnConflict);
+                }
+
+                if (foundDefaultInterfaceImplementation)
                 {
                     // Now, construct a DispatchSlot to return in *pImplSlot
                     DispatchSlot ds(pDefaultMethod->GetMethodEntryPoint());
@@ -7062,6 +7078,7 @@ BOOL MethodTable::FindDefaultInterfaceImplementation(
     MethodDesc *pInterfaceMD,
     MethodTable *pInterfaceMT,
     MethodDesc **ppDefaultMethod,
+    BOOL allowVariance,
     BOOL throwOnConflict
 )
 {
@@ -7120,7 +7137,7 @@ BOOL MethodTable::FindDefaultInterfaceImplementation(
                 {
                     if (pCurMT->HasSameTypeDefAs(pInterfaceMT))
                     {
-                        if (!pInterfaceMD->IsAbstract())
+                        if (allowVariance && !pInterfaceMD->IsAbstract())
                         {
                             // Generic variance match - we'll instantiate pCurMD with the right type arguments later
                             pCurMD = pInterfaceMD;
@@ -7178,7 +7195,8 @@ BOOL MethodTable::FindDefaultInterfaceImplementation(
                                     // We do CanCastToInterface to also cover variance.
                                     // We already know this is a method on the same type definition as the (generic)
                                     // interface but we need to make sure the instantiations match.
-                                    if (pDeclMT->CanCastToInterface(pInterfaceMT))
+                                    if ((allowVariance && pDeclMT->CanCastToInterface(pInterfaceMT))
+                                        || pDeclMT == pInterfaceMT)
                                     {
                                         // We have a match
                                         pCurMD = pMD;
@@ -7234,7 +7252,11 @@ BOOL MethodTable::FindDefaultInterfaceImplementation(
                             break;
                         }
 
-                        if (pCurMT->CanCastToInterface(pCandidateMT))
+                        if (allowVariance && pCandidateMT->HasSameTypeDefAs(pCurMT))
+                        {
+                            // Variant match on the same type - this is a tie
+                        }
+                        else if (pCurMT->CanCastToInterface(pCandidateMT))
                         {
                             // pCurMT is a more specific choice than IFoo/IBar both overrides IBlah :
                             if (!seenMoreSpecific)
@@ -7281,6 +7303,8 @@ BOOL MethodTable::FindDefaultInterfaceImplementation(
     }
 
     // scan to see if there are any conflicts
+    // If we are doing second pass (allowing variance), we know don't actually look for
+    // a conflict anymore, but pick the first match.
     MethodTable *pBestCandidateMT = NULL;
     MethodDesc *pBestCandidateMD = NULL;
     for (unsigned i = 0; i < candidatesCount; ++i)
@@ -7292,6 +7316,11 @@ BOOL MethodTable::FindDefaultInterfaceImplementation(
         {
             pBestCandidateMT = candidates[i].pMT;
             pBestCandidateMD = candidates[i].pMD;
+
+            // If this is a second pass lookup, we know this is a variant match. As such
+            // we pick the first result as the winner and don't look for a conflict.
+            if (allowVariance)
+                break;
         }
         else if (pBestCandidateMT != candidates[i].pMT)
         {
index c80384d..395d050 100644 (file)
@@ -2459,6 +2459,7 @@ public:
         MethodDesc *pInterfaceMD,
         MethodTable *pObjectMT,
         MethodDesc **ppDefaultMethod,
+        BOOL allowVariance,
         BOOL throwOnConflict);
 #endif // DACCESS_COMPILE
 
diff --git a/tests/src/Regressions/coreclr/20452/twopassvariance.il b/tests/src/Regressions/coreclr/20452/twopassvariance.il
new file mode 100644 (file)
index 0000000..c4504d9
--- /dev/null
@@ -0,0 +1,326 @@
+// 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 mscorlib { }
+
+.assembly twopassvariance { }
+
+.class Base { }
+
+.class Derived extends Base { }
+
+.class SuperDerived extends Derived { }
+
+.class MegaSuperDerived extends SuperDerived { }
+
+.class interface IFoo<-T>
+{
+    .method public newslot virtual instance class [mscorlib]System.Type Gimme()
+    {
+        ldtoken class IFoo<!0>
+        call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+        ret
+    }
+}
+
+.class interface IBar<T> implements class IFoo<!T>
+{
+    .method public virtual final instance class [mscorlib]System.Type Gimme()
+    {
+        .override method instance class [mscorlib]System.Type class IFoo<!T>::Gimme()
+        ldtoken class IBar<!0>
+        call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+        ret
+    }
+}
+
+.class interface IBaz<T> implements class IFoo<!T>
+{
+    .method public virtual final instance class [mscorlib]System.Type Gimme()
+    {
+        .override method instance class [mscorlib]System.Type class IFoo<!T>::Gimme()
+        ldtoken class IBar<!0>
+        call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+        ret
+    }
+}
+
+.class interface IQux implements class IFoo<class Base>, class IFoo<class Derived>
+{
+    .method public virtual final instance class [mscorlib]System.Type Gimme()
+    {
+        .override method instance class [mscorlib]System.Type class IFoo<class Base>::Gimme()
+        ldtoken IQux
+        call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+        ret
+    }
+}
+
+.class Fooer1
+    implements class IFoo<class Base>, class IFoo<class Derived>
+{
+    .method public specialname rtspecialname instance void .ctor()
+    {
+        ldarg.0
+        call instance void [mscorlib]System.Object::.ctor()
+        ret
+    }
+}
+
+.class Fooer2
+    implements class IFoo<class Derived>, class IFoo<class Base>
+{
+    .method public specialname rtspecialname instance void .ctor()
+    {
+        ldarg.0
+        call instance void [mscorlib]System.Object::.ctor()
+        ret
+    }
+}
+
+.class Fooer3
+    implements class IBaz<class Base>, class IBar<class Derived>, class IFoo<object>
+{
+    .method public specialname rtspecialname instance void .ctor()
+    {
+        ldarg.0
+        call instance void [mscorlib]System.Object::.ctor()
+        ret
+    }
+}
+
+.class Fooer4
+    implements IQux, class IBar<class Derived>
+{
+    .method public specialname rtspecialname instance void .ctor()
+    {
+        ldarg.0
+        call instance void [mscorlib]System.Object::.ctor()
+        ret
+    }
+}
+
+.class Fooer5
+    implements class IBar<class Derived>, IQux
+{
+    .method public specialname rtspecialname instance void .ctor()
+    {
+        ldarg.0
+        call instance void [mscorlib]System.Object::.ctor()
+        ret
+    }
+}
+
+.class Fooer6
+    implements class IFoo<class Base>, class IBar<class Base>
+{
+    .method public specialname rtspecialname instance void .ctor()
+    {
+        ldarg.0
+        call instance void [mscorlib]System.Object::.ctor()
+        ret
+    }
+}
+
+.class Fooer7
+    implements class IBar<class Base>, class IFoo<class Base>
+{
+    .method public specialname rtspecialname instance void .ctor()
+    {
+        ldarg.0
+        call instance void [mscorlib]System.Object::.ctor()
+        ret
+    }
+}
+
+.method static int32 main()
+{
+    .entrypoint
+
+    newobj instance void Fooer1::.ctor()
+    castclass class IFoo<class SuperDerived>
+    callvirt instance class [mscorlib]System.Type class IFoo<class SuperDerived>::Gimme()
+    dup
+    callvirt instance string [mscorlib]System.Object::ToString()
+    call void [mscorlib]System.Console::WriteLine(string)
+    ldtoken class IFoo<class Base>
+    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+    ceq
+    brtrue Fooer1_IFoo_SuperDerivedOK
+    ldc.i4.1
+    ret
+
+Fooer1_IFoo_SuperDerivedOK:
+    newobj instance void Fooer2::.ctor()
+    castclass class IFoo<class SuperDerived>
+    callvirt instance class [mscorlib]System.Type class IFoo<class SuperDerived>::Gimme()
+    dup
+    callvirt instance string [mscorlib]System.Object::ToString()
+    call void [mscorlib]System.Console::WriteLine(string)
+    ldtoken class IFoo<class Derived>
+    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+    ceq
+    brtrue Fooer2_IFoo_SuperDerivedOK
+    ldc.i4.2
+    ret
+
+Fooer2_IFoo_SuperDerivedOK:
+    newobj instance void Fooer3::.ctor()
+    castclass class IFoo<class Base>
+    callvirt instance class [mscorlib]System.Type class IFoo<class Base>::Gimme()
+    dup
+    callvirt instance string [mscorlib]System.Object::ToString()
+    call void [mscorlib]System.Console::WriteLine(string)
+    ldtoken class IBar<class Base>
+    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+    ceq
+    brtrue Fooer3_IFoo_BaseOK
+    ldc.i4.3
+    ret
+
+Fooer3_IFoo_BaseOK:
+    newobj instance void Fooer3::.ctor()
+    castclass class IFoo<class Derived>
+    callvirt instance class [mscorlib]System.Type class IFoo<class Derived>::Gimme()
+    dup
+    callvirt instance string [mscorlib]System.Object::ToString()
+    call void [mscorlib]System.Console::WriteLine(string)
+    ldtoken class IBar<class Derived>
+    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+    ceq
+    brtrue Fooer3_IFoo_DerivedOK
+    ldc.i4.4
+    ret
+
+Fooer3_IFoo_DerivedOK:
+    newobj instance void Fooer3::.ctor()
+    castclass class IFoo<class SuperDerived>
+    callvirt instance class [mscorlib]System.Type class IFoo<class SuperDerived>::Gimme()
+    dup
+    callvirt instance string [mscorlib]System.Object::ToString()
+    call void [mscorlib]System.Console::WriteLine(string)
+    ldtoken class IBar<class Base>
+    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+    ceq
+    brtrue Fooer3_IFoo_SuperDerivedOK
+    ldc.i4.5
+    ret
+
+Fooer3_IFoo_SuperDerivedOK:
+    newobj instance void Fooer4::.ctor()
+    castclass class IFoo<class Base>
+    callvirt instance class [mscorlib]System.Type class IFoo<class Base>::Gimme()
+    dup
+    callvirt instance string [mscorlib]System.Object::ToString()
+    call void [mscorlib]System.Console::WriteLine(string)
+    ldtoken IQux
+    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+    ceq
+    brtrue Fooer4_IFoo_BaseOK
+    ldc.i4.6
+    ret
+
+Fooer4_IFoo_BaseOK:
+    newobj instance void Fooer4::.ctor()
+    castclass class IFoo<class Derived>
+    callvirt instance class [mscorlib]System.Type class IFoo<class Derived>::Gimme()
+    dup
+    callvirt instance string [mscorlib]System.Object::ToString()
+    call void [mscorlib]System.Console::WriteLine(string)
+    ldtoken class IBar<class Derived>
+    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+    ceq
+    brtrue Fooer4_IFoo_DerivedOK
+    ldc.i4.7
+    ret
+
+Fooer4_IFoo_DerivedOK:
+    newobj instance void Fooer4::.ctor()
+    castclass class IFoo<class SuperDerived>
+    callvirt instance class [mscorlib]System.Type class IFoo<class SuperDerived>::Gimme()
+    dup
+    callvirt instance string [mscorlib]System.Object::ToString()
+    call void [mscorlib]System.Console::WriteLine(string)
+    ldtoken IQux
+    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+    ceq
+    brtrue Fooer4_IFoo_SuperDerivedOK
+    ldc.i4.8
+    ret
+
+Fooer4_IFoo_SuperDerivedOK:
+    newobj instance void Fooer5::.ctor()
+    castclass class IFoo<class SuperDerived>
+    callvirt instance class [mscorlib]System.Type class IFoo<class SuperDerived>::Gimme()
+    dup
+    callvirt instance string [mscorlib]System.Object::ToString()
+    call void [mscorlib]System.Console::WriteLine(string)
+    ldtoken class IBar<class Derived>
+    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+    ceq
+    brtrue Fooer5_IFoo_SuperDerivedOK
+    ldc.i4 9
+    ret
+
+Fooer5_IFoo_SuperDerivedOK:
+    newobj instance void Fooer6::.ctor()
+    castclass class IFoo<class Base>
+    callvirt instance class [mscorlib]System.Type class IFoo<class Base>::Gimme()
+    dup
+    callvirt instance string [mscorlib]System.Object::ToString()
+    call void [mscorlib]System.Console::WriteLine(string)
+    ldtoken class IBar<class Base>
+    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+    ceq
+    brtrue Fooer6_IFoo_BaseOK
+    ldc.i4 10
+    ret
+
+Fooer6_IFoo_BaseOK:
+    newobj instance void Fooer6::.ctor()
+    castclass class IFoo<class Derived>
+    callvirt instance class [mscorlib]System.Type class IFoo<class Derived>::Gimme()
+    dup
+    callvirt instance string [mscorlib]System.Object::ToString()
+    call void [mscorlib]System.Console::WriteLine(string)
+    ldtoken class IBar<class Base>
+    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+    ceq
+    brtrue Fooer6_IFoo_DerivedOK
+    ldc.i4 11
+    ret
+
+Fooer6_IFoo_DerivedOK:
+    newobj instance void Fooer7::.ctor()
+    castclass class IFoo<class Base>
+    callvirt instance class [mscorlib]System.Type class IFoo<class Base>::Gimme()
+    dup
+    callvirt instance string [mscorlib]System.Object::ToString()
+    call void [mscorlib]System.Console::WriteLine(string)
+    ldtoken class IBar<class Base>
+    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+    ceq
+    brtrue Fooer7_IFoo_BaseOK
+    ldc.i4 12
+    ret
+
+Fooer7_IFoo_BaseOK:
+    newobj instance void Fooer7::.ctor()
+    castclass class IFoo<class Derived>
+    callvirt instance class [mscorlib]System.Type class IFoo<class Derived>::Gimme()
+    dup
+    callvirt instance string [mscorlib]System.Object::ToString()
+    call void [mscorlib]System.Console::WriteLine(string)
+    ldtoken class IBar<class Base>
+    call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+    ceq
+    brtrue Fooer7_IFoo_DerivedOK
+    ldc.i4 13
+    ret
+
+Fooer7_IFoo_DerivedOK:
+
+    ldc.i4 100
+    ret
+}
diff --git a/tests/src/Regressions/coreclr/20452/twopassvariance.ilproj b/tests/src/Regressions/coreclr/20452/twopassvariance.ilproj
new file mode 100644 (file)
index 0000000..4ffff77
--- /dev/null
@@ -0,0 +1,38 @@
+<?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>
+       
+    <!-- Issue 21044, https://github.com/dotnet/coreclr/issues/21044 -->
+    <GCStressIncompatible>true</GCStressIncompatible>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+
+  <ItemGroup>
+    <Compile Include="twopassvariance.il" />
+  </ItemGroup>
+
+
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+</Project>