Skip to content

Commit 6958008

Browse files
Fix runtime dispatch to static virtuals on interface types (#91374)
We were not generating information about static virtuals on interface types. Information about default interface methods normally goes to the class, but if the T we're dispatching on is an interface, this information wasn't generated. The fix is to put this information into dispatch maps and sealed vtables, same way we do for classes. The test shows what the problem is - if we change `IBar` to be a class, things would work even before this PR.
1 parent 43f3e60 commit 6958008

File tree

6 files changed

+132
-22
lines changed

6 files changed

+132
-22
lines changed

src/coreclr/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,8 @@ private static MethodDesc ResolveInterfaceMethodToVirtualMethodOnType(MethodDesc
614614
{
615615
Debug.Assert(!interfaceMethod.Signature.IsStatic);
616616

617+
// This would be a default interface method resolution. The algorithm below would sort of work, but doesn't handle
618+
// things like diamond cases and it's better not to let it resolve as such.
617619
if (currentType.IsInterface)
618620
return null;
619621

@@ -781,7 +783,7 @@ private static DefaultInterfaceMethodResolution ResolveInterfaceMethodToDefaultI
781783
// If we're asking about an interface, include the interface in the list.
782784
consideredInterfaces = new DefType[currentType.RuntimeInterfaces.Length + 1];
783785
Array.Copy(currentType.RuntimeInterfaces, consideredInterfaces, currentType.RuntimeInterfaces.Length);
784-
consideredInterfaces[consideredInterfaces.Length - 1] = (DefType)currentType.InstantiateAsOpen();
786+
consideredInterfaces[consideredInterfaces.Length - 1] = currentType.IsGenericDefinition ? (DefType)currentType.InstantiateAsOpen() : currentType;
785787
}
786788

787789
foreach (MetadataType runtimeInterface in consideredInterfaces)
@@ -921,6 +923,11 @@ public static IEnumerable<MethodDesc> EnumAllVirtualSlots(MetadataType type)
921923
/// <returns>MethodDesc of the resolved virtual static method, null when not found (runtime lookup must be used)</returns>
922924
public static MethodDesc ResolveInterfaceMethodToStaticVirtualMethodOnType(MethodDesc interfaceMethod, MetadataType currentType)
923925
{
926+
// This would be a default interface method resolution. The algorithm below would sort of work, but doesn't handle
927+
// things like diamond cases and it's better not to let it resolve as such.
928+
if (currentType.IsInterface)
929+
return null;
930+
924931
// Search for match on a per-level in the type hierarchy
925932
for (MetadataType typeToCheck = currentType; typeToCheck != null; typeToCheck = typeToCheck.MetadataBaseType)
926933
{

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -372,16 +372,8 @@ public sealed override IEnumerable<CombinedDependencyListEntry> GetConditionalSt
372372

373373
DefType defType = _type.GetClosestDefType();
374374

375-
// Interfaces don't have vtables and we don't need to track their slot use.
376-
// The only exception are those interfaces that provide IDynamicInterfaceCastable implementations;
377-
// those have slots and we dispatch on them.
378-
bool needsDependenciesForVirtualMethodImpls = !defType.IsInterface
379-
|| ((MetadataType)defType).IsDynamicInterfaceCastableImplementation();
380-
381375
// If we're producing a full vtable, none of the dependencies are conditional.
382-
needsDependenciesForVirtualMethodImpls &= !factory.VTable(defType).HasFixedSlots;
383-
384-
if (needsDependenciesForVirtualMethodImpls)
376+
if (!factory.VTable(defType).HasFixedSlots)
385377
{
386378
bool isNonInterfaceAbstractType = !defType.IsInterface && ((MetadataType)defType).IsAbstract;
387379

@@ -436,6 +428,12 @@ public sealed override IEnumerable<CombinedDependencyListEntry> GetConditionalSt
436428
((System.Collections.IStructuralEquatable)defType.RuntimeInterfaces).Equals(_type.RuntimeInterfaces,
437429
EqualityComparer<DefType>.Default));
438430

431+
// Interfaces don't have vtables and we don't need to track their instance method slot use.
432+
// The only exception are those interfaces that provide IDynamicInterfaceCastable implementations;
433+
// those have slots and we dispatch on them.
434+
bool needsDependenciesForInstanceInterfaceMethodImpls = !defType.IsInterface
435+
|| ((MetadataType)defType).IsDynamicInterfaceCastableImplementation();
436+
439437
// Add conditional dependencies for interface methods the type implements. For example, if the type T implements
440438
// interface IFoo which has a method M1, add a dependency on T.M1 dependent on IFoo.M1 being called, since it's
441439
// possible for any IFoo object to actually be an instance of T.
@@ -456,6 +454,9 @@ public sealed override IEnumerable<CombinedDependencyListEntry> GetConditionalSt
456454

457455
bool isStaticInterfaceMethod = interfaceMethod.Signature.IsStatic;
458456

457+
if (!isStaticInterfaceMethod && !needsDependenciesForInstanceInterfaceMethodImpls)
458+
continue;
459+
459460
MethodDesc implMethod = isStaticInterfaceMethod ?
460461
defType.ResolveInterfaceMethodToStaticVirtualMethodOnType(interfaceMethod) :
461462
defType.ResolveInterfaceMethodToVirtualMethodOnType(interfaceMethod);

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

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public static bool MightHaveInterfaceDispatchMap(TypeDesc type, NodeFactory fact
7373
if (!type.IsArray && !type.IsDefType)
7474
return false;
7575

76-
// Interfaces don't have a dispatch map because we dispatch them based on the
76+
// Interfaces don't have a dispatch map for instance methods because we dispatch them based on the
7777
// dispatch map of the implementing class.
7878
// The only exception are IDynamicInterfaceCastable scenarios that dispatch
7979
// using the interface dispatch map.
@@ -83,8 +83,9 @@ public static bool MightHaveInterfaceDispatchMap(TypeDesc type, NodeFactory fact
8383
// wasn't marked as [DynamicInterfaceCastableImplementation]" and "we couldn't find an
8484
// implementation". We don't want to use the custom attribute for that at runtime because
8585
// that's reflection and this should work without reflection.
86-
if (type.IsInterface)
87-
return ((MetadataType)type).IsDynamicInterfaceCastableImplementation();
86+
bool isInterface = type.IsInterface;
87+
if (isInterface && ((MetadataType)type).IsDynamicInterfaceCastableImplementation())
88+
return true;
8889

8990
DefType declType = type.GetClosestDefType();
9091

@@ -112,6 +113,11 @@ public static bool MightHaveInterfaceDispatchMap(TypeDesc type, NodeFactory fact
112113

113114
Debug.Assert(declMethod.IsVirtual);
114115

116+
// Only static methods get placed in dispatch maps of interface types (modulo
117+
// IDynamicInterfaceCastable we already handled above).
118+
if (isInterface && !declMethod.Signature.IsStatic)
119+
continue;
120+
115121
if (interfaceOnDefinitionType != null)
116122
declMethod = factory.TypeSystemContext.GetMethodForInstantiatedType(declMethod.GetTypicalMethodDefinition(), interfaceOnDefinitionType);
117123

@@ -154,6 +160,10 @@ private void EmitDispatchMap(ref ObjectDataBuilder builder, NodeFactory factory)
154160
var staticImplementations = new List<(int InterfaceIndex, int InterfaceMethodSlot, int ImplMethodSlot, int Context)>();
155161
var staticDefaultImplementations = new List<(int InterfaceIndex, int InterfaceMethodSlot, int ImplMethodSlot, int Context)>();
156162

163+
bool isInterface = declType.IsInterface;
164+
bool needsEntriesForInstanceInterfaceMethodImpls = !isInterface
165+
|| ((MetadataType)declType).IsDynamicInterfaceCastableImplementation();
166+
157167
// Resolve all the interfaces, but only emit non-static and non-default implementations
158168
for (int interfaceIndex = 0; interfaceIndex < declTypeRuntimeInterfaces.Length; interfaceIndex++)
159169
{
@@ -166,6 +176,10 @@ private void EmitDispatchMap(ref ObjectDataBuilder builder, NodeFactory factory)
166176
for (int interfaceMethodSlot = 0; interfaceMethodSlot < virtualSlots.Count; interfaceMethodSlot++)
167177
{
168178
MethodDesc declMethod = virtualSlots[interfaceMethodSlot];
179+
180+
if (!declMethod.Signature.IsStatic && !needsEntriesForInstanceInterfaceMethodImpls)
181+
continue;
182+
169183
if(!interfaceType.IsTypeDefinition)
170184
declMethod = factory.TypeSystemContext.GetMethodForInstantiatedType(declMethod.GetTypicalMethodDefinition(), (InstantiatedType)interfaceDefinitionType);
171185

@@ -244,9 +258,17 @@ private void EmitDispatchMap(ref ObjectDataBuilder builder, NodeFactory factory)
244258
// For default interface methods, the generic context is acquired by indexing
245259
// into the interface list of the owning type.
246260
Debug.Assert(providingInterfaceDefinitionType != null);
247-
int indexOfInterface = Array.IndexOf(declTypeDefinitionRuntimeInterfaces, providingInterfaceDefinitionType);
248-
Debug.Assert(indexOfInterface >= 0);
249-
genericContext = StaticVirtualMethodContextSource.ContextFromFirstInterface + indexOfInterface;
261+
if (declTypeDefinition.HasSameTypeDefinition(providingInterfaceDefinitionType) &&
262+
providingInterfaceDefinitionType == declTypeDefinition.InstantiateAsOpen())
263+
{
264+
genericContext = StaticVirtualMethodContextSource.ContextFromThisClass;
265+
}
266+
else
267+
{
268+
int indexOfInterface = Array.IndexOf(declTypeDefinitionRuntimeInterfaces, providingInterfaceDefinitionType);
269+
Debug.Assert(indexOfInterface >= 0);
270+
genericContext = StaticVirtualMethodContextSource.ContextFromFirstInterface + indexOfInterface;
271+
}
250272
}
251273
staticDefaultImplementations.Add((
252274
interfaceIndex,

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,17 +108,21 @@ public bool BuildSealedVTableSlots(NodeFactory factory, bool relocsOnly)
108108

109109
_sealedVTableEntries = new List<SealedVTableEntry>();
110110

111-
// Interfaces don't have any virtual slots with the exception of interfaces that provide
111+
// Interfaces don't have any instance virtual slots with the exception of interfaces that provide
112112
// IDynamicInterfaceCastable implementation.
113113
// Normal interface don't need one because the dispatch is done at the class level.
114114
// For IDynamicInterfaceCastable, we don't have an implementing class.
115-
if (_type.IsInterface && !((MetadataType)_type).IsDynamicInterfaceCastableImplementation())
116-
return true;
115+
bool isInterface = declType.IsInterface;
116+
bool needsEntriesForInstanceInterfaceMethodImpls = !isInterface
117+
|| ((MetadataType)declType).IsDynamicInterfaceCastableImplementation();
117118

118119
IReadOnlyList<MethodDesc> virtualSlots = factory.VTable(declType).Slots;
119120

120121
for (int i = 0; i < virtualSlots.Count; i++)
121122
{
123+
if (!virtualSlots[i].Signature.IsStatic && !needsEntriesForInstanceInterfaceMethodImpls)
124+
continue;
125+
122126
MethodDesc implMethod = declType.FindVirtualFunctionTargetMethodOnObjectType(virtualSlots[i]);
123127

124128
if (implMethod.CanMethodBeInSealedVTable())
@@ -143,6 +147,10 @@ public bool BuildSealedVTableSlots(NodeFactory factory, bool relocsOnly)
143147
for (int interfaceMethodSlot = 0; interfaceMethodSlot < virtualSlots.Count; interfaceMethodSlot++)
144148
{
145149
MethodDesc declMethod = virtualSlots[interfaceMethodSlot];
150+
151+
if (!declMethod.Signature.IsStatic && !needsEntriesForInstanceInterfaceMethodImpls)
152+
continue;
153+
146154
if (!interfaceType.IsTypeDefinition)
147155
declMethod = factory.TypeSystemContext.GetMethodForInstantiatedType(declMethod.GetTypicalMethodDefinition(), (InstantiatedType)interfaceDefinitionType);
148156

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/VirtualMethodCallHelper.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,8 @@ private static int GetNumberOfSlotsInCurrentType(NodeFactory factory, TypeDesc i
9393
{
9494
if (implType.IsInterface)
9595
{
96-
// We normally don't need to ask about vtable slots of interfaces. It's not wrong to ask
97-
// that question, but we currently only ask it for IDynamicInterfaceCastable implementations.
98-
Debug.Assert(((MetadataType)implType).IsDynamicInterfaceCastableImplementation());
96+
// Interface types don't have physically assigned virtual slots, so the number of slots
97+
// is always 0. They may have sealed slots.
9998
return (implType.HasGenericDictionarySlot() && countDictionarySlots) ? 1 : 0;
10099
}
101100

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ public static int Run()
5252
TestMoreConstraints.Run();
5353
TestSimpleNonGeneric.Run();
5454
TestSimpleGeneric.Run();
55+
TestDefaultDynamicStaticNonGeneric.Run();
56+
TestDefaultDynamicStaticGeneric.Run();
5557
TestDynamicStaticGenericVirtualMethods.Run();
5658

5759
return Pass;
@@ -1502,6 +1504,77 @@ public static void Run()
15021504
}
15031505
}
15041506

1507+
class TestDefaultDynamicStaticNonGeneric
1508+
{
1509+
interface IFoo
1510+
{
1511+
abstract static string ImHungryGiveMeCookie();
1512+
}
1513+
1514+
interface IBar : IFoo
1515+
{
1516+
static string IFoo.ImHungryGiveMeCookie() => "IBar";
1517+
}
1518+
1519+
class Baz : IBar
1520+
{
1521+
}
1522+
1523+
class Gen<T> where T : IFoo
1524+
{
1525+
public static string GrabCookie() => T.ImHungryGiveMeCookie();
1526+
}
1527+
1528+
public static void Run()
1529+
{
1530+
var r = (string)typeof(Gen<>).MakeGenericType(typeof(Baz)).GetMethod("GrabCookie").Invoke(null, Array.Empty<object>());
1531+
if (r != "IBar")
1532+
throw new Exception(r);
1533+
1534+
r = (string)typeof(Gen<>).MakeGenericType(typeof(IBar)).GetMethod("GrabCookie").Invoke(null, Array.Empty<object>());
1535+
if (r != "IBar")
1536+
throw new Exception(r);
1537+
}
1538+
}
1539+
1540+
class TestDefaultDynamicStaticGeneric
1541+
{
1542+
class Atom1 { }
1543+
class Atom2 { }
1544+
1545+
interface IFoo
1546+
{
1547+
abstract static string ImHungryGiveMeCookie();
1548+
}
1549+
1550+
interface IBar<T> : IFoo
1551+
{
1552+
static string IFoo.ImHungryGiveMeCookie() => $"IBar<{typeof(T).Name}>";
1553+
}
1554+
1555+
class Baz<T> : IBar<T>
1556+
{
1557+
}
1558+
1559+
class Gen<T> where T : IFoo
1560+
{
1561+
public static string GrabCookie() => T.ImHungryGiveMeCookie();
1562+
}
1563+
1564+
public static void Run()
1565+
{
1566+
Activator.CreateInstance(typeof(Baz<>).MakeGenericType(typeof(Atom1)));
1567+
1568+
var r = (string)typeof(Gen<>).MakeGenericType(typeof(Baz<>).MakeGenericType(typeof(Atom1))).GetMethod("GrabCookie").Invoke(null, Array.Empty<object>());
1569+
if (r != "IBar<Atom1>")
1570+
throw new Exception(r);
1571+
1572+
r = (string)typeof(Gen<>).MakeGenericType(typeof(IBar<>).MakeGenericType(typeof(Atom2))).GetMethod("GrabCookie").Invoke(null, Array.Empty<object>());
1573+
if (r != "IBar<Atom2>")
1574+
throw new Exception(r);
1575+
}
1576+
}
1577+
15051578
class TestDynamicStaticGenericVirtualMethods
15061579
{
15071580
interface IEntry

0 commit comments

Comments
 (0)