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

[release/7.0] [mono] Assert that we don't need to inflate types when applying DIM overrides #74519

Merged
merged 9 commits into from
Aug 25, 2022
44 changes: 20 additions & 24 deletions src/mono/mono/metadata/class-setup-vtable.c
Original file line number Diff line number Diff line change
Expand Up @@ -932,7 +932,8 @@ mono_class_setup_vtable_full (MonoClass *klass, GList *in_setup)
context = mono_class_get_context (klass);
type_token = mono_class_get_generic_class (klass)->container_class->type_token;
} else {
context = (MonoGenericContext *) mono_class_try_get_generic_container (klass); //FIXME is this a case of a try?
MonoGenericContainer *container = mono_class_try_get_generic_container (klass); //FIXME is this a case of a try?
context = container ? &container->context : NULL;
type_token = klass->type_token;
}

Expand Down Expand Up @@ -1010,30 +1011,14 @@ apply_override (MonoClass *klass, MonoClass *override_class, MonoMethod **vtable
MonoMethod *prev_override = (MonoMethod*)g_hash_table_lookup (map, decl);
MonoClass *prev_override_class = (MonoClass*)g_hash_table_lookup (class_map, decl);

g_assert (override_class == override->klass);

g_hash_table_insert (map, decl, override);
g_hash_table_insert (class_map, decl, override_class);

/* Collect potentially conflicting overrides which are introduced by default interface methods */
if (prev_override) {
ERROR_DECL (error);

/*
* The override methods are part of the generic definition, need to inflate them so their
* parent class becomes the actual interface/class containing the override, i.e.
* IFace<T> in:
* class Foo<T> : IFace<T>
* This is needed so the mono_class_is_assignable_from_internal () calls in the
* conflict resolution work.
*/
if (mono_class_is_ginst (override_class)) {
override = mono_class_inflate_generic_method_checked (override, &mono_class_get_generic_class (override_class)->context, error);
mono_error_assert_ok (error);
}

if (mono_class_is_ginst (prev_override_class)) {
prev_override = mono_class_inflate_generic_method_checked (prev_override, &mono_class_get_generic_class (prev_override_class)->context, error);
mono_error_assert_ok (error);
}
g_assert (prev_override->klass == prev_override_class);

if (!*conflict_map)
*conflict_map = g_hash_table_new (mono_aligned_addr_hash, NULL);
Expand Down Expand Up @@ -1771,10 +1756,9 @@ mono_class_setup_vtable_general (MonoClass *klass, MonoMethod **overrides, int o
MonoMethod *override = iface_overrides [i*2 + 1];
if (mono_class_is_gtd (override->klass)) {
override = mono_class_inflate_generic_method_full_checked (override, ic, mono_class_get_context (ic), error);
} else if (decl->is_inflated) {
override = mono_class_inflate_generic_method_checked (override, mono_method_get_context (decl), error);
mono_error_assert_ok (error);
}
}
// there used to be code here to inflate decl if decl->is_inflated, but in https://github.com/dotnet/runtime/pull/64102#discussion_r790019545 we
// think that this does not correspond to any real code.
if (!apply_override (klass, ic, vtable, decl, override, &override_map, &override_class_map, &conflict_map))
goto fail;
}
Expand All @@ -1786,6 +1770,18 @@ mono_class_setup_vtable_general (MonoClass *klass, MonoMethod **overrides, int o
MonoMethod *decl = overrides [i*2];
MonoMethod *override = overrides [i*2 + 1];
if (MONO_CLASS_IS_INTERFACE_INTERNAL (decl->klass)) {
/*
* We expect override methods that are part of a generic definition, to have
* their parent class be the actual interface/class containing the override,
* i.e.
*
* IFace<T> in:
* class Foo<T> : IFace<T>
*
* This is needed so the mono_class_is_assignable_from_internal () calls in the
* conflict resolution work.
*/
g_assert (override->klass == klass);
if (!apply_override (klass, klass, vtable, decl, override, &override_map, &override_class_map, &conflict_map))
goto fail;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,19 @@
// derived interface contexts, but the order is changed (or different.)
// When this occurs the generic info is incorrect for the inflated method.

// TestClass2 tests a regression due to the fix for the previous
// regression that caused Mono to incorrectly instantiate generic
// interfaces that appeared in the MethodImpl table

class Program
{
static int Main(string[] args)
{
return new TestClass().DoTest();
int result = new TestClass().DoTest();
if (result != 100)
return result;
result = new TestClass2().DoTest();
return result;
}
}

Expand Down Expand Up @@ -78,4 +86,33 @@ public int DoTest ()
Console.WriteLine("Passed => 100");
return 100;
}
}
}

public interface IA
{
public int Foo();
}

public interface IB<T> : IA
{
int IA.Foo() { return 104; }
}

public interface IC<H1, H2> : IB<H2>
{
int IA.Foo() { return 105; }
}

public class C<U, V, W> : IC<V, W>
{
int IA.Foo() { return 100; }
}

public class TestClass2
{
public int DoTest()
{
IA c = new C<byte, short, int>();
return c.Foo();
}
}
4 changes: 2 additions & 2 deletions src/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -2019,10 +2019,10 @@
<Issue>These tests are not supposed to be run with mono.</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/reflection/DefaultInterfaceMethods/Emit/**">
<Issue>needs triage</Issue>
<Issue>https://github.com/dotnet/runtime/issues/36113</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/reflection/DefaultInterfaceMethods/InvokeConsumer/**">
<Issue>needs triage</Issue>
<Issue>https://github.com/dotnet/runtime/issues/36113</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/reflection/Modifiers/modifiers/**">
<Issue>needs triage</Issue>
Expand Down