Merge pull request #4162 from jbj/ssa-ref-parameters

C++: SSA and range analysis for reference parameters

cpp/ql/src/semmle/code/cpp/controlflow/SSAUtils.qll

@@ -73,8 +73,20 @@ private predicate addressTakenVariable(StackVariable var) {
   )
 }
 
+/**
+ * Holds if `v` is a stack-allocated reference-typed local variable. We don't
+ * build SSA for such variables since they are likely to change values even
+ * when not syntactically mentioned. For the same reason,
+ * `addressTakenVariable` is used to prevent tracking variables that may be
+ * aliased by such a reference.
+ *
+ * Reference-typed parameters are treated as if they weren't references.
+ * That's because it's in practice highly unlikely that they alias other data
+ * accessible from the function body.
+ */
 private predicate isReferenceVar(StackVariable v) {
-  v.getUnspecifiedType() instanceof ReferenceType
+  v.getUnspecifiedType() instanceof ReferenceType and
+  not v instanceof Parameter
 }
 
 /**

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

@@ -191,6 +191,8 @@ private predicate linearAccessImpl(Expr expr, VariableAccess v, float p, float q
   // Base case
   expr = v and p = 1.0 and q = 0.0
   or
+  expr.(ReferenceDereferenceExpr).getExpr() = v and p = 1.0 and q = 0.0
+  or
   // a+(p*v+b) == p*v + (a+b)
   exists(AddExpr addExpr, float a, float b |
     addExpr.getLeftOperand().isConstant() and
@@ -349,13 +351,20 @@ private predicate typeBounds(ArithmeticType t, float lb, float ub) {
   t instanceof FloatingPointType and lb = -(1.0 / 0.0) and ub = 1.0 / 0.0
 }
 
+private Type stripReference(Type t) {
+  if t instanceof ReferenceType then result = t.(ReferenceType).getBaseType() else result = t
+}
+
+/** Gets the type used by range analysis for the given `StackVariable`. */
+Type getVariableRangeType(StackVariable v) { result = stripReference(v.getUnspecifiedType()) }
+
 /**
  * Gets the lower bound for the unspecified type `t`.
  *
  * For example, if `t` is a signed 32-bit type then the result is
  * `-2^31`.
  */
-float typeLowerBound(ArithmeticType t) { typeBounds(t, result, _) }
+float typeLowerBound(Type t) { typeBounds(stripReference(t), result, _) }
 
 /**
  * Gets the upper bound for the unspecified type `t`.
@@ -363,7 +372,7 @@ float typeLowerBound(ArithmeticType t) { typeBounds(t, result, _) }
  * For example, if `t` is a signed 32-bit type then the result is
  * `2^31 - 1`.
  */
-float typeUpperBound(ArithmeticType t) { typeBounds(t, _, result) }
+float typeUpperBound(Type t) { typeBounds(stripReference(t), _, result) }
 
 /**
  * Gets the minimum value that this expression could represent, based on

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

@@ -485,7 +485,7 @@ private predicate isRecursiveDef(RangeSsaDefinition def, StackVariable v) {
  * This predicate finds all the definitions in the first set.
  */
 private predicate assignmentDef(RangeSsaDefinition def, StackVariable v, Expr expr) {
-  v.getUnspecifiedType() instanceof ArithmeticType and
+  getVariableRangeType(v) instanceof ArithmeticType and
   (
     def = v.getInitializer().getExpr() and def = expr
     or
@@ -1329,7 +1329,7 @@ private float getDefLowerBounds(RangeSsaDefinition def, StackVariable v) {
       // recursion from exploding.
       result =
         max(float widenLB |
-          widenLB = wideningLowerBounds(v.getUnspecifiedType()) and
+          widenLB = wideningLowerBounds(getVariableRangeType(v)) and
           not widenLB > truncatedLB
         |
           widenLB
@@ -1359,7 +1359,7 @@ private float getDefUpperBounds(RangeSsaDefinition def, StackVariable v) {
       // from exploding.
       result =
         min(float widenUB |
-          widenUB = wideningUpperBounds(v.getUnspecifiedType()) and
+          widenUB = wideningUpperBounds(getVariableRangeType(v)) and
           not widenUB < truncatedUB
         |
           widenUB
@@ -1391,9 +1391,10 @@ private predicate unanalyzableDefBounds(RangeSsaDefinition def, StackVariable v,
  */
 bindingset[guard, v, branch]
 predicate nonNanGuardedVariable(ComparisonOperation guard, VariableAccess v, boolean branch) {
-  v.getUnspecifiedType() instanceof IntegralType
+  getVariableRangeType(v.getTarget()) instanceof IntegralType
   or
-  v.getUnspecifiedType() instanceof FloatingPointType and v instanceof NonNanVariableAccess
+  getVariableRangeType(v.getTarget()) instanceof FloatingPointType and
+  v instanceof NonNanVariableAccess
   or
   // The reason the following case is here is to ensure that when we say
   // `if (x > 5) { ...then... } else { ...else... }`
@@ -1418,7 +1419,7 @@ private predicate lowerBoundFromGuard(
     then
       if
         strictness = Nonstrict() or
-        not v.getUnspecifiedType() instanceof IntegralType
+        not getVariableRangeType(v.getTarget()) instanceof IntegralType
       then lb = childLB
       else lb = childLB + 1
     else lb = varMinVal(v.getTarget())
@@ -1440,7 +1441,7 @@ private predicate upperBoundFromGuard(
     then
       if
         strictness = Nonstrict() or
-        not v.getUnspecifiedType() instanceof IntegralType
+        not getVariableRangeType(v.getTarget()) instanceof IntegralType
       then ub = childUB
       else ub = childUB - 1
     else ub = varMaxVal(v.getTarget())

cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/lowerBound.expected

@@ -573,3 +573,15 @@
 | test.cpp:75:22:75:30 | c_times_2 | 0 |
 | test.cpp:77:5:77:13 | c_times_2 | 0 |
 | test.cpp:79:3:79:11 | c_times_2 | 0 |
+| test.cpp:83:16:83:22 | aliased | -2147483648 |
+| test.cpp:85:7:85:7 | i | -2147483648 |
+| test.cpp:86:12:86:12 | i | 2 |
+| test.cpp:88:7:88:8 | ci | -2147483648 |
+| test.cpp:89:12:89:13 | ci | 2 |
+| test.cpp:91:7:91:13 | aliased | -2147483648 |
+| test.cpp:92:12:92:18 | aliased | -2147483648 |
+| test.cpp:94:7:94:11 | alias | -2147483648 |
+| test.cpp:95:12:95:16 | alias | -2147483648 |
+| test.cpp:97:10:97:10 | i | -2147483648 |
+| test.cpp:97:22:97:22 | i | -2147483648 |
+| test.cpp:98:5:98:5 | i | -2147483648 |

cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/test.cpp

@@ -78,3 +78,25 @@ void use_after_cast(unsigned char c)
   }
   c_times_2;
 }
+
+int ref_to_number(int &i, const int &ci, int &aliased) {
+  int &alias = aliased; // no range analysis for either of the two aliased variables
+
+  if (i >= 2)
+    return i;
+
+  if (ci >= 2)
+    return ci;
+
+  if (aliased >= 2)
+    return aliased;
+
+  if (alias >= 2)
+    return alias;
+
+  for (; i <= 12345; i++) { // test that widening works for references
+    i;
+  }
+
+  return 0;
+}

cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/upperBound.expected

@@ -573,3 +573,15 @@
 | test.cpp:75:22:75:30 | c_times_2 | 510 |
 | test.cpp:77:5:77:13 | c_times_2 | 510 |
 | test.cpp:79:3:79:11 | c_times_2 | 510 |
+| test.cpp:83:16:83:22 | aliased | 2147483647 |
+| test.cpp:85:7:85:7 | i | 2147483647 |
+| test.cpp:86:12:86:12 | i | 2147483647 |
+| test.cpp:88:7:88:8 | ci | 2147483647 |
+| test.cpp:89:12:89:13 | ci | 2147483647 |
+| test.cpp:91:7:91:13 | aliased | 2147483647 |
+| test.cpp:92:12:92:18 | aliased | 2147483647 |
+| test.cpp:94:7:94:11 | alias | 2147483647 |
+| test.cpp:95:12:95:16 | alias | 2147483647 |
+| test.cpp:97:10:97:10 | i | 65535 |
+| test.cpp:97:22:97:22 | i | 32767 |
+| test.cpp:98:5:98:5 | i | 32767 |