Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify callback calling convention for callbacks on Windows x86 causes crashes on Mono. #89571

Merged
merged 5 commits into from
Aug 3, 2023

Conversation

lateralusX
Copy link
Member

@lateralusX lateralusX commented Jul 27, 2023

Fixes #88992.

CoreCLR runtime build using stdcall while Mono uses cdecl (default for c/c++ source files) as default calling convention. If a native runtime function takes a reverse pinvoke managed callback, this difference will cause issues on Mono Windows x86 since reverse pinvoke callbacks uses stdcall on Windows x86, but native functions calling callbacks have been prototyped to use the "default" calling convention used by native runtime build.

  • Since CoreCLR already uses stdcall on Windows x86 this change won't change anything on CoreCLR.
  • On Windows x64 stdcall == cdecl so this change won't change anything on CoreCLR, Mono or NativeAOT.

PR also re-enable test suites previously disabled due to this issue as well as making sure native EventPipe tests suites build/run/ pass using Mono Windows x86.

…dows x86.

CoreCLR runtime get build using stdcall while Mono uses cdecl (default).
If a native runtime function takes a reverse pivnoke callback, this
difference will cause issues for Mono on Windows x86 since reverse
pinvoke callbacks have stdcall on Windows x86, but callbacks have
been prototyped to use the calling convention setup in build.

Since CoreCLR already uses stdcall on Windows x86 this change won't change
anything, while on Mono it will fixes the crashes observed in dotnet#8892.
@lateralusX
Copy link
Member Author

Timeout seen in runtime (Build tvos-arm64 Release AllSubsets_Mono) occurs on other PR's as well, so unrelated to changes done in this PR.

@lateralusX
Copy link
Member Author

lateralusX commented Aug 1, 2023

@davmason, mainly a nop change on all platforms except Mono Windows x86 since CoreCLR Windows x86 already uses stdcall as default calling convention for native code. Would you mind take a look?

Copy link
Member

@davmason davmason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lateralusX
Copy link
Member Author

lateralusX commented Aug 2, 2023

/azp run runtime-libraries-mono outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dotnet dotnet deleted a comment from azure-pipelines bot Aug 2, 2023
@lateralusX
Copy link
Member Author

Failures in runtime-libraries-mono outerloop (Build windows-x86 Release AllSubsets_Mono) test run after fix and re-enabled tests are not related (failure in System.Numerics.Tests.logTest.RunLargeValueLogTests, and System.Collections.Frozen.Tests.FrozenDictionary_Generic_Tests_ulong_ulong.CreateHugeDictionary_Success), so re-enabled tests seems to have been fixed by changes in this PR.

@lateralusX
Copy link
Member Author

Errors are infrastructure or known issues.

@lateralusX lateralusX merged commit b0f88da into dotnet:main Aug 3, 2023
174 of 181 checks passed
@ghost ghost locked as resolved and limited conversation to collaborators Sep 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[mono] Access Violation when running certain outerloop tests on Windows x86
2 participants