Merge pull request #2918 from jbj/UnsignedGEZero-recursion

C++: Fix performance of UnsignedGEZero.ql

Author: geoffw0

cpp/ql/src/Likely Bugs/Arithmetic/UnsignedGEZero.qll

@@ -15,24 +15,33 @@ class ConstantZero extends Expr {
   }
 }
 
+/**
+ * Holds if `candidate` is an expression such that if it's unsigned then we
+ * want an alert at `ge`.
+ */
+private predicate lookForUnsignedAt(GEExpr ge, Expr candidate) {
+  // Base case: `candidate >= 0`
+  ge.getRightOperand() instanceof ConstantZero and
+  candidate = ge.getLeftOperand().getFullyConverted() and
+  // left operand was a signed or unsigned IntegralType before conversions
+  // (not a pointer, checking a pointer >= 0 is an entirely different mistake)
+  // (not an enum, as the fully converted type of an enum is compiler dependent
+  //  so checking an enum >= 0 is always reasonable)
+  ge.getLeftOperand().getUnderlyingType() instanceof IntegralType
+  or
+  // Recursive case: `...(largerType)candidate >= 0`
+  exists(Conversion conversion |
+    lookForUnsignedAt(ge, conversion) and
+    candidate = conversion.getExpr() and
+    conversion.getType().getSize() > candidate.getType().getSize()
+  )
+}
+
 class UnsignedGEZero extends GEExpr {
   UnsignedGEZero() {
-    this.getRightOperand() instanceof ConstantZero and
-    // left operand was a signed or unsigned IntegralType before conversions
-    // (not a pointer, checking a pointer >= 0 is an entirely different mistake)
-    // (not an enum, as the fully converted type of an enum is compiler dependent
-    //  so checking an enum >= 0 is always reasonable)
-    getLeftOperand().getUnderlyingType() instanceof IntegralType and
     exists(Expr ue |
-      // ue is some conversion of the left operand
-      ue = getLeftOperand().getConversion*() and
-      // ue is unsigned
-      ue.getUnderlyingType().(IntegralType).isUnsigned() and
-      // ue may be converted to zero or more strictly larger possibly signed types
-      // before it is fully converted
-      forall(Expr following | following = ue.getConversion+() |
-        following.getType().getSize() > ue.getType().getSize()
-      )
+      lookForUnsignedAt(this, ue) and
+      ue.getUnderlyingType().(IntegralType).isUnsigned()
     )
   }
 }