Skip to content
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

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

Merged
merged 2 commits into from
Apr 20, 2023
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
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>