Merge pull request #16345 from MathiasVP/cleanup-ssa-and-iterator-flow

C++: Clean up SSA and stop relying on memory edges for iterator flow

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

@@ -1665,3 +1665,311 @@ class DataFlowSecondLevelScope extends TDataFlowSecondLevelScope {
 
 /** Gets the second-level scope containing the node `n`, if any. */
 DataFlowSecondLevelScope getSecondLevelScope(Node n) { result.getANode() = n }
+
+/**
+ * Module that defines flow through iterators.
+ * For example,
+ * ```cpp
+ * auto it = v.begin();
+ * *it = source();
+ * ...
+ * sink(v[0]);
+ * ```
+ */
+module IteratorFlow {
+  private import codeql.ssa.Ssa as SsaImpl
+  private import semmle.code.cpp.models.interfaces.Iterator as Interface
+  private import semmle.code.cpp.models.implementations.Iterator as Impl
+
+  /**
+   * A variable of some type that can produce an iterator.
+   */
+  class SourceVariable extends Ssa::SourceVariable {
+    SourceVariable() {
+      exists(Interface::GetIteratorFunction gets, Cpp::FunctionInput input, int i |
+        input.isParameterDerefOrQualifierObject(i) and
+        gets.getsIterator(input, _)
+      |
+        this.getType().stripType() = gets.getParameter(i).getType().stripType()
+        or
+        i = -1 and
+        this.getType().stripType() = gets.getDeclaringType()
+      )
+    }
+  }
+
+  private module SsaInput implements SsaImpl::InputSig<Location> {
+    import Ssa::InputSigCommon
+
+    class SourceVariable = IteratorFlow::SourceVariable;
+
+    /** A call to function that dereferences an iterator. */
+    private class IteratorPointerDereferenceCall extends CallInstruction {
+      IteratorPointerDereferenceCall() {
+        this.getStaticCallTarget() instanceof Impl::IteratorPointerDereferenceOperator
+      }
+    }
+
+    /** A call to a function that obtains an iterator. */
+    private class GetsIteratorCall extends CallInstruction {
+      GetsIteratorCall() { this.getStaticCallTarget() instanceof Impl::GetIteratorFunction }
+    }
+
+    /** A call to `operator++` or `operator--` on an iterator. */
+    private class IteratorCrementCall extends CallInstruction {
+      IteratorCrementCall() { this.getStaticCallTarget() instanceof Impl::IteratorCrementOperator }
+    }
+
+    /**
+     * Gets an ultimate definition of `def`.
+     *
+     * Note: Unlike `def.getAnUltimateDefinition()` this predicate also
+     * traverses back through iterator increment and decrement operations.
+     */
+    private Ssa::Def getAnUltimateDefinition(Ssa::Def def) {
+      result = def.getAnUltimateDefinition()
+      or
+      exists(IRBlock bb, int i, IteratorCrementCall crementCall, Ssa::SourceVariable sv |
+        crementCall = def.getValue().asInstruction().(StoreInstruction).getSourceValue() and
+        sv = def.getSourceVariable() and
+        bb.getInstruction(i) = crementCall and
+        Ssa::ssaDefReachesRead(sv, result.asDef(), bb, i)
+      )
+    }
+
+    /**
+     * Holds if `write` is an instruction that writes to address `address`
+     */
+    private predicate isIteratorWrite(Instruction write, Operand address) {
+      exists(Ssa::DefImpl writeDef, IRBlock bb, int i |
+        writeDef.hasIndexInBlock(bb, i, _) and
+        bb.getInstruction(i) = write and
+        address = writeDef.getAddressOperand()
+      )
+    }
+
+    /**
+     * Holds if `writeToDeref` is a write to an iterator that was obtained
+     * by `beginCall`. That is, the following instruction sequence holds:
+     * ```cpp
+     * it = container.begin(); // or a similar iterator-obtaining function call
+     * ...
+     * *it = value;
+     * ```
+     */
+    private predicate isIteratorStoreInstruction(
+      GetsIteratorCall beginCall, Instruction writeToDeref
+    ) {
+      exists(
+        StoreInstruction beginStore, IRBlock bbStar, int iStar, Ssa::Def def,
+        IteratorPointerDereferenceCall starCall, Ssa::Def ultimate, Operand address
+      |
+        isIteratorWrite(writeToDeref, address) and
+        operandForFullyConvertedCall(address, starCall) and
+        bbStar.getInstruction(iStar) = starCall and
+        Ssa::ssaDefReachesRead(_, def.asDef(), bbStar, iStar) and
+        ultimate = getAnUltimateDefinition*(def) and
+        beginStore = ultimate.getValue().asInstruction() and
+        operandForFullyConvertedCall(beginStore.getSourceValueOperand(), beginCall)
+      )
+    }
+
+    /**
+     * Holds if `(bb, i)` contains a write to an iterator that may have been obtained
+     * by calling `begin` (or related functions) on the variable `v`.
+     */
+    predicate variableWrite(IRBlock bb, int i, SourceVariable v, boolean certain) {
+      certain = false and
+      exists(GetsIteratorCall beginCall, Instruction writeToDeref, IRBlock bbQual, int iQual |
+        isIteratorStoreInstruction(beginCall, writeToDeref) and
+        bb.getInstruction(i) = writeToDeref and
+        bbQual.getInstruction(iQual) = beginCall and
+        Ssa::variableRead(bbQual, iQual, v, _)
+      )
+    }
+
+    /** Holds if `(bb, i)` reads the container variable `v`. */
+    predicate variableRead(IRBlock bb, int i, SourceVariable v, boolean certain) {
+      Ssa::variableRead(bb, i, v, certain)
+    }
+  }
+
+  private module IteratorSsa = SsaImpl::Make<Location, SsaInput>;
+
+  cached
+  private newtype TSsaDef =
+    TDef(IteratorSsa::DefinitionExt def) or
+    TPhi(PhiNode phi)
+
+  abstract private class SsaDef extends TSsaDef {
+    /** Gets a textual representation of this element. */
+    string toString() { none() }
+
+    /** Gets the underlying non-phi definition or use. */
+    IteratorSsa::DefinitionExt asDef() { none() }
+
+    /** Gets the underlying phi node. */
+    PhiNode asPhi() { none() }
+
+    /** Gets the location of this element. */
+    abstract Location getLocation();
+  }
+
+  private class Def extends TDef, SsaDef {
+    IteratorSsa::DefinitionExt def;
+
+    Def() { this = TDef(def) }
+
+    final override IteratorSsa::DefinitionExt asDef() { result = def }
+
+    final override Location getLocation() { result = this.getImpl().getLocation() }
+
+    /** Gets the variable written to by this definition. */
+    final SourceVariable getSourceVariable() { result = def.getSourceVariable() }
+
+    override string toString() { result = def.toString() }
+
+    /**
+     * 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) {
+      def.definesAt(sv, block, index, _)
+    }
+
+    private Ssa::DefImpl getImpl() {
+      exists(IRBlock bb, int i |
+        this.hasIndexInBlock(bb, i, _) and
+        result.hasIndexInBlock(bb, i)
+      )
+    }
+
+    /** Gets the value written by this definition (i.e., the "right-hand side"). */
+    Node0Impl getValue() { result = this.getImpl().getValue() }
+
+    /** Gets the indirection index of this definition. */
+    int getIndirectionIndex() { result = this.getImpl().getIndirectionIndex() }
+  }
+
+  private class Phi extends TPhi, SsaDef {
+    PhiNode phi;
+
+    Phi() { this = TPhi(phi) }
+
+    final override PhiNode asPhi() { result = phi }
+
+    final override Location getLocation() { result = phi.getBasicBlock().getLocation() }
+
+    override string toString() { result = phi.toString() }
+
+    SsaIteratorNode getNode() { result.getIteratorFlowNode() = phi }
+  }
+
+  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 {
+    /** 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
+        result = sv.getType()
+      )
+    }
+
+    /** Gets the `Declaration` that contains this block. */
+    Declaration getFunction() { result = super.getBasicBlock().getEnclosingFunction() }
+
+    /** Gets the locatino of this node. */
+    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)
+    )
+    or
+    useToNode(bb, i, sv, node) and
+    uncertain = false
+  }
+
+  private predicate useToNode(IRBlock bb, int i, SourceVariable sv, Node nodeTo) {
+    exists(PhiNode phi |
+      phi.definesAt(sv, bb, i, _) and
+      nodeTo = phi.getNode()
+    )
+    or
+    exists(Ssa::UseImpl use |
+      use.hasIndexInBlock(bb, i, sv) and
+      nodeTo = use.getNode()
+    )
+  }
+
+  /**
+   * 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
+    )
+  }
+}

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

@@ -46,6 +46,7 @@ private newtype TIRDataFlowNode =
     Ssa::isModifiableByCall(operand, indirectionIndex)
   } or
   TSsaPhiNode(Ssa::PhiNode phi) or
+  TSsaIteratorNode(IteratorFlow::IteratorFlowNode n) or
   TRawIndirectOperand0(Node0Impl node, int indirectionIndex) {
     Ssa::hasRawIndirectOperand(node.asOperand(), indirectionIndex)
   } or
@@ -653,6 +654,30 @@ class SsaPhiNode extends Node, TSsaPhiNode {
   predicate isPhiRead() { phi.isPhiRead() }
 }
 
+/**
+ * INTERNAL: do not use.
+ *
+ * Dataflow nodes necessary for iterator flow
+ */
+class SsaIteratorNode extends Node, TSsaIteratorNode {
+  IteratorFlow::IteratorFlowNode node;
+
+  SsaIteratorNode() { this = TSsaIteratorNode(node) }
+
+  /** Gets the phi node associated with this node. */
+  IteratorFlow::IteratorFlowNode getIteratorFlowNode() { result = node }
+
+  override Declaration getEnclosingCallable() { result = this.getFunction() }
+
+  override Declaration getFunction() { result = node.getFunction() }
+
+  override DataFlowType getType() { result = node.getType() }
+
+  final override Location getLocationImpl() { result = node.getLocation() }
+
+  override string toStringImpl() { result = node.toString() }
+}
+
 /**
  * INTERNAL: do not use.
  *
@@ -1190,11 +1215,11 @@ class UninitializedNode extends Node {
   LocalVariable v;
 
   UninitializedNode() {
-    exists(Ssa::Def def |
+    exists(Ssa::Def def, Ssa::SourceVariable sv |
       def.getIndirectionIndex() = 0 and
       def.getValue().asInstruction() instanceof UninitializedInstruction and
-      Ssa::nodeToDefOrUse(this, def, _) and
-      v = def.getSourceVariable().getBaseVariable().(Ssa::BaseIRVariable).getIRVariable().getAst()
+      Ssa::defToNode(this, def, sv, _, _, _) and
+      v = sv.getBaseVariable().(Ssa::BaseIRVariable).getIRVariable().getAst()
     )
   }
 
@@ -2151,6 +2176,8 @@ private module Cached {
       // Def-use/Use-use flow
       Ssa::ssaFlow(nodeFrom, nodeTo)
       or
+      IteratorFlow::localFlowStep(nodeFrom, nodeTo)
+      or
       // Operand -> Instruction flow
       simpleInstructionLocalFlowStep(nodeFrom.asOperand(), nodeTo.asInstruction())
       or