dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.16k stars 4.72k forks source link

`DiagnosticMethodInfo.Create(Delegate)` doesn't work with open instance delegates over methods on a generic type in CoreCLR #108619

Open SingleAccretion opened 2 weeks ago

SingleAccretion commented 2 weeks ago

Reproduction: [godbolt](https://godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(filename:'1',fontScale:14,fontUsePx:'0',j:1,lang:csharp,selection:(endColumn:1,endLineNumber:13,positionColumn:1,positionLineNumber:13,selectionStartColumn:1,selectionStartLineNumber:13,startColumn:1,startLineNumber:13),source:'%23nullable+disable%0A%0Ausing+System%3B%0Ausing+System.Diagnostics%3B%0A%0Aclass+Program%0A%7B%0A++++static+void+Main()%0A++++%7B%0A++++++++OpenInstanceDelStruct%3Cint%3E+s%3B%0A++++++++var+delMethod+%3D+typeof(OpenInstanceDelStruct%3Cint%3E).GetMethod(%22InstanceMethod%22)%3B%0A++++++++var+openDel+%3D+delMethod.CreateDelegate%3COpenInstanceDel%3E()%3B%0A++++++++var+openDmi+%3D+DiagnosticMethodInfo.Create(openDel)%3B%0A++++++++Console.WriteLine($%22%7BopenDmi.DeclaringTypeName%7D::%7BopenDmi.Name%7D%22)%3B%0A++++%7D%0A%0A++++delegate+void+OpenInstanceDel(ref+OpenInstanceDelStruct%3Cint%3E+del)%3B%0A%0A++++struct+OpenInstanceDelStruct%3CT%3E%0A++++%7B%0A++++++++public+void+InstanceMethod()+%7B+%7D%0A++++%7D%0A%7D%0A'),l:'5',n:'1',o:'C%23+source+%231',t:'0')),k:33.86931715084305,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((h:compiler,i:(compiler:dotnettrunkcsharpcoreclr,filters:(b:'0',binary:'1',binaryObject:'1',commentOnly:'0',debugCalls:'1',demangle:'0',directives:'0',execute:'0',intel:'0',libraryCode:'0',trim:'1',verboseDemangling:'0'),flagsViewOpen:'1',fontScale:14,fontUsePx:'0',j:3,lang:csharp,libs:!(),options:'',overrides:!(),selection:(endColumn:1,endLineNumber:1,positionColumn:1,positionLineNumber:1,selectionStartColumn:1,selectionStartLineNumber:1,startColumn:1,startLineNumber:1),source:1),l:'5',n:'0',o:'+.NET+CoreCLR+(main)+(Editor+%231)',t:'0')),k:32.79734951582363,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((h:output,i:(compilerName:'x86-64+clang+(trunk)',editorid:1,fontScale:14,fontUsePx:'0',j:3,wrap:'1'),l:'5',n:'0',o:'Output+of+.NET+CoreCLR+(main)+(Compiler+%233)',t:'0')),k:33.33333333333333,l:'4',n:'0',o:'',s:0,t:'0')),l:'2',n:'0',o:'',t:'0')),version:4).

Expected behavior: things work. Actual behavior:

Unhandled exception. System.ArgumentException: Type handle 'Program+OpenInstanceDelStruct`1[System.Int32]&' and method handle with declaring type 'Program+OpenInstanceDelStruct`1[System.Int32]' are incompatible. Get RuntimeMethodHandle and declaring RuntimeTypeHandle off the same MethodBase.
   at System.RuntimeType.GetMethodBase(RuntimeType reflectedType, RuntimeMethodHandleInternal methodHandle)
   at System.Delegate.GetMethodImpl()
   at System.Diagnostics.DiagnosticMethodInfo.Create(Delegate delegate)
   at Program.Main() in /app/example.cs:line 11
tommcdon commented 2 weeks ago

@MichalStrehovsky would you mind take a look?

MichalStrehovsky commented 2 weeks ago

This API doesn't do anything but call Delegate.Method property on CoreCLR. If it's throwing, it would be a reflection bug.

steveharter commented 1 week ago

Here's a minimal repro:

...
        MethodInfo delMethod = typeof(OpenInstanceDelStruct<int>).GetMethod("InstanceMethod");
        OpenInstanceDel openDel = delMethod.CreateDelegate<OpenInstanceDel>();
        MethodInfo mi = openDel.Method;
....

    delegate void OpenInstanceDel(ref OpenInstanceDelStruct<int> del);

    struct OpenInstanceDelStruct<T>
    {
        public void InstanceMethod() { }
    }
}

which also repros in v8 and v7.

steveharter commented 1 week ago

I moved this to Future; SingleAccretion is there priority for addressing this?

SingleAccretion commented 1 week ago

is there priority for addressing this?

Nothing in particular. I have just noticed this bug while working on stack trace support downstream.

steveharter commented 1 week ago

Note that changing:

MethodInfo mi = openDel.Method;

to

MethodInfo mi = openDel.GetType().GetMethod("Invoke")

works.

MichalStrehovsky commented 6 days ago

Note that changing:

MethodInfo mi = openDel.Method; to

MethodInfo mi = openDel.GetType().GetMethod("Invoke") works.

That returns a different MethodInfo - not the method the delegate points to. Not many scenarios can use it as a workaround since name, owning type and even parameter types are going to be all different. The only thing that's reliably same is parameter count.