Repository navigation
Unexpected results returning struct with floats in RCW #10901
Description
In trying to wrap ID2D1RenderTarget::GetSize which returns [D2D_SIZE_F] the expected way it would corrupt the stack.
Flipping it to an out parameter made it work, but I'm unclear as to why. Discussing with @davidwrighton, there might be a bug here. I was unable to create the same scenario via straight [DllImport]s against a sample project. I'll see if I can get a smaller repro together.
In d2d1.h it is defined as:
STDMETHOD_(D2D1_SIZE_F, GetSize)(
) CONST PURE;Which expands to:
virtual __declspec(nothrow) D2D1_SIZE_F __stdcall GetSize() const = 0;The struct is:
typedef struct D2D_SIZE_F
{
FLOAT width;
FLOAT height;
} D2D_SIZE_F;I wrapped this here (SizeF is from System.Drawing):
[PreserveSig]
void GetSize(out SizeF size);
// Doesn't Work:
// [PreserveSig]
// SizeF GetSize();In the WInterop project I have a test Direct2dTests.RenderTarget.GetSize() that exercises it. One can also look at this by tweaking the Direct2dDemo project (i.e. call the method) which runs on .NET Core and sets up a render target that you can use.
As stated, I'll try and get a smaller COM repro together for this.
Activity
AaronRobinsonMSFT commentedon Aug 14, 2018
cc @luqunl
jkotas commentedon Aug 14, 2018
It will work on some platforms, but not others, depending on calling convention details.
JeremyKuhne commentedon Aug 14, 2018
Thanks. @davidwrighton mentioned that to me too- I should have called that out lest anyone think this is a good workaround. :)
AaronRobinsonMSFT commentedon Aug 14, 2018
I created a repro for when the
[PreserveSig]is used, but if removed, the struct is returned successfully around 50 % of the time 😕This is definitely an issue we need to root cause.
cc @luqunl
jkotas commentedon Aug 14, 2018
Here is what I see with
[PreserveSig]:On x86, it reliably throws
System.Runtime.InteropServices.MarshalDirectiveException: Method's type signature is not Interop compatible.here: https://github.com/dotnet/coreclr/blob/92d2c4bde42569d2aa22e44550d69f7d743bf9a0/src/vm/dllimport.cpp#L3383 . Notice that somebody ifdefed out this limitation on non-x86.On x64, it reliably crashes with access violation. The managed side expects the value to be returned in register, but the unmanaged side returns it via return buffer. Apparently, the calling convention for returning structs differs between instance methods and static methods on Windows x64. From https://docs.microsoft.com/en-us/cpp/build/return-values-cpp:
User-defined types can be returned by value from global functions and static member functions.Notice that this excludes non-static member functions. The bug is that interop marshaling does not differentiate between static and non-static return values on x64.luqunl commentedon Aug 14, 2018
@AaronRobinsonMSFT , are you working on this? if not, I can try to fix this bug.
JeremyKuhne commentedon Aug 21, 2018
If you need anything else from me, let me know. I'm finding this particular pattern all over the DirectX APIs (struct return).
jkoritzinsky commentedon Sep 17, 2018
We have this worked around in SharpGenTools (as @JeremyKuhne noticed, this pattern is common in DirectX).
See the following code:
https://github.com/SharpGenTools/SharpGenTools/blob/master/SharpGen/Transform/MethodTransform.cs#L222
https://github.com/SharpGenTools/SharpGenTools/blob/master/SharpGen/Generator/NativeInvocationCodeGenerator.cs#L33
https://github.com/SharpGenTools/SharpGenTools/blob/master/SharpGen/Generator/ShadowCallbackGenerator.cs#L89
As I read the documentation referenced in the PR I notice that our condition for
IsReturnStructLargeisn't perfect, but the codegen when the condition is accurate works perfectly.jkotas commentedon Sep 17, 2018
Submitted PR to link the workaround to this issue: SharpGenTools/SharpGenTools#82
AaronRobinsonMSFT commentedon Mar 8, 2019
@jkoritzinsky Do we have any additional thoughts on this work for Core 3.0?
jkoritzinsky commentedon Mar 8, 2019
Luqun's solution looked like it would work from a cursory glance, so I'm going to try it out and see what I get with it.