From 28891ef8838eaf789d512cfc7e46922f6bf852d6 Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi <35780660+anakrish@users.noreply.github.com> Date: Tue, 30 Dec 2025 18:20:07 -0600 Subject: [PATCH] chore: Harden instructions and program (#535) Also enforce sane limits in program Signed-off-by: Anand Krishnamoorthi --- src/compiler/hoist.rs | 2 +- src/languages/rego/compiler/program.rs | 9 +- src/rvm/instructions/display.rs | 234 ++++++------ src/rvm/instructions/mod.rs | 25 +- src/rvm/instructions/params.rs | 114 +++--- src/rvm/program/core.rs | 122 ++++++- src/rvm/program/listing.rs | 225 ++++++------ src/rvm/program/recompile.rs | 4 +- src/rvm/program/rule_tree.rs | 32 +- src/rvm/program/serialization/binary.rs | 451 ++++++++++-------------- src/rvm/program/serialization/json.rs | 16 +- src/rvm/program/serialization/mod.rs | 6 +- src/rvm/program/serialization/value.rs | 34 +- src/rvm/program/types.rs | 18 +- src/rvm/tests/vm.rs | 4 +- src/rvm/vm/execution.rs | 4 +- src/rvm/vm/machine.rs | 4 +- 17 files changed, 670 insertions(+), 634 deletions(-) diff --git a/src/compiler/hoist.rs b/src/compiler/hoist.rs index 67eeafd..79eba4b 100644 --- a/src/compiler/hoist.rs +++ b/src/compiler/hoist.rs @@ -284,7 +284,7 @@ impl HoistedLoopsLookup { } #[cfg(debug_assertions)] - pub fn module_len(&self) -> usize { + pub const fn module_len(&self) -> usize { self.statement_loops.module_len() } } diff --git a/src/languages/rego/compiler/program.rs b/src/languages/rego/compiler/program.rs index 5e67182..350b519 100644 --- a/src/languages/rego/compiler/program.rs +++ b/src/languages/rego/compiler/program.rs @@ -18,6 +18,7 @@ use alloc::format; use alloc::string::{String, ToString}; use alloc::vec; use alloc::vec::Vec; +use anyhow::anyhow; impl<'a> Compiler<'a> { pub(super) fn emit_return(&mut self, result_reg: super::Register) { @@ -38,8 +39,8 @@ impl<'a> Compiler<'a> { self.program.main_entry_point = 0; self.program.max_rule_window_size = - self.rule_num_registers.iter().cloned().max().unwrap_or(0) as usize; - self.program.dispatch_window_size = self.register_counter as usize; + self.rule_num_registers.iter().cloned().max().unwrap_or(0); + self.program.dispatch_window_size = self.register_counter; let mut rule_infos_map = BTreeMap::new(); @@ -140,6 +141,10 @@ impl<'a> Compiler<'a> { .map_err(CompilerError::from)?; } + self.program + .validate_limits() + .map_err(|e| CompilerError::from(anyhow!(e)))?; + Ok(self.program) } diff --git a/src/rvm/instructions/display.rs b/src/rvm/instructions/display.rs index f025775..b681cba 100644 --- a/src/rvm/instructions/display.rs +++ b/src/rvm/instructions/display.rs @@ -1,13 +1,8 @@ -#![allow( - clippy::option_if_let_else, - clippy::unused_trait_names, - clippy::pattern_type_mismatch -)] // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. use alloc::format; -use alloc::string::{String, ToString}; +use alloc::string::String; use alloc::vec::Vec; use super::{Instruction, InstructionData, LiteralOrRegister}; @@ -15,129 +10,136 @@ use super::{Instruction, InstructionData, LiteralOrRegister}; impl Instruction { /// Get detailed display string with parameter resolution for debugging pub fn display_with_params(&self, instruction_data: &InstructionData) -> String { - match self { + match *self { Instruction::LoopStart { params_index } => { - if let Some(params) = instruction_data.get_loop_params(*params_index) { - format!( - "LOOP_START {:?} R({}) R({}) R({}) R({}) {} {}", - params.mode, - params.collection, - params.key_reg, - params.value_reg, - params.result_reg, - params.body_start, - params.loop_end - ) - } else { - format!("LOOP_START P({}) [INVALID INDEX]", params_index) - } - } - Instruction::BuiltinCall { params_index } => { - if let Some(params) = instruction_data.get_builtin_call_params(*params_index) { - let args_str = params - .arg_registers() - .iter() - .map(|&r| format!("R({})", r)) - .collect::>() - .join(" "); - format!( - "BUILTIN_CALL R({}) B({}) [{}]", - params.dest, params.builtin_index, args_str - ) - } else { - format!("BUILTIN_CALL P({}) [INVALID INDEX]", params_index) - } + instruction_data.get_loop_params(params_index).map_or_else( + || format!("LOOP_START P({}) [INVALID INDEX]", params_index), + |params| { + format!( + "LOOP_START {:?} R({}) R({}) R({}) R({}) {} {}", + params.mode, + params.collection, + params.key_reg, + params.value_reg, + params.result_reg, + params.body_start, + params.loop_end + ) + }, + ) } + Instruction::BuiltinCall { params_index } => instruction_data + .get_builtin_call_params(params_index) + .map_or_else( + || format!("BUILTIN_CALL P({}) [INVALID INDEX]", params_index), + |params| { + let args_str = params + .arg_registers() + .iter() + .map(|&r| format!("R({})", r)) + .collect::>() + .join(" "); + format!( + "BUILTIN_CALL R({}) B({}) [{}]", + params.dest, params.builtin_index, args_str + ) + }, + ), Instruction::HostAwait { dest, arg, id } => { format!("HOST_AWAIT R({}) R({}) R({})", dest, arg, id) } - Instruction::FunctionCall { params_index } => { - if let Some(params) = instruction_data.get_function_call_params(*params_index) { - let args_str = params - .arg_registers() - .iter() - .map(|&r| format!("R({})", r)) - .collect::>() - .join(" "); - format!( - "FUNCTION_CALL R({}) RULE({}) [{}]", - params.dest, params.func_rule_index, args_str - ) - } else { - format!("FUNCTION_CALL P({}) [INVALID INDEX]", params_index) - } - } + Instruction::FunctionCall { params_index } => instruction_data + .get_function_call_params(params_index) + .map_or_else( + || format!("FUNCTION_CALL P({}) [INVALID INDEX]", params_index), + |params| { + let args_str = params + .arg_registers() + .iter() + .map(|&r| format!("R({})", r)) + .collect::>() + .join(" "); + format!( + "FUNCTION_CALL R({}) RULE({}) [{}]", + params.dest, params.func_rule_index, args_str + ) + }, + ), Instruction::ObjectCreate { params_index } => { - if let Some(params) = instruction_data.get_object_create_params(*params_index) { - let mut field_parts = Vec::new(); + instruction_data + .get_object_create_params(params_index) + .map_or_else( + || format!("OBJECT_CREATE P({}) [INVALID INDEX]", params_index), + |params| { + let mut field_parts = Vec::new(); - // Add literal key fields - for &(literal_idx, value_reg) in params.literal_key_field_pairs() { - field_parts.push(format!("L({}):R({})", literal_idx, value_reg)); - } + // Add literal key fields + for &(literal_idx, value_reg) in params.literal_key_field_pairs() { + field_parts.push(format!("L({}):R({})", literal_idx, value_reg)); + } - // Add non-literal key fields - for &(key_reg, value_reg) in params.field_pairs() { - field_parts.push(format!("R({}):R({})", key_reg, value_reg)); - } + // Add non-literal key fields + for &(key_reg, value_reg) in params.field_pairs() { + field_parts.push(format!("R({}):R({})", key_reg, value_reg)); + } - let fields_str = field_parts.join(" "); - format!( - "OBJECT_CREATE R({}) L({}) [{}]", - params.dest, params.template_literal_idx, fields_str + let fields_str = field_parts.join(" "); + format!( + "OBJECT_CREATE R({}) L({}) [{}]", + params.dest, params.template_literal_idx, fields_str + ) + }, ) - } else { - format!("OBJECT_CREATE P({}) [INVALID INDEX]", params_index) - } } - Instruction::VirtualDataDocumentLookup { params_index } => { - if let Some(params) = - instruction_data.get_virtual_data_document_lookup_params(*params_index) - { - let components_str = params - .path_components - .iter() - .map(|comp| match comp { - LiteralOrRegister::Literal(idx) => format!("L({})", idx), - LiteralOrRegister::Register(reg) => format!("R({})", reg), - }) - .collect::>() - .join("."); - format!( - "VIRTUAL_DATA_DOCUMENT_LOOKUP R({}) [data.{}]", - params.dest, components_str - ) - } else { - format!( - "VIRTUAL_DATA_DOCUMENT_LOOKUP P({}) [INVALID INDEX]", - params_index - ) - } - } - Instruction::ComprehensionBegin { params_index } => { - if let Some(params) = instruction_data.get_comprehension_begin_params(*params_index) - { - format!( - "COMPREHENSION_BEGIN {:?} R({}) R({}) R({}) {} {}", - params.mode, - params.collection_reg, - params.key_reg, - params.value_reg, - params.body_start, - params.comprehension_end - ) - } else { - format!("COMPREHENSION_BEGIN P({}) [INVALID INDEX]", params_index) - } - } - _ => self.to_string(), + Instruction::VirtualDataDocumentLookup { params_index } => instruction_data + .get_virtual_data_document_lookup_params(params_index) + .map_or_else( + || { + format!( + "VIRTUAL_DATA_DOCUMENT_LOOKUP P({}) [INVALID INDEX]", + params_index + ) + }, + |params| { + let components_str = params + .path_components + .iter() + .map(|comp| match *comp { + LiteralOrRegister::Literal(idx) => format!("L({})", idx), + LiteralOrRegister::Register(reg) => format!("R({})", reg), + }) + .collect::>() + .join("."); + format!( + "VIRTUAL_DATA_DOCUMENT_LOOKUP R({}) [data.{}]", + params.dest, components_str + ) + }, + ), + Instruction::ComprehensionBegin { params_index } => instruction_data + .get_comprehension_begin_params(params_index) + .map_or_else( + || format!("COMPREHENSION_BEGIN P({}) [INVALID INDEX]", params_index), + |params| { + format!( + "COMPREHENSION_BEGIN {:?} R({}) R({}) R({}) {} {}", + params.mode, + params.collection_reg, + params.key_reg, + params.value_reg, + params.body_start, + params.comprehension_end + ) + }, + ), + _ => format!("{}", self), } } } impl core::fmt::Display for Instruction { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - let text = match self { + let text = match *self { Instruction::Load { dest, literal_idx } => { format!("LOAD R({}) L({})", dest, literal_idx) } @@ -274,10 +276,10 @@ impl core::fmt::Display for Instruction { Instruction::ComprehensionBegin { params_index } => { format!("COMPREHENSION_BEGIN P({})", params_index) } - Instruction::ComprehensionYield { value_reg, key_reg } => match key_reg { - Some(k) => format!("COMPREHENSION_YIELD R({}) R({})", k, value_reg), - None => format!("COMPREHENSION_YIELD R({})", value_reg), - }, + Instruction::ComprehensionYield { value_reg, key_reg } => key_reg.as_ref().map_or_else( + || format!("COMPREHENSION_YIELD R({})", value_reg), + |k| format!("COMPREHENSION_YIELD R({}) R({})", k, value_reg), + ), Instruction::ComprehensionEnd {} => String::from("COMPREHENSION_END"), }; write!(f, "{}", text) diff --git a/src/rvm/instructions/mod.rs b/src/rvm/instructions/mod.rs index 4829038..d829038 100644 --- a/src/rvm/instructions/mod.rs +++ b/src/rvm/instructions/mod.rs @@ -1,4 +1,3 @@ -#![allow(clippy::missing_const_for_fn)] // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. @@ -17,7 +16,7 @@ use serde::{Deserialize, Serialize}; /// RVM Instructions - simplified enum-based design #[repr(C)] -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, Serialize, Deserialize)] pub enum Instruction { /// Load literal value from literal table into register Load { @@ -320,47 +319,47 @@ pub enum Instruction { impl Instruction { /// Create a new LoopStart instruction with parameter table index - pub fn loop_start(params_index: u16) -> Self { + pub const fn loop_start(params_index: u16) -> Self { Self::LoopStart { params_index } } /// Create a new BuiltinCall instruction with parameter table index - pub fn builtin_call(params_index: u16) -> Self { + pub const fn builtin_call(params_index: u16) -> Self { Self::BuiltinCall { params_index } } /// Create a new HostAwait instruction - pub fn host_await(dest: u8, arg: u8, id: u8) -> Self { + pub const fn host_await(dest: u8, arg: u8, id: u8) -> Self { Self::HostAwait { dest, arg, id } } /// Create a new FunctionCall instruction with parameter table index - pub fn function_call(params_index: u16) -> Self { + pub const fn function_call(params_index: u16) -> Self { Self::FunctionCall { params_index } } /// Create a new ObjectCreate instruction with parameter table index - pub fn object_create(params_index: u16) -> Self { + pub const fn object_create(params_index: u16) -> Self { Self::ObjectCreate { params_index } } /// Create a new ArrayCreate instruction with parameter table index - pub fn array_create(params_index: u16) -> Self { + pub const fn array_create(params_index: u16) -> Self { Self::ArrayCreate { params_index } } /// Create a new SetCreate instruction with parameter table index - pub fn set_create(params_index: u16) -> Self { + pub const fn set_create(params_index: u16) -> Self { Self::SetCreate { params_index } } /// Create a new ComprehensionBegin instruction with parameter table index - pub fn comprehension_begin(params_index: u16) -> Self { + pub const fn comprehension_begin(params_index: u16) -> Self { Self::ComprehensionBegin { params_index } } /// Create a new ComprehensionYield instruction - pub fn comprehension_yield(value_reg: u8) -> Self { + pub const fn comprehension_yield(value_reg: u8) -> Self { Self::ComprehensionYield { value_reg, key_reg: None, @@ -368,7 +367,7 @@ impl Instruction { } /// Create a new ComprehensionYield instruction for object comprehensions - pub fn comprehension_yield_object(key_reg: u8, value_reg: u8) -> Self { + pub const fn comprehension_yield_object(key_reg: u8, value_reg: u8) -> Self { Self::ComprehensionYield { value_reg, key_reg: Some(key_reg), @@ -376,7 +375,7 @@ impl Instruction { } /// Create a new ComprehensionEnd instruction - pub fn comprehension_end() -> Self { + pub const fn comprehension_end() -> Self { Self::ComprehensionEnd {} } } diff --git a/src/rvm/instructions/params.rs b/src/rvm/instructions/params.rs index b14d44c..ab2ed3f 100644 --- a/src/rvm/instructions/params.rs +++ b/src/rvm/instructions/params.rs @@ -1,13 +1,5 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::indexing_slicing, - clippy::arithmetic_side_effects, - clippy::missing_const_for_fn, - clippy::as_conversions, - clippy::pattern_type_mismatch -)] - use alloc::vec::Vec; use serde::{Deserialize, Serialize}; @@ -50,12 +42,13 @@ pub struct BuiltinCallParams { impl BuiltinCallParams { /// Get the number of arguments actually used pub fn arg_count(&self) -> usize { - self.num_args as usize + usize::from(self.num_args) } /// Get argument register numbers as a slice pub fn arg_registers(&self) -> &[u8] { - &self.args[..self.num_args as usize] + let count = usize::from(self.num_args).min(self.args.len()); + self.args.get(..count).unwrap_or(&[]) } } @@ -76,12 +69,13 @@ pub struct FunctionCallParams { impl FunctionCallParams { /// Get the number of arguments actually used pub fn arg_count(&self) -> usize { - self.num_args as usize + usize::from(self.num_args) } /// Get argument register numbers as a slice pub fn arg_registers(&self) -> &[u8] { - &self.args[..self.num_args as usize] + let count = usize::from(self.num_args).min(self.args.len()); + self.args.get(..count).unwrap_or(&[]) } } @@ -102,8 +96,10 @@ pub struct ObjectCreateParams { impl ObjectCreateParams { /// Get the total number of fields - pub fn field_count(&self) -> usize { - self.literal_key_fields.len() + self.fields.len() + pub const fn field_count(&self) -> usize { + self.literal_key_fields + .len() + .saturating_add(self.fields.len()) } /// Get literal key field pairs as a slice @@ -129,7 +125,7 @@ pub struct ArrayCreateParams { impl ArrayCreateParams { /// Get the number of elements - pub fn element_count(&self) -> usize { + pub const fn element_count(&self) -> usize { self.elements.len() } @@ -151,7 +147,7 @@ pub struct SetCreateParams { impl SetCreateParams { /// Get the number of elements - pub fn element_count(&self) -> usize { + pub const fn element_count(&self) -> usize { self.elements.len() } @@ -175,7 +171,7 @@ pub struct VirtualDataDocumentLookupParams { impl VirtualDataDocumentLookupParams { /// Get the number of path components - pub fn component_count(&self) -> usize { + pub const fn component_count(&self) -> usize { self.path_components.len() } @@ -190,8 +186,8 @@ impl VirtualDataDocumentLookupParams { pub fn literal_indices(&self) -> Vec { self.path_components .iter() - .filter_map(|c| match c { - LiteralOrRegister::Literal(idx) => Some(*idx), + .filter_map(|c| match *c { + LiteralOrRegister::Literal(idx) => Some(idx), _ => None, }) .collect() @@ -201,8 +197,8 @@ impl VirtualDataDocumentLookupParams { pub fn register_numbers(&self) -> Vec { self.path_components .iter() - .filter_map(|c| match c { - LiteralOrRegister::Register(reg) => Some(*reg), + .filter_map(|c| match *c { + LiteralOrRegister::Register(reg) => Some(reg), _ => None, }) .collect() @@ -223,7 +219,7 @@ pub struct ChainedIndexParams { impl ChainedIndexParams { /// Get the number of path components - pub fn component_count(&self) -> usize { + pub const fn component_count(&self) -> usize { self.path_components.len() } @@ -238,8 +234,8 @@ impl ChainedIndexParams { pub fn literal_indices(&self) -> Vec { self.path_components .iter() - .filter_map(|c| match c { - LiteralOrRegister::Literal(idx) => Some(*idx), + .filter_map(|c| match *c { + LiteralOrRegister::Literal(idx) => Some(idx), _ => None, }) .collect() @@ -249,8 +245,8 @@ impl ChainedIndexParams { pub fn register_numbers(&self) -> Vec { self.path_components .iter() - .filter_map(|c| match c { - LiteralOrRegister::Register(reg) => Some(*reg), + .filter_map(|c| match *c { + LiteralOrRegister::Register(reg) => Some(reg), _ => None, }) .collect() @@ -302,8 +298,13 @@ pub struct InstructionData { } impl InstructionData { + fn ensure_u16_index(len: usize) -> u16 { + debug_assert!(len <= usize::from(u16::MAX)); + u16::try_from(len).unwrap_or(u16::MAX) + } + /// Create a new empty instruction data container - pub fn new() -> Self { + pub const fn new() -> Self { Self { loop_params: Vec::new(), builtin_call_params: Vec::new(), @@ -319,74 +320,74 @@ impl InstructionData { /// Add loop parameters and return the index pub fn add_loop_params(&mut self, params: LoopStartParams) -> u16 { - let index = self.loop_params.len(); + let index = Self::ensure_u16_index(self.loop_params.len()); self.loop_params.push(params); - index as u16 + index } /// Add builtin call parameters and return the index pub fn add_builtin_call_params(&mut self, params: BuiltinCallParams) -> u16 { - let index = self.builtin_call_params.len(); + let index = Self::ensure_u16_index(self.builtin_call_params.len()); self.builtin_call_params.push(params); - index as u16 + index } /// Add function call parameters and return the index pub fn add_function_call_params(&mut self, params: FunctionCallParams) -> u16 { - let index = self.function_call_params.len(); + let index = Self::ensure_u16_index(self.function_call_params.len()); self.function_call_params.push(params); - index as u16 + index } /// Add object create parameters and return the index pub fn add_object_create_params(&mut self, params: ObjectCreateParams) -> u16 { - let index = self.object_create_params.len(); + let index = Self::ensure_u16_index(self.object_create_params.len()); self.object_create_params.push(params); - index as u16 + index } /// Add array create parameters and return the index pub fn add_array_create_params(&mut self, params: ArrayCreateParams) -> u16 { - let index = self.array_create_params.len(); + let index = Self::ensure_u16_index(self.array_create_params.len()); self.array_create_params.push(params); - index as u16 + index } /// Add set create parameters and return the index pub fn add_set_create_params(&mut self, params: SetCreateParams) -> u16 { - let index = self.set_create_params.len(); + let index = Self::ensure_u16_index(self.set_create_params.len()); self.set_create_params.push(params); - index as u16 + index } /// Get loop parameters by index pub fn get_loop_params(&self, index: u16) -> Option<&LoopStartParams> { - self.loop_params.get(index as usize) + self.loop_params.get(usize::from(index)) } /// Get builtin call parameters by index pub fn get_builtin_call_params(&self, index: u16) -> Option<&BuiltinCallParams> { - self.builtin_call_params.get(index as usize) + self.builtin_call_params.get(usize::from(index)) } /// Get function call parameters by index pub fn get_function_call_params(&self, index: u16) -> Option<&FunctionCallParams> { - self.function_call_params.get(index as usize) + self.function_call_params.get(usize::from(index)) } /// Get object create parameters by index pub fn get_object_create_params(&self, index: u16) -> Option<&ObjectCreateParams> { - self.object_create_params.get(index as usize) + self.object_create_params.get(usize::from(index)) } /// Get array create parameters by index pub fn get_array_create_params(&self, index: u16) -> Option<&ArrayCreateParams> { - self.array_create_params.get(index as usize) + self.array_create_params.get(usize::from(index)) } /// Get set create parameters by index pub fn get_set_create_params(&self, index: u16) -> Option<&SetCreateParams> { - self.set_create_params.get(index as usize) + self.set_create_params.get(usize::from(index)) } /// Add virtual data document lookup parameters and return the index @@ -394,9 +395,9 @@ impl InstructionData { &mut self, params: VirtualDataDocumentLookupParams, ) -> u16 { - let index = self.virtual_data_document_lookup_params.len(); + let index = Self::ensure_u16_index(self.virtual_data_document_lookup_params.len()); self.virtual_data_document_lookup_params.push(params); - index as u16 + index } /// Get virtual data document lookup parameters by index @@ -404,36 +405,37 @@ impl InstructionData { &self, index: u16, ) -> Option<&VirtualDataDocumentLookupParams> { - self.virtual_data_document_lookup_params.get(index as usize) + self.virtual_data_document_lookup_params + .get(usize::from(index)) } /// Add chained index parameters and return the index pub fn add_chained_index_params(&mut self, params: ChainedIndexParams) -> u16 { - let index = self.chained_index_params.len(); + let index = Self::ensure_u16_index(self.chained_index_params.len()); self.chained_index_params.push(params); - index as u16 + index } /// Get chained index parameters by index pub fn get_chained_index_params(&self, index: u16) -> Option<&ChainedIndexParams> { - self.chained_index_params.get(index as usize) + self.chained_index_params.get(usize::from(index)) } /// Get mutable reference to loop parameters by index pub fn get_loop_params_mut(&mut self, index: u16) -> Option<&mut LoopStartParams> { - self.loop_params.get_mut(index as usize) + self.loop_params.get_mut(usize::from(index)) } /// Add comprehension begin parameters and return the index pub fn add_comprehension_begin_params(&mut self, params: ComprehensionBeginParams) -> u16 { - let index = self.comprehension_begin_params.len(); + let index = Self::ensure_u16_index(self.comprehension_begin_params.len()); self.comprehension_begin_params.push(params); - index as u16 + index } /// Get comprehension begin parameters by index pub fn get_comprehension_begin_params(&self, index: u16) -> Option<&ComprehensionBeginParams> { - self.comprehension_begin_params.get(index as usize) + self.comprehension_begin_params.get(usize::from(index)) } /// Get mutable reference to comprehension begin parameters by index @@ -441,7 +443,7 @@ impl InstructionData { &mut self, index: u16, ) -> Option<&mut ComprehensionBeginParams> { - self.comprehension_begin_params.get_mut(index as usize) + self.comprehension_begin_params.get_mut(usize::from(index)) } } diff --git a/src/rvm/program/core.rs b/src/rvm/program/core.rs index c00a120e..dccc1dd 100644 --- a/src/rvm/program/core.rs +++ b/src/rvm/program/core.rs @@ -1,12 +1,8 @@ -#![allow( - clippy::missing_const_for_fn, - clippy::as_conversions, - clippy::unused_trait_names -)] // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use alloc::string::{String, ToString}; +use alloc::format; +use alloc::string::{String, ToString as _}; use alloc::vec::Vec; use anyhow::Result as AnyResult; use indexmap::IndexMap; @@ -49,13 +45,13 @@ pub struct Program { pub instruction_spans: Vec>, /// Main program entry point - pub main_entry_point: usize, + pub main_entry_point: u32, /// Maximum register window size observed across all rule definitions - pub max_rule_window_size: usize, + pub max_rule_window_size: u8, /// Register window size needed for entry point dispatch - pub dispatch_window_size: usize, + pub dispatch_window_size: u8, /// Program metadata pub metadata: ProgramMetadata, @@ -92,6 +88,20 @@ impl Program { pub const SERIALIZATION_VERSION: u32 = 3; /// Magic bytes to identify Regorus program files pub const MAGIC: [u8; 4] = *b"REGO"; + /// Maximum instructions supported (matches u16 jump targets) + pub const MAX_INSTRUCTIONS: usize = 65_535; // u16::MAX + /// Maximum literals supported (matches u16 literal indices) + pub const MAX_LITERALS: usize = 65_535; // u16::MAX + /// Generous cap for rules within a single policy bundle + pub const MAX_RULES: usize = 4_000; + /// Generous cap for exported entry points + pub const MAX_ENTRY_POINTS: usize = 1_000; + /// Generous cap for source/helper files + pub const MAX_SOURCES: usize = 256; + /// Generous cap for builtin declarations + pub const MAX_BUILTINS: usize = 512; + /// Maximum path depth for rule paths (e.g., data.a.b.c.d.rule) + pub const MAX_PATH_DEPTH: usize = 32; /// Create a new empty program pub fn new() -> Self { @@ -121,6 +131,86 @@ impl Program { } } + /// Validate that the program stays within supported bounds + pub fn validate_limits(&self) -> Result<(), String> { + if self.instructions.len() > Self::MAX_INSTRUCTIONS { + return Err(format!( + "Program exceeds max instructions ({} > {})", + self.instructions.len(), + Self::MAX_INSTRUCTIONS + )); + } + + if self.literals.len() > Self::MAX_LITERALS { + return Err(format!( + "Program exceeds max literals ({} > {})", + self.literals.len(), + Self::MAX_LITERALS + )); + } + + if self.rule_infos.len() > Self::MAX_RULES { + return Err(format!( + "Program exceeds max rules ({} > {})", + self.rule_infos.len(), + Self::MAX_RULES + )); + } + + if self.entry_points.len() > Self::MAX_ENTRY_POINTS { + return Err(format!( + "Program exceeds max entry points ({} > {})", + self.entry_points.len(), + Self::MAX_ENTRY_POINTS + )); + } + + if self.sources.len() > Self::MAX_SOURCES { + return Err(format!( + "Program exceeds max sources ({} > {})", + self.sources.len(), + Self::MAX_SOURCES + )); + } + + if self.builtin_info_table.len() > Self::MAX_BUILTINS { + return Err(format!( + "Program exceeds max builtins ({} > {})", + self.builtin_info_table.len(), + Self::MAX_BUILTINS + )); + } + + for params in &self.instruction_data.loop_params { + let body_end = core::cmp::max(params.body_start, params.loop_end); + if usize::from(body_end) > Self::MAX_INSTRUCTIONS { + return Err("Loop offsets exceed supported instruction range".to_string()); + } + } + + for params in &self.instruction_data.comprehension_begin_params { + let body_end = core::cmp::max(params.body_start, params.comprehension_end); + if usize::from(body_end) > Self::MAX_INSTRUCTIONS { + return Err("Comprehension offsets exceed supported instruction range".to_string()); + } + } + + for instr in &self.instructions { + if let &crate::rvm::Instruction::LoopNext { + body_start, + loop_end, + } = instr + { + let body_end = core::cmp::max(body_start, loop_end); + if usize::from(body_end) > Self::MAX_INSTRUCTIONS { + return Err("LoopNext offsets exceed supported instruction range".to_string()); + } + } + } + + Ok(()) + } + /// Add a source file and return its index pub fn add_source(&mut self, name: String, content: String) -> usize { let source_file = SourceFile::new(name.clone(), content); @@ -162,12 +252,12 @@ impl Program { pub fn add_builtin_info(&mut self, builtin_info: BuiltinInfo) -> u16 { let index = self.builtin_info_table.len(); self.builtin_info_table.push(builtin_info); - index as u16 + u16::try_from(index).unwrap_or(u16::MAX) } /// Get builtin info by index pub fn get_builtin_info(&self, index: u16) -> Option<&BuiltinInfo> { - self.builtin_info_table.get(index as usize) + self.builtin_info_table.get(usize::from(index)) } /// Update loop parameters by index @@ -281,7 +371,7 @@ impl Program { /// Get resolved builtin function by index pub fn get_resolved_builtin(&self, index: u16) -> Option<&BuiltinFcn> { - self.resolved_builtins.get(index as usize) + self.resolved_builtins.get(usize::from(index)) } /// Check if resolved builtins are initialized @@ -300,22 +390,22 @@ impl Program { } /// Get all entry points as IndexMap - pub fn get_entry_points(&self) -> &IndexMap { + pub const fn get_entry_points(&self) -> &IndexMap { &self.entry_points } /// Check if recompilation is needed due to partial deserialization failure - pub fn needs_recompilation(&self) -> bool { + pub const fn needs_recompilation(&self) -> bool { self.needs_recompilation } /// Mark that recompilation is needed - pub fn set_needs_recompilation(&mut self, needs_recompilation: bool) { + pub const fn set_needs_recompilation(&mut self, needs_recompilation: bool) { self.needs_recompilation = needs_recompilation; } /// Check if the program is fully functional (not needing recompilation) - pub fn is_fully_functional(&self) -> bool { + pub const fn is_fully_functional(&self) -> bool { !self.needs_recompilation } } diff --git a/src/rvm/program/listing.rs b/src/rvm/program/listing.rs index 2307eba..5c19f4f 100644 --- a/src/rvm/program/listing.rs +++ b/src/rvm/program/listing.rs @@ -1,26 +1,23 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::unwrap_used, - clippy::arithmetic_side_effects, - clippy::indexing_slicing, - clippy::option_if_let_else, - clippy::missing_const_for_fn, - clippy::as_conversions, - clippy::unused_trait_names, - clippy::pattern_type_mismatch -)] +#![allow(clippy::option_if_let_else)] use alloc::format; -use alloc::string::{String, ToString}; +use alloc::string::{String, ToString as _}; use alloc::vec::Vec; -use core::fmt::Write; +use core::fmt::{self, Write as _}; use crate::rvm::{ instructions::{Instruction, InstructionData, LoopMode}, program::Program, }; +// Writing into a String via fmt never fails, so we intentionally ignore writeln! results. +fn push_line(buf: &mut String, args: fmt::Arguments) { + let _ = buf.write_fmt(args); + let _ = buf.write_char('\n'); +} + /// Configuration for assembly listing output #[derive(Debug, Clone)] pub struct AssemblyListingConfig { @@ -61,65 +58,81 @@ pub fn generate_assembly_listing(program: &Program, config: &AssemblyListingConf let mut active_ends: Vec = Vec::new(); // Add header - writeln!( - output, - "; RVM Assembly - {} instructions, {} literals, {} builtins", - program.instructions.len(), - program.literals.len(), - program.builtin_info_table.len() - ) - .unwrap(); + push_line( + &mut output, + format_args!( + "; RVM Assembly - {} instructions, {} literals, {} builtins", + program.instructions.len(), + program.literals.len(), + program.builtin_info_table.len() + ), + ); // Add builtins table if !program.builtin_info_table.is_empty() { - writeln!(output, ";").unwrap(); - writeln!(output, "; BUILTINS TABLE:").unwrap(); + push_line(&mut output, format_args!(";")); + push_line(&mut output, format_args!("; BUILTINS TABLE:")); for (idx, builtin_info) in program.builtin_info_table.iter().enumerate() { - writeln!(output, "; B{:2}: {}", idx, builtin_info.name).unwrap(); + push_line( + &mut output, + format_args!("; B{:2}: {}", idx, builtin_info.name), + ); } } // Add literals table if config.show_literal_values && !program.literals.is_empty() { - writeln!(output, ";").unwrap(); - writeln!(output, "; LITERALS (JSON values):").unwrap(); + push_line(&mut output, format_args!(";")); + push_line(&mut output, format_args!("; LITERALS (JSON values):")); for (idx, literal) in program.literals.iter().enumerate() { let literal_json = serde_json::to_string(literal).unwrap_or_else(|_| "".to_string()); - writeln!(output, "; L{:2}: {}", idx, literal_json).unwrap(); + push_line( + &mut output, + format_args!("; L{:2}: {}", idx, literal_json), + ); } } // Add rules table if available if !program.rule_infos.is_empty() { - writeln!(output, ";").unwrap(); - writeln!(output, "; RULES TABLE:").unwrap(); + push_line(&mut output, format_args!(";")); + push_line(&mut output, format_args!("; RULES TABLE:")); for (idx, rule_info) in program.rule_infos.iter().enumerate() { - writeln!(output, "; R{:2}: {}", idx, rule_info.name).unwrap(); + push_line( + &mut output, + format_args!("; R{:2}: {}", idx, rule_info.name), + ); } } - writeln!(output, ";").unwrap(); + push_line(&mut output, format_args!(";")); for (pc, instruction) in program.instructions.iter().enumerate() { // Handle rule transitions and add gaps - if let Instruction::RuleInit { rule_index, .. } = instruction { + if let &Instruction::RuleInit { rule_index, .. } = instruction { // Add gap before new rule (except for the first rule) if current_rule_index.is_some() { - writeln!(output).unwrap(); + push_line(&mut output, format_args!("")); } - current_rule_index = Some(*rule_index); + current_rule_index = Some(rule_index); // Add rule name prefix - if let Some(rule_info) = program.rule_infos.get(*rule_index as usize) { - writeln!(output, "; ===== RULE: {} =====", rule_info.name).unwrap(); + if let Some(rule_info) = program.rule_infos.get(usize::from(rule_index)) { + push_line( + &mut output, + format_args!("; ===== RULE: {} =====", rule_info.name), + ); } else { - writeln!(output, "; ===== RULE: rule_{} =====", rule_index).unwrap(); + push_line( + &mut output, + format_args!("; ===== RULE: rule_{} =====", rule_index), + ); } } // Check if current PC matches any active end addresses (loops, comprehensions, rules) - let current_pc = pc as u16; + let current_pc = u16::try_from(pc).unwrap_or(u16::MAX); while let Some(&end_addr) = active_ends.last() { if current_pc >= end_addr { active_ends.pop(); @@ -130,7 +143,7 @@ pub fn generate_assembly_listing(program: &Program, config: &AssemblyListingConf } // Handle explicit end instructions - match instruction { + match *instruction { Instruction::LoopNext { .. } => { // LoopNext already handled by end address tracking above } @@ -141,13 +154,13 @@ pub fn generate_assembly_listing(program: &Program, config: &AssemblyListingConf } // Special case: Block end instructions should be indented at their block level (one level out) - let effective_indent_level = match instruction { + let effective_indent_level = match *instruction { Instruction::ComprehensionEnd {} => indent_level.saturating_sub(1), Instruction::LoopNext { .. } => indent_level.saturating_sub(1), _ => indent_level, }; - let indent = " ".repeat(effective_indent_level * config.indent_size); + let indent = " ".repeat(effective_indent_level.saturating_mul(config.indent_size)); // Format address let addr_str = if config.show_addresses { @@ -165,27 +178,27 @@ pub fn generate_assembly_listing(program: &Program, config: &AssemblyListingConf config, ); - writeln!(output, "{}{}", addr_str, inst_str).unwrap(); + push_line(&mut output, format_args!("{}{}", addr_str, inst_str)); // Increase indentation for loop/rule/comprehension starts and track their end addresses - match instruction { + match *instruction { Instruction::LoopStart { params_index } => { - if let Some(params) = program.instruction_data.get_loop_params(*params_index) { + if let Some(params) = program.instruction_data.get_loop_params(params_index) { active_ends.push(params.loop_end); - indent_level += 1; + indent_level = indent_level.saturating_add(1); } } Instruction::ComprehensionBegin { params_index } => { if let Some(params) = program .instruction_data - .get_comprehension_begin_params(*params_index) + .get_comprehension_begin_params(params_index) { active_ends.push(params.comprehension_end); - indent_level += 1; + indent_level = indent_level.saturating_add(1); } } Instruction::RuleInit { .. } => { - indent_level += 1; + indent_level = indent_level.saturating_add(1); // Note: Rules end with RuleReturn, not an address, so we don't track them here } _ => {} @@ -201,7 +214,7 @@ fn align_comment(base_text: &str, comment: &str, target_column: usize) -> String if current_len >= target_column { format!("{} ; {}", base_text, comment) } else { - let padding = " ".repeat(target_column - current_len); + let padding = " ".repeat(target_column.saturating_sub(current_len)); format!("{}{} ; {}", base_text, padding, comment) } } @@ -214,15 +227,16 @@ fn format_instruction_readable( program: &Program, config: &AssemblyListingConfig, ) -> String { - match instruction { + match *instruction { Instruction::Load { dest, literal_idx } => { let base = format!("{}Load r{} ← L{}", indent, dest, literal_idx); - let comment = if *literal_idx < program.literals.len() as u16 { - let literal_json = serde_json::to_string(&program.literals[*literal_idx as usize]) - .unwrap_or_else(|_| "".to_string()); - format!("Load literal: {}", literal_json) - } else { - "Load literal: ".to_string() + let comment = match program.literals.get(usize::from(literal_idx)) { + Some(literal) => { + let literal_json = + serde_json::to_string(literal).unwrap_or_else(|_| "".to_string()); + format!("Load literal: {}", literal_json) + } + None => "Load literal: ".to_string(), }; align_comment(&base, &comment, config.comment_column) } @@ -348,7 +362,7 @@ fn format_instruction_readable( align_comment(&base, &comment, config.comment_column) } Instruction::BuiltinCall { params_index } => { - if let Some(params) = instruction_data.get_builtin_call_params(*params_index) { + if let Some(params) = instruction_data.get_builtin_call_params(params_index) { let args_str = params .arg_registers() .iter() @@ -358,7 +372,7 @@ fn format_instruction_readable( let builtin_name = program .builtin_info_table - .get(params.builtin_index as usize) + .get(usize::from(params.builtin_index)) .map(|info| info.name.as_str()) .unwrap_or(""); @@ -381,7 +395,7 @@ fn format_instruction_readable( } } Instruction::FunctionCall { params_index } => { - if let Some(params) = instruction_data.get_function_call_params(*params_index) { + if let Some(params) = instruction_data.get_function_call_params(params_index) { let args_str = params .arg_registers() .iter() @@ -391,7 +405,7 @@ fn format_instruction_readable( let func_name = program .rule_infos - .get(params.func_rule_index as usize) + .get(usize::from(params.func_rule_index)) .map(|info| info.name.as_str()) .unwrap_or(""); @@ -440,7 +454,7 @@ fn format_instruction_readable( Instruction::ObjectCreate { params_index } => { let params = program .instruction_data - .get_object_create_params(*params_index); + .get_object_create_params(params_index); let base = format!( "{}ObjectCreate r{} ← {{...}}", indent, @@ -477,15 +491,16 @@ fn format_instruction_readable( "{}IndexLiteral r{} ← r{}[L{}]", indent, dest, container, literal_idx ); - let comment = if *literal_idx < program.literals.len() as u16 { - let literal_json = serde_json::to_string(&program.literals[*literal_idx as usize]) - .unwrap_or_else(|_| "".to_string()); - format!("Index with literal key: r{}[{}]", container, literal_json) - } else { - format!( + let comment = match program.literals.get(usize::from(literal_idx)) { + Some(literal) => { + let literal_json = + serde_json::to_string(literal).unwrap_or_else(|_| "".to_string()); + format!("Index with literal key: r{}[{}]", container, literal_json) + } + None => format!( "Index with literal: r{}[L{}] (invalid index)", container, literal_idx - ) + ), }; align_comment(&base, &comment, config.comment_column) } @@ -499,7 +514,7 @@ fn format_instruction_readable( align_comment(&base, &comment, config.comment_column) } Instruction::ArrayCreate { params_index } => { - if let Some(params) = instruction_data.get_array_create_params(*params_index) { + if let Some(params) = instruction_data.get_array_create_params(params_index) { let elements = params .element_registers() .iter() @@ -526,7 +541,7 @@ fn format_instruction_readable( align_comment(&base, &comment, config.comment_column) } Instruction::SetCreate { params_index } => { - if let Some(params) = instruction_data.get_set_create_params(*params_index) { + if let Some(params) = instruction_data.get_set_create_params(params_index) { let elements = params .element_registers() .iter() @@ -574,7 +589,7 @@ fn format_instruction_readable( align_comment(&base, &comment, config.comment_column) } Instruction::LoopStart { params_index } => { - if let Some(params) = instruction_data.get_loop_params(*params_index) { + if let Some(params) = instruction_data.get_loop_params(params_index) { let mode_str = match params.mode { LoopMode::Any => "any", LoopMode::Every => "every", @@ -620,7 +635,7 @@ fn format_instruction_readable( Instruction::CallRule { dest, rule_index } => { let rule_name = program .rule_infos - .get(*rule_index as usize) + .get(usize::from(rule_index)) .map(|info| info.name.as_str()) .unwrap_or(""); @@ -634,7 +649,7 @@ fn format_instruction_readable( } => { let rule_name = program .rule_infos - .get(*rule_index as usize) + .get(usize::from(rule_index)) .map(|info| info.name.as_str()) .unwrap_or(""); @@ -651,16 +666,16 @@ fn format_instruction_readable( } Instruction::ChainedIndex { params_index } => { let (base, comment) = - if let Some(params) = instruction_data.get_chained_index_params(*params_index) { + if let Some(params) = instruction_data.get_chained_index_params(params_index) { let chain_parts: Vec = params .path_components .iter() - .map(|component| match component { + .map(|component| match *component { crate::rvm::instructions::LiteralOrRegister::Literal(idx) => { - if let Some(literal) = program.literals.get(*idx as usize) { - match literal { - crate::Value::String(s) => format!(".{}", s.as_ref()), - other => format!( + if let Some(literal) = program.literals.get(usize::from(idx)) { + match *literal { + crate::Value::String(ref s) => format!(".{}", s.as_ref()), + ref other => format!( "[{}]", serde_json::to_string(other) .unwrap_or_else(|_| "?".to_string()) @@ -723,7 +738,7 @@ fn format_instruction_readable( align_comment(&base, "Stop execution", config.comment_column) } Instruction::ComprehensionBegin { params_index } => { - if let Some(params) = instruction_data.get_comprehension_begin_params(*params_index) { + if let Some(params) = instruction_data.get_comprehension_begin_params(params_index) { let mode_str = match params.mode { crate::rvm::instructions::ComprehensionMode::Array => "array", crate::rvm::instructions::ComprehensionMode::Set => "set", @@ -785,21 +800,28 @@ pub fn generate_tabular_assembly_listing( let mut indent_level: usize = 0; // Add header - writeln!(output, "; RVM Assembly (Tabular Format)").unwrap(); - writeln!( - output, - "; {} instructions, {} literals", - program.instructions.len(), - program.literals.len() - ) - .unwrap(); - writeln!(output, ";").unwrap(); - writeln!(output, "; PC | Instruction | Operation").unwrap(); - writeln!(output, ";-----|--------------|----------").unwrap(); + push_line(&mut output, format_args!("; RVM Assembly (Tabular Format)")); + push_line( + &mut output, + format_args!( + "; {} instructions, {} literals", + program.instructions.len(), + program.literals.len() + ), + ); + push_line(&mut output, format_args!(";")); + push_line( + &mut output, + format_args!("; PC | Instruction | Operation"), + ); + push_line( + &mut output, + format_args!(";-----|--------------|----------"), + ); for (pc, instruction) in program.instructions.iter().enumerate() { // Handle loop indentation - match instruction { + match *instruction { Instruction::LoopNext { .. } => { indent_level = indent_level.saturating_sub(1); } @@ -809,7 +831,7 @@ pub fn generate_tabular_assembly_listing( _ => {} } - let indent = " ".repeat(indent_level * 2); // Smaller indent for tabular format + let indent = " ".repeat(indent_level.saturating_mul(2)); // Smaller indent for tabular format // Format in tabular style let addr_str = format!("{:03}", pc); @@ -817,15 +839,18 @@ pub fn generate_tabular_assembly_listing( let operation = format_operation_compact(instruction, &indent, &program.instruction_data, program); - writeln!(output, "{:>4} | {:12} | {}", addr_str, inst_name, operation).unwrap(); + push_line( + &mut output, + format_args!("{:>4} | {:12} | {}", addr_str, inst_name, operation), + ); // Increase indentation for loop/rule starts - match instruction { + match *instruction { Instruction::LoopStart { .. } => { - indent_level += 1; + indent_level = indent_level.saturating_add(1); } Instruction::RuleInit { .. } => { - indent_level += 1; + indent_level = indent_level.saturating_add(1); } _ => {} } @@ -834,8 +859,8 @@ pub fn generate_tabular_assembly_listing( output } -fn get_instruction_name(instruction: &Instruction) -> &'static str { - match instruction { +const fn get_instruction_name(instruction: &Instruction) -> &'static str { + match *instruction { Instruction::Load { .. } => "LOAD", Instruction::LoadTrue { .. } => "LOAD_TRUE", Instruction::LoadFalse { .. } => "LOAD_FALSE", @@ -897,7 +922,7 @@ fn format_operation_compact( instruction_data: &InstructionData, _program: &Program, ) -> String { - match instruction { + match *instruction { Instruction::Load { dest, literal_idx } => { format!("{}r{} ← L{}", indent, dest, literal_idx) } @@ -928,7 +953,7 @@ fn format_operation_compact( format!("{}r{} ← r{}[L{}]", indent, dest, container, literal_idx) } Instruction::LoopStart { params_index } => { - if let Some(params) = instruction_data.get_loop_params(*params_index) { + if let Some(params) = instruction_data.get_loop_params(params_index) { format!( "{}loop r{} in r{} {{", indent, params.value_reg, params.collection diff --git a/src/rvm/program/recompile.rs b/src/rvm/program/recompile.rs index bee7f36..7435c18 100644 --- a/src/rvm/program/recompile.rs +++ b/src/rvm/program/recompile.rs @@ -1,9 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow(clippy::unused_trait_names)] - use super::Program; -use alloc::string::{String, ToString}; +use alloc::string::{String, ToString as _}; impl Program { /// Compile a partial deserialized program to a complete one diff --git a/src/rvm/program/rule_tree.rs b/src/rvm/program/rule_tree.rs index 3f25a00..c46bffb 100644 --- a/src/rvm/program/rule_tree.rs +++ b/src/rvm/program/rule_tree.rs @@ -1,13 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::arithmetic_side_effects, - clippy::unused_trait_names, - clippy::pattern_type_mismatch -)] use alloc::format; -use alloc::string::{String, ToString}; +use alloc::string::{String, ToString as _}; use alloc::vec::Vec; use anyhow::Result as AnyResult; @@ -26,7 +21,16 @@ impl Program { rule_name: &str, rule_index: usize, ) -> AnyResult<()> { - let mut full_path = Vec::with_capacity(path.len() + 1); + if path.len() >= Program::MAX_PATH_DEPTH { + return Err(anyhow::anyhow!( + "Rule path depth exceeds maximum ({} >= {})", + path.len(), + Program::MAX_PATH_DEPTH + )); + } + + let capacity = path.len().checked_add(1).unwrap_or(path.len()); + let mut full_path = Vec::with_capacity(capacity); full_path.extend(path.iter().map(|s| s.as_str())); full_path.push(rule_name); @@ -41,9 +45,9 @@ impl Program { pub fn check_rule_data_conflicts(&self, data: &Value) -> Result<(), crate::rvm::vm::VmError> { let actual_rule_tree = &self.rule_tree["data"]; - match actual_rule_tree { + match *actual_rule_tree { Value::Undefined => return Ok(()), - Value::Object(rule_obj) if rule_obj.is_empty() => return Ok(()), + Value::Object(ref rule_obj) if rule_obj.is_empty() => return Ok(()), _ => {} } @@ -55,15 +59,15 @@ impl Program { data: &Value, current_path: &mut Vec, ) -> Result<(), crate::rvm::vm::VmError> { - match rule_tree { - Value::Object(rule_obj) => { + match *rule_tree { + Value::Object(ref rule_obj) => { for (key, rule_value) in rule_obj.iter() { - if let Value::String(key_str) = key { + if let Value::String(ref key_str) = *key { current_path.push(key_str.to_string()); let data_value = &data[key]; - match rule_value { + match *rule_value { Value::Number(_) => { if data_value != &Value::Undefined { return Err(crate::rvm::vm::VmError::RuleDataConflict(format!( @@ -73,7 +77,7 @@ impl Program { } } Value::Object(_) => { - if let Value::Object(_) = data_value { + if let Value::Object(_) = *data_value { Self::check_conflicts_recursive( rule_value, data_value, diff --git a/src/rvm/program/serialization/binary.rs b/src/rvm/program/serialization/binary.rs index 7410679..adafa80 100644 --- a/src/rvm/program/serialization/binary.rs +++ b/src/rvm/program/serialization/binary.rs @@ -1,32 +1,79 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::indexing_slicing, - clippy::arithmetic_side_effects, - clippy::as_conversions, - clippy::unused_trait_names -)] use alloc::format; -use alloc::string::{String, ToString}; +use alloc::string::{String, ToString as _}; use alloc::vec::Vec; use bincode::config::standard; use bincode::serde::{decode_from_slice, encode_to_vec}; -use indexmap::IndexMap; -use super::super::types::SourceFile; use super::{DeserializationResult, Program}; use crate::value::Value; use super::value::{ binaries_to_values, binary_to_value, BinaryValue, BinaryValueRef, BinaryValueSlice, }; -type ArtifactData = (IndexMap, Vec, bool); + +/// Helper for tracking offsets with overflow checking +struct OffsetTracker { + current: usize, +} + +impl OffsetTracker { + const fn new(start: usize) -> Self { + Self { current: start } + } + + fn advance(&mut self, amount: usize) -> Result { + let start = self.current; + self.current = self.current.checked_add(amount).ok_or("Offset overflow")?; + Ok(start) + } + + const fn current(&self) -> usize { + self.current + } +} impl Program { + /// Helper: safely read u32 from 4 bytes starting at offset + fn read_u32(data: &[u8], offset: usize) -> Result { + let bytes = data + .get(offset..offset.checked_add(4).ok_or("Offset overflow")?) + .ok_or_else(|| format!("Cannot read u32 at offset {}", offset))?; + Ok(u32::from_le_bytes([ + *bytes.first().ok_or("Missing byte 0")?, + *bytes.get(1).ok_or("Missing byte 1")?, + *bytes.get(2).ok_or("Missing byte 2")?, + *bytes.get(3).ok_or("Missing byte 3")?, + ])) + } + + /// Helper: safely read u32 as usize + fn read_u32_as_usize(data: &[u8], offset: usize) -> Result { + Self::read_u32(data, offset)? + .try_into() + .map_err(|_| "Size conversion overflow".to_string()) + } + + /// Helper: safely get a byte at offset + fn read_byte(data: &[u8], offset: usize) -> Result { + data.get(offset) + .copied() + .ok_or_else(|| format!("Cannot read byte at offset {}", offset)) + } + + /// Helper: safely get slice + fn get_slice(data: &[u8], start: usize, end: usize) -> Result<&[u8], String> { + data.get(start..end) + .ok_or_else(|| format!("Cannot get slice [{}..{}]", start, end)) + } + /// Serialize program to binary format. /// Uses pure bincode for all sections now that `Value` supports serde. pub fn serialize_binary(&self) -> Result, String> { + self.validate_limits()?; + let mut buffer = Vec::new(); buffer.extend_from_slice(&Self::MAGIC); @@ -47,10 +94,26 @@ impl Program { let binary_data = encode_to_vec(self, standard()) .map_err(|e| format!("Program structure binary serialization failed: {}", e))?; - buffer.extend_from_slice(&(entry_points_bin.len() as u32).to_le_bytes()); - buffer.extend_from_slice(&(sources_bin.len() as u32).to_le_bytes()); - buffer.extend_from_slice(&(literals_bin.len() as u32).to_le_bytes()); - buffer.extend_from_slice(&(rule_tree_bin.len() as u32).to_le_bytes()); + buffer.extend_from_slice( + &u32::try_from(entry_points_bin.len()) + .map_err(|_| "Entry points size too large")? + .to_le_bytes(), + ); + buffer.extend_from_slice( + &u32::try_from(sources_bin.len()) + .map_err(|_| "Sources size too large")? + .to_le_bytes(), + ); + buffer.extend_from_slice( + &u32::try_from(literals_bin.len()) + .map_err(|_| "Literals size too large")? + .to_le_bytes(), + ); + buffer.extend_from_slice( + &u32::try_from(rule_tree_bin.len()) + .map_err(|_| "Rule tree size too large")? + .to_le_bytes(), + ); buffer.push(if self.rego_v0 { 1 } else { 0 }); buffer.extend_from_slice(&entry_points_bin); @@ -58,105 +121,28 @@ impl Program { buffer.extend_from_slice(&literals_bin); buffer.extend_from_slice(&rule_tree_bin); - buffer.extend_from_slice(&(binary_data.len() as u32).to_le_bytes()); + buffer.extend_from_slice( + &u32::try_from(binary_data.len()) + .map_err(|_| "Binary data size too large")? + .to_le_bytes(), + ); buffer.extend_from_slice(&binary_data); Ok(buffer) } - /// Deserialize only the artifact section (entry_points and sources) from binary format - pub fn deserialize_artifacts_only(data: &[u8]) -> Result { - if data.len() < 9 { - return Err("Data too short for artifact header".to_string()); - } - - if data[0..4] != Self::MAGIC { - return Err("Invalid file format - magic number mismatch".to_string()); - } - - let version = u32::from_le_bytes([data[4], data[5], data[6], data[7]]); - - match version { - 1 => { - if data.len() < 17 { - return Err("Data too short for artifact header".to_string()); - } - - let entry_points_len = - u32::from_le_bytes([data[8], data[9], data[10], data[11]]) as usize; - let sources_len = - u32::from_le_bytes([data[12], data[13], data[14], data[15]]) as usize; - let rego_v0 = data[16] != 0; - let entry_points_start = 17; - let sources_start = entry_points_start + entry_points_len; - let sources_end = sources_start + sources_len; - - if data.len() < sources_end { - return Err("Data truncated in artifact section".to_string()); - } - - let entry_points = - decode_from_slice(&data[entry_points_start..sources_start], standard()) - .map(|(value, _)| value) - .unwrap_or_else(|_| IndexMap::new()); - - let sources = decode_from_slice(&data[sources_start..sources_end], standard()) - .map(|(value, _)| value) - .unwrap_or_else(|_| Vec::new()); - - Ok((entry_points, sources, rego_v0)) - } - 2 | 3 => { - if data.len() < 25 { - return Err("Data too short for artifact header".to_string()); - } - - let entry_points_len = - u32::from_le_bytes([data[8], data[9], data[10], data[11]]) as usize; - let sources_len = - u32::from_le_bytes([data[12], data[13], data[14], data[15]]) as usize; - let literals_len = - u32::from_le_bytes([data[16], data[17], data[18], data[19]]) as usize; - let rule_tree_len = - u32::from_le_bytes([data[20], data[21], data[22], data[23]]) as usize; - let rego_v0 = data[24] != 0; - - let entry_points_start = 25; - let sources_start = entry_points_start + entry_points_len; - let literals_start = sources_start + sources_len; - let rule_tree_start = literals_start + literals_len; - let rule_tree_end = rule_tree_start + rule_tree_len; - - if data.len() < rule_tree_end { - return Err("Data truncated in artifact section".to_string()); - } - - let entry_points = - decode_from_slice(&data[entry_points_start..sources_start], standard()) - .map(|(value, _)| value) - .unwrap_or_else(|_| IndexMap::new()); - - let sources = decode_from_slice(&data[sources_start..literals_start], standard()) - .map(|(value, _)| value) - .unwrap_or_else(|_| Vec::new()); - - Ok((entry_points, sources, rego_v0)) - } - v => Err(format!("Unsupported version {}", v)), - } - } - /// Deserialize program from binary format with version checking pub fn deserialize_binary(data: &[u8]) -> Result { if data.len() < 9 { return Err("Data too short for header".to_string()); } - if data[0..4] != Self::MAGIC { + let magic = Self::get_slice(data, 0, 4)?; + if magic != Self::MAGIC { return Err("Invalid file format - magic number mismatch".to_string()); } - let version = u32::from_le_bytes([data[4], data[5], data[6], data[7]]); + let version = Self::read_u32(data, 4)?; if version > Self::SERIALIZATION_VERSION { return Err(format!( "Unsupported version {}. Maximum supported version is {}", @@ -171,59 +157,63 @@ impl Program { return Err("Data too short for header".to_string()); } - let entry_points_len = - u32::from_le_bytes([data[8], data[9], data[10], data[11]]) as usize; - let sources_len = - u32::from_le_bytes([data[12], data[13], data[14], data[15]]) as usize; - let rego_v0 = data[16] != 0; - let entry_points_start = 17; - let sources_start = entry_points_start + entry_points_len; - let binary_len_start = sources_start + sources_len; + let entry_points_len = Self::read_u32_as_usize(data, 8)?; + let sources_len = Self::read_u32_as_usize(data, 12)?; + let rego_v0 = Self::read_byte(data, 16)? != 0; - if data.len() < binary_len_start + 4 { + let mut offset = OffsetTracker::new(17); + let entry_points_start = offset.advance(entry_points_len)?; + let sources_start = offset.advance(sources_len)?; + let binary_len_start = offset.current(); + + if data.len() < binary_len_start.checked_add(4).ok_or("Offset overflow")? { return Err("Data too short for binary length".to_string()); } - let binary_len = u32::from_le_bytes([ - data[binary_len_start], - data[binary_len_start + 1], - data[binary_len_start + 2], - data[binary_len_start + 3], - ]) as usize; + let binary_len = Self::read_u32_as_usize(data, binary_len_start)?; - let json_len_start = binary_len_start + 4 + binary_len; - if data.len() < json_len_start + 4 { + let mut binary_offset = OffsetTracker::new(binary_len_start); + binary_offset.advance(4)?; // Skip the binary_len u32 + let binary_start = binary_offset.advance(binary_len)?; + let json_start = binary_offset.current(); + + if data.len() < json_start.checked_add(4).ok_or("Offset overflow")? { return Err("Data too short for JSON length".to_string()); } - let json_len = u32::from_le_bytes([ - data[json_len_start], - data[json_len_start + 1], - data[json_len_start + 2], - data[json_len_start + 3], - ]) as usize; + let json_len = Self::read_u32_as_usize(data, json_start)?; - let total_expected = json_len_start + 4 + json_len; + let total_expected = json_start + .checked_add(4) + .and_then(|v| v.checked_add(json_len)) + .ok_or("Offset overflow")?; if data.len() < total_expected { return Err("Data truncated".to_string()); } - let binary_start = binary_len_start + 4; - let json_start = json_len_start + 4; + let json_end = json_start + .checked_add(4) + .and_then(|v| v.checked_add(json_len)) + .ok_or("Offset overflow")?; - let entry_points = - decode_from_slice(&data[entry_points_start..sources_start], standard()) - .map(|(value, _)| value) - .map_err(|e| format!("Entry points deserialization failed: {}", e))?; + let entry_points = decode_from_slice( + Self::get_slice(data, entry_points_start, sources_start)?, + standard(), + ) + .map(|(value, _)| value) + .map_err(|e| format!("Entry points deserialization failed: {}", e))?; - let sources = decode_from_slice(&data[sources_start..binary_len_start], standard()) - .map(|(value, _)| value) - .map_err(|e| format!("Sources deserialization failed: {}", e))?; + let sources = decode_from_slice( + Self::get_slice(data, sources_start, binary_len_start)?, + standard(), + ) + .map(|(value, _)| value) + .map_err(|e| format!("Sources deserialization failed: {}", e))?; let mut needs_recompilation = false; let mut program = match decode_from_slice::( - &data[binary_start..json_start], + Self::get_slice(data, binary_start, json_start)?, standard(), ) { Ok((prog, _)) => prog, @@ -233,33 +223,36 @@ impl Program { } }; - let (literals, rule_tree) = match serde_json::from_slice::( - &data[json_start..json_start + json_len], - ) { - Ok(combined) => { - let literals = combined - .get("literals") - .and_then(|v| serde_json::from_value::>(v.clone()).ok()) - .unwrap_or_else(|| { - needs_recompilation = true; - Vec::new() - }); + let (literals, rule_tree) = + match serde_json::from_slice::(Self::get_slice( + data, + json_start.checked_add(4).ok_or("Offset overflow")?, + json_end, + )?) { + Ok(combined) => { + let literals = combined + .get("literals") + .and_then(|v| serde_json::from_value::>(v.clone()).ok()) + .unwrap_or_else(|| { + needs_recompilation = true; + Vec::new() + }); - let rule_tree = combined - .get("rule_tree") - .and_then(|v| serde_json::from_value::(v.clone()).ok()) - .unwrap_or_else(|| { - needs_recompilation = true; - Value::new_object() - }); + let rule_tree = combined + .get("rule_tree") + .and_then(|v| serde_json::from_value::(v.clone()).ok()) + .unwrap_or_else(|| { + needs_recompilation = true; + Value::new_object() + }); - (literals, rule_tree) - } - Err(_e) => { - needs_recompilation = true; - (Vec::new(), Value::new_object()) - } - }; + (literals, rule_tree) + } + Err(_e) => { + needs_recompilation = true; + (Vec::new(), Value::new_object()) + } + }; program.entry_points = entry_points; program.sources = sources; @@ -285,53 +278,52 @@ impl Program { return Err("Data too short for header".to_string()); } - let entry_points_len = - u32::from_le_bytes([data[8], data[9], data[10], data[11]]) as usize; - let sources_len = - u32::from_le_bytes([data[12], data[13], data[14], data[15]]) as usize; - let literals_len = - u32::from_le_bytes([data[16], data[17], data[18], data[19]]) as usize; - let rule_tree_len = - u32::from_le_bytes([data[20], data[21], data[22], data[23]]) as usize; - let rego_v0 = data[24] != 0; + let entry_points_len = Self::read_u32_as_usize(data, 8)?; + let sources_len = Self::read_u32_as_usize(data, 12)?; + let literals_len = Self::read_u32_as_usize(data, 16)?; + let rule_tree_len = Self::read_u32_as_usize(data, 20)?; + let rego_v0 = Self::read_byte(data, 24)? != 0; - let entry_points_start = 25; - let sources_start = entry_points_start + entry_points_len; - let literals_start = sources_start + sources_len; - let rule_tree_start = literals_start + literals_len; - let binary_len_start = rule_tree_start + rule_tree_len; + let mut offset = OffsetTracker::new(25); + let entry_points_start = offset.advance(entry_points_len)?; + let sources_start = offset.advance(sources_len)?; + let literals_start = offset.advance(literals_len)?; + let rule_tree_start = offset.advance(rule_tree_len)?; + let binary_len_start = offset.current(); - if data.len() < binary_len_start + 4 { + if data.len() < binary_len_start.checked_add(4).ok_or("Offset overflow")? { return Err("Data too short for binary length".to_string()); } - let binary_len = u32::from_le_bytes([ - data[binary_len_start], - data[binary_len_start + 1], - data[binary_len_start + 2], - data[binary_len_start + 3], - ]) as usize; + let binary_len = Self::read_u32_as_usize(data, binary_len_start)?; - let binary_start = binary_len_start + 4; - let binary_end = binary_start + binary_len; + let mut binary_offset = OffsetTracker::new(binary_len_start); + binary_offset.advance(4)?; // Skip the binary_len u32 + let binary_start = binary_offset.advance(binary_len)?; + let binary_end = binary_offset.current(); if data.len() < binary_end { return Err("Data truncated".to_string()); } - let entry_points = - decode_from_slice(&data[entry_points_start..sources_start], standard()) - .map(|(value, _)| value) - .map_err(|e| format!("Entry points deserialization failed: {}", e))?; + let entry_points = decode_from_slice( + Self::get_slice(data, entry_points_start, sources_start)?, + standard(), + ) + .map(|(value, _)| value) + .map_err(|e| format!("Entry points deserialization failed: {}", e))?; - let sources = decode_from_slice(&data[sources_start..literals_start], standard()) - .map(|(value, _)| value) - .map_err(|e| format!("Sources deserialization failed: {}", e))?; + let sources = decode_from_slice( + Self::get_slice(data, sources_start, literals_start)?, + standard(), + ) + .map(|(value, _)| value) + .map_err(|e| format!("Sources deserialization failed: {}", e))?; let mut needs_recompilation = false; let literals = match decode_from_slice::, _>( - &data[literals_start..rule_tree_start], + Self::get_slice(data, literals_start, rule_tree_start)?, standard(), ) { Ok((binary_literals, _)) => match binaries_to_values(binary_literals) { @@ -348,7 +340,7 @@ impl Program { }; let rule_tree = match decode_from_slice::( - &data[rule_tree_start..binary_len_start], + Self::get_slice(data, rule_tree_start, binary_len_start)?, standard(), ) { Ok((binary_tree, _)) => match binary_to_value(binary_tree) { @@ -365,7 +357,7 @@ impl Program { }; let mut program = match decode_from_slice::( - &data[binary_start..binary_end], + Self::get_slice(data, binary_start, binary_end)?, standard(), ) { Ok((prog, _)) => prog, @@ -404,85 +396,16 @@ impl Program { return Ok(false); } - if data[0..4] != Self::MAGIC { + let magic = Self::get_slice(data, 0, 4).ok(); + if magic != Some(&Self::MAGIC[..]) { return Ok(false); } - let version = u32::from_le_bytes([data[4], data[5], data[6], data[7]]); + let version = Self::read_u32(data, 4).ok(); match version { - 1..=3 => Ok(true), + Some(1..=3) => Ok(true), _ => Ok(false), } } - - /// Get file format information without deserializing - pub fn get_file_info(data: &[u8]) -> Result<(u32, usize), String> { - if data.len() < 9 { - return Err("Data too short for header".to_string()); - } - - if data[0..4] != Self::MAGIC { - return Err("Invalid file format".to_string()); - } - - let version = u32::from_le_bytes([data[4], data[5], data[6], data[7]]); - - match version { - 1 => { - if data.len() < 25 { - return Err("Data too short for header".to_string()); - } - - let entry_points_len = - u32::from_le_bytes([data[8], data[9], data[10], data[11]]) as usize; - let sources_len = - u32::from_le_bytes([data[12], data[13], data[14], data[15]]) as usize; - let binary_len_start = 17 + entry_points_len + sources_len; - - if data.len() < binary_len_start + 4 { - return Err("Data too short for binary length".to_string()); - } - - let binary_len = u32::from_le_bytes([ - data[binary_len_start], - data[binary_len_start + 1], - data[binary_len_start + 2], - data[binary_len_start + 3], - ]) as usize; - - Ok((version, binary_len)) - } - 2 | 3 => { - if data.len() < 29 { - return Err("Data too short for header".to_string()); - } - - let entry_points_len = - u32::from_le_bytes([data[8], data[9], data[10], data[11]]) as usize; - let sources_len = - u32::from_le_bytes([data[12], data[13], data[14], data[15]]) as usize; - let literals_len = - u32::from_le_bytes([data[16], data[17], data[18], data[19]]) as usize; - let rule_tree_len = - u32::from_le_bytes([data[20], data[21], data[22], data[23]]) as usize; - let binary_len_start = - 25 + entry_points_len + sources_len + literals_len + rule_tree_len; - - if data.len() < binary_len_start + 4 { - return Err("Data too short for binary length".to_string()); - } - - let binary_len = u32::from_le_bytes([ - data[binary_len_start], - data[binary_len_start + 1], - data[binary_len_start + 2], - data[binary_len_start + 3], - ]) as usize; - - Ok((version, binary_len)) - } - v => Err(format!("Unsupported version {}", v)), - } - } } diff --git a/src/rvm/program/serialization/json.rs b/src/rvm/program/serialization/json.rs index 005e4d7..63ffa02 100644 --- a/src/rvm/program/serialization/json.rs +++ b/src/rvm/program/serialization/json.rs @@ -1,10 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow(clippy::as_conversions, clippy::unused_trait_names)] - use alloc::format; -use alloc::string::{String, ToString}; +use alloc::string::{String, ToString as _}; use alloc::vec::Vec; use super::super::types::SourceFile; @@ -84,7 +82,8 @@ impl Program { let optimization_level = metadata .get("optimization_level") .and_then(|v| v.as_u64()) - .unwrap_or(0) as u8; + .and_then(|v| u8::try_from(v).ok()) + .unwrap_or(0); let rego_v0 = metadata .get("rego_v0") .and_then(|v| v.as_bool()) @@ -104,15 +103,18 @@ impl Program { let main_entry_point = program_structure .get("main_entry_point") .and_then(|v| v.as_u64()) - .unwrap_or(0) as usize; + .and_then(|v| u32::try_from(v).ok()) + .unwrap_or(0); let max_rule_window_size = program_structure .get("max_rule_window_size") .and_then(|v| v.as_u64()) - .unwrap_or(0) as usize; + .and_then(|v| u8::try_from(v).ok()) + .unwrap_or(0); let dispatch_window_size = program_structure .get("dispatch_window_size") .and_then(|v| v.as_u64()) - .unwrap_or(0) as usize; + .and_then(|v| u8::try_from(v).ok()) + .unwrap_or(0); let instructions: Vec = serde_json::from_value( json_data diff --git a/src/rvm/program/serialization/mod.rs b/src/rvm/program/serialization/mod.rs index f8c4406..ff64eb8 100644 --- a/src/rvm/program/serialization/mod.rs +++ b/src/rvm/program/serialization/mod.rs @@ -1,10 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow(clippy::redundant_pub_crate)] - -pub(crate) mod binary; +pub mod binary; mod json; -pub(crate) mod value; +pub mod value; use serde::{Deserialize, Serialize}; diff --git a/src/rvm/program/serialization/value.rs b/src/rvm/program/serialization/value.rs index e1abc2a..d0bacee 100644 --- a/src/rvm/program/serialization/value.rs +++ b/src/rvm/program/serialization/value.rs @@ -1,19 +1,13 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::redundant_pub_crate, - clippy::unused_trait_names, - clippy::pattern_type_mismatch -)] - use alloc::collections::{BTreeMap, BTreeSet}; use alloc::format; use alloc::string::String; use alloc::vec::Vec; use core::fmt; -use core::str::FromStr; -use serde::de::{self, EnumAccess, VariantAccess, Visitor}; -use serde::ser::{SerializeSeq, SerializeTuple}; +use core::str::FromStr as _; +use serde::de::{self, EnumAccess, VariantAccess as _, Visitor}; +use serde::ser::{SerializeSeq as _, SerializeTuple as _}; use serde::{Deserialize, Serialize}; use crate::number::Number; @@ -33,19 +27,19 @@ const VARIANT_NUMBER_F64: u32 = 10; /// Wrapper type for zero-copy binary serialization of a `Value`. /// Keeps references into the original data so collections and strings are not cloned. -pub(crate) struct BinaryValueRef<'a>(pub &'a Value); +pub struct BinaryValueRef<'a>(pub &'a Value); impl<'a> Serialize for BinaryValueRef<'a> { fn serialize(&self, serializer: S) -> Result where S: serde::Serializer, { - match self.0 { + match *self.0 { Value::Null => serializer.serialize_unit_variant("BinaryValue", VARIANT_NULL, "Null"), Value::Bool(b) => { serializer.serialize_newtype_variant("BinaryValue", VARIANT_BOOL, "Bool", &b) } - Value::Number(n) => { + Value::Number(ref n) => { if let Some(value) = n.as_i64() { serializer.serialize_newtype_variant( "BinaryValue", @@ -76,25 +70,25 @@ impl<'a> Serialize for BinaryValueRef<'a> { ) } } - Value::String(s) => serializer.serialize_newtype_variant( + Value::String(ref s) => serializer.serialize_newtype_variant( "BinaryValue", VARIANT_STRING, "String", s.as_ref(), ), - Value::Array(items) => serializer.serialize_newtype_variant( + Value::Array(ref items) => serializer.serialize_newtype_variant( "BinaryValue", VARIANT_ARRAY, "Array", &BinaryValueSlice(items.as_slice()), ), - Value::Set(items) => serializer.serialize_newtype_variant( + Value::Set(ref items) => serializer.serialize_newtype_variant( "BinaryValue", VARIANT_SET, "Set", &BinarySetRef(items.as_ref()), ), - Value::Object(entries) => serializer.serialize_newtype_variant( + Value::Object(ref entries) => serializer.serialize_newtype_variant( "BinaryValue", VARIANT_OBJECT, "Object", @@ -108,7 +102,7 @@ impl<'a> Serialize for BinaryValueRef<'a> { } /// Slice wrapper allowing zero-copy serialization of value collections. -pub(crate) struct BinaryValueSlice<'a>(pub &'a [Value]); +pub struct BinaryValueSlice<'a>(pub &'a [Value]); impl<'a> Serialize for BinaryValueSlice<'a> { fn serialize(&self, serializer: S) -> Result @@ -169,7 +163,7 @@ impl<'a> Serialize for BinaryEntryRef<'a> { /// Owned counterpart used during deserialization. #[derive(Debug, Clone)] -pub(crate) struct BinaryValue(pub Value); +pub struct BinaryValue(pub Value); impl BinaryValue { fn into_value(self) -> Value { @@ -290,10 +284,10 @@ impl<'de> Deserialize<'de> for BinaryValue { } } -pub(crate) fn binaries_to_values(binaries: Vec) -> Result, String> { +pub fn binaries_to_values(binaries: Vec) -> Result, String> { Ok(binaries.into_iter().map(BinaryValue::into_value).collect()) } -pub(crate) fn binary_to_value(binary: BinaryValue) -> Result { +pub fn binary_to_value(binary: BinaryValue) -> Result { Ok(binary.into_value()) } diff --git a/src/rvm/program/types.rs b/src/rvm/program/types.rs index 390360b..72869d9 100644 --- a/src/rvm/program/types.rs +++ b/src/rvm/program/types.rs @@ -1,11 +1,5 @@ -#![allow(clippy::missing_const_for_fn)] // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![allow( - clippy::indexing_slicing, - clippy::arithmetic_side_effects, - clippy::as_conversions -)] use alloc::string::String; use alloc::vec::Vec; use serde::{Deserialize, Serialize}; @@ -34,7 +28,7 @@ pub struct SpanInfo { } impl SpanInfo { - pub fn new(source_index: usize, line: usize, column: usize, length: usize) -> Self { + pub const fn new(source_index: usize, line: usize, column: usize, length: usize) -> Self { Self { source_index, line, @@ -47,8 +41,8 @@ impl SpanInfo { pub fn from_lexer_span(span: &crate::lexer::Span, source_index: usize) -> Self { Self { source_index, - line: span.line as usize, - column: span.col as usize, + line: span.line.try_into().unwrap_or(usize::MAX), + column: span.col.try_into().unwrap_or(usize::MAX), length: span.text().len(), } } @@ -135,7 +129,7 @@ impl RuleInfo { result_reg: u8, num_registers: u8, ) -> Self { - let num_params = param_names.len() as u32; + let num_params = u32::try_from(param_names.len()).unwrap_or(u32::MAX); let num_definitions = definitions.len(); Self { name, @@ -153,7 +147,7 @@ impl RuleInfo { } /// Set the default literal index for this rule - pub fn set_default_literal_index(&mut self, default_literal_index: u16) { + pub const fn set_default_literal_index(&mut self, default_literal_index: u16) { self.default_literal_index = Some(default_literal_index); } } @@ -168,7 +162,7 @@ pub struct SourceFile { } impl SourceFile { - pub fn new(name: String, content: String) -> Self { + pub const fn new(name: String, content: String) -> Self { Self { name, content } } } diff --git a/src/rvm/tests/vm.rs b/src/rvm/tests/vm.rs index aca0de7..208091e 100644 --- a/src/rvm/tests/vm.rs +++ b/src/rvm/tests/vm.rs @@ -616,8 +616,8 @@ mod tests { program.main_entry_point = 0; // Set a reasonable default for register window size in VM tests - // Most tests use registers 0-10, so we'll allocate 256 registers to be safe - program.max_rule_window_size = 256; + // Most tests use registers 0-10, so we'll allocate 255 registers (u8 max) + program.max_rule_window_size = 255; program.dispatch_window_size = 50; // Initialize resolved builtins if we have builtin info diff --git a/src/rvm/vm/execution.rs b/src/rvm/vm/execution.rs index cbe63b3..c796e0a 100644 --- a/src/rvm/vm/execution.rs +++ b/src/rvm/vm/execution.rs @@ -148,7 +148,7 @@ impl RegoVM { } self.executed_instructions += 1; - let instruction = program.instructions[self.pc].clone(); + let instruction = program.instructions[self.pc]; match self.execute_instruction(&program, instruction)? { InstructionOutcome::Continue => { @@ -351,7 +351,7 @@ impl RegoVM { } self.pc = frame_pc; - let instruction = program.instructions[self.pc].clone(); + let instruction = program.instructions[self.pc]; if let Some(frame_info) = self.execution_stack.last() { if let FrameKind::Comprehension { context, .. } = &frame_info.kind { if context.iteration_state.is_none() diff --git a/src/rvm/vm/machine.rs b/src/rvm/vm/machine.rs index 9b0d396..d07f562 100644 --- a/src/rvm/vm/machine.rs +++ b/src/rvm/vm/machine.rs @@ -160,7 +160,7 @@ impl RegoVM { self.program = program.clone(); // Use the dispatch window size from the program for initial register allocation - let dispatch_size = program.dispatch_window_size.max(2); // Ensure at least 2 registers + let dispatch_size = usize::from(program.dispatch_window_size).max(2); // Ensure at least 2 registers self.base_register_count = dispatch_size; // Resize registers to match program requirements @@ -171,7 +171,7 @@ impl RegoVM { self.rule_cache = vec![(false, Value::Undefined); program.rule_infos.len()]; // Set PC to main entry point - self.pc = program.main_entry_point; + self.pc = usize::try_from(program.main_entry_point).unwrap_or(0); self.executed_instructions = 0; // Reset instruction counter }