Skip to content

Commit d2dee5d

Browse files
authored
Handle OOM in {Func,Memory,Table,Global}::new and when calling an instance's exported function (bytecodealliance#12855)
* Use `try_new` for `Box<dyn RuntimeLinearMemory>` in `DefaultMemoryCreator` * Use `TryPrimaryMap` for `host_globals` in `Store` * Add `Func::try_wrap` and use `try_new` for `Box<HostFunc>` Add `Func::try_wrap` as a fallible version of `Func::wrap` that returns an error on out-of-memory instead of panicking. `Func::wrap` now delegates to `try_wrap`. Also use `try_new::<Box<_>>` instead of `Box::new` for `HostFunc`. * Use `bumpalo`'s `try_alloc` for `FuncRefs` * Use `try_new` for `Arc<Module>` in "trampoline" code * Test that we handle OOM in `{Func,Memory,Table,Global}::new` and when calling an instance's exported function * cargo fmt
1 parent c2abcc2 commit d2dee5d

File tree

14 files changed

+170
-16
lines changed

14 files changed

+170
-16
lines changed

crates/fuzzing/tests/oom/func.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#![cfg(arc_try_new)]
2+
3+
use wasmtime::{Config, Engine, Func, Result, Store};
4+
use wasmtime_fuzzing::oom::OomTest;
5+
6+
#[test]
7+
fn func_new() -> Result<()> {
8+
let mut config = Config::new();
9+
config.enable_compiler(false);
10+
config.concurrency_support(false);
11+
let engine = Engine::new(&config)?;
12+
13+
OomTest::new().test(|| {
14+
let mut store = Store::try_new(&engine, ())?;
15+
let _func = Func::try_wrap(&mut store, |x: i32| x * 2)?;
16+
Ok(())
17+
})
18+
}

crates/fuzzing/tests/oom/global.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
#![cfg(arc_try_new)]
2+
3+
use wasmtime::{Config, Engine, GlobalType, Mutability, Result, Store, Val, ValType};
4+
use wasmtime_fuzzing::oom::OomTest;
5+
6+
#[test]
7+
fn global_new() -> Result<()> {
8+
let mut config = Config::new();
9+
config.enable_compiler(false);
10+
config.concurrency_support(false);
11+
let engine = Engine::new(&config)?;
12+
13+
OomTest::new().test(|| {
14+
let mut store = Store::try_new(&engine, ())?;
15+
let ty = GlobalType::new(ValType::I32, Mutability::Var);
16+
let _global = wasmtime::Global::new(&mut store, ty, Val::I32(42))?;
17+
Ok(())
18+
})
19+
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
#![cfg(arc_try_new)]
2+
3+
use wasmtime::{Config, Engine, Linker, Module, Result, Store};
4+
use wasmtime_fuzzing::oom::OomTest;
5+
6+
#[test]
7+
fn call_exported_func() -> Result<()> {
8+
let module_bytes = {
9+
let mut config = Config::new();
10+
config.concurrency_support(false);
11+
let engine = Engine::new(&config)?;
12+
let module = Module::new(
13+
&engine,
14+
r#"
15+
(module
16+
(func (export "add") (param i32 i32) (result i32)
17+
(i32.add (local.get 0) (local.get 1))
18+
)
19+
)
20+
"#,
21+
)?;
22+
module.serialize()?
23+
};
24+
25+
let mut config = Config::new();
26+
config.enable_compiler(false);
27+
config.concurrency_support(false);
28+
let engine = Engine::new(&config)?;
29+
30+
let module = unsafe { Module::deserialize(&engine, &module_bytes)? };
31+
let linker = Linker::<()>::new(&engine);
32+
let instance_pre = linker.instantiate_pre(&module)?;
33+
34+
OomTest::new().test(|| {
35+
let mut store = Store::try_new(&engine, ())?;
36+
let instance = instance_pre.instantiate(&mut store)?;
37+
let add = instance.get_typed_func::<(i32, i32), i32>(&mut store, "add")?;
38+
let result = add.call(&mut store, (1, 2))?;
39+
assert_eq!(result, 3);
40+
Ok(())
41+
})
42+
}

crates/fuzzing/tests/oom/main.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,23 @@ mod config;
88
mod engine;
99
mod entity_set;
1010
mod error;
11+
mod func;
1112
mod func_type;
13+
mod global;
1214
mod hash_map;
1315
mod hash_set;
1416
mod index_map;
17+
mod instance;
1518
mod instance_pre;
1619
mod linker;
20+
mod memory;
1721
mod module;
1822
mod primary_map;
1923
mod secondary_map;
2024
mod smoke;
2125
mod store;
2226
mod string;
27+
mod table;
2328
mod vec;
2429

2530
use wasmtime_fuzzing::oom::OomTestAllocator;

crates/fuzzing/tests/oom/memory.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#![cfg(arc_try_new)]
2+
3+
use wasmtime::{Config, Engine, MemoryType, Result, Store};
4+
use wasmtime_fuzzing::oom::OomTest;
5+
6+
#[test]
7+
fn memory_new() -> Result<()> {
8+
let mut config = Config::new();
9+
config.enable_compiler(false);
10+
config.concurrency_support(false);
11+
let engine = Engine::new(&config)?;
12+
13+
OomTest::new()
14+
// `IndexMap::reserve` will try to allocate double space, but if that
15+
// fails, will attempt to allocate the minimal space necessary.
16+
.allow_alloc_after_oom(true)
17+
.test(|| {
18+
let mut store = Store::try_new(&engine, ())?;
19+
let _memory = wasmtime::Memory::new(&mut store, MemoryType::new(1, None))?;
20+
Ok(())
21+
})
22+
}

crates/fuzzing/tests/oom/table.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#![cfg(arc_try_new)]
2+
3+
use wasmtime::{Config, Engine, Ref, RefType, Result, Store, TableType};
4+
use wasmtime_fuzzing::oom::OomTest;
5+
6+
#[test]
7+
fn table_new() -> Result<()> {
8+
let mut config = Config::new();
9+
config.enable_compiler(false);
10+
config.concurrency_support(false);
11+
let engine = Engine::new(&config)?;
12+
13+
OomTest::new()
14+
// `IndexMap::reserve` will try to allocate double space, but if that
15+
// fails, will attempt to allocate the minimal space necessary.
16+
.allow_alloc_after_oom(true)
17+
.test(|| {
18+
let mut store = Store::try_new(&engine, ())?;
19+
let ty = TableType::new(RefType::FUNCREF, 1, None);
20+
let _table = wasmtime::Table::new(&mut store, ty, Ref::Func(None))?;
21+
Ok(())
22+
})
23+
}

crates/wasmtime/src/runtime/func.rs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -783,19 +783,33 @@ impl Func {
783783
/// # }
784784
/// ```
785785
pub fn wrap<T, Params, Results>(
786-
mut store: impl AsContextMut<Data = T>,
786+
store: impl AsContextMut<Data = T>,
787787
func: impl IntoFunc<T, Params, Results>,
788788
) -> Func
789+
where
790+
T: 'static,
791+
{
792+
Self::try_wrap(store, func).expect(
793+
"allocation failure during `Func::wrap` (use `Func::try_wrap` to handle such errors)",
794+
)
795+
}
796+
797+
/// Fallible version of [`Func::wrap`] that returns an error on
798+
/// out-of-memory instead of panicking.
799+
pub fn try_wrap<T, Params, Results>(
800+
mut store: impl AsContextMut<Data = T>,
801+
func: impl IntoFunc<T, Params, Results>,
802+
) -> Result<Func>
789803
where
790804
T: 'static,
791805
{
792806
let store = store.as_context_mut().0;
793807
let engine = store.engine();
794-
let host = func.into_func(engine).panic_on_oom();
808+
let host = func.into_func(engine)?;
795809

796810
// SAFETY: The `T` the closure takes is the same as the `T` of the store
797811
// we're inserting into via the type signature above.
798-
unsafe { host.into_func(store).panic_on_oom() }
812+
Ok(unsafe { host.into_func(store)? })
799813
}
800814

801815
/// Same as [`Func::wrap`], except the closure asynchronously produces the
@@ -2693,7 +2707,7 @@ impl HostFunc {
26932707
store.set_async_required(self.asyncness);
26942708

26952709
let (funcrefs, modules) = store.func_refs_and_modules();
2696-
let funcref = funcrefs.push_box_host(Box::new(self), modules)?;
2710+
let funcref = funcrefs.push_box_host(try_new::<Box<_>>(self)?, modules)?;
26972711
// SAFETY: this funcref was just pushed within `store`, so it's safe to
26982712
// say it's owned by the store's id.
26992713
Ok(unsafe { Func::from_vm_func_ref(store.id(), funcref) })

crates/wasmtime/src/runtime/store.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ use core::num::NonZeroU64;
112112
use core::ops::{Deref, DerefMut};
113113
use core::pin::Pin;
114114
use core::ptr::NonNull;
115-
use wasmtime_environ::{DefinedGlobalIndex, DefinedTableIndex, EntityRef, PrimaryMap, TripleExt};
115+
use wasmtime_environ::{DefinedGlobalIndex, DefinedTableIndex, EntityRef, TripleExt};
116116

117117
mod context;
118118
pub use self::context::*;
@@ -478,7 +478,7 @@ pub struct StoreOpaque {
478478
signal_handler: Option<SignalHandler>,
479479
modules: ModuleRegistry,
480480
func_refs: FuncRefs,
481-
host_globals: PrimaryMap<DefinedGlobalIndex, StoreBox<VMHostGlobalContext>>,
481+
host_globals: TryPrimaryMap<DefinedGlobalIndex, StoreBox<VMHostGlobalContext>>,
482482
// GC-related fields.
483483
gc_store: Option<GcStore>,
484484
gc_roots: RootSet,
@@ -756,7 +756,7 @@ impl<T> Store<T> {
756756
pending_exception: None,
757757
modules: ModuleRegistry::default(),
758758
func_refs: FuncRefs::default(),
759-
host_globals: PrimaryMap::new(),
759+
host_globals: TryPrimaryMap::new(),
760760
instance_count: 0,
761761
instance_limit: crate::DEFAULT_INSTANCE_LIMIT,
762762
memory_count: 0,
@@ -1687,13 +1687,13 @@ impl StoreOpaque {
16871687

16881688
pub(crate) fn host_globals(
16891689
&self,
1690-
) -> &PrimaryMap<DefinedGlobalIndex, StoreBox<VMHostGlobalContext>> {
1690+
) -> &TryPrimaryMap<DefinedGlobalIndex, StoreBox<VMHostGlobalContext>> {
16911691
&self.host_globals
16921692
}
16931693

16941694
pub(crate) fn host_globals_mut(
16951695
&mut self,
1696-
) -> &mut PrimaryMap<DefinedGlobalIndex, StoreBox<VMHostGlobalContext>> {
1696+
) -> &mut TryPrimaryMap<DefinedGlobalIndex, StoreBox<VMHostGlobalContext>> {
16971697
&mut self.host_globals
16981698
}
16991699

crates/wasmtime/src/runtime/store/func_refs.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use crate::prelude::*;
77
use crate::runtime::HostFunc;
88
use crate::runtime::vm::{AlwaysMut, SendSyncPtr, VMArrayCallHostFuncContext, VMFuncRef};
99
use alloc::sync::Arc;
10+
use core::mem::size_of;
1011
use core::ptr::NonNull;
1112

1213
/// An arena of `VMFuncRef`s.
@@ -92,7 +93,11 @@ impl FuncRefs {
9293
modules: &ModuleRegistry,
9394
) -> Result<NonNull<VMFuncRef>, OutOfMemory> {
9495
debug_assert!(func_ref.wasm_call.is_none());
95-
let func_ref = self.bump.get_mut().alloc(func_ref);
96+
let func_ref = self
97+
.bump
98+
.get_mut()
99+
.try_alloc(func_ref)
100+
.map_err(|_| OutOfMemory::new(size_of::<VMFuncRef>()))?;
96101
// SAFETY: it's a contract of this function itself that `func_ref` has a
97102
// valid vmctx field to read.
98103
let has_hole = unsafe { !try_fill(func_ref, modules) };

crates/wasmtime/src/runtime/trampoline/global.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,6 @@ pub fn generate_global_export(
8383
}
8484
}
8585

86-
let index = store.host_globals_mut().push(ctx);
86+
let index = store.host_globals_mut().push(ctx)?;
8787
Ok(crate::Global::from_host(store.id(), index))
8888
}

0 commit comments

Comments
 (0)