From bacc37d886a4122a8f374c8d6955dfc6c127b935 Mon Sep 17 00:00:00 2001
From: Simon Friis Vindum <simonfv@gmail.com>
Date: Tue, 19 Nov 2024 12:07:26 +0100
Subject: [PATCH 1/4] Rust: Add additional tests for intraprocedural data flow

---
 .../dataflow/local/DataFlowStep.expected      |  92 +++++++----
 .../dataflow/local/inline-flow.expected       |   8 +
 .../dataflow/local/inline-flow.ql             |  12 ++
 .../test/library-tests/dataflow/local/main.rs | 145 ++++++++++++++----
 4 files changed, 197 insertions(+), 60 deletions(-)
 create mode 100644 rust/ql/test/library-tests/dataflow/local/inline-flow.expected
 create mode 100644 rust/ql/test/library-tests/dataflow/local/inline-flow.ql

diff --git a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected
index a9631f78aff..7803c981855 100644
--- a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected
+++ b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected
@@ -1,31 +1,61 @@
-| main.rs:2:9:2:9 | [SSA] s | main.rs:3:33:3:33 | s |
-| main.rs:2:13:2:19 | "Hello" | main.rs:2:9:2:9 | s |
-| main.rs:6:18:6:21 | [SSA] cond | main.rs:9:16:9:19 | cond |
-| main.rs:7:9:7:9 | [SSA] a | main.rs:10:9:10:9 | a |
-| main.rs:7:13:7:13 | 1 | main.rs:7:9:7:9 | a |
-| main.rs:8:9:8:9 | [SSA] b | main.rs:12:9:12:9 | b |
-| main.rs:8:13:8:13 | 2 | main.rs:8:9:8:9 | b |
-| main.rs:9:9:9:9 | [SSA] c | main.rs:14:5:14:5 | c |
-| main.rs:9:13:13:5 | IfExpr | main.rs:9:9:9:9 | c |
-| main.rs:9:21:11:5 | BlockExpr | main.rs:9:13:13:5 | IfExpr |
-| main.rs:10:9:10:9 | a | main.rs:9:21:11:5 | BlockExpr |
-| main.rs:11:12:13:5 | BlockExpr | main.rs:9:13:13:5 | IfExpr |
-| main.rs:12:9:12:9 | b | main.rs:11:12:13:5 | BlockExpr |
-| main.rs:14:5:14:5 | c | main.rs:6:37:15:1 | BlockExpr |
-| main.rs:18:9:18:9 | [SSA] a | main.rs:20:15:20:15 | a |
-| main.rs:18:13:18:13 | 1 | main.rs:18:9:18:9 | a |
-| main.rs:19:9:19:9 | [SSA] b | main.rs:22:5:22:5 | b |
-| main.rs:19:13:21:5 | LoopExpr | main.rs:19:9:19:9 | b |
-| main.rs:20:9:20:15 | BreakExpr | main.rs:19:13:21:5 | LoopExpr |
-| main.rs:20:15:20:15 | a | main.rs:20:9:20:15 | BreakExpr |
-| main.rs:22:5:22:5 | b | main.rs:17:29:23:1 | BlockExpr |
-| main.rs:26:17:26:17 | 1 | main.rs:26:9:26:13 | i |
-| main.rs:27:5:27:5 | [SSA] i | main.rs:28:5:28:5 | i |
-| main.rs:27:5:27:5 | i | main.rs:27:5:27:5 | [SSA] i |
-| main.rs:28:5:28:5 | i | main.rs:25:24:29:1 | BlockExpr |
-| main.rs:31:21:31:21 | [SSA] a | main.rs:33:20:33:20 | a |
-| main.rs:31:29:31:29 | [SSA] b | main.rs:34:17:34:17 | b |
-| main.rs:31:37:31:37 | [SSA] c | main.rs:32:11:32:11 | c |
-| main.rs:32:5:35:5 | MatchExpr | main.rs:31:60:36:1 | BlockExpr |
-| main.rs:33:20:33:20 | a | main.rs:32:5:35:5 | MatchExpr |
-| main.rs:34:17:34:17 | b | main.rs:32:5:35:5 | MatchExpr |
+| main.rs:3:11:3:11 | [SSA] i | main.rs:4:12:4:12 | i |
+| main.rs:4:5:4:12 | ... + ... | main.rs:3:26:5:1 | BlockExpr |
+| main.rs:7:9:7:9 | [SSA] s | main.rs:8:20:8:20 | s |
+| main.rs:19:9:19:9 | [SSA] s | main.rs:20:10:20:10 | s |
+| main.rs:19:13:19:21 | CallExpr | main.rs:19:9:19:9 | s |
+| main.rs:23:18:23:21 | [SSA] cond | main.rs:26:16:26:19 | cond |
+| main.rs:24:9:24:9 | [SSA] a | main.rs:26:23:26:23 | a |
+| main.rs:24:13:24:21 | CallExpr | main.rs:24:9:24:9 | a |
+| main.rs:25:9:25:9 | [SSA] b | main.rs:26:34:26:34 | b |
+| main.rs:25:13:25:13 | 2 | main.rs:25:9:25:9 | b |
+| main.rs:26:9:26:9 | [SSA] c | main.rs:27:10:27:10 | c |
+| main.rs:26:13:26:36 | IfExpr | main.rs:26:9:26:9 | c |
+| main.rs:26:21:26:25 | BlockExpr | main.rs:26:13:26:36 | IfExpr |
+| main.rs:26:23:26:23 | a | main.rs:26:21:26:25 | BlockExpr |
+| main.rs:26:32:26:36 | BlockExpr | main.rs:26:13:26:36 | IfExpr |
+| main.rs:26:34:26:34 | b | main.rs:26:32:26:36 | BlockExpr |
+| main.rs:30:21:30:21 | [SSA] m | main.rs:32:19:32:19 | m |
+| main.rs:31:9:31:9 | [SSA] a | main.rs:33:20:33:20 | a |
+| main.rs:31:13:31:21 | CallExpr | main.rs:31:9:31:9 | a |
+| main.rs:32:9:32:9 | [SSA] b | main.rs:36:10:36:10 | b |
+| main.rs:32:13:35:5 | MatchExpr | main.rs:32:9:32:9 | b |
+| main.rs:33:20:33:20 | a | main.rs:32:13:35:5 | MatchExpr |
+| main.rs:34:17:34:17 | 0 | main.rs:32:13:35:5 | MatchExpr |
+| main.rs:40:9:40:9 | [SSA] a | main.rs:43:10:43:10 | a |
+| main.rs:40:13:42:5 | LoopExpr | main.rs:40:9:40:9 | a |
+| main.rs:41:9:41:15 | BreakExpr | main.rs:40:13:42:5 | LoopExpr |
+| main.rs:41:15:41:15 | 1 | main.rs:41:9:41:15 | BreakExpr |
+| main.rs:44:9:44:9 | [SSA] b | main.rs:47:10:47:10 | b |
+| main.rs:44:13:46:5 | LoopExpr | main.rs:44:9:44:9 | b |
+| main.rs:45:9:45:23 | BreakExpr | main.rs:44:13:46:5 | LoopExpr |
+| main.rs:45:15:45:23 | CallExpr | main.rs:45:9:45:23 | BreakExpr |
+| main.rs:51:9:51:13 | [SSA] i | main.rs:52:10:52:10 | i |
+| main.rs:51:17:51:17 | 1 | main.rs:51:9:51:13 | i |
+| main.rs:53:5:53:5 | [SSA] i | main.rs:54:10:54:10 | i |
+| main.rs:53:5:53:5 | i | main.rs:53:5:53:5 | [SSA] i |
+| main.rs:61:9:61:9 | [SSA] i | main.rs:62:11:62:11 | i |
+| main.rs:61:13:61:31 | CallExpr | main.rs:61:9:61:9 | i |
+| main.rs:66:9:66:9 | [SSA] a | main.rs:67:10:67:10 | a |
+| main.rs:66:13:66:26 | TupleExpr | main.rs:66:9:66:9 | a |
+| main.rs:67:10:67:10 | a | main.rs:68:10:68:10 | a |
+| main.rs:78:9:78:9 | [SSA] p | main.rs:83:10:83:10 | p |
+| main.rs:78:13:82:5 | RecordExpr | main.rs:78:9:78:9 | p |
+| main.rs:83:10:83:10 | p | main.rs:84:10:84:10 | p |
+| main.rs:84:10:84:10 | p | main.rs:85:10:85:10 | p |
+| main.rs:92:9:92:9 | [SSA] p | main.rs:97:38:97:38 | p |
+| main.rs:92:13:96:5 | RecordExpr | main.rs:92:9:92:9 | p |
+| main.rs:97:20:97:20 | [SSA] a | main.rs:98:10:98:10 | a |
+| main.rs:97:26:97:26 | [SSA] b | main.rs:99:10:99:10 | b |
+| main.rs:97:32:97:32 | [SSA] c | main.rs:100:10:100:10 | c |
+| main.rs:97:38:97:38 | p | main.rs:97:9:97:34 | RecordPat |
+| main.rs:104:9:104:10 | [SSA] s1 | main.rs:106:11:106:12 | s1 |
+| main.rs:104:14:104:28 | CallExpr | main.rs:104:9:104:10 | s1 |
+| main.rs:105:9:105:10 | [SSA] s2 | main.rs:110:11:110:12 | s2 |
+| main.rs:105:14:105:20 | CallExpr | main.rs:105:9:105:10 | s2 |
+| main.rs:107:14:107:14 | [SSA] n | main.rs:107:25:107:25 | n |
+| main.rs:107:20:107:26 | CallExpr | main.rs:106:5:109:5 | MatchExpr |
+| main.rs:108:17:108:23 | CallExpr | main.rs:106:5:109:5 | MatchExpr |
+| main.rs:110:5:113:5 | MatchExpr | main.rs:103:27:114:1 | BlockExpr |
+| main.rs:111:14:111:14 | [SSA] n | main.rs:111:25:111:25 | n |
+| main.rs:111:20:111:26 | CallExpr | main.rs:110:5:113:5 | MatchExpr |
+| main.rs:112:17:112:23 | CallExpr | main.rs:110:5:113:5 | MatchExpr |
diff --git a/rust/ql/test/library-tests/dataflow/local/inline-flow.expected b/rust/ql/test/library-tests/dataflow/local/inline-flow.expected
new file mode 100644
index 00000000000..816045223e2
--- /dev/null
+++ b/rust/ql/test/library-tests/dataflow/local/inline-flow.expected
@@ -0,0 +1,8 @@
+models
+edges
+nodes
+| main.rs:15:10:15:18 | CallExpr | semmle.label | CallExpr |
+subpaths
+testFailures
+#select
+| main.rs:15:10:15:18 | CallExpr | main.rs:15:10:15:18 | CallExpr | main.rs:15:10:15:18 | CallExpr | $@ | main.rs:15:10:15:18 | CallExpr | CallExpr |
diff --git a/rust/ql/test/library-tests/dataflow/local/inline-flow.ql b/rust/ql/test/library-tests/dataflow/local/inline-flow.ql
new file mode 100644
index 00000000000..ad553fe548d
--- /dev/null
+++ b/rust/ql/test/library-tests/dataflow/local/inline-flow.ql
@@ -0,0 +1,12 @@
+/**
+ * @kind path-problem
+ */
+
+import rust
+import utils.InlineFlowTest
+import DefaultFlowTest
+import ValueFlow::PathGraph
+
+from ValueFlow::PathNode source, ValueFlow::PathNode sink
+where ValueFlow::flowPath(source, sink)
+select sink, source, sink, "$@", source, source.toString()
diff --git a/rust/ql/test/library-tests/dataflow/local/main.rs b/rust/ql/test/library-tests/dataflow/local/main.rs
index e607ad35759..f4c2eeec2e1 100644
--- a/rust/ql/test/library-tests/dataflow/local/main.rs
+++ b/rust/ql/test/library-tests/dataflow/local/main.rs
@@ -1,41 +1,128 @@
-fn variable() {
-    let s = "Hello";
-    println!("{:?} data flow!", s);
+// Tests for intraprocedural data flow.
+
+fn source(i: i64) -> i64 {
+    1000 + i
 }
 
-fn if_expression(cond: bool) -> i64 {
-    let a = 1;
+fn sink(s: i64) {
+    println!("{}", s);
+}
+
+// -----------------------------------------------------------------------------
+// Simple data flow through expressions and assignments
+
+fn direct() {
+    sink(source(1)); // $ hasValueFlow=1
+}
+
+fn variable_usage() {
+    let s = source(1);
+    sink(s); // $ MISSING: hasValueFlow=1
+}
+
+fn if_expression(cond: bool) {
+    let a = source(1);
     let b = 2;
-    let c = if cond {
-        a
-    } else {
-        b
-    };
-    c
+    let c = if cond { a } else { b };
+    sink(c); // $ MISSING: hasValueFlow=1
 }
 
-fn loop_expression() -> i64 {
-    let a = 1;
-    let b = loop {
-        break a;
-    };
-    b
-}
-
-fn assignment() -> i64 {
-    let mut i = 1;
-    i = 2;
-    i
-}
-
-fn match_expression(a: i64, b: i64, c: Option<i64>) -> i64 {
-    match c {
+fn match_expression(m: Option<i64>) {
+    let a = source(1);
+    let b = match m {
         Some(_) => a,
-        None => b,
+        None => 0,
+    };
+    sink(b); // $ MISSING: hasValueFlow=1
+}
+
+fn loop_with_break() {
+    let a = loop {
+        break 1;
+    };
+    sink(a);
+    let b = loop {
+        break source(1);
+    };
+    sink(b); // $ MISSING: hasValueFlow=1
+}
+
+fn assignment() {
+    let mut i = 1;
+    sink(i);
+    i = source(2);
+    sink(i); // $ MISSING: hasValueFlow=2
+}
+
+// -----------------------------------------------------------------------------
+// Data flow through data structures by writing and reading
+
+fn box_deref() {
+    let i = Box::new(source(1));
+    sink(*i); // $ MISSING: hasValueFlow=1
+}
+
+fn tuple() {
+    let a = (source(1), 2);
+    sink(a.0); // $ MISSING: hasValueFlow=1
+    sink(a.1);
+}
+
+struct Point {
+    x: i64,
+    y: i64,
+    z: i64,
+}
+
+fn struct_field() {
+    let p = Point {
+        x: source(1),
+        y: 2,
+        z: source(3),
+    };
+    sink(p.x); // MISSING: hasValueFlow=1
+    sink(p.y);
+    sink(p.z); // MISSING: hasValueFlow=3
+}
+
+// -----------------------------------------------------------------------------
+// Data flow through data structures by pattern matching
+
+fn struct_pattern_match() {
+    let p = Point {
+        x: source(1),
+        y: 2,
+        z: source(3),
+    };
+    let Point { x: a, y: b, z: c } = p;
+    sink(a); // MISSING: hasValueFlow=1
+    sink(b);
+    sink(c); // MISSING: hasValueFlow=3
+}
+
+fn option_pattern_match() {
+    let s1 = Some(source(1));
+    let s2 = Some(2);
+    match s1 {
+        Some(n) => sink(n), // MISSING: hasValueFlow=3
+        None => sink(0),
+    }
+    match s2 {
+        Some(n) => sink(n),
+        None => sink(0),
     }
 }
 
 fn main() {
-    variable();
+    direct();
+    variable_usage();
     if_expression(true);
+    match_expression(Some(0));
+    loop_with_break();
+    assignment();
+    box_deref();
+    tuple();
+    struct_field();
+    struct_pattern_match();
+    option_pattern_match();
 }

From 23bfa8a9bcbb459d6d23743f61b53b5d01aff23c Mon Sep 17 00:00:00 2001
From: Simon Friis Vindum <simonfv@gmail.com>
Date: Tue, 19 Nov 2024 12:19:47 +0100
Subject: [PATCH 2/4] Rust: Add local data flow edge for SSA definitons

---
 .../rust/dataflow/internal/DataFlowImpl.qll   | 15 +++++++++--
 .../dataflow/local/DataFlowStep.expected      | 25 +++++++++++++++++++
 .../dataflow/local/inline-flow.expected       | 16 ++++++++++++
 .../test/library-tests/dataflow/local/main.rs |  8 +++---
 4 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
index def57805373..d4400213936 100644
--- a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
+++ b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
@@ -145,7 +145,7 @@ module Node {
 
     PatNode() { this = TPatNode(n) }
 
-    /** Gets the Pat in the AST that this node corresponds to. */
+    /** Gets the `Pat` in the AST that this node corresponds to. */
     Pat getPat() { result = n.getPat() }
   }
 
@@ -282,6 +282,10 @@ module LocalFlow {
       nodeFrom.getCfgNode().getAstNode() = s.getInitializer() and
       nodeTo.getCfgNode().getAstNode() = s.getPat()
     )
+    or
+    // An edge from a pattern to its corresponding SSA definition.
+    nodeFrom.(Node::PatNode).getPat() =
+      nodeTo.(Node::SsaNode).getDefinitionExt().getSourceVariable().getPat()
   }
 }
 
@@ -395,7 +399,14 @@ module RustDataFlow implements InputSig<Location> {
    * Holds if there is a simple local flow step from `node1` to `node2`. These
    * are the value-preserving intra-callable flow steps.
    */
-  predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo, string model) { none() }
+  predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo, string model) {
+    (
+      LocalFlow::localFlowStepCommon(nodeFrom, nodeTo)
+      or
+      SsaFlow::localFlowStep(_, nodeFrom, nodeTo, _)
+    ) and
+    model = ""
+  }
 
   /**
    * Holds if data can flow from `node1` to `node2` through a non-local step
diff --git a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected
index 7803c981855..b8a6f6b6201 100644
--- a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected
+++ b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected
@@ -1,61 +1,86 @@
 | main.rs:3:11:3:11 | [SSA] i | main.rs:4:12:4:12 | i |
+| main.rs:3:11:3:11 | i | main.rs:3:11:3:11 | [SSA] i |
 | main.rs:4:5:4:12 | ... + ... | main.rs:3:26:5:1 | BlockExpr |
 | main.rs:7:9:7:9 | [SSA] s | main.rs:8:20:8:20 | s |
+| main.rs:7:9:7:9 | s | main.rs:7:9:7:9 | [SSA] s |
 | main.rs:19:9:19:9 | [SSA] s | main.rs:20:10:20:10 | s |
+| main.rs:19:9:19:9 | s | main.rs:19:9:19:9 | [SSA] s |
 | main.rs:19:13:19:21 | CallExpr | main.rs:19:9:19:9 | s |
 | main.rs:23:18:23:21 | [SSA] cond | main.rs:26:16:26:19 | cond |
+| main.rs:23:18:23:21 | cond | main.rs:23:18:23:21 | [SSA] cond |
 | main.rs:24:9:24:9 | [SSA] a | main.rs:26:23:26:23 | a |
+| main.rs:24:9:24:9 | a | main.rs:24:9:24:9 | [SSA] a |
 | main.rs:24:13:24:21 | CallExpr | main.rs:24:9:24:9 | a |
 | main.rs:25:9:25:9 | [SSA] b | main.rs:26:34:26:34 | b |
+| main.rs:25:9:25:9 | b | main.rs:25:9:25:9 | [SSA] b |
 | main.rs:25:13:25:13 | 2 | main.rs:25:9:25:9 | b |
 | main.rs:26:9:26:9 | [SSA] c | main.rs:27:10:27:10 | c |
+| main.rs:26:9:26:9 | c | main.rs:26:9:26:9 | [SSA] c |
 | main.rs:26:13:26:36 | IfExpr | main.rs:26:9:26:9 | c |
 | main.rs:26:21:26:25 | BlockExpr | main.rs:26:13:26:36 | IfExpr |
 | main.rs:26:23:26:23 | a | main.rs:26:21:26:25 | BlockExpr |
 | main.rs:26:32:26:36 | BlockExpr | main.rs:26:13:26:36 | IfExpr |
 | main.rs:26:34:26:34 | b | main.rs:26:32:26:36 | BlockExpr |
 | main.rs:30:21:30:21 | [SSA] m | main.rs:32:19:32:19 | m |
+| main.rs:30:21:30:21 | m | main.rs:30:21:30:21 | [SSA] m |
 | main.rs:31:9:31:9 | [SSA] a | main.rs:33:20:33:20 | a |
+| main.rs:31:9:31:9 | a | main.rs:31:9:31:9 | [SSA] a |
 | main.rs:31:13:31:21 | CallExpr | main.rs:31:9:31:9 | a |
 | main.rs:32:9:32:9 | [SSA] b | main.rs:36:10:36:10 | b |
+| main.rs:32:9:32:9 | b | main.rs:32:9:32:9 | [SSA] b |
 | main.rs:32:13:35:5 | MatchExpr | main.rs:32:9:32:9 | b |
 | main.rs:33:20:33:20 | a | main.rs:32:13:35:5 | MatchExpr |
 | main.rs:34:17:34:17 | 0 | main.rs:32:13:35:5 | MatchExpr |
 | main.rs:40:9:40:9 | [SSA] a | main.rs:43:10:43:10 | a |
+| main.rs:40:9:40:9 | a | main.rs:40:9:40:9 | [SSA] a |
 | main.rs:40:13:42:5 | LoopExpr | main.rs:40:9:40:9 | a |
 | main.rs:41:9:41:15 | BreakExpr | main.rs:40:13:42:5 | LoopExpr |
 | main.rs:41:15:41:15 | 1 | main.rs:41:9:41:15 | BreakExpr |
 | main.rs:44:9:44:9 | [SSA] b | main.rs:47:10:47:10 | b |
+| main.rs:44:9:44:9 | b | main.rs:44:9:44:9 | [SSA] b |
 | main.rs:44:13:46:5 | LoopExpr | main.rs:44:9:44:9 | b |
 | main.rs:45:9:45:23 | BreakExpr | main.rs:44:13:46:5 | LoopExpr |
 | main.rs:45:15:45:23 | CallExpr | main.rs:45:9:45:23 | BreakExpr |
 | main.rs:51:9:51:13 | [SSA] i | main.rs:52:10:52:10 | i |
+| main.rs:51:9:51:13 | i | main.rs:51:9:51:13 | [SSA] i |
+| main.rs:51:9:51:13 | i | main.rs:53:5:53:5 | [SSA] i |
 | main.rs:51:17:51:17 | 1 | main.rs:51:9:51:13 | i |
 | main.rs:53:5:53:5 | [SSA] i | main.rs:54:10:54:10 | i |
 | main.rs:53:5:53:5 | i | main.rs:53:5:53:5 | [SSA] i |
 | main.rs:61:9:61:9 | [SSA] i | main.rs:62:11:62:11 | i |
+| main.rs:61:9:61:9 | i | main.rs:61:9:61:9 | [SSA] i |
 | main.rs:61:13:61:31 | CallExpr | main.rs:61:9:61:9 | i |
 | main.rs:66:9:66:9 | [SSA] a | main.rs:67:10:67:10 | a |
+| main.rs:66:9:66:9 | a | main.rs:66:9:66:9 | [SSA] a |
 | main.rs:66:13:66:26 | TupleExpr | main.rs:66:9:66:9 | a |
 | main.rs:67:10:67:10 | a | main.rs:68:10:68:10 | a |
 | main.rs:78:9:78:9 | [SSA] p | main.rs:83:10:83:10 | p |
+| main.rs:78:9:78:9 | p | main.rs:78:9:78:9 | [SSA] p |
 | main.rs:78:13:82:5 | RecordExpr | main.rs:78:9:78:9 | p |
 | main.rs:83:10:83:10 | p | main.rs:84:10:84:10 | p |
 | main.rs:84:10:84:10 | p | main.rs:85:10:85:10 | p |
 | main.rs:92:9:92:9 | [SSA] p | main.rs:97:38:97:38 | p |
+| main.rs:92:9:92:9 | p | main.rs:92:9:92:9 | [SSA] p |
 | main.rs:92:13:96:5 | RecordExpr | main.rs:92:9:92:9 | p |
 | main.rs:97:20:97:20 | [SSA] a | main.rs:98:10:98:10 | a |
+| main.rs:97:20:97:20 | a | main.rs:97:20:97:20 | [SSA] a |
 | main.rs:97:26:97:26 | [SSA] b | main.rs:99:10:99:10 | b |
+| main.rs:97:26:97:26 | b | main.rs:97:26:97:26 | [SSA] b |
 | main.rs:97:32:97:32 | [SSA] c | main.rs:100:10:100:10 | c |
+| main.rs:97:32:97:32 | c | main.rs:97:32:97:32 | [SSA] c |
 | main.rs:97:38:97:38 | p | main.rs:97:9:97:34 | RecordPat |
 | main.rs:104:9:104:10 | [SSA] s1 | main.rs:106:11:106:12 | s1 |
+| main.rs:104:9:104:10 | s1 | main.rs:104:9:104:10 | [SSA] s1 |
 | main.rs:104:14:104:28 | CallExpr | main.rs:104:9:104:10 | s1 |
 | main.rs:105:9:105:10 | [SSA] s2 | main.rs:110:11:110:12 | s2 |
+| main.rs:105:9:105:10 | s2 | main.rs:105:9:105:10 | [SSA] s2 |
 | main.rs:105:14:105:20 | CallExpr | main.rs:105:9:105:10 | s2 |
 | main.rs:107:14:107:14 | [SSA] n | main.rs:107:25:107:25 | n |
+| main.rs:107:14:107:14 | n | main.rs:107:14:107:14 | [SSA] n |
 | main.rs:107:20:107:26 | CallExpr | main.rs:106:5:109:5 | MatchExpr |
 | main.rs:108:17:108:23 | CallExpr | main.rs:106:5:109:5 | MatchExpr |
 | main.rs:110:5:113:5 | MatchExpr | main.rs:103:27:114:1 | BlockExpr |
 | main.rs:111:14:111:14 | [SSA] n | main.rs:111:25:111:25 | n |
+| main.rs:111:14:111:14 | n | main.rs:111:14:111:14 | [SSA] n |
 | main.rs:111:20:111:26 | CallExpr | main.rs:110:5:113:5 | MatchExpr |
 | main.rs:112:17:112:23 | CallExpr | main.rs:110:5:113:5 | MatchExpr |
diff --git a/rust/ql/test/library-tests/dataflow/local/inline-flow.expected b/rust/ql/test/library-tests/dataflow/local/inline-flow.expected
index 816045223e2..72273334c6f 100644
--- a/rust/ql/test/library-tests/dataflow/local/inline-flow.expected
+++ b/rust/ql/test/library-tests/dataflow/local/inline-flow.expected
@@ -1,8 +1,24 @@
 models
 edges
+| main.rs:19:13:19:21 | CallExpr : unit | main.rs:20:10:20:10 | s | provenance |  |
+| main.rs:24:13:24:21 | CallExpr : unit | main.rs:27:10:27:10 | c | provenance |  |
+| main.rs:31:13:31:21 | CallExpr : unit | main.rs:36:10:36:10 | b | provenance |  |
+| main.rs:45:15:45:23 | CallExpr : unit | main.rs:47:10:47:10 | b | provenance |  |
 nodes
 | main.rs:15:10:15:18 | CallExpr | semmle.label | CallExpr |
+| main.rs:19:13:19:21 | CallExpr : unit | semmle.label | CallExpr : unit |
+| main.rs:20:10:20:10 | s | semmle.label | s |
+| main.rs:24:13:24:21 | CallExpr : unit | semmle.label | CallExpr : unit |
+| main.rs:27:10:27:10 | c | semmle.label | c |
+| main.rs:31:13:31:21 | CallExpr : unit | semmle.label | CallExpr : unit |
+| main.rs:36:10:36:10 | b | semmle.label | b |
+| main.rs:45:15:45:23 | CallExpr : unit | semmle.label | CallExpr : unit |
+| main.rs:47:10:47:10 | b | semmle.label | b |
 subpaths
 testFailures
 #select
 | main.rs:15:10:15:18 | CallExpr | main.rs:15:10:15:18 | CallExpr | main.rs:15:10:15:18 | CallExpr | $@ | main.rs:15:10:15:18 | CallExpr | CallExpr |
+| main.rs:20:10:20:10 | s | main.rs:19:13:19:21 | CallExpr : unit | main.rs:20:10:20:10 | s | $@ | main.rs:19:13:19:21 | CallExpr : unit | CallExpr : unit |
+| main.rs:27:10:27:10 | c | main.rs:24:13:24:21 | CallExpr : unit | main.rs:27:10:27:10 | c | $@ | main.rs:24:13:24:21 | CallExpr : unit | CallExpr : unit |
+| main.rs:36:10:36:10 | b | main.rs:31:13:31:21 | CallExpr : unit | main.rs:36:10:36:10 | b | $@ | main.rs:31:13:31:21 | CallExpr : unit | CallExpr : unit |
+| main.rs:47:10:47:10 | b | main.rs:45:15:45:23 | CallExpr : unit | main.rs:47:10:47:10 | b | $@ | main.rs:45:15:45:23 | CallExpr : unit | CallExpr : unit |
diff --git a/rust/ql/test/library-tests/dataflow/local/main.rs b/rust/ql/test/library-tests/dataflow/local/main.rs
index f4c2eeec2e1..fb79baa70bd 100644
--- a/rust/ql/test/library-tests/dataflow/local/main.rs
+++ b/rust/ql/test/library-tests/dataflow/local/main.rs
@@ -17,14 +17,14 @@ fn direct() {
 
 fn variable_usage() {
     let s = source(1);
-    sink(s); // $ MISSING: hasValueFlow=1
+    sink(s); // $ hasValueFlow=1
 }
 
 fn if_expression(cond: bool) {
     let a = source(1);
     let b = 2;
     let c = if cond { a } else { b };
-    sink(c); // $ MISSING: hasValueFlow=1
+    sink(c); // $ hasValueFlow=1
 }
 
 fn match_expression(m: Option<i64>) {
@@ -33,7 +33,7 @@ fn match_expression(m: Option<i64>) {
         Some(_) => a,
         None => 0,
     };
-    sink(b); // $ MISSING: hasValueFlow=1
+    sink(b); // $ hasValueFlow=1
 }
 
 fn loop_with_break() {
@@ -44,7 +44,7 @@ fn loop_with_break() {
     let b = loop {
         break source(1);
     };
-    sink(b); // $ MISSING: hasValueFlow=1
+    sink(b); // $ hasValueFlow=1
 }
 
 fn assignment() {

From 6ae979293ceffc6e0931206ac125351aa9c8d711 Mon Sep 17 00:00:00 2001
From: Simon Friis Vindum <simonfv@gmail.com>
Date: Tue, 19 Nov 2024 13:26:01 +0100
Subject: [PATCH 3/4] Rust: Accept inconsistencies

---
 .../dataflow/barrier/inline-flow.expected              | 10 ++++++++--
 .../CONSISTENCY/DataFlowConsistency.expected           |  4 ++++
 .../CONSISTENCY/DataFlowConsistency.expected           |  5 +++++
 .../variables/CONSISTENCY/DataFlowConsistency.expected |  5 +++++
 .../CONSISTENCY/DataFlowConsistency.expected           |  7 +++++++
 5 files changed, 29 insertions(+), 2 deletions(-)
 create mode 100644 rust/ql/test/library-tests/definitions/CONSISTENCY/DataFlowConsistency.expected
 create mode 100644 rust/ql/test/library-tests/formatstrings/CONSISTENCY/DataFlowConsistency.expected
 create mode 100644 rust/ql/test/library-tests/variables/CONSISTENCY/DataFlowConsistency.expected
 create mode 100644 rust/ql/test/query-tests/unusedentities/CONSISTENCY/DataFlowConsistency.expected

diff --git a/rust/ql/test/library-tests/dataflow/barrier/inline-flow.expected b/rust/ql/test/library-tests/dataflow/barrier/inline-flow.expected
index c59ea4cc670..a54c75d0c17 100644
--- a/rust/ql/test/library-tests/dataflow/barrier/inline-flow.expected
+++ b/rust/ql/test/library-tests/dataflow/barrier/inline-flow.expected
@@ -1,10 +1,16 @@
 models
 edges
+| main.rs:21:13:21:21 | CallExpr : unit | main.rs:22:10:22:10 | s | provenance |  |
+| main.rs:32:13:32:21 | CallExpr : unit | main.rs:33:10:33:10 | s | provenance |  |
 nodes
 | main.rs:17:10:17:18 | CallExpr | semmle.label | CallExpr |
+| main.rs:21:13:21:21 | CallExpr : unit | semmle.label | CallExpr : unit |
+| main.rs:22:10:22:10 | s | semmle.label | s |
+| main.rs:32:13:32:21 | CallExpr : unit | semmle.label | CallExpr : unit |
+| main.rs:33:10:33:10 | s | semmle.label | s |
 subpaths
 testFailures
-| main.rs:22:14:22:32 | Comment | Missing result: hasValueFlow=1 |
-| main.rs:33:14:33:32 | Comment | Missing result: hasValueFlow=1 |
 #select
 | main.rs:17:10:17:18 | CallExpr | main.rs:17:10:17:18 | CallExpr | main.rs:17:10:17:18 | CallExpr | $@ | main.rs:17:10:17:18 | CallExpr | CallExpr |
+| main.rs:22:10:22:10 | s | main.rs:21:13:21:21 | CallExpr : unit | main.rs:22:10:22:10 | s | $@ | main.rs:21:13:21:21 | CallExpr : unit | CallExpr : unit |
+| main.rs:33:10:33:10 | s | main.rs:32:13:32:21 | CallExpr : unit | main.rs:33:10:33:10 | s | $@ | main.rs:32:13:32:21 | CallExpr : unit | CallExpr : unit |
diff --git a/rust/ql/test/library-tests/definitions/CONSISTENCY/DataFlowConsistency.expected b/rust/ql/test/library-tests/definitions/CONSISTENCY/DataFlowConsistency.expected
new file mode 100644
index 00000000000..8d729bb5fd9
--- /dev/null
+++ b/rust/ql/test/library-tests/definitions/CONSISTENCY/DataFlowConsistency.expected
@@ -0,0 +1,4 @@
+uniqueEnclosingCallable
+| main.rs:5:29:5:33 | width | Node should have one enclosing callable but has 0. |
+| main.rs:5:36:5:44 | precision | Node should have one enclosing callable but has 0. |
+| main.rs:9:22:9:27 | people | Node should have one enclosing callable but has 0. |
diff --git a/rust/ql/test/library-tests/formatstrings/CONSISTENCY/DataFlowConsistency.expected b/rust/ql/test/library-tests/formatstrings/CONSISTENCY/DataFlowConsistency.expected
new file mode 100644
index 00000000000..5bd870a7205
--- /dev/null
+++ b/rust/ql/test/library-tests/formatstrings/CONSISTENCY/DataFlowConsistency.expected
@@ -0,0 +1,5 @@
+uniqueEnclosingCallable
+| main.rs:5:29:5:33 | width | Node should have one enclosing callable but has 0. |
+| main.rs:5:36:5:44 | precision | Node should have one enclosing callable but has 0. |
+| main.rs:16:22:16:27 | people | Node should have one enclosing callable but has 0. |
+| main.rs:27:23:27:27 | width | Node should have one enclosing callable but has 0. |
diff --git a/rust/ql/test/library-tests/variables/CONSISTENCY/DataFlowConsistency.expected b/rust/ql/test/library-tests/variables/CONSISTENCY/DataFlowConsistency.expected
new file mode 100644
index 00000000000..1a8949ec25f
--- /dev/null
+++ b/rust/ql/test/library-tests/variables/CONSISTENCY/DataFlowConsistency.expected
@@ -0,0 +1,5 @@
+localFlowIsLocal
+| variables.rs:400:9:400:9 | x | variables.rs:402:15:404:5 | [SSA] <captured entry> x | Local flow step does not preserve enclosing callable. |
+| variables.rs:410:9:410:13 | x | variables.rs:412:20:414:5 | [SSA] <captured entry> x | Local flow step does not preserve enclosing callable. |
+| variables.rs:418:9:418:13 | y | variables.rs:421:9:421:9 | [SSA] y | Local flow step does not preserve enclosing callable. |
+| variables.rs:436:9:436:13 | i | variables.rs:438:9:438:9 | [SSA] i | Local flow step does not preserve enclosing callable. |
diff --git a/rust/ql/test/query-tests/unusedentities/CONSISTENCY/DataFlowConsistency.expected b/rust/ql/test/query-tests/unusedentities/CONSISTENCY/DataFlowConsistency.expected
new file mode 100644
index 00000000000..47300d3b9fa
--- /dev/null
+++ b/rust/ql/test/query-tests/unusedentities/CONSISTENCY/DataFlowConsistency.expected
@@ -0,0 +1,7 @@
+uniqueEnclosingCallable
+| main.rs:194:25:194:25 | x | Node should have one enclosing callable but has 0. |
+| main.rs:198:28:198:28 | x | Node should have one enclosing callable but has 0. |
+| main.rs:202:28:202:28 | x | Node should have one enclosing callable but has 0. |
+| main.rs:206:28:206:28 | x | Node should have one enclosing callable but has 0. |
+localFlowIsLocal
+| main.rs:432:9:432:10 | i6 | main.rs:434:20:434:44 | [SSA] <captured entry> i6 | Local flow step does not preserve enclosing callable. |

From 2c9bee6208ec957d3400ba3dd66dc3a0d8af68c6 Mon Sep 17 00:00:00 2001
From: Simon Friis Vindum <simonfv@gmail.com>
Date: Tue, 19 Nov 2024 14:32:31 +0100
Subject: [PATCH 4/4] Rust: Only add data flow edge to SSA write definitions
 from their underlying CFG node

---
 .../rust/dataflow/internal/DataFlowImpl.qll    | 18 +++++++-----------
 .../dataflow/local/DataFlowStep.expected       |  1 -
 .../CONSISTENCY/DataFlowConsistency.expected   |  5 -----
 .../CONSISTENCY/DataFlowConsistency.expected   |  2 --
 4 files changed, 7 insertions(+), 19 deletions(-)
 delete mode 100644 rust/ql/test/library-tests/variables/CONSISTENCY/DataFlowConsistency.expected

diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
index d4400213936..c31ef9e8107 100644
--- a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
+++ b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
@@ -112,7 +112,7 @@ module Node {
   }
 
   /** A data flow node that corresponds to a CFG node for an AST node. */
-  abstract private class AstCfgFlowNode extends Node {
+  abstract class AstCfgFlowNode extends Node {
     AstCfgNode n;
 
     override CfgNode getCfgNode() { result = n }
@@ -283,9 +283,11 @@ module LocalFlow {
       nodeTo.getCfgNode().getAstNode() = s.getPat()
     )
     or
-    // An edge from a pattern to its corresponding SSA definition.
-    nodeFrom.(Node::PatNode).getPat() =
-      nodeTo.(Node::SsaNode).getDefinitionExt().getSourceVariable().getPat()
+    // An edge from a pattern/expression to its corresponding SSA definition.
+    nodeFrom.(Node::AstCfgFlowNode).getCfgNode() =
+      nodeTo.(Node::SsaNode).getDefinitionExt().(Ssa::WriteDefinition).getControlFlowNode()
+    or
+    SsaFlow::localFlowStep(_, nodeFrom, nodeTo, _)
   }
 }
 
@@ -400,11 +402,7 @@ module RustDataFlow implements InputSig<Location> {
    * are the value-preserving intra-callable flow steps.
    */
   predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo, string model) {
-    (
-      LocalFlow::localFlowStepCommon(nodeFrom, nodeTo)
-      or
-      SsaFlow::localFlowStep(_, nodeFrom, nodeTo, _)
-    ) and
+    LocalFlow::localFlowStepCommon(nodeFrom, nodeTo) and
     model = ""
   }
 
@@ -528,8 +526,6 @@ private module Cached {
   cached
   predicate localFlowStepImpl(Node::Node nodeFrom, Node::Node nodeTo) {
     LocalFlow::localFlowStepCommon(nodeFrom, nodeTo)
-    or
-    SsaFlow::localFlowStep(_, nodeFrom, nodeTo, _)
   }
 }
 
diff --git a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected
index b8a6f6b6201..8cc12598f62 100644
--- a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected
+++ b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected
@@ -43,7 +43,6 @@
 | main.rs:45:15:45:23 | CallExpr | main.rs:45:9:45:23 | BreakExpr |
 | main.rs:51:9:51:13 | [SSA] i | main.rs:52:10:52:10 | i |
 | main.rs:51:9:51:13 | i | main.rs:51:9:51:13 | [SSA] i |
-| main.rs:51:9:51:13 | i | main.rs:53:5:53:5 | [SSA] i |
 | main.rs:51:17:51:17 | 1 | main.rs:51:9:51:13 | i |
 | main.rs:53:5:53:5 | [SSA] i | main.rs:54:10:54:10 | i |
 | main.rs:53:5:53:5 | i | main.rs:53:5:53:5 | [SSA] i |
diff --git a/rust/ql/test/library-tests/variables/CONSISTENCY/DataFlowConsistency.expected b/rust/ql/test/library-tests/variables/CONSISTENCY/DataFlowConsistency.expected
deleted file mode 100644
index 1a8949ec25f..00000000000
--- a/rust/ql/test/library-tests/variables/CONSISTENCY/DataFlowConsistency.expected
+++ /dev/null
@@ -1,5 +0,0 @@
-localFlowIsLocal
-| variables.rs:400:9:400:9 | x | variables.rs:402:15:404:5 | [SSA] <captured entry> x | Local flow step does not preserve enclosing callable. |
-| variables.rs:410:9:410:13 | x | variables.rs:412:20:414:5 | [SSA] <captured entry> x | Local flow step does not preserve enclosing callable. |
-| variables.rs:418:9:418:13 | y | variables.rs:421:9:421:9 | [SSA] y | Local flow step does not preserve enclosing callable. |
-| variables.rs:436:9:436:13 | i | variables.rs:438:9:438:9 | [SSA] i | Local flow step does not preserve enclosing callable. |
diff --git a/rust/ql/test/query-tests/unusedentities/CONSISTENCY/DataFlowConsistency.expected b/rust/ql/test/query-tests/unusedentities/CONSISTENCY/DataFlowConsistency.expected
index 47300d3b9fa..0a61a151c20 100644
--- a/rust/ql/test/query-tests/unusedentities/CONSISTENCY/DataFlowConsistency.expected
+++ b/rust/ql/test/query-tests/unusedentities/CONSISTENCY/DataFlowConsistency.expected
@@ -3,5 +3,3 @@ uniqueEnclosingCallable
 | main.rs:198:28:198:28 | x | Node should have one enclosing callable but has 0. |
 | main.rs:202:28:202:28 | x | Node should have one enclosing callable but has 0. |
 | main.rs:206:28:206:28 | x | Node should have one enclosing callable but has 0. |
-localFlowIsLocal
-| main.rs:432:9:432:10 | i6 | main.rs:434:20:434:44 | [SSA] <captured entry> i6 | Local flow step does not preserve enclosing callable. |