Skip to content

Commit 122ddc7

Browse files
authored
Handle OOM in Func::call_async and fiber creation (#12954)
* Handle OOM in `Func::call_async` and fiber creation * fix clippy * fix build * really fix build * address review feedback * fix build * fix warnings
1 parent 7088e01 commit 122ddc7

File tree

6 files changed

+66
-26
lines changed

6 files changed

+66
-26
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.

crates/fiber/src/nostd.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
use crate::stackswitch::*;
2929
use crate::{Result, RunResult, RuntimeFiberStack};
3030
use alloc::boxed::Box;
31-
use alloc::{vec, vec::Vec};
3231
use core::cell::Cell;
3332
use core::ops::Range;
3433
use wasmtime_environ::prelude::*;
@@ -42,7 +41,7 @@ pub struct FiberStack {
4241
len: usize,
4342
/// Backing storage, if owned. Allocated once at startup and then
4443
/// not reallocated afterward.
45-
storage: Vec<u8>,
44+
storage: TryVec<u8>,
4645
}
4746

4847
struct BasePtr(*mut u8);
@@ -66,10 +65,10 @@ impl FiberStack {
6665
pub fn new(size: usize, zeroed: bool) -> Result<Self> {
6766
// Round up the size to at least one page.
6867
let size = core::cmp::max(4096, size);
69-
let mut storage = Vec::new();
70-
storage.try_reserve_exact(size)?;
68+
let mut storage = TryVec::new();
69+
storage.reserve_exact(size)?;
7170
if zeroed {
72-
storage.resize(size, 0);
71+
storage.resize(size, 0)?;
7372
}
7473
let (base, len) = align_ptr(storage.as_mut_ptr(), size, STACK_ALIGN);
7574
Ok(FiberStack {
@@ -81,7 +80,7 @@ impl FiberStack {
8180

8281
pub unsafe fn from_raw_parts(base: *mut u8, guard_size: usize, len: usize) -> Result<Self> {
8382
Ok(FiberStack {
84-
storage: vec![],
83+
storage: TryVec::default(),
8584
base: BasePtr(unsafe { base.offset(isize::try_from(guard_size).unwrap()) }),
8685
len,
8786
})
@@ -139,7 +138,7 @@ impl Fiber {
139138
bail!("fibers unsupported on this host architecture");
140139
}
141140
unsafe {
142-
let data = Box::into_raw(Box::new(func)).cast();
141+
let data = Box::into_raw(try_new::<Box<_>>(func)?).cast();
143142
wasmtime_fiber_init(stack.top().unwrap(), fiber_start::<F, A, B, C>, data);
144143
}
145144

crates/fiber/src/unix.rs

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,13 @@ use crate::stackswitch::*;
3333
use crate::{RunResult, RuntimeFiberStack};
3434
use std::boxed::Box;
3535
use std::cell::Cell;
36-
use std::io;
3736
use std::ops::Range;
3837
use std::ptr;
3938
use std::sync::atomic::{AtomicUsize, Ordering};
39+
use wasmtime_environ::error::{OutOfMemory, Result};
40+
use wasmtime_environ::prelude::*;
4041

41-
pub type Error = io::Error;
42+
pub type Error = wasmtime_environ::error::Error;
4243

4344
pub struct FiberStack {
4445
base: BasePtr,
@@ -78,7 +79,7 @@ fn host_page_size() -> usize {
7879
}
7980

8081
impl FiberStack {
81-
pub fn new(size: usize, zeroed: bool) -> io::Result<Self> {
82+
pub fn new(size: usize, zeroed: bool) -> Result<Self> {
8283
let page_size = host_page_size();
8384
// The anonymous `mmap`s we use for `FiberStackStorage` are always
8485
// zeroed.
@@ -101,7 +102,7 @@ impl FiberStack {
101102
})
102103
}
103104

104-
pub unsafe fn from_raw_parts(base: *mut u8, guard_size: usize, len: usize) -> io::Result<Self> {
105+
pub unsafe fn from_raw_parts(base: *mut u8, guard_size: usize, len: usize) -> Result<Self> {
105106
// See comments in `mod asan` below for why asan has a different stack
106107
// allocation strategy.
107108
if cfg!(asan) {
@@ -118,7 +119,7 @@ impl FiberStack {
118119
matches!(self.storage, FiberStackStorage::Unmanaged(_))
119120
}
120121

121-
pub fn from_custom(custom: Box<dyn RuntimeFiberStack>) -> io::Result<Self> {
122+
pub fn from_custom(custom: Box<dyn RuntimeFiberStack>) -> Result<Self> {
122123
let range = custom.range();
123124
let page_size = host_page_size();
124125
let start_ptr = range.start as *mut u8;
@@ -168,7 +169,7 @@ unsafe impl Send for MmapFiberStack {}
168169
unsafe impl Sync for MmapFiberStack {}
169170

170171
impl MmapFiberStack {
171-
fn new(size: usize) -> io::Result<Self> {
172+
fn new(size: usize) -> Result<Self> {
172173
// Round up our stack size request to the nearest multiple of the
173174
// page size.
174175
let page_size = host_page_size();
@@ -177,7 +178,7 @@ impl MmapFiberStack {
177178
} else {
178179
let with_extra = size
179180
.checked_add(page_size - 1)
180-
.ok_or(io::ErrorKind::OutOfMemory)?;
181+
.ok_or_else(|| OutOfMemory::new(usize::MAX))?;
181182
with_extra & (!(page_size - 1))
182183
};
183184

@@ -243,20 +244,17 @@ where
243244
}
244245

245246
impl Fiber {
246-
pub fn new<F, A, B, C>(stack: &FiberStack, func: F) -> io::Result<Self>
247+
pub fn new<F, A, B, C>(stack: &FiberStack, func: F) -> Result<Self>
247248
where
248249
F: FnOnce(A, &mut super::Suspend<A, B, C>) -> C,
249250
{
250251
// On unsupported platforms `wasmtime_fiber_init` is a panicking shim so
251252
// return an error saying the host architecture isn't supported instead.
252253
if !SUPPORTED_ARCH {
253-
return Err(io::Error::new(
254-
io::ErrorKind::Other,
255-
"fibers not supported on this host architecture",
256-
));
254+
bail!("fibers not supported on this host architecture");
257255
}
258256
unsafe {
259-
let data = Box::into_raw(Box::new(func)).cast();
257+
let data = Box::into_raw(try_new::<Box<_>>(func)?).cast();
260258
wasmtime_fiber_init(stack.top().unwrap(), fiber_start::<F, A, B, C>, data);
261259
}
262260

@@ -330,7 +328,7 @@ impl Suspend {
330328
/// called around every stack switch with some other fiddly bits as well.
331329
#[cfg(asan)]
332330
mod asan {
333-
use super::{FiberStack, MmapFiberStack, RuntimeFiberStack, host_page_size};
331+
use super::*;
334332
use alloc::boxed::Box;
335333
use alloc::vec::Vec;
336334
use std::mem::ManuallyDrop;
@@ -442,11 +440,11 @@ mod asan {
442440
/// meaning that this should only ever be a relatively small set of stacks.
443441
static FIBER_STACKS: Mutex<Vec<MmapFiberStack>> = Mutex::new(Vec::new());
444442

445-
pub fn new_fiber_stack(size: usize) -> std::io::Result<Box<dyn RuntimeFiberStack>> {
443+
pub fn new_fiber_stack(size: usize) -> Result<Box<dyn RuntimeFiberStack>> {
446444
let page_size = host_page_size();
447445
let needed_size = size
448446
.checked_add(page_size)
449-
.ok_or(std::io::ErrorKind::OutOfMemory)?;
447+
.ok_or_else(|| OutOfMemory::new(usize::MAX))?;
450448
let mut stacks = FIBER_STACKS.lock().unwrap();
451449

452450
let stack = match stacks.iter().position(|i| needed_size <= i.mapping_len) {
@@ -500,7 +498,7 @@ mod asan {
500498
// Shim module that's the same as above but only has stubs.
501499
#[cfg(not(asan))]
502500
mod asan_disabled {
503-
use super::{FiberStack, RuntimeFiberStack};
501+
use super::*;
504502
use std::boxed::Box;
505503

506504
#[derive(Default)]
@@ -527,7 +525,7 @@ mod asan_disabled {
527525
PreviousStack
528526
}
529527

530-
pub fn new_fiber_stack(_size: usize) -> std::io::Result<Box<dyn RuntimeFiberStack>> {
528+
pub fn new_fiber_stack(_size: usize) -> Result<Box<dyn RuntimeFiberStack>> {
531529
unimplemented!()
532530
}
533531
}

crates/fuzzing/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ rand = { version = "0.8.0", features = ["small_rng"] }
7474
wasmtime-environ = { workspace = true }
7575
cranelift-bforest = { workspace = true }
7676
cranelift-bitset = { workspace = true }
77+
tokio = { workspace = true, features = ["rt", "time", "macros", "rt-multi-thread"] }
7778

7879
# Only enable the `build-libinterpret` feature when fuzzing is enabled, enabling
7980
# commands like `cargo test --workspace` or similar to not need an ocaml

crates/fuzzing/tests/oom/func.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,40 @@ fn func_call() -> Result<()> {
6666
})
6767
}
6868

69+
#[tokio::test]
70+
async fn func_call_async() -> Result<()> {
71+
let module_bytes = {
72+
let mut config = Config::new();
73+
config.concurrency_support(false);
74+
let engine = Engine::new(&config)?;
75+
Module::new(
76+
&engine,
77+
r#"(module (func (export "id") (param i32) (result i32) (local.get 0)))"#,
78+
)?
79+
.serialize()?
80+
};
81+
let mut config = Config::new();
82+
config.enable_compiler(false);
83+
config.concurrency_support(false);
84+
let engine = Engine::new(&config)?;
85+
let module = unsafe { Module::deserialize(&engine, &module_bytes)? };
86+
let linker = Linker::<()>::new(&engine);
87+
let instance_pre = linker.instantiate_pre(&module)?;
88+
89+
OomTest::new()
90+
.test_async(|| async {
91+
let mut store = Store::try_new(&engine, ())?;
92+
let instance = instance_pre.instantiate_async(&mut store).await?;
93+
let id = instance.get_func(&mut store, "id").unwrap();
94+
let mut results = [Val::I32(0)];
95+
id.call_async(&mut store, &[Val::I32(42)], &mut results)
96+
.await?;
97+
assert_eq!(results[0].unwrap_i32(), 42);
98+
Ok(())
99+
})
100+
.await
101+
}
102+
69103
#[test]
70104
fn func_typed() -> Result<()> {
71105
let module_bytes = {

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,14 @@ impl StackPool {
6565
Ok(stack) => Ok(stack),
6666
Err(e) => {
6767
self.live_stacks.fetch_sub(1, Ordering::AcqRel);
68-
Err(crate::Error::from(e))
68+
69+
#[allow(
70+
clippy::useless_conversion,
71+
reason = "some `cfg`s have `wasmtime::Error` as the error type, others don't"
72+
)]
73+
let e = crate::Error::from(e);
74+
75+
Err(e)
6976
}
7077
}
7178
}

0 commit comments

Comments
 (0)