Skip to content

Commit 258c0e6

Browse files
committed
Duplicate page size determination in wasmtime-fiber
Currently Wasmtime has a function `crate::runtime::vm::host_page_size` but this isn't reachable from the `wasmtime-fiber` crate and instead tha crate uses `rustix::param::page_size` to determine the host page size. It looks like this usage of `rustix` is causing a panic in #10802. Ideally `wasmtime-fiber` would be able to use the same function but the crate separation does not currently make that feasible. For now duplicate the logic of `wasmtime` into `wasmtime-fiber` as it's modest enough to ensure that this does not panic. Closes #10802
1 parent f2eff68 commit 258c0e6

File tree

10 files changed

+89
-59
lines changed

10 files changed

+89
-59
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ http = { workspace = true, optional = true }
8888
http-body-util = { workspace = true, optional = true }
8989

9090
[target.'cfg(unix)'.dependencies]
91-
rustix = { workspace = true, features = ["mm", "param", "process"] }
91+
rustix = { workspace = true, features = ["mm", "process"] }
9292

9393
[dev-dependencies]
9494
# depend again on wasmtime to activate its default features for tests
@@ -133,6 +133,9 @@ cranelift-native = { workspace = true }
133133
[target.'cfg(windows)'.dev-dependencies]
134134
windows-sys = { workspace = true, features = ["Win32_System_Memory"] }
135135

136+
[target.'cfg(unix)'.dev-dependencies]
137+
rustix = { workspace = true, features = ["param"] }
138+
136139
[build-dependencies]
137140
anyhow = { workspace = true, features = ['std'] }
138141

crates/fiber/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ wasmtime-versioned-export-macros = { workspace = true }
1818
wasmtime-asm-macros = { workspace = true }
1919

2020
[target.'cfg(unix)'.dependencies]
21-
rustix = { workspace = true, features = ["mm", "param"] }
21+
rustix = { workspace = true, features = ["mm"] }
22+
libc = { workspace = true }
2223

2324
[target.'cfg(windows)'.dependencies.windows-sys]
2425
workspace = true

crates/fiber/src/lib.rs

Lines changed: 32 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -273,13 +273,17 @@ mod tests {
273273
use std::cell::Cell;
274274
use std::rc::Rc;
275275

276+
fn fiber_stack(size: usize) -> FiberStack {
277+
FiberStack::new(size, false).unwrap()
278+
}
279+
276280
#[test]
277281
fn small_stacks() {
278-
Fiber::<(), (), ()>::new(FiberStack::new(0, false).unwrap(), |_, _| {})
282+
Fiber::<(), (), ()>::new(fiber_stack(0), |_, _| {})
279283
.unwrap()
280284
.resume(())
281285
.unwrap();
282-
Fiber::<(), (), ()>::new(FiberStack::new(1, false).unwrap(), |_, _| {})
286+
Fiber::<(), (), ()>::new(fiber_stack(1), |_, _| {})
283287
.unwrap()
284288
.resume(())
285289
.unwrap();
@@ -289,11 +293,10 @@ mod tests {
289293
fn smoke() {
290294
let hit = Rc::new(Cell::new(false));
291295
let hit2 = hit.clone();
292-
let fiber =
293-
Fiber::<(), (), ()>::new(FiberStack::new(1024 * 1024, false).unwrap(), move |_, _| {
294-
hit2.set(true);
295-
})
296-
.unwrap();
296+
let fiber = Fiber::<(), (), ()>::new(fiber_stack(1024 * 1024), move |_, _| {
297+
hit2.set(true);
298+
})
299+
.unwrap();
297300
assert!(!hit.get());
298301
fiber.resume(()).unwrap();
299302
assert!(hit.get());
@@ -303,13 +306,12 @@ mod tests {
303306
fn suspend_and_resume() {
304307
let hit = Rc::new(Cell::new(false));
305308
let hit2 = hit.clone();
306-
let fiber =
307-
Fiber::<(), (), ()>::new(FiberStack::new(1024 * 1024, false).unwrap(), move |_, s| {
308-
s.suspend(());
309-
hit2.set(true);
310-
s.suspend(());
311-
})
312-
.unwrap();
309+
let fiber = Fiber::<(), (), ()>::new(fiber_stack(1024 * 1024), move |_, s| {
310+
s.suspend(());
311+
hit2.set(true);
312+
s.suspend(());
313+
})
314+
.unwrap();
313315
assert!(!hit.get());
314316
assert!(fiber.resume(()).is_err());
315317
assert!(!hit.get());
@@ -348,16 +350,13 @@ mod tests {
348350
}
349351

350352
fn run_test() {
351-
let fiber = Fiber::<(), (), ()>::new(
352-
FiberStack::new(1024 * 1024, false).unwrap(),
353-
move |(), s| {
354-
assert_contains_host();
355-
s.suspend(());
356-
assert_contains_host();
357-
s.suspend(());
358-
assert_contains_host();
359-
},
360-
)
353+
let fiber = Fiber::<(), (), ()>::new(fiber_stack(1024 * 1024), move |(), s| {
354+
assert_contains_host();
355+
s.suspend(());
356+
assert_contains_host();
357+
s.suspend(());
358+
assert_contains_host();
359+
})
361360
.unwrap();
362361
assert!(fiber.resume(()).is_err());
363362
assert!(fiber.resume(()).is_err());
@@ -374,13 +373,10 @@ mod tests {
374373

375374
let a = Rc::new(Cell::new(false));
376375
let b = SetOnDrop(a.clone());
377-
let fiber = Fiber::<(), (), ()>::new(
378-
FiberStack::new(1024 * 1024, false).unwrap(),
379-
move |(), _s| {
380-
let _ = &b;
381-
panic!();
382-
},
383-
)
376+
let fiber = Fiber::<(), (), ()>::new(fiber_stack(1024 * 1024), move |(), _s| {
377+
let _ = &b;
378+
panic!();
379+
})
384380
.unwrap();
385381
assert!(panic::catch_unwind(AssertUnwindSafe(|| fiber.resume(()))).is_err());
386382
assert!(a.get());
@@ -396,14 +392,11 @@ mod tests {
396392

397393
#[test]
398394
fn suspend_and_resume_values() {
399-
let fiber = Fiber::new(
400-
FiberStack::new(1024 * 1024, false).unwrap(),
401-
move |first, s| {
402-
assert_eq!(first, 2.0);
403-
assert_eq!(s.suspend(4), 3.0);
404-
"hello".to_string()
405-
},
406-
)
395+
let fiber = Fiber::new(fiber_stack(1024 * 1024), move |first, s| {
396+
assert_eq!(first, 2.0);
397+
assert_eq!(s.suspend(4), 3.0);
398+
"hello".to_string()
399+
})
407400
.unwrap();
408401
assert_eq!(fiber.resume(2.0), Err(4));
409402
assert_eq!(fiber.resume(3.0), Ok("hello".to_string()));

crates/fiber/src/unix.rs

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ use std::cell::Cell;
3636
use std::io;
3737
use std::ops::Range;
3838
use std::ptr;
39+
use std::sync::atomic::{AtomicUsize, Ordering};
3940

4041
pub type Error = io::Error;
4142

@@ -59,8 +60,26 @@ enum FiberStackStorage {
5960
Custom(Box<dyn RuntimeFiberStack>),
6061
}
6162

63+
// FIXME: this is a duplicate copy of what's already in the `wasmtime` crate. If
64+
// this changes that should change over there, and ideally one day we should
65+
// probably deduplicate the two.
66+
fn host_page_size() -> usize {
67+
static PAGE_SIZE: AtomicUsize = AtomicUsize::new(0);
68+
69+
return match PAGE_SIZE.load(Ordering::Relaxed) {
70+
0 => {
71+
let size = unsafe { libc::sysconf(libc::_SC_PAGESIZE).try_into().unwrap() };
72+
assert!(size != 0);
73+
PAGE_SIZE.store(size, Ordering::Relaxed);
74+
size
75+
}
76+
n => n,
77+
};
78+
}
79+
6280
impl FiberStack {
6381
pub fn new(size: usize, zeroed: bool) -> io::Result<Self> {
82+
let page_size = host_page_size();
6483
// The anonymous `mmap`s we use for `FiberStackStorage` are alawys
6584
// zeroed.
6685
let _ = zeroed;
@@ -70,7 +89,6 @@ impl FiberStack {
7089
if cfg!(asan) {
7190
return Self::from_custom(asan::new_fiber_stack(size)?);
7291
}
73-
let page_size = rustix::param::page_size();
7492
let stack = MmapFiberStack::new(size)?;
7593

7694
// An `MmapFiberStack` allocates a guard page at the bottom of the
@@ -102,7 +120,7 @@ impl FiberStack {
102120

103121
pub fn from_custom(custom: Box<dyn RuntimeFiberStack>) -> io::Result<Self> {
104122
let range = custom.range();
105-
let page_size = rustix::param::page_size();
123+
let page_size = host_page_size();
106124
let start_ptr = range.start as *mut u8;
107125
assert!(
108126
start_ptr.align_offset(page_size) == 0,
@@ -153,7 +171,7 @@ impl MmapFiberStack {
153171
fn new(size: usize) -> io::Result<Self> {
154172
// Round up our stack size request to the nearest multiple of the
155173
// page size.
156-
let page_size = rustix::param::page_size();
174+
let page_size = host_page_size();
157175
let size = if size == 0 {
158176
page_size
159177
} else {
@@ -306,10 +324,9 @@ impl Suspend {
306324
/// called around every stack switch with some other fiddly bits as well.
307325
#[cfg(asan)]
308326
mod asan {
309-
use super::{FiberStack, MmapFiberStack, RuntimeFiberStack};
327+
use super::{FiberStack, MmapFiberStack, RuntimeFiberStack, host_page_size};
310328
use alloc::boxed::Box;
311329
use alloc::vec::Vec;
312-
use rustix::param::page_size;
313330
use std::mem::ManuallyDrop;
314331
use std::ops::Range;
315332
use std::sync::Mutex;
@@ -426,7 +443,8 @@ mod asan {
426443
static FIBER_STACKS: Mutex<Vec<MmapFiberStack>> = Mutex::new(Vec::new());
427444

428445
pub fn new_fiber_stack(size: usize) -> std::io::Result<Box<dyn RuntimeFiberStack>> {
429-
let needed_size = size + page_size();
446+
let page_size = host_page_size();
447+
let needed_size = size + page_size;
430448
let mut stacks = FIBER_STACKS.lock().unwrap();
431449

432450
let stack = match stacks.iter().position(|i| needed_size <= i.mapping_len) {
@@ -436,7 +454,9 @@ mod asan {
436454
// ... otherwise allocate a brand new stack.
437455
None => MmapFiberStack::new(size)?,
438456
};
439-
let stack = AsanFiberStack(ManuallyDrop::new(stack));
457+
let stack = AsanFiberStack {
458+
mmap: ManuallyDrop::new(stack),
459+
};
440460
Ok(Box::new(stack))
441461
}
442462

@@ -445,27 +465,31 @@ mod asan {
445465
///
446466
/// On drop this stack will return the interior stack to the global
447467
/// `FIBER_STACKS` list.
448-
struct AsanFiberStack(ManuallyDrop<MmapFiberStack>);
468+
struct AsanFiberStack {
469+
mmap: ManuallyDrop<MmapFiberStack>,
470+
}
449471

450472
unsafe impl RuntimeFiberStack for AsanFiberStack {
451473
fn top(&self) -> *mut u8 {
452-
self.0.mapping_base.wrapping_byte_add(self.0.mapping_len)
474+
self.mmap
475+
.mapping_base
476+
.wrapping_byte_add(self.mmap.mapping_len)
453477
}
454478

455479
fn range(&self) -> Range<usize> {
456-
let base = self.0.mapping_base as usize;
457-
let end = base + self.0.mapping_len;
458-
base + page_size()..end
480+
let base = self.mmap.mapping_base as usize;
481+
let end = base + self.mmap.mapping_len;
482+
base + host_page_size()..end
459483
}
460484

461485
fn guard_range(&self) -> Range<*mut u8> {
462-
self.0.mapping_base..self.0.mapping_base.wrapping_add(page_size())
486+
self.mmap.mapping_base..self.mmap.mapping_base.wrapping_add(host_page_size())
463487
}
464488
}
465489

466490
impl Drop for AsanFiberStack {
467491
fn drop(&mut self) {
468-
let stack = unsafe { ManuallyDrop::take(&mut self.0) };
492+
let stack = unsafe { ManuallyDrop::take(&mut self.mmap) };
469493
FIBER_STACKS.lock().unwrap().push(stack);
470494
}
471495
}

crates/jit-debug/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ object = { workspace = true, optional = true }
2323
wasmtime-versioned-export-macros = { workspace = true }
2424

2525
[target.'cfg(target_os = "linux")'.dependencies]
26-
rustix = { workspace = true, features = ["mm", "param", "time"], optional = true }
26+
rustix = { workspace = true, features = ["mm", "time"], optional = true }
2727

2828
[features]
2929
gdb_jit_int = []

crates/jit-debug/src/perf_jitdump.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ pub struct JitDumpFile {
131131
jitdump_file: File,
132132

133133
map_addr: usize,
134+
map_len: usize,
134135

135136
/// Unique identifier for jitted code
136137
code_index: u64,
@@ -157,10 +158,11 @@ impl JitDumpFile {
157158
//
158159
// To match what some perf examples are doing we keep this `mmap` alive
159160
// until this agent goes away.
161+
let map_len = 1024;
160162
let map_addr = unsafe {
161163
let ptr = rustix::mm::mmap(
162164
ptr::null_mut(),
163-
rustix::param::page_size(),
165+
map_len,
164166
rustix::mm::ProtFlags::EXEC | rustix::mm::ProtFlags::READ,
165167
rustix::mm::MapFlags::PRIVATE,
166168
&jitdump_file,
@@ -171,6 +173,7 @@ impl JitDumpFile {
171173
let mut state = JitDumpFile {
172174
jitdump_file,
173175
map_addr,
176+
map_len,
174177
code_index: 0,
175178
e_machine,
176179
};
@@ -283,7 +286,7 @@ impl JitDumpFile {
283286
impl Drop for JitDumpFile {
284287
fn drop(&mut self) {
285288
unsafe {
286-
rustix::mm::munmap(self.map_addr as *mut _, rustix::param::page_size()).unwrap();
289+
rustix::mm::munmap(self.map_addr as *mut _, self.map_len).unwrap();
287290
}
288291
}
289292
}

crates/wasmtime/src/runtime/vm.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,8 @@ impl ModuleRuntimeInfo {
449449
/// Returns the host OS page size, in bytes.
450450
#[cfg(has_virtual_memory)]
451451
pub fn host_page_size() -> usize {
452+
// NB: this function is duplicated in `crates/fiber/src/unix.rs` so if this
453+
// changes that should probably get updated as well.
452454
static PAGE_SIZE: AtomicUsize = AtomicUsize::new(0);
453455

454456
return match PAGE_SIZE.load(Ordering::Relaxed) {

crates/wasmtime/src/runtime/vm/instance/allocator/pooling/generic_stack_pool.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
#![cfg_attr(not(asan), allow(dead_code))]
22

3+
use crate::PoolConcurrencyLimitError;
34
use crate::prelude::*;
4-
use crate::{PoolConcurrencyLimitError, runtime::vm::PoolingInstanceAllocatorConfig};
5+
use crate::runtime::vm::PoolingInstanceAllocatorConfig;
56
use std::sync::atomic::{AtomicU64, Ordering};
67

78
/// A generic implementation of a stack pool.

crates/wasmtime/src/runtime/vm/sys/unix/vm.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ pub unsafe fn decommit_pages(addr: *mut u8, len: usize) -> io::Result<()> {
6666
Ok(())
6767
}
6868

69+
// NB: this function is duplicated in `crates/fiber/src/unix.rs` so if this
70+
// changes that should probably get updated as well.
6971
pub fn get_page_size() -> usize {
7072
unsafe { libc::sysconf(libc::_SC_PAGESIZE).try_into().unwrap() }
7173
}

0 commit comments

Comments
 (0)