Rust: &x is neither a read nor a write

Author: hvitved

rust/ql/lib/codeql/rust/elements/AssignmentOperation.qll

@@ -0,0 +1,49 @@
+/** Provides classes for assignment operations. */
+
+private import rust
+private import codeql.rust.elements.internal.BinaryExprImpl
+
+/** An assignment operation. */
+abstract private class AssignmentOperationImpl extends Impl::BinaryExpr { }
+
+final class AssignmentOperation = AssignmentOperationImpl;
+
+/**
+ * An assignment expression, for example
+ *
+ * ```rust
+ * x = y;
+ * ```
+ */
+final class AssignmentExpr extends AssignmentOperationImpl {
+  AssignmentExpr() { this.getOperatorName() = "=" }
+
+  override string getAPrimaryQlClass() { result = "AssignmentExpr" }
+}
+
+/**
+ * A compound assignment expression, for example
+ *
+ * ```rust
+ * x += y;
+ * ```
+ *
+ * Note that compound assignment expressions are syntatic sugar for
+ * trait invocations, i.e., the above actually means
+ *
+ * ```rust
+ * (&mut x).add_assign(y);
+ * ```
+ */
+final class CompoundAssignmentExpr extends AssignmentOperationImpl {
+  private string operator;
+
+  CompoundAssignmentExpr() { this.getOperatorName().regexpCapture("(.)=", 1) = operator }
+
+  /**
+   * Gets the operator of this compound assignment expression.
+   */
+  string getOperator() { result = operator }
+
+  override string getAPrimaryQlClass() { result = "CompoundAssignmentExpr" }
+}

rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll

@@ -395,34 +395,27 @@ module Impl {
   }
 
   /** Holds if `e` occurs in the LHS of an assignment or compound assignment. */
-  private predicate assignLhs(Expr e, boolean compound) {
-    exists(BinaryExpr be, string op |
-      op = be.getOperatorName().regexpCapture("(.*)=", 1) and
-      e = be.getLhs()
-    |
-      op = "" and compound = false
-      or
-      op != "" and compound = true
-    )
+  private predicate assignmentExprDescendant(Expr e) {
+    e = any(AssignmentExpr ae).getLhs()
     or
     exists(Expr mid |
-      assignLhs(mid, compound) and
-      getImmediateParent(e) = mid
+      assignmentExprDescendant(mid) and
+      getImmediateParent(e) = mid and
+      not mid.(PrefixExpr).getOperatorName() = "*"
     )
   }
 
   /** A variable write. */
   class VariableWriteAccess extends VariableAccess {
-    VariableWriteAccess() { assignLhs(this, _) }
+    VariableWriteAccess() { assignmentExprDescendant(this) }
   }
 
   /** A variable read. */
   class VariableReadAccess extends VariableAccess {
     VariableReadAccess() {
-      not this instanceof VariableWriteAccess
-      or
-      // consider LHS in compound assignments both reads and writes
-      assignLhs(this, true)
+      not this instanceof VariableWriteAccess and
+      not this = any(RefExpr re).getExpr() and
+      not this = any(CompoundAssignmentExpr cae).getLhs()
     }
   }
 

rust/ql/lib/rust.qll

@@ -3,5 +3,6 @@
 import codeql.rust.elements
 import codeql.Locations
 import codeql.files.FileSystem
+import codeql.rust.elements.AssignmentOperation
 import codeql.rust.elements.LogicalOperation
 import codeql.rust.elements.Variable

rust/ql/test/library-tests/variables/variables.expected

@@ -1,17 +1,4 @@
 testFailures
-| variables.rs:331:5:331:5 | a | Unexpected result: read_access=a |
-| variables.rs:331:5:331:5 | a | Unexpected result: write_access=a |
-| variables.rs:331:13:331:25 | Comment | Missing result:access=a |
-| variables.rs:333:11:333:11 | a | Unexpected result: read_access=a |
-| variables.rs:333:30:333:42 | Comment | Missing result:access=a |
-| variables.rs:340:14:340:14 | i | Unexpected result: read_access=i |
-| variables.rs:340:17:340:29 | Comment | Missing result:access=i |
-| variables.rs:341:6:341:10 | ref_i | Unexpected result: write_access=ref_i |
-| variables.rs:341:17:341:38 | Comment | Missing result:read_access=ref_i |
-| variables.rs:346:6:346:6 | x | Unexpected result: write_access=x |
-| variables.rs:346:10:346:27 | Comment | Missing result:read_access=x |
-| variables.rs:353:23:353:23 | x | Unexpected result: read_access=x |
-| variables.rs:353:27:353:39 | Comment | Missing result:access=x |
 failures
 variable
 | variables.rs:3:14:3:14 | s |
@@ -185,9 +172,6 @@ variableWriteAccess
 | variables.rs:277:9:277:10 | c2 | variables.rs:270:13:270:14 | c2 |
 | variables.rs:278:9:278:10 | b4 | variables.rs:269:13:269:14 | b4 |
 | variables.rs:279:9:279:11 | a10 | variables.rs:268:13:268:15 | a10 |
-| variables.rs:331:5:331:5 | a | variables.rs:330:13:330:13 | a |
-| variables.rs:341:6:341:10 | ref_i | variables.rs:339:9:339:13 | ref_i |
-| variables.rs:346:6:346:6 | x | variables.rs:345:17:345:17 | x |
 variableReadAccess
 | variables.rs:13:15:13:16 | x1 | variables.rs:12:9:12:10 | x1 |
 | variables.rs:18:15:18:16 | x2 | variables.rs:17:13:17:14 | x2 |
@@ -267,15 +251,13 @@ variableReadAccess
 | variables.rs:317:15:317:16 | n2 | variables.rs:315:9:315:10 | n2 |
 | variables.rs:324:12:324:12 | v | variables.rs:321:9:321:9 | v |
 | variables.rs:325:19:325:22 | text | variables.rs:323:9:323:12 | text |
-| variables.rs:331:5:331:5 | a | variables.rs:330:13:330:13 | a |
 | variables.rs:332:15:332:15 | a | variables.rs:330:13:330:13 | a |
-| variables.rs:333:11:333:11 | a | variables.rs:330:13:330:13 | a |
 | variables.rs:334:15:334:15 | a | variables.rs:330:13:330:13 | a |
-| variables.rs:340:14:340:14 | i | variables.rs:338:13:338:13 | i |
+| variables.rs:341:6:341:10 | ref_i | variables.rs:339:9:339:13 | ref_i |
 | variables.rs:342:15:342:15 | i | variables.rs:338:13:338:13 | i |
+| variables.rs:346:6:346:6 | x | variables.rs:345:17:345:17 | x |
 | variables.rs:347:10:347:10 | x | variables.rs:345:17:345:17 | x |
 | variables.rs:348:10:348:10 | x | variables.rs:345:17:345:17 | x |
-| variables.rs:353:23:353:23 | x | variables.rs:352:13:352:13 | x |
 | variables.rs:354:15:354:15 | x | variables.rs:352:13:352:13 | x |
 variableInitializer
 | variables.rs:12:9:12:10 | x1 | variables.rs:12:14:12:16 | "a" |