Skip to content

Commit

Permalink
[mono][metadata] Fix lookup of matching interface method with covaria…
Browse files Browse the repository at this point in the history
…nt returns (#84160)

* [mono][metadata] Fix lookup of matching interface method with covariant returns

When a class is implementing an interface, we lookup up a matching method (with exact signature) in the class methods and inherited virtual methods. This doesn't handle the situation where the interface was matching a virtual method declared in one of the parents, which was then overriden with a covariant return method. If that is the case, the covariant return method is considered as the implementation of the interface method.

* Add test
  • Loading branch information
BrzVlad committed Apr 20, 2023
1 parent 66ba9d5 commit 7222061
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 0 deletions.
27 changes: 27 additions & 0 deletions src/mono/mono/metadata/class-setup-vtable.c
Original file line number Diff line number Diff line change
Expand Up @@ -1918,6 +1918,33 @@ mono_class_setup_vtable_general (MonoClass *klass, MonoMethod **overrides, int o
}
}

if ((vtable [im_slot] == NULL) && klass->parent != NULL) {
// For covariant returns we might need to lookup matching virtual methods in parent types
// that were overriden with a method that doesn't exactly match interface method signature.
gboolean found = FALSE;
for (MonoClass *parent_klass = klass->parent; parent_klass != NULL && !found; parent_klass = parent_klass->parent) {
gpointer iter = NULL;
while ((cm = mono_class_get_virtual_methods (parent_klass, &iter))) {
TRACE_INTERFACE_VTABLE ((cm != NULL) && printf (" For slot %d ('%s'.'%s':'%s'), trying (ancestor) method '%s'.'%s':'%s'... ", im_slot, ic->name_space, ic->name, im->name, cm->klass->name_space, cm->klass->name, cm->name));
if ((cm != NULL) && check_interface_method_override (klass, im, cm, MONO_ITF_OVERRIDE_SLOT_EMPTY)) {
TRACE_INTERFACE_VTABLE (printf ("[everything ok]: ASSIGNING\n"));
found = TRUE;
if (vtable [cm->slot]) {
// We match the current method was overriding it. If this method will
// get overriden again, the interface slot will also be updated
vtable [im_slot] = vtable [cm->slot];
} else {
// We add abstract method in the vtable. This method will be overriden
// with the actual implementation once we resolve the abstract method later.
// FIXME If klass is abstract, we can end up with abstract method in the vtable. Is this a problem ?
vtable [im_slot] = cm;
}
break;
}
}
}
}

if (vtable [im_slot] == NULL) {
if (!(im->flags & METHOD_ATTRIBUTE_ABSTRACT)) {
TRACE_INTERFACE_VTABLE (printf (" Using default iface method %s.\n", mono_method_full_name (im, 1)));
Expand Down
118 changes: 118 additions & 0 deletions src/tests/Regressions/coreclr/GitHub_78635/test78635.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;

public abstract class BaseProp
{
}

public class DerivedProp : BaseProp
{
}

public interface ICov
{
BaseProp GetMyProp ();
}

// Abstract
public abstract class BaseAbstract
{
public abstract BaseProp GetMyProp ();
}

// TEST1
public class DerivedAbstract1 : BaseAbstract, ICov
{
public override DerivedProp GetMyProp () { Console.WriteLine ("DERIVED"); return new DerivedProp (); }
}

// TEST2
public class DerivedAbstract21 : BaseAbstract
{
public override DerivedProp GetMyProp () { Console.WriteLine ("DERIVED21"); return new DerivedProp (); }
}

public class DerivedAbstract22 : DerivedAbstract21, ICov
{
public override DerivedProp GetMyProp () { Console.WriteLine ("DERIVED22"); return new DerivedProp (); }
}

// TEST3
public abstract class DerivedAbstract31 : BaseAbstract, ICov
{
}

public class DerivedAbstract32 : DerivedAbstract31
{
public override DerivedProp GetMyProp () { Console.WriteLine ("DERIVED32"); return new DerivedProp (); }
}

// Virtual
public abstract class BaseVirtual
{
public virtual BaseProp GetMyProp () { Console.WriteLine ("BASE"); return null; }
}

// TEST1
public class DerivedVirtual1 : BaseVirtual, ICov
{
public override DerivedProp GetMyProp () { Console.WriteLine ("DERIVED"); return new DerivedProp (); }
}

// TEST2
public class DerivedVirtual21 : BaseVirtual
{
public override DerivedProp GetMyProp () { Console.WriteLine ("DERIVED21"); return new DerivedProp (); }
}

public class DerivedVirtual22 : DerivedVirtual21, ICov
{
public override DerivedProp GetMyProp () { Console.WriteLine ("DERIVED22"); return new DerivedProp (); }
}

// TEST3
public abstract class DerivedVirtual31 : BaseVirtual, ICov
{
}

public class DerivedVirtual32 : DerivedVirtual31
{
public override DerivedProp GetMyProp () { Console.WriteLine ("DERIVED32"); return new DerivedProp (); }
}


public class Program {

public static void TestInstance (ICov derived)
{
BaseProp prop = derived.GetMyProp ();
if (prop == null)
throw new Exception ("Invalid return");
}

public static void Test ()
{
TestInstance (new DerivedAbstract1 ());
TestInstance (new DerivedAbstract22 ());
TestInstance (new DerivedAbstract32 ());
TestInstance (new DerivedVirtual1 ());
TestInstance (new DerivedVirtual22 ());
TestInstance (new DerivedVirtual32 ());
}

public static int Main (string[] args)
{
try {
Test ();
} catch (Exception e) {
Console.WriteLine ("NOK");
Console.WriteLine (e);
return 1;
}
Console.WriteLine ("OK");
return 100;
}
}

8 changes: 8 additions & 0 deletions src/tests/Regressions/coreclr/GitHub_78635/test78635.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
</PropertyGroup>
<ItemGroup>
<Compile Include="test78635.cs" />
</ItemGroup>
</Project>

0 comments on commit 7222061

Please sign in to comment.