Skip to content

Commit 2e19d10

Browse files
github-actions[bot]MichalStrehovskyagocke
authored
[release/8.0] Fix analysis of interface methods on generic types (#93748)
* Fix analysis of interface methods on generic types Fixes an issue observed in #92881. The dependency analysis within the compiler was incorrectly considering `Equals(object x, object y)` to be the implementation of `IEqualityComparer<T>.Equals(T, T)`. When we generate the interface dispatch table, we'd use the correct algorithm (that looks at uninstantiated types) and fail the compilation. The fix is to use the same algorithm during dependency analysis. Looks like this has been broken ever since interface support was added to CoreRT: dotnet/corert#626. * Stop building a test --------- Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com> Co-authored-by: Andy Gocke <angocke@microsoft.com>
1 parent 0b0222b commit 2e19d10

File tree

3 files changed

+70
-8
lines changed

3 files changed

+70
-8
lines changed

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -437,10 +437,14 @@ public sealed override IEnumerable<CombinedDependencyListEntry> GetConditionalSt
437437
// Add conditional dependencies for interface methods the type implements. For example, if the type T implements
438438
// interface IFoo which has a method M1, add a dependency on T.M1 dependent on IFoo.M1 being called, since it's
439439
// possible for any IFoo object to actually be an instance of T.
440+
DefType defTypeDefinition = (DefType)defType.GetTypeDefinition();
440441
DefType[] defTypeRuntimeInterfaces = defType.RuntimeInterfaces;
442+
DefType[] defTypeDefinitionRuntimeInterfaces = defTypeDefinition.RuntimeInterfaces;
443+
Debug.Assert(defTypeDefinitionRuntimeInterfaces.Length == defTypeRuntimeInterfaces.Length);
441444
for (int interfaceIndex = 0; interfaceIndex < defTypeRuntimeInterfaces.Length; interfaceIndex++)
442445
{
443446
DefType interfaceType = defTypeRuntimeInterfaces[interfaceIndex];
447+
DefType interfaceDefinitionType = defTypeDefinitionRuntimeInterfaces[interfaceIndex];
444448

445449
Debug.Assert(interfaceType.IsInterface);
446450

@@ -457,11 +461,22 @@ public sealed override IEnumerable<CombinedDependencyListEntry> GetConditionalSt
457461
if (!isStaticInterfaceMethod && !needsDependenciesForInstanceInterfaceMethodImpls)
458462
continue;
459463

464+
MethodDesc interfaceMethodDefinition = interfaceMethod;
465+
if (interfaceType != interfaceDefinitionType)
466+
interfaceMethodDefinition = factory.TypeSystemContext.GetMethodForInstantiatedType(interfaceMethodDefinition.GetTypicalMethodDefinition(), (InstantiatedType)interfaceDefinitionType);
467+
460468
MethodDesc implMethod = isStaticInterfaceMethod ?
461-
defType.ResolveInterfaceMethodToStaticVirtualMethodOnType(interfaceMethod) :
462-
defType.ResolveInterfaceMethodToVirtualMethodOnType(interfaceMethod);
469+
defTypeDefinition.ResolveInterfaceMethodToStaticVirtualMethodOnType(interfaceMethodDefinition) :
470+
defTypeDefinition.ResolveInterfaceMethodToVirtualMethodOnType(interfaceMethodDefinition);
463471
if (implMethod != null)
464472
{
473+
TypeDesc implType = defType;
474+
while (!implType.HasSameTypeDefinition(implMethod.OwningType))
475+
implType = implType.BaseType;
476+
477+
if (!implType.IsTypeDefinition)
478+
implMethod = factory.TypeSystemContext.GetMethodForInstantiatedType(implMethod.GetTypicalMethodDefinition(), (InstantiatedType)implType);
479+
465480
if (isStaticInterfaceMethod)
466481
{
467482
Debug.Assert(!implMethod.IsVirtual);
@@ -500,12 +515,7 @@ public sealed override IEnumerable<CombinedDependencyListEntry> GetConditionalSt
500515
// Is the implementation provided by a default interface method?
501516
// If so, add a dependency on the entrypoint directly since nobody else is going to do that
502517
// (interface types have an empty vtable, modulo their generic dictionary).
503-
TypeDesc interfaceOnDefinition = defType.GetTypeDefinition().RuntimeInterfaces[interfaceIndex];
504-
MethodDesc interfaceMethodDefinition = interfaceMethod;
505-
if (!interfaceType.IsTypeDefinition)
506-
interfaceMethodDefinition = factory.TypeSystemContext.GetMethodForInstantiatedType(interfaceMethod.GetTypicalMethodDefinition(), (InstantiatedType)interfaceOnDefinition);
507-
508-
var resolution = defType.GetTypeDefinition().ResolveInterfaceMethodToDefaultImplementationOnType(interfaceMethodDefinition, out implMethod);
518+
var resolution = defTypeDefinition.ResolveInterfaceMethodToDefaultImplementationOnType(interfaceMethodDefinition, out implMethod);
509519
if (resolution == DefaultInterfaceMethodResolution.DefaultImplementation)
510520
{
511521
DefType providingInterfaceDefinitionType = (DefType)implMethod.OwningType;

src/tests/Loader/classloader/StaticVirtualMethods/NegativeTestCases/MethodBodyOnUnrelatedType.ilproj

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44

55
<!-- Crossgen2 currently doesn't support this negative check - that should be fine as runtime behavior is undefined in the presence of invalid IL. -->
66
<CrossGenTest>false</CrossGenTest>
7+
8+
<!-- Testing TypeLoad/MissingMethod exceptions in situations that are expensive to detect -->
9+
<NativeAotIncompatible>true</NativeAotIncompatible>
710
</PropertyGroup>
811
<PropertyGroup>
912
<DebugType>Full</DebugType>

src/tests/nativeaot/SmokeTests/UnitTests/Interfaces.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public static int Run()
4040
TestDefaultInterfaceVariance.Run();
4141
TestVariantInterfaceOptimizations.Run();
4242
TestSharedInterfaceMethods.Run();
43+
TestGenericAnalysis.Run();
4344
TestCovariantReturns.Run();
4445
TestDynamicInterfaceCastable.Run();
4546
TestStaticInterfaceMethodsAnalysis.Run();
@@ -653,6 +654,54 @@ public static void Run()
653654
}
654655
}
655656

657+
class TestGenericAnalysis
658+
{
659+
interface IInterface
660+
{
661+
string Method(object p);
662+
}
663+
664+
interface IInterface<T>
665+
{
666+
string Method(T p);
667+
}
668+
669+
class C1<T> : IInterface, IInterface<T>
670+
{
671+
public string Method(object p) => "Method(object)";
672+
public string Method(T p) => "Method(T)";
673+
}
674+
675+
class C2<T> : IInterface, IInterface<T>
676+
{
677+
public string Method(object p) => "Method(object)";
678+
public string Method(T p) => "Method(T)";
679+
}
680+
681+
class C3<T> : IInterface, IInterface<T>
682+
{
683+
public string Method(object p) => "Method(object)";
684+
public string Method(T p) => "Method(T)";
685+
}
686+
687+
static IInterface s_c1 = new C1<object>();
688+
static IInterface<object> s_c2 = new C2<object>();
689+
static IInterface<object> s_c3a = new C3<object>();
690+
static IInterface s_c3b = new C3<object>();
691+
692+
public static void Run()
693+
{
694+
if (s_c1.Method(null) != "Method(object)")
695+
throw new Exception();
696+
if (s_c2.Method(null) != "Method(T)")
697+
throw new Exception();
698+
if (s_c3a.Method(null) != "Method(T)")
699+
throw new Exception();
700+
if (s_c3b.Method(null) != "Method(object)")
701+
throw new Exception();
702+
}
703+
}
704+
656705
class TestCovariantReturns
657706
{
658707
interface IFoo

0 commit comments

Comments
 (0)