dotnet / runtime

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

Incorrect argument passing for P/Invoke returning a struct in .NET Core 3.1 32-bit #40297

Closed SteveKCheng closed 4 years ago

SteveKCheng commented 4 years ago

I am not sure if this concerns "P/Invoke" in general or just "reverse P/Invoke".

When I call a managed function of the form:

S myFunc(void* p)

through a function pointer given to native code (using Marshal.GetFunctionPointerForDelegate) where S is a struct, the argument p is not received correctly when running on .NET Core 3.1 32-bit Windows.

On .NET Framework, both 32-bit and 64-bit, the behavior is as expected. On .NET Core 3.1 64-bit Windows and Linux, the behavior is as expected. The problem seems to occur only on .NET Core 32-bit Windows. On 32-bit Windows, there is of course the complication of calling conventions, but I have set them all consistently to cdecl and the problem persists.

I understand passing a struct as a return value can be messy at the ABI level, so probably the bug has something to do with that. The bug does not manifest if I change S to void.

I have a test case that reproduces the bug here:

https://github.com/SteveKCheng/PInvokeBugInDotNetCore32Bit

You can open the solution in Visual Studio. Inside there is a .NET console project and a C++ DLL project. The build rules and configurations have been set so that, building the .NET console project will also build the C++ DLL and copy the DLL into the right folder. So you can just hit F5 to run the application.

There are four configuration combinations of interest: 32-bit net452, 32-bit netcoreapp3.1, 64-bit net452, 64-bit netcoreapp3.1.

The program, when run, just prints the received argument of p. The correct and expected result is 0x12345678. The incorrect result which shows up when running on 32-bit netcoreapp3.1 is 0x0.

AaronRobinsonMSFT commented 4 years ago

/cc @jkoritzinsky

jkoritzinsky commented 4 years ago

I've tested this against out main branch and it outputs:

ReversePInvokedFunction received: 0x12345678

I'm going to close this as fixed in our main branch.

SteveKCheng commented 4 years ago

Oh, thank you very much for looking into this. I can hardly wait for the .NET 5 release then :)

danmoseley commented 4 years ago

@SteveKCheng would it be possible for you to test your app in general against the latest 5.0 preview? Just to help verify you're 100% good.

SteveKCheng commented 4 years ago

Yup, I just did so - thanks for the suggestion. I confirm it works now. Just to be clear, I confirmed it working in .NET 5 preview 7 on all 3 platforms: win-x86, win-x64 and linux-x64.

Edit: I should say this is just testing my reduced bug repo: for the app itself, I will have to get back to you in a few weeks time. But I suspect it addresses the sole critical issue for the 32-bit build of my app.