Change stack walking to stop at a precise fp

crates/cranelift/src/compiler.rs

@@ -280,7 +280,7 @@ impl wasmtime_environ::Compiler for Compiler {
         debug_assert_vmctx_kind(isa, &mut builder, vmctx, wasmtime_environ::VMCONTEXT_MAGIC);
         let offsets = VMOffsets::new(isa.pointer_bytes(), &translation.module);
         let vm_runtime_limits_offset = offsets.ptr.vmctx_runtime_limits();
-        save_last_wasm_entry_sp(
+        save_last_wasm_entry_fp(
             &mut builder,
             pointer_type,
             &offsets.ptr,
@@ -969,7 +969,7 @@ fn debug_assert_vmctx_kind(
     }
 }
 
-fn save_last_wasm_entry_sp(
+fn save_last_wasm_entry_fp(
     builder: &mut FunctionBuilder,
     pointer_type: ir::Type,
     ptr_size: &impl PtrSize,
@@ -985,12 +985,12 @@ fn save_last_wasm_entry_sp(
     );
 
     // Then store our current stack pointer into the appropriate slot.
-    let sp = builder.ins().get_stack_pointer(pointer_type);
+    let fp = builder.ins().get_frame_pointer(pointer_type);
     builder.ins().store(
         MemFlags::trusted(),
-        sp,
+        fp,
         limits,
-        ptr_size.vmruntime_limits_last_wasm_entry_sp(),
+        ptr_size.vmruntime_limits_last_wasm_entry_fp(),
     );
 }
 

crates/environ/src/vmoffsets.rs

@@ -172,8 +172,8 @@ pub trait PtrSize {
         self.vmruntime_limits_last_wasm_exit_fp() + self.size()
     }
 
-    /// Return the offset of the `last_wasm_entry_sp` field of `VMRuntimeLimits`.
-    fn vmruntime_limits_last_wasm_entry_sp(&self) -> u8 {
+    /// Return the offset of the `last_wasm_entry_fp` field of `VMRuntimeLimits`.
+    fn vmruntime_limits_last_wasm_entry_fp(&self) -> u8 {
         self.vmruntime_limits_last_wasm_exit_pc() + self.size()
     }
 

crates/fuzzing/src/generators/stacks.rs

@@ -8,10 +8,11 @@
 use std::mem;
 
 use arbitrary::{Arbitrary, Result, Unstructured};
-use wasm_encoder::Instruction;
+use wasm_encoder::{Instruction, ValType};
 
-const MAX_FUNCS: usize = 20;
+const MAX_FUNCS: u32 = 20;
 const MAX_OPS: usize = 1_000;
+const MAX_PARAMS: usize = 10;
 
 /// Generate a Wasm module that keeps track of its current call stack, to
 /// compare to the host.
@@ -24,13 +25,16 @@ pub struct Stacks {
 #[derive(Debug, Default)]
 struct Function {
     ops: Vec<Op>,
+    params: usize,
+    results: usize,
 }
 
-#[derive(Arbitrary, Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy)]
 enum Op {
     CheckStackInHost,
     Call(u32),
     CallThroughHost(u32),
+    ReturnCall(u32),
 }
 
 impl<'a> Arbitrary<'a> for Stacks {
@@ -44,35 +48,44 @@ impl<'a> Arbitrary<'a> for Stacks {
 
 impl Stacks {
     fn arbitrary_funcs(u: &mut Unstructured) -> Result<Vec<Function>> {
-        let mut funcs = vec![Function::default()];
-
-        // The indices of functions within `funcs` that we still need to
-        // generate.
-        let mut work_list = vec![0];
+        // Generate a list of functions first with a number of parameters and
+        // results. Bodies are generated afterwards.
+        let nfuncs = u.int_in_range(1..=MAX_FUNCS)?;
+        let mut funcs = (0..nfuncs)
+            .map(|_| {
+                Ok(Function {
+                    ops: Vec::new(), // generated later
+                    params: u.int_in_range(0..=MAX_PARAMS)?,
+                    results: u.int_in_range(0..=MAX_PARAMS)?,
+                })
+            })
+            .collect::<Result<Vec<_>>>()?;
+        let mut funcs_by_result = vec![Vec::new(); MAX_PARAMS + 1];
+        for (i, func) in funcs.iter().enumerate() {
+            funcs_by_result[func.results].push(i as u32);
+        }
 
-        while let Some(f) = work_list.pop() {
-            let mut ops = Vec::with_capacity(u.arbitrary_len::<Op>()?.min(MAX_OPS));
-            for _ in 0..ops.capacity() {
-                ops.push(u.arbitrary()?);
-            }
-            for op in &mut ops {
-                match op {
-                    Op::CallThroughHost(idx) | Op::Call(idx) => {
-                        if u.is_empty() || funcs.len() >= MAX_FUNCS || u.ratio(4, 5)? {
-                            // Call an existing function.
-                            *idx = *idx % u32::try_from(funcs.len()).unwrap();
-                        } else {
-                            // Call a new function...
-                            *idx = u32::try_from(funcs.len()).unwrap();
-                            // ...which means we also need to eventually define it.
-                            work_list.push(funcs.len());
-                            funcs.push(Function::default());
-                        }
-                    }
-                    Op::CheckStackInHost => {}
+        // Fill in each function body with various instructions/operations now
+        // that the set of functions is known.
+        for f in funcs.iter_mut() {
+            let funcs_with_same_results = &funcs_by_result[f.results];
+            for _ in 0..u.arbitrary_len::<usize>()?.min(MAX_OPS) {
+                let op = match u.int_in_range(0..=3)? {
+                    0 => Op::CheckStackInHost,
+                    1 => Op::Call(u.int_in_range(0..=nfuncs - 1)?),
+                    2 => Op::CallThroughHost(u.int_in_range(0..=nfuncs - 1)?),
+                    // This only works if the target function has the same
+                    // number of results, so choose from a different set here.
+                    3 => Op::ReturnCall(*u.choose(funcs_with_same_results)?),
+                    _ => unreachable!(),
+                };
+                f.ops.push(op);
+                // once a `return_call` has been generated there's no need to
+                // generate any more instructions, so fall through to below.
+                if let Some(Op::ReturnCall(_)) = f.ops.last() {
+                    break;
                 }
             }
-            funcs[f].ops = ops;
         }
 
         Ok(funcs)
@@ -120,22 +133,30 @@ impl Stacks {
             vec![wasm_encoder::ValType::I32, wasm_encoder::ValType::I32],
         );
 
-        let null_type = types.len();
-        types.ty().function(vec![], vec![]);
-
         let call_func_type = types.len();
         types
             .ty()
             .function(vec![wasm_encoder::ValType::FUNCREF], vec![]);
 
+        let check_stack_type = types.len();
+        types.ty().function(vec![], vec![]);
+
+        let func_types_start = types.len();
+        for func in self.funcs.iter() {
+            types.ty().function(
+                vec![ValType::I32; func.params],
+                vec![ValType::I32; func.results],
+            );
+        }
+
         section(&mut module, types);
 
         let mut imports = wasm_encoder::ImportSection::new();
         let check_stack_func = 0;
         imports.import(
             "host",
             "check_stack",
-            wasm_encoder::EntityType::Function(null_type),
+            wasm_encoder::EntityType::Function(check_stack_type),
         );
         let call_func_func = 1;
         imports.import(
@@ -147,8 +168,8 @@ impl Stacks {
         section(&mut module, imports);
 
         let mut funcs = wasm_encoder::FunctionSection::new();
-        for _ in &self.funcs {
-            funcs.function(null_type);
+        for (i, _) in self.funcs.iter().enumerate() {
+            funcs.function(func_types_start + (i as u32));
         }
         let run_func = funcs.len() + num_imported_funcs;
         funcs.function(run_type);
@@ -246,6 +267,25 @@ impl Stacks {
                 .instruction(&Instruction::GlobalSet(stack_len_global));
         };
 
+        let push_params = |body: &mut wasm_encoder::Function, func: u32| {
+            let func = &self.funcs[func as usize];
+            for _ in 0..func.params {
+                body.instruction(&Instruction::I32Const(0));
+            }
+        };
+        let pop_results = |body: &mut wasm_encoder::Function, func: u32| {
+            let func = &self.funcs[func as usize];
+            for _ in 0..func.results {
+                body.instruction(&Instruction::Drop);
+            }
+        };
+        let push_results = |body: &mut wasm_encoder::Function, func: u32| {
+            let func = &self.funcs[func as usize];
+            for _ in 0..func.results {
+                body.instruction(&Instruction::I32Const(0));
+            }
+        };
+
         let mut code = wasm_encoder::CodeSection::new();
         for (func_index, func) in self.funcs.iter().enumerate() {
             let mut body = wasm_encoder::Function::new(vec![]);
@@ -256,19 +296,35 @@ impl Stacks {
             );
             check_fuel(&mut body);
 
+            let mut check_fuel_and_pop_at_end = true;
+
             // Perform our specified operations.
             for op in &func.ops {
+                assert!(check_fuel_and_pop_at_end);
                 match op {
                     Op::CheckStackInHost => {
                         body.instruction(&Instruction::Call(check_stack_func));
                     }
                     Op::Call(f) => {
+                        push_params(&mut body, *f);
                         body.instruction(&Instruction::Call(f + num_imported_funcs));
+                        pop_results(&mut body, *f);
                     }
                     Op::CallThroughHost(f) => {
                         body.instruction(&Instruction::RefFunc(f + num_imported_funcs))
                             .instruction(&Instruction::Call(call_func_func));
                     }
+
+                    // For a `return_call` preemptively check fuel to possibly
+                    // trap and then pop our function from the in-wasm managed
+                    // stack. After that execute the `return_call` itself.
+                    Op::ReturnCall(f) => {
+                        push_params(&mut body, *f);
+                        check_fuel(&mut body);
+                        pop_func_from_stack(&mut body);
+                        check_fuel_and_pop_at_end = false;
+                        body.instruction(&Instruction::ReturnCall(f + num_imported_funcs));
+                    }
                 }
             }
 
@@ -278,9 +334,11 @@ impl Stacks {
             // function, but then we returned back to Wasm and then trapped
             // while `last_wasm_exit_sp` et al are still initialized from that
             // previous host call.
-            check_fuel(&mut body);
-
-            pop_func_from_stack(&mut body);
+            if check_fuel_and_pop_at_end {
+                check_fuel(&mut body);
+                pop_func_from_stack(&mut body);
+                push_results(&mut body, func_index as u32);
+            }
 
             function(&mut code, body);
         }
@@ -307,7 +365,9 @@ impl Stacks {
         check_fuel(&mut run_body);
 
         // Call the first locally defined function.
+        push_params(&mut run_body, 0);
         run_body.instruction(&Instruction::Call(num_imported_funcs));
+        pop_results(&mut run_body, 0);
 
         check_fuel(&mut run_body);
         pop_func_from_stack(&mut run_body);

crates/fuzzing/src/oracles/stacks.rs

@@ -40,7 +40,10 @@ pub fn check_stacks(stacks: Stacks) -> usize {
             "call_func",
             |mut caller: Caller<'_, ()>, f: Option<Func>| {
                 let f = f.unwrap();
-                f.call(&mut caller, &[], &mut [])?;
+                let ty = f.ty(&caller);
+                let params = vec![Val::I32(0); ty.params().len()];
+                let mut results = vec![Val::I32(0); ty.results().len()];
+                f.call(&mut caller, &params, &mut results)?;
                 Ok(())
             },
         )

crates/wasmtime/src/runtime/vm/arch/aarch64.rs

@@ -50,14 +50,6 @@ pub unsafe fn get_next_older_pc_from_fp(fp: usize) -> usize {
 // And the current frame pointer points to the next older frame pointer.
 pub const NEXT_OLDER_FP_FROM_FP_OFFSET: usize = 0;
 
-pub fn reached_entry_sp(fp: usize, entry_sp: usize) -> bool {
-    fp >= entry_sp
-}
-
-pub fn assert_entry_sp_is_aligned(sp: usize) {
-    assert_eq!(sp % 16, 0, "stack should always be aligned to 16");
-}
-
 pub fn assert_fp_is_aligned(_fp: usize) {
     // From AAPCS64, section 6.2.3 The Frame Pointer[0]:
     //

crates/wasmtime/src/runtime/vm/arch/mod.rs

@@ -40,14 +40,6 @@ pub unsafe fn get_next_older_pc_from_fp(fp: usize) -> usize {
 
 pub const NEXT_OLDER_FP_FROM_FP_OFFSET: usize = imp::NEXT_OLDER_FP_FROM_FP_OFFSET;
 
-pub fn reached_entry_sp(fp: usize, entry_sp: usize) -> bool {
-    imp::reached_entry_sp(fp, entry_sp)
-}
-
-pub fn assert_entry_sp_is_aligned(sp: usize) {
-    imp::assert_entry_sp_is_aligned(sp)
-}
-
 pub fn assert_fp_is_aligned(fp: usize) {
     imp::assert_fp_is_aligned(fp)
 }

crates/wasmtime/src/runtime/vm/arch/riscv64.rs

@@ -21,14 +21,6 @@ pub unsafe fn get_next_older_pc_from_fp(fp: usize) -> usize {
 // And the current frame pointer points to the next older frame pointer.
 pub const NEXT_OLDER_FP_FROM_FP_OFFSET: usize = 0;
 
-pub fn reached_entry_sp(fp: usize, entry_sp: usize) -> bool {
-    fp >= entry_sp
-}
-
-pub fn assert_entry_sp_is_aligned(sp: usize) {
-    assert_eq!(sp % 16, 0, "stack should always be aligned to 16");
-}
-
 pub fn assert_fp_is_aligned(fp: usize) {
     assert_eq!(fp % 16, 0, "stack should always be aligned to 16");
 }

crates/wasmtime/src/runtime/vm/arch/s390x.rs

@@ -17,14 +17,6 @@ pub unsafe fn get_next_older_pc_from_fp(fp: usize) -> usize {
 // by the current "FP".
 pub const NEXT_OLDER_FP_FROM_FP_OFFSET: usize = 0;
 
-pub fn reached_entry_sp(fp: usize, entry_sp: usize) -> bool {
-    fp > entry_sp
-}
-
-pub fn assert_entry_sp_is_aligned(sp: usize) {
-    assert_eq!(sp % 8, 0, "stack should always be aligned to 8");
-}
-
 pub fn assert_fp_is_aligned(fp: usize) {
     assert_eq!(fp % 8, 0, "stack should always be aligned to 8");
 }

crates/wasmtime/src/runtime/vm/arch/unsupported.rs

@@ -31,14 +31,6 @@ pub unsafe fn get_next_older_pc_from_fp(_fp: usize) -> usize {
 
 pub const NEXT_OLDER_FP_FROM_FP_OFFSET: usize = 0;
 
-pub fn reached_entry_sp(_fp: usize, _entry_sp: usize) -> bool {
-    panic!()
-}
-
-pub fn assert_entry_sp_is_aligned(_sp: usize) {
-    panic!()
-}
-
 pub fn assert_fp_is_aligned(_fp: usize) {
     panic!()
 }

crates/wasmtime/src/runtime/vm/arch/x86_64.rs

@@ -23,14 +23,6 @@ pub unsafe fn get_next_older_pc_from_fp(fp: usize) -> usize {
 // And the current frame pointer points to the next older frame pointer.
 pub const NEXT_OLDER_FP_FROM_FP_OFFSET: usize = 0;
 
-pub fn reached_entry_sp(fp: usize, entry_sp: usize) -> bool {
-    fp >= entry_sp
-}
-
-pub fn assert_entry_sp_is_aligned(sp: usize) {
-    assert_eq!(sp % 16, 0, "stack should always be aligned to 16");
-}
-
 pub fn assert_fp_is_aligned(fp: usize) {
     assert_eq!(fp % 16, 0, "stack should always be aligned to 16");
 }