Bug 29585

Summary: Type.GetMethods returns generic method that cannot be specialized
Product: [Mono] Runtime Reporter: Matthew Evans <matthew>
Component: DebuggerAssignee: Rodrigo Kumpera <kumpera>
Status: RESOLVED FIXED    
Severity: normal CC: kumpera, masafa, mono-bugs+mono, mono-bugs+runtime
Priority: ---    
Version: 4.0.0   
Target Milestone: ---   
Hardware: All   
OS: All   
Tags: Is this bug a regression?: ---
Last known good build:
Attachments: Test case
Minimal Test Case
Drop class Derived

Description Matthew Evans 2015-04-30 07:31:50 UTC
Created attachment 10976 [details]
Test case

Output on CLR v4.0.30319:

matched = Boolean Visit[TInput,TOutput](TestCase.ConvertNode`2[TInput,TOutput])
matched.DeclaringType = TestCase.AbstractRoutingEngineVisitor`1[TestCase.MatchJoinNode`1[TestCase.DerivedInterface]]
matched.IsGenericMethod = True
matched.IsGenericMethodDefinition = True
matched = Boolean Visit[T](TestCase.AlphaNode`1[T])
matched.DeclaringType = TestCase.AbstractRoutingEngineVisitor`1[TestCase.MatchJoinNode`1[TestCase.DerivedInterface]]
matched.IsGenericMethod = True
matched.IsGenericMethodDefinition = True
result = TestCase.JoinNode`1[TestCase.DerivedInterface]

Output on Mono 3.12.1:

matched = Boolean Visit[TInput,TOutput](TestCase.ConvertNode`2[TInput,TOutput])
matched.DeclaringType = TestCase.AbstractRoutingEngineVisitor`1[TestCase.MatchJoinNode`1[TestCase.DerivedInterface]]
matched.IsGenericMethod = True
matched.IsGenericMethodDefinition = True
matched = Boolean Visit[T](TestCase.AlphaNode`1[T])
matched.DeclaringType = TestCase.AbstractRoutingEngineVisitor`1[TestCase.MatchJoinNode`1[TestCase.DerivedInterface]]
matched.IsGenericMethod = True
matched.IsGenericMethodDefinition = False
System.InvalidOperationException: not a generic method definition
at System.Reflection.MonoMethod.MakeGenericMethod (System.Type[]) <IL 0x000c9, 0x0018f>
at TestCase.TypeSpecializationExtensions.ToSpecializedMethod (System.Reflection.MethodInfo,object[]) [0x00050] in /var/repositories/Stact/src/TestCase/Program.cs:390
at TestCase.AbstractRoutingEngineVisitor`1<TestCase.MatchJoinNode`1<TestCase.DerivedInterface>>.Visit (object) <0x0028b>
at TestCase.AbstractRoutingEngineVisitor`1<TestCase.MatchJoinNode`1<TestCase.DerivedInterface>>.Visit<TestCase.DerivedInterface, TestCase.BaseInterface> (TestCase.ConvertNode`2<TestCase.DerivedInterface, TestCase.BaseInterface>) <0x00033>
at (wrapper dynamic-method) object.lambda_method (System.Runtime.CompilerServices.Closure,TestCase.MatchJoinNode`1<TestCase.DerivedInterface>,object) <IL 0x00007, 0x00056>
at TestCase.AbstractRoutingEngineVisitor`1<TestCase.MatchJoinNode`1<TestCase.DerivedInterface>>.Visit (object) <0x00459>
at TestCase.AbstractRoutingEngineVisitor`1<TestCase.MatchJoinNode`1<TestCase.DerivedInterface>>.Visit<TestCase.DerivedInterface> (TestCase.AlphaNode`1<TestCase.DerivedInterface>) <0x00093>
at TestCase.MatchJoinNode`1<TestCase.DerivedInterface>..ctor (TestCase.AlphaNode`1<TestCase.DerivedInterface>,System.Action`1<TestCase.JoinNode`1<TestCase.DerivedInterface>>) <0x00057>
at TestCase.MainClass/<Main>c__AnonStorey0.<>m__0 (TestCase.AlphaNode`1<TestCase.DerivedInterface>) [0x0000e] in /var/repositories/Stact/src/TestCase/Program.cs:30
at TestCase.MatchAlphaNode`1<TestCase.DerivedInterface>..ctor (System.Action`1<TestCase.AlphaNode`1<TestCase.DerivedInterface>>) <0x00075>
at TestCase.MainClass.Main (string[]) [0x0001b] in /var/repositories/Stact/src/TestCase/Program.cs:30

Apologies for the giant test case, this is a hack n' slashed version of one of the tests from https://github.com/phatboyg/Stact.

Changing AbstractRoutingEngineVisitor<TVisitor>.Visit<T>(AlphaNode<T> node) to not be virtual causes it to work correctly.
Comment 1 Matthew Evans 2015-04-30 19:27:49 UTC
Created attachment 11002 [details]
Minimal Test Case

I spent some more time squeezing the test case down, this is as tight as I can get it. However, it now needs to be run with --debug=mdb-optimizations to reproduce.

Output on CLR v4.0.30319:

method = Void FindGenericMethod[T]()
method.DeclaringType = TestCase.Abstract`1[TestCase.GenericDerived`1[TestCase.Param]]
method.IsGenericMethod = True
method.IsGenericMethodDefinition = True

Output on Mono 3.12.1:

matched = Void FindGenericMethod[T]()
matched.DeclaringType = TestCase.Abstract`1[TestCase.GenericDerived`1[TestCase.Param]]
matched.IsGenericMethod = True
matched.IsGenericMethodDefinition = False
Comment 2 Matthew Evans 2015-04-30 19:42:40 UTC
Created attachment 11003 [details]
Drop class Derived

Seems like the Derived class was just causing some side-effect that disabled the right optimizations. Removed it completely.
Comment 3 Marek Safar 2015-05-07 12:39:12 UTC
I can reproduce it only when using --debug=mdb-optimizations
Comment 4 Zoltan Varga 2015-05-07 21:02:37 UTC
This is because of this line in class.c:1082:

	if (!context->method_inst && method->is_generic)
		iresult->context.method_inst = mono_method_get_generic_container (method)->context.method_inst;

This causes us to search the method cache for a method with a different context than the one passed in by the caller, so we randomly return a different method instance.
Comment 5 Rodrigo Kumpera 2016-04-14 07:02:12 UTC
The test case works with mono 4.4.
Comment 6 Matthew Evans 2016-04-14 17:00:55 UTC
It appears to still fail with --debug=mdb-optimizations

$ mcs --version
Mono C# compiler version 4.4.0.0
$ mcs 11003.cs
$ mono --version
Mono JIT compiler version 4.4.0 (mono-4.4.0-branch/a3fabf1 Fri Apr  8 13:48:29 EDT 2016)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
	TLS:           normal
	SIGSEGV:       altstack
	Notification:  kqueue
	Architecture:  x86
	Disabled:      none
	Misc:          softdebug
	LLVM:          yes(3.6.0svn-mono-master/a173357)
	GC:            sgen
$ mono 11003.exe
method = Void FindGenericMethod[T]()
method.DeclaringType = TestCase.Abstract`1[TestCase.GenericDerived`1[TestCase.Param]]
method.IsGenericMethod = True
method.IsGenericMethodDefinition = True
$ mono --debug=mdb-optimizations 11003.exe
method = Void FindGenericMethod[T]()
method.DeclaringType = TestCase.Abstract`1[TestCase.GenericDerived`1[TestCase.Param]]
method.IsGenericMethod = True
method.IsGenericMethodDefinition = False
Comment 7 Rodrigo Kumpera 2016-04-15 00:58:47 UTC
OMG, this turned out to be a mess.

PR with a fix: https://github.com/mono/mono/pull/2893
Comment 8 Rodrigo Kumpera 2016-04-15 22:28:31 UTC
Merged into master.