diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c index 94536ed5abbf3..76459536c2497 100644 --- a/src/mono/mono/metadata/class-init.c +++ b/src/mono/mono/metadata/class-init.c @@ -4197,24 +4197,38 @@ mono_class_set_runtime_vtable (MonoClass *klass, MonoVTable *vtable) } static int -index_of_class (MonoClass *needle, MonoClass **haystack, int haystack_size) { +index_of_class (MonoClass *needle, MonoVarianceSearchTableEntry *haystack, int haystack_size) { for (int i = 0; i < haystack_size; i++) - if (haystack[i] == needle) + if (haystack[i].klass == 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); +append_variance_entry (MonoClass *klass, MonoVarianceSearchTableEntry *buf, int buf_size, int *buf_count, MonoClass *value) { + int index = *buf_count; + (*buf_count) += 1; + g_assert (index < buf_size); + buf[index].klass = value; + buf[index].offset = value ? mono_class_interface_offset (klass, value) : 0; +} + +static gboolean +build_variance_search_table_inner (MonoClass *klass, MonoVarianceSearchTableEntry *buf, int buf_size, int *buf_count, MonoClass *current, gboolean terminator) { + // We have to track separately whether the buffer contains any actual interfaces, since we're appending + // null terminators between levels of the inheritance hierarchy + gboolean result = FALSE; + + while (current) { + if (!m_class_is_interfaces_inited (current)) { + ERROR_DECL (error); + mono_class_setup_interfaces (current, error); + return_val_if_nok (error, FALSE); + } + + guint c = m_class_get_interface_count (current); + MonoClass **ifaces = m_class_get_interfaces (current); for (guint i = 0; i < c; i++) { MonoClass *iface = ifaces [i]; // Avoid adding duplicates or recursing into them. @@ -4222,50 +4236,62 @@ build_variance_search_table_inner (MonoClass *klass, MonoClass **buf, int buf_si continue; if (mono_class_has_variant_generic_params (iface)) { - g_assert (*buf_count < buf_size); - buf[*buf_count] = iface; - (*buf_count) += 1; + append_variance_entry (klass, buf, buf_size, buf_count, iface); + result = TRUE; } - build_variance_search_table_inner (iface, buf, buf_size, buf_count); + if (build_variance_search_table_inner (klass, buf, buf_size, buf_count, iface, FALSE)) + result = TRUE; + } + + current = current->parent; + + if (terminator) { + // HACK: Don't append another NULL if we already have one, it's unnecessary + if (*buf_count && buf[(*buf_count) - 1].klass != NULL) + append_variance_entry (klass, buf, buf_size, buf_count, NULL); } } + + return result; } +typedef struct VarianceSearchTable { + int count; + MonoVarianceSearchTableEntry entries[1]; // a total of count items, at least 1 +} VarianceSearchTable; + // 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); + int buf_size = m_class_get_interface_offsets_count (klass) + klass->idepth + 1, buf_count = 0; + MonoVarianceSearchTableEntry *buf = g_alloca (buf_size * sizeof(MonoVarianceSearchTableEntry)); + VarianceSearchTable *result = NULL; + memset (buf, 0, buf_size * sizeof(MonoVarianceSearchTableEntry)); + gboolean any_items = build_variance_search_table_inner (klass, buf, buf_size, &buf_count, klass, TRUE); + + if (any_items) { + guint bytes = (buf_count * sizeof(MonoVarianceSearchTableEntry)); + result = (VarianceSearchTable *)mono_mem_manager_alloc (m_class_get_mem_manager (klass), bytes + sizeof(VarianceSearchTable)); + result->count = buf_count; + memcpy (result->entries, 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) { +MonoVarianceSearchTableEntry * +mono_class_get_variance_search_table (MonoClass *klass, 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; + return NULL; } if (!klass->variant_search_table_inited) { @@ -4275,8 +4301,14 @@ mono_class_get_variance_search_table (MonoClass *klass, MonoClass ***table, int mono_loader_unlock (); } - *table = klass->variant_search_table; - *table_size = klass->variant_search_table_length; + VarianceSearchTable *vst = klass->variant_search_table; + if (vst) { + *table_size = vst->count; + return vst->entries; + } else { + *table_size = 0; + return NULL; + } } /** diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index 7044e59262db6..fc967bb0adb5f 100644 --- a/src/mono/mono/metadata/class-internals.h +++ b/src/mono/mono/metadata/class-internals.h @@ -1465,8 +1465,13 @@ 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); +typedef struct _MonoVarianceSearchTableEntry { + MonoClass *klass; + guint16 offset; +} MonoVarianceSearchTableEntry; + +MonoVarianceSearchTableEntry * +mono_class_get_variance_search_table (MonoClass *klass, int *table_size); // There are many ways to do on-demand initialization. // Some allow multiple concurrent initializations. Some do not. diff --git a/src/mono/mono/metadata/class-private-definition.h b/src/mono/mono/metadata/class-private-definition.h index e4b04b0284b81..829ac16d12903 100644 --- a/src/mono/mono/metadata/class-private-definition.h +++ b/src/mono/mono/metadata/class-private-definition.h @@ -129,8 +129,7 @@ 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; + void *variant_search_table; }; struct _MonoClassDef { diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 5e789d2af9638..834cf8688cd3e 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -1985,39 +1985,49 @@ mono_class_interface_offset_with_variance (MonoClass *klass, MonoClass *itf, gbo 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); - + int vst_count, offset = 0; + MonoVarianceSearchTableEntry *vst = mono_class_get_variance_search_table (klass, &vst_count); + + // The variance search table is a buffer containing all interfaces with in/out params in the type's inheritance + // hierarchy, with a NULL separator between each level of the hierarchy. This allows us to skip recursing down + // the whole chain and avoid performing duplicate compatibility checks, since duplicates are stripped from the + // buffer. To comply with the spec, we do an exact-match pass and then a variance pass for each level in the + // hierarchy, then move on to the next level and do two passes for that one. + while (offset < 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]) + for (i = offset; i < vst_count; i++) { + // If we hit a null, that's the next level in the inheritance hierarchy, so stop there + if (vst [i].klass == NULL) + break; + + if (itf != vst [i].klass) continue; *non_exact_match = FALSE; + g_assert (vst [i].offset == exact_match); return exact_match; } // 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)) + for (i = offset; i < vst_count; i++) { + // If we hit a null, that's the next level in the inheritance hierarchy, so bump offset past it + if (vst [i].klass == NULL) { + offset = i + 1; + break; + } + + if (!mono_class_is_variant_compatible (itf, vst [i].klass, FALSE)) continue; - int inexact_match = mono_class_interface_offset (klass, vst[i]); - g_assert (inexact_match != exact_match); - *non_exact_match = TRUE; + int inexact_match = vst [i].offset; + // FIXME: Is it correct that this is possible? + // g_assert (inexact_match != exact_match); + *non_exact_match = inexact_match != exact_match; 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). diff --git a/src/mono/mono/metadata/loader.c b/src/mono/mono/metadata/loader.c index 467fe0e668c07..921fe29e2743e 100644 --- a/src/mono/mono/metadata/loader.c +++ b/src/mono/mono/metadata/loader.c @@ -1303,6 +1303,7 @@ get_method_constrained (MonoImage *image, MonoMethod *method, MonoClass *constra g_assert (itf_slot >= 0); gboolean variant = FALSE; int itf_base = mono_class_interface_offset_with_variance (constrained_class, base_class, &variant); + g_assert (itf_base >= 0); vtable_slot = itf_slot + itf_base; } g_assert (vtable_slot >= 0); diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index 1e31ba4cdca65..61e1ff55d2f9b 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -625,6 +625,8 @@ get_virtual_method (InterpMethod *imethod, MonoVTable *vtable) } MonoMethod *virtual_method = m_class_get_vtable (vtable->klass) [slot]; + g_assert (virtual_method); + if (m->is_inflated && mono_method_get_context (m)->method_inst) { MonoGenericContext context = { NULL, NULL }; @@ -7479,7 +7481,7 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK; if (local_cmethod->is_generic || mono_class_is_gtd (local_cmethod->klass)) { MonoException *ex = mono_exception_from_name_msg (mono_defaults.corlib, "System", "InvalidOperationException", ""); THROW_EX (ex, ip); - } + } // FIXME push/pop LMF if (G_UNLIKELY (mono_method_has_unmanaged_callers_only_attribute (local_cmethod))) {