CTR55-CPP: address review comments

Author: knewbury01

cpp/cert/src/rules/CTR52-CPP/GuaranteeGenericCppLibraryFunctionsDoNotOverflow.ql

@@ -80,7 +80,7 @@ where
     iteratorCreationCall = outputContainer.getAnIteratorFunctionCall() and
     iteratorCreationCall = c.getOutputIteratorSource()
   |
-    size_compare_bounds_checked(iteratorCreationCall, c)
+    sizeCompareBoundsChecked(iteratorCreationCall, c)
     or
     // Container created with sufficient size for the input
     exists(ContainerAccessWithoutRangeCheck::ContainerConstructorCall outputIteratorConstructor |

cpp/cert/src/rules/CTR55-CPP/DoNotUseAnAdditiveOperatorOnAnIterator.ql

@@ -14,14 +14,14 @@
 import cpp
 import codingstandards.cpp.cert
 import codingstandards.cpp.Iterators
+import semmle.code.cpp.controlflow.Dominance
 
 /**
  * any `.size()` check above our access
  */
-predicate size_checked_above(ContainerIteratorAccess it, IteratorSource source) {
-  exists(STLContainer c, FunctionCall guardCall |
-    c.getACallToSize() = guardCall and
-    guardCall = it.getAPredecessor*() and
+predicate sizeCheckedAbove(ContainerIteratorAccess it, IteratorSource source) {
+  exists(ContainerAccessWithoutRangeCheck::ContainerSizeCall guardCall |
+    strictlyDominates(guardCall, it) and
     //make sure its the same container providing its size as giving the iterator
     globalValueNumber(guardCall.getQualifier()) = globalValueNumber(source.getQualifier()) and
     // and the size call we match must be after the assignment call
@@ -30,16 +30,22 @@ predicate size_checked_above(ContainerIteratorAccess it, IteratorSource source)
 }
 
 /**
- * some loop check exists like: `iterator != end`
+ * some guard exists like: `iterator != end`
  * where a relevant`.end()` call flowed into end
  */
-predicate valid_end_bound_check(ContainerIteratorAccess it, IteratorSource source) {
-  exists(STLContainer c, Loop l, ContainerIteratorAccess otherAccess, IteratorSource end |
+predicate validEndBoundCheck(ContainerIteratorAccess it, IteratorSource source) {
+  exists(
+    STLContainer c, BasicBlock b, GuardCondition l, ContainerIteratorAccess otherAccess,
+    IteratorSource end
+  |
     end = c.getAnIteratorEndFunctionCall() and
-    //flow exists between end() and the loop condition
-    DataFlow::localFlow(DataFlow::exprNode(end), DataFlow::exprNode(l.getCondition().getAChild())) and
-    l.getCondition().getAChild() = otherAccess and
-    //make sure its the same iterator being checked as incremented
+    //guard controls the access
+    l.controls(b, _) and
+    b.contains(it) and
+    //guard is comprised of (anything flowing to) end check and an iterator access
+    DataFlow::localFlow(DataFlow::exprNode(end), DataFlow::exprNode(l.getChild(_))) and
+    l.getChild(_) = otherAccess and
+    //make sure its the same iterator being checked in the guard as accessed
     otherAccess.getOwningContainer() = it.getOwningContainer() and
     //make sure its the same container providing its end as giving the iterator
     globalValueNumber(end.getQualifier()) = globalValueNumber(source.getQualifier())
@@ -52,7 +58,7 @@ where
   it.isAdditiveOperation() and
   not exists(RangeBasedForStmt fs | fs.getUpdate().getAChild*() = it) and
   source = it.getANearbyAssigningIteratorCall() and
-  not size_compare_bounds_checked(source, it) and
-  not valid_end_bound_check(it, source) and
-  not size_checked_above(it, source)
+  not sizeCompareBoundsChecked(source, it) and
+  not validEndBoundCheck(it, source) and
+  not sizeCheckedAbove(it, source)
 select it, "Increment of iterator may overflow since its bounds are not checked."

cpp/common/src/codingstandards/cpp/Iterators.qll

@@ -472,7 +472,7 @@ ControlFlowNode getANonInvalidatedSuccessor(ContainerInvalidationOperation op) {
 /**
  * Guarded by a bounds check that ensures our destination is larger than "some" value
  */
-predicate size_compare_bounds_checked(IteratorSource iteratorCreationCall, Expr guarded) {
+predicate sizeCompareBoundsChecked(IteratorSource iteratorCreationCall, Expr guarded) {
   exists(
     GuardCondition guard, ContainerAccessWithoutRangeCheck::ContainerSizeCall sizeCall,
     boolean branch