diff --git a/src/rvm/instructions/types.rs b/src/rvm/instructions/types.rs index c19c243..04629f1 100644 --- a/src/rvm/instructions/types.rs +++ b/src/rvm/instructions/types.rs @@ -15,7 +15,7 @@ pub enum LiteralOrRegister { /// Loop execution modes for different Rego iteration constructs #[repr(C)] -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, Serialize, Deserialize)] pub enum LoopMode { /// Any quantification: some x in arr, x := arr[_], etc. /// Succeeds if ANY iteration succeeds, exits early on first success diff --git a/src/rvm/program/rule_tree.rs b/src/rvm/program/rule_tree.rs index c46bffb..e2d626b 100644 --- a/src/rvm/program/rule_tree.rs +++ b/src/rvm/program/rule_tree.rs @@ -70,10 +70,13 @@ impl Program { match *rule_value { Value::Number(_) => { if data_value != &Value::Undefined { - return Err(crate::rvm::vm::VmError::RuleDataConflict(format!( - "Conflict: rule defines path '{}' but data also provides this path", - current_path.join("."), - ))); + return Err(crate::rvm::vm::VmError::RuleDataConflict { + message: format!( + "Conflict: rule defines path '{}' but data also provides this path", + current_path.join("."), + ), + pc: 0, + }); } } Value::Object(_) => { @@ -84,17 +87,23 @@ impl Program { current_path, )?; } else if data_value != &Value::Undefined { - return Err(crate::rvm::vm::VmError::RuleDataConflict(format!( - "Conflict: rule defines subpaths under '{}' but data provides a non-object value at this path", - current_path.join("."), - ))); + return Err(crate::rvm::vm::VmError::RuleDataConflict { + message: format!( + "Conflict: rule defines subpaths under '{}' but data provides a non-object value at this path", + current_path.join("."), + ), + pc: 0, + }); } } _ => { - return Err(crate::rvm::vm::VmError::RuleDataConflict(format!( - "Invalid rule tree structure at path '{}'", - current_path.join("."), - ))); + return Err(crate::rvm::vm::VmError::RuleDataConflict { + message: format!( + "Invalid rule tree structure at path '{}'", + current_path.join("."), + ), + pc: 0, + }); } } @@ -103,9 +112,10 @@ impl Program { } } _ => { - return Err(crate::rvm::vm::VmError::RuleDataConflict( - "Rule tree root must be an object".to_string(), - )); + return Err(crate::rvm::vm::VmError::RuleDataConflict { + message: "Rule tree root must be an object".to_string(), + pc: 0, + }); } } diff --git a/src/rvm/tests/vm.rs b/src/rvm/tests/vm.rs index 208091e..25b4cd3 100644 --- a/src/rvm/tests/vm.rs +++ b/src/rvm/tests/vm.rs @@ -702,6 +702,7 @@ mod tests { VmError::HostAwaitResponseMissing { dest, identifier: identifier.clone(), + pc: 0, } ) })?; @@ -712,6 +713,7 @@ mod tests { VmError::HostAwaitResponseMissing { dest, identifier: identifier.clone(), + pc: 0, } ) })?; @@ -722,6 +724,7 @@ mod tests { VmError::HostAwaitResponseMissing { dest, identifier: identifier.clone(), + pc: 0, } ) })?; diff --git a/src/rvm/vm/arithmetic.rs b/src/rvm/vm/arithmetic.rs index 7dbc773..557fb2b 100644 --- a/src/rvm/vm/arithmetic.rs +++ b/src/rvm/vm/arithmetic.rs @@ -1,11 +1,6 @@ -#![allow( - clippy::unused_self, - clippy::missing_const_for_fn, - clippy::unseparated_literal_suffix, - clippy::pattern_type_mismatch -)] // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +#![allow(clippy::pattern_type_mismatch)] use alloc::collections::BTreeSet; @@ -23,6 +18,7 @@ impl RegoVM { _ => Err(VmError::InvalidAddition { left: a.clone(), right: b.clone(), + pc: self.pc, }), } } @@ -38,6 +34,7 @@ impl RegoVM { _ => Err(VmError::InvalidSubtraction { left: a.clone(), right: b.clone(), + pc: self.pc, }), } } @@ -49,6 +46,7 @@ impl RegoVM { _ => Err(VmError::InvalidMultiplication { left: a.clone(), right: b.clone(), + pc: self.pc, }), } } @@ -57,11 +55,12 @@ impl RegoVM { pub(super) fn div_values(&self, a: &Value, b: &Value) -> Result { match (a, b) { (Value::Number(x), Value::Number(y)) => { - if *y == Number::from(0u64) { + if *y == Number::from(0_u64) { if self.strict_builtin_errors { return Err(VmError::InvalidDivision { left: a.clone(), right: b.clone(), + pc: self.pc, }); } return Ok(Value::Undefined); @@ -72,6 +71,7 @@ impl RegoVM { _ => Err(VmError::InvalidDivision { left: a.clone(), right: b.clone(), + pc: self.pc, }), } } @@ -80,18 +80,23 @@ impl RegoVM { pub(super) fn mod_values(&self, a: &Value, b: &Value) -> Result { match (a, b) { (Value::Number(x), Value::Number(y)) => { - if *y == Number::from(0u64) { + if *y == Number::from(0_u64) { if self.strict_builtin_errors { return Err(VmError::InvalidModulo { left: a.clone(), right: b.clone(), + pc: self.pc, }); } return Ok(Value::Undefined); } if !x.is_integer() || !y.is_integer() { - return Err(VmError::ModuloOnFloat); + return Err(VmError::ModuloOnFloat { + left: a.clone(), + right: b.clone(), + pc: self.pc, + }); } Ok(Value::from(x.clone().modulo(y)?)) @@ -99,11 +104,12 @@ impl RegoVM { _ => Err(VmError::InvalidModulo { left: a.clone(), right: b.clone(), + pc: self.pc, }), } } - pub(super) fn to_bool(&self, value: &Value) -> Option { + pub(super) const fn to_bool(&self, value: &Value) -> Option { match value { Value::Bool(b) => Some(*b), Value::Null if !self.strict_builtin_errors => Some(true), diff --git a/src/rvm/vm/comprehension.rs b/src/rvm/vm/comprehension.rs index 2db4ea0..298b52b 100644 --- a/src/rvm/vm/comprehension.rs +++ b/src/rvm/vm/comprehension.rs @@ -1,13 +1,5 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::indexing_slicing, - clippy::arithmetic_side_effects, - clippy::option_if_let_else, - clippy::useless_let_if_seq, - clippy::as_conversions, - clippy::pattern_type_mismatch -)] use crate::rvm::instructions::{ComprehensionBeginParams, ComprehensionMode}; use crate::value::Value; @@ -44,99 +36,11 @@ impl RegoVM { ComprehensionMode::Array => Value::new_array(), ComprehensionMode::Object => Value::Object(Rc::new(BTreeMap::new())), }; - self.registers[params.result_reg as usize] = initial_result.clone(); + self.set_register(params.result_reg, initial_result.clone())?; let auto_iterate = params.collection_reg != params.result_reg; let iteration_state = if auto_iterate { - let source_value = self.registers[params.collection_reg as usize].clone(); - match source_value { - Value::Array(items) => { - if items.is_empty() { - None - } else { - Some(IterationState::Array { items, index: 0 }) - } - } - Value::Object(obj) => { - if obj.is_empty() { - None - } else { - Some(IterationState::Object { - obj, - current_key: None, - first_iteration: true, - }) - } - } - Value::Set(set) => { - if set.is_empty() { - None - } else { - Some(IterationState::Set { - items: set, - current_item: None, - first_iteration: true, - }) - } - } - Value::Undefined => None, - Value::Null => None, - _ => None, - } - } else { - None - }; - - let mut has_iteration = false; - if let Some(state) = iteration_state.as_ref() { - has_iteration = self.setup_next_iteration(state, params.key_reg, params.value_reg)?; - } - - let resume_pc = if auto_iterate { - params.comprehension_end as usize - } else { - params.comprehension_end.saturating_sub(1) as usize - }; - - let mut comprehension_context = ComprehensionContext { - mode: params.mode.clone(), - result_reg: params.result_reg, - key_reg: params.key_reg, - value_reg: params.value_reg, - body_start: params.body_start, - comprehension_end: params.comprehension_end, - iteration_state, - resume_pc, - }; - - if auto_iterate { - if has_iteration { - self.pc = params.body_start as usize - 1; - } else { - comprehension_context.iteration_state = None; - self.pc = params.comprehension_end as usize - 1; - } - } - - self.comprehension_stack.push(comprehension_context); - - Ok(()) - } - - fn execute_comprehension_begin_suspendable( - &mut self, - params: &ComprehensionBeginParams, - ) -> Result<()> { - let initial_result = match params.mode { - ComprehensionMode::Set => Value::new_set(), - ComprehensionMode::Array => Value::new_array(), - ComprehensionMode::Object => Value::Object(Rc::new(BTreeMap::new())), - }; - self.registers[params.result_reg as usize] = initial_result.clone(); - - let auto_iterate = params.collection_reg != params.result_reg; - let iteration_state = if auto_iterate { - let source_value = self.registers[params.collection_reg as usize].clone(); + let source_value = self.get_register(params.collection_reg)?.clone(); match source_value { Value::Array(items) => { if items.is_empty() { @@ -182,9 +86,98 @@ impl RegoVM { }; let resume_pc = if auto_iterate { - params.comprehension_end as usize + usize::from(params.comprehension_end) } else { - params.comprehension_end.saturating_sub(1) as usize + usize::from(params.comprehension_end.saturating_sub(1)) + }; + + let mut comprehension_context = ComprehensionContext { + mode: params.mode.clone(), + result_reg: params.result_reg, + key_reg: params.key_reg, + value_reg: params.value_reg, + body_start: params.body_start, + comprehension_end: params.comprehension_end, + iteration_state, + resume_pc, + }; + + if auto_iterate { + if has_iteration { + self.pc = usize::from(params.body_start).saturating_sub(1); + } else { + comprehension_context.iteration_state = None; + self.pc = usize::from(params.comprehension_end).saturating_sub(1); + } + } + + self.comprehension_stack.push(comprehension_context); + + Ok(()) + } + + fn execute_comprehension_begin_suspendable( + &mut self, + params: &ComprehensionBeginParams, + ) -> Result<()> { + let initial_result = match params.mode { + ComprehensionMode::Set => Value::new_set(), + ComprehensionMode::Array => Value::new_array(), + ComprehensionMode::Object => Value::Object(Rc::new(BTreeMap::new())), + }; + self.set_register(params.result_reg, initial_result.clone())?; + + let auto_iterate = params.collection_reg != params.result_reg; + let iteration_state = if auto_iterate { + let source_value = self.get_register(params.collection_reg)?.clone(); + match source_value { + Value::Array(items) => { + if items.is_empty() { + None + } else { + Some(IterationState::Array { items, index: 0 }) + } + } + Value::Object(obj) => { + if obj.is_empty() { + None + } else { + Some(IterationState::Object { + obj, + current_key: None, + first_iteration: true, + }) + } + } + Value::Set(set) => { + if set.is_empty() { + None + } else { + Some(IterationState::Set { + items: set, + current_item: None, + first_iteration: true, + }) + } + } + Value::Undefined => None, + Value::Null => None, + _ => None, + } + } else { + None + }; + + let has_iteration = if let Some(state) = iteration_state.as_ref() { + self.setup_next_iteration(state, params.key_reg, params.value_reg)? + } else { + false + }; + + let resume_pc = if auto_iterate { + usize::from(params.comprehension_end) + } else { + usize::from(params.comprehension_end.saturating_sub(1)) }; let mut comprehension_context = ComprehensionContext { @@ -200,13 +193,13 @@ impl RegoVM { let next_pc = if auto_iterate { if has_iteration { - params.body_start as usize + usize::from(params.body_start) } else { comprehension_context.iteration_state = None; - params.comprehension_end as usize + usize::from(params.comprehension_end) } } else { - self.pc + 1 + self.pc.saturating_add(1) }; let return_pc = comprehension_context.resume_pc; @@ -248,20 +241,21 @@ impl RegoVM { } else { return Err(VmError::InvalidIteration { value: Value::String(Arc::from("No active comprehension")), + pc: self.pc, }); }; - let value_to_add = self.registers[value_reg as usize].clone(); + let value_to_add = self.get_register(value_reg)?.clone(); let key_value = if let Some(key_reg) = key_reg { - Some(self.registers[key_reg as usize].clone()) + Some(self.get_register(key_reg)?.clone()) } else if matches!(comprehension_context.mode, ComprehensionMode::Object) { - Some(self.registers[comprehension_context.key_reg as usize].clone()) + Some(self.get_register(comprehension_context.key_reg)?.clone()) } else { None }; - let result_reg = comprehension_context.result_reg as usize; - let current_result = self.registers[result_reg].clone(); + let result_reg = comprehension_context.result_reg; + let current_result = self.get_register(result_reg)?.clone(); let mode = comprehension_context.mode.clone(); let updated_result = match (mode, current_result) { @@ -284,31 +278,41 @@ impl RegoVM { self.comprehension_stack.push(comprehension_context); return Err(VmError::InvalidIteration { value: Value::String(Arc::from("Object comprehension requires key")), + pc: self.pc, }); } } (_mode, other) => { self.comprehension_stack.push(comprehension_context); - return Err(VmError::InvalidIteration { value: other }); + return Err(VmError::InvalidIteration { + value: other, + pc: self.pc, + }); } }; - self.registers[result_reg] = updated_result; + self.set_register(result_reg, updated_result)?; if let Some(iter_state) = comprehension_context.iteration_state.as_mut() { - match iter_state { - IterationState::Object { current_key, .. } => { + match *iter_state { + IterationState::Object { + ref mut current_key, + .. + } => { let tracked_key = if comprehension_context.key_reg != comprehension_context.value_reg { - self.registers[comprehension_context.key_reg as usize].clone() + self.get_register(comprehension_context.key_reg)?.clone() } else { - self.registers[comprehension_context.value_reg as usize].clone() + self.get_register(comprehension_context.value_reg)?.clone() }; *current_key = Some(tracked_key); } - IterationState::Set { current_item, .. } => { + IterationState::Set { + ref mut current_item, + .. + } => { *current_item = - Some(self.registers[comprehension_context.value_reg as usize].clone()); + Some(self.get_register(comprehension_context.value_reg)?.clone()); } IterationState::Array { .. } => {} } @@ -321,10 +325,10 @@ impl RegoVM { )?; if has_next { - self.pc = comprehension_context.body_start as usize - 1; + self.pc = usize::from(comprehension_context.body_start).saturating_sub(1); } else { comprehension_context.iteration_state = None; - self.pc = comprehension_context.comprehension_end as usize - 1; + self.pc = usize::from(comprehension_context.comprehension_end).saturating_sub(1); } } @@ -347,110 +351,162 @@ impl RegoVM { }) .ok_or(VmError::InvalidIteration { value: Value::String(Arc::from("No active comprehension")), + pc: self.pc, })?; - let (iteration_state_snapshot, key_reg_idx, value_reg_idx, body_start, comprehension_end) = { - let frame = self.execution_stack.get_mut(comprehension_index).ok_or( - VmError::InvalidIteration { - value: Value::String(Arc::from("No active comprehension")), - }, - )?; - - match &mut frame.kind { - FrameKind::Comprehension { context, .. } => { - let value_to_add = self.registers[value_reg as usize].clone(); - let key_value = if let Some(key_reg) = key_reg { - Some(self.registers[key_reg as usize].clone()) - } else if matches!(context.mode, ComprehensionMode::Object) { - Some(self.registers[context.key_reg as usize].clone()) - } else { - None - }; - - let result_reg_idx = context.result_reg as usize; - let current_result = self.registers[result_reg_idx].clone(); - let mode = context.mode.clone(); - - let updated_result = match (mode, current_result) { - (ComprehensionMode::Set, Value::Set(set)) => { - let mut new_set = set.as_ref().clone(); - new_set.insert(value_to_add); - Value::Set(crate::Rc::new(new_set)) - } - (ComprehensionMode::Array, Value::Array(arr)) => { - let mut new_arr = arr.as_ref().to_vec(); - new_arr.push(value_to_add); - Value::Array(crate::Rc::new(new_arr)) - } - (ComprehensionMode::Object, Value::Object(obj)) => { - if let Some(key) = key_value { - let mut new_obj = obj.as_ref().clone(); - new_obj.insert(key, value_to_add); - Value::Object(crate::Rc::new(new_obj)) - } else { - return Err(VmError::InvalidIteration { - value: Value::String(Arc::from( - "Object comprehension requires key", - )), - }); - } - } - (_mode, other) => { - return Err(VmError::InvalidIteration { value: other }); - } - }; - - self.registers[result_reg_idx] = updated_result; - - if let Some(iter_state) = context.iteration_state.as_mut() { - match iter_state { - IterationState::Object { current_key, .. } => { - let tracked_key = if context.key_reg != context.value_reg { - self.registers[context.key_reg as usize].clone() - } else { - self.registers[context.value_reg as usize].clone() - }; - *current_key = Some(tracked_key); - } - IterationState::Set { current_item, .. } => { - *current_item = - Some(self.registers[context.value_reg as usize].clone()); - } - IterationState::Array { .. } => {} - } - - iter_state.advance(); - } - - ( - context.iteration_state.clone(), - context.key_reg, - context.value_reg, - context.body_start, - context.comprehension_end, - ) - } - _ => { - return Err(VmError::InvalidIteration { + let ( + value_to_add, + key_value, + current_result, + mode, + result_reg_idx, + key_reg_idx, + value_reg_idx, + iteration_key, + iteration_value, + ) = { + let frame = + self.execution_stack + .get(comprehension_index) + .ok_or(VmError::InvalidIteration { value: Value::String(Arc::from("No active comprehension")), + pc: self.pc, + })?; + + if let FrameKind::Comprehension { ref context, .. } = frame.kind { + let value_to_add = self.get_register(value_reg)?.clone(); + let key_value = if let Some(key_reg) = key_reg { + Some(self.get_register(key_reg)?.clone()) + } else if matches!(context.mode, ComprehensionMode::Object) { + Some(self.get_register(context.key_reg)?.clone()) + } else { + None + }; + + let result_reg_idx = context.result_reg; + let current_result = self.get_register(result_reg_idx)?.clone(); + let mode = context.mode.clone(); + let iteration_key = self.get_register(context.key_reg)?.clone(); + let iteration_value = self.get_register(context.value_reg)?.clone(); + + ( + value_to_add, + key_value, + current_result, + mode, + result_reg_idx, + context.key_reg, + context.value_reg, + iteration_key, + iteration_value, + ) + } else { + return Err(VmError::InvalidIteration { + value: Value::String(Arc::from("No active comprehension")), + pc: self.pc, + }); + } + }; + + let updated_result = match (mode, current_result) { + (ComprehensionMode::Set, Value::Set(set)) => { + let mut new_set = set.as_ref().clone(); + new_set.insert(value_to_add); + Value::Set(crate::Rc::new(new_set)) + } + (ComprehensionMode::Array, Value::Array(arr)) => { + let mut new_arr = arr.as_ref().to_vec(); + new_arr.push(value_to_add); + Value::Array(crate::Rc::new(new_arr)) + } + (ComprehensionMode::Object, Value::Object(obj)) => { + if let Some(key) = key_value { + let mut new_obj = obj.as_ref().clone(); + new_obj.insert(key, value_to_add); + Value::Object(crate::Rc::new(new_obj)) + } else { + return Err(VmError::InvalidIteration { + value: Value::String(Arc::from("Object comprehension requires key")), + pc: self.pc, }); } } + (_mode, other) => { + return Err(VmError::InvalidIteration { + value: other, + pc: self.pc, + }); + } }; + let (iteration_state_snapshot, body_start, comprehension_end) = { + let frame = self.execution_stack.get_mut(comprehension_index).ok_or( + VmError::InvalidIteration { + value: Value::String(Arc::from("No active comprehension")), + pc: self.pc, + }, + )?; + + if let &mut FrameKind::Comprehension { + ref mut context, .. + } = &mut frame.kind + { + if let Some(iter_state) = context.iteration_state.as_mut() { + match *iter_state { + IterationState::Object { + ref mut current_key, + .. + } => { + let tracked_key = if context.key_reg != context.value_reg { + iteration_key.clone() + } else { + iteration_value.clone() + }; + *current_key = Some(tracked_key); + } + IterationState::Set { + ref mut current_item, + .. + } => { + *current_item = Some(iteration_value.clone()); + } + IterationState::Array { .. } => {} + } + + iter_state.advance(); + } + + ( + context.iteration_state.clone(), + context.body_start, + context.comprehension_end, + ) + } else { + return Err(VmError::InvalidIteration { + value: Value::String(Arc::from("No active comprehension")), + pc: self.pc, + }); + } + }; + + self.set_register(result_reg_idx, updated_result)?; + if let Some(state) = iteration_state_snapshot.as_ref() { let has_next = self.setup_next_iteration(state, key_reg_idx, value_reg_idx)?; if has_next { if let Some(frame) = self.execution_stack.get_mut(comprehension_index) { - frame.pc = body_start as usize; + frame.pc = usize::from(body_start); self.frame_pc_overridden = true; } } else if let Some(frame) = self.execution_stack.get_mut(comprehension_index) { - if let FrameKind::Comprehension { context, .. } = &mut frame.kind { + if let &mut FrameKind::Comprehension { + ref mut context, .. + } = &mut frame.kind + { context.iteration_state = None; } - frame.pc = comprehension_end as usize; + frame.pc = usize::from(comprehension_end); self.frame_pc_overridden = true; } } @@ -479,7 +535,10 @@ impl RegoVM { pub(super) fn handle_comprehension_condition_failure_suspendable(&mut self) -> Result { if let Some(mut frame) = self.execution_stack.pop() { - let handled = if let FrameKind::Comprehension { context, .. } = &mut frame.kind { + let handled = if let &mut FrameKind::Comprehension { + ref mut context, .. + } = &mut frame.kind + { self.advance_comprehension_after_failure(context)?; true } else { @@ -503,18 +562,18 @@ impl RegoVM { iter_state, context.key_reg, context.value_reg, - ); + )?; iter_state.advance(); let has_next = self.setup_next_iteration(iter_state, context.key_reg, context.value_reg)?; if has_next { - self.pc = context.body_start.saturating_sub(1) as usize; + self.pc = usize::from(context.body_start.saturating_sub(1)); } else { context.iteration_state = None; - self.pc = context.comprehension_end.saturating_sub(1) as usize; + self.pc = usize::from(context.comprehension_end.saturating_sub(1)); } } else { - self.pc = context.comprehension_end.saturating_sub(1) as usize; + self.pc = usize::from(context.comprehension_end.saturating_sub(1)); } Ok(()) @@ -525,31 +584,41 @@ impl RegoVM { iter_state: &mut IterationState, key_reg: u8, value_reg: u8, - ) { - match iter_state { - IterationState::Object { current_key, .. } => { + ) -> Result<()> { + match *iter_state { + IterationState::Object { + ref mut current_key, + .. + } => { let tracked_key = if key_reg != value_reg { - self.registers[key_reg as usize].clone() + self.get_register(key_reg)?.clone() } else { - self.registers[value_reg as usize].clone() + self.get_register(value_reg)?.clone() }; *current_key = Some(tracked_key); } - IterationState::Set { current_item, .. } => { - *current_item = Some(self.registers[value_reg as usize].clone()); + IterationState::Set { + ref mut current_item, + .. + } => { + *current_item = Some(self.get_register(value_reg)?.clone()); } IterationState::Array { .. } => {} } + + Ok(()) } fn execute_comprehension_end_run_to_completion(&mut self) -> Result<()> { - if let Some(_context) = self.comprehension_stack.pop() { - Ok(()) - } else { - Err(VmError::InvalidIteration { - value: Value::String(Arc::from("No active comprehension context")), - }) - } + self.comprehension_stack.pop().map_or_else( + || { + Err(VmError::InvalidIteration { + value: Value::String(Arc::from("No active comprehension context")), + pc: self.pc, + }) + }, + |_context| Ok(()), + ) } fn execute_comprehension_end_suspendable(&mut self) -> Result<()> { @@ -565,6 +634,7 @@ impl RegoVM { } return Err(VmError::InvalidIteration { value: Value::String(Arc::from("No active comprehension context")), + pc: self.pc, }); } }; @@ -620,6 +690,7 @@ impl RegoVM { } return Err(VmError::InvalidIteration { value: Value::String(Arc::from(message.into_boxed_str())), + pc: self.pc, }); } } diff --git a/src/rvm/vm/context.rs b/src/rvm/vm/context.rs index 302413d..b290de4 100644 --- a/src/rvm/vm/context.rs +++ b/src/rvm/vm/context.rs @@ -1,10 +1,5 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::arithmetic_side_effects, - clippy::missing_const_for_fn, - clippy::pattern_type_mismatch -)] use crate::rvm::instructions::{ComprehensionMode, LoopMode}; use crate::value::Value; @@ -49,18 +44,18 @@ pub enum IterationState { } impl IterationState { - pub(super) fn advance(&mut self) { - match self { - IterationState::Array { index, .. } => { - *index += 1; + pub(super) const fn advance(&mut self) { + match *self { + Self::Array { ref mut index, .. } => { + *index = index.saturating_add(1); } - IterationState::Object { - first_iteration, .. - } => { - *first_iteration = false; + Self::Object { + ref mut first_iteration, + .. } - IterationState::Set { - first_iteration, .. + | Self::Set { + ref mut first_iteration, + .. } => { *first_iteration = false; } diff --git a/src/rvm/vm/dispatch.rs b/src/rvm/vm/dispatch.rs index b4916d1..234299f 100644 --- a/src/rvm/vm/dispatch.rs +++ b/src/rvm/vm/dispatch.rs @@ -1,12 +1,5 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::indexing_slicing, - clippy::arithmetic_side_effects, - clippy::used_underscore_binding, - clippy::as_conversions, - clippy::pattern_type_mismatch -)] use crate::rvm::instructions::{Instruction, LiteralOrRegister}; use crate::rvm::program::Program; @@ -44,41 +37,43 @@ impl RegoVM { use Instruction::*; match instruction { Load { dest, literal_idx } => { - if let Some(value) = program.literals.get(literal_idx as usize) { - self.registers[dest as usize] = value.clone(); + if let Some(value) = program.literals.get(usize::from(literal_idx)) { + self.set_register(dest, value.clone())?; Ok(InstructionOutcome::Continue) } else { Err(VmError::LiteralIndexOutOfBounds { - index: literal_idx as usize, + index: literal_idx, + pc: self.pc, }) } } LoadTrue { dest } => { - self.registers[dest as usize] = Value::Bool(true); + self.set_register(dest, Value::Bool(true))?; Ok(InstructionOutcome::Continue) } LoadFalse { dest } => { - self.registers[dest as usize] = Value::Bool(false); + self.set_register(dest, Value::Bool(false))?; Ok(InstructionOutcome::Continue) } LoadNull { dest } => { - self.registers[dest as usize] = Value::Null; + self.set_register(dest, Value::Null)?; Ok(InstructionOutcome::Continue) } LoadBool { dest, value } => { - self.registers[dest as usize] = Value::Bool(value); + self.set_register(dest, Value::Bool(value))?; Ok(InstructionOutcome::Continue) } LoadData { dest } => { - self.registers[dest as usize] = self.data.clone(); + self.set_register(dest, self.data.clone())?; Ok(InstructionOutcome::Continue) } LoadInput { dest } => { - self.registers[dest as usize] = self.input.clone(); + self.set_register(dest, self.input.clone())?; Ok(InstructionOutcome::Continue) } Move { dest, src } => { - self.registers[dest as usize] = self.registers[src as usize].clone(); + let value = self.get_register(src)?.clone(); + self.set_register(dest, value)?; Ok(InstructionOutcome::Continue) } other => self.execute_arithmetic_instruction(program, other), @@ -87,246 +82,267 @@ impl RegoVM { fn execute_arithmetic_instruction( &mut self, - _program: &Program, + program: &Program, instruction: Instruction, ) -> Result { use Instruction::*; match instruction { Add { dest, left, right } => { - let a = &self.registers[left as usize]; - let b = &self.registers[right as usize]; + let a = self.get_register(left)?; + let b = self.get_register(right)?; if a == &Value::Undefined || b == &Value::Undefined { - self.registers[dest as usize] = Value::Undefined; + self.set_register(dest, Value::Undefined)?; return Ok(InstructionOutcome::Continue); } let result = self.add_values(a, b)?; - self.registers[dest as usize] = result; + self.set_register(dest, result)?; Ok(InstructionOutcome::Continue) } Sub { dest, left, right } => { - let a = &self.registers[left as usize]; - let b = &self.registers[right as usize]; + let a = self.get_register(left)?; + let b = self.get_register(right)?; if a == &Value::Undefined || b == &Value::Undefined { - self.registers[dest as usize] = Value::Undefined; + self.set_register(dest, Value::Undefined)?; return Ok(InstructionOutcome::Continue); } let result = self.sub_values(a, b)?; - self.registers[dest as usize] = result; + self.set_register(dest, result)?; Ok(InstructionOutcome::Continue) } Mul { dest, left, right } => { - let a = &self.registers[left as usize]; - let b = &self.registers[right as usize]; + let a = self.get_register(left)?; + let b = self.get_register(right)?; if a == &Value::Undefined || b == &Value::Undefined { - self.registers[dest as usize] = Value::Undefined; + self.set_register(dest, Value::Undefined)?; return Ok(InstructionOutcome::Continue); } let result = self.mul_values(a, b)?; - self.registers[dest as usize] = result; + self.set_register(dest, result)?; Ok(InstructionOutcome::Continue) } Div { dest, left, right } => { - let a = &self.registers[left as usize]; - let b = &self.registers[right as usize]; + let a = self.get_register(left)?; + let b = self.get_register(right)?; if a == &Value::Undefined || b == &Value::Undefined { - self.registers[dest as usize] = Value::Undefined; + self.set_register(dest, Value::Undefined)?; return Ok(InstructionOutcome::Continue); } let result = self.div_values(a, b)?; - self.registers[dest as usize] = result; + self.set_register(dest, result)?; Ok(InstructionOutcome::Continue) } Mod { dest, left, right } => { - let a = &self.registers[left as usize]; - let b = &self.registers[right as usize]; + let a = self.get_register(left)?; + let b = self.get_register(right)?; if a == &Value::Undefined || b == &Value::Undefined { - self.registers[dest as usize] = Value::Undefined; + self.set_register(dest, Value::Undefined)?; return Ok(InstructionOutcome::Continue); } let result = self.mod_values(a, b)?; - self.registers[dest as usize] = result; + self.set_register(dest, result)?; Ok(InstructionOutcome::Continue) } - other => self.execute_comparison_instruction(_program, other), + other => self.execute_comparison_instruction(program, other), } } fn execute_comparison_instruction( &mut self, - _program: &Program, + program: &Program, instruction: Instruction, ) -> Result { use Instruction::*; match instruction { Eq { dest, left, right } => { - let a = &self.registers[left as usize]; - let b = &self.registers[right as usize]; + let a = self.get_register(left)?; + let b = self.get_register(right)?; if a == &Value::Undefined || b == &Value::Undefined { - self.registers[dest as usize] = Value::Undefined; + self.set_register(dest, Value::Undefined)?; return Ok(InstructionOutcome::Continue); } - self.registers[dest as usize] = Value::Bool(a == b); + self.set_register(dest, Value::Bool(a == b))?; Ok(InstructionOutcome::Continue) } Ne { dest, left, right } => { - let a = &self.registers[left as usize]; - let b = &self.registers[right as usize]; + let a = self.get_register(left)?; + let b = self.get_register(right)?; if a == &Value::Undefined || b == &Value::Undefined { - self.registers[dest as usize] = Value::Undefined; + self.set_register(dest, Value::Undefined)?; return Ok(InstructionOutcome::Continue); } - self.registers[dest as usize] = Value::Bool(a != b); + self.set_register(dest, Value::Bool(a != b))?; Ok(InstructionOutcome::Continue) } Lt { dest, left, right } => { - let a = &self.registers[left as usize]; - let b = &self.registers[right as usize]; + let a = self.get_register(left)?; + let b = self.get_register(right)?; if a == &Value::Undefined || b == &Value::Undefined { - self.registers[dest as usize] = Value::Undefined; + self.set_register(dest, Value::Undefined)?; return Ok(InstructionOutcome::Continue); } if self.strict_builtin_errors && mem::discriminant(a) != mem::discriminant(b) { - return Err(VmError::ArithmeticError(alloc::format!( - "#undefined: cannot compare values of different types (left={a:?}, right={b:?})" - ))); + return Err(VmError::ArithmeticError { + message: alloc::format!( + "#undefined: cannot compare values of different types (left={a:?}, right={b:?})" + ), + pc: self.pc, + }); } - self.registers[dest as usize] = Value::Bool(a < b); + self.set_register(dest, Value::Bool(a < b))?; Ok(InstructionOutcome::Continue) } Le { dest, left, right } => { - let a = &self.registers[left as usize]; - let b = &self.registers[right as usize]; + let a = self.get_register(left)?; + let b = self.get_register(right)?; if a == &Value::Undefined || b == &Value::Undefined { - self.registers[dest as usize] = Value::Undefined; + self.set_register(dest, Value::Undefined)?; return Ok(InstructionOutcome::Continue); } if self.strict_builtin_errors && mem::discriminant(a) != mem::discriminant(b) { - return Err(VmError::ArithmeticError(alloc::format!( - "#undefined: cannot compare values of different types (left={a:?}, right={b:?})" - ))); + return Err(VmError::ArithmeticError { + message: alloc::format!( + "#undefined: cannot compare values of different types (left={a:?}, right={b:?})" + ), + pc: self.pc, + }); } - self.registers[dest as usize] = Value::Bool(a <= b); + self.set_register(dest, Value::Bool(a <= b))?; Ok(InstructionOutcome::Continue) } Gt { dest, left, right } => { - let a = &self.registers[left as usize]; - let b = &self.registers[right as usize]; + let a = self.get_register(left)?; + let b = self.get_register(right)?; if a == &Value::Undefined || b == &Value::Undefined { - self.registers[dest as usize] = Value::Undefined; + self.set_register(dest, Value::Undefined)?; return Ok(InstructionOutcome::Continue); } if self.strict_builtin_errors && mem::discriminant(a) != mem::discriminant(b) { - return Err(VmError::ArithmeticError(alloc::format!( - "#undefined: cannot compare values of different types (left={a:?}, right={b:?})" - ))); + return Err(VmError::ArithmeticError { + message: alloc::format!( + "#undefined: cannot compare values of different types (left={a:?}, right={b:?})" + ), + pc: self.pc, + }); } - self.registers[dest as usize] = Value::Bool(a > b); + self.set_register(dest, Value::Bool(a > b))?; Ok(InstructionOutcome::Continue) } Ge { dest, left, right } => { - let a = &self.registers[left as usize]; - let b = &self.registers[right as usize]; + let a = self.get_register(left)?; + let b = self.get_register(right)?; if a == &Value::Undefined || b == &Value::Undefined { - self.registers[dest as usize] = Value::Undefined; + self.set_register(dest, Value::Undefined)?; return Ok(InstructionOutcome::Continue); } if self.strict_builtin_errors && mem::discriminant(a) != mem::discriminant(b) { - return Err(VmError::ArithmeticError(alloc::format!( - "#undefined: cannot compare values of different types (left={a:?}, right={b:?})" - ))); + return Err(VmError::ArithmeticError { + message: alloc::format!( + "#undefined: cannot compare values of different types (left={a:?}, right={b:?})" + ), + pc: self.pc, + }); } - self.registers[dest as usize] = Value::Bool(a >= b); + self.set_register(dest, Value::Bool(a >= b))?; Ok(InstructionOutcome::Continue) } And { dest, left, right } => { - let left_value = &self.registers[left as usize]; - let right_value = &self.registers[right as usize]; + let left_value = self.get_register(left)?; + let right_value = self.get_register(right)?; if left_value == &Value::Undefined || right_value == &Value::Undefined { - self.registers[dest as usize] = Value::Undefined; + self.set_register(dest, Value::Undefined)?; return Ok(InstructionOutcome::Continue); } match (self.to_bool(left_value), self.to_bool(right_value)) { (Some(a), Some(b)) => { - self.registers[dest as usize] = Value::Bool(a && b); + self.set_register(dest, Value::Bool(a && b))?; Ok(InstructionOutcome::Continue) } - _ => Err(VmError::ArithmeticError(alloc::format!( - "#undefined: logical AND expects booleans (left={left_value:?}, right={right_value:?})" - ))), + _ => Err(VmError::ArithmeticError { + message: alloc::format!( + "#undefined: logical AND expects booleans (left={left_value:?}, right={right_value:?})" + ), + pc: self.pc, + }), } } Or { dest, left, right } => { - let left_value = &self.registers[left as usize]; - let right_value = &self.registers[right as usize]; + let left_value = self.get_register(left)?; + let right_value = self.get_register(right)?; if left_value == &Value::Undefined || right_value == &Value::Undefined { - self.registers[dest as usize] = Value::Undefined; + self.set_register(dest, Value::Undefined)?; return Ok(InstructionOutcome::Continue); } match (self.to_bool(left_value), self.to_bool(right_value)) { (Some(a), Some(b)) => { - self.registers[dest as usize] = Value::Bool(a || b); + self.set_register(dest, Value::Bool(a || b))?; Ok(InstructionOutcome::Continue) } - _ => Err(VmError::ArithmeticError(alloc::format!( - "#undefined: logical OR expects booleans (left={left_value:?}, right={right_value:?})" - ))), + _ => Err(VmError::ArithmeticError { + message: alloc::format!( + "#undefined: logical OR expects booleans (left={left_value:?}, right={right_value:?})" + ), + pc: self.pc, + }), } } Not { dest, operand } => { - let operand_value = &self.registers[operand as usize]; + let operand_value = self.get_register(operand)?; if operand_value == &Value::Undefined { // In Rego, `not expr` succeeds when `expr` has no results. // When the operand evaluates to undefined we should treat it as // a successful negation instead of propagating undefined. - self.registers[dest as usize] = Value::Bool(true); + self.set_register(dest, Value::Bool(true))?; return Ok(InstructionOutcome::Continue); } if let Some(value) = self.to_bool(operand_value) { - self.registers[dest as usize] = Value::Bool(!value); + self.set_register(dest, Value::Bool(!value))?; Ok(InstructionOutcome::Continue) } else { - Err(VmError::ArithmeticError(alloc::format!( - "#undefined: logical NOT expects a boolean (operand={operand_value:?})" - ))) + Err(VmError::ArithmeticError { + message: alloc::format!( + "#undefined: logical NOT expects a boolean (operand={operand_value:?})" + ), + pc: self.pc, + }) } } AssertCondition { condition } => { - let value = &self.registers[condition as usize]; + let value = self.get_register(condition)?; - let condition_result = match value { - Value::Bool(b) => *b, + let condition_result = match *value { + Value::Bool(b) => b, Value::Undefined => false, _ => true, }; @@ -335,19 +351,19 @@ impl RegoVM { Ok(InstructionOutcome::Continue) } AssertNotUndefined { register } => { - let value = &self.registers[register as usize]; + let value = self.get_register(register)?; let is_undefined = matches!(value, Value::Undefined); self.handle_condition(!is_undefined)?; Ok(InstructionOutcome::Continue) } - other => self.execute_call_instruction(_program, other), + other => self.execute_call_instruction(program, other), } } fn execute_call_instruction( &mut self, - _program: &Program, + program: &Program, instruction: Instruction, ) -> Result { use Instruction::*; @@ -357,19 +373,16 @@ impl RegoVM { Ok(InstructionOutcome::Continue) } HostAwait { dest, arg, id } => { - let argument = self.registers[arg as usize].clone(); + let argument = self.get_register(arg)?.clone(); let identifier = self .registers - .get(id as usize) + .get(usize::from(id)) .cloned() .unwrap_or(Value::Undefined); match self.execution_mode { ExecutionMode::RunToCompletion => { let response = self.next_host_await_response(&identifier, dest)?; - if self.registers.len() <= dest as usize { - self.registers.resize(dest as usize + 1, Value::Undefined); - } - self.registers[dest as usize] = response; + self.set_register(dest, response)?; Ok(InstructionOutcome::Continue) } ExecutionMode::Suspendable => Ok(InstructionOutcome::Suspend { @@ -386,7 +399,7 @@ impl RegoVM { Ok(InstructionOutcome::Continue) } Return { value } => { - let result = self.registers[value as usize].clone(); + let result = self.get_register(value)?.clone(); Ok(InstructionOutcome::Return(result)) } CallRule { dest, rule_index } => { @@ -405,7 +418,7 @@ impl RegoVM { self.execute_rule_return()?; Ok(InstructionOutcome::Break) } - other => self.execute_collection_instruction(_program, other), + other => self.execute_collection_instruction(program, other), } } @@ -417,17 +430,22 @@ impl RegoVM { use Instruction::*; match instruction { ObjectSet { obj, key, value } => { - let key_value = self.registers[key as usize].clone(); - let value_value = self.registers[value as usize].clone(); + let key_value = self.get_register(key)?.clone(); + let value_value = self.get_register(value)?.clone(); - let mut obj_value = mem::replace(&mut self.registers[obj as usize], Value::Null); + let mut obj_value = self.get_register(obj)?.clone(); if let Ok(obj_mut) = obj_value.as_object_mut() { obj_mut.insert(key_value, value_value); - self.registers[obj as usize] = obj_value; + self.set_register(obj, obj_value)?; } else { - self.registers[obj as usize] = obj_value; - return Err(VmError::RegisterNotObject { register: obj }); + let offending = obj_value.clone(); + self.set_register(obj, obj_value)?; + return Err(VmError::RegisterNotObject { + register: obj, + value: offending, + pc: self.pc, + }); } Ok(InstructionOutcome::Continue) } @@ -437,12 +455,14 @@ impl RegoVM { .get_object_create_params(params_index) .ok_or(VmError::InvalidObjectCreateParams { index: params_index, + pc: self.pc, + available: program.instruction_data.object_create_params.len(), })?; let mut any_undefined = false; for &(_, value_reg) in params.literal_key_field_pairs() { - if matches!(self.registers[value_reg as usize], Value::Undefined) { + if matches!(self.get_register(value_reg)?, Value::Undefined) { any_undefined = true; break; } @@ -450,8 +470,8 @@ impl RegoVM { if !any_undefined { for &(key_reg, value_reg) in params.field_pairs() { - if matches!(self.registers[key_reg as usize], Value::Undefined) - || matches!(self.registers[value_reg as usize], Value::Undefined) + if matches!(self.get_register(key_reg)?, Value::Undefined) + || matches!(self.get_register(value_reg)?, Value::Undefined) { any_undefined = true; break; @@ -460,13 +480,15 @@ impl RegoVM { } if any_undefined { - self.registers[params.dest as usize] = Value::Undefined; + self.set_register(params.dest, Value::Undefined)?; } else { let mut obj_value = program .literals - .get(params.template_literal_idx as usize) + .get(usize::from(params.template_literal_idx)) .ok_or(VmError::InvalidTemplateLiteralIndex { index: params.template_literal_idx, + pc: self.pc, + available: program.literals.len(), })? .clone(); @@ -477,10 +499,10 @@ impl RegoVM { for (key, value) in obj_mut.iter_mut() { if let Some(&(literal_idx, value_reg)) = current_literal_update { if let Some(literal_key) = - program.literals.get(literal_idx as usize) + program.literals.get(usize::from(literal_idx)) { if key == literal_key { - *value = self.registers[value_reg as usize].clone(); + *value = self.get_register(value_reg)?.clone(); current_literal_update = literal_updates.next(); } } @@ -490,23 +512,27 @@ impl RegoVM { } while let Some(&(literal_idx, value_reg)) = current_literal_update { - if let Some(key_value) = program.literals.get(literal_idx as usize) { - let value_value = self.registers[value_reg as usize].clone(); + if let Some(key_value) = program.literals.get(usize::from(literal_idx)) + { + let value_value = self.get_register(value_reg)?.clone(); obj_mut.insert(key_value.clone(), value_value); } current_literal_update = literal_updates.next(); } for &(key_reg, value_reg) in params.field_pairs() { - let key_value = self.registers[key_reg as usize].clone(); - let value_value = self.registers[value_reg as usize].clone(); + let key_value = self.get_register(key_reg)?.clone(); + let value_value = self.get_register(value_reg)?.clone(); obj_mut.insert(key_value, value_value); } } else { - return Err(VmError::ObjectCreateInvalidTemplate); + return Err(VmError::ObjectCreateInvalidTemplate { + template: obj_value, + pc: self.pc, + }); } - self.registers[params.dest as usize] = obj_value; + self.set_register(params.dest, obj_value)?; } Ok(InstructionOutcome::Continue) } @@ -515,10 +541,10 @@ impl RegoVM { container, key, } => { - let key_value = &self.registers[key as usize]; - let container_value = &self.registers[container as usize]; + let key_value = self.get_register(key)?; + let container_value = self.get_register(container)?; let result = container_value[key_value].clone(); - self.registers[dest as usize] = result; + self.set_register(dest, result)?; Ok(InstructionOutcome::Continue) } IndexLiteral { @@ -526,34 +552,40 @@ impl RegoVM { container, literal_idx, } => { - let container_value = &self.registers[container as usize]; + let container_value = self.get_register(container)?; - if let Some(key_value) = program.literals.get(literal_idx as usize) { + if let Some(key_value) = program.literals.get(usize::from(literal_idx)) { let result = container_value[key_value].clone(); - self.registers[dest as usize] = result; + self.set_register(dest, result)?; Ok(InstructionOutcome::Continue) } else { Err(VmError::LiteralIndexOutOfBounds { - index: literal_idx as usize, + index: literal_idx, + pc: self.pc, }) } } ArrayNew { dest } => { let empty_array = Value::Array(crate::Rc::new(Vec::new())); - self.registers[dest as usize] = empty_array; + self.set_register(dest, empty_array)?; Ok(InstructionOutcome::Continue) } ArrayPush { arr, value } => { - let value_to_push = self.registers[value as usize].clone(); + let value_to_push = self.get_register(value)?.clone(); - let mut arr_value = mem::replace(&mut self.registers[arr as usize], Value::Null); + let mut arr_value = self.get_register(arr)?.clone(); if let Ok(arr_mut) = arr_value.as_array_mut() { arr_mut.push(value_to_push); - self.registers[arr as usize] = arr_value; + self.set_register(arr, arr_value)?; } else { - self.registers[arr as usize] = arr_value; - return Err(VmError::RegisterNotArray { register: arr }); + let offending = arr_value.clone(); + self.set_register(arr, arr_value)?; + return Err(VmError::RegisterNotArray { + register: arr, + value: offending, + pc: self.pc, + }); } Ok(InstructionOutcome::Continue) } @@ -564,47 +596,54 @@ impl RegoVM { { let mut any_undefined = false; for ® in params.element_registers() { - if matches!(self.registers[reg as usize], Value::Undefined) { + if matches!(self.get_register(reg)?, Value::Undefined) { any_undefined = true; break; } } if any_undefined { - self.registers[params.dest as usize] = Value::Undefined; + self.set_register(params.dest, Value::Undefined)?; } else { let elements: Vec = params .element_registers() .iter() - .map(|®| self.registers[reg as usize].clone()) - .collect(); + .map(|®| self.get_register(reg).cloned()) + .collect::>>()?; let array_value = Value::Array(crate::Rc::new(elements)); - self.registers[params.dest as usize] = array_value; + self.set_register(params.dest, array_value)?; } Ok(InstructionOutcome::Continue) } else { Err(VmError::InvalidArrayCreateParams { index: params_index, + pc: self.pc, + available: program.instruction_data.array_create_params.len(), }) } } SetNew { dest } => { let empty_set = Value::Set(crate::Rc::new(BTreeSet::new())); - self.registers[dest as usize] = empty_set; + self.set_register(dest, empty_set)?; Ok(InstructionOutcome::Continue) } SetAdd { set, value } => { - let value_to_add = self.registers[value as usize].clone(); + let value_to_add = self.get_register(value)?.clone(); - let mut set_value = mem::replace(&mut self.registers[set as usize], Value::Null); + let mut set_value = self.get_register(set)?.clone(); if let Ok(set_mut) = set_value.as_set_mut() { set_mut.insert(value_to_add); - self.registers[set as usize] = set_value; + self.set_register(set, set_value)?; } else { - self.registers[set as usize] = set_value; - return Err(VmError::RegisterNotSet { register: set }); + let offending = set_value.clone(); + self.set_register(set, set_value)?; + return Err(VmError::RegisterNotSet { + register: set, + value: offending, + pc: self.pc, + }); } Ok(InstructionOutcome::Continue) } @@ -612,27 +651,29 @@ impl RegoVM { if let Some(params) = program.instruction_data.get_set_create_params(params_index) { let mut any_undefined = false; for ® in params.element_registers() { - if matches!(self.registers[reg as usize], Value::Undefined) { + if matches!(self.get_register(reg)?, Value::Undefined) { any_undefined = true; break; } } if any_undefined { - self.registers[params.dest as usize] = Value::Undefined; + self.set_register(params.dest, Value::Undefined)?; } else { let mut set = BTreeSet::new(); for ® in params.element_registers() { - set.insert(self.registers[reg as usize].clone()); + set.insert(self.get_register(reg)?.clone()); } let set_value = Value::Set(crate::Rc::new(set)); - self.registers[params.dest as usize] = set_value; + self.set_register(params.dest, set_value)?; } Ok(InstructionOutcome::Continue) } else { Err(VmError::InvalidSetCreateParams { index: params_index, + pc: self.pc, + available: program.instruction_data.set_create_params.len(), }) } } @@ -641,33 +682,37 @@ impl RegoVM { collection, value, } => { - let value_to_check = &self.registers[value as usize]; - let collection_value = &self.registers[collection as usize]; + let value_to_check = self.get_register(value)?; + let collection_value = self.get_register(collection)?; - let result = match collection_value { - Value::Set(set_elements) => Value::Bool(set_elements.contains(value_to_check)), - Value::Array(array_items) => Value::Bool(array_items.contains(value_to_check)), - Value::Object(object_fields) => Value::Bool( + let result = match *collection_value { + Value::Set(ref set_elements) => { + Value::Bool(set_elements.contains(value_to_check)) + } + Value::Array(ref array_items) => { + Value::Bool(array_items.contains(value_to_check)) + } + Value::Object(ref object_fields) => Value::Bool( object_fields.contains_key(value_to_check) || object_fields.values().any(|v| v == value_to_check), ), _ => Value::Bool(false), }; - self.registers[dest as usize] = result; + self.set_register(dest, result)?; Ok(InstructionOutcome::Continue) } Count { dest, collection } => { - let collection_value = &self.registers[collection as usize]; + let collection_value = self.get_register(collection)?; - let result = match collection_value { - Value::Array(array_items) => Value::from(array_items.len()), - Value::Object(object_fields) => Value::from(object_fields.len()), - Value::Set(set_elements) => Value::from(set_elements.len()), + let result = match *collection_value { + Value::Array(ref array_items) => Value::from(array_items.len()), + Value::Object(ref object_fields) => Value::from(object_fields.len()), + Value::Set(ref set_elements) => Value::from(set_elements.len()), _ => Value::Undefined, }; - self.registers[dest as usize] = result; + self.set_register(dest, result)?; Ok(InstructionOutcome::Continue) } other => self.execute_loop_instruction(program, other), @@ -682,8 +727,17 @@ impl RegoVM { use Instruction::*; match instruction { LoopStart { params_index } => { - let loop_params = &self.program.instruction_data.loop_params[params_index as usize]; - let mode = loop_params.mode.clone(); + let loop_params_len = program.instruction_data.loop_params.len(); + + let loop_params = program + .instruction_data + .get_loop_params(params_index) + .ok_or(VmError::InvalidLoopParams { + index: params_index, + pc: self.pc, + available: loop_params_len, + })?; + let mode = loop_params.mode; let params = LoopParams { collection: loop_params.collection, key_reg: loop_params.key_reg, @@ -703,7 +757,7 @@ impl RegoVM { Ok(InstructionOutcome::Continue) } Halt {} => { - let result = self.registers[0].clone(); + let result = self.get_register(0)?.clone(); Ok(InstructionOutcome::Return(result)) } other => self.execute_virtual_instruction(program, other), @@ -723,20 +777,23 @@ impl RegoVM { .get_chained_index_params(params_index) .ok_or(VmError::InvalidChainedIndexParams { index: params_index, + pc: self.pc, + available: program.instruction_data.chained_index_params.len(), })?; - let mut current_value = self.registers[params.root as usize].clone(); + let mut current_value = self.get_register(params.root)?.clone(); for component in ¶ms.path_components { - let key_value = match component { + let key_value = match *component { LiteralOrRegister::Literal(idx) => program .literals - .get(*idx as usize) + .get(usize::from(idx)) .ok_or(VmError::LiteralIndexOutOfBounds { - index: *idx as usize, + index: idx, + pc: self.pc, })? .clone(), - LiteralOrRegister::Register(reg) => self.registers[*reg as usize].clone(), + LiteralOrRegister::Register(reg) => self.get_register(reg)?.clone(), }; current_value = current_value[&key_value].clone(); @@ -746,7 +803,7 @@ impl RegoVM { } } - self.registers[params.dest as usize] = current_value; + self.set_register(params.dest, current_value)?; Ok(InstructionOutcome::Continue) } VirtualDataDocumentLookup { params_index } => { @@ -759,6 +816,8 @@ impl RegoVM { .get_comprehension_begin_params(params_index) .ok_or(VmError::InvalidComprehensionBeginParams { index: params_index, + pc: self.pc, + available: program.instruction_data.comprehension_begin_params.len(), })? .clone(); self.execute_comprehension_begin(¶ms)?; @@ -772,10 +831,10 @@ impl RegoVM { self.execute_comprehension_end()?; Ok(InstructionOutcome::Continue) } - unexpected => Err(VmError::Internal(alloc::format!( - "Unhandled instruction variant: {:?}", - unexpected - ))), + unexpected => Err(VmError::UnhandledInstruction { + instruction: alloc::format!("{:?}", unexpected), + pc: self.pc, + }), } } } diff --git a/src/rvm/vm/errors.rs b/src/rvm/vm/errors.rs index 4c2d3bd..3048acd 100644 --- a/src/rvm/vm/errors.rs +++ b/src/rvm/vm/errors.rs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +use super::execution_model::SuspendReason; use crate::value::Value; use alloc::string::String; use alloc::vec::Vec; @@ -9,112 +10,287 @@ use thiserror::Error; /// VM execution errors #[derive(Error, Debug, Clone, PartialEq)] pub enum VmError { - #[error("Execution stopped: exceeded maximum instruction limit of {limit}")] - InstructionLimitExceeded { limit: usize }, + #[error("Execution stopped: exceeded maximum instruction limit of {limit} after {executed} instructions (pc={pc})")] + InstructionLimitExceeded { + limit: usize, + executed: usize, + pc: usize, + }, - #[error("Literal index {index} out of bounds")] - LiteralIndexOutOfBounds { index: usize }, + #[error("Literal index {index} out of bounds (pc={pc})")] + LiteralIndexOutOfBounds { index: u16, pc: usize }, - #[error("Register {register} does not contain an object")] - RegisterNotObject { register: u8 }, + #[error("Register {register} does not contain an object (value={value:?}, pc={pc})")] + RegisterNotObject { + register: u8, + value: Value, + pc: usize, + }, - #[error("ObjectCreate: template is not an object")] - ObjectCreateInvalidTemplate, + #[error("ObjectCreate: template is not an object (pc={pc}, template={template:?})")] + ObjectCreateInvalidTemplate { template: Value, pc: usize }, - #[error("Register {register} does not contain an array")] - RegisterNotArray { register: u8 }, + #[error("Register {register} does not contain an array (value={value:?}, pc={pc})")] + RegisterNotArray { + register: u8, + value: Value, + pc: usize, + }, - #[error("Register {register} does not contain a set")] - RegisterNotSet { register: u8 }, + #[error("Register {register} does not contain a set (value={value:?}, pc={pc})")] + RegisterNotSet { + register: u8, + value: Value, + pc: usize, + }, - #[error("Rule index {index} out of bounds")] - RuleIndexOutOfBounds { index: u16 }, + #[error("Register index {index} out of bounds (pc={pc}, register_count={register_count})")] + RegisterIndexOutOfBounds { + index: u8, + pc: usize, + register_count: usize, + }, - #[error("Rule index {index} has no info")] - RuleInfoMissing { index: u16 }, + #[error("Rule index {index} out of bounds (pc={pc}, available={available})")] + RuleIndexOutOfBounds { + index: u16, + pc: usize, + available: usize, + }, - #[error("Invalid object create params index: {index}")] - InvalidObjectCreateParams { index: u16 }, + #[error("Rule index {index} has no info (pc={pc}, available={available})")] + RuleInfoMissing { + index: u16, + pc: usize, + available: usize, + }, - #[error("Invalid template literal index: {index}")] - InvalidTemplateLiteralIndex { index: u16 }, + #[error("Invalid object create params index: {index} (pc={pc}, available={available})")] + InvalidObjectCreateParams { + index: u16, + pc: usize, + available: usize, + }, - #[error("Invalid chained index params index: {index}")] - InvalidChainedIndexParams { index: u16 }, + #[error("Invalid template literal index: {index} (pc={pc}, available={available})")] + InvalidTemplateLiteralIndex { + index: u16, + pc: usize, + available: usize, + }, - #[error("Invalid array create params index: {index}")] - InvalidArrayCreateParams { index: u16 }, + #[error("Invalid chained index params index: {index} (pc={pc}, available={available})")] + InvalidChainedIndexParams { + index: u16, + pc: usize, + available: usize, + }, - #[error("Invalid set create params index: {index}")] - InvalidSetCreateParams { index: u16 }, + #[error("Invalid array create params index: {index} (pc={pc}, available={available})")] + InvalidArrayCreateParams { + index: u16, + pc: usize, + available: usize, + }, - #[error("Invalid virtual data document lookup params index: {index}")] - InvalidVirtualDataDocumentLookupParams { index: u16 }, + #[error("Invalid set create params index: {index} (pc={pc}, available={available})")] + InvalidSetCreateParams { + index: u16, + pc: usize, + available: usize, + }, - #[error("Invalid comprehension start params index: {index}")] - InvalidComprehensionBeginParams { index: u16 }, + #[error("Invalid virtual data document lookup params index: {index} (pc={pc}, available={available})")] + InvalidVirtualDataDocumentLookupParams { + index: u16, + pc: usize, + available: usize, + }, - #[error("Invalid rule index: {rule_index:?}")] - InvalidRuleIndex { rule_index: Value }, + #[error("Invalid comprehension start params index: {index} (pc={pc}, available={available})")] + InvalidComprehensionBeginParams { + index: u16, + pc: usize, + available: usize, + }, - #[error("Invalid rule tree entry: {value:?}")] - InvalidRuleTreeEntry { value: Value }, + #[error("Invalid loop params index: {index} (pc={pc}, available={available})")] + InvalidLoopParams { + index: u16, + pc: usize, + available: usize, + }, - #[error("Builtin function expects exactly {expected} arguments, got {actual}")] - BuiltinArgumentMismatch { expected: u16, actual: usize }, + #[error("Invalid rule index: {rule_index:?} (pc={pc})")] + InvalidRuleIndex { rule_index: Value, pc: usize }, - #[error("Builtin function not resolved: {name}")] - BuiltinNotResolved { name: String }, + #[error("Invalid rule tree entry: {value:?} (pc={pc})")] + InvalidRuleTreeEntry { value: Value, pc: usize }, - #[error("Cannot add {left:?} and {right:?}")] - InvalidAddition { left: Value, right: Value }, + #[error("Builtin function expects exactly {expected} arguments, got {actual} (pc={pc})")] + BuiltinArgumentMismatch { + expected: u16, + actual: usize, + pc: usize, + }, - #[error("Cannot subtract {left:?} and {right:?}")] - InvalidSubtraction { left: Value, right: Value }, + #[error("Builtin function not resolved: {name} (pc={pc})")] + BuiltinNotResolved { name: String, pc: usize }, - #[error("Cannot multiply {left:?} and {right:?}")] - InvalidMultiplication { left: Value, right: Value }, + #[error("Cannot add {left:?} and {right:?} (pc={pc})")] + InvalidAddition { + left: Value, + right: Value, + pc: usize, + }, - #[error("Cannot divide {left:?} and {right:?}")] - InvalidDivision { left: Value, right: Value }, + #[error("Cannot subtract {left:?} and {right:?} (pc={pc})")] + InvalidSubtraction { + left: Value, + right: Value, + pc: usize, + }, - #[error("modulo on floating-point number")] - ModuloOnFloat, + #[error("Cannot multiply {left:?} and {right:?} (pc={pc})")] + InvalidMultiplication { + left: Value, + right: Value, + pc: usize, + }, - #[error("Cannot modulo {left:?} and {right:?}")] - InvalidModulo { left: Value, right: Value }, + #[error("Cannot divide {left:?} and {right:?} (pc={pc})")] + InvalidDivision { + left: Value, + right: Value, + pc: usize, + }, - #[error("Cannot iterate over {value:?}")] - InvalidIteration { value: Value }, + #[error("modulo on floating-point number (left={left:?}, right={right:?}, pc={pc})")] + ModuloOnFloat { + left: Value, + right: Value, + pc: usize, + }, - #[error("HostAwait executed but no response provided for destination register {dest} (id: {identifier:?})")] - HostAwaitResponseMissing { dest: u8, identifier: Value }, + #[error("Cannot modulo {left:?} and {right:?} (pc={pc})")] + InvalidModulo { + left: Value, + right: Value, + pc: usize, + }, - #[error("Assertion failed")] - AssertionFailed, + #[error("Cannot iterate over {value:?} (pc={pc})")] + InvalidIteration { value: Value, pc: usize }, - #[error("Rule-data conflict: {0}")] - RuleDataConflict(String), + #[error("HostAwait executed but no response provided for destination register {dest} (id: {identifier:?}, pc={pc})")] + HostAwaitResponseMissing { + dest: u8, + identifier: Value, + pc: usize, + }, - #[error("Arithmetic error: {0}")] - ArithmeticError(String), + #[error("Assertion failed (pc={pc})")] + AssertionFailed { pc: usize }, - #[error("Entry point index {index} out of bounds (max: {max_index})")] - InvalidEntryPointIndex { index: usize, max_index: usize }, + #[error("Rule-data conflict: {message} (pc={pc})")] + RuleDataConflict { message: String, pc: usize }, - #[error("Entry point '{name}' not found. Available entry points: {available:?}")] + #[error("Arithmetic error: {message} (pc={pc})")] + ArithmeticError { message: String, pc: usize }, + + #[error("Entry point index {index} out of bounds (max: {max_index}, pc={pc})")] + InvalidEntryPointIndex { + index: usize, + max_index: usize, + pc: usize, + }, + + #[error("Entry point '{name}' not found (pc={pc}). Available entry points: {available:?}")] EntryPointNotFound { name: String, available: Vec, + pc: usize, }, - #[error("Internal VM error: {0}")] - Internal(String), + #[error("Entry point PC {pc} >= instruction count {instruction_count} for entry point '{entry_point}'")] + EntryPointPcOutOfBounds { + pc: usize, + instruction_count: usize, + entry_point: String, + }, + + #[error("Register count {register_count} below base count {base_count} (pc={pc})")] + RegisterCountBelowBase { + register_count: usize, + base_count: usize, + pc: usize, + }, + + #[error("Program counter {pc} out of bounds for instruction count {instruction_count}")] + ProgramCounterOutOfBounds { pc: usize, instruction_count: usize }, + + #[error("Rule cache size {cache_size} != rule info count {rule_info_count} (pc={pc})")] + RuleCacheSizeMismatch { + cache_size: usize, + rule_info_count: usize, + pc: usize, + }, + + #[error("Suspend reason {reason:?} is not supported in run-to-completion execution (pc={pc})")] + UnsupportedSuspendInRunToCompletion { reason: SuspendReason, pc: usize }, + + #[error("Cannot resume VM when execution state is {state} (pc={pc})")] + InvalidResumeState { state: String, pc: usize }, + + #[error("HostAwait suspension requires a resume value for reason {reason:?} (pc={pc})")] + MissingResumeValue { reason: SuspendReason, pc: usize }, + + #[error("Unexpected resume value supplied for reason {reason:?} (pc={pc})")] + UnexpectedResumeValue { reason: SuspendReason, pc: usize }, + + #[error("Missing execution frame: {context} (pc={pc})")] + MissingExecutionFrame { context: &'static str, pc: usize }, + + #[error("Unhandled instruction variant: {instruction} (pc={pc})")] + UnhandledInstruction { instruction: String, pc: usize }, + + #[error("Invalid function call params index: {index} (pc={pc}, available={available})")] + InvalidFunctionCallParamsIndex { + index: u16, + pc: usize, + available: usize, + }, + + #[error("Invalid builtin call params index: {index} (pc={pc}, available={available})")] + InvalidBuiltinCallParamsIndex { + index: u16, + pc: usize, + available: usize, + }, + + #[error("Invalid builtin info index: {index} (pc={pc}, available={available})")] + InvalidBuiltinInfoIndex { + index: u16, + pc: usize, + available: usize, + }, + + #[error("Rule frame has no initial PC (pc={pc})")] + RuleFrameMissingInitialPc { pc: usize }, + + #[error("Call rule stack underflow during rule finalization (pc={pc})")] + CallRuleStackUnderflow { pc: usize }, + + #[error("Internal VM error: {message} (pc={pc})")] + Internal { message: String, pc: usize }, } impl From for VmError { fn from(err: anyhow::Error) -> Self { - VmError::ArithmeticError(alloc::format!("{}", err)) + VmError::ArithmeticError { + message: alloc::format!("{}", err), + pc: 0, + } } } diff --git a/src/rvm/vm/execution.rs b/src/rvm/vm/execution.rs index c796e0a..588ee5c 100644 --- a/src/rvm/vm/execution.rs +++ b/src/rvm/vm/execution.rs @@ -1,19 +1,11 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::indexing_slicing, - clippy::arithmetic_side_effects, - clippy::expect_used, - clippy::option_if_let_else, - clippy::as_conversions, - clippy::needless_continue, - clippy::pattern_type_mismatch -)] use crate::rvm::instructions::Instruction; use crate::rvm::program::Program; use crate::value::Value; use alloc::string::String; use alloc::vec::Vec; +use core::convert::TryFrom as _; use super::dispatch::InstructionOutcome; use super::errors::{Result, VmError}; @@ -43,47 +35,47 @@ impl RegoVM { return Err(VmError::InvalidEntryPointIndex { index, max_index: entry_points.len().saturating_sub(1), + pc: self.pc, }); } - let (_entry_point_name, entry_point_pc) = &entry_points[index]; + let &(ref entry_point_name, entry_point_pc) = + entry_points + .get(index) + .ok_or(VmError::InvalidEntryPointIndex { + index, + max_index: entry_points.len().saturating_sub(1), + pc: self.pc, + })?; - if *entry_point_pc >= self.program.instructions.len() { - return Err(VmError::Internal(alloc::format!( - "Entry point PC {} >= instruction count {} for index {} | {}", - entry_point_pc, - self.program.instructions.len(), - index, - self.get_debug_state() - ))); + if entry_point_pc >= self.program.instructions.len() { + return Err(VmError::EntryPointPcOutOfBounds { + pc: entry_point_pc, + instruction_count: self.program.instructions.len(), + entry_point: entry_point_name.clone(), + }); } match self.execution_mode { ExecutionMode::RunToCompletion => { self.reset_execution_state(); - if let Err(e) = self.validate_vm_state() { - return Err(VmError::Internal(alloc::format!( - "VM state validation failed before entry point execution: {} | {}", - e, - self.get_debug_state() - ))); - } + self.validate_vm_state()?; + let entry_point_pc_u32 = u32::try_from(entry_point_pc).map_err(|_| { + VmError::EntryPointPcOutOfBounds { + pc: entry_point_pc, + instruction_count: self.program.instructions.len(), + entry_point: entry_point_name.clone(), + } + })?; - self.jump_to(*entry_point_pc) + self.jump_to(entry_point_pc_u32) } ExecutionMode::Suspendable => { self.reset_execution_state(); - if let Err(e) = self.validate_vm_state() { - return Err(VmError::Internal(alloc::format!( - "VM state validation failed before entry point execution: {} | {}", - e, - self.get_debug_state() - ))); - } - - self.execute_suspendable_entry(*entry_point_pc) + self.validate_vm_state()?; + self.execute_suspendable_entry(entry_point_pc) } } } @@ -95,87 +87,88 @@ impl RegoVM { .ok_or_else(|| VmError::EntryPointNotFound { name: String::from(name), available: self.program.entry_points.keys().cloned().collect(), + pc: self.pc, })?; if entry_point_pc >= self.program.instructions.len() { - return Err(VmError::Internal(alloc::format!( - "Entry point PC {} >= instruction count {} for '{}' | {}", - entry_point_pc, - self.program.instructions.len(), - name, - self.get_debug_state() - ))); + return Err(VmError::EntryPointPcOutOfBounds { + pc: entry_point_pc, + instruction_count: self.program.instructions.len(), + entry_point: String::from(name), + }); } match self.execution_mode { ExecutionMode::RunToCompletion => { self.reset_execution_state(); - if let Err(e) = self.validate_vm_state() { - return Err(VmError::Internal(alloc::format!( - "VM state validation failed before entry point execution: {} | {}", - e, - self.get_debug_state() - ))); - } + self.validate_vm_state()?; + let entry_point_pc_u32 = u32::try_from(entry_point_pc).map_err(|_| { + VmError::EntryPointPcOutOfBounds { + pc: entry_point_pc, + instruction_count: self.program.instructions.len(), + entry_point: String::from(name), + } + })?; - self.jump_to(entry_point_pc) + self.jump_to(entry_point_pc_u32) } ExecutionMode::Suspendable => { self.reset_execution_state(); - if let Err(e) = self.validate_vm_state() { - return Err(VmError::Internal(alloc::format!( - "VM state validation failed before entry point execution: {} | {}", - e, - self.get_debug_state() - ))); - } - + self.validate_vm_state()?; self.execute_suspendable_entry(entry_point_pc) } } } - pub(super) fn jump_to(&mut self, target: usize) -> Result { + pub(super) fn jump_to(&mut self, target: u32) -> Result { let program = self.program.clone(); + let target = self.convert_pc(target, "jump target")?; self.pc = target; while self.pc < program.instructions.len() { if self.executed_instructions >= self.max_instructions { return Err(VmError::InstructionLimitExceeded { limit: self.max_instructions, + executed: self.executed_instructions, + pc: self.pc, }); } - self.executed_instructions += 1; - let instruction = program.instructions[self.pc]; + self.executed_instructions = self.executed_instructions.saturating_add(1); + let instruction = program.instructions.get(self.pc).cloned().ok_or( + VmError::ProgramCounterOutOfBounds { + pc: self.pc, + instruction_count: program.instructions.len(), + }, + )?; match self.execute_instruction(&program, instruction)? { InstructionOutcome::Continue => { - self.pc += 1; + self.pc = self.pc.saturating_add(1); } InstructionOutcome::Return(value) => { return Ok(value); } InstructionOutcome::Break => { - return Ok(self.registers[0].clone()); + return Ok(self.registers.first().cloned().unwrap_or(Value::Undefined)); } InstructionOutcome::Suspend { reason } => { - return Err(VmError::Internal(alloc::format!( - "Suspend instruction {:?} is not supported in run-to-completion execution", - reason - ))); + return Err(VmError::UnsupportedSuspendInRunToCompletion { + reason, + pc: self.pc, + }); } } } - Ok(self.registers[0].clone()) + Ok(self.registers.first().cloned().unwrap_or(Value::Undefined)) } fn execute_run_to_completion(&mut self) -> Result { self.reset_execution_state(); self.execution_state = ExecutionState::Running; - match self.jump_to(0) { + match self.jump_to(0_u32) { Ok(value) => { self.execution_state = ExecutionState::Completed { result: value.clone(), @@ -220,30 +213,43 @@ impl RegoVM { .. } => (reason, last_result), current_state => { - return Err(VmError::Internal(alloc::format!( - "Cannot resume VM when execution state is {:?}", - current_state - ))); + return Err(VmError::InvalidResumeState { + state: alloc::format!("{:?}", current_state), + pc: self.pc, + }); } }; - match reason.clone() { - SuspendReason::HostAwait { dest, .. } => { - let value = resume_value.ok_or_else(|| { - VmError::Internal("HostAwait suspension requires a resume value".into()) + match reason { + SuspendReason::HostAwait { + dest, + argument, + identifier, + } => { + let value = resume_value.ok_or_else(|| VmError::MissingResumeValue { + reason: SuspendReason::HostAwait { + dest, + argument: argument.clone(), + identifier: identifier.clone(), + }, + pc: self.pc, })?; - if self.registers.len() <= dest as usize { - self.registers.resize(dest as usize + 1, Value::Undefined); + let dest_index = usize::from(dest); + if self.registers.len() <= dest_index { + let new_len = dest_index.saturating_add(1); + self.registers.resize(new_len, Value::Undefined); + } + if let Some(slot) = self.registers.get_mut(dest_index) { + *slot = value; } - self.registers[dest as usize] = value; } other_reason => { if resume_value.is_some() { - return Err(VmError::Internal(alloc::format!( - "Unexpected resume value supplied for {:?}", - other_reason - ))); + return Err(VmError::UnexpectedResumeValue { + reason: other_reason.clone(), + pc: self.pc, + }); } } } @@ -288,7 +294,7 @@ impl RegoVM { fn run_stackless_loop(&mut self, program: &Program, last_result: &mut Value) -> Result<()> { while !self.execution_stack.is_empty() { self.frame_pc_overridden = false; - let should_finalize_rule = if let Some(frame) = self.execution_stack.last() { + let should_finalize_rule = self.execution_stack.last().is_some_and(|frame| { matches!( frame.kind, FrameKind::Rule(RuleFrameData { @@ -296,12 +302,16 @@ impl RegoVM { .. }) ) - } else { - false - }; + }); if should_finalize_rule { - let frame = self.execution_stack.pop().expect("frame available"); + let frame = self + .execution_stack + .pop() + .ok_or(VmError::MissingExecutionFrame { + context: "finalizing rule", + pc: self.pc, + })?; self.finalize_rule_execution_frame(frame, last_result)?; if self.execution_stack.is_empty() { break; @@ -313,7 +323,10 @@ impl RegoVM { let frame = self .execution_stack .last() - .expect("stack checked to be non-empty"); + .ok_or(VmError::MissingExecutionFrame { + context: "determining pc", + pc: self.pc, + })?; frame.pc }; @@ -331,7 +344,13 @@ impl RegoVM { } if frame_pc >= program.instructions.len() { - let frame = self.execution_stack.pop().expect("frame exists"); + let frame = self + .execution_stack + .pop() + .ok_or(VmError::MissingExecutionFrame { + context: "finalizing out-of-range pc", + pc: self.pc, + })?; self.finalize_rule_execution_frame(frame, last_result)?; if self.execution_stack.is_empty() { break; @@ -343,23 +362,38 @@ impl RegoVM { self.execution_state = ExecutionState::Error { error: VmError::InstructionLimitExceeded { limit: self.max_instructions, + executed: self.executed_instructions, + pc: frame_pc, }, }; return Err(VmError::InstructionLimitExceeded { limit: self.max_instructions, + executed: self.executed_instructions, + pc: frame_pc, }); } self.pc = frame_pc; - let instruction = program.instructions[self.pc]; + let instruction = program.instructions.get(self.pc).cloned().ok_or( + VmError::ProgramCounterOutOfBounds { + pc: self.pc, + instruction_count: program.instructions.len(), + }, + )?; if let Some(frame_info) = self.execution_stack.last() { - if let FrameKind::Comprehension { context, .. } = &frame_info.kind { + if let FrameKind::Comprehension { ref context, .. } = frame_info.kind { if context.iteration_state.is_none() - && frame_pc == context.comprehension_end as usize + && frame_pc == usize::from(context.comprehension_end) && !matches!(instruction, Instruction::ComprehensionEnd { .. }) { let resume_pc = frame_pc; - let _completed = self.execution_stack.pop().expect("frame exists"); + let _completed = + self.execution_stack + .pop() + .ok_or(VmError::MissingExecutionFrame { + context: "unwinding comprehension", + pc: self.pc, + })?; if let Some(parent) = self.execution_stack.last_mut() { parent.pc = resume_pc; self.frame_pc_overridden = true; @@ -368,7 +402,7 @@ impl RegoVM { } } } - self.executed_instructions += 1; + self.executed_instructions = self.executed_instructions.saturating_add(1); let stack_depth_before = self.execution_stack.len(); @@ -377,7 +411,7 @@ impl RegoVM { let stack_depth_after = self.execution_stack.len(); if stack_depth_after == stack_depth_before && !self.frame_pc_overridden { if let Some(frame) = self.execution_stack.last_mut() { - frame.pc = self.pc + 1; + frame.pc = self.pc.saturating_add(1); } } if self.step_mode { @@ -406,7 +440,6 @@ impl RegoVM { if self.execution_stack.is_empty() { break; } - continue; } else { self.execution_stack.clear(); return Err(err); @@ -421,7 +454,7 @@ impl RegoVM { fn handle_instruction_suspend(&mut self, reason: SuspendReason, last_result: &Value) { if let Some(frame) = self.execution_stack.last_mut() { if !self.frame_pc_overridden && frame.pc <= self.pc { - frame.pc = self.pc + 1; + frame.pc = self.pc.saturating_add(1); } } @@ -439,7 +472,7 @@ impl RegoVM { } => { *last_result = self .registers - .get(return_value_register as usize) + .get(usize::from(return_value_register)) .cloned() .unwrap_or(Value::Undefined); } @@ -464,11 +497,14 @@ impl RegoVM { match frame.kind { FrameKind::Rule(mut data) => { - if self.registers.len() <= data.result_reg as usize { - self.registers - .resize(data.result_reg as usize + 1, Value::Undefined); + let result_index = usize::from(data.result_reg); + if self.registers.len() <= result_index { + let new_len = result_index.saturating_add(1); + self.registers.resize(new_len, Value::Undefined); + } + if let Some(slot) = self.registers.get_mut(result_index) { + *slot = value.clone(); } - self.registers[data.result_reg as usize] = value.clone(); data.accumulated_result = Some(value.clone()); data.any_body_succeeded = true; @@ -476,18 +512,21 @@ impl RegoVM { *last_result = result.clone(); if let Some(parent_frame) = self.execution_stack.last_mut() { - parent_frame.pc = self.pc + 1; + parent_frame.pc = self.pc.saturating_add(1); } return Ok(()); } FrameKind::Main { return_value_register, } => { - if self.registers.len() <= return_value_register as usize { - self.registers - .resize(return_value_register as usize + 1, Value::Undefined); + let ret_index = usize::from(return_value_register); + if self.registers.len() <= ret_index { + let new_len = ret_index.saturating_add(1); + self.registers.resize(new_len, Value::Undefined); + } + if let Some(slot) = self.registers.get_mut(ret_index) { + *slot = value.clone(); } - self.registers[return_value_register as usize] = value.clone(); *last_result = value; return Ok(()); } @@ -496,7 +535,6 @@ impl RegoVM { parent_frame.pc = return_pc; } // Propagate the return value outward until we reach the owning frame - continue; } } } @@ -564,7 +602,7 @@ impl RegoVM { } => { *last_result = self .registers - .get(return_value_register as usize) + .get(usize::from(return_value_register)) .cloned() .unwrap_or(Value::Undefined); } @@ -588,7 +626,7 @@ impl RegoVM { let result = self.finalize_rule_frame_data(data)?; *last_result = result.clone(); if let Some(parent_frame) = self.execution_stack.last_mut() { - parent_frame.pc = self.pc + 1; + parent_frame.pc = self.pc.saturating_add(1); } } other_kind => { diff --git a/src/rvm/vm/execution_model.rs b/src/rvm/vm/execution_model.rs index c581d25..e38d890 100644 --- a/src/rvm/vm/execution_model.rs +++ b/src/rvm/vm/execution_model.rs @@ -1,4 +1,3 @@ -#![allow(clippy::missing_const_for_fn)] // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. @@ -20,11 +19,11 @@ pub(super) struct ExecutionFrame { } impl ExecutionFrame { - pub(super) fn new(pc: usize, kind: FrameKind) -> Self { + pub(super) const fn new(pc: usize, kind: FrameKind) -> Self { Self { pc, kind } } - pub(super) fn main(pc: usize, return_register: u8) -> Self { + pub(super) const fn main(pc: usize, return_register: u8) -> Self { Self { pc, kind: FrameKind::Main { @@ -90,7 +89,7 @@ pub(super) struct ExecutionStack { } impl ExecutionStack { - pub fn new() -> Self { + pub const fn new() -> Self { Self { frames: Vec::new() } } @@ -110,11 +109,11 @@ impl ExecutionStack { self.frames.last_mut() } - pub fn is_empty(&self) -> bool { + pub const fn is_empty(&self) -> bool { self.frames.is_empty() } - pub fn len(&self) -> usize { + pub const fn len(&self) -> usize { self.frames.len() } diff --git a/src/rvm/vm/functions.rs b/src/rvm/vm/functions.rs index 31c8f66..b92ee50 100644 --- a/src/rvm/vm/functions.rs +++ b/src/rvm/vm/functions.rs @@ -1,6 +1,5 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow(clippy::indexing_slicing, clippy::as_conversions)] use crate::builtins; use crate::value::Value; use alloc::string::String; @@ -12,8 +11,16 @@ use super::machine::RegoVM; impl RegoVM { pub(super) fn execute_function_call(&mut self, params_index: u16) -> Result<()> { - let params = - self.program.instruction_data.function_call_params[params_index as usize].clone(); + let params = self + .program + .instruction_data + .get_function_call_params(params_index) + .cloned() + .ok_or(VmError::InvalidFunctionCallParamsIndex { + index: params_index, + pc: self.pc, + available: self.program.instruction_data.function_call_params.len(), + })?; match self.execution_mode { ExecutionMode::RunToCompletion => { self.execute_call_rule_common(params.dest, params.func_rule_index, Some(¶ms)) @@ -27,24 +34,41 @@ impl RegoVM { } pub(super) fn execute_builtin_call(&mut self, params_index: u16) -> Result<()> { - let params = &self.program.instruction_data.builtin_call_params[params_index as usize]; - let builtin_info = &self.program.builtin_info_table[params.builtin_index as usize]; + let params = self + .program + .instruction_data + .get_builtin_call_params(params_index) + .ok_or(VmError::InvalidBuiltinCallParamsIndex { + index: params_index, + pc: self.pc, + available: self.program.instruction_data.builtin_call_params.len(), + })?; + let builtin_info = self.program.get_builtin_info(params.builtin_index).ok_or( + VmError::InvalidBuiltinInfoIndex { + index: params.builtin_index, + pc: self.pc, + available: self.program.builtin_info_table.len(), + }, + )?; let mut args = Vec::new(); for &arg_reg in params.arg_registers().iter() { - let arg_value = self.registers[arg_reg as usize].clone(); + let arg_value = self.get_register(arg_reg)?.clone(); args.push(arg_value); } - if (args.len() as u16) != builtin_info.num_args { + let expected_args = builtin_info.num_args; + let actual_args = args.len(); + if u16::try_from(actual_args).unwrap_or(u16::MAX) != expected_args { return Err(VmError::BuiltinArgumentMismatch { - expected: builtin_info.num_args, - actual: args.len(), + expected: expected_args, + actual: actual_args, + pc: self.pc, }); } if args.iter().any(|a| a == &Value::Undefined) { - self.registers[params.dest as usize] = Value::Undefined; + self.set_register(params.dest, Value::Undefined)?; return Ok(()); } @@ -71,7 +95,7 @@ impl RegoVM { let cache_name = builtins::must_cache(builtin_info.name.as_str()); if let Some(name) = cache_name { if let Some(value) = self.builtins_cache.get(&(name, args.clone())) { - self.registers[params.dest as usize] = value.clone(); + self.set_register(params.dest, value.clone())?; return Ok(()); } } @@ -85,9 +109,9 @@ impl RegoVM { }; if result == Value::Undefined { - self.registers[params.dest as usize] = Value::Undefined; + self.set_register(params.dest, Value::Undefined)?; } else { - self.registers[params.dest as usize] = result.clone(); + self.set_register(params.dest, result.clone())?; } if let Some(name) = cache_name { @@ -96,6 +120,7 @@ impl RegoVM { } else { return Err(VmError::BuiltinNotResolved { name: builtin_info.name.clone(), + pc: self.pc, }); } diff --git a/src/rvm/vm/loops.rs b/src/rvm/vm/loops.rs index e32cff9..494759f 100644 --- a/src/rvm/vm/loops.rs +++ b/src/rvm/vm/loops.rs @@ -1,18 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::indexing_slicing, - clippy::expect_used, - clippy::arithmetic_side_effects, - clippy::unwrap_used, - clippy::shadow_unrelated, - clippy::missing_const_for_fn, - clippy::unused_self, - clippy::as_conversions, - clippy::pattern_type_mismatch -)] // VM loop handling indexes directly for performance and clarity - use crate::rvm::instructions::LoopMode; use crate::value::Value; @@ -26,9 +14,9 @@ fn compute_body_resume_pc(loop_start_pc: usize, body_start: u16) -> usize { return 0; } - let candidate = body_start.saturating_sub(1) as usize; + let candidate = usize::from(body_start.saturating_sub(1)); if candidate == loop_start_pc { - body_start as usize + usize::from(body_start) } else { candidate } @@ -84,15 +72,14 @@ impl RegoVM { mode: &LoopMode, params: LoopParams, ) -> Result<()> { - let initial_result = match mode { + let initial_result = match *mode { LoopMode::Any | LoopMode::Every | LoopMode::ForEach => Value::Bool(false), }; - self.registers[params.result_reg as usize] = initial_result.clone(); + self.set_register(params.result_reg, initial_result.clone())?; + let collection_value = self.get_register(params.collection)?.clone(); - let collection_value = self.registers[params.collection as usize].clone(); - - let iteration_state = match &collection_value { - Value::Array(items) => { + let iteration_state = match collection_value { + Value::Array(ref items) => { if items.is_empty() { self.handle_empty_collection(mode, params.result_reg, params.loop_end)?; return Ok(()); @@ -102,7 +89,7 @@ impl RegoVM { index: 0, } } - Value::Object(obj) => { + Value::Object(ref obj) => { if obj.is_empty() { self.handle_empty_collection(mode, params.result_reg, params.loop_end)?; return Ok(()); @@ -113,7 +100,7 @@ impl RegoVM { first_iteration: true, } } - Value::Set(set) => { + Value::Set(ref set) => { if set.is_empty() { self.handle_empty_collection(mode, params.result_reg, params.loop_end)?; return Ok(()); @@ -133,15 +120,15 @@ impl RegoVM { let has_next = self.setup_next_iteration(&iteration_state, params.key_reg, params.value_reg)?; if !has_next { - self.pc = params.loop_end as usize; + self.pc = usize::from(params.loop_end); return Ok(()); } - let loop_next_pc = params.loop_end - 1; + let loop_next_pc = params.loop_end.saturating_sub(1); let body_resume_pc = compute_body_resume_pc(self.pc, params.body_start); let loop_context = LoopContext { - mode: mode.clone(), + mode: *mode, iteration_state, key_reg: params.key_reg, value_reg: params.value_reg, @@ -157,7 +144,7 @@ impl RegoVM { self.loop_stack.push(loop_context); - self.pc = params.body_start as usize - 1; + self.pc = usize::from(params.body_start.saturating_sub(1)); Ok(()) } @@ -169,46 +156,46 @@ impl RegoVM { ) -> Result<()> { if let Some(mut loop_ctx) = self.loop_stack.pop() { let body_start = loop_ctx.body_start; - let loop_end = loop_ctx.loop_end; + let loop_end_local = loop_ctx.loop_end; - loop_ctx.total_iterations += 1; + loop_ctx.total_iterations = loop_ctx.total_iterations.saturating_add(1); - let iteration_succeeded = self.check_iteration_success(&loop_ctx)?; + let iteration_succeeded = Self::check_iteration_success(&loop_ctx)?; if iteration_succeeded { - loop_ctx.success_count += 1; + loop_ctx.success_count = loop_ctx.success_count.saturating_add(1); } - let action = self.determine_loop_action(&loop_ctx.mode, iteration_succeeded); + let action = Self::determine_loop_action(&loop_ctx.mode, iteration_succeeded); match action { LoopAction::ExitWithSuccess => { - self.registers[loop_ctx.result_reg as usize] = Value::Bool(true); - self.pc = loop_end as usize - 1; + self.set_register(loop_ctx.result_reg, Value::Bool(true))?; + self.pc = usize::from(loop_end_local.saturating_sub(1)); return Ok(()); } LoopAction::ExitWithFailure => { - self.registers[loop_ctx.result_reg as usize] = Value::Bool(false); - self.pc = loop_end as usize - 1; + self.set_register(loop_ctx.result_reg, Value::Bool(false))?; + self.pc = usize::from(loop_end_local.saturating_sub(1)); return Ok(()); } LoopAction::Continue => {} } - if let IterationState::Object { + if let &mut IterationState::Object { ref mut current_key, .. } = &mut loop_ctx.iteration_state { if loop_ctx.key_reg != loop_ctx.value_reg { - *current_key = Some(self.registers[loop_ctx.key_reg as usize].clone()); + *current_key = Some(self.get_register(loop_ctx.key_reg)?.clone()); } - } else if let IterationState::Set { + } else if let &mut IterationState::Set { ref mut current_item, .. } = &mut loop_ctx.iteration_state { - *current_item = Some(self.registers[loop_ctx.value_reg as usize].clone()); + *current_item = Some(self.get_register(loop_ctx.value_reg)?.clone()); } loop_ctx.iteration_state.advance(); @@ -222,7 +209,7 @@ impl RegoVM { loop_ctx.current_iteration_failed = false; self.loop_stack.push(loop_ctx); - self.pc = body_start as usize - 1; + self.pc = usize::from(body_start.saturating_sub(1)); } else { let final_result = match loop_ctx.mode { LoopMode::Any => Value::Bool(loop_ctx.success_count > 0), @@ -232,14 +219,14 @@ impl RegoVM { LoopMode::ForEach => Value::Bool(loop_ctx.success_count > 0), }; - self.registers[loop_ctx.result_reg as usize] = final_result; + self.set_register(loop_ctx.result_reg, final_result)?; - self.pc = loop_end as usize - 1; + self.pc = usize::from(loop_end_local.saturating_sub(1)); } Ok(()) } else { - self.pc = loop_end as usize; + self.pc = usize::from(loop_end); Ok(()) } } @@ -249,15 +236,15 @@ impl RegoVM { mode: &LoopMode, params: LoopParams, ) -> Result<()> { - let initial_result = match mode { + let initial_result = match *mode { LoopMode::Any | LoopMode::Every | LoopMode::ForEach => Value::Bool(false), }; - self.registers[params.result_reg as usize] = initial_result.clone(); + self.set_register(params.result_reg, initial_result.clone())?; - let collection_value = self.registers[params.collection as usize].clone(); + let collection_value = self.get_register(params.collection)?.clone(); - let iteration_state = match &collection_value { - Value::Array(items) => { + let iteration_state = match collection_value { + Value::Array(ref items) => { if items.is_empty() { self.handle_empty_collection(mode, params.result_reg, params.loop_end)?; return Ok(()); @@ -267,7 +254,7 @@ impl RegoVM { index: 0, } } - Value::Object(obj) => { + Value::Object(ref obj) => { if obj.is_empty() { self.handle_empty_collection(mode, params.result_reg, params.loop_end)?; return Ok(()); @@ -278,7 +265,7 @@ impl RegoVM { first_iteration: true, } } - Value::Set(set) => { + Value::Set(ref set) => { if set.is_empty() { self.handle_empty_collection(mode, params.result_reg, params.loop_end)?; return Ok(()); @@ -298,15 +285,15 @@ impl RegoVM { let has_next = self.setup_next_iteration(&iteration_state, params.key_reg, params.value_reg)?; if !has_next { - self.pc = params.loop_end as usize; + self.pc = usize::from(params.loop_end); return Ok(()); } - let loop_next_pc = params.loop_end - 1; + let loop_next_pc = params.loop_end.saturating_sub(1); let body_resume_pc = compute_body_resume_pc(self.pc, params.body_start); let loop_context = LoopContext { - mode: mode.clone(), + mode: *mode, iteration_state, key_reg: params.key_reg, value_reg: params.value_reg, @@ -321,9 +308,9 @@ impl RegoVM { }; let frame = ExecutionFrame::new( - params.body_start as usize, + usize::from(params.body_start), FrameKind::Loop { - return_pc: params.loop_end as usize, + return_pc: usize::from(params.loop_end), context: loop_context, }, ); @@ -345,7 +332,7 @@ impl RegoVM { // resume at the same LoopNext when the owning loop frame has // already been popped (for example after a manual comprehension // finalizes in suspendable mode). - let mut target_pc = loop_end as usize; + let mut target_pc = usize::from(loop_end); if target_pc <= self.pc { target_pc = self.pc.saturating_add(1); } @@ -359,32 +346,33 @@ impl RegoVM { let frame = self .execution_stack .last_mut() - .ok_or(VmError::AssertionFailed)?; + .ok_or(VmError::AssertionFailed { pc: self.pc })?; match &mut frame.kind { - FrameKind::Loop { return_pc, context } => { - context.total_iterations += 1; + &mut FrameKind::Loop { + ref return_pc, + ref mut context, + } => { + context.total_iterations = context.total_iterations.saturating_add(1); let succeeded = !context.current_iteration_failed; if succeeded { - context.success_count += 1; + context.success_count = context.success_count.saturating_add(1); } - ( - *return_pc, - context.result_reg, - context.mode.clone(), - succeeded, - ) + (*return_pc, context.result_reg, context.mode, succeeded) } - _ => return Err(VmError::AssertionFailed), + _ => return Err(VmError::AssertionFailed { pc: self.pc }), } }; - let action = self.determine_loop_action(&loop_mode, iteration_succeeded); + let action = Self::determine_loop_action(&loop_mode, iteration_succeeded); match action { LoopAction::ExitWithSuccess => { - self.registers[result_reg as usize] = Value::Bool(true); - let completed_frame = self.execution_stack.pop().expect("loop frame exists"); + self.set_register(result_reg, Value::Bool(true))?; + let completed_frame = self + .execution_stack + .pop() + .ok_or(VmError::AssertionFailed { pc: self.pc })?; if let Some(parent) = self.execution_stack.last_mut() { parent.pc = resume_pc; self.frame_pc_overridden = true; @@ -393,8 +381,11 @@ impl RegoVM { Ok(()) } LoopAction::ExitWithFailure => { - self.registers[result_reg as usize] = Value::Bool(false); - let completed_frame = self.execution_stack.pop().expect("loop frame exists"); + self.set_register(result_reg, Value::Bool(false))?; + let completed_frame = self + .execution_stack + .pop() + .ok_or(VmError::AssertionFailed { pc: self.pc })?; if let Some(parent) = self.execution_stack.last_mut() { parent.pc = resume_pc; self.frame_pc_overridden = true; @@ -404,43 +395,67 @@ impl RegoVM { } LoopAction::Continue => { let (mode, success_count, total_iterations, key_reg, value_reg, iteration_state) = { + let (mode, success_count, total_iterations, key_reg, value_reg) = { + let frame = self + .execution_stack + .last() + .ok_or(VmError::AssertionFailed { pc: self.pc })?; + match frame.kind { + FrameKind::Loop { ref context, .. } => ( + context.mode, + context.success_count, + context.total_iterations, + context.key_reg, + context.value_reg, + ), + _ => return Err(VmError::AssertionFailed { pc: self.pc }), + } + }; + + let key_value = if key_reg != value_reg { + Some(self.get_register(key_reg)?.clone()) + } else { + None + }; + let value_value = self.get_register(value_reg)?.clone(); + let frame = self .execution_stack .last_mut() - .ok_or(VmError::AssertionFailed)?; + .ok_or(VmError::AssertionFailed { pc: self.pc })?; match &mut frame.kind { - FrameKind::Loop { context, .. } => { - if let IterationState::Object { + &mut FrameKind::Loop { + ref mut context, .. + } => { + if let &mut IterationState::Object { ref mut current_key, .. } = &mut context.iteration_state { if context.key_reg != context.value_reg { - *current_key = - Some(self.registers[context.key_reg as usize].clone()); + *current_key = key_value; } - } else if let IterationState::Set { + } else if let &mut IterationState::Set { ref mut current_item, .. } = &mut context.iteration_state { - *current_item = - Some(self.registers[context.value_reg as usize].clone()); + *current_item = Some(value_value.clone()); } context.iteration_state.advance(); context.current_iteration_failed = false; ( - context.mode.clone(), - context.success_count, - context.total_iterations, - context.key_reg, - context.value_reg, + mode, + success_count, + total_iterations, + key_reg, + value_reg, context.iteration_state.clone(), ) } - _ => return Err(VmError::AssertionFailed), + _ => return Err(VmError::AssertionFailed { pc: self.pc }), } }; @@ -448,10 +463,10 @@ impl RegoVM { if has_next { if let Some(frame) = self.execution_stack.last_mut() { - if let FrameKind::Loop { context, .. } = &frame.kind { + if let FrameKind::Loop { ref context, .. } = frame.kind { frame.pc = context.body_resume_pc; } else { - frame.pc = body_start as usize; + frame.pc = usize::from(body_start); } self.frame_pc_overridden = true; } @@ -463,9 +478,12 @@ impl RegoVM { LoopMode::ForEach => Value::Bool(success_count > 0), }; - self.registers[result_reg as usize] = final_result; + self.set_register(result_reg, final_result)?; - let completed_frame = self.execution_stack.pop().expect("loop frame exists"); + let completed_frame = self + .execution_stack + .pop() + .ok_or(VmError::AssertionFailed { pc: self.pc })?; if let Some(parent) = self.execution_stack.last_mut() { parent.pc = resume_pc; self.frame_pc_overridden = true; @@ -484,14 +502,14 @@ impl RegoVM { result_reg: u8, loop_end: u16, ) -> Result<()> { - let result = match mode { + let result = match *mode { LoopMode::Any => Value::Bool(false), LoopMode::Every => Value::Bool(true), LoopMode::ForEach => Value::Bool(false), }; - self.registers[result_reg as usize] = result; - self.pc = (loop_end as usize).saturating_sub(1); + self.set_register(result_reg, result)?; + self.pc = usize::from(loop_end).saturating_sub(1); Ok(()) } @@ -501,45 +519,51 @@ impl RegoVM { key_reg: u8, value_reg: u8, ) -> Result { - match state { - IterationState::Array { items, index } => { + match *state { + IterationState::Array { + ref items, + ref index, + } => { if *index < items.len() { if key_reg != value_reg { - let key_value = Value::from(*index as f64); - self.registers[key_reg as usize] = key_value; + let key_value = Value::from(*index); + self.set_register(key_reg, key_value)?; + } + if let Some(item_value) = items.get(*index).cloned() { + self.set_register(value_reg, item_value)?; + Ok(true) + } else { + Ok(false) } - let item_value = items[*index].clone(); - self.registers[value_reg as usize] = item_value; - Ok(true) } else { Ok(false) } } IterationState::Object { - obj, - current_key, - first_iteration, + ref obj, + ref current_key, + ref first_iteration, } => { if *first_iteration { if let Some((key, value)) = obj.iter().next() { if key_reg != value_reg { - self.registers[key_reg as usize] = key.clone(); + self.set_register(key_reg, key.clone())?; } - self.registers[value_reg as usize] = value.clone(); + self.set_register(value_reg, value.clone())?; Ok(true) } else { Ok(false) } - } else if let Some(ref current) = current_key { + } else if let Some(ref current) = *current_key { let mut range_iter = obj.range(( core::ops::Bound::Excluded(current), core::ops::Bound::Unbounded, )); if let Some((key, value)) = range_iter.next() { if key_reg != value_reg { - self.registers[key_reg as usize] = key.clone(); + self.set_register(key_reg, key.clone())?; } - self.registers[value_reg as usize] = value.clone(); + self.set_register(value_reg, value.clone())?; Ok(true) } else { Ok(false) @@ -549,30 +573,30 @@ impl RegoVM { } } IterationState::Set { - items, - current_item, - first_iteration, + ref items, + ref current_item, + ref first_iteration, } => { if *first_iteration { if let Some(item) = items.iter().next() { if key_reg != value_reg { - self.registers[key_reg as usize] = item.clone(); + self.set_register(key_reg, item.clone())?; } - self.registers[value_reg as usize] = item.clone(); + self.set_register(value_reg, item.clone())?; Ok(true) } else { Ok(false) } - } else if let Some(ref current) = current_item { + } else if let Some(ref current) = *current_item { let mut range_iter = items.range(( core::ops::Bound::Excluded(current), core::ops::Bound::Unbounded, )); if let Some(item) = range_iter.next() { if key_reg != value_reg { - self.registers[key_reg as usize] = item.clone(); + self.set_register(key_reg, item.clone())?; } - self.registers[value_reg as usize] = item.clone(); + self.set_register(value_reg, item.clone())?; Ok(true) } else { Ok(false) @@ -584,15 +608,15 @@ impl RegoVM { } } - fn check_iteration_success(&self, loop_ctx: &LoopContext) -> Result { + const fn check_iteration_success(loop_ctx: &LoopContext) -> Result { Ok(!loop_ctx.current_iteration_failed) } - fn determine_loop_action(&self, mode: &LoopMode, success: bool) -> LoopAction { + const fn determine_loop_action(mode: &LoopMode, success: bool) -> LoopAction { match (mode, success) { - (LoopMode::Any, true) => LoopAction::ExitWithSuccess, - (LoopMode::Every, false) => LoopAction::ExitWithFailure, - (LoopMode::ForEach, _) => LoopAction::Continue, + (&LoopMode::Any, true) => LoopAction::ExitWithSuccess, + (&LoopMode::Every, false) => LoopAction::ExitWithFailure, + (&LoopMode::ForEach, _) => LoopAction::Continue, _ => LoopAction::Continue, } } @@ -604,9 +628,12 @@ impl RegoVM { if !self.loop_stack.is_empty() { let (loop_mode, loop_next_pc, loop_end, result_reg) = { - let loop_ctx = self.loop_stack.last().unwrap(); + let loop_ctx = self + .loop_stack + .last() + .ok_or(VmError::AssertionFailed { pc: self.pc })?; ( - loop_ctx.mode.clone(), + loop_ctx.mode, loop_ctx.loop_next_pc, loop_ctx.loop_end, loop_ctx.result_reg, @@ -619,24 +646,24 @@ impl RegoVM { loop_ctx_mut.current_iteration_failed = true; } - self.pc = loop_next_pc as usize - 1; + self.pc = usize::from(loop_next_pc.saturating_sub(1)); } LoopMode::Every => { self.loop_stack.pop(); - self.pc = loop_end as usize - 1; - self.registers[result_reg as usize] = Value::Bool(false); + self.pc = usize::from(loop_end.saturating_sub(1)); + self.set_register(result_reg, Value::Bool(false))?; } _ => { if let Some(loop_ctx_mut) = self.loop_stack.last_mut() { loop_ctx_mut.current_iteration_failed = true; } - self.pc = loop_next_pc as usize - 1; + self.pc = usize::from(loop_next_pc.saturating_sub(1)); } } } else if self.handle_comprehension_condition_failure_run_to_completion()? { // handled by comprehension context } else { - return Err(VmError::AssertionFailed); + return Err(VmError::AssertionFailed { pc: self.pc }); } Ok(()) @@ -647,31 +674,76 @@ impl RegoVM { return Ok(()); } - if let Some(ExecutionFrame { - kind: FrameKind::Loop { return_pc, context }, + if let Some(&ExecutionFrame { + kind: FrameKind::Loop { .. }, .. - }) = self.execution_stack.last_mut() + }) = self.execution_stack.last() { - let resume_pc = *return_pc; - match context.mode { + let (mode, resume_pc, loop_next_pc, result_reg) = { + let frame = self + .execution_stack + .last_mut() + .ok_or(VmError::AssertionFailed { pc: self.pc })?; + match &mut frame.kind { + &mut FrameKind::Loop { + ref return_pc, + ref mut context, + } => ( + context.mode, + *return_pc, + context.loop_next_pc, + context.result_reg, + ), + _ => return Err(VmError::AssertionFailed { pc: self.pc }), + } + }; + + match mode { LoopMode::Any | LoopMode::ForEach => { - context.current_iteration_failed = true; - self.pc = context.loop_next_pc as usize - 1; + if let Some(&mut ExecutionFrame { + kind: + FrameKind::Loop { + context: ref mut ctx, + .. + }, + .. + }) = self.execution_stack.last_mut() + { + ctx.current_iteration_failed = true; + self.pc = usize::from(loop_next_pc.saturating_sub(1)); + } + Ok(()) } LoopMode::Every => { - self.registers[context.result_reg as usize] = Value::Bool(false); - let completed_frame = self.execution_stack.pop().expect("loop frame exists"); + if let Some(&mut ExecutionFrame { + kind: + FrameKind::Loop { + context: ref mut ctx, + .. + }, + .. + }) = self.execution_stack.last_mut() + { + ctx.current_iteration_failed = true; + } + + self.set_register(result_reg, Value::Bool(false))?; + let completed_frame = self + .execution_stack + .pop() + .ok_or(VmError::AssertionFailed { pc: self.pc })?; if let Some(parent) = self.execution_stack.last_mut() { parent.pc = resume_pc; + self.frame_pc_overridden = true; } drop(completed_frame); + Ok(()) } } - Ok(()) } else if self.handle_comprehension_condition_failure_suspendable()? { Ok(()) } else { - Err(VmError::AssertionFailed) + Err(VmError::AssertionFailed { pc: self.pc }) } } } diff --git a/src/rvm/vm/machine.rs b/src/rvm/vm/machine.rs index d07f562..4f72f4a 100644 --- a/src/rvm/vm/machine.rs +++ b/src/rvm/vm/machine.rs @@ -1,12 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - missing_debug_implementations, - clippy::missing_const_for_fn, - clippy::pattern_type_mismatch -)] // VM structs are not debug printed - use crate::rvm::program::Program; use crate::value::Value; use crate::CompiledPolicy; @@ -23,6 +17,7 @@ use super::execution_model::{ }; /// The Rego Virtual Machine +#[derive(Debug)] pub struct RegoVM { /// Registers for storing values during execution pub(super) registers: Vec, @@ -181,7 +176,7 @@ impl RegoVM { } /// Set the maximum number of instructions that can be executed - pub fn set_max_instructions(&mut self, max: usize) { + pub const fn set_max_instructions(&mut self, max: usize) { self.max_instructions = max; } @@ -220,47 +215,47 @@ impl RegoVM { } // Public getters for visualization - pub fn get_pc(&self) -> usize { + pub const fn get_pc(&self) -> usize { self.pc } - pub fn get_registers(&self) -> &Vec { + pub const fn get_registers(&self) -> &Vec { &self.registers } - pub fn get_program(&self) -> &Arc { + pub const fn get_program(&self) -> &Arc { &self.program } - pub fn get_call_stack(&self) -> &Vec { + pub const fn get_call_stack(&self) -> &Vec { &self.call_rule_stack } - pub fn get_loop_stack(&self) -> &Vec { + pub const fn get_loop_stack(&self) -> &Vec { &self.loop_stack } - pub fn get_cache_hits(&self) -> usize { + pub const fn get_cache_hits(&self) -> usize { self.cache_hits } /// Set the execution mode for the VM - pub fn set_execution_mode(&mut self, mode: ExecutionMode) { + pub const fn set_execution_mode(&mut self, mode: ExecutionMode) { self.execution_mode = mode; } /// Configure whether builtin operations should raise errors strictly - pub fn set_strict_builtin_errors(&mut self, strict: bool) { + pub const fn set_strict_builtin_errors(&mut self, strict: bool) { self.strict_builtin_errors = strict; } /// Returns whether builtin operations raise errors strictly - pub fn strict_builtin_errors(&self) -> bool { + pub const fn strict_builtin_errors(&self) -> bool { self.strict_builtin_errors } /// Enable or disable single-step execution for suspendable runs - pub fn set_step_mode(&mut self, enabled: bool) { + pub const fn set_step_mode(&mut self, enabled: bool) { self.step_mode = enabled; } @@ -295,6 +290,7 @@ impl RegoVM { let missing_error = || VmError::HostAwaitResponseMissing { dest, identifier: identifier.clone(), + pc: self.pc, }; let (response, should_remove) = { @@ -316,20 +312,48 @@ impl RegoVM { } /// Get the current execution mode - pub fn get_execution_mode(&self) -> ExecutionMode { + pub const fn get_execution_mode(&self) -> ExecutionMode { self.execution_mode } /// Get the current execution state of the VM - pub fn execution_state(&self) -> &ExecutionState { + pub const fn execution_state(&self) -> &ExecutionState { &self.execution_state } /// Get the suspend reason if the VM is currently suspended - pub fn suspend_reason(&self) -> Option<&SuspendReason> { - match &self.execution_state { - ExecutionState::Suspended { reason, .. } => Some(reason), + pub const fn suspend_reason(&self) -> Option<&SuspendReason> { + match self.execution_state { + ExecutionState::Suspended { ref reason, .. } => Some(reason), _ => None, } } + + #[inline] + #[allow(dead_code)] + pub(super) fn get_register(&self, index: u8) -> Result<&Value> { + self.registers + .get(usize::from(index)) + .ok_or(VmError::RegisterIndexOutOfBounds { + index, + pc: self.pc, + register_count: self.registers.len(), + }) + } + + #[inline] + #[allow(dead_code)] + pub(super) fn set_register(&mut self, index: u8, value: Value) -> Result<()> { + let register_count = self.registers.len(); + + let slot = self.registers.get_mut(usize::from(index)).ok_or( + VmError::RegisterIndexOutOfBounds { + index, + pc: self.pc, + register_count, + }, + )?; + *slot = value; + Ok(()) + } } diff --git a/src/rvm/vm/rules.rs b/src/rvm/vm/rules.rs index 7394c1a..5173a23 100644 --- a/src/rvm/vm/rules.rs +++ b/src/rvm/vm/rules.rs @@ -1,23 +1,12 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::indexing_slicing, - clippy::arithmetic_side_effects, - clippy::expect_used, - clippy::shadow_unrelated, - clippy::unused_self, - clippy::semicolon_if_nothing_returned, - clippy::missing_const_for_fn, - clippy::as_conversions, - clippy::needless_continue, - clippy::pattern_type_mismatch -)] // VM rule execution indexes directly and uses expect for invariant checks - use crate::rvm::instructions::FunctionCallParams; use crate::rvm::program::{RuleInfo, RuleType}; use crate::value::Value; +use alloc::format; use alloc::vec::Vec; +use core::convert::TryFrom as _; use core::mem; use super::context::CallRuleContext; @@ -37,9 +26,9 @@ impl RegoVM { let mut first_successful_result: Option = None; let mut rule_failed_due_to_inconsistency = false; let is_function_call = rule_info.function_info.is_some(); - let result_reg = rule_info.result_reg as usize; + let result_reg = rule_info.result_reg; - let num_registers = rule_info.num_registers as usize; + let num_registers = usize::from(rule_info.num_registers); let mut register_window = self.new_register_window(); register_window.clear(); register_window.reserve(num_registers); @@ -48,10 +37,10 @@ impl RegoVM { let num_retained_registers = match function_call_params { Some(params) => { - for arg in params.args[0..params.num_args as usize].iter() { - register_window.push(self.registers[*arg as usize].clone()); + for &arg in params.arg_registers() { + register_window.push(self.get_register(arg)?.clone()); } - params.num_args as usize + 1 + self.checked_add_one(params.arg_count(), "retained function registers")? } _ => match rule_info.rule_type { RuleType::PartialSet | RuleType::PartialObject => 1, @@ -59,16 +48,19 @@ impl RegoVM { }, }; - let mut old_registers = Vec::default(); - mem::swap(&mut old_registers, &mut self.registers); + let mut previous_registers = Vec::default(); + mem::swap(&mut previous_registers, &mut self.registers); - let mut old_loop_stack = Vec::default(); - mem::swap(&mut old_loop_stack, &mut self.loop_stack); + let mut previous_loop_stack = Vec::default(); + mem::swap(&mut previous_loop_stack, &mut self.loop_stack); - let mut old_comprehension_stack = Vec::default(); - mem::swap(&mut old_comprehension_stack, &mut self.comprehension_stack); + let mut previous_comprehension_stack = Vec::default(); + mem::swap( + &mut previous_comprehension_stack, + &mut self.comprehension_stack, + ); - self.register_stack.push(old_registers); + self.register_stack.push(previous_registers); self.registers = register_window; 'outer: for (def_idx, definition_bodies) in rule_definitions.iter().enumerate() { @@ -85,7 +77,7 @@ impl RegoVM { if let Some(destructuring_entry_point) = rule_info.destructuring_blocks.get(def_idx).and_then(|x| *x) { - match self.jump_to(destructuring_entry_point as usize) { + match self.jump_to(destructuring_entry_point) { Ok(_result) => {} Err(_e) => { continue 'outer; @@ -93,15 +85,15 @@ impl RegoVM { } } - match self.jump_to(*body_entry_point as usize) { + match self.jump_to(*body_entry_point) { Ok(_) => { if matches!(rule_info.rule_type, RuleType::Complete) || is_function_call { - let current_result = self.registers[result_reg].clone(); + let current_result = self.get_register(result_reg)?.clone(); if current_result != Value::Undefined { if let Some(ref expected) = first_successful_result { if *expected != current_result { rule_failed_due_to_inconsistency = true; - self.registers[result_reg] = Value::Undefined; + self.set_register(result_reg, Value::Undefined)?; break; } } else { @@ -114,9 +106,7 @@ impl RegoVM { // are treated as else-branches and must not be evaluated. break; } - Err(_e) => { - continue; - } + Err(_e) => {} } } @@ -130,19 +120,19 @@ impl RegoVM { } else if let Some(successful_result) = first_successful_result { successful_result } else { - self.registers[result_reg].clone() + self.get_register(result_reg)?.clone() }; - if let Some(old_registers) = self.register_stack.pop() { + if let Some(restored_registers) = self.register_stack.pop() { let mut current_register_window = Vec::default(); mem::swap(&mut current_register_window, &mut self.registers); self.return_register_window(current_register_window); - self.registers = old_registers; + self.registers = restored_registers; } - self.loop_stack = old_loop_stack; - self.comprehension_stack = old_comprehension_stack; + self.loop_stack = previous_loop_stack; + self.comprehension_stack = previous_comprehension_stack; Ok((final_result, rule_failed_due_to_inconsistency)) } @@ -153,25 +143,41 @@ impl RegoVM { rule_index: u16, function_call_params: Option<&FunctionCallParams>, ) -> Result<()> { - let rule_idx = rule_index as usize; + let rule_idx = usize::from(rule_index); if rule_idx >= self.rule_cache.len() { - return Err(VmError::RuleIndexOutOfBounds { index: rule_index }); + return Err(VmError::RuleIndexOutOfBounds { + index: rule_index, + pc: self.pc, + available: self.rule_cache.len(), + }); } let rule_info = self .program .rule_infos .get(rule_idx) - .ok_or(VmError::RuleInfoMissing { index: rule_index })? + .ok_or(VmError::RuleInfoMissing { + index: rule_index, + pc: self.pc, + available: self.program.rule_infos.len(), + })? .clone(); let is_function_rule = rule_info.function_info.is_some(); if !is_function_rule { - let (computed, cached_result) = &self.rule_cache[rule_idx]; + let (ref computed, ref cached_result) = + *self + .rule_cache + .get(rule_idx) + .ok_or(VmError::RuleIndexOutOfBounds { + index: rule_index, + pc: self.pc, + available: self.rule_cache.len(), + })?; if *computed { - self.registers[dest as usize] = cached_result.clone(); + self.set_register(dest, cached_result.clone())?; return Ok(()); } } @@ -182,9 +188,18 @@ impl RegoVM { if rule_definitions.is_empty() { let result = Value::Undefined; if !is_function_rule { - self.rule_cache[rule_idx] = (true, result.clone()); + let available = self.rule_cache.len(); + let entry = + self.rule_cache + .get_mut(rule_idx) + .ok_or(VmError::RuleIndexOutOfBounds { + index: rule_index, + pc: self.pc, + available, + })?; + *entry = (true, result.clone()); } - self.registers[dest as usize] = result; + self.set_register(dest, result)?; return Ok(()); } @@ -201,9 +216,12 @@ impl RegoVM { let (final_result, rule_failed_due_to_inconsistency) = self .execute_rule_definitions_common(&rule_definitions, &rule_info, function_call_params)?; - self.registers[dest as usize] = Value::Undefined; + self.set_register(dest, Value::Undefined)?; - let call_context = self.call_rule_stack.pop().expect("Call stack underflow"); + let call_context = self + .call_rule_stack + .pop() + .ok_or(VmError::CallRuleStackUnderflow { pc: self.pc })?; self.pc = call_context.return_pc; let result_from_rule = if !rule_failed_due_to_inconsistency { @@ -212,27 +230,30 @@ impl RegoVM { Value::Undefined }; - self.registers[dest as usize] = result_from_rule.clone(); + self.set_register(dest, result_from_rule.clone())?; - if self.registers[dest as usize] == Value::Undefined && !rule_failed_due_to_inconsistency { + if self.get_register(dest)? == &Value::Undefined && !rule_failed_due_to_inconsistency { match call_context.rule_type { RuleType::PartialSet => { - self.registers[dest as usize] = Value::new_set(); + self.set_register(dest, Value::new_set())?; } RuleType::PartialObject => { - self.registers[dest as usize] = Value::new_object(); + self.set_register(dest, Value::new_object())?; } RuleType::Complete => { - if let Some(rule_info) = self + if let Some(rule_metadata) = self .program .rule_infos - .get(call_context.rule_index as usize) + .get(usize::from(call_context.rule_index)) { - if let Some(default_literal_index) = rule_info.default_literal_index { - if let Some(default_value) = - self.program.literals.get(default_literal_index as usize) + if let Some(default_literal_index) = rule_metadata.default_literal_index { + if let Some(default_value) = self + .program + .literals + .get(usize::from(default_literal_index)) + .cloned() { - self.registers[dest as usize] = default_value.clone(); + self.set_register(dest, default_value)?; } } } @@ -240,13 +261,21 @@ impl RegoVM { } } - let final_result = self.registers[dest as usize].clone(); + let final_value = self.get_register(dest)?.clone(); if !is_function_rule { - self.rule_cache[rule_idx] = (true, final_result); + let available = self.rule_cache.len(); + let entry = self + .rule_cache + .get_mut(rule_idx) + .ok_or(VmError::RuleIndexOutOfBounds { + index: rule_index, + pc: self.pc, + available, + })?; + *entry = (true, final_value.clone()); } Ok(()) } - pub(super) fn execute_call_rule(&mut self, dest: u8, rule_index: u16) -> Result<()> { match self.execution_mode { ExecutionMode::RunToCompletion => self.execute_call_rule_common(dest, rule_index, None), @@ -262,25 +291,41 @@ impl RegoVM { rule_index: u16, function_call_params: Option<&FunctionCallParams>, ) -> Result<()> { - let rule_idx = rule_index as usize; + let rule_idx = usize::from(rule_index); if rule_idx >= self.rule_cache.len() { - return Err(VmError::RuleIndexOutOfBounds { index: rule_index }); + return Err(VmError::RuleIndexOutOfBounds { + index: rule_index, + pc: self.pc, + available: self.rule_cache.len(), + }); } let rule_info = self .program .rule_infos .get(rule_idx) - .ok_or(VmError::RuleInfoMissing { index: rule_index })? + .ok_or(VmError::RuleInfoMissing { + index: rule_index, + pc: self.pc, + available: self.program.rule_infos.len(), + })? .clone(); let is_function_rule = rule_info.function_info.is_some(); if !is_function_rule { - let (computed, cached_result) = &self.rule_cache[rule_idx]; + let (ref computed, ref cached_result) = + *self + .rule_cache + .get(rule_idx) + .ok_or(VmError::RuleIndexOutOfBounds { + index: rule_index, + pc: self.pc, + available: self.rule_cache.len(), + })?; if *computed { - self.registers[dest as usize] = cached_result.clone(); + self.set_register(dest, cached_result.clone())?; return Ok(()); } } @@ -288,19 +333,33 @@ impl RegoVM { if rule_info.definitions.is_empty() { let result = Value::Undefined; if !is_function_rule { - self.rule_cache[rule_idx] = (true, result.clone()); + let available = self.rule_cache.len(); + let entry = + self.rule_cache + .get_mut(rule_idx) + .ok_or(VmError::RuleIndexOutOfBounds { + index: rule_index, + pc: self.pc, + available, + })?; + *entry = (true, result.clone()); } - if self.registers.len() <= dest as usize { - self.registers.resize(dest as usize + 1, Value::Undefined); + let dest_index = usize::from(dest); + if self.registers.len() <= dest_index { + let new_len = + self.checked_add_one(dest_index, "register capacity for destination")?; + self.registers.resize(new_len, Value::Undefined); } - self.registers[dest as usize] = result; + self.set_register(dest, result)?; return Ok(()); } - let num_registers = rule_info.num_registers as usize; + let num_registers = usize::from(rule_info.num_registers); let num_retained_registers = match function_call_params { - Some(params) => params.arg_count() + 1, + Some(params) => { + self.checked_add_one(params.arg_count(), "retained function registers")? + } None => match rule_info.rule_type { RuleType::PartialSet | RuleType::PartialObject => 1, RuleType::Complete => 0, @@ -314,7 +373,7 @@ impl RegoVM { if let Some(params) = function_call_params { for &arg in params.arg_registers() { - register_window.push(self.registers[arg as usize].clone()); + register_window.push(self.get_register(arg)?.clone()); } } @@ -367,7 +426,7 @@ impl RegoVM { let initial_pc = self .prepare_rule_frame_initial_pc(&mut frame_data, &rule_info)? - .ok_or_else(|| VmError::Internal("Rule frame has no initial PC".into()))?; + .ok_or(VmError::RuleFrameMissingInitialPc { pc: self.pc })?; let frame = ExecutionFrame::new(initial_pc, FrameKind::Rule(frame_data)); self.execution_stack.push(frame); @@ -379,29 +438,30 @@ impl RegoVM { let current_ctx = self .call_rule_stack .last_mut() - .expect("Call stack underflow"); + .ok_or(VmError::CallRuleStackUnderflow { pc: self.pc })?; current_ctx.result_reg = result_reg; match current_ctx.rule_type { RuleType::Complete => { - self.registers[result_reg as usize] = Value::Undefined; + self.set_register(result_reg, Value::Undefined)?; } RuleType::PartialSet => { if current_ctx.current_definition_index == 0 && current_ctx.current_body_index == 0 { - self.registers[result_reg as usize] = Value::new_set(); + self.set_register(result_reg, Value::new_set())?; } } RuleType::PartialObject => { if current_ctx.current_definition_index == 0 && current_ctx.current_body_index == 0 { - self.registers[result_reg as usize] = Value::new_object(); + self.set_register(result_reg, Value::new_object())?; } } } Ok(()) } - pub(super) fn execute_rule_return(&mut self) -> Result<()> { + pub(super) const fn execute_rule_return(&mut self) -> Result<()> { + let _ = self; Ok(()) } @@ -427,7 +487,16 @@ impl RegoVM { } while frame_data.current_definition_index < frame_data.total_definitions { - let definition_bodies = &rule_info.definitions[frame_data.current_definition_index]; + let definition_bodies = match rule_info + .definitions + .get(frame_data.current_definition_index) + { + Some(bodies) => bodies, + None => { + frame_data.current_definition_index = frame_data.total_definitions; + break; + } + }; if frame_data.current_body_index < definition_bodies.len() { if let Some(ctx) = self.call_rule_stack.last_mut() { @@ -446,15 +515,29 @@ impl RegoVM { .and_then(|opt| *opt) { frame_data.phase = RuleFramePhase::ExecutingDestructuring; - return Ok(Some(destructuring_entry_point as usize)); - } else { - frame_data.phase = RuleFramePhase::ExecutingBody; - return Ok(Some( - definition_bodies[frame_data.current_body_index] as usize, - )); + let next_pc = + self.convert_pc(destructuring_entry_point, "destructuring entry point")?; + return Ok(Some(next_pc)); } + + if let Some(&body_entry_point) = + definition_bodies.get(frame_data.current_body_index) + { + frame_data.phase = RuleFramePhase::ExecutingBody; + let next_pc = self.convert_pc(body_entry_point, "rule body entry point")?; + return Ok(Some(next_pc)); + } + + self.increment_counter( + &mut frame_data.current_definition_index, + "rule definition index", + )?; + frame_data.current_body_index = 0; } else { - frame_data.current_definition_index += 1; + self.increment_counter( + &mut frame_data.current_definition_index, + "rule definition index", + )?; frame_data.current_body_index = 0; } } @@ -469,15 +552,24 @@ impl RegoVM { rule_info: &RuleInfo, ) -> Result> { frame_data.phase = RuleFramePhase::ExecutingBody; - let definition_bodies = &rule_info.definitions[frame_data.current_definition_index]; - if frame_data.current_body_index >= definition_bodies.len() { - frame_data.current_body_index += 1; - return self.rule_frame_schedule_segment(frame_data, rule_info); - } + let definition_bodies = match rule_info + .definitions + .get(frame_data.current_definition_index) + { + Some(bodies) => bodies, + None => { + frame_data.current_definition_index = frame_data.total_definitions; + return Ok(None); + } + }; - Ok(Some( - definition_bodies[frame_data.current_body_index] as usize, - )) + if let Some(&entry_point) = definition_bodies.get(frame_data.current_body_index) { + let next_pc = self.convert_pc(entry_point, "rule body entry point")?; + Ok(Some(next_pc)) + } else { + self.increment_counter(&mut frame_data.current_body_index, "rule body index")?; + self.rule_frame_schedule_segment(frame_data, rule_info) + } } fn rule_frame_after_failure( @@ -485,7 +577,7 @@ impl RegoVM { frame_data: &mut RuleFrameData, rule_info: &RuleInfo, ) -> Result> { - frame_data.current_body_index += 1; + self.increment_counter(&mut frame_data.current_body_index, "rule body index")?; self.rule_frame_schedule_segment(frame_data, rule_info) } @@ -499,16 +591,16 @@ impl RegoVM { if matches!(frame_data.rule_type, RuleType::Complete) || frame_data.is_function_rule { let current_result = self .registers - .get(frame_data.result_reg as usize) + .get(usize::from(frame_data.result_reg)) .cloned() .unwrap_or(Value::Undefined); if current_result != Value::Undefined { - if let Some(expected) = &frame_data.accumulated_result { + if let Some(ref expected) = frame_data.accumulated_result { if *expected != current_result { frame_data.rule_failed_due_to_inconsistency = true; if let Some(result_slot) = - self.registers.get_mut(frame_data.result_reg as usize) + self.registers.get_mut(usize::from(frame_data.result_reg)) { *result_slot = Value::Undefined; } @@ -525,7 +617,7 @@ impl RegoVM { { frame_data.current_body_index = definition_bodies.len(); } else { - frame_data.current_body_index += 1; + self.increment_counter(&mut frame_data.current_body_index, "rule body index")?; } self.rule_frame_schedule_segment(frame_data, rule_info) } @@ -546,12 +638,16 @@ impl RegoVM { .. } = frame_data; - let rule_idx = rule_index as usize; + let rule_idx = usize::from(rule_index); let rule_info = self .program .rule_infos .get(rule_idx) - .ok_or(VmError::RuleInfoMissing { index: rule_index })? + .ok_or(VmError::RuleInfoMissing { + index: rule_index, + pc: self.pc, + available: self.program.rule_infos.len(), + })? .clone(); let result_from_rule = if rule_failed_due_to_inconsistency { @@ -560,7 +656,7 @@ impl RegoVM { value } else { self.registers - .get(result_reg as usize) + .get(usize::from(result_reg)) .cloned() .unwrap_or(Value::Undefined) }; @@ -573,44 +669,104 @@ impl RegoVM { self.comprehension_stack = saved_comprehension_stack; let mut parent_registers = saved_registers; - if parent_registers.len() <= dest_reg as usize { - parent_registers.resize(dest_reg as usize + 1, Value::Undefined); + let dest_idx = usize::from(dest_reg); + if parent_registers.len() <= dest_idx { + let new_len = self.checked_add_one(dest_idx, "parent register capacity")?; + parent_registers.resize(new_len, Value::Undefined); } - parent_registers[dest_reg as usize] = result_from_rule.clone(); - if parent_registers[dest_reg as usize] == Value::Undefined - && !rule_failed_due_to_inconsistency { + let register_count = parent_registers.len(); + let slot = + parent_registers + .get_mut(dest_idx) + .ok_or(VmError::RegisterIndexOutOfBounds { + index: dest_reg, + pc: self.pc, + register_count, + })?; + *slot = result_from_rule.clone(); + } + + let needs_default = parent_registers + .get(dest_idx) + .is_some_and(|value| *value == Value::Undefined); + + if needs_default && !rule_failed_due_to_inconsistency { match rule_type { - RuleType::PartialSet => parent_registers[dest_reg as usize] = Value::new_set(), + RuleType::PartialSet => { + let register_count = parent_registers.len(); + let slot = parent_registers.get_mut(dest_idx).ok_or( + VmError::RegisterIndexOutOfBounds { + index: dest_reg, + pc: self.pc, + register_count, + }, + )?; + *slot = Value::new_set(); + } RuleType::PartialObject => { - parent_registers[dest_reg as usize] = Value::new_object() + let register_count = parent_registers.len(); + let slot = parent_registers.get_mut(dest_idx).ok_or( + VmError::RegisterIndexOutOfBounds { + index: dest_reg, + pc: self.pc, + register_count, + }, + )?; + *slot = Value::new_object(); } RuleType::Complete => { if let Some(default_literal_index) = rule_info.default_literal_index { - if let Some(default_value) = - self.program.literals.get(default_literal_index as usize) + if let Some(default_value) = self + .program + .literals + .get(usize::from(default_literal_index)) + .cloned() { - parent_registers[dest_reg as usize] = default_value.clone(); + let register_count = parent_registers.len(); + let slot = parent_registers.get_mut(dest_idx).ok_or( + VmError::RegisterIndexOutOfBounds { + index: dest_reg, + pc: self.pc, + register_count, + }, + )?; + *slot = default_value; } } } } } - let final_value = parent_registers[dest_reg as usize].clone(); + let register_count = parent_registers.len(); + let final_value = + parent_registers + .get(dest_idx) + .cloned() + .ok_or(VmError::RegisterIndexOutOfBounds { + index: dest_reg, + pc: self.pc, + register_count, + })?; if !is_function_rule { - self.rule_cache[rule_idx] = (true, final_value.clone()); + let available = self.rule_cache.len(); + let entry = self + .rule_cache + .get_mut(rule_idx) + .ok_or(VmError::RuleIndexOutOfBounds { + index: rule_index, + pc: self.pc, + available, + })?; + *entry = (true, final_value.clone()); } self.registers = parent_registers; if self.call_rule_stack.pop().is_none() { - return Err(VmError::Internal(alloc::format!( - "Call rule stack underflow during rule finalization | {}", - self.get_debug_state() - ))); + return Err(VmError::CallRuleStackUnderflow { pc: self.pc }); } self.pc = return_pc; @@ -641,11 +797,40 @@ impl RegoVM { } fn get_rule_info(&self, rule_index: u16) -> Result { - let idx = rule_index as usize; + let idx = usize::from(rule_index); self.program .rule_infos .get(idx) .cloned() - .ok_or(VmError::RuleInfoMissing { index: rule_index }) + .ok_or(VmError::RuleInfoMissing { + index: rule_index, + pc: self.pc, + available: self.program.rule_infos.len(), + }) + } + + pub(super) fn checked_add_one(&self, value: usize, context: &'static str) -> Result { + value + .checked_add(1) + .ok_or_else(|| VmError::ArithmeticError { + message: format!("{context} overflow"), + pc: self.pc, + }) + } + + pub(super) fn increment_counter( + &self, + counter: &mut usize, + context: &'static str, + ) -> Result<()> { + *counter = self.checked_add_one(*counter, context)?; + Ok(()) + } + + pub(super) fn convert_pc(&self, value: u32, context: &'static str) -> Result { + usize::try_from(value).map_err(|_| VmError::ArithmeticError { + message: format!("{context} exceeds addressable range"), + pc: self.pc, + }) } } diff --git a/src/rvm/vm/state.rs b/src/rvm/vm/state.rs index 3001448..1c7cde9 100644 --- a/src/rvm/vm/state.rs +++ b/src/rvm/vm/state.rs @@ -2,7 +2,6 @@ // Licensed under the MIT License. use crate::value::Value; -use alloc::string::String; use alloc::vec::Vec; use super::errors::{Result, VmError}; @@ -65,46 +64,30 @@ impl RegoVM { pub(super) fn validate_vm_state(&self) -> Result<()> { // Check register bounds if self.registers.len() < self.base_register_count { - return Err(VmError::Internal(alloc::format!( - "Register count {} < base count {}", - self.registers.len(), - self.base_register_count - ))); + return Err(VmError::RegisterCountBelowBase { + register_count: self.registers.len(), + base_count: self.base_register_count, + pc: self.pc, + }); } // Check PC bounds if self.pc >= self.program.instructions.len() { - return Err(VmError::Internal(alloc::format!( - "PC {} >= instruction count {}", - self.pc, - self.program.instructions.len() - ))); + return Err(VmError::ProgramCounterOutOfBounds { + pc: self.pc, + instruction_count: self.program.instructions.len(), + }); } // Check rule cache bounds if self.rule_cache.len() != self.program.rule_infos.len() { - return Err(VmError::Internal(alloc::format!( - "Rule cache size {} != rule info count {}", - self.rule_cache.len(), - self.program.rule_infos.len() - ))); + return Err(VmError::RuleCacheSizeMismatch { + cache_size: self.rule_cache.len(), + rule_info_count: self.program.rule_infos.len(), + pc: self.pc, + }); } Ok(()) } - - /// Get current VM state for debugging - pub(super) fn get_debug_state(&self) -> String { - alloc::format!( - "VM State: PC={}, registers={}, executed={}/{}, stacks: loop={}, call={}, register={}, comprehension={}", - self.pc, - self.registers.len(), - self.executed_instructions, - self.max_instructions, - self.loop_stack.len(), - self.call_rule_stack.len(), - self.register_stack.len(), - self.comprehension_stack.len() - ) - } } diff --git a/src/rvm/vm/virtual_data.rs b/src/rvm/vm/virtual_data.rs index ba289c8..bb66ef9 100644 --- a/src/rvm/vm/virtual_data.rs +++ b/src/rvm/vm/virtual_data.rs @@ -1,18 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::indexing_slicing, - clippy::arithmetic_side_effects, - clippy::unwrap_used, - clippy::unused_self, - clippy::as_conversions, - clippy::pattern_type_mismatch -)] // virtual data paths index directly for speed; unwraps assert invariants - use crate::rvm::instructions::LiteralOrRegister; use crate::value::Value; use alloc::vec::Vec; +use core::convert::TryFrom as _; use super::errors::{Result, VmError}; use super::machine::RegoVM; @@ -25,17 +17,7 @@ impl RegoVM { ) -> Result { let mut root_path = Vec::new(); for component in path_components { - let key_value = match component { - LiteralOrRegister::Literal(idx) => self - .program - .literals - .get(*idx as usize) - .ok_or(VmError::LiteralIndexOutOfBounds { - index: *idx as usize, - })? - .clone(), - LiteralOrRegister::Register(reg) => self.registers[*reg as usize].clone(), - }; + let key_value = self.literal_or_register_value(component)?; root_path.push(key_value); } @@ -54,33 +36,32 @@ impl RegoVM { Ok(result_subobject) } - fn set_nested_value(&self, target: &mut Value, path: &[Value], value: Value) -> Result<()> { + fn set_nested_value(target: &mut Value, path: &[Value], value: Value) -> Result<()> { Self::set_nested_value_static(target, path, value) } fn set_nested_value_static(target: &mut Value, path: &[Value], value: Value) -> Result<()> { - if path.is_empty() { + let Some((head, tail)) = path.split_first() else { *target = value; return Ok(()); - } + }; if *target == Value::Undefined { *target = Value::new_object(); } - if let Value::Object(ref mut map) = target { - let key = &path[0]; - - if !map.contains_key(key) { - crate::Rc::make_mut(map).insert(key.clone(), Value::Undefined); + if let Value::Object(ref mut map) = *target { + if !map.contains_key(head) { + crate::Rc::make_mut(map).insert(head.clone(), Value::Undefined); } - if let Some(next_target) = crate::Rc::make_mut(map).get_mut(key) { - Self::set_nested_value_static(next_target, &path[1..], value)?; + if let Some(next_target) = crate::Rc::make_mut(map).get_mut(head) { + Self::set_nested_value_static(next_target, tail, value)?; } } else { return Err(VmError::InvalidRuleTreeEntry { value: target.clone(), + pc: 0, }); } @@ -108,8 +89,8 @@ impl RegoVM { root_path: &[Value], relative_path: &[Value], ) -> Result<()> { - match rule_tree_node { - Value::Number(rule_idx) => { + match *rule_tree_node { + Value::Number(ref rule_idx) => { if let Some(rule_index) = rule_idx.as_u64() { let mut full_cache_path = root_path.to_vec(); full_cache_path.extend_from_slice(relative_path); @@ -119,7 +100,7 @@ impl RegoVM { let mut path_exists = true; for path_component in &full_cache_path { - if let Value::Object(ref map) = cache_lookup { + if let Value::Object(ref map) = *cache_lookup { if let Some(next_value) = map.get(path_component) { cache_lookup = next_value; } else { @@ -133,7 +114,7 @@ impl RegoVM { } if path_exists { - if let Value::Object(ref map) = cache_lookup { + if let Value::Object(ref map) = *cache_lookup { map.get(&Value::Undefined).cloned() } else { None @@ -144,13 +125,33 @@ impl RegoVM { }; let rule_result = if let Some(cached) = cached_result { - self.cache_hits += 1; + self.cache_hits = + self.checked_add_one(self.cache_hits, "cache hits counter")?; cached } else { - let temp_reg = self.registers.len() as u8; + let temp_reg = u8::try_from(self.registers.len()).map_err(|_| { + VmError::RegisterIndexOutOfBounds { + index: u8::MAX, + pc: self.pc, + register_count: self.registers.len(), + } + })?; self.registers.push(Value::Undefined); - self.execute_call_rule_common(temp_reg, rule_index as u16, None)?; - let result = self.registers.pop().unwrap(); + let rule_index_u16 = + u16::try_from(rule_index).map_err(|_| VmError::InvalidRuleIndex { + rule_index: Value::Number(rule_idx.clone()), + pc: self.pc, + })?; + self.execute_call_rule_common(temp_reg, rule_index_u16, None)?; + let register_count = self.registers.len(); + let result = + self.registers + .pop() + .ok_or(VmError::RegisterIndexOutOfBounds { + index: temp_reg, + pc: self.pc, + register_count, + })?; let mut cache_path = full_cache_path.clone(); cache_path.push(Value::Undefined); @@ -163,14 +164,15 @@ impl RegoVM { result }; - self.set_nested_value(result_subobject, relative_path, rule_result)?; + Self::set_nested_value(result_subobject, relative_path, rule_result)?; } else { return Err(VmError::InvalidRuleIndex { rule_index: Value::Number(rule_idx.clone()), + pc: self.pc, }); } } - Value::Object(obj) => { + Value::Object(ref obj) => { for (key, value) in obj.iter() { let mut new_relative_path = relative_path.to_vec(); new_relative_path.push(key.clone()); @@ -194,6 +196,12 @@ impl RegoVM { .get_virtual_data_document_lookup_params(params_index) .ok_or(VmError::InvalidVirtualDataDocumentLookupParams { index: params_index, + pc: self.pc, + available: self + .program + .instruction_data + .virtual_data_document_lookup_params + .len(), })? .clone(); @@ -201,60 +209,43 @@ impl RegoVM { let mut components_consumed = 0; for (i, component) in params.path_components.iter().enumerate() { - let key_value = match component { - LiteralOrRegister::Literal(idx) => self - .program - .literals - .get(*idx as usize) - .ok_or(VmError::LiteralIndexOutOfBounds { - index: *idx as usize, - })? - .clone(), - LiteralOrRegister::Register(reg) => self.registers[*reg as usize].clone(), - }; + let key_value = self.literal_or_register_value(component)?; current_node = ¤t_node[&key_value]; - components_consumed = i + 1; + components_consumed = self.checked_add_one(i, "path components traversed")?; - match current_node { + match *current_node { Value::Undefined | Value::Number(_) => break, _ => {} } } - match current_node { - Value::Number(rule_index_value) => { + match *current_node { + Value::Number(ref rule_index_value) => { if let Some(rule_index) = rule_index_value.as_u64() { - let rule_index = rule_index as u16; + let rule_index = + u16::try_from(rule_index).map_err(|_| VmError::InvalidRuleIndex { + rule_index: Value::Number(rule_index_value.clone()), + pc: self.pc, + })?; self.execute_call_rule_common(params.dest, rule_index, None)?; if components_consumed < params.path_components.len() { - let mut rule_result = self.registers[params.dest as usize].clone(); + let mut rule_result = self.get_register(params.dest)?.clone(); - for component in ¶ms.path_components[components_consumed..] { - let key_value = match component { - LiteralOrRegister::Literal(idx) => self - .program - .literals - .get(*idx as usize) - .ok_or(VmError::LiteralIndexOutOfBounds { - index: *idx as usize, - })? - .clone(), - LiteralOrRegister::Register(reg) => { - self.registers[*reg as usize].clone() - } - }; + for component in params.path_components.iter().skip(components_consumed) { + let key_value = self.literal_or_register_value(component)?; rule_result = rule_result[&key_value].clone(); } - self.registers[params.dest as usize] = rule_result; + self.set_register(params.dest, rule_result)?; } } else { return Err(VmError::InvalidRuleIndex { rule_index: Value::Number(rule_index_value.clone()), + pc: self.pc, }); } } @@ -264,22 +255,12 @@ impl RegoVM { let mut result = self.data.clone(); for component in ¶ms.path_components { - let key_value = match component { - LiteralOrRegister::Literal(idx) => self - .program - .literals - .get(*idx as usize) - .ok_or(VmError::LiteralIndexOutOfBounds { - index: *idx as usize, - })? - .clone(), - LiteralOrRegister::Register(reg) => self.registers[*reg as usize].clone(), - }; + let key_value = self.literal_or_register_value(component)?; result = result[&key_value].clone(); } - self.registers[params.dest as usize] = result; + self.set_register(params.dest, result)?; } Value::Object(_) => { let rule_tree_subobject = current_node.clone(); @@ -288,15 +269,33 @@ impl RegoVM { ¶ms.path_components, &rule_tree_subobject, )?; - self.registers[params.dest as usize] = result; + self.set_register(params.dest, result)?; } _ => { return Err(VmError::InvalidRuleTreeEntry { value: current_node.clone(), + pc: self.pc, }); } } Ok(()) } + + fn literal_or_register_value(&self, source: &LiteralOrRegister) -> Result { + let value = match *source { + LiteralOrRegister::Literal(ref idx) => self + .program + .literals + .get(usize::from(*idx)) + .ok_or(VmError::LiteralIndexOutOfBounds { + index: *idx, + pc: self.pc, + })? + .clone(), + LiteralOrRegister::Register(ref reg) => self.get_register(*reg)?.clone(), + }; + + Ok(value) + } }