Skip to content

Commit 38c2e2a

Browse files
authored
Add OOM test for component InstancePre::instantiate_async (#12993)
* Add OOM test for component InstancePre::instantiate_async Fix infallible allocations in the component model instantiation path: - Use TryPrimaryMap for instance_states and instances in ComponentInstance - Use TryPrimaryMap for instances in ComponentStoreData - Use TryVec for scopes in ComponentTasksNotConcurrent - Use TryPrimaryMap for guest field in ResourceTables - Use try_new for Arc and Box allocations in instantiation/call paths - Make push_component_instance and push_instance_id return Result - Make enter_call_not_concurrent return Result * remove now-unused import * fix imports for different cfgs * really fix imports this time
1 parent 3aab955 commit 38c2e2a

9 files changed

Lines changed: 96 additions & 47 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
#![cfg(arc_try_new)]
2+
3+
use wasmtime::component::{Component, Linker};
4+
use wasmtime::{Config, Engine, Result, Store};
5+
use wasmtime_fuzzing::oom::OomTest;
6+
7+
#[tokio::test]
8+
async fn component_instance_pre_instantiate_async() -> Result<()> {
9+
let component_bytes = {
10+
let mut config = Config::new();
11+
config.concurrency_support(false);
12+
let engine = Engine::new(&config)?;
13+
Component::new(
14+
&engine,
15+
r#"
16+
(component
17+
(core module $m
18+
(func (export "id") (param i32) (result i32) (local.get 0))
19+
)
20+
(core instance $i (instantiate $m))
21+
(func (export "id") (param "x" s32) (result s32)
22+
(canon lift (core func $i "id"))
23+
)
24+
)
25+
"#,
26+
)?
27+
.serialize()?
28+
};
29+
let mut config = Config::new();
30+
config.enable_compiler(false);
31+
config.concurrency_support(false);
32+
let engine = Engine::new(&config)?;
33+
let component = unsafe { Component::deserialize(&engine, &component_bytes)? };
34+
let linker = Linker::<()>::new(&engine);
35+
let instance_pre = linker.instantiate_pre(&component)?;
36+
37+
OomTest::new()
38+
.test_async(|| async {
39+
let mut store = Store::try_new(&engine, ())?;
40+
let _instance = instance_pre.instantiate_async(&mut store).await?;
41+
Ok(())
42+
})
43+
.await
44+
}

crates/fuzzing/tests/oom/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ mod bit_set;
55
mod boxed;
66
mod btree_map;
77
mod caller;
8+
mod component_func;
89
mod config;
910
mod engine;
1011
mod entity_set;

crates/wasmtime/src/runtime/component/concurrent.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,7 +1450,7 @@ impl StoreOpaque {
14501450
) -> Result<()> {
14511451
log::trace!("enter sync call {callee:?}");
14521452
if !self.concurrency_support() {
1453-
return Ok(self.enter_call_not_concurrent());
1453+
return self.enter_call_not_concurrent();
14541454
}
14551455

14561456
let state = self.concurrent_state_mut();
@@ -1548,7 +1548,7 @@ impl StoreOpaque {
15481548
/// situations.
15491549
pub(crate) fn host_task_create(&mut self) -> Result<Option<TableId<HostTask>>> {
15501550
if !self.concurrency_support() {
1551-
self.enter_call_not_concurrent();
1551+
self.enter_call_not_concurrent()?;
15521552
return Ok(None);
15531553
}
15541554
let state = self.concurrent_state_mut();

crates/wasmtime/src/runtime/component/concurrent_disabled.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,15 +168,15 @@ impl StoreOpaque {
168168
_callee_async: bool,
169169
_callee: RuntimeInstance,
170170
) -> Result<()> {
171-
Ok(self.enter_call_not_concurrent())
171+
self.enter_call_not_concurrent()
172172
}
173173

174174
pub(crate) fn exit_guest_sync_call(&mut self) -> Result<()> {
175175
Ok(self.exit_call_not_concurrent())
176176
}
177177

178178
pub(crate) fn host_task_create(&mut self) -> Result<()> {
179-
Ok(self.enter_call_not_concurrent())
179+
self.enter_call_not_concurrent()
180180
}
181181

182182
pub(crate) fn host_task_reenter_caller(&mut self) -> Result<()> {

crates/wasmtime/src/runtime/component/func.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -771,15 +771,15 @@ impl Func {
771771
};
772772
if results_ty.abi.flat_count(max_flat).is_some() {
773773
let mut flat = src.iter();
774-
Ok(Box::new(
774+
Ok(try_new::<Box<_>>(
775775
results_ty
776776
.types
777777
.iter()
778778
.map(move |ty| Val::lift(cx, *ty, &mut flat)),
779-
))
779+
)?)
780780
} else {
781781
let iter = Self::load_results(cx, results_ty, &mut src.iter())?;
782-
Ok(Box::new(iter))
782+
Ok(try_new::<Box<_>>(iter)?)
783783
}
784784
}
785785

crates/wasmtime/src/runtime/component/instance.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -732,11 +732,11 @@ impl<'a> Instantiator<'a> {
732732
let instance = ComponentInstance::new(
733733
store.store_data().components.next_component_instance_id(),
734734
component,
735-
Arc::new(imported_resources),
735+
try_new::<Arc<_>>(imported_resources)?,
736736
imports,
737737
store.traitobj(),
738738
)?;
739-
let id = store.store_data_mut().push_component_instance(instance);
739+
let id = store.store_data_mut().push_component_instance(instance)?;
740740

741741
Ok(Instantiator {
742742
component,
@@ -878,7 +878,7 @@ impl<'a> Instantiator<'a> {
878878
store.0.exit_guest_sync_call()?;
879879
}
880880

881-
self.instance_mut(store.0).push_instance_id(i.id());
881+
self.instance_mut(store.0).push_instance_id(i.id())?;
882882
}
883883

884884
GlobalInitializer::LowerImport { import, index } => {

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

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,21 @@
11
use crate::prelude::*;
2-
#[cfg(feature = "component-model-async")]
3-
use crate::runtime::component::ResourceTable;
42
use crate::runtime::component::concurrent::ConcurrentState;
53
use crate::runtime::component::{HostResourceData, Instance};
64
use crate::runtime::vm;
7-
#[cfg(feature = "component-model-async")]
8-
use crate::runtime::vm::VMStore;
95
use crate::runtime::vm::component::{
106
CallContext, ComponentInstance, HandleTable, OwnedComponentInstance,
117
};
128
use crate::store::{StoreData, StoreId, StoreOpaque};
139
use crate::{AsContext, AsContextMut, Engine, Store, StoreContextMut};
1410
use core::pin::Pin;
15-
use wasmtime_environ::PrimaryMap;
1611
use wasmtime_environ::component::RuntimeComponentInstanceIndex;
12+
use wasmtime_environ::prelude::TryPrimaryMap;
13+
14+
#[cfg(feature = "component-model-async")]
15+
use crate::{
16+
component::ResourceTable,
17+
runtime::vm::{VMStore, component::InstanceState},
18+
};
1719

1820
/// Default amount of fuel allowed for all guest-to-host calls in the component
1921
/// model.
@@ -29,7 +31,7 @@ const DEFAULT_HOSTCALL_FUEL: usize = 128 << 20;
2931
pub struct ComponentStoreData {
3032
/// All component instances, in a similar manner to how core wasm instances
3133
/// are managed.
32-
instances: PrimaryMap<ComponentInstanceId, Option<OwnedComponentInstance>>,
34+
instances: TryPrimaryMap<ComponentInstanceId, Option<OwnedComponentInstance>>,
3335

3436
/// Whether an instance belonging to this store has trapped.
3537
trapped: bool,
@@ -151,15 +153,10 @@ impl ComponentStoreData {
151153
continue;
152154
};
153155

154-
assert!(
155-
instance
156-
.get_mut()
157-
.instance_states()
158-
.0
159-
.iter_mut()
160-
.all(|(_, state)| state.handle_table().is_empty()
161-
&& state.concurrent_state().pending_is_empty())
162-
);
156+
assert!(instance.get_mut().instance_states().0.iter_mut().all(
157+
|(_, state): (_, &mut InstanceState)| state.handle_table().is_empty()
158+
&& state.concurrent_state().pending_is_empty()
159+
));
163160
}
164161
}
165162

@@ -259,11 +256,11 @@ impl StoreData {
259256
pub(crate) fn push_component_instance(
260257
&mut self,
261258
data: OwnedComponentInstance,
262-
) -> ComponentInstanceId {
259+
) -> Result<ComponentInstanceId, OutOfMemory> {
263260
let expected = data.get().id();
264-
let ret = self.components.instances.push(Some(data));
261+
let ret = self.components.instances.push(Some(data))?;
265262
assert_eq!(expected, ret);
266-
ret
263+
Ok(ret)
267264
}
268265

269266
pub(crate) fn component_instance(&self, id: ComponentInstanceId) -> &ComponentInstance {
@@ -394,12 +391,13 @@ impl StoreOpaque {
394391
)
395392
}
396393

397-
pub(crate) fn enter_call_not_concurrent(&mut self) {
394+
pub(crate) fn enter_call_not_concurrent(&mut self) -> Result<()> {
398395
let state = match &mut self.component_data_mut().task_state {
399396
ComponentTaskState::NotConcurrent(state) => state,
400397
ComponentTaskState::Concurrent(_) => unreachable!(),
401398
};
402-
state.scopes.push(CallContext::default());
399+
state.scopes.push(CallContext::default())?;
400+
Ok(())
403401
}
404402

405403
pub(crate) fn exit_call_not_concurrent(&mut self) {
@@ -499,7 +497,7 @@ impl<T> StoreContextMut<'_, T> {
499497

500498
#[derive(Default)]
501499
pub struct ComponentTasksNotConcurrent {
502-
scopes: Vec<CallContext>,
500+
scopes: TryVec<CallContext>,
503501
}
504502

505503
impl ComponentTaskState {

crates/wasmtime/src/runtime/vm/component.rs

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use core::pin::Pin;
2929
use core::ptr::NonNull;
3030
use wasmtime_environ::component::*;
3131
use wasmtime_environ::error::OutOfMemory;
32+
use wasmtime_environ::prelude::TryPrimaryMap;
3233
use wasmtime_environ::{HostPtr, PrimaryMap, VMSharedTypeIndex};
3334

3435
#[allow(
@@ -129,11 +130,11 @@ pub struct ComponentInstance {
129130

130131
/// Contains state specific to each (sub-)component instance within this
131132
/// top-level instance.
132-
instance_states: PrimaryMap<RuntimeComponentInstanceIndex, InstanceState>,
133+
instance_states: TryPrimaryMap<RuntimeComponentInstanceIndex, InstanceState>,
133134

134135
/// What all compile-time-identified core instances are mapped to within the
135136
/// `Store` that this component belongs to.
136-
instances: PrimaryMap<RuntimeInstanceIndex, InstanceId>,
137+
instances: TryPrimaryMap<RuntimeInstanceIndex, InstanceId>,
137138

138139
/// Storage for the type information about resources within this component
139140
/// instance.
@@ -324,22 +325,24 @@ impl ComponentInstance {
324325
) -> Result<OwnedComponentInstance, OutOfMemory> {
325326
let offsets = VMComponentOffsets::new(HostPtr, component.env_component());
326327
let num_instances = component.env_component().num_runtime_component_instances;
327-
let mut instance_states = PrimaryMap::with_capacity(num_instances.try_into().unwrap());
328+
let mut instance_states = TryPrimaryMap::with_capacity(num_instances.try_into().unwrap())?;
328329
for _ in 0..num_instances {
329-
instance_states.push(InstanceState::default());
330+
instance_states.push(InstanceState::default())?;
330331
}
331332

333+
let instances = TryPrimaryMap::with_capacity(
334+
component
335+
.env_component()
336+
.num_runtime_instances
337+
.try_into()
338+
.unwrap(),
339+
)?;
340+
332341
let mut ret = OwnedInstance::new(ComponentInstance {
333342
id,
334343
offsets,
335344
instance_states,
336-
instances: PrimaryMap::with_capacity(
337-
component
338-
.env_component()
339-
.num_runtime_instances
340-
.try_into()
341-
.unwrap(),
342-
),
345+
instances,
343346
component: component.clone(),
344347
resource_types,
345348
imports: imports.clone(),
@@ -860,7 +863,7 @@ impl ComponentInstance {
860863
pub fn instance_states(
861864
self: Pin<&mut Self>,
862865
) -> (
863-
&mut PrimaryMap<RuntimeComponentInstanceIndex, InstanceState>,
866+
&mut TryPrimaryMap<RuntimeComponentInstanceIndex, InstanceState>,
864867
&ComponentTypes,
865868
) {
866869
// safety: we've chosen the `pin` guarantee of `self` to not apply to
@@ -906,7 +909,10 @@ impl ComponentInstance {
906909

907910
/// Pushes a new runtime instance that's been created into
908911
/// `self.instances`.
909-
pub fn push_instance_id(self: Pin<&mut Self>, id: InstanceId) -> RuntimeInstanceIndex {
912+
pub fn push_instance_id(
913+
self: Pin<&mut Self>,
914+
id: InstanceId,
915+
) -> Result<RuntimeInstanceIndex, OutOfMemory> {
910916
self.instances_mut().push(id)
911917
}
912918

@@ -920,7 +926,7 @@ impl ComponentInstance {
920926
self.instances[idx]
921927
}
922928

923-
fn instances_mut(self: Pin<&mut Self>) -> &mut PrimaryMap<RuntimeInstanceIndex, InstanceId> {
929+
fn instances_mut(self: Pin<&mut Self>) -> &mut TryPrimaryMap<RuntimeInstanceIndex, InstanceId> {
924930
// SAFETY: we've chosen the `Pin` guarantee of `Self` to not apply to
925931
// the map returned.
926932
unsafe { &mut self.get_unchecked_mut().instances }

crates/wasmtime/src/runtime/vm/component/resources.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ use crate::prelude::*;
2626
use core::error::Error;
2727
use core::fmt;
2828
use core::mem;
29-
use wasmtime_environ::PrimaryMap;
3029
use wasmtime_environ::component::{
3130
ComponentTypes, RuntimeComponentInstanceIndex, TypeResourceTableIndex,
3231
};
32+
use wasmtime_environ::prelude::TryPrimaryMap;
3333

3434
/// Contextual state necessary to perform resource-related operations.
3535
///
@@ -55,7 +55,7 @@ pub struct ResourceTables<'a> {
5555
/// `ResourceAny::resource_drop` which won't consult this table as it's
5656
/// only operating over the host table.
5757
pub guest: Option<(
58-
&'a mut PrimaryMap<RuntimeComponentInstanceIndex, InstanceState>,
58+
&'a mut TryPrimaryMap<RuntimeComponentInstanceIndex, InstanceState>,
5959
&'a ComponentTypes,
6060
)>,
6161

0 commit comments

Comments
 (0)