Merge pull request #4322 from jbj/range-analysis-custom-defs

C++: Support custom defs in SimpleRangeAnalysis

Author: geoffw0

cpp/ql/src/experimental/semmle/code/cpp/models/interfaces/SimpleRangeAnalysisDefinition.qll

@@ -0,0 +1,65 @@
+/**
+ * EXPERIMENTAL: The API of this module may change without notice.
+ *
+ * Provides a class for modeling `RangeSsaDefinition`s with a restricted range.
+ */
+
+import cpp
+import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
+
+/**
+ * EXPERIMENTAL: The API of this class may change without notice.
+ *
+ * An SSA definition for which a range can be deduced. As with
+ * `RangeSsaDefinition` and `SsaDefinition`, instances of this class
+ * correspond to points in the program where one or more variables are defined
+ * or have their value constrained in some way.
+ *
+ * Extend this class to add functionality to the range analysis library.
+ */
+abstract class SimpleRangeAnalysisDefinition extends RangeSsaDefinition {
+  /**
+   * Holds if this `SimpleRangeAnalysisDefinition` adds range information for
+   * `v`. Because a `SimpleRangeAnalysisDefinition` is just a point in the
+   * program, it's possible that more than one variable might be defined at
+   * this point. This predicate clarifies which variable(s) should get range
+   * information from `this`.
+   *
+   * This predicate **must be overridden** to hold for any `v` that can show
+   * up in the other members of `SimpleRangeAnalysisDefinition`. Conversely,
+   * the other members **must be accurate** for any `v` in this predicate.
+   */
+  abstract predicate hasRangeInformationFor(StackVariable v);
+
+  /**
+   * Holds if `(this, v)` depends on the range of the unconverted expression
+   * `e`. This information is used to inform the range analysis about cyclic
+   * dependencies. Without this information, range analysis might work for
+   * simple cases but will go into infinite loops on complex code.
+   *
+   * For example, when modelling the definition by reference in a call to an
+   * overloaded `operator=`, written as `v = e`, the definition of `(this, v)`
+   * depends on `e`.
+   */
+  abstract predicate dependsOnExpr(StackVariable v, Expr e);
+
+  /**
+   * Gets the lower bound of the variable `v` defined by this definition.
+   *
+   * Implementations of this predicate should use
+   * `getFullyConvertedLowerBounds` and `getFullyConvertedUpperBounds` for
+   * recursive calls to get the bounds of their dependencies.
+   */
+  abstract float getLowerBounds(StackVariable v);
+
+  /**
+   * Gets the upper bound of the variable `v` defined by this definition.
+   *
+   * Implementations of this predicate should use
+   * `getFullyConvertedLowerBounds` and `getFullyConvertedUpperBounds` for
+   * recursive calls to get the bounds of their dependencies.
+   */
+  abstract float getUpperBounds(StackVariable v);
+}
+
+import SimpleRangeAnalysisInternal

cpp/ql/src/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll

@@ -45,6 +45,7 @@
 import cpp
 private import RangeAnalysisUtils
 private import experimental.semmle.code.cpp.models.interfaces.SimpleRangeAnalysisExpr
+private import experimental.semmle.code.cpp.models.interfaces.SimpleRangeAnalysisDefinition
 import RangeSSA
 import SimpleRangeAnalysisCached
 private import NanAnalysis
@@ -335,6 +336,11 @@ private predicate defDependsOnDef(
   or
   // Phi nodes.
   phiDependsOnDef(def, v, srcDef, srcVar)
+  or
+  // Extensions
+  exists(Expr expr | def.(SimpleRangeAnalysisDefinition).dependsOnExpr(v, expr) |
+    exprDependsOnDef(expr, srcDef, srcVar)
+  )
 }
 
 /**
@@ -492,6 +498,9 @@ private predicate analyzableDef(RangeSsaDefinition def, StackVariable v) {
   v = def.getAVariable()
   or
   phiDependsOnDef(def, v, _, _)
+  or
+  // A modeled def for range analysis
+  def.(SimpleRangeAnalysisDefinition).hasRangeInformationFor(v)
 }
 
 /**
@@ -1215,6 +1224,9 @@ private float getDefLowerBoundsImpl(RangeSsaDefinition def, StackVariable v) {
   // Phi nodes.
   result = getPhiLowerBounds(v, def)
   or
+  // A modeled def for range analysis
+  result = def.(SimpleRangeAnalysisDefinition).getLowerBounds(v)
+  or
   // Unanalyzable definitions.
   unanalyzableDefBounds(def, v, result, _)
 }
@@ -1248,6 +1260,9 @@ private float getDefUpperBoundsImpl(RangeSsaDefinition def, StackVariable v) {
   // Phi nodes.
   result = getPhiUpperBounds(v, def)
   or
+  // A modeled def for range analysis
+  result = def.(SimpleRangeAnalysisDefinition).getUpperBounds(v)
+  or
   // Unanalyzable definitions.
   unanalyzableDefBounds(def, v, _, result)
 }

cpp/ql/test/experimental/library-tests/rangeanalysis/extensibility/extensibility.c

@@ -11,4 +11,9 @@ int test_extensibility_add(int x) {
 int test_overridability_sub(int x) {
   int result = x - (unsigned char)x; // Returns 0 due to custom modeling for this test being deliberately wrong
   return result; // 0
-}
+}
+
+void test_parameter_override(int magic_name_at_most_10, int magic_name_at_most_20) {
+  magic_name_at_most_10;
+  magic_name_at_most_20;
+}

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

@@ -5,3 +5,5 @@
 | extensibility.c:12:16:12:16 | x | -2.147483648E9 | 2.147483647E9 |
 | extensibility.c:12:35:12:35 | x | -2.147483648E9 | 2.147483647E9 |
 | extensibility.c:13:10:13:15 | result | 0.0 | 0.0 |
+| extensibility.c:17:3:17:23 | magic_name_at_most_10 | -2.147483648E9 | 10.0 |
+| extensibility.c:18:3:18:23 | magic_name_at_most_20 | -2.147483648E9 | 20.0 |

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

@@ -1,5 +1,7 @@
 import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
+import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
 import experimental.semmle.code.cpp.models.interfaces.SimpleRangeAnalysisExpr
+import experimental.semmle.code.cpp.models.interfaces.SimpleRangeAnalysisDefinition
 
 class CustomAddFunctionCall extends SimpleRangeAnalysisExpr, FunctionCall {
   CustomAddFunctionCall() { this.getTarget().hasGlobalName("custom_add_function") }
@@ -37,6 +39,40 @@ class SelfSub extends SimpleRangeAnalysisExpr, SubExpr {
   override predicate dependsOnChild(Expr child) { child = this.getAnOperand() }
 }
 
+/**
+ * A definition for test purposes of a parameter `p` that starts with a
+ * special prefix. This class is written to exploit how QL behaves when class
+ * fields are not functionally determined by `this`. When multiple parameters
+ * of the same function have the special prefix, there is still only one
+ * instance of this class.
+ */
+class MagicParameterName extends SimpleRangeAnalysisDefinition {
+  Parameter p;
+  float value;
+
+  MagicParameterName() {
+    this.definedByParameter(p) and
+    value = p.getName().regexpCapture("magic_name_at_most_(\\d+)", 1).toFloat()
+  }
+
+  override predicate hasRangeInformationFor(StackVariable v) { v = p }
+
+  override predicate dependsOnExpr(StackVariable v, Expr e) {
+    // No dependencies. This sample class yields constant values.
+    none()
+  }
+
+  override float getLowerBounds(StackVariable var) {
+    var = p and
+    result = typeLowerBound(p.getUnspecifiedType())
+  }
+
+  override float getUpperBounds(StackVariable var) {
+    var = p and
+    result = value
+  }
+}
+
 from VariableAccess expr, float lower, float upper
 where
   lower = lowerBound(expr) and