Merge pull request #4389 from gsingh93/bitwise-and

Improve range analysis for bitwise and

cpp/ql/src/experimental/semmle/code/cpp/rangeanalysis/ExtendedRangeAnalysis.qll

@@ -2,3 +2,4 @@ import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
 //
 // Import each extension we want to enable
 import extensions.SubtractSelf
+import extensions.ConstantBitwiseAndExprRange

cpp/ql/src/experimental/semmle/code/cpp/rangeanalysis/extensions/ConstantBitwiseAndExprRange.qll

@@ -0,0 +1,90 @@
+private import cpp
+private import experimental.semmle.code.cpp.models.interfaces.SimpleRangeAnalysisExpr
+private import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
+
+/**
+ * Holds if `e` is a constant or if it is a variable with a constant value
+ */
+float evaluateConstantExpr(Expr e) {
+  result = e.getValue().toFloat()
+  or
+  exists(SsaDefinition defn, StackVariable sv |
+    defn.getAUse(sv) = e and
+    result = defn.getDefiningValue(sv).getValue().toFloat()
+  )
+}
+
+/**
+ * The current implementation for `BitwiseAndExpr` only handles cases where both operands are
+ * either unsigned or non-negative constants. This class not only covers these cases, but also
+ * adds support for `&` expressions between a signed integer with a non-negative range and a
+ * non-negative constant. It also adds support for `&=` for the same set of cases as `&`.
+ */
+private class ConstantBitwiseAndExprRange extends SimpleRangeAnalysisExpr {
+  ConstantBitwiseAndExprRange() {
+    exists(Expr l, Expr r |
+      l = this.(BitwiseAndExpr).getLeftOperand() and
+      r = this.(BitwiseAndExpr).getRightOperand()
+      or
+      l = this.(AssignAndExpr).getLValue() and
+      r = this.(AssignAndExpr).getRValue()
+    |
+      // No operands can be negative constants
+      not (evaluateConstantExpr(l) < 0 or evaluateConstantExpr(r) < 0) and
+      // At least one operand must be a non-negative constant
+      (evaluateConstantExpr(l) >= 0 or evaluateConstantExpr(r) >= 0)
+    )
+  }
+
+  Expr getLeftOperand() {
+    result = this.(BitwiseAndExpr).getLeftOperand() or
+    result = this.(AssignAndExpr).getLValue()
+  }
+
+  Expr getRightOperand() {
+    result = this.(BitwiseAndExpr).getRightOperand() or
+    result = this.(AssignAndExpr).getRValue()
+  }
+
+  override float getLowerBounds() {
+    // If an operand can have negative values, the lower bound is unconstrained.
+    // Otherwise, the lower bound is zero.
+    exists(float lLower, float rLower |
+      lLower = getFullyConvertedLowerBounds(getLeftOperand()) and
+      rLower = getFullyConvertedLowerBounds(getRightOperand()) and
+      (
+        (lLower < 0 or rLower < 0) and
+        result = exprMinVal(this)
+        or
+        // This technically results in two lowerBounds when an operand range is negative, but
+        // that's fine since `exprMinVal(x) <= 0`. We can't use an if statement here without
+        // non-monotonic recursion issues
+        result = 0
+      )
+    )
+  }
+
+  override float getUpperBounds() {
+    // If an operand can have negative values, the upper bound is unconstrained.
+    // Otherwise, the upper bound is the minimum of the upper bounds of the operands
+    exists(float lLower, float lUpper, float rLower, float rUpper |
+      lLower = getFullyConvertedLowerBounds(getLeftOperand()) and
+      lUpper = getFullyConvertedUpperBounds(getLeftOperand()) and
+      rLower = getFullyConvertedLowerBounds(getRightOperand()) and
+      rUpper = getFullyConvertedUpperBounds(getRightOperand()) and
+      (
+        (lLower < 0 or rLower < 0) and
+        result = exprMaxVal(this)
+        or
+        // This technically results in two upperBounds when an operand range is negative, but
+        // that's fine since `exprMaxVal(b) >= result`. We can't use an if statement here without
+        // non-monotonic recursion issues
+        result = rUpper.minimum(lUpper)
+      )
+    )
+  }
+
+  override predicate dependsOnChild(Expr child) {
+    child = getLeftOperand() or child = getRightOperand()
+  }
+}

cpp/ql/test/experimental/library-tests/rangeanalysis/bitwiseand/bitwiseand.cpp

@@ -0,0 +1,59 @@
+typedef unsigned char uint8_t;
+typedef signed char int8_t;
+typedef unsigned uint32_t;
+typedef signed long long int64_t;
+
+void test_assign_operator(uint8_t x) {
+  x &= 7; // [0 .. 7]
+}
+
+void test_non_negative_const(uint8_t x) {
+  uint8_t unsigned_const = 7;
+
+  // Non-negative range operand and non-negative constant. The operands are promoted
+  // to signed ints.
+  x & 0; // [0 .. 0]
+  x & 7; // [0 .. 7]
+  x & unsigned_const; // [0 .. 7]
+
+  // This tests what happens when both arguments are promoted to `unsigned int` instead
+  // of `int`, and when the constant is larger than the max bound
+  x & 0xFFFFFFFF; // [0 .. 255]
+}
+
+void test_non_const(uint8_t a, uint8_t b, uint32_t c, uint32_t d) {
+  if (b <= 100) {
+    // `a` and `b` are promoted to signed ints, meaning neither the range analysis library
+    // nor this extension handle it
+    a & b; // [-2147483648 .. 2147483647]
+  }
+  if (d <= 100) {
+    // Handled by the range analysis library
+    c & d; // [0 .. 100]
+  }
+}
+
+void test_negative_operand(uint8_t x, int8_t y) {
+  uint8_t unsigned_const = 7;
+  int8_t signed_const = -7;
+
+  // The right operand can be negative
+  x & -7; // [-2147483648 .. 2147483647]
+  x & signed_const; // [-2147483648 .. 2147483647]
+  x & y; // [-2147483648 .. 2147483647]
+
+  // The left operand can be negative
+  y & 7; // [-2147483648 .. 2147483647]
+  y & unsigned_const; // [-2147483648 .. 2147483647]
+  y & 0xFFFFFFFF; // [0 .. 4294967295]
+  (int64_t)y & 0xFFFFFFFF; // [-9223372036854776000 .. 9223372036854776000]
+  y & x; // [-2147483648 .. 2147483647]
+
+  // Both can be negative
+  y & -7; // [-2147483648 .. 2147483647]
+  y & signed_const; // [-2147483648 .. 2147483647]
+  signed_const & -7; // [-2147483648 .. 2147483647]
+  signed_const & y; // [-2147483648 .. 2147483647]
+  -7 & y; // [-2147483648 .. 2147483647]
+  -7 & signed_const; // [-2147483648 .. 2147483647]
+}

cpp/ql/test/experimental/library-tests/rangeanalysis/bitwiseand/bitwiseand.expected

@@ -0,0 +1,21 @@
+| bitwiseand.cpp:7:3:7:8 | ... &= ... | 0.0 | 7.0 |
+| bitwiseand.cpp:15:3:15:7 | ... & ... | 0.0 | 0.0 |
+| bitwiseand.cpp:16:3:16:7 | ... & ... | 0.0 | 7.0 |
+| bitwiseand.cpp:17:3:17:20 | ... & ... | 0.0 | 7.0 |
+| bitwiseand.cpp:21:3:21:16 | ... & ... | 0.0 | 255.0 |
+| bitwiseand.cpp:28:5:28:9 | ... & ... | -2.147483648E9 | 2.147483647E9 |
+| bitwiseand.cpp:32:5:32:9 | ... & ... | 0.0 | 100.0 |
+| bitwiseand.cpp:41:3:41:8 | ... & ... | -2.147483648E9 | 2.147483647E9 |
+| bitwiseand.cpp:42:3:42:18 | ... & ... | -2.147483648E9 | 2.147483647E9 |
+| bitwiseand.cpp:43:3:43:7 | ... & ... | -2.147483648E9 | 2.147483647E9 |
+| bitwiseand.cpp:46:3:46:7 | ... & ... | -2.147483648E9 | 2.147483647E9 |
+| bitwiseand.cpp:47:3:47:20 | ... & ... | -2.147483648E9 | 2.147483647E9 |
+| bitwiseand.cpp:48:3:48:16 | ... & ... | 0.0 | 4.294967295E9 |
+| bitwiseand.cpp:49:3:49:25 | ... & ... | -9.223372036854776E18 | 9.223372036854776E18 |
+| bitwiseand.cpp:50:3:50:7 | ... & ... | -2.147483648E9 | 2.147483647E9 |
+| bitwiseand.cpp:53:3:53:8 | ... & ... | -2.147483648E9 | 2.147483647E9 |
+| bitwiseand.cpp:54:3:54:18 | ... & ... | -2.147483648E9 | 2.147483647E9 |
+| bitwiseand.cpp:55:3:55:19 | ... & ... | -2.147483648E9 | 2.147483647E9 |
+| bitwiseand.cpp:56:3:56:18 | ... & ... | -2.147483648E9 | 2.147483647E9 |
+| bitwiseand.cpp:57:3:57:8 | ... & ... | -2.147483648E9 | 2.147483647E9 |
+| bitwiseand.cpp:58:3:58:19 | ... & ... | -2.147483648E9 | 2.147483647E9 |

cpp/ql/test/experimental/library-tests/rangeanalysis/bitwiseand/bitwiseand.ql

@@ -0,0 +1,8 @@
+import experimental.semmle.code.cpp.rangeanalysis.ExtendedRangeAnalysis
+
+from Operation expr, float lower, float upper
+where
+  (expr instanceof BitwiseAndExpr or expr instanceof AssignAndExpr) and
+  lower = lowerBound(expr) and
+  upper = upperBound(expr)
+select expr, lower, upper