From 7222061f18b8342a0bd4edbd37df318fd43c47d3 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 20 Apr 2023 10:19:06 +0300 Subject: [PATCH] [mono][metadata] Fix lookup of matching interface method with covariant 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 --- src/mono/mono/metadata/class-setup-vtable.c | 27 ++++ .../coreclr/GitHub_78635/test78635.cs | 118 ++++++++++++++++++ .../coreclr/GitHub_78635/test78635.csproj | 8 ++ 3 files changed, 153 insertions(+) create mode 100644 src/tests/Regressions/coreclr/GitHub_78635/test78635.cs create mode 100644 src/tests/Regressions/coreclr/GitHub_78635/test78635.csproj diff --git a/src/mono/mono/metadata/class-setup-vtable.c b/src/mono/mono/metadata/class-setup-vtable.c index ec6745b77d079..9a235c1ec0cce 100644 --- a/src/mono/mono/metadata/class-setup-vtable.c +++ b/src/mono/mono/metadata/class-setup-vtable.c @@ -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))); diff --git a/src/tests/Regressions/coreclr/GitHub_78635/test78635.cs b/src/tests/Regressions/coreclr/GitHub_78635/test78635.cs new file mode 100644 index 0000000000000..66bb3708de3bb --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_78635/test78635.cs @@ -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; + } +} + diff --git a/src/tests/Regressions/coreclr/GitHub_78635/test78635.csproj b/src/tests/Regressions/coreclr/GitHub_78635/test78635.csproj new file mode 100644 index 0000000000000..6ac141631869e --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_78635/test78635.csproj @@ -0,0 +1,8 @@ + + + Exe + + + + +