-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix(jit): Add MAP_JIT support for 100% stable JIT on ARM64 macOS #12077
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,5 +76,16 @@ pub(crate) fn set_readable_and_executable( | |
| } | ||
| } | ||
|
|
||
| // ARM64 macOS: Switch to execute mode (W^X: disable writing, enable execution). | ||
| #[cfg(all(target_arch = "aarch64", target_os = "macos"))] | ||
| { | ||
| unsafe extern "C" { | ||
| fn pthread_jit_write_protect_np(enabled: libc::c_int); | ||
| } | ||
| unsafe { | ||
| pthread_jit_write_protect_np(1); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be possible to scope those pthread_jit_write_protect_np calls to the actual section that does the writes. That would be more secure. |
||
| } | ||
| } | ||
|
|
||
| Ok(()) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,11 @@ use cranelift_module::{ModuleError, ModuleResult}; | |
| #[cfg(all(not(target_os = "windows"), feature = "selinux-fix"))] | ||
| use memmap2::MmapMut; | ||
|
|
||
| #[cfg(not(any(feature = "selinux-fix", windows)))] | ||
| #[cfg(not(any( | ||
| feature = "selinux-fix", | ||
| windows, | ||
| all(target_arch = "aarch64", target_os = "macos") | ||
| )))] | ||
| use std::alloc; | ||
| use std::io; | ||
| use std::mem; | ||
|
|
@@ -49,7 +53,51 @@ impl PtrLen { | |
| }) | ||
| } | ||
|
|
||
| #[cfg(all(not(target_os = "windows"), not(feature = "selinux-fix")))] | ||
| // macOS ARM64: Use mmap with MAP_JIT for W^X policy compliance. | ||
| #[cfg(all( | ||
| target_arch = "aarch64", | ||
| target_os = "macos", | ||
| not(feature = "selinux-fix") | ||
| ))] | ||
| fn with_size(size: usize) -> io::Result<Self> { | ||
| assert_ne!(size, 0); | ||
| let alloc_size = region::page::ceil(size as *const ()) as usize; | ||
|
|
||
| let ptr = unsafe { | ||
| libc::mmap( | ||
| ptr::null_mut(), | ||
| alloc_size, | ||
| libc::PROT_READ | libc::PROT_WRITE, | ||
| libc::MAP_PRIVATE | libc::MAP_ANON | libc::MAP_JIT, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should only pass MAP_JIT for pages that will actually end up being executable in the end. |
||
| -1, | ||
| 0, | ||
| ) | ||
| }; | ||
|
|
||
| if ptr == libc::MAP_FAILED { | ||
| return Err(io::Error::last_os_error()); | ||
| } | ||
|
|
||
| // Enable writing to MAP_JIT memory (W^X: start in write mode) | ||
| unsafe extern "C" { | ||
| fn pthread_jit_write_protect_np(enabled: libc::c_int); | ||
| } | ||
| unsafe { | ||
| pthread_jit_write_protect_np(0); | ||
| } | ||
|
|
||
| Ok(Self { | ||
| ptr: ptr as *mut u8, | ||
| len: alloc_size, | ||
| }) | ||
| } | ||
|
|
||
| // Non-macOS ARM64: Use standard allocator. | ||
| #[cfg(all( | ||
| not(target_os = "windows"), | ||
| not(feature = "selinux-fix"), | ||
| not(all(target_arch = "aarch64", target_os = "macos")) | ||
| ))] | ||
| fn with_size(size: usize) -> io::Result<Self> { | ||
| assert_ne!(size, 0); | ||
| let page_size = region::page::size(); | ||
|
|
@@ -95,7 +143,30 @@ impl PtrLen { | |
| } | ||
|
|
||
| // `MMapMut` from `cfg(feature = "selinux-fix")` already deallocates properly. | ||
| #[cfg(all(not(target_os = "windows"), not(feature = "selinux-fix")))] | ||
|
|
||
| // macOS ARM64: Free MAP_JIT memory with munmap. | ||
| #[cfg(all( | ||
| target_arch = "aarch64", | ||
| target_os = "macos", | ||
| not(feature = "selinux-fix") | ||
| ))] | ||
| impl Drop for PtrLen { | ||
| fn drop(&mut self) { | ||
| if !self.ptr.is_null() { | ||
| unsafe { | ||
| let _ = region::protect(self.ptr, self.len, region::Protection::READ_WRITE); | ||
| libc::munmap(self.ptr as *mut libc::c_void, self.len); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Other Unix platforms: Use standard allocator dealloc. | ||
| #[cfg(all( | ||
| not(target_os = "windows"), | ||
| not(feature = "selinux-fix"), | ||
| not(all(target_arch = "aarch64", target_os = "macos")) | ||
| ))] | ||
| impl Drop for PtrLen { | ||
| fn drop(&mut self) { | ||
| if !self.ptr.is_null() { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -141,6 +141,12 @@ fn riscv_flush_icache(start: u64, end: u64) -> Result<()> { | |
|
|
||
| pub(crate) use details::*; | ||
|
|
||
| // macOS ARM64: Use sys_icache_invalidate for icache coherence. | ||
| #[cfg(all(target_arch = "aarch64", target_os = "macos"))] | ||
| unsafe extern "C" { | ||
| fn sys_icache_invalidate(start: *const c_void, len: usize); | ||
| } | ||
|
|
||
| /// See docs on [crate::clear_cache] for a description of what this function is trying to do. | ||
| #[inline] | ||
| pub(crate) fn clear_cache(_ptr: *const c_void, _len: usize) -> Result<()> { | ||
|
|
@@ -150,7 +156,15 @@ pub(crate) fn clear_cache(_ptr: *const c_void, _len: usize) -> Result<()> { | |
| // We should call some implementation of `clear_cache` here. | ||
| // | ||
| // See: https://github.com/bytecodealliance/wasmtime/issues/3310 | ||
darmie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // macOS ARM64: Use sys_icache_invalidate for icache coherence. | ||
| #[cfg(all(target_arch = "aarch64", target_os = "macos"))] | ||
| unsafe { | ||
| sys_icache_invalidate(_ptr, _len); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #12133 implemented another approach for this.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, yes, I had forgotten that this PR also handled this detail. Thanks for connecting the dots! |
||
|
|
||
| #[cfg(all(target_arch = "riscv64", target_os = "linux"))] | ||
| riscv_flush_icache(_ptr as u64, (_ptr as u64) + (_len as u64))?; | ||
|
|
||
| Ok(()) | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.