Skip to content

Commit 8f57b28

Browse files
alexcrichtonbjorn3
andauthored
Always use mmap in cranelift-jit (#13180) (#13186)
* Always use mmap in cranelift-jit This was previously not done with the theory that using the regular heap allocator could be faster. We now have an arena memory provider and using mmap is necessary anyway when SELinux is enabled. I don't think the benefits outweight the code complexity cost. * Use memmap2 on Windows too in cranelift-jit Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com>
1 parent 23a410e commit 8f57b28

3 files changed

Lines changed: 8 additions & 84 deletions

File tree

cranelift/filetests/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ cranelift-frontend = { workspace = true }
1919
cranelift-interpreter = { workspace = true }
2020
cranelift-native = { workspace = true }
2121
cranelift-reader = { workspace = true }
22-
cranelift-jit = { workspace = true, features = ["selinux-fix", "wasmtime-unwinder"] }
22+
cranelift-jit = { workspace = true, features = ["wasmtime-unwinder"] }
2323
cranelift-module = { workspace = true }
2424
cranelift-control = { workspace = true }
2525
wasmtime-unwinder = { workspace = true, features = ["cranelift"] }

cranelift/jit/Cargo.toml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ anyhow = { workspace = true }
2424
region = "3.0.2"
2525
libc = { workspace = true }
2626
target-lexicon = { workspace = true }
27-
memmap2 = { version = "0.2.1", optional = true }
27+
memmap2 = "0.2.1"
2828
log = { workspace = true }
2929
wasmtime-jit-icache-coherence = { workspace = true }
3030

@@ -37,7 +37,6 @@ features = [
3737
]
3838

3939
[features]
40-
selinux-fix = ['memmap2']
4140
default = []
4241

4342
wasmtime-unwinder = ["dep:wasmtime-unwinder"]

cranelift/jit/src/memory/system.rs

Lines changed: 6 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,15 @@
1-
use cranelift_module::{ModuleError, ModuleResult};
2-
3-
#[cfg(all(not(target_os = "windows"), feature = "selinux-fix"))]
4-
use memmap2::MmapMut;
5-
6-
#[cfg(not(any(feature = "selinux-fix", windows)))]
7-
use std::alloc;
81
use std::io;
92
use std::mem;
103
use std::ptr;
114

5+
use cranelift_module::{ModuleError, ModuleResult};
6+
use memmap2::MmapMut;
7+
128
use super::{BranchProtection, JITMemoryKind, JITMemoryProvider};
139

1410
/// A simple struct consisting of a pointer and length.
1511
struct PtrLen {
16-
#[cfg(all(not(target_os = "windows"), feature = "selinux-fix"))]
1712
map: Option<MmapMut>,
18-
1913
ptr: *mut u8,
2014
len: usize,
2115
}
@@ -24,17 +18,14 @@ impl PtrLen {
2418
/// Create a new empty `PtrLen`.
2519
fn new() -> Self {
2620
Self {
27-
#[cfg(all(not(target_os = "windows"), feature = "selinux-fix"))]
2821
map: None,
29-
3022
ptr: ptr::null_mut(),
3123
len: 0,
3224
}
3325
}
3426

3527
/// Create a new `PtrLen` pointing to at least `size` bytes of memory,
3628
/// suitably sized and aligned for memory protection.
37-
#[cfg(all(not(target_os = "windows"), feature = "selinux-fix"))]
3829
fn with_size(size: usize) -> io::Result<Self> {
3930
let alloc_size = region::page::ceil(size as *const ()) as usize;
4031
MmapMut::map_anon(alloc_size).map(|mut mmap| {
@@ -47,70 +38,8 @@ impl PtrLen {
4738
}
4839
})
4940
}
50-
51-
#[cfg(all(not(target_os = "windows"), not(feature = "selinux-fix")))]
52-
fn with_size(size: usize) -> io::Result<Self> {
53-
assert_ne!(size, 0);
54-
let page_size = region::page::size();
55-
let alloc_size = region::page::ceil(size as *const ()) as usize;
56-
let layout = alloc::Layout::from_size_align(alloc_size, page_size).unwrap();
57-
// Safety: We assert that the size is non-zero above.
58-
let ptr = unsafe { alloc::alloc(layout) };
59-
60-
if !ptr.is_null() {
61-
Ok(Self {
62-
ptr,
63-
len: alloc_size,
64-
})
65-
} else {
66-
Err(io::Error::from(io::ErrorKind::OutOfMemory))
67-
}
68-
}
69-
70-
#[cfg(target_os = "windows")]
71-
fn with_size(size: usize) -> io::Result<Self> {
72-
use windows_sys::Win32::System::Memory::{
73-
MEM_COMMIT, MEM_RESERVE, PAGE_READWRITE, VirtualAlloc,
74-
};
75-
76-
// VirtualAlloc always rounds up to the next multiple of the page size
77-
let ptr = unsafe {
78-
VirtualAlloc(
79-
ptr::null_mut(),
80-
size,
81-
MEM_COMMIT | MEM_RESERVE,
82-
PAGE_READWRITE,
83-
)
84-
};
85-
if !ptr.is_null() {
86-
Ok(Self {
87-
ptr: ptr as *mut u8,
88-
len: region::page::ceil(size as *const ()) as usize,
89-
})
90-
} else {
91-
Err(io::Error::last_os_error())
92-
}
93-
}
9441
}
9542

96-
// `MMapMut` from `cfg(feature = "selinux-fix")` already deallocates properly.
97-
#[cfg(all(not(target_os = "windows"), not(feature = "selinux-fix")))]
98-
impl Drop for PtrLen {
99-
fn drop(&mut self) {
100-
if !self.ptr.is_null() {
101-
let page_size = region::page::size();
102-
let layout = alloc::Layout::from_size_align(self.len, page_size).unwrap();
103-
unsafe {
104-
region::protect(self.ptr, self.len, region::Protection::READ_WRITE)
105-
.expect("unable to unprotect memory");
106-
alloc::dealloc(self.ptr, layout)
107-
}
108-
}
109-
}
110-
}
111-
112-
// TODO: add a `Drop` impl for `cfg(target_os = "windows")`
113-
11443
/// JIT memory manager. This manages pages of suitably aligned and
11544
/// accessible memory. Memory will be leaked by default to have
11645
/// function pointers remain valid for the remainder of the
@@ -201,13 +130,9 @@ impl Memory {
201130

202131
/// Iterates non protected memory allocations that are of not zero bytes in size.
203132
fn non_protected_allocations_iter(&self) -> impl Iterator<Item = &PtrLen> {
204-
let iter = self.allocations[self.already_protected..].iter();
205-
206-
#[cfg(all(not(target_os = "windows"), feature = "selinux-fix"))]
207-
return iter.filter(|&PtrLen { map, len, .. }| *len != 0 && map.is_some());
208-
209-
#[cfg(any(target_os = "windows", not(feature = "selinux-fix")))]
210-
return iter.filter(|&PtrLen { len, .. }| *len != 0);
133+
self.allocations[self.already_protected..]
134+
.iter()
135+
.filter(|&PtrLen { map, len, .. }| *len != 0 && map.is_some())
211136
}
212137

213138
/// Frees all allocated memory regions that would be leaked otherwise.

0 commit comments

Comments
 (0)