From d291cda52790a34ed10bf72b4c5e27c3ab89a242 Mon Sep 17 00:00:00 2001 From: "Eileen M. Uchitelle" Date: Mon, 15 Apr 2024 01:38:42 -0400 Subject: [PATCH] Don't use `PROT_EXEC` with `mmap` on macos (#1110) On macos I was running `./miniruby --mmtk -e 'puts "Hello world!"'` and seeing a panic: ``` thread '' panicked at /src/util/raw_memory_freelist.rs:202:9: Can't get more space with mmap() note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace fatal runtime error: failed to initiate panic, error 5 Abort trap: 6 ``` On macos, you can't combine `PROT_EXEC` with `PROT_WRITE` and `PROT_READ` in `mmap`, it will return a permission denied error. It's not clear from the `Can't get more space with mmap()` that's what was happening, but I could see it when inspecting the `ret` from `mmap_fixed` in `dzmmap_noreplace`. I found a thread on apple confirming that `PROT_EXEC` doesn't make sense for macos in this context. https://forums.developer.apple.com/forums/thread/740017 Note: there are still failing tests for `mmap` on macos. The main change with this PR is that it no longer panics when runnning a simple hello world with Ruby on macos. Previously the build wasn't finishing and would throw the following error: ``` process didn't exit successfully: `/mmtk-core/target/debug/deps/mmtk-ef0f14e36a03fbba` (signal: 11, SIGSEGV: invalid memory reference) ``` Now the tests do finish, with 25 failures. --- src/util/memory.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/util/memory.rs b/src/util/memory.rs index 3963ce5fe8..09d8d7d719 100644 --- a/src/util/memory.rs +++ b/src/util/memory.rs @@ -40,7 +40,7 @@ pub fn set(start: Address, val: u8, len: usize) { /// may corrupt others' data. #[allow(clippy::let_and_return)] // Zeroing is not neceesary for some OS/s pub unsafe fn dzmmap(start: Address, size: usize, strategy: MmapStrategy) -> Result<()> { - let prot = PROT_READ | PROT_WRITE | PROT_EXEC; + let prot = MMAP_PROT; let flags = libc::MAP_ANON | libc::MAP_PRIVATE | libc::MAP_FIXED; let ret = mmap_fixed(start, size, prot, flags, strategy); // We do not need to explicitly zero for Linux (memory is guaranteed to be zeroed) @@ -58,6 +58,12 @@ const MMAP_FLAGS: libc::c_int = libc::MAP_ANON | libc::MAP_PRIVATE | libc::MAP_F // MAP_FIXED is used instead of MAP_FIXED_NOREPLACE (which is not available on macOS). We are at the risk of overwriting pre-existing mappings. const MMAP_FLAGS: libc::c_int = libc::MAP_ANON | libc::MAP_PRIVATE | libc::MAP_FIXED; +#[cfg(target_os = "linux")] +const MMAP_PROT: libc::c_int = PROT_READ | PROT_WRITE | PROT_EXEC; +#[cfg(target_os = "macos")] +// PROT_EXEC cannot be used with PROT_READ on Apple Silicon +const MMAP_PROT: libc::c_int = PROT_READ | PROT_WRITE; + /// Strategy for performing mmap /// /// This currently supports switching between different huge page allocation @@ -77,7 +83,7 @@ pub enum MmapStrategy { /// This function will not overwrite existing memory mapping, and it will result Err if there is an existing mapping. #[allow(clippy::let_and_return)] // Zeroing is not neceesary for some OS/s pub fn dzmmap_noreplace(start: Address, size: usize, strategy: MmapStrategy) -> Result<()> { - let prot = PROT_READ | PROT_WRITE | PROT_EXEC; + let prot = MMAP_PROT; let flags = MMAP_FLAGS; let ret = mmap_fixed(start, size, prot, flags, strategy); // We do not need to explicitly zero for Linux (memory is guaranteed to be zeroed)