Skip to content

Commit 095ce64

Browse files
anakrishCopilot
andcommitted
rvm: add debug-mode invariant assertions
Encode VM stack/context/register lifecycle invariants as debug_assert!s. Zero cost in release; surfaces violations during debug-mode tests and CI. Invariants covered: - reset_execution_state postcondition: all stacks empty, registers resized to base and Undefined, rule_cache reset, pc/executed counters zeroed, builtins_cache cleared, execution_state Ready. - Per-opcode invariant check (assert_vm_invariants) invoked at the top of run_stackless_loop and jump_to iterations: state is Ready/Running, registers non-empty, rule_cache sized to program, execution stack bounded. - resume() precondition: execution_state is Suspended. - execute_suspendable_entry precondition: clean state (callers reset immediately before). - ComprehensionEnd dispatch: comprehension_stack non-empty. - Rule finalize / call_rule completion: call_rule_stack non-empty and popped frame's rule_index matches. - IterationState::advance: Single iterator not advanced past consumption, Array index not at usize::MAX before saturating_add. All gated by #[cfg(debug_assertions)] (directly or via debug_assert!) so release builds are unaffected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ba7d29b commit 095ce64

5 files changed

Lines changed: 180 additions & 1 deletion

File tree

src/rvm/vm/comprehension.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,6 @@ impl RegoVM {
528528
Ok(false)
529529
}
530530
}
531-
532531
pub(super) fn handle_comprehension_condition_failure_suspendable(&mut self) -> Result<bool> {
533532
if let Some(mut frame) = self.execution_stack.pop() {
534533
let handled = if let &mut FrameKind::Comprehension {
@@ -606,6 +605,9 @@ impl RegoVM {
606605
}
607606

608607
fn execute_comprehension_end_run_to_completion(&mut self) -> Result<()> {
608+
// `ComprehensionEnd` is reached from a loaded program; an empty stack
609+
// here means malformed user-supplied bytecode, which must still surface
610+
// as a typed error rather than a panic — including in debug builds.
609611
self.comprehension_stack.pop().map_or_else(
610612
|| {
611613
Err(VmError::InvalidIteration {

src/rvm/vm/context.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,13 @@ impl IterationState {
5454
pub(super) const fn advance(&mut self) {
5555
match *self {
5656
Self::Array { ref mut index, .. } => {
57+
// Array iteration uses `usize` as the cursor; overflow would
58+
// wrap and silently restart the iteration. Bytecode that gets
59+
// here with a maxed-out index is malformed.
60+
debug_assert!(
61+
*index < usize::MAX,
62+
"IterationState::Array index overflow on advance"
63+
);
5764
*index = index.saturating_add(1);
5865
}
5966
Self::Object {
@@ -69,6 +76,12 @@ impl IterationState {
6976
Self::Single {
7077
ref mut consumed, ..
7178
} => {
79+
// `Single` yields exactly once; advancing a consumed Single
80+
// means the compiler emitted a redundant LoopNext.
81+
debug_assert!(
82+
!*consumed,
83+
"IterationState::Single advanced after consumption"
84+
);
7285
*consumed = true;
7386
}
7487
}

src/rvm/vm/execution.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,10 @@ impl RegoVM {
117117
let target = self.convert_pc(target, "jump target")?;
118118
self.pc = target;
119119
while self.pc < program.instructions.len() {
120+
// Per-instruction sanity check: every iteration of the dispatch
121+
// loop must re-enter with the VM in a Running/Ready state and the
122+
// working data structures coherent.
123+
self.assert_vm_invariants();
120124
self.memory_check()?;
121125
if self.executed_instructions >= self.max_instructions {
122126
return Err(VmError::InstructionLimitExceeded {
@@ -189,6 +193,9 @@ impl RegoVM {
189193
}
190194

191195
fn execute_suspendable_entry(&mut self, entry_point_pc: usize) -> Result<Value> {
196+
// Precondition: callers (execute_entry_point_by_{index,name}) reset the
197+
// VM before invoking this method, so the VM must be in a clean state.
198+
self.debug_assert_state_is_clean();
192199
self.execution_state = ExecutionState::Running;
193200
self.reset_execution_timer_state();
194201
match self.run_stackless_from(entry_point_pc) {
@@ -201,6 +208,12 @@ impl RegoVM {
201208
}
202209

203210
pub fn resume(&mut self, resume_value: Option<Value>) -> Result<Value> {
211+
// Precondition: resume only makes sense from a Suspended state.
212+
debug_assert!(
213+
matches!(self.execution_state, ExecutionState::Suspended { .. }),
214+
"resume precondition: execution_state must be Suspended, was {:?}",
215+
self.execution_state
216+
);
204217
let (reason, mut last_result) = match self.execution_state.clone() {
205218
ExecutionState::Suspended {
206219
reason,
@@ -289,6 +302,9 @@ impl RegoVM {
289302

290303
fn run_stackless_loop(&mut self, program: &Program, last_result: &mut Value) -> Result<()> {
291304
while !self.execution_stack.is_empty() {
305+
// Per-instruction sanity check: see `assert_vm_invariants` for the
306+
// exact contract. Compiled out in release.
307+
self.assert_vm_invariants();
292308
self.memory_check()?;
293309
self.frame_pc_overridden = false;
294310
let should_finalize_rule = self.execution_stack.last().is_some_and(|frame| {

src/rvm/vm/rules.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,14 @@ impl RegoVM {
277277
.call_rule_stack
278278
.pop()
279279
.ok_or(VmError::CallRuleStackUnderflow { pc: self.pc })?;
280+
// Stack discipline: the context we just popped must belong to the
281+
// rule we are finalizing. A mismatch would indicate a missing push
282+
// or an extra pop elsewhere in this rule's execution.
283+
debug_assert_eq!(
284+
rule_index, call_context.rule_index,
285+
"call_rule_stack pop mismatch: expected rule_index {} got {}",
286+
rule_index, call_context.rule_index,
287+
);
280288
self.pc = call_context.return_pc;
281289

282290
let result_from_rule = if !rule_failed_due_to_inconsistency {
@@ -831,6 +839,9 @@ impl RegoVM {
831839

832840
self.registers = parent_registers;
833841

842+
// Underflow here means malformed/poisoned program state; surface as a
843+
// typed error rather than a debug-only panic so the public load_program
844+
// contract holds the same in debug and release.
834845
if self.call_rule_stack.pop().is_none() {
835846
return Err(VmError::CallRuleStackUnderflow { pc: self.pc });
836847
}

src/rvm/vm/state.rs

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ use super::errors::{Result, VmError};
88
use super::execution_model::ExecutionState;
99
use super::machine::RegoVM;
1010

11+
/// Debug-only sanity ceiling for `execution_stack` depth. Far above any
12+
/// legitimate nesting observed in the test suite; trips early on a runaway
13+
/// frame leak. Not a production execution limit — use `enforce_limit` /
14+
/// `MachineLimits` for those.
15+
#[cfg(debug_assertions)]
16+
const DEBUG_MAX_EXECUTION_STACK_DEPTH: usize = 4096;
17+
1118
impl RegoVM {
1219
/// Reset all execution state and return objects to pools for reuse
1320
pub(super) fn reset_execution_state(&mut self) {
@@ -34,6 +41,136 @@ impl RegoVM {
3441

3542
// Builtin cache entries only live for a single execution
3643
self.builtins_cache.clear();
44+
45+
// Postcondition: every stack/cache that `reset_execution_state` touches
46+
// must be in its documented "clean" shape. This catches accidental
47+
// omissions in future edits to this function.
48+
self.debug_assert_state_is_clean();
49+
}
50+
51+
/// Debug-only postcondition for `reset_execution_state`.
52+
///
53+
/// Asserts the invariants every caller of `reset_execution_state` relies on
54+
/// before starting a fresh execution. The body is fully gated by
55+
/// `#[cfg(debug_assertions)]` so this is a zero-cost no-op in release.
56+
#[inline]
57+
pub(super) fn debug_assert_state_is_clean(&self) {
58+
#[cfg(debug_assertions)]
59+
{
60+
// --- Stacks: every per-execution stack must be drained. ---
61+
debug_assert!(
62+
self.execution_stack.is_empty(),
63+
"reset_execution_state postcondition: execution_stack must be empty"
64+
);
65+
debug_assert!(
66+
self.loop_stack.is_empty(),
67+
"reset_execution_state postcondition: loop_stack must be empty"
68+
);
69+
debug_assert!(
70+
self.comprehension_stack.is_empty(),
71+
"reset_execution_state postcondition: comprehension_stack must be empty"
72+
);
73+
debug_assert!(
74+
self.call_rule_stack.is_empty(),
75+
"reset_execution_state postcondition: call_rule_stack must be empty"
76+
);
77+
debug_assert!(
78+
self.register_stack.is_empty(),
79+
"reset_execution_state postcondition: register_stack must be empty"
80+
);
81+
82+
// --- Caches: cleared so a new program/input cannot read stale entries. ---
83+
debug_assert!(
84+
self.builtins_cache.is_empty(),
85+
"reset_execution_state postcondition: builtins_cache must be empty"
86+
);
87+
88+
// --- Registers: window resized to the program's base count and zeroed. ---
89+
debug_assert_eq!(
90+
self.registers.len(),
91+
self.base_register_count,
92+
"reset_execution_state postcondition: registers must be sized to base_register_count"
93+
);
94+
debug_assert!(
95+
self.registers.iter().all(|v| matches!(v, Value::Undefined)),
96+
"reset_execution_state postcondition: all registers must be Undefined"
97+
);
98+
99+
// --- Rule cache: sized to the current program and marked uncomputed. ---
100+
debug_assert_eq!(
101+
self.rule_cache.len(),
102+
self.program.rule_infos.len(),
103+
"reset_execution_state postcondition: rule_cache size must match program rule_infos"
104+
);
105+
debug_assert!(
106+
self.rule_cache.iter().all(|entry| !entry.0),
107+
"reset_execution_state postcondition: rule_cache entries must be uncomputed"
108+
);
109+
110+
// --- Counters and execution-state machine: zeroed and back to Ready. ---
111+
debug_assert_eq!(
112+
self.pc, 0,
113+
"reset_execution_state postcondition: pc must be 0"
114+
);
115+
debug_assert_eq!(
116+
self.executed_instructions, 0,
117+
"reset_execution_state postcondition: executed_instructions must be 0"
118+
);
119+
debug_assert!(
120+
matches!(self.execution_state, ExecutionState::Ready),
121+
"reset_execution_state postcondition: execution_state must be Ready"
122+
);
123+
}
124+
}
125+
126+
/// Per-opcode VM invariants checked from the inner dispatch loop.
127+
///
128+
/// These hold every time control re-enters the dispatch loop with another
129+
/// instruction to execute. Fully `#[cfg(debug_assertions)]`-gated so the
130+
/// method body compiles out in release.
131+
#[inline]
132+
pub(super) fn assert_vm_invariants(&self) {
133+
#[cfg(debug_assertions)]
134+
{
135+
// The dispatch loop only runs while execution is live. Once the VM
136+
// has transitioned to a terminal state (Suspended/Completed/Error)
137+
// the loop must have exited. Note `Ready` is also valid here because
138+
// some entry points (e.g. `execute_entry_point_by_index` in
139+
// RunToCompletion mode) drive `jump_to` without flipping the state.
140+
debug_assert!(
141+
matches!(
142+
self.execution_state,
143+
ExecutionState::Ready | ExecutionState::Running
144+
),
145+
"vm invariant: execution_state must be Ready or Running inside the dispatch loop, was {:?}",
146+
self.execution_state
147+
);
148+
149+
// Registers must always be non-empty when the dispatch loop is
150+
// running — every instruction operates on registers.
151+
debug_assert!(
152+
!self.registers.is_empty(),
153+
"vm invariant: register window must be non-empty inside the dispatch loop"
154+
);
155+
156+
// Rule cache is sized once at reset and must not change shape mid-run.
157+
debug_assert_eq!(
158+
self.rule_cache.len(),
159+
self.program.rule_infos.len(),
160+
"vm invariant: rule_cache size must equal program.rule_infos size"
161+
);
162+
163+
// Bound the execution stack. There is no hard MAX_FRAMES today;
164+
// this is a debug-only sanity net well above any legitimate test or
165+
// policy nesting depth in the suite, and catches runaway
166+
// frame-leaks early. Not a production limit.
167+
debug_assert!(
168+
self.execution_stack.len() <= DEBUG_MAX_EXECUTION_STACK_DEPTH,
169+
"vm invariant: execution_stack depth ({}) exceeds sanity bound ({})",
170+
self.execution_stack.len(),
171+
DEBUG_MAX_EXECUTION_STACK_DEPTH,
172+
);
173+
}
37174
}
38175

39176
/// Return all active objects to their respective pools for reuse

0 commit comments

Comments
 (0)