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.

Sign up for GitHub

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] Check for additional implemented variant interfaces #57086

Merged
merged 4 commits into from
Aug 17, 2021
Merged

[mono] Check for additional implemented variant interfaces #57086

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
afc5cee
[class-setup-vtable] Use flags bitmask in check_interface_method_over…
lambdageek Aug 9, 2021
f1d453e
[class-setup-vtable] Check additional slots when explicitly implement…
lambdageek Aug 9, 2021
a6142c2
oops, disable vtable tracing
lambdageek Aug 9, 2021
022e2a7
Use explicit variant interface to implement base type interfaces
lambdageek Aug 16, 2021
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
76 changes: 70 additions & 6 deletions src/mono/mono/metadata/class-setup-vtable.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,9 +447,42 @@ is_wcf_hack_disabled (void)
return disabled == 1;
}

enum MonoInterfaceMethodOverrideFlags {
MONO_ITF_OVERRIDE_REQUIRE_NEWSLOT = 0x01,
MONO_ITF_OVERRIDE_EXPLICITLY_IMPLEMENTED = 0x02,
MONO_ITF_OVERRIDE_SLOT_EMPTY = 0x04,
MONO_ITF_OVERRIDE_VARIANT_ITF = 0x08,
};

static gboolean
signature_is_subsumed (MonoMethod *impl_method, MonoMethod *decl_method, MonoError *error);

/*
* Returns TRUE if the signature of \c decl is assignable from the signature of \c impl. That is, if \c sig_impl is more
* specific than \c sig_decl.
*/
static gboolean
check_interface_method_override (MonoClass *klass, MonoMethod *im, MonoMethod *cm, gboolean require_newslot, gboolean interface_is_explicitly_implemented_by_class, gboolean slot_is_empty)
signature_assignable_from (MonoMethod *decl, MonoMethod *impl)
{
ERROR_DECL (error);
/* FIXME: the "signature_is_subsumed" check for covariant returns is not good enough:
* 1. It doesn't check that the arguments are contravariant.
* 2. it's too general: we don't want Foo SomeMethod() to be subsumed by Bar SomeMethod() unless
* at least of Foo or Bar was a generic parameter.
*/
if (signature_is_subsumed (impl, decl, error))
return TRUE;
mono_error_cleanup (error);
return FALSE;
}

static gboolean
check_interface_method_override (MonoClass *klass, MonoMethod *im, MonoMethod *cm, int flags)
{
gboolean require_newslot = (flags & MONO_ITF_OVERRIDE_REQUIRE_NEWSLOT) != 0;
gboolean interface_is_explicitly_implemented_by_class = (flags & MONO_ITF_OVERRIDE_EXPLICITLY_IMPLEMENTED) != 0;
gboolean slot_is_empty = (flags & MONO_ITF_OVERRIDE_SLOT_EMPTY) != 0;
gboolean variant_itf = (flags & MONO_ITF_OVERRIDE_VARIANT_ITF) != 0;
MonoMethodSignature *cmsig, *imsig;
if (strcmp (im->name, cm->name) == 0) {
if (! (cm->flags & METHOD_ATTRIBUTE_PUBLIC)) {
Expand Down Expand Up @@ -477,7 +510,19 @@ check_interface_method_override (MonoClass *klass, MonoMethod *im, MonoMethod *c
return FALSE;
}

if (! mono_metadata_signature_equal (cmsig, imsig)) {
/* if there's a variant interface, the method could be using the generic param in a
* variant position compared to the interface sig
*
* public interface IFactory<out T> { T Get(); }
* public class Foo {}
* public class Bar : Foo {}
* public class FooFactory : IFactory<Foo> { public Foo Get() => new Foo(); }
* public class BarFactory : FooFactory, IFactory<Bar> { public new Bar Get() => new Bar(); }
*
* In this case, there's an explicit newslot but we want to match up 'Bar BarFactory:Get ()'
* with 'Foo IFactory<Foo>:Get ()'.
*/
if (! mono_metadata_signature_equal (cmsig, imsig) && !(variant_itf && signature_assignable_from (im, cm))) {
TRACE_INTERFACE_VTABLE (printf ("[SIGNATURE CHECK FAILED "));
TRACE_INTERFACE_VTABLE (print_method_signatures (im, cm));
TRACE_INTERFACE_VTABLE (printf ("]"));
Expand Down Expand Up @@ -1744,6 +1789,7 @@ mono_class_setup_vtable_general (MonoClass *klass, MonoMethod **overrides, int o
MonoClass *parent = klass->parent;
int ic_offset;
gboolean interface_is_explicitly_implemented_by_class;
gboolean variant_itf;
int im_index;

ic = klass->interfaces_packed [i];
Expand All @@ -1757,11 +1803,21 @@ mono_class_setup_vtable_general (MonoClass *klass, MonoMethod **overrides, int o
if (parent != NULL) {
int implemented_interfaces_index;
interface_is_explicitly_implemented_by_class = FALSE;
variant_itf = mono_class_has_variant_generic_params (ic);
for (implemented_interfaces_index = 0; implemented_interfaces_index < klass->interface_count; implemented_interfaces_index++) {
if (ic == klass->interfaces [implemented_interfaces_index]) {
interface_is_explicitly_implemented_by_class = TRUE;
break;
}
if (variant_itf) {
if (mono_class_is_variant_compatible (ic, klass->interfaces [implemented_interfaces_index], FALSE)) {
MonoClass *impl_itf;
impl_itf = klass->interfaces [implemented_interfaces_index];
TRACE_INTERFACE_VTABLE (printf (" variant interface '%s' is explicitly implemented by '%s'\n", mono_type_full_name (m_class_get_byval_arg (ic)), mono_type_full_name (m_class_get_byval_arg (impl_itf))));
interface_is_explicitly_implemented_by_class = TRUE;
break;
}
}
}
} else {
interface_is_explicitly_implemented_by_class = TRUE;
Expand All @@ -1787,8 +1843,16 @@ mono_class_setup_vtable_general (MonoClass *klass, MonoMethod **overrides, int o
for (l = virt_methods; l; l = l->next) {
cm = (MonoMethod *)l->data;
TRACE_INTERFACE_VTABLE (printf (" For slot %d ('%s'.'%s':'%s'), trying method '%s'.'%s':'%s'... [EXPLICIT IMPLEMENTATION = %d][SLOT IS NULL = %d]", im_slot, ic->name_space, ic->name, im->name, cm->klass->name_space, cm->klass->name, cm->name, interface_is_explicitly_implemented_by_class, (vtable [im_slot] == NULL)));
if (check_interface_method_override (klass, im, cm, TRUE, interface_is_explicitly_implemented_by_class, (vtable [im_slot] == NULL))) {
TRACE_INTERFACE_VTABLE (printf ("[check ok]: ASSIGNING"));
int flags;
flags = MONO_ITF_OVERRIDE_REQUIRE_NEWSLOT;
if (interface_is_explicitly_implemented_by_class)
flags |= MONO_ITF_OVERRIDE_EXPLICITLY_IMPLEMENTED;
if (interface_is_explicitly_implemented_by_class && variant_itf)
flags |= MONO_ITF_OVERRIDE_VARIANT_ITF;
if (vtable [im_slot] == NULL)
flags |= MONO_ITF_OVERRIDE_SLOT_EMPTY;
if (check_interface_method_override (klass, im, cm, flags)) {
TRACE_INTERFACE_VTABLE (printf ("[check ok]: ASSIGNING\n"));
vtable [im_slot] = cm;
/* Why do we need this? */
if (cm->slot < 0) {
Expand All @@ -1811,8 +1875,8 @@ mono_class_setup_vtable_general (MonoClass *klass, MonoMethod **overrides, int o
MonoMethod *cm = parent->vtable [cm_index];

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, FALSE, FALSE, TRUE)) {
TRACE_INTERFACE_VTABLE (printf ("[everything ok]: ASSIGNING"));
if ((cm != NULL) && check_interface_method_override (klass, im, cm, MONO_ITF_OVERRIDE_SLOT_EMPTY)) {
TRACE_INTERFACE_VTABLE (printf ("[everything ok]: ASSIGNING\n"));
vtable [im_slot] = cm;
/* Why do we need this? */
if (cm->slot < 0) {
Expand Down