Merge branch 'mathiasvp/replace-ast-with-ir-use-usedataflow' into global-flow

cpp/ql/lib/CHANGELOG.md

@@ -1,3 +1,36 @@
+## 0.5.0
+
+### Breaking Changes
+
+The predicates in the `MustFlow::Configuration` class used by the `MustFlow` library (`semmle.code.cpp.ir.dataflow.MustFlow`) have changed to be defined directly in terms of the C++ IR instead of IR dataflow nodes.
+
+### Deprecated APIs
+
+* Deprecated `semmle.code.cpp.ir.dataflow.DefaultTaintTracking`. Use `semmle.code.cpp.ir.dataflow.TaintTracking`.
+* Deprecated `semmle.code.cpp.security.TaintTrackingImpl`. Use `semmle.code.cpp.ir.dataflow.TaintTracking`.
+* Deprecated `semmle.code.cpp.valuenumbering.GlobalValueNumberingImpl`. Use `semmle.code.cpp.valuenumbering.GlobalValueNumbering`, which exposes the same API.
+
+### Minor Analysis Improvements
+
+* The `ArgvSource` flow source now uses the second parameter of `main` as its source instead of the uses of this parameter.
+* The `ArgvSource` flow source has been generalized to handle cases where the argument vector of `main` is not named `argv`.
+* The `getaddrinfo` function is now recognized as a flow source.
+* The `secure_getenv` and `_wgetenv` functions are now recognized as local flow sources.
+* The `scanf` and `fscanf` functions and their variants are now recognized as flow sources.
+* Deleted the deprecated `getName` and `getShortName` predicates from the `Folder` class.
+
+## 0.4.6
+
+No user-facing changes.
+
+## 0.4.5
+
+No user-facing changes.
+
+## 0.4.4
+
+No user-facing changes.
+
 ## 0.4.3
 
 ### Minor Analysis Improvements

cpp/ql/lib/change-notes/released/0.4.4.md

@@ -0,0 +1,3 @@
+## 0.4.4
+
+No user-facing changes.

cpp/ql/lib/change-notes/released/0.4.5.md

@@ -0,0 +1,3 @@
+## 0.4.5
+
+No user-facing changes.

cpp/ql/lib/change-notes/released/0.4.6.md

@@ -0,0 +1,3 @@
+## 0.4.6
+
+No user-facing changes.

cpp/ql/lib/change-notes/released/0.5.0.md

@@ -0,0 +1,20 @@
+## 0.5.0
+
+### Breaking Changes
+
+The predicates in the `MustFlow::Configuration` class used by the `MustFlow` library (`semmle.code.cpp.ir.dataflow.MustFlow`) have changed to be defined directly in terms of the C++ IR instead of IR dataflow nodes.
+
+### Deprecated APIs
+
+* Deprecated `semmle.code.cpp.ir.dataflow.DefaultTaintTracking`. Use `semmle.code.cpp.ir.dataflow.TaintTracking`.
+* Deprecated `semmle.code.cpp.security.TaintTrackingImpl`. Use `semmle.code.cpp.ir.dataflow.TaintTracking`.
+* Deprecated `semmle.code.cpp.valuenumbering.GlobalValueNumberingImpl`. Use `semmle.code.cpp.valuenumbering.GlobalValueNumbering`, which exposes the same API.
+
+### Minor Analysis Improvements
+
+* The `ArgvSource` flow source now uses the second parameter of `main` as its source instead of the uses of this parameter.
+* The `ArgvSource` flow source has been generalized to handle cases where the argument vector of `main` is not named `argv`.
+* The `getaddrinfo` function is now recognized as a flow source.
+* The `secure_getenv` and `_wgetenv` functions are now recognized as local flow sources.
+* The `scanf` and `fscanf` functions and their variants are now recognized as flow sources.
+* Deleted the deprecated `getName` and `getShortName` predicates from the `Folder` class.

cpp/ql/lib/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.4.3
+lastReleaseVersion: 0.5.0

cpp/ql/lib/definitions.qll

@@ -12,8 +12,8 @@ import IDEContextual
  *
  * In some cases it is preferable to modify locations (the
  * `hasLocationInfo()` predicate) so that they are short, and
- * non-overlapping with other locations that might be highlighted in
- * the LGTM interface.
+ * non-overlapping with other locations that might be reported as
+ * code scanning alerts on GitHub.
  *
  * We need to give locations that may not be in the database, so
  * we use `hasLocationInfo()` rather than `getLocation()`.
@@ -159,9 +159,9 @@ Top definitionOf(Top e, string kind) {
     // Multiple type mentions can be generated when a typedef is used, and
     // in such cases we want to exclude all but the originating typedef.
     not exists(Type secondary |
-      exists(TypeMention tm, File f, int startline, int startcol |
+      exists(File f, int startline, int startcol |
         typeMentionStartLoc(e, result, f, startline, startcol) and
-        typeMentionStartLoc(tm, secondary, f, startline, startcol) and
+        typeMentionStartLoc(_, secondary, f, startline, startcol) and
         (
           result = secondary.(TypedefType).getBaseType() or
           result = secondary.(TypedefType).getBaseType().(SpecifiedType).getBaseType()

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll

@@ -915,18 +915,57 @@ private module Cached {
     TDataFlowCallNone() or
     TDataFlowCallSome(DataFlowCall call)
 
+  cached
+  newtype TParamNodeOption =
+    TParamNodeNone() or
+    TParamNodeSome(ParamNode p)
+
+  cached
+  newtype TReturnCtx =
+    TReturnCtxNone() or
+    TReturnCtxNoFlowThrough() or
+    TReturnCtxMaybeFlowThrough(ReturnPosition pos)
+
+  cached
+  newtype TTypedContentApprox =
+    MkTypedContentApprox(ContentApprox c, DataFlowType t) {
+      exists(Content cont |
+        c = getContentApprox(cont) and
+        store(_, cont, _, _, t)
+      )
+    }
+
   cached
   newtype TTypedContent = MkTypedContent(Content c, DataFlowType t) { store(_, c, _, _, t) }
 
+  cached
+  TypedContent getATypedContent(TypedContentApprox c) {
+    exists(ContentApprox cls, DataFlowType t, Content cont |
+      c = MkTypedContentApprox(cls, pragma[only_bind_into](t)) and
+      result = MkTypedContent(cont, pragma[only_bind_into](t)) and
+      cls = getContentApprox(cont)
+    )
+  }
+
   cached
   newtype TAccessPathFront =
     TFrontNil(DataFlowType t) or
     TFrontHead(TypedContent tc)
 
+  cached
+  newtype TApproxAccessPathFront =
+    TApproxFrontNil(DataFlowType t) or
+    TApproxFrontHead(TypedContentApprox tc)
+
   cached
   newtype TAccessPathFrontOption =
     TAccessPathFrontNone() or
     TAccessPathFrontSome(AccessPathFront apf)
+
+  cached
+  newtype TApproxAccessPathFrontOption =
+    TApproxAccessPathFrontNone() or
+    TApproxAccessPathFrontSome(ApproxAccessPathFront apf)
 }
 
 /**
@@ -1304,6 +1343,113 @@ class DataFlowCallOption extends TDataFlowCallOption {
   }
 }
 
+/** An optional `ParamNode`. */
+class ParamNodeOption extends TParamNodeOption {
+  string toString() {
+    this = TParamNodeNone() and
+    result = "(none)"
+    or
+    exists(ParamNode p |
+      this = TParamNodeSome(p) and
+      result = p.toString()
+    )
+  }
+}
+
+/**
+ * A return context used to calculate flow summaries in reverse flow.
+ *
+ * The possible values are:
+ *
+ * - `TReturnCtxNone()`: no return flow.
+ * - `TReturnCtxNoFlowThrough()`: return flow, but flow through is not possible.
+ * - `TReturnCtxMaybeFlowThrough(ReturnPosition pos)`: return flow, of kind `pos`, and
+ *    flow through may be possible.
+ */
+class ReturnCtx extends TReturnCtx {
+  string toString() {
+    this = TReturnCtxNone() and
+    result = "(none)"
+    or
+    this = TReturnCtxNoFlowThrough() and
+    result = "(no flow through)"
+    or
+    exists(ReturnPosition pos |
+      this = TReturnCtxMaybeFlowThrough(pos) and
+      result = pos.toString()
+    )
+  }
+}
+
+/** An approximated `Content` tagged with the type of a containing object. */
+class TypedContentApprox extends MkTypedContentApprox {
+  private ContentApprox c;
+  private DataFlowType t;
+
+  TypedContentApprox() { this = MkTypedContentApprox(c, t) }
+
+  /** Gets a typed content approximated by this value. */
+  TypedContent getATypedContent() { result = getATypedContent(this) }
+
+  /** Gets the container type. */
+  DataFlowType getContainerType() { result = t }
+
+  /** Gets a textual representation of this approximated content. */
+  string toString() { result = c.toString() }
+}
+
+/**
+ * The front of an approximated access path. This is either a head or a nil.
+ */
+abstract class ApproxAccessPathFront extends TApproxAccessPathFront {
+  abstract string toString();
+
+  abstract DataFlowType getType();
+
+  abstract boolean toBoolNonEmpty();
+
+  pragma[nomagic]
+  TypedContent getAHead() {
+    exists(TypedContentApprox cont |
+      this = TApproxFrontHead(cont) and
+      result = cont.getATypedContent()
+    )
+  }
+}
+
+class ApproxAccessPathFrontNil extends ApproxAccessPathFront, TApproxFrontNil {
+  private DataFlowType t;
+
+  ApproxAccessPathFrontNil() { this = TApproxFrontNil(t) }
+
+  override string toString() { result = ppReprType(t) }
+
+  override DataFlowType getType() { result = t }
+
+  override boolean toBoolNonEmpty() { result = false }
+}
+
+class ApproxAccessPathFrontHead extends ApproxAccessPathFront, TApproxFrontHead {
+  private TypedContentApprox tc;
+
+  ApproxAccessPathFrontHead() { this = TApproxFrontHead(tc) }
+
+  override string toString() { result = tc.toString() }
+
+  override DataFlowType getType() { result = tc.getContainerType() }
+
+  override boolean toBoolNonEmpty() { result = true }
+}
+
+/** An optional approximated access path front. */
+class ApproxAccessPathFrontOption extends TApproxAccessPathFrontOption {
+  string toString() {
+    this = TApproxAccessPathFrontNone() and result = "<none>"
+    or
+    this = TApproxAccessPathFrontSome(any(ApproxAccessPathFront apf | result = apf.toString()))
+  }
+}
+
 /** A `Content` tagged with the type of a containing object. */
 class TypedContent extends MkTypedContent {
   private Content c;
@@ -1336,7 +1482,7 @@ abstract class AccessPathFront extends TAccessPathFront {
 
   abstract DataFlowType getType();
 
-  abstract boolean toBoolNonEmpty();
+  abstract ApproxAccessPathFront toApprox();
 
   TypedContent getHead() { this = TFrontHead(result) }
 }
@@ -1350,7 +1496,7 @@ class AccessPathFrontNil extends AccessPathFront, TFrontNil {
 
   override DataFlowType getType() { result = t }
 
-  override boolean toBoolNonEmpty() { result = false }
+  override ApproxAccessPathFront toApprox() { result = TApproxFrontNil(t) }
 }
 
 class AccessPathFrontHead extends AccessPathFront, TFrontHead {
@@ -1362,7 +1508,7 @@ class AccessPathFrontHead extends AccessPathFront, TFrontHead {
 
   override DataFlowType getType() { result = tc.getContainerType() }
 
-  override boolean toBoolNonEmpty() { result = true }
+  override ApproxAccessPathFront toApprox() { result.getAHead() = tc }
 }
 
 /** An optional access path front. */

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll

@@ -101,9 +101,7 @@ module Consistency {
     exists(int c |
       c =
         strictcount(Node n |
-          not exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
-            n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-          ) and
+          not n.hasLocationInfo(_, _, _, _, _) and
           not any(ConsistencyConfiguration conf).missingLocationExclude(n)
         ) and
       msg = "Nodes without location: " + c
@@ -136,6 +134,18 @@ module Consistency {
     msg = "Local flow step does not preserve enclosing callable."
   }
 
+  query predicate readStepIsLocal(Node n1, Node n2, string msg) {
+    readStep(n1, _, n2) and
+    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
+    msg = "Read step does not preserve enclosing callable."
+  }
+
+  query predicate storeStepIsLocal(Node n1, Node n2, string msg) {
+    storeStep(n1, _, n2) and
+    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
+    msg = "Store step does not preserve enclosing callable."
+  }
+
   private DataFlowType typeRepr() { result = getNodeType(_) }
 
   query predicate compatibleTypesReflexive(DataFlowType t, string msg) {
@@ -232,4 +242,25 @@ module Consistency {
     not callable = viableCallable(call) and
     not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable)
   }
+
+  query predicate uniqueParameterNodeAtPosition(
+    DataFlowCallable c, ParameterPosition pos, Node p, string msg
+  ) {
+    isParameterNode(p, c, pos) and
+    not exists(unique(Node p0 | isParameterNode(p0, c, pos))) and
+    msg = "Parameters with overlapping positions."
+  }
+
+  query predicate uniqueParameterNodePosition(
+    DataFlowCallable c, ParameterPosition pos, Node p, string msg
+  ) {
+    isParameterNode(p, c, pos) and
+    not exists(unique(ParameterPosition pos0 | isParameterNode(p, c, pos0))) and
+    msg = "Parameter node with multiple positions."
+  }
+
+  query predicate uniqueContentApprox(Content c, string msg) {
+    not exists(unique(ContentApprox approx | approx = getContentApprox(c))) and
+    msg = "Non-unique content approximation."
+  }
 }

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

@@ -551,6 +551,13 @@ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preserves
  */
 predicate allowParameterReturnInSelf(ParameterNode p) { none() }
 
+/** An approximated `Content`. */
+class ContentApprox = Unit;
+
+/** Gets an approximated value for content `c`. */
+pragma[inline]
+ContentApprox getContentApprox(Content c) { any() }
+
 private class MyConsistencyConfiguration extends Consistency::ConsistencyConfiguration {
   override predicate argHasPostUpdateExclude(ArgumentNode n) {
     // The rules for whether an IR argument gets a post-update node are too

cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/ArrayLengthAnalysis.qll

@@ -218,7 +218,7 @@ private predicate allocation(Instruction array, Length length, int delta) {
           length.(VNLength).getInstruction().getConvertedResultExpression() = lengthExpr
         )
         or
-        not exists(int d | deconstructMallocSizeExpr(alloc.getSizeExpr(), _, d)) and
+        not deconstructMallocSizeExpr(alloc.getSizeExpr(), _, _) and
         length.(VNLength).getInstruction().getConvertedResultExpression() = alloc.getSizeExpr() and
         delta = 0
       )

cpp/ql/lib/experimental/semmle/code/cpp/rangeanalysis/RangeAnalysis.qll

@@ -543,9 +543,7 @@ private predicate boundedPhiCand(
   PhiInstruction phi, boolean upper, Bound b, int delta, boolean fromBackEdge, int origdelta,
   Reason reason
 ) {
-  exists(PhiInputOperand op |
-    boundedPhiInp(phi, op, b, delta, upper, fromBackEdge, origdelta, reason)
-  )
+  boundedPhiInp(phi, _, b, delta, upper, fromBackEdge, origdelta, reason)
 }
 
 /**

cpp/ql/lib/experimental/semmle/code/cpp/semantic/SemanticExprSpecific.qll

@@ -292,12 +292,8 @@ module SemanticExprConfig {
     final Location getLocation() { result = super.getLocation() }
   }
 
-  private class ValueNumberBound extends Bound {
-    IRBound::ValueNumberBound bound;
-
-    ValueNumberBound() { bound = this }
-
-    override string toString() { result = bound.toString() }
+  private class ValueNumberBound extends Bound instanceof IRBound::ValueNumberBound {
+    override string toString() { result = IRBound::ValueNumberBound.super.toString() }
   }
 
   predicate zeroBound(Bound bound) { bound instanceof IRBound::ZeroBound }

cpp/ql/lib/experimental/semmle/code/cpp/semantic/analysis/RangeAnalysis.qll

@@ -664,9 +664,7 @@ private predicate boundedPhiCand(
   SemSsaPhiNode phi, boolean upper, SemBound b, int delta, boolean fromBackEdge, int origdelta,
   SemReason reason
 ) {
-  exists(SemSsaVariable inp, SemSsaReadPositionPhiInputEdge edge |
-    boundedPhiInp(phi, inp, edge, b, delta, upper, fromBackEdge, origdelta, reason)
-  )
+  boundedPhiInp(phi, _, _, b, delta, upper, fromBackEdge, origdelta, reason)
 }
 
 /**

cpp/ql/lib/experimental/semmle/code/cpp/semantic/analysis/SignAnalysisCommon.qll

@@ -33,23 +33,15 @@ abstract private class FlowSignDef extends SignDef {
 }
 
 /** An SSA definition whose sign is determined by the sign of that definitions source expression. */
-private class ExplicitSignDef extends FlowSignDef {
-  SemSsaExplicitUpdate update;
-
-  ExplicitSignDef() { update = this }
-
-  final override Sign getSign() { result = semExprSign(update.getSourceExpr()) }
+private class ExplicitSignDef extends FlowSignDef instanceof SemSsaExplicitUpdate {
+  final override Sign getSign() { result = semExprSign(super.getSourceExpr()) }
 }
 
 /** An SSA Phi definition, whose sign is the union of the signs of its inputs. */
-private class PhiSignDef extends FlowSignDef {
-  SemSsaPhiNode phi;
-
-  PhiSignDef() { phi = this }
-
+private class PhiSignDef extends FlowSignDef instanceof SemSsaPhiNode {
   final override Sign getSign() {
     exists(SemSsaVariable inp, SemSsaReadPositionPhiInputEdge edge |
-      edge.phiInput(phi, inp) and
+      edge.phiInput(this, inp) and
       result = semSsaSign(inp, edge)
     )
   }

cpp/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-all
-version: 0.4.4-dev
+version: 0.5.1-dev
 groups: cpp
 dbscheme: semmlecode.cpp.dbscheme
 extractor: cpp
@@ -7,3 +7,4 @@ library: true
 upgrades: upgrades
 dependencies:
   codeql/ssa: ${workspace}
+  codeql/tutorial: ${workspace}

cpp/ql/lib/semmle/code/cpp/File.qll

@@ -189,18 +189,6 @@ class Folder extends Container, @folder {
    * Gets the URL of this folder.
    */
   deprecated override string getURL() { result = "file://" + this.getAbsolutePath() + ":0:0:0:0" }
-
-  /**
-   * DEPRECATED: use `getAbsolutePath` instead.
-   * Gets the name of this folder.
-   */
-  deprecated string getName() { folders(underlyingElement(this), result) }
-
-  /**
-   * DEPRECATED: use `getBaseName` instead.
-   * Gets the last part of the folder name.
-   */
-  deprecated string getShortName() { result = this.getBaseName() }
 }
 
 /**

cpp/ql/lib/semmle/code/cpp/Function.qll

@@ -318,7 +318,7 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
   MetricFunction getMetrics() { result = this }
 
   /** Holds if this function calls the function `f`. */
-  predicate calls(Function f) { exists(Locatable l | this.calls(f, l)) }
+  predicate calls(Function f) { this.calls(f, _) }
 
   /**
    * Holds if this function calls the function `f` in the `FunctionCall`
@@ -335,7 +335,7 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
   }
 
   /** Holds if this function accesses a function or variable or enumerator `a`. */
-  predicate accesses(Declaration a) { exists(Locatable l | this.accesses(a, l)) }
+  predicate accesses(Declaration a) { this.accesses(a, _) }
 
   /**
    * Holds if this function accesses a function or variable or enumerator `a`

cpp/ql/lib/semmle/code/cpp/Type.qll

@@ -816,6 +816,12 @@ private predicate floatingPointTypeMapping(
   or
   // _Float128x
   kind = 50 and base = 2 and domain = TRealDomain() and realKind = 50 and extended = true
+  or
+  // _Float16
+  kind = 52 and base = 2 and domain = TRealDomain() and realKind = 52 and extended = false
+  or
+  // _Complex _Float16
+  kind = 53 and base = 2 and domain = TComplexDomain() and realKind = 52 and extended = false
 }
 
 /**

cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll

@@ -73,67 +73,29 @@ private int isSource(Expr bufferExpr, Element why) {
   )
 }
 
-/**
- * Holds if `e2` is an expression that is derived from `e1` such that if `e1[n]` is a
- * well-defined expression for some number `n`, then `e2[n + delta]` is also a well-defined
- * expression.
- */
-private predicate step(Expr e1, Expr e2, int delta) {
-  getBufferSizeCand0(e1) and
-  (
-    exists(Variable bufferVar, Class parentClass, VariableAccess parentPtr, int bufferSize |
-      e1 = parentPtr
-    |
-      bufferVar = e2.(VariableAccess).getTarget() and
-      // buffer is the parentPtr->bufferVar of a 'variable size struct'
-      memberMayBeVarSize(parentClass, bufferVar) and
-      parentPtr = e2.(VariableAccess).getQualifier() and
-      parentPtr.getTarget().getUnspecifiedType().(PointerType).getBaseType() = parentClass and
-      (
-        if exists(bufferVar.getType().getSize())
-        then bufferSize = bufferVar.getType().getSize()
-        else bufferSize = 0
-      ) and
-      delta = bufferSize - parentClass.getSize()
-    )
-    or
-    DataFlow::localExprFlowStep(e1, e2) and
-    delta = 0
-  )
-}
-
-pragma[nomagic]
-private predicate getBufferSizeCand0(Expr e) {
-  exists(isSource(e, _))
-  or
-  exists(Expr e0 |
-    getBufferSizeCand0(e0) and
-    step(e0, e, _)
-  )
-}
-
-/**
- * Get the size in bytes of the buffer pointed to by an expression (if this can be determined).
- *
- * NOTE: There can be multiple `(result, why)` for a given `bufferExpr`.
- */
-private int getBufferSizeCand(Expr bufferExpr, Element why) {
-  getBufferSizeCand0(bufferExpr) and
-  (
-    result = isSource(bufferExpr, why)
-    or
-    exists(Expr e0, int delta, int size |
-      size = getBufferSizeCand(e0, why) and
-      step(e0, bufferExpr, delta) and
-      result = size + delta
-    )
-  )
-}
-
 /**
  * Get the size in bytes of the buffer pointed to by an expression (if this can be determined).
  */
+language[monotonicAggregates]
 int getBufferSize(Expr bufferExpr, Element why) {
-  result = max( | | getBufferSizeCand(bufferExpr, _)) and
-  result = getBufferSizeCand(bufferExpr, why)
+  result = isSource(bufferExpr, why)
+  or
+  exists(Class parentClass, VariableAccess parentPtr, int bufferSize, Variable bufferVar |
+    bufferVar = bufferExpr.(VariableAccess).getTarget() and
+    // buffer is the parentPtr->bufferVar of a 'variable size struct'
+    memberMayBeVarSize(parentClass, bufferVar) and
+    why = bufferVar and
+    parentPtr = bufferExpr.(VariableAccess).getQualifier() and
+    parentPtr.getTarget().getUnspecifiedType().(PointerType).getBaseType() = parentClass and
+    result = getBufferSize(parentPtr, _) + bufferSize - parentClass.getSize()
+  |
+    if exists(bufferVar.getType().getSize())
+    then bufferSize = bufferVar.getType().getSize()
+    else bufferSize = 0
+  )
+  or
+  // dataflow (all sources must be the same size)
+  result = unique(Expr def | DataFlow::localExprFlowStep(def, bufferExpr) | getBufferSize(def, _)) and
+  // find reason
+  exists(Expr def | DataFlow::localExprFlowStep(def, bufferExpr) | exists(getBufferSize(def, why)))
 }

cpp/ql/lib/semmle/code/cpp/commons/Dependency.qll

@@ -33,7 +33,7 @@ DependencyOptions getDependencyOptions() { any() }
 class DependsSource extends Element {
   DependsSource() {
     // not inside a template instantiation
-    not exists(Element other | this.isFromTemplateInstantiation(other)) or
+    not this.isFromTemplateInstantiation(_) or
     // allow DeclarationEntrys of template specializations
     this.(DeclarationEntry).getDeclaration().(Function).isConstructedFrom(_) or
     this.(DeclarationEntry).getDeclaration().(Class).isConstructedFrom(_)

cpp/ql/lib/semmle/code/cpp/commons/Exclusions.qll

@@ -69,12 +69,9 @@ predicate functionContainsDisabledCode(Function f) {
  */
 predicate functionContainsPreprocCode(Function f) {
   // `f` contains a preprocessor branch
-  exists(
-    PreprocessorBranchDirective pbd, string file, int pbdStartLine, int fBlockStartLine,
-    int fBlockEndLine
-  |
+  exists(string file, int pbdStartLine, int fBlockStartLine, int fBlockEndLine |
     functionLocation(f, file, fBlockStartLine, fBlockEndLine) and
-    pbdLocation(pbd, file, pbdStartLine) and
+    pbdLocation(_, file, pbdStartLine) and
     pbdStartLine <= fBlockEndLine and
     pbdStartLine >= fBlockStartLine
   )

cpp/ql/lib/semmle/code/cpp/commons/Printf.qll

@@ -397,11 +397,8 @@ private int lengthInBase16(float f) {
 /**
  * A class to represent format strings that occur as arguments to invocations of formatting functions.
  */
-class FormatLiteral extends Literal {
-  FormatLiteral() {
-    exists(FormattingFunctionCall ffc | ffc.getFormat() = this) and
-    this instanceof StringLiteral
-  }
+class FormatLiteral extends Literal instanceof StringLiteral {
+  FormatLiteral() { exists(FormattingFunctionCall ffc | ffc.getFormat() = this) }
 
   /**
    * Gets the function call where this format string is used.

cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll

@@ -30,15 +30,12 @@ abstract class ScanfFunction extends Function {
 /**
  * The standard function `scanf` (and variations).
  */
-class Scanf extends ScanfFunction {
+class Scanf extends ScanfFunction instanceof TopLevelFunction {
   Scanf() {
-    this instanceof TopLevelFunction and
-    (
-      this.hasGlobalOrStdOrBslName("scanf") or // scanf(format, args...)
-      this.hasGlobalOrStdOrBslName("wscanf") or // wscanf(format, args...)
-      this.hasGlobalName("_scanf_l") or // _scanf_l(format, locale, args...)
-      this.hasGlobalName("_wscanf_l") // _wscanf_l(format, locale, args...)
-    )
+    this.hasGlobalOrStdOrBslName("scanf") or // scanf(format, args...)
+    this.hasGlobalOrStdOrBslName("wscanf") or // wscanf(format, args...)
+    this.hasGlobalName("_scanf_l") or // _scanf_l(format, locale, args...)
+    this.hasGlobalName("_wscanf_l")
   }
 
   override int getInputParameterIndex() { none() }
@@ -49,15 +46,12 @@ class Scanf extends ScanfFunction {
 /**
  * The standard function `fscanf` (and variations).
  */
-class Fscanf extends ScanfFunction {
+class Fscanf extends ScanfFunction instanceof TopLevelFunction {
   Fscanf() {
-    this instanceof TopLevelFunction and
-    (
-      this.hasGlobalOrStdOrBslName("fscanf") or // fscanf(src_stream, format, args...)
-      this.hasGlobalOrStdOrBslName("fwscanf") or // fwscanf(src_stream, format, args...)
-      this.hasGlobalName("_fscanf_l") or // _fscanf_l(src_stream, format, locale, args...)
-      this.hasGlobalName("_fwscanf_l") // _fwscanf_l(src_stream, format, locale, args...)
-    )
+    this.hasGlobalOrStdOrBslName("fscanf") or // fscanf(src_stream, format, args...)
+    this.hasGlobalOrStdOrBslName("fwscanf") or // fwscanf(src_stream, format, args...)
+    this.hasGlobalName("_fscanf_l") or // _fscanf_l(src_stream, format, locale, args...)
+    this.hasGlobalName("_fwscanf_l")
   }
 
   override int getInputParameterIndex() { result = 0 }
@@ -68,15 +62,12 @@ class Fscanf extends ScanfFunction {
 /**
  * The standard function `sscanf` (and variations).
  */
-class Sscanf extends ScanfFunction {
+class Sscanf extends ScanfFunction instanceof TopLevelFunction {
   Sscanf() {
-    this instanceof TopLevelFunction and
-    (
-      this.hasGlobalOrStdOrBslName("sscanf") or // sscanf(src_stream, format, args...)
-      this.hasGlobalOrStdOrBslName("swscanf") or // swscanf(src, format, args...)
-      this.hasGlobalName("_sscanf_l") or // _sscanf_l(src, format, locale, args...)
-      this.hasGlobalName("_swscanf_l") // _swscanf_l(src, format, locale, args...)
-    )
+    this.hasGlobalOrStdOrBslName("sscanf") or // sscanf(src_stream, format, args...)
+    this.hasGlobalOrStdOrBslName("swscanf") or // swscanf(src, format, args...)
+    this.hasGlobalName("_sscanf_l") or // _sscanf_l(src, format, locale, args...)
+    this.hasGlobalName("_swscanf_l")
   }
 
   override int getInputParameterIndex() { result = 0 }
@@ -87,17 +78,12 @@ class Sscanf extends ScanfFunction {
 /**
  * The standard(ish) function `snscanf` (and variations).
  */
-class Snscanf extends ScanfFunction {
+class Snscanf extends ScanfFunction instanceof TopLevelFunction {
   Snscanf() {
-    this instanceof TopLevelFunction and
-    (
-      this.hasGlobalName("_snscanf") or // _snscanf(src, max_amount, format, args...)
-      this.hasGlobalName("_snwscanf") or // _snwscanf(src, max_amount, format, args...)
-      this.hasGlobalName("_snscanf_l") or // _snscanf_l(src, max_amount, format, locale, args...)
-      this.hasGlobalName("_snwscanf_l") // _snwscanf_l(src, max_amount, format, locale, args...)
-      // note that the max_amount is not a limit on the output length, it's an input length
-      // limit used with non null-terminated strings.
-    )
+    this.hasGlobalName("_snscanf") or // _snscanf(src, max_amount, format, args...)
+    this.hasGlobalName("_snwscanf") or // _snwscanf(src, max_amount, format, args...)
+    this.hasGlobalName("_snscanf_l") or // _snscanf_l(src, max_amount, format, locale, args...)
+    this.hasGlobalName("_snwscanf_l")
   }
 
   override int getInputParameterIndex() { result = 0 }
@@ -258,9 +244,7 @@ class ScanfFormatLiteral extends Expr {
   /**
    * Gets the maximum width option of the nth input (empty string if none is given).
    */
-  string getMaxWidthOpt(int n) {
-    exists(string spec, string len, string conv | this.parseConvSpec(n, spec, result, len, conv))
-  }
+  string getMaxWidthOpt(int n) { this.parseConvSpec(n, _, result, _, _) }
 
   /**
    * Gets the maximum width of the nth input.
@@ -270,18 +254,12 @@ class ScanfFormatLiteral extends Expr {
   /**
    * Gets the length flag of the nth conversion specifier.
    */
-  string getLength(int n) {
-    exists(string spec, string width, string conv |
-      this.parseConvSpec(n, spec, width, result, conv)
-    )
-  }
+  string getLength(int n) { this.parseConvSpec(n, _, _, result, _) }
 
   /**
    * Gets the conversion character of the nth conversion specifier.
    */
-  string getConversionChar(int n) {
-    exists(string spec, string width, string len | this.parseConvSpec(n, spec, width, len, result))
-  }
+  string getConversionChar(int n) { this.parseConvSpec(n, _, _, _, result) }
 
   /**
    * Gets the maximum length of the string that can be produced by the nth

cpp/ql/lib/semmle/code/cpp/controlflow/SubBasicBlocks.qll

@@ -54,7 +54,7 @@ class SubBasicBlock extends ControlFlowNodeBase {
    * only condition under which a `SubBasicBlock` may have multiple
    * predecessors.
    */
-  predicate firstInBB() { exists(BasicBlock bb | this.getRankInBasicBlock(bb) = 1) }
+  predicate firstInBB() { this.getRankInBasicBlock(_) = 1 }
 
   /**
    * Holds if this `SubBasicBlock` comes last in its basic block. This is the

cpp/ql/lib/semmle/code/cpp/controlflow/internal/ConstantExprs.qll

@@ -441,8 +441,8 @@ library class ExprEvaluator extends int {
       req = mid.(AssignExpr).getRValue()
     )
     or
-    exists(VariableAccess va, Variable v, boolean sub1 |
-      this.interestingVariableAccess(e, va, v, sub1) and
+    exists(Variable v, boolean sub1 |
+      this.interestingVariableAccess(e, _, v, sub1) and
       req = v.getAnAssignedValue() and
       (sub1 = true implies not this.ignoreVariableAssignment(e, v, req)) and
       sub = false
@@ -876,7 +876,7 @@ private predicate nonAnalyzableVariableDefinition(Variable v, StmtParent def) {
  * empirically to have effect only on a few rare and pathological examples.
  */
 private predicate tractableVariable(Variable v) {
-  not exists(StmtParent def | nonAnalyzableVariableDefinition(v, def)) or
+  not nonAnalyzableVariableDefinition(v, _) or
   strictcount(StmtParent def | nonAnalyzableVariableDefinition(v, def)) < 1000
 }
 

cpp/ql/lib/semmle/code/cpp/dataflow/old/DataFlow2.qll

@@ -6,7 +6,7 @@
  * `DataFlow2::Configuration`, a `DataFlow3::Configuration`, or a
  * `DataFlow4::Configuration`.
  *
- * See `semmle.code.cpp.ir.dataflow.DataFlow` for the full documentation.
+ * See `semmle.code.cpp.dataflow.old.DataFlow` for the full documentation.
  */
 
 import cpp
@@ -15,6 +15,6 @@ import cpp
  * Provides classes for performing local (intra-procedural) and
  * global (inter-procedural) data flow analyses.
  */
-module DataFlow {
+module DataFlow2 {
   import semmle.code.cpp.dataflow.old.internal.DataFlowImpl2
 }

cpp/ql/lib/semmle/code/cpp/dataflow/old/DataFlow3.qll

@@ -6,7 +6,7 @@
  * `DataFlow2::Configuration`, a `DataFlow3::Configuration`, or a
  * `DataFlow4::Configuration`.
  *
- * See `semmle.code.cpp.ir.dataflow.DataFlow` for the full documentation.
+ * See `semmle.code.cpp.dataflow.old.DataFlow` for the full documentation.
  */
 
 import cpp
@@ -15,6 +15,6 @@ import cpp
  * Provides classes for performing local (intra-procedural) and
  * global (inter-procedural) data flow analyses.
  */
-module DataFlow {
+module DataFlow3 {
   import semmle.code.cpp.dataflow.old.internal.DataFlowImpl3
 }

cpp/ql/lib/semmle/code/cpp/dataflow/old/DataFlow4.qll

@@ -6,7 +6,7 @@
  * `DataFlow2::Configuration`, a `DataFlow3::Configuration`, or a
  * `DataFlow4::Configuration`.
  *
- * See `semmle.code.cpp.ir.dataflow.DataFlow` for the full documentation.
+ * See `semmle.code.cpp.dataflow.old.DataFlow` for the full documentation.
  */
 
 import cpp
@@ -15,6 +15,6 @@ import cpp
  * Provides classes for performing local (intra-procedural) and
  * global (inter-procedural) data flow analyses.
  */
-module DataFlow {
+module DataFlow4 {
   import semmle.code.cpp.dataflow.old.internal.DataFlowImpl4
 }

cpp/ql/lib/semmle/code/cpp/dataflow/old/TaintTracking.qll

@@ -15,4 +15,13 @@
  * `TaintTracking::localTaintStep` with arguments of type `DataFlow::Node`.
  */
 
-import semmle.code.cpp.ir.dataflow.TaintTracking
+import semmle.code.cpp.dataflow.old.DataFlow
+import semmle.code.cpp.dataflow.old.DataFlow2
+
+/**
+ * Provides classes for performing local (intra-procedural) and
+ * global (inter-procedural) taint-tracking analyses.
+ */
+module TaintTracking {
+  import semmle.code.cpp.dataflow.old.internal.tainttracking1.TaintTrackingImpl
+}

cpp/ql/lib/semmle/code/cpp/dataflow/old/TaintTracking2.qll

@@ -8,7 +8,13 @@
  * `TaintTracking::Configuration` class extends `DataFlow::Configuration`, and
  * `TaintTracking2::Configuration` extends `DataFlow2::Configuration`.
  *
- * See `semmle.code.cpp.ir.dataflow.TaintTracking` for the full documentation.
+ * See `semmle.code.cpp.dataflow.old.TaintTracking` for the full documentation.
  */
 
-import semmle.code.cpp.ir.dataflow.TaintTracking2
+/**
+ * Provides classes for performing local (intra-procedural) and
+ * global (inter-procedural) taint-tracking analyses.
+ */
+module TaintTracking2 {
+  import semmle.code.cpp.dataflow.old.internal.tainttracking2.TaintTrackingImpl
+}

cpp/ql/lib/semmle/code/cpp/dataflow/old/TaintTracking3.qll

@@ -1,14 +0,0 @@
-/**
- * Provides a `TaintTracking3` module, which is a copy of the `TaintTracking`
- * module. Use this class when data-flow configurations or taint-tracking
- * configurations must depend on each other. Two classes extending
- * `DataFlow::Configuration` should never depend on each other, but one of them
- * should instead depend on a `DataFlow2::Configuration`, a
- * `DataFlow3::Configuration`, or a `DataFlow4::Configuration`. The
- * `TaintTracking::Configuration` class extends `DataFlow::Configuration`, and
- * `TaintTracking2::Configuration` extends `DataFlow2::Configuration`.
- *
- * See `semmle.code.cpp.ir.dataflow.TaintTracking` for the full documentation.
- */
-
-import semmle.code.cpp.ir.dataflow.TaintTracking3

cpp/ql/lib/semmle/code/cpp/dataflow/old/internal/DataFlowImplCommon.qll

@@ -915,18 +915,57 @@ private module Cached {
     TDataFlowCallNone() or
     TDataFlowCallSome(DataFlowCall call)
 
+  cached
+  newtype TParamNodeOption =
+    TParamNodeNone() or
+    TParamNodeSome(ParamNode p)
+
+  cached
+  newtype TReturnCtx =
+    TReturnCtxNone() or
+    TReturnCtxNoFlowThrough() or
+    TReturnCtxMaybeFlowThrough(ReturnPosition pos)
+
+  cached
+  newtype TTypedContentApprox =
+    MkTypedContentApprox(ContentApprox c, DataFlowType t) {
+      exists(Content cont |
+        c = getContentApprox(cont) and
+        store(_, cont, _, _, t)
+      )
+    }
+
   cached
   newtype TTypedContent = MkTypedContent(Content c, DataFlowType t) { store(_, c, _, _, t) }
 
+  cached
+  TypedContent getATypedContent(TypedContentApprox c) {
+    exists(ContentApprox cls, DataFlowType t, Content cont |
+      c = MkTypedContentApprox(cls, pragma[only_bind_into](t)) and
+      result = MkTypedContent(cont, pragma[only_bind_into](t)) and
+      cls = getContentApprox(cont)
+    )
+  }
+
   cached
   newtype TAccessPathFront =
     TFrontNil(DataFlowType t) or
     TFrontHead(TypedContent tc)
 
+  cached
+  newtype TApproxAccessPathFront =
+    TApproxFrontNil(DataFlowType t) or
+    TApproxFrontHead(TypedContentApprox tc)
+
   cached
   newtype TAccessPathFrontOption =
     TAccessPathFrontNone() or
     TAccessPathFrontSome(AccessPathFront apf)
+
+  cached
+  newtype TApproxAccessPathFrontOption =
+    TApproxAccessPathFrontNone() or
+    TApproxAccessPathFrontSome(ApproxAccessPathFront apf)
 }
 
 /**
@@ -1304,6 +1343,113 @@ class DataFlowCallOption extends TDataFlowCallOption {
   }
 }
 
+/** An optional `ParamNode`. */
+class ParamNodeOption extends TParamNodeOption {
+  string toString() {
+    this = TParamNodeNone() and
+    result = "(none)"
+    or
+    exists(ParamNode p |
+      this = TParamNodeSome(p) and
+      result = p.toString()
+    )
+  }
+}
+
+/**
+ * A return context used to calculate flow summaries in reverse flow.
+ *
+ * The possible values are:
+ *
+ * - `TReturnCtxNone()`: no return flow.
+ * - `TReturnCtxNoFlowThrough()`: return flow, but flow through is not possible.
+ * - `TReturnCtxMaybeFlowThrough(ReturnPosition pos)`: return flow, of kind `pos`, and
+ *    flow through may be possible.
+ */
+class ReturnCtx extends TReturnCtx {
+  string toString() {
+    this = TReturnCtxNone() and
+    result = "(none)"
+    or
+    this = TReturnCtxNoFlowThrough() and
+    result = "(no flow through)"
+    or
+    exists(ReturnPosition pos |
+      this = TReturnCtxMaybeFlowThrough(pos) and
+      result = pos.toString()
+    )
+  }
+}
+
+/** An approximated `Content` tagged with the type of a containing object. */
+class TypedContentApprox extends MkTypedContentApprox {
+  private ContentApprox c;
+  private DataFlowType t;
+
+  TypedContentApprox() { this = MkTypedContentApprox(c, t) }
+
+  /** Gets a typed content approximated by this value. */
+  TypedContent getATypedContent() { result = getATypedContent(this) }
+
+  /** Gets the container type. */
+  DataFlowType getContainerType() { result = t }
+
+  /** Gets a textual representation of this approximated content. */
+  string toString() { result = c.toString() }
+}
+
+/**
+ * The front of an approximated access path. This is either a head or a nil.
+ */
+abstract class ApproxAccessPathFront extends TApproxAccessPathFront {
+  abstract string toString();
+
+  abstract DataFlowType getType();
+
+  abstract boolean toBoolNonEmpty();
+
+  pragma[nomagic]
+  TypedContent getAHead() {
+    exists(TypedContentApprox cont |
+      this = TApproxFrontHead(cont) and
+      result = cont.getATypedContent()
+    )
+  }
+}
+
+class ApproxAccessPathFrontNil extends ApproxAccessPathFront, TApproxFrontNil {
+  private DataFlowType t;
+
+  ApproxAccessPathFrontNil() { this = TApproxFrontNil(t) }
+
+  override string toString() { result = ppReprType(t) }
+
+  override DataFlowType getType() { result = t }
+
+  override boolean toBoolNonEmpty() { result = false }
+}
+
+class ApproxAccessPathFrontHead extends ApproxAccessPathFront, TApproxFrontHead {
+  private TypedContentApprox tc;
+
+  ApproxAccessPathFrontHead() { this = TApproxFrontHead(tc) }
+
+  override string toString() { result = tc.toString() }
+
+  override DataFlowType getType() { result = tc.getContainerType() }
+
+  override boolean toBoolNonEmpty() { result = true }
+}
+
+/** An optional approximated access path front. */
+class ApproxAccessPathFrontOption extends TApproxAccessPathFrontOption {
+  string toString() {
+    this = TApproxAccessPathFrontNone() and result = "<none>"
+    or
+    this = TApproxAccessPathFrontSome(any(ApproxAccessPathFront apf | result = apf.toString()))
+  }
+}
+
 /** A `Content` tagged with the type of a containing object. */
 class TypedContent extends MkTypedContent {
   private Content c;
@@ -1336,7 +1482,7 @@ abstract class AccessPathFront extends TAccessPathFront {
 
   abstract DataFlowType getType();
 
-  abstract boolean toBoolNonEmpty();
+  abstract ApproxAccessPathFront toApprox();
 
   TypedContent getHead() { this = TFrontHead(result) }
 }
@@ -1350,7 +1496,7 @@ class AccessPathFrontNil extends AccessPathFront, TFrontNil {
 
   override DataFlowType getType() { result = t }
 
-  override boolean toBoolNonEmpty() { result = false }
+  override ApproxAccessPathFront toApprox() { result = TApproxFrontNil(t) }
 }
 
 class AccessPathFrontHead extends AccessPathFront, TFrontHead {
@@ -1362,7 +1508,7 @@ class AccessPathFrontHead extends AccessPathFront, TFrontHead {
 
   override DataFlowType getType() { result = tc.getContainerType() }
 
-  override boolean toBoolNonEmpty() { result = true }
+  override ApproxAccessPathFront toApprox() { result.getAHead() = tc }
 }
 
 /** An optional access path front. */

cpp/ql/lib/semmle/code/cpp/dataflow/old/internal/DataFlowImplConsistency.qll

@@ -101,9 +101,7 @@ module Consistency {
     exists(int c |
       c =
         strictcount(Node n |
-          not exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
-            n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-          ) and
+          not n.hasLocationInfo(_, _, _, _, _) and
           not any(ConsistencyConfiguration conf).missingLocationExclude(n)
         ) and
       msg = "Nodes without location: " + c
@@ -136,6 +134,18 @@ module Consistency {
     msg = "Local flow step does not preserve enclosing callable."
   }
 
+  query predicate readStepIsLocal(Node n1, Node n2, string msg) {
+    readStep(n1, _, n2) and
+    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
+    msg = "Read step does not preserve enclosing callable."
+  }
+
+  query predicate storeStepIsLocal(Node n1, Node n2, string msg) {
+    storeStep(n1, _, n2) and
+    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
+    msg = "Store step does not preserve enclosing callable."
+  }
+
   private DataFlowType typeRepr() { result = getNodeType(_) }
 
   query predicate compatibleTypesReflexive(DataFlowType t, string msg) {
@@ -232,4 +242,25 @@ module Consistency {
     not callable = viableCallable(call) and
     not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable)
   }
+
+  query predicate uniqueParameterNodeAtPosition(
+    DataFlowCallable c, ParameterPosition pos, Node p, string msg
+  ) {
+    isParameterNode(p, c, pos) and
+    not exists(unique(Node p0 | isParameterNode(p0, c, pos))) and
+    msg = "Parameters with overlapping positions."
+  }
+
+  query predicate uniqueParameterNodePosition(
+    DataFlowCallable c, ParameterPosition pos, Node p, string msg
+  ) {
+    isParameterNode(p, c, pos) and
+    not exists(unique(ParameterPosition pos0 | isParameterNode(p, c, pos0))) and
+    msg = "Parameter node with multiple positions."
+  }
+
+  query predicate uniqueContentApprox(Content c, string msg) {
+    not exists(unique(ContentApprox approx | approx = getContentApprox(c))) and
+    msg = "Non-unique content approximation."
+  }
 }

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

@@ -296,6 +296,13 @@ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preserves
  */
 predicate allowParameterReturnInSelf(ParameterNode p) { none() }
 
+/** An approximated `Content`. */
+class ContentApprox = Unit;
+
+/** Gets an approximated value for content `c`. */
+pragma[inline]
+ContentApprox getContentApprox(Content c) { any() }
+
 private class MyConsistencyConfiguration extends Consistency::ConsistencyConfiguration {
   override predicate argHasPostUpdateExclude(ArgumentNode n) {
     // Is the null pointer (or something that's not really a pointer)

cpp/ql/lib/semmle/code/cpp/dataflow/old/internal/FlowVar.qll

@@ -450,10 +450,8 @@ module FlowVar_internal {
     }
 
     override string toString() {
-      exists(Expr e |
-        this.definedByExpr(e, _) and
-        result = "assignment to " + v
-      )
+      this.definedByExpr(_, _) and
+      result = "assignment to " + v
       or
       this.definedByInitialValue(_) and
       result = "initial value of " + v

cpp/ql/lib/semmle/code/cpp/dataflow/old/internal/SubBasicBlocks.qll

@@ -54,7 +54,7 @@ class SubBasicBlock extends ControlFlowNodeBase {
    * only condition under which a `SubBasicBlock` may have multiple
    * predecessors.
    */
-  predicate firstInBB() { exists(BasicBlock bb | this.getRankInBasicBlock(bb) = 1) }
+  predicate firstInBB() { this.getRankInBasicBlock(_) = 1 }
 
   /**
    * Holds if this `SubBasicBlock` comes last in its basic block. This is the

cpp/ql/lib/semmle/code/cpp/dataflow/old/internal/tainttracking2/TaintTrackingParameter.qll

@@ -1,5 +1,5 @@
 import semmle.code.cpp.dataflow.old.internal.TaintTrackingUtil as Public
 
 module Private {
-  import semmle.code.cpp.dataflow.old.DataFlow2::DataFlow as DataFlow
+  import semmle.code.cpp.dataflow.old.DataFlow2::DataFlow2 as DataFlow
 }

cpp/ql/lib/semmle/code/cpp/dispatch/VirtualDispatchPrototype.qll

@@ -70,8 +70,8 @@ module VirtualDispatch {
    * that is, `c` or one of its supertypes overrides `f`.
    */
   private predicate cannotInherit(Class c, MemberFunction f) {
-    exists(Class overridingType, MemberFunction override |
-      cannotInheritHelper(c, f, overridingType, override) and
+    exists(MemberFunction override |
+      cannotInheritHelper(c, f, _, override) and
       override.overrides+(f)
     )
   }

cpp/ql/lib/semmle/code/cpp/exprs/Expr.qll

@@ -53,7 +53,7 @@ class Expr extends StmtParent, @expr {
   Declaration getEnclosingDeclaration() { result = exprEnclosingElement(this) }
 
   /** Gets a child of this expression. */
-  Expr getAChild() { exists(int n | result = this.getChild(n)) }
+  Expr getAChild() { result = this.getChild(_) }
 
   /** Gets the parent of this expression, if any. */
   Element getParent() { exprparents(underlyingElement(this), _, unresolveElement(result)) }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

@@ -1,642 +1,21 @@
 /**
+ * DEPRECATED: Use `semmle.code.cpp.ir.dataflow.TaintTracking` as a replacement.
+ *
  * An IR taint tracking library that uses an IR DataFlow configuration to track
  * taint from user inputs as defined by `semmle.code.cpp.security.Security`.
  */
 
 import cpp
 import semmle.code.cpp.security.Security
-private import semmle.code.cpp.ir.dataflow.DataFlow
-private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
-private import semmle.code.cpp.ir.dataflow.DataFlow3
-private import semmle.code.cpp.ir.IR
-private import semmle.code.cpp.ir.dataflow.ResolveCall
-private import semmle.code.cpp.controlflow.IRGuards
-private import semmle.code.cpp.models.interfaces.Taint
-private import semmle.code.cpp.models.interfaces.DataFlow
-private import semmle.code.cpp.ir.dataflow.TaintTracking
-private import semmle.code.cpp.ir.dataflow.TaintTracking2
-private import semmle.code.cpp.ir.dataflow.TaintTracking3
-private import semmle.code.cpp.ir.dataflow.internal.ModelUtil
+private import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl as DefaultTaintTrackingImpl
 
-/**
- * A predictable instruction is one where an external user can predict
- * the value. For example, a literal in the source code is considered
- * predictable.
- */
-private predicate predictableInstruction(Instruction instr) {
-  instr instanceof ConstantInstruction
-  or
-  instr instanceof StringConstantInstruction
-  or
-  // This could be a conversion on a string literal
-  predictableInstruction(instr.(UnaryInstruction).getUnary())
-}
+deprecated predicate predictableOnlyFlow = DefaultTaintTrackingImpl::predictableOnlyFlow/1;
 
-/**
- * Functions that we should only allow taint to flow through (to the return
- * value) if all but the source argument are 'predictable'.  This is done to
- * emulate the old security library's implementation rather than due to any
- * strong belief that this is the right approach.
- *
- * Note that the list itself is not very principled; it consists of all the
- * functions listed in the old security library's [default] `isPureFunction`
- * that have more than one argument, but are not in the old taint tracking
- * library's `returnArgument` predicate.
- */
-predicate predictableOnlyFlow(string name) {
-  name =
-    [
-      "strcasestr", "strchnul", "strchr", "strchrnul", "strcmp", "strcspn", "strncmp", "strndup",
-      "strnlen", "strrchr", "strspn", "strstr", "strtod", "strtof", "strtol", "strtoll", "strtoq",
-      "strtoul"
-    ]
-}
+deprecated predicate tainted = DefaultTaintTrackingImpl::tainted/2;
 
-private DataFlow::Node getNodeForSource(Expr source) {
-  isUserInput(source, _) and
-  result = getNodeForExpr(source)
-}
+deprecated predicate taintedIncludingGlobalVars =
+  DefaultTaintTrackingImpl::taintedIncludingGlobalVars/3;
 
-private DataFlow::Node getNodeForExpr(Expr node) {
-  result = DataFlow::exprNode(node)
-  or
-  // Some of the sources in `isUserInput` are intended to match the value of
-  // an expression, while others (those modeled below) are intended to match
-  // the taint that propagates out of an argument, like the `char *` argument
-  // to `gets`. It's impossible here to tell which is which, but the "access
-  // to argv" source is definitely not intended to match an output argument,
-  // and it causes false positives if we let it.
-  //
-  // This case goes together with the similar (but not identical) rule in
-  // `nodeIsBarrierIn`.
-  result = DataFlow::definitionByReferenceNodeFromArgument(node) and
-  not argv(node.(VariableAccess).getTarget())
-}
+deprecated predicate globalVarFromId = DefaultTaintTrackingImpl::globalVarFromId/1;
 
-private class DefaultTaintTrackingCfg extends TaintTracking::Configuration {
-  DefaultTaintTrackingCfg() { this = "DefaultTaintTrackingCfg" }
-
-  override predicate isSource(DataFlow::Node source) { source = getNodeForSource(_) }
-
-  override predicate isSink(DataFlow::Node sink) { exists(adjustedSink(sink)) }
-
-  override predicate isSanitizer(DataFlow::Node node) { nodeIsBarrier(node) }
-
-  override predicate isSanitizerIn(DataFlow::Node node) { nodeIsBarrierIn(node) }
-}
-
-private class ToGlobalVarTaintTrackingCfg extends TaintTracking::Configuration {
-  ToGlobalVarTaintTrackingCfg() { this = "GlobalVarTaintTrackingCfg" }
-
-  override predicate isSource(DataFlow::Node source) { source = getNodeForSource(_) }
-
-  override predicate isSink(DataFlow::Node sink) {
-    sink.asVariable() instanceof GlobalOrNamespaceVariable
-  }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
-    writesVariable(n1.asInstruction(), n2.asVariable().(GlobalOrNamespaceVariable))
-    or
-    readsVariable(n2.asInstruction(), n1.asVariable().(GlobalOrNamespaceVariable))
-  }
-
-  override predicate isSanitizer(DataFlow::Node node) { nodeIsBarrier(node) }
-
-  override predicate isSanitizerIn(DataFlow::Node node) { nodeIsBarrierIn(node) }
-}
-
-private class FromGlobalVarTaintTrackingCfg extends TaintTracking2::Configuration {
-  FromGlobalVarTaintTrackingCfg() { this = "FromGlobalVarTaintTrackingCfg" }
-
-  override predicate isSource(DataFlow::Node source) {
-    // This set of sources should be reasonably small, which is good for
-    // performance since the set of sinks is very large.
-    exists(ToGlobalVarTaintTrackingCfg otherCfg | otherCfg.hasFlowTo(source))
-  }
-
-  override predicate isSink(DataFlow::Node sink) { exists(adjustedSink(sink)) }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
-    // Additional step for flow out of variables. There is no flow _into_
-    // variables in this configuration, so this step only serves to take flow
-    // out of a variable that's a source.
-    readsVariable(n2.asInstruction(), n1.asVariable())
-  }
-
-  override predicate isSanitizer(DataFlow::Node node) { nodeIsBarrier(node) }
-
-  override predicate isSanitizerIn(DataFlow::Node node) { nodeIsBarrierIn(node) }
-}
-
-private predicate readsVariable(LoadInstruction load, Variable var) {
-  load.getSourceAddress().(VariableAddressInstruction).getAstVariable() = var
-}
-
-private predicate writesVariable(StoreInstruction store, Variable var) {
-  store.getDestinationAddress().(VariableAddressInstruction).getAstVariable() = var
-}
-
-/**
- * A variable that has any kind of upper-bound check anywhere in the program.  This is
- * biased towards being inclusive because there are a lot of valid ways of doing an
- * upper bounds checks if we don't consider where it occurs, for example:
- * ```
- *   if (x < 10) { sink(x); }
- *
- *   if (10 > y) { sink(y); }
- *
- *   if (z > 10) { z = 10; }
- *   sink(z);
- * ```
- */
-// TODO: This coarse overapproximation, ported from the old taint tracking
-// library, could be replaced with an actual semantic check that a particular
-// variable _access_ is guarded by an upper-bound check. We probably don't want
-// to do this right away since it could expose a lot of FPs that were
-// previously suppressed by this predicate by coincidence.
-private predicate hasUpperBoundsCheck(Variable var) {
-  exists(RelationalOperation oper, VariableAccess access |
-    oper.getAnOperand() = access and
-    access.getTarget() = var and
-    // Comparing to 0 is not an upper bound check
-    not oper.getAnOperand().getValue() = "0"
-  )
-}
-
-private predicate nodeIsBarrierEqualityCandidate(
-  DataFlow::Node node, Operand access, Variable checkedVar
-) {
-  readsVariable(node.asInstruction(), checkedVar) and
-  any(IRGuardCondition guard).ensuresEq(access, _, _, node.asInstruction().getBlock(), true)
-}
-
-cached
-private module Cached {
-  cached
-  predicate nodeIsBarrier(DataFlow::Node node) {
-    exists(Variable checkedVar |
-      readsVariable(node.asInstruction(), checkedVar) and
-      hasUpperBoundsCheck(checkedVar)
-    )
-    or
-    exists(Variable checkedVar, Operand access |
-      /*
-       * This node is guarded by a condition that forces the accessed variable
-       * to equal something else.  For example:
-       * ```
-       * x = taintsource()
-       * if (x == 10) {
-       *   taintsink(x); // not considered tainted
-       * }
-       * ```
-       */
-
-      nodeIsBarrierEqualityCandidate(node, access, checkedVar) and
-      readsVariable(access.getDef(), checkedVar)
-    )
-  }
-
-  cached
-  predicate nodeIsBarrierIn(DataFlow::Node node) {
-    // don't use dataflow into taint sources, as this leads to duplicate results.
-    exists(Expr source | isUserInput(source, _) |
-      node = DataFlow::exprNode(source)
-      or
-      // This case goes together with the similar (but not identical) rule in
-      // `getNodeForSource`.
-      node = DataFlow::definitionByReferenceNodeFromArgument(source)
-    )
-    or
-    // don't use dataflow into binary instructions if both operands are unpredictable
-    exists(BinaryInstruction iTo |
-      iTo = node.asInstruction() and
-      not predictableInstruction(iTo.getLeft()) and
-      not predictableInstruction(iTo.getRight()) and
-      // propagate taint from either the pointer or the offset, regardless of predictability
-      not iTo instanceof PointerArithmeticInstruction
-    )
-    or
-    // don't use dataflow through calls to pure functions if two or more operands
-    // are unpredictable
-    exists(Instruction iFrom1, Instruction iFrom2, CallInstruction iTo |
-      iTo = node.asInstruction() and
-      isPureFunction(iTo.getStaticCallTarget().getName()) and
-      iFrom1 = iTo.getAnArgument() and
-      iFrom2 = iTo.getAnArgument() and
-      not predictableInstruction(iFrom1) and
-      not predictableInstruction(iFrom2) and
-      iFrom1 != iFrom2
-    )
-  }
-
-  cached
-  Element adjustedSink(DataFlow::Node sink) {
-    // TODO: is it more appropriate to use asConvertedExpr here and avoid
-    // `getConversion*`? Or will that cause us to miss some cases where there's
-    // flow to a conversion (like a `ReferenceDereferenceExpr`) and we want to
-    // pretend there was flow to the converted `Expr` for the sake of
-    // compatibility.
-    sink.asExpr().getConversion*() = result
-    or
-    // For compatibility, send flow from arguments to parameters, even for
-    // functions with no body.
-    exists(FunctionCall call, int i |
-      sink.asExpr() = call.getArgument(pragma[only_bind_into](i)) and
-      result = resolveCall(call).getParameter(pragma[only_bind_into](i))
-    )
-    or
-    // For compatibility, send flow into a `Variable` if there is flow to any
-    // Load or Store of that variable.
-    exists(CopyInstruction copy |
-      copy.getSourceValue() = sink.asInstruction() and
-      (
-        readsVariable(copy, result) or
-        writesVariable(copy, result)
-      ) and
-      not hasUpperBoundsCheck(result)
-    )
-    or
-    // For compatibility, send flow into a `NotExpr` even if it's part of a
-    // short-circuiting condition and thus might get skipped.
-    result.(NotExpr).getOperand() = sink.asExpr()
-    or
-    // Taint postfix and prefix crement operations when their operand is tainted.
-    result.(CrementOperation).getAnOperand() = sink.asExpr()
-    or
-    // Taint `e1 += e2`, `e &= e2` and friends when `e1` or `e2` is tainted.
-    result.(AssignOperation).getAnOperand() = sink.asExpr()
-    or
-    result =
-      sink.asOperand()
-          .(SideEffectOperand)
-          .getUse()
-          .(ReadSideEffectInstruction)
-          .getArgumentDef()
-          .getUnconvertedResultExpression()
-  }
-
-  /**
-   * Step to return value of a modeled function when an input taints the
-   * dereference of the return value.
-   */
-  cached
-  predicate additionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
-    exists(CallInstruction call, Function func, FunctionInput modelIn, FunctionOutput modelOut |
-      n1 = callInput(call, modelIn) and
-      (
-        func.(TaintFunction).hasTaintFlow(modelIn, modelOut)
-        or
-        func.(DataFlowFunction).hasDataFlow(modelIn, modelOut)
-      ) and
-      call.getStaticCallTarget() = func and
-      modelOut.isReturnValueDeref() and
-      call = n2.asInstruction()
-    )
-  }
-}
-
-private import Cached
-
-/**
- * Holds if `tainted` may contain taint from `source`.
- *
- * A tainted expression is either directly user input, or is
- * computed from user input in a way that users can probably
- * control the exact output of the computation.
- *
- * This doesn't include data flow through global variables.
- * If you need that you must call `taintedIncludingGlobalVars`.
- */
-cached
-predicate tainted(Expr source, Element tainted) {
-  exists(DefaultTaintTrackingCfg cfg, DataFlow::Node sink |
-    cfg.hasFlow(getNodeForSource(source), sink) and
-    tainted = adjustedSink(sink)
-  )
-}
-
-/**
- * Holds if `tainted` may contain taint from `source`, where the taint passed
- * through a global variable named `globalVar`.
- *
- * A tainted expression is either directly user input, or is
- * computed from user input in a way that users can probably
- * control the exact output of the computation.
- *
- * This version gives the same results as tainted but also includes
- * data flow through global variables.
- *
- * The parameter `globalVar` is the qualified name of the last global variable
- * used to move the value from source to tainted. If the taint did not pass
- * through a global variable, then `globalVar = ""`.
- */
-cached
-predicate taintedIncludingGlobalVars(Expr source, Element tainted, string globalVar) {
-  tainted(source, tainted) and
-  globalVar = ""
-  or
-  exists(
-    ToGlobalVarTaintTrackingCfg toCfg, FromGlobalVarTaintTrackingCfg fromCfg,
-    DataFlow::VariableNode variableNode, GlobalOrNamespaceVariable global, DataFlow::Node sink
-  |
-    global = variableNode.getVariable() and
-    toCfg.hasFlow(getNodeForSource(source), variableNode) and
-    fromCfg.hasFlow(variableNode, sink) and
-    tainted = adjustedSink(sink) and
-    global = globalVarFromId(globalVar)
-  )
-}
-
-/**
- * Gets the global variable whose qualified name is `id`. Use this predicate
- * together with `taintedIncludingGlobalVars`. Example:
- *
- * ```
- * exists(string varName |
- *   taintedIncludingGlobalVars(source, tainted, varName) and
- *   var = globalVarFromId(varName)
- * )
- * ```
- */
-GlobalOrNamespaceVariable globalVarFromId(string id) { id = result.getQualifiedName() }
-
-/**
- * Provides definitions for augmenting source/sink pairs with data-flow paths
- * between them. From a `@kind path-problem` query, import this module in the
- * global scope, extend `TaintTrackingConfiguration`, and use `taintedWithPath`
- * in place of `tainted`.
- *
- * Importing this module will also import the query predicates that contain the
- * taint paths.
- */
-module TaintedWithPath {
-  private newtype TSingleton = MkSingleton()
-
-  /**
-   * A taint-tracking configuration that matches sources and sinks in the same
-   * way as the `tainted` predicate.
-   *
-   * Override `isSink` and `taintThroughGlobals` as needed, but do not provide
-   * a characteristic predicate.
-   */
-  class TaintTrackingConfiguration extends TSingleton {
-    /** Override this to specify which elements are sources in this configuration. */
-    predicate isSource(Expr source) { exists(getNodeForSource(source)) }
-
-    /** Override this to specify which elements are sinks in this configuration. */
-    abstract predicate isSink(Element e);
-
-    /** Override this to specify which expressions are barriers in this configuration. */
-    predicate isBarrier(Expr e) { nodeIsBarrier(getNodeForExpr(e)) }
-
-    /**
-     * Override this predicate to `any()` to allow taint to flow through global
-     * variables.
-     */
-    predicate taintThroughGlobals() { none() }
-
-    /** Gets a textual representation of this element. */
-    string toString() { result = "TaintTrackingConfiguration" }
-  }
-
-  private class AdjustedConfiguration extends TaintTracking3::Configuration {
-    AdjustedConfiguration() { this = "AdjustedConfiguration" }
-
-    override predicate isSource(DataFlow::Node source) {
-      exists(TaintTrackingConfiguration cfg, Expr e |
-        cfg.isSource(e) and source = getNodeForExpr(e)
-      )
-    }
-
-    override predicate isSink(DataFlow::Node sink) {
-      exists(TaintTrackingConfiguration cfg | cfg.isSink(adjustedSink(sink)))
-    }
-
-    override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
-      // Steps into and out of global variables
-      exists(TaintTrackingConfiguration cfg | cfg.taintThroughGlobals() |
-        writesVariable(n1.asInstruction(), n2.asVariable().(GlobalOrNamespaceVariable))
-        or
-        readsVariable(n2.asInstruction(), n1.asVariable().(GlobalOrNamespaceVariable))
-      )
-      or
-      additionalTaintStep(n1, n2)
-    }
-
-    override predicate isSanitizer(DataFlow::Node node) {
-      exists(TaintTrackingConfiguration cfg, Expr e | cfg.isBarrier(e) and node = getNodeForExpr(e))
-    }
-
-    override predicate isSanitizerIn(DataFlow::Node node) { nodeIsBarrierIn(node) }
-  }
-
-  /*
-   * A sink `Element` may map to multiple `DataFlowX::PathNode`s via (the
-   * inverse of) `adjustedSink`. For example, an `Expr` maps to all its
-   * conversions, and a `Variable` maps to all loads and stores from it. Because
-   * the path node is part of the tuple that constitutes the alert, this leads
-   * to duplicate alerts.
-   *
-   * To avoid showing duplicates, we edit the graph to replace the final node
-   * coming from the data-flow library with a node that matches exactly the
-   * `Element` sink that's requested.
-   *
-   * The same is done for sources.
-   */
-
-  private newtype TPathNode =
-    TWrapPathNode(DataFlow3::PathNode n) or
-    // There's a single newtype constructor for both sources and sinks since
-    // that makes it easiest to deal with the case where source = sink.
-    TEndpointPathNode(Element e) {
-      exists(AdjustedConfiguration cfg, DataFlow3::Node sourceNode, DataFlow3::Node sinkNode |
-        cfg.hasFlow(sourceNode, sinkNode)
-      |
-        sourceNode = getNodeForExpr(e) and
-        exists(TaintTrackingConfiguration ttCfg | ttCfg.isSource(e))
-        or
-        e = adjustedSink(sinkNode) and
-        exists(TaintTrackingConfiguration ttCfg | ttCfg.isSink(e))
-      )
-    }
-
-  /** An opaque type used for the nodes of a data-flow path. */
-  class PathNode extends TPathNode {
-    /** Gets a textual representation of this element. */
-    string toString() { none() }
-
-    /**
-     * Holds if this element is at the specified location.
-     * The location spans column `startcolumn` of line `startline` to
-     * column `endcolumn` of line `endline` in file `filepath`.
-     * For more information, see
-     * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
-     */
-    predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      none()
-    }
-  }
-
-  /**
-   * INTERNAL: Do not use.
-   */
-  module Private {
-    /** Gets a predecessor `PathNode` of `pathNode`, if any. */
-    PathNode getAPredecessor(PathNode pathNode) { edges(result, pathNode) }
-
-    /** Gets the element that `pathNode` wraps, if any. */
-    Element getElementFromPathNode(PathNode pathNode) {
-      exists(DataFlow::Node node | node = pathNode.(WrapPathNode).inner().getNode() |
-        result = node.asInstruction().getAst()
-        or
-        result = node.asOperand().getDef().getAst()
-      )
-      or
-      result = pathNode.(EndpointPathNode).inner()
-    }
-  }
-
-  private class WrapPathNode extends PathNode, TWrapPathNode {
-    DataFlow3::PathNode inner() { this = TWrapPathNode(result) }
-
-    override string toString() { result = this.inner().toString() }
-
-    override predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      this.inner().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
-  }
-
-  private class EndpointPathNode extends PathNode, TEndpointPathNode {
-    Expr inner() { this = TEndpointPathNode(result) }
-
-    override string toString() { result = this.inner().toString() }
-
-    override predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    ) {
-      this.inner()
-          .getLocation()
-          .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    }
-  }
-
-  /** A PathNode whose `Element` is a source. It may also be a sink. */
-  private class InitialPathNode extends EndpointPathNode {
-    InitialPathNode() { exists(TaintTrackingConfiguration cfg | cfg.isSource(this.inner())) }
-  }
-
-  /** A PathNode whose `Element` is a sink. It may also be a source. */
-  private class FinalPathNode extends EndpointPathNode {
-    FinalPathNode() { exists(TaintTrackingConfiguration cfg | cfg.isSink(this.inner())) }
-  }
-
-  /** Holds if `(a,b)` is an edge in the graph of data flow path explanations. */
-  query predicate edges(PathNode a, PathNode b) {
-    DataFlow3::PathGraph::edges(a.(WrapPathNode).inner(), b.(WrapPathNode).inner())
-    or
-    // To avoid showing trivial-looking steps, we _replace_ the last node instead
-    // of adding an edge out of it.
-    exists(WrapPathNode sinkNode |
-      DataFlow3::PathGraph::edges(a.(WrapPathNode).inner(), sinkNode.inner()) and
-      b.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
-    )
-    or
-    // Same for the first node
-    exists(WrapPathNode sourceNode |
-      DataFlow3::PathGraph::edges(sourceNode.inner(), b.(WrapPathNode).inner()) and
-      sourceNode.inner().getNode() = getNodeForExpr(a.(InitialPathNode).inner())
-    )
-    or
-    // Finally, handle the case where the path goes directly from a source to a
-    // sink, meaning that they both need to be translated.
-    exists(WrapPathNode sinkNode, WrapPathNode sourceNode |
-      DataFlow3::PathGraph::edges(sourceNode.inner(), sinkNode.inner()) and
-      sourceNode.inner().getNode() = getNodeForExpr(a.(InitialPathNode).inner()) and
-      b.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
-    )
-  }
-
-  /**
-   * Holds if there is flow from `arg` to `out` across a call that can by summarized by the flow
-   * from `par` to `ret` within it, in the graph of data flow path explanations.
-   */
-  query predicate subpaths(PathNode arg, PathNode par, PathNode ret, PathNode out) {
-    DataFlow3::PathGraph::subpaths(arg.(WrapPathNode).inner(), par.(WrapPathNode).inner(),
-      ret.(WrapPathNode).inner(), out.(WrapPathNode).inner())
-    or
-    // To avoid showing trivial-looking steps, we _replace_ the last node instead
-    // of adding an edge out of it.
-    exists(WrapPathNode sinkNode |
-      DataFlow3::PathGraph::subpaths(arg.(WrapPathNode).inner(), par.(WrapPathNode).inner(),
-        ret.(WrapPathNode).inner(), sinkNode.inner()) and
-      out.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
-    )
-    or
-    // Same for the first node
-    exists(WrapPathNode sourceNode |
-      DataFlow3::PathGraph::subpaths(sourceNode.inner(), par.(WrapPathNode).inner(),
-        ret.(WrapPathNode).inner(), out.(WrapPathNode).inner()) and
-      sourceNode.inner().getNode() = getNodeForExpr(arg.(InitialPathNode).inner())
-    )
-    or
-    // Finally, handle the case where the path goes directly from a source to a
-    // sink, meaning that they both need to be translated.
-    exists(WrapPathNode sinkNode, WrapPathNode sourceNode |
-      DataFlow3::PathGraph::subpaths(sourceNode.inner(), par.(WrapPathNode).inner(),
-        ret.(WrapPathNode).inner(), sinkNode.inner()) and
-      sourceNode.inner().getNode() = getNodeForExpr(arg.(InitialPathNode).inner()) and
-      out.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
-    )
-  }
-
-  /** Holds if `n` is a node in the graph of data flow path explanations. */
-  query predicate nodes(PathNode n, string key, string val) {
-    key = "semmle.label" and val = n.toString()
-  }
-
-  /**
-   * Holds if `tainted` may contain taint from `source`, where `sourceNode` and
-   * `sinkNode` are the corresponding `PathNode`s that can be used in a query
-   * to provide path explanations. Extend `TaintTrackingConfiguration` to use
-   * this predicate.
-   *
-   * A tainted expression is either directly user input, or is computed from
-   * user input in a way that users can probably control the exact output of
-   * the computation.
-   */
-  predicate taintedWithPath(Expr source, Element tainted, PathNode sourceNode, PathNode sinkNode) {
-    exists(AdjustedConfiguration cfg, DataFlow3::Node flowSource, DataFlow3::Node flowSink |
-      source = sourceNode.(InitialPathNode).inner() and
-      flowSource = getNodeForExpr(source) and
-      cfg.hasFlow(flowSource, flowSink) and
-      tainted = adjustedSink(flowSink) and
-      tainted = sinkNode.(FinalPathNode).inner()
-    )
-  }
-
-  private predicate isGlobalVariablePathNode(WrapPathNode n) {
-    n.inner().getNode().asVariable() instanceof GlobalOrNamespaceVariable
-  }
-
-  private predicate edgesWithoutGlobals(PathNode a, PathNode b) {
-    edges(a, b) and
-    not isGlobalVariablePathNode(a) and
-    not isGlobalVariablePathNode(b)
-  }
-
-  /**
-   * Holds if `tainted` can be reached from a taint source without passing
-   * through a global variable.
-   */
-  predicate taintedWithoutGlobals(Element tainted) {
-    exists(AdjustedConfiguration cfg, PathNode sourceNode, FinalPathNode sinkNode |
-      cfg.isSource(sourceNode.(WrapPathNode).inner().getNode()) and
-      edgesWithoutGlobals+(sourceNode, sinkNode) and
-      tainted = sinkNode.inner()
-    )
-  }
-}
+deprecated module TaintedWithPath = DefaultTaintTrackingImpl::TaintedWithPath;

cpp/ql/lib/semmle/code/cpp/ir/dataflow/MustFlow.qll

@@ -5,7 +5,6 @@
  */
 
 private import cpp
-import semmle.code.cpp.ir.dataflow.DataFlow
 private import semmle.code.cpp.ir.IR
 
 /**
@@ -25,18 +24,18 @@ abstract class MustFlowConfiguration extends string {
   /**
    * Holds if `source` is a relevant data flow source.
    */
-  abstract predicate isSource(DataFlow::Node source);
+  abstract predicate isSource(Instruction source);
 
   /**
    * Holds if `sink` is a relevant data flow sink.
    */
-  abstract predicate isSink(DataFlow::Node sink);
+  abstract predicate isSink(Operand sink);
 
   /**
    * Holds if the additional flow step from `node1` to `node2` must be taken
    * into account in the analysis.
    */
-  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
+  predicate isAdditionalFlowStep(Operand node1, Instruction node2) { none() }
 
   /** Holds if this configuration allows flow from arguments to parameters. */
   predicate allowInterproceduralFlow() { any() }
@@ -48,17 +47,17 @@ abstract class MustFlowConfiguration extends string {
    * included in the module `PathGraph`.
    */
   final predicate hasFlowPath(MustFlowPathNode source, MustFlowPathSink sink) {
-    this.isSource(source.getNode()) and
+    this.isSource(source.getInstruction()) and
     source.getASuccessor+() = sink
   }
 }
 
 /** Holds if `node` flows from a source. */
 pragma[nomagic]
-private predicate flowsFromSource(DataFlow::Node node, MustFlowConfiguration config) {
+private predicate flowsFromSource(Instruction node, MustFlowConfiguration config) {
   config.isSource(node)
   or
-  exists(DataFlow::Node mid |
+  exists(Instruction mid |
     step(mid, node, config) and
     flowsFromSource(mid, pragma[only_bind_into](config))
   )
@@ -66,12 +65,12 @@ private predicate flowsFromSource(DataFlow::Node node, MustFlowConfiguration con
 
 /** Holds if `node` flows to a sink. */
 pragma[nomagic]
-private predicate flowsToSink(DataFlow::Node node, MustFlowConfiguration config) {
+private predicate flowsToSink(Instruction node, MustFlowConfiguration config) {
   flowsFromSource(node, pragma[only_bind_into](config)) and
   (
-    config.isSink(node)
+    config.isSink(node.getAUse())
     or
-    exists(DataFlow::Node mid |
+    exists(Instruction mid |
       step(node, mid, config) and
       flowsToSink(mid, pragma[only_bind_into](config))
     )
@@ -198,12 +197,13 @@ private module Cached {
   }
 
   cached
-  predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
-    instructionToOperandStep(nodeFrom.asInstruction(), nodeTo.asOperand())
+  predicate step(Instruction nodeFrom, Instruction nodeTo) {
+    exists(Operand mid |
+      instructionToOperandStep(nodeFrom, mid) and
+      operandToInstructionStep(mid, nodeTo)
+    )
     or
-    flowThroughCallable(nodeFrom.asInstruction(), nodeTo.asInstruction())
-    or
-    operandToInstructionStep(nodeFrom.asOperand(), nodeTo.asInstruction())
+    flowThroughCallable(nodeFrom, nodeTo)
   }
 }
 
@@ -213,12 +213,12 @@ private module Cached {
  * way around.
  */
 pragma[inline]
-private Declaration getEnclosingCallable(DataFlow::Node n) {
-  pragma[only_bind_into](result) = pragma[only_bind_out](n).getEnclosingCallable()
+private IRFunction getEnclosingCallable(Instruction n) {
+  pragma[only_bind_into](result) = pragma[only_bind_out](n).getEnclosingIRFunction()
 }
 
 /** Holds if `nodeFrom` flows to `nodeTo`. */
-private predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo, MustFlowConfiguration config) {
+private predicate step(Instruction nodeFrom, Instruction nodeTo, MustFlowConfiguration config) {
   exists(config) and
   Cached::step(pragma[only_bind_into](nodeFrom), pragma[only_bind_into](nodeTo)) and
   (
@@ -227,37 +227,37 @@ private predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo, MustFlowC
     getEnclosingCallable(nodeFrom) = getEnclosingCallable(nodeTo)
   )
   or
-  config.isAdditionalFlowStep(nodeFrom, nodeTo)
+  config.isAdditionalFlowStep(nodeFrom.getAUse(), nodeTo)
 }
 
 private newtype TLocalPathNode =
-  MkLocalPathNode(DataFlow::Node n, MustFlowConfiguration config) {
+  MkLocalPathNode(Instruction n, MustFlowConfiguration config) {
     flowsToSink(n, config) and
     (
       config.isSource(n)
       or
-      exists(MustFlowPathNode mid | step(mid.getNode(), n, config))
+      exists(MustFlowPathNode mid | step(mid.getInstruction(), n, config))
     )
   }
 
 /** A `Node` that is in a path from a source to a sink. */
 class MustFlowPathNode extends TLocalPathNode {
-  DataFlow::Node n;
+  Instruction n;
 
   MustFlowPathNode() { this = MkLocalPathNode(n, _) }
 
   /** Gets the underlying node. */
-  DataFlow::Node getNode() { result = n }
+  Instruction getInstruction() { result = n }
 
   /** Gets a textual representation of this node. */
-  string toString() { result = n.toString() }
+  string toString() { result = n.getAst().toString() }
 
   /** Gets the location of this element. */
   Location getLocation() { result = n.getLocation() }
 
   /** Gets a successor node, if any. */
   MustFlowPathNode getASuccessor() {
-    step(this.getNode(), result.getNode(), this.getConfiguration())
+    step(this.getInstruction(), result.getInstruction(), this.getConfiguration())
   }
 
   /** Gets the associated configuration. */
@@ -265,7 +265,7 @@ class MustFlowPathNode extends TLocalPathNode {
 }
 
 private class MustFlowPathSink extends MustFlowPathNode {
-  MustFlowPathSink() { this.getConfiguration().isSink(this.getNode()) }
+  MustFlowPathSink() { this.getConfiguration().isSink(this.getInstruction().getAUse()) }
 }
 
 /**

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

@@ -143,7 +143,7 @@ private module VirtualDispatch {
   private class DataSensitiveExprCall extends DataSensitiveCall {
     DataSensitiveExprCall() { not exists(this.getStaticCallTarget()) }
 
-    override DataFlow::Node getDispatchValue() { result.asInstruction() = this.getCallTarget() }
+    override DataFlow::Node getDispatchValue() { result.asOperand() = this.getCallTargetOperand() }
 
     override Function resolve() {
       exists(FunctionInstruction fi |

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

@@ -915,18 +915,57 @@ private module Cached {
     TDataFlowCallNone() or
     TDataFlowCallSome(DataFlowCall call)
 
+  cached
+  newtype TParamNodeOption =
+    TParamNodeNone() or
+    TParamNodeSome(ParamNode p)
+
+  cached
+  newtype TReturnCtx =
+    TReturnCtxNone() or
+    TReturnCtxNoFlowThrough() or
+    TReturnCtxMaybeFlowThrough(ReturnPosition pos)
+
+  cached
+  newtype TTypedContentApprox =
+    MkTypedContentApprox(ContentApprox c, DataFlowType t) {
+      exists(Content cont |
+        c = getContentApprox(cont) and
+        store(_, cont, _, _, t)
+      )
+    }
+
   cached
   newtype TTypedContent = MkTypedContent(Content c, DataFlowType t) { store(_, c, _, _, t) }
 
+  cached
+  TypedContent getATypedContent(TypedContentApprox c) {
+    exists(ContentApprox cls, DataFlowType t, Content cont |
+      c = MkTypedContentApprox(cls, pragma[only_bind_into](t)) and
+      result = MkTypedContent(cont, pragma[only_bind_into](t)) and
+      cls = getContentApprox(cont)
+    )
+  }
+
   cached
   newtype TAccessPathFront =
     TFrontNil(DataFlowType t) or
     TFrontHead(TypedContent tc)
 
+  cached
+  newtype TApproxAccessPathFront =
+    TApproxFrontNil(DataFlowType t) or
+    TApproxFrontHead(TypedContentApprox tc)
+
   cached
   newtype TAccessPathFrontOption =
     TAccessPathFrontNone() or
     TAccessPathFrontSome(AccessPathFront apf)
+
+  cached
+  newtype TApproxAccessPathFrontOption =
+    TApproxAccessPathFrontNone() or
+    TApproxAccessPathFrontSome(ApproxAccessPathFront apf)
 }
 
 /**
@@ -1304,6 +1343,113 @@ class DataFlowCallOption extends TDataFlowCallOption {
   }
 }
 
+/** An optional `ParamNode`. */
+class ParamNodeOption extends TParamNodeOption {
+  string toString() {
+    this = TParamNodeNone() and
+    result = "(none)"
+    or
+    exists(ParamNode p |
+      this = TParamNodeSome(p) and
+      result = p.toString()
+    )
+  }
+}
+
+/**
+ * A return context used to calculate flow summaries in reverse flow.
+ *
+ * The possible values are:
+ *
+ * - `TReturnCtxNone()`: no return flow.
+ * - `TReturnCtxNoFlowThrough()`: return flow, but flow through is not possible.
+ * - `TReturnCtxMaybeFlowThrough(ReturnPosition pos)`: return flow, of kind `pos`, and
+ *    flow through may be possible.
+ */
+class ReturnCtx extends TReturnCtx {
+  string toString() {
+    this = TReturnCtxNone() and
+    result = "(none)"
+    or
+    this = TReturnCtxNoFlowThrough() and
+    result = "(no flow through)"
+    or
+    exists(ReturnPosition pos |
+      this = TReturnCtxMaybeFlowThrough(pos) and
+      result = pos.toString()
+    )
+  }
+}
+
+/** An approximated `Content` tagged with the type of a containing object. */
+class TypedContentApprox extends MkTypedContentApprox {
+  private ContentApprox c;
+  private DataFlowType t;
+
+  TypedContentApprox() { this = MkTypedContentApprox(c, t) }
+
+  /** Gets a typed content approximated by this value. */
+  TypedContent getATypedContent() { result = getATypedContent(this) }
+
+  /** Gets the container type. */
+  DataFlowType getContainerType() { result = t }
+
+  /** Gets a textual representation of this approximated content. */
+  string toString() { result = c.toString() }
+}
+
+/**
+ * The front of an approximated access path. This is either a head or a nil.
+ */
+abstract class ApproxAccessPathFront extends TApproxAccessPathFront {
+  abstract string toString();
+
+  abstract DataFlowType getType();
+
+  abstract boolean toBoolNonEmpty();
+
+  pragma[nomagic]
+  TypedContent getAHead() {
+    exists(TypedContentApprox cont |
+      this = TApproxFrontHead(cont) and
+      result = cont.getATypedContent()
+    )
+  }
+}
+
+class ApproxAccessPathFrontNil extends ApproxAccessPathFront, TApproxFrontNil {
+  private DataFlowType t;
+
+  ApproxAccessPathFrontNil() { this = TApproxFrontNil(t) }
+
+  override string toString() { result = ppReprType(t) }
+
+  override DataFlowType getType() { result = t }
+
+  override boolean toBoolNonEmpty() { result = false }
+}
+
+class ApproxAccessPathFrontHead extends ApproxAccessPathFront, TApproxFrontHead {
+  private TypedContentApprox tc;
+
+  ApproxAccessPathFrontHead() { this = TApproxFrontHead(tc) }
+
+  override string toString() { result = tc.toString() }
+
+  override DataFlowType getType() { result = tc.getContainerType() }
+
+  override boolean toBoolNonEmpty() { result = true }
+}
+
+/** An optional approximated access path front. */
+class ApproxAccessPathFrontOption extends TApproxAccessPathFrontOption {
+  string toString() {
+    this = TApproxAccessPathFrontNone() and result = "<none>"
+    or
+    this = TApproxAccessPathFrontSome(any(ApproxAccessPathFront apf | result = apf.toString()))
+  }
+}
+
 /** A `Content` tagged with the type of a containing object. */
 class TypedContent extends MkTypedContent {
   private Content c;
@@ -1336,7 +1482,7 @@ abstract class AccessPathFront extends TAccessPathFront {
 
   abstract DataFlowType getType();
 
-  abstract boolean toBoolNonEmpty();
+  abstract ApproxAccessPathFront toApprox();
 
   TypedContent getHead() { this = TFrontHead(result) }
 }
@@ -1350,7 +1496,7 @@ class AccessPathFrontNil extends AccessPathFront, TFrontNil {
 
   override DataFlowType getType() { result = t }
 
-  override boolean toBoolNonEmpty() { result = false }
+  override ApproxAccessPathFront toApprox() { result = TApproxFrontNil(t) }
 }
 
 class AccessPathFrontHead extends AccessPathFront, TFrontHead {
@@ -1362,7 +1508,7 @@ class AccessPathFrontHead extends AccessPathFront, TFrontHead {
 
   override DataFlowType getType() { result = tc.getContainerType() }
 
-  override boolean toBoolNonEmpty() { result = true }
+  override ApproxAccessPathFront toApprox() { result.getAHead() = tc }
 }
 
 /** An optional access path front. */

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

@@ -101,9 +101,7 @@ module Consistency {
     exists(int c |
       c =
         strictcount(Node n |
-          not exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
-            n.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-          ) and
+          not n.hasLocationInfo(_, _, _, _, _) and
           not any(ConsistencyConfiguration conf).missingLocationExclude(n)
         ) and
       msg = "Nodes without location: " + c
@@ -136,6 +134,18 @@ module Consistency {
     msg = "Local flow step does not preserve enclosing callable."
   }
 
+  query predicate readStepIsLocal(Node n1, Node n2, string msg) {
+    readStep(n1, _, n2) and
+    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
+    msg = "Read step does not preserve enclosing callable."
+  }
+
+  query predicate storeStepIsLocal(Node n1, Node n2, string msg) {
+    storeStep(n1, _, n2) and
+    nodeGetEnclosingCallable(n1) != nodeGetEnclosingCallable(n2) and
+    msg = "Store step does not preserve enclosing callable."
+  }
+
   private DataFlowType typeRepr() { result = getNodeType(_) }
 
   query predicate compatibleTypesReflexive(DataFlowType t, string msg) {
@@ -232,4 +242,25 @@ module Consistency {
     not callable = viableCallable(call) and
     not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable)
   }
+
+  query predicate uniqueParameterNodeAtPosition(
+    DataFlowCallable c, ParameterPosition pos, Node p, string msg
+  ) {
+    isParameterNode(p, c, pos) and
+    not exists(unique(Node p0 | isParameterNode(p0, c, pos))) and
+    msg = "Parameters with overlapping positions."
+  }
+
+  query predicate uniqueParameterNodePosition(
+    DataFlowCallable c, ParameterPosition pos, Node p, string msg
+  ) {
+    isParameterNode(p, c, pos) and
+    not exists(unique(ParameterPosition pos0 | isParameterNode(p, c, pos0))) and
+    msg = "Parameter node with multiple positions."
+  }
+
+  query predicate uniqueContentApprox(Content c, string msg) {
+    not exists(unique(ContentApprox approx | approx = getContentApprox(c))) and
+    msg = "Non-unique content approximation."
+  }
 }

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

@@ -6,6 +6,191 @@ private import DataFlowImplConsistency
 private import semmle.code.cpp.ir.internal.IRCppLanguage
 private import SsaInternals as Ssa
 
+cached
+private module Cached {
+  cached
+  newtype TIRDataFlowNode0 =
+    TInstructionNode0(Instruction i) {
+      not Ssa::ignoreInstruction(i) and
+      // We exclude `void`-typed instructions because they cannot contain data.
+      // However, if the instruction is a glvalue, and their type is `void`, then the result
+      // type of the instruction is really `void*`, and thus we still want to have a dataflow
+      // node for it.
+      (not i.getResultType() instanceof VoidType or i.isGLValue())
+    } or
+    TOperandNode0(Operand op) { not Ssa::ignoreOperand(op) }
+}
+
+private import Cached
+
+class Node0Impl extends TIRDataFlowNode0 {
+  /**
+   * INTERNAL: Do not use.
+   */
+  Declaration getEnclosingCallable() { none() } // overridden in subclasses
+
+  /** Gets the function to which this node belongs, if any. */
+  Declaration getFunction() { none() } // overridden in subclasses
+
+  /**
+   * Gets the type of this node.
+   *
+   * If `asInstruction().isGLValue()` holds, then the type of this node
+   * should be thought of as "pointer to `getType()`".
+   */
+  DataFlowType getType() { none() } // overridden in subclasses
+
+  /** Gets the instruction corresponding to this node, if any. */
+  Instruction asInstruction() { result = this.(InstructionNode0).getInstruction() }
+
+  /** Gets the operands corresponding to this node, if any. */
+  Operand asOperand() { result = this.(OperandNode0).getOperand() }
+
+  /** INTERNAL: Do not use. */
+  Location getLocationImpl() {
+    none() // overridden by subclasses
+  }
+
+  /** INTERNAL: Do not use. */
+  string toStringImpl() {
+    none() // overridden by subclasses
+  }
+
+  /** Gets the location of this node. */
+  final Location getLocation() { result = this.getLocationImpl() }
+
+  /** Gets a textual representation of this node. */
+  final string toString() { result = this.toStringImpl() }
+}
+
+/**
+ * An instruction, viewed as a node in a data flow graph.
+ */
+class InstructionNode0 extends Node0Impl, TInstructionNode0 {
+  Instruction instr;
+
+  InstructionNode0() { this = TInstructionNode0(instr) }
+
+  /** Gets the instruction corresponding to this node. */
+  Instruction getInstruction() { result = instr }
+
+  override Declaration getEnclosingCallable() { result = this.getFunction() }
+
+  override Declaration getFunction() { result = instr.getEnclosingFunction() }
+
+  override DataFlowType getType() { result = instr.getResultType() }
+
+  final override Location getLocationImpl() { result = instr.getLocation() }
+
+  override string toStringImpl() {
+    // This predicate is overridden in subclasses. This default implementation
+    // does not use `Instruction.toString` because that's expensive to compute.
+    result = this.getInstruction().getOpcode().toString()
+  }
+}
+
+/**
+ * An operand, viewed as a node in a data flow graph.
+ */
+class OperandNode0 extends Node0Impl, TOperandNode0 {
+  Operand op;
+
+  OperandNode0() { this = TOperandNode0(op) }
+
+  /** Gets the operand corresponding to this node. */
+  Operand getOperand() { result = op }
+
+  override Declaration getEnclosingCallable() { result = this.getFunction() }
+
+  override Declaration getFunction() { result = op.getUse().getEnclosingFunction() }
+
+  override DataFlowType getType() { result = op.getType() }
+
+  final override Location getLocationImpl() { result = op.getLocation() }
+
+  override string toStringImpl() { result = this.getOperand().toString() }
+}
+
+/**
+ * INTERNAL: Do not use.
+ *
+ * A node that represents the indirect value of an operand in the IR
+ * after `index` number of loads.
+ *
+ * Note: Unlike `RawIndirectOperand`, a value of type `IndirectOperand` may
+ * be an `OperandNode`.
+ */
+class IndirectOperand extends Node {
+  Operand operand;
+  int indirectionIndex;
+
+  IndirectOperand() {
+    this.(RawIndirectOperand).getOperand() = operand and
+    this.(RawIndirectOperand).getIndirectionIndex() = indirectionIndex
+    or
+    this.(OperandNode).getOperand() =
+      Ssa::getIRRepresentationOfIndirectOperand(operand, indirectionIndex)
+  }
+
+  /** Gets the underlying operand. */
+  Operand getOperand() { result = operand }
+
+  /** Gets the underlying indirection index. */
+  int getIndirectionIndex() { result = indirectionIndex }
+
+  /**
+   * Holds if this `IndirectOperand` is represented directly in the IR instead of
+   * a `RawIndirectionOperand` with operand `op` and indirection index `index`.
+   */
+  predicate isIRRepresentationOf(Operand op, int index) {
+    this instanceof OperandNode and
+    (
+      op = operand and
+      index = indirectionIndex
+    )
+  }
+}
+
+/**
+ * INTERNAL: Do not use.
+ *
+ * A node that represents the indirect value of an instruction in the IR
+ * after `index` number of loads.
+ *
+ * Note: Unlike `RawIndirectInstruction`, a value of type `IndirectInstruction` may
+ * be an `InstructionNode`.
+ */
+class IndirectInstruction extends Node {
+  Instruction instr;
+  int indirectionIndex;
+
+  IndirectInstruction() {
+    this.(RawIndirectInstruction).getInstruction() = instr and
+    this.(RawIndirectInstruction).getIndirectionIndex() = indirectionIndex
+    or
+    this.(InstructionNode).getInstruction() =
+      Ssa::getIRRepresentationOfIndirectInstruction(instr, indirectionIndex)
+  }
+
+  /** Gets the underlying instruction. */
+  Instruction getInstruction() { result = instr }
+
+  /** Gets the underlying indirection index. */
+  int getIndirectionIndex() { result = indirectionIndex }
+
+  /**
+   * Holds if this `IndirectInstruction` is represented directly in the IR instead of
+   * a `RawIndirectionInstruction` with instruction `i` and indirection index `index`.
+   */
+  predicate isIRRepresentationOf(Instruction i, int index) {
+    this instanceof InstructionNode and
+    (
+      i = instr and
+      index = indirectionIndex
+    )
+  }
+}
+
 /** Gets the callable in which this node occurs. */
 DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.getEnclosingCallable() }
 
@@ -47,24 +232,6 @@ private class PrimaryArgumentNode extends ArgumentNode, OperandNode {
   override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
     op = call.getArgumentOperand(pos.(DirectPosition).getIndex())
   }
-
-  override string toStringImpl() { result = argumentOperandToString(op) }
-}
-
-private string argumentOperandToString(ArgumentOperand op) {
-  exists(Expr unconverted |
-    unconverted = op.getDef().getUnconvertedResultExpression() and
-    result = unconverted.toString()
-  )
-  or
-  // Certain instructions don't map to an unconverted result expression. For these cases
-  // we fall back to a simpler naming scheme. This can happen in IR-generated constructors.
-  not exists(op.getDef().getUnconvertedResultExpression()) and
-  (
-    result = "Argument " + op.(PositionalArgumentOperand).getIndex()
-    or
-    op instanceof ThisArgumentOperand and result = "Argument this"
-  )
 }
 
 private class SideEffectArgumentNode extends ArgumentNode, SideEffectOperandNode {
@@ -73,10 +240,6 @@ private class SideEffectArgumentNode extends ArgumentNode, SideEffectOperandNode
     pos.(IndirectionPosition).getArgumentIndex() = this.getArgumentIndex() and
     pos.(IndirectionPosition).getIndirectionIndex() = super.getIndirectionIndex()
   }
-
-  override string toStringImpl() {
-    result = argumentOperandToString(this.getAddressOperand()) + " indirection"
-  }
 }
 
 /** A parameter position represented by an integer. */
@@ -193,22 +356,28 @@ class ReturnNode extends Node instanceof IndirectReturnNode {
  */
 private predicate hasNonInitializeParameterDef(IRVariable v) {
   exists(Ssa::Def def |
-    not def.getDefiningInstruction() instanceof InitializeParameterInstruction and
+    not def.getValue().asInstruction() instanceof InitializeParameterInstruction and
     v = def.getSourceVariable().getBaseVariable().(Ssa::BaseIRVariable).getIRVariable()
   )
 }
 
 class ReturnIndirectionNode extends IndirectReturnNode, ReturnNode {
   override ReturnKind getKind() {
-    exists(int argumentIndex, ReturnIndirectionInstruction returnInd |
-      returnInd.hasIndex(argumentIndex) and
-      this.getAddressOperand() = returnInd.getSourceAddressOperand() and
-      result = TIndirectReturnKind(argumentIndex, this.getIndirectionIndex()) and
-      hasNonInitializeParameterDef(returnInd.getIRVariable())
+    exists(Operand op, int i |
+      hasOperandAndIndex(this, pragma[only_bind_into](op), pragma[only_bind_into](i))
+    |
+      exists(int argumentIndex, ReturnIndirectionInstruction returnInd |
+        op = returnInd.getSourceAddressOperand() and
+        returnInd.hasIndex(argumentIndex) and
+        hasNonInitializeParameterDef(returnInd.getIRVariable()) and
+        result = TIndirectReturnKind(argumentIndex, pragma[only_bind_into](i))
+      )
+      or
+      exists(ReturnValueInstruction return |
+        op = return.getReturnAddressOperand() and
+        result = TNormalReturnKind(i - 1)
+      )
     )
-    or
-    this.getAddressOperand() = any(ReturnValueInstruction r).getReturnAddressOperand() and
-    result = TNormalReturnKind(this.getIndirectionIndex() - 1)
   }
 }
 
@@ -388,7 +557,7 @@ predicate storeStep(Node node1, Content c, PostFieldUpdateNode node2) {
  * operations and exactly `n` `LoadInstruction` operations.
  */
 private predicate numberOfLoadsFromOperandRec(Operand operandFrom, Operand operandTo, int ind) {
-  exists(LoadInstruction load | load.getSourceAddressOperand() = operandFrom |
+  exists(Instruction load | Ssa::isDereference(load, operandFrom) |
     operandTo = operandFrom and ind = 0
     or
     numberOfLoadsFromOperand(load.getAUse(), operandTo, ind - 1)
@@ -408,7 +577,7 @@ private predicate numberOfLoadsFromOperandRec(Operand operandFrom, Operand opera
 private predicate numberOfLoadsFromOperand(Operand operandFrom, Operand operandTo, int n) {
   numberOfLoadsFromOperandRec(operandFrom, operandTo, n)
   or
-  not any(LoadInstruction load).getSourceAddressOperand() = operandFrom and
+  not Ssa::isDereference(_, operandFrom) and
   not conversionFlow(operandFrom, _, _) and
   operandFrom = operandTo and
   n = 0
@@ -438,7 +607,9 @@ predicate nodeHasInstruction(Node node, Instruction instr, int indirectionIndex)
 predicate readStep(Node node1, Content c, Node node2) {
   exists(FieldAddress fa1, Operand operand, int numberOfLoads, int indirectionIndex2 |
     nodeHasOperand(node2, operand, indirectionIndex2) and
-    nodeHasOperand(node1, fa1.getObjectAddressOperand(), _) and
+    // The `1` here matches the `node2.getIndirectionIndex() = 1` conjunct
+    // in `storeStep`.
+    nodeHasOperand(node1, fa1.getObjectAddressOperand(), 1) and
     numberOfLoadsFromOperand(fa1, operand, numberOfLoads)
   |
     exists(FieldContent fc | fc = c |
@@ -559,6 +730,13 @@ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preserves
  */
 predicate allowParameterReturnInSelf(ParameterNode p) { none() }
 
+/** An approximated `Content`. */
+class ContentApprox = Unit;
+
+/** Gets an approximated value for content `c`. */
+pragma[inline]
+ContentApprox getContentApprox(Content c) { any() }
+
 private class MyConsistencyConfiguration extends Consistency::ConsistencyConfiguration {
   override predicate argHasPostUpdateExclude(ArgumentNode n) {
     // The rules for whether an IR argument gets a post-update node are too

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

@@ -13,56 +13,45 @@ private import semmle.code.cpp.models.interfaces.DataFlow
 private import DataFlowPrivate
 private import ModelUtil
 private import SsaInternals as Ssa
+private import DataFlowImplCommon as DataFlowImplCommon
 
+/**
+ * The IR dataflow graph consists of the following nodes:
+ * - `Node0`, which injects most instructions and operands directly into the dataflow graph.
+ * - `VariableNode`, which is used to model flow through global variables.
+ * - `PostFieldUpdateNode`, which is used to model the state of a field after a value has been stored
+ * into an address after a number of loads.
+ * - `SsaPhiNode`, which represents phi nodes as computed by the shared SSA library.
+ * - `IndirectArgumentOutNode`, which represents the value of an argument (and its indirections) after
+ * it leaves a function call.
+ * - `RawIndirectOperand`, which represents the value of `operand` after loading the address a number
+ * of times.
+ * - `RawIndirectInstruction`, which represents the value of `instr` after loading the address a number
+ * of times.
+ */
 cached
-private module Cached {
-  /**
-   * The IR dataflow graph consists of the following nodes:
-   * - `InstructionNode`, which injects most instructions directly into the dataflow graph.
-   * - `OperandNode`, which similarly injects most operands directly into the dataflow graph.
-   * - `VariableNode`, which is used to model flow through global variables.
-   * - `PostFieldUpdateNode`, which is used to model the state of a field after a value has been stored
-   * into an address after a number of loads.
-   * - `SsaPhiNode`, which represents phi nodes as computed by the shared SSA library.
-   * - `IndirectArgumentOutNode`, which represents the value of an argument (and its indirections) after
-   * it leaves a function call.
-   * - `IndirectOperand`, which represents the value of `operand` after loading the address a number
-   * of times.
-   * - `IndirectInstruction`, which represents the value of `instr` after loading the address a number
-   * of times.
-   */
-  cached
-  newtype TIRDataFlowNode =
-    TInstructionNode(Instruction i) {
-      not Ssa::ignoreInstruction(i) and
-      // We exclude `void`-typed instructions because they cannot contain data.
-      // However, if the instruction is a glvalue, and their type is `void`, then the result
-      // type of the instruction is really `void*`, and thus we still want to have a dataflow
-      // node for it.
-      (not i.getResultType() instanceof VoidType or i.isGLValue())
-    } or
-    TOperandNode(Operand op) { not Ssa::ignoreOperand(op) } or
-    TVariableNode(Variable var, int indirectionIndex) {
-      indirectionIndex = [1 .. Ssa::getMaxIndirectionsForType(var.getUnspecifiedType())]
-    } or
-    TPostFieldUpdateNode(FieldAddress operand, int indirectionIndex) {
-      indirectionIndex =
-        [1 .. Ssa::countIndirectionsForCppType(operand.getObjectAddress().getResultLanguageType())]
-    } or
-    TSsaPhiNode(Ssa::PhiNode phi) or
-    TIndirectArgumentOutNode(ArgumentOperand operand, int indirectionIndex) {
-      Ssa::isModifiableByCall(operand) and
-      indirectionIndex = [1 .. Ssa::countIndirectionsForCppType(operand.getLanguageType())]
-    } or
-    TIndirectOperand(Operand op, int indirectionIndex) {
-      Ssa::hasIndirectOperand(op, indirectionIndex)
-    } or
-    TIndirectInstruction(Instruction instr, int indirectionIndex) {
-      Ssa::hasIndirectInstruction(instr, indirectionIndex)
-    } or
-    TFinalGlobalValue(Ssa::GlobalUse globalUse) or
-    TInitialGlobalValue(Ssa::GlobalDef globalUse)
-}
+private newtype TIRDataFlowNode =
+  TNode0(Node0Impl node) { DataFlowImplCommon::forceCachingInSameStage() } or
+  TVariableNode(Variable var, int indirectionIndex) {
+    indirectionIndex = [1 .. Ssa::getMaxIndirectionsForType(var.getUnspecifiedType())]
+  } or
+  TPostFieldUpdateNode(FieldAddress operand, int indirectionIndex) {
+    indirectionIndex =
+      [1 .. Ssa::countIndirectionsForCppType(operand.getObjectAddress().getResultLanguageType())]
+  } or
+  TSsaPhiNode(Ssa::PhiNode phi) or
+  TIndirectArgumentOutNode(ArgumentOperand operand, int indirectionIndex) {
+    Ssa::isModifiableByCall(operand) and
+    indirectionIndex = [1 .. Ssa::countIndirectionsForCppType(operand.getLanguageType())]
+  } or
+  TRawIndirectOperand(Operand op, int indirectionIndex) {
+    Ssa::hasRawIndirectOperand(op, indirectionIndex)
+  } or
+  TRawIndirectInstruction(Instruction instr, int indirectionIndex) {
+    Ssa::hasRawIndirectInstruction(instr, indirectionIndex)
+  } or
+  TFinalGlobalValue(Ssa::GlobalUse globalUse) or
+  TInitialGlobalValue(Ssa::GlobalDef globalUse)
 
 /**
  * An operand that is defined by a `FieldAddressInstruction`.
@@ -98,14 +87,14 @@ predicate conversionFlow(Operand opFrom, Instruction instrTo, boolean isPointerA
     instrTo.(CheckedConvertOrNullInstruction).getUnaryOperand() = opFrom
     or
     instrTo.(InheritanceConversionInstruction).getUnaryOperand() = opFrom
+    or
+    Ssa::isAdditionalConversionFlow(opFrom, instrTo)
   )
   or
   isPointerArith = true and
   instrTo.(PointerArithmeticInstruction).getLeftOperand() = opFrom
 }
 
-private import Cached
-
 /**
  * A node in a data flow graph.
  *
@@ -239,7 +228,13 @@ class Node extends TIRDataFlowNode {
   Expr asIndirectArgument() { result = this.asIndirectArgument(_) }
 
   /** Gets the positional parameter corresponding to this node, if any. */
-  Parameter asParameter() { result = this.asParameter(0) }
+  Parameter asParameter() {
+    exists(int indirectionIndex | result = this.asParameter(indirectionIndex) |
+      if result.getUnspecifiedType() instanceof ReferenceType
+      then indirectionIndex = 1
+      else indirectionIndex = 0
+    )
+  }
 
   /**
    * Gets the uninitialized local variable corresponding to this node, if
@@ -258,7 +253,7 @@ class Node extends TIRDataFlowNode {
    * the value of `x`.
    * - The node `n` such that `n.asParameter(1)` is the parameter `x` represents
    * the value of `*x`.
-   * The node `n` such that `n.asParameter(2)` is the parameter `x` represents
+   * - The node `n` such that `n.asParameter(2)` is the parameter `x` represents
    * the value of `**x`.
    */
   Parameter asParameter(int index) {
@@ -341,51 +336,52 @@ private string toExprString(Node n) {
 }
 
 /**
- * An instruction, viewed as a node in a data flow graph.
+ * A class that lifts pre-SSA dataflow nodes to regular dataflow nodes.
  */
-class InstructionNode extends Node, TInstructionNode {
-  Instruction instr;
+private class Node0 extends Node, TNode0 {
+  Node0Impl node;
 
-  InstructionNode() { this = TInstructionNode(instr) }
+  Node0() { this = TNode0(node) }
 
-  /** Gets the instruction corresponding to this node. */
-  Instruction getInstruction() { result = instr }
+  override Declaration getEnclosingCallable() { result = node.getEnclosingCallable() }
 
-  override Declaration getEnclosingCallable() { result = this.getFunction() }
+  override Declaration getFunction() { result = node.getFunction() }
 
-  override Declaration getFunction() { result = instr.getEnclosingFunction() }
+  override DataFlowType getType() { result = node.getType() }
 
-  override DataFlowType getType() { result = instr.getResultType() }
-
-  final override Location getLocationImpl() { result = instr.getLocation() }
+  final override Location getLocationImpl() { result = node.getLocationImpl() }
 
   override string toStringImpl() {
     // This predicate is overridden in subclasses. This default implementation
     // does not use `Instruction.toString` because that's expensive to compute.
-    result = this.getInstruction().getOpcode().toString()
+    result = node.toStringImpl()
   }
 }
 
+/**
+ * An instruction, viewed as a node in a data flow graph.
+ */
+class InstructionNode extends Node0 {
+  override InstructionNode0 node;
+  Instruction instr;
+
+  InstructionNode() { instr = node.getInstruction() }
+
+  /** Gets the instruction corresponding to this node. */
+  Instruction getInstruction() { result = instr }
+}
+
 /**
  * An operand, viewed as a node in a data flow graph.
  */
-class OperandNode extends Node, TOperandNode {
+class OperandNode extends Node, Node0 {
+  override OperandNode0 node;
   Operand op;
 
-  OperandNode() { this = TOperandNode(op) }
+  OperandNode() { op = node.getOperand() }
 
   /** Gets the operand corresponding to this node. */
-  Operand getOperand() { result = op }
-
-  override Declaration getEnclosingCallable() { result = this.getFunction() }
-
-  override Declaration getFunction() { result = op.getUse().getEnclosingFunction() }
-
-  override DataFlowType getType() { result = op.getType() }
-
-  final override Location getLocationImpl() { result = op.getLocation() }
-
-  override string toStringImpl() { result = this.getOperand().toString() }
+  Operand getOperand() { result = node.getOperand() }
 }
 
 /**
@@ -394,12 +390,12 @@ class OperandNode extends Node, TOperandNode {
  * For example, `stripPointers(int*&)` is `int*` and `stripPointers(int*)` is `int`.
  */
 private Type stripPointer(Type t) {
-  result = t.(PointerType).getBaseType()
+  result = any(Ssa::Indirection ind | ind.getType() = t).getBaseType()
   or
+  // These types have a sensible base type, but don't receive additional
+  // dataflow nodes representing their indirections. So for now we special case them.
   result = t.(ArrayType).getBaseType()
   or
-  result = t.(ReferenceType).getBaseType()
-  or
   result = t.(PointerToMemberType).getBaseType()
   or
   result = t.(FunctionPointerIshType).getBaseType()
@@ -630,7 +626,7 @@ class IndirectArgumentOutNode extends Node, TIndirectArgumentOutNode, PartialDef
 
 pragma[nomagic]
 predicate indirectReturnOutNodeOperand0(CallInstruction call, Operand operand, int indirectionIndex) {
-  Ssa::hasIndirectInstruction(call, indirectionIndex) and
+  Ssa::hasRawIndirectInstruction(call, indirectionIndex) and
   operandForfullyConvertedCall(operand, call)
 }
 
@@ -638,14 +634,37 @@ pragma[nomagic]
 predicate indirectReturnOutNodeInstruction0(
   CallInstruction call, Instruction instr, int indirectionIndex
 ) {
-  Ssa::hasIndirectInstruction(call, indirectionIndex) and
+  Ssa::hasRawIndirectInstruction(call, indirectionIndex) and
   instructionForfullyConvertedCall(instr, call)
 }
 
+/**
+ * Holds if `node` is an indirect operand with columns `(operand, indirectionIndex)`, and
+ * `operand` represents a use of the fully converted value of `call`.
+ */
+private predicate hasOperand(Node node, CallInstruction call, int indirectionIndex, Operand operand) {
+  indirectReturnOutNodeOperand0(call, operand, indirectionIndex) and
+  hasOperandAndIndex(node, operand, indirectionIndex)
+}
+
+/**
+ * Holds if `node` is an indirect instruction with columns `(instr, indirectionIndex)`, and
+ * `instr` represents a use of the fully converted value of `call`.
+ *
+ * Note that `hasOperand(node, _, _, _)` implies `not hasInstruction(node, _, _, _)`.
+ */
+private predicate hasInstruction(
+  Node node, CallInstruction call, int indirectionIndex, Instruction instr
+) {
+  indirectReturnOutNodeInstruction0(call, instr, indirectionIndex) and
+  hasInstructionAndIndex(node, instr, indirectionIndex)
+}
+
 /**
  * INTERNAL: do not use.
  *
- * A node representing the value of a function call.
+ * A node representing the indirect value of a function call (i.e., a value hidden
+ * behind a number of indirections).
  */
 class IndirectReturnOutNode extends Node {
   CallInstruction call;
@@ -654,20 +673,43 @@ class IndirectReturnOutNode extends Node {
   IndirectReturnOutNode() {
     // Annoyingly, we need to pick the fully converted value as the output of the function to
     // make flow through in the shared dataflow library work correctly.
-    exists(Operand operand |
-      indirectReturnOutNodeOperand0(call, operand, indirectionIndex) and
-      hasOperandAndIndex(this, operand, indirectionIndex)
-    )
+    hasOperand(this, call, indirectionIndex, _)
     or
-    exists(Instruction instr |
-      indirectReturnOutNodeInstruction0(call, instr, indirectionIndex) and
-      hasInstructionAndIndex(this, instr, indirectionIndex)
-    )
+    hasInstruction(this, call, indirectionIndex, _)
   }
 
   CallInstruction getCallInstruction() { result = call }
 
   int getIndirectionIndex() { result = indirectionIndex }
+
+  /** Gets the operand associated with this node, if any. */
+  Operand getOperand() { hasOperand(this, call, indirectionIndex, result) }
+
+  /** Gets the instruction associated with this node, if any. */
+  Instruction getInstruction() { hasInstruction(this, call, indirectionIndex, result) }
+}
+
+/**
+ * An `IndirectReturnOutNode` which is used as a destination of a store operation.
+ * When it's used for a store operation it's useful to have this be a `PostUpdateNode` for
+ * the shared dataflow library's flow-through mechanism to detect flow in cases such as:
+ * ```cpp
+ * struct MyInt {
+ *   int i;
+ *   int& getRef() { return i; }
+ * };
+ * ...
+ * MyInt mi;
+ * mi.getRef() = source(); // this is detected as a store to `i` via flow-through.
+ * sink(mi.i);
+ * ```
+ */
+private class PostIndirectReturnOutNode extends IndirectReturnOutNode, PostUpdateNode {
+  PostIndirectReturnOutNode() {
+    any(StoreInstruction store).getDestinationAddressOperand() = this.getOperand()
+  }
+
+  override Node getPreUpdateNode() { result = this }
 }
 
 private Type getTypeImpl(Type t, int indirectionIndex) {
@@ -675,7 +717,15 @@ private Type getTypeImpl(Type t, int indirectionIndex) {
   result = t
   or
   indirectionIndex > 0 and
-  result = getTypeImpl(stripPointer(t), indirectionIndex - 1)
+  exists(Type stripped |
+    stripped = stripPointer(t) and
+    // We need to avoid the case where `stripPointer(t) = t` (which can happen on
+    // iterators that specify a `value_type` that is the iterator itself). Such a type
+    // would create an infinite loop otherwise. For these cases we simply don't produce
+    // a result for `getType`.
+    stripped.getUnspecifiedType() != t.getUnspecifiedType() and
+    result = getTypeImpl(stripPointer(t), indirectionIndex - 1)
+  )
 }
 
 /**
@@ -684,15 +734,16 @@ private Type getTypeImpl(Type t, int indirectionIndex) {
  * A node that represents the indirect value of an operand in the IR
  * after `index` number of loads.
  */
-class IndirectOperand extends Node, TIndirectOperand {
+class RawIndirectOperand extends Node, TRawIndirectOperand {
   Operand operand;
   int indirectionIndex;
 
-  IndirectOperand() { this = TIndirectOperand(operand, indirectionIndex) }
+  RawIndirectOperand() { this = TRawIndirectOperand(operand, indirectionIndex) }
 
   /** Gets the underlying instruction. */
   Operand getOperand() { result = operand }
 
+  /** Gets the underlying indirection index. */
   int getIndirectionIndex() { result = indirectionIndex }
 
   override Function getFunction() { result = this.getOperand().getDef().getEnclosingFunction() }
@@ -721,8 +772,8 @@ class UninitializedNode extends Node {
 
   UninitializedNode() {
     exists(Ssa::Def def |
-      def.getDefiningInstruction() instanceof UninitializedInstruction and
-      Ssa::nodeToDefOrUse(this, def) and
+      def.getValue().asInstruction() instanceof UninitializedInstruction and
+      Ssa::nodeToDefOrUse(this, def, _) and
       v = def.getSourceVariable().getBaseVariable().(Ssa::BaseIRVariable).getIRVariable().getAst()
     )
   }
@@ -737,15 +788,16 @@ class UninitializedNode extends Node {
  * A node that represents the indirect value of an instruction in the IR
  * after `index` number of loads.
  */
-class IndirectInstruction extends Node, TIndirectInstruction {
+class RawIndirectInstruction extends Node, TRawIndirectInstruction {
   Instruction instr;
   int indirectionIndex;
 
-  IndirectInstruction() { this = TIndirectInstruction(instr, indirectionIndex) }
+  RawIndirectInstruction() { this = TRawIndirectInstruction(instr, indirectionIndex) }
 
   /** Gets the underlying instruction. */
   Instruction getInstruction() { result = instr }
 
+  /** Gets the underlying indirection index. */
   int getIndirectionIndex() { result = indirectionIndex }
 
   override Function getFunction() { result = this.getInstruction().getEnclosingFunction() }
@@ -765,49 +817,30 @@ class IndirectInstruction extends Node, TIndirectInstruction {
   }
 }
 
-private predicate isFullyConvertedArgument(Expr e) {
-  e = any(Call call).getAnArgument().getFullyConverted()
-}
-
-private predicate isFullyConvertedCall(Expr e) { e = any(Call call).getFullyConverted() }
-
-/** Holds if `Node::asExpr` should map an some operand node to `e`. */
-private predicate convertedExprMustBeOperand(Expr e) {
-  isFullyConvertedArgument(e)
-  or
-  isFullyConvertedCall(e)
-}
-
 /** Holds if `node` is an `OperandNode` that should map `node.asExpr()` to `e`. */
-predicate exprNodeShouldBeOperand(Node node, Expr e) {
-  e = node.asOperand().getDef().getConvertedResultExpression() and
-  convertedExprMustBeOperand(e)
+predicate exprNodeShouldBeOperand(OperandNode node, Expr e) {
+  exists(Instruction def |
+    unique( | | getAUse(def)) = node.getOperand() and
+    e = def.getConvertedResultExpression()
+  )
 }
 
-/**
- * Holds if `load` is a `LoadInstruction` that is the result of evaluating `e`
- * and `node` is an `IndirectOperandNode` that should map `node.asExpr()` to `e`.
- *
- * We map `e` to `node.asExpr()` when `node` semantically represents the
- * same value as `load`. A subsequent flow step will flow `node` to
- * the `LoadInstruction`.
- */
-private predicate exprNodeShouldBeIndirectOperand(IndirectOperand node, Expr e, LoadInstruction load) {
-  node.getIndirectionIndex() = 1 and
-  e = load.getConvertedResultExpression() and
-  load.getSourceAddressOperand() = node.getOperand() and
-  not convertedExprMustBeOperand(e)
+private predicate indirectExprNodeShouldBeIndirectOperand0(
+  VariableAddressInstruction instr, RawIndirectOperand node, Expr e
+) {
+  instr = node.getOperand().getDef() and
+  e = instr.getAst().(Expr).getUnconverted()
 }
 
 /** Holds if `node` should be an `IndirectOperand` that maps `node.asIndirectExpr()` to `e`. */
-private predicate indirectExprNodeShouldBeIndirectOperand(IndirectOperand node, Expr e) {
-  exists(Instruction instr |
-    instr = node.getOperand().getDef() and
-    not node instanceof ExprNode
-  |
-    e = instr.(VariableAddressInstruction).getAst().(Expr).getFullyConverted()
+private predicate indirectExprNodeShouldBeIndirectOperand(RawIndirectOperand node, Expr e) {
+  exists(Instruction instr | instr = node.getOperand().getDef() |
+    exists(Expr e0 |
+      indirectExprNodeShouldBeIndirectOperand0(instr, node, e0) and
+      e = e0.getFullyConverted()
+    )
     or
-    not instr instanceof VariableAddressInstruction and
+    not indirectExprNodeShouldBeIndirectOperand0(_, _, e.getUnconverted()) and
     e = instr.getConvertedResultExpression()
   )
 }
@@ -822,10 +855,27 @@ private predicate exprNodeShouldBeIndirectOutNode(IndirectArgumentOutNode node,
 
 /** Holds if `node` should be an instruction node that maps `node.asExpr()` to `e`. */
 predicate exprNodeShouldBeInstruction(Node node, Expr e) {
-  e = node.asInstruction().getConvertedResultExpression() and
   not exprNodeShouldBeOperand(_, e) and
-  not exprNodeShouldBeIndirectOperand(_, e, _) and
-  not exprNodeShouldBeIndirectOutNode(_, e)
+  not exprNodeShouldBeIndirectOutNode(_, e) and
+  (
+    e = node.asInstruction().getConvertedResultExpression()
+    or
+    // The instruction that contains the result of an `AssignOperation` is
+    // the unloaded left operand (see the comments in `TranslatedAssignOperation`).
+    // That means that for cases like
+    // ```cpp
+    // int x = ...;
+    // x += 1;
+    // ```
+    // the result of `x += 1` is the `VariableAddressInstruction` that represents `x`. But
+    // that instruction doesn't receive the flow from this `AssignOperation`. So instead we
+    // map the operation to the `AddInstruction`.
+    node.asInstruction().getAst() = e.(AssignOperation)
+    or
+    // Same story for `CrementOperation`s (cf. the comments in the subclasses
+    // of `TranslatedCrementOperation`).
+    node.asInstruction().getAst() = e.(CrementOperation)
+  )
 }
 
 /** Holds if `node` should be an `IndirectInstruction` that maps `node.asIndirectExpr()` to `e`. */
@@ -868,23 +918,6 @@ private class OperandExprNode extends ExprNodeBase, OperandNode {
 
   final override Expr getExpr() { result = this.getConvertedExpr().getUnconverted() }
 
-  final override string toStringImpl() {
-    // Avoid generating multiple `toString` results for `ArgumentNode`s
-    // since they have a better `toString`.
-    result = this.(ArgumentNode).toStringImpl()
-    or
-    not this instanceof ArgumentNode and
-    result = this.getConvertedExpr().toString()
-  }
-}
-
-private class IndirectOperandExprNode extends ExprNodeBase, IndirectOperand {
-  IndirectOperandExprNode() { exprNodeShouldBeIndirectOperand(this, _, _) }
-
-  final override Expr getConvertedExpr() { exprNodeShouldBeIndirectOperand(this, result, _) }
-
-  final override Expr getExpr() { result = this.getConvertedExpr().getUnconverted() }
-
   final override string toStringImpl() { result = this.getConvertedExpr().toString() }
 }
 
@@ -899,7 +932,7 @@ abstract private class IndirectExprNodeBase extends Node {
   abstract Expr getExpr(int indirectionIndex);
 }
 
-private class IndirectOperandIndirectExprNode extends IndirectExprNodeBase, IndirectOperand {
+private class IndirectOperandIndirectExprNode extends IndirectExprNodeBase, RawIndirectOperand {
   IndirectOperandIndirectExprNode() { indirectExprNodeShouldBeIndirectOperand(this, _) }
 
   final override Expr getConvertedExpr(int index) {
@@ -913,7 +946,8 @@ private class IndirectOperandIndirectExprNode extends IndirectExprNodeBase, Indi
   }
 }
 
-private class IndirectInstructionIndirectExprNode extends IndirectExprNodeBase, IndirectInstruction {
+private class IndirectInstructionIndirectExprNode extends IndirectExprNodeBase,
+  RawIndirectInstruction {
   IndirectInstructionIndirectExprNode() { indirectExprNodeShouldBeIndirectInstruction(this, _) }
 
   final override Expr getConvertedExpr(int index) {
@@ -933,8 +967,6 @@ private class IndirectArgumentOutExprNode extends ExprNodeBase, IndirectArgument
   final override Expr getConvertedExpr() { exprNodeShouldBeIndirectOutNode(this, result) }
 
   final override Expr getExpr() { result = this.getConvertedExpr() }
-
-  final override string toStringImpl() { result = this.getConvertedExpr().toString() }
 }
 
 /**
@@ -1208,35 +1240,6 @@ VariableNode variableNode(Variable v) {
  */
 Node uninitializedNode(LocalVariable v) { none() }
 
-/**
- * Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local
- * (intra-procedural) step.
- */
-predicate localFlowStep = simpleLocalFlowStep/2;
-
-private predicate indirectionOperandFlow(IndirectOperand nodeFrom, Node nodeTo) {
-  // Reduce the indirection count by 1 if we're passing through a `LoadInstruction`.
-  exists(int ind, LoadInstruction load |
-    hasOperandAndIndex(nodeFrom, load.getSourceAddressOperand(), ind) and
-    nodeHasInstruction(nodeTo, load, ind - 1)
-  )
-  or
-  // If an operand flows to an instruction, then the indirection of
-  // the operand also flows to the indirction of the instruction.
-  exists(Operand operand, Instruction instr, int indirectionIndex |
-    simpleInstructionLocalFlowStep(operand, instr) and
-    hasOperandAndIndex(nodeFrom, operand, indirectionIndex) and
-    hasInstructionAndIndex(nodeTo, instr, indirectionIndex)
-  )
-  or
-  // If there's indirect flow to an operand, then there's also indirect
-  // flow to the operand after applying some pointer arithmetic.
-  exists(PointerArithmeticInstruction pointerArith, int indirectionIndex |
-    hasOperandAndIndex(nodeFrom, pointerArith.getAnOperand(), indirectionIndex) and
-    hasInstructionAndIndex(nodeTo, pointerArith, indirectionIndex)
-  )
-}
-
 pragma[noinline]
 predicate hasOperandAndIndex(IndirectOperand indirectOperand, Operand operand, int indirectionIndex) {
   indirectOperand.getOperand() = operand and
@@ -1251,115 +1254,130 @@ predicate hasInstructionAndIndex(
   indirectInstr.getIndirectionIndex() = indirectionIndex
 }
 
-private predicate indirectionInstructionFlow(IndirectInstruction nodeFrom, IndirectOperand nodeTo) {
-  // If there's flow from an instruction to an operand, then there's also flow from the
-  // indirect instruction to the indirect operand.
-  exists(Operand operand, Instruction instr, int indirectionIndex |
-    simpleOperandLocalFlowStep(pragma[only_bind_into](instr), pragma[only_bind_into](operand))
-  |
-    hasOperandAndIndex(nodeTo, operand, indirectionIndex) and
-    hasInstructionAndIndex(nodeFrom, instr, indirectionIndex)
-  )
-}
+cached
+private module Cached {
+  /**
+   * Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local
+   * (intra-procedural) step.
+   */
+  cached
+  predicate localFlowStep(Node nodeFrom, Node nodeTo) { simpleLocalFlowStep(nodeFrom, nodeTo) }
 
-/**
- * INTERNAL: do not use.
- *
- * This is the local flow predicate that's used as a building block in global
- * data flow. It may have less flow than the `localFlowStep` predicate.
- */
-predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
-  // Post update node -> Node flow
-  Ssa::ssaFlow(nodeFrom.(PostUpdateNode).getPreUpdateNode(), nodeTo)
-  or
-  // Def-use/Use-use flow
-  Ssa::ssaFlow(nodeFrom, nodeTo)
-  or
-  // Operand -> Instruction flow
-  simpleInstructionLocalFlowStep(nodeFrom.asOperand(), nodeTo.asInstruction())
-  or
-  // Instruction -> Operand flow
-  simpleOperandLocalFlowStep(nodeFrom.asInstruction(), nodeTo.asOperand())
-  or
-  // Phi node -> Node flow
-  Ssa::fromPhiNode(nodeFrom, nodeTo)
-  or
-  // Indirect operand -> (indirect) instruction flow
-  indirectionOperandFlow(nodeFrom, nodeTo)
-  or
-  // Indirect instruction -> indirect operand flow
-  indirectionInstructionFlow(nodeFrom, nodeTo)
-  or
-  // Flow through modeled functions
-  modelFlow(nodeFrom, nodeTo)
-  or
-  // Reverse flow: data that flows from the definition node back into the indirection returned
-  // by a function. This allows data to flow 'in' through references returned by a modeled
-  // function such as `operator[]`.
-  exists(Operand address, int indirectionIndex |
-    nodeHasOperand(nodeTo.(IndirectReturnOutNode), address, indirectionIndex)
-  |
-    exists(StoreInstruction store |
-      nodeHasInstruction(nodeFrom, store, indirectionIndex - 1) and
-      store.getDestinationAddressOperand() = address
+  private predicate indirectionOperandFlow(RawIndirectOperand nodeFrom, Node nodeTo) {
+    // Reduce the indirection count by 1 if we're passing through a `LoadInstruction`.
+    exists(int ind, LoadInstruction load |
+      hasOperandAndIndex(nodeFrom, load.getSourceAddressOperand(), ind) and
+      nodeHasInstruction(nodeTo, load, ind - 1)
     )
     or
-    Ssa::outNodeHasAddressAndIndex(nodeFrom, address, indirectionIndex)
-  )
-}
+    // If an operand flows to an instruction, then the indirection of
+    // the operand also flows to the indirction of the instruction.
+    exists(Operand operand, Instruction instr, int indirectionIndex |
+      simpleInstructionLocalFlowStep(operand, instr) and
+      hasOperandAndIndex(nodeFrom, operand, pragma[only_bind_into](indirectionIndex)) and
+      hasInstructionAndIndex(nodeTo, instr, pragma[only_bind_into](indirectionIndex))
+    )
+    or
+    // If there's indirect flow to an operand, then there's also indirect
+    // flow to the operand after applying some pointer arithmetic.
+    exists(PointerArithmeticInstruction pointerArith, int indirectionIndex |
+      hasOperandAndIndex(nodeFrom, pointerArith.getAnOperand(),
+        pragma[only_bind_into](indirectionIndex)) and
+      hasInstructionAndIndex(nodeTo, pointerArith, pragma[only_bind_into](indirectionIndex))
+    )
+  }
 
-pragma[noinline]
-private predicate getAddressType(LoadInstruction load, Type t) {
-  exists(Instruction address |
-    address = load.getSourceAddress() and
-    t = address.getResultType()
-  )
-}
+  private predicate indirectionInstructionFlow(
+    RawIndirectInstruction nodeFrom, IndirectOperand nodeTo
+  ) {
+    // If there's flow from an instruction to an operand, then there's also flow from the
+    // indirect instruction to the indirect operand.
+    exists(Operand operand, Instruction instr, int indirectionIndex |
+      simpleOperandLocalFlowStep(pragma[only_bind_into](instr), pragma[only_bind_into](operand))
+    |
+      hasOperandAndIndex(nodeTo, operand, pragma[only_bind_into](indirectionIndex)) and
+      hasInstructionAndIndex(nodeFrom, instr, pragma[only_bind_into](indirectionIndex))
+    )
+  }
 
-/**
- * Like the AST dataflow library, we want to conflate the address and value of a reference. This class
- * represents the `LoadInstruction` that is generated from a reference dereference.
- */
-private class ReferenceDereferenceInstruction extends LoadInstruction {
-  ReferenceDereferenceInstruction() {
-    exists(ReferenceType ref |
-      getAddressType(this, ref) and
-      this.getResultType() = ref.getBaseType()
+  /**
+   * INTERNAL: do not use.
+   *
+   * This is the local flow predicate that's used as a building block in global
+   * data flow. It may have less flow than the `localFlowStep` predicate.
+   */
+  cached
+  predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
+    // Post update node -> Node flow
+    Ssa::ssaFlow(nodeFrom.(PostUpdateNode).getPreUpdateNode(), nodeTo)
+    or
+    // Def-use/Use-use flow
+    Ssa::ssaFlow(nodeFrom, nodeTo)
+    or
+    // Operand -> Instruction flow
+    simpleInstructionLocalFlowStep(nodeFrom.asOperand(), nodeTo.asInstruction())
+    or
+    // Instruction -> Operand flow
+    simpleOperandLocalFlowStep(nodeFrom.asInstruction(), nodeTo.asOperand())
+    or
+    // Phi node -> Node flow
+    Ssa::fromPhiNode(nodeFrom, nodeTo)
+    or
+    // Indirect operand -> (indirect) instruction flow
+    indirectionOperandFlow(nodeFrom, nodeTo)
+    or
+    // Indirect instruction -> indirect operand flow
+    indirectionInstructionFlow(nodeFrom, nodeTo)
+    or
+    // Flow through modeled functions
+    modelFlow(nodeFrom, nodeTo)
+    or
+    // Reverse flow: data that flows from the definition node back into the indirection returned
+    // by a function. This allows data to flow 'in' through references returned by a modeled
+    // function such as `operator[]`.
+    exists(Operand address, int indirectionIndex |
+      nodeHasOperand(nodeTo.(IndirectReturnOutNode), address, indirectionIndex)
+    |
+      exists(StoreInstruction store |
+        nodeHasInstruction(nodeFrom, store, indirectionIndex - 1) and
+        store.getDestinationAddressOperand() = address
+      )
+      or
+      Ssa::outNodeHasAddressAndIndex(nodeFrom, address, indirectionIndex)
+    )
+  }
+
+  private predicate simpleInstructionLocalFlowStep(Operand opFrom, Instruction iTo) {
+    // Treat all conversions as flow, even conversions between different numeric types.
+    conversionFlow(opFrom, iTo, false)
+    or
+    iTo.(CopyInstruction).getSourceValueOperand() = opFrom
+  }
+
+  private predicate simpleOperandLocalFlowStep(Instruction iFrom, Operand opTo) {
+    not opTo instanceof MemoryOperand and
+    opTo.getDef() = iFrom
+  }
+
+  private predicate modelFlow(Node nodeFrom, Node nodeTo) {
+    exists(
+      CallInstruction call, DataFlowFunction func, FunctionInput modelIn, FunctionOutput modelOut
+    |
+      call.getStaticCallTarget() = func and
+      func.hasDataFlow(modelIn, modelOut)
+    |
+      nodeFrom = callInput(call, modelIn) and
+      nodeTo = callOutput(call, modelOut)
+      or
+      exists(int d |
+        nodeFrom = callInput(call, modelIn, d) and
+        nodeTo = callOutput(call, modelOut, d)
+      )
     )
   }
 }
 
-private predicate simpleInstructionLocalFlowStep(Operand opFrom, Instruction iTo) {
-  // Treat all conversions as flow, even conversions between different numeric types.
-  conversionFlow(opFrom, iTo, false)
-  or
-  iTo.(CopyInstruction).getSourceValueOperand() = opFrom
-  or
-  // Conflate references and values like in AST dataflow.
-  iTo.(ReferenceDereferenceInstruction).getSourceAddressOperand() = opFrom
-}
-
-private predicate simpleOperandLocalFlowStep(Instruction iFrom, Operand opTo) {
-  not opTo instanceof MemoryOperand and
-  opTo.getDef() = iFrom
-}
-
-private predicate modelFlow(Node nodeFrom, Node nodeTo) {
-  exists(
-    CallInstruction call, DataFlowFunction func, FunctionInput modelIn, FunctionOutput modelOut
-  |
-    call.getStaticCallTarget() = func and
-    func.hasDataFlow(modelIn, modelOut)
-  |
-    nodeFrom = callInput(call, modelIn) and
-    nodeTo = callOutput(call, modelOut)
-    or
-    exists(int d |
-      nodeFrom = callInput(call, modelIn, d) and
-      nodeTo = callOutput(call, modelOut, d)
-    )
-  )
-}
+import Cached
 
 /**
  * Holds if data flows from `source` to `sink` in zero or more local
@@ -1377,40 +1395,81 @@ predicate localInstructionFlow(Instruction e1, Instruction e2) {
   localFlow(instructionNode(e1), instructionNode(e2))
 }
 
+cached
+private module ExprFlowCached {
+  /**
+   * Holds if `n1.asExpr()` doesn't have a result and `n1` flows to `n2` in a single
+   * dataflow step.
+   */
+  private predicate localStepFromNonExpr(Node n1, Node n2) {
+    not exists(n1.asExpr()) and
+    localFlowStep(n1, n2)
+  }
+
+  /**
+   * Holds if `n1.asExpr()` doesn't have a result, `n2.asExpr() = e2` and
+   * `n2` is the first node reachable from `n1` such that `n2.asExpr()` exists.
+   */
+  pragma[nomagic]
+  private predicate localStepsToExpr(Node n1, Node n2, Expr e2) {
+    localStepFromNonExpr*(n1, n2) and
+    e2 = n2.asExpr()
+  }
+
+  /**
+   * Holds if `n1.asExpr() = e1` and `n2.asExpr() = e2` and `n2` is the first node
+   * reacahble from `n1` such that `n2.asExpr()` exists.
+   */
+  private predicate localExprFlowSingleExprStep(Node n1, Expr e1, Node n2, Expr e2) {
+    exists(Node mid |
+      localFlowStep(n1, mid) and
+      localStepsToExpr(mid, n2, e2) and
+      e1 = n1.asExpr()
+    )
+  }
+
+  /**
+   * Holds if `n1.asExpr() = e1` and `e1 != e2` and `n2` is the first reachable node from
+   * `n1` such that `n2.asExpr() = e2`.
+   */
+  private predicate localExprFlowStepImpl(Node n1, Expr e1, Node n2, Expr e2) {
+    exists(Node n, Expr e | localExprFlowSingleExprStep(n1, e1, n, e) |
+      // If `n.asExpr()` and `n1.asExpr()` both resolve to the same node (which can
+      // happen if `n2` is the node attached to a conversion of `e1`), then we recursively
+      // perform another expression step.
+      if e1 = e
+      then localExprFlowStepImpl(n, e, n2, e2)
+      else (
+        // If we manage to step to a different expression we're done.
+        e2 = e and
+        n2 = n
+      )
+    )
+  }
+
+  /** Holds if data can flow from `e1` to `e2` in one local (intra-procedural) step. */
+  cached
+  predicate localExprFlowStep(Expr e1, Expr e2) { localExprFlowStepImpl(_, e1, _, e2) }
+}
+
+import ExprFlowCached
+
+/**
+ * Holds if data can flow from `e1` to `e2` in one or more
+ * local (intra-procedural) steps.
+ */
+pragma[inline]
+private predicate localExprFlowPlus(Expr e1, Expr e2) = fastTC(localExprFlowStep/2)(e1, e2)
+
 /**
  * Holds if data can flow from `e1` to `e2` in zero or more
  * local (intra-procedural) steps.
  */
 pragma[inline]
-predicate localExprFlow(Expr e1, Expr e2) { localExprFlowStep*(e1, e2) }
-
-/**
- * Holds if `n1.asExpr()` doesn't have a result and `n1` flows to `n2` in a single
- * dataflow step.
- */
-private predicate localStepFromNonExpr(Node n1, Node n2) {
-  not exists(n1.asExpr()) and
-  localFlowStep(n1, n2)
-}
-
-/**
- * Holds if `n1.asExpr()` doesn't have a result, `n2.asExpr() = e2` and
- * `n2` is the first node reachable from `n1` such that `n2.asExpr()` exists.
- */
-pragma[nomagic]
-private predicate localStepsToExpr(Node n1, Node n2, Expr e2) {
-  localStepFromNonExpr*(n1, n2) and
-  e2 = n2.asExpr()
-}
-
-/** Holds if data can flow from `e1` to `e2` in one local (intra-procedural) step. */
-cached
-predicate localExprFlowStep(Expr e1, Expr e2) {
-  exists(Node mid, Node n1, Node n2 |
-    localFlowStep(n1, mid) and
-    localStepsToExpr(mid, n2, e2) and
-    e1 = n1.asExpr()
-  )
+predicate localExprFlow(Expr e1, Expr e2) {
+  e1 = e2
+  or
+  localExprFlowPlus(e1, e2)
 }
 
 cached
@@ -1530,9 +1589,9 @@ private IRBlock getBasicBlock(Node node) {
   or
   node.(SsaPhiNode).getPhiNode().getBasicBlock() = result
   or
-  node.(IndirectOperand).getOperand().getUse().getBlock() = result
+  node.(RawIndirectOperand).getOperand().getUse().getBlock() = result
   or
-  node.(IndirectInstruction).getInstruction().getBlock() = result
+  node.(RawIndirectInstruction).getInstruction().getBlock() = result
   or
   result = getBasicBlock(node.(PostUpdateNode).getPreUpdateNode())
 }
@@ -1578,10 +1637,11 @@ signature predicate instructionGuardChecksSig(IRGuardCondition g, Instruction in
 module InstructionBarrierGuard<instructionGuardChecksSig/3 instructionGuardChecks> {
   /** Gets a node that is safely guarded by the given guard check. */
   ExprNode getABarrierNode() {
-    exists(IRGuardCondition g, ValueNumber value, boolean edge |
+    exists(IRGuardCondition g, ValueNumber value, boolean edge, Operand use |
       instructionGuardChecks(g, value.getAnInstruction(), edge) and
-      result.asInstruction() = value.getAnInstruction() and
-      g.controls(result.asInstruction().getBlock(), edge)
+      use = value.getAnInstruction().getAUse() and
+      result.asOperand() = use and
+      g.controls(use.getDef().getBlock(), edge)
     )
   }
 }

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

@@ -0,0 +1,644 @@
+/**
+ * INTERNAL: Do not use.
+ *
+ * An IR taint tracking library that uses an IR DataFlow configuration to track
+ * taint from user inputs as defined by `semmle.code.cpp.security.Security`.
+ */
+
+import cpp
+import semmle.code.cpp.security.Security
+private import semmle.code.cpp.ir.dataflow.DataFlow
+private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
+private import semmle.code.cpp.ir.dataflow.DataFlow3
+private import semmle.code.cpp.ir.IR
+private import semmle.code.cpp.ir.dataflow.ResolveCall
+private import semmle.code.cpp.controlflow.IRGuards
+private import semmle.code.cpp.models.interfaces.Taint
+private import semmle.code.cpp.models.interfaces.DataFlow
+private import semmle.code.cpp.ir.dataflow.TaintTracking
+private import semmle.code.cpp.ir.dataflow.TaintTracking2
+private import semmle.code.cpp.ir.dataflow.TaintTracking3
+private import semmle.code.cpp.ir.dataflow.internal.ModelUtil
+
+/**
+ * A predictable instruction is one where an external user can predict
+ * the value. For example, a literal in the source code is considered
+ * predictable.
+ */
+private predicate predictableInstruction(Instruction instr) {
+  instr instanceof ConstantInstruction
+  or
+  instr instanceof StringConstantInstruction
+  or
+  // This could be a conversion on a string literal
+  predictableInstruction(instr.(UnaryInstruction).getUnary())
+}
+
+/**
+ * Functions that we should only allow taint to flow through (to the return
+ * value) if all but the source argument are 'predictable'.  This is done to
+ * emulate the old security library's implementation rather than due to any
+ * strong belief that this is the right approach.
+ *
+ * Note that the list itself is not very principled; it consists of all the
+ * functions listed in the old security library's [default] `isPureFunction`
+ * that have more than one argument, but are not in the old taint tracking
+ * library's `returnArgument` predicate.
+ */
+predicate predictableOnlyFlow(string name) {
+  name =
+    [
+      "strcasestr", "strchnul", "strchr", "strchrnul", "strcmp", "strcspn", "strncmp", "strndup",
+      "strnlen", "strrchr", "strspn", "strstr", "strtod", "strtof", "strtol", "strtoll", "strtoq",
+      "strtoul"
+    ]
+}
+
+private DataFlow::Node getNodeForSource(Expr source) {
+  isUserInput(source, _) and
+  result = getNodeForExpr(source)
+}
+
+private DataFlow::Node getNodeForExpr(Expr node) {
+  result = DataFlow::exprNode(node)
+  or
+  // Some of the sources in `isUserInput` are intended to match the value of
+  // an expression, while others (those modeled below) are intended to match
+  // the taint that propagates out of an argument, like the `char *` argument
+  // to `gets`. It's impossible here to tell which is which, but the "access
+  // to argv" source is definitely not intended to match an output argument,
+  // and it causes false positives if we let it.
+  //
+  // This case goes together with the similar (but not identical) rule in
+  // `nodeIsBarrierIn`.
+  result = DataFlow::definitionByReferenceNodeFromArgument(node) and
+  not argv(node.(VariableAccess).getTarget())
+}
+
+private class DefaultTaintTrackingCfg extends TaintTracking::Configuration {
+  DefaultTaintTrackingCfg() { this = "DefaultTaintTrackingCfg" }
+
+  override predicate isSource(DataFlow::Node source) { source = getNodeForSource(_) }
+
+  override predicate isSink(DataFlow::Node sink) { exists(adjustedSink(sink)) }
+
+  override predicate isSanitizer(DataFlow::Node node) { nodeIsBarrier(node) }
+
+  override predicate isSanitizerIn(DataFlow::Node node) { nodeIsBarrierIn(node) }
+}
+
+private class ToGlobalVarTaintTrackingCfg extends TaintTracking::Configuration {
+  ToGlobalVarTaintTrackingCfg() { this = "GlobalVarTaintTrackingCfg" }
+
+  override predicate isSource(DataFlow::Node source) { source = getNodeForSource(_) }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink.asVariable() instanceof GlobalOrNamespaceVariable
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
+    writesVariable(n1.asInstruction(), n2.asVariable().(GlobalOrNamespaceVariable))
+    or
+    readsVariable(n2.asInstruction(), n1.asVariable().(GlobalOrNamespaceVariable))
+  }
+
+  override predicate isSanitizer(DataFlow::Node node) { nodeIsBarrier(node) }
+
+  override predicate isSanitizerIn(DataFlow::Node node) { nodeIsBarrierIn(node) }
+}
+
+private class FromGlobalVarTaintTrackingCfg extends TaintTracking2::Configuration {
+  FromGlobalVarTaintTrackingCfg() { this = "FromGlobalVarTaintTrackingCfg" }
+
+  override predicate isSource(DataFlow::Node source) {
+    // This set of sources should be reasonably small, which is good for
+    // performance since the set of sinks is very large.
+    exists(ToGlobalVarTaintTrackingCfg otherCfg | otherCfg.hasFlowTo(source))
+  }
+
+  override predicate isSink(DataFlow::Node sink) { exists(adjustedSink(sink)) }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
+    // Additional step for flow out of variables. There is no flow _into_
+    // variables in this configuration, so this step only serves to take flow
+    // out of a variable that's a source.
+    readsVariable(n2.asInstruction(), n1.asVariable())
+  }
+
+  override predicate isSanitizer(DataFlow::Node node) { nodeIsBarrier(node) }
+
+  override predicate isSanitizerIn(DataFlow::Node node) { nodeIsBarrierIn(node) }
+}
+
+private predicate readsVariable(LoadInstruction load, Variable var) {
+  load.getSourceAddress().(VariableAddressInstruction).getAstVariable() = var
+}
+
+private predicate writesVariable(StoreInstruction store, Variable var) {
+  store.getDestinationAddress().(VariableAddressInstruction).getAstVariable() = var
+}
+
+/**
+ * A variable that has any kind of upper-bound check anywhere in the program.  This is
+ * biased towards being inclusive because there are a lot of valid ways of doing an
+ * upper bounds checks if we don't consider where it occurs, for example:
+ * ```
+ *   if (x < 10) { sink(x); }
+ *
+ *   if (10 > y) { sink(y); }
+ *
+ *   if (z > 10) { z = 10; }
+ *   sink(z);
+ * ```
+ */
+// TODO: This coarse overapproximation, ported from the old taint tracking
+// library, could be replaced with an actual semantic check that a particular
+// variable _access_ is guarded by an upper-bound check. We probably don't want
+// to do this right away since it could expose a lot of FPs that were
+// previously suppressed by this predicate by coincidence.
+private predicate hasUpperBoundsCheck(Variable var) {
+  exists(RelationalOperation oper, VariableAccess access |
+    oper.getAnOperand() = access and
+    access.getTarget() = var and
+    // Comparing to 0 is not an upper bound check
+    not oper.getAnOperand().getValue() = "0"
+  )
+}
+
+private predicate nodeIsBarrierEqualityCandidate(
+  DataFlow::Node node, Operand access, Variable checkedVar
+) {
+  readsVariable(node.asInstruction(), checkedVar) and
+  any(IRGuardCondition guard).ensuresEq(access, _, _, node.asInstruction().getBlock(), true)
+}
+
+cached
+private module Cached {
+  cached
+  predicate nodeIsBarrier(DataFlow::Node node) {
+    exists(Variable checkedVar |
+      node.asExpr().(VariableAccess).getTarget() = checkedVar and
+      hasUpperBoundsCheck(checkedVar)
+    )
+    or
+    exists(Variable checkedVar, Operand access |
+      /*
+       * This node is guarded by a condition that forces the accessed variable
+       * to equal something else.  For example:
+       * ```
+       * x = taintsource()
+       * if (x == 10) {
+       *   taintsink(x); // not considered tainted
+       * }
+       * ```
+       */
+
+      nodeIsBarrierEqualityCandidate(node, access, checkedVar) and
+      readsVariable(access.getDef(), checkedVar)
+    )
+  }
+
+  cached
+  predicate nodeIsBarrierIn(DataFlow::Node node) {
+    // don't use dataflow into taint sources, as this leads to duplicate results.
+    exists(Expr source | isUserInput(source, _) |
+      node = DataFlow::exprNode(source)
+      or
+      // This case goes together with the similar (but not identical) rule in
+      // `getNodeForSource`.
+      node = DataFlow::definitionByReferenceNodeFromArgument(source)
+    )
+    or
+    // don't use dataflow into binary instructions if both operands are unpredictable
+    exists(BinaryInstruction iTo |
+      iTo = node.asInstruction() and
+      not predictableInstruction(iTo.getLeft()) and
+      not predictableInstruction(iTo.getRight()) and
+      // propagate taint from either the pointer or the offset, regardless of predictability
+      not iTo instanceof PointerArithmeticInstruction
+    )
+    or
+    // don't use dataflow through calls to pure functions if two or more operands
+    // are unpredictable
+    exists(Instruction iFrom1, Instruction iFrom2, CallInstruction iTo |
+      iTo = node.asInstruction() and
+      isPureFunction(iTo.getStaticCallTarget().getName()) and
+      iFrom1 = iTo.getAnArgument() and
+      iFrom2 = iTo.getAnArgument() and
+      not predictableInstruction(iFrom1) and
+      not predictableInstruction(iFrom2) and
+      iFrom1 != iFrom2
+    )
+  }
+
+  cached
+  Element adjustedSink(DataFlow::Node sink) {
+    // TODO: is it more appropriate to use asConvertedExpr here and avoid
+    // `getConversion*`? Or will that cause us to miss some cases where there's
+    // flow to a conversion (like a `ReferenceDereferenceExpr`) and we want to
+    // pretend there was flow to the converted `Expr` for the sake of
+    // compatibility.
+    sink.asExpr().getConversion*() = result
+    or
+    // For compatibility, send flow from arguments to parameters, even for
+    // functions with no body.
+    exists(FunctionCall call, int i |
+      sink.asExpr() = call.getArgument(pragma[only_bind_into](i)) and
+      result = resolveCall(call).getParameter(pragma[only_bind_into](i))
+    )
+    or
+    // For compatibility, send flow into a `Variable` if there is flow to any
+    // Load or Store of that variable.
+    exists(CopyInstruction copy |
+      copy.getSourceValue() = sink.asInstruction() and
+      (
+        readsVariable(copy, result) or
+        writesVariable(copy, result)
+      ) and
+      not hasUpperBoundsCheck(result)
+    )
+    or
+    // For compatibility, send flow into a `NotExpr` even if it's part of a
+    // short-circuiting condition and thus might get skipped.
+    result.(NotExpr).getOperand() = sink.asExpr()
+    or
+    // Taint postfix and prefix crement operations when their operand is tainted.
+    result.(CrementOperation).getAnOperand() = sink.asExpr()
+    or
+    // Taint `e1 += e2`, `e &= e2` and friends when `e1` or `e2` is tainted.
+    result.(AssignOperation).getAnOperand() = sink.asExpr()
+    or
+    result =
+      sink.asOperand()
+          .(SideEffectOperand)
+          .getUse()
+          .(ReadSideEffectInstruction)
+          .getArgumentDef()
+          .getUnconvertedResultExpression()
+  }
+
+  /**
+   * Step to return value of a modeled function when an input taints the
+   * dereference of the return value.
+   */
+  cached
+  predicate additionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
+    exists(CallInstruction call, Function func, FunctionInput modelIn, FunctionOutput modelOut |
+      n1 = callInput(call, modelIn) and
+      (
+        func.(TaintFunction).hasTaintFlow(modelIn, modelOut)
+        or
+        func.(DataFlowFunction).hasDataFlow(modelIn, modelOut)
+      ) and
+      call.getStaticCallTarget() = func and
+      modelOut.isReturnValueDeref() and
+      call = n2.asInstruction()
+    )
+  }
+}
+
+private import Cached
+
+/**
+ * Holds if `tainted` may contain taint from `source`.
+ *
+ * A tainted expression is either directly user input, or is
+ * computed from user input in a way that users can probably
+ * control the exact output of the computation.
+ *
+ * This doesn't include data flow through global variables.
+ * If you need that you must call `taintedIncludingGlobalVars`.
+ */
+cached
+predicate tainted(Expr source, Element tainted) {
+  exists(DefaultTaintTrackingCfg cfg, DataFlow::Node sink |
+    cfg.hasFlow(getNodeForSource(source), sink) and
+    tainted = adjustedSink(sink)
+  )
+}
+
+/**
+ * Holds if `tainted` may contain taint from `source`, where the taint passed
+ * through a global variable named `globalVar`.
+ *
+ * A tainted expression is either directly user input, or is
+ * computed from user input in a way that users can probably
+ * control the exact output of the computation.
+ *
+ * This version gives the same results as tainted but also includes
+ * data flow through global variables.
+ *
+ * The parameter `globalVar` is the qualified name of the last global variable
+ * used to move the value from source to tainted. If the taint did not pass
+ * through a global variable, then `globalVar = ""`.
+ */
+cached
+predicate taintedIncludingGlobalVars(Expr source, Element tainted, string globalVar) {
+  tainted(source, tainted) and
+  globalVar = ""
+  or
+  exists(
+    ToGlobalVarTaintTrackingCfg toCfg, FromGlobalVarTaintTrackingCfg fromCfg,
+    DataFlow::VariableNode variableNode, GlobalOrNamespaceVariable global, DataFlow::Node sink
+  |
+    global = variableNode.getVariable() and
+    toCfg.hasFlow(getNodeForSource(source), variableNode) and
+    fromCfg.hasFlow(variableNode, sink) and
+    tainted = adjustedSink(sink) and
+    global = globalVarFromId(globalVar)
+  )
+}
+
+/**
+ * Gets the global variable whose qualified name is `id`. Use this predicate
+ * together with `taintedIncludingGlobalVars`. Example:
+ *
+ * ```
+ * exists(string varName |
+ *   taintedIncludingGlobalVars(source, tainted, varName) and
+ *   var = globalVarFromId(varName)
+ * )
+ * ```
+ */
+GlobalOrNamespaceVariable globalVarFromId(string id) { id = result.getQualifiedName() }
+
+/**
+ * Provides definitions for augmenting source/sink pairs with data-flow paths
+ * between them. From a `@kind path-problem` query, import this module in the
+ * global scope, extend `TaintTrackingConfiguration`, and use `taintedWithPath`
+ * in place of `tainted`.
+ *
+ * Importing this module will also import the query predicates that contain the
+ * taint paths.
+ */
+module TaintedWithPath {
+  private newtype TSingleton = MkSingleton()
+
+  /**
+   * A taint-tracking configuration that matches sources and sinks in the same
+   * way as the `tainted` predicate.
+   *
+   * Override `isSink` and `taintThroughGlobals` as needed, but do not provide
+   * a characteristic predicate.
+   */
+  class TaintTrackingConfiguration extends TSingleton {
+    /** Override this to specify which elements are sources in this configuration. */
+    predicate isSource(Expr source) { exists(getNodeForSource(source)) }
+
+    /** Override this to specify which elements are sinks in this configuration. */
+    abstract predicate isSink(Element e);
+
+    /** Override this to specify which expressions are barriers in this configuration. */
+    predicate isBarrier(Expr e) { nodeIsBarrier(getNodeForExpr(e)) }
+
+    /**
+     * Override this predicate to `any()` to allow taint to flow through global
+     * variables.
+     */
+    predicate taintThroughGlobals() { none() }
+
+    /** Gets a textual representation of this element. */
+    string toString() { result = "TaintTrackingConfiguration" }
+  }
+
+  private class AdjustedConfiguration extends TaintTracking3::Configuration {
+    AdjustedConfiguration() { this = "AdjustedConfiguration" }
+
+    override predicate isSource(DataFlow::Node source) {
+      exists(TaintTrackingConfiguration cfg, Expr e |
+        cfg.isSource(e) and source = getNodeForExpr(e)
+      )
+    }
+
+    override predicate isSink(DataFlow::Node sink) {
+      exists(TaintTrackingConfiguration cfg | cfg.isSink(adjustedSink(sink)))
+    }
+
+    override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
+      // Steps into and out of global variables
+      exists(TaintTrackingConfiguration cfg | cfg.taintThroughGlobals() |
+        writesVariable(n1.asInstruction(), n2.asVariable().(GlobalOrNamespaceVariable))
+        or
+        readsVariable(n2.asInstruction(), n1.asVariable().(GlobalOrNamespaceVariable))
+      )
+      or
+      additionalTaintStep(n1, n2)
+    }
+
+    override predicate isSanitizer(DataFlow::Node node) {
+      exists(TaintTrackingConfiguration cfg, Expr e | cfg.isBarrier(e) and node = getNodeForExpr(e))
+    }
+
+    override predicate isSanitizerIn(DataFlow::Node node) { nodeIsBarrierIn(node) }
+  }
+
+  /*
+   * A sink `Element` may map to multiple `DataFlowX::PathNode`s via (the
+   * inverse of) `adjustedSink`. For example, an `Expr` maps to all its
+   * conversions, and a `Variable` maps to all loads and stores from it. Because
+   * the path node is part of the tuple that constitutes the alert, this leads
+   * to duplicate alerts.
+   *
+   * To avoid showing duplicates, we edit the graph to replace the final node
+   * coming from the data-flow library with a node that matches exactly the
+   * `Element` sink that's requested.
+   *
+   * The same is done for sources.
+   */
+
+  private newtype TPathNode =
+    TWrapPathNode(DataFlow3::PathNode n) or
+    // There's a single newtype constructor for both sources and sinks since
+    // that makes it easiest to deal with the case where source = sink.
+    TEndpointPathNode(Element e) {
+      exists(AdjustedConfiguration cfg, DataFlow3::Node sourceNode, DataFlow3::Node sinkNode |
+        cfg.hasFlow(sourceNode, sinkNode)
+      |
+        sourceNode = getNodeForExpr(e) and
+        exists(TaintTrackingConfiguration ttCfg | ttCfg.isSource(e))
+        or
+        e = adjustedSink(sinkNode) and
+        exists(TaintTrackingConfiguration ttCfg | ttCfg.isSink(e))
+      )
+    }
+
+  /** An opaque type used for the nodes of a data-flow path. */
+  class PathNode extends TPathNode {
+    /** Gets a textual representation of this element. */
+    string toString() { none() }
+
+    /**
+     * Holds if this element is at the specified location.
+     * The location spans column `startcolumn` of line `startline` to
+     * column `endcolumn` of line `endline` in file `filepath`.
+     * For more information, see
+     * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
+     */
+    predicate hasLocationInfo(
+      string filepath, int startline, int startcolumn, int endline, int endcolumn
+    ) {
+      none()
+    }
+  }
+
+  /**
+   * INTERNAL: Do not use.
+   */
+  module Private {
+    /** Gets a predecessor `PathNode` of `pathNode`, if any. */
+    PathNode getAPredecessor(PathNode pathNode) { edges(result, pathNode) }
+
+    /** Gets the element that `pathNode` wraps, if any. */
+    Element getElementFromPathNode(PathNode pathNode) {
+      exists(DataFlow::Node node | node = pathNode.(WrapPathNode).inner().getNode() |
+        result = node.asInstruction().getAst()
+        or
+        result = node.asOperand().getDef().getAst()
+      )
+      or
+      result = pathNode.(EndpointPathNode).inner()
+    }
+  }
+
+  private class WrapPathNode extends PathNode, TWrapPathNode {
+    DataFlow3::PathNode inner() { this = TWrapPathNode(result) }
+
+    override string toString() { result = this.inner().toString() }
+
+    override predicate hasLocationInfo(
+      string filepath, int startline, int startcolumn, int endline, int endcolumn
+    ) {
+      this.inner().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    }
+  }
+
+  private class EndpointPathNode extends PathNode, TEndpointPathNode {
+    Expr inner() { this = TEndpointPathNode(result) }
+
+    override string toString() { result = this.inner().toString() }
+
+    override predicate hasLocationInfo(
+      string filepath, int startline, int startcolumn, int endline, int endcolumn
+    ) {
+      this.inner()
+          .getLocation()
+          .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    }
+  }
+
+  /** A PathNode whose `Element` is a source. It may also be a sink. */
+  private class InitialPathNode extends EndpointPathNode {
+    InitialPathNode() { exists(TaintTrackingConfiguration cfg | cfg.isSource(this.inner())) }
+  }
+
+  /** A PathNode whose `Element` is a sink. It may also be a source. */
+  private class FinalPathNode extends EndpointPathNode {
+    FinalPathNode() { exists(TaintTrackingConfiguration cfg | cfg.isSink(this.inner())) }
+  }
+
+  /** Holds if `(a,b)` is an edge in the graph of data flow path explanations. */
+  query predicate edges(PathNode a, PathNode b) {
+    DataFlow3::PathGraph::edges(a.(WrapPathNode).inner(), b.(WrapPathNode).inner())
+    or
+    // To avoid showing trivial-looking steps, we _replace_ the last node instead
+    // of adding an edge out of it.
+    exists(WrapPathNode sinkNode |
+      DataFlow3::PathGraph::edges(a.(WrapPathNode).inner(), sinkNode.inner()) and
+      b.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
+    )
+    or
+    // Same for the first node
+    exists(WrapPathNode sourceNode |
+      DataFlow3::PathGraph::edges(sourceNode.inner(), b.(WrapPathNode).inner()) and
+      sourceNode.inner().getNode() = getNodeForExpr(a.(InitialPathNode).inner())
+    )
+    or
+    // Finally, handle the case where the path goes directly from a source to a
+    // sink, meaning that they both need to be translated.
+    exists(WrapPathNode sinkNode, WrapPathNode sourceNode |
+      DataFlow3::PathGraph::edges(sourceNode.inner(), sinkNode.inner()) and
+      sourceNode.inner().getNode() = getNodeForExpr(a.(InitialPathNode).inner()) and
+      b.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
+    )
+  }
+
+  /**
+   * Holds if there is flow from `arg` to `out` across a call that can by summarized by the flow
+   * from `par` to `ret` within it, in the graph of data flow path explanations.
+   */
+  query predicate subpaths(PathNode arg, PathNode par, PathNode ret, PathNode out) {
+    DataFlow3::PathGraph::subpaths(arg.(WrapPathNode).inner(), par.(WrapPathNode).inner(),
+      ret.(WrapPathNode).inner(), out.(WrapPathNode).inner())
+    or
+    // To avoid showing trivial-looking steps, we _replace_ the last node instead
+    // of adding an edge out of it.
+    exists(WrapPathNode sinkNode |
+      DataFlow3::PathGraph::subpaths(arg.(WrapPathNode).inner(), par.(WrapPathNode).inner(),
+        ret.(WrapPathNode).inner(), sinkNode.inner()) and
+      out.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
+    )
+    or
+    // Same for the first node
+    exists(WrapPathNode sourceNode |
+      DataFlow3::PathGraph::subpaths(sourceNode.inner(), par.(WrapPathNode).inner(),
+        ret.(WrapPathNode).inner(), out.(WrapPathNode).inner()) and
+      sourceNode.inner().getNode() = getNodeForExpr(arg.(InitialPathNode).inner())
+    )
+    or
+    // Finally, handle the case where the path goes directly from a source to a
+    // sink, meaning that they both need to be translated.
+    exists(WrapPathNode sinkNode, WrapPathNode sourceNode |
+      DataFlow3::PathGraph::subpaths(sourceNode.inner(), par.(WrapPathNode).inner(),
+        ret.(WrapPathNode).inner(), sinkNode.inner()) and
+      sourceNode.inner().getNode() = getNodeForExpr(arg.(InitialPathNode).inner()) and
+      out.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
+    )
+  }
+
+  /** Holds if `n` is a node in the graph of data flow path explanations. */
+  query predicate nodes(PathNode n, string key, string val) {
+    key = "semmle.label" and val = n.toString()
+  }
+
+  /**
+   * Holds if `tainted` may contain taint from `source`, where `sourceNode` and
+   * `sinkNode` are the corresponding `PathNode`s that can be used in a query
+   * to provide path explanations. Extend `TaintTrackingConfiguration` to use
+   * this predicate.
+   *
+   * A tainted expression is either directly user input, or is computed from
+   * user input in a way that users can probably control the exact output of
+   * the computation.
+   */
+  predicate taintedWithPath(Expr source, Element tainted, PathNode sourceNode, PathNode sinkNode) {
+    exists(AdjustedConfiguration cfg, DataFlow3::Node flowSource, DataFlow3::Node flowSink |
+      source = sourceNode.(InitialPathNode).inner() and
+      flowSource = getNodeForExpr(source) and
+      cfg.hasFlow(flowSource, flowSink) and
+      tainted = adjustedSink(flowSink) and
+      tainted = sinkNode.(FinalPathNode).inner()
+    )
+  }
+
+  private predicate isGlobalVariablePathNode(WrapPathNode n) {
+    n.inner().getNode().asVariable() instanceof GlobalOrNamespaceVariable
+  }
+
+  private predicate edgesWithoutGlobals(PathNode a, PathNode b) {
+    edges(a, b) and
+    not isGlobalVariablePathNode(a) and
+    not isGlobalVariablePathNode(b)
+  }
+
+  /**
+   * Holds if `tainted` can be reached from a taint source without passing
+   * through a global variable.
+   */
+  predicate taintedWithoutGlobals(Element tainted) {
+    exists(AdjustedConfiguration cfg, PathNode sourceNode, FinalPathNode sinkNode |
+      cfg.isSource(sourceNode.(WrapPathNode).inner().getNode()) and
+      edgesWithoutGlobals+(sourceNode, sinkNode) and
+      tainted = sinkNode.inner()
+    )
+  }
+}

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

@@ -19,12 +19,6 @@ private module SourceVariables {
     )
   }
 
-  class BaseSourceVariable = SsaInternals0::BaseSourceVariable;
-
-  class BaseIRVariable = SsaInternals0::BaseIRVariable;
-
-  class BaseCallVariable = SsaInternals0::BaseCallVariable;
-
   cached
   private newtype TSourceVariable =
     TSourceIRVariable(BaseIRVariable baseVar, int ind) {
@@ -86,21 +80,31 @@ private module SourceVariables {
 
 import SourceVariables
 
-predicate hasIndirectOperand(Operand op, int indirectionIndex) {
+/**
+ * Holds if the `(operand, indirectionIndex)` columns should be
+ * assigned a `RawIndirectOperand` value.
+ */
+predicate hasRawIndirectOperand(Operand op, int indirectionIndex) {
   exists(CppType type, int m |
     not ignoreOperand(op) and
     type = getLanguageType(op) and
     m = countIndirectionsForCppType(type) and
-    indirectionIndex = [1 .. m]
+    indirectionIndex = [1 .. m] and
+    not exists(getIRRepresentationOfIndirectOperand(op, indirectionIndex))
   )
 }
 
-predicate hasIndirectInstruction(Instruction instr, int indirectionIndex) {
+/**
+ * Holds if the `(instr, indirectionIndex)` columns should be
+ * assigned a `RawIndirectInstruction` value.
+ */
+predicate hasRawIndirectInstruction(Instruction instr, int indirectionIndex) {
   exists(CppType type, int m |
     not ignoreInstruction(instr) and
     type = getResultLanguageType(instr) and
     m = countIndirectionsForCppType(type) and
-    indirectionIndex = [1 .. m]
+    indirectionIndex = [1 .. m] and
+    not exists(getIRRepresentationOfIndirectInstruction(instr, indirectionIndex))
   )
 }
 
@@ -140,6 +144,17 @@ private newtype TDefOrUseImpl =
       isUse(_, _, vai, _, indirectionIndex) and
       not isDef(_, _, vai.getAUse(), _, _, _)
     )
+  } or
+  TIteratorDef(
+    Operand iteratorAddress, BaseSourceVariableInstruction container, int indirectionIndex
+  ) {
+    isIteratorDef(container, iteratorAddress, _, _, indirectionIndex) and
+    any(SsaInternals0::Def def | def.isIteratorDef()).getAddressOperand() = iteratorAddress
+  } or
+  TIteratorUse(
+    Operand iteratorAddress, BaseSourceVariableInstruction container, int indirectionIndex
+  ) {
+    isIteratorUse(container, iteratorAddress, _, indirectionIndex)
   }
 
 abstract private class DefOrUseImpl extends TDefOrUseImpl {
@@ -174,15 +189,10 @@ abstract private class DefOrUseImpl extends TDefOrUseImpl {
    * Gets the instruction that computes the base of this definition or use.
    * This is always a `VariableAddressInstruction` or an `AllocationInstruction`.
    */
-  abstract Instruction getBase();
+  abstract BaseSourceVariableInstruction getBase();
 
   final BaseSourceVariable getBaseSourceVariable() {
-    exists(IRVariable var |
-      result.(BaseIRVariable).getIRVariable() = var and
-      instructionHasIRVariable(this.getBase(), var)
-    )
-    or
-    result.(BaseCallVariable).getCallInstruction() = this.getBase()
+    this.getBase().getBaseSourceVariable() = result
   }
 
   /** Gets the variable that is defined or used. */
@@ -194,11 +204,6 @@ abstract private class DefOrUseImpl extends TDefOrUseImpl {
   }
 }
 
-pragma[noinline]
-private predicate instructionHasIRVariable(VariableAddressInstruction vai, IRVariable var) {
-  vai.getIRVariable() = var
-}
-
 private predicate defOrUseHasSourceVariable(DefOrUseImpl defOrUse, BaseSourceVariable bv, int ind) {
   defHasSourceVariable(defOrUse, bv, ind)
   or
@@ -223,40 +228,66 @@ private predicate sourceVariableHasBaseAndIndex(SourceVariable v, BaseSourceVari
   v.getIndirection() = ind
 }
 
-class DefImpl extends DefOrUseImpl, TDefImpl {
+abstract class DefImpl extends DefOrUseImpl {
   Operand address;
   int ind;
 
-  DefImpl() { this = TDefImpl(address, ind) }
+  bindingset[ind]
+  DefImpl() { any() }
 
-  override Instruction getBase() { isDef(_, _, address, result, _, _) }
+  abstract int getIndirection();
+
+  abstract Node0Impl getValue();
+
+  abstract predicate isCertain();
 
   Operand getAddressOperand() { result = address }
 
-  int getIndirection() { isDef(_, _, address, _, result, ind) }
-
   override int getIndirectionIndex() { result = ind }
 
-  Instruction getDefiningInstruction() { isDef(_, result, address, _, _, _) }
-
   override string toString() { result = "DefImpl" }
 
-  override IRBlock getBlock() { result = this.getDefiningInstruction().getBlock() }
+  override IRBlock getBlock() { result = this.getAddressOperand().getUse().getBlock() }
 
-  override Cpp::Location getLocation() { result = this.getDefiningInstruction().getLocation() }
+  override Cpp::Location getLocation() { result = this.getAddressOperand().getUse().getLocation() }
 
   final override predicate hasIndexInBlock(IRBlock block, int index) {
-    this.getDefiningInstruction() = block.getInstruction(index)
+    this.getAddressOperand().getUse() = block.getInstruction(index)
   }
-
-  predicate isCertain() { isDef(true, _, address, _, _, ind) }
 }
 
-class UseImpl extends DefOrUseImpl, TUseImpl {
+private class DirectDef extends DefImpl, TDefImpl {
+  DirectDef() { this = TDefImpl(address, ind) }
+
+  override BaseSourceVariableInstruction getBase() { isDef(_, _, address, result, _, _) }
+
+  override int getIndirection() { isDef(_, _, address, _, result, ind) }
+
+  override Node0Impl getValue() { isDef(_, result, address, _, _, _) }
+
+  override predicate isCertain() { isDef(true, _, address, _, _, ind) }
+}
+
+private class IteratorDef extends DefImpl, TIteratorDef {
+  BaseSourceVariableInstruction container;
+
+  IteratorDef() { this = TIteratorDef(address, container, ind) }
+
+  override BaseSourceVariableInstruction getBase() { result = container }
+
+  override int getIndirection() { isIteratorDef(container, address, _, result, ind) }
+
+  override Node0Impl getValue() { isIteratorDef(container, address, result, _, _) }
+
+  override predicate isCertain() { none() }
+}
+
+abstract class UseImpl extends DefOrUseImpl {
   Operand operand;
   int ind;
 
-  UseImpl() { this = TUseImpl(operand, ind) }
+  bindingset[ind]
+  UseImpl() { any() }
 
   Operand getOperand() { result = operand }
 
@@ -270,13 +301,33 @@ class UseImpl extends DefOrUseImpl, TUseImpl {
 
   final override Cpp::Location getLocation() { result = operand.getLocation() }
 
-  final int getIndirection() { isUse(_, operand, _, result, ind) }
-
   override int getIndirectionIndex() { result = ind }
 
-  override Instruction getBase() { isUse(_, operand, result, _, ind) }
+  abstract int getIndirection();
 
-  predicate isCertain() { isUse(true, operand, _, _, ind) }
+  abstract predicate isCertain();
+}
+
+private class DirectUse extends UseImpl, TUseImpl {
+  DirectUse() { this = TUseImpl(operand, ind) }
+
+  override int getIndirection() { isUse(_, operand, _, result, ind) }
+
+  override BaseSourceVariableInstruction getBase() { isUse(_, operand, result, _, ind) }
+
+  override predicate isCertain() { isUse(true, operand, _, _, ind) }
+}
+
+private class IteratorUse extends UseImpl, TIteratorUse {
+  BaseSourceVariableInstruction container;
+
+  IteratorUse() { this = TIteratorUse(operand, container, ind) }
+
+  override int getIndirection() { isIteratorUse(container, operand, result, ind) }
+
+  override BaseSourceVariableInstruction getBase() { result = container }
+
+  override predicate isCertain() { none() }
 }
 
 class GlobalUse extends TGlobalUse {
@@ -354,10 +405,10 @@ predicate adjacentDefRead(DefOrUse defOrUse1, UseOrPhi use) {
   exists(IRBlock bb1, int i1, SourceVariable v |
     defOrUse1.asDefOrUse().hasIndexInBlock(bb1, i1, v)
   |
-    exists(IRBlock bb2, int i2 |
-      adjacentDefRead(_, pragma[only_bind_into](bb1), pragma[only_bind_into](i1),
-        pragma[only_bind_into](bb2), pragma[only_bind_into](i2))
-    |
+    exists(IRBlock bb2, int i2, Definition def |
+      adjacentDefRead(pragma[only_bind_into](def), pragma[only_bind_into](bb1),
+        pragma[only_bind_into](i1), pragma[only_bind_into](bb2), pragma[only_bind_into](i2)) and
+      def.getSourceVariable() = v and
       use.asDefOrUse().(UseImpl).hasIndexInBlock(bb2, i2, v)
     )
     or
@@ -412,8 +463,13 @@ predicate outNodeHasAddressAndIndex(
   out.getIndirectionIndex() = indirectionIndex
 }
 
-private predicate defToNode(Node nodeFrom, Def def) {
-  nodeHasInstruction(nodeFrom, def.getDefiningInstruction(), def.getIndirectionIndex())
+private predicate defToNode(Node nodeFrom, Def def, boolean uncertain) {
+  (
+    nodeHasOperand(nodeFrom, def.getValue().asOperand(), def.getIndirectionIndex())
+    or
+    nodeHasInstruction(nodeFrom, def.getValue().asInstruction(), def.getIndirectionIndex())
+  ) and
+  if def.isCertain() then uncertain = false else uncertain = true
 }
 
 /**
@@ -421,12 +477,13 @@ private predicate defToNode(Node nodeFrom, Def def) {
  *
  * Holds if `nodeFrom` is the node that correspond to the definition or use `defOrUse`.
  */
-predicate nodeToDefOrUse(Node nodeFrom, SsaDefOrUse defOrUse) {
+predicate nodeToDefOrUse(Node nodeFrom, SsaDefOrUse defOrUse, boolean uncertain) {
   // Node -> Def
-  defToNode(nodeFrom, defOrUse)
+  defToNode(nodeFrom, defOrUse, uncertain)
   or
   // Node -> Use
-  useToNode(defOrUse, nodeFrom)
+  useToNode(defOrUse, nodeFrom) and
+  uncertain = false
 }
 
 /**
@@ -435,7 +492,7 @@ predicate nodeToDefOrUse(Node nodeFrom, SsaDefOrUse defOrUse) {
  */
 private predicate indirectConversionFlowStep(Node nFrom, Node nTo) {
   not exists(UseOrPhi defOrUse |
-    nodeToDefOrUse(nTo, defOrUse) and
+    nodeToDefOrUse(nTo, defOrUse, _) and
     adjacentDefRead(defOrUse, _)
   ) and
   exists(Operand op1, Operand op2, int indirectionIndex, Instruction instr |
@@ -461,33 +518,32 @@ private predicate indirectConversionFlowStep(Node nFrom, Node nTo) {
  * So this predicate recurses back along conversions and `PointerArithmeticInstruction`s to find the
  * first use that has provides use-use flow, and uses that target as the target of the `nodeFrom`.
  */
-private predicate adjustForPointerArith(Node nodeFrom, UseOrPhi use) {
+private predicate adjustForPointerArith(Node nodeFrom, UseOrPhi use, boolean uncertain) {
   nodeFrom = any(PostUpdateNode pun).getPreUpdateNode() and
   exists(DefOrUse defOrUse, Node adjusted |
     indirectConversionFlowStep*(adjusted, nodeFrom) and
-    nodeToDefOrUse(adjusted, defOrUse) and
+    nodeToDefOrUse(adjusted, defOrUse, uncertain) and
     adjacentDefRead(defOrUse, use)
   )
 }
 
-/** Holds if there is def-use or use-use flow from `nodeFrom` to `nodeTo`. */
-predicate ssaFlow(Node nodeFrom, Node nodeTo) {
+private predicate ssaFlowImpl(Node nodeFrom, Node nodeTo, boolean uncertain) {
   // `nodeFrom = any(PostUpdateNode pun).getPreUpdateNode()` is implied by adjustedForPointerArith.
   exists(UseOrPhi use |
-    adjustForPointerArith(nodeFrom, use) and
+    adjustForPointerArith(nodeFrom, use, uncertain) and
     useToNode(use, nodeTo)
   )
   or
   not nodeFrom = any(PostUpdateNode pun).getPreUpdateNode() and
   exists(DefOrUse defOrUse1, UseOrPhi use |
-    nodeToDefOrUse(nodeFrom, defOrUse1) and
+    nodeToDefOrUse(nodeFrom, defOrUse1, uncertain) and
     adjacentDefRead(defOrUse1, use) and
     useToNode(use, nodeTo)
   )
   or
   // Def/use to final value of global vairable
   exists(DefOrUse defOrUse, GlobalUse globalUse |
-    nodeToDefOrUse(nodeFrom, defOrUse) and
+    nodeToDefOrUse(nodeFrom, defOrUse, uncertain) and
     defOrUseToGlobalUse(defOrUse, globalUse) and
     nodeTo.(FinalGlobalValue).getGlobalUse() = globalUse
   )
@@ -496,7 +552,38 @@ predicate ssaFlow(Node nodeFrom, Node nodeTo) {
   exists(GlobalDef globalDef, UseOrPhi use |
     nodeFrom.(InitialGlobalValue).getGlobalDef() = globalDef and
     globalDefToUse(globalDef, use) and
-    useToNode(use, nodeTo)
+    useToNode(use, nodeTo) and
+    uncertain = false
+  )
+}
+
+/**
+ * Holds if `def` is the corresponding definition of
+ * the SSA library's `definition`.
+ */
+private predicate ssaDefinition(Def def, Definition definition) {
+  exists(IRBlock block, int i, SourceVariable sv |
+    def.hasIndexInBlock(block, i, sv) and
+    definition.definesAt(sv, block, i)
+  )
+}
+
+/** Gets a node that represents the prior definition of `node`. */
+private Node getAPriorDefinition(Node node) {
+  exists(Def def, Definition definition, Definition inp, Def input |
+    defToNode(node, def, true) and
+    ssaDefinition(def, definition) and
+    uncertainWriteDefinitionInput(pragma[only_bind_into](definition), pragma[only_bind_into](inp)) and
+    ssaDefinition(input, inp) and
+    defToNode(result, input, _)
+  )
+}
+
+/** Holds if there is def-use or use-use flow from `nodeFrom` to `nodeTo`. */
+predicate ssaFlow(Node nodeFrom, Node nodeTo) {
+  exists(Node nFrom, boolean uncertain |
+    ssaFlowImpl(nFrom, nodeTo, uncertain) and
+    if uncertain = true then nodeFrom = [nFrom, getAPriorDefinition(nFrom)] else nodeFrom = nFrom
   )
 }
 
@@ -531,14 +618,6 @@ predicate fromPhiNode(SsaPhiNode nodeFrom, Node nodeTo) {
   )
 }
 
-private SsaInternals0::SourceVariable getOldSourceVariable(SourceVariable v) {
-  v.getBaseVariable().(BaseIRVariable).getIRVariable() =
-    result.getBaseVariable().(SsaInternals0::BaseIRVariable).getIRVariable()
-  or
-  v.getBaseVariable().(BaseCallVariable).getCallInstruction() =
-    result.getBaseVariable().(SsaInternals0::BaseCallVariable).getCallInstruction()
-}
-
 /**
  * Holds if there is a write at index `i` in basic block `bb` to variable `v` that's
  * subsequently read (as determined by the SSA pruning stage).
@@ -546,7 +625,7 @@ private SsaInternals0::SourceVariable getOldSourceVariable(SourceVariable v) {
 private predicate variableWriteCand(IRBlock bb, int i, SourceVariable v) {
   exists(SsaInternals0::Def def, SsaInternals0::SourceVariable v0 |
     def.asDefOrUse().hasIndexInBlock(bb, i, v0) and
-    v0 = getOldSourceVariable(v)
+    v0 = v.getBaseVariable()
   )
 }
 
@@ -625,6 +704,11 @@ module SsaCached {
   predicate lastRefRedef(Definition def, IRBlock bb, int i, Definition next) {
     SsaImpl::lastRefRedef(def, bb, i, next)
   }
+
+  cached
+  predicate uncertainWriteDefinitionInput(SsaImpl::UncertainWriteDefinition def, Definition inp) {
+    SsaImpl::uncertainWriteDefinitionInput(def, inp)
+  }
 }
 
 cached
@@ -663,6 +747,10 @@ class DefOrUse extends TDefOrUse, SsaDefOrUse {
   final SourceVariable getSourceVariable() { result = defOrUse.getSourceVariable() }
 
   override string toString() { result = defOrUse.toString() }
+
+  predicate hasIndexInBlock(IRBlock block, int index, SourceVariable sv) {
+    defOrUse.hasIndexInBlock(block, index, sv)
+  }
 }
 
 class Phi extends TPhi, SsaDefOrUse {
@@ -705,7 +793,9 @@ class Def extends DefOrUse {
     pragma[only_bind_into](result) = pragma[only_bind_out](defOrUse).getIndirectionIndex()
   }
 
-  Instruction getDefiningInstruction() { result = defOrUse.getDefiningInstruction() }
+  Node0Impl getValue() { result = defOrUse.getValue() }
+
+  predicate isCertain() { defOrUse.isCertain() }
 }
 
 private module SsaImpl = SsaImplCommon::Make<SsaInput>;
@@ -714,4 +804,6 @@ class PhiNode = SsaImpl::PhiNode;
 
 class Definition = SsaImpl::Definition;
 
+class UncertainWriteDefinition = SsaImpl::UncertainWriteDefinition;
+
 import SsaCached

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

@@ -4,6 +4,7 @@ import semmle.code.cpp.ir.internal.IRCppLanguage
 private import semmle.code.cpp.ir.implementation.raw.internal.SideEffects as SideEffects
 private import DataFlowImplCommon as DataFlowImplCommon
 private import DataFlowUtil
+private import DataFlowPrivate
 
 /**
  * Holds if `operand` is an operand that is not used by the dataflow library.
@@ -28,7 +29,9 @@ predicate ignoreInstruction(Instruction instr) {
     instr instanceof PhiInstruction or
     instr instanceof ReadSideEffectInstruction or
     instr instanceof ChiInstruction or
-    instr instanceof InitializeIndirectionInstruction
+    instr instanceof InitializeIndirectionInstruction or
+    instr instanceof AliasedDefinitionInstruction or
+    instr instanceof InitializeNonLocalInstruction
   )
 }
 
@@ -81,6 +84,14 @@ int getMaxIndirectionsForType(Type type) {
   result = countIndirectionsForCppType(getTypeForGLValue(type))
 }
 
+private class PointerOrReferenceType extends Cpp::DerivedType {
+  PointerOrReferenceType() {
+    this instanceof Cpp::PointerType
+    or
+    this instanceof Cpp::ReferenceType
+  }
+}
+
 /**
  * Gets the maximum number of indirections a value of type `type` can have.
  *
@@ -88,12 +99,9 @@ int getMaxIndirectionsForType(Type type) {
  * (i.e., `countIndirections(e.getUnspecifiedType())`).
  */
 private int countIndirections(Type t) {
-  result =
-    1 +
-      countIndirections([t.(Cpp::PointerType).getBaseType(), t.(Cpp::ReferenceType).getBaseType()])
+  result = any(Indirection ind | ind.getType() = t).getNumberOfIndirections()
   or
-  not t instanceof Cpp::PointerType and
-  not t instanceof Cpp::ReferenceType and
+  not exists(Indirection ind | ind.getType() = t) and
   result = 0
 }
 
@@ -120,14 +128,224 @@ class AllocationInstruction extends CallInstruction {
 }
 
 /**
- * Holds if `i` is a base instruction that starts a sequence of uses
- * of some variable that SSA can handle.
+ * An abstract class for handling indirections.
  *
- * This is either when `i` is a `VariableAddressInstruction` or when
- * `i` is a fresh allocation produced by an `AllocationInstruction`.
+ * Extend this class to make a type behave as a pointer for the
+ * purposes of dataflow.
  */
-private predicate isSourceVariableBase(Instruction i) {
-  i instanceof VariableAddressInstruction or i instanceof AllocationInstruction
+abstract class Indirection extends Type {
+  Type baseType;
+
+  /** Gets the type of this indirection. */
+  final Type getType() { result = super.getUnspecifiedType() }
+
+  /**
+   * Gets the number of indirections supported by this type.
+   *
+   * For example, the number of indirections of a variable `p` of type
+   * `int**` is `3` (i.e., `p`, `*p` and `**p`).
+   */
+  abstract int getNumberOfIndirections();
+
+  /**
+   * Holds if `deref` is an instruction that behaves as a `LoadInstruction`
+   * that loads the value computed by `address`.
+   */
+  predicate isAdditionalDereference(Instruction deref, Operand address) { none() }
+
+  /**
+   * Holds if `value` is written to the address computed by `address`.
+   *
+   * `certain` is `true` if this write is guaranteed to write to the address.
+   */
+  predicate isAdditionalWrite(Node0Impl value, Operand address, boolean certain) { none() }
+
+  /**
+   * Gets the base type of this indirection, after specifiers have been deeply
+   * stripped and typedefs have been resolved.
+   *
+   * For example, the base type of `int*&` is `int*`, and the base type of `int*` is `int`.
+   */
+  final Type getBaseType() { result = baseType.getUnspecifiedType() }
+
+  /** Holds if there should be an additional taint step from `node1` to `node2`. */
+  predicate isAdditionalTaintStep(Node node1, Node node2) { none() }
+
+  /**
+   * Holds if the step from `opFrom` to `instrTo` should be considered a conversion
+   * from `opFrom` to `instrTo`.
+   */
+  predicate isAdditionalConversionFlow(Operand opFrom, Instruction instrTo) { none() }
+}
+
+private class PointerOrReferenceTypeIndirection extends Indirection instanceof PointerOrReferenceType {
+  PointerOrReferenceTypeIndirection() { baseType = PointerOrReferenceType.super.getBaseType() }
+
+  override int getNumberOfIndirections() { result = 1 + countIndirections(this.getBaseType()) }
+
+  override predicate isAdditionalDereference(Instruction deref, Operand address) { none() }
+
+  override predicate isAdditionalWrite(Node0Impl value, Operand address, boolean certain) { none() }
+}
+
+private module IteratorIndirections {
+  import semmle.code.cpp.models.interfaces.Iterator as Interfaces
+  import semmle.code.cpp.models.implementations.Iterator as Iterator
+  import semmle.code.cpp.models.implementations.StdContainer as StdContainer
+
+  class IteratorIndirection extends Indirection instanceof Interfaces::Iterator {
+    IteratorIndirection() {
+      not this instanceof PointerOrReferenceTypeIndirection and baseType = super.getValueType()
+    }
+
+    override int getNumberOfIndirections() { result = 1 + countIndirections(this.getBaseType()) }
+
+    override predicate isAdditionalDereference(Instruction deref, Operand address) {
+      exists(CallInstruction call |
+        operandForfullyConvertedCall(deref.getAUse(), call) and
+        this = call.getStaticCallTarget().getClassAndName("operator*") and
+        address = call.getThisArgumentOperand()
+      )
+    }
+
+    override predicate isAdditionalWrite(Node0Impl value, Operand address, boolean certain) {
+      exists(CallInstruction call | call.getArgumentOperand(0) = value.asOperand() |
+        this = call.getStaticCallTarget().getClassAndName("operator=") and
+        address = call.getThisArgumentOperand() and
+        certain = false
+      )
+    }
+
+    override predicate isAdditionalTaintStep(Node node1, Node node2) {
+      exists(CallInstruction call |
+        // Taint through `operator+=` and `operator-=` on iterators.
+        call.getStaticCallTarget() instanceof Iterator::IteratorAssignArithmeticOperator and
+        node2.(IndirectArgumentOutNode).getPreUpdateNode() = node1 and
+        node1.(IndirectOperand).getOperand() = call.getArgumentOperand(0) and
+        node1.getType().getUnspecifiedType() = this
+      )
+    }
+
+    override predicate isAdditionalConversionFlow(Operand opFrom, Instruction instrTo) {
+      // This is a bit annoying: Consider the following snippet:
+      // ```
+      // struct MyIterator {
+      //       ...
+      //       insert_iterator_by_trait operator*();
+      //       insert_iterator_by_trait operator=(int x);
+      //   };
+      // ...
+      // MyIterator it;
+      // ...
+      // *it = source();
+      // ```
+      // The qualifier to `operator*` will undergo prvalue-to-xvalue conversion and a
+      // temporary object will be created. Thus, the IR for the call to `operator=` will
+      // look like (simplified):
+      // ```
+      // r1(glval<MyIterator>) = VariableAddress[it]        :
+      // r2(glval<unknown>)    = FunctionAddress[operator*] :
+      // r3(MyIterator)        = Call[operator*]            : func:r2, this:r1
+      // r4(glval<MyIterator>) = VariableAddress[#temp]     :
+      // m1(MyIterator)        = Store[#temp]               : &:r4, r3
+      // r5(glval<unknown>)    = FunctionAddress[operator=] :
+      // r6(glval<unknown>)    = FunctionAddress[source]    :
+      // r7(int)               = Call[source]               : func:r6
+      // r8(MyIterator)        = Call[operator=]            : func:r5, this:r4, 0:r7
+      // ```
+      // in order to properly recognize that the qualifier to the call to `operator=` accesses
+      // `it` we look for the store that writes to the temporary object, and use the source value
+      // of that store as the "address" to continue searching for the base variable `it`.
+      exists(StoreInstruction store, VariableInstruction var |
+        var = instrTo and
+        var.getIRVariable() instanceof IRTempVariable and
+        opFrom.getType() = this and
+        store.getSourceValueOperand() = opFrom and
+        store.getDestinationAddress() = var
+      )
+      or
+      // A call to `operator++` or `operator--` is the iterator equivalent version of a
+      // pointer arithmetic instruction.
+      exists(CallInstruction call |
+        instrTo = call and
+        call.getStaticCallTarget() instanceof Iterator::IteratorCrementMemberOperator and
+        opFrom = call.getThisArgumentOperand()
+      )
+    }
+  }
+}
+
+predicate isDereference(Instruction deref, Operand address) {
+  any(Indirection ind).isAdditionalDereference(deref, address)
+  or
+  deref.(LoadInstruction).getSourceAddressOperand() = address
+}
+
+predicate isWrite(Node0Impl value, Operand address, boolean certain) {
+  any(Indirection ind).isAdditionalWrite(value, address, certain)
+  or
+  certain = true and
+  (
+    exists(StoreInstruction store |
+      value.asInstruction() = store and
+      address = store.getDestinationAddressOperand()
+    )
+    or
+    exists(InitializeParameterInstruction init |
+      value.asInstruction() = init and
+      address = init.getAnOperand()
+    )
+    or
+    exists(InitializeDynamicAllocationInstruction init |
+      value.asInstruction() = init and
+      address = init.getAllocationAddressOperand()
+    )
+    or
+    exists(UninitializedInstruction uninitialized |
+      value.asInstruction() = uninitialized and
+      address = uninitialized.getAnOperand()
+    )
+  )
+}
+
+predicate isAdditionalConversionFlow(Operand opFrom, Instruction instrTo) {
+  any(Indirection ind).isAdditionalConversionFlow(opFrom, instrTo)
+}
+
+newtype TBaseSourceVariable =
+  // Each IR variable gets its own source variable
+  TBaseIRVariable(IRVariable var) or
+  // Each allocation gets its own source variable
+  TBaseCallVariable(AllocationInstruction call)
+
+abstract class BaseSourceVariable extends TBaseSourceVariable {
+  abstract string toString();
+
+  abstract DataFlowType getType();
+}
+
+class BaseIRVariable extends BaseSourceVariable, TBaseIRVariable {
+  IRVariable var;
+
+  IRVariable getIRVariable() { result = var }
+
+  BaseIRVariable() { this = TBaseIRVariable(var) }
+
+  override string toString() { result = var.toString() }
+
+  override DataFlowType getType() { result = var.getType() }
+}
+
+class BaseCallVariable extends BaseSourceVariable, TBaseCallVariable {
+  AllocationInstruction call;
+
+  BaseCallVariable() { this = TBaseCallVariable(call) }
+
+  AllocationInstruction getCallInstruction() { result = call }
+
+  override string toString() { result = call.toString() }
+
+  override DataFlowType getType() { result = call.getResultType() }
 }
 
 /**
@@ -139,13 +357,189 @@ predicate isModifiableByCall(ArgumentOperand operand) {
     type = getLanguageType(operand) and
     call.getArgumentOperand(index) = operand and
     if index = -1
-    then not call.getStaticCallTarget() instanceof Cpp::ConstMemberFunction
-    else not SideEffects::isConstPointerLike(any(Type t | type.hasType(t, _)))
+    then
+      // A qualifier is "modifiable" if:
+      // 1. the member function is not const specified, or
+      // 2. the member function is `const` specified, but returns a pointer or reference
+      // type that is non-const.
+      //
+      // To see why this is necessary, consider the following function:
+      // ```
+      // struct C {
+      //   void* data_;
+      //   void* data() const { return data; }
+      // };
+      // ...
+      // C c;
+      // memcpy(c.data(), source, 16)
+      // ```
+      // the data pointed to by `c.data_` is potentially modified by the call to `memcpy` even though
+      // `C::data` has a const specifier. So we further place the restriction that the type returned
+      // by `call` should not be of the form `const T*` (for some deeply const type `T`).
+      if call.getStaticCallTarget() instanceof Cpp::ConstMemberFunction
+      then
+        exists(PointerOrReferenceType resultType |
+          resultType = call.getResultType() and
+          not resultType.isDeeplyConstBelow()
+        )
+      else any()
+    else
+      // An argument is modifiable if it's a non-const pointer or reference type.
+      exists(Type t, boolean isGLValue | type.hasType(t, isGLValue) |
+        // If t is a glvalue it means that t is always a pointer-like type.
+        isGLValue = true
+        or
+        t instanceof PointerOrReferenceType and
+        not SideEffects::isConstPointerLike(t)
+      )
   )
 }
 
+abstract class BaseSourceVariableInstruction extends Instruction {
+  abstract BaseSourceVariable getBaseSourceVariable();
+}
+
+private class BaseIRVariableInstruction extends BaseSourceVariableInstruction,
+  VariableAddressInstruction {
+  override BaseIRVariable getBaseSourceVariable() { result.getIRVariable() = this.getIRVariable() }
+}
+
+private class BaseAllocationInstruction extends BaseSourceVariableInstruction, AllocationInstruction {
+  override BaseCallVariable getBaseSourceVariable() { result.getCallInstruction() = this }
+}
+
 cached
 private module Cached {
+  private import semmle.code.cpp.models.interfaces.Iterator as Interfaces
+  private import semmle.code.cpp.models.implementations.Iterator as Iterator
+
+  /**
+   * Holds if `next` is a instruction with a memory result that potentially
+   * updates the memory produced by `prev`.
+   */
+  private predicate memorySucc(Instruction prev, Instruction next) {
+    prev = next.(ChiInstruction).getTotal()
+    or
+    // Phi inputs can be inexact.
+    prev = next.(PhiInstruction).getAnInputOperand().getAnyDef()
+    or
+    prev = next.(CopyInstruction).getSourceValue()
+    or
+    exists(ReadSideEffectInstruction read |
+      next = read.getPrimaryInstruction() and
+      isAdditionalConversionFlow(_, next) and
+      prev = read.getSideEffectOperand().getAnyDef()
+    )
+  }
+
+  /**
+   * Holds if `iteratorDerefAddress` is an address of an iterator dereference (i.e., `*it`)
+   * that is used for a write operation that writes the value `value`. The `memory` instruction
+   * represents the memory that the IR's SSA analysis determined was read by the call to `operator*`.
+   *
+   * The `numberOfLoads` integer represents the number of dereferences this write corresponds to
+   * on the underlying container that produced the iterator.
+   */
+  private predicate isChiAfterIteratorDef(
+    Instruction memory, Operand iteratorDerefAddress, Node0Impl value, int numberOfLoads
+  ) {
+    exists(
+      BaseSourceVariableInstruction iteratorBase, ReadSideEffectInstruction read,
+      Operand iteratorAddress
+    |
+      numberOfLoads >= 0 and
+      isDef(_, value, iteratorDerefAddress, iteratorBase, numberOfLoads + 2, 0) and
+      isUse(_, iteratorAddress, iteratorBase, numberOfLoads + 1, 0) and
+      iteratorBase.getResultType() instanceof Interfaces::Iterator and
+      isDereference(iteratorAddress.getDef(), read.getArgumentDef().getAUse()) and
+      memory = read.getSideEffectOperand().getAnyDef()
+    )
+  }
+
+  /**
+   * Holds if `iterator` is a `StoreInstruction` that stores the result of some function
+   * returning an iterator into an address computed started at `containerBase`.
+   *
+   * For example, given a declaration like `std::vector<int>::iterator it = v.begin()`,
+   * the `iterator` will be the `StoreInstruction` generated by the write to `it`, and
+   * `containerBase` will be the address of `v`.
+   */
+  private predicate isChiAfterBegin(
+    BaseSourceVariableInstruction containerBase, StoreInstruction iterator
+  ) {
+    exists(CallInstruction getIterator |
+      getIterator = iterator.getSourceValue() and
+      getIterator.getStaticCallTarget() instanceof Iterator::GetIteratorFunction and
+      isDef(_, any(Node0Impl n | n.asInstruction() = iterator), _, _, 1, 0) and
+      isUse(_, getIterator.getThisArgumentOperand(), containerBase, 0, 0)
+    )
+  }
+
+  /**
+   * Holds if `iteratorDerefAddress` is an address of an iterator dereference (i.e., `*it`)
+   * that is used for a read operation. The `memory` instruction represents the memory that
+   * the IR's SSA analysis determined was read by the call to `operator*`.
+   *
+   * Finally, the `numberOfLoads` integer represents the number of dereferences this read
+   * corresponds to on the underlying container that produced the iterator.
+   */
+  private predicate isChiBeforeIteratorUse(
+    Operand iteratorDerefAddress, Instruction memory, int numberOfLoads
+  ) {
+    exists(
+      BaseSourceVariableInstruction iteratorBase, LoadInstruction load,
+      ReadSideEffectInstruction read
+    |
+      numberOfLoads >= 0 and
+      isUse(_, iteratorDerefAddress, iteratorBase, numberOfLoads + 2, 0) and
+      iteratorBase.getResultType() instanceof Interfaces::Iterator and
+      read.getPrimaryInstruction() = load.getSourceAddress() and
+      memory = read.getSideEffectOperand().getAnyDef()
+    )
+  }
+
+  /**
+   * Holds if `iteratorDerefAddress` is an address of an iterator dereference (i.e., `*it`)
+   * that is used for a write operation that writes the value `value` to a container that
+   * created the iterator. `container` represents the base of the address of the container
+   * that was used to create the iterator.
+   */
+  cached
+  predicate isIteratorDef(
+    BaseSourceVariableInstruction container, Operand iteratorDerefAddress, Node0Impl value,
+    int numberOfLoads, int indirectionIndex
+  ) {
+    exists(Instruction memory, Instruction begin, int upper, int ind |
+      isChiAfterIteratorDef(memory, iteratorDerefAddress, value, numberOfLoads) and
+      memorySucc*(begin, memory) and
+      isChiAfterBegin(container, begin) and
+      upper = countIndirectionsForCppType(container.getResultLanguageType()) and
+      ind = numberOfLoads + [1 .. upper] and
+      indirectionIndex = ind - (numberOfLoads + 1)
+    )
+  }
+
+  /**
+   * Holds if `iteratorDerefAddress` is an address of an iterator dereference (i.e., `*it`)
+   * that is used for a read operation to read a value from a container that created the iterator.
+   * `container` represents the base of the address of the container that was used to create
+   * the iterator.
+   */
+  cached
+  predicate isIteratorUse(
+    BaseSourceVariableInstruction container, Operand iteratorDerefAddress, int numberOfLoads,
+    int indirectionIndex
+  ) {
+    exists(Instruction begin, Instruction memory, int upper, int ind |
+      isChiBeforeIteratorUse(iteratorDerefAddress, memory, numberOfLoads) and
+      memorySucc*(begin, memory) and
+      isChiAfterBegin(container, begin) and
+      upper = countIndirectionsForCppType(container.getResultLanguageType()) and
+      ind = numberOfLoads + [1 .. upper] and
+      indirectionIndex = ind - (numberOfLoads + 1)
+    )
+  }
+
   /**
    * Holds if `op` is a use of an SSA variable rooted at `base` with `ind` number
    * of indirections.
@@ -154,27 +548,61 @@ private module Cached {
    * `indirectionIndex` specifies the number of loads required to read the variable.
    */
   cached
-  predicate isUse(boolean certain, Operand op, Instruction base, int ind, int indirectionIndex) {
+  predicate isUse(
+    boolean certain, Operand op, BaseSourceVariableInstruction base, int ind, int indirectionIndex
+  ) {
     not ignoreOperand(op) and
     certain = true and
-    exists(LanguageType type, int m, int ind0 |
+    exists(LanguageType type, int upper, int ind0 |
       type = getLanguageType(op) and
-      m = countIndirectionsForCppType(type) and
+      upper = countIndirectionsForCppType(type) and
       isUseImpl(op, base, ind0) and
-      ind = ind0 + [0 .. m] and
+      ind = ind0 + [0 .. upper] and
       indirectionIndex = ind - ind0
     )
   }
 
+  /**
+   * Holds if the underlying IR has a suitable operand to represent a value
+   * that would otherwise need to be represented by a dedicated `RawIndirectOperand` value.
+   *
+   * Such operands do not create new `RawIndirectOperand` values, but are
+   * instead associated with the operand returned by this predicate.
+   */
+  cached
+  Operand getIRRepresentationOfIndirectOperand(Operand operand, int indirectionIndex) {
+    exists(Instruction load |
+      isDereference(load, operand) and
+      result = unique( | | load.getAUse()) and
+      isUseImpl(operand, _, indirectionIndex - 1)
+    )
+  }
+
+  /**
+   * Holds if the underlying IR has a suitable instruction to represent a value
+   * that would otherwise need to be represented by a dedicated `RawIndirectInstruction` value.
+   *
+   * Such instruction do not create new `RawIndirectOperand` values, but are
+   * instead associated with the instruction returned by this predicate.
+   */
+  cached
+  Instruction getIRRepresentationOfIndirectInstruction(Instruction instr, int indirectionIndex) {
+    exists(Instruction load, Operand address |
+      address.getDef() = instr and
+      isDereference(load, address) and
+      isUseImpl(address, _, indirectionIndex - 1) and
+      result = instr
+    )
+  }
+
   /**
    * Holds if `operand` is a use of an SSA variable rooted at `base`, and the
    * path from `base` to `operand` passes through `ind` load-like instructions.
    */
-  private predicate isUseImpl(Operand operand, Instruction base, int ind) {
+  private predicate isUseImpl(Operand operand, BaseSourceVariableInstruction base, int ind) {
     DataFlowImplCommon::forceCachingInSameStage() and
     ind = 0 and
-    operand.getDef() = base and
-    isSourceVariableBase(base)
+    operand = base.getAUse()
     or
     exists(Operand mid, Instruction instr |
       isUseImpl(mid, base, ind) and
@@ -183,7 +611,10 @@ private module Cached {
     )
     or
     exists(int ind0 |
-      isUseImpl(operand.getDef().(LoadInstruction).getSourceAddressOperand(), base, ind0)
+      exists(Operand address |
+        isDereference(operand.getDef(), address) and
+        isUseImpl(address, base, ind0)
+      )
       or
       isUseImpl(operand.getDef().(InitializeParameterInstruction).getAnOperand(), base, ind0)
     |
@@ -201,27 +632,33 @@ private module Cached {
    */
   cached
   predicate isDef(
-    boolean certain, Instruction instr, Operand address, Instruction base, int ind,
+    boolean certain, Node0Impl value, Operand address, BaseSourceVariableInstruction base, int ind,
     int indirectionIndex
   ) {
-    certain = true and
-    exists(int ind0, CppType type, int m |
-      address =
-        [
-          instr.(StoreInstruction).getDestinationAddressOperand(),
-          instr.(InitializeParameterInstruction).getAnOperand(),
-          instr.(InitializeDynamicAllocationInstruction).getAllocationAddressOperand(),
-          instr.(UninitializedInstruction).getAnOperand()
-        ]
+    exists(
+      boolean writeIsCertain, boolean addressIsCertain, int ind0, CppType type, int lower, int upper
     |
-      isDefImpl(address, base, ind0) and
+      isWrite(value, address, writeIsCertain) and
+      isDefImpl(address, base, ind0, addressIsCertain) and
+      certain = writeIsCertain.booleanAnd(addressIsCertain) and
       type = getLanguageType(address) and
-      m = countIndirectionsForCppType(type) and
-      ind = ind0 + [1 .. m] and
-      indirectionIndex = ind - (ind0 + 1)
+      upper = countIndirectionsForCppType(type) and
+      ind = ind0 + [lower .. upper] and
+      indirectionIndex = ind - (ind0 + lower) and
+      (if type.hasType(any(Cpp::ArrayType arrayType), true) then lower = 0 else lower = 1)
     )
   }
 
+  /**
+   * Holds if the address computed by `operand` is guarenteed to write
+   * to a specific address.
+   */
+  private predicate isCertainAddress(Operand operand) {
+    operand.getDef() instanceof VariableAddressInstruction
+    or
+    operand.getType() instanceof Cpp::ReferenceType
+  }
+
   /**
    * Holds if `address` is a use of an SSA variable rooted at `base`, and the
    * path from `base` to `address` passes through `ind` load-like instructions.
@@ -229,25 +666,30 @@ private module Cached {
    * Note: Unlike `isUseImpl`, this predicate recurses through pointer-arithmetic
    * instructions.
    */
-  private predicate isDefImpl(Operand address, Instruction base, int ind) {
+  private predicate isDefImpl(
+    Operand operand, BaseSourceVariableInstruction base, int ind, boolean certain
+  ) {
     DataFlowImplCommon::forceCachingInSameStage() and
     ind = 0 and
-    address.getDef() = base and
-    isSourceVariableBase(base)
+    operand = base.getAUse() and
+    (if isCertainAddress(operand) then certain = true else certain = false)
     or
-    exists(Operand mid, Instruction instr |
-      isDefImpl(mid, base, ind) and
-      instr = address.getDef() and
-      conversionFlow(mid, instr, _)
+    exists(Operand mid, Instruction instr, boolean certain0, boolean isPointerArith |
+      isDefImpl(mid, base, ind, certain0) and
+      instr = operand.getDef() and
+      conversionFlow(mid, instr, isPointerArith) and
+      if isPointerArith = true then certain = false else certain = certain0
     )
     or
-    exists(int ind0 |
-      isDefImpl(address.getDef().(LoadInstruction).getSourceAddressOperand(), base, ind0)
-      or
-      isDefImpl(address.getDef().(InitializeParameterInstruction).getAnOperand(), base, ind0)
+    exists(Operand address, boolean certain0 |
+      isDereference(operand.getDef(), address) and
+      isDefImpl(address, base, ind - 1, certain0)
     |
-      ind0 = ind - 1
+      if isCertainAddress(operand) then certain = certain0 else certain = false
     )
+    or
+    isDefImpl(operand.getDef().(InitializeParameterInstruction).getAnOperand(), base, ind - 1, _) and
+    certain = true
   }
 }
 

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

@@ -5,6 +5,7 @@ private import semmle.code.cpp.models.interfaces.DataFlow
 private import semmle.code.cpp.models.interfaces.SideEffect
 private import DataFlowUtil
 private import DataFlowPrivate
+private import SsaInternals as Ssa
 
 /**
  * Holds if taint propagates from `nodeFrom` to `nodeTo` in exactly one local
@@ -46,6 +47,8 @@ predicate localAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeT
     nodeHasOperand(nodeFrom, pai.getAnOperand(), pragma[only_bind_into](indirectionIndex)) and
     hasInstructionAndIndex(nodeTo, pai, indirectionIndex + 1)
   )
+  or
+  any(Ssa::Indirection ind).isAdditionalTaintStep(nodeFrom, nodeTo)
 }
 
 /**
@@ -65,10 +68,8 @@ private predicate operandToInstructionTaintStep(Operand opFrom, Instruction inst
     instrTo instanceof PointerArithmeticInstruction
   )
   or
-  // The `CopyInstruction` case is also present in non-taint data flow, but
-  // that uses `getDef` rather than `getAnyDef`. For taint, we want flow
-  // from a definition of `myStruct` to a `myStruct.myField` expression.
-  instrTo.(LoadInstruction).getSourceAddressOperand() = opFrom
+  // Taint flow from an address to its dereference.
+  Ssa::isDereference(instrTo, opFrom)
   or
   // Unary instructions tend to preserve enough information in practice that we
   // want taint to flow through.

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

@@ -15,75 +15,15 @@ private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
 private import semmle.code.cpp.ir.dataflow.internal.SsaInternalsCommon
 
 private module SourceVariables {
-  newtype TBaseSourceVariable =
-    // Each IR variable gets its own source variable
-    TBaseIRVariable(IRVariable var) or
-    // Each allocation gets its own source variable
-    TBaseCallVariable(AllocationInstruction call)
+  class SourceVariable instanceof BaseSourceVariable {
+    string toString() { result = BaseSourceVariable.super.toString() }
 
-  abstract class BaseSourceVariable extends TBaseSourceVariable {
-    abstract string toString();
-
-    abstract DataFlowType getType();
+    BaseSourceVariable getBaseVariable() { result = this }
   }
 
-  class BaseIRVariable extends BaseSourceVariable, TBaseIRVariable {
-    IRVariable var;
+  class SourceIRVariable = BaseIRVariable;
 
-    IRVariable getIRVariable() { result = var }
-
-    BaseIRVariable() { this = TBaseIRVariable(var) }
-
-    override string toString() { result = var.toString() }
-
-    override DataFlowType getType() { result = var.getType() }
-  }
-
-  class BaseCallVariable extends BaseSourceVariable, TBaseCallVariable {
-    AllocationInstruction call;
-
-    BaseCallVariable() { this = TBaseCallVariable(call) }
-
-    AllocationInstruction getCallInstruction() { result = call }
-
-    override string toString() { result = call.toString() }
-
-    override DataFlowType getType() { result = call.getResultType() }
-  }
-
-  private newtype TSourceVariable =
-    TSourceIRVariable(BaseIRVariable baseVar) or
-    TCallVariable(AllocationInstruction call)
-
-  abstract class SourceVariable extends TSourceVariable {
-    abstract string toString();
-
-    abstract BaseSourceVariable getBaseVariable();
-  }
-
-  class SourceIRVariable extends SourceVariable, TSourceIRVariable {
-    BaseIRVariable var;
-
-    SourceIRVariable() { this = TSourceIRVariable(var) }
-
-    IRVariable getIRVariable() { result = var.getIRVariable() }
-
-    override BaseIRVariable getBaseVariable() { result.getIRVariable() = this.getIRVariable() }
-
-    override string toString() { result = this.getIRVariable().toString() }
-  }
-
-  class CallVariable extends SourceVariable, TCallVariable {
-    AllocationInstruction call;
-
-    CallVariable() { this = TCallVariable(call) }
-
-    AllocationInstruction getCall() { result = call }
-
-    override BaseCallVariable getBaseVariable() { result.getCallInstruction() = call }
-
-    override string toString() { result = "Call" }
-  }
+  class CallVariable = BaseCallVariable;
 }
 
 import SourceVariables
@@ -92,7 +32,13 @@ private newtype TDefOrUseImpl =
   TDefImpl(Operand address) { isDef(_, _, address, _, _, _) } or
   TUseImpl(Operand operand) {
     isUse(_, operand, _, _, _) and
-    not isDef(_, _, operand, _, _, _)
+    not isDef(true, _, operand, _, _, _)
+  } or
+  TIteratorDef(BaseSourceVariableInstruction container, Operand iteratorAddress) {
+    isIteratorDef(container, iteratorAddress, _, _, _)
+  } or
+  TIteratorUse(BaseSourceVariableInstruction container, Operand iteratorAddress) {
+    isIteratorUse(container, iteratorAddress, _, _)
   }
 
 abstract private class DefOrUseImpl extends TDefOrUseImpl {
@@ -113,80 +59,62 @@ abstract private class DefOrUseImpl extends TDefOrUseImpl {
   /** Gets the location of this element. */
   abstract Cpp::Location getLocation();
 
-  abstract Instruction getBase();
+  abstract BaseSourceVariableInstruction getBase();
 
   final BaseSourceVariable getBaseSourceVariable() {
-    exists(IRVariable var |
-      result.(BaseIRVariable).getIRVariable() = var and
-      instructionHasIRVariable(this.getBase(), var)
-    )
-    or
-    result.(BaseCallVariable).getCallInstruction() = this.getBase()
+    result = this.getBase().getBaseSourceVariable()
   }
 
   /** Gets the variable that is defined or used. */
   final SourceVariable getSourceVariable() {
-    exists(BaseSourceVariable v |
-      sourceVariableHasBaseAndIndex(result, v) and
-      defOrUseHasSourceVariable(this, v)
-    )
+    result.getBaseVariable() = this.getBaseSourceVariable()
   }
+
+  abstract predicate isCertain();
 }
 
-pragma[noinline]
-private predicate instructionHasIRVariable(VariableAddressInstruction vai, IRVariable var) {
-  vai.getIRVariable() = var
-}
-
-private predicate defOrUseHasSourceVariable(DefOrUseImpl defOrUse, BaseSourceVariable bv) {
-  defHasSourceVariable(defOrUse, bv)
-  or
-  useHasSourceVariable(defOrUse, bv)
-}
-
-pragma[noinline]
-private predicate defHasSourceVariable(DefImpl def, BaseSourceVariable bv) {
-  bv = def.getBaseSourceVariable()
-}
-
-pragma[noinline]
-private predicate useHasSourceVariable(UseImpl use, BaseSourceVariable bv) {
-  bv = use.getBaseSourceVariable()
-}
-
-pragma[noinline]
-private predicate sourceVariableHasBaseAndIndex(SourceVariable v, BaseSourceVariable bv) {
-  v.getBaseVariable() = bv
-}
-
-class DefImpl extends DefOrUseImpl, TDefImpl {
+abstract class DefImpl extends DefOrUseImpl {
   Operand address;
 
-  DefImpl() { this = TDefImpl(address) }
-
-  override Instruction getBase() { isDef(_, _, address, result, _, _) }
-
   Operand getAddressOperand() { result = address }
 
-  Instruction getDefiningInstruction() { isDef(_, result, address, _, _, _) }
+  abstract Node0Impl getValue();
 
   override string toString() { result = address.toString() }
 
-  override IRBlock getBlock() { result = this.getDefiningInstruction().getBlock() }
+  override IRBlock getBlock() { result = this.getAddressOperand().getDef().getBlock() }
 
-  override Cpp::Location getLocation() { result = this.getDefiningInstruction().getLocation() }
+  override Cpp::Location getLocation() { result = this.getAddressOperand().getLocation() }
 
   final override predicate hasIndexInBlock(IRBlock block, int index) {
-    this.getDefiningInstruction() = block.getInstruction(index)
+    this.getAddressOperand().getUse() = block.getInstruction(index)
   }
-
-  predicate isCertain() { isDef(true, _, address, _, _, _) }
 }
 
-class UseImpl extends DefOrUseImpl, TUseImpl {
-  Operand operand;
+private class DirectDef extends DefImpl, TDefImpl {
+  DirectDef() { this = TDefImpl(address) }
 
-  UseImpl() { this = TUseImpl(operand) }
+  override BaseSourceVariableInstruction getBase() { isDef(_, _, address, result, _, _) }
+
+  override Node0Impl getValue() { isDef(_, result, address, _, _, _) }
+
+  override predicate isCertain() { isDef(true, _, address, _, _, _) }
+}
+
+private class IteratorDef extends DefImpl, TIteratorDef {
+  BaseSourceVariableInstruction container;
+
+  IteratorDef() { this = TIteratorDef(container, address) }
+
+  override BaseSourceVariableInstruction getBase() { result = container }
+
+  override Node0Impl getValue() { isIteratorDef(_, address, result, _, _) }
+
+  override predicate isCertain() { none() }
+}
+
+abstract class UseImpl extends DefOrUseImpl {
+  Operand operand;
 
   Operand getOperand() { result = operand }
 
@@ -199,10 +127,24 @@ class UseImpl extends DefOrUseImpl, TUseImpl {
   final override IRBlock getBlock() { result = operand.getUse().getBlock() }
 
   final override Cpp::Location getLocation() { result = operand.getLocation() }
+}
 
-  override Instruction getBase() { isUse(_, operand, result, _, _) }
+private class DirectUse extends UseImpl, TUseImpl {
+  DirectUse() { this = TUseImpl(operand) }
 
-  predicate isCertain() { isUse(true, operand, _, _, _) }
+  override BaseSourceVariableInstruction getBase() { isUse(_, operand, result, _, _) }
+
+  override predicate isCertain() { isUse(true, operand, _, _, _) }
+}
+
+private class IteratorUse extends UseImpl, TIteratorUse {
+  BaseSourceVariableInstruction container;
+
+  IteratorUse() { this = TIteratorUse(container, operand) }
+
+  override BaseSourceVariableInstruction getBase() { result = container }
+
+  override predicate isCertain() { none() }
 }
 
 private module SsaInput implements SsaImplCommon::InputSig {
@@ -302,9 +244,13 @@ class Def extends DefOrUse {
 
   Instruction getAddress() { result = this.getAddressOperand().getDef() }
 
-  Instruction getDefiningInstruction() { result = defOrUse.getDefiningInstruction() }
+  Node0Impl getValue() { result = defOrUse.getValue() }
 
-  override string toString() { result = this.asDefOrUse().toString() + " (def)" }
+  override string toString() { result = this.asDefOrUse().toString() }
+
+  BaseSourceVariableInstruction getBase() { result = defOrUse.getBase() }
+
+  predicate isIteratorDef() { defOrUse instanceof IteratorDef }
 }
 
 private module SsaImpl = SsaImplCommon::Make<SsaInput>;

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/Operand.qll

@@ -45,7 +45,7 @@ class Operand extends TStageOperand {
       this = reusedPhiOperand(use, def, predecessorBlock, _)
     )
     or
-    exists(Instruction use | this = chiOperand(use, _))
+    this = chiOperand(_, _)
   }
 
   /** Gets a textual representation of this element. */

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll

@@ -329,12 +329,12 @@ private module Cached {
   cached
   Instruction getChiInstructionTotalOperand(ChiInstruction chiInstr) {
     exists(
-      Alias::VirtualVariable vvar, OldInstruction oldInstr, Alias::MemoryLocation defLocation,
-      OldBlock defBlock, int defRank, int defOffset, OldBlock useBlock, int useRank
+      Alias::VirtualVariable vvar, OldInstruction oldInstr, OldBlock defBlock, int defRank,
+      int defOffset, OldBlock useBlock, int useRank
     |
       chiInstr = getChi(oldInstr) and
       vvar = Alias::getResultMemoryLocation(oldInstr).getVirtualVariable() and
-      hasDefinitionAtRank(vvar, defLocation, defBlock, defRank, defOffset) and
+      hasDefinitionAtRank(vvar, _, defBlock, defRank, defOffset) and
       hasUseAtRank(vvar, useBlock, useRank, oldInstr) and
       definitionReachesUse(vvar, defBlock, defRank, useBlock, useRank) and
       result = getDefinitionOrChiInstruction(defBlock, defOffset, vvar, _)

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/Operand.qll

@@ -45,7 +45,7 @@ class Operand extends TStageOperand {
       this = reusedPhiOperand(use, def, predecessorBlock, _)
     )
     or
-    exists(Instruction use | this = chiOperand(use, _))
+    this = chiOperand(_, _)
   }
 
   /** Gets a textual representation of this element. */

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/InstructionTag.qll

@@ -72,7 +72,19 @@ newtype TInstructionTag =
   AsmInputTag(int elementIndex) { exists(AsmStmt asm | exists(asm.getChild(elementIndex))) } or
   ThisAddressTag() or
   ThisLoadTag() or
-  StructuredBindingAccessTag()
+  StructuredBindingAccessTag() or
+  // The next three cases handle generation of the constants -1, 0 and 1 for __except handling.
+  TryExceptGenerateNegativeOne() or
+  TryExceptGenerateZero() or
+  TryExceptGenerateOne() or
+  // The next three cases handle generation of comparisons for __except handling.
+  TryExceptCompareNegativeOne() or
+  TryExceptCompareZero() or
+  TryExceptCompareOne() or
+  // The next three cases handle generation of branching for __except handling.
+  TryExceptCompareNegativeOneBranch() or
+  TryExceptCompareZeroBranch() or
+  TryExceptCompareOneBranch()
 
 class InstructionTag extends TInstructionTag {
   final string toString() { result = "Tag" }
@@ -224,4 +236,22 @@ string getInstructionTagId(TInstructionTag tag) {
   tag = ThisLoadTag() and result = "ThisLoad"
   or
   tag = StructuredBindingAccessTag() and result = "StructuredBindingAccess"
+  or
+  tag = TryExceptCompareNegativeOne() and result = "TryExceptCompareNegativeOne"
+  or
+  tag = TryExceptCompareZero() and result = "TryExceptCompareZero"
+  or
+  tag = TryExceptCompareOne() and result = "TryExceptCompareOne"
+  or
+  tag = TryExceptGenerateNegativeOne() and result = "TryExceptGenerateNegativeOne"
+  or
+  tag = TryExceptGenerateZero() and result = "TryExceptGenerateNegativeOne"
+  or
+  tag = TryExceptGenerateOne() and result = "TryExceptGenerateOne"
+  or
+  tag = TryExceptCompareNegativeOneBranch() and result = "TryExceptCompareNegativeOneBranch"
+  or
+  tag = TryExceptCompareZeroBranch() and result = "TryExceptCompareZeroBranch"
+  or
+  tag = TryExceptCompareOneBranch() and result = "TryExceptCompareOneBranch"
 }

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll

@@ -675,6 +675,7 @@ newtype TTranslatedElement =
   } or
   // A statement
   TTranslatedStmt(Stmt stmt) { translateStmt(stmt) } or
+  TTranslatedMicrosoftTryExceptHandler(MicrosoftTryExceptStmt stmt) or
   // A function
   TTranslatedFunction(Function func) { translateFunction(func) } or
   // A constructor init list

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedInitialization.qll

@@ -298,11 +298,11 @@ class TranslatedStringLiteralInitialization extends TranslatedDirectInitializati
     opcode instanceof Opcode::Store and
     resultType = getTypeForPRValue(expr.getType())
     or
-    exists(int startIndex, int elementCount |
+    exists(int elementCount |
       // If the initializer string isn't large enough to fill the target, then
       // we have to generate another instruction sequence to store a constant
       // zero into the remainder of the array.
-      zeroInitRange(startIndex, elementCount) and
+      zeroInitRange(_, elementCount) and
       (
         // Create a constant zero whose size is the size of the remaining
         // space in the target array.

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll

@@ -13,6 +13,222 @@ private import TranslatedInitialization
 
 TranslatedStmt getTranslatedStmt(Stmt stmt) { result.getAst() = stmt }
 
+TranslatedMicrosoftTryExceptHandler getTranslatedMicrosoftTryExceptHandler(
+  MicrosoftTryExceptStmt tryExcept
+) {
+  result.getAst() = tryExcept.getExcept()
+}
+
+class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,
+  TTranslatedMicrosoftTryExceptHandler {
+  MicrosoftTryExceptStmt tryExcept;
+
+  TranslatedMicrosoftTryExceptHandler() { this = TTranslatedMicrosoftTryExceptHandler(tryExcept) }
+
+  final override string toString() { result = tryExcept.toString() }
+
+  final override Locatable getAst() { result = tryExcept.getExcept() }
+
+  override Instruction getFirstInstruction() { result = this.getChild(0).getFirstInstruction() }
+
+  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
+    // t1 = -1
+    tag = TryExceptGenerateNegativeOne() and
+    opcode instanceof Opcode::Constant and
+    resultType = getIntType()
+    or
+    // t2 = cmp t1, condition
+    tag = TryExceptCompareNegativeOne() and
+    opcode instanceof Opcode::CompareEQ and
+    resultType = getBoolType()
+    or
+    // if t2 goto ... else goto ...
+    tag = TryExceptCompareNegativeOneBranch() and
+    opcode instanceof Opcode::ConditionalBranch and
+    resultType = getVoidType()
+    or
+    // t1 = 0
+    tag = TryExceptGenerateZero() and
+    opcode instanceof Opcode::Constant and
+    resultType = getIntType()
+    or
+    // t2 = cmp t1, condition
+    tag = TryExceptCompareZero() and
+    opcode instanceof Opcode::CompareEQ and
+    resultType = getBoolType()
+    or
+    // if t2 goto ... else goto ...
+    tag = TryExceptCompareZeroBranch() and
+    opcode instanceof Opcode::ConditionalBranch and
+    resultType = getVoidType()
+    or
+    // t1 = 1
+    tag = TryExceptGenerateOne() and
+    opcode instanceof Opcode::Constant and
+    resultType = getIntType()
+    or
+    // t2 = cmp t1, condition
+    tag = TryExceptCompareOne() and
+    opcode instanceof Opcode::CompareEQ and
+    resultType = getBoolType()
+    or
+    // if t2 goto ... else goto ...
+    tag = TryExceptCompareOneBranch() and
+    opcode instanceof Opcode::ConditionalBranch and
+    resultType = getVoidType()
+    or
+    // unwind stack
+    tag = UnwindTag() and
+    opcode instanceof Opcode::Unwind and
+    resultType = getVoidType()
+  }
+
+  final override Instruction getInstructionRegisterOperand(InstructionTag tag, OperandTag operandTag) {
+    tag = TryExceptCompareNegativeOne() and
+    (
+      operandTag instanceof LeftOperandTag and
+      result = this.getTranslatedCondition().getResult()
+      or
+      operandTag instanceof RightOperandTag and
+      result = this.getInstruction(TryExceptGenerateNegativeOne())
+    )
+    or
+    tag = TryExceptCompareNegativeOneBranch() and
+    operandTag instanceof ConditionOperandTag and
+    result = this.getInstruction(TryExceptCompareNegativeOne())
+    or
+    tag = TryExceptCompareZero() and
+    (
+      operandTag instanceof LeftOperandTag and
+      result = this.getTranslatedCondition().getResult()
+      or
+      operandTag instanceof RightOperandTag and
+      result = this.getInstruction(TryExceptGenerateZero())
+    )
+    or
+    tag = TryExceptCompareZeroBranch() and
+    operandTag instanceof ConditionOperandTag and
+    result = this.getInstruction(TryExceptCompareZero())
+    or
+    tag = TryExceptCompareOne() and
+    (
+      operandTag instanceof LeftOperandTag and
+      result = this.getTranslatedCondition().getResult()
+      or
+      operandTag instanceof RightOperandTag and
+      result = this.getInstruction(TryExceptGenerateOne())
+    )
+    or
+    tag = TryExceptCompareOneBranch() and
+    operandTag instanceof ConditionOperandTag and
+    result = this.getInstruction(TryExceptCompareOne())
+  }
+
+  override string getInstructionConstantValue(InstructionTag tag) {
+    tag = TryExceptGenerateNegativeOne() and
+    result = "-1"
+    or
+    tag = TryExceptGenerateZero() and
+    result = "0"
+    or
+    tag = TryExceptGenerateOne() and
+    result = "1"
+  }
+
+  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
+    // Generate -1 -> Compare condition
+    tag = TryExceptGenerateNegativeOne() and
+    kind instanceof GotoEdge and
+    result = this.getInstruction(TryExceptCompareNegativeOne())
+    or
+    // Compare condition -> Branch
+    tag = TryExceptCompareNegativeOne() and
+    kind instanceof GotoEdge and
+    result = this.getInstruction(TryExceptCompareNegativeOneBranch())
+    or
+    // Branch -> Unwind or Generate 0
+    tag = TryExceptCompareNegativeOneBranch() and
+    (
+      kind instanceof TrueEdge and
+      // TODO: This is not really correct. The semantics of `EXCEPTION_CONTINUE_EXECUTION` is that
+      // we should continue execution at the point where the exception occurred. But we don't have
+      // any instruction to model this behavior.
+      result = this.getInstruction(UnwindTag())
+      or
+      kind instanceof FalseEdge and
+      result = this.getInstruction(TryExceptGenerateZero())
+    )
+    or
+    // Generate 0 -> Compare condition
+    tag = TryExceptGenerateZero() and
+    kind instanceof GotoEdge and
+    result = this.getInstruction(TryExceptCompareZero())
+    or
+    // Compare condition -> Branch
+    tag = TryExceptCompareZero() and
+    kind instanceof GotoEdge and
+    result = this.getInstruction(TryExceptCompareZeroBranch())
+    or
+    // Branch -> Unwind or Generate 1
+    tag = TryExceptCompareZeroBranch() and
+    (
+      kind instanceof TrueEdge and
+      result = this.getInstruction(UnwindTag())
+      or
+      kind instanceof FalseEdge and
+      result = this.getInstruction(TryExceptGenerateOne())
+    )
+    or
+    // Generate 1 -> Compare condition
+    tag = TryExceptGenerateOne() and
+    kind instanceof GotoEdge and
+    result = this.getInstruction(TryExceptCompareOne())
+    or
+    // Compare condition -> Branch
+    tag = TryExceptCompareOne() and
+    kind instanceof GotoEdge and
+    result = this.getInstruction(TryExceptCompareOneBranch())
+    or
+    // Branch -> Handler (the condition value is always 0, -1 or 1, and we've checked for 0 or -1 already.)
+    tag = TryExceptCompareOneBranch() and
+    (
+      kind instanceof TrueEdge and
+      result = this.getTranslatedHandler().getFirstInstruction()
+    )
+    or
+    // Unwind -> Parent
+    tag = UnwindTag() and
+    kind instanceof GotoEdge and
+    result = this.getParent().getChildSuccessor(this)
+  }
+
+  override Instruction getChildSuccessor(TranslatedElement child) {
+    child = this.getTranslatedCondition() and
+    result = this.getInstruction(TryExceptGenerateNegativeOne())
+    or
+    child = this.getTranslatedHandler() and
+    result = this.getParent().getChildSuccessor(this)
+  }
+
+  private TranslatedExpr getTranslatedCondition() {
+    result = getTranslatedExpr(tryExcept.getCondition())
+  }
+
+  private TranslatedStmt getTranslatedHandler() {
+    result = getTranslatedStmt(tryExcept.getExcept())
+  }
+
+  override TranslatedElement getChild(int id) {
+    id = 0 and
+    result = this.getTranslatedCondition()
+    or
+    id = 1 and
+    result = this.getTranslatedHandler()
+  }
+
+  final override Function getFunction() { result = tryExcept.getEnclosingFunction() }
+}
+
 abstract class TranslatedStmt extends TranslatedElement, TTranslatedStmt {
   Stmt stmt;
 
@@ -249,15 +465,57 @@ class TranslatedUnreachableReturnStmt extends TranslatedReturnStmt {
 }
 
 /**
- * The IR translation of a C++ `try` statement.
+ * A C/C++ `try` statement, or a `__try __except` or `__try __finally` statement.
+ */
+private class TryOrMicrosoftTryStmt extends Stmt {
+  TryOrMicrosoftTryStmt() {
+    this instanceof TryStmt or
+    this instanceof MicrosoftTryStmt
+  }
+
+  /** Gets the number of `catch block`s of this statement. */
+  int getNumberOfCatchClauses() {
+    result = this.(TryStmt).getNumberOfCatchClauses()
+    or
+    this instanceof MicrosoftTryExceptStmt and
+    result = 1
+    or
+    this instanceof MicrosoftTryFinallyStmt and
+    result = 0
+  }
+
+  /** Gets the `body` statement of this statement. */
+  Stmt getStmt() {
+    result = this.(TryStmt).getStmt()
+    or
+    result = this.(MicrosoftTryStmt).getStmt()
+  }
+
+  /** Gets the `i`th translated handler of this statement. */
+  TranslatedElement getTranslatedHandler(int index) {
+    result = getTranslatedStmt(this.(TryStmt).getChild(index + 1))
+    or
+    index = 0 and
+    result = getTranslatedMicrosoftTryExceptHandler(this)
+  }
+
+  /** Gets the `finally` statement (usually a BlockStmt), if any. */
+  Stmt getFinally() { result = this.(MicrosoftTryFinallyStmt).getFinally() }
+}
+
+/**
+ * The IR translation of a C++ `try` (or a `__try __except` or `__try __finally`) statement.
  */
 class TranslatedTryStmt extends TranslatedStmt {
-  override TryStmt stmt;
+  override TryOrMicrosoftTryStmt stmt;
 
   override TranslatedElement getChild(int id) {
     id = 0 and result = getBody()
     or
     result = getHandler(id - 1)
+    or
+    id = stmt.getNumberOfCatchClauses() + 1 and
+    result = this.getFinally()
   }
 
   override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
@@ -269,8 +527,20 @@ class TranslatedTryStmt extends TranslatedStmt {
   override Instruction getFirstInstruction() { result = getBody().getFirstInstruction() }
 
   override Instruction getChildSuccessor(TranslatedElement child) {
-    // All children go to the successor of the `try`.
-    child = getAChild() and result = getParent().getChildSuccessor(this)
+    // All non-finally children go to the successor of the `try` if
+    // there is no finally block, but if there is a finally block
+    // then we go to that one.
+    child = [this.getBody(), this.getHandler(_)] and
+    (
+      not exists(this.getFinally()) and
+      result = this.getParent().getChildSuccessor(this)
+      or
+      result = this.getFinally().getFirstInstruction()
+    )
+    or
+    // And after the finally block we go to the successor of the `try`.
+    child = this.getFinally() and
+    result = this.getParent().getChildSuccessor(this)
   }
 
   final Instruction getNextHandler(TranslatedHandler handler) {
@@ -290,9 +560,9 @@ class TranslatedTryStmt extends TranslatedStmt {
     result = getHandler(0).getFirstInstruction()
   }
 
-  private TranslatedHandler getHandler(int index) {
-    result = getTranslatedStmt(stmt.getChild(index + 1))
-  }
+  private TranslatedElement getHandler(int index) { result = stmt.getTranslatedHandler(index) }
+
+  private TranslatedStmt getFinally() { result = getTranslatedStmt(stmt.getFinally()) }
 
   private TranslatedStmt getBody() { result = getTranslatedStmt(stmt.getStmt()) }
 }

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/Operand.qll

@@ -45,7 +45,7 @@ class Operand extends TStageOperand {
       this = reusedPhiOperand(use, def, predecessorBlock, _)
     )
     or
-    exists(Instruction use | this = chiOperand(use, _))
+    this = chiOperand(_, _)
   }
 
   /** Gets a textual representation of this element. */

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll

@@ -329,12 +329,12 @@ private module Cached {
   cached
   Instruction getChiInstructionTotalOperand(ChiInstruction chiInstr) {
     exists(
-      Alias::VirtualVariable vvar, OldInstruction oldInstr, Alias::MemoryLocation defLocation,
-      OldBlock defBlock, int defRank, int defOffset, OldBlock useBlock, int useRank
+      Alias::VirtualVariable vvar, OldInstruction oldInstr, OldBlock defBlock, int defRank,
+      int defOffset, OldBlock useBlock, int useRank
     |
       chiInstr = getChi(oldInstr) and
       vvar = Alias::getResultMemoryLocation(oldInstr).getVirtualVariable() and
-      hasDefinitionAtRank(vvar, defLocation, defBlock, defRank, defOffset) and
+      hasDefinitionAtRank(vvar, _, defBlock, defRank, defOffset) and
       hasUseAtRank(vvar, useBlock, useRank, oldInstr) and
       definitionReachesUse(vvar, defBlock, defRank, useBlock, useRank) and
       result = getDefinitionOrChiInstruction(defBlock, defOffset, vvar, _)

cpp/ql/lib/semmle/code/cpp/ir/internal/IRCppLanguage.qll

@@ -67,9 +67,7 @@ class Class = Cpp::Class; // Used for inheritance conversions
 
 predicate getIdentityString = Print::getIdentityString/1;
 
-predicate hasCaseEdge(string minValue, string maxValue) {
-  exists(Cpp::SwitchCase switchCase | hasCaseEdge(switchCase, minValue, maxValue))
-}
+predicate hasCaseEdge(string minValue, string maxValue) { hasCaseEdge(_, minValue, maxValue) }
 
 predicate hasPositionalArgIndex(int argIndex) {
   exists(Cpp::FunctionCall call | exists(call.getArgument(argIndex))) or

cpp/ql/lib/semmle/code/cpp/metrics/MetricClass.qll

@@ -99,10 +99,10 @@ class MetricClass extends Class {
   }
 
   /** Gets any method that accesses some local field. */
-  Function getAccessingMethod() { exists(Field f | this.accessesLocalField(result, f)) }
+  Function getAccessingMethod() { this.accessesLocalField(result, _) }
 
   /** Gets any field that is accessed by a local method. */
-  Field getAccessedField() { exists(Function func | this.accessesLocalField(func, result)) }
+  Field getAccessedField() { this.accessesLocalField(_, result) }
 
   /** Gets the Henderson-Sellers lack-of-cohesion metric. */
   float getLackOfCohesionHS() {
@@ -517,10 +517,10 @@ private predicate dependsOnClassSimple(Class source, Class dest) {
     )
     or
     // a class depends on classes for which a call to its member function is done from a function
-    exists(MemberFunction target, MemberFunction f, Locatable l |
+    exists(MemberFunction target, MemberFunction f |
       f.getDeclaringType() = source and
       f instanceof MemberFunction and
-      f.calls(target, l) and
+      f.calls(target, _) and
       target instanceof MemberFunction and
       target.getDeclaringType() = dest
     )

cpp/ql/lib/semmle/code/cpp/models/Models.qll

@@ -27,7 +27,7 @@ private import implementations.StdString
 private import implementations.Swap
 private import implementations.GetDelim
 private import implementations.SmartPointer
-private import implementations.Sscanf
+private import implementations.Scanf
 private import implementations.Send
 private import implementations.Recv
 private import implementations.Accept

cpp/ql/lib/semmle/code/cpp/models/implementations/Fread.qll

@@ -15,6 +15,6 @@ private class Fread extends AliasFunction, RemoteFlowSourceFunction {
 
   override predicate hasRemoteFlowSource(FunctionOutput output, string description) {
     output.isParameterDeref(0) and
-    description = "String read by " + this.getName()
+    description = "string read by " + this.getName()
   }
 }

cpp/ql/lib/semmle/code/cpp/models/implementations/GetDelim.qll

@@ -36,6 +36,6 @@ private class GetDelimFunction extends TaintFunction, AliasFunction, SideEffectF
 
   override predicate hasRemoteFlowSource(FunctionOutput output, string description) {
     output.isParameterDeref(0) and
-    description = "String read by " + this.getName()
+    description = "string read by " + this.getName()
   }
 }

cpp/ql/lib/semmle/code/cpp/models/implementations/Getenv.qll

@@ -1,15 +1,19 @@
 /**
- * Provides an implementation class modeling the POSIX function `getenv`.
+ * Provides an implementation class modeling the POSIX function `getenv` and
+ * various similar functions.
  */
 
 import cpp
 import semmle.code.cpp.models.interfaces.FlowSource
 
 /**
- * The POSIX function `getenv`.
+ * The POSIX function `getenv`, the GNU function `secure_getenv`, and the
+ * Windows function `_wgetenv`.
  */
 class Getenv extends LocalFlowSourceFunction {
-  Getenv() { this.hasGlobalOrStdOrBslName("getenv") }
+  Getenv() {
+    this.hasGlobalOrStdOrBslName("getenv") or this.hasGlobalName(["secure_getenv", "_wgetenv"])
+  }
 
   override predicate hasLocalFlowSource(FunctionOutput output, string description) {
     (

cpp/ql/lib/semmle/code/cpp/models/implementations/Gets.qll

@@ -49,10 +49,10 @@ private class FgetsFunction extends DataFlowFunction, TaintFunction, ArrayFuncti
 
   override predicate hasRemoteFlowSource(FunctionOutput output, string description) {
     output.isParameterDeref(0) and
-    description = "String read by " + this.getName()
+    description = "string read by " + this.getName()
     or
     output.isReturnValue() and
-    description = "String read by " + this.getName()
+    description = "string read by " + this.getName()
   }
 
   override predicate hasArrayWithVariableSize(int bufParam, int countParam) {
@@ -98,10 +98,10 @@ private class GetsFunction extends DataFlowFunction, ArrayFunction, AliasFunctio
 
   override predicate hasLocalFlowSource(FunctionOutput output, string description) {
     output.isParameterDeref(0) and
-    description = "String read by " + this.getName()
+    description = "string read by " + this.getName()
     or
     output.isReturnValue() and
-    description = "String read by " + this.getName()
+    description = "string read by " + this.getName()
   }
 
   override predicate hasArrayWithUnknownSize(int bufParam) { bufParam = 0 }

cpp/ql/lib/semmle/code/cpp/models/implementations/Inet.qll

@@ -1,9 +1,10 @@
 import semmle.code.cpp.models.interfaces.Taint
 import semmle.code.cpp.models.interfaces.Alias
 import semmle.code.cpp.models.interfaces.ArrayFunction
+import semmle.code.cpp.models.interfaces.FlowSource
 
 private class InetNtoa extends TaintFunction {
-  InetNtoa() { hasGlobalName("inet_ntoa") }
+  InetNtoa() { this.hasGlobalName("inet_ntoa") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isParameter(0) and
@@ -12,7 +13,7 @@ private class InetNtoa extends TaintFunction {
 }
 
 private class InetAton extends TaintFunction, ArrayFunction {
-  InetAton() { hasGlobalName("inet_aton") }
+  InetAton() { this.hasGlobalName("inet_aton") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isParameterDeref(0) and
@@ -32,7 +33,7 @@ private class InetAton extends TaintFunction, ArrayFunction {
 }
 
 private class InetAddr extends TaintFunction, ArrayFunction, AliasFunction {
-  InetAddr() { hasGlobalName("inet_addr") }
+  InetAddr() { this.hasGlobalName("inet_addr") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isParameterDeref(0) and
@@ -51,7 +52,7 @@ private class InetAddr extends TaintFunction, ArrayFunction, AliasFunction {
 }
 
 private class InetNetwork extends TaintFunction, ArrayFunction {
-  InetNetwork() { hasGlobalName("inet_network") }
+  InetNetwork() { this.hasGlobalName("inet_network") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isParameterDeref(0) and
@@ -64,7 +65,7 @@ private class InetNetwork extends TaintFunction, ArrayFunction {
 }
 
 private class InetMakeaddr extends TaintFunction {
-  InetMakeaddr() { hasGlobalName("inet_makeaddr") }
+  InetMakeaddr() { this.hasGlobalName("inet_makeaddr") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     (
@@ -76,7 +77,7 @@ private class InetMakeaddr extends TaintFunction {
 }
 
 private class InetLnaof extends TaintFunction {
-  InetLnaof() { hasGlobalName("inet_lnaof") }
+  InetLnaof() { this.hasGlobalName("inet_lnaof") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isParameter(0) and
@@ -85,7 +86,7 @@ private class InetLnaof extends TaintFunction {
 }
 
 private class InetNetof extends TaintFunction {
-  InetNetof() { hasGlobalName("inet_netof") }
+  InetNetof() { this.hasGlobalName("inet_netof") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isParameter(0) and
@@ -94,7 +95,7 @@ private class InetNetof extends TaintFunction {
 }
 
 private class InetPton extends TaintFunction, ArrayFunction {
-  InetPton() { hasGlobalName("inet_pton") }
+  InetPton() { this.hasGlobalName("inet_pton") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     (
@@ -114,7 +115,7 @@ private class InetPton extends TaintFunction, ArrayFunction {
 }
 
 private class Gethostbyname extends TaintFunction, ArrayFunction {
-  Gethostbyname() { hasGlobalName("gethostbyname") }
+  Gethostbyname() { this.hasGlobalName("gethostbyname") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isParameterDeref(0) and
@@ -127,7 +128,7 @@ private class Gethostbyname extends TaintFunction, ArrayFunction {
 }
 
 private class Gethostbyaddr extends TaintFunction, ArrayFunction {
-  Gethostbyaddr() { hasGlobalName("gethostbyaddr") }
+  Gethostbyaddr() { this.hasGlobalName("gethostbyaddr") }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     (
@@ -142,3 +143,21 @@ private class Gethostbyaddr extends TaintFunction, ArrayFunction {
 
   override predicate hasArrayWithNullTerminator(int bufParam) { bufParam = 0 }
 }
+
+private class Getaddrinfo extends TaintFunction, ArrayFunction, RemoteFlowSourceFunction {
+  Getaddrinfo() { this.hasGlobalName("getaddrinfo") }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    input.isParameterDeref([0 .. 2]) and
+    output.isParameterDeref(3)
+  }
+
+  override predicate hasArrayInput(int bufParam) { bufParam in [0, 1] }
+
+  override predicate hasArrayWithNullTerminator(int bufParam) { bufParam in [0, 1] }
+
+  override predicate hasRemoteFlowSource(FunctionOutput output, string description) {
+    output.isParameterDeref(3) and
+    description = "address returned by " + this.getName()
+  }
+}

cpp/ql/lib/semmle/code/cpp/models/implementations/Iterator.qll

@@ -31,7 +31,17 @@ private class IteratorTraits extends Class {
  * `std::iterator_traits` instantiation for it.
  */
 private class IteratorByTraits extends Iterator {
-  IteratorByTraits() { exists(IteratorTraits it | it.getIteratorType() = this) }
+  IteratorTraits trait;
+
+  IteratorByTraits() { trait.getIteratorType() = this }
+
+  override Type getValueType() {
+    exists(TypedefType t |
+      trait.getAMember() = t and
+      t.getName() = "value_type" and
+      result = t.getUnderlyingType()
+    )
+  }
 }
 
 /**
@@ -42,20 +52,27 @@ private class IteratorByTraits extends Iterator {
  */
 private class IteratorByPointer extends Iterator instanceof PointerType {
   IteratorByPointer() { not this instanceof IteratorByTraits }
+
+  override Type getValueType() { result = super.getBaseType() }
 }
 
 /**
  * A type which has the typedefs expected for an iterator.
  */
 private class IteratorByTypedefs extends Iterator, Class {
+  TypedefType valueType;
+
   IteratorByTypedefs() {
     this.getAMember().(TypedefType).hasName("difference_type") and
-    this.getAMember().(TypedefType).hasName("value_type") and
+    valueType = this.getAMember() and
+    valueType.hasName("value_type") and
     this.getAMember().(TypedefType).hasName("pointer") and
     this.getAMember().(TypedefType).hasName("reference") and
     this.getAMember().(TypedefType).hasName("iterator_category") and
     not this.hasQualifiedName(["std", "bsl"], "iterator_traits")
   }
+
+  override Type getValueType() { result = valueType.getUnderlyingType() }
 }
 
 /**
@@ -63,6 +80,8 @@ private class IteratorByTypedefs extends Iterator, Class {
  */
 private class StdIterator extends Iterator, Class {
   StdIterator() { this.hasQualifiedName(["std", "bsl"], "iterator") }
+
+  override Type getValueType() { result = this.getTemplateArgument(1).(Type).getUnderlyingType() }
 }
 
 /**
@@ -166,12 +185,15 @@ private class IteratorSubOperator extends Operator, TaintFunction {
 /**
  * A non-member `operator+=` or `operator-=` function for an iterator type.
  */
-private class IteratorAssignArithmeticOperator extends Operator, DataFlowFunction, TaintFunction {
+class IteratorAssignArithmeticOperator extends Operator {
   IteratorAssignArithmeticOperator() {
     this.hasName(["operator+=", "operator-="]) and
     exists(getIteratorArgumentInput(this, 0))
   }
+}
 
+private class IteratorAssignArithmeticOperatorModel extends IteratorAssignArithmeticOperator,
+  DataFlowFunction, TaintFunction {
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
     input.isParameter(0) and
     output.isReturnValue()
@@ -210,11 +232,14 @@ class IteratorPointerDereferenceMemberOperator extends MemberFunction, TaintFunc
 /**
  * An `operator++` or `operator--` member function for an iterator type.
  */
-private class IteratorCrementMemberOperator extends MemberFunction, DataFlowFunction, TaintFunction {
+class IteratorCrementMemberOperator extends MemberFunction {
   IteratorCrementMemberOperator() {
     this.getClassAndName(["operator++", "operator--"]) instanceof Iterator
   }
+}
 
+private class IteratorCrementMemberOperatorModel extends IteratorCrementMemberOperator,
+  DataFlowFunction, TaintFunction {
   override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
     input.isQualifierAddress() and
     output.isReturnValue()

cpp/ql/lib/semmle/code/cpp/models/implementations/Recv.qll

@@ -83,7 +83,7 @@ private class Recv extends AliasFunction, ArrayFunction, SideEffectFunction,
       or
       this.hasGlobalName("recvfrom") and output.isParameterDeref([4, 5])
     ) and
-    description = "Buffer read by " + this.getName()
+    description = "buffer read by " + this.getName()
   }
 
   override predicate hasSocketInput(FunctionInput input) { input.isParameter(0) }

cpp/ql/lib/semmle/code/cpp/models/implementations/Scanf.qll

@@ -1,6 +1,6 @@
 /**
- * Provides implementation classes modeling `sscanf`, `fscanf` and various similar
- * functions. See `semmle.code.cpp.models.Models` for usage information.
+ * Provides implementation classes modeling the `scanf` family of functions.
+ * See `semmle.code.cpp.models.Models` for usage information.
  */
 
 import semmle.code.cpp.Function
@@ -9,18 +9,15 @@ import semmle.code.cpp.models.interfaces.ArrayFunction
 import semmle.code.cpp.models.interfaces.Taint
 import semmle.code.cpp.models.interfaces.Alias
 import semmle.code.cpp.models.interfaces.SideEffect
+import semmle.code.cpp.models.interfaces.FlowSource
 
 /**
- * The standard function `sscanf`, `fscanf` and its assorted variants
+ * The `scanf` family of functions.
  */
-private class SscanfModel extends ArrayFunction, TaintFunction, AliasFunction, SideEffectFunction {
-  SscanfModel() { this instanceof Sscanf or this instanceof Fscanf or this instanceof Snscanf }
-
+abstract private class ScanfFunctionModel extends ArrayFunction, TaintFunction, AliasFunction,
+  SideEffectFunction {
   override predicate hasArrayWithNullTerminator(int bufParam) {
     bufParam = this.(ScanfFunction).getFormatParameterIndex()
-    or
-    not this instanceof Fscanf and
-    bufParam = this.(ScanfFunction).getInputParameterIndex()
   }
 
   override predicate hasArrayInput(int bufParam) { this.hasArrayWithNullTerminator(bufParam) }
@@ -36,7 +33,7 @@ private class SscanfModel extends ArrayFunction, TaintFunction, AliasFunction, S
     )
   }
 
-  private int getArgsStartPosition() { result = this.getNumberOfParameters() }
+  int getArgsStartPosition() { result = this.getNumberOfParameters() }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isParameterDeref(this.(ScanfFunction).getInputParameterIndex()) and
@@ -70,3 +67,36 @@ private class SscanfModel extends ArrayFunction, TaintFunction, AliasFunction, S
       ]
   }
 }
+
+/**
+ * The standard function `scanf` and its assorted variants
+ */
+private class ScanfModel extends ScanfFunctionModel, LocalFlowSourceFunction instanceof Scanf {
+  override predicate hasLocalFlowSource(FunctionOutput output, string description) {
+    output.isParameterDeref(any(int i | i >= this.getArgsStartPosition())) and
+    description = "value read by " + this.getName()
+  }
+}
+
+/**
+ * The standard function `fscanf` and its assorted variants
+ */
+private class FscanfModel extends ScanfFunctionModel, RemoteFlowSourceFunction instanceof Fscanf {
+  override predicate hasRemoteFlowSource(FunctionOutput output, string description) {
+    output.isParameterDeref(any(int i | i >= this.getArgsStartPosition())) and
+    description = "value read by " + this.getName()
+  }
+}
+
+/**
+ * The standard function `sscanf` and its assorted variants
+ */
+private class SscanfModel extends ScanfFunctionModel {
+  SscanfModel() { this instanceof Sscanf or this instanceof Snscanf }
+
+  override predicate hasArrayWithNullTerminator(int bufParam) {
+    super.hasArrayWithNullTerminator(bufParam)
+    or
+    bufParam = this.(ScanfFunction).getInputParameterIndex()
+  }
+}

cpp/ql/lib/semmle/code/cpp/models/implementations/Send.qll

@@ -58,7 +58,7 @@ private class Send extends AliasFunction, ArrayFunction, SideEffectFunction, Rem
   override ParameterIndex getParameterSizeIndex(ParameterIndex i) { i = 1 and result = 2 }
 
   override predicate hasRemoteFlowSink(FunctionInput input, string description) {
-    input.isParameterDeref(1) and description = "Buffer sent by " + this.getName()
+    input.isParameterDeref(1) and description = "buffer sent by " + this.getName()
   }
 
   override predicate hasSocketInput(FunctionInput input) { input.isParameter(0) }

cpp/ql/lib/semmle/code/cpp/models/implementations/StdContainer.qll

@@ -5,38 +5,53 @@
 import semmle.code.cpp.models.interfaces.Taint
 import semmle.code.cpp.models.interfaces.Iterator
 
+/**
+ * A sequence container template class (for example, `std::vector`) from the
+ * standard library.
+ */
+abstract class StdSequenceContainer extends Class {
+  Type getElementType() { result = this.getTemplateArgument(0) }
+}
+
 /**
  * The `std::array` template class.
  */
-private class Array extends Class {
+private class Array extends StdSequenceContainer {
   Array() { this.hasQualifiedName(["std", "bsl"], "array") }
 }
 
+/**
+ * The `std::string` template class.
+ */
+private class String extends StdSequenceContainer {
+  String() { this.hasQualifiedName(["std", "bsl"], "basic_string") }
+}
+
 /**
  * The `std::deque` template class.
  */
-private class Deque extends Class {
+private class Deque extends StdSequenceContainer {
   Deque() { this.hasQualifiedName(["std", "bsl"], "deque") }
 }
 
 /**
  * The `std::forward_list` template class.
  */
-private class ForwardList extends Class {
+private class ForwardList extends StdSequenceContainer {
   ForwardList() { this.hasQualifiedName(["std", "bsl"], "forward_list") }
 }
 
 /**
  * The `std::list` template class.
  */
-private class List extends Class {
+private class List extends StdSequenceContainer {
   List() { this.hasQualifiedName(["std", "bsl"], "list") }
 }
 
 /**
  * The `std::vector` template class.
  */
-private class Vector extends Class {
+private class Vector extends StdSequenceContainer {
   Vector() { this.hasQualifiedName(["std", "bsl"], "vector") }
 }
 

cpp/ql/lib/semmle/code/cpp/models/implementations/StdString.qll

@@ -16,21 +16,32 @@ private class StdBasicString extends ClassTemplateInstantiation {
 }
 
 /**
- * Additional model for `std::string` constructors that reference the character
- * type of the container, or an iterator.  For example construction from
- * iterators:
- * ```
- * std::string b(a.begin(), a.end());
- * ```
+ * The `std::basic_string::iterator` declaration.
+ *
+ * Intuitively, this class shouldn't be necessary as it's already captured
+ * by the `StdIterator` class. However, this class ensures that the typedef inside the
+ * body of the `std::string` class is also seen as an iterator.
+ *
+ * Eventually, we should be consistent about which of the following should be recognized as iterators:
+ * 1. The typedef type.
+ * 2. The template class of the resolved type.
+ * 3. Any instantiation of the resolved type.
  */
-private class StdStringConstructor extends Constructor, TaintFunction {
-  StdStringConstructor() { this.getDeclaringType() instanceof StdBasicString }
+private class StdBasicStringIterator extends Iterator, Type {
+  StdBasicStringIterator() {
+    this.getEnclosingElement() instanceof StdBasicString and this.hasName("iterator")
+  }
+}
 
+/**
+ * A `std::string` function for which taint should be propagated.
+ */
+abstract private class StdStringTaintFunction extends TaintFunction {
   /**
    * Gets the index of a parameter to this function that is a string (or
    * character).
    */
-  int getAStringParameterIndex() {
+  final int getAStringParameterIndex() {
     exists(Type paramType | paramType = this.getParameter(result).getUnspecifiedType() |
       // e.g. `std::basic_string::CharT *`
       paramType instanceof PointerType
@@ -41,15 +52,28 @@ private class StdStringConstructor extends Constructor, TaintFunction {
         this.getDeclaringType().getTemplateArgument(2).(Type).getUnspecifiedType()
       or
       // i.e. `std::basic_string::CharT`
-      this.getParameter(result).getUnspecifiedType() =
-        this.getDeclaringType().getTemplateArgument(0).(Type).getUnspecifiedType()
+      paramType = this.getDeclaringType().getTemplateArgument(0).(Type).getUnspecifiedType()
     )
   }
 
   /**
    * Gets the index of a parameter to this function that is an iterator.
    */
-  int getAnIteratorParameterIndex() { this.getParameter(result).getType() instanceof Iterator }
+  final int getAnIteratorParameterIndex() {
+    this.getParameter(result).getType() instanceof Iterator
+  }
+}
+
+/**
+ * Additional model for `std::string` constructors that reference the character
+ * type of the container, or an iterator.  For example construction from
+ * iterators:
+ * ```
+ * std::string b(a.begin(), a.end());
+ * ```
+ */
+private class StdStringConstructor extends Constructor, StdStringTaintFunction {
+  StdStringConstructor() { this.getDeclaringType() instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // taint flow from any parameter of the value type to the returned object
@@ -68,7 +92,7 @@ private class StdStringConstructor extends Constructor, TaintFunction {
 /**
  * The `std::string` function `c_str`.
  */
-private class StdStringCStr extends TaintFunction {
+private class StdStringCStr extends StdStringTaintFunction {
   StdStringCStr() { this.getClassAndName("c_str") instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -81,7 +105,7 @@ private class StdStringCStr extends TaintFunction {
 /**
  * The `std::string` function `data`.
  */
-private class StdStringData extends TaintFunction {
+private class StdStringData extends StdStringTaintFunction {
   StdStringData() { this.getClassAndName("data") instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -99,7 +123,7 @@ private class StdStringData extends TaintFunction {
 /**
  * The `std::string` function `push_back`.
  */
-private class StdStringPush extends TaintFunction {
+private class StdStringPush extends StdStringTaintFunction {
   StdStringPush() { this.getClassAndName("push_back") instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -112,7 +136,7 @@ private class StdStringPush extends TaintFunction {
 /**
  * The `std::string` functions `front` and `back`.
  */
-private class StdStringFrontBack extends TaintFunction {
+private class StdStringFrontBack extends StdStringTaintFunction {
   StdStringFrontBack() { this.getClassAndName(["front", "back"]) instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -125,7 +149,7 @@ private class StdStringFrontBack extends TaintFunction {
 /**
  * The (non-member) `std::string` function `operator+`.
  */
-private class StdStringPlus extends TaintFunction {
+private class StdStringPlus extends StdStringTaintFunction {
   StdStringPlus() {
     this.hasQualifiedName(["std", "bsl"], "operator+") and
     this.getUnspecifiedType() instanceof StdBasicString
@@ -142,31 +166,15 @@ private class StdStringPlus extends TaintFunction {
 }
 
 /**
- * The `std::string` functions `operator+=`, `append`, `insert` and
- * `replace`. All of these functions combine the existing string
- * with a new string (or character) from one of the arguments.
+ * The `std::string` functions `operator+=`, `append` and `replace`.
+ * All of these functions combine the existing string with a new
+ * string (or character) from one of the arguments.
  */
-private class StdStringAppend extends TaintFunction {
+private class StdStringAppend extends StdStringTaintFunction {
   StdStringAppend() {
-    this.getClassAndName(["operator+=", "append", "insert", "replace"]) instanceof StdBasicString
+    this.getClassAndName(["operator+=", "append", "replace"]) instanceof StdBasicString
   }
 
-  /**
-   * Gets the index of a parameter to this function that is a string (or
-   * character).
-   */
-  int getAStringParameterIndex() {
-    this.getParameter(result).getType() instanceof PointerType or // e.g. `std::basic_string::CharT *`
-    this.getParameter(result).getType() instanceof ReferenceType or // e.g. `std::basic_string &`
-    this.getParameter(result).getUnspecifiedType() =
-      this.getDeclaringType().getTemplateArgument(0).(Type).getUnspecifiedType() // i.e. `std::basic_string::CharT`
-  }
-
-  /**
-   * Gets the index of a parameter to this function that is an iterator.
-   */
-  int getAnIteratorParameterIndex() { this.getParameter(result).getType() instanceof Iterator }
-
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // flow from string and parameter to string (qualifier) and return value
     (
@@ -186,28 +194,44 @@ private class StdStringAppend extends TaintFunction {
   }
 }
 
+/**
+ * The `std::string` function `insert`.
+ */
+private class StdStringInsert extends StdStringTaintFunction {
+  StdStringInsert() { this.getClassAndName("insert") instanceof StdBasicString }
+
+  /**
+   * Holds if the return type is an iterator.
+   */
+  predicate hasIteratorReturnValue() { this.getType() instanceof Iterator }
+
+  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+    // flow from string and parameter to string (qualifier) and return value
+    (
+      input.isQualifierObject() or
+      input.isParameterDeref(this.getAStringParameterIndex()) or
+      input.isParameter(this.getAnIteratorParameterIndex())
+    ) and
+    (
+      output.isQualifierObject()
+      or
+      if this.hasIteratorReturnValue() then output.isReturnValue() else output.isReturnValueDeref()
+    )
+    or
+    // reverse flow from returned reference to the qualifier (for writes to
+    // the result)
+    not this.hasIteratorReturnValue() and
+    input.isReturnValueDeref() and
+    output.isQualifierObject()
+  }
+}
+
 /**
  * The standard function `std::string.assign`.
  */
-private class StdStringAssign extends TaintFunction {
+private class StdStringAssign extends StdStringTaintFunction {
   StdStringAssign() { this.getClassAndName("assign") instanceof StdBasicString }
 
-  /**
-   * Gets the index of a parameter to this function that is a string (or
-   * character).
-   */
-  int getAStringParameterIndex() {
-    this.getParameter(result).getType() instanceof PointerType or // e.g. `std::basic_string::CharT *`
-    this.getParameter(result).getType() instanceof ReferenceType or // e.g. `std::basic_string &`
-    this.getParameter(result).getUnspecifiedType() =
-      this.getDeclaringType().getTemplateArgument(0).(Type).getUnspecifiedType() // i.e. `std::basic_string::CharT`
-  }
-
-  /**
-   * Gets the index of a parameter to this function that is an iterator.
-   */
-  int getAnIteratorParameterIndex() { this.getParameter(result).getType() instanceof Iterator }
-
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // flow from parameter to string itself (qualifier) and return value
     (
@@ -229,7 +253,7 @@ private class StdStringAssign extends TaintFunction {
 /**
  * The standard function `std::string.copy`.
  */
-private class StdStringCopy extends TaintFunction {
+private class StdStringCopy extends StdStringTaintFunction {
   StdStringCopy() { this.getClassAndName("copy") instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -242,7 +266,7 @@ private class StdStringCopy extends TaintFunction {
 /**
  * The standard function `std::string.substr`.
  */
-private class StdStringSubstr extends TaintFunction {
+private class StdStringSubstr extends StdStringTaintFunction {
   StdStringSubstr() { this.getClassAndName("substr") instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
@@ -255,7 +279,7 @@ private class StdStringSubstr extends TaintFunction {
 /**
  * The `std::string` functions `at` and `operator[]`.
  */
-private class StdStringAt extends TaintFunction {
+private class StdStringAt extends StdStringTaintFunction {
   StdStringAt() { this.getClassAndName(["at", "operator[]"]) instanceof StdBasicString }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {

cpp/ql/lib/semmle/code/cpp/models/interfaces/Allocation.qll

@@ -11,38 +11,6 @@
 import semmle.code.cpp.Function
 import semmle.code.cpp.models.Models
 
-/**
- * An allocation function such as `malloc`.
- */
-abstract class AllocationFunction extends Function {
-  /**
-   * Gets the index of the argument for the allocation size, if any. The actual
-   * allocation size is the value of this argument multiplied by the result of
-   * `getSizeMult()`, in bytes.
-   */
-  int getSizeArg() { none() }
-
-  /**
-   * Gets the index of an argument that multiplies the allocation size given by
-   * `getSizeArg`, if any.
-   */
-  int getSizeMult() { none() }
-
-  /**
-   * Gets the index of the input pointer argument to be reallocated, if this
-   * is a `realloc` function.
-   */
-  int getReallocPtrArg() { none() }
-
-  /**
-   * Whether or not this allocation requires a corresponding deallocation of
-   * some sort (most do, but `alloca` for example does not).  If it is unclear,
-   * we default to no (for example a placement `new` allocation may or may not
-   * require a corresponding `delete`).
-   */
-  predicate requiresDealloc() { any() }
-}
-
 /**
  * An allocation expression such as call to `malloc` or a `new` expression.
  */
@@ -86,6 +54,41 @@ abstract class AllocationExpr extends Expr {
   predicate requiresDealloc() { any() }
 }
 
+/**
+ * An allocation function such as `malloc`.
+ *
+ * Note: `AllocationExpr` includes calls to allocation functions, so prefer
+ * to use that class unless you specifically need to reason about functions.
+ */
+abstract class AllocationFunction extends Function {
+  /**
+   * Gets the index of the argument for the allocation size, if any. The actual
+   * allocation size is the value of this argument multiplied by the result of
+   * `getSizeMult()`, in bytes.
+   */
+  int getSizeArg() { none() }
+
+  /**
+   * Gets the index of an argument that multiplies the allocation size given by
+   * `getSizeArg`, if any.
+   */
+  int getSizeMult() { none() }
+
+  /**
+   * Gets the index of the input pointer argument to be reallocated, if this
+   * is a `realloc` function.
+   */
+  int getReallocPtrArg() { none() }
+
+  /**
+   * Whether or not this allocation requires a corresponding deallocation of
+   * some sort (most do, but `alloca` for example does not).  If it is unclear,
+   * we default to no (for example a placement `new` allocation may or may not
+   * require a corresponding `delete`).
+   */
+  predicate requiresDealloc() { any() }
+}
+
 /**
  * An `operator new` or `operator new[]` function that may be associated with
  * `new` or `new[]` expressions.  Note that `new` and `new[]` are not function
@@ -130,13 +133,15 @@ abstract class HeuristicAllocationExpr extends Expr {
 
   /**
    * Gets a constant multiplier for the allocation size given by `getSizeExpr`,
-   * in bytes.
+   * in bytes. This predicate should be used with caution as it can be
+   * inaccurate for allocations identified using heuristics.
    */
   int getSizeMult() { none() }
 
   /**
    * Gets the size of this allocation in bytes, if it is a fixed size and that
-   * size can be determined.
+   * size can be determined. This predicate should be used with caution as it
+   * can be inaccurate for allocations identified using heuristics.
    */
   int getSizeBytes() { none() }
 

cpp/ql/lib/semmle/code/cpp/models/interfaces/Deallocation.qll

@@ -11,16 +11,6 @@
 import semmle.code.cpp.Function
 import semmle.code.cpp.models.Models
 
-/**
- * A deallocation function such as `free`.
- */
-abstract class DeallocationFunction extends Function {
-  /**
-   * Gets the index of the argument that is freed by this function.
-   */
-  int getFreedArg() { none() }
-}
-
 /**
  * An deallocation expression such as call to `free` or a `delete` expression.
  */
@@ -31,6 +21,19 @@ abstract class DeallocationExpr extends Expr {
   Expr getFreedExpr() { none() }
 }
 
+/**
+ * A deallocation function such as `free`.
+ *
+ * Note: `DeallocationExpr` includes calls to deallocation functions, so prefer
+ * to use that class unless you specifically need to reason about functions.
+ */
+abstract class DeallocationFunction extends Function {
+  /**
+   * Gets the index of the argument that is freed by this function.
+   */
+  int getFreedArg() { none() }
+}
+
 /**
  * An `operator delete` or `operator delete[]` function that may be associated
  * with `delete` or `delete[]` expressions.  Note that `delete` and `delete[]`

cpp/ql/lib/semmle/code/cpp/models/interfaces/Iterator.qll

@@ -29,5 +29,17 @@ abstract class GetIteratorFunction extends Function {
 
 /**
  * A type which can be used as an iterator.
+ *
+ * Note: Do _not_ `extend` when inheriting from this class in queries. Always use `instanceof`:
+ * ```
+ * class MyIterator instanceof Iterator { ... }
+ * ```
  */
-abstract class Iterator extends Type { }
+abstract class Iterator extends Type {
+  /**
+   * Gets the value type of this iterator, if any.
+   *
+   * For example, the value type of a `std::vector<int>::iterator` is `int`.
+   */
+  Type getValueType() { none() }
+}

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

@@ -296,7 +296,7 @@ private predicate analyzableExpr(Expr e) {
     or
     // Also allow variable accesses, provided that they have SSA
     // information.
-    exists(RangeSsaDefinition def, StackVariable v | e = def.getAUse(v))
+    exists(RangeSsaDefinition def | e = def.getAUse(_))
     or
     e instanceof UnsignedBitwiseAndExpr
     or

cpp/ql/lib/semmle/code/cpp/security/FlowSources.qll

@@ -50,10 +50,10 @@ private class LocalModelSource extends LocalFlowSource {
 
 private class ArgvSource extends LocalFlowSource {
   ArgvSource() {
-    exists(Parameter argv |
-      argv.hasName("argv") and
-      argv.getFunction().hasGlobalName("main") and
-      this.asExpr() = argv.getAnAccess()
+    exists(Function main, Parameter argv |
+      main.hasGlobalName("main") and
+      main.getParameter(1) = argv and
+      this.asParameter(_) = argv
     )
   }
 

cpp/ql/lib/semmle/code/cpp/security/TaintTracking.qll

@@ -1,4 +1,4 @@
-/*
+/**
  * Support for tracking tainted data through the program. This is an alias for
  * `semmle.code.cpp.ir.dataflow.DefaultTaintTracking` provided for backwards
  * compatibility.

cpp/ql/lib/semmle/code/cpp/security/TaintTrackingImpl.qll

@@ -121,13 +121,15 @@ private predicate moveToDependingOnSide(Expr src, Expr dest) {
  *   (this is done to avoid false positives). Because of this we need to track if the tainted element came from an argument
  *   or not, and for that we use destFromArg
  */
-private predicate betweenFunctionsValueMoveTo(Element src, Element dest, boolean destFromArg) {
+deprecated private predicate betweenFunctionsValueMoveTo(
+  Element src, Element dest, boolean destFromArg
+) {
   not unreachable(src) and
   not unreachable(dest) and
   (
-    exists(Call call, Function called, int i |
+    exists(Call call, int i |
       src = call.getArgument(i) and
-      resolveCallWithParam(call, called, i, dest) and
+      resolveCallWithParam(call, _, i, dest) and
       destFromArg = true
     )
     or
@@ -149,8 +151,8 @@ private predicate betweenFunctionsValueMoveTo(Element src, Element dest, boolean
     )
     or
     // If a parameter of type reference is tainted inside a function, taint the argument too
-    exists(Call call, Function f, int pi, Parameter p |
-      resolveCallWithParam(call, f, pi, p) and
+    exists(Call call, int pi, Parameter p |
+      resolveCallWithParam(call, _, pi, p) and
       p.getType() instanceof ReferenceType and
       src = p and
       dest = call.getArgument(pi) and
@@ -162,13 +164,13 @@ private predicate betweenFunctionsValueMoveTo(Element src, Element dest, boolean
 // predicate folding for proper join-order
 // bad magic: pushes down predicate that ruins join-order
 pragma[nomagic]
-private predicate resolveCallWithParam(Call call, Function called, int i, Parameter p) {
+deprecated private predicate resolveCallWithParam(Call call, Function called, int i, Parameter p) {
   called = resolveCall(call) and
   p = called.getParameter(i)
 }
 
 /** A variable for which flow through is allowed. */
-library class FlowVariable extends Variable {
+deprecated library class FlowVariable extends Variable {
   FlowVariable() {
     (
       this instanceof LocalScopeVariable or
@@ -179,11 +181,11 @@ library class FlowVariable extends Variable {
 }
 
 /** A local scope variable for which flow through is allowed. */
-library class FlowLocalScopeVariable extends Variable {
+deprecated library class FlowLocalScopeVariable extends Variable {
   FlowLocalScopeVariable() { this instanceof LocalScopeVariable }
 }
 
-private predicate insideFunctionValueMoveTo(Element src, Element dest) {
+deprecated private predicate insideFunctionValueMoveTo(Element src, Element dest) {
   not unreachable(src) and
   not unreachable(dest) and
   (
@@ -324,7 +326,7 @@ private predicate unionAccess(Variable v, Field f, FieldAccess a) {
   a.getQualifier() = v.getAnAccess()
 }
 
-GlobalOrNamespaceVariable globalVarFromId(string id) {
+deprecated GlobalOrNamespaceVariable globalVarFromId(string id) {
   if result instanceof NamespaceVariable
   then id = result.getNamespace() + "::" + result.getName()
   else id = result.getName()
@@ -353,7 +355,7 @@ private predicate hasUpperBoundsCheck(Variable var) {
 }
 
 cached
-private predicate taintedWithArgsAndGlobalVars(
+deprecated private predicate taintedWithArgsAndGlobalVars(
   Element src, Element dest, boolean destFromArg, string globalVar
 ) {
   isUserInput(src, _) and
@@ -395,7 +397,7 @@ private predicate taintedWithArgsAndGlobalVars(
  * This doesn't include data flow through global variables.
  * If you need that you must call taintedIncludingGlobalVars.
  */
-predicate tainted(Expr source, Element tainted) {
+deprecated predicate tainted(Expr source, Element tainted) {
   taintedWithArgsAndGlobalVars(source, tainted, _, "")
 }
 
@@ -410,7 +412,7 @@ predicate tainted(Expr source, Element tainted) {
  * The parameter `globalVar` is the name of the last global variable used to move the
  * value from source to tainted.
  */
-predicate taintedIncludingGlobalVars(Expr source, Element tainted, string globalVar) {
+deprecated predicate taintedIncludingGlobalVars(Expr source, Element tainted, string globalVar) {
   taintedWithArgsAndGlobalVars(source, tainted, _, globalVar)
 }
 
@@ -541,14 +543,14 @@ private predicate returnArgument(Function f, int sourceArg) {
  * targets a virtual method, simple data flow analysis is performed
  * in order to identify target(s).
  */
-Function resolveCall(Call call) {
+deprecated Function resolveCall(Call call) {
   result = call.getTarget()
   or
   result = call.(DataSensitiveCallExpr).resolve()
 }
 
 /** A data sensitive call expression. */
-abstract library class DataSensitiveCallExpr extends Expr {
+abstract deprecated library class DataSensitiveCallExpr extends Expr {
   DataSensitiveCallExpr() { not unreachable(this) }
 
   abstract Expr getSrc();
@@ -579,7 +581,7 @@ abstract library class DataSensitiveCallExpr extends Expr {
 }
 
 /** Call through a function pointer. */
-library class DataSensitiveExprCall extends DataSensitiveCallExpr, ExprCall {
+deprecated library class DataSensitiveExprCall extends DataSensitiveCallExpr, ExprCall {
   override Expr getSrc() { result = getExpr() }
 
   override Function resolve() {
@@ -588,7 +590,8 @@ library class DataSensitiveExprCall extends DataSensitiveCallExpr, ExprCall {
 }
 
 /** Call to a virtual function. */
-library class DataSensitiveOverriddenFunctionCall extends DataSensitiveCallExpr, FunctionCall {
+deprecated library class DataSensitiveOverriddenFunctionCall extends DataSensitiveCallExpr,
+  FunctionCall {
   DataSensitiveOverriddenFunctionCall() {
     exists(getTarget().(VirtualFunction).getAnOverridingFunction())
   }

cpp/ql/lib/semmle/code/cpp/stmts/Stmt.qll

@@ -34,7 +34,7 @@ class Stmt extends StmtParent, @stmt {
   }
 
   /** Gets a child of this statement. */
-  Element getAChild() { exists(int n | result = this.getChild(n)) }
+  Element getAChild() { result = this.getChild(_) }
 
   /** Gets the parent of this statement, if any. */
   StmtParent getParent() { stmtparents(underlyingElement(this), _, unresolveElement(result)) }

cpp/ql/lib/semmle/code/cpp/valuenumbering/GlobalValueNumberingImpl.qll

@@ -1,4 +1,8 @@
 /**
+ * DEPRECATED: This library has been replaced with a newer version which
+ * provides better performance and precision. Use
+ * `semmle.code.cpp.valuenumbering.GlobalValueNumbering` instead.
+ *
  * Provides an implementation of Global Value Numbering.
  * See https://en.wikipedia.org/wiki/Global_value_numbering
  *
@@ -179,7 +183,7 @@ private newtype GvnBase =
   // global variable will only get the same value number if they are
   // guaranteed to have the same value.
   GVN_OtherVariable(Variable x, ControlFlowNode dominator) { mk_OtherVariable(x, dominator, _) } or
-  GVN_FieldAccess(GVN s, Field f) {
+  deprecated GVN_FieldAccess(GVN s, Field f) {
     mk_DotFieldAccess(s, f, _) or
     mk_PointerFieldAccess_with_deref(s, f, _) or
     mk_ImplicitThisFieldAccess_with_deref(s, f, _)
@@ -188,7 +192,7 @@ private newtype GvnBase =
   // time the pointer was dereferenced, so we need to include a definition
   // location. As a crude (but safe) approximation, we use
   // `mostRecentSideEffect` to compute a definition location.
-  GVN_Deref(GVN p, ControlFlowNode dominator) {
+  deprecated GVN_Deref(GVN p, ControlFlowNode dominator) {
     mk_Deref(p, dominator, _) or
     mk_PointerFieldAccess(p, _, dominator, _) or
     mk_ImplicitThisFieldAccess_with_qualifier(p, _, dominator, _)
@@ -197,10 +201,12 @@ private newtype GvnBase =
     mk_ThisExpr(fcn, _) or
     mk_ImplicitThisFieldAccess(fcn, _, _, _)
   } or
-  GVN_Conversion(Type t, GVN child) { mk_Conversion(t, child, _) } or
-  GVN_BinaryOp(GVN lhs, GVN rhs, string opname) { mk_BinaryOp(lhs, rhs, opname, _) } or
-  GVN_UnaryOp(GVN child, string opname) { mk_UnaryOp(child, opname, _) } or
-  GVN_ArrayAccess(GVN x, GVN i, ControlFlowNode dominator) { mk_ArrayAccess(x, i, dominator, _) } or
+  deprecated GVN_Conversion(Type t, GVN child) { mk_Conversion(t, child, _) } or
+  deprecated GVN_BinaryOp(GVN lhs, GVN rhs, string opname) { mk_BinaryOp(lhs, rhs, opname, _) } or
+  deprecated GVN_UnaryOp(GVN child, string opname) { mk_UnaryOp(child, opname, _) } or
+  deprecated GVN_ArrayAccess(GVN x, GVN i, ControlFlowNode dominator) {
+    mk_ArrayAccess(x, i, dominator, _)
+  } or
   // Any expression that is not handled by the cases above is
   // given a unique number based on the expression itself.
   GVN_Unanalyzable(Expr e) { not analyzableExpr(e) }
@@ -221,7 +227,7 @@ private newtype GvnBase =
  * expression with this `GVN` and using its `toString` and `getLocation`
  * methods.
  */
-class GVN extends GvnBase {
+deprecated class GVN extends GvnBase {
   GVN() { this instanceof GvnBase }
 
   /** Gets an expression that has this GVN. */
@@ -336,7 +342,7 @@ private predicate analyzableDotFieldAccess(DotFieldAccess access) {
   not analyzableConst(access)
 }
 
-private predicate mk_DotFieldAccess(GVN qualifier, Field target, DotFieldAccess access) {
+deprecated private predicate mk_DotFieldAccess(GVN qualifier, Field target, DotFieldAccess access) {
   analyzableDotFieldAccess(access) and
   target = access.getTarget() and
   qualifier = globalValueNumber(access.getQualifier().getFullyConverted())
@@ -349,7 +355,7 @@ private predicate analyzablePointerFieldAccess(PointerFieldAccess access) {
   not analyzableConst(access)
 }
 
-private predicate mk_PointerFieldAccess(
+deprecated private predicate mk_PointerFieldAccess(
   GVN qualifier, Field target, ControlFlowNode dominator, PointerFieldAccess access
 ) {
   analyzablePointerFieldAccess(access) and
@@ -362,7 +368,7 @@ private predicate mk_PointerFieldAccess(
  * `obj->field` is equivalent to `(*obj).field`, so we need to wrap an
  * extra `GVN_Deref` around the qualifier.
  */
-private predicate mk_PointerFieldAccess_with_deref(
+deprecated private predicate mk_PointerFieldAccess_with_deref(
   GVN new_qualifier, Field target, PointerFieldAccess access
 ) {
   exists(GVN qualifier, ControlFlowNode dominator |
@@ -387,7 +393,7 @@ private predicate mk_ImplicitThisFieldAccess(
   fcn = access.getEnclosingFunction()
 }
 
-private predicate mk_ImplicitThisFieldAccess_with_qualifier(
+deprecated private predicate mk_ImplicitThisFieldAccess_with_qualifier(
   GVN qualifier, Field target, ControlFlowNode dominator, ImplicitThisFieldAccess access
 ) {
   exists(Function fcn |
@@ -396,7 +402,7 @@ private predicate mk_ImplicitThisFieldAccess_with_qualifier(
   )
 }
 
-private predicate mk_ImplicitThisFieldAccess_with_deref(
+deprecated private predicate mk_ImplicitThisFieldAccess_with_deref(
   GVN new_qualifier, Field target, ImplicitThisFieldAccess access
 ) {
   exists(GVN qualifier, ControlFlowNode dominator |
@@ -430,7 +436,7 @@ private predicate analyzableConversion(Conversion conv) {
   not analyzableConst(conv)
 }
 
-private predicate mk_Conversion(Type t, GVN child, Conversion conv) {
+deprecated private predicate mk_Conversion(Type t, GVN child, Conversion conv) {
   analyzableConversion(conv) and
   t = conv.getUnspecifiedType() and
   child = globalValueNumber(conv.getExpr())
@@ -444,7 +450,7 @@ private predicate analyzableBinaryOp(BinaryOperation op) {
   not analyzableConst(op)
 }
 
-private predicate mk_BinaryOp(GVN lhs, GVN rhs, string opname, BinaryOperation op) {
+deprecated private predicate mk_BinaryOp(GVN lhs, GVN rhs, string opname, BinaryOperation op) {
   analyzableBinaryOp(op) and
   lhs = globalValueNumber(op.getLeftOperand().getFullyConverted()) and
   rhs = globalValueNumber(op.getRightOperand().getFullyConverted()) and
@@ -459,7 +465,7 @@ private predicate analyzableUnaryOp(UnaryOperation op) {
   not analyzableConst(op)
 }
 
-private predicate mk_UnaryOp(GVN child, string opname, UnaryOperation op) {
+deprecated private predicate mk_UnaryOp(GVN child, string opname, UnaryOperation op) {
   analyzableUnaryOp(op) and
   child = globalValueNumber(op.getOperand().getFullyConverted()) and
   opname = op.getOperator()
@@ -482,7 +488,9 @@ private predicate analyzableArrayAccess(ArrayExpr ae) {
   not analyzableConst(ae)
 }
 
-private predicate mk_ArrayAccess(GVN base, GVN offset, ControlFlowNode dominator, ArrayExpr ae) {
+deprecated private predicate mk_ArrayAccess(
+  GVN base, GVN offset, ControlFlowNode dominator, ArrayExpr ae
+) {
   analyzableArrayAccess(ae) and
   base = globalValueNumber(ae.getArrayBase().getFullyConverted()) and
   offset = globalValueNumber(ae.getArrayOffset().getFullyConverted()) and
@@ -495,7 +503,7 @@ private predicate analyzablePointerDereferenceExpr(PointerDereferenceExpr deref)
   not analyzableConst(deref)
 }
 
-private predicate mk_Deref(GVN p, ControlFlowNode dominator, PointerDereferenceExpr deref) {
+deprecated private predicate mk_Deref(GVN p, ControlFlowNode dominator, PointerDereferenceExpr deref) {
   analyzablePointerDereferenceExpr(deref) and
   p = globalValueNumber(deref.getOperand().getFullyConverted()) and
   dominator = mostRecentSideEffect(deref)
@@ -503,7 +511,7 @@ private predicate mk_Deref(GVN p, ControlFlowNode dominator, PointerDereferenceE
 
 /** Gets the global value number of expression `e`. */
 cached
-GVN globalValueNumber(Expr e) {
+deprecated GVN globalValueNumber(Expr e) {
   exists(int val, Type t |
     mk_IntConst(val, t, e) and
     result = GVN_IntConst(val, t)

cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll

@@ -475,9 +475,7 @@ private predicate mk_NonmemberFunctionCall(Function fcn, HC_Args args, FunctionC
   fc.getTarget() = fcn and
   analyzableNonmemberFunctionCall(fc) and
   (
-    exists(HashCons head, HC_Args tail |
-      mk_ArgConsInner(head, tail, fc.getNumberOfArguments() - 1, args, fc)
-    )
+    mk_ArgConsInner(_, _, fc.getNumberOfArguments() - 1, args, fc)
     or
     fc.getNumberOfArguments() = 0 and
     args = HC_EmptyArgs()
@@ -494,9 +492,7 @@ private predicate analyzableExprCall(ExprCall ec) {
 private predicate mk_ExprCall(HashCons hc, HC_Args args, ExprCall ec) {
   hc.getAnExpr() = ec.getExpr() and
   (
-    exists(HashCons head, HC_Args tail |
-      mk_ArgConsInner(head, tail, ec.getNumberOfArguments() - 1, args, ec)
-    )
+    mk_ArgConsInner(_, _, ec.getNumberOfArguments() - 1, args, ec)
     or
     ec.getNumberOfArguments() = 0 and
     args = HC_EmptyArgs()
@@ -516,9 +512,7 @@ private predicate mk_MemberFunctionCall(Function fcn, HashCons qual, HC_Args arg
   analyzableMemberFunctionCall(fc) and
   hashCons(fc.getQualifier().getFullyConverted()) = qual and
   (
-    exists(HashCons head, HC_Args tail |
-      mk_ArgConsInner(head, tail, fc.getNumberOfArguments() - 1, args, fc)
-    )
+    mk_ArgConsInner(_, _, fc.getNumberOfArguments() - 1, args, fc)
     or
     fc.getNumberOfArguments() = 0 and
     args = HC_EmptyArgs()
@@ -541,10 +535,8 @@ private predicate mk_ArgCons(HashCons hc, int i, HC_Args list, Call c) {
   analyzableCall(c) and
   hc = hashCons(c.getArgument(i).getFullyConverted()) and
   (
-    exists(HashCons head, HC_Args tail |
-      mk_ArgConsInner(head, tail, i - 1, list, c) and
-      i > 0
-    )
+    mk_ArgConsInner(_, _, i - 1, list, c) and
+    i > 0
     or
     i = 0 and
     list = HC_EmptyArgs()

cpp/ql/lib/semmlecode.cpp.dbscheme

@@ -303,12 +303,11 @@ affectedbymacroexpansion(
     int inv: @macroinvocation ref
 );
 
-/*
-  case @macroinvocations.kind of
-    1 = macro expansion
-  | 2 = other macro reference
-  ;
-*/
+case @macroinvocation.kind of
+  1 = @macro_expansion
+| 2 = @other_macro_reference
+;
+
 macroinvocations(
     unique int id: @macroinvocation,
     int macro_id: @ppd_define ref,
@@ -345,28 +344,37 @@ macro_argument_expanded(
 );
 
 /*
-  case @function.kind of
-    1 = normal
-  | 2 = constructor
-  | 3 = destructor
-  | 4 = conversion
-  | 5 = operator
-  | 6 = builtin     // GCC built-in functions, e.g. __builtin___memcpy_chk
-  ;
+case @function.kind of
+  1 = @normal_function
+| 2 = @constructor
+| 3 = @destructor
+| 4 = @conversion_function
+| 5 = @operator
+| 6 = @builtin_function     // GCC built-in functions, e.g. __builtin___memcpy_chk
+;
 */
+
 functions(
     unique int id: @function,
     string name: string ref,
     int kind: int ref
 );
 
-function_entry_point(int id: @function ref, unique int entry_point: @stmt ref);
+function_entry_point(
+    int id: @function ref,
+    unique int entry_point: @stmt ref
+);
 
-function_return_type(int id: @function ref, int return_type: @type ref);
+function_return_type(
+    int id: @function ref,
+    int return_type: @type ref
+);
 
-/** If `function` is a coroutine, then this gives the
-    std::experimental::resumable_traits instance associated with it,
-    and the variables representing the `handle` and `promise` for it. */
+/**
+ * If `function` is a coroutine, then this gives the `std::experimental::resumable_traits`
+ * instance associated with it, and the variables representing the `handle` and `promise`
+ * for it.
+ */
 coroutine(
   unique int function: @function ref,
   int traits: @type ref,
@@ -392,7 +400,10 @@ function_deleted(unique int id: @function ref);
 
 function_defaulted(unique int id: @function ref);
 
-member_function_this_type(unique int id: @function ref, int this_type: @type ref);
+member_function_this_type(
+    unique int id: @function ref,
+    int this_type: @type ref
+);
 
 #keyset[id, type_id]
 fun_decls(
@@ -495,7 +506,10 @@ params(
     int type_id: @type ref
 );
 
-overrides(int new: @function ref, int old: @function ref);
+overrides(
+    int new: @function ref,
+    int old: @function ref
+);
 
 #keyset[id, type_id]
 membervariables(
@@ -541,63 +555,65 @@ enumconstants(
 
 @localscopevariable = @localvariable | @parameter;
 
-/*
-  Built-in types are the fundamental types, e.g., integral, floating, and void.
+/**
+ * Built-in types are the fundamental types, e.g., integral, floating, and void.
+ */
+case @builtintype.kind of
+   1 = @errortype
+|  2 = @unknowntype
+|  3 = @void
+|  4 = @boolean
+|  5 = @char
+|  6 = @unsigned_char
+|  7 = @signed_char
+|  8 = @short
+|  9 = @unsigned_short
+| 10 = @signed_short
+| 11 = @int
+| 12 = @unsigned_int
+| 13 = @signed_int
+| 14 = @long
+| 15 = @unsigned_long
+| 16 = @signed_long
+| 17 = @long_long
+| 18 = @unsigned_long_long
+| 19 = @signed_long_long
+// ... 20 Microsoft-specific __int8
+// ... 21 Microsoft-specific __int16
+// ... 22 Microsoft-specific __int32
+// ... 23 Microsoft-specific __int64
+| 24 = @float
+| 25 = @double
+| 26 = @long_double
+| 27 = @complex_float         // C99-specific _Complex float
+| 28 = @complex_double        // C99-specific _Complex double
+| 29 = @complex_long_double   // C99-specific _Complex long double
+| 30 = @imaginary_float       // C99-specific _Imaginary float
+| 31 = @imaginary_double      // C99-specific _Imaginary double
+| 32 = @imaginary_long_double // C99-specific _Imaginary long double
+| 33 = @wchar_t               // Microsoft-specific
+| 34 = @decltype_nullptr      // C++11
+| 35 = @int128                // __int128
+| 36 = @unsigned_int128       // unsigned __int128
+| 37 = @signed_int128         // signed __int128
+| 38 = @float128              // __float128
+| 39 = @complex_float128      // _Complex __float128
+| 40 = @decimal32             // _Decimal32
+| 41 = @decimal64             // _Decimal64
+| 42 = @decimal128            // _Decimal128
+| 43 = @char16_t
+| 44 = @char32_t
+| 45 = @std_float32           // _Float32
+| 46 = @float32x              // _Float32x
+| 47 = @std_float64           // _Float64
+| 48 = @float64x              // _Float64x
+| 49 = @std_float128          // _Float128
+| 50 = @float128x             // _Float128x
+| 51 = @char8_t
+| 52 = @float16               // _Float16
+| 53 = @complex_float16       // _Complex _Float16
+;
 
-  case @builtintype.kind of
-    1 = error
-  | 2 = unknown
-  | 3 = void
-  | 4 = boolean
-  | 5 = char
-  | 6 = unsigned_char
-  | 7 = signed_char
-  | 8 = short
-  | 9 = unsigned_short
-  | 10 = signed_short
-  | 11 = int
-  | 12 = unsigned_int
-  | 13 = signed_int
-  | 14 = long
-  | 15 = unsigned_long
-  | 16 = signed_long
-  | 17 = long_long
-  | 18 = unsigned_long_long
-  | 19 = signed_long_long
-  | 20 = __int8                 // Microsoft-specific
-  | 21 = __int16                // Microsoft-specific
-  | 22 = __int32                // Microsoft-specific
-  | 23 = __int64                // Microsoft-specific
-  | 24 = float
-  | 25 = double
-  | 26 = long_double
-  | 27 = _Complex_float         // C99-specific
-  | 28 = _Complex_double        // C99-specific
-  | 29 = _Complex_long double   // C99-specific
-  | 30 = _Imaginary_float       // C99-specific
-  | 31 = _Imaginary_double      // C99-specific
-  | 32 = _Imaginary_long_double // C99-specific
-  | 33 = wchar_t                // Microsoft-specific
-  | 34 = decltype_nullptr       // C++11
-  | 35 = __int128
-  | 36 = unsigned___int128
-  | 37 = signed___int128
-  | 38 = __float128
-  | 39 = _Complex___float128
-  | 40 = _Decimal32
-  | 41 = _Decimal64
-  | 42 = _Decimal128
-  | 43 = char16_t
-  | 44 = char32_t
-  | 45 = _Float32
-  | 46 = _Float32x
-  | 47 = _Float64
-  | 48 = _Float64x
-  | 49 = _Float128
-  | 50 = _Float128x
-  | 51 = char8_t
-  ;
-*/
 builtintypes(
     unique int id: @builtintype,
     string name: string ref,
@@ -607,23 +623,23 @@ builtintypes(
     int alignment: int ref
 );
 
-/*
-  Derived types are types that are directly derived from existing types and
-  point to, refer to, transform type data to return a new type.
-
-  case @derivedtype.kind of
-    1 = pointer
-  | 2 = reference
-  | 3 = type_with_specifiers
-  | 4 = array
-  | 5 = gnu_vector
-  | 6 = routineptr
-  | 7 = routinereference
-  | 8 = rvalue_reference // C++11
+/**
+ * Derived types are types that are directly derived from existing types and
+ * point to, refer to, transform type data to return a new type.
+ */
+case @derivedtype.kind of
+   1 = @pointer
+|  2 = @reference
+|  3 = @type_with_specifiers
+|  4 = @array
+|  5 = @gnu_vector
+|  6 = @routineptr
+|  7 = @routinereference
+|  8 = @rvalue_reference // C++11
 // ... 9 type_conforming_to_protocols deprecated
-  | 10 = block
-  ;
-*/
+| 10 = @block
+;
+
 derivedtypes(
     unique int id: @derivedtype,
     string name: string ref,
@@ -675,23 +691,24 @@ decltypes(
 );
 
 /*
-  case @usertype.kind of
-    1 = struct
-  | 2 = class
-  | 3 = union
-  | 4 = enum
-  | 5 = typedef                       // classic C: typedef typedef type name
-  | 6 = template
-  | 7 = template_parameter
-  | 8 = template_template_parameter
-  | 9 = proxy_class                   // a proxy class associated with a template parameter
+case @usertype.kind of
+   1 = @struct
+|  2 = @class
+|  3 = @union
+|  4 = @enum
+|  5 = @typedef                       // classic C: typedef typedef type name
+|  6 = @template
+|  7 = @template_parameter
+|  8 = @template_template_parameter
+|  9 = @proxy_class                   // a proxy class associated with a template parameter
 // ... 10 objc_class deprecated
 // ... 11 objc_protocol deprecated
 // ... 12 objc_category deprecated
-  | 13 = scoped_enum
-  | 14 = using_alias                  // a using name = type style typedef
-  ;
+| 13 = @scoped_enum
+| 14 = @using_alias                  // a using name = type style typedef
+;
 */
+
 usertypes(
     unique int id: @usertype,
     string name: string ref,
@@ -1162,7 +1179,10 @@ case @funbindexpr.kind of
 | 2 = @adl_call     // a call whose target is only found by ADL
 ;
 */
-iscall(unique int caller: @funbindexpr ref, int kind: int ref);
+iscall(
+    unique int caller: @funbindexpr ref,
+    int kind: int ref
+);
 
 numtemplatearguments(
     unique int expr_id: @expr ref,