Rust: Account for captured variables

github · Oct 8, 2024 · 381caf9 · 381caf9
1 parent 63ec566
commit 381caf9
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 32 deletions.
diff --git a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll
@@ -119,6 +119,9 @@ module Impl {
         result = let.getInitializer()
       )
     }
+
+    /** Holds if this variable is captured. */
+    predicate isCaptured() { this.getAnAccess().isCapture() }
   }
 
   /** A path expression that may access a local variable. */
@@ -246,12 +249,9 @@ module Impl {
     nestLevel = 0
     or
     exists(VariableScope inner |
-      variableAccessCandInScope(cand, inner, name, nestLevel - 1, _, _, _, _) and
-      scope = getEnclosingScope(inner) and
-      // Use the location of the inner scope as the location of the access, instead of the
-      // actual access location. This allows us to collapse multiple accesses in inner
-      // scopes to a single entity
-      scope.getLocation().hasLocationInfo(_, startline, startcolumn, endline, endcolumn)
+      variableAccessCandInScope(cand, inner, name, nestLevel - 1, startline, startcolumn, endline,
+        endcolumn) and
+      scope = getEnclosingScope(inner)
     )
   }
 
@@ -379,16 +379,21 @@ module Impl {
     )
   }
 
+  private import codeql.rust.controlflow.internal.Scope
+
   /** A variable access. */
   class VariableAccess extends PathExprImpl::PathExpr instanceof VariableAccessCand {
     private string name;
     private Variable v;
 
-    VariableAccess() { variableAccess(_, name, v, this) }
+    VariableAccess() { variableAccess(name, v, this) }
 
     /** Gets the variable being accessed. */
     Variable getVariable() { result = v }
 
+    /** Holds if this access is a capture. */
+    predicate isCapture() { scopeOfAst(this) != scopeOfAst(v.getPat()) }
+
     override string toString() { result = name }
 
     override string getAPrimaryQlClass() { result = "VariableAccess" }
@@ -426,10 +431,10 @@ module Impl {
       MkVariable(AstNode definingNode, string name) { variableDecl(definingNode, _, name) }
 
     cached
-    predicate variableAccess(VariableScope scope, string name, Variable v, VariableAccessCand cand) {
+    predicate variableAccess(string name, Variable v, VariableAccessCand cand) {
       v =
         min(Variable v0, int nestLevel |
-          variableReachesCand(scope, name, v0, cand, nestLevel)
+          variableReachesCand(_, name, v0, cand, nestLevel)
         |
           v0 order by nestLevel
         )

diff --git a/rust/ql/test/library-tests/controlflow/Cfg.expected b/rust/ql/test/library-tests/controlflow/Cfg.expected
@@ -20,42 +20,42 @@ edges
 | test.rs:10:9:22:9 | ExprStmt | test.rs:11:13:11:24 | ExprStmt |  |
 | test.rs:10:9:22:9 | LoopExpr | test.rs:23:9:23:20 | ExprStmt |  |
 | test.rs:10:14:22:9 | BlockExpr | test.rs:11:13:11:24 | ExprStmt |  |
-| test.rs:11:13:11:13 | PathExpr | test.rs:11:17:11:20 | PathExpr |  |
+| test.rs:11:13:11:13 | i | test.rs:11:17:11:20 | PathExpr |  |
 | test.rs:11:13:11:23 | ... = ... | test.rs:12:13:14:13 | ExprStmt |  |
-| test.rs:11:13:11:24 | ExprStmt | test.rs:11:13:11:13 | PathExpr |  |
-| test.rs:11:17:11:20 | PathExpr | test.rs:11:22:11:22 | PathExpr |  |
+| test.rs:11:13:11:24 | ExprStmt | test.rs:11:13:11:13 | i |  |
+| test.rs:11:17:11:20 | PathExpr | test.rs:11:22:11:22 | i |  |
 | test.rs:11:17:11:23 | CallExpr | test.rs:11:13:11:23 | ... = ... |  |
-| test.rs:11:22:11:22 | PathExpr | test.rs:11:17:11:23 | CallExpr |  |
-| test.rs:12:13:14:13 | ExprStmt | test.rs:12:16:12:16 | PathExpr |  |
+| test.rs:11:22:11:22 | i | test.rs:11:17:11:23 | CallExpr |  |
+| test.rs:12:13:14:13 | ExprStmt | test.rs:12:16:12:16 | i |  |
 | test.rs:12:13:14:13 | IfExpr | test.rs:15:13:17:13 | ExprStmt |  |
-| test.rs:12:16:12:16 | PathExpr | test.rs:12:20:12:24 | 10000 |  |
+| test.rs:12:16:12:16 | i | test.rs:12:20:12:24 | 10000 |  |
 | test.rs:12:16:12:24 | ... > ... | test.rs:12:13:14:13 | IfExpr | false |
 | test.rs:12:16:12:24 | ... > ... | test.rs:13:17:13:29 | ExprStmt | true |
 | test.rs:12:20:12:24 | 10000 | test.rs:12:16:12:24 | ... > ... |  |
 | test.rs:13:17:13:28 | ReturnExpr | test.rs:8:5:24:5 | exit test_break_and_continue (normal) | return |
 | test.rs:13:17:13:29 | ExprStmt | test.rs:13:24:13:28 | false |  |
 | test.rs:13:24:13:28 | false | test.rs:13:17:13:28 | ReturnExpr |  |
-| test.rs:15:13:17:13 | ExprStmt | test.rs:15:16:15:16 | PathExpr |  |
+| test.rs:15:13:17:13 | ExprStmt | test.rs:15:16:15:16 | i |  |
 | test.rs:15:13:17:13 | IfExpr | test.rs:18:13:20:13 | ExprStmt |  |
-| test.rs:15:16:15:16 | PathExpr | test.rs:15:21:15:21 | 1 |  |
+| test.rs:15:16:15:16 | i | test.rs:15:21:15:21 | 1 |  |
 | test.rs:15:16:15:21 | ... == ... | test.rs:15:13:17:13 | IfExpr | false |
 | test.rs:15:16:15:21 | ... == ... | test.rs:16:17:16:22 | ExprStmt | true |
 | test.rs:15:21:15:21 | 1 | test.rs:15:16:15:21 | ... == ... |  |
 | test.rs:16:17:16:21 | BreakExpr | test.rs:10:9:22:9 | LoopExpr | break |
 | test.rs:16:17:16:22 | ExprStmt | test.rs:16:17:16:21 | BreakExpr |  |
-| test.rs:18:13:20:13 | ExprStmt | test.rs:18:16:18:16 | PathExpr |  |
-| test.rs:18:13:20:13 | IfExpr | test.rs:21:13:21:13 | PathExpr |  |
-| test.rs:18:16:18:16 | PathExpr | test.rs:18:20:18:20 | 2 |  |
+| test.rs:18:13:20:13 | ExprStmt | test.rs:18:16:18:16 | i |  |
+| test.rs:18:13:20:13 | IfExpr | test.rs:21:13:21:13 | i |  |
+| test.rs:18:16:18:16 | i | test.rs:18:20:18:20 | 2 |  |
 | test.rs:18:16:18:20 | ... % ... | test.rs:18:25:18:25 | 0 |  |
 | test.rs:18:16:18:25 | ... != ... | test.rs:18:13:20:13 | IfExpr | false |
 | test.rs:18:16:18:25 | ... != ... | test.rs:19:17:19:25 | ExprStmt | true |
 | test.rs:18:20:18:20 | 2 | test.rs:18:16:18:20 | ... % ... |  |
 | test.rs:18:25:18:25 | 0 | test.rs:18:16:18:25 | ... != ... |  |
 | test.rs:19:17:19:24 | ContinueExpr | test.rs:11:13:11:24 | ExprStmt | continue |
 | test.rs:19:17:19:25 | ExprStmt | test.rs:19:17:19:24 | ContinueExpr |  |
-| test.rs:21:13:21:13 | PathExpr | test.rs:21:17:21:17 | PathExpr |  |
+| test.rs:21:13:21:13 | i | test.rs:21:17:21:17 | i |  |
 | test.rs:21:13:21:21 | ... = ... | test.rs:10:14:22:9 | BlockExpr |  |
-| test.rs:21:17:21:17 | PathExpr | test.rs:21:21:21:21 | 2 |  |
+| test.rs:21:17:21:17 | i | test.rs:21:21:21:21 | 2 |  |
 | test.rs:21:17:21:21 | ... / ... | test.rs:21:13:21:21 | ... = ... |  |
 | test.rs:21:21:21:21 | 2 | test.rs:21:17:21:21 | ... / ... |  |
 | test.rs:23:9:23:19 | ReturnExpr | test.rs:8:5:24:5 | exit test_break_and_continue (normal) | return |
@@ -134,9 +134,9 @@ edges
 | test.rs:72:21:72:21 | 0 | test.rs:72:17:72:21 | ... > ... |  |
 | test.rs:73:17:73:21 | BreakExpr | test.rs:70:9:76:9 | WhileExpr | break |
 | test.rs:73:17:73:22 | ExprStmt | test.rs:73:17:73:21 | BreakExpr |  |
-| test.rs:75:13:75:13 | PathExpr | test.rs:75:17:75:21 | false |  |
+| test.rs:75:13:75:13 | b | test.rs:75:17:75:21 | false |  |
 | test.rs:75:13:75:21 | ... = ... | test.rs:70:17:76:9 | BlockExpr |  |
-| test.rs:75:13:75:22 | ExprStmt | test.rs:75:13:75:13 | PathExpr |  |
+| test.rs:75:13:75:22 | ExprStmt | test.rs:75:13:75:13 | b |  |
 | test.rs:75:17:75:21 | false | test.rs:75:13:75:21 | ... = ... |  |
 | test.rs:79:5:86:5 | enter test_while_let | test.rs:80:9:80:29 | LetStmt |  |
 | test.rs:79:5:86:5 | exit test_while_let (normal) | test.rs:79:5:86:5 | exit test_while_let |  |
@@ -230,13 +230,13 @@ edges
 | test.rs:122:28:124:9 | BlockExpr | test.rs:122:9:124:9 | IfExpr |  |
 | test.rs:123:13:123:13 | n | test.rs:122:28:124:9 | BlockExpr |  |
 | test.rs:125:9:125:9 | 0 | test.rs:121:43:126:5 | BlockExpr |  |
-| test.rs:128:5:134:5 | enter test_nested_if | test.rs:129:16:129:16 | PathExpr |  |
+| test.rs:128:5:134:5 | enter test_nested_if | test.rs:129:16:129:16 | a |  |
 | test.rs:128:5:134:5 | exit test_nested_if (normal) | test.rs:128:5:134:5 | exit test_nested_if |  |
 | test.rs:128:38:134:5 | BlockExpr | test.rs:128:5:134:5 | exit test_nested_if (normal) |  |
 | test.rs:129:9:133:9 | IfExpr | test.rs:128:38:134:5 | BlockExpr |  |
 | test.rs:129:13:129:48 | [boolean(false)] IfExpr | test.rs:132:13:132:13 | 0 | false |
 | test.rs:129:13:129:48 | [boolean(true)] IfExpr | test.rs:130:13:130:13 | 1 | true |
-| test.rs:129:16:129:16 | PathExpr | test.rs:129:20:129:20 | 0 |  |
+| test.rs:129:16:129:16 | a | test.rs:129:20:129:20 | 0 |  |
 | test.rs:129:16:129:20 | ... < ... | test.rs:129:24:129:24 | a | true |
 | test.rs:129:16:129:20 | ... < ... | test.rs:129:41:129:41 | a | false |
 | test.rs:129:20:129:20 | 0 | test.rs:129:16:129:20 | ... < ... |  |

diff --git a/rust/ql/test/library-tests/variables/Cfg.expected b/rust/ql/test/library-tests/variables/Cfg.expected
@@ -633,13 +633,13 @@ edges
 | variables.rs:367:19:370:5 | enter ClosureExpr | variables.rs:368:9:368:21 | ExprStmt |  |
 | variables.rs:367:19:370:5 | exit ClosureExpr (normal) | variables.rs:367:19:370:5 | exit ClosureExpr |  |
 | variables.rs:367:22:370:5 | BlockExpr | variables.rs:367:19:370:5 | exit ClosureExpr (normal) |  |
-| variables.rs:368:9:368:17 | PathExpr | variables.rs:368:19:368:19 | PathExpr |  |
+| variables.rs:368:9:368:17 | PathExpr | variables.rs:368:19:368:19 | x |  |
 | variables.rs:368:9:368:20 | CallExpr | variables.rs:369:9:369:15 | ExprStmt |  |
 | variables.rs:368:9:368:21 | ExprStmt | variables.rs:368:9:368:17 | PathExpr |  |
-| variables.rs:368:19:368:19 | PathExpr | variables.rs:368:9:368:20 | CallExpr |  |
-| variables.rs:369:9:369:9 | PathExpr | variables.rs:369:14:369:14 | 1 |  |
+| variables.rs:368:19:368:19 | x | variables.rs:368:9:368:20 | CallExpr |  |
+| variables.rs:369:9:369:9 | x | variables.rs:369:14:369:14 | 1 |  |
 | variables.rs:369:9:369:14 | ... += ... | variables.rs:367:22:370:5 | BlockExpr |  |
-| variables.rs:369:9:369:15 | ExprStmt | variables.rs:369:9:369:9 | PathExpr |  |
+| variables.rs:369:9:369:15 | ExprStmt | variables.rs:369:9:369:9 | x |  |
 | variables.rs:369:14:369:14 | 1 | variables.rs:369:9:369:14 | ... += ... |  |
 | variables.rs:371:5:371:7 | cap | variables.rs:371:5:371:9 | CallExpr |  |
 | variables.rs:371:5:371:9 | CallExpr | variables.rs:372:5:372:17 | ExprStmt |  |

diff --git a/rust/ql/test/library-tests/variables/variables.expected b/rust/ql/test/library-tests/variables/variables.expected
@@ -1,6 +1,4 @@
 testFailures
-| variables.rs:368:23:368:40 | Comment | Missing result:read_access=x |
-| variables.rs:369:17:369:29 | Comment | Missing result:access=x |
 failures
 variable
 | variables.rs:3:14:3:14 | s |
@@ -176,6 +174,8 @@ variableAccess
 | variables.rs:360:14:360:14 | x | variables.rs:358:13:358:13 | x |
 | variables.rs:361:6:361:6 | y | variables.rs:359:9:359:9 | y |
 | variables.rs:362:15:362:15 | x | variables.rs:358:13:358:13 | x |
+| variables.rs:368:19:368:19 | x | variables.rs:366:13:366:13 | x |
+| variables.rs:369:9:369:9 | x | variables.rs:366:13:366:13 | x |
 | variables.rs:371:5:371:7 | cap | variables.rs:367:13:367:15 | cap |
 | variables.rs:372:15:372:15 | x | variables.rs:366:13:366:13 | x |
 variableWriteAccess
@@ -272,6 +272,7 @@ variableReadAccess
 | variables.rs:354:15:354:15 | x | variables.rs:352:13:352:13 | x |
 | variables.rs:361:6:361:6 | y | variables.rs:359:9:359:9 | y |
 | variables.rs:362:15:362:15 | x | variables.rs:358:13:358:13 | x |
+| variables.rs:368:19:368:19 | x | variables.rs:366:13:366:13 | x |
 | variables.rs:371:5:371:7 | cap | variables.rs:367:13:367:15 | cap |
 | variables.rs:372:15:372:15 | x | variables.rs:366:13:366:13 | x |
 variableInitializer
@@ -307,3 +308,8 @@ variableInitializer
 | variables.rs:359:9:359:9 | y | variables.rs:360:9:360:14 | RefExpr |
 | variables.rs:366:13:366:13 | x | variables.rs:366:17:366:18 | 10 |
 | variables.rs:367:13:367:15 | cap | variables.rs:367:19:370:5 | ClosureExpr |
+capturedVariable
+| variables.rs:366:13:366:13 | x |
+capturedAccess
+| variables.rs:368:19:368:19 | x |
+| variables.rs:369:9:369:9 | x |
diff --git a/rust/ql/test/library-tests/variables/variables.ql b/rust/ql/test/library-tests/variables/variables.ql
@@ -11,6 +11,10 @@ query predicate variableReadAccess(VariableReadAccess va, Variable v) { v = va.g
 
 query predicate variableInitializer(Variable v, Expr e) { e = v.getInitializer() }
 
+query predicate capturedVariable(Variable v) { v.isCaptured() }
+
+query predicate capturedAccess(VariableAccess va) { va.isCapture() }
+
 module VariableAccessTest implements TestSig {
   string getARelevantTag() { result = ["", "write_", "read_"] + "access" }