Skip to content

Commit

Permalink
fix: handle integer overflows in constant folding
Browse files Browse the repository at this point in the history
  • Loading branch information
@plusvic
plusvic committed May 6, 2024
1 parent c573ec5 commit 8ccad97
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 24 deletions.
15 changes: 10 additions & 5 deletions lib/src/compiler/ir/ast2ir.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1180,6 +1180,7 @@ fn matches_expr_from_ast(
ctx: &mut CompileContext,
expr: &ast::BinaryExpr,
) -> Result<Expr, Box<CompileError>> {
let span = expr.span();
let lhs_span = expr.lhs.span();
let rhs_span = expr.rhs.span();

Expand All @@ -1192,7 +1193,7 @@ fn matches_expr_from_ast(
let expr = Expr::Matches { lhs, rhs };

if cfg!(feature = "constant-folding") {
Ok(expr.fold())
expr.fold(ctx, span)
} else {
Ok(expr)
}
Expand Down Expand Up @@ -1326,6 +1327,7 @@ macro_rules! gen_unary_op {
ctx: &mut CompileContext,
expr: &ast::UnaryExpr,
) -> Result<Expr, Box<CompileError>> {
let span = expr.span();
let operand = Box::new(expr_from_ast(ctx, &expr.operand)?);

check_type(
Expand All @@ -1346,7 +1348,7 @@ macro_rules! gen_unary_op {
let expr = Expr::$variant { operand };

if cfg!(feature = "constant-folding") {
Ok(expr.fold())
expr.fold(ctx, span)
} else {
Ok(expr)
}
Expand All @@ -1360,6 +1362,7 @@ macro_rules! gen_binary_op {
ctx: &mut CompileContext,
expr: &ast::BinaryExpr,
) -> Result<Expr, Box<CompileError>> {
let span = expr.span();
let lhs_span = expr.lhs.span();
let rhs_span = expr.rhs.span();

Expand Down Expand Up @@ -1387,7 +1390,7 @@ macro_rules! gen_binary_op {
let expr = Expr::$variant { lhs, rhs };

if cfg!(feature = "constant-folding") {
Ok(expr.fold())
expr.fold(ctx, span)
} else {
Ok(expr)
}
Expand All @@ -1401,6 +1404,7 @@ macro_rules! gen_string_op {
ctx: &mut CompileContext,
expr: &ast::BinaryExpr,
) -> Result<Expr, Box<CompileError>> {
let span = expr.span();
let lhs_span = expr.lhs.span();
let rhs_span = expr.rhs.span();

Expand All @@ -1420,7 +1424,7 @@ macro_rules! gen_string_op {
let expr = Expr::$variant { lhs, rhs };

if cfg!(feature = "constant-folding") {
Ok(expr.fold())
expr.fold(ctx, span)
} else {
Ok(expr)
}
Expand All @@ -1434,6 +1438,7 @@ macro_rules! gen_n_ary_operation {
ctx: &mut CompileContext,
expr: &ast::NAryExpr,
) -> Result<Expr, Box<CompileError>> {
let span = expr.span();
let accepted_types = &[$( $accepted_types ),+];
let compatible_types = &[$( $compatible_types ),+];

Expand Down Expand Up @@ -1488,7 +1493,7 @@ macro_rules! gen_n_ary_operation {
let expr = Expr::$variant { operands: operands_hir };

if cfg!(feature = "constant-folding") {
Ok(expr.fold())
expr.fold(ctx, span)
} else {
Ok(expr)
}
Expand Down
59 changes: 40 additions & 19 deletions lib/src/compiler/ir/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,15 @@ use bitmask::bitmask;
use bstr::BString;
use serde::{Deserialize, Serialize};

use crate::compiler::context::{Var, VarStackFrame};
use crate::compiler::context::{CompileContext, Var, VarStackFrame};
use crate::symbols::Symbol;
use crate::types::{Type, TypeValue, Value};

pub(in crate::compiler) use ast2ir::bool_expr_from_ast;
pub(in crate::compiler) use ast2ir::patterns_from_ast;
use yara_x_parser::ast::Span;

use crate::re;
use crate::{re, CompileError};

mod ast2ir;
mod hex2hir;
Expand Down Expand Up @@ -781,7 +782,11 @@ impl Expr {
}
}

pub fn fold(self) -> Self {
pub fn fold(
self,
ctx: &mut CompileContext,
span: Span,
) -> Result<Self, Box<CompileError>> {
match self {
Expr::And { mut operands } => {
// Retain the operands whose value is not constant, or is
Expand All @@ -796,17 +801,17 @@ impl Expr {
// No operands left, all were true and therefore the AND is
// also true.
if operands.is_empty() {
return Expr::Const(TypeValue::const_bool_from(true));
return Ok(Expr::Const(TypeValue::const_bool_from(true)));
}

// If any of the remaining operands is constant it has to be
// false because true values were removed, the result is false
// regardless of the operands with unknown values.
if operands.iter().any(|op| op.type_value().is_const()) {
return Expr::Const(TypeValue::const_bool_from(false));
return Ok(Expr::Const(TypeValue::const_bool_from(false)));
}

Expr::And { operands }
Ok(Expr::And { operands })
}
Expr::Or { mut operands } => {
// Retain the operands whose value is not constant, or is
Expand All @@ -821,51 +826,58 @@ impl Expr {
// No operands left, all were false and therefore the OR is
// also false.
if operands.is_empty() {
return Expr::Const(TypeValue::const_bool_from(false));
return Ok(Expr::Const(TypeValue::const_bool_from(false)));
}

// If any of the remaining operands is constant it has to be
// true because false values were removed, the result is true
// regardless of the operands with unknown values.
if operands.iter().any(|op| op.type_value().is_const()) {
return Expr::Const(TypeValue::const_bool_from(true));
return Ok(Expr::Const(TypeValue::const_bool_from(true)));
}

Expr::Or { operands }
Ok(Expr::Or { operands })
}

Expr::Add { operands } => {
// If not all operands are constant, there's nothing to fold.
if !operands.iter().all(|op| op.type_value().is_const()) {
return Expr::Add { operands };
return Ok(Expr::Add { operands });
}

Self::fold_arithmetic(operands, |acc, x| acc + x)
Self::fold_arithmetic(ctx, span, operands, |acc, x| acc + x)
}
Expr::Sub { operands } => {
// If not all operands are constant, there's nothing to fold.
if !operands.iter().all(|op| op.type_value().is_const()) {
return Expr::Sub { operands };
return Ok(Expr::Sub { operands });
}

Self::fold_arithmetic(operands, |acc, x| acc - x)
Self::fold_arithmetic(ctx, span, operands, |acc, x| acc - x)
}
Expr::Mul { operands } => {
// If not all operands are constant, there's nothing to fold.
if !operands.iter().all(|op| op.type_value().is_const()) {
return Expr::Mul { operands };
return Ok(Expr::Mul { operands });
}

Self::fold_arithmetic(operands, |acc, x| acc * x)
Self::fold_arithmetic(ctx, span, operands, |acc, x| acc * x)
}
_ => self,
_ => Ok(self),
}
}

pub fn fold_arithmetic<F>(operands: Vec<Expr>, f: F) -> Self
pub fn fold_arithmetic<F>(
ctx: &mut CompileContext,
span: Span,
operands: Vec<Expr>,
f: F,
) -> Result<Self, Box<CompileError>>
where
F: FnMut(f64, f64) -> f64,
{
debug_assert!(!operands.is_empty());

let mut is_float = false;

let result = operands
Expand All @@ -879,12 +891,21 @@ impl Expr {
_ => unreachable!(),
})
.reduce(f)
// It's safe to call unwrap because there must be at least
// one iterator.
.unwrap();

if is_float {
Expr::Const(TypeValue::const_float_from(result))
Ok(Expr::Const(TypeValue::const_float_from(result)))
} else if result >= i64::MIN as f64 && result <= i64::MAX as f64 {
Ok(Expr::Const(TypeValue::const_integer_from(result as i64)))
} else {
Expr::Const(TypeValue::const_integer_from(result as i64))
Err(Box::new(CompileError::number_out_of_range(
ctx.report_builder,
i64::MIN,
i64::MAX,
span,
)))
}
}
}
4 changes: 4 additions & 0 deletions lib/src/compiler/tests/testdata/errors/124.in
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
rule test {
condition:
9223372036854775807 + 1000000000 == 0
}
6 changes: 6 additions & 0 deletions lib/src/compiler/tests/testdata/errors/124.out
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
error: number out of range
--> line:3:4
|
3 | 9223372036854775807 + 1000000000 == 0
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this number is out of the allowed range [-9223372036854775808-9223372036854775807]
|

0 comments on commit 8ccad97

Please sign in to comment.