Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
6 changes: 3 additions & 3 deletions src/mono/mono/metadata/class-setup-vtable.c
Original file line number Diff line number Diff line change
Expand Up @@ -1742,11 +1742,11 @@ mono_class_setup_vtable_general (MonoClass *klass, MonoMethod **overrides, int o
for (int i = 0; i < iface_onum; i++) {
MonoMethod *decl = iface_overrides [i*2];
MonoMethod *override = iface_overrides [i*2 + 1];
if (decl->is_inflated) {
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);
} else if (mono_class_is_gtd (override->klass)) {
override = mono_class_inflate_generic_method_full_checked (override, ic, mono_class_get_context (ic), error);
}
Comment on lines +1747 to 1750
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not certain that we need the decl->is_inflated case at all. I can not think of a scenario where this is needed. If we can find the testcase I would like to add that to the tests in this PR as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did change the code locally to assert if we hit the decl->is_inflated case and override changed. I then ran all the tests and I do think this this assert was hit and I got the same results. Depending on how well dims are covered with our test code, this code block may really be unnecessary.

} else if (decl->is_inflated) {
	MonoMethod *toverride = mono_class_inflate_generic_method_checked (override, mono_method_get_context (decl), error);
	g_assert(toverride == override);
	override = toverride;
	mono_error_assert_ok (error);
}

Copy link
Member

@lambdageek lambdageek Feb 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll do a separate PR for net7 with

} else if (decl->is_inflated) {
	g_assert_not_reached ();
}

we'll backport the current version to mono/mono and net6

if (!apply_override (klass, ic, vtable, decl, override, &override_map, &override_class_map, &conflict_map))
goto fail;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;

// In GH issue 61244 the mono runtime aborted when inflating the default
// interface method because the context used was from the base interface.

// The OneArgBaseInterface portion of this test handles the original bug
// where the base interface has less generic arguments than the derived
// interface and the runtime aborts.

// The SecondInterface portion of this test handles an additional scenario
// where the number of generic arguments are the same in the base and
// derived interface contexts, but the order is changed (or different.)
// When this occurs the generic info is incorrect for the inflated method.

class Program
{
static int Main(string[] args)
{
return new TestClass().DoTest();
}
}

public interface OneArgBaseInterface<T1>
{
int SomeFunc1(T1 someParam1, Type someParam1Type);
}

public interface TwoArgBaseInterface<T1, T2>
{
int SomeFunc1(T1 someParam1, T2 someParam2, Type someParam1Type, Type someParam2Type);
}

public interface SecondInterface<TParam2Type, TParam1Type> : OneArgBaseInterface<TParam1Type>, TwoArgBaseInterface<TParam1Type, TParam2Type>
{
int OneArgBaseInterface<TParam1Type>.SomeFunc1(TParam1Type someParam1, Type someParam1Type)
{
if (typeof(TParam1Type) != someParam1Type)
{
Console.WriteLine("Failed => 101");
return 101;
}

return 100;
}

int TwoArgBaseInterface<TParam1Type, TParam2Type>.SomeFunc1(TParam1Type someParam1, TParam2Type someParam2, Type someParam1Type, Type someParam2Type)
{
if (typeof(TParam1Type) != someParam1Type)
{
Console.WriteLine("Failed => 102");
return 102;
}

if (typeof(TParam2Type) != someParam2Type)
{
Console.WriteLine("Failed => 103");
return 103;
}

return 100;
}
}

public class TestClass : SecondInterface<int, string>
{
public int DoTest ()
{
int ret = (this as OneArgBaseInterface<string>).SomeFunc1("test string", typeof(string));
if (ret != 100)
return ret;

ret = (this as TwoArgBaseInterface<string, int>).SomeFunc1("test string", 0, typeof(string), typeof(int));
if (ret != 100)
return ret;

Console.WriteLine("Passed => 100");
return 100;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<OutputType>Exe</OutputType>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>