From 04454ef18422737bf519b764c83a4653b1bdfc18 Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Fri, 5 Jul 2019 16:29:41 +0100
Subject: [PATCH 01/28] Begin extending dataflow node model for field flow (and
 other stuff)

---
 .../cpp/dataflow/internal/DataFlowPrivate.qll |  37 +++++-
 .../cpp/dataflow/internal/DataFlowUtil.qll    | 110 +++++++++++++++++-
 2 files changed, 140 insertions(+), 7 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
index 5de8fffca5be..a63ea897a13a 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
@@ -4,7 +4,8 @@ private import DataFlowDispatch
 
 /** Gets the instance argument of a non-static call. */
 private Node getInstanceArgument(Call call) {
-  result.asExpr() = call.getQualifier()
+  result.asExpr() = call.getQualifier() or
+  result.(ImplicitInstancePostCall).getCall() = call
   // This does not include the implicit `this` argument on auto-generated
   // base class destructor calls as those do not have an AST element.
 }
@@ -161,13 +162,38 @@ private class ArrayContent extends Content, TArrayContent {
   override Type getType() { none() }
 }
 
+/**
+ * Gets the dataflow node corresponding to the field qualifier of `fa`.
+ *
+ * Note that this might be an implicit access to the `this` pointer.
+ */
+Node getFieldQualifier(FieldAccess fa) {
+  not fa.getTarget().isStatic() and
+  (
+    result.asExpr() = fa.getQualifier() or
+    result
+        .(ImplicitInstanceAccess)
+        .getInstanceAccess()
+        .(ImplicitThisForFieldAccess)
+        .getBackingExpr() = fa
+  )
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` via an assignment to `f`.
  * Thus, `node2` references an object with a field `f` that contains the
  * value of `node1`.
  */
 predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
-  none() // stub implementation
+  exists(FieldAccess fa |
+    exists(Assignment a |
+      (a.getRValue() = node1.asExpr() or node1.asExpr() = a) and
+      a.getLValue() = fa
+    ) and
+    not fa.getTarget().isStatic() and
+    node2.getPreUpdateNode() = getFieldQualifier(fa) and
+    f.(FieldContent).getField() = fa.getTarget()
+  )
 }
 
 /**
@@ -176,7 +202,12 @@ predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
  * `node2`.
  */
 predicate readStep(Node node1, Content f, Node node2) {
-  none() // stub implementation
+  exists(FieldAccess fr |
+    node1 = getFieldQualifier(fr) and
+    fr.getTarget() = f.(FieldContent).getField() and
+    fr = node2.asExpr() and
+    not fr = any(AssignExpr a).getLValue()
+  )
 }
 
 /**
diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index 56f8727d213b..92791eab2e6b 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -6,10 +6,56 @@ private import cpp
 private import semmle.code.cpp.dataflow.internal.FlowVar
 private import semmle.code.cpp.models.interfaces.DataFlow
 
+private newtype TInstanceAccess =
+  TExplicitThisAccess(ThisExpr e) or
+  TImplicitThisForFieldAccess(FieldAccess fa) {
+    not exists(fa.getQualifier()) and not fa.getTarget().isStatic()
+  } or
+  TImplicitThisForCall(FunctionCall fc) {
+    fc.getTarget() instanceof MemberFunction and
+    not exists(fc.getQualifier()) and
+    not fc.getTarget().isStatic()
+  }
+
+private class InstanceAccess extends TInstanceAccess {
+  abstract Expr getBackingExpr();
+
+  Location getLocation() { result = getBackingExpr().getLocation() }
+
+  abstract string toString();
+}
+
+class ExplicitThisAccess extends InstanceAccess, TExplicitThisAccess {
+  override Expr getBackingExpr() { this = TExplicitThisAccess(result) }
+
+  override string toString() { result = getBackingExpr().toString() }
+}
+
+class ImplicitThisForFieldAccess extends InstanceAccess, TImplicitThisForFieldAccess {
+  override FieldAccess getBackingExpr() { this = TImplicitThisForFieldAccess(result) }
+
+  override string toString() { result = "<implicit this> for field access" }
+}
+
+class ImplicitThisForCall extends InstanceAccess, TImplicitThisForCall {
+  override FunctionCall getBackingExpr() { this = TImplicitThisForCall(result) }
+
+  override string toString() { result = "<implicit this> for call" }
+}
+
 cached
 private newtype TNode =
   TExprNode(Expr e) or
   TParameterNode(Parameter p) { exists(p.getFunction().getBlock()) } or
+  TInstanceParameterNode(MemberFunction f) { exists(f.getBlock()) and not f.isStatic() } or
+  TImplicitInstanceAccessNode(InstanceAccess ia) { not ia instanceof ExplicitThisAccess } or
+  TImplicitInstancePostCallNode(InstanceAccess a) { a instanceof ImplicitThisForCall } or
+  TExplicitArgumentPostCallNode(Expr e) {
+    e = any(Call c).getAnArgument() or
+    e = any(Call c).getQualifier()
+  } or
+  TImplicitInstancePostStoreNode(InstanceAccess a) { a instanceof ImplicitThisForFieldAccess } or
+  TExplicitInstancePostStoreNode(Expr e) { e = any(FieldAccess f).getQualifier() } or
   TDefinitionByReferenceNode(VariableAccess va, Expr argument) {
     definitionByReference(va, argument)
   } or
@@ -108,6 +154,24 @@ class ParameterNode extends Node, TParameterNode {
   predicate isParameterOf(Function f, int i) { f.getParameter(i) = param }
 }
 
+/**
+ * The value of the implicit instance parameter (in other words, the `this`
+ * pointer) at function entry, viewed as a node in a data flow graph.
+ */
+class InstanceParameterNode extends Node, TInstanceParameterNode {
+  MemberFunction f;
+
+  InstanceParameterNode() { this = TInstanceParameterNode(f) }
+
+  override Function getFunction() { result = f }
+
+  override Type getType() { result.(PointerType).getBaseType() = f.getDeclaringType() }
+
+  override string toString() { result = "<this> param" }
+
+  override Location getLocation() { result = f.getLocation() }
+}
+
 /**
  * A node that represents the value of a variable after a function call that
  * may have changed the variable because it's passed by reference.
@@ -177,13 +241,51 @@ class UninitializedNode extends Node, TUninitializedNode {
  * to the value before the update with the exception of `ClassInstanceExpr`,
  * which represents the value after the constructor has run.
  */
-class PostUpdateNode extends Node {
-  PostUpdateNode() { none() } // stub implementation
-
+abstract class PostUpdateNode extends Node {
   /**
    * Gets the node before the state update.
    */
-  Node getPreUpdateNode() { none() } // stub implementation
+  abstract Node getPreUpdateNode();
+
+  override Location getLocation() { result = getPreUpdateNode().getLocation() }
+
+  override string toString() { result = getPreUpdateNode().toString() + " [post update]" }
+}
+
+class ImplicitInstanceAccess extends Node, TImplicitInstanceAccessNode {
+  InstanceAccess ia;
+
+  ImplicitInstanceAccess() { this = TImplicitInstanceAccessNode(ia) }
+
+  override string toString() { result = ia.toString() }
+
+  override Location getLocation() { result = ia.getLocation() }
+
+  InstanceAccess getInstanceAccess() { result = ia }
+}
+
+class ImplicitInstancePostCall extends PostUpdateNode, TImplicitInstancePostCallNode {
+  ImplicitThisForCall ia;
+
+  ImplicitInstancePostCall() { this = TImplicitInstancePostCallNode(ia) }
+
+  override Node getPreUpdateNode() { ia = result.(ImplicitInstanceAccess).getInstanceAccess() }
+
+  FunctionCall getCall() { result = ia.getBackingExpr() }
+}
+
+class ExplicitArgPostCall extends PostUpdateNode, TExplicitArgumentPostCallNode {
+  override Node getPreUpdateNode() { this = TExplicitArgumentPostCallNode(result.asExpr()) }
+}
+
+class ImplicitStoreTarget extends PostUpdateNode, TImplicitInstancePostStoreNode {
+  override Node getPreUpdateNode() {
+    this = TImplicitInstancePostStoreNode(result.(ImplicitInstanceAccess).getInstanceAccess())
+  }
+}
+
+class ExplicitStoreTarget extends PostUpdateNode, TExplicitInstancePostStoreNode {
+  override Node getPreUpdateNode() { this = TExplicitInstancePostStoreNode(result.asExpr()) }
 }
 
 /**

From 15b56d93bdd8465c7751213a411c668a8a682c51 Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Mon, 8 Jul 2019 12:19:44 +0100
Subject: [PATCH 02/28] No implicit instances for constructor call

---
 cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index 92791eab2e6b..586dfb2fc8e5 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -14,7 +14,8 @@ private newtype TInstanceAccess =
   TImplicitThisForCall(FunctionCall fc) {
     fc.getTarget() instanceof MemberFunction and
     not exists(fc.getQualifier()) and
-    not fc.getTarget().isStatic()
+    not fc.getTarget().isStatic() and
+    not fc.getTarget() instanceof Constructor
   }
 
 private class InstanceAccess extends TInstanceAccess {

From 623652247d38a8625606caeac32d302714e23eda Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Tue, 9 Jul 2019 19:01:10 +0100
Subject: [PATCH 03/28] Introduce partial-definition nodes

---
 .../cpp/dataflow/internal/DataFlowPrivate.qll |  24 +-
 .../cpp/dataflow/internal/DataFlowUtil.qll    | 178 ++++++-------
 .../code/cpp/dataflow/internal/FlowVar.qll    | 247 +++++++++++++-----
 3 files changed, 270 insertions(+), 179 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
index a63ea897a13a..9e301791a7f8 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
@@ -4,8 +4,7 @@ private import DataFlowDispatch
 
 /** Gets the instance argument of a non-static call. */
 private Node getInstanceArgument(Call call) {
-  result.asExpr() = call.getQualifier() or
-  result.(ImplicitInstancePostCall).getCall() = call
+  result.asExpr() = call.getQualifier()
   // This does not include the implicit `this` argument on auto-generated
   // base class destructor calls as those do not have an AST element.
 }
@@ -162,23 +161,6 @@ private class ArrayContent extends Content, TArrayContent {
   override Type getType() { none() }
 }
 
-/**
- * Gets the dataflow node corresponding to the field qualifier of `fa`.
- *
- * Note that this might be an implicit access to the `this` pointer.
- */
-Node getFieldQualifier(FieldAccess fa) {
-  not fa.getTarget().isStatic() and
-  (
-    result.asExpr() = fa.getQualifier() or
-    result
-        .(ImplicitInstanceAccess)
-        .getInstanceAccess()
-        .(ImplicitThisForFieldAccess)
-        .getBackingExpr() = fa
-  )
-}
-
 /**
  * Holds if data can flow from `node1` to `node2` via an assignment to `f`.
  * Thus, `node2` references an object with a field `f` that contains the
@@ -191,7 +173,7 @@ predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
       a.getLValue() = fa
     ) and
     not fa.getTarget().isStatic() and
-    node2.getPreUpdateNode() = getFieldQualifier(fa) and
+    node2.getPreUpdateNode().asExpr() = fa.getQualifier() and
     f.(FieldContent).getField() = fa.getTarget()
   )
 }
@@ -203,7 +185,7 @@ predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
  */
 predicate readStep(Node node1, Content f, Node node2) {
   exists(FieldAccess fr |
-    node1 = getFieldQualifier(fr) and
+    node1.asExpr() = fr.getQualifier() and
     fr.getTarget() = f.(FieldContent).getField() and
     fr = node2.asExpr() and
     not fr = any(AssignExpr a).getLValue()
diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index 586dfb2fc8e5..c7f1ccfef4a8 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -6,57 +6,12 @@ private import cpp
 private import semmle.code.cpp.dataflow.internal.FlowVar
 private import semmle.code.cpp.models.interfaces.DataFlow
 
-private newtype TInstanceAccess =
-  TExplicitThisAccess(ThisExpr e) or
-  TImplicitThisForFieldAccess(FieldAccess fa) {
-    not exists(fa.getQualifier()) and not fa.getTarget().isStatic()
-  } or
-  TImplicitThisForCall(FunctionCall fc) {
-    fc.getTarget() instanceof MemberFunction and
-    not exists(fc.getQualifier()) and
-    not fc.getTarget().isStatic() and
-    not fc.getTarget() instanceof Constructor
-  }
-
-private class InstanceAccess extends TInstanceAccess {
-  abstract Expr getBackingExpr();
-
-  Location getLocation() { result = getBackingExpr().getLocation() }
-
-  abstract string toString();
-}
-
-class ExplicitThisAccess extends InstanceAccess, TExplicitThisAccess {
-  override Expr getBackingExpr() { this = TExplicitThisAccess(result) }
-
-  override string toString() { result = getBackingExpr().toString() }
-}
-
-class ImplicitThisForFieldAccess extends InstanceAccess, TImplicitThisForFieldAccess {
-  override FieldAccess getBackingExpr() { this = TImplicitThisForFieldAccess(result) }
-
-  override string toString() { result = "<implicit this> for field access" }
-}
-
-class ImplicitThisForCall extends InstanceAccess, TImplicitThisForCall {
-  override FunctionCall getBackingExpr() { this = TImplicitThisForCall(result) }
-
-  override string toString() { result = "<implicit this> for call" }
-}
-
 cached
 private newtype TNode =
   TExprNode(Expr e) or
-  TParameterNode(Parameter p) { exists(p.getFunction().getBlock()) } or
+  TPartialDefNode(PartialDefinition pd) or
+  TExplicitParameterNode(Parameter p) { exists(p.getFunction().getBlock()) } or
   TInstanceParameterNode(MemberFunction f) { exists(f.getBlock()) and not f.isStatic() } or
-  TImplicitInstanceAccessNode(InstanceAccess ia) { not ia instanceof ExplicitThisAccess } or
-  TImplicitInstancePostCallNode(InstanceAccess a) { a instanceof ImplicitThisForCall } or
-  TExplicitArgumentPostCallNode(Expr e) {
-    e = any(Call c).getAnArgument() or
-    e = any(Call c).getQualifier()
-  } or
-  TImplicitInstancePostStoreNode(InstanceAccess a) { a instanceof ImplicitThisForFieldAccess } or
-  TExplicitInstancePostStoreNode(Expr e) { e = any(FieldAccess f).getQualifier() } or
   TDefinitionByReferenceNode(VariableAccess va, Expr argument) {
     definitionByReference(va, argument)
   } or
@@ -85,11 +40,23 @@ class Node extends TNode {
   Expr asExpr() { result = this.(ExprNode).getExpr() }
 
   /** Gets the parameter corresponding to this node, if any. */
-  Parameter asParameter() { result = this.(ParameterNode).getParameter() }
+  Parameter asParameter() { result = this.(ExplicitParameterNode).getParameter() }
 
   /** Gets the argument that defines this `DefinitionByReferenceNode`, if any. */
   Expr asDefiningArgument() { result = this.(DefinitionByReferenceNode).getArgument() }
 
+  /**
+   * Gets the expression that is partially defined by this node, if any.
+   *
+   * Partial definitions are created for field stores (`x.y = taint();` is a partial
+   * definition of `x`), and for calls that may change the value of an object (so
+   * `x.set(taint())` is a partial definition of `x`, annd `transfer(&x, taint())` is
+   * a partial definition of `&x`).s
+   */
+  Expr asPartialDefinition() {
+    result = this.(PartialDefNode).getPartialDefinition().getDefinedExpr()
+  }
+
   /**
    * Gets the uninitialized local variable corresponding to this node, if
    * any.
@@ -128,14 +95,22 @@ class ExprNode extends Node, TExprNode {
   Expr getExpr() { result = expr }
 }
 
+abstract class ParameterNode extends Node, TNode {
+  /**
+   * Holds if this node is the parameter of `c` at the specified (zero-based)
+   * position. The implicit `this` parameter is considered to have index `-1`.
+   */
+  abstract predicate isParameterOf(Function f, int i);
+}
+
 /**
  * The value of a parameter at function entry, viewed as a node in a data
  * flow graph.
  */
-class ParameterNode extends Node, TParameterNode {
+class ExplicitParameterNode extends ParameterNode, TExplicitParameterNode {
   Parameter param;
 
-  ParameterNode() { this = TParameterNode(param) }
+  ExplicitParameterNode() { this = TExplicitParameterNode(param) }
 
   override Function getFunction() { result = param.getFunction() }
 
@@ -148,29 +123,23 @@ class ParameterNode extends Node, TParameterNode {
   /** Gets the parameter corresponding to this node. */
   Parameter getParameter() { result = param }
 
-  /**
-   * Holds if this node is the parameter of `c` at the specified (zero-based)
-   * position. The implicit `this` parameter is considered to have index `-1`.
-   */
-  predicate isParameterOf(Function f, int i) { f.getParameter(i) = param }
+  override predicate isParameterOf(Function f, int i) { f.getParameter(i) = param }
 }
 
-/**
- * The value of the implicit instance parameter (in other words, the `this`
- * pointer) at function entry, viewed as a node in a data flow graph.
- */
-class InstanceParameterNode extends Node, TInstanceParameterNode {
+class ImplicitParameterNode extends ParameterNode, TInstanceParameterNode {
   MemberFunction f;
 
-  InstanceParameterNode() { this = TInstanceParameterNode(f) }
+  ImplicitParameterNode() { this = TInstanceParameterNode(f) }
 
   override Function getFunction() { result = f }
 
-  override Type getType() { result.(PointerType).getBaseType() = f.getDeclaringType() }
+  override Type getType() { result = f.getDeclaringType() }
 
-  override string toString() { result = "<this> param" }
+  override string toString() { result = "`this` parameter in " + f.getName() }
 
   override Location getLocation() { result = f.getLocation() }
+
+  override predicate isParameterOf(Function fun, int i) { f = fun and i = -1 }
 }
 
 /**
@@ -253,40 +222,18 @@ abstract class PostUpdateNode extends Node {
   override string toString() { result = getPreUpdateNode().toString() + " [post update]" }
 }
 
-class ImplicitInstanceAccess extends Node, TImplicitInstanceAccessNode {
-  InstanceAccess ia;
-
-  ImplicitInstanceAccess() { this = TImplicitInstanceAccessNode(ia) }
-
-  override string toString() { result = ia.toString() }
-
-  override Location getLocation() { result = ia.getLocation() }
-
-  InstanceAccess getInstanceAccess() { result = ia }
-}
-
-class ImplicitInstancePostCall extends PostUpdateNode, TImplicitInstancePostCallNode {
-  ImplicitThisForCall ia;
+class PartialDefNode extends PostUpdateNode, TPartialDefNode {
+  PartialDefinition pd;
 
-  ImplicitInstancePostCall() { this = TImplicitInstancePostCallNode(ia) }
+  PartialDefNode() { this = TPartialDefNode(pd) }
 
-  override Node getPreUpdateNode() { ia = result.(ImplicitInstanceAccess).getInstanceAccess() }
+  override Node getPreUpdateNode() { result.asExpr() = pd.getDefinedExpr() }
 
-  FunctionCall getCall() { result = ia.getBackingExpr() }
-}
-
-class ExplicitArgPostCall extends PostUpdateNode, TExplicitArgumentPostCallNode {
-  override Node getPreUpdateNode() { this = TExplicitArgumentPostCallNode(result.asExpr()) }
-}
+  override string toString() { result = pd.toString() }
 
-class ImplicitStoreTarget extends PostUpdateNode, TImplicitInstancePostStoreNode {
-  override Node getPreUpdateNode() {
-    this = TImplicitInstancePostStoreNode(result.(ImplicitInstanceAccess).getInstanceAccess())
-  }
-}
+  override Location getLocation() { result = pd.getLocation() }
 
-class ExplicitStoreTarget extends PostUpdateNode, TExplicitInstancePostStoreNode {
-  override Node getPreUpdateNode() { this = TExplicitInstancePostStoreNode(result.asExpr()) }
+  PartialDefinition getPartialDefinition() { result = pd }
 }
 
 /**
@@ -297,7 +244,7 @@ ExprNode exprNode(Expr e) { result.getExpr() = e }
 /**
  * Gets the `Node` corresponding to the value of `p` at function entry.
  */
-ParameterNode parameterNode(Parameter p) { result.getParameter() = p }
+ParameterNode parameterNode(Parameter p) { result.(ExplicitParameterNode).getParameter() = p }
 
 /**
  * Gets the `Node` corresponding to a definition by reference of the variable
@@ -313,6 +260,43 @@ DefinitionByReferenceNode definitionByReferenceNodeFromArgument(Expr argument) {
  */
 UninitializedNode uninitializedNode(LocalVariable v) { result.getLocalVariable() = v }
 
+private module ThisFlow {
+  private Node thisAccessNode(ControlFlowNode cfn) {
+    result.(ImplicitParameterNode).getFunction().getBlock() = cfn or
+    result.asExpr().(ThisExpr) = cfn
+  }
+
+  private int basicBlockThisIndex(BasicBlock b, Node thisNode) {
+    thisNode = thisAccessNode(b.getNode(result))
+  }
+
+  private int thisRank(BasicBlock b, Node thisNode) {
+    thisNode = rank[result](thisAccessNode(_) as node order by basicBlockThisIndex(b, node))
+  }
+
+  private int lastThisRank(BasicBlock b) { result = max(thisRank(b, _)) }
+
+  private predicate thisAccessBlockReaches(BasicBlock b1, BasicBlock b2) {
+    exists(basicBlockThisIndex(b1, _)) and b2 = b1.getASuccessor()
+    or
+    exists(BasicBlock mid |
+      thisAccessBlockReaches(b1, mid) and
+      b2 = mid.getASuccessor() and
+      not exists(basicBlockThisIndex(mid, _))
+    )
+  }
+
+  predicate adjacentThisRefs(Node n1, Node n2) {
+    exists(BasicBlock b | thisRank(b, n1) + 1 = thisRank(b, n2))
+    or
+    exists(BasicBlock b1, BasicBlock b2 |
+      lastThisRank(b1) = thisRank(b1, n1) and
+      thisAccessBlockReaches(b1, b2) and
+      thisRank(b2, n2) = 1
+    )
+  }
+}
+
 /**
  * Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local
  * (intra-procedural) step.
@@ -332,12 +316,20 @@ predicate localFlowStep(Node nodeFrom, Node nodeTo) {
       varSourceBaseCase(var, nodeFrom.asUninitialized())
       or
       var.definedByReference(nodeFrom.asDefiningArgument())
+      or
+      var.definedPartiallyAt(nodeFrom.asPartialDefinition())
     ) and
     varToExprStep(var, nodeTo.asExpr())
   )
   or
   // Expr -> DefinitionByReferenceNode
   exprToDefinitionByReferenceStep(nodeFrom.asExpr(), nodeTo.asDefiningArgument())
+  or
+  // `this` -> adjacent-`this`
+  ThisFlow::adjacentThisRefs(nodeFrom, nodeTo)
+  or
+  // post-update-`this` -> following-`this`-ref
+  ThisFlow::adjacentThisRefs(nodeFrom.(PostUpdateNode).getPreUpdateNode(), nodeTo)
 }
 
 /**
diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
index c5b1fcdba52f..ce4850629ecc 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
@@ -1,6 +1,7 @@
 /**
  * Provides a class for handling variables in the data flow analysis.
  */
+
 import cpp
 private import semmle.code.cpp.controlflow.SSA
 private import semmle.code.cpp.dataflow.internal.SubBasicBlocks
@@ -18,7 +19,8 @@ private import semmle.code.cpp.dataflow.internal.SubBasicBlocks
  * member predicates explains how a `FlowVar` relates to syntactic constructs of
  * the language.
  */
-cached class FlowVar extends TFlowVar {
+cached
+class FlowVar extends TFlowVar {
   /**
    * Gets a `VariableAccess` that _may_ take its value from `this`. Consider
    * the following snippet.
@@ -35,7 +37,8 @@ cached class FlowVar extends TFlowVar {
    * corresponding to `x = 1`. The `x` in `x = 1` is not considered to be an
    * access.
    */
-  cached abstract VariableAccess getAnAccess();
+  cached
+  abstract VariableAccess getAnAccess();
 
   /**
    * Holds if this `FlowVar` corresponds to a modification occurring when `node` is
@@ -49,13 +52,18 @@ cached class FlowVar extends TFlowVar {
    *   `node instanceof PostCrementOperation` is an exception to the rule that
    *   `this` contains the value of `e` after the evaluation of `node`.
    */
-  cached abstract predicate definedByExpr(Expr e, ControlFlowNode node);
+  cached
+  abstract predicate definedByExpr(Expr e, ControlFlowNode node);
 
   /**
    * Holds if this `FlowVar` corresponds to the data written by a call that
    * passes a variable as argument `arg`.
    */
-  cached abstract predicate definedByReference(Expr arg);
+  cached
+  abstract predicate definedByReference(Expr arg);
+
+  cached
+  abstract predicate definedPartiallyAt(Expr e);
 
   /**
    * Holds if this `FlowVar` corresponds to the initial value of `v`. The following
@@ -68,15 +76,76 @@ cached class FlowVar extends TFlowVar {
    *   local variable is always overwritten before it is used, there is no
    *   `FlowVar` instance for the uninitialized value of that variable.
    */
-  cached abstract predicate definedByInitialValue(LocalScopeVariable v);
+  cached
+  abstract predicate definedByInitialValue(LocalScopeVariable v);
 
   /** Gets a textual representation of this element. */
-  cached abstract string toString();
+  cached
+  abstract string toString();
 
   /** Gets the location of this element. */
-  cached abstract Location getLocation();
+  cached
+  abstract Location getLocation();
 }
 
+/**
+ * Provides classes and predicates dealing with "partial definitions".
+ *
+ * In contrast to a normal "definition", which provides a new value for
+ * something, a partial definition is an expression that may affect a
+ * value, but does not necessarily replace it entirely. For example,
+ * `x.y = 1;` is a partial definition of the object `x`.
+ */
+private module PartialDefinitions {
+  private newtype TPartialDefinition =
+    TExplicitFieldStoreQualifier(Expr qualifier, FieldAccess fa, Expr assignment, Expr assigned) {
+      isInstanceFieldWrite(fa, assignment, assigned) and qualifier = fa.getQualifier()
+    } or
+    TExplicitCallQualifier(Expr qualifier, Call call) { qualifier = call.getQualifier() } or
+    TReferenceArgument(Expr arg, Call call, int i) { isReferenceOrPointerArg(arg, call, i) }
+
+  private predicate isInstanceFieldWrite(FieldAccess fa, Expr assignment, Expr assigned) {
+    not fa.getTarget().isStatic() and
+    assignmentLikeOperation(assignment, fa.getTarget(), fa, assigned)
+  }
+
+  private predicate isReferenceOrPointerArg(Expr arg, Call call, int i) {
+    arg = call.getArgument(i) and
+    exists(Type t | t = arg.getFullyConverted().getType().getUnspecifiedType() |
+      t instanceof ReferenceType or t instanceof PointerType
+    )
+  }
+
+  class PartialDefinition extends TPartialDefinition {
+    Expr definedExpr;
+
+    PartialDefinition() {
+      this = TExplicitFieldStoreQualifier(definedExpr, _, _, _) or
+      this = TExplicitCallQualifier(definedExpr, _) or
+      this = TReferenceArgument(definedExpr, _, _)
+    }
+
+    predicate partiallyDefines(Variable v) { definedExpr = v.getAnAccess() }
+
+    predicate partiallyDefinesThis(ThisExpr e) { definedExpr = e }
+
+    ControlFlowNode getSubBasicBlockStart() { result = definedExpr }
+
+    Expr getDefinedExpr() { result = definedExpr }
+
+    Location getLocation() {
+      not exists(definedExpr.getLocation()) and result = definedExpr.getParent().getLocation()
+      or
+      definedExpr.getLocation() instanceof UnknownLocation and
+      result = definedExpr.getParent().getLocation()
+      or
+      result = definedExpr.getLocation() and not result instanceof UnknownLocation
+    }
+
+    string toString() { result = "partial def of " + definedExpr }
+  }
+}
+import PartialDefinitions
 private import FlowVar_internal
 
 /**
@@ -112,9 +181,7 @@ module FlowVar_internal {
     // always executes at least once, we give it special treatment in
     // `BlockVar`, somewhat analogous to unrolling the first iteration of the
     // loop.
-    not exists(AlwaysTrueUponEntryLoop loop |
-      loop.alwaysAssignsBeforeLeavingCondition(_, _, v)
-    ) and
+    not exists(AlwaysTrueUponEntryLoop loop | loop.alwaysAssignsBeforeLeavingCondition(_, _, v)) and
     // The SSA library has a theoretically accurate treatment of reference types,
     // treating them as immutable, but for data flow it gives better results in
     // practice to make the variable synonymous with its contents.
@@ -125,9 +192,7 @@ module FlowVar_internal {
    * Holds if `sbb` is the `SubBasicBlock` where `v` receives its initial value.
    * See the documentation for `FlowVar.definedByInitialValue`.
    */
-  predicate blockVarDefinedByVariable(
-      SubBasicBlock sbb, LocalScopeVariable v)
-  {
+  predicate blockVarDefinedByVariable(SubBasicBlock sbb, LocalScopeVariable v) {
     sbb = v.(Parameter).getFunction().getEntryPoint()
     or
     exists(DeclStmt declStmt |
@@ -141,20 +206,27 @@ module FlowVar_internal {
     TSsaVar(SsaDefinition def, LocalScopeVariable v) {
       fullySupportedSsaVariable(v) and
       v = def.getAVariable()
-    }
-    or
+    } or
     TBlockVar(SubBasicBlock sbb, Variable v) {
       not fullySupportedSsaVariable(v) and
       reachable(sbb) and
       (
         initializer(sbb.getANode(), v, _)
         or
-        assignmentLikeOperation(sbb, v, _)
+        assignmentLikeOperation(sbb, v, _, _)
         or
         blockVarDefinedByReference(sbb, v, _)
         or
+        sbb = any(PartialDefinitions::PartialDefinition p | p.partiallyDefines(v))
+              .getSubBasicBlockStart()
+        or
         blockVarDefinedByVariable(sbb, v)
       )
+    } or
+    TThisVar(SubBasicBlock sbb, ThisExpr t) {
+      reachable(sbb) and
+      sbb = any(PartialDefinitions::PartialDefinition p | p.partiallyDefinesThis(t))
+            .getSubBasicBlockStart()
     }
 
   /**
@@ -162,6 +234,7 @@ module FlowVar_internal {
    */
   class SsaVar extends TSsaVar, FlowVar {
     SsaDefinition def;
+
     LocalScopeVariable v;
 
     SsaVar() { this = TSsaVar(def, v) }
@@ -171,7 +244,8 @@ module FlowVar_internal {
       // the data flow library will never see those, and the `FlowVar` library
       // is only meant to be used by the data flow library.
       this.isNonPhi() and
-      ( // This base case could be included in the transitive case by changing
+      (
+        // This base case could be included in the transitive case by changing
         // `+` to `*`, but that's slower because it goes through the `TSsaVar`
         // indirection.
         result = def.getAUse(v)
@@ -185,15 +259,19 @@ module FlowVar_internal {
 
     override predicate definedByExpr(Expr e, ControlFlowNode node) {
       e = def.getDefiningValue(v) and
-      (if def.getDefinition() = v.getInitializer().getExpr()
-       then node = v.getInitializer()
-       else node = def.getDefinition())
+      (
+        if def.getDefinition() = v.getInitializer().getExpr()
+        then node = v.getInitializer()
+        else node = def.getDefinition()
+      )
     }
 
     override predicate definedByReference(Expr arg) {
       none() // Not supported for SSA. See `fullySupportedSsaVariable`.
     }
 
+    override predicate definedPartiallyAt(Expr e) { none() }
+
     override predicate definedByInitialValue(LocalScopeVariable param) {
       def.definedByParameter(param) and
       param = v
@@ -236,6 +314,7 @@ module FlowVar_internal {
    */
   class BlockVar extends TBlockVar, FlowVar {
     SubBasicBlock sbb;
+
     Variable v;
 
     BlockVar() { this = TBlockVar(sbb, v) }
@@ -250,46 +329,84 @@ module FlowVar_internal {
     }
 
     override predicate definedByExpr(Expr e, ControlFlowNode node) {
-      assignmentLikeOperation(node, v, e) and
+      assignmentLikeOperation(node, v, _, e) and
       node = sbb
       or
       initializer(node, v, e) and
       node = sbb.getANode()
     }
 
-    override predicate definedByReference(Expr arg) {
-      blockVarDefinedByReference(sbb, v, arg)
+    override predicate definedByReference(Expr arg) { blockVarDefinedByReference(sbb, v, arg) }
+
+    override predicate definedPartiallyAt(Expr e) {
+      exists(PartialDefinitions::PartialDefinition p |
+        p.partiallyDefines(v) and
+        sbb = p.getSubBasicBlockStart() and
+        e = p.getDefinedExpr()
+      )
     }
 
     override string toString() {
       exists(Expr e |
         this.definedByExpr(e, _) and
-        result = "assignment to "+ v
+        result = "assignment to " + v
       )
       or
       this.definedByInitialValue(_) and
-      result = "initial value of "+ v
+      result = "initial value of " + v
       or
       exists(Expr arg |
         this.definedByReference(arg) and
-        result = "definition by reference of "+ v
+        result = "definition by reference of " + v
+      )
+      or
+      exists(Expr partialDef |
+        this.definedPartiallyAt(partialDef) and
+        result = "partial definition at " + partialDef
       )
       or
       // impossible case
       not this.definedByExpr(_, _) and
       not this.definedByInitialValue(_) and
       not this.definedByReference(_) and
-      result = "undefined "+ v
+      not this.definedPartiallyAt(_) and
+      result = "undefined " + v
     }
 
     override Location getLocation() { result = sbb.getStart().getLocation() }
   }
 
-  /** Type-specialized version of `getEnclosingElement`. */
-  private ControlFlowNode getCFNParent(ControlFlowNode node) {
-    result = node.getEnclosingElement()
+  class ThisVar extends TThisVar, FlowVar {
+    SubBasicBlock sbb;
+
+    ThisExpr t;
+
+    PartialDefinitions::PartialDefinition pd;
+
+    ThisVar() { this = TThisVar(sbb, t) and pd.partiallyDefinesThis(t) }
+
+    override VariableAccess getAnAccess() {
+      none() // TODO: Widen type to `Expr`.
+    }
+
+    override predicate definedByExpr(Expr e, ControlFlowNode node) { none() }
+
+    override predicate definedByReference(Expr arg) { none() }
+
+    override predicate definedPartiallyAt(Expr arg) {
+      none() // TODO
+    }
+
+    override predicate definedByInitialValue(LocalScopeVariable param) { none() }
+
+    override string toString() { result = pd.toString() }
+
+    override Location getLocation() { result = pd.getLocation() }
   }
 
+  /** Type-specialized version of `getEnclosingElement`. */
+  private ControlFlowNode getCFNParent(ControlFlowNode node) { result = node.getEnclosingElement() }
+
   /**
    * A for-loop or while-loop whose condition is always true upon entry but not
    * always true after the first iteration.
@@ -334,7 +451,7 @@ module FlowVar_internal {
     pragma[noinline]
     private Variable getAVariableAssignedInLoop() {
       exists(BasicBlock bbAssign |
-        assignmentLikeOperation(bbAssign.getANode(), result, _) and
+        assignmentLikeOperation(bbAssign.getANode(), result, _, _) and
         this.bbInLoop(bbAssign)
       )
     }
@@ -368,7 +485,7 @@ module FlowVar_internal {
         reachesWithoutAssignment(bb.getAPredecessor(), v) and
         this.bbInLoop(bb)
       ) and
-      not assignmentLikeOperation(bb.getANode(), v, _)
+      not assignmentLikeOperation(bb.getANode(), v, _, _)
     }
   }
 
@@ -379,11 +496,9 @@ module FlowVar_internal {
    * be used outside the loop.
    */
   predicate skipLoop(
-    SubBasicBlock sbbInside, SubBasicBlock sbbOutside,
-    SubBasicBlock sbbDef, Variable v
+    SubBasicBlock sbbInside, SubBasicBlock sbbOutside, SubBasicBlock sbbDef, Variable v
   ) {
-    exists(AlwaysTrueUponEntryLoop loop,
-           BasicBlock bbInside, BasicBlock bbOutside |
+    exists(AlwaysTrueUponEntryLoop loop, BasicBlock bbInside, BasicBlock bbOutside |
       loop.alwaysAssignsBeforeLeavingCondition(bbInside, bbOutside, v) and
       bbInside = sbbInside.getBasicBlock() and
       bbOutside = sbbOutside.getBasicBlock() and
@@ -403,7 +518,7 @@ module FlowVar_internal {
       mid = getAReachedBlockVarSBB(start) and
       result = mid.getASuccessor() and
       not skipLoop(mid, result, sbbDef, v) and
-      not assignmentLikeOperation(result, v, _)
+      not assignmentLikeOperation(result, v, _, _)
     )
   }
 
@@ -468,18 +583,18 @@ module FlowVar_internal {
     forall(VariableAccess va |
       va.getTarget() = v and
       readAccess(va)
-    | dominatedByOverwrite(v, va)
+    |
+      dominatedByOverwrite(v, va)
     )
   }
 
   /** Holds if `va` accesses `v` and is dominated by an overwrite of `v`. */
-  predicate dominatedByOverwrite(UninitializedLocalVariable v,
-                                 VariableAccess va)
-  {
+  predicate dominatedByOverwrite(UninitializedLocalVariable v, VariableAccess va) {
     exists(BasicBlock bb, int vaIndex |
       va = bb.getNode(vaIndex) and
       va.getTarget() = v
-    | vaIndex > indexOfFirstOverwriteInBB(v, bb)
+    |
+      vaIndex > indexOfFirstOverwriteInBB(v, bb)
       or
       bbStrictlyDominates(getAnOverwritingBB(v), bb)
     )
@@ -520,29 +635,32 @@ module FlowVar_internal {
    * `initializer` instead of this predicate.
    */
   predicate assignmentLikeOperation(
-      ControlFlowNode node, Variable v, Expr assignedExpr)
-  {
+    ControlFlowNode node, Variable v, VariableAccess va, Expr assignedExpr
+  ) {
     // Together, the two following cases cover `Assignment`
     node = any(AssignExpr ae |
-      v = ae.getLValue().(VariableAccess).getTarget() and
-      assignedExpr = ae.getRValue()
-    )
+        va = ae.getLValue() and
+        v = va.getTarget() and
+        assignedExpr = ae.getRValue()
+      )
     or
     node = any(AssignOperation ao |
-      v = ao.getLValue().(VariableAccess).getTarget() and
-      // Here and in the `PrefixCrementOperation` case, we say that the assigned
-      // expression is the operation itself. For example, we say that `x += 1`
-      // assigns `x += 1` to `x`. The justification is that after this operation,
-      // `x` will contain the same value that `x += 1` evaluated to.
-      assignedExpr = ao
-    )
+        va = ao.getLValue() and
+        v = va.getTarget() and
+        // Here and in the `PrefixCrementOperation` case, we say that the assigned
+        // expression is the operation itself. For example, we say that `x += 1`
+        // assigns `x += 1` to `x`. The justification is that after this operation,
+        // `x` will contain the same value that `x += 1` evaluated to.
+        assignedExpr = ao
+      )
     or
     // This case does not add further data flow paths, except if a
     // `PrefixCrementOperation` is itself a source
     node = any(CrementOperation op |
-      v = op.getOperand().(VariableAccess).getTarget() and
-      assignedExpr = op
-    )
+        va = op.getOperand() and
+        v = va.getTarget() and
+        assignedExpr = op
+      )
   }
 
   predicate blockVarDefinedByReference(ControlFlowNode node, Variable v, Expr argument) {
@@ -553,9 +671,7 @@ module FlowVar_internal {
   /**
    * Holds if `v` is initialized by `init` to have value `assignedExpr`.
    */
-  predicate initializer(
-      Initializer init, LocalVariable v, Expr assignedExpr)
-  {
+  predicate initializer(Initializer init, LocalVariable v, Expr assignedExpr) {
     v = init.getDeclaration() and
     assignedExpr = init.getExpr()
   }
@@ -568,12 +684,12 @@ module FlowVar_internal {
    */
   class DataFlowSubBasicBlockCutNode extends SubBasicBlockCutNode {
     DataFlowSubBasicBlockCutNode() {
-      exists(Variable v |
-        not fullySupportedSsaVariable(v)
-      |
-        assignmentLikeOperation(this, v, _)
+      exists(Variable v | not fullySupportedSsaVariable(v) |
+        assignmentLikeOperation(this, v, _, _)
         or
         blockVarDefinedByReference(this, v, _)
+        or
+        this = any(PartialDefinitions::PartialDefinition p).getSubBasicBlockStart()
         // It is not necessary to cut the basic blocks at `Initializer` nodes
         // because the affected variable can have no _other_ value before its
         // initializer. It is not necessary to cut basic blocks at procedure
@@ -582,4 +698,5 @@ module FlowVar_internal {
       )
     }
   }
-} /* module FlowVar_internal */
+}
+/* module FlowVar_internal */

From 66164eb06f1d4c91a2155cde52bc1fe19e434d39 Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Wed, 10 Jul 2019 11:19:28 +0100
Subject: [PATCH 04/28] Propagate data flow through `NewExpr`s

---
 cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index c7f1ccfef4a8..451a4f83c5ef 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -376,6 +376,8 @@ private predicate exprToExprStep_nocfg(Expr fromExpr, Expr toExpr) {
   or
   toExpr = any(StmtExpr stmtExpr | fromExpr = stmtExpr.getResultExpr())
   or
+  toExpr.(NewExpr).getInitializer() = fromExpr
+  or
   toExpr = any(Call call |
       exists(DataFlowFunction f, FunctionInput inModel, FunctionOutput outModel, int iIn |
         call.getTarget() = f and

From bb8eb235e0ae4f8ec3f4433d717e8a8a9b8a84b1 Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Wed, 10 Jul 2019 11:38:05 +0100
Subject: [PATCH 05/28] Handle constructor call qualifiers

---
 .../src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
index 9e301791a7f8..058d138dd6e2 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
@@ -5,6 +5,10 @@ private import DataFlowDispatch
 /** Gets the instance argument of a non-static call. */
 private Node getInstanceArgument(Call call) {
   result.asExpr() = call.getQualifier()
+  or
+  // For constructors, there is no qualifier, so we pretend the call itself
+  // is the instance argument.
+  result.asExpr() = call.(ConstructorCall)
   // This does not include the implicit `this` argument on auto-generated
   // base class destructor calls as those do not have an AST element.
 }

From 1b9a2d3d877615c6632b257812fa3e43d207d08c Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Wed, 10 Jul 2019 14:41:50 +0100
Subject: [PATCH 06/28] Reduce partial definition flow edge redundancy

---
 .../src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll  | 2 +-
 cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll       | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
index 058d138dd6e2..447b59bd326c 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
@@ -173,7 +173,7 @@ private class ArrayContent extends Content, TArrayContent {
 predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
   exists(FieldAccess fa |
     exists(Assignment a |
-      (a.getRValue() = node1.asExpr() or node1.asExpr() = a) and
+      node1.asExpr() = a and
       a.getLValue() = fa
     ) and
     not fa.getTarget().isStatic() and
diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
index ce4850629ecc..6d36ca55d383 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
@@ -320,7 +320,8 @@ module FlowVar_internal {
     BlockVar() { this = TBlockVar(sbb, v) }
 
     override VariableAccess getAnAccess() {
-      variableAccessInSBB(v, getAReachedBlockVarSBB(this), result)
+      variableAccessInSBB(v, getAReachedBlockVarSBB(this), result) and
+      result != sbb
     }
 
     override predicate definedByInitialValue(LocalScopeVariable lsv) {

From 5fbe98208478d6c2c986a7a9de17fc704dd220c8 Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Wed, 10 Jul 2019 14:42:04 +0100
Subject: [PATCH 07/28] Add missing `getType` override

---
 cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index 451a4f83c5ef..a05c3d808e3f 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -229,6 +229,8 @@ class PartialDefNode extends PostUpdateNode, TPartialDefNode {
 
   override Node getPreUpdateNode() { result.asExpr() = pd.getDefinedExpr() }
 
+  override Type getType() { result = pd.getDefinedExpr().getType() }
+
   override string toString() { result = pd.toString() }
 
   override Location getLocation() { result = pd.getLocation() }

From 46e6b587bc3dcb268e8be1a32e9ecfe6537a7d3a Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Wed, 10 Jul 2019 14:42:26 +0100
Subject: [PATCH 08/28] Exclude partial defs from ordinary SSA handling

---
 cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
index 6d36ca55d383..2a9b51288fd5 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
@@ -173,6 +173,8 @@ module FlowVar_internal {
     // the call. It's fundamental in SSA that each use is only associated with
     // one def, so we can't easily get this effect with SSA.
     not definitionByReference(v.getAnAccess(), _) and
+    // A partially-defined variable is handled using the partial definitions logic.
+    not any(PartialDefinitions::PartialDefinition p).partiallyDefines(v) and
     // SSA variables do not exist before their first assignment, but one
     // feature of this data flow library is to track where uninitialized data
     // ends up.

From abcaeded23e474d91d0c5ba497c8fe0e66895093 Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Wed, 10 Jul 2019 16:48:15 +0100
Subject: [PATCH 09/28] Only split BBs for var-defining partial-defs

---
 cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
index 2a9b51288fd5..eaec20037189 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
@@ -692,7 +692,8 @@ module FlowVar_internal {
         or
         blockVarDefinedByReference(this, v, _)
         or
-        this = any(PartialDefinitions::PartialDefinition p).getSubBasicBlockStart()
+        this = any(PartialDefinitions::PartialDefinition p | p.partiallyDefines(v))
+              .getSubBasicBlockStart()
         // It is not necessary to cut the basic blocks at `Initializer` nodes
         // because the affected variable can have no _other_ value before its
         // initializer. It is not necessary to cut basic blocks at procedure

From 9e6c240ee21bf10b942596b4f340f53175881540 Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Wed, 10 Jul 2019 18:15:25 +0100
Subject: [PATCH 10/28] Override getFunction on PostUpdateNodes

---
 .../src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll  | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index a05c3d808e3f..5ffcfde0c241 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -217,6 +217,10 @@ abstract class PostUpdateNode extends Node {
    */
   abstract Node getPreUpdateNode();
 
+  override Function getFunction() { result = getPreUpdateNode().getFunction() }
+
+  override Type getType() { result = getPreUpdateNode().getType() }
+
   override Location getLocation() { result = getPreUpdateNode().getLocation() }
 
   override string toString() { result = getPreUpdateNode().toString() + " [post update]" }
@@ -229,8 +233,6 @@ class PartialDefNode extends PostUpdateNode, TPartialDefNode {
 
   override Node getPreUpdateNode() { result.asExpr() = pd.getDefinedExpr() }
 
-  override Type getType() { result = pd.getDefinedExpr().getType() }
-
   override string toString() { result = pd.toString() }
 
   override Location getLocation() { result = pd.getLocation() }

From dccc0f4db10e20d27ecf87763a2c3b9a90048955 Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Thu, 11 Jul 2019 11:11:46 +0100
Subject: [PATCH 11/28] Add handling of post-constructor-call nodes

---
 .../code/cpp/dataflow/internal/DataFlowUtil.qll     | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index 5ffcfde0c241..ba608c3e1d1c 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -10,6 +10,7 @@ cached
 private newtype TNode =
   TExprNode(Expr e) or
   TPartialDefNode(PartialDefinition pd) or
+  TPostConstructorCallNode(ConstructorCall call) or
   TExplicitParameterNode(Parameter p) { exists(p.getFunction().getBlock()) } or
   TInstanceParameterNode(MemberFunction f) { exists(f.getBlock()) and not f.isStatic() } or
   TDefinitionByReferenceNode(VariableAccess va, Expr argument) {
@@ -233,13 +234,19 @@ class PartialDefNode extends PostUpdateNode, TPartialDefNode {
 
   override Node getPreUpdateNode() { result.asExpr() = pd.getDefinedExpr() }
 
-  override string toString() { result = pd.toString() }
-
   override Location getLocation() { result = pd.getLocation() }
 
   PartialDefinition getPartialDefinition() { result = pd }
 }
 
+class PostConstructorCallNode extends PostUpdateNode, TPostConstructorCallNode {
+  ConstructorCall call;
+
+  PostConstructorCallNode() { this = TPostConstructorCallNode(call) }
+
+  override Node getPreUpdateNode() { result.asExpr() = call }
+}
+
 /**
  * Gets the `Node` corresponding to `e`.
  */
@@ -310,6 +317,8 @@ predicate localFlowStep(Node nodeFrom, Node nodeTo) {
   // Expr -> Expr
   exprToExprStep_nocfg(nodeFrom.asExpr(), nodeTo.asExpr())
   or
+  exprToExprStep_nocfg(nodeFrom.(PostUpdateNode).getPreUpdateNode().asExpr(), nodeTo.asExpr())
+  or
   // Node -> FlowVar -> VariableAccess
   exists(FlowVar var |
     (

From 6d4d131ad477c3525b75612abf803a1f528b9a14 Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Thu, 11 Jul 2019 11:14:22 +0100
Subject: [PATCH 12/28] C++ field flow: Add test.

This is a fairly direct translation of the Java field flow test to
C++. Not all the `// flow` annotations are currently accurate.
---
 .../test/library-tests/dataflow/fields/A.cpp  | 187 ++++++++++++++++++
 .../test/library-tests/dataflow/fields/B.cpp  |  49 +++++
 .../test/library-tests/dataflow/fields/C.cpp  |  37 ++++
 .../dataflow/fields/flow.expected             | 112 +++++++++++
 .../library-tests/dataflow/fields/flow.ql     |  32 +++
 5 files changed, 417 insertions(+)
 create mode 100644 cpp/ql/test/library-tests/dataflow/fields/A.cpp
 create mode 100644 cpp/ql/test/library-tests/dataflow/fields/B.cpp
 create mode 100644 cpp/ql/test/library-tests/dataflow/fields/C.cpp
 create mode 100644 cpp/ql/test/library-tests/dataflow/fields/flow.expected
 create mode 100644 cpp/ql/test/library-tests/dataflow/fields/flow.ql

diff --git a/cpp/ql/test/library-tests/dataflow/fields/A.cpp b/cpp/ql/test/library-tests/dataflow/fields/A.cpp
new file mode 100644
index 000000000000..8490a560b083
--- /dev/null
+++ b/cpp/ql/test/library-tests/dataflow/fields/A.cpp
@@ -0,0 +1,187 @@
+class A
+{
+
+  class C
+  {
+  public:
+    virtual void f(void *) {}
+  };
+  class C1 : public C
+  {
+  public:
+    A *a;
+  };
+  class C2 : public C
+  {
+  };
+
+  class B
+  {
+  public:
+    C *c;
+    B() {}
+    B(C *c)
+    {
+      this->c = c;
+    }
+    void set(C *c) { this->c = c; }
+    C *get() { return this->c; }
+    static B *make(C *c)
+    {
+      return new B(c);
+    }
+  };
+
+public:
+  void f0()
+  {
+    C cc;
+    C ct;
+    cc.f(nullptr);
+    ct.f(new C());
+    sink(&cc); // no flow
+    sink(&ct); // flow
+  }
+  void f1()
+  {
+    C *c = new C();
+    B *b = B::make(c);
+    sink(b->c); // flow
+  }
+
+  void f2()
+  {
+    B *b = new B();
+    b->set(new C1());
+    sink(b->get());                // flow
+    sink((new B(new C()))->get()); // flow
+  }
+
+  void f3()
+  {
+    B *b1 = new B();
+    B *b2;
+    b2 = setOnB(b1, new C2());
+    sink(b1->c); // no flow
+    sink(b2->c); // flow
+  }
+
+  void f4()
+  {
+    B *b1 = new B();
+    B *b2;
+    b2 = setOnBWrap(b1, new C2());
+    sink(b1->c); // no flow
+    sink(b2->c); // flow
+  }
+
+  B *setOnBWrap(B *b1, C *c)
+  {
+    B *b2;
+    b2 = setOnB(b1, c);
+    return r() ? b1 : b2;
+  }
+
+  A::B *setOnB(B *b1, C *c)
+  {
+    if (r())
+    {
+      B *b2 = new B();
+      b2->set(c);
+      return b2;
+    }
+    return b1;
+  }
+
+  void f5()
+  {
+    A *a = new A();
+    C1 *c1 = new C1();
+    c1->a = a;
+    f6(c1);
+  }
+  void f6(C *c)
+  {
+    if (C1 *c1 = dynamic_cast<C1 *>(c))
+    {
+      sink(c1->a); // flow
+    }
+    C *cc;
+    if (C2 *c2 = dynamic_cast<C2 *>(c))
+    {
+      cc = c2;
+    }
+    else
+    {
+      cc = new C1();
+    }
+    if (C1 *c1 = dynamic_cast<C1 *>(cc))
+    {
+      sink(c1->a); // no flow, stopped by cast to C2
+    }
+  }
+
+  void f7(B *b)
+  {
+    b->set(new C());
+  }
+  void f8()
+  {
+    B *b = new B();
+    f7(b);
+    sink(b->c); // flow
+  }
+
+  class D
+  {
+  public:
+    A::B *b;
+
+    D(A::B *b, bool x)
+    {
+      b->c = new C();
+      this->b = x ? b : new B();
+    }
+  };
+
+  void
+  f9()
+  {
+    B *b = new B();
+    D *d = new D(b, r());
+    sink(d->b);    // flow x2
+    sink(d->b->c); // flow
+    sink(b->c);    // flow
+  }
+
+  void f10()
+  {
+    B *b = new B();
+    MyList *l1 = new MyList(b, new MyList(nullptr, nullptr));
+    MyList *l2 = new MyList(nullptr, l1);
+    MyList *l3 = new MyList(nullptr, l2);
+    sink(l3->head);                   // no flow, b is nested beneath at least one ->next
+    sink(l3->next->head);             // flow, the precise nesting depth isn't tracked
+    sink(l3->next->next->head);       // flow
+    sink(l3->next->next->next->head); // flow
+    for (MyList *l = l3; l != nullptr; l = l->next)
+    {
+      sink(l->head); // flow
+    }
+  }
+
+  static void sink(void *o) {}
+  bool r() { return reinterpret_cast<long long>(this) % 10 > 5; }
+
+  class MyList
+  {
+  public:
+    B *head;
+    MyList *next;
+    MyList(B *newHead, MyList *next)
+    {
+      head = newHead;
+      this->next = next;
+    }
+  };
+};
diff --git a/cpp/ql/test/library-tests/dataflow/fields/B.cpp b/cpp/ql/test/library-tests/dataflow/fields/B.cpp
new file mode 100644
index 000000000000..13581ce3e60f
--- /dev/null
+++ b/cpp/ql/test/library-tests/dataflow/fields/B.cpp
@@ -0,0 +1,49 @@
+class B
+{
+
+  void f1()
+  {
+    Elem *e = new Elem();
+    Box1 *b1 = new Box1(e, nullptr);
+    Box2 *b2 = new Box2(b1);
+    sink(b2->box1->elem1); // flow
+    sink(b2->box1->elem2); // FP due to flow in f2 below
+  }
+
+  void f2()
+  {
+    Elem *e = new B::Elem();
+    Box1 *b1 = new B::Box1(nullptr, e);
+    Box2 *b2 = new Box2(b1);
+    sink(b2->box1->elem1); // FP due to flow in f1 above
+    sink(b2->box1->elem2); // flow
+  }
+
+  static void sink(void *o) {}
+
+  class Elem
+  {
+  };
+
+  class Box1
+  {
+  public:
+    Elem *elem1;
+    Elem *elem2;
+    Box1(Elem *e1, Elem *e2)
+    {
+      this->elem1 = e1;
+      this->elem2 = e2;
+    }
+  };
+
+  class Box2
+  {
+  public:
+    Box1 *box1;
+    Box2(Box1 *b1)
+    {
+      this->box1 = b1;
+    }
+  };
+};
diff --git a/cpp/ql/test/library-tests/dataflow/fields/C.cpp b/cpp/ql/test/library-tests/dataflow/fields/C.cpp
new file mode 100644
index 000000000000..ac87f48204bb
--- /dev/null
+++ b/cpp/ql/test/library-tests/dataflow/fields/C.cpp
@@ -0,0 +1,37 @@
+
+class C
+{
+  class Elem
+  {
+  };
+
+private:
+  Elem *s1 = new Elem();
+  const Elem *s2 = new Elem();
+  Elem *s3;
+
+public:
+  const static Elem *s4;
+
+  void main(void)
+  {
+    C *c = new C();
+    c->func();
+  }
+
+  C() : s1(new Elem())
+  {
+    this->s3 = new Elem();
+  }
+
+  void func()
+  {
+    sink(s1);
+    sink(s2);
+    sink(s3);
+    sink(s4);
+  }
+
+  static void sink(const void *o) {}
+};
+const C::Elem *C::s4 = new Elem();
\ No newline at end of file
diff --git a/cpp/ql/test/library-tests/dataflow/fields/flow.expected b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
new file mode 100644
index 000000000000..f5ff8cb31132
--- /dev/null
+++ b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
@@ -0,0 +1,112 @@
+edges
+| A.cpp:41:10:41:16 | new [void] | A.cpp:43:10:43:12 | & ... |
+| A.cpp:47:12:47:18 | new [void] | A.cpp:48:20:48:20 | c [void] |
+| A.cpp:48:12:48:18 | call to make [c, ... (1)] | A.cpp:49:10:49:10 | b [c, ... (1)] |
+| A.cpp:48:20:48:20 | c [void] | A.cpp:48:12:48:18 | call to make [c, ... (1)] |
+| A.cpp:49:10:49:10 | b [c, ... (1)] | A.cpp:49:13:49:13 | c |
+| A.cpp:55:5:55:5 | b [post update] [c, ... (1)] | A.cpp:56:10:56:10 | b [c, ... (1)] |
+| A.cpp:55:12:55:19 | new [void] | A.cpp:55:5:55:5 | b [post update] [c, ... (1)] |
+| A.cpp:56:10:56:10 | b [c, ... (1)] | A.cpp:56:13:56:15 | call to get |
+| A.cpp:57:11:57:24 | call to B [post update] [c, ... (1)] | A.cpp:57:11:57:24 | new [c, ... (1)] |
+| A.cpp:57:11:57:24 | new [c, ... (1)] | A.cpp:57:28:57:30 | call to get |
+| A.cpp:57:17:57:23 | new [void] | A.cpp:57:11:57:24 | call to B [post update] [c, ... (1)] |
+| A.cpp:64:10:64:15 | call to setOnB [c, ... (1)] | A.cpp:66:10:66:11 | b2 [c, ... (1)] |
+| A.cpp:64:21:64:28 | new [void] | A.cpp:64:10:64:15 | call to setOnB [c, ... (1)] |
+| A.cpp:66:10:66:11 | b2 [c, ... (1)] | A.cpp:66:14:66:14 | c |
+| A.cpp:73:10:73:19 | call to setOnBWrap [c, ... (1)] | A.cpp:75:10:75:11 | b2 [c, ... (1)] |
+| A.cpp:73:25:73:32 | new [void] | A.cpp:73:10:73:19 | call to setOnBWrap [c, ... (1)] |
+| A.cpp:75:10:75:11 | b2 [c, ... (1)] | A.cpp:75:14:75:14 | c |
+| A.cpp:98:12:98:18 | new [void] | A.cpp:100:5:100:13 | ... = ... [void] |
+| A.cpp:100:5:100:6 | c1 [post update] [a, ... (1)] | A.cpp:101:8:101:9 | c1 [a, ... (1)] |
+| A.cpp:100:5:100:13 | ... = ... [void] | A.cpp:100:5:100:6 | c1 [post update] [a, ... (1)] |
+| A.cpp:101:8:101:9 | c1 [a, ... (1)] | A.cpp:103:14:103:14 | c [a, ... (1)] |
+| A.cpp:103:14:103:14 | c [a, ... (1)] | A.cpp:107:12:107:13 | c1 [a, ... (1)] |
+| A.cpp:103:14:103:14 | c [a, ... (1)] | A.cpp:120:12:120:13 | c1 [a, ... (1)] |
+| A.cpp:107:12:107:13 | c1 [a, ... (1)] | A.cpp:107:16:107:16 | a |
+| A.cpp:120:12:120:13 | c1 [a, ... (1)] | A.cpp:120:16:120:16 | a |
+| A.cpp:126:5:126:5 | b [post update] [c, ... (1)] | A.cpp:131:8:131:8 | b [post update] [c, ... (1)] |
+| A.cpp:126:12:126:18 | new [void] | A.cpp:126:5:126:5 | b [post update] [c, ... (1)] |
+| A.cpp:131:8:131:8 | b [post update] [c, ... (1)] | A.cpp:132:10:132:10 | b [c, ... (1)] |
+| A.cpp:132:10:132:10 | b [c, ... (1)] | A.cpp:132:13:132:13 | c |
+| A.cpp:142:7:142:7 | b [post update] [c, ... (1)] | A.cpp:143:7:143:31 | ... = ... [c, ... (1)] |
+| A.cpp:142:7:142:7 | b [post update] [c, ... (1)] | A.cpp:151:18:151:18 | b [post update] [c, ... (1)] |
+| A.cpp:142:7:142:20 | ... = ... [void] | A.cpp:142:7:142:7 | b [post update] [c, ... (1)] |
+| A.cpp:142:14:142:20 | new [void] | A.cpp:142:7:142:20 | ... = ... [void] |
+| A.cpp:143:7:143:10 | this [post update] [b, ... (1)] | A.cpp:151:12:151:24 | call to D [post update] [b, ... (1)] |
+| A.cpp:143:7:143:10 | this [post update] [b, ... (2)] | A.cpp:151:12:151:24 | call to D [post update] [b, ... (2)] |
+| A.cpp:143:7:143:31 | ... = ... [c, ... (1)] | A.cpp:143:7:143:10 | this [post update] [b, ... (2)] |
+| A.cpp:143:7:143:31 | ... = ... [void] | A.cpp:143:7:143:10 | this [post update] [b, ... (1)] |
+| A.cpp:143:25:143:31 | new [void] | A.cpp:143:7:143:31 | ... = ... [void] |
+| A.cpp:150:12:150:18 | new [void] | A.cpp:151:18:151:18 | b [void] |
+| A.cpp:151:12:151:24 | call to D [post update] [b, ... (1)] | A.cpp:152:10:152:10 | d [b, ... (1)] |
+| A.cpp:151:12:151:24 | call to D [post update] [b, ... (2)] | A.cpp:153:10:153:10 | d [b, ... (2)] |
+| A.cpp:151:18:151:18 | b [post update] [c, ... (1)] | A.cpp:154:10:154:10 | b [c, ... (1)] |
+| A.cpp:151:18:151:18 | b [void] | A.cpp:151:12:151:24 | call to D [post update] [b, ... (1)] |
+| A.cpp:152:10:152:10 | d [b, ... (1)] | A.cpp:152:13:152:13 | b |
+| A.cpp:153:10:153:10 | d [b, ... (2)] | A.cpp:153:13:153:13 | b [c, ... (1)] |
+| A.cpp:153:13:153:13 | b [c, ... (1)] | A.cpp:153:16:153:16 | c |
+| A.cpp:154:10:154:10 | b [c, ... (1)] | A.cpp:154:13:154:13 | c |
+| A.cpp:159:12:159:18 | new [void] | A.cpp:160:29:160:29 | b [void] |
+| A.cpp:160:18:160:60 | call to MyList [post update] [head, ... (1)] | A.cpp:161:38:161:39 | l1 [head, ... (1)] |
+| A.cpp:160:29:160:29 | b [void] | A.cpp:160:18:160:60 | call to MyList [post update] [head, ... (1)] |
+| A.cpp:161:18:161:40 | call to MyList [post update] [next, ... (2)] | A.cpp:162:38:162:39 | l2 [next, ... (2)] |
+| A.cpp:161:38:161:39 | l1 [head, ... (1)] | A.cpp:161:18:161:40 | call to MyList [post update] [next, ... (2)] |
+| A.cpp:162:18:162:40 | call to MyList [post update] [next, ... (3)] | A.cpp:165:10:165:11 | l3 [next, ... (3)] |
+| A.cpp:162:18:162:40 | call to MyList [post update] [next, ... (3)] | A.cpp:167:44:167:44 | l [next, ... (3)] |
+| A.cpp:162:38:162:39 | l2 [next, ... (2)] | A.cpp:162:18:162:40 | call to MyList [post update] [next, ... (3)] |
+| A.cpp:165:10:165:11 | l3 [next, ... (3)] | A.cpp:165:14:165:17 | next [next, ... (2)] |
+| A.cpp:165:14:165:17 | next [next, ... (2)] | A.cpp:165:20:165:23 | next [head, ... (1)] |
+| A.cpp:165:20:165:23 | next [head, ... (1)] | A.cpp:165:26:165:29 | head |
+| A.cpp:167:44:167:44 | l [next, ... (2)] | A.cpp:167:47:167:50 | next [head, ... (1)] |
+| A.cpp:167:44:167:44 | l [next, ... (3)] | A.cpp:167:47:167:50 | next [next, ... (2)] |
+| A.cpp:167:47:167:50 | next [head, ... (1)] | A.cpp:169:12:169:12 | l [head, ... (1)] |
+| A.cpp:167:47:167:50 | next [next, ... (2)] | A.cpp:167:44:167:44 | l [next, ... (2)] |
+| A.cpp:169:12:169:12 | l [head, ... (1)] | A.cpp:169:15:169:18 | head |
+| B.cpp:6:15:6:24 | new [void] | B.cpp:7:25:7:25 | e [void] |
+| B.cpp:7:16:7:35 | call to Box1 [post update] [elem1, ... (1)] | B.cpp:8:25:8:26 | b1 [elem1, ... (1)] |
+| B.cpp:7:25:7:25 | e [void] | B.cpp:7:16:7:35 | call to Box1 [post update] [elem1, ... (1)] |
+| B.cpp:8:16:8:27 | call to Box2 [post update] [box1, ... (2)] | B.cpp:9:10:9:11 | b2 [box1, ... (2)] |
+| B.cpp:8:16:8:27 | call to Box2 [post update] [box1, ... (2)] | B.cpp:10:10:10:11 | b2 [box1, ... (2)] |
+| B.cpp:8:25:8:26 | b1 [elem1, ... (1)] | B.cpp:8:16:8:27 | call to Box2 [post update] [box1, ... (2)] |
+| B.cpp:9:10:9:11 | b2 [box1, ... (2)] | B.cpp:9:14:9:17 | box1 [elem1, ... (1)] |
+| B.cpp:9:14:9:17 | box1 [elem1, ... (1)] | B.cpp:9:20:9:24 | elem1 |
+| B.cpp:10:10:10:11 | b2 [box1, ... (2)] | B.cpp:10:14:10:17 | box1 [elem2, ... (1)] |
+| B.cpp:10:14:10:17 | box1 [elem2, ... (1)] | B.cpp:10:20:10:24 | elem2 |
+| B.cpp:15:15:15:27 | new [void] | B.cpp:16:37:16:37 | e [void] |
+| B.cpp:16:16:16:38 | call to Box1 [post update] [elem2, ... (1)] | B.cpp:17:25:17:26 | b1 [elem2, ... (1)] |
+| B.cpp:16:37:16:37 | e [void] | B.cpp:16:16:16:38 | call to Box1 [post update] [elem2, ... (1)] |
+| B.cpp:17:16:17:27 | call to Box2 [post update] [box1, ... (2)] | B.cpp:18:10:18:11 | b2 [box1, ... (2)] |
+| B.cpp:17:16:17:27 | call to Box2 [post update] [box1, ... (2)] | B.cpp:19:10:19:11 | b2 [box1, ... (2)] |
+| B.cpp:17:25:17:26 | b1 [elem2, ... (1)] | B.cpp:17:16:17:27 | call to Box2 [post update] [box1, ... (2)] |
+| B.cpp:18:10:18:11 | b2 [box1, ... (2)] | B.cpp:18:14:18:17 | box1 [elem1, ... (1)] |
+| B.cpp:18:14:18:17 | box1 [elem1, ... (1)] | B.cpp:18:20:18:24 | elem1 |
+| B.cpp:19:10:19:11 | b2 [box1, ... (2)] | B.cpp:19:14:19:17 | box1 [elem2, ... (1)] |
+| B.cpp:19:14:19:17 | box1 [elem2, ... (1)] | B.cpp:19:20:19:24 | elem2 |
+| C.cpp:18:12:18:18 | call to C [post update] [s3, ... (1)] | C.cpp:19:5:19:5 | c [s3, ... (1)] |
+| C.cpp:19:5:19:5 | c [s3, ... (1)] | C.cpp:27:8:27:11 | `this` parameter in func [s3, ... (1)] |
+| C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] | C.cpp:18:12:18:18 | call to C [post update] [s3, ... (1)] |
+| C.cpp:24:5:24:25 | ... = ... [void] | C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] |
+| C.cpp:24:16:24:25 | new [void] | C.cpp:24:5:24:25 | ... = ... [void] |
+| C.cpp:27:8:27:11 | `this` parameter in func [s3, ... (1)] | file://:0:0:0:0 | this [s3, ... (1)] |
+| file://:0:0:0:0 | this [s3, ... (1)] | C.cpp:31:10:31:11 | s3 |
+#select
+| A.cpp:43:10:43:12 | & ... | A.cpp:41:10:41:16 | new [void] | A.cpp:43:10:43:12 | & ... | & ... flows from $@ | A.cpp:41:10:41:16 | new [void] | new [void] |
+| A.cpp:49:13:49:13 | c | A.cpp:47:12:47:18 | new [void] | A.cpp:49:13:49:13 | c | c flows from $@ | A.cpp:47:12:47:18 | new [void] | new [void] |
+| A.cpp:56:13:56:15 | call to get | A.cpp:55:12:55:19 | new [void] | A.cpp:56:13:56:15 | call to get | call to get flows from $@ | A.cpp:55:12:55:19 | new [void] | new [void] |
+| A.cpp:57:28:57:30 | call to get | A.cpp:57:17:57:23 | new [void] | A.cpp:57:28:57:30 | call to get | call to get flows from $@ | A.cpp:57:17:57:23 | new [void] | new [void] |
+| A.cpp:66:14:66:14 | c | A.cpp:64:21:64:28 | new [void] | A.cpp:66:14:66:14 | c | c flows from $@ | A.cpp:64:21:64:28 | new [void] | new [void] |
+| A.cpp:75:14:75:14 | c | A.cpp:73:25:73:32 | new [void] | A.cpp:75:14:75:14 | c | c flows from $@ | A.cpp:73:25:73:32 | new [void] | new [void] |
+| A.cpp:107:16:107:16 | a | A.cpp:98:12:98:18 | new [void] | A.cpp:107:16:107:16 | a | a flows from $@ | A.cpp:98:12:98:18 | new [void] | new [void] |
+| A.cpp:120:16:120:16 | a | A.cpp:98:12:98:18 | new [void] | A.cpp:120:16:120:16 | a | a flows from $@ | A.cpp:98:12:98:18 | new [void] | new [void] |
+| A.cpp:132:13:132:13 | c | A.cpp:126:12:126:18 | new [void] | A.cpp:132:13:132:13 | c | c flows from $@ | A.cpp:126:12:126:18 | new [void] | new [void] |
+| A.cpp:152:13:152:13 | b | A.cpp:143:25:143:31 | new [void] | A.cpp:152:13:152:13 | b | b flows from $@ | A.cpp:143:25:143:31 | new [void] | new [void] |
+| A.cpp:152:13:152:13 | b | A.cpp:150:12:150:18 | new [void] | A.cpp:152:13:152:13 | b | b flows from $@ | A.cpp:150:12:150:18 | new [void] | new [void] |
+| A.cpp:153:16:153:16 | c | A.cpp:142:14:142:20 | new [void] | A.cpp:153:16:153:16 | c | c flows from $@ | A.cpp:142:14:142:20 | new [void] | new [void] |
+| A.cpp:154:13:154:13 | c | A.cpp:142:14:142:20 | new [void] | A.cpp:154:13:154:13 | c | c flows from $@ | A.cpp:142:14:142:20 | new [void] | new [void] |
+| A.cpp:165:26:165:29 | head | A.cpp:159:12:159:18 | new [void] | A.cpp:165:26:165:29 | head | head flows from $@ | A.cpp:159:12:159:18 | new [void] | new [void] |
+| A.cpp:169:15:169:18 | head | A.cpp:159:12:159:18 | new [void] | A.cpp:169:15:169:18 | head | head flows from $@ | A.cpp:159:12:159:18 | new [void] | new [void] |
+| B.cpp:9:20:9:24 | elem1 | B.cpp:6:15:6:24 | new [void] | B.cpp:9:20:9:24 | elem1 | elem1 flows from $@ | B.cpp:6:15:6:24 | new [void] | new [void] |
+| B.cpp:10:20:10:24 | elem2 | B.cpp:6:15:6:24 | new [void] | B.cpp:10:20:10:24 | elem2 | elem2 flows from $@ | B.cpp:6:15:6:24 | new [void] | new [void] |
+| B.cpp:18:20:18:24 | elem1 | B.cpp:15:15:15:27 | new [void] | B.cpp:18:20:18:24 | elem1 | elem1 flows from $@ | B.cpp:15:15:15:27 | new [void] | new [void] |
+| B.cpp:19:20:19:24 | elem2 | B.cpp:15:15:15:27 | new [void] | B.cpp:19:20:19:24 | elem2 | elem2 flows from $@ | B.cpp:15:15:15:27 | new [void] | new [void] |
+| C.cpp:31:10:31:11 | s3 | C.cpp:24:16:24:25 | new [void] | C.cpp:31:10:31:11 | s3 | s3 flows from $@ | C.cpp:24:16:24:25 | new [void] | new [void] |
diff --git a/cpp/ql/test/library-tests/dataflow/fields/flow.ql b/cpp/ql/test/library-tests/dataflow/fields/flow.ql
new file mode 100644
index 000000000000..be028eb7cb8b
--- /dev/null
+++ b/cpp/ql/test/library-tests/dataflow/fields/flow.ql
@@ -0,0 +1,32 @@
+/**
+ * @kind path-problem
+ */
+
+import semmle.code.cpp.dataflow.DataFlow
+import DataFlow::PathGraph
+import DataFlow
+import cpp
+class Conf extends Configuration {
+  Conf() { this = "FieldFlowConf" }
+
+  override predicate isSource(Node src) { src.asExpr() instanceof NewExpr }
+
+  override predicate isSink(Node sink) {
+    exists(Call c |
+      c.getTarget().hasName("sink") and
+      c.getAnArgument() = sink.asExpr()
+    )
+  }
+
+  override predicate isAdditionalFlowStep(Node a, Node b) {
+    b.asPartialDefinition() = any(Call c |
+        c.getTarget().hasName("f") and c.getAnArgument() = a.asExpr()
+      ).getQualifier()
+    or
+    b.asExpr().(AddressOfExpr).getOperand() = a.asExpr()
+  }
+}
+
+from DataFlow::PathNode src, DataFlow::PathNode sink, Conf conf
+where conf.hasFlowPath(src, sink)
+select sink, src, sink, sink + " flows from $@", src, src.toString()

From b1632587bc7ed7961a030d6332244a2e192933c7 Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Thu, 11 Jul 2019 12:34:20 +0100
Subject: [PATCH 13/28] Use more meaningful name

---
 cpp/ql/test/library-tests/dataflow/fields/A.cpp   | 6 +++---
 cpp/ql/test/library-tests/dataflow/fields/flow.ql | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/cpp/ql/test/library-tests/dataflow/fields/A.cpp b/cpp/ql/test/library-tests/dataflow/fields/A.cpp
index 8490a560b083..703d1b9a73fa 100644
--- a/cpp/ql/test/library-tests/dataflow/fields/A.cpp
+++ b/cpp/ql/test/library-tests/dataflow/fields/A.cpp
@@ -4,7 +4,7 @@ class A
   class C
   {
   public:
-    virtual void f(void *) {}
+    virtual void insert(void *) {}
   };
   class C1 : public C
   {
@@ -37,8 +37,8 @@ class A
   {
     C cc;
     C ct;
-    cc.f(nullptr);
-    ct.f(new C());
+    cc.insert(nullptr);
+    ct.insert(new C());
     sink(&cc); // no flow
     sink(&ct); // flow
   }
diff --git a/cpp/ql/test/library-tests/dataflow/fields/flow.ql b/cpp/ql/test/library-tests/dataflow/fields/flow.ql
index be028eb7cb8b..b62d577e36e1 100644
--- a/cpp/ql/test/library-tests/dataflow/fields/flow.ql
+++ b/cpp/ql/test/library-tests/dataflow/fields/flow.ql
@@ -20,7 +20,7 @@ class Conf extends Configuration {
 
   override predicate isAdditionalFlowStep(Node a, Node b) {
     b.asPartialDefinition() = any(Call c |
-        c.getTarget().hasName("f") and c.getAnArgument() = a.asExpr()
+        c.getTarget().hasName("insert") and c.getAnArgument() = a.asExpr()
       ).getQualifier()
     or
     b.asExpr().(AddressOfExpr).getOperand() = a.asExpr()

From 835e495e7cf9776b71d7308ad374dd522e6799f6 Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Thu, 11 Jul 2019 16:51:30 +0100
Subject: [PATCH 14/28] Remove unused args

---
 .../src/semmle/code/cpp/dataflow/internal/FlowVar.qll  | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
index eaec20037189..40b2367dd84c 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
@@ -98,15 +98,15 @@ class FlowVar extends TFlowVar {
  */
 private module PartialDefinitions {
   private newtype TPartialDefinition =
-    TExplicitFieldStoreQualifier(Expr qualifier, FieldAccess fa, Expr assignment, Expr assigned) {
-      isInstanceFieldWrite(fa, assignment, assigned) and qualifier = fa.getQualifier()
+    TExplicitFieldStoreQualifier(Expr qualifier, FieldAccess fa) {
+      isInstanceFieldWrite(fa) and qualifier = fa.getQualifier()
     } or
     TExplicitCallQualifier(Expr qualifier, Call call) { qualifier = call.getQualifier() } or
     TReferenceArgument(Expr arg, Call call, int i) { isReferenceOrPointerArg(arg, call, i) }
 
-  private predicate isInstanceFieldWrite(FieldAccess fa, Expr assignment, Expr assigned) {
+  private predicate isInstanceFieldWrite(FieldAccess fa) {
     not fa.getTarget().isStatic() and
-    assignmentLikeOperation(assignment, fa.getTarget(), fa, assigned)
+    assignmentLikeOperation(_, fa.getTarget(), fa, _)
   }
 
   private predicate isReferenceOrPointerArg(Expr arg, Call call, int i) {
@@ -120,7 +120,7 @@ private module PartialDefinitions {
     Expr definedExpr;
 
     PartialDefinition() {
-      this = TExplicitFieldStoreQualifier(definedExpr, _, _, _) or
+      this = TExplicitFieldStoreQualifier(definedExpr, _) or
       this = TExplicitCallQualifier(definedExpr, _) or
       this = TReferenceArgument(definedExpr, _, _)
     }

From 79d75d7d18db8ed10befa3c73c99d1a12e5b7094 Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Thu, 18 Jul 2019 11:30:10 +0100
Subject: [PATCH 15/28] Add extra test cases

---
 .../library-tests/dataflow/fields/complex.cpp | 64 +++++++++++++++++++
 .../dataflow/fields/constructors.cpp          | 51 +++++++++++++++
 .../dataflow/fields/flow.expected             |  4 +-
 .../library-tests/dataflow/fields/flow.ql     |  7 +-
 .../library-tests/dataflow/fields/simple.cpp  | 56 ++++++++++++++++
 5 files changed, 179 insertions(+), 3 deletions(-)
 create mode 100644 cpp/ql/test/library-tests/dataflow/fields/complex.cpp
 create mode 100644 cpp/ql/test/library-tests/dataflow/fields/constructors.cpp
 create mode 100644 cpp/ql/test/library-tests/dataflow/fields/simple.cpp

diff --git a/cpp/ql/test/library-tests/dataflow/fields/complex.cpp b/cpp/ql/test/library-tests/dataflow/fields/complex.cpp
new file mode 100644
index 000000000000..4b5d563072b0
--- /dev/null
+++ b/cpp/ql/test/library-tests/dataflow/fields/complex.cpp
@@ -0,0 +1,64 @@
+namespace Complex
+{
+class Foo
+{
+  int a_;
+  int b_;
+
+public:
+  int a() { return a_; }
+  int b() { return b_; }
+  void setA(int a) { a_ = a; }
+  void setB(int b) { b_ = b; }
+
+  Foo(int a, int b) : a_(a), b_(b){};
+};
+
+class Bar
+{
+public:
+  Foo f;
+
+  Bar() : f(0, 0) {}
+};
+
+int user_input()
+{
+  return 42;
+}
+
+void sink(int x)
+{
+}
+
+void bar(Bar &b)
+{
+  sink(b.f.a());
+  sink(b.f.b());
+}
+
+void foo()
+{
+  Bar b1;
+  Bar b2;
+  Bar b3;
+  Bar b4;
+
+  b1.f.setA(user_input());
+  b2.f.setB(user_input());
+  b3.f.setA(user_input());
+  b3.f.setB(user_input());
+
+  // Only a() should alert
+  bar(b1);
+
+  // Only b() should alert
+  bar(b2);
+
+  // Both a() and b() should alert
+  bar(b3);
+
+  // Nothing should alert
+  bar(b4);
+}
+}; // namespace Complex
\ No newline at end of file
diff --git a/cpp/ql/test/library-tests/dataflow/fields/constructors.cpp b/cpp/ql/test/library-tests/dataflow/fields/constructors.cpp
new file mode 100644
index 000000000000..cfa9be35f2b2
--- /dev/null
+++ b/cpp/ql/test/library-tests/dataflow/fields/constructors.cpp
@@ -0,0 +1,51 @@
+namespace Constructors
+{
+int user_input()
+{
+    return 42;
+}
+
+void sink(int x)
+{
+}
+
+class Foo
+{
+    int a_;
+    int b_;
+
+public:
+    int a() { return a_; }
+    int b() { return b_; }
+    void setA(int a) { a_ = a; }
+    void setB(int b) { b_ = b; }
+
+    Foo(int a, int b) : a_(a), b_(b){};
+};
+
+void bar(Foo &f)
+{
+    sink(f.a());
+    sink(f.b());
+}
+
+void foo()
+{
+    Foo f(user_input(), 0);
+    Foo g(0, user_input());
+    Foo h(user_input(), user_input());
+    Foo i(0, 0);
+
+    // Only a() should alert
+    bar(f);
+
+    // Only b() should alert
+    bar(g);
+
+    // Both a() and b() should alert
+    bar(h);
+
+    // Nothing should alert
+    bar(i);
+}
+}; // namespace Constructors
\ No newline at end of file
diff --git a/cpp/ql/test/library-tests/dataflow/fields/flow.expected b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
index f5ff8cb31132..e9cb5465b3d7 100644
--- a/cpp/ql/test/library-tests/dataflow/fields/flow.expected
+++ b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
@@ -1,5 +1,5 @@
 edges
-| A.cpp:41:10:41:16 | new [void] | A.cpp:43:10:43:12 | & ... |
+| A.cpp:41:15:41:21 | new [void] | A.cpp:43:10:43:12 | & ... |
 | A.cpp:47:12:47:18 | new [void] | A.cpp:48:20:48:20 | c [void] |
 | A.cpp:48:12:48:18 | call to make [c, ... (1)] | A.cpp:49:10:49:10 | b [c, ... (1)] |
 | A.cpp:48:20:48:20 | c [void] | A.cpp:48:12:48:18 | call to make [c, ... (1)] |
@@ -90,7 +90,7 @@ edges
 | C.cpp:27:8:27:11 | `this` parameter in func [s3, ... (1)] | file://:0:0:0:0 | this [s3, ... (1)] |
 | file://:0:0:0:0 | this [s3, ... (1)] | C.cpp:31:10:31:11 | s3 |
 #select
-| A.cpp:43:10:43:12 | & ... | A.cpp:41:10:41:16 | new [void] | A.cpp:43:10:43:12 | & ... | & ... flows from $@ | A.cpp:41:10:41:16 | new [void] | new [void] |
+| A.cpp:43:10:43:12 | & ... | A.cpp:41:15:41:21 | new [void] | A.cpp:43:10:43:12 | & ... | & ... flows from $@ | A.cpp:41:15:41:21 | new [void] | new [void] |
 | A.cpp:49:13:49:13 | c | A.cpp:47:12:47:18 | new [void] | A.cpp:49:13:49:13 | c | c flows from $@ | A.cpp:47:12:47:18 | new [void] | new [void] |
 | A.cpp:56:13:56:15 | call to get | A.cpp:55:12:55:19 | new [void] | A.cpp:56:13:56:15 | call to get | call to get flows from $@ | A.cpp:55:12:55:19 | new [void] | new [void] |
 | A.cpp:57:28:57:30 | call to get | A.cpp:57:17:57:23 | new [void] | A.cpp:57:28:57:30 | call to get | call to get flows from $@ | A.cpp:57:17:57:23 | new [void] | new [void] |
diff --git a/cpp/ql/test/library-tests/dataflow/fields/flow.ql b/cpp/ql/test/library-tests/dataflow/fields/flow.ql
index b62d577e36e1..131973534bf9 100644
--- a/cpp/ql/test/library-tests/dataflow/fields/flow.ql
+++ b/cpp/ql/test/library-tests/dataflow/fields/flow.ql
@@ -6,10 +6,15 @@ import semmle.code.cpp.dataflow.DataFlow
 import DataFlow::PathGraph
 import DataFlow
 import cpp
+
 class Conf extends Configuration {
   Conf() { this = "FieldFlowConf" }
 
-  override predicate isSource(Node src) { src.asExpr() instanceof NewExpr }
+  override predicate isSource(Node src) {
+    src.asExpr() instanceof NewExpr
+    or
+    src.asExpr().(Call).getTarget().hasName("user_input")
+  }
 
   override predicate isSink(Node sink) {
     exists(Call c |
diff --git a/cpp/ql/test/library-tests/dataflow/fields/simple.cpp b/cpp/ql/test/library-tests/dataflow/fields/simple.cpp
new file mode 100644
index 000000000000..1d75fe4e850f
--- /dev/null
+++ b/cpp/ql/test/library-tests/dataflow/fields/simple.cpp
@@ -0,0 +1,56 @@
+namespace Simple
+{
+int user_input()
+{
+    return 42;
+}
+
+void sink(int x)
+{
+}
+
+class Foo
+{
+    int a_;
+    int b_;
+
+public:
+    int a() { return a_; }
+    int b() { return b_; }
+    void setA(int a) { a_ = a; }
+    void setB(int b) { b_ = b; }
+
+    Foo(int a, int b) : a_(a), b_(b){};
+};
+
+void bar(Foo &f)
+{
+    sink(f.a());
+    sink(f.b());
+}
+
+void foo()
+{
+    Foo f(0, 0);
+    Foo g(0, 0);
+    Foo h(0, 0);
+    Foo i(0, 0);
+
+    f.setA(user_input());
+    g.setB(user_input());
+    h.setA(user_input());
+    h.setB(user_input());
+
+    // Only a() should alert
+    bar(f);
+
+    // Only b() should alert
+    bar(g);
+
+    // Both a() and b() should alert
+    bar(h);
+
+    // Nothing should alert
+    bar(i);
+}
+} // namespace Simple
\ No newline at end of file

From 861964337c06fb0c7f440cae4d299808afa1a82a Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Thu, 8 Aug 2019 11:31:46 +0200
Subject: [PATCH 16/28] C++: Undo autoformat of FlowVar.qll

The formatting changes were good, but were tangled in with other
changes, making it hard to review this file.
---
 .../code/cpp/dataflow/internal/FlowVar.qll    | 122 +++++++++---------
 1 file changed, 62 insertions(+), 60 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
index 40b2367dd84c..432cb54d212e 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
@@ -1,7 +1,6 @@
 /**
  * Provides a class for handling variables in the data flow analysis.
  */
-
 import cpp
 private import semmle.code.cpp.controlflow.SSA
 private import semmle.code.cpp.dataflow.internal.SubBasicBlocks
@@ -19,8 +18,7 @@ private import semmle.code.cpp.dataflow.internal.SubBasicBlocks
  * member predicates explains how a `FlowVar` relates to syntactic constructs of
  * the language.
  */
-cached
-class FlowVar extends TFlowVar {
+cached class FlowVar extends TFlowVar {
   /**
    * Gets a `VariableAccess` that _may_ take its value from `this`. Consider
    * the following snippet.
@@ -37,8 +35,7 @@ class FlowVar extends TFlowVar {
    * corresponding to `x = 1`. The `x` in `x = 1` is not considered to be an
    * access.
    */
-  cached
-  abstract VariableAccess getAnAccess();
+  cached abstract VariableAccess getAnAccess();
 
   /**
    * Holds if this `FlowVar` corresponds to a modification occurring when `node` is
@@ -52,18 +49,15 @@ class FlowVar extends TFlowVar {
    *   `node instanceof PostCrementOperation` is an exception to the rule that
    *   `this` contains the value of `e` after the evaluation of `node`.
    */
-  cached
-  abstract predicate definedByExpr(Expr e, ControlFlowNode node);
+  cached abstract predicate definedByExpr(Expr e, ControlFlowNode node);
 
   /**
    * Holds if this `FlowVar` corresponds to the data written by a call that
    * passes a variable as argument `arg`.
    */
-  cached
-  abstract predicate definedByReference(Expr arg);
+  cached abstract predicate definedByReference(Expr arg);
 
-  cached
-  abstract predicate definedPartiallyAt(Expr e);
+  cached abstract predicate definedPartiallyAt(Expr e);
 
   /**
    * Holds if this `FlowVar` corresponds to the initial value of `v`. The following
@@ -76,16 +70,13 @@ class FlowVar extends TFlowVar {
    *   local variable is always overwritten before it is used, there is no
    *   `FlowVar` instance for the uninitialized value of that variable.
    */
-  cached
-  abstract predicate definedByInitialValue(LocalScopeVariable v);
+  cached abstract predicate definedByInitialValue(LocalScopeVariable v);
 
   /** Gets a textual representation of this element. */
-  cached
-  abstract string toString();
+  cached abstract string toString();
 
   /** Gets the location of this element. */
-  cached
-  abstract Location getLocation();
+  cached abstract Location getLocation();
 }
 
 /**
@@ -183,7 +174,9 @@ module FlowVar_internal {
     // always executes at least once, we give it special treatment in
     // `BlockVar`, somewhat analogous to unrolling the first iteration of the
     // loop.
-    not exists(AlwaysTrueUponEntryLoop loop | loop.alwaysAssignsBeforeLeavingCondition(_, _, v)) and
+    not exists(AlwaysTrueUponEntryLoop loop |
+      loop.alwaysAssignsBeforeLeavingCondition(_, _, v)
+    ) and
     // The SSA library has a theoretically accurate treatment of reference types,
     // treating them as immutable, but for data flow it gives better results in
     // practice to make the variable synonymous with its contents.
@@ -194,7 +187,9 @@ module FlowVar_internal {
    * Holds if `sbb` is the `SubBasicBlock` where `v` receives its initial value.
    * See the documentation for `FlowVar.definedByInitialValue`.
    */
-  predicate blockVarDefinedByVariable(SubBasicBlock sbb, LocalScopeVariable v) {
+  predicate blockVarDefinedByVariable(
+      SubBasicBlock sbb, LocalScopeVariable v)
+  {
     sbb = v.(Parameter).getFunction().getEntryPoint()
     or
     exists(DeclStmt declStmt |
@@ -208,7 +203,8 @@ module FlowVar_internal {
     TSsaVar(SsaDefinition def, LocalScopeVariable v) {
       fullySupportedSsaVariable(v) and
       v = def.getAVariable()
-    } or
+    }
+    or
     TBlockVar(SubBasicBlock sbb, Variable v) {
       not fullySupportedSsaVariable(v) and
       reachable(sbb) and
@@ -246,8 +242,7 @@ module FlowVar_internal {
       // the data flow library will never see those, and the `FlowVar` library
       // is only meant to be used by the data flow library.
       this.isNonPhi() and
-      (
-        // This base case could be included in the transitive case by changing
+      ( // This base case could be included in the transitive case by changing
         // `+` to `*`, but that's slower because it goes through the `TSsaVar`
         // indirection.
         result = def.getAUse(v)
@@ -261,11 +256,9 @@ module FlowVar_internal {
 
     override predicate definedByExpr(Expr e, ControlFlowNode node) {
       e = def.getDefiningValue(v) and
-      (
-        if def.getDefinition() = v.getInitializer().getExpr()
-        then node = v.getInitializer()
-        else node = def.getDefinition()
-      )
+      (if def.getDefinition() = v.getInitializer().getExpr()
+       then node = v.getInitializer()
+       else node = def.getDefinition())
     }
 
     override predicate definedByReference(Expr arg) {
@@ -339,7 +332,9 @@ module FlowVar_internal {
       node = sbb.getANode()
     }
 
-    override predicate definedByReference(Expr arg) { blockVarDefinedByReference(sbb, v, arg) }
+    override predicate definedByReference(Expr arg) {
+      blockVarDefinedByReference(sbb, v, arg)
+    }
 
     override predicate definedPartiallyAt(Expr e) {
       exists(PartialDefinitions::PartialDefinition p |
@@ -352,15 +347,15 @@ module FlowVar_internal {
     override string toString() {
       exists(Expr e |
         this.definedByExpr(e, _) and
-        result = "assignment to " + v
+        result = "assignment to "+ v
       )
       or
       this.definedByInitialValue(_) and
-      result = "initial value of " + v
+      result = "initial value of "+ v
       or
       exists(Expr arg |
         this.definedByReference(arg) and
-        result = "definition by reference of " + v
+        result = "definition by reference of "+ v
       )
       or
       exists(Expr partialDef |
@@ -373,7 +368,7 @@ module FlowVar_internal {
       not this.definedByInitialValue(_) and
       not this.definedByReference(_) and
       not this.definedPartiallyAt(_) and
-      result = "undefined " + v
+      result = "undefined "+ v
     }
 
     override Location getLocation() { result = sbb.getStart().getLocation() }
@@ -408,7 +403,9 @@ module FlowVar_internal {
   }
 
   /** Type-specialized version of `getEnclosingElement`. */
-  private ControlFlowNode getCFNParent(ControlFlowNode node) { result = node.getEnclosingElement() }
+  private ControlFlowNode getCFNParent(ControlFlowNode node) {
+    result = node.getEnclosingElement()
+  }
 
   /**
    * A for-loop or while-loop whose condition is always true upon entry but not
@@ -499,9 +496,11 @@ module FlowVar_internal {
    * be used outside the loop.
    */
   predicate skipLoop(
-    SubBasicBlock sbbInside, SubBasicBlock sbbOutside, SubBasicBlock sbbDef, Variable v
+    SubBasicBlock sbbInside, SubBasicBlock sbbOutside,
+    SubBasicBlock sbbDef, Variable v
   ) {
-    exists(AlwaysTrueUponEntryLoop loop, BasicBlock bbInside, BasicBlock bbOutside |
+    exists(AlwaysTrueUponEntryLoop loop,
+           BasicBlock bbInside, BasicBlock bbOutside |
       loop.alwaysAssignsBeforeLeavingCondition(bbInside, bbOutside, v) and
       bbInside = sbbInside.getBasicBlock() and
       bbOutside = sbbOutside.getBasicBlock() and
@@ -586,18 +585,18 @@ module FlowVar_internal {
     forall(VariableAccess va |
       va.getTarget() = v and
       readAccess(va)
-    |
-      dominatedByOverwrite(v, va)
+    | dominatedByOverwrite(v, va)
     )
   }
 
   /** Holds if `va` accesses `v` and is dominated by an overwrite of `v`. */
-  predicate dominatedByOverwrite(UninitializedLocalVariable v, VariableAccess va) {
+  predicate dominatedByOverwrite(UninitializedLocalVariable v,
+                                 VariableAccess va)
+  {
     exists(BasicBlock bb, int vaIndex |
       va = bb.getNode(vaIndex) and
       va.getTarget() = v
-    |
-      vaIndex > indexOfFirstOverwriteInBB(v, bb)
+    | vaIndex > indexOfFirstOverwriteInBB(v, bb)
       or
       bbStrictlyDominates(getAnOverwritingBB(v), bb)
     )
@@ -642,28 +641,28 @@ module FlowVar_internal {
   ) {
     // Together, the two following cases cover `Assignment`
     node = any(AssignExpr ae |
-        va = ae.getLValue() and
-        v = va.getTarget() and
-        assignedExpr = ae.getRValue()
-      )
+      va = ae.getLValue() and
+      v = va.getTarget() and
+      assignedExpr = ae.getRValue()
+    )
     or
     node = any(AssignOperation ao |
-        va = ao.getLValue() and
-        v = va.getTarget() and
-        // Here and in the `PrefixCrementOperation` case, we say that the assigned
-        // expression is the operation itself. For example, we say that `x += 1`
-        // assigns `x += 1` to `x`. The justification is that after this operation,
-        // `x` will contain the same value that `x += 1` evaluated to.
-        assignedExpr = ao
-      )
+      va = ao.getLValue() and
+      v = va.getTarget() and
+      // Here and in the `PrefixCrementOperation` case, we say that the assigned
+      // expression is the operation itself. For example, we say that `x += 1`
+      // assigns `x += 1` to `x`. The justification is that after this operation,
+      // `x` will contain the same value that `x += 1` evaluated to.
+      assignedExpr = ao
+    )
     or
     // This case does not add further data flow paths, except if a
     // `PrefixCrementOperation` is itself a source
     node = any(CrementOperation op |
-        va = op.getOperand() and
-        v = va.getTarget() and
-        assignedExpr = op
-      )
+      va = op.getOperand() and
+      v = va.getTarget() and
+      assignedExpr = op
+    )
   }
 
   predicate blockVarDefinedByReference(ControlFlowNode node, Variable v, Expr argument) {
@@ -674,7 +673,9 @@ module FlowVar_internal {
   /**
    * Holds if `v` is initialized by `init` to have value `assignedExpr`.
    */
-  predicate initializer(Initializer init, LocalVariable v, Expr assignedExpr) {
+  predicate initializer(
+      Initializer init, LocalVariable v, Expr assignedExpr)
+  {
     v = init.getDeclaration() and
     assignedExpr = init.getExpr()
   }
@@ -687,7 +688,9 @@ module FlowVar_internal {
    */
   class DataFlowSubBasicBlockCutNode extends SubBasicBlockCutNode {
     DataFlowSubBasicBlockCutNode() {
-      exists(Variable v | not fullySupportedSsaVariable(v) |
+      exists(Variable v |
+        not fullySupportedSsaVariable(v)
+      |
         assignmentLikeOperation(this, v, _, _)
         or
         blockVarDefinedByReference(this, v, _)
@@ -702,5 +705,4 @@ module FlowVar_internal {
       )
     }
   }
-}
-/* module FlowVar_internal */
+} /* module FlowVar_internal */

From 6a3f5efc1b7146b39357b0bf36a4c637f7530c14 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Wed, 7 Aug 2019 15:02:54 +0200
Subject: [PATCH 17/28] C++: Accept AST field flow test output

---
 .../dataflow-tests/localFlow.expected         | 11 +++++---
 .../dataflow/dataflow-tests/test.cpp          |  6 ++--
 .../dataflow/dataflow-tests/test.expected     |  3 ++
 .../dataflow-tests/test_diff.expected         |  3 ++
 .../library-tests/dataflow/fields/complex.cpp |  6 ++--
 .../dataflow/fields/constructors.cpp          |  6 ++--
 .../dataflow/fields/flow.expected             | 20 +++++++++++++
 .../library-tests/dataflow/fields/simple.cpp  |  6 ++--
 .../dataflow/taint-tests/localTaint.expected  | 28 +++++++++++++------
 .../dataflow/taint-tests/taint.cpp            |  2 +-
 .../dataflow/taint-tests/taint.expected       |  1 +
 .../dataflow/taint-tests/test_diff.expected   |  1 +
 12 files changed, 68 insertions(+), 25 deletions(-)

diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
index da5001b0b575..9ff45c0e172f 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
@@ -5,12 +5,15 @@
 | example.c:17:19:17:22 | {...} | example.c:24:13:24:18 | coords |
 | example.c:17:19:17:22 | {...} | example.c:26:2:26:7 | coords |
 | example.c:17:19:17:22 | {...} | example.c:26:19:26:24 | coords |
+| example.c:24:2:24:7 | coords [post update] | example.c:26:2:26:7 | coords |
+| example.c:24:2:24:7 | coords [post update] | example.c:26:19:26:24 | coords |
+| example.c:24:13:24:18 | coords [post update] | example.c:24:2:24:7 | coords |
+| example.c:24:13:24:18 | coords [post update] | example.c:26:2:26:7 | coords |
+| example.c:24:13:24:18 | coords [post update] | example.c:26:19:26:24 | coords |
 | example.c:24:13:24:30 | ... = ... | example.c:24:2:24:30 | ... = ... |
 | example.c:24:24:24:30 | ... + ... | example.c:24:13:24:30 | ... = ... |
 | example.c:26:13:26:16 | call to getX | example.c:26:2:26:25 | ... = ... |
 | example.c:26:18:26:24 | ref arg & ... | example.c:26:2:26:7 | coords |
-| example.c:26:18:26:24 | ref arg & ... | example.c:26:19:26:24 | coords |
-| example.c:28:22:28:25 | ref arg & ... | example.c:28:23:28:25 | pos |
 | test.cpp:6:12:6:17 | call to source | test.cpp:7:8:7:9 | t1 |
 | test.cpp:6:12:6:17 | call to source | test.cpp:8:8:8:9 | t1 |
 | test.cpp:6:12:6:17 | call to source | test.cpp:9:8:9:9 | t1 |
@@ -39,7 +42,7 @@
 | test.cpp:431:12:431:13 | 0 | test.cpp:432:33:432:35 | tmp |
 | test.cpp:431:12:431:13 | 0 | test.cpp:433:8:433:10 | tmp |
 | test.cpp:432:10:432:13 | & ... | test.cpp:432:3:432:8 | call to memcpy |
-| test.cpp:432:10:432:13 | ref arg & ... | test.cpp:432:11:432:13 | tmp |
+| test.cpp:432:10:432:13 | & ... [post update] | test.cpp:432:3:432:8 | call to memcpy |
 | test.cpp:432:10:432:13 | ref arg & ... | test.cpp:432:33:432:35 | tmp |
 | test.cpp:432:10:432:13 | ref arg & ... | test.cpp:433:8:433:10 | tmp |
 | test.cpp:432:17:432:23 | source1 | test.cpp:432:10:432:13 | ref arg & ... |
@@ -51,7 +54,7 @@
 | test.cpp:437:12:437:13 | 0 | test.cpp:440:8:440:10 | tmp |
 | test.cpp:437:12:437:13 | 0 | test.cpp:442:10:442:12 | tmp |
 | test.cpp:439:10:439:13 | & ... | test.cpp:439:3:439:8 | call to memcpy |
-| test.cpp:439:10:439:13 | ref arg & ... | test.cpp:439:11:439:13 | tmp |
+| test.cpp:439:10:439:13 | & ... [post update] | test.cpp:439:3:439:8 | call to memcpy |
 | test.cpp:439:10:439:13 | ref arg & ... | test.cpp:439:33:439:35 | tmp |
 | test.cpp:439:10:439:13 | ref arg & ... | test.cpp:440:8:440:10 | tmp |
 | test.cpp:439:10:439:13 | ref arg & ... | test.cpp:442:10:442:12 | tmp |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
index 4e5bcdc015d8..d91c273f2dbf 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
@@ -135,7 +135,7 @@ void following_pointers(
 
   sourceStruct1_ptr->m1 = source();
   sink(sourceStruct1_ptr->m1); // flow
-  sink(sourceStruct1_ptr->getFirst()); // no flow (due to limitations of the analysis)
+  sink(sourceStruct1_ptr->getFirst()); // flow [NOT DETECTED with IR]
   sink(sourceStruct1_ptr->m2); // no flow
   sink(sourceStruct1.m1); // flow (due to lack of no-alias tracking)
 
@@ -410,11 +410,11 @@ class FlowThroughFields {
   int f() {
     sink(field); // tainted or clean? Not sure.
     taintField();
-    sink(field); // tainted (FALSE NEGATIVE)
+    sink(field); // tainted [NOT DETECTED with IR]
   }
 
   int calledAfterTaint() {
-    sink(field); // tainted (FALSE NEGATIVE)
+    sink(field); // tainted [NOT DETECTED with IR]
   }
 
   int taintAndCall() {
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected
index 3a7167f2cd77..9968d63e3a88 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.expected
@@ -15,6 +15,7 @@
 | test.cpp:103:10:103:12 | ref | test.cpp:100:13:100:18 | call to source |
 | test.cpp:126:8:126:19 | sourceArray1 | test.cpp:120:9:120:20 | sourceArray1 |
 | test.cpp:137:27:137:28 | m1 | test.cpp:136:27:136:32 | call to source |
+| test.cpp:138:27:138:34 | call to getFirst | test.cpp:136:27:136:32 | call to source |
 | test.cpp:140:22:140:23 | m1 | test.cpp:136:27:136:32 | call to source |
 | test.cpp:188:8:188:8 | y | test.cpp:186:27:186:32 | call to source |
 | test.cpp:192:8:192:8 | s | test.cpp:199:33:199:38 | call to source |
@@ -27,6 +28,8 @@
 | test.cpp:337:14:337:14 | x | test.cpp:353:17:353:22 | call to source |
 | test.cpp:366:7:366:7 | x | test.cpp:362:4:362:9 | call to source |
 | test.cpp:397:10:397:18 | globalVar | test.cpp:395:17:395:22 | call to source |
+| test.cpp:413:10:413:14 | field | test.cpp:407:13:407:18 | call to source |
+| test.cpp:417:10:417:14 | field | test.cpp:421:13:421:18 | call to source |
 | test.cpp:423:10:423:14 | field | test.cpp:421:13:421:18 | call to source |
 | test.cpp:433:8:433:10 | tmp | test.cpp:430:48:430:54 | source1 |
 | test.cpp:440:8:440:10 | tmp | test.cpp:436:53:436:59 | source1 |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
index b6d11257315d..6a2a933b6c7b 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
@@ -2,10 +2,13 @@
 | test.cpp:100:13:100:18 | test.cpp:103:10:103:12 | AST only |
 | test.cpp:109:9:109:14 | test.cpp:110:10:110:12 | IR only |
 | test.cpp:136:27:136:32 | test.cpp:137:27:137:28 | AST only |
+| test.cpp:136:27:136:32 | test.cpp:138:27:138:34 | AST only |
 | test.cpp:136:27:136:32 | test.cpp:140:22:140:23 | AST only |
 | test.cpp:142:32:142:37 | test.cpp:145:10:145:11 | IR only |
 | test.cpp:151:35:151:40 | test.cpp:153:17:153:18 | IR only |
 | test.cpp:395:17:395:22 | test.cpp:397:10:397:18 | AST only |
+| test.cpp:407:13:407:18 | test.cpp:413:10:413:14 | AST only |
+| test.cpp:421:13:421:18 | test.cpp:417:10:417:14 | AST only |
 | test.cpp:421:13:421:18 | test.cpp:423:10:423:14 | AST only |
 | test.cpp:430:48:430:54 | test.cpp:433:8:433:10 | AST only |
 | test.cpp:436:53:436:59 | test.cpp:440:8:440:10 | AST only |
diff --git a/cpp/ql/test/library-tests/dataflow/fields/complex.cpp b/cpp/ql/test/library-tests/dataflow/fields/complex.cpp
index 4b5d563072b0..5a6c67fa8df9 100644
--- a/cpp/ql/test/library-tests/dataflow/fields/complex.cpp
+++ b/cpp/ql/test/library-tests/dataflow/fields/complex.cpp
@@ -33,8 +33,8 @@ void sink(int x)
 
 void bar(Bar &b)
 {
-  sink(b.f.a());
-  sink(b.f.b());
+  sink(b.f.a()); // flow (through `b1.f.setA` and `b3.f.setA`) [NOT DETECTED]
+  sink(b.f.b()); // flow (through `b2.f.setB` and `b3.f.setB`) [NOT DETECTED]
 }
 
 void foo()
@@ -61,4 +61,4 @@ void foo()
   // Nothing should alert
   bar(b4);
 }
-}; // namespace Complex
\ No newline at end of file
+}; // namespace Complex
diff --git a/cpp/ql/test/library-tests/dataflow/fields/constructors.cpp b/cpp/ql/test/library-tests/dataflow/fields/constructors.cpp
index cfa9be35f2b2..4aacab0c9e57 100644
--- a/cpp/ql/test/library-tests/dataflow/fields/constructors.cpp
+++ b/cpp/ql/test/library-tests/dataflow/fields/constructors.cpp
@@ -25,8 +25,8 @@ class Foo
 
 void bar(Foo &f)
 {
-    sink(f.a());
-    sink(f.b());
+    sink(f.a()); // flow (through `f` and `h`) [NOT DETECTED]
+    sink(f.b()); // flow (through `g` and `h`) [NOT DETECTED]
 }
 
 void foo()
@@ -48,4 +48,4 @@ void foo()
     // Nothing should alert
     bar(i);
 }
-}; // namespace Constructors
\ No newline at end of file
+}; // namespace Constructors
diff --git a/cpp/ql/test/library-tests/dataflow/fields/flow.expected b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
index e9cb5465b3d7..2f5ea14557d7 100644
--- a/cpp/ql/test/library-tests/dataflow/fields/flow.expected
+++ b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
@@ -89,6 +89,22 @@ edges
 | C.cpp:24:16:24:25 | new [void] | C.cpp:24:5:24:25 | ... = ... [void] |
 | C.cpp:27:8:27:11 | `this` parameter in func [s3, ... (1)] | file://:0:0:0:0 | this [s3, ... (1)] |
 | file://:0:0:0:0 | this [s3, ... (1)] | C.cpp:31:10:31:11 | s3 |
+| simple.cpp:26:15:26:15 | f [a_, ... (1)] | simple.cpp:28:10:28:10 | f [a_, ... (1)] |
+| simple.cpp:26:15:26:15 | f [b_, ... (1)] | simple.cpp:29:10:29:10 | f [b_, ... (1)] |
+| simple.cpp:28:10:28:10 | f [a_, ... (1)] | simple.cpp:28:12:28:12 | call to a |
+| simple.cpp:29:10:29:10 | f [b_, ... (1)] | simple.cpp:29:12:29:12 | call to b |
+| simple.cpp:39:5:39:5 | f [post update] [a_, ... (1)] | simple.cpp:45:9:45:9 | f [a_, ... (1)] |
+| simple.cpp:39:12:39:21 | call to user_input [void] | simple.cpp:39:5:39:5 | f [post update] [a_, ... (1)] |
+| simple.cpp:40:5:40:5 | g [post update] [b_, ... (1)] | simple.cpp:48:9:48:9 | g [b_, ... (1)] |
+| simple.cpp:40:12:40:21 | call to user_input [void] | simple.cpp:40:5:40:5 | g [post update] [b_, ... (1)] |
+| simple.cpp:41:5:41:5 | h [post update] [a_, ... (1)] | simple.cpp:51:9:51:9 | h [a_, ... (1)] |
+| simple.cpp:41:12:41:21 | call to user_input [void] | simple.cpp:41:5:41:5 | h [post update] [a_, ... (1)] |
+| simple.cpp:42:5:42:5 | h [post update] [b_, ... (1)] | simple.cpp:51:9:51:9 | h [b_, ... (1)] |
+| simple.cpp:42:12:42:21 | call to user_input [void] | simple.cpp:42:5:42:5 | h [post update] [b_, ... (1)] |
+| simple.cpp:45:9:45:9 | f [a_, ... (1)] | simple.cpp:26:15:26:15 | f [a_, ... (1)] |
+| simple.cpp:48:9:48:9 | g [b_, ... (1)] | simple.cpp:26:15:26:15 | f [b_, ... (1)] |
+| simple.cpp:51:9:51:9 | h [a_, ... (1)] | simple.cpp:26:15:26:15 | f [a_, ... (1)] |
+| simple.cpp:51:9:51:9 | h [b_, ... (1)] | simple.cpp:26:15:26:15 | f [b_, ... (1)] |
 #select
 | A.cpp:43:10:43:12 | & ... | A.cpp:41:15:41:21 | new [void] | A.cpp:43:10:43:12 | & ... | & ... flows from $@ | A.cpp:41:15:41:21 | new [void] | new [void] |
 | A.cpp:49:13:49:13 | c | A.cpp:47:12:47:18 | new [void] | A.cpp:49:13:49:13 | c | c flows from $@ | A.cpp:47:12:47:18 | new [void] | new [void] |
@@ -110,3 +126,7 @@ edges
 | B.cpp:18:20:18:24 | elem1 | B.cpp:15:15:15:27 | new [void] | B.cpp:18:20:18:24 | elem1 | elem1 flows from $@ | B.cpp:15:15:15:27 | new [void] | new [void] |
 | B.cpp:19:20:19:24 | elem2 | B.cpp:15:15:15:27 | new [void] | B.cpp:19:20:19:24 | elem2 | elem2 flows from $@ | B.cpp:15:15:15:27 | new [void] | new [void] |
 | C.cpp:31:10:31:11 | s3 | C.cpp:24:16:24:25 | new [void] | C.cpp:31:10:31:11 | s3 | s3 flows from $@ | C.cpp:24:16:24:25 | new [void] | new [void] |
+| simple.cpp:28:12:28:12 | call to a | simple.cpp:39:12:39:21 | call to user_input [void] | simple.cpp:28:12:28:12 | call to a | call to a flows from $@ | simple.cpp:39:12:39:21 | call to user_input [void] | call to user_input [void] |
+| simple.cpp:28:12:28:12 | call to a | simple.cpp:41:12:41:21 | call to user_input [void] | simple.cpp:28:12:28:12 | call to a | call to a flows from $@ | simple.cpp:41:12:41:21 | call to user_input [void] | call to user_input [void] |
+| simple.cpp:29:12:29:12 | call to b | simple.cpp:40:12:40:21 | call to user_input [void] | simple.cpp:29:12:29:12 | call to b | call to b flows from $@ | simple.cpp:40:12:40:21 | call to user_input [void] | call to user_input [void] |
+| simple.cpp:29:12:29:12 | call to b | simple.cpp:42:12:42:21 | call to user_input [void] | simple.cpp:29:12:29:12 | call to b | call to b flows from $@ | simple.cpp:42:12:42:21 | call to user_input [void] | call to user_input [void] |
diff --git a/cpp/ql/test/library-tests/dataflow/fields/simple.cpp b/cpp/ql/test/library-tests/dataflow/fields/simple.cpp
index 1d75fe4e850f..6f36fa6551df 100644
--- a/cpp/ql/test/library-tests/dataflow/fields/simple.cpp
+++ b/cpp/ql/test/library-tests/dataflow/fields/simple.cpp
@@ -25,8 +25,8 @@ class Foo
 
 void bar(Foo &f)
 {
-    sink(f.a());
-    sink(f.b());
+    sink(f.a()); // flow (through `f.setA` and `h.setA`)
+    sink(f.b()); // flow (through `g.setB` and `h.setB`)
 }
 
 void foo()
@@ -53,4 +53,4 @@ void foo()
     // Nothing should alert
     bar(i);
 }
-} // namespace Simple
\ No newline at end of file
+} // namespace Simple
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
index ac9d34d7cfd3..a8346219c267 100644
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
@@ -1,3 +1,4 @@
+| file://:0:0:0:0 | this | file://:0:0:0:0 | this |  |
 | file://:0:0:0:0 | this | taint.cpp:72:3:72:3 | c | TAINT |
 | file://:0:0:0:0 | this | taint.cpp:73:3:73:3 | d | TAINT |
 | file://:0:0:0:0 | this | taint.cpp:77:3:77:3 | d | TAINT |
@@ -43,10 +44,13 @@
 | taint.cpp:37:12:37:20 | call to increment | taint.cpp:43:7:43:13 | global9 |  |
 | taint.cpp:38:13:38:16 | call to zero | taint.cpp:38:2:38:26 | ... = ... |  |
 | taint.cpp:38:13:38:16 | call to zero | taint.cpp:44:7:44:14 | global10 |  |
+| taint.cpp:71:2:71:8 | `this` parameter in MyClass | file://:0:0:0:0 | this |  |
 | taint.cpp:71:14:71:17 | 0 | taint.cpp:71:14:71:17 | constructor init of field a | TAINT |
 | taint.cpp:71:22:71:27 | call to source | taint.cpp:71:20:71:30 | constructor init of field b | TAINT |
+| taint.cpp:72:3:72:3 | this [post update] | file://:0:0:0:0 | this |  |
 | taint.cpp:72:7:72:12 | call to source | taint.cpp:72:3:72:14 | ... = ... |  |
 | taint.cpp:73:7:73:7 | 0 | taint.cpp:73:3:73:7 | ... = ... |  |
+| taint.cpp:76:7:76:14 | `this` parameter in myMethod | file://:0:0:0:0 | this |  |
 | taint.cpp:77:7:77:12 | call to source | taint.cpp:77:3:77:14 | ... = ... |  |
 | taint.cpp:84:10:84:12 | call to MyClass | taint.cpp:86:2:86:4 | mc1 |  |
 | taint.cpp:84:10:84:12 | call to MyClass | taint.cpp:88:7:88:9 | mc1 |  |
@@ -57,6 +61,10 @@
 | taint.cpp:84:15:84:17 | call to MyClass | taint.cpp:93:7:93:9 | mc2 |  |
 | taint.cpp:84:15:84:17 | call to MyClass | taint.cpp:94:7:94:9 | mc2 |  |
 | taint.cpp:84:15:84:17 | call to MyClass | taint.cpp:95:7:95:9 | mc2 |  |
+| taint.cpp:86:2:86:4 | mc1 [post update] | taint.cpp:88:7:88:9 | mc1 |  |
+| taint.cpp:86:2:86:4 | mc1 [post update] | taint.cpp:89:7:89:9 | mc1 |  |
+| taint.cpp:86:2:86:4 | mc1 [post update] | taint.cpp:90:7:90:9 | mc1 |  |
+| taint.cpp:86:2:86:4 | mc1 [post update] | taint.cpp:91:7:91:9 | mc1 |  |
 | taint.cpp:88:7:88:9 | mc1 | taint.cpp:88:11:88:11 | a | TAINT |
 | taint.cpp:89:7:89:9 | mc1 | taint.cpp:89:11:89:11 | b | TAINT |
 | taint.cpp:90:7:90:9 | mc1 | taint.cpp:90:11:90:11 | c | TAINT |
@@ -132,31 +140,35 @@
 | taint.cpp:165:22:165:25 | {...} | taint.cpp:172:10:172:15 | buffer |  |
 | taint.cpp:165:22:165:25 | {...} | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:165:24:165:24 | 0 | taint.cpp:165:22:165:25 | {...} | TAINT |
+| taint.cpp:168:8:168:14 | tainted [post update] | taint.cpp:172:18:172:24 | tainted |  |
 | taint.cpp:170:10:170:15 | buffer | taint.cpp:170:3:170:8 | call to strcpy |  |
-| taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:170:10:170:15 | buffer |  |
+| taint.cpp:170:10:170:15 | buffer [post update] | taint.cpp:170:3:170:8 | call to strcpy |  |
+| taint.cpp:170:10:170:15 | buffer [post update] | taint.cpp:171:8:171:13 | buffer |  |
+| taint.cpp:170:10:170:15 | buffer [post update] | taint.cpp:172:10:172:15 | buffer |  |
+| taint.cpp:170:10:170:15 | buffer [post update] | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:171:8:171:13 | buffer |  |
 | taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:172:10:172:15 | buffer |  |
 | taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:170:18:170:26 | Hello,  | taint.cpp:170:10:170:15 | ref arg buffer | TAINT |
-| taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:171:8:171:13 | buffer |  |
+| taint.cpp:171:8:171:13 | buffer [post update] | taint.cpp:172:10:172:15 | buffer |  |
+| taint.cpp:171:8:171:13 | buffer [post update] | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:172:10:172:15 | buffer |  |
 | taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:172:10:172:15 | buffer | taint.cpp:172:3:172:8 | call to strcat |  |
 | taint.cpp:172:10:172:15 | buffer | taint.cpp:172:10:172:15 | ref arg buffer | TAINT |
-| taint.cpp:172:10:172:15 | ref arg buffer | taint.cpp:172:10:172:15 | buffer |  |
+| taint.cpp:172:10:172:15 | buffer [post update] | taint.cpp:172:3:172:8 | call to strcat |  |
+| taint.cpp:172:10:172:15 | buffer [post update] | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:172:10:172:15 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:172:18:172:24 | tainted | taint.cpp:172:10:172:15 | ref arg buffer | TAINT |
-| taint.cpp:173:8:173:13 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:180:19:180:19 | p | taint.cpp:181:9:181:9 | p |  |
 | taint.cpp:181:9:181:9 | p | taint.cpp:181:8:181:9 | * ... | TAINT |
 | taint.cpp:185:11:185:16 | call to source | taint.cpp:186:11:186:11 | x |  |
-| taint.cpp:186:10:186:11 | ref arg & ... | taint.cpp:186:11:186:11 | x |  |
 | taint.cpp:186:11:186:11 | x | taint.cpp:186:10:186:11 | & ... | TAINT |
 | taint.cpp:192:23:192:28 | source | taint.cpp:194:13:194:18 | source |  |
 | taint.cpp:193:6:193:6 | x | taint.cpp:194:10:194:10 | x |  |
 | taint.cpp:193:6:193:6 | x | taint.cpp:195:7:195:7 | x |  |
 | taint.cpp:194:9:194:10 | & ... | taint.cpp:194:2:194:7 | call to memcpy |  |
-| taint.cpp:194:9:194:10 | ref arg & ... | taint.cpp:194:10:194:10 | x |  |
+| taint.cpp:194:9:194:10 | & ... [post update] | taint.cpp:194:2:194:7 | call to memcpy |  |
 | taint.cpp:194:9:194:10 | ref arg & ... | taint.cpp:195:7:195:7 | x |  |
 | taint.cpp:194:10:194:10 | x | taint.cpp:194:9:194:10 | & ... | TAINT |
 | taint.cpp:194:13:194:18 | source | taint.cpp:194:9:194:10 | ref arg & ... | TAINT |
@@ -169,9 +181,9 @@
 | taint.cpp:208:6:208:6 | 0 | taint.cpp:211:7:211:7 | y |  |
 | taint.cpp:208:6:208:6 | 0 | taint.cpp:213:15:213:15 | y |  |
 | taint.cpp:208:6:208:6 | 0 | taint.cpp:216:7:216:7 | y |  |
-| taint.cpp:213:12:213:12 | ref arg x | taint.cpp:213:12:213:12 | x |  |
 | taint.cpp:213:12:213:12 | ref arg x | taint.cpp:215:7:215:7 | x |  |
 | taint.cpp:213:12:213:12 | x | taint.cpp:213:15:213:15 | ref arg y |  |
-| taint.cpp:213:15:213:15 | ref arg y | taint.cpp:213:15:213:15 | y |  |
+| taint.cpp:213:12:213:12 | x [post update] | taint.cpp:215:7:215:7 | x |  |
 | taint.cpp:213:15:213:15 | ref arg y | taint.cpp:216:7:216:7 | y |  |
 | taint.cpp:213:15:213:15 | y | taint.cpp:213:12:213:12 | ref arg x |  |
+| taint.cpp:213:15:213:15 | y [post update] | taint.cpp:216:7:216:7 | y |  |
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp
index 90d043dd45b6..e764fbd784df 100644
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp
@@ -88,7 +88,7 @@ void class_field_test() {
 	sink(mc1.a);
 	sink(mc1.b); // tainted [NOT DETECTED]
 	sink(mc1.c); // tainted [NOT DETECTED]
-	sink(mc1.d); // tainted [NOT DETECTED]
+	sink(mc1.d); // tainted [NOT DETECTED with IR]
 	sink(mc2.a);
 	sink(mc2.b); // tainted [NOT DETECTED]
 	sink(mc2.c); // tainted [NOT DETECTED]
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected
index dd03a3b49c52..90b1d9814af9 100644
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected
@@ -4,6 +4,7 @@
 | taint.cpp:41:7:41:13 | global7 | taint.cpp:35:12:35:17 | call to source |
 | taint.cpp:42:7:42:13 | global8 | taint.cpp:35:12:35:17 | call to source |
 | taint.cpp:43:7:43:13 | global9 | taint.cpp:37:22:37:27 | call to source |
+| taint.cpp:91:11:91:11 | d | taint.cpp:77:7:77:12 | call to source |
 | taint.cpp:129:7:129:9 | * ... | taint.cpp:120:11:120:16 | call to source |
 | taint.cpp:134:7:134:9 | * ... | taint.cpp:120:11:120:16 | call to source |
 | taint.cpp:137:7:137:9 | * ... | taint.cpp:120:11:120:16 | call to source |
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected
index c43a3511ee89..8a9fc7676137 100644
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected
@@ -1,6 +1,7 @@
 | taint.cpp:41:7:41:13 | taint.cpp:35:12:35:17 | AST only |
 | taint.cpp:42:7:42:13 | taint.cpp:35:12:35:17 | AST only |
 | taint.cpp:43:7:43:13 | taint.cpp:37:22:37:27 | AST only |
+| taint.cpp:91:11:91:11 | taint.cpp:77:7:77:12 | AST only |
 | taint.cpp:130:7:130:9 | taint.cpp:127:8:127:13 | IR only |
 | taint.cpp:137:7:137:9 | taint.cpp:120:11:120:16 | AST only |
 | taint.cpp:173:8:173:13 | taint.cpp:164:19:164:24 | AST only |

From 5370e7d69310e13305511bd4f86c13e310de9d07 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Wed, 7 Aug 2019 15:32:07 +0200
Subject: [PATCH 18/28] C++: Remove TThisVar

There's no need to model `this` as a variable because it's never
reassigned.
---
 .../code/cpp/dataflow/internal/FlowVar.qll    | 33 -------------------
 1 file changed, 33 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
index 432cb54d212e..5c849fa1b2fd 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
@@ -220,11 +220,6 @@ module FlowVar_internal {
         or
         blockVarDefinedByVariable(sbb, v)
       )
-    } or
-    TThisVar(SubBasicBlock sbb, ThisExpr t) {
-      reachable(sbb) and
-      sbb = any(PartialDefinitions::PartialDefinition p | p.partiallyDefinesThis(t))
-            .getSubBasicBlockStart()
     }
 
   /**
@@ -374,34 +369,6 @@ module FlowVar_internal {
     override Location getLocation() { result = sbb.getStart().getLocation() }
   }
 
-  class ThisVar extends TThisVar, FlowVar {
-    SubBasicBlock sbb;
-
-    ThisExpr t;
-
-    PartialDefinitions::PartialDefinition pd;
-
-    ThisVar() { this = TThisVar(sbb, t) and pd.partiallyDefinesThis(t) }
-
-    override VariableAccess getAnAccess() {
-      none() // TODO: Widen type to `Expr`.
-    }
-
-    override predicate definedByExpr(Expr e, ControlFlowNode node) { none() }
-
-    override predicate definedByReference(Expr arg) { none() }
-
-    override predicate definedPartiallyAt(Expr arg) {
-      none() // TODO
-    }
-
-    override predicate definedByInitialValue(LocalScopeVariable param) { none() }
-
-    override string toString() { result = pd.toString() }
-
-    override Location getLocation() { result = pd.getLocation() }
-  }
-
   /** Type-specialized version of `getEnclosingElement`. */
   private ControlFlowNode getCFNParent(ControlFlowNode node) {
     result = node.getEnclosingElement()

From 98d6f3cadaedff65aed255678a43f821c79ccb66 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Thu, 8 Aug 2019 10:16:25 +0200
Subject: [PATCH 19/28] C++: Unify partial def and def-by-ref

This removes a lot of flow steps, but it all seems to be flow that was
present twice: both exiting a `PartialDefNode` and a
`DefinitionByReferenceNode`. All `DefinitionByReferenceNode`s are now
`PartialDefNode`s.
---
 .../cpp/dataflow/internal/DataFlowUtil.qll    | 13 +++--
 .../code/cpp/dataflow/internal/FlowVar.qll    | 57 +++++++++----------
 .../dataflow-tests/localFlow.expected         |  4 +-
 .../dataflow/fields/flow.expected             |  9 ---
 .../dataflow/taint-tests/localTaint.expected  | 15 +----
 5 files changed, 40 insertions(+), 58 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index ba608c3e1d1c..73cfaf219dac 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -13,9 +13,6 @@ private newtype TNode =
   TPostConstructorCallNode(ConstructorCall call) or
   TExplicitParameterNode(Parameter p) { exists(p.getFunction().getBlock()) } or
   TInstanceParameterNode(MemberFunction f) { exists(f.getBlock()) and not f.isStatic() } or
-  TDefinitionByReferenceNode(VariableAccess va, Expr argument) {
-    definitionByReference(va, argument)
-  } or
   TUninitializedNode(LocalVariable v) { not v.hasInitializer() }
 
 /**
@@ -152,12 +149,18 @@ class ImplicitParameterNode extends ParameterNode, TInstanceParameterNode {
  * `DefinitionByReferenceNode` to represent the value of `x` after the call has
  * returned. This node will have its `getArgument()` equal to `&x`.
  */
-class DefinitionByReferenceNode extends Node, TDefinitionByReferenceNode {
+class DefinitionByReferenceNode extends PartialDefNode {
   VariableAccess va;
 
   Expr argument;
 
-  DefinitionByReferenceNode() { this = TDefinitionByReferenceNode(va, argument) }
+  DefinitionByReferenceNode() {
+    exists(DefinitionByReference def |
+      def = this.getPartialDefinition() and
+      argument = def.getDefinedExpr() and
+      va = def.getVariableAccess()
+    )
+  }
 
   override Function getFunction() { result = va.getEnclosingFunction() }
 
diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
index 5c849fa1b2fd..712bbb72c731 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
@@ -93,27 +93,20 @@ private module PartialDefinitions {
       isInstanceFieldWrite(fa) and qualifier = fa.getQualifier()
     } or
     TExplicitCallQualifier(Expr qualifier, Call call) { qualifier = call.getQualifier() } or
-    TReferenceArgument(Expr arg, Call call, int i) { isReferenceOrPointerArg(arg, call, i) }
+    TReferenceArgument(Expr arg, VariableAccess va) { definitionByReference(va, arg) }
 
   private predicate isInstanceFieldWrite(FieldAccess fa) {
     not fa.getTarget().isStatic() and
     assignmentLikeOperation(_, fa.getTarget(), fa, _)
   }
 
-  private predicate isReferenceOrPointerArg(Expr arg, Call call, int i) {
-    arg = call.getArgument(i) and
-    exists(Type t | t = arg.getFullyConverted().getType().getUnspecifiedType() |
-      t instanceof ReferenceType or t instanceof PointerType
-    )
-  }
-
   class PartialDefinition extends TPartialDefinition {
     Expr definedExpr;
 
     PartialDefinition() {
       this = TExplicitFieldStoreQualifier(definedExpr, _) or
       this = TExplicitCallQualifier(definedExpr, _) or
-      this = TReferenceArgument(definedExpr, _, _)
+      this = TReferenceArgument(definedExpr, _)
     }
 
     predicate partiallyDefines(Variable v) { definedExpr = v.getAnAccess() }
@@ -135,6 +128,22 @@ private module PartialDefinitions {
 
     string toString() { result = "partial def of " + definedExpr }
   }
+
+  /**
+   * A partial definition that's a definition by reference (in the sense of the
+   * `definitionByReference` predicate).
+   */
+  class DefinitionByReference extends PartialDefinition, TReferenceArgument {
+    VariableAccess va;
+
+    DefinitionByReference() { definitionByReference(va, definedExpr) }
+
+    VariableAccess getVariableAccess() { result = va }
+
+    override predicate partiallyDefines(Variable v) { va = v.getAnAccess() }
+
+    override ControlFlowNode getSubBasicBlockStart() { result = va }
+  }
 }
 import PartialDefinitions
 private import FlowVar_internal
@@ -159,13 +168,8 @@ module FlowVar_internal {
    */
   predicate fullySupportedSsaVariable(Variable v) {
     v = any(SsaDefinition def).getAVariable() and
-    // After `foo(&x.field)` we need for there to be two definitions of `x`:
-    // the one that existed before the call to `foo` and the def-by-ref from
-    // the call. It's fundamental in SSA that each use is only associated with
-    // one def, so we can't easily get this effect with SSA.
-    not definitionByReference(v.getAnAccess(), _) and
     // A partially-defined variable is handled using the partial definitions logic.
-    not any(PartialDefinitions::PartialDefinition p).partiallyDefines(v) and
+    not any(PartialDefinition p).partiallyDefines(v) and
     // SSA variables do not exist before their first assignment, but one
     // feature of this data flow library is to track where uninitialized data
     // ends up.
@@ -213,10 +217,7 @@ module FlowVar_internal {
         or
         assignmentLikeOperation(sbb, v, _, _)
         or
-        blockVarDefinedByReference(sbb, v, _)
-        or
-        sbb = any(PartialDefinitions::PartialDefinition p | p.partiallyDefines(v))
-              .getSubBasicBlockStart()
+        sbb = any(PartialDefinition p | p.partiallyDefines(v)).getSubBasicBlockStart()
         or
         blockVarDefinedByVariable(sbb, v)
       )
@@ -328,11 +329,15 @@ module FlowVar_internal {
     }
 
     override predicate definedByReference(Expr arg) {
-      blockVarDefinedByReference(sbb, v, arg)
+      exists(DefinitionByReference def |
+        def.partiallyDefines(v) and
+        sbb = def.getSubBasicBlockStart() and
+        arg = def.getDefinedExpr()
+      )
     }
 
     override predicate definedPartiallyAt(Expr e) {
-      exists(PartialDefinitions::PartialDefinition p |
+      exists(PartialDefinition p |
         p.partiallyDefines(v) and
         sbb = p.getSubBasicBlockStart() and
         e = p.getDefinedExpr()
@@ -632,11 +637,6 @@ module FlowVar_internal {
     )
   }
 
-  predicate blockVarDefinedByReference(ControlFlowNode node, Variable v, Expr argument) {
-    node = v.getAnAccess() and
-    definitionByReference(node, argument)
-  }
-
   /**
    * Holds if `v` is initialized by `init` to have value `assignedExpr`.
    */
@@ -660,10 +660,7 @@ module FlowVar_internal {
       |
         assignmentLikeOperation(this, v, _, _)
         or
-        blockVarDefinedByReference(this, v, _)
-        or
-        this = any(PartialDefinitions::PartialDefinition p | p.partiallyDefines(v))
-              .getSubBasicBlockStart()
+        this = any(PartialDefinition p | p.partiallyDefines(v)).getSubBasicBlockStart()
         // It is not necessary to cut the basic blocks at `Initializer` nodes
         // because the affected variable can have no _other_ value before its
         // initializer. It is not necessary to cut basic blocks at procedure
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
index 9ff45c0e172f..a14cc1d09292 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
@@ -42,7 +42,7 @@
 | test.cpp:431:12:431:13 | 0 | test.cpp:432:33:432:35 | tmp |
 | test.cpp:431:12:431:13 | 0 | test.cpp:433:8:433:10 | tmp |
 | test.cpp:432:10:432:13 | & ... | test.cpp:432:3:432:8 | call to memcpy |
-| test.cpp:432:10:432:13 | & ... [post update] | test.cpp:432:3:432:8 | call to memcpy |
+| test.cpp:432:10:432:13 | ref arg & ... | test.cpp:432:3:432:8 | call to memcpy |
 | test.cpp:432:10:432:13 | ref arg & ... | test.cpp:432:33:432:35 | tmp |
 | test.cpp:432:10:432:13 | ref arg & ... | test.cpp:433:8:433:10 | tmp |
 | test.cpp:432:17:432:23 | source1 | test.cpp:432:10:432:13 | ref arg & ... |
@@ -54,7 +54,7 @@
 | test.cpp:437:12:437:13 | 0 | test.cpp:440:8:440:10 | tmp |
 | test.cpp:437:12:437:13 | 0 | test.cpp:442:10:442:12 | tmp |
 | test.cpp:439:10:439:13 | & ... | test.cpp:439:3:439:8 | call to memcpy |
-| test.cpp:439:10:439:13 | & ... [post update] | test.cpp:439:3:439:8 | call to memcpy |
+| test.cpp:439:10:439:13 | ref arg & ... | test.cpp:439:3:439:8 | call to memcpy |
 | test.cpp:439:10:439:13 | ref arg & ... | test.cpp:439:33:439:35 | tmp |
 | test.cpp:439:10:439:13 | ref arg & ... | test.cpp:440:8:440:10 | tmp |
 | test.cpp:439:10:439:13 | ref arg & ... | test.cpp:442:10:442:12 | tmp |
diff --git a/cpp/ql/test/library-tests/dataflow/fields/flow.expected b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
index 2f5ea14557d7..99c52b8f2f31 100644
--- a/cpp/ql/test/library-tests/dataflow/fields/flow.expected
+++ b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
@@ -24,12 +24,7 @@ edges
 | A.cpp:103:14:103:14 | c [a, ... (1)] | A.cpp:120:12:120:13 | c1 [a, ... (1)] |
 | A.cpp:107:12:107:13 | c1 [a, ... (1)] | A.cpp:107:16:107:16 | a |
 | A.cpp:120:12:120:13 | c1 [a, ... (1)] | A.cpp:120:16:120:16 | a |
-| A.cpp:126:5:126:5 | b [post update] [c, ... (1)] | A.cpp:131:8:131:8 | b [post update] [c, ... (1)] |
-| A.cpp:126:12:126:18 | new [void] | A.cpp:126:5:126:5 | b [post update] [c, ... (1)] |
-| A.cpp:131:8:131:8 | b [post update] [c, ... (1)] | A.cpp:132:10:132:10 | b [c, ... (1)] |
-| A.cpp:132:10:132:10 | b [c, ... (1)] | A.cpp:132:13:132:13 | c |
 | A.cpp:142:7:142:7 | b [post update] [c, ... (1)] | A.cpp:143:7:143:31 | ... = ... [c, ... (1)] |
-| A.cpp:142:7:142:7 | b [post update] [c, ... (1)] | A.cpp:151:18:151:18 | b [post update] [c, ... (1)] |
 | A.cpp:142:7:142:20 | ... = ... [void] | A.cpp:142:7:142:7 | b [post update] [c, ... (1)] |
 | A.cpp:142:14:142:20 | new [void] | A.cpp:142:7:142:20 | ... = ... [void] |
 | A.cpp:143:7:143:10 | this [post update] [b, ... (1)] | A.cpp:151:12:151:24 | call to D [post update] [b, ... (1)] |
@@ -40,12 +35,10 @@ edges
 | A.cpp:150:12:150:18 | new [void] | A.cpp:151:18:151:18 | b [void] |
 | A.cpp:151:12:151:24 | call to D [post update] [b, ... (1)] | A.cpp:152:10:152:10 | d [b, ... (1)] |
 | A.cpp:151:12:151:24 | call to D [post update] [b, ... (2)] | A.cpp:153:10:153:10 | d [b, ... (2)] |
-| A.cpp:151:18:151:18 | b [post update] [c, ... (1)] | A.cpp:154:10:154:10 | b [c, ... (1)] |
 | A.cpp:151:18:151:18 | b [void] | A.cpp:151:12:151:24 | call to D [post update] [b, ... (1)] |
 | A.cpp:152:10:152:10 | d [b, ... (1)] | A.cpp:152:13:152:13 | b |
 | A.cpp:153:10:153:10 | d [b, ... (2)] | A.cpp:153:13:153:13 | b [c, ... (1)] |
 | A.cpp:153:13:153:13 | b [c, ... (1)] | A.cpp:153:16:153:16 | c |
-| A.cpp:154:10:154:10 | b [c, ... (1)] | A.cpp:154:13:154:13 | c |
 | A.cpp:159:12:159:18 | new [void] | A.cpp:160:29:160:29 | b [void] |
 | A.cpp:160:18:160:60 | call to MyList [post update] [head, ... (1)] | A.cpp:161:38:161:39 | l1 [head, ... (1)] |
 | A.cpp:160:29:160:29 | b [void] | A.cpp:160:18:160:60 | call to MyList [post update] [head, ... (1)] |
@@ -114,11 +107,9 @@ edges
 | A.cpp:75:14:75:14 | c | A.cpp:73:25:73:32 | new [void] | A.cpp:75:14:75:14 | c | c flows from $@ | A.cpp:73:25:73:32 | new [void] | new [void] |
 | A.cpp:107:16:107:16 | a | A.cpp:98:12:98:18 | new [void] | A.cpp:107:16:107:16 | a | a flows from $@ | A.cpp:98:12:98:18 | new [void] | new [void] |
 | A.cpp:120:16:120:16 | a | A.cpp:98:12:98:18 | new [void] | A.cpp:120:16:120:16 | a | a flows from $@ | A.cpp:98:12:98:18 | new [void] | new [void] |
-| A.cpp:132:13:132:13 | c | A.cpp:126:12:126:18 | new [void] | A.cpp:132:13:132:13 | c | c flows from $@ | A.cpp:126:12:126:18 | new [void] | new [void] |
 | A.cpp:152:13:152:13 | b | A.cpp:143:25:143:31 | new [void] | A.cpp:152:13:152:13 | b | b flows from $@ | A.cpp:143:25:143:31 | new [void] | new [void] |
 | A.cpp:152:13:152:13 | b | A.cpp:150:12:150:18 | new [void] | A.cpp:152:13:152:13 | b | b flows from $@ | A.cpp:150:12:150:18 | new [void] | new [void] |
 | A.cpp:153:16:153:16 | c | A.cpp:142:14:142:20 | new [void] | A.cpp:153:16:153:16 | c | c flows from $@ | A.cpp:142:14:142:20 | new [void] | new [void] |
-| A.cpp:154:13:154:13 | c | A.cpp:142:14:142:20 | new [void] | A.cpp:154:13:154:13 | c | c flows from $@ | A.cpp:142:14:142:20 | new [void] | new [void] |
 | A.cpp:165:26:165:29 | head | A.cpp:159:12:159:18 | new [void] | A.cpp:165:26:165:29 | head | head flows from $@ | A.cpp:159:12:159:18 | new [void] | new [void] |
 | A.cpp:169:15:169:18 | head | A.cpp:159:12:159:18 | new [void] | A.cpp:169:15:169:18 | head | head flows from $@ | A.cpp:159:12:159:18 | new [void] | new [void] |
 | B.cpp:9:20:9:24 | elem1 | B.cpp:6:15:6:24 | new [void] | B.cpp:9:20:9:24 | elem1 | elem1 flows from $@ | B.cpp:6:15:6:24 | new [void] | new [void] |
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
index a8346219c267..875e5619c840 100644
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
@@ -140,24 +140,17 @@
 | taint.cpp:165:22:165:25 | {...} | taint.cpp:172:10:172:15 | buffer |  |
 | taint.cpp:165:22:165:25 | {...} | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:165:24:165:24 | 0 | taint.cpp:165:22:165:25 | {...} | TAINT |
-| taint.cpp:168:8:168:14 | tainted [post update] | taint.cpp:172:18:172:24 | tainted |  |
 | taint.cpp:170:10:170:15 | buffer | taint.cpp:170:3:170:8 | call to strcpy |  |
-| taint.cpp:170:10:170:15 | buffer [post update] | taint.cpp:170:3:170:8 | call to strcpy |  |
-| taint.cpp:170:10:170:15 | buffer [post update] | taint.cpp:171:8:171:13 | buffer |  |
-| taint.cpp:170:10:170:15 | buffer [post update] | taint.cpp:172:10:172:15 | buffer |  |
-| taint.cpp:170:10:170:15 | buffer [post update] | taint.cpp:173:8:173:13 | buffer |  |
+| taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:170:3:170:8 | call to strcpy |  |
 | taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:171:8:171:13 | buffer |  |
 | taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:172:10:172:15 | buffer |  |
 | taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:170:18:170:26 | Hello,  | taint.cpp:170:10:170:15 | ref arg buffer | TAINT |
-| taint.cpp:171:8:171:13 | buffer [post update] | taint.cpp:172:10:172:15 | buffer |  |
-| taint.cpp:171:8:171:13 | buffer [post update] | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:172:10:172:15 | buffer |  |
 | taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:172:10:172:15 | buffer | taint.cpp:172:3:172:8 | call to strcat |  |
 | taint.cpp:172:10:172:15 | buffer | taint.cpp:172:10:172:15 | ref arg buffer | TAINT |
-| taint.cpp:172:10:172:15 | buffer [post update] | taint.cpp:172:3:172:8 | call to strcat |  |
-| taint.cpp:172:10:172:15 | buffer [post update] | taint.cpp:173:8:173:13 | buffer |  |
+| taint.cpp:172:10:172:15 | ref arg buffer | taint.cpp:172:3:172:8 | call to strcat |  |
 | taint.cpp:172:10:172:15 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:172:18:172:24 | tainted | taint.cpp:172:10:172:15 | ref arg buffer | TAINT |
 | taint.cpp:180:19:180:19 | p | taint.cpp:181:9:181:9 | p |  |
@@ -168,7 +161,7 @@
 | taint.cpp:193:6:193:6 | x | taint.cpp:194:10:194:10 | x |  |
 | taint.cpp:193:6:193:6 | x | taint.cpp:195:7:195:7 | x |  |
 | taint.cpp:194:9:194:10 | & ... | taint.cpp:194:2:194:7 | call to memcpy |  |
-| taint.cpp:194:9:194:10 | & ... [post update] | taint.cpp:194:2:194:7 | call to memcpy |  |
+| taint.cpp:194:9:194:10 | ref arg & ... | taint.cpp:194:2:194:7 | call to memcpy |  |
 | taint.cpp:194:9:194:10 | ref arg & ... | taint.cpp:195:7:195:7 | x |  |
 | taint.cpp:194:10:194:10 | x | taint.cpp:194:9:194:10 | & ... | TAINT |
 | taint.cpp:194:13:194:18 | source | taint.cpp:194:9:194:10 | ref arg & ... | TAINT |
@@ -183,7 +176,5 @@
 | taint.cpp:208:6:208:6 | 0 | taint.cpp:216:7:216:7 | y |  |
 | taint.cpp:213:12:213:12 | ref arg x | taint.cpp:215:7:215:7 | x |  |
 | taint.cpp:213:12:213:12 | x | taint.cpp:213:15:213:15 | ref arg y |  |
-| taint.cpp:213:12:213:12 | x [post update] | taint.cpp:215:7:215:7 | x |  |
 | taint.cpp:213:15:213:15 | ref arg y | taint.cpp:216:7:216:7 | y |  |
 | taint.cpp:213:15:213:15 | y | taint.cpp:213:12:213:12 | ref arg x |  |
-| taint.cpp:213:15:213:15 | y [post update] | taint.cpp:216:7:216:7 | y |  |

From 0a13d7a33794a32b136014a87f7467c99b0bd0d7 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Thu, 8 Aug 2019 10:44:27 +0200
Subject: [PATCH 20/28] C++: PartialDefNode -> PartialDefinitionNode

---
 .../semmle/code/cpp/dataflow/internal/DataFlowUtil.qll | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index 73cfaf219dac..9eba5846fe56 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -9,7 +9,7 @@ private import semmle.code.cpp.models.interfaces.DataFlow
 cached
 private newtype TNode =
   TExprNode(Expr e) or
-  TPartialDefNode(PartialDefinition pd) or
+  TPartialDefinitionNode(PartialDefinition pd) or
   TPostConstructorCallNode(ConstructorCall call) or
   TExplicitParameterNode(Parameter p) { exists(p.getFunction().getBlock()) } or
   TInstanceParameterNode(MemberFunction f) { exists(f.getBlock()) and not f.isStatic() } or
@@ -52,7 +52,7 @@ class Node extends TNode {
    * a partial definition of `&x`).s
    */
   Expr asPartialDefinition() {
-    result = this.(PartialDefNode).getPartialDefinition().getDefinedExpr()
+    result = this.(PartialDefinitionNode).getPartialDefinition().getDefinedExpr()
   }
 
   /**
@@ -149,7 +149,7 @@ class ImplicitParameterNode extends ParameterNode, TInstanceParameterNode {
  * `DefinitionByReferenceNode` to represent the value of `x` after the call has
  * returned. This node will have its `getArgument()` equal to `&x`.
  */
-class DefinitionByReferenceNode extends PartialDefNode {
+class DefinitionByReferenceNode extends PartialDefinitionNode {
   VariableAccess va;
 
   Expr argument;
@@ -230,10 +230,10 @@ abstract class PostUpdateNode extends Node {
   override string toString() { result = getPreUpdateNode().toString() + " [post update]" }
 }
 
-class PartialDefNode extends PostUpdateNode, TPartialDefNode {
+class PartialDefinitionNode extends PostUpdateNode, TPartialDefinitionNode {
   PartialDefinition pd;
 
-  PartialDefNode() { this = TPartialDefNode(pd) }
+  PartialDefinitionNode() { this = TPartialDefinitionNode(pd) }
 
   override Node getPreUpdateNode() { result.asExpr() = pd.getDefinedExpr() }
 

From 2c6dbacd2b1b6099c00e8b08aec85d2645a9f2e3 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Thu, 8 Aug 2019 11:17:28 +0200
Subject: [PATCH 21/28] C++: Tidy up DataFlowUtil.qll

---
 .../code/cpp/dataflow/internal/DataFlowUtil.qll      | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index 9eba5846fe56..8ad7e76fbf76 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -331,8 +331,6 @@ predicate localFlowStep(Node nodeFrom, Node nodeTo) {
       or
       varSourceBaseCase(var, nodeFrom.asUninitialized())
       or
-      var.definedByReference(nodeFrom.asDefiningArgument())
-      or
       var.definedPartiallyAt(nodeFrom.asPartialDefinition())
     ) and
     varToExprStep(var, nodeTo.asExpr())
@@ -392,6 +390,16 @@ private predicate exprToExprStep_nocfg(Expr fromExpr, Expr toExpr) {
   or
   toExpr = any(StmtExpr stmtExpr | fromExpr = stmtExpr.getResultExpr())
   or
+  // The following case is needed to track the qualifier object for flow
+  // through fields. It gives flow from `T(x)` to `new T(x)`. That's not
+  // strictly _data_ flow but _taint_ flow because the type of `fromExpr` is
+  // `T` while the type of `toExpr` is `T*`.
+  //
+  // This discrepancy is an artifact of how `new`-expressions are represented
+  // in the database in a way that slightly varies from what the standard
+  // specifies. In the C++ standard, there is no constructor call expression
+  // `T(x)` after `new`. Instead there is a type `T` and an optional
+  // initializer `(x)`.
   toExpr.(NewExpr).getInitializer() = fromExpr
   or
   toExpr = any(Call call |

From 8aa24fe5c90556b3fd806948d4390c2d15b5fe5e Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Fri, 9 Aug 2019 15:06:19 +0200
Subject: [PATCH 22/28] C++: Improve QLDoc on assignmentLikeOperation

---
 .../semmle/code/cpp/dataflow/internal/FlowVar.qll    | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
index 712bbb72c731..895d08a92873 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
@@ -603,10 +603,14 @@ module FlowVar_internal {
   }
 
   /**
-   * Holds if `v` is modified as a side effect of evaluating `node`, receiving a
-   * value best described by `e`. This corresponds to `FlowVar::definedByExpr`,
-   * except that the case where `node instanceof Initializer` is covered by
-   * `initializer` instead of this predicate.
+   * Holds if `v` is modified through `va` as a side effect of evaluating
+   * `node`, receiving a value best described by `assignedExpr`.
+   * Assignment-like operations are those that desugar to a non-overloaded `=`
+   * assignment: `=`, `+=`, `++`, `--`, etc.
+   *
+   * This corresponds to `FlowVar::definedByExpr`, except that the case where
+   * `node instanceof Initializer` is covered by `initializer` instead of this
+   * predicate.
    */
   predicate assignmentLikeOperation(
     ControlFlowNode node, Variable v, VariableAccess va, Expr assignedExpr

From 0507d51f0cd3f3859980fff63eab539022ad7517 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Fri, 9 Aug 2019 15:09:51 +0200
Subject: [PATCH 23/28] C++: Prune getAReachedBlockVarSBB using live vars

On a Postgres snapshot, where the `getAReachedBlockVarSBB` predicate
performs badly because of a Yacc-generated 20,000-line parser loop, that
predicate is reduced from 4m22s to 1m32s plus 5.2s for the live
variables analysis.

This change removes 17,142 rows from `BlockVar.getAnAccess` on Postgres.
I sampled some of them, and they were all of the following form:

    while (...) {
      T x;
      f1(&x); // access
      f2(&x); // definition
    }

Such accesses are ruled out now because we deliberately lose track of
variables when they go out of scope.
---
 .../code/cpp/dataflow/internal/FlowVar.qll    | 35 +++++++++++++++++--
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
index 895d08a92873..1243bac78459 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
@@ -483,19 +483,48 @@ module FlowVar_internal {
     )
   }
 
-  pragma[nomagic]
+  // The noopt is needed to avoid getting `variableLiveInSBB` joined in before
+  // `getASuccessor`.
+  pragma[noopt]
   private SubBasicBlock getAReachedBlockVarSBB(TBlockVar start) {
-    start = TBlockVar(result, _)
+    exists(Variable v |
+      start = TBlockVar(result, v) and
+      variableLiveInSBB(result, v)
+    )
     or
     exists(SubBasicBlock mid, SubBasicBlock sbbDef, Variable v |
-      start = TBlockVar(sbbDef, v) and
       mid = getAReachedBlockVarSBB(start) and
+      start = TBlockVar(sbbDef, v) and
       result = mid.getASuccessor() and
+      variableLiveInSBB(result, v) and
       not skipLoop(mid, result, sbbDef, v) and
       not assignmentLikeOperation(result, v, _, _)
     )
   }
 
+  /**
+   * Holds if a value held in `v` at the start of `sbb` (or after the first
+   * assignment, if that assignment is to `v`) might later be read.
+   */
+  private predicate variableLiveInSBB(SubBasicBlock sbb, Variable v) {
+    variableAccessInSBB(v, sbb, _)
+    or
+    exists(SubBasicBlock succ | succ = sbb.getASuccessor() |
+      variableLiveInSBB(succ, v) and
+      not variableNotLiveBefore(succ, v)
+    )
+  }
+
+  /**
+   * Holds if liveness of `v` should stop propagating backwards from `sbb`.
+   */
+  private predicate variableNotLiveBefore(SubBasicBlock sbb, Variable v) {
+    assignmentLikeOperation(sbb, v, _, _)
+    or
+    // Liveness of `v` is killed when going backwards from a block that declares it
+    exists(DeclStmt ds | ds.getADeclaration().(LocalVariable) = v and sbb.contains(ds))
+  }
+
   /** Holds if `va` is a read access to `v` in `sbb`, where `v` is modeled by `BlockVar`. */
   pragma[noinline]
   private predicate variableAccessInSBB(Variable v, SubBasicBlock sbb, VariableAccess va) {

From 3f531380d18b5b511dbb661004356cb0665f7d9c Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Mon, 12 Aug 2019 15:58:35 +0200
Subject: [PATCH 24/28] C++: Reduce number of SubBasicBlocks in FlowVar

by cutting basic blocks at the same place for the `x.a` partial
definition in `x.a = ...` as they were already cut for assignment to
`a`.
---
 .../code/cpp/dataflow/internal/FlowVar.qll    | 21 ++++++++++---------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
index 1243bac78459..be599cc7ce2f 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
@@ -89,31 +89,34 @@ cached class FlowVar extends TFlowVar {
  */
 private module PartialDefinitions {
   private newtype TPartialDefinition =
-    TExplicitFieldStoreQualifier(Expr qualifier, FieldAccess fa) {
-      isInstanceFieldWrite(fa) and qualifier = fa.getQualifier()
+    TExplicitFieldStoreQualifier(Expr qualifier, ControlFlowNode node) {
+      exists(FieldAccess fa |
+        isInstanceFieldWrite(fa, node) and qualifier = fa.getQualifier()
+      )
     } or
     TExplicitCallQualifier(Expr qualifier, Call call) { qualifier = call.getQualifier() } or
     TReferenceArgument(Expr arg, VariableAccess va) { definitionByReference(va, arg) }
 
-  private predicate isInstanceFieldWrite(FieldAccess fa) {
+  private predicate isInstanceFieldWrite(FieldAccess fa, ControlFlowNode node) {
     not fa.getTarget().isStatic() and
-    assignmentLikeOperation(_, fa.getTarget(), fa, _)
+    assignmentLikeOperation(node, fa.getTarget(), fa, _)
   }
 
   class PartialDefinition extends TPartialDefinition {
     Expr definedExpr;
+    ControlFlowNode node;
 
     PartialDefinition() {
-      this = TExplicitFieldStoreQualifier(definedExpr, _) or
-      this = TExplicitCallQualifier(definedExpr, _) or
-      this = TReferenceArgument(definedExpr, _)
+      this = TExplicitFieldStoreQualifier(definedExpr, node) or
+      this = TExplicitCallQualifier(definedExpr, _) and node = definedExpr or
+      this = TReferenceArgument(definedExpr, node)
     }
 
     predicate partiallyDefines(Variable v) { definedExpr = v.getAnAccess() }
 
     predicate partiallyDefinesThis(ThisExpr e) { definedExpr = e }
 
-    ControlFlowNode getSubBasicBlockStart() { result = definedExpr }
+    ControlFlowNode getSubBasicBlockStart() { result = node }
 
     Expr getDefinedExpr() { result = definedExpr }
 
@@ -141,8 +144,6 @@ private module PartialDefinitions {
     VariableAccess getVariableAccess() { result = va }
 
     override predicate partiallyDefines(Variable v) { va = v.getAnAccess() }
-
-    override ControlFlowNode getSubBasicBlockStart() { result = va }
   }
 }
 import PartialDefinitions

From 1f1824cb9b33882483e9a6f1609b8483b58cc149 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Mon, 12 Aug 2019 16:53:48 +0200
Subject: [PATCH 25/28] C++: Exclude BlockVar computation for "large" vars

---
 .../semmle/code/cpp/dataflow/internal/FlowVar.qll   | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
index be599cc7ce2f..e2b9cc0d39f5 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
@@ -490,7 +490,8 @@ module FlowVar_internal {
   private SubBasicBlock getAReachedBlockVarSBB(TBlockVar start) {
     exists(Variable v |
       start = TBlockVar(result, v) and
-      variableLiveInSBB(result, v)
+      variableLiveInSBB(result, v) and
+      not largeVariable(v, _, _)
     )
     or
     exists(SubBasicBlock mid, SubBasicBlock sbbDef, Variable v |
@@ -503,6 +504,16 @@ module FlowVar_internal {
     )
   }
 
+  /**
+   * Holds if `v` may have too many combinations of definitions and reached
+   * blocks for us the feasibly compute its def-use relation.
+   */
+  private predicate largeVariable(Variable v, int liveBlocks, int defs) {
+    liveBlocks = strictcount(SubBasicBlock sbb | variableLiveInSBB(sbb, v)) and
+    defs = strictcount(SubBasicBlock sbb | exists(TBlockVar(sbb, v))) and
+    liveBlocks * defs > 1000000
+  }
+
   /**
    * Holds if a value held in `v` at the start of `sbb` (or after the first
    * assignment, if that assignment is to `v`) might later be read.

From 38ec693eada57cecaa09b0593fe570fdd48a858e Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Tue, 13 Aug 2019 10:55:27 +0200
Subject: [PATCH 26/28] C++: Improved ConstructorCall field flow

This commit changes C++ `ConstructorCall` to behave like
`new`-expressions in Java: they are both `ExprNode`s and
`PostUpdateNodes`, and there's a "pre-update node" (here called
`PreConstructorCallNode`) to play the role of the qualifier argument
when calling a constructor.
---
 .../cpp/dataflow/internal/DataFlowPrivate.qll |  4 +-
 .../cpp/dataflow/internal/DataFlowUtil.qll    | 38 ++++++++++----
 .../dataflow/fields/flow.expected             | 52 +++++++++----------
 .../dataflow/taint-tests/taint.cpp            |  4 +-
 .../dataflow/taint-tests/taint.expected       |  2 +
 .../dataflow/taint-tests/test_diff.expected   |  2 +
 6 files changed, 62 insertions(+), 40 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
index 447b59bd326c..d036052c14fe 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
@@ -6,9 +6,7 @@ private import DataFlowDispatch
 private Node getInstanceArgument(Call call) {
   result.asExpr() = call.getQualifier()
   or
-  // For constructors, there is no qualifier, so we pretend the call itself
-  // is the instance argument.
-  result.asExpr() = call.(ConstructorCall)
+  result.(PreConstructorCallNode).getConstructorCall() = call
   // This does not include the implicit `this` argument on auto-generated
   // base class destructor calls as those do not have an AST element.
 }
diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index 8ad7e76fbf76..f88c95a74fc6 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -10,7 +10,7 @@ cached
 private newtype TNode =
   TExprNode(Expr e) or
   TPartialDefinitionNode(PartialDefinition pd) or
-  TPostConstructorCallNode(ConstructorCall call) or
+  TPreConstructorCallNode(ConstructorCall call) or
   TExplicitParameterNode(Parameter p) { exists(p.getFunction().getBlock()) } or
   TInstanceParameterNode(MemberFunction f) { exists(f.getBlock()) and not f.isStatic() } or
   TUninitializedNode(LocalVariable v) { not v.hasInitializer() }
@@ -48,8 +48,8 @@ class Node extends TNode {
    *
    * Partial definitions are created for field stores (`x.y = taint();` is a partial
    * definition of `x`), and for calls that may change the value of an object (so
-   * `x.set(taint())` is a partial definition of `x`, annd `transfer(&x, taint())` is
-   * a partial definition of `&x`).s
+   * `x.set(taint())` is a partial definition of `x`, and `transfer(&x, taint())` is
+   * a partial definition of `&x`).
    */
   Expr asPartialDefinition() {
     result = this.(PartialDefinitionNode).getPartialDefinition().getDefinedExpr()
@@ -226,8 +226,6 @@ abstract class PostUpdateNode extends Node {
   override Type getType() { result = getPreUpdateNode().getType() }
 
   override Location getLocation() { result = getPreUpdateNode().getLocation() }
-
-  override string toString() { result = getPreUpdateNode().toString() + " [post update]" }
 }
 
 class PartialDefinitionNode extends PostUpdateNode, TPartialDefinitionNode {
@@ -240,14 +238,36 @@ class PartialDefinitionNode extends PostUpdateNode, TPartialDefinitionNode {
   override Location getLocation() { result = pd.getLocation() }
 
   PartialDefinition getPartialDefinition() { result = pd }
+
+  override string toString() { result = getPreUpdateNode().toString() + " [post update]" }
+}
+
+private class PostConstructorCallNode extends PostUpdateNode, TExprNode {
+  PostConstructorCallNode() { this = TExprNode(any(ConstructorCall c)) }
+
+  override PreConstructorCallNode getPreUpdateNode() {
+    TExprNode(result.getConstructorCall()) = this
+  }
 }
 
-class PostConstructorCallNode extends PostUpdateNode, TPostConstructorCallNode {
-  ConstructorCall call;
+/**
+ * INTERNAL: do not use.
+ *
+ * A synthetic data-flow node that plays the role of the qualifier (or
+ * `this`-argument) to a constructor call.
+ */
+class PreConstructorCallNode extends Node, TPreConstructorCallNode {
+  PreConstructorCallNode() { this = TPreConstructorCallNode(_) }
+
+  ConstructorCall getConstructorCall() { this = TPreConstructorCallNode(result) }
+
+  override Function getFunction() { result = getConstructorCall().getEnclosingFunction() }
+
+  override Type getType() { result = getConstructorCall().getType() }
 
-  PostConstructorCallNode() { this = TPostConstructorCallNode(call) }
+  override Location getLocation() { result = getConstructorCall().getLocation() }
 
-  override Node getPreUpdateNode() { result.asExpr() = call }
+  override string toString() { result = getConstructorCall().toString() + " [pre constructor call]" }
 }
 
 /**
diff --git a/cpp/ql/test/library-tests/dataflow/fields/flow.expected b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
index 99c52b8f2f31..5c627aa6c57b 100644
--- a/cpp/ql/test/library-tests/dataflow/fields/flow.expected
+++ b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
@@ -7,9 +7,9 @@ edges
 | A.cpp:55:5:55:5 | b [post update] [c, ... (1)] | A.cpp:56:10:56:10 | b [c, ... (1)] |
 | A.cpp:55:12:55:19 | new [void] | A.cpp:55:5:55:5 | b [post update] [c, ... (1)] |
 | A.cpp:56:10:56:10 | b [c, ... (1)] | A.cpp:56:13:56:15 | call to get |
-| A.cpp:57:11:57:24 | call to B [post update] [c, ... (1)] | A.cpp:57:11:57:24 | new [c, ... (1)] |
+| A.cpp:57:11:57:24 | call to B [c, ... (1)] | A.cpp:57:11:57:24 | new [c, ... (1)] |
 | A.cpp:57:11:57:24 | new [c, ... (1)] | A.cpp:57:28:57:30 | call to get |
-| A.cpp:57:17:57:23 | new [void] | A.cpp:57:11:57:24 | call to B [post update] [c, ... (1)] |
+| A.cpp:57:17:57:23 | new [void] | A.cpp:57:11:57:24 | call to B [c, ... (1)] |
 | A.cpp:64:10:64:15 | call to setOnB [c, ... (1)] | A.cpp:66:10:66:11 | b2 [c, ... (1)] |
 | A.cpp:64:21:64:28 | new [void] | A.cpp:64:10:64:15 | call to setOnB [c, ... (1)] |
 | A.cpp:66:10:66:11 | b2 [c, ... (1)] | A.cpp:66:14:66:14 | c |
@@ -27,26 +27,26 @@ edges
 | A.cpp:142:7:142:7 | b [post update] [c, ... (1)] | A.cpp:143:7:143:31 | ... = ... [c, ... (1)] |
 | A.cpp:142:7:142:20 | ... = ... [void] | A.cpp:142:7:142:7 | b [post update] [c, ... (1)] |
 | A.cpp:142:14:142:20 | new [void] | A.cpp:142:7:142:20 | ... = ... [void] |
-| A.cpp:143:7:143:10 | this [post update] [b, ... (1)] | A.cpp:151:12:151:24 | call to D [post update] [b, ... (1)] |
-| A.cpp:143:7:143:10 | this [post update] [b, ... (2)] | A.cpp:151:12:151:24 | call to D [post update] [b, ... (2)] |
+| A.cpp:143:7:143:10 | this [post update] [b, ... (1)] | A.cpp:151:12:151:24 | call to D [b, ... (1)] |
+| A.cpp:143:7:143:10 | this [post update] [b, ... (2)] | A.cpp:151:12:151:24 | call to D [b, ... (2)] |
 | A.cpp:143:7:143:31 | ... = ... [c, ... (1)] | A.cpp:143:7:143:10 | this [post update] [b, ... (2)] |
 | A.cpp:143:7:143:31 | ... = ... [void] | A.cpp:143:7:143:10 | this [post update] [b, ... (1)] |
 | A.cpp:143:25:143:31 | new [void] | A.cpp:143:7:143:31 | ... = ... [void] |
 | A.cpp:150:12:150:18 | new [void] | A.cpp:151:18:151:18 | b [void] |
-| A.cpp:151:12:151:24 | call to D [post update] [b, ... (1)] | A.cpp:152:10:152:10 | d [b, ... (1)] |
-| A.cpp:151:12:151:24 | call to D [post update] [b, ... (2)] | A.cpp:153:10:153:10 | d [b, ... (2)] |
-| A.cpp:151:18:151:18 | b [void] | A.cpp:151:12:151:24 | call to D [post update] [b, ... (1)] |
+| A.cpp:151:12:151:24 | call to D [b, ... (1)] | A.cpp:152:10:152:10 | d [b, ... (1)] |
+| A.cpp:151:12:151:24 | call to D [b, ... (2)] | A.cpp:153:10:153:10 | d [b, ... (2)] |
+| A.cpp:151:18:151:18 | b [void] | A.cpp:151:12:151:24 | call to D [b, ... (1)] |
 | A.cpp:152:10:152:10 | d [b, ... (1)] | A.cpp:152:13:152:13 | b |
 | A.cpp:153:10:153:10 | d [b, ... (2)] | A.cpp:153:13:153:13 | b [c, ... (1)] |
 | A.cpp:153:13:153:13 | b [c, ... (1)] | A.cpp:153:16:153:16 | c |
 | A.cpp:159:12:159:18 | new [void] | A.cpp:160:29:160:29 | b [void] |
-| A.cpp:160:18:160:60 | call to MyList [post update] [head, ... (1)] | A.cpp:161:38:161:39 | l1 [head, ... (1)] |
-| A.cpp:160:29:160:29 | b [void] | A.cpp:160:18:160:60 | call to MyList [post update] [head, ... (1)] |
-| A.cpp:161:18:161:40 | call to MyList [post update] [next, ... (2)] | A.cpp:162:38:162:39 | l2 [next, ... (2)] |
-| A.cpp:161:38:161:39 | l1 [head, ... (1)] | A.cpp:161:18:161:40 | call to MyList [post update] [next, ... (2)] |
-| A.cpp:162:18:162:40 | call to MyList [post update] [next, ... (3)] | A.cpp:165:10:165:11 | l3 [next, ... (3)] |
-| A.cpp:162:18:162:40 | call to MyList [post update] [next, ... (3)] | A.cpp:167:44:167:44 | l [next, ... (3)] |
-| A.cpp:162:38:162:39 | l2 [next, ... (2)] | A.cpp:162:18:162:40 | call to MyList [post update] [next, ... (3)] |
+| A.cpp:160:18:160:60 | call to MyList [head, ... (1)] | A.cpp:161:38:161:39 | l1 [head, ... (1)] |
+| A.cpp:160:29:160:29 | b [void] | A.cpp:160:18:160:60 | call to MyList [head, ... (1)] |
+| A.cpp:161:18:161:40 | call to MyList [next, ... (2)] | A.cpp:162:38:162:39 | l2 [next, ... (2)] |
+| A.cpp:161:38:161:39 | l1 [head, ... (1)] | A.cpp:161:18:161:40 | call to MyList [next, ... (2)] |
+| A.cpp:162:18:162:40 | call to MyList [next, ... (3)] | A.cpp:165:10:165:11 | l3 [next, ... (3)] |
+| A.cpp:162:18:162:40 | call to MyList [next, ... (3)] | A.cpp:167:44:167:44 | l [next, ... (3)] |
+| A.cpp:162:38:162:39 | l2 [next, ... (2)] | A.cpp:162:18:162:40 | call to MyList [next, ... (3)] |
 | A.cpp:165:10:165:11 | l3 [next, ... (3)] | A.cpp:165:14:165:17 | next [next, ... (2)] |
 | A.cpp:165:14:165:17 | next [next, ... (2)] | A.cpp:165:20:165:23 | next [head, ... (1)] |
 | A.cpp:165:20:165:23 | next [head, ... (1)] | A.cpp:165:26:165:29 | head |
@@ -56,28 +56,28 @@ edges
 | A.cpp:167:47:167:50 | next [next, ... (2)] | A.cpp:167:44:167:44 | l [next, ... (2)] |
 | A.cpp:169:12:169:12 | l [head, ... (1)] | A.cpp:169:15:169:18 | head |
 | B.cpp:6:15:6:24 | new [void] | B.cpp:7:25:7:25 | e [void] |
-| B.cpp:7:16:7:35 | call to Box1 [post update] [elem1, ... (1)] | B.cpp:8:25:8:26 | b1 [elem1, ... (1)] |
-| B.cpp:7:25:7:25 | e [void] | B.cpp:7:16:7:35 | call to Box1 [post update] [elem1, ... (1)] |
-| B.cpp:8:16:8:27 | call to Box2 [post update] [box1, ... (2)] | B.cpp:9:10:9:11 | b2 [box1, ... (2)] |
-| B.cpp:8:16:8:27 | call to Box2 [post update] [box1, ... (2)] | B.cpp:10:10:10:11 | b2 [box1, ... (2)] |
-| B.cpp:8:25:8:26 | b1 [elem1, ... (1)] | B.cpp:8:16:8:27 | call to Box2 [post update] [box1, ... (2)] |
+| B.cpp:7:16:7:35 | call to Box1 [elem1, ... (1)] | B.cpp:8:25:8:26 | b1 [elem1, ... (1)] |
+| B.cpp:7:25:7:25 | e [void] | B.cpp:7:16:7:35 | call to Box1 [elem1, ... (1)] |
+| B.cpp:8:16:8:27 | call to Box2 [box1, ... (2)] | B.cpp:9:10:9:11 | b2 [box1, ... (2)] |
+| B.cpp:8:16:8:27 | call to Box2 [box1, ... (2)] | B.cpp:10:10:10:11 | b2 [box1, ... (2)] |
+| B.cpp:8:25:8:26 | b1 [elem1, ... (1)] | B.cpp:8:16:8:27 | call to Box2 [box1, ... (2)] |
 | B.cpp:9:10:9:11 | b2 [box1, ... (2)] | B.cpp:9:14:9:17 | box1 [elem1, ... (1)] |
 | B.cpp:9:14:9:17 | box1 [elem1, ... (1)] | B.cpp:9:20:9:24 | elem1 |
 | B.cpp:10:10:10:11 | b2 [box1, ... (2)] | B.cpp:10:14:10:17 | box1 [elem2, ... (1)] |
 | B.cpp:10:14:10:17 | box1 [elem2, ... (1)] | B.cpp:10:20:10:24 | elem2 |
 | B.cpp:15:15:15:27 | new [void] | B.cpp:16:37:16:37 | e [void] |
-| B.cpp:16:16:16:38 | call to Box1 [post update] [elem2, ... (1)] | B.cpp:17:25:17:26 | b1 [elem2, ... (1)] |
-| B.cpp:16:37:16:37 | e [void] | B.cpp:16:16:16:38 | call to Box1 [post update] [elem2, ... (1)] |
-| B.cpp:17:16:17:27 | call to Box2 [post update] [box1, ... (2)] | B.cpp:18:10:18:11 | b2 [box1, ... (2)] |
-| B.cpp:17:16:17:27 | call to Box2 [post update] [box1, ... (2)] | B.cpp:19:10:19:11 | b2 [box1, ... (2)] |
-| B.cpp:17:25:17:26 | b1 [elem2, ... (1)] | B.cpp:17:16:17:27 | call to Box2 [post update] [box1, ... (2)] |
+| B.cpp:16:16:16:38 | call to Box1 [elem2, ... (1)] | B.cpp:17:25:17:26 | b1 [elem2, ... (1)] |
+| B.cpp:16:37:16:37 | e [void] | B.cpp:16:16:16:38 | call to Box1 [elem2, ... (1)] |
+| B.cpp:17:16:17:27 | call to Box2 [box1, ... (2)] | B.cpp:18:10:18:11 | b2 [box1, ... (2)] |
+| B.cpp:17:16:17:27 | call to Box2 [box1, ... (2)] | B.cpp:19:10:19:11 | b2 [box1, ... (2)] |
+| B.cpp:17:25:17:26 | b1 [elem2, ... (1)] | B.cpp:17:16:17:27 | call to Box2 [box1, ... (2)] |
 | B.cpp:18:10:18:11 | b2 [box1, ... (2)] | B.cpp:18:14:18:17 | box1 [elem1, ... (1)] |
 | B.cpp:18:14:18:17 | box1 [elem1, ... (1)] | B.cpp:18:20:18:24 | elem1 |
 | B.cpp:19:10:19:11 | b2 [box1, ... (2)] | B.cpp:19:14:19:17 | box1 [elem2, ... (1)] |
 | B.cpp:19:14:19:17 | box1 [elem2, ... (1)] | B.cpp:19:20:19:24 | elem2 |
-| C.cpp:18:12:18:18 | call to C [post update] [s3, ... (1)] | C.cpp:19:5:19:5 | c [s3, ... (1)] |
+| C.cpp:18:12:18:18 | call to C [s3, ... (1)] | C.cpp:19:5:19:5 | c [s3, ... (1)] |
 | C.cpp:19:5:19:5 | c [s3, ... (1)] | C.cpp:27:8:27:11 | `this` parameter in func [s3, ... (1)] |
-| C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] | C.cpp:18:12:18:18 | call to C [post update] [s3, ... (1)] |
+| C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] | C.cpp:18:12:18:18 | call to C [s3, ... (1)] |
 | C.cpp:24:5:24:25 | ... = ... [void] | C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] |
 | C.cpp:24:16:24:25 | new [void] | C.cpp:24:5:24:25 | ... = ... [void] |
 | C.cpp:27:8:27:11 | `this` parameter in func [s3, ... (1)] | file://:0:0:0:0 | this [s3, ... (1)] |
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp
index e764fbd784df..9cca2dffa918 100644
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp
@@ -87,11 +87,11 @@ void class_field_test() {
 
 	sink(mc1.a);
 	sink(mc1.b); // tainted [NOT DETECTED]
-	sink(mc1.c); // tainted [NOT DETECTED]
+	sink(mc1.c); // tainted [NOT DETECTED with IR]
 	sink(mc1.d); // tainted [NOT DETECTED with IR]
 	sink(mc2.a);
 	sink(mc2.b); // tainted [NOT DETECTED]
-	sink(mc2.c); // tainted [NOT DETECTED]
+	sink(mc2.c); // tainted [NOT DETECTED with IR]
 	sink(mc2.d);
 }
 
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected
index 90b1d9814af9..c27bc148c760 100644
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected
@@ -4,7 +4,9 @@
 | taint.cpp:41:7:41:13 | global7 | taint.cpp:35:12:35:17 | call to source |
 | taint.cpp:42:7:42:13 | global8 | taint.cpp:35:12:35:17 | call to source |
 | taint.cpp:43:7:43:13 | global9 | taint.cpp:37:22:37:27 | call to source |
+| taint.cpp:90:11:90:11 | c | taint.cpp:72:7:72:12 | call to source |
 | taint.cpp:91:11:91:11 | d | taint.cpp:77:7:77:12 | call to source |
+| taint.cpp:94:11:94:11 | c | taint.cpp:72:7:72:12 | call to source |
 | taint.cpp:129:7:129:9 | * ... | taint.cpp:120:11:120:16 | call to source |
 | taint.cpp:134:7:134:9 | * ... | taint.cpp:120:11:120:16 | call to source |
 | taint.cpp:137:7:137:9 | * ... | taint.cpp:120:11:120:16 | call to source |
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected
index 8a9fc7676137..d86b5b43ef05 100644
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected
@@ -1,7 +1,9 @@
 | taint.cpp:41:7:41:13 | taint.cpp:35:12:35:17 | AST only |
 | taint.cpp:42:7:42:13 | taint.cpp:35:12:35:17 | AST only |
 | taint.cpp:43:7:43:13 | taint.cpp:37:22:37:27 | AST only |
+| taint.cpp:90:11:90:11 | taint.cpp:72:7:72:12 | AST only |
 | taint.cpp:91:11:91:11 | taint.cpp:77:7:77:12 | AST only |
+| taint.cpp:94:11:94:11 | taint.cpp:72:7:72:12 | AST only |
 | taint.cpp:130:7:130:9 | taint.cpp:127:8:127:13 | IR only |
 | taint.cpp:137:7:137:9 | taint.cpp:120:11:120:16 | AST only |
 | taint.cpp:173:8:173:13 | taint.cpp:164:19:164:24 | AST only |

From e94dbe926b01ef27b85f6e0a5599e56556da10c3 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Wed, 14 Aug 2019 16:16:28 +0200
Subject: [PATCH 27/28] C++: Add forgotten toString override

This makes `PostConstructorCallNode`s show up in the test output.
---
 .../cpp/dataflow/internal/DataFlowUtil.qll    | 10 +++++--
 .../dataflow/fields/flow.expected             | 26 +++++++++++++++++++
 .../dataflow/taint-tests/localTaint.expected  |  9 +++++++
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index f88c95a74fc6..3643b50404ee 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -242,12 +242,20 @@ class PartialDefinitionNode extends PostUpdateNode, TPartialDefinitionNode {
   override string toString() { result = getPreUpdateNode().toString() + " [post update]" }
 }
 
+/**
+ * A node representing the object that was just constructed and is identified
+ * with the "return value" of the constructor call.
+ */
 private class PostConstructorCallNode extends PostUpdateNode, TExprNode {
   PostConstructorCallNode() { this = TExprNode(any(ConstructorCall c)) }
 
   override PreConstructorCallNode getPreUpdateNode() {
     TExprNode(result.getConstructorCall()) = this
   }
+
+  override string toString() {
+    result = getPreUpdateNode().getConstructorCall().toString() + " [post constructor]"
+  }
 }
 
 /**
@@ -257,8 +265,6 @@ private class PostConstructorCallNode extends PostUpdateNode, TExprNode {
  * `this`-argument) to a constructor call.
  */
 class PreConstructorCallNode extends Node, TPreConstructorCallNode {
-  PreConstructorCallNode() { this = TPreConstructorCallNode(_) }
-
   ConstructorCall getConstructorCall() { this = TPreConstructorCallNode(result) }
 
   override Function getFunction() { result = getConstructorCall().getEnclosingFunction() }
diff --git a/cpp/ql/test/library-tests/dataflow/fields/flow.expected b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
index 5c627aa6c57b..42f36312c7ff 100644
--- a/cpp/ql/test/library-tests/dataflow/fields/flow.expected
+++ b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
@@ -8,8 +8,10 @@ edges
 | A.cpp:55:12:55:19 | new [void] | A.cpp:55:5:55:5 | b [post update] [c, ... (1)] |
 | A.cpp:56:10:56:10 | b [c, ... (1)] | A.cpp:56:13:56:15 | call to get |
 | A.cpp:57:11:57:24 | call to B [c, ... (1)] | A.cpp:57:11:57:24 | new [c, ... (1)] |
+| A.cpp:57:11:57:24 | call to B [post constructor] [c, ... (1)] | A.cpp:57:11:57:24 | new [c, ... (1)] |
 | A.cpp:57:11:57:24 | new [c, ... (1)] | A.cpp:57:28:57:30 | call to get |
 | A.cpp:57:17:57:23 | new [void] | A.cpp:57:11:57:24 | call to B [c, ... (1)] |
+| A.cpp:57:17:57:23 | new [void] | A.cpp:57:11:57:24 | call to B [post constructor] [c, ... (1)] |
 | A.cpp:64:10:64:15 | call to setOnB [c, ... (1)] | A.cpp:66:10:66:11 | b2 [c, ... (1)] |
 | A.cpp:64:21:64:28 | new [void] | A.cpp:64:10:64:15 | call to setOnB [c, ... (1)] |
 | A.cpp:66:10:66:11 | b2 [c, ... (1)] | A.cpp:66:14:66:14 | c |
@@ -28,25 +30,37 @@ edges
 | A.cpp:142:7:142:20 | ... = ... [void] | A.cpp:142:7:142:7 | b [post update] [c, ... (1)] |
 | A.cpp:142:14:142:20 | new [void] | A.cpp:142:7:142:20 | ... = ... [void] |
 | A.cpp:143:7:143:10 | this [post update] [b, ... (1)] | A.cpp:151:12:151:24 | call to D [b, ... (1)] |
+| A.cpp:143:7:143:10 | this [post update] [b, ... (1)] | A.cpp:151:12:151:24 | call to D [post constructor] [b, ... (1)] |
 | A.cpp:143:7:143:10 | this [post update] [b, ... (2)] | A.cpp:151:12:151:24 | call to D [b, ... (2)] |
+| A.cpp:143:7:143:10 | this [post update] [b, ... (2)] | A.cpp:151:12:151:24 | call to D [post constructor] [b, ... (2)] |
 | A.cpp:143:7:143:31 | ... = ... [c, ... (1)] | A.cpp:143:7:143:10 | this [post update] [b, ... (2)] |
 | A.cpp:143:7:143:31 | ... = ... [void] | A.cpp:143:7:143:10 | this [post update] [b, ... (1)] |
 | A.cpp:143:25:143:31 | new [void] | A.cpp:143:7:143:31 | ... = ... [void] |
 | A.cpp:150:12:150:18 | new [void] | A.cpp:151:18:151:18 | b [void] |
 | A.cpp:151:12:151:24 | call to D [b, ... (1)] | A.cpp:152:10:152:10 | d [b, ... (1)] |
 | A.cpp:151:12:151:24 | call to D [b, ... (2)] | A.cpp:153:10:153:10 | d [b, ... (2)] |
+| A.cpp:151:12:151:24 | call to D [post constructor] [b, ... (1)] | A.cpp:152:10:152:10 | d [b, ... (1)] |
+| A.cpp:151:12:151:24 | call to D [post constructor] [b, ... (2)] | A.cpp:153:10:153:10 | d [b, ... (2)] |
 | A.cpp:151:18:151:18 | b [void] | A.cpp:151:12:151:24 | call to D [b, ... (1)] |
+| A.cpp:151:18:151:18 | b [void] | A.cpp:151:12:151:24 | call to D [post constructor] [b, ... (1)] |
 | A.cpp:152:10:152:10 | d [b, ... (1)] | A.cpp:152:13:152:13 | b |
 | A.cpp:153:10:153:10 | d [b, ... (2)] | A.cpp:153:13:153:13 | b [c, ... (1)] |
 | A.cpp:153:13:153:13 | b [c, ... (1)] | A.cpp:153:16:153:16 | c |
 | A.cpp:159:12:159:18 | new [void] | A.cpp:160:29:160:29 | b [void] |
 | A.cpp:160:18:160:60 | call to MyList [head, ... (1)] | A.cpp:161:38:161:39 | l1 [head, ... (1)] |
+| A.cpp:160:18:160:60 | call to MyList [post constructor] [head, ... (1)] | A.cpp:161:38:161:39 | l1 [head, ... (1)] |
 | A.cpp:160:29:160:29 | b [void] | A.cpp:160:18:160:60 | call to MyList [head, ... (1)] |
+| A.cpp:160:29:160:29 | b [void] | A.cpp:160:18:160:60 | call to MyList [post constructor] [head, ... (1)] |
 | A.cpp:161:18:161:40 | call to MyList [next, ... (2)] | A.cpp:162:38:162:39 | l2 [next, ... (2)] |
+| A.cpp:161:18:161:40 | call to MyList [post constructor] [next, ... (2)] | A.cpp:162:38:162:39 | l2 [next, ... (2)] |
 | A.cpp:161:38:161:39 | l1 [head, ... (1)] | A.cpp:161:18:161:40 | call to MyList [next, ... (2)] |
+| A.cpp:161:38:161:39 | l1 [head, ... (1)] | A.cpp:161:18:161:40 | call to MyList [post constructor] [next, ... (2)] |
 | A.cpp:162:18:162:40 | call to MyList [next, ... (3)] | A.cpp:165:10:165:11 | l3 [next, ... (3)] |
 | A.cpp:162:18:162:40 | call to MyList [next, ... (3)] | A.cpp:167:44:167:44 | l [next, ... (3)] |
+| A.cpp:162:18:162:40 | call to MyList [post constructor] [next, ... (3)] | A.cpp:165:10:165:11 | l3 [next, ... (3)] |
+| A.cpp:162:18:162:40 | call to MyList [post constructor] [next, ... (3)] | A.cpp:167:44:167:44 | l [next, ... (3)] |
 | A.cpp:162:38:162:39 | l2 [next, ... (2)] | A.cpp:162:18:162:40 | call to MyList [next, ... (3)] |
+| A.cpp:162:38:162:39 | l2 [next, ... (2)] | A.cpp:162:18:162:40 | call to MyList [post constructor] [next, ... (3)] |
 | A.cpp:165:10:165:11 | l3 [next, ... (3)] | A.cpp:165:14:165:17 | next [next, ... (2)] |
 | A.cpp:165:14:165:17 | next [next, ... (2)] | A.cpp:165:20:165:23 | next [head, ... (1)] |
 | A.cpp:165:20:165:23 | next [head, ... (1)] | A.cpp:165:26:165:29 | head |
@@ -57,26 +71,38 @@ edges
 | A.cpp:169:12:169:12 | l [head, ... (1)] | A.cpp:169:15:169:18 | head |
 | B.cpp:6:15:6:24 | new [void] | B.cpp:7:25:7:25 | e [void] |
 | B.cpp:7:16:7:35 | call to Box1 [elem1, ... (1)] | B.cpp:8:25:8:26 | b1 [elem1, ... (1)] |
+| B.cpp:7:16:7:35 | call to Box1 [post constructor] [elem1, ... (1)] | B.cpp:8:25:8:26 | b1 [elem1, ... (1)] |
 | B.cpp:7:25:7:25 | e [void] | B.cpp:7:16:7:35 | call to Box1 [elem1, ... (1)] |
+| B.cpp:7:25:7:25 | e [void] | B.cpp:7:16:7:35 | call to Box1 [post constructor] [elem1, ... (1)] |
 | B.cpp:8:16:8:27 | call to Box2 [box1, ... (2)] | B.cpp:9:10:9:11 | b2 [box1, ... (2)] |
 | B.cpp:8:16:8:27 | call to Box2 [box1, ... (2)] | B.cpp:10:10:10:11 | b2 [box1, ... (2)] |
+| B.cpp:8:16:8:27 | call to Box2 [post constructor] [box1, ... (2)] | B.cpp:9:10:9:11 | b2 [box1, ... (2)] |
+| B.cpp:8:16:8:27 | call to Box2 [post constructor] [box1, ... (2)] | B.cpp:10:10:10:11 | b2 [box1, ... (2)] |
 | B.cpp:8:25:8:26 | b1 [elem1, ... (1)] | B.cpp:8:16:8:27 | call to Box2 [box1, ... (2)] |
+| B.cpp:8:25:8:26 | b1 [elem1, ... (1)] | B.cpp:8:16:8:27 | call to Box2 [post constructor] [box1, ... (2)] |
 | B.cpp:9:10:9:11 | b2 [box1, ... (2)] | B.cpp:9:14:9:17 | box1 [elem1, ... (1)] |
 | B.cpp:9:14:9:17 | box1 [elem1, ... (1)] | B.cpp:9:20:9:24 | elem1 |
 | B.cpp:10:10:10:11 | b2 [box1, ... (2)] | B.cpp:10:14:10:17 | box1 [elem2, ... (1)] |
 | B.cpp:10:14:10:17 | box1 [elem2, ... (1)] | B.cpp:10:20:10:24 | elem2 |
 | B.cpp:15:15:15:27 | new [void] | B.cpp:16:37:16:37 | e [void] |
 | B.cpp:16:16:16:38 | call to Box1 [elem2, ... (1)] | B.cpp:17:25:17:26 | b1 [elem2, ... (1)] |
+| B.cpp:16:16:16:38 | call to Box1 [post constructor] [elem2, ... (1)] | B.cpp:17:25:17:26 | b1 [elem2, ... (1)] |
 | B.cpp:16:37:16:37 | e [void] | B.cpp:16:16:16:38 | call to Box1 [elem2, ... (1)] |
+| B.cpp:16:37:16:37 | e [void] | B.cpp:16:16:16:38 | call to Box1 [post constructor] [elem2, ... (1)] |
 | B.cpp:17:16:17:27 | call to Box2 [box1, ... (2)] | B.cpp:18:10:18:11 | b2 [box1, ... (2)] |
 | B.cpp:17:16:17:27 | call to Box2 [box1, ... (2)] | B.cpp:19:10:19:11 | b2 [box1, ... (2)] |
+| B.cpp:17:16:17:27 | call to Box2 [post constructor] [box1, ... (2)] | B.cpp:18:10:18:11 | b2 [box1, ... (2)] |
+| B.cpp:17:16:17:27 | call to Box2 [post constructor] [box1, ... (2)] | B.cpp:19:10:19:11 | b2 [box1, ... (2)] |
 | B.cpp:17:25:17:26 | b1 [elem2, ... (1)] | B.cpp:17:16:17:27 | call to Box2 [box1, ... (2)] |
+| B.cpp:17:25:17:26 | b1 [elem2, ... (1)] | B.cpp:17:16:17:27 | call to Box2 [post constructor] [box1, ... (2)] |
 | B.cpp:18:10:18:11 | b2 [box1, ... (2)] | B.cpp:18:14:18:17 | box1 [elem1, ... (1)] |
 | B.cpp:18:14:18:17 | box1 [elem1, ... (1)] | B.cpp:18:20:18:24 | elem1 |
 | B.cpp:19:10:19:11 | b2 [box1, ... (2)] | B.cpp:19:14:19:17 | box1 [elem2, ... (1)] |
 | B.cpp:19:14:19:17 | box1 [elem2, ... (1)] | B.cpp:19:20:19:24 | elem2 |
+| C.cpp:18:12:18:18 | call to C [post constructor] [s3, ... (1)] | C.cpp:19:5:19:5 | c [s3, ... (1)] |
 | C.cpp:18:12:18:18 | call to C [s3, ... (1)] | C.cpp:19:5:19:5 | c [s3, ... (1)] |
 | C.cpp:19:5:19:5 | c [s3, ... (1)] | C.cpp:27:8:27:11 | `this` parameter in func [s3, ... (1)] |
+| C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] | C.cpp:18:12:18:18 | call to C [post constructor] [s3, ... (1)] |
 | C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] | C.cpp:18:12:18:18 | call to C [s3, ... (1)] |
 | C.cpp:24:5:24:25 | ... = ... [void] | C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] |
 | C.cpp:24:16:24:25 | new [void] | C.cpp:24:5:24:25 | ... = ... [void] |
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
index 875e5619c840..4a320590036d 100644
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
@@ -57,10 +57,19 @@
 | taint.cpp:84:10:84:12 | call to MyClass | taint.cpp:89:7:89:9 | mc1 |  |
 | taint.cpp:84:10:84:12 | call to MyClass | taint.cpp:90:7:90:9 | mc1 |  |
 | taint.cpp:84:10:84:12 | call to MyClass | taint.cpp:91:7:91:9 | mc1 |  |
+| taint.cpp:84:10:84:12 | call to MyClass [post constructor] | taint.cpp:86:2:86:4 | mc1 |  |
+| taint.cpp:84:10:84:12 | call to MyClass [post constructor] | taint.cpp:88:7:88:9 | mc1 |  |
+| taint.cpp:84:10:84:12 | call to MyClass [post constructor] | taint.cpp:89:7:89:9 | mc1 |  |
+| taint.cpp:84:10:84:12 | call to MyClass [post constructor] | taint.cpp:90:7:90:9 | mc1 |  |
+| taint.cpp:84:10:84:12 | call to MyClass [post constructor] | taint.cpp:91:7:91:9 | mc1 |  |
 | taint.cpp:84:15:84:17 | call to MyClass | taint.cpp:92:7:92:9 | mc2 |  |
 | taint.cpp:84:15:84:17 | call to MyClass | taint.cpp:93:7:93:9 | mc2 |  |
 | taint.cpp:84:15:84:17 | call to MyClass | taint.cpp:94:7:94:9 | mc2 |  |
 | taint.cpp:84:15:84:17 | call to MyClass | taint.cpp:95:7:95:9 | mc2 |  |
+| taint.cpp:84:15:84:17 | call to MyClass [post constructor] | taint.cpp:92:7:92:9 | mc2 |  |
+| taint.cpp:84:15:84:17 | call to MyClass [post constructor] | taint.cpp:93:7:93:9 | mc2 |  |
+| taint.cpp:84:15:84:17 | call to MyClass [post constructor] | taint.cpp:94:7:94:9 | mc2 |  |
+| taint.cpp:84:15:84:17 | call to MyClass [post constructor] | taint.cpp:95:7:95:9 | mc2 |  |
 | taint.cpp:86:2:86:4 | mc1 [post update] | taint.cpp:88:7:88:9 | mc1 |  |
 | taint.cpp:86:2:86:4 | mc1 [post update] | taint.cpp:89:7:89:9 | mc1 |  |
 | taint.cpp:86:2:86:4 | mc1 [post update] | taint.cpp:90:7:90:9 | mc1 |  |

From fdd8de79da06709479fab6d1ddad95cb783926f6 Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jonas@semmle.com>
Date: Thu, 15 Aug 2019 11:32:11 +0200
Subject: [PATCH 28/28] C++: Remove redundant toString override

This time I left a comment to prevent myself from getting confused again
and adding the override in the future.
---
 .../cpp/dataflow/internal/DataFlowUtil.qll    |  5 ++--
 .../dataflow/fields/flow.expected             | 26 -------------------
 .../dataflow/taint-tests/localTaint.expected  |  9 -------
 3 files changed, 2 insertions(+), 38 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
index 3643b50404ee..3d9338f475da 100644
--- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
+++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -253,9 +253,8 @@ private class PostConstructorCallNode extends PostUpdateNode, TExprNode {
     TExprNode(result.getConstructorCall()) = this
   }
 
-  override string toString() {
-    result = getPreUpdateNode().getConstructorCall().toString() + " [post constructor]"
-  }
+  // No override of `toString` since these nodes already have a `toString` from
+  // their overlap with `ExprNode`.
 }
 
 /**
diff --git a/cpp/ql/test/library-tests/dataflow/fields/flow.expected b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
index 42f36312c7ff..5c627aa6c57b 100644
--- a/cpp/ql/test/library-tests/dataflow/fields/flow.expected
+++ b/cpp/ql/test/library-tests/dataflow/fields/flow.expected
@@ -8,10 +8,8 @@ edges
 | A.cpp:55:12:55:19 | new [void] | A.cpp:55:5:55:5 | b [post update] [c, ... (1)] |
 | A.cpp:56:10:56:10 | b [c, ... (1)] | A.cpp:56:13:56:15 | call to get |
 | A.cpp:57:11:57:24 | call to B [c, ... (1)] | A.cpp:57:11:57:24 | new [c, ... (1)] |
-| A.cpp:57:11:57:24 | call to B [post constructor] [c, ... (1)] | A.cpp:57:11:57:24 | new [c, ... (1)] |
 | A.cpp:57:11:57:24 | new [c, ... (1)] | A.cpp:57:28:57:30 | call to get |
 | A.cpp:57:17:57:23 | new [void] | A.cpp:57:11:57:24 | call to B [c, ... (1)] |
-| A.cpp:57:17:57:23 | new [void] | A.cpp:57:11:57:24 | call to B [post constructor] [c, ... (1)] |
 | A.cpp:64:10:64:15 | call to setOnB [c, ... (1)] | A.cpp:66:10:66:11 | b2 [c, ... (1)] |
 | A.cpp:64:21:64:28 | new [void] | A.cpp:64:10:64:15 | call to setOnB [c, ... (1)] |
 | A.cpp:66:10:66:11 | b2 [c, ... (1)] | A.cpp:66:14:66:14 | c |
@@ -30,37 +28,25 @@ edges
 | A.cpp:142:7:142:20 | ... = ... [void] | A.cpp:142:7:142:7 | b [post update] [c, ... (1)] |
 | A.cpp:142:14:142:20 | new [void] | A.cpp:142:7:142:20 | ... = ... [void] |
 | A.cpp:143:7:143:10 | this [post update] [b, ... (1)] | A.cpp:151:12:151:24 | call to D [b, ... (1)] |
-| A.cpp:143:7:143:10 | this [post update] [b, ... (1)] | A.cpp:151:12:151:24 | call to D [post constructor] [b, ... (1)] |
 | A.cpp:143:7:143:10 | this [post update] [b, ... (2)] | A.cpp:151:12:151:24 | call to D [b, ... (2)] |
-| A.cpp:143:7:143:10 | this [post update] [b, ... (2)] | A.cpp:151:12:151:24 | call to D [post constructor] [b, ... (2)] |
 | A.cpp:143:7:143:31 | ... = ... [c, ... (1)] | A.cpp:143:7:143:10 | this [post update] [b, ... (2)] |
 | A.cpp:143:7:143:31 | ... = ... [void] | A.cpp:143:7:143:10 | this [post update] [b, ... (1)] |
 | A.cpp:143:25:143:31 | new [void] | A.cpp:143:7:143:31 | ... = ... [void] |
 | A.cpp:150:12:150:18 | new [void] | A.cpp:151:18:151:18 | b [void] |
 | A.cpp:151:12:151:24 | call to D [b, ... (1)] | A.cpp:152:10:152:10 | d [b, ... (1)] |
 | A.cpp:151:12:151:24 | call to D [b, ... (2)] | A.cpp:153:10:153:10 | d [b, ... (2)] |
-| A.cpp:151:12:151:24 | call to D [post constructor] [b, ... (1)] | A.cpp:152:10:152:10 | d [b, ... (1)] |
-| A.cpp:151:12:151:24 | call to D [post constructor] [b, ... (2)] | A.cpp:153:10:153:10 | d [b, ... (2)] |
 | A.cpp:151:18:151:18 | b [void] | A.cpp:151:12:151:24 | call to D [b, ... (1)] |
-| A.cpp:151:18:151:18 | b [void] | A.cpp:151:12:151:24 | call to D [post constructor] [b, ... (1)] |
 | A.cpp:152:10:152:10 | d [b, ... (1)] | A.cpp:152:13:152:13 | b |
 | A.cpp:153:10:153:10 | d [b, ... (2)] | A.cpp:153:13:153:13 | b [c, ... (1)] |
 | A.cpp:153:13:153:13 | b [c, ... (1)] | A.cpp:153:16:153:16 | c |
 | A.cpp:159:12:159:18 | new [void] | A.cpp:160:29:160:29 | b [void] |
 | A.cpp:160:18:160:60 | call to MyList [head, ... (1)] | A.cpp:161:38:161:39 | l1 [head, ... (1)] |
-| A.cpp:160:18:160:60 | call to MyList [post constructor] [head, ... (1)] | A.cpp:161:38:161:39 | l1 [head, ... (1)] |
 | A.cpp:160:29:160:29 | b [void] | A.cpp:160:18:160:60 | call to MyList [head, ... (1)] |
-| A.cpp:160:29:160:29 | b [void] | A.cpp:160:18:160:60 | call to MyList [post constructor] [head, ... (1)] |
 | A.cpp:161:18:161:40 | call to MyList [next, ... (2)] | A.cpp:162:38:162:39 | l2 [next, ... (2)] |
-| A.cpp:161:18:161:40 | call to MyList [post constructor] [next, ... (2)] | A.cpp:162:38:162:39 | l2 [next, ... (2)] |
 | A.cpp:161:38:161:39 | l1 [head, ... (1)] | A.cpp:161:18:161:40 | call to MyList [next, ... (2)] |
-| A.cpp:161:38:161:39 | l1 [head, ... (1)] | A.cpp:161:18:161:40 | call to MyList [post constructor] [next, ... (2)] |
 | A.cpp:162:18:162:40 | call to MyList [next, ... (3)] | A.cpp:165:10:165:11 | l3 [next, ... (3)] |
 | A.cpp:162:18:162:40 | call to MyList [next, ... (3)] | A.cpp:167:44:167:44 | l [next, ... (3)] |
-| A.cpp:162:18:162:40 | call to MyList [post constructor] [next, ... (3)] | A.cpp:165:10:165:11 | l3 [next, ... (3)] |
-| A.cpp:162:18:162:40 | call to MyList [post constructor] [next, ... (3)] | A.cpp:167:44:167:44 | l [next, ... (3)] |
 | A.cpp:162:38:162:39 | l2 [next, ... (2)] | A.cpp:162:18:162:40 | call to MyList [next, ... (3)] |
-| A.cpp:162:38:162:39 | l2 [next, ... (2)] | A.cpp:162:18:162:40 | call to MyList [post constructor] [next, ... (3)] |
 | A.cpp:165:10:165:11 | l3 [next, ... (3)] | A.cpp:165:14:165:17 | next [next, ... (2)] |
 | A.cpp:165:14:165:17 | next [next, ... (2)] | A.cpp:165:20:165:23 | next [head, ... (1)] |
 | A.cpp:165:20:165:23 | next [head, ... (1)] | A.cpp:165:26:165:29 | head |
@@ -71,38 +57,26 @@ edges
 | A.cpp:169:12:169:12 | l [head, ... (1)] | A.cpp:169:15:169:18 | head |
 | B.cpp:6:15:6:24 | new [void] | B.cpp:7:25:7:25 | e [void] |
 | B.cpp:7:16:7:35 | call to Box1 [elem1, ... (1)] | B.cpp:8:25:8:26 | b1 [elem1, ... (1)] |
-| B.cpp:7:16:7:35 | call to Box1 [post constructor] [elem1, ... (1)] | B.cpp:8:25:8:26 | b1 [elem1, ... (1)] |
 | B.cpp:7:25:7:25 | e [void] | B.cpp:7:16:7:35 | call to Box1 [elem1, ... (1)] |
-| B.cpp:7:25:7:25 | e [void] | B.cpp:7:16:7:35 | call to Box1 [post constructor] [elem1, ... (1)] |
 | B.cpp:8:16:8:27 | call to Box2 [box1, ... (2)] | B.cpp:9:10:9:11 | b2 [box1, ... (2)] |
 | B.cpp:8:16:8:27 | call to Box2 [box1, ... (2)] | B.cpp:10:10:10:11 | b2 [box1, ... (2)] |
-| B.cpp:8:16:8:27 | call to Box2 [post constructor] [box1, ... (2)] | B.cpp:9:10:9:11 | b2 [box1, ... (2)] |
-| B.cpp:8:16:8:27 | call to Box2 [post constructor] [box1, ... (2)] | B.cpp:10:10:10:11 | b2 [box1, ... (2)] |
 | B.cpp:8:25:8:26 | b1 [elem1, ... (1)] | B.cpp:8:16:8:27 | call to Box2 [box1, ... (2)] |
-| B.cpp:8:25:8:26 | b1 [elem1, ... (1)] | B.cpp:8:16:8:27 | call to Box2 [post constructor] [box1, ... (2)] |
 | B.cpp:9:10:9:11 | b2 [box1, ... (2)] | B.cpp:9:14:9:17 | box1 [elem1, ... (1)] |
 | B.cpp:9:14:9:17 | box1 [elem1, ... (1)] | B.cpp:9:20:9:24 | elem1 |
 | B.cpp:10:10:10:11 | b2 [box1, ... (2)] | B.cpp:10:14:10:17 | box1 [elem2, ... (1)] |
 | B.cpp:10:14:10:17 | box1 [elem2, ... (1)] | B.cpp:10:20:10:24 | elem2 |
 | B.cpp:15:15:15:27 | new [void] | B.cpp:16:37:16:37 | e [void] |
 | B.cpp:16:16:16:38 | call to Box1 [elem2, ... (1)] | B.cpp:17:25:17:26 | b1 [elem2, ... (1)] |
-| B.cpp:16:16:16:38 | call to Box1 [post constructor] [elem2, ... (1)] | B.cpp:17:25:17:26 | b1 [elem2, ... (1)] |
 | B.cpp:16:37:16:37 | e [void] | B.cpp:16:16:16:38 | call to Box1 [elem2, ... (1)] |
-| B.cpp:16:37:16:37 | e [void] | B.cpp:16:16:16:38 | call to Box1 [post constructor] [elem2, ... (1)] |
 | B.cpp:17:16:17:27 | call to Box2 [box1, ... (2)] | B.cpp:18:10:18:11 | b2 [box1, ... (2)] |
 | B.cpp:17:16:17:27 | call to Box2 [box1, ... (2)] | B.cpp:19:10:19:11 | b2 [box1, ... (2)] |
-| B.cpp:17:16:17:27 | call to Box2 [post constructor] [box1, ... (2)] | B.cpp:18:10:18:11 | b2 [box1, ... (2)] |
-| B.cpp:17:16:17:27 | call to Box2 [post constructor] [box1, ... (2)] | B.cpp:19:10:19:11 | b2 [box1, ... (2)] |
 | B.cpp:17:25:17:26 | b1 [elem2, ... (1)] | B.cpp:17:16:17:27 | call to Box2 [box1, ... (2)] |
-| B.cpp:17:25:17:26 | b1 [elem2, ... (1)] | B.cpp:17:16:17:27 | call to Box2 [post constructor] [box1, ... (2)] |
 | B.cpp:18:10:18:11 | b2 [box1, ... (2)] | B.cpp:18:14:18:17 | box1 [elem1, ... (1)] |
 | B.cpp:18:14:18:17 | box1 [elem1, ... (1)] | B.cpp:18:20:18:24 | elem1 |
 | B.cpp:19:10:19:11 | b2 [box1, ... (2)] | B.cpp:19:14:19:17 | box1 [elem2, ... (1)] |
 | B.cpp:19:14:19:17 | box1 [elem2, ... (1)] | B.cpp:19:20:19:24 | elem2 |
-| C.cpp:18:12:18:18 | call to C [post constructor] [s3, ... (1)] | C.cpp:19:5:19:5 | c [s3, ... (1)] |
 | C.cpp:18:12:18:18 | call to C [s3, ... (1)] | C.cpp:19:5:19:5 | c [s3, ... (1)] |
 | C.cpp:19:5:19:5 | c [s3, ... (1)] | C.cpp:27:8:27:11 | `this` parameter in func [s3, ... (1)] |
-| C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] | C.cpp:18:12:18:18 | call to C [post constructor] [s3, ... (1)] |
 | C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] | C.cpp:18:12:18:18 | call to C [s3, ... (1)] |
 | C.cpp:24:5:24:25 | ... = ... [void] | C.cpp:24:5:24:8 | this [post update] [s3, ... (1)] |
 | C.cpp:24:16:24:25 | new [void] | C.cpp:24:5:24:25 | ... = ... [void] |
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
index 4a320590036d..875e5619c840 100644
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
@@ -57,19 +57,10 @@
 | taint.cpp:84:10:84:12 | call to MyClass | taint.cpp:89:7:89:9 | mc1 |  |
 | taint.cpp:84:10:84:12 | call to MyClass | taint.cpp:90:7:90:9 | mc1 |  |
 | taint.cpp:84:10:84:12 | call to MyClass | taint.cpp:91:7:91:9 | mc1 |  |
-| taint.cpp:84:10:84:12 | call to MyClass [post constructor] | taint.cpp:86:2:86:4 | mc1 |  |
-| taint.cpp:84:10:84:12 | call to MyClass [post constructor] | taint.cpp:88:7:88:9 | mc1 |  |
-| taint.cpp:84:10:84:12 | call to MyClass [post constructor] | taint.cpp:89:7:89:9 | mc1 |  |
-| taint.cpp:84:10:84:12 | call to MyClass [post constructor] | taint.cpp:90:7:90:9 | mc1 |  |
-| taint.cpp:84:10:84:12 | call to MyClass [post constructor] | taint.cpp:91:7:91:9 | mc1 |  |
 | taint.cpp:84:15:84:17 | call to MyClass | taint.cpp:92:7:92:9 | mc2 |  |
 | taint.cpp:84:15:84:17 | call to MyClass | taint.cpp:93:7:93:9 | mc2 |  |
 | taint.cpp:84:15:84:17 | call to MyClass | taint.cpp:94:7:94:9 | mc2 |  |
 | taint.cpp:84:15:84:17 | call to MyClass | taint.cpp:95:7:95:9 | mc2 |  |
-| taint.cpp:84:15:84:17 | call to MyClass [post constructor] | taint.cpp:92:7:92:9 | mc2 |  |
-| taint.cpp:84:15:84:17 | call to MyClass [post constructor] | taint.cpp:93:7:93:9 | mc2 |  |
-| taint.cpp:84:15:84:17 | call to MyClass [post constructor] | taint.cpp:94:7:94:9 | mc2 |  |
-| taint.cpp:84:15:84:17 | call to MyClass [post constructor] | taint.cpp:95:7:95:9 | mc2 |  |
 | taint.cpp:86:2:86:4 | mc1 [post update] | taint.cpp:88:7:88:9 | mc1 |  |
 | taint.cpp:86:2:86:4 | mc1 [post update] | taint.cpp:89:7:89:9 | mc1 |  |
 | taint.cpp:86:2:86:4 | mc1 [post update] | taint.cpp:90:7:90:9 | mc1 |  |