fix(cubecode): share one data stack across CALL_LINK frames
Ad-hoc verification (factorial via recursive CALL_LINK) surfaced a real defect: each exec_cell had its own private stack, but the design (and the doc contract) is that callees run on the SHARED data stack so a caller passes args by leaving them on the stack and reads the callee result there after RET. With per-frame stacks, a callee that popped its argument faulted with PcOutOfRange. - exec_cell now takes &mut Vec<u8> (the one shared stack) instead of owning a fresh one; run() owns it and threads it through recursion. - Added regression test shared_stack_passes_args_across_cube_edges (caller leaves 21, callee doubles it via Store/Load, caller sees 42) so the convention is locked by ./check, not just ad-hoc. The prior commit's unit tests didn't exercise cross-frame stack args, which is why the bug slipped through; suite is now 50 tests green (incl. 14 cubecode).
This commit is contained in:
+46
-7
@@ -120,10 +120,14 @@ impl<B: CubeBackend> Vm<B> {
|
||||
/// Run the entry cell to completion, returning its result or a fault.
|
||||
///
|
||||
/// `entry` must exist in the store and hold valid bytecode. Calls follow
|
||||
/// `linked_records` edges; recursion depth is bounded.
|
||||
/// `linked_records` edges; recursion depth is bounded. All frames share a
|
||||
/// single data stack, so a caller passes arguments by leaving them on the
|
||||
/// stack and reads a callee's result (which the callee leaves there) after
|
||||
/// `RET` returns.
|
||||
pub fn run(&mut self, entry: Czyx) -> RunResult {
|
||||
self.output.clear();
|
||||
match self.exec_cell(entry, 0) {
|
||||
let mut stack: Vec<u8> = Vec::new();
|
||||
match self.exec_cell(entry, 0, &mut stack) {
|
||||
Ok(v) => RunResult::Halted { top: v },
|
||||
Err((cell, pc, fault)) => RunResult::Fault {
|
||||
cell,
|
||||
@@ -133,8 +137,15 @@ impl<B: CubeBackend> Vm<B> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Execute one cell. `depth` is the current call nesting.
|
||||
fn exec_cell(&mut self, label: Czyx, depth: usize) -> Result<Option<u8>, (Czyx, usize, Fault)> {
|
||||
/// Execute one cell. `depth` is the current call nesting. `stack` is the
|
||||
/// single shared data stack (callees run on it), so arguments and return
|
||||
/// values flow across cube edges via the stack.
|
||||
fn exec_cell(
|
||||
&mut self,
|
||||
label: Czyx,
|
||||
depth: usize,
|
||||
stack: &mut Vec<u8>,
|
||||
) -> Result<Option<u8>, (Czyx, usize, Fault)> {
|
||||
// Fetch and decode the callee record.
|
||||
let (header, body) =
|
||||
self.store
|
||||
@@ -142,7 +153,6 @@ impl<B: CubeBackend> Vm<B> {
|
||||
.ok_or((label, 0, Fault::MissingCallee(label)))?;
|
||||
let code = decode(&body).map_err(|_| (label, 0, Fault::BadCalleeBytecode(label)))?;
|
||||
|
||||
let mut stack: Vec<u8> = Vec::new();
|
||||
let mut locals: [u8; LOCALS] = [0; LOCALS];
|
||||
let mut pc: usize = 0;
|
||||
|
||||
@@ -216,7 +226,7 @@ impl<B: CubeBackend> Vm<B> {
|
||||
return Err((label, pc, Fault::CallStackOverflow));
|
||||
}
|
||||
let callee = header.linked_records[n as usize];
|
||||
let r = self.exec_cell(callee, depth + 1)?;
|
||||
let r = self.exec_cell(callee, depth + 1, stack)?;
|
||||
// Callee leaves its result on the shared stack; propagate
|
||||
// it up so the caller can read it after RET returns.
|
||||
if let Some(v) = r {
|
||||
@@ -227,7 +237,7 @@ impl<B: CubeBackend> Vm<B> {
|
||||
return Ok(stack.pop());
|
||||
}
|
||||
Op::Syscall(id) => {
|
||||
let v = self.syscall(id, &header, &mut stack);
|
||||
let v = self.syscall(id, &header, stack);
|
||||
if let Err(f) = v {
|
||||
return Err((label, pc, f));
|
||||
}
|
||||
@@ -410,6 +420,35 @@ mod tests {
|
||||
assert_eq!(vm.run(entry), RunResult::Halted { top: Some(10) });
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shared_stack_passes_args_across_cube_edges() {
|
||||
// Locks in the calling convention: the caller leaves the argument on
|
||||
// the SHARED data stack; the callee pops it via Store into a local,
|
||||
// computes, and returns a value the caller then uses. Without a shared
|
||||
// stack this would fault (callee sees an empty frame stack).
|
||||
let mut store = CubeStore::new(HashBackend::new());
|
||||
let entry = Czyx::new(1, 1, 1, 1);
|
||||
let double = Czyx::new(2, 0, 0, 1);
|
||||
// double: Store 0 (pop arg), Load 0, Const 2, Mul, Ret -> arg*2
|
||||
let (dh, db) = cell_code(
|
||||
double,
|
||||
&[],
|
||||
&[Op::Store(0), Op::Load(0), Op::Const(2), Op::Mul, Op::Ret],
|
||||
"fn",
|
||||
);
|
||||
store.put_record(double, &dh, &db);
|
||||
// entry: Const 21, CallLink 0 (double), Halt -> 42
|
||||
let (h, b) = cell_code(
|
||||
entry,
|
||||
&[double],
|
||||
&[Op::Const(21), Op::CallLink(0), Op::Halt],
|
||||
"fn",
|
||||
);
|
||||
store.put_record(entry, &h, &b);
|
||||
let mut vm = Vm::new(store);
|
||||
assert_eq!(vm.run(entry), RunResult::Halted { top: Some(42) });
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bad_link_index_faults() {
|
||||
let mut store = CubeStore::new(HashBackend::new());
|
||||
|
||||
Reference in New Issue
Block a user