[NativeAOT/Apple] Map the thunks using vm_allocate/vm_remap#95914
[NativeAOT/Apple] Map the thunks using vm_allocate/vm_remap#95914jkotas merged 1 commit intodotnet:mainfrom
Conversation
…ed for specific executable segment layout
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas Issue DetailsMap the thunks using vm_allocate/vm_map to avoid need for specific executable segment layout. Fixes #94655
|
|
Tagging subscribers to 'os-ios': @steveisok, @akoeplinger, @kotlarmilos Issue DetailsMap the thunks using vm_allocate/vm_map to avoid need for specific executable segment layout. Fixes #94655
|
ivanpovazan
left a comment
There was a problem hiding this comment.
LGTM, thank you!
I just left a minor question/comment above.
@filipnavara would it make sense to add some kind of unit test verifying that the app has only a single executable segment?
| do | ||
| { | ||
| ret = vm_remap( | ||
| mach_task_self(), &addr, templateSize, 0, VM_FLAGS_FIXED | VM_FLAGS_OVERWRITE, |
There was a problem hiding this comment.
Just trying to understand the difference here, Mono seems to pass only FALSE in place of flags when calling vm_remap, why are we passing VM_FLAGS_FIXED | VM_FLAGS_OVERWRITE instead (or vice versa should we use the same on Mono side as well)?
There was a problem hiding this comment.
We should use this on Mono side as well. Mono uses a loop with vm_allocate(2*size)+vm_deallocate(1*size)+vm_remap. There's a race condition between the vm_deallocate+vm_remap not ending up at the same address. If the race condition is hit then it starts again from the beginning. VM_FLAGS_OVERWRITE gets rid of this race condition by skipping the intermediate reallocation.
There was a problem hiding this comment.
Yea seems like a good idea (I'm not intimately familiar with our infinite trampolines code, but it seems workable).
So instead of the loop we would do a single pass:
vm_allocate(2*size)returningaddr- set
tpage = addr+size vm_remap(&tpage, VM_FLAGS_FIXED | VM_FLAGS_OVERWRITE)- if success, we're done; if failure, dump core
?
I don't think it's necessary. You have to go out of your way to create a second executable segment, it doesn't happen without an explicit effort to do so. |
|
/backport to release/8.0-staging |
|
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/7196873974 |
|
Thanks for review and backport. |
Map the thunks using vm_allocate/vm_remap to avoid need for specific executable segment layout.
Fixes #94655