wip

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

@@ -6,13 +6,41 @@ private import Cfg
 private import codeql.rust.controlflow.internal.ControlFlowGraphImpl as ControlFlowGraphImpl
 private import codeql.ssa.Ssa as SsaImplCommon
 
+/** Holds if `v` is introduced like `let v : i64;`. */
+private predicate isUnitializedLet(IdentPat pat, Variable v) {
+  pat = v.getPat() and
+  exists(LetStmt let |
+    let = v.getLetStmt() and
+    not let.hasInitializer()
+  )
+}
+
+/** Holds if `write` writes to variable `v`. */
+predicate variableWrite(AstNode write, Variable v) {
+  exists(IdentPat pat |
+    pat = write and
+    pat = v.getPat() and
+    not isUnitializedLet(pat, v)
+  )
+  or
+  exists(VariableAccess access |
+    access = write and
+    access.getVariable() = v
+  |
+    access instanceof VariableWriteAccess
+    or
+    access = any(CompoundAssignmentExpr cae).getLhs()
+  )
+}
+
 private Expr withParens(Expr e) {
   result = e
   or
   result.(ParenExpr).getExpr() = withParens(e)
 }
 
-private predicate isUncertainWrite(VariableAccess va) {
+private predicate isRefTarget(VariableAccess va, Variable v) {
+  va = v.getAnAccess() and
   exists(RefExpr re, Expr arg |
     va = re.getExpr() and // todo: restrict to `mut`
     arg = withParens(re)
@@ -24,8 +52,6 @@ private predicate isUncertainWrite(VariableAccess va) {
       arg = mce.getReceiver()
     )
   )
-  or
-  va = any(CompoundAssignmentExpr cae).getLhs()
 }
 
 module SsaInput implements SsaImplCommon::InputSig<Location> {
@@ -45,9 +71,9 @@ module SsaInput implements SsaImplCommon::InputSig<Location> {
       forall(VariableAccess va | va = this.getAnAccess() |
         va instanceof VariableReadAccess
         or
-        va instanceof VariableWriteAccess
+        variableWrite(va, this)
         or
-        isUncertainWrite(va)
+        isRefTarget(va, this)
       )
     }
   }
@@ -61,16 +87,30 @@ module SsaInput implements SsaImplCommon::InputSig<Location> {
     certain = true
     or
     exists(VariableAccess va |
-      isUncertainWrite(va) and
+      isRefTarget(va, v) and
       va = bb.getNode(i).getAstNode() and
-      v = va.getVariable() and
       certain = false
     )
   }
 
   predicate variableRead(BasicBlock bb, int i, SourceVariable v, boolean certain) {
-    bb.getNode(i).getAstNode() = v.getAnAccess().(VariableReadAccess) and
+    exists(VariableAccess va |
+      bb.getNode(i).getAstNode() = va and
+      va = v.getAnAccess()
+    |
+      va instanceof VariableReadAccess
+      or
+      // although compound assignments, like `x += y`, may in fact not update `x`,
+      // it makes sense to treat them as such
+      va = any(CompoundAssignmentExpr cae).getLhs()
+    ) and
     certain = true
+    or
+    exists(VariableAccess va |
+      isRefTarget(va, v) and
+      va = bb.getNode(i).getAstNode() and
+      certain = false
+    )
   }
 }
 
@@ -185,11 +225,7 @@ private module Cached {
   cached
   predicate variableWriteActual(BasicBlock bb, int i, Variable v, CfgNode write) {
     bb.getNode(i) = write and
-    (
-      write.getAstNode() = v.getPat()
-      or
-      write.getAstNode().(VariableWriteAccess).getVariable() = v
-    )
+    variableWrite(write.getAstNode(), v)
   }
 
   cached

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

@@ -113,16 +113,17 @@ module Impl {
      */
     IdentPat getPat() { variableDecl(definingNode, result, name) }
 
+    /** Gets the `let` statement that introduces this variable, if any. */
+    LetStmt getLetStmt() { this.getPat() = result.getPat() }
+
     /** Gets the initial value of this variable, if any. */
-    Expr getInitializer() {
-      exists(LetStmt let |
-        this.getPat() = let.getPat() and
-        result = let.getInitializer()
-      )
-    }
+    Expr getInitializer() { result = this.getLetStmt().getInitializer() }
 
     /** Holds if this variable is captured. */
     predicate isCaptured() { this.getAnAccess().isCapture() }
+
+    /** Gets the parameter that introduces this variable, if any. */
+    Param getParameter() { parameterDeclInScope(result, this, _) }
   }
 
   /** A path expression that may access a local variable. */
@@ -175,6 +176,27 @@ module Impl {
     )
   }
 
+  /**
+   * Holds if parameter `p` introduces the variable `v` inside variable scope
+   * `scope`.
+   */
+  private predicate parameterDeclInScope(Param p, Variable v, VariableScope scope) {
+    exists(Pat pat |
+      pat = getAVariablePatAncestor(v) and
+      p.getPat() = pat
+    |
+      exists(Function f |
+        f.getParamList().getAParam() = p and
+        scope = f.getBody()
+      )
+      or
+      exists(ClosureExpr ce |
+        ce.getParamList().getAParam() = p and
+        scope = ce.getBody()
+      )
+    )
+  }
+
   /**
    * Holds if `v` is named `name` and is declared inside variable scope
    * `scope`, and `v` is bound starting from `(line, column)`.
@@ -183,51 +205,44 @@ module Impl {
     Variable v, VariableScope scope, string name, int line, int column
   ) {
     name = v.getName() and
-    exists(Pat pat | pat = getAVariablePatAncestor(v) |
-      scope =
-        any(MatchArmScope arm |
-          arm.getPat() = pat and
-          arm.getLocation().hasLocationInfo(_, line, column, _, _)
-        )
-      or
-      exists(Function f |
-        f.getParamList().getAParam().getPat() = pat and
-        scope = f.getBody() and
-        scope.getLocation().hasLocationInfo(_, line, column, _, _)
-      )
-      or
-      exists(LetStmt let |
-        let.getPat() = pat and
-        scope = getEnclosingScope(let) and
-        // for `let` statements, variables are bound _after_ the statement, i.e.
-        // not in the RHS
-        let.getLocation().hasLocationInfo(_, _, _, line, column)
-      )
-      or
-      exists(IfExpr ie, LetExpr let |
-        let.getPat() = pat and
-        ie.getCondition() = let and
-        scope = ie.getThen() and
-        scope.getLocation().hasLocationInfo(_, line, column, _, _)
-      )
-      or
-      exists(ForExpr fe |
-        fe.getPat() = pat and
-        scope = fe.getLoopBody() and
-        scope.getLocation().hasLocationInfo(_, line, column, _, _)
-      )
-      or
-      exists(ClosureExpr ce |
-        ce.getParamList().getAParam().getPat() = pat and
-        scope = ce.getBody() and
-        scope.getLocation().hasLocationInfo(_, line, column, _, _)
-      )
+    (
+      parameterDeclInScope(_, v, scope) and
+      scope.getLocation().hasLocationInfo(_, line, column, _, _)
       or
-      exists(WhileExpr we, LetExpr let |
-        let.getPat() = pat and
-        we.getCondition() = let and
-        scope = we.getLoopBody() and
-        scope.getLocation().hasLocationInfo(_, line, column, _, _)
+      exists(Pat pat | pat = getAVariablePatAncestor(v) |
+        scope =
+          any(MatchArmScope arm |
+            arm.getPat() = pat and
+            arm.getLocation().hasLocationInfo(_, line, column, _, _)
+          )
+        or
+        exists(LetStmt let |
+          let.getPat() = pat and
+          scope = getEnclosingScope(let) and
+          // for `let` statements, variables are bound _after_ the statement, i.e.
+          // not in the RHS
+          let.getLocation().hasLocationInfo(_, _, _, line, column)
+        )
+        or
+        exists(IfExpr ie, LetExpr let |
+          let.getPat() = pat and
+          ie.getCondition() = let and
+          scope = ie.getThen() and
+          scope.getLocation().hasLocationInfo(_, line, column, _, _)
+        )
+        or
+        exists(ForExpr fe |
+          fe.getPat() = pat and
+          scope = fe.getLoopBody() and
+          scope.getLocation().hasLocationInfo(_, line, column, _, _)
+        )
+        or
+        exists(WhileExpr we, LetExpr let |
+          let.getPat() = pat and
+          we.getCondition() = let and
+          scope = we.getLoopBody() and
+          scope.getLocation().hasLocationInfo(_, line, column, _, _)
+        )
       )
     )
   }
@@ -422,7 +437,8 @@ module Impl {
     exists(Expr mid |
       assignmentExprDescendant(mid) and
       getImmediateParent(e) = mid and
-      not mid.(PrefixExpr).getOperatorName() = "*"
+      not mid.(PrefixExpr).getOperatorName() = "*" and
+      not mid instanceof FieldExpr
     )
   }
 

rust/ql/src/queries/unusedentities/UnusedValue.ql

@@ -9,7 +9,17 @@
  */
 
 import rust
+import codeql.rust.controlflow.ControlFlowGraph
+import codeql.rust.dataflow.Ssa
+import codeql.rust.dataflow.internal.SsaImpl
+import UnusedVariable
 
-from Locatable e
-where none() // TODO: implement query
-select e, "Variable is assigned a value that is never used."
+from AstNode write, Ssa::Variable v
+where
+  variableWrite(write, v) and
+  // SSA definitions are only created for live writes
+  not write = any(Ssa::WriteDefinition def).getWriteAccess().getAstNode() and
+  // avoid overlap with the unused variable query
+  not isUnused(v) and
+  not v instanceof DiscardVariable
+select write, "Variable is assigned a value that is never used."

rust/ql/src/queries/unusedentities/UnusedVariable.ql

@@ -9,11 +9,8 @@
  */
 
 import rust
+import UnusedVariable
 
 from Variable v
-where
-  not exists(v.getAnAccess()) and
-  not exists(v.getInitializer()) and
-  not v.getName().charAt(0) = "_" and
-  exists(File f | f.getBaseName() = "main.rs" | v.getLocation().getFile() = f) // temporarily severely limit results
+where isUnused(v)
 select v, "Variable is not used."

rust/ql/src/queries/unusedentities/UnusedVariable.qll

@@ -0,0 +1,14 @@
+import rust
+
+/** A deliberately unused variable. */
+class DiscardVariable extends Variable {
+  DiscardVariable() { this.getName().charAt(0) = "_" }
+}
+
+/** Holds if variable `v` is unused. */
+predicate isUnused(Variable v) {
+  not exists(v.getAnAccess()) and
+  not exists(v.getInitializer()) and
+  not v instanceof DiscardVariable and
+  exists(File f | f.getBaseName() = "main.rs" | v.getLocation().getFile() = f) // temporarily severely limit results
+}