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] Issue #103365 follow-up PR #105006

Merged
merged 7 commits into from
Jul 21, 2024
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
100 changes: 66 additions & 34 deletions src/mono/mono/metadata/class-init.c
Original file line number Diff line number Diff line change
Expand Up @@ -4197,75 +4197,101 @@ 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.
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;
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;
lambdageek marked this conversation as resolved.
Show resolved Hide resolved
}

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) {
Expand All @@ -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;
}
}

/**
Expand Down
9 changes: 7 additions & 2 deletions src/mono/mono/metadata/class-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 1 addition & 2 deletions src/mono/mono/metadata/class-private-definition.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
46 changes: 28 additions & 18 deletions src/mono/mono/metadata/class.c
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/metadata/loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion src/mono/mono/mini/interp/interp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 };

Expand Down Expand Up @@ -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))) {
Expand Down
Loading