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

* do we need to inflate here? it seems like we always get the right class

* use member access, not type punning

* Add regression test for #70190

* Assert code from #64102 is unreachable

In #64102 (comment)
we concluded that this branch is never taken.

* Assert that overrides are already inflated how we expect

* try to enable some disabled DIM tests

* remove unused var

* Don't assert - code is reachable, there's just nothing to do

* Add link to issue for failing tests

Co-authored-by: Aleksey Kliger <[email protected]>
Co-authored-by: Aleksey Kliger <[email protected]>

Author: github-actions[bot]

src/mono/mono/metadata/class-setup-vtable.c

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

src/tests/Loader/classloader/DefaultInterfaceMethods/regressions/github61244.cs

@@ -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;
     }
 }
 
@@ -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();
+    }
+}

src/tests/issues.targets

@@ -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>