C++: Better support for flow-through.

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

@@ -595,10 +595,33 @@ predicate indirectReturnOutNodeInstruction0(
   instructionForfullyConvertedCall(instr, call)
 }
 
+/**
+ * Holds if `node` is an indirect operand with columns `(operand, indirectionIndex)`, and
+ * `operand` represents a use of the fully converted value of `call`.
+ */
+private predicate hasOperand(Node node, CallInstruction call, int indirectionIndex, Operand operand) {
+  indirectReturnOutNodeOperand0(call, operand, indirectionIndex) and
+  hasOperandAndIndex(node, operand, indirectionIndex)
+}
+
+/**
+ * Holds if `node` is an indirect instruction with columns `(instr, indirectionIndex)`, and
+ * `instr` represents a use of the fully converted value of `call`.
+ *
+ * Note that `hasOperand(node, _, _, _)` implies `not hasInstruction(node, _, _, _)`.
+ */
+private predicate hasInstruction(
+  Node node, CallInstruction call, int indirectionIndex, Instruction instr
+) {
+  indirectReturnOutNodeInstruction0(call, instr, indirectionIndex) and
+  hasInstructionAndIndex(node, instr, indirectionIndex)
+}
+
 /**
  * INTERNAL: do not use.
  *
- * A node representing the value of a function call.
+ * A node representing the indirect value of a function call (i.e., a value hidden
+ * behind a number of indirections).
  */
 class IndirectReturnOutNode extends Node {
   CallInstruction call;
@@ -607,20 +630,43 @@ class IndirectReturnOutNode extends Node {
   IndirectReturnOutNode() {
     // Annoyingly, we need to pick the fully converted value as the output of the function to
     // make flow through in the shared dataflow library work correctly.
-    exists(Operand operand |
+    hasOperand(this, call, indirectionIndex, _)
-      indirectReturnOutNodeOperand0(call, operand, indirectionIndex) and
-      hasOperandAndIndex(this, operand, indirectionIndex)
-    )
     or
-    exists(Instruction instr |
+    hasInstruction(this, call, indirectionIndex, _)
-      indirectReturnOutNodeInstruction0(call, instr, indirectionIndex) and
-      hasInstructionAndIndex(this, instr, indirectionIndex)
-    )
   }
 
   CallInstruction getCallInstruction() { result = call }
 
   int getIndirectionIndex() { result = indirectionIndex }
+
+  /** Gets the operand associated with this node, if any. */
+  Operand getOperand() { hasOperand(this, call, indirectionIndex, result) }
+
+  /** Gets the instruction associated with this node, if any. */
+  Instruction getInstruction() { hasInstruction(this, call, indirectionIndex, result) }
+}
+
+/**
+ * An `IndirectReturnOutNode` which is used as a destination of a store operation.
+ * When it's used for a store operation it's useful to have this be a `PostUpdateNode` for
+ * the shared dataflow library's flow-through mechanism to detect flow in cases such as:
+ * ```cpp
+ * struct MyInt {
+ *   int i;
+ *   int& getRef() { return i; }
+ * };
+ * ...
+ * MyInt mi;
+ * mi.getRef() = source(); // this is detected as a store to `i` via flow-through.
+ * sink(mi.i);
+ * ```
+ */
+private class PostIndirectReturnOutNode extends IndirectReturnOutNode, PostUpdateNode {
+  PostIndirectReturnOutNode() {
+    any(StoreInstruction store).getDestinationAddressOperand() = this.getOperand()
+  }
+
+  override Node getPreUpdateNode() { result = this }
 }
 
 private Type getTypeImpl(Type t, int indirectionIndex) {

cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected

@@ -21,6 +21,16 @@ compatibleTypesReflexive
 unreachableNodeCCtx
 localCallNodes
 postIsNotPre
+| A.cpp:98:12:98:18 | new indirection | PostUpdateNode should not equal its pre-update node. |
+| B.cpp:6:15:6:24 | new indirection | PostUpdateNode should not equal its pre-update node. |
+| B.cpp:15:15:15:27 | new indirection | PostUpdateNode should not equal its pre-update node. |
+| C.cpp:22:12:22:21 | new indirection | PostUpdateNode should not equal its pre-update node. |
+| C.cpp:24:16:24:25 | new indirection | PostUpdateNode should not equal its pre-update node. |
+| D.cpp:28:15:28:24 | new indirection | PostUpdateNode should not equal its pre-update node. |
+| D.cpp:35:15:35:24 | new indirection | PostUpdateNode should not equal its pre-update node. |
+| D.cpp:42:15:42:24 | new indirection | PostUpdateNode should not equal its pre-update node. |
+| D.cpp:49:15:49:24 | new indirection | PostUpdateNode should not equal its pre-update node. |
+| D.cpp:56:15:56:24 | new indirection | PostUpdateNode should not equal its pre-update node. |
 postHasUniquePre
 uniquePostUpdate
 | aliasing.cpp:70:11:70:11 | VariableAddress indirection | Node has multiple PostUpdateNodes. |
@@ -41,6 +51,16 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+| A.cpp:98:12:98:18 | new indirection | PostUpdateNode should not be the target of local flow. |
+| B.cpp:6:15:6:24 | new indirection | PostUpdateNode should not be the target of local flow. |
+| B.cpp:15:15:15:27 | new indirection | PostUpdateNode should not be the target of local flow. |
+| C.cpp:22:12:22:21 | new indirection | PostUpdateNode should not be the target of local flow. |
+| C.cpp:24:16:24:25 | new indirection | PostUpdateNode should not be the target of local flow. |
+| D.cpp:28:15:28:24 | new indirection | PostUpdateNode should not be the target of local flow. |
+| D.cpp:35:15:35:24 | new indirection | PostUpdateNode should not be the target of local flow. |
+| D.cpp:42:15:42:24 | new indirection | PostUpdateNode should not be the target of local flow. |
+| D.cpp:49:15:49:24 | new indirection | PostUpdateNode should not be the target of local flow. |
+| D.cpp:56:15:56:24 | new indirection | PostUpdateNode should not be the target of local flow. |
 | realistic.cpp:54:16:54:47 | memcpy output argument | PostUpdateNode should not be the target of local flow. |
 | realistic.cpp:60:16:60:18 | memcpy output argument | PostUpdateNode should not be the target of local flow. |
 viableImplInCallContextTooLarge

cpp/ql/test/library-tests/dataflow/taint-tests/arrayassignment.cpp

@@ -64,7 +64,7 @@ void test_myint_method_assignment()
 	mi.get() = source();
 
 	sink(mi); // $ MISSING: ast,ir
-	sink(mi.get()); // $ ast MISSING: ir
+	sink(mi.get()); // $ ast,ir
 }
 
 void test_myint_overloaded_assignment()
@@ -107,7 +107,7 @@ void test_myarray_method_assignment()
 
 	ma.get(0) = source();
 
-	sink(ma.get(0)); // $ MISSING: ast,ir
+	sink(ma.get(0)); // $ ir MISSING: ast
 }
 
 void test_myarray_overloaded_assignment()

cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected

@@ -320,6 +320,11 @@ compatibleTypesReflexive
 unreachableNodeCCtx
 localCallNodes
 postIsNotPre
+| condition_decls.cpp:3:13:3:22 | new indirection | PostUpdateNode should not equal its pre-update node. |
+| defdestructordeleteexpr.cpp:4:9:4:15 | new indirection | PostUpdateNode should not equal its pre-update node. |
+| deleteexpr.cpp:7:9:7:15 | new indirection | PostUpdateNode should not equal its pre-update node. |
+| ir.cpp:943:3:943:11 | new indirection | PostUpdateNode should not equal its pre-update node. |
+| ir.cpp:947:3:947:25 | new indirection | PostUpdateNode should not equal its pre-update node. |
 postHasUniquePre
 uniquePostUpdate
 | cpp11.cpp:82:17:82:17 | this indirection | Node has multiple PostUpdateNodes. |
@@ -343,13 +348,18 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+| condition_decls.cpp:3:13:3:22 | new indirection | PostUpdateNode should not be the target of local flow. |
 | cpp11.cpp:77:19:77:21 | call to Val | PostUpdateNode should not be the target of local flow. |
 | cpp11.cpp:82:11:82:14 | call to Val | PostUpdateNode should not be the target of local flow. |
 | cpp11.cpp:82:45:82:48 | call to Val | PostUpdateNode should not be the target of local flow. |
 | cpp11.cpp:82:51:82:51 | call to Val | PostUpdateNode should not be the target of local flow. |
+| defdestructordeleteexpr.cpp:4:9:4:15 | new indirection | PostUpdateNode should not be the target of local flow. |
+| deleteexpr.cpp:7:9:7:15 | new indirection | PostUpdateNode should not be the target of local flow. |
 | ir.cpp:809:7:809:13 | call to Base | PostUpdateNode should not be the target of local flow. |
 | ir.cpp:810:7:810:26 | call to Base | PostUpdateNode should not be the target of local flow. |
 | ir.cpp:823:7:823:13 | call to Base | PostUpdateNode should not be the target of local flow. |
 | ir.cpp:824:7:824:26 | call to Base | PostUpdateNode should not be the target of local flow. |
+| ir.cpp:943:3:943:11 | new indirection | PostUpdateNode should not be the target of local flow. |
+| ir.cpp:947:3:947:25 | new indirection | PostUpdateNode should not be the target of local flow. |
 | try_catch.cpp:7:8:7:8 | call to exception | PostUpdateNode should not be the target of local flow. |
 viableImplInCallContextTooLarge