Skip to content

Fix type loader not recognizing overridden method #56337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions src/coreclr/vm/methodtable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7950,7 +7950,8 @@ MethodTable::MethodData::ProcessMap(
UINT32 cTypeIDs,
MethodTable * pMT,
UINT32 iCurrentChainDepth,
MethodDataEntry * rgWorkingData)
MethodDataEntry * rgWorkingData,
size_t cWorkingData)
{
LIMITED_METHOD_CONTRACT;

Expand All @@ -7961,10 +7962,8 @@ MethodTable::MethodData::ProcessMap(
if (it.Entry()->GetTypeID() == rgTypeIDs[nTypeIDIndex])
{
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))
// This if check is defensive, and should never fail
if (curSlot < cWorkingData)
{
MethodDataEntry * pCurEntry = &rgWorkingData[curSlot];
if (!pCurEntry->IsDeclInit() && !pCurEntry->IsImplInit())
Expand Down Expand Up @@ -8331,7 +8330,7 @@ MethodTable::MethodDataInterfaceImpl::PopulateNextLevel()

if (m_cDeclTypeIDs != 0)
{ // We got the TypeIDs from TypeLoader, use them
ProcessMap(m_rgDeclTypeIDs, m_cDeclTypeIDs, pMTCur, iChainDepth, GetEntryData());
ProcessMap(m_rgDeclTypeIDs, m_cDeclTypeIDs, pMTCur, iChainDepth, GetEntryData(), GetNumVirtuals());
}
else
{ // We should decode all interface duplicates of code:m_pDecl
Expand All @@ -8348,7 +8347,7 @@ MethodTable::MethodDataInterfaceImpl::PopulateNextLevel()
INDEBUG(dbg_fInterfaceFound = TRUE);
DispatchMapTypeID declTypeID = DispatchMapTypeID::InterfaceClassID(it.GetIndex());

ProcessMap(&declTypeID, 1, pMTCur, iChainDepth, GetEntryData());
ProcessMap(&declTypeID, 1, pMTCur, iChainDepth, GetEntryData(), GetNumVirtuals());
}
}
// The interface code:m_Decl should be found at least once in the interface map of code:m_pImpl,
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/vm/methodtable.h
Original file line number Diff line number Diff line change
Expand Up @@ -3242,7 +3242,8 @@ public :
UINT32 cTypeIDs,
MethodTable * pMT,
UINT32 cCurrentChainDepth,
MethodDataEntry * rgWorkingData);
MethodDataEntry * rgWorkingData,
size_t cWorkingData);
}; // class MethodData

typedef ::Holder < MethodData *, MethodData::HolderAcquire, MethodData::HolderRelease > MethodDataHolder;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Linq;

namespace BugInReflection
{
class Program
{
static int Main(string[] args)
{
// This tests the ability to load a type when
// 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.
new BlogPost();
return 100;
}
}

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 { }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<OutputType>Exe</OutputType>
<CLRTestKind>BuildAndRun</CLRTestKind>
<CLRTestPriority>1</CLRTestPriority>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>