From 1753a7e1464b538ca70360f5ae1a02c449eedf53 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Wed, 15 Nov 2023 14:19:50 +0000
Subject: [PATCH 1/8] C++: Add tests.

---
 .../dataflow-consistency.expected             |  6 ++
 .../dataflow/dataflow-tests/flowOut.cpp       | 66 +++++++++++++++++--
 .../dataflow-tests/uninitialized.expected     |  4 ++
 3 files changed, 70 insertions(+), 6 deletions(-)

diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected
index a98cfd7e22ac..c8d2d4bb97b8 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected
@@ -23,6 +23,7 @@ uniquePostUpdate
 postIsInSameCallable
 reverseRead
 argHasPostUpdate
+| flowOut.cpp:55:14:55:16 | * ... | ArgumentNode is missing PostUpdateNode. |
 | lambdas.cpp:18:7:18:7 | a | ArgumentNode is missing PostUpdateNode. |
 | lambdas.cpp:25:2:25:2 | b | ArgumentNode is missing PostUpdateNode. |
 | lambdas.cpp:32:2:32:2 | c | ArgumentNode is missing PostUpdateNode. |
@@ -51,7 +52,12 @@ postWithInFlow
 | example.c:28:23:28:25 | pos [inner post update] | PostUpdateNode should not be the target of local flow. |
 | flowOut.cpp:5:5:5:12 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
 | flowOut.cpp:5:6:5:12 | toTaint [inner post update] | PostUpdateNode should not be the target of local flow. |
+| flowOut.cpp:8:5:8:12 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
+| flowOut.cpp:8:6:8:12 | toTaint [inner post update] | PostUpdateNode should not be the target of local flow. |
 | flowOut.cpp:18:17:18:17 | x [inner post update] | PostUpdateNode should not be the target of local flow. |
+| flowOut.cpp:30:12:30:12 | x [inner post update] | PostUpdateNode should not be the target of local flow. |
+| flowOut.cpp:37:5:37:6 | p2 [inner post update] | PostUpdateNode should not be the target of local flow. |
+| flowOut.cpp:37:5:37:9 | access to array [post update] | PostUpdateNode should not be the target of local flow. |
 | globals.cpp:13:5:13:19 | flowTestGlobal1 [post update] | PostUpdateNode should not be the target of local flow. |
 | globals.cpp:23:5:23:19 | flowTestGlobal2 [post update] | PostUpdateNode should not be the target of local flow. |
 | lambdas.cpp:23:3:23:14 | v [post update] | PostUpdateNode should not be the target of local flow. |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp
index 2503a2acc07c..9553bc5d4f5e 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp
@@ -1,20 +1,74 @@
-int source();
-void sink(int);
+int source(); char source(bool);
+void sink(int); void sink(char);
 
 void source_ref(int *toTaint) { // $ ir-def=*toTaint ast-def=toTaint
     *toTaint = source();
 }
-
-
-
+void source_ref(char *toTaint) { // $ ir-def=*toTaint ast-def=toTaint
+    *toTaint = source();
+}
 void modify_copy(int* ptr) { // $ ast-def=ptr
   int deref = *ptr;
   int* other = &deref;
   source_ref(other);
 }
 
-void test_output() {
+void test_output_copy() {
    int x = 0;
    modify_copy(&x);
    sink(x); // $ SPURIOUS: ir
+}
+
+void modify(int* ptr) { // $ ast-def=ptr
+  int* deref = ptr;
+  int* other = &*deref;
+  source_ref(other);
+}
+
+void test_output() {
+   int x = 0;
+   modify(&x);
+   sink(x); // $ SPURIOUS: ir MISSING: ast
+}
+
+void modify_copy_of_pointer(int* p, unsigned len) { // $ ast-def=p
+  int* p2 = new int[len];
+  for(unsigned i = 0; i < len; ++i) {
+    p2[i] = p[i];
+  }
+
+  source_ref(p2);
+}
+
+void test_modify_copy_of_pointer() {
+  int x[10];
+  modify_copy_of_pointer(x, 10);
+  sink(x[0]); // $ SPURIOUS: ir,ast
+}
+
+void modify_pointer(int* p, unsigned len) { // $ ast-def=p
+  int** p2 = &p;
+  for(unsigned i = 0; i < len; ++i) {
+    *p2[i] = p[i];
+  }
+
+  source_ref(*p2);
+}
+
+void test_modify_of_pointer() {
+  int x[10];
+  modify_pointer(x, 10);
+  sink(x[0]); // $ ast,ir
+}
+
+char* strdup(const char* p);
+
+void modify_copy_via_strdup(char* p) { // $ ast-def=p
+  char* p2 = strdup(p);
+  source_ref(p2);
+}
+
+void test_modify_copy_via_strdup(char* p) { // $ ast-def=p
+  modify_copy_via_strdup(p);
+  sink(*p); // $ SPURIOUS: ir
 }
\ No newline at end of file
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected
index 722909678573..a7539412adb5 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected
@@ -1,3 +1,7 @@
+| flowOut.cpp:44:7:44:7 | x | flowOut.cpp:45:26:45:26 | x |
+| flowOut.cpp:44:7:44:7 | x | flowOut.cpp:46:8:46:8 | x |
+| flowOut.cpp:59:7:59:7 | x | flowOut.cpp:60:18:60:18 | x |
+| flowOut.cpp:59:7:59:7 | x | flowOut.cpp:61:8:61:8 | x |
 | ref.cpp:53:9:53:10 | x1 | ref.cpp:55:19:55:20 | x1 |
 | ref.cpp:53:9:53:10 | x1 | ref.cpp:56:10:56:11 | x1 |
 | ref.cpp:53:13:53:14 | x2 | ref.cpp:58:15:58:16 | x2 |

From 064f68fdcaf9970f0e1de94d864c182af0cbd250 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Wed, 15 Nov 2023 14:01:10 +0000
Subject: [PATCH 2/8] DataFlow: Add a predicate for modifying which dataflow
 steps participate in flow-through summaries.

---
 shared/dataflow/codeql/dataflow/DataFlow.qll               | 7 +++++++
 .../codeql/dataflow/internal/DataFlowImplCommon.qll        | 6 ++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/shared/dataflow/codeql/dataflow/DataFlow.qll b/shared/dataflow/codeql/dataflow/DataFlow.qll
index 57694df09488..fb262a9d23bc 100644
--- a/shared/dataflow/codeql/dataflow/DataFlow.qll
+++ b/shared/dataflow/codeql/dataflow/DataFlow.qll
@@ -170,6 +170,13 @@ signature module InputSig {
 
   predicate simpleLocalFlowStep(Node node1, Node node2);
 
+  /**
+   * Holds if the data flow step from `node1` to `node2` can be used when
+   * computing flow-through summaries.
+   */
+  bindingset[node1, node2]
+  default predicate flowThroughStepAllowed(Node node1, Node node2) { any() }
+
   /**
    * Holds if data can flow from `node1` to `node2` through a non-local step
    * that does not follow a call edge. For example, a step through a global
diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll
index ab0562e17cb8..4098157d535d 100644
--- a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll
+++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll
@@ -551,7 +551,8 @@ module MakeImplCommon<InputSig Lang> {
           // local flow
           exists(Node mid |
             parameterValueFlowCand(p, mid, read) and
-            simpleLocalFlowStep(mid, node)
+            simpleLocalFlowStep(mid, node) and
+            flowThroughStepAllowed(mid, node)
           )
           or
           // read
@@ -670,7 +671,8 @@ module MakeImplCommon<InputSig Lang> {
           // local flow
           exists(Node mid |
             parameterValueFlow(p, mid, read) and
-            simpleLocalFlowStep(mid, node)
+            simpleLocalFlowStep(mid, node) and
+            flowThroughStepAllowed(mid, node)
           )
           or
           // read

From 9049932f42ee094014a615685e044296c8717b23 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Wed, 15 Nov 2023 14:17:58 +0000
Subject: [PATCH 3/8] C++: Implement the new predicate.

---
 .../internal/DataFlowImplSpecific.qll         |  2 +
 .../ir/dataflow/internal/DataFlowPrivate.qll  | 50 +++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplSpecific.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplSpecific.qll
index f49eaf359972..ef0b0aa52a9c 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplSpecific.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplSpecific.qll
@@ -20,4 +20,6 @@ module CppDataFlow implements InputSig {
   Node exprNode(DataFlowExpr e) { result = Public::exprNode(e) }
 
   predicate getAdditionalFlowIntoCallNodeTerm = Private::getAdditionalFlowIntoCallNodeTerm/2;
+
+  predicate flowThroughStepAllowed = Private::flowThroughStepAllowed/2;
 }
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
index 7b1a9ca31238..8653a2d8b563 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
@@ -1149,3 +1149,53 @@ private int countNumberOfBranchesUsingParameter(SwitchInstruction switch, Parame
       )
   )
 }
+
+/**
+ * Holds if the dataflow step from `node1` to `node2` should not be used when
+ * computing flow-through summaries because the dataflow step copies the value
+ * of `node1` to `node2` in a way that does not preserve the identity of the
+ * value. For example the assignment to `x` that reads the value of `*p` in:
+ * ```cpp
+ * int* p = ...
+ * int x = *p;
+ * ```
+ */
+bindingset[node1, node2]
+pragma[inline_late]
+predicate flowThroughStepAllowed(Node node1, Node node2) {
+  // When flow-through summaries are computed we track which parameters flow to out-going parameters.
+  // In an example such as:
+  // ```
+  // modify(int* px) { *px = source(); }
+  // void modify_copy(int* p) {
+  //   int x = *p;
+  //   modify(&x);
+  // }
+  // ```
+  // since dataflow tracks each indirection as a separate SSA variable dataflow
+  // sees the above roughly as
+  // ```
+  // modify(int* px, int deref_px) { deref_px = source(); }
+  // void modify_copy(int* p, int deref_p) {
+  //   int x = deref_p;
+  //   modify(&x, x);
+  // }
+  // ```
+  // and when dataflow computes flow from a parameter to a post-update node to
+  // conclude which parameters are "updated" by the call to `modify_copy` it
+  // finds flow from `x [post update]` to `deref_p [post update]`.
+  // To prevent this we exclude steps that don't preserve identity. We do this
+  // by excluding flow from the right-hand side of `StoreInstruction`s to the
+  // `StoreInstruction`. This is sufficient because, for flow-through summaries,
+  // we're only interested in indirect parameters such as `deref_p` in the
+  // exampe above (i.e., the parameters with a non-zero indirection index), and
+  // if that ever flows to the right-hand side of a `StoreInstruction` then
+  // there must have been a dereference to reduce its indirection index down to
+  // 0.
+  not exists(Operand operand |
+    node1.asOperand() = operand and
+    node2.asInstruction().(StoreInstruction).getSourceValueOperand() = operand
+  )
+  // TODO: Also block flow through models that don't preserve identity such
+  // as `strdup`.
+}

From 1771d77c23294a2aefad5e20952f1902832f5beb Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Wed, 15 Nov 2023 14:38:27 +0000
Subject: [PATCH 4/8] C++: Accept test changes.

---
 cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp
index 9553bc5d4f5e..bd09da05dd6a 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp
@@ -16,7 +16,7 @@ void modify_copy(int* ptr) { // $ ast-def=ptr
 void test_output_copy() {
    int x = 0;
    modify_copy(&x);
-   sink(x); // $ SPURIOUS: ir
+   sink(x); // clean
 }
 
 void modify(int* ptr) { // $ ast-def=ptr
@@ -43,7 +43,7 @@ void modify_copy_of_pointer(int* p, unsigned len) { // $ ast-def=p
 void test_modify_copy_of_pointer() {
   int x[10];
   modify_copy_of_pointer(x, 10);
-  sink(x[0]); // $ SPURIOUS: ir,ast
+  sink(x[0]); // $ SPURIOUS: ast // clean
 }
 
 void modify_pointer(int* p, unsigned len) { // $ ast-def=p

From fb6329fbc1d7040273bcb5dd9998c0fdecc68363 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 28 Nov 2023 12:06:33 +0000
Subject: [PATCH 5/8] C++: Fix test annotation

---
 cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp
index bd09da05dd6a..fbcad72f1df1 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp
@@ -28,7 +28,7 @@ void modify(int* ptr) { // $ ast-def=ptr
 void test_output() {
    int x = 0;
    modify(&x);
-   sink(x); // $ SPURIOUS: ir MISSING: ast
+   sink(x); // $ ir MISSING: ast
 }
 
 void modify_copy_of_pointer(int* p, unsigned len) { // $ ast-def=p
@@ -71,4 +71,4 @@ void modify_copy_via_strdup(char* p) { // $ ast-def=p
 void test_modify_copy_via_strdup(char* p) { // $ ast-def=p
   modify_copy_via_strdup(p);
   sink(*p); // $ SPURIOUS: ir
-}
\ No newline at end of file
+}

From e47ad274ea9e06c1a3880786632c587df754387e Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 28 Nov 2023 13:37:08 +0000
Subject: [PATCH 6/8] C++: Add Schack's tests.

---
 .../dataflow-consistency.expected             |  6 ++++
 .../dataflow-ir-consistency.expected          |  2 ++
 .../dataflow/dataflow-tests/flowOut.cpp       | 29 +++++++++++++++++++
 .../has-parameter-flow-out.expected           |  2 +-
 .../dataflow-tests/test-source-sink.expected  |  8 ++++-
 5 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected
index c8d2d4bb97b8..441bef2cddd5 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected
@@ -58,6 +58,12 @@ postWithInFlow
 | flowOut.cpp:30:12:30:12 | x [inner post update] | PostUpdateNode should not be the target of local flow. |
 | flowOut.cpp:37:5:37:6 | p2 [inner post update] | PostUpdateNode should not be the target of local flow. |
 | flowOut.cpp:37:5:37:9 | access to array [post update] | PostUpdateNode should not be the target of local flow. |
+| flowOut.cpp:84:3:84:7 | call to deref [inner post update] | PostUpdateNode should not be the target of local flow. |
+| flowOut.cpp:84:3:84:14 | access to array [post update] | PostUpdateNode should not be the target of local flow. |
+| flowOut.cpp:84:10:84:10 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
+| flowOut.cpp:90:3:90:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
+| flowOut.cpp:90:4:90:4 | q [inner post update] | PostUpdateNode should not be the target of local flow. |
+| flowOut.cpp:101:14:101:14 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
 | globals.cpp:13:5:13:19 | flowTestGlobal1 [post update] | PostUpdateNode should not be the target of local flow. |
 | globals.cpp:23:5:23:19 | flowTestGlobal2 [post update] | PostUpdateNode should not be the target of local flow. |
 | lambdas.cpp:23:3:23:14 | v [post update] | PostUpdateNode should not be the target of local flow. |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-ir-consistency.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-ir-consistency.expected
index bacd714e6148..f6e5429946ce 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-ir-consistency.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-ir-consistency.expected
@@ -12,6 +12,7 @@ compatibleTypesReflexive
 unreachableNodeCCtx
 localCallNodes
 postIsNotPre
+| flowOut.cpp:84:3:84:14 | access to array indirection | PostUpdateNode should not equal its pre-update node. |
 postHasUniquePre
 uniquePostUpdate
 | example.c:24:13:24:18 | coords indirection | Node has multiple PostUpdateNodes. |
@@ -19,6 +20,7 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+| flowOut.cpp:84:3:84:14 | access to array indirection | PostUpdateNode should not be the target of local flow. |
 | test.cpp:384:10:384:13 | memcpy output argument | PostUpdateNode should not be the target of local flow. |
 | test.cpp:391:10:391:13 | memcpy output argument | PostUpdateNode should not be the target of local flow. |
 | test.cpp:400:10:400:13 | memcpy output argument | PostUpdateNode should not be the target of local flow. |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp
index fbcad72f1df1..820174cce391 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/flowOut.cpp
@@ -72,3 +72,32 @@ void test_modify_copy_via_strdup(char* p) { // $ ast-def=p
   modify_copy_via_strdup(p);
   sink(*p); // $ SPURIOUS: ir
 }
+
+int* deref(int** p) { // $ ast-def=p
+  int* q = *p;
+  return q;
+}
+
+void test1() {
+  int x = 0;
+  int* p = &x;
+  deref(&p)[0] = source();
+  sink(*p); // $ ir MISSING: ast
+}
+
+
+void addtaint1(int* q) { // $ ast-def=q ir-def=*q
+  *q = source();
+}
+
+void addtaint2(int** p) { // $ ast-def=p
+  int* q = *p;
+  addtaint1(q);
+}
+
+void test2() {
+  int x = 0;
+  int* p = &x;
+  addtaint2(&p);
+  sink(*p); // $ ir MISSING: ast
+}
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/has-parameter-flow-out.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/has-parameter-flow-out.expected
index 6e4b117b7bbc..c831b18e2f5f 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/has-parameter-flow-out.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/has-parameter-flow-out.expected
@@ -1,3 +1,3 @@
 WARNING: Module DataFlow has been deprecated and may be removed in future (has-parameter-flow-out.ql:5,18-61)
-failures
 testFailures
+failures
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected
index c98bc68c884d..7f1acafbde6f 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected
@@ -28,6 +28,8 @@ astFlow
 | dispatch.cpp:10:37:10:42 | call to source | dispatch.cpp:44:15:44:24 | call to notSource2 |
 | dispatch.cpp:37:19:37:24 | call to source | dispatch.cpp:11:38:11:38 | x |
 | dispatch.cpp:45:18:45:23 | call to source | dispatch.cpp:11:38:11:38 | x |
+| flowOut.cpp:44:7:44:7 | x | flowOut.cpp:46:8:46:11 | access to array |
+| flowOut.cpp:59:7:59:7 | x | flowOut.cpp:61:8:61:11 | access to array |
 | globals.cpp:5:17:5:22 | call to source | globals.cpp:6:10:6:14 | local |
 | lambdas.cpp:8:10:8:15 | call to source | lambdas.cpp:14:3:14:6 | t |
 | lambdas.cpp:8:10:8:15 | call to source | lambdas.cpp:18:8:18:8 | call to operator() |
@@ -170,7 +172,11 @@ irFlow
 | dispatch.cpp:107:17:107:22 | call to source | dispatch.cpp:96:8:96:8 | x |
 | dispatch.cpp:140:8:140:13 | call to source | dispatch.cpp:96:8:96:8 | x |
 | dispatch.cpp:144:8:144:13 | call to source | dispatch.cpp:96:8:96:8 | x |
-| flowOut.cpp:5:16:5:21 | call to source | flowOut.cpp:19:9:19:9 | x |
+| flowOut.cpp:5:16:5:21 | call to source | flowOut.cpp:31:9:31:9 | x |
+| flowOut.cpp:5:16:5:21 | call to source | flowOut.cpp:61:8:61:11 | access to array |
+| flowOut.cpp:8:16:8:23 | call to source | flowOut.cpp:73:8:73:9 | * ... |
+| flowOut.cpp:84:18:84:23 | call to source | flowOut.cpp:85:8:85:9 | * ... |
+| flowOut.cpp:90:8:90:13 | call to source | flowOut.cpp:102:8:102:9 | * ... |
 | globals.cpp:5:17:5:22 | call to source | globals.cpp:6:10:6:14 | local |
 | globals.cpp:13:23:13:28 | call to source | globals.cpp:12:10:12:24 | flowTestGlobal1 |
 | globals.cpp:23:23:23:28 | call to source | globals.cpp:19:10:19:24 | flowTestGlobal2 |

From 339bf1363a16c84e815cc498bd545e40b6e1e3da Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 28 Nov 2023 13:40:19 +0000
Subject: [PATCH 7/8] DataFlow:
 s/flowThroughStepAllowed/validParameterAliasStep.

---
 .../code/cpp/ir/dataflow/internal/DataFlowImplSpecific.qll    | 2 +-
 .../semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll  | 2 +-
 shared/dataflow/codeql/dataflow/DataFlow.qll                  | 2 +-
 .../dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll  | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplSpecific.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplSpecific.qll
index ef0b0aa52a9c..21328f881f1d 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplSpecific.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplSpecific.qll
@@ -21,5 +21,5 @@ module CppDataFlow implements InputSig {
 
   predicate getAdditionalFlowIntoCallNodeTerm = Private::getAdditionalFlowIntoCallNodeTerm/2;
 
-  predicate flowThroughStepAllowed = Private::flowThroughStepAllowed/2;
+  predicate validParameterAliasStep = Private::validParameterAliasStep/2;
 }
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
index 8653a2d8b563..65aa36178248 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
@@ -1162,7 +1162,7 @@ private int countNumberOfBranchesUsingParameter(SwitchInstruction switch, Parame
  */
 bindingset[node1, node2]
 pragma[inline_late]
-predicate flowThroughStepAllowed(Node node1, Node node2) {
+predicate validParameterAliasStep(Node node1, Node node2) {
   // When flow-through summaries are computed we track which parameters flow to out-going parameters.
   // In an example such as:
   // ```
diff --git a/shared/dataflow/codeql/dataflow/DataFlow.qll b/shared/dataflow/codeql/dataflow/DataFlow.qll
index fb262a9d23bc..062cd261a0df 100644
--- a/shared/dataflow/codeql/dataflow/DataFlow.qll
+++ b/shared/dataflow/codeql/dataflow/DataFlow.qll
@@ -175,7 +175,7 @@ signature module InputSig {
    * computing flow-through summaries.
    */
   bindingset[node1, node2]
-  default predicate flowThroughStepAllowed(Node node1, Node node2) { any() }
+  default predicate validParameterAliasStep(Node node1, Node node2) { any() }
 
   /**
    * Holds if data can flow from `node1` to `node2` through a non-local step
diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll
index 4098157d535d..8955f3f6d516 100644
--- a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll
+++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll
@@ -552,7 +552,7 @@ module MakeImplCommon<InputSig Lang> {
           exists(Node mid |
             parameterValueFlowCand(p, mid, read) and
             simpleLocalFlowStep(mid, node) and
-            flowThroughStepAllowed(mid, node)
+            validParameterAliasStep(mid, node)
           )
           or
           // read
@@ -672,7 +672,7 @@ module MakeImplCommon<InputSig Lang> {
           exists(Node mid |
             parameterValueFlow(p, mid, read) and
             simpleLocalFlowStep(mid, node) and
-            flowThroughStepAllowed(mid, node)
+            validParameterAliasStep(mid, node)
           )
           or
           // read

From 911f1543e02cee987c451eb3063ef6da9dfac77a Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Tue, 28 Nov 2023 15:26:48 +0000
Subject: [PATCH 8/8] DataFlow: Adjust QLDoc.

---
 .../code/cpp/ir/dataflow/internal/DataFlowPrivate.qll  | 10 ++++++----
 shared/dataflow/codeql/dataflow/DataFlow.qll           |  4 ++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
index 65aa36178248..cd387d5137a1 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
@@ -1151,14 +1151,16 @@ private int countNumberOfBranchesUsingParameter(SwitchInstruction switch, Parame
 }
 
 /**
- * Holds if the dataflow step from `node1` to `node2` should not be used when
- * computing flow-through summaries because the dataflow step copies the value
- * of `node1` to `node2` in a way that does not preserve the identity of the
- * value. For example the assignment to `x` that reads the value of `*p` in:
+ * Holds if the data-flow step from `node1` to `node2` can be used to
+ * determine where side-effects may return from a callable.
+ * For C/C++, this means that the step from `node1` to `node2` not only
+ * preserves the value, but also preserves the identity of the value.
+ * For example, the assignment to `x` that reads the value of `*p` in
  * ```cpp
  * int* p = ...
  * int x = *p;
  * ```
+ * does not preserve the identity of `*p`.
  */
 bindingset[node1, node2]
 pragma[inline_late]
diff --git a/shared/dataflow/codeql/dataflow/DataFlow.qll b/shared/dataflow/codeql/dataflow/DataFlow.qll
index 062cd261a0df..e9d96445fa8a 100644
--- a/shared/dataflow/codeql/dataflow/DataFlow.qll
+++ b/shared/dataflow/codeql/dataflow/DataFlow.qll
@@ -171,8 +171,8 @@ signature module InputSig {
   predicate simpleLocalFlowStep(Node node1, Node node2);
 
   /**
-   * Holds if the data flow step from `node1` to `node2` can be used when
-   * computing flow-through summaries.
+   * Holds if the data-flow step from `node1` to `node2` can be used to
+   * determine where side-effects may return from a callable.
    */
   bindingset[node1, node2]
   default predicate validParameterAliasStep(Node node1, Node node2) { any() }