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][metadata] Fixes for static virtual methods with interface variance #91664

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
32 changes: 32 additions & 0 deletions src/mono/mono/metadata/class-setup-vtable.c
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,37 @@ apply_override (MonoClass *klass, MonoClass *override_class, MonoMethod **vtable
return TRUE;
}

/*
* Interface method `decl` is overridden in `klass` with `override`. In addition to overriding the method slot
* for this interface method, if parent klass implements a different variant compatible interface, then we also
* need to override the correpsonding method slot in this other interface.
*/
static void
apply_override_variant_interface (MonoClass *klass, MonoMethod *decl, MonoMethod *override, MonoMethod **vtable)
{
MonoClass *parent = klass->parent;

// This handles variant overrides for static virtual methods only
if (!m_method_is_static (decl))
return;

if (mono_class_has_variant_generic_params (decl->klass) && parent) {
for (int k = 0; k < klass->parent->interface_offsets_count; k++) {
MonoClass *impl_itf = parent->interfaces_packed [k];
if (decl->klass != impl_itf &&
mono_class_has_variant_generic_params (impl_itf) &&
mono_class_get_generic_type_definition (impl_itf) == mono_class_get_generic_type_definition (decl->klass)) {
if (mono_class_is_variant_compatible (impl_itf, decl->klass, FALSE)) {
int vt_slot = parent->interface_offsets_packed [k] + decl->slot;
vtable [vt_slot] = override;
TRACE_INTERFACE_VTABLE (printf ("\tvariance override slot %d with method %s\n", vt_slot, mono_method_full_name (override, TRUE)));
break;
}
}
}
}
}

static void
handle_dim_conflicts (MonoMethod **vtable, MonoClass *klass, GHashTable *conflict_map)
{
Expand Down Expand Up @@ -1788,6 +1819,7 @@ mono_class_setup_vtable_general (MonoClass *klass, MonoMethod **overrides, int o
g_assert (override->klass == klass);
if (!apply_override (klass, klass, vtable, decl, override, &override_map, &override_class_map, &conflict_map))
goto fail;
apply_override_variant_interface (klass, decl, override, vtable);
}
}

Expand Down
39 changes: 36 additions & 3 deletions src/mono/mono/metadata/class.c
Original file line number Diff line number Diff line change
Expand Up @@ -1921,6 +1921,31 @@ mono_class_interface_offset (MonoClass *klass, MonoClass *itf)
return -1;
}

static MonoClass*
lookup_variant_compatible_interface (MonoClass *klass, MonoClass *itf)
{
if (!m_class_is_interfaces_inited (klass)) {
ERROR_DECL(error);
mono_class_setup_interfaces (klass, error);
if (!is_ok (error)) {
mono_error_cleanup (error);
return NULL;
}
}
for (int i = 0; i < m_class_get_interface_count (klass); i++) {
MonoClass *impl_itf = m_class_get_interfaces (klass) [i];
if (mono_class_is_variant_compatible (itf, impl_itf, FALSE))
return impl_itf;

// Look up also in all the interfaces it implements
impl_itf = lookup_variant_compatible_interface (impl_itf, itf);
if (impl_itf)
return impl_itf;
}

return NULL;
}

/**
* mono_class_interface_offset_with_variance:
*
Expand Down Expand Up @@ -1973,11 +1998,19 @@ mono_class_interface_offset_with_variance (MonoClass *klass, MonoClass *itf, gbo
if (!mono_class_has_variant_generic_params (itf))
return -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)) {
// Look up first variant interface in the current class, and then in parent slots.
while (klass) {
MonoClass *impl_itf = lookup_variant_compatible_interface (klass, itf);
if (impl_itf) {
// Found compatible implemented interface. Look up its slot in the vtable
*non_exact_match = TRUE;
return m_class_get_interface_offsets_packed (klass) [i];
for (i = 0; i < klass_interface_offsets_count; i++) {
if (m_class_get_interfaces_packed (klass) [i] == impl_itf)
return m_class_get_interface_offsets_packed (klass) [i];
}
}

klass = m_class_get_parent (klass);
}

return -1;
Expand Down
6 changes: 0 additions & 6 deletions src/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -1192,9 +1192,6 @@
<ExcludeList Include="$(XunitTestBinBase)/Loader/classloader/StaticVirtualMethods/Reabstraction/**">
<Issue>https://github.com/dotnet/runtime/issues/88775</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Loader/classloader/StaticVirtualMethods/InterfaceVariance/**">
<Issue>https://github.com/dotnet/runtime/issues/88689</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Regressions/coreclr/GitHub_85240/test85240/**">
<Issue>https://github.com/dotnet/runtime/issues/71095</Issue>
</ExcludeList>
Expand Down Expand Up @@ -1297,9 +1294,6 @@
<ExcludeList Include="$(XunitTestBinBase)/Loader/classloader/StaticVirtualMethods/DiamondShape/**">
<Issue>https://github.com/dotnet/runtime/issues/80350</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Loader/classloader/StaticVirtualMethods/InterfaceVariance/**">
<Issue>Static virtual methods are not yet implemented in the Mono runtime.</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Loader/classloader/StaticVirtualMethods/NegativeTestCases/**">
<Issue>Static virtual methods are not yet implemented in the Mono runtime.</Issue>
</ExcludeList>
Expand Down
Loading