Address review comments, store step for tuple indexing

Author: paldepind

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

@@ -465,7 +465,7 @@ private class StructCanonicalPath extends MkStructCanonicalPath {
 }
 
 /** Content stored in a field on a struct. */
-private class StructFieldContent extends VariantContent, TStructFieldContent {
+private class StructFieldContent extends Content, TStructFieldContent {
   private StructCanonicalPath s;
   private string field_;
 
@@ -482,7 +482,7 @@ private class StructFieldContent extends VariantContent, TStructFieldContent {
  * NOTE: Unlike `struct`s and `enum`s tuples are structural and not nominal,
  * hence we don't store a canonical path for them.
  */
-private class TuplePositionContent extends VariantContent, TTuplePositionContent {
+private class TuplePositionContent extends Content, TTuplePositionContent {
   private int pos;
 
   TuplePositionContent() { this = TTuplePositionContent(pos) }
@@ -492,6 +492,11 @@ private class TuplePositionContent extends VariantContent, TTuplePositionContent
   override string toString() { result = "tuple." + pos.toString() }
 }
 
+/** Holds if `access` indexes a tuple at an index corresponding to `c`. */
+private predicate fieldTuplePositionContent(FieldExprCfgNode access, TuplePositionContent c) {
+  access.getNameRef().getText().toInt() = c.getPosition()
+}
+
 /** A value that represents a set of `Content`s. */
 abstract class ContentSet extends TContentSet {
   /** Gets a textual representation of this element. */
@@ -687,7 +692,7 @@ module RustDataFlow implements InputSig<Location> {
     pathResolveToVariantCanonicalPath(p.getPath(), v)
   }
 
-  /** Holds if `p` destructs an struct `s`. */
+  /** Holds if `p` destructs a struct `s`. */
   pragma[nomagic]
   private predicate structDestruction(RecordPat p, StructCanonicalPath s) {
     pathResolveToStructCanonicalPath(p.getPath(), s)
@@ -722,7 +727,7 @@ module RustDataFlow implements InputSig<Location> {
       or
       exists(FieldExprCfgNode access |
         // Read of a tuple entry
-        access.getNameRef().getText().toInt() = c.(TuplePositionContent).getPosition() and
+        fieldTuplePositionContent(access, c) and
         // TODO: Handle read of a struct field.
         node1.asExpr() = access.getExpr() and
         node2.asExpr() = access
@@ -742,7 +747,7 @@ module RustDataFlow implements InputSig<Location> {
     pathResolveToVariantCanonicalPath(re.getPath(), v)
   }
 
-  /** Holds if `re` constructs a struct value of type `v`. */
+  /** Holds if `re` constructs a struct value of type `s`. */
   pragma[nomagic]
   private predicate structConstruction(RecordExpr re, StructCanonicalPath s) {
     pathResolveToStructCanonicalPath(re.getPath(), s)
@@ -780,6 +785,13 @@ module RustDataFlow implements InputSig<Location> {
         node1.asExpr() = tuple.getField(c.(TuplePositionContent).getPosition()) and
         node2.asExpr() = tuple
       )
+      or
+      exists(AssignmentExprCfgNode assignment, FieldExprCfgNode access |
+        assignment.getLhs() = access and
+        fieldTuplePositionContent(access, c) and
+        node1.asExpr() = assignment.getRhs() and
+        node2.asExpr() = access.getExpr()
+      )
     )
   }
 
@@ -788,12 +800,11 @@ module RustDataFlow implements InputSig<Location> {
    * any value stored inside `f` is cleared at the pre-update node associated with `x`
    * in `x.f = newValue`.
    */
-  predicate clearsContent(Node n, ContentSet c) {
+  predicate clearsContent(Node n, ContentSet cs) {
     exists(AssignmentExprCfgNode assignment, FieldExprCfgNode access |
       assignment.getLhs() = access and
       n.asExpr() = access.getExpr() and
-      access.getNameRef().getText().toInt() =
-        c.(SingletonContentSet).getContent().(TuplePositionContent).getPosition()
+      fieldTuplePositionContent(access, cs.(SingletonContentSet).getContent())
     )
   }
 

rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected

@@ -336,6 +336,8 @@ storeStep
 | main.rs:100:29:100:29 | 2 | tuple.2 | main.rs:100:13:100:30 | TupleExpr |
 | main.rs:108:18:108:18 | 2 | tuple.0 | main.rs:108:17:108:31 | TupleExpr |
 | main.rs:108:21:108:30 | source(...) | tuple.1 | main.rs:108:17:108:31 | TupleExpr |
+| main.rs:111:11:111:20 | source(...) | tuple.0 | main.rs:111:5:111:5 | a |
+| main.rs:112:11:112:11 | 2 | tuple.1 | main.rs:112:5:112:5 | a |
 | main.rs:118:14:118:14 | 3 | tuple.0 | main.rs:118:13:118:27 | TupleExpr |
 | main.rs:118:17:118:26 | source(...) | tuple.1 | main.rs:118:13:118:27 | TupleExpr |
 | main.rs:119:14:119:14 | a | tuple.0 | main.rs:119:13:119:18 | TupleExpr |

rust/ql/test/library-tests/dataflow/local/inline-flow.expected

@@ -11,6 +11,10 @@ edges
 | main.rs:108:17:108:31 | TupleExpr [tuple.1] | main.rs:110:10:110:10 | a [tuple.1] | provenance |  |
 | main.rs:108:21:108:30 | source(...) | main.rs:108:17:108:31 | TupleExpr [tuple.1] | provenance |  |
 | main.rs:110:10:110:10 | a [tuple.1] | main.rs:110:10:110:12 | a.1 | provenance |  |
+| main.rs:111:5:111:5 | a [tuple.0] | main.rs:112:5:112:5 | a [tuple.0] | provenance |  |
+| main.rs:111:11:111:20 | source(...) | main.rs:111:5:111:5 | a [tuple.0] | provenance |  |
+| main.rs:112:5:112:5 | a [tuple.0] | main.rs:113:10:113:10 | a [tuple.0] | provenance |  |
+| main.rs:113:10:113:10 | a [tuple.0] | main.rs:113:10:113:12 | a.0 | provenance |  |
 | main.rs:118:13:118:27 | TupleExpr [tuple.1] | main.rs:119:14:119:14 | a [tuple.1] | provenance |  |
 | main.rs:118:17:118:26 | source(...) | main.rs:118:13:118:27 | TupleExpr [tuple.1] | provenance |  |
 | main.rs:119:13:119:18 | TupleExpr [tuple.0, tuple.1] | main.rs:121:10:121:10 | b [tuple.0, tuple.1] | provenance |  |
@@ -59,6 +63,11 @@ nodes
 | main.rs:108:21:108:30 | source(...) | semmle.label | source(...) |
 | main.rs:110:10:110:10 | a [tuple.1] | semmle.label | a [tuple.1] |
 | main.rs:110:10:110:12 | a.1 | semmle.label | a.1 |
+| main.rs:111:5:111:5 | a [tuple.0] | semmle.label | a [tuple.0] |
+| main.rs:111:11:111:20 | source(...) | semmle.label | source(...) |
+| main.rs:112:5:112:5 | a [tuple.0] | semmle.label | a [tuple.0] |
+| main.rs:113:10:113:10 | a [tuple.0] | semmle.label | a [tuple.0] |
+| main.rs:113:10:113:12 | a.0 | semmle.label | a.0 |
 | main.rs:118:13:118:27 | TupleExpr [tuple.1] | semmle.label | TupleExpr [tuple.1] |
 | main.rs:118:17:118:26 | source(...) | semmle.label | source(...) |
 | main.rs:119:13:119:18 | TupleExpr [tuple.0, tuple.1] | semmle.label | TupleExpr [tuple.0, tuple.1] |
@@ -103,6 +112,7 @@ testFailures
 | main.rs:54:10:54:10 | i | main.rs:53:9:53:17 | source(...) | main.rs:54:10:54:10 | i | $@ | main.rs:53:9:53:17 | source(...) | source(...) |
 | main.rs:95:10:95:12 | a.0 | main.rs:94:14:94:22 | source(...) | main.rs:95:10:95:12 | a.0 | $@ | main.rs:94:14:94:22 | source(...) | source(...) |
 | main.rs:110:10:110:12 | a.1 | main.rs:108:21:108:30 | source(...) | main.rs:110:10:110:12 | a.1 | $@ | main.rs:108:21:108:30 | source(...) | source(...) |
+| main.rs:113:10:113:12 | a.0 | main.rs:111:11:111:20 | source(...) | main.rs:113:10:113:12 | a.0 | $@ | main.rs:111:11:111:20 | source(...) | source(...) |
 | main.rs:121:10:121:14 | ... .1 | main.rs:118:17:118:26 | source(...) | main.rs:121:10:121:14 | ... .1 | $@ | main.rs:118:17:118:26 | source(...) | source(...) |
 | main.rs:158:10:158:10 | a | main.rs:154:12:154:21 | source(...) | main.rs:158:10:158:10 | a | $@ | main.rs:154:12:154:21 | source(...) | source(...) |
 | main.rs:217:25:217:25 | n | main.rs:214:19:214:28 | source(...) | main.rs:217:25:217:25 | n | $@ | main.rs:214:19:214:28 | source(...) | source(...) |

rust/ql/test/library-tests/dataflow/local/main.rs

@@ -110,7 +110,7 @@ fn tuple_mutation() {
     sink(a.1); // $ hasValueFlow=38
     a.0 = source(70);
     a.1 = 2;
-    sink(a.0); // $ MISSING: hasValueFlow=70
+    sink(a.0); // $ hasValueFlow=70
     sink(a.1);
 }