Skip to content

Commit

Permalink
fix: issue with constant folding
Browse files Browse the repository at this point in the history
Constant folding for boolean AND and OR operations was broken. Boolean variables with known values were being folded, even if they were not constant.
  • Loading branch information
plusvic committed Feb 28, 2024
1 parent d2f6626 commit e158260
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 66 deletions.
28 changes: 12 additions & 16 deletions lib/src/compiler/ir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
23 changes: 18 additions & 5 deletions lib/src/scanner/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
"#,
)
Expand All @@ -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(&[])
Expand All @@ -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]
Expand Down
85 changes: 40 additions & 45 deletions lib/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -295,20 +295,6 @@ impl TypeValue {
}
}

pub fn as_string(&self) -> Rc<BString> {
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<Array> {
if let TypeValue::Array(array) = self {
array.clone()
Expand Down Expand Up @@ -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<BString> {
self.try_as_string()
.expect("TypeValue doesn't have an associated value")
}

pub fn try_as_bool(&self) -> Option<bool> {
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<i64> {
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<bool> {
if let TypeValue::Bool(value) = self {
pub fn try_as_float(&self) -> Option<f64> {
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<i64> {
if let TypeValue::Integer(value) = self {
pub fn try_as_string(&self) -> Option<Rc<BString>> {
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
)
}
}

Expand Down

0 comments on commit e158260

Please sign in to comment.