From e158260bb46a92b67c5d3f3c89e9e485debba9b3 Mon Sep 17 00:00:00 2001 From: "Victor M. Alvarez" Date: Wed, 28 Feb 2024 14:14:46 +0100 Subject: [PATCH] fix: issue with constant folding Constant folding for boolean AND and OR operations was broken. Boolean variables with known values were being folded, even if they were not constant. --- lib/src/compiler/ir/mod.rs | 28 ++++++------- lib/src/scanner/tests.rs | 23 ++++++++--- lib/src/types/mod.rs | 85 ++++++++++++++++++-------------------- 3 files changed, 70 insertions(+), 66 deletions(-) diff --git a/lib/src/compiler/ir/mod.rs b/lib/src/compiler/ir/mod.rs index 8d9f77644..96b1d5146 100644 --- a/lib/src/compiler/ir/mod.rs +++ b/lib/src/compiler/ir/mod.rs @@ -786,15 +786,13 @@ impl Expr { pub fn fold(self) -> Self { match self { Expr::And { mut operands } => { - // Retain the operands whose value is unknown or false, and - // remove those that are known to be true. True values in - // the list of operands don't alter the result of the AND - // operation. + // Retain the operands whose value is not constant, or is + // constant but false, remove those that are known to be + // true. True values in the list of operands don't alter + // the result of the AND operation. operands.retain(|op| { - !op.type_value() - .cast_to_bool() - .try_as_bool() - .unwrap_or(false) + let type_value = op.type_value().cast_to_bool(); + !type_value.is_const() || !type_value.as_bool() }); // No operands left, all were true and therefore the AND is @@ -817,15 +815,13 @@ impl Expr { Expr::And { operands } } Expr::Or { mut operands } => { - // Retain the operands whose value is unknown or true, and - // remove those that are known to be false. False values in - // the list of operands don't alter the result of the OR - // operation. + // Retain the operands whose value is not constant, or is + // constant but true, remove those that are known to be false. + // False values in the list of operands don't alter the result + // of the OR operation. operands.retain(|op| { - op.type_value() - .cast_to_bool() - .try_as_bool() - .unwrap_or(true) + let type_value = op.type_value().cast_to_bool(); + !type_value.is_const() || type_value.as_bool() }); // No operands left, all were false and therefore the OR is diff --git a/lib/src/scanner/tests.rs b/lib/src/scanner/tests.rs index 501ba9366..67eabe84d 100644 --- a/lib/src/scanner/tests.rs +++ b/lib/src/scanner/tests.rs @@ -283,13 +283,16 @@ fn variables_2() { let mut compiler = crate::Compiler::new(); compiler - .define_global("some_int", 0) + .define_global("some_bool", true) + .unwrap() + .define_global("some_str", "") .unwrap() .add_source( r#" rule test { condition: - some_int == 1 + some_bool and + some_str == "foo" } "#, ) @@ -307,17 +310,17 @@ fn variables_2() { 0 ); - scanner.set_global("some_int", 1).unwrap(); + scanner.set_global("some_bool", false).unwrap(); assert_eq!( scanner .scan(&[]) .expect("scan should not fail") .matching_rules() .len(), - 1 + 0 ); - scanner.set_global("some_int", 2).unwrap(); + scanner.set_global("some_str", "foo").unwrap(); assert_eq!( scanner .scan(&[]) @@ -326,6 +329,16 @@ fn variables_2() { .len(), 0 ); + + scanner.set_global("some_bool", true).unwrap(); + assert_eq!( + scanner + .scan(&[]) + .expect("scan should not fail") + .matching_rules() + .len(), + 1 + ); } #[test] diff --git a/lib/src/types/mod.rs b/lib/src/types/mod.rs index 8cb780d25..b0afca073 100644 --- a/lib/src/types/mod.rs +++ b/lib/src/types/mod.rs @@ -17,7 +17,7 @@ pub(crate) use func::*; pub(crate) use map::*; pub(crate) use structure::*; -/// The type of a YARA expression or identifier. +/// The type of YARA expression or identifier. #[derive(Clone, Copy, PartialEq)] pub(crate) enum Type { Unknown, @@ -295,20 +295,6 @@ impl TypeValue { } } - pub fn as_string(&self) -> Rc { - if let TypeValue::String(value) = self { - value - .extract() - .cloned() - .expect("TypeValue doesn't have an associated value") - } else { - panic!( - "called `as_string` on a TypeValue that is not TypeValue::String, it is: {:?}", - self - ) - } - } - pub fn as_array(&self) -> Rc { if let TypeValue::Array(array) = self { array.clone() @@ -353,61 +339,70 @@ impl TypeValue { } } + #[inline] + pub fn as_bool(&self) -> bool { + self.try_as_bool().expect("TypeValue doesn't have an associated value") + } + + #[inline] pub fn as_integer(&self) -> i64 { - if let TypeValue::Integer(value) = self { - value - .extract() - .cloned() - .expect("TypeValue doesn't have an associated value") - } else { - panic!( - "called `as_integer` on a TypeValue that is not TypeValue::Integer, it is: {:?}", - self - ) - } + self.try_as_integer() + .expect("TypeValue doesn't have an associated value") } + #[inline] pub fn as_float(&self) -> f64 { - if let TypeValue::Float(value) = self { - value - .extract() - .cloned() - .expect("TypeValue doesn't have an associated value") + self.try_as_float() + .expect("TypeValue doesn't have an associated value") + } + + #[inline] + pub fn as_string(&self) -> Rc { + self.try_as_string() + .expect("TypeValue doesn't have an associated value") + } + + pub fn try_as_bool(&self) -> Option { + if let TypeValue::Bool(value) = self { + value.extract().cloned() } else { panic!( - "called `as_float` on a TypeValue that is not TypeValue::Float, it is: {:?}", + "called `try_as_bool` on a TypeValue that is not TypeValue::Bool, it is: {:?}", self ) } } - pub fn as_bool(&self) -> bool { - if let TypeValue::Bool(value) = self { - value - .extract() - .cloned() - .expect("TypeValue doesn't have an associated value") + pub fn try_as_integer(&self) -> Option { + if let TypeValue::Integer(value) = self { + value.extract().cloned() } else { panic!( - "called `as_bool` on a TypeValue that is not TypeValue::Bool, it is: {:?}", + "called `try_as_integer` on a TypeValue that is not TypeValue::Integer, it is: {:?}", self ) } } - pub fn try_as_bool(&self) -> Option { - if let TypeValue::Bool(value) = self { + pub fn try_as_float(&self) -> Option { + if let TypeValue::Float(value) = self { value.extract().cloned() } else { - None + panic!( + "called `try_as_float` on a TypeValue that is not TypeValue::Float, it is: {:?}", + self + ) } } - pub fn try_as_integer(&self) -> Option { - if let TypeValue::Integer(value) = self { + pub fn try_as_string(&self) -> Option> { + if let TypeValue::String(value) = self { value.extract().cloned() } else { - None + panic!( + "called `as_string` on a TypeValue that is not TypeValue::String, it is: {:?}", + self + ) } }