Runtime: Calli with thiscall convention fails at runtime

[333fred]

This program (IL at the bottom of the issue) causes .net core 3.1 to fail with:

Unhandled exception. System.InvalidProgramException: Common Language Runtime detected an invalid program.
   at UnmanagedFunctionPointer.Main()

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 thiscall function pointer stub around the delegate by using Marshal.CreateFunctionPointerForDelegate, and calli'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 UnmanagedFunctionPointer

[jkoritzinsky]

This is likely related to #12375

[jkoritzinsky]

After more investigating, this is actually an interop bug with the IL stubs. I'll fix it.

[jkoritzinsky]

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.

[jkoritzinsky]

The second bug would fall on the JIT's support of thiscall. cc: @davidwrighton

[davidwrighton]

@dotnet/jit-contrib Could someone look at the incorrect behavior that @jkoritzinsky is talking about here?

[AaronRobinsonMSFT]

The second bug would fall on the JIT's support of thiscall.

Updating tag based on this statement.

/cc @dotnet/jit-contrib

[AndyAyersMS]

I'll take a look.

[AndyAyersMS]

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:

// reverse pinvoke stub
 UnmarshalReturn {
          ( 2) nop             // return {
          ( 2) stloc.2                        // result -> local ReturnWrapper2
          ( 1) ldloc.2            
          ( 2) stloc.3                        // result -> localReturnWrapper3
          ( 1) ldarg.0                        // &S, not &ReturnWrapper ??
          ( 2) ldloca.s        0x3
          ( 3) cpobj           0x02000003 ReturnWrapper   // result -> S
          ( 1) nop             // } return

Not sure who gets it wrong, but seems more likely the ReverseStub.

[jkoritzinsky]

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.

[AndyAyersMS]

The above is from ef72b95937 (May 27), so let me update and see.

[jkoritzinsky]

If the program is still throwing an InvalidProgramException then there's another bug somewhere that we need to fix.

[AndyAyersMS]

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 ldarg.1...?

TID 999c: NDirect IL stub dump: ILStubClass::IL_STUB_ReversePInvoke
TID 999c: incoming managed sig: 08BCA820: void *(int32,native int,float32)
TID 999c: locals sig:           0335F5F9: (int32,int32,ReturnWrapper /* MT: 0x08BC8500 */,ReturnWrapper /* MT: 0x08BC8500 */,float32)
TID 999c: try_begin: 0x0006 try_end: 0x0059 finally_begin: 0x005a finally_end: 0x006e 
TID 999c: dwStubFlags: 0x00000252
TID 999c:    NDIRECTSTUB_FL_BESTFIT
TID 999c:    NDIRECTSTUB_FL_DELEGATE
TID 999c:    NDIRECTSTUB_FL_REVERSE_INTEROP
TID 999c:    NDIRECTSTUB_FL_GENERATEDEBUGGABLEIL
TID 999c: jitFlags:
TID 999c:    CORJIT_FLAGS::CORJIT_FLAG_IL_STUB
TID 999c:    CORJIT_FLAGS::CORJIT_FLAG_PUBLISH_SECRET_PARAM
TID 999c: Marshal {
TID 999c:          ( 0) ldc.i4          0x0 
TID 999c:          ( 1) stloc.0          
TID 999c: IL_0006: ( 0) nop             // argument {  
TID 999c:          ( 0) ldarg.0          
TID 999c:          ( 1) stloc.1          
TID 999c:          ( 0) nop             // } argument 
TID 999c:          ( 0) nop             // return {  
TID 999c:          ( 0) nop             // } return 
TID 999c:          ( 0) nop             // argument {  
TID 999c:          ( 0) ldarg.2          
TID 999c:          ( 1) stloc.s         0x4 
TID 999c:          ( 0) nop             // } argument 
TID 999c: }
TID 999c: CallMethod {
TID 999c:          ( 0) call            0x06000001 native int [System.Private.CoreLib] System.StubHelpers.StubHelpers::GetStubContext()
TID 999c:          ( 1) ldc.i4          0x8 
TID 999c:          ( 2) add              
TID 999c:          ( 1) ldind.i          
TID 999c:          ( 1) ldind.ref        
TID 999c:          ( 1) ldfld           0x04000002 System.Delegate::_target
TID 999c:          ( 1) ldloc.1          
TID 999c:          ( 2) ldloc.s         0x4 
TID 999c:          ( 3) call            0x06000006 native int [System.Private.CoreLib] System.StubHelpers.StubHelpers::GetStubContext()
TID 999c:          ( 4) ldc.i4          0x8 
TID 999c:          ( 5) add              
TID 999c:          ( 4) ldind.i          
TID 999c:          ( 4) ldind.ref        
TID 999c:          ( 4) ldfld           0x04000005 System.Delegate::_methodPtr
TID 999c:          ( 4) calli           0x11ffffff instance ReturnWrapper /* MT: 0x08BC8500 */(S /* MT: 0x08BC8400 */*,float32)
TID 999c: }
TID 999c: UnmarshalReturn {
TID 999c:          ( 2) nop             // return {  
TID 999c:          ( 2) stloc.2          
TID 999c:          ( 1) ldloc.2          
TID 999c:          ( 2) stloc.3          
TID 999c:          ( 1) ldarg.0          
TID 999c:          ( 2) ldloca.s        0x3 
TID 999c:          ( 3) cpobj           0x02000003 ReturnWrapper
TID 999c:          ( 1) nop             // } return 
TID 999c: }
TID 999c: Unmarshal {
TID 999c:          ( 1) nop             // argument {  
TID 999c:          ( 1) nop             // } argument 
TID 999c:          ( 1) nop             // argument {  
TID 999c:          ( 1) nop             // } argument 
TID 999c:          ( 1) ldc.i4          0x7fffffff 
TID 999c:          ( 2) stloc.0          
TID 999c:          ( 1) leave           IL_0059 
TID 999c: IL_0059: ( 1) ret              
TID 999c: }
TID 999c: ExceptionCleanup {
TID 999c: IL_005a: ( 0) ldloc.0          
TID 999c:          ( 1) ldc.i4          0x7ffffffe 
TID 999c:          ( 2) bgt             IL_006d 
TID 999c:          ( 0) ldloca.s        0x3 
TID 999c:          ( 1) initobj         0x02000004 ReturnWrapper
TID 999c: IL_006d:
TID 999c: }
TID 999c: Cleanup {
TID 999c:          ( 0) endfinally       
TID 999c: IL_006e:
TID 999c: }

[jkoritzinsky]

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 [UnmanagedCallersOnly] and marked to be thiscall, then there would be no IL stub and the JIT would incorrectly not use a return buffer. I'd like to push again for https://github.com/dotnet/runtime/issues/12375 to be fixed so the interop space doesn't need to handle these calling-convention-specific issues.

[AaronRobinsonMSFT]

@jkoritzinsky I think fixing https://github.com/dotnet/runtime/issues/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?

[jkoritzinsky]

I was just chatting with @CarolEidt about #12375 and apparently the code to implement it would be spread across the JIT in various locations.

[AndyAyersMS]

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

[jkoritzinsky]

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 [UnmanagedFunctionPointer(CallingConvention.ThisCall)] or [UnmanagedCallersOnly(CallConvs = typeof(CallConvThiscall))] that can be passed to native and called as though they are native instance methods even when they return structs.

[jkoritzinsky]

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 UnmanagedCallersOnly where there is no IL stub, this needs to be done by the JIT to work correctly.

[tannergooding]

@jkoritzinsky, should that comment be a new issue tracking support for delegate* unmanaged[Thiscall, *] where * is Stdcall, Cdecl, or Fastcall?

I think Stdcall is the primary, but IIRC Cdecl and Fastcalll were slightly tweaked as well (although I've never seen them in practice).

[jkoritzinsky]

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)

[PathogenDavid]

I think we have an issue tracking it

Is this an internal issue? If not, can you link it? I couldn't find it anywhere on the runtime repo.

[AaronRobinsonMSFT]

@jkoritzinsky Doesn't look like we have had an official request at the moment. We have one for CallConvSuppressGCTransition, but not for a value indicating the combination of stdcall and thiscall.

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 https://github.com/dotnet/runtime/issues/34805.