Skip to content

Commit 2283e84

Browse files
authored
Fix a panic with a massive max_wasm_stack configured (#12869)
* Fix a panic with a massive `max_wasm_stack` configured This commit fixes a panic through a `checked_add(...).unwrap()` which can happen when `Config::max_wasm_stack` is configured to be a very large value. This is a mostly benign panic as it's unlikely this is configured much in the wild, but nevertheless seems like a good issues to fix regardless. * Fix an overflow/OOM panic in pulley prtest:full * Fix CI * Another CI fix * Fix test on 32-bit * Fix miri test
1 parent 0fbbb75 commit 2283e84

7 files changed

Lines changed: 60 additions & 32 deletions

File tree

cranelift/filetests/src/function_runner.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ impl<'a> Trampoline<'a> {
446446
| Architecture::Pulley64
447447
| Architecture::Pulley32be
448448
| Architecture::Pulley64be => {
449-
let mut state = pulley::Vm::new();
449+
let mut state = pulley::Vm::new().unwrap();
450450
unsafe {
451451
state.call(
452452
NonNull::new(trampoline_ptr.cast_mut()).unwrap(),

crates/core/src/alloc/vec.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,17 @@ impl<T> TryVec<T> {
309309
pub fn clear(&mut self) {
310310
self.inner.clear();
311311
}
312+
313+
/// Same as [`std::vec::Vec::as_mut_ptr`].
314+
//
315+
// Note that this is technically inherited through the `DerefMut` impl but
316+
// that converts `&mut Self` to `&mut [T]` which invalidates all previously
317+
// derived pointers. This causes problems in Miri so by having an inherent
318+
// method here it means that the borrow scope matches what we want with
319+
// Miri.
320+
pub fn as_mut_ptr(&mut self) -> *mut T {
321+
self.inner.as_mut_ptr()
322+
}
312323
}
313324

314325
impl<T> Deref for TryVec<T> {

crates/wasmtime/src/runtime/func.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1530,9 +1530,12 @@ impl EntryStoreContext {
15301530
//
15311531
// After we've got the stack limit then we store it into the `stack_limit`
15321532
// variable.
1533-
let wasm_stack_limit = stack_pointer
1534-
.checked_sub(store.engine().config().max_wasm_stack)
1535-
.unwrap();
1533+
//
1534+
// Also note that `saturating_sub` is used here since if the user
1535+
// said that the function gets nigh-infinite stack well then by
1536+
// golly it'll get nigh-infinite stack in which case the limit is 0.
1537+
let wasm_stack_limit =
1538+
stack_pointer.saturating_sub(store.engine().config().max_wasm_stack);
15361539
let prev_stack = unsafe {
15371540
mem::replace(
15381541
&mut *store.0.vm_store_context().stack_limit.get(),

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ impl Interpreter {
6161
pub fn new(engine: &Engine) -> Result<Interpreter, OutOfMemory> {
6262
let ret = Interpreter {
6363
pulley: StoreBox::new(VmState {
64-
vm: Vm::with_stack(engine.config().max_wasm_stack),
64+
vm: Vm::with_stack(engine.config().max_wasm_stack)?,
6565
resume_at_pc: None,
6666
})?,
6767
};

pulley/src/interp.rs

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@ use crate::imms::*;
66
use crate::profile::{ExecutingPc, ExecutingPcRef};
77
use crate::regs::*;
88
use alloc::string::ToString;
9-
use alloc::vec::Vec;
109
use core::fmt;
1110
use core::mem;
1211
use core::ops::ControlFlow;
1312
use core::ops::{Index, IndexMut};
1413
use core::ptr::NonNull;
1514
use pulley_macros::interp_disable_if_cfg;
15+
use wasmtime_core::alloc::TryVec;
16+
use wasmtime_core::error::OutOfMemory;
1617
use wasmtime_core::math::{WasmFloat, f32_cvt_to_int_bounds, f64_cvt_to_int_bounds};
1718

1819
mod debug;
@@ -29,24 +30,18 @@ pub struct Vm {
2930
executing_pc: ExecutingPc,
3031
}
3132

32-
impl Default for Vm {
33-
fn default() -> Self {
34-
Vm::new()
35-
}
36-
}
37-
3833
impl Vm {
3934
/// Create a new virtual machine with the default stack size.
40-
pub fn new() -> Self {
35+
pub fn new() -> Result<Self, OutOfMemory> {
4136
Self::with_stack(DEFAULT_STACK_SIZE)
4237
}
4338

4439
/// Create a new virtual machine with the given stack.
45-
pub fn with_stack(stack_size: usize) -> Self {
46-
Self {
47-
state: MachineState::with_stack(stack_size),
40+
pub fn with_stack(stack_size: usize) -> Result<Self, OutOfMemory> {
41+
Ok(Self {
42+
state: MachineState::with_stack(stack_size)?,
4843
executing_pc: ExecutingPc::default(),
49-
}
44+
})
5045
}
5146

5247
/// Get a shared reference to this VM's machine state.
@@ -762,7 +757,7 @@ unsafe impl Sync for MachineState {}
762757
/// done with a custom `Vec<T>` internally where `T` has size and align of 16.
763758
/// This is manually done with a helper `Align16` type below.
764759
struct Stack {
765-
storage: Vec<Align16>,
760+
storage: TryVec<Align16>,
766761
}
767762

768763
/// Helper type used with `Stack` above.
@@ -778,14 +773,14 @@ impl Stack {
778773
/// Creates a new stack which will have a byte size of at least `size`.
779774
///
780775
/// The allocated stack might be slightly larger due to rounding necessary.
781-
fn new(size: usize) -> Stack {
782-
Stack {
783-
// Round up `size` to the nearest multiple of 16. Note that the
784-
// stack is also allocated here but not initialized, and that's
785-
// intentional as pulley bytecode should always initialize the stack
786-
// before use.
787-
storage: Vec::with_capacity((size + 15) / 16),
788-
}
776+
fn new(size: usize) -> Result<Stack, OutOfMemory> {
777+
let mut storage = TryVec::new();
778+
// Round up `size` to the nearest multiple of 16. Note that the
779+
// stack is also allocated here but not initialized, and that's
780+
// intentional as pulley bytecode should always initialize the stack
781+
// before use.
782+
storage.reserve_exact(size.checked_next_multiple_of(16).unwrap_or(usize::MAX) / 16)?;
783+
Ok(Stack { storage })
789784
}
790785

791786
/// Returns a pointer to the top of the stack (the highest address).
@@ -896,13 +891,13 @@ index_reg!(VReg, VRegVal, v_regs);
896891
const HOST_RETURN_ADDR: *mut u8 = usize::MAX as *mut u8;
897892

898893
impl MachineState {
899-
fn with_stack(stack_size: usize) -> Self {
894+
fn with_stack(stack_size: usize) -> Result<Self, OutOfMemory> {
900895
let mut state = Self {
901896
x_regs: [Default::default(); XReg::RANGE.end as usize],
902897
f_regs: Default::default(),
903898
#[cfg(not(pulley_disable_interp_simd))]
904899
v_regs: Default::default(),
905-
stack: Stack::new(stack_size),
900+
stack: Stack::new(stack_size)?,
906901
done_reason: None,
907902
fp: HOST_RETURN_ADDR,
908903
lr: HOST_RETURN_ADDR,
@@ -911,7 +906,7 @@ impl MachineState {
911906
let sp = state.stack.top();
912907
state[XReg::sp] = XRegVal::new_ptr(sp);
913908

914-
state
909+
Ok(state)
915910
}
916911
}
917912

@@ -1294,7 +1289,7 @@ impl AddressingMode for AddrG32Bne {
12941289

12951290
#[test]
12961291
fn simple_push_pop() {
1297-
let mut state = MachineState::with_stack(16);
1292+
let mut state = MachineState::with_stack(16).unwrap();
12981293
let pc = ExecutingPc::default();
12991294
unsafe {
13001295
let mut bytecode = [0; 10];

pulley/tests/all/interp.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ unsafe fn assert_one<R0, R1, V>(
3737
V: Into<Val>,
3838
{
3939
eprintln!("=======================================================");
40-
let mut vm = Vm::new();
40+
let mut vm = Vm::new().unwrap();
4141

4242
for (reg, val) in xs {
4343
let reg = reg.into();
@@ -797,7 +797,7 @@ fn bitcast_float_from_int_64() {
797797

798798
#[test]
799799
fn trap() {
800-
let mut vm = Vm::new();
800+
let mut vm = Vm::new().unwrap();
801801
let dst = XReg::new(0).unwrap();
802802

803803
unsafe {

tests/all/stack_overflow.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,3 +162,22 @@ fn big_stack_works_ok(config: &mut Config) -> Result<()> {
162162
assert_eq!(func.call(&mut store, ())?, 0);
163163
Ok(())
164164
}
165+
166+
#[test]
167+
fn infinite_wasm_stack_does_not_panic() -> Result<()> {
168+
let mut config = Config::new();
169+
config.max_wasm_stack(usize::MAX);
170+
config.async_stack_size(usize::MAX);
171+
let engine = Engine::new(&config)?;
172+
// If the store can't get allocated that's probably because we're using
173+
// pulley and can't allocate a usize::MAX stack, ignore that failure.
174+
// This'll still run on cranelift-native platforms.
175+
let Ok(mut store) = Store::try_new(&engine, ()) else {
176+
return Ok(());
177+
};
178+
let module = Module::new(store.engine(), r#"(module (func (export "f")))"#)?;
179+
let instance = Instance::new(&mut store, &module, &[])?;
180+
let func = instance.get_typed_func::<(), ()>(&mut store, "f")?;
181+
func.call(&mut store, ())?;
182+
Ok(())
183+
}

0 commit comments

Comments
 (0)