Merge pull request github#19155 from aschackmull/cpp/iterator-ssa

C++: Refactor Iterator SSA flow to use the data flow integration module.

Author: aschackmull

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

@@ -1834,15 +1834,55 @@ module IteratorFlow {
 
   private module IteratorSsa = SsaImpl::Make<Location, SsaInput>;
 
-  private class Def extends IteratorSsa::DefinitionExt {
+  private module DataFlowIntegrationInput implements IteratorSsa::DataFlowIntegrationInputSig {
+    private import codeql.util.Void
+
+    class Expr extends Instruction {
+      Expr() {
+        exists(IRBlock bb, int i |
+          SsaInput::variableRead(bb, i, _, true) and
+          this = bb.getInstruction(i)
+        )
+      }
+
+      predicate hasCfgNode(SsaInput::BasicBlock bb, int i) { bb.getInstruction(i) = this }
+    }
+
+    predicate ssaDefHasSource(IteratorSsa::WriteDefinition def) { none() }
+
+    predicate allowFlowIntoUncertainDef(IteratorSsa::UncertainWriteDefinition def) { any() }
+
+    class Guard extends Void {
+      predicate controlsBranchEdge(
+        SsaInput::BasicBlock bb1, SsaInput::BasicBlock bb2, boolean branch
+      ) {
+        none()
+      }
+    }
+
+    predicate guardDirectlyControlsBlock(Guard guard, SsaInput::BasicBlock bb, boolean branch) {
+      none()
+    }
+
+    predicate supportBarrierGuardsOnPhiEdges() { none() }
+  }
+
+  private module DataFlowIntegrationImpl =
+    IteratorSsa::DataFlowIntegration<DataFlowIntegrationInput>;
+
+  private class IteratorSynthNode extends DataFlowIntegrationImpl::SsaNode {
+    IteratorSynthNode() { not this.asDefinition() instanceof IteratorSsa::WriteDefinition }
+  }
+
+  private class Def extends IteratorSsa::Definition {
     final override Location getLocation() { result = this.getImpl().getLocation() }
 
     /**
      * Holds if this definition (or use) has index `index` in block `block`,
      * and is a definition (or use) of the variable `sv`.
      */
     predicate hasIndexInBlock(IRBlock block, int index, SourceVariable sv) {
-      super.definesAt(sv, block, index, _)
+      super.definesAt(sv, block, index)
     }
 
     private Ssa::DefImpl getImpl() {
@@ -1859,46 +1899,15 @@ module IteratorFlow {
     int getIndirectionIndex() { result = this.getImpl().getIndirectionIndex() }
   }
 
-  private class PhiNode extends IteratorSsa::DefinitionExt {
-    PhiNode() {
-      this instanceof IteratorSsa::PhiNode or
-      this instanceof IteratorSsa::PhiReadNode
-    }
-
-    SsaIteratorNode getNode() { result.getIteratorFlowNode() = this }
-  }
-
-  cached
-  private module IteratorSsaCached {
-    cached
-    predicate adjacentDefRead(IRBlock bb1, int i1, SourceVariable sv, IRBlock bb2, int i2) {
-      IteratorSsa::adjacentDefReadExt(_, sv, bb1, i1, bb2, i2)
-      or
-      exists(PhiNode phi |
-        IteratorSsa::lastRefRedefExt(_, sv, bb1, i1, phi) and
-        phi.definesAt(sv, bb2, i2, _)
-      )
-    }
-
-    cached
-    Node getAPriorDefinition(IteratorSsa::DefinitionExt next) {
-      exists(IRBlock bb, int i, SourceVariable sv, IteratorSsa::DefinitionExt def |
-        IteratorSsa::lastRefRedefExt(pragma[only_bind_into](def), pragma[only_bind_into](sv),
-          pragma[only_bind_into](bb), pragma[only_bind_into](i), next) and
-        nodeToDefOrUse(result, sv, bb, i, _)
-      )
-    }
-  }
-
   /** The set of nodes necessary for iterator flow. */
-  class IteratorFlowNode instanceof PhiNode {
+  class IteratorFlowNode instanceof IteratorSynthNode {
     /** Gets a textual representation of this node. */
     string toString() { result = super.toString() }
 
     /** Gets the type of this node. */
     DataFlowType getType() {
       exists(Ssa::SourceVariable sv |
-        super.definesAt(sv, _, _, _) and
+        super.getSourceVariable() = sv and
         result = sv.getType()
       )
     }
@@ -1910,60 +1919,33 @@ module IteratorFlow {
     Location getLocation() { result = super.getBasicBlock().getLocation() }
   }
 
-  private import IteratorSsaCached
-
-  private predicate defToNode(Node node, Def def, boolean uncertain) {
-    (
-      nodeHasOperand(node, def.getValue().asOperand(), def.getIndirectionIndex())
-      or
-      nodeHasInstruction(node, def.getValue().asInstruction(), def.getIndirectionIndex())
-    ) and
-    uncertain = false
-  }
-
-  private predicate nodeToDefOrUse(
-    Node node, SourceVariable sv, IRBlock bb, int i, boolean uncertain
-  ) {
-    exists(Def def |
-      def.hasIndexInBlock(bb, i, sv) and
-      defToNode(node, def, uncertain)
-    )
+  private predicate defToNode(Node node, Def def) {
+    nodeHasOperand(node, def.getValue().asOperand(), def.getIndirectionIndex())
     or
-    useToNode(bb, i, sv, node) and
-    uncertain = false
+    nodeHasInstruction(node, def.getValue().asInstruction(), def.getIndirectionIndex())
   }
 
-  private predicate useToNode(IRBlock bb, int i, SourceVariable sv, Node nodeTo) {
-    exists(PhiNode phi |
-      phi.definesAt(sv, bb, i, _) and
-      nodeTo = phi.getNode()
-    )
+  bindingset[result, v]
+  pragma[inline_late]
+  private DataFlowIntegrationImpl::Node fromDfNode(Node n, SourceVariable v) {
+    result = n.(SsaIteratorNode).getIteratorFlowNode()
     or
-    exists(Ssa::UseImpl use |
-      use.hasIndexInBlock(bb, i, sv) and
-      nodeTo = use.getNode()
+    exists(Ssa::UseImpl use, IRBlock bb, int i |
+      result.(DataFlowIntegrationImpl::ExprNode).getExpr().hasCfgNode(bb, i) and
+      use.hasIndexInBlock(bb, i, v) and
+      use.getNode() = n
     )
+    or
+    defToNode(n, result.(DataFlowIntegrationImpl::SsaDefinitionNode).getDefinition())
   }
 
   /**
    * Holds if `nodeFrom` flows to `nodeTo` in a single step.
    */
   predicate localFlowStep(Node nodeFrom, Node nodeTo) {
-    exists(
-      Node nFrom, SourceVariable sv, IRBlock bb1, int i1, IRBlock bb2, int i2, boolean uncertain
-    |
-      adjacentDefRead(bb1, i1, sv, bb2, i2) and
-      nodeToDefOrUse(nFrom, sv, bb1, i1, uncertain) and
-      useToNode(bb2, i2, sv, nodeTo)
-    |
-      if uncertain = true
-      then
-        nodeFrom =
-          [
-            nFrom,
-            getAPriorDefinition(any(IteratorSsa::DefinitionExt next | next.definesAt(sv, bb1, i1, _)))
-          ]
-      else nFrom = nodeFrom
+    exists(SourceVariable v |
+      nodeFrom != nodeTo and
+      DataFlowIntegrationImpl::localFlowStep(v, fromDfNode(nodeFrom, v), fromDfNode(nodeTo, v), _)
     )
   }
 }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll

@@ -1069,7 +1069,7 @@ module BarrierGuard<guardChecksNodeSig/3 guardChecksNode> {
 
 bindingset[result, v]
 pragma[inline_late]
-DataFlowIntegrationImpl::Node fromDfNode(Node n, SourceVariable v) {
+private DataFlowIntegrationImpl::Node fromDfNode(Node n, SourceVariable v) {
   result = n.(SsaSynthNode).getSynthNode()
   or
   exists(UseImpl use, IRBlock bb, int i |