-
Notifications
You must be signed in to change notification settings - Fork 80
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
Apple Bigsur support #90
Conversation
#if !(defined SLJIT_CONFIG_ARM && SLJIT_CONFIG_ARM) | ||
#error Unsupported architecture | ||
#endif /* SLJIT_CONFIG_ARM */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
practically not needed and could be made more strict by checking for ARM_64, but will hopefully help when apple does its next CPU migration
This patch looks ok, but it would be good if somebody with hw could try it. |
I tried this patch on hardware (applied to the pcre2 vendored version) and I am seeing the following crash:
|
if you want to backport it to an older version than pcre 10.36-RC1[1] then you will need to apply more patches than just this one so the crash makes sense if by "vendored" you mean your own local copy of it, then you will have to first update it to the RC1; apologize for not being clear about it. [1] https://lists.exim.org/lurker/message/20201106.173459.b8ecdf06.en.html |
This was the version of sljit in PCRE 10.36-RC1. |
there must be a problem then on the way it is detecting that it is compiled for macOS BigSur (make sure it uses the right SDK and minimum OS target) or you have other restrictions (maybe a notarized app)? if the later will need to also apply the correct entitlements at codesign time (see the carenas/sljit fork for hints on how to do that within sljit's own test framework and built through CI)
|
Apple Silicon requires that pages that will hold JIT code are marked with MAP_JIT (even if not using the hardened runtime) and that a call be made to a pthread function before writing to them, so a special exception could be made to the current thread[1]; add support for both. since the allocator keeps the metadata about chunk/block in the executable pages, all functions that modify that metadata will also need to be updated. note that since there is no need for an accurate pointer range with the apple implementation, NULL is passed for the pointers. historically, adding MAP_JIT was only recommended when the hardened runtime was being used as it adds several undocumented restrictions (like not being able to use JIT pages accross fork()) so the new codepath won't be used if running in Intel. Tested-by: @Keno Fixes: zherczeg#51 [1] https://developer.apple.com/documentation/apple_silicon/porting_just-in-time_compilers_to_apple_silicon?language=objc
the last version, including @Keno bugfix to the only line of code that was not exercised through the "emulator" should be safe to merge IMHO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Take my whole day to figure out why such crash happens...... Thanks for your fix. |
Starting with macOS 11.2, mprotect calls for RWX pages will fail in Apple Silicon, even if the page was granted permission and it was requested the MAP_JIT flag, to better reflect the fact that the page returned by mmap wasn't really RWX. In macOS, there is an implementation for the executable allocator since e87e1cc (macos: add BigSur support to execalloc (zherczeg#90), 2020-11-30) that flips the bits as needed, so this extra safeward is no longer needed. HardenedBSD seems to be the last implementation of PaX that still lies, so restrict the code only to that platform. Fixes: zherczeg#99
Starting with macOS 11.2, mprotect calls for RWX pages will fail in Apple Silicon, even if the page was granted permission and it was requested the MAP_JIT flag, to better reflect the fact that the page returned by mmap wasn't really RWX. In macOS, there is an implementation for the executable allocator since e87e1cc (macos: add BigSur support to execalloc (#90), 2020-11-30) that flips the bits as needed, so this extra safeward is no longer needed. HardenedBSD seems to be the last implementation of PaX that still lies, so restrict the code only to that platform. Fixes: #99
Has been tested extensibly (albeit not using real hardware/software but a POSIX look alike) and while is expected to work it might not be as efficient/secure/elegant as it could be, because it needs to return writeable pages for consistency with the other allocators, and also needs to update metadata in those executable pages and therefore has to make the pages writeable more often than ideal.
Since enabling MAP_JIT adds undocumented restrictions to the use of those pages, would be important to confirm that notarized applications with the right entitlement (com.apple.security.cs.allow-jit) to allow creating JIT pages, can still use those pages after fork() and if it doesn't raise the issue with Apple ASAP.
if fork() is no longer restricted it might be worth enabling MAP_JIT unconditionally also in Intel (at least for BigSur) but that will need further testing and therefore is punted from this PR.
Fixes: #51