Multiple default implementation properties in an interface, applied to an abstract hierarchy, causes TypeLoadException

[vslee]

Description

Consider an interface which uses the default implementation feature on some temporary variables. If too many (5+) of those variables are used, and the interface is applied to an inheritance hierarchy which contains an abstract ancestor, then a reflection type bug is exposed. Please see the code example below (doing any of the three things in the code comments fixes the problem):

Configuration

Target framework: .NET 5.0 RTM
Operating system: Windows 10 Pro 20H2 x64
IDE: Visual Studio 2019 16.8.0

Regression?

Does not seem to be a regression as the same issue occurred in .NET core 3.1

Other information

using System.Linq;

namespace BugInReflection
{
    class Program
    {
        static void Main(string[] args)
        {
            var p = typeof(Program).GetProperties().Select(e => e.GetIndexParameters()).ToList();
        }

        public BlogPost BlogPost { get; set; }
    }

    public interface ITitle
    {
        // commenting out one or more of these NotMapped properties fixes the problem
        public string Temp1 => "abcd";
        public string Temp2 => "abcd";
        public string Temp3 => "abcd";
        public string Temp4 => "abcd";
        public string Temp5 => "abcd";

        public string Title { get; set; } // commenting out this property also fixes the problem
    }

    public abstract class Post : ITitle // making this non-abstract also fixes the problem
    {
        public string Title { get; set; }
    }
    public class BlogPost : Post { }
}

System.TypeLoadException
  HResult=0x80131522
  Message=Method 'set_Title' in type 'BugInReflection.BlogPost' from assembly 'DefaultImplementationOnAbstract, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' does not have an implementation.
  Source=System.Private.CoreLib
  StackTrace:
   at System.Signature.GetSignature(Void* pCorSig, Int32 cCorSig, RuntimeFieldHandleInternal fieldHandle, IRuntimeMethodInfo methodHandle, RuntimeType declaringType)
   at System.Reflection.RuntimeMethodInfo.FetchNonReturnParameters()
   at System.Reflection.RuntimeMethodInfo.GetParametersNoCopy()
   at System.Reflection.RuntimePropertyInfo.GetIndexParametersNoCopy()
   at System.Reflection.RuntimePropertyInfo.GetIndexParameters()
   at BugInReflection.Program.<>c.<Main>b__0_0(PropertyInfo e) in G:\Google Drive\TestProjects\InterfaceOnDerived\DefaultImplementationOnAbstract\Program.cs:line 9
   at System.Linq.Enumerable.SelectArrayIterator`2.ToList()
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at BugInReflection.Program.Main(String[] args) in G:\Google Drive\TestProjects\InterfaceOnDerived\DefaultImplementationOnAbstract\Program.cs:line 9

[ajcvickers]

Note: this impacts EF Core model building. See dotnet/efcore#23261

[MichalStrehovsky]

This is not reflection, it's type system. Reduced repro:

using System;
using System.Linq;

namespace BugInReflection
{
    class Program
    {
        static void Main(string[] args)
        {
            new BlogPost();
        }
    }

    public interface ITitle
    {
        // commenting out one or more of these NotMapped properties fixes the problem
        public string Temp1 => "abcd";
        public string Temp2 => "abcd";
        public string Temp3 => "abcd";
        public string Temp4 => "abcd";
        public string Temp5 => "abcd";

        public string Title { get; set; } // commenting out this property also fixes the problem
    }

    public abstract class Post : ITitle // making this non-abstract also fixes the problem
    {
        public string Title { get; set; }
    }
    public class BlogPost : Post { }
}

[wzchua]

I believe the bug is here.
GetTargetSlotNumber() seems to be the virtual slot number

runtime/src/coreclr/vm/methodtable.cpp

Lines 7960 to 7971 in f3bad4f

	UINT32 curSlot = it.Entry()->GetSlotNumber();
	// If we're processing an interface, or it's for a virtual, or it's for a non-virtual
	// for the most derived type, we want to process the entry. In other words, we
	// want to ignore non-virtuals for parent classes.
	if ((curSlot < pMT->GetNumVirtuals()) || (iCurrentChainDepth == 0))
	{
	MethodDataEntry * pCurEntry = &rgWorkingData[curSlot];
	if (!pCurEntry->IsDeclInit() && !pCurEntry->IsImplInit())
	{
	pCurEntry->SetImplData(it.Entry()->GetTargetSlotNumber());
	}
	}

[davidwrighton]

I'll take a look at this. This isn't something I'm comfortable pushing to 7.0

[davidwrighton]

The issue is reproduces under the following situation.

1. The type implements an interface
2. The interface has more virtual methods on it than the number of virtual methods
   on the base type of the type.
3. The base type implements the interface partially (and the partial implementation
   has a slot number greater than the number of virtual methods on the base type + its base types)
4. The type does not re-implement the interface methods implemented by the base type.

This is permitted in IL, but is a situation which can only be reached with .il code in versions of
.NET prior to .NET 5.

In .NET 5, this became straightforward to hit with default interface methods.
To workaround the bug in .NET 5, simply make the Post class have enough virtual methods to match
the number of virtual methods on the ITitle interface.