From 1649da12db73c8c07513c9168cb30ec70c310063 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 14 Dec 2018 12:21:45 +0100 Subject: [PATCH] Implement two pass algorithm for variant interface dispatch (#21355) Fixes #20452. --- src/vm/methodtable.cpp | 39 ++- src/vm/methodtable.h | 1 + .../Regressions/coreclr/20452/twopassvariance.il | 326 +++++++++++++++++++++ .../coreclr/20452/twopassvariance.ilproj | 38 +++ 4 files changed, 399 insertions(+), 5 deletions(-) create mode 100644 tests/src/Regressions/coreclr/20452/twopassvariance.il create mode 100644 tests/src/Regressions/coreclr/20452/twopassvariance.ilproj diff --git a/src/vm/methodtable.cpp b/src/vm/methodtable.cpp index fe4a18b..91f6edd 100644 --- a/src/vm/methodtable.cpp +++ b/src/vm/methodtable.cpp @@ -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) { diff --git a/src/vm/methodtable.h b/src/vm/methodtable.h index c80384d..395d050 100644 --- a/src/vm/methodtable.h +++ b/src/vm/methodtable.h @@ -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 index 0000000..c4504d9 --- /dev/null +++ b/tests/src/Regressions/coreclr/20452/twopassvariance.il @@ -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 + call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle) + ret + } +} + +.class interface IBar implements class IFoo +{ + .method public virtual final instance class [mscorlib]System.Type Gimme() + { + .override method instance class [mscorlib]System.Type class IFoo::Gimme() + ldtoken class IBar + call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle) + ret + } +} + +.class interface IBaz implements class IFoo +{ + .method public virtual final instance class [mscorlib]System.Type Gimme() + { + .override method instance class [mscorlib]System.Type class IFoo::Gimme() + ldtoken class IBar + call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle) + ret + } +} + +.class interface IQux implements class IFoo, class IFoo +{ + .method public virtual final instance class [mscorlib]System.Type Gimme() + { + .override method instance class [mscorlib]System.Type class IFoo::Gimme() + ldtoken IQux + call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle) + ret + } +} + +.class Fooer1 + implements class IFoo, class IFoo +{ + .method public specialname rtspecialname instance void .ctor() + { + ldarg.0 + call instance void [mscorlib]System.Object::.ctor() + ret + } +} + +.class Fooer2 + implements class IFoo, class IFoo +{ + .method public specialname rtspecialname instance void .ctor() + { + ldarg.0 + call instance void [mscorlib]System.Object::.ctor() + ret + } +} + +.class Fooer3 + implements class IBaz, class IBar, class IFoo +{ + .method public specialname rtspecialname instance void .ctor() + { + ldarg.0 + call instance void [mscorlib]System.Object::.ctor() + ret + } +} + +.class Fooer4 + implements IQux, class IBar +{ + .method public specialname rtspecialname instance void .ctor() + { + ldarg.0 + call instance void [mscorlib]System.Object::.ctor() + ret + } +} + +.class Fooer5 + implements class IBar, IQux +{ + .method public specialname rtspecialname instance void .ctor() + { + ldarg.0 + call instance void [mscorlib]System.Object::.ctor() + ret + } +} + +.class Fooer6 + implements class IFoo, class IBar +{ + .method public specialname rtspecialname instance void .ctor() + { + ldarg.0 + call instance void [mscorlib]System.Object::.ctor() + ret + } +} + +.class Fooer7 + implements class IBar, class IFoo +{ + .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 + callvirt instance class [mscorlib]System.Type class IFoo::Gimme() + dup + callvirt instance string [mscorlib]System.Object::ToString() + call void [mscorlib]System.Console::WriteLine(string) + ldtoken class IFoo + 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 + callvirt instance class [mscorlib]System.Type class IFoo::Gimme() + dup + callvirt instance string [mscorlib]System.Object::ToString() + call void [mscorlib]System.Console::WriteLine(string) + ldtoken class IFoo + 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 + callvirt instance class [mscorlib]System.Type class IFoo::Gimme() + dup + callvirt instance string [mscorlib]System.Object::ToString() + call void [mscorlib]System.Console::WriteLine(string) + ldtoken class IBar + 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 + callvirt instance class [mscorlib]System.Type class IFoo::Gimme() + dup + callvirt instance string [mscorlib]System.Object::ToString() + call void [mscorlib]System.Console::WriteLine(string) + ldtoken class IBar + 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 + callvirt instance class [mscorlib]System.Type class IFoo::Gimme() + dup + callvirt instance string [mscorlib]System.Object::ToString() + call void [mscorlib]System.Console::WriteLine(string) + ldtoken class IBar + 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 + callvirt instance class [mscorlib]System.Type class IFoo::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 + callvirt instance class [mscorlib]System.Type class IFoo::Gimme() + dup + callvirt instance string [mscorlib]System.Object::ToString() + call void [mscorlib]System.Console::WriteLine(string) + ldtoken class IBar + 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 + callvirt instance class [mscorlib]System.Type class IFoo::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 + callvirt instance class [mscorlib]System.Type class IFoo::Gimme() + dup + callvirt instance string [mscorlib]System.Object::ToString() + call void [mscorlib]System.Console::WriteLine(string) + ldtoken class IBar + 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 + callvirt instance class [mscorlib]System.Type class IFoo::Gimme() + dup + callvirt instance string [mscorlib]System.Object::ToString() + call void [mscorlib]System.Console::WriteLine(string) + ldtoken class IBar + 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 + callvirt instance class [mscorlib]System.Type class IFoo::Gimme() + dup + callvirt instance string [mscorlib]System.Object::ToString() + call void [mscorlib]System.Console::WriteLine(string) + ldtoken class IBar + 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 + callvirt instance class [mscorlib]System.Type class IFoo::Gimme() + dup + callvirt instance string [mscorlib]System.Object::ToString() + call void [mscorlib]System.Console::WriteLine(string) + ldtoken class IBar + 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 + callvirt instance class [mscorlib]System.Type class IFoo::Gimme() + dup + callvirt instance string [mscorlib]System.Object::ToString() + call void [mscorlib]System.Console::WriteLine(string) + ldtoken class IBar + 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 index 0000000..4ffff77 --- /dev/null +++ b/tests/src/Regressions/coreclr/20452/twopassvariance.ilproj @@ -0,0 +1,38 @@ + + + + + $(MSBuildProjectName) + Debug + AnyCPU + 2.0 + {85DFC527-4DB1-595E-A7D7-E94EE1F8140D} + 512 + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + 7a9bfb7d + true + true + Exe + BuildAndRun + 0 + + + true + + + + + False + + + + + + + + + + + + + -- 2.7.4