From 50ad1928c7b2e0980c7c1202acd8c775b9c0e0a4 Mon Sep 17 00:00:00 2001 From: Vincent Thiberville Date: Sun, 22 Oct 2023 15:02:59 +0200 Subject: [PATCH 1/3] feat: rework how module values are evaluated Put all expressions in a single vec, so that it can be evaluated on its own, before evaluating the module operations. This will allow cleaning the code in the next commits are removing some Arc. --- boreal/src/compiler/module.rs | 124 +++++++++++++++++----------- boreal/src/evaluator/module.rs | 143 ++++++++++++++++++--------------- 2 files changed, 156 insertions(+), 111 deletions(-) diff --git a/boreal/src/compiler/module.rs b/boreal/src/compiler/module.rs index 038b6959..96b61222 100644 --- a/boreal/src/compiler/module.rs +++ b/boreal/src/compiler/module.rs @@ -2,7 +2,7 @@ use std::{collections::HashMap, ops::Range}; use boreal_parser::expression::{Identifier, IdentifierOperation, IdentifierOperationType}; -use super::expression::{compile_expression, Expression, Type}; +use super::expression::{compile_expression, Expr, Expression, Type}; use super::rule::RuleCompiler; use super::{CompilationError, ImportedModule}; use crate::module::{self, ScanContext, StaticValue, Type as ValueType, Value}; @@ -18,17 +18,6 @@ pub struct Module { dynamic_types: ValueType, } -/// Operations on identifiers. -#[derive(Debug)] -pub enum ValueOperation { - /// Object subfield, i.e. `value.subfield`. - Subfield(String), - /// Array/dict subscript, i.e. `value[subscript]`. - Subscript(Box), - /// Function call, i.e. `value(arguments)`. - FunctionCall(Vec), -} - /// Index on a bounded value #[derive(Debug)] pub enum BoundedValueIndex { @@ -49,20 +38,53 @@ pub enum ModuleExpression { index: BoundedValueIndex, /// List of operations to apply to the value to get the final value. - operations: Vec, + operations: ModuleOperations, }, /// A value coming from a function exposed by a module. - Function { - /// The function to call with the computed index + StaticFunction { + /// The function to call. fun: fn(&ScanContext, Vec) -> Option, - /// The expressions that provides the arguments of the function. - arguments: Vec, - /// List of operations to apply on the value returned by the function. - operations: Vec, + + /// List of operations to apply to the value to get the final value. + /// + /// These operations must start with a [`ValueOperation::FunctionCall`] operation. + operations: ModuleOperations, }, } +/// Operations to apply to a module value. +#[derive(Debug)] +pub struct ModuleOperations { + /// List of expressions that needs to be evaluated to compute all of the operations. + /// + /// This list is in its own Vec instead of inlined in each operation so that they + /// can be evaluated separately, which makes the evaluation of operations simpler + /// as it can be done immutably. + /// + /// The expressions are stored in the same order the operations will be evaluated. + pub expressions: Vec, + + /// List of operations to apply to the module value. + pub operations: Vec, +} + +/// Operations on identifiers. +#[derive(Debug)] +pub enum ValueOperation { + /// Object subfield, i.e. `value.subfield`. + Subfield(String), + /// Array/dict subscript, i.e. `value[subscript]`. + /// + /// The value used as the subscript index is stored in [`ModuleOperations::expressions`]. + Subscript, + /// Function call, i.e. `value(arguments)`. + /// + /// The value is the number of expressions to pick from [`ModuleOperations::expressions`] + /// to form the arguments of the function call. + FunctionCall(usize), +} + // XXX: custom Debug impl needed because derive does not work with the fn fields. impl std::fmt::Debug for ModuleExpression { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -72,14 +94,9 @@ impl std::fmt::Debug for ModuleExpression { .field("index", index) .field("operations", operations) .finish(), - Self::Function { - fun, - arguments, - operations, - } => f + Self::StaticFunction { fun, operations } => f .debug_struct("Function") .field("fun", &(*fun as usize)) - .field("arguments", arguments) .field("operations", operations) .finish(), } @@ -128,6 +145,7 @@ pub(super) fn compile_bounded_identifier_use<'a, 'b>( compiler, last_immediate_value: None, current_value: ValueOrType::Type(starting_type), + operations_expressions: Vec::new(), operations: Vec::with_capacity(identifier.operations.len()), current_span: identifier.name_span, bounded_value_index: Some(BoundedValueIndex::BoundedStack(identifier_stack_index)), @@ -184,6 +202,7 @@ pub(super) fn compile_identifier<'a, 'b>( compiler, last_immediate_value: Some(value), current_value: ValueOrType::Value(value), + operations_expressions: Vec::new(), operations: Vec::with_capacity(nb_ops), current_span: identifier.name_span, bounded_value_index: None, @@ -195,6 +214,7 @@ pub(super) fn compile_identifier<'a, 'b>( compiler, last_immediate_value: None, current_value: ValueOrType::Type(&module.module.dynamic_types), + operations_expressions: Vec::new(), operations: Vec::with_capacity(nb_ops), current_span: identifier.name_span, bounded_value_index: Some(BoundedValueIndex::Module(module.module_index)), @@ -220,6 +240,9 @@ pub(super) struct ModuleUse<'a, 'b> { // Current value (or type). current_value: ValueOrType<'b>, + // Expressions that needs to be evaluated to be able to evaluate the operations. + operations_expressions: Vec, + // Operations that will need to be evaluated at scanning time. operations: Vec, @@ -245,22 +268,23 @@ impl ModuleUse<'_, '_> { res } IdentifierOperationType::Subscript(subscript) => { - let subscript = compile_expression(self.compiler, *subscript)?; + let Expr { expr, ty, span } = compile_expression(self.compiler, *subscript)?; - self.operations - .push(ValueOperation::Subscript(Box::new(subscript.expr))); - self.current_value.subscript(subscript.ty, subscript.span) + self.operations_expressions.push(expr); + self.operations.push(ValueOperation::Subscript); + self.current_value.subscript(ty, span) } IdentifierOperationType::FunctionCall(arguments) => { - let mut arguments_exprs = Vec::with_capacity(arguments.len()); + self.operations + .push(ValueOperation::FunctionCall(arguments.len())); + let mut arguments_types = Vec::with_capacity(arguments.len()); for arg in arguments { let res = compile_expression(self.compiler, arg)?; - arguments_exprs.push(res.expr); + self.operations_expressions.push(res.expr); arguments_types.push(res.ty); } - self.operations - .push(ValueOperation::FunctionCall(arguments_exprs)); + self.current_value.function_call(&arguments_types) } }; @@ -306,7 +330,10 @@ impl ModuleUse<'_, '_> { let ty = self.current_value.into_type()?; let expr = ModuleExpression::BoundedModuleValueUse { index: self.bounded_value_index?, - operations: self.operations, + operations: ModuleOperations { + expressions: self.operations_expressions, + operations: self.operations, + }, }; Some((expr, ty)) } @@ -325,14 +352,12 @@ impl ModuleUse<'_, '_> { StaticValue::Object(_) => return None, StaticValue::Function { fun, .. } => { - let mut ops = self.operations.into_iter(); - let Some(ValueOperation::FunctionCall(arguments)) = ops.next() else { - return None; - }; - Expression::Module(ModuleExpression::Function { + Expression::Module(ModuleExpression::StaticFunction { fun: *fun, - arguments, - operations: ops.collect(), + operations: ModuleOperations { + expressions: self.operations_expressions, + operations: self.operations, + }, }) } }; @@ -608,14 +633,23 @@ mod tests { test_type_traits_non_clonable(compile_module(&crate::module::Time)); test_type_traits_non_clonable(ValueOperation::Subfield("a".to_owned())); test_type_traits_non_clonable(BoundedValueIndex::Module(0)); + test_type_traits_non_clonable(ModuleOperations { + expressions: Vec::new(), + operations: Vec::new(), + }); test_type_traits_non_clonable(ModuleExpression::BoundedModuleValueUse { index: BoundedValueIndex::Module(0), - operations: Vec::new(), + operations: ModuleOperations { + expressions: Vec::new(), + operations: Vec::new(), + }, }); - test_type_traits_non_clonable(ModuleExpression::Function { + test_type_traits_non_clonable(ModuleExpression::StaticFunction { fun: test_fun, - arguments: Vec::new(), - operations: Vec::new(), + operations: ModuleOperations { + expressions: Vec::new(), + operations: Vec::new(), + }, }); test_type_traits_non_clonable(IteratorType::Array(ValueType::Integer)); test_type_traits_non_clonable(TypeError::UnknownSubfield("a".to_owned())); diff --git a/boreal/src/evaluator/module.rs b/boreal/src/evaluator/module.rs index 93cf8447..dc99f352 100644 --- a/boreal/src/evaluator/module.rs +++ b/boreal/src/evaluator/module.rs @@ -1,9 +1,11 @@ //! Provides methods to evaluate module values during scanning. -use std::collections::HashMap; +use std::iter::Peekable; +use std::slice::Iter; use std::sync::Arc; -use crate::compiler::expression::Expression; -use crate::compiler::module::{BoundedValueIndex, ModuleExpression, ValueOperation}; +use crate::compiler::module::{ + BoundedValueIndex, ModuleExpression, ModuleOperations, ValueOperation, +}; use crate::module::Value as ModuleValue; use super::{Evaluator, PoisonKind, Value}; @@ -13,7 +15,14 @@ pub(super) fn evaluate_expr( expr: &ModuleExpression, ) -> Result { match expr { - ModuleExpression::BoundedModuleValueUse { index, operations } => { + ModuleExpression::BoundedModuleValueUse { + index, + operations: + ModuleOperations { + expressions, + operations, + }, + } => { let value = match index { BoundedValueIndex::Module(index) => { &evaluator @@ -29,17 +38,44 @@ pub(super) fn evaluate_expr( .ok_or(PoisonKind::Undefined)?, }; let value = Arc::clone(value); - evaluate_ops(evaluator, &value, operations, 0) + + let expressions = expressions + .iter() + .map(|expr| evaluator.evaluate_expr(expr)) + .collect::, PoisonKind>>()?; + evaluate_ops( + evaluator, + &value, + operations.iter().peekable(), + expressions.into_iter(), + ) } - ModuleExpression::Function { + ModuleExpression::StaticFunction { fun, - arguments, - operations, + operations: + ModuleOperations { + expressions, + operations, + }, } => { - let arguments = eval_function_args(evaluator, arguments)?; + let mut ops = operations.iter().peekable(); + let Some(ValueOperation::FunctionCall(nb_arguments)) = ops.next() else { + return Err(PoisonKind::Undefined); + }; + + let expressions: Vec = expressions + .iter() + .map(|expr| evaluator.evaluate_expr(expr)) + .collect::, PoisonKind>>()?; + let mut expressions = expressions.into_iter(); + + let arguments: Vec = (&mut expressions) + .take(*nb_arguments) + .map(expr_value_to_module_value) + .collect(); let value = fun(&evaluator.scan_data.module_ctx, arguments).ok_or(PoisonKind::Undefined)?; - evaluate_ops(evaluator, &value, operations, 0) + evaluate_ops(evaluator, &value, ops, expressions) } } } @@ -47,43 +83,55 @@ pub(super) fn evaluate_expr( fn evaluate_ops( evaluator: &mut Evaluator, mut value: &ModuleValue, - operations: &[ValueOperation], - mut index: usize, + mut operations: Peekable>, + mut expressions: std::vec::IntoIter, ) -> Result { - while index < operations.len() { - match &operations[index] { + while let Some(op) = operations.next() { + match op { ValueOperation::Subfield(subfield) => match value { ModuleValue::Object(map) => { value = map.get(&**subfield).ok_or(PoisonKind::Undefined)?; } _ => return Err(PoisonKind::Undefined), }, - ValueOperation::Subscript(subscript) => match value { - ModuleValue::Array(array) => { - value = eval_array_op(evaluator, subscript, array)?; - } - ModuleValue::Dictionary(dict) => { - value = eval_dict_op(evaluator, subscript, dict)?; - } - _ => return Err(PoisonKind::Undefined), - }, - ValueOperation::FunctionCall(arguments) => match value { + ValueOperation::Subscript => { + let subscript = expressions.next().ok_or(PoisonKind::Undefined)?; + + value = match value { + ModuleValue::Array(array) => { + let index = subscript.unwrap_number()?; + + usize::try_from(index) + .ok() + .and_then(|i| array.get(i)) + .ok_or(PoisonKind::Undefined)? + } + ModuleValue::Dictionary(dict) => { + let val = subscript.unwrap_bytes()?; + + dict.get(&val).ok_or(PoisonKind::Undefined)? + } + _ => return Err(PoisonKind::Undefined), + }; + } + ValueOperation::FunctionCall(nb_arguments) => match value { ModuleValue::Function(fun) => { - let arguments = eval_function_args(evaluator, arguments)?; + let arguments: Vec = (&mut expressions) + .take(*nb_arguments) + .map(expr_value_to_module_value) + .collect(); let new_value = fun(&evaluator.scan_data.module_ctx, arguments) .ok_or(PoisonKind::Undefined)?; // Avoid cloning the value if possible - return if index + 1 >= operations.len() { + return if operations.peek().is_none() { Ok(new_value) } else { - evaluate_ops(evaluator, &new_value, operations, index + 1) + evaluate_ops(evaluator, &new_value, operations, expressions) }; } _ => return Err(PoisonKind::Undefined), }, } - - index += 1; } Ok(value.clone()) @@ -111,43 +159,6 @@ pub(super) fn module_value_to_expr_value(value: ModuleValue) -> Result( - evaluator: &mut Evaluator, - subscript: &Expression, - array: &'a [ModuleValue], -) -> Result<&'a ModuleValue, PoisonKind> { - let index = evaluator.evaluate_expr(subscript)?.unwrap_number()?; - - usize::try_from(index) - .ok() - .and_then(|i| array.get(i)) - .ok_or(PoisonKind::Undefined) -} - -fn eval_dict_op<'a>( - evaluator: &mut Evaluator, - subscript: &Expression, - dict: &'a HashMap, ModuleValue>, -) -> Result<&'a ModuleValue, PoisonKind> { - let val = evaluator.evaluate_expr(subscript)?.unwrap_bytes()?; - - dict.get(&val).ok_or(PoisonKind::Undefined) -} - -fn eval_function_args( - evaluator: &mut Evaluator, - arguments: &[Expression], -) -> Result, PoisonKind> { - arguments - .iter() - .map(|expr| { - evaluator - .evaluate_expr(expr) - .map(expr_value_to_module_value) - }) - .collect() -} - fn expr_value_to_module_value(v: Value) -> ModuleValue { match v { Value::Integer(v) => ModuleValue::Integer(v), From a2caf02e143b18e8e97b900e3e96b9d1013c396a Mon Sep 17 00:00:00 2001 From: Vincent Thiberville Date: Sun, 22 Oct 2023 15:13:52 +0200 Subject: [PATCH 2/3] refacto: rework ModuleExpression object --- boreal/src/compiler/module.rs | 64 ++++++++++++++++------------------ boreal/src/evaluator/module.rs | 53 ++++++++++------------------ 2 files changed, 49 insertions(+), 68 deletions(-) diff --git a/boreal/src/compiler/module.rs b/boreal/src/compiler/module.rs index 96b61222..57d9b7dd 100644 --- a/boreal/src/compiler/module.rs +++ b/boreal/src/compiler/module.rs @@ -31,25 +31,26 @@ pub enum BoundedValueIndex { } /// Different type of expressions related to the use of a module. -pub enum ModuleExpression { +#[derive(Debug)] +pub struct ModuleExpression { + /// Kind of the expression. + pub kind: ModuleExpressionKind, + + /// List of operations to apply to the value to get the final value. + pub operations: ModuleOperations, +} + +pub enum ModuleExpressionKind { /// Operations on a bounded module value. BoundedModuleValueUse { /// Index of the bounded value. index: BoundedValueIndex, - - /// List of operations to apply to the value to get the final value. - operations: ModuleOperations, }, /// A value coming from a function exposed by a module. StaticFunction { /// The function to call. fun: fn(&ScanContext, Vec) -> Option, - - /// List of operations to apply to the value to get the final value. - /// - /// These operations must start with a [`ValueOperation::FunctionCall`] operation. - operations: ModuleOperations, }, } @@ -86,18 +87,16 @@ pub enum ValueOperation { } // XXX: custom Debug impl needed because derive does not work with the fn fields. -impl std::fmt::Debug for ModuleExpression { +impl std::fmt::Debug for ModuleExpressionKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::BoundedModuleValueUse { index, operations } => f + Self::BoundedModuleValueUse { index } => f .debug_struct("BoundedModuleValueUse") .field("index", index) - .field("operations", operations) .finish(), - Self::StaticFunction { fun, operations } => f + Self::StaticFunction { fun } => f .debug_struct("Function") .field("fun", &(*fun as usize)) - .field("operations", operations) .finish(), } } @@ -328,8 +327,10 @@ impl ModuleUse<'_, '_> { fn into_module_expression(self) -> Option<(ModuleExpression, ValueType)> { let ty = self.current_value.into_type()?; - let expr = ModuleExpression::BoundedModuleValueUse { - index: self.bounded_value_index?, + let expr = ModuleExpression { + kind: ModuleExpressionKind::BoundedModuleValueUse { + index: self.bounded_value_index?, + }, operations: ModuleOperations { expressions: self.operations_expressions, operations: self.operations, @@ -351,15 +352,13 @@ impl ModuleUse<'_, '_> { StaticValue::Object(_) => return None, - StaticValue::Function { fun, .. } => { - Expression::Module(ModuleExpression::StaticFunction { - fun: *fun, - operations: ModuleOperations { - expressions: self.operations_expressions, - operations: self.operations, - }, - }) - } + StaticValue::Function { fun, .. } => Expression::Module(ModuleExpression { + kind: ModuleExpressionKind::StaticFunction { fun: *fun }, + operations: ModuleOperations { + expressions: self.operations_expressions, + operations: self.operations, + }, + }), }; let ty = self.current_value.into_type()?; @@ -637,20 +636,19 @@ mod tests { expressions: Vec::new(), operations: Vec::new(), }); - test_type_traits_non_clonable(ModuleExpression::BoundedModuleValueUse { - index: BoundedValueIndex::Module(0), - operations: ModuleOperations { - expressions: Vec::new(), - operations: Vec::new(), + test_type_traits_non_clonable(ModuleExpression { + kind: ModuleExpressionKind::BoundedModuleValueUse { + index: BoundedValueIndex::Module(0), }, - }); - test_type_traits_non_clonable(ModuleExpression::StaticFunction { - fun: test_fun, operations: ModuleOperations { expressions: Vec::new(), operations: Vec::new(), }, }); + test_type_traits_non_clonable(ModuleExpressionKind::BoundedModuleValueUse { + index: BoundedValueIndex::Module(0), + }); + test_type_traits_non_clonable(ModuleExpressionKind::StaticFunction { fun: test_fun }); test_type_traits_non_clonable(IteratorType::Array(ValueType::Integer)); test_type_traits_non_clonable(TypeError::UnknownSubfield("a".to_owned())); test_type_traits_non_clonable(ValueOrType::Type(&ValueType::Integer)); diff --git a/boreal/src/evaluator/module.rs b/boreal/src/evaluator/module.rs index dc99f352..5a781725 100644 --- a/boreal/src/evaluator/module.rs +++ b/boreal/src/evaluator/module.rs @@ -4,7 +4,7 @@ use std::slice::Iter; use std::sync::Arc; use crate::compiler::module::{ - BoundedValueIndex, ModuleExpression, ModuleOperations, ValueOperation, + BoundedValueIndex, ModuleExpression, ModuleExpressionKind, ModuleOperations, ValueOperation, }; use crate::module::Value as ModuleValue; @@ -14,15 +14,21 @@ pub(super) fn evaluate_expr( evaluator: &mut Evaluator, expr: &ModuleExpression, ) -> Result { - match expr { - ModuleExpression::BoundedModuleValueUse { - index, - operations: - ModuleOperations { - expressions, - operations, - }, - } => { + let ModuleOperations { + expressions, + operations, + } = &expr.operations; + + let expressions = expressions + .iter() + .map(|expr| evaluator.evaluate_expr(expr)) + .collect::, PoisonKind>>()?; + let mut expressions = expressions.into_iter(); + + let mut ops = operations.iter().peekable(); + + match &expr.kind { + ModuleExpressionKind::BoundedModuleValueUse { index } => { let value = match index { BoundedValueIndex::Module(index) => { &evaluator @@ -39,36 +45,13 @@ pub(super) fn evaluate_expr( }; let value = Arc::clone(value); - let expressions = expressions - .iter() - .map(|expr| evaluator.evaluate_expr(expr)) - .collect::, PoisonKind>>()?; - evaluate_ops( - evaluator, - &value, - operations.iter().peekable(), - expressions.into_iter(), - ) + evaluate_ops(evaluator, &value, ops, expressions) } - ModuleExpression::StaticFunction { - fun, - operations: - ModuleOperations { - expressions, - operations, - }, - } => { - let mut ops = operations.iter().peekable(); + ModuleExpressionKind::StaticFunction { fun } => { let Some(ValueOperation::FunctionCall(nb_arguments)) = ops.next() else { return Err(PoisonKind::Undefined); }; - let expressions: Vec = expressions - .iter() - .map(|expr| evaluator.evaluate_expr(expr)) - .collect::, PoisonKind>>()?; - let mut expressions = expressions.into_iter(); - let arguments: Vec = (&mut expressions) .take(*nb_arguments) .map(expr_value_to_module_value) From 63cdd749675ab829456f3b133d85fb471a1221ae Mon Sep 17 00:00:00 2001 From: Vincent Thiberville Date: Sun, 22 Oct 2023 15:19:38 +0200 Subject: [PATCH 3/3] feat: remove Arc used around module values Those were needed to fix ownership issues when evaluating module values with operations chains. This has been fixed by moving the evaluation of the expressions used in operations outside of the evaluation of those operations. --- boreal-cli/src/main.rs | 2 +- boreal/src/evaluator/mod.rs | 22 +++++++++------------- boreal/src/evaluator/module.rs | 6 ++---- boreal/src/scanner/mod.rs | 2 +- boreal/tests/it/utils.rs | 3 +-- 5 files changed, 14 insertions(+), 21 deletions(-) diff --git a/boreal-cli/src/main.rs b/boreal-cli/src/main.rs index 0e2a39b9..03c3d5a2 100644 --- a/boreal-cli/src/main.rs +++ b/boreal-cli/src/main.rs @@ -249,7 +249,7 @@ fn scan_file(scanner: &Scanner, path: &Path, options: ScanOptions) -> std::io::R for (module_name, module_value) in res.module_values { // A module value must be an object. Filter out empty ones, it means the module has not // generated any values. - if let ModuleValue::Object(map) = &*module_value { + if let ModuleValue::Object(map) = &module_value { if !map.is_empty() { print!("{module_name}"); print_module_value(&module_value, 4); diff --git a/boreal/src/evaluator/mod.rs b/boreal/src/evaluator/mod.rs index 81fa3fe7..7cb1e0cc 100644 --- a/boreal/src/evaluator/mod.rs +++ b/boreal/src/evaluator/mod.rs @@ -35,7 +35,6 @@ //! - `defined` //! //! For all of those, an undefined value is considered to be equivalent to a false boolean value. -use std::sync::Arc; use std::time::Duration; use crate::compiler::expression::{Expression, ForIterator, ForSelection, VariableIndex}; @@ -114,7 +113,7 @@ impl From for Value { pub struct ScanData<'a> { pub mem: &'a [u8], - pub module_values: Vec<(&'static str, Arc)>, + pub module_values: Vec<(&'static str, ModuleValue)>, // Context used when calling module functions module_ctx: ScanContext<'a>, @@ -153,9 +152,7 @@ impl<'a> ScanData<'a> { .map(|module| { ( module.get_name(), - Arc::new(crate::module::Value::Object( - module.get_dynamic_values(&mut module_ctx), - )), + crate::module::Value::Object(module.get_dynamic_values(&mut module_ctx)), ) }) .collect(), @@ -222,7 +219,7 @@ struct Evaluator<'scan, 'rule> { currently_selected_variable_index: Option, // Stack of bounded identifiers to their integer values. - bounded_identifiers_stack: Vec>, + bounded_identifiers_stack: Vec, // Data related only to the scan, independent of the rule. scan_data: &'rule mut ScanData<'scan>, @@ -845,7 +842,7 @@ impl Evaluator<'_, '_> { match value { ModuleValue::Array(array) => { for value in array { - self.bounded_identifiers_stack.push(Arc::new(value)); + self.bounded_identifiers_stack.push(value); let v = self.evaluate_expr(body); self.bounded_identifiers_stack.truncate(prev_stack_len); let v = match v { @@ -865,9 +862,8 @@ impl Evaluator<'_, '_> { } ModuleValue::Dictionary(dict) => { for (key, value) in dict { - self.bounded_identifiers_stack - .push(Arc::new(ModuleValue::Bytes(key))); - self.bounded_identifiers_stack.push(Arc::new(value)); + self.bounded_identifiers_stack.push(ModuleValue::Bytes(key)); + self.bounded_identifiers_stack.push(value); let v = self.evaluate_expr(body); self.bounded_identifiers_stack.truncate(prev_stack_len); let v = match v { @@ -900,7 +896,7 @@ impl Evaluator<'_, '_> { for value in from..=to { self.bounded_identifiers_stack - .push(Arc::new(ModuleValue::Integer(value))); + .push(ModuleValue::Integer(value)); let v = self.evaluate_expr(body); self.bounded_identifiers_stack.truncate(prev_stack_len); let v = match v { @@ -925,11 +921,11 @@ impl Evaluator<'_, '_> { match self.evaluate_expr(expr)? { Value::Integer(value) => { self.bounded_identifiers_stack - .push(Arc::new(ModuleValue::Integer(value))); + .push(ModuleValue::Integer(value)); } Value::Bytes(value) => { self.bounded_identifiers_stack - .push(Arc::new(ModuleValue::Bytes(value))); + .push(ModuleValue::Bytes(value)); } _ => return Err(PoisonKind::Undefined), } diff --git a/boreal/src/evaluator/module.rs b/boreal/src/evaluator/module.rs index 5a781725..10b60247 100644 --- a/boreal/src/evaluator/module.rs +++ b/boreal/src/evaluator/module.rs @@ -1,7 +1,6 @@ //! Provides methods to evaluate module values during scanning. use std::iter::Peekable; use std::slice::Iter; -use std::sync::Arc; use crate::compiler::module::{ BoundedValueIndex, ModuleExpression, ModuleExpressionKind, ModuleOperations, ValueOperation, @@ -43,9 +42,8 @@ pub(super) fn evaluate_expr( .get(*index) .ok_or(PoisonKind::Undefined)?, }; - let value = Arc::clone(value); - evaluate_ops(evaluator, &value, ops, expressions) + evaluate_ops(evaluator, value, ops, expressions) } ModuleExpressionKind::StaticFunction { fun } => { let Some(ValueOperation::FunctionCall(nb_arguments)) = ops.next() else { @@ -64,7 +62,7 @@ pub(super) fn evaluate_expr( } fn evaluate_ops( - evaluator: &mut Evaluator, + evaluator: &Evaluator, mut value: &ModuleValue, mut operations: Peekable>, mut expressions: std::vec::IntoIter, diff --git a/boreal/src/scanner/mod.rs b/boreal/src/scanner/mod.rs index 5f928bcd..cfc0e171 100644 --- a/boreal/src/scanner/mod.rs +++ b/boreal/src/scanner/mod.rs @@ -485,7 +485,7 @@ pub struct ScanResult<'scanner> { /// On-scan values of all modules used in the scanner. /// /// First element is the module name, second one is the dynamic values produced by the module. - pub module_values: Vec<(&'static str, Arc)>, + pub module_values: Vec<(&'static str, crate::module::Value)>, /// Indicates if evaluation timed out. /// diff --git a/boreal/tests/it/utils.rs b/boreal/tests/it/utils.rs index d077ba57..ca7ca427 100644 --- a/boreal/tests/it/utils.rs +++ b/boreal/tests/it/utils.rs @@ -562,7 +562,7 @@ pub fn compare_module_values_on_mem( let scanner = compiler.into_scanner(); let res = scanner.scan_mem(mem); - let boreal_value = res + let mut boreal_value = res .module_values .into_iter() .find_map(|(name, module_value)| { @@ -575,7 +575,6 @@ pub fn compare_module_values_on_mem( .unwrap(); // Enrich value using the static values, so that it can be compared with yara's - let mut boreal_value = Arc::try_unwrap(boreal_value).unwrap(); enrich_with_static_values(&mut boreal_value, module.get_static_values()); let c = yara::Compiler::new().unwrap();