KEMBAR78
[NativeAOT] Implement thunk page generation and mapping for iOS-like platforms by filipnavara · Pull Request #82317 · dotnet/runtime · GitHub
Skip to content

[NativeAOT] Implement thunk page generation and mapping for iOS-like platforms #82317

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Feb 25, 2023
Merged

[NativeAOT] Implement thunk page generation and mapping for iOS-like platforms #82317

merged 4 commits into from
Feb 25, 2023

Conversation

@filipnavara
Copy link
Member

@filipnavara filipnavara commented Feb 17, 2023 •
edited
Loading

Fixes #82090

Marked as draft for the moment since there are still missing bits in the build and test infrastructure. I manually tested it using the PInvoke NativeAOT test compiled for maccatalyst-arm64 and maccatalyst-x64. The PAL interfaces could likely be cleaned up quite a bit since the implementation has different requirements from the old Windows-only code.

@ghost ghost added area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member labels Feb 17, 2023
@ghost
Copy link

ghost commented Feb 17, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #82090

Marked as draft for the moment since there are still missing bits in the build and test infrastructure. I manually tested it using the PInvoke NativeAOT test compiled for maccatalyst-arm64. The x64 code is completely untested but it compiles.

Author: filipnavara
Assignees: -
Labels:

area-NativeAOT-coreclr
Milestone: -

@jkotas
Copy link
Member

jkotas commented Feb 18, 2023

I manually tested it using the PInvoke NativeAOT test compiled for maccatalyst-arm64 and maccatalyst-x64.

Does this test create enough delegates to hit the mmap path?

@build-analysis build-analysis bot mentioned this pull request Feb 18, 2023
Closed
4 tasks
@filipnavara
Copy link
Member Author

I manually tested it using the PInvoke NativeAOT test compiled for maccatalyst-arm64 and maccatalyst-x64.

Does this test create enough delegates to hit the mmap path?

No, but I manually modified the thunk allocation to always take the path and it still succeeded.

@filipnavara
Copy link
Member Author

filipnavara commented Feb 18, 2023 •
edited
Loading

For reference, here are all the build hacks and steps I did to make the test run.

Start with this branch - https://github.com/filipnavara/runtime/tree/build-hacks. It's PR #82086 and bunch of hacks/fixes stacked on top of it.

Build with the following steps:

./build.sh clr+clr.aot
./build.sh clr.nativeaotruntime+clr.nativeaotlibs+libs -os maccatalyst
./dotnet.sh build src/coreclr/nativeaot/BuildIntegration/BuildIntegration.proj /p:TargetOS=maccatalyst
./src/tests/build.sh --os maccatalyst arm64 skiptestwrappers skipgeneratelayout -dir:nativeaot/SmokeTests/PInvoke -nativeaot /p:LibrariesConfiguration=Debug
./artifacts/tests/coreclr/maccatalyst.arm64.Debug/nativeaot/SmokeTests/PInvoke/PInvoke/native/PInvoke

It should fail with

...
2023-02-18 21:54:28.730 PInvoke[37508:4288446] Testing Delegate
zsh: abort

Then merge this feature branch on top, rebuild the runtime, rebuild the test and rerun it:

git merge thunks-1
./build.sh clr.nativeaotruntime+clr.nativeaotlibs+libs -os maccatalyst
./dotnet.sh build src/coreclr/nativeaot/BuildIntegration/BuildIntegration.proj /p:TargetOS=maccatalyst
./src/tests/build.sh --os maccatalyst arm64 skiptestwrappers skipgeneratelayout -dir:nativeaot/SmokeTests/PInvoke -nativeaot /p:LibrariesConfiguration=Debug
./artifacts/tests/coreclr/maccatalyst.arm64.Debug/nativeaot/SmokeTests/PInvoke/PInvoke/native/PInvoke

The test will succeed.

In order to test the mmap part I commented out this line.

akoeplinger
akoeplinger reviewed Feb 21, 2023
View reviewed changes
@build-analysis build-analysis bot mentioned this pull request Feb 22, 2023
Closed
akoeplinger
akoeplinger reviewed Feb 24, 2023
View reviewed changes
jkotas
jkotas reviewed Feb 24, 2023
View reviewed changes
@filipnavara filipnavara marked this pull request as ready for review February 25, 2023 09:34
jkotas
jkotas reviewed Feb 25, 2023
View reviewed changes
jkotas
jkotas approved these changes Feb 25, 2023
View reviewed changes
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@jkotas jkotas merged commit 75a32b8 into dotnet:main Feb 25, 2023
@akoeplinger akoeplinger mentioned this pull request Feb 27, 2023
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Mar 27, 2023
@filipnavara filipnavara deleted the thunks-1 branch May 31, 2023 07:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@akoeplinger akoeplinger akoeplinger left review comments

@jkotas jkotas jkotas approved these changes

@ivanpovazan ivanpovazan Awaiting requested review from ivanpovazan

@MichalStrehovsky MichalStrehovsky Awaiting requested review from MichalStrehovsky MichalStrehovsky is a code owner

Assignees

No one assigned

Labels

area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support marshalled delegates with NativeAOT on iOS platforms

3 participants

@filipnavara @jkotas @akoeplinger