Substitute more trivial expressions with their results (#138)

cwe_checker_rs/src/intermediate_representation/expression.rs

@@ -81,6 +81,55 @@ impl Expression {
         }
     }
 
+    /// Substitute trivial BinOp-expressions with their results,
+    /// e.g. substitute `a or a` with `a`.
+    ///
+    /// This function assumes that `self` is a `BinOp`
+    /// and it does not substitute trivial expressions in the two input expressions of the `BinOp`.
+    fn substitute_trivial_binops(&mut self) {
+        use BinOpType::*;
+        use Expression::*;
+        if let BinOp { op, lhs, rhs } = self {
+            if lhs == rhs {
+                match op {
+                    BoolAnd | BoolOr | IntAnd | IntOr => {
+                        // This is an identity operation
+                        *self = (**lhs).clone();
+                    }
+                    BoolXOr | IntXOr => {
+                        // `a xor a` always equals zero.
+                        *self = Expression::Const(Bitvector::zero(lhs.bytesize().into()));
+                    }
+                    IntEqual | IntLessEqual | IntSLessEqual => {
+                        *self = Expression::Const(Bitvector::one(ByteSize::new(1).into()));
+                    }
+                    IntNotEqual | IntLess | IntSLess => {
+                        *self = Expression::Const(Bitvector::zero(ByteSize::new(1).into()));
+                    }
+                    _ => (),
+                }
+            } else {
+                match (&**lhs, &**rhs) {
+                    (Const(bitvec), other) | (other, Const(bitvec)) if bitvec.is_zero() => {
+                        if matches!(op, IntOr | IntXOr | BoolOr | BoolXOr) {
+                            // `a or 0 = a` and `a xor 0 = a`
+                            *self = other.clone();
+                        }
+                    }
+                    (Const(bitvec), other) | (other, Const(bitvec))
+                        if bitvec.clone().into_bitnot().is_zero() =>
+                    {
+                        if matches!(op, IntAnd | BoolAnd) {
+                            // `a and -1 = a` since all bits of -1 are 1.
+                            *self = other.clone()
+                        }
+                    }
+                    _ => (),
+                }
+            }
+        }
+    }
+
     /// Substitute some trivial expressions with their result.
     /// E.g. substitute `a XOR a` with zero or substitute `a OR a` with `a`.
     pub fn substitute_trivial_operations(&mut self) {
@@ -106,33 +155,10 @@ impl Expression {
                 }
             }
             UnOp { op: _, arg } => arg.substitute_trivial_operations(),
-            BinOp { op, lhs, rhs } => {
+            BinOp { op: _, lhs, rhs } => {
                 lhs.substitute_trivial_operations();
                 rhs.substitute_trivial_operations();
-                if lhs == rhs {
-                    match op {
-                        BinOpType::BoolAnd
-                        | BinOpType::BoolOr
-                        | BinOpType::IntAnd
-                        | BinOpType::IntOr => {
-                            // This is an identity operation
-                            *self = (**lhs).clone();
-                        }
-                        BinOpType::BoolXOr | BinOpType::IntXOr => {
-                            // `a xor a` always equals zero.
-                            *self = Expression::Const(Bitvector::zero(lhs.bytesize().into()));
-                        }
-                        BinOpType::IntEqual
-                        | BinOpType::IntLessEqual
-                        | BinOpType::IntSLessEqual => {
-                            *self = Expression::Const(Bitvector::one(ByteSize::new(1).into()));
-                        }
-                        BinOpType::IntNotEqual | BinOpType::IntLess | BinOpType::IntSLess => {
-                            *self = Expression::Const(Bitvector::zero(ByteSize::new(1).into()));
-                        }
-                        _ => (),
-                    }
-                }
+                self.substitute_trivial_binops();
             }
         }
     }

cwe_checker_rs/src/intermediate_representation/expression/tests.rs

@@ -116,6 +116,13 @@ fn trivial_expression_substitution() {
         expr,
         Expression::Const(Bitvector::zero(ByteSize::new(8).into()))
     );
+    let mut expr = Expression::BinOp {
+        op: BinOpType::IntOr,
+        lhs: Box::new(setup.rax_variable.clone()),
+        rhs: Box::new(Expression::Const(Bitvector::zero(ByteSize::new(8).into()))),
+    };
+    expr.substitute_trivial_operations();
+    assert_eq!(expr, setup.rax_variable);
 }
 
 #[test]

test/src/lib.rs

@@ -174,13 +174,6 @@ mod tests {
         let mut error_log = Vec::new();
         let mut tests = all_test_cases("cwe_190", "CWE190");
 
-        // Ghidra does not recognize all extern function calls in the disassembly step for MIPS.
-        // Needs own control flow graph analysis to be fixed.
-        mark_skipped(&mut tests, "mips64", "clang");
-        mark_skipped(&mut tests, "mips64el", "clang");
-        mark_skipped(&mut tests, "mips", "gcc");
-        mark_skipped(&mut tests, "mipsel", "gcc");
-
         mark_architecture_skipped(&mut tests, "ppc64"); // Ghidra generates mangled function names here for some reason.
         mark_architecture_skipped(&mut tests, "ppc64le"); // Ghidra generates mangled function names here for some reason.
 
@@ -289,13 +282,6 @@ mod tests {
         let mut error_log = Vec::new();
         let mut tests = all_test_cases("cwe_415", "Memory");
 
-        // Ghidra does not recognize all extern function calls in the disassembly step for MIPS.
-        // Needs own control flow graph analysis to be fixed.
-        mark_architecture_skipped(&mut tests, "mips64");
-        mark_architecture_skipped(&mut tests, "mips64el");
-        mark_architecture_skipped(&mut tests, "mips");
-        mark_architecture_skipped(&mut tests, "mipsel");
-
         mark_architecture_skipped(&mut tests, "ppc64"); // Ghidra generates mangled function names here for some reason.
         mark_architecture_skipped(&mut tests, "ppc64le"); // Ghidra generates mangled function names here for some reason.
 
@@ -324,20 +310,13 @@ mod tests {
         let mut error_log = Vec::new();
         let mut tests = all_test_cases("cwe_416", "Memory");
 
-        // Ghidra does not recognize all extern function calls in the disassembly step for MIPS.
-        // Needs own control flow graph analysis to be fixed.
-        mark_architecture_skipped(&mut tests, "mips64");
-        mark_architecture_skipped(&mut tests, "mips64el");
-        mark_architecture_skipped(&mut tests, "mips");
-        mark_architecture_skipped(&mut tests, "mipsel");
-
         mark_architecture_skipped(&mut tests, "ppc64"); // Ghidra generates mangled function names here for some reason.
         mark_architecture_skipped(&mut tests, "ppc64le"); // Ghidra generates mangled function names here for some reason.
 
         // The analysis loses track of the stack pointer offset in the main() function
         // because of a "INT_AND ESP 0xfffffff0" instruction.
         // We would need knowledge about alignment guarantees for the stack pointer at the start of main() to fix this.
-        mark_architecture_skipped(&mut tests, "x86");
+        mark_skipped(&mut tests, "x86", "gcc");
 
         mark_compiler_skipped(&mut tests, "mingw32-gcc"); // TODO: Check reason for failure!
 
@@ -359,13 +338,6 @@ mod tests {
         let mut error_log = Vec::new();
         let mut tests = all_test_cases("cwe_426", "CWE426");
 
-        // Ghidra does not recognize all extern function calls in the disassembly step for MIPS.
-        // Needs own control flow graph analysis to be fixed.
-        mark_skipped(&mut tests, "mips64", "clang");
-        mark_skipped(&mut tests, "mips64el", "clang");
-        mark_skipped(&mut tests, "mips", "gcc");
-        mark_skipped(&mut tests, "mipsel", "gcc");
-
         mark_architecture_skipped(&mut tests, "ppc64"); // Ghidra generates mangled function names here for some reason.
         mark_architecture_skipped(&mut tests, "ppc64le"); // Ghidra generates mangled function names here for some reason.
 
@@ -395,21 +367,12 @@ mod tests {
         mark_skipped(&mut tests, "arm", "clang");
         mark_skipped(&mut tests, "mips", "clang");
         mark_skipped(&mut tests, "mipsel", "clang");
-
-        // Ghidra does not recognize all extern function calls in the disassembly step for MIPS.
-        // Needs own control flow graph analysis to be fixed.
         mark_skipped(&mut tests, "mips64", "clang");
         mark_skipped(&mut tests, "mips64el", "clang");
-        mark_skipped(&mut tests, "mips", "gcc");
-        mark_skipped(&mut tests, "mipsel", "gcc");
 
         mark_architecture_skipped(&mut tests, "ppc64"); // Ghidra generates mangled function names here for some reason.
         mark_architecture_skipped(&mut tests, "ppc64le"); // Ghidra generates mangled function names here for some reason.
 
-        // This is a bug in the handling of sub-registers.
-        // Register `ECX` is read, but the analysis doesn't know that `ECX` is a sub-register of `RCX`.
-        mark_skipped(&mut tests, "x64", "clang");
-
         mark_compiler_skipped(&mut tests, "mingw32-gcc"); // TODO: Check reason for failure!
 
         for test_case in tests {
@@ -430,12 +393,6 @@ mod tests {
         let mut error_log = Vec::new();
         let mut tests = all_test_cases("cwe_476", "CWE476");
 
-        // TODO: Check reason for failure!
-        mark_architecture_skipped(&mut tests, "mips64");
-        mark_architecture_skipped(&mut tests, "mips64el");
-        mark_architecture_skipped(&mut tests, "mips");
-        mark_architecture_skipped(&mut tests, "mipsel");
-
         mark_architecture_skipped(&mut tests, "ppc64"); // Ghidra generates mangled function names here for some reason.
         mark_architecture_skipped(&mut tests, "ppc64le"); // Ghidra generates mangled function names here for some reason.
 
@@ -459,9 +416,6 @@ mod tests {
         let mut error_log = Vec::new();
         let mut tests = linux_test_cases("cwe_560", "CWE560");
 
-        mark_skipped(&mut tests, "arm", "gcc"); // The parameter is loaded from global memory (which is not supported yet)
-        mark_skipped(&mut tests, "mips", "gcc"); // The parameter is loaded from global memory (which is not supported yet)
-        mark_skipped(&mut tests, "mipsel", "gcc"); // The parameter is loaded from global memory (which is not supported yet)
         mark_architecture_skipped(&mut tests, "ppc64"); // Ghidra generates mangled function names here for some reason.
         mark_architecture_skipped(&mut tests, "ppc64le"); // Ghidra generates mangled function names here for some reason.