Calli with thiscall convention fails at runtime #33129
Comments
Dotnet-GitSync-Bot
added
area-Interop-coreclr
untriaged
|
This is likely related to #12375 |
|
After more investigating, this is actually an interop bug with the IL stubs. I'll fix it. |
jkoritzinsky
removed
the
untriaged
|
After I've fixed the interop IL stub bug (PR incoming), there's an additional bug here. On .NET Core, the program outputs 00 instead of 12.3. |
|
The second bug would fall on the JIT's support of thiscall. cc: @davidwrighton |
|
@dotnet/jit-contrib Could someone look at the incorrect behavior that @jkoritzinsky is talking about here? |
AaronRobinsonMSFT
added
area-CodeGen-coreclr
Updating tag based on this statement. /cc @dotnet/jit-contrib |
|
I'll take a look. |
|
I think there is a disconnect between the pinvoke and reverse pinvoke stubs. The pinvoke stub passes &S, &ReturnWrapper, float to its calli. Here the intent is that &S and float are the inputs and the &ReturnWrapper is the result. The reverse pinvoke stub writes its result back to &S and not to &ReturnWrapper: Not sure who gets it wrong, but seems more likely the ReverseStub. |
|
Are you using the above IL stub as referenced? I fixed the issue with the reverse PInvoke stub so the IL is valid, but the test still fails since the native "instance method" convention (which thiscall implies) on Windows (excluding Windows ARM) always uses a return buffer for structure types; however, the JIT doesn't account for that when emitting the code for a thiscall calli. |
|
The above is from ef72b95 (May 27), so let me update and see. |
|
If the program is still throwing an InvalidProgramException then there's another bug somewhere that we need to fix. |
|
Just retried this with recent master, still getting 00 for both x86 and x64. In the IL for the reverse pinvoke stub, I am guessing arg1 is the pointer for the result, but the IL never refers to it...? Seems like in the "UnmarshalReturn" section it should be |
|
I found another bug in the IL emit that incorrectly accounted for the return buffer offset. I'll put out a PR in a few minutes. This only fixes the bug for when IL stubs are used. If the native function were adorned with |
The m_paramidx value is always -1 for return values, so we ignore it when emitting in case we are using a byref return value that needs a real arg index (which is 0-based). Fixes dotnet#33129
|
@jkoritzinsky I think fixing #12375 would be a good thing. We would need to get that going soon though. Do you know what the JIT would need to do? Is there prior art on what needs to happen? |
|
I was just chatting with @CarolEidt about #12375 and apparently the code to implement it would be spread across the JIT in various locations. |
|
@jkoritzinsky so your fix in #38829 will fix this particular test case? If so, I'll assign this back to you and re-label. For broader thiscall support, it would be good to clarify what exactly the convention is for non-x86, non-windows, since technically those ABIs don't have thiscall. |
|
Yes, #38829 will fix this specific case. The broader support is being able to call function pointers pointing to native instance methods that return structs on Windows x86, x64, and ARM64 and being able to define reverse-P/Invoke delegates marked with |
AndyAyersMS
added
area-Interop-coreclr
and removed
area-CodeGen-coreclr
|
We'd like to be able to define as part of the "extensible calling convention" support in .NET 6 an "instance stdcall" (stdcall but struct return values are always returned byref) calling convention for Windows since that's technically what COM uses (the differences are visible in DirectX) to enable people like @tannergooding to provide DirectX bindings without having to manually account for the calling convention issues. In cases like |
|
@jkoritzinsky, should that comment be a new issue tracking support for I think Stdcall is the primary, but IIRC Cdecl and Fastcalll were slightly tweaked as well (although I've never seen them in practice). |
|
I think we have an issue tracking it but we haven’t decided on the final shape (if we’re going to do Thiscall + Stdcall or make a new type CallConvInstanceMethod to avoid confusion) |
Is this an internal issue? If not, can you link it? I couldn't find it anywhere on the runtime repo. |
|
@jkoritzinsky Doesn't look like we have had an official request at the moment. We have one for In fact the semantics here are still up for debate since we haven't implemented the supported callconv matrix for function pointers - which will be in the runtime. See #34805. |
ghost
locked as resolved and limited conversation to collaborators
This program (IL at the bottom of the issue) causes .net core 3.1 to fail with:
The program defines a delegate type that takes a pointer to a structure and a float, and returns a structure that contains a field from that first pointer and the float. It then creates a
thiscallfunction pointer stub around the delegate by usingMarshal.CreateFunctionPointerForDelegate, andcalli's that pointer. On .net framework, this correctly prints 12.3. On .net core, this fails with the above exception. /cc @jkoritzinsky.assembly UnmanagedFunctionPointer { .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilationRelaxationsAttribute::.ctor(int32) = ( 01 00 08 00 00 00 00 00 ) .custom instance void [mscorlib]System.Runtime.CompilerServices.RuntimeCompatibilityAttribute::.ctor() = ( 01 00 01 00 54 02 16 57 72 61 70 4e 6f 6e 45 78 63 65 70 74 69 6f 6e 54 68 72 6f 77 73 01 ) .custom instance void [mscorlib]System.Diagnostics.DebuggableAttribute::.ctor(valuetype [mscorlib]System.Diagnostics.DebuggableAttribute/DebuggingModes) = ( 01 00 07 01 00 00 00 00 ) .permissionset reqmin = { [mscorlib]System.Security.Permissions.SecurityPermissionAttribute = { property bool SkipVerification = bool(true) } } .hash algorithm 0x00008004 // SHA1 .ver 0:0:0:0 } .module UnmanagedFunctionPointer.dll .class public sequential ansi sealed beforefieldinit ReturnWrapper extends [mscorlib]System.ValueType { // Fields .field public int32 i1 .field public float32 f2 // Methods .method public hidebysig specialname rtspecialname instance void .ctor ( int32 i1, float32 f2 ) cil managed { // Method begins at RVA 0x205a // Code size 16 (0x10) .maxstack 8 nop ldarg.0 ldarg.1 stfld int32 ReturnWrapper::i1 ldarg.0 ldarg.2 stfld float32 ReturnWrapper::f2 ret } // end of method ReturnWrapper::.ctor } // end of class ReturnWrapper .class public sequential ansi sealed beforefieldinit S extends [mscorlib]System.ValueType { // Fields .field public int32 i // Methods .method public hidebysig static valuetype ReturnWrapper GetReturn ( valuetype S* s, float32 f ) cil managed { // Method begins at RVA 0x208c // Code size 18 (0x12) .maxstack 2 .locals init ( [0] valuetype ReturnWrapper ) nop ldarg.0 ldfld int32 S::i ldarg.1 newobj instance void ReturnWrapper::.ctor(int32, float32) ret } // end of method S::GetReturn } // end of class S .class public auto ansi beforefieldinit UnmanagedFunctionPointer extends [mscorlib]System.Object { // Nested Types .class nested private auto ansi sealed MultipleParams extends [mscorlib]System.MulticastDelegate { .custom instance void [mscorlib]System.Runtime.InteropServices.UnmanagedFunctionPointerAttribute::.ctor(valuetype [mscorlib]System.Runtime.InteropServices.CallingConvention) = ( 01 00 04 00 00 00 00 00 ) // Methods .method public hidebysig specialname rtspecialname instance void .ctor ( object 'object', native int 'method' ) runtime managed { } // end of method MultipleParams::.ctor .method public hidebysig newslot virtual instance valuetype ReturnWrapper Invoke ( valuetype S* s, float32 f ) runtime managed { } // end of method MultipleParams::Invoke .method public hidebysig newslot virtual instance class [mscorlib]System.IAsyncResult BeginInvoke ( valuetype S* s, float32 f, class [mscorlib]System.AsyncCallback callback, object 'object' ) runtime managed { } // end of method MultipleParams::BeginInvoke .method public hidebysig newslot virtual instance valuetype ReturnWrapper EndInvoke ( class [mscorlib]System.IAsyncResult result ) runtime managed { } // end of method MultipleParams::EndInvoke } // end of class MultipleParams // Methods .method public hidebysig static void Main () cil managed { // Method begins at RVA 0x20e0 // Code size 37 (0x25) .maxstack 4 .entrypoint .locals init ( [0] native int, [1] valuetype S, [2] valuetype ReturnWrapper ) nop ldnull ldftn valuetype ReturnWrapper S::GetReturn(valuetype S*, float32) newobj instance void UnmanagedFunctionPointer/MultipleParams::.ctor(object, native int) call native int [mscorlib]System.Runtime.InteropServices.Marshal::GetFunctionPointerForDelegate<class UnmanagedFunctionPointer/MultipleParams>(!!0) stloc.0 ldloc.0 box [mscorlib]System.IntPtr call void [mscorlib]System.GC::KeepAlive(object) ldloca.s 1 initobj [UnmanagedFunctionPointer]S ldloca.s 1 ldc.i4.1 stfld int32 [UnmanagedFunctionPointer]S::i ldloca.s 1 conv.u ldc.r4 2.3 ldloc.0 calli unmanaged thiscall valuetype ReturnWrapper(valuetype S*, float32) stloc.2 ldloc.2 ldfld int32 [UnmanagedFunctionPointer]ReturnWrapper::i1 call void [mscorlib]System.Console::Write(int32) ldloc.2 ldfld float32 [UnmanagedFunctionPointer]ReturnWrapper::f2 call void [mscorlib]System.Console::Write(float32) ret } // end of method UnmanagedFunctionPointer::GetFuncPtrMultipleParams } // end of class UnmanagedFunctionPointerThe text was updated successfully, but these errors were encountered: