Skip to content

Commit

Permalink
Fix ordering in default interface method lookup (#67379)
Browse files Browse the repository at this point in the history
* Fix ordering in default interface method lookup

The previous implementation would find a default implementation in the current type before looking for non-default implementation in the base. Default implementations should be looked at last.

In fact the default implementation in the current type is unreachable if there's a non-default implementation in the base type - the compiler shouldn't even have bothered to emit it into the dispatch map. That can be a separate fix - this fix is still logically correct and necessary to get variant dispatch corner cases correctly.

* Regression test
  • Loading branch information
MichalStrehovsky committed Mar 31, 2022
1 parent 43ab6b8 commit 631389f
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,16 @@ public static IntPtr FindInterfaceMethodImplementationTarget(MethodTable* pTgtTy
if (pItfType->IsCloned)
pItfType = pItfType->CanonicalEEType;

// We first look at non-default implementation. Default implementations are only considered
// if the "old algorithm" didn't come up with an answer.
bool fDoDefaultImplementationLookup = false;

again:
while (pCur != null)
{
ushort implSlotNumber;
if (FindImplSlotForCurrentType(
pCur, pItfType, itfSlotNumber, &implSlotNumber))
pCur, pItfType, itfSlotNumber, fDoDefaultImplementationLookup, &implSlotNumber))
{
IntPtr targetMethod;
if (implSlotNumber < pCur->NumVtableSlots)
Expand Down Expand Up @@ -63,13 +68,23 @@ public static IntPtr FindInterfaceMethodImplementationTarget(MethodTable* pTgtTy
else
pCur = pCur->NonArrayBaseType;
}

// If we haven't found an implementation, do a second pass looking for a default implementation.
if (!fDoDefaultImplementationLookup)
{
fDoDefaultImplementationLookup = true;
pCur = pTgtType;
goto again;
}

return IntPtr.Zero;
}


private static bool FindImplSlotForCurrentType(MethodTable* pTgtType,
MethodTable* pItfType,
ushort itfSlotNumber,
bool fDoDefaultImplementationLookup,
ushort* pImplSlotNumber)
{
bool fRes = false;
Expand All @@ -87,17 +102,11 @@ private static bool FindImplSlotForCurrentType(MethodTable* pTgtType,

if (pTgtType->HasDispatchMap)
{
// We first look at non-default implementation. Default implementations are only considered
// if the "old algorithm" didn't come up with an answer.

bool fDoDefaultImplementationLookup = false;

// For variant interface dispatch, the algorithm is to walk the parent hierarchy, and at each level
// attempt to dispatch exactly first, and then if that fails attempt to dispatch variantly. This can
// result in interesting behavior such as a derived type only overriding one particular instantiation
// and funneling all the dispatches to it, but its the algorithm.

again:
bool fDoVariantLookup = false; // do not check variance for first scan of dispatch map

fRes = FindImplSlotInSimpleMap(
Expand All @@ -109,13 +118,6 @@ private static bool FindImplSlotForCurrentType(MethodTable* pTgtType,
fRes = FindImplSlotInSimpleMap(
pTgtType, pItfType, itfSlotNumber, pImplSlotNumber, fDoVariantLookup, fDoDefaultImplementationLookup);
}

// If we haven't found anything and haven't looked at the default implementations yet, look now
if (!fRes && !fDoDefaultImplementationLookup)
{
fDoDefaultImplementationLookup = true;
goto again;
}
}

return fRes;
Expand Down
10 changes: 10 additions & 0 deletions src/tests/nativeaot/SmokeTests/UnitTests/Interfaces.cs
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,13 @@ interface IFoo<T>

class Foo<T> : IFoo<T> { }

class Base : IFoo
{
int IFoo.GetNumber() => 100;
}

class Derived : Base, IBar { }

public static void Run()
{
Console.WriteLine("Testing default interface methods...");
Expand All @@ -508,6 +515,9 @@ public static void Run()
if (((IFoo)new Baz()).GetNumber() != 100)
throw new Exception();

if (((IFoo)new Derived()).GetNumber() != 100)
throw new Exception();

if (((IFoo<object>)new Foo<object>()).GetInterfaceType() != typeof(IFoo<object>))
throw new Exception();

Expand Down

0 comments on commit 631389f

Please sign in to comment.