diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c index 0f5643e663f0f4..94536ed5abbf32 100644 --- a/src/mono/mono/metadata/class-init.c +++ b/src/mono/mono/metadata/class-init.c @@ -3286,7 +3286,14 @@ mono_class_setup_interface_id_nolock (MonoClass *klass) * a != b ==> true */ const char *name = m_class_get_name (klass); - if (!strcmp (name, "IList`1") || !strcmp (name, "ICollection`1") || !strcmp (name, "IEnumerable`1") || !strcmp (name, "IEnumerator`1")) + if ( + !strcmp (name, "IList`1") || + !strcmp (name, "IReadOnlyList`1") || + !strcmp (name, "ICollection`1") || + !strcmp (name, "IReadOnlyCollection`1") || + !strcmp (name, "IEnumerable`1") || + !strcmp (name, "IEnumerator`1") + ) klass->is_array_special_interface = 1; } } @@ -4189,6 +4196,89 @@ mono_class_set_runtime_vtable (MonoClass *klass, MonoVTable *vtable) klass->runtime_vtable = vtable; } +static int +index_of_class (MonoClass *needle, MonoClass **haystack, int haystack_size) { + for (int i = 0; i < haystack_size; i++) + if (haystack[i] == needle) + return i; + + return -1; +} + +static void +build_variance_search_table_inner (MonoClass *klass, MonoClass **buf, int buf_size, int *buf_count) { + if (!m_class_is_interfaces_inited (klass)) { + ERROR_DECL (error); + mono_class_setup_interfaces (klass, error); + return_if_nok (error); + } + guint c = m_class_get_interface_count (klass); + if (c) { + MonoClass **ifaces = m_class_get_interfaces (klass); + for (guint i = 0; i < c; i++) { + MonoClass *iface = ifaces [i]; + // Avoid adding duplicates or recursing into them. + if (index_of_class (iface, buf, *buf_count) >= 0) + continue; + + if (mono_class_has_variant_generic_params (iface)) { + g_assert (*buf_count < buf_size); + buf[*buf_count] = iface; + (*buf_count) += 1; + } + + build_variance_search_table_inner (iface, buf, buf_size, buf_count); + } + } +} + +// Only call this with the loader lock held +static void +build_variance_search_table (MonoClass *klass) { + // FIXME: Is there a way to deterministically compute the right capacity? + int buf_size = m_class_get_interface_offsets_count (klass), buf_count = 0; + MonoClass **buf = g_alloca (buf_size * sizeof(MonoClass *)); + MonoClass **result = NULL; + memset (buf, 0, buf_size * sizeof(MonoClass *)); + build_variance_search_table_inner (klass, buf, buf_size, &buf_count); + + if (buf_count) { + guint bytes = buf_count * sizeof(MonoClass *); + result = mono_mem_manager_alloc (m_class_get_mem_manager (klass), bytes); + memcpy (result, buf, bytes); + } + klass->variant_search_table_length = buf_count; + klass->variant_search_table = result; + // Ensure we do not set the inited flag until we've stored the result pointer + mono_memory_barrier (); + klass->variant_search_table_inited = TRUE; +} + +void +mono_class_get_variance_search_table (MonoClass *klass, MonoClass ***table, int *table_size) { + g_assert (klass); + g_assert (table); + g_assert (table_size); + + // We will never do a variance search to locate a given interface on an interface, only on + // a fully-defined type or generic instance + if (m_class_is_interface (klass)) { + *table = NULL; + *table_size = 0; + return; + } + + if (!klass->variant_search_table_inited) { + mono_loader_lock (); + if (!klass->variant_search_table_inited) + build_variance_search_table (klass); + mono_loader_unlock (); + } + + *table = klass->variant_search_table; + *table_size = klass->variant_search_table_length; +} + /** * mono_classes_init: * diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index a6a908e8bab10f..7044e59262db65 100644 --- a/src/mono/mono/metadata/class-internals.h +++ b/src/mono/mono/metadata/class-internals.h @@ -1465,6 +1465,9 @@ mono_class_has_default_constructor (MonoClass *klass, gboolean public_only); gboolean mono_method_has_unmanaged_callers_only_attribute (MonoMethod *method); +void +mono_class_get_variance_search_table (MonoClass *klass, MonoClass ***table, int *table_size); + // There are many ways to do on-demand initialization. // Some allow multiple concurrent initializations. Some do not. // Some allow multiple concurrent writes to the global. Some do not. diff --git a/src/mono/mono/metadata/class-private-definition.h b/src/mono/mono/metadata/class-private-definition.h index 1e0c01ab336b3b..e4b04b0284b815 100644 --- a/src/mono/mono/metadata/class-private-definition.h +++ b/src/mono/mono/metadata/class-private-definition.h @@ -80,6 +80,7 @@ struct _MonoClass { guint has_deferred_failure : 1; /* next byte*/ guint is_exception_class : 1; /* is System.Exception or derived from it */ + guint variant_search_table_inited : 1; MonoClass *parent; MonoClass *nested_in; @@ -127,6 +128,9 @@ struct _MonoClass { /* Infrequently used items. See class-accessors.c: InfrequentDataKind for what goes into here. */ MonoPropertyBag infrequent_data; + + MonoClass **variant_search_table; + int variant_search_table_length; }; struct _MonoClassDef { diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 043338c14899d9..192153d1b0cc8d 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -1946,21 +1946,25 @@ mono_class_interface_offset (MonoClass *klass, MonoClass *itf) int mono_class_interface_offset_with_variance (MonoClass *klass, MonoClass *itf, gboolean *non_exact_match) { - int i = mono_class_interface_offset (klass, itf); + gboolean has_variance = mono_class_has_variant_generic_params (itf); + int exact_match = mono_class_interface_offset (klass, itf), i = -1; *non_exact_match = FALSE; - if (i >= 0) - return i; + + if (exact_match >= 0) { + if (!has_variance) + return exact_match; + } int klass_interface_offsets_count = m_class_get_interface_offsets_count (klass); - if (m_class_is_array_special_interface (itf) && m_class_get_rank (klass) < 2) { + if (m_class_is_array_special_interface (itf) && m_class_get_rank (klass) < 2) { MonoClass *gtd = mono_class_get_generic_type_definition (itf); int found = -1; for (i = 0; i < klass_interface_offsets_count; i++) { if (mono_class_is_variant_compatible (itf, m_class_get_interfaces_packed (klass) [i], FALSE)) { found = i; - *non_exact_match = TRUE; + *non_exact_match = (i != exact_match); break; } @@ -1971,7 +1975,7 @@ mono_class_interface_offset_with_variance (MonoClass *klass, MonoClass *itf, gbo for (i = 0; i < klass_interface_offsets_count; i++) { if (mono_class_get_generic_type_definition (m_class_get_interfaces_packed (klass) [i]) == gtd) { found = i; - *non_exact_match = TRUE; + *non_exact_match = (i != exact_match); break; } } @@ -1980,16 +1984,45 @@ mono_class_interface_offset_with_variance (MonoClass *klass, MonoClass *itf, gbo return -1; return m_class_get_interface_offsets_packed (klass) [found]; - } + } else if (has_variance) { + MonoClass **vst; + int vst_count; + MonoClass *current = klass; + + // Perform two passes per class, then check the base class + while (current) { + mono_class_get_variance_search_table (current, &vst, &vst_count); + + // Exact match pass: Is there an exact match at this level of the type hierarchy? + // If so, we can use the interface_offset we computed earlier, since we're walking from most derived to least. + for (i = 0; i < vst_count; i++) { + if (itf != vst [i]) + continue; + + *non_exact_match = FALSE; + return exact_match; + } - if (!mono_class_has_variant_generic_params (itf)) - return -1; + // Inexact match (variance) pass: + // Is any interface at this level of the type hierarchy variantly compatible with the desired interface? + // If so, select the first compatible one we find. + for (i = 0; i < vst_count; i++) { + if (!mono_class_is_variant_compatible (itf, vst [i], FALSE)) + continue; - for (i = 0; i < klass_interface_offsets_count; i++) { - if (mono_class_is_variant_compatible (itf, m_class_get_interfaces_packed (klass) [i], FALSE)) { - *non_exact_match = TRUE; - return m_class_get_interface_offsets_packed (klass) [i]; + int inexact_match = mono_class_interface_offset (klass, vst[i]); + g_assert (inexact_match != exact_match); + *non_exact_match = TRUE; + return inexact_match; + } + + // Now check base class if present + current = m_class_get_parent (current); } + + // If the variance search failed to find a match, return the exact match search result (probably -1). + *non_exact_match = (exact_match < 0); + return exact_match; } return -1; diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index 3728c995e9dd96..e50d64cdbbe3d3 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -619,7 +619,9 @@ get_virtual_method (InterpMethod *imethod, MonoVTable *vtable) g_assert (vtable->klass != m->klass); /* TODO: interface offset lookup is slow, go through IMT instead */ gboolean non_exact_match; - slot += mono_class_interface_offset_with_variance (vtable->klass, m->klass, &non_exact_match); + int ioffset = mono_class_interface_offset_with_variance (vtable->klass, m->klass, &non_exact_match); + g_assert (ioffset >= 0); + slot += ioffset; } MonoMethod *virtual_method = m_class_get_vtable (vtable->klass) [slot]; diff --git a/src/tests/Regressions/coreclr/103365/103365.cs b/src/tests/Regressions/coreclr/103365/103365.cs new file mode 100644 index 00000000000000..867ff68128ab34 --- /dev/null +++ b/src/tests/Regressions/coreclr/103365/103365.cs @@ -0,0 +1,51 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +/* +https://github.com/dotnet/runtime/issues/103365 +When using an interface with a generic out type, an explicit implementation, and a derived class, the base classes implementation is called instead of the derived class when running on Mono; CoreCLR has the expected behavior. +*/ + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using Xunit; + +public interface IBaseInterface +{ + string explicitDeclaration(); +} + +public class BasicBaseClass : IBaseInterface +{ + string className = "BasicBaseClass"; + string IBaseInterface.explicitDeclaration() + { + return className; + } +} + +public class BasicDerivedClass : BasicBaseClass, IBaseInterface +{ + string className = "BasicDerivedClass"; + + string IBaseInterface.explicitDeclaration() + { + return className; + } +} + +public static class Test_Issue103365 +{ + [Fact] + public static void Main () + { + var instances = new IBaseInterface[2]; + instances[0] = new BasicBaseClass(); + instances[1] = new BasicDerivedClass(); + Assert.Equal("BasicBaseClass", instances[0].explicitDeclaration()); + Assert.Equal("BasicDerivedClass", instances[1].explicitDeclaration()); + } +} + diff --git a/src/tests/Regressions/coreclr/103365/103365.csproj b/src/tests/Regressions/coreclr/103365/103365.csproj new file mode 100644 index 00000000000000..ba30d94c716f74 --- /dev/null +++ b/src/tests/Regressions/coreclr/103365/103365.csproj @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 5da277d04e87b4..0282e5821dba7a 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -1756,7 +1756,7 @@ needs triage - needs triage + ldtoken in default interface methods does not appear to work in minijit or interp needs triage