From 5afd2d5bf0d63286e46126a31dbfbd02c45e5551 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Tue, 8 Oct 2024 16:19:03 +0200 Subject: [PATCH] Rust: Account for captured variables --- .../rust/elements/internal/VariableImpl.qll | 43 ++++++++++++++----- .../library-tests/controlflow/Cfg.expected | 34 +++++++-------- .../test/library-tests/variables/Cfg.expected | 8 ++-- .../variables/variables.expected | 10 ++++- .../test/library-tests/variables/variables.ql | 4 ++ 5 files changed, 65 insertions(+), 34 deletions(-) diff --git a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll index b9e9bb7cb48a..99cdeba4e1b2 100644 --- a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll +++ b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll @@ -1,6 +1,7 @@ private import rust private import codeql.rust.elements.internal.generated.ParentChild private import codeql.rust.elements.internal.PathExprImpl::Impl as PathExprImpl +private import codeql.util.DenseRank module Impl { /** @@ -119,6 +120,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. */ @@ -251,7 +255,7 @@ module Impl { // 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) + inner.getLocation().hasLocationInfo(_, startline, startcolumn, endline, endcolumn) ) } @@ -334,18 +338,30 @@ module Impl { } } + private module DenseRankInput implements DenseRankInputSig3 { + class C1 = VariableScope; + + class C2 = string; + + class Ranked = VariableOrAccessCand; + + int getRank(VariableScope scope, string name, VariableOrAccessCand v) { + v = + rank[result](VariableOrAccessCand v0, int startline, int startcolumn, int endline, + int endcolumn | + v0.rankBy(name, scope, startline, startcolumn, endline, endcolumn) + | + v0 order by startline, startcolumn, endline, endcolumn + ) + } + } + /** * Gets the rank of `v` amongst all other declarations or access candidates * to a variable named `name` in the variable scope `scope`. */ private int rankVariableOrAccess(VariableScope scope, string name, VariableOrAccessCand v) { - v = - rank[result + 1](VariableOrAccessCand v0, int startline, int startcolumn, int endline, - int endcolumn | - v0.rankBy(name, scope, startline, startcolumn, endline, endcolumn) - | - v0 order by startline, startcolumn, endline, endcolumn - ) + result = DenseRank3::denseRank(scope, name, v) - 1 } /** @@ -379,16 +395,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 +447,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 index 61b71f15118d..036dca8af5a3 100644 --- a/rust/ql/test/library-tests/controlflow/Cfg.expected +++ b/rust/ql/test/library-tests/controlflow/Cfg.expected @@ -20,32 +20,32 @@ 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 | @@ -53,9 +53,9 @@ edges | 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 index 628a738db7e7..89e7aaec297e 100644 --- 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 index 8258f0ce6b97..09430b5497c0 100644 --- 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 index 9657c361fd59..23eab9774457 100644 --- 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" }