From cd10687dbda167822f8bfa1e90fbef9301aa81d9 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 20 May 2026 13:02:10 +0200 Subject: [PATCH 1/8] Rust: Move local name resolution logic into shared library --- .../rust/elements/internal/VariableImpl.qll | 633 ++++++++---------- .../test/library-tests/variables/Ssa.expected | 4 + .../variables/variables.expected | 8 +- 3 files changed, 274 insertions(+), 371 deletions(-) diff --git a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll index 37a2e4dacc07..977537a7c13e 100644 --- a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll +++ b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll @@ -16,24 +16,28 @@ module Impl { class BlockExprScope extends VariableScope, BlockExpr { } - class MatchArmExprScope extends VariableScope { - MatchArmExprScope() { this = any(MatchArm arm).getExpr() } + class MatchScope extends VariableScope, MatchArm { + // MatchArmExprScope() { this = any(MatchArm arm).getExpr() } } - class MatchArmGuardScope extends VariableScope { - MatchArmGuardScope() { this = any(MatchArm arm).getGuard() } + // class MatchArmExprScope extends VariableScope { + // MatchArmExprScope() { this = any(MatchArm arm).getExpr() } + // } + // class MatchArmGuardScope extends VariableScope { + // MatchArmGuardScope() { this = any(MatchArm arm).getGuard() } + // } + class CallableScope extends VariableScope, Callable { + // ClosureBodyScope() { this = any(ClosureExpr ce).getBody() } } - class ClosureBodyScope extends VariableScope { - ClosureBodyScope() { this = any(ClosureExpr ce).getBody() } - } + class LetScope extends VariableScope, Let { } /** * A scope for conditions, which may introduce variables using `let` expressions. * * Such variables are only available in the body guarded by the condition. */ - class ConditionScope extends VariableScope { + class ConditionScope extends AstNode { private AstNode parent; private AstNode body; @@ -64,6 +68,41 @@ module Impl { * Gets the body in which variables introduced in this scope are available. */ AstNode getBody() { result = body } + + AstNode getElse() { result = any(IfExpr ie | this = ie.getCondition()).getElse() } + } + + /** + * A scope for conditions, which may introduce variables using `let` expressions. + * + * Such variables are only available in the body guarded by the condition. + */ + class Conditional extends AstNode { + Conditional() { + this instanceof IfExpr + or + this instanceof WhileExpr + or + this = any(MatchArm ma | ma.hasGuard()) + } + + AstNode getCondition() { + result = this.(IfExpr).getCondition() + or + result = this.(WhileExpr).getCondition() + or + result = this.(MatchArm).getGuard() + } + + AstNode getThen() { + result = this.(IfExpr).getThen() + or + result = this.(WhileExpr).getLoopBody() + or + result = this.(MatchArm).getExpr() + } + + AstNode getElse() { result = this.(IfExpr).getElse() } } private Pat getAPatAncestor(Pat p) { @@ -225,55 +264,194 @@ module Impl { string getName() { result = name_ } } - pragma[nomagic] - private Element getImmediateChildAdj(Element e, int preOrd, int index) { - result = ParentChild::getImmediateChild(e, index) and - preOrd = 0 and - not exists(ConditionScope cs | - e = cs.getParent() and - result = cs.getBody() + class Let extends AstNode { + Let() { this instanceof LetStmt or this instanceof LetExpr } + + AstNode getLhs() { + result = this.(LetStmt).getPat() + or + result = this.(LetExpr).getPat() + } + + AstNode getRhs() { + result = this.(LetStmt).getInitializer() + or + result = this.(LetExpr).getScrutinee() + } + + AstNode getElse() { result = this.(LetStmt).getLetElse() } + } + + /** A case in a switch. */ + class Case extends MatchArm { + /** Gets the pattern being matched by this case at the specified (zero-based) `index`. */ + AstNode getPattern(int index) { + result = this.getPat() and + index = 0 + } + + /** Gets the guard expression of this case, if any. */ + AstNode getGuard() { result = super.getGuard() } + + /** + * Gets the body of this case, if any. + * + * A case can either have a body as a single child AST node given by this + * predicate, or it can have an implicit body given by the sequence of + * statements between this case and the next case. + */ + AstNode getBody() { result = this.getExpr() } + } + + private LogicalAndExpr getALetChainAncestor(LetExpr let) { + let = result.getAnOperand() + or + result.getAnOperand() = getALetChainAncestor(let) + } + + /** Gets the outermost enclosing `|` pattern parent of `p`, if any. */ + private LogicalAndExpr getOutermostLetChainAncestor(LetExpr let) { + result = getALetChainAncestor(let) and + not result.getParentNode() instanceof LogicalAndExpr + } + + private AstNode getLetChainRootDescendant(LogicalAndExpr lae, string path) { + lae = getOutermostLetChainAncestor(_) and + ( + result = lae.getLhs() and path = "0" + or + result = lae.getRhs() and path = "1" ) or - result = e.(ConditionScope).getBody() and - preOrd = 1 and - index = 0 + exists(LogicalAndExpr mid, string prefix | mid = getLetChainRootDescendant(lae, prefix) | + result = mid.getLhs() and path = prefix + ".0" + or + result = mid.getRhs() and path = prefix + ".1" + ) } - /** - * An adjusted version of `ParentChild::getImmediateChild`, which makes the following - * two adjustments: - * - * 1. For conditions like `if cond body`, instead of letting `body` be the second child - * of `if`, we make it the last child of `cond`. This ensures that variables - * introduced in the `cond` scope are available in `body`. - * - * 2. A similar adjustment is made for `while` loops: the body of the loop is made a - * child of the loop condition instead of the loop itself. - */ - pragma[nomagic] - private Element getImmediateChildAdj(Element e, int index) { + private AstNode getLetChainChildAt(LogicalAndExpr lae, string path) { + result = getLetChainRootDescendant(lae, path) and + not result instanceof LogicalAndExpr + } + + private AstNode getLetChainChild(LogicalAndExpr lae, int i) { result = - rank[index + 1](Element res, int preOrd, int i | - res = getImmediateChildAdj(e, preOrd, i) - | - res order by preOrd, i - ) + rank[i + 1](AstNode res, string path | res = getLetChainChildAt(lae, path) | res order by path) } - private Element getImmediateParentAdj(Element e) { e = getImmediateChildAdj(result, _) } + pragma[nomagic] + private AstNode getImmediateChildAdjRust(AstNode parent, int index) { + result = ParentChild::getImmediateChild(parent, index) and + not result = getLetChainRootDescendant(_, _) //and + or + result = getLetChainChild(parent, index) + or + exists(Format f | + f = result.(FormatTemplateVariableAccess).getArgument().getParent() and + parent = f.getParent() and + index = f.getIndex() + ) + } - private AstNode getAnAncestorInVariableScope(AstNode n) { + private import codeql.util.DenseRank + + private module ChildDenseRankInput implements DenseRankInputSig1 { + class C = AstNode; + + class Ranked = AstNode; + + /** + * An adjusted version of `ParentChild::getImmediateChild`, which makes the following + * two adjustments: + * + * 1. For conditions like `if cond body`, instead of letting `body` be the second child + * of `if`, we make it the last child of `cond`. This ensures that variables + * introduced in the `cond` scope are available in `body`. + * + * 2. A similar adjustment is made for `while` loops: the body of the loop is made a + * child of the loop condition instead of the loop itself. + */ + int getRank(C parent, Ranked child) { + // parent.getEnclosingCallable().(Function).getName().getText() = "match_pattern14" and + child = + rank[result](AstNode res, int preOrd, int i | + res = getImmediateChildAdjRust(parent, i) and + preOrd = 0 and + not exists(Conditional cond | res = [cond.getThen(), cond.getElse()]) + or + res = parent.(Conditional).getElse() and + preOrd = -1 and + i = 0 + or + exists(Conditional cond | + parent = cond.getCondition() and + res = cond.getThen() and + preOrd = 1 and + i = 0 + ) + | + res order by preOrd, i + ) + } + } + + predicate getRankedChild = DenseRank1::denseRank/2; + + private predicate isLetParent(AstNode parent, int i, Let let) { let = getRankedChild(parent, i) } + + private predicate shouldBeLetChild(AstNode parent, int i, Let let, AstNode child) { + // parent.getEnclosingCallable().(Function).getName().getText() = "let_pattern3" and + child = getRankedChild(parent, i) and + ( + isLetParent(parent, i - 1, let) + or + shouldBeLetChild(parent, i - 1, let, any(AstNode n | not n instanceof Let)) + ) + } + + AstNode getAChild(AstNode parent) { + // parent.getEnclosingCallable().(Function).getName().getText() = "let_pattern3" and + // result = ParentChild::getImmediateChild(parent, _) and ( - n instanceof Pat or - n instanceof VariableAccessCand or - n instanceof LetStmt or - n = any(LetExpr le).getScrutinee() or - n instanceof VariableScope - ) and - exists(AstNode n0 | - result = getImmediateParentAdj(n0) or - result = n0.(FormatTemplateVariableAccess).getArgument().getParent().getParent() - | + not shouldBeLetChild(_, _, _, result) and + not result = [any(Let let).getRhs(), any(Let let).getElse()] and + // not result = [any(Case c).getPattern(_), any(Case c).getGuard(), any(Case c).getBody()] and + // not result instanceof Let and + result = getRankedChild(parent, _) + or + shouldBeLetChild(_, _, parent, result) + or + exists(Let let | let = getAChild(parent) | result = [let.getRhs(), let.getElse()]) + // or + // exists(AstNode par, Let let, int k, AstNode child | shouldBeLetChild(par, k, let, child) | + // result = child and + // parent = let + // or + // ( + // shouldBeLetChild(_, _, parent, let) + // or + // not shouldBeLetChild(_, _, _, let) and + // parent = par + // ) and + // ( + // result = let.getRhs() + // or + // result = let + // ) + // ) + ) + } + + private AstNode getAnAncestorInVariableScope(AstNode n) { + // ( + // n instanceof Pat or + // n instanceof VariableAccessCand or + // n instanceof LetStmt or + // n = any(LetExpr le).getScrutinee() or + // n instanceof VariableScope + // ) and + exists(AstNode n0 | n0 = getAChild(result) | n0 = n or n0 = getAnAncestorInVariableScope(n) and @@ -303,99 +481,7 @@ module Impl { private predicate parameterDeclInScope(Variable v, VariableScope scope) { exists(Callable f | v.getParameter() = f.getParamList().getAParamBase() and - scope = f.getBody() - ) - } - - /** A subset of `Element`s for which we want to compute pre-order numbers. */ - private class RelevantElement extends Element { - RelevantElement() { - this instanceof VariableScope or - this instanceof VariableAccessCand or - this instanceof LetStmt or - this = any(LetExpr le).getScrutinee() or - getImmediateChildAdj(this, _) instanceof RelevantElement - } - - pragma[nomagic] - private RelevantElement getChild(int index) { result = getImmediateChildAdj(this, index) } - - pragma[nomagic] - private RelevantElement getImmediateChildAdjMin(int index) { - // A child may have multiple positions for different accessors, - // so always use the first - result = this.getChild(index) and - index = min(int i | result = this.getChild(i) | i) - } - - pragma[nomagic] - RelevantElement getImmediateChildAdj(int index) { - result = - rank[index + 1](Element res, int i | res = this.getImmediateChildAdjMin(i) | res order by i) - } - - pragma[nomagic] - RelevantElement getImmediateLastChild() { - exists(int last | - result = this.getImmediateChildAdj(last) and - not exists(this.getImmediateChildAdj(last + 1)) - ) - } - } - - /** - * Gets the pre-order numbering of `n`, where the immediately enclosing - * variable scope of `n` is `scope`. - */ - pragma[nomagic] - private int getPreOrderNumbering(VariableScope scope, RelevantElement n) { - n = scope and - result = 0 - or - exists(RelevantElement parent | - not parent instanceof VariableScope - or - parent = scope - | - // first child of a previously numbered node - result = getPreOrderNumbering(scope, parent) + 1 and - n = parent.getImmediateChildAdj(0) - or - // non-first child of a previously numbered node - exists(RelevantElement child, int i | - result = getLastPreOrderNumbering(scope, child) + 1 and - child = parent.getImmediateChildAdj(i) and - n = parent.getImmediateChildAdj(i + 1) - ) - ) - } - - /** - * Gets the pre-order numbering of the _last_ node nested under `n`, where the - * immediately enclosing variable scope of `n` (and the last node) is `scope`. - */ - pragma[nomagic] - private int getLastPreOrderNumbering(VariableScope scope, RelevantElement n) { - exists(RelevantElement leaf | - result = getPreOrderNumbering(scope, leaf) and - leaf != scope and - ( - not exists(leaf.getImmediateChildAdj(_)) - or - leaf instanceof VariableScope - ) - | - n = leaf - or - n.getImmediateLastChild() = leaf and - not n instanceof VariableScope - ) - or - exists(RelevantElement mid | - mid = n.getImmediateLastChild() and - result = getLastPreOrderNumbering(scope, mid) and - not mid instanceof VariableScope and - not n instanceof VariableScope + scope = f // f.getBody() ) } @@ -404,262 +490,73 @@ module Impl { * `scope`. The pre-order numbering of the binding site of `v`, amongst * all nodes nested under `scope`, is `ord`. */ - private predicate variableDeclInScope(Variable v, VariableScope scope, string name, int ord) { + private predicate variableDeclInScope(Variable v, VariableScope scope, string name) { name = v.getText() and ( - parameterDeclInScope(v, scope) and - ord = getPreOrderNumbering(scope, scope) + parameterDeclInScope(v, scope) //and or + // ord = getPreOrderNumbering(scope, scope) exists(Pat pat | pat = getAVariablePatAncestor(v) | exists(MatchArm arm | - pat = arm.getPat() and - ord = getPreOrderNumbering(scope, scope) + pat = arm.getPat() //and | - scope = arm.getGuard() - or - not arm.hasGuard() and scope = arm.getExpr() + // ord = getPreOrderNumbering(scope, scope) + scope = arm //.getGuard() + // or + // not arm.hasGuard() and scope = arm.getExpr() ) or exists(LetStmt let | let.getPat() = pat and - scope = getEnclosingScope(let) and + // scope = getEnclosingScope(let) and // for `let` statements, variables are bound _after_ the statement, i.e. // not in the RHS - ord = getLastPreOrderNumbering(scope, let) + 1 + scope = let + // ord = getLastPreOrderNumbering(scope, let) + 1 ) or exists(LetExpr let, Expr scrutinee | let.getPat() = pat and scrutinee = let.getScrutinee() and - scope = getEnclosingScope(scrutinee) and + // scope = getEnclosingScope(scrutinee) and // for `let` expressions, variables are bound _after_ the expression, i.e. // not in the RHS - ord = getLastPreOrderNumbering(scope, scrutinee) + 1 + scope = let + // ord = getLastPreOrderNumbering(scope, scrutinee) + 1 ) or exists(ForExpr fe | fe.getPat() = pat and - scope = fe.getLoopBody() and - ord = getPreOrderNumbering(scope, scope) + scope = fe.getLoopBody() //and + // ord = getPreOrderNumbering(scope, scope) ) ) ) } - /** - * Holds if `cand` may access a variable named `name` at pre-order number `ord` - * in the variable scope `scope`. - * - * `nestLevel` is the number of nested scopes that need to be traversed - * to reach `scope` from `cand`. - */ - private predicate variableAccessCandInScope( - VariableAccessCand cand, VariableScope scope, string name, int nestLevel, int ord - ) { - name = cand.getName() and - ( - scope = cand - or - not cand instanceof VariableScope and - scope = getEnclosingScope(cand) - ) and - ord = getPreOrderNumbering(scope, cand) and - nestLevel = 0 - or - exists(VariableScope inner | - variableAccessCandInScope(cand, inner, name, nestLevel - 1, _) and - scope = getEnclosingScope(inner) and - // Use the pre-order number of the inner scope as the number of the access. This allows - // us to collapse multiple accesses in inner scopes to a single entity - ord = getPreOrderNumbering(scope, inner) + pragma[nomagic] + private predicate lookupInScope(string name, VariableScope scope, VariableScope outer) { + exists(VariableAccessCand cand | + cand.getName() = name and + scope = getEnclosingScope(cand) and + outer = scope ) - } - - private newtype TDefOrAccessCand = - TDefOrAccessCandNestedFunction(Function f, BlockExprScope scope) { - f = scope.getStmtList().getAStatement() - } or - TDefOrAccessCandVariable(Variable v) or - TDefOrAccessCandVariableAccessCand(VariableAccessCand va) - - /** - * A nested function declaration, variable declaration, or variable (or function) - * access candidate. - * - * In order to determine whether a candidate is an actual variable/function access, - * we rank declarations and candidates by their position in the AST. - * - * The ranking must take names into account, but also variable scopes; below a comment - * `rank(scope, name, i)` means that the declaration/access on the given line has rank - * `i` amongst all declarations/accesses inside variable scope `scope`, for name `name`: - * - * ```rust - * fn f() { // scope0 - * let x = 0; // rank(scope0, "x", 0) - * use(x); // rank(scope0, "x", 1) - * let x = // rank(scope0, "x", 3) - * x + 1; // rank(scope0, "x", 2) - * let y = // rank(scope0, "y", 0) - * x; // rank(scope0, "x", 4) - * - * { // scope1 - * use(x); // rank(scope1, "x", 0), rank(scope0, "x", 4) - * use(y); // rank(scope1, "y", 0), rank(scope0, "y", 1) - * let x = 2; // rank(scope1, "x", 1) - * use(x); // rank(scope1, "x", 2), rank(scope0, "x", 4) - * } - * } - * ``` - * - * Function/variable declarations are only ranked in the scope that they bind into, - * while accesses candidates propagate outwards through scopes, as they may access - * declarations from outer scopes. - * - * For an access candidate with ranks `{ rank(scope_i, name, rnk_i) | i in I }` and - * declarations `d in D` with ranks `rnk(scope_d, name, rnk_d)`, the target is - * calculated as - * ``` - * max_{i in I} ( - * max_{d in D | scope_d = scope_i and rnk_d < rnk_i} ( - * d - * ) - * ) - * ``` - * - * i.e., its the nearest declaration before the access in the same (or outer) scope - * as the access. - */ - abstract private class DefOrAccessCand extends TDefOrAccessCand { - abstract string toString(); - - abstract Location getLocation(); - - pragma[nomagic] - abstract predicate rankBy(string name, VariableScope scope, int ord, int kind); - } - - abstract private class NestedFunctionOrVariable extends DefOrAccessCand { } - - private class DefOrAccessCandNestedFunction extends NestedFunctionOrVariable, - TDefOrAccessCandNestedFunction - { - private Function f; - private BlockExprScope scope_; - - DefOrAccessCandNestedFunction() { this = TDefOrAccessCandNestedFunction(f, scope_) } - - override string toString() { result = f.toString() } - - override Location getLocation() { result = f.getLocation() } - - override predicate rankBy(string name, VariableScope scope, int ord, int kind) { - // nested functions behave as if they are defined at the beginning of the scope - name = f.getName().getText() and - scope = scope_ and - ord = 0 and - kind = 0 - } - } - - private class DefOrAccessCandVariable extends NestedFunctionOrVariable, TDefOrAccessCandVariable { - private Variable v; - - DefOrAccessCandVariable() { this = TDefOrAccessCandVariable(v) } - - override string toString() { result = v.toString() } - - override Location getLocation() { result = v.getLocation() } - - override predicate rankBy(string name, VariableScope scope, int ord, int kind) { - variableDeclInScope(v, scope, name, ord) and - kind = 1 - } - } - - private class DefOrAccessCandVariableAccessCand extends DefOrAccessCand, - TDefOrAccessCandVariableAccessCand - { - private VariableAccessCand va; - - DefOrAccessCandVariableAccessCand() { this = TDefOrAccessCandVariableAccessCand(va) } - - override string toString() { result = va.toString() } - - override Location getLocation() { result = va.getLocation() } - - override predicate rankBy(string name, VariableScope scope, int ord, int kind) { - variableAccessCandInScope(va, scope, name, _, ord) and - kind = 2 - } - } - - private module DenseRankInput implements DenseRankInputSig2 { - class C1 = VariableScope; - - class C2 = string; - - class Ranked = DefOrAccessCand; - - int getRank(VariableScope scope, string name, DefOrAccessCand v) { - v = - rank[result](DefOrAccessCand v0, int ord, int kind | - v0.rankBy(name, scope, ord, kind) - | - v0 order by ord, kind - ) - } - } - - /** - * 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, DefOrAccessCand v) { - v = DenseRank2::denseRank(scope, name, result + 1) - } - - /** - * Holds if `v` can reach rank `rnk` in the variable scope `scope`. This is needed to - * take shadowing into account, for example in - * - * ```rust - * let x = 0; // rank 0 - * use(x); // rank 1 - * let x = ""; // rank 2 - * use(x); // rank 3 - * ``` - * - * the declaration at rank 0 can only reach the access at rank 1, while the declaration - * at rank 2 can only reach the access at rank 3. - */ - private predicate variableReachesRank( - VariableScope scope, string name, NestedFunctionOrVariable v, int rnk - ) { - rnk = rankVariableOrAccess(scope, name, v) or - variableReachesRank(scope, name, v, rnk - 1) and - rnk = rankVariableOrAccess(scope, name, TDefOrAccessCandVariableAccessCand(_)) - } - - private predicate variableReachesCand( - VariableScope scope, string name, NestedFunctionOrVariable v, VariableAccessCand cand, - int nestLevel - ) { - exists(int rnk | - variableReachesRank(scope, name, v, rnk) and - rnk = rankVariableOrAccess(scope, name, TDefOrAccessCandVariableAccessCand(cand)) and - variableAccessCandInScope(cand, scope, name, nestLevel, _) + exists(VariableScope mid | + lookupInScope(name, scope, mid) and + not variableDeclInScope(_, mid, name) and + outer = getEnclosingScope(mid) ) } pragma[nomagic] - predicate access(string name, NestedFunctionOrVariable v, VariableAccessCand cand) { - v = - min(NestedFunctionOrVariable v0, int nestLevel | - variableReachesCand(_, name, v0, cand, nestLevel) - | - v0 order by nestLevel - ) + private predicate access(VariableAccessCand cand, string name, Variable v) { + exists(VariableScope scope, VariableScope outer | + cand.getName() = name and + scope = getEnclosingScope(cand) and + lookupInScope(name, scope, outer) and + variableDeclInScope(v, outer, name) + ) } /** A variable access. */ @@ -667,7 +564,7 @@ module Impl { private string name; private Variable v; - VariableAccess() { variableAccess(name, v, this) } + VariableAccess() { access(this, name, v) } /** Gets the variable being accessed. */ Variable getVariable() { result = v } @@ -682,7 +579,7 @@ module Impl { or exists(Expr mid | assignmentOperationDescendant(ao, mid) and - getImmediateParentAdj(e) = mid and + mid = e.getParentNode() and not mid instanceof DerefExpr and not mid instanceof FieldExpr and not mid instanceof IndexExpr @@ -748,13 +645,11 @@ module Impl { cached predicate variableAccess(string name, Variable v, VariableAccessCand cand) { - access(name, TDefOrAccessCandVariable(v), cand) + access(cand, name, v) } cached - predicate nestedFunctionAccess(string name, Function f, VariableAccessCand cand) { - access(name, TDefOrAccessCandNestedFunction(f, _), cand) - } + predicate nestedFunctionAccess(string name, Function f, VariableAccessCand cand) { none() } } private import Cached diff --git a/rust/ql/test/library-tests/variables/Ssa.expected b/rust/ql/test/library-tests/variables/Ssa.expected index 0acf3e94c5b5..6ac46727548e 100644 --- a/rust/ql/test/library-tests/variables/Ssa.expected +++ b/rust/ql/test/library-tests/variables/Ssa.expected @@ -322,6 +322,8 @@ read | main.rs:449:9:449:10 | n2 | main.rs:449:9:449:10 | n2 | main.rs:451:15:451:16 | n2 | | main.rs:456:9:456:9 | f | main.rs:456:9:456:9 | f | main.rs:459:15:459:15 | f | | main.rs:456:9:456:9 | f | main.rs:456:9:456:9 | f | main.rs:466:15:466:15 | f | +| main.rs:456:9:456:9 | f | main.rs:456:9:456:9 | f | main.rs:469:19:469:19 | f | +| main.rs:456:9:456:9 | f | main.rs:456:9:456:9 | f | main.rs:476:23:476:23 | f | | main.rs:457:10:457:10 | x | main.rs:457:10:457:10 | x | main.rs:458:9:458:9 | x | | main.rs:461:10:461:10 | x | main.rs:461:10:461:10 | x | main.rs:463:9:463:9 | x | | main.rs:470:14:470:14 | x | main.rs:470:14:470:14 | x | main.rs:472:17:472:17 | x | @@ -617,6 +619,8 @@ adjacentReads | main.rs:412:9:412:10 | b4 | main.rs:403:13:403:14 | b4 | main.rs:420:15:420:16 | b4 | main.rs:434:15:434:16 | b4 | | main.rs:413:9:413:11 | a10 | main.rs:402:13:402:15 | a10 | main.rs:419:15:419:17 | a10 | main.rs:433:15:433:17 | a10 | | main.rs:456:9:456:9 | f | main.rs:456:9:456:9 | f | main.rs:459:15:459:15 | f | main.rs:466:15:466:15 | f | +| main.rs:456:9:456:9 | f | main.rs:456:9:456:9 | f | main.rs:466:15:466:15 | f | main.rs:469:19:469:19 | f | +| main.rs:456:9:456:9 | f | main.rs:456:9:456:9 | f | main.rs:469:19:469:19 | f | main.rs:476:23:476:23 | f | | main.rs:497:5:497:5 | a | main.rs:496:13:496:13 | a | main.rs:498:15:498:15 | a | main.rs:499:11:499:11 | a | | main.rs:511:17:511:17 | x | main.rs:511:17:511:17 | x | main.rs:512:6:512:6 | x | main.rs:513:10:513:10 | x | | main.rs:511:17:511:17 | x | main.rs:511:17:511:17 | x | main.rs:513:10:513:10 | x | main.rs:514:10:514:10 | x | diff --git a/rust/ql/test/library-tests/variables/variables.expected b/rust/ql/test/library-tests/variables/variables.expected index ea360357d970..bf108144e5b1 100644 --- a/rust/ql/test/library-tests/variables/variables.expected +++ b/rust/ql/test/library-tests/variables/variables.expected @@ -1,4 +1,6 @@ testFailures +| main.rs:469:19:469:19 | f | Unexpected result: read_access=f1 | +| main.rs:476:23:476:23 | f | Unexpected result: read_access=f1 | variable | main.rs:5:14:5:14 | s | | main.rs:10:14:10:14 | i | @@ -277,7 +279,9 @@ variableAccess | main.rs:459:15:459:15 | f | main.rs:456:9:456:9 | f | | main.rs:463:9:463:9 | x | main.rs:461:10:461:10 | x | | main.rs:466:15:466:15 | f | main.rs:456:9:456:9 | f | +| main.rs:469:19:469:19 | f | main.rs:456:9:456:9 | f | | main.rs:472:17:472:17 | x | main.rs:470:14:470:14 | x | +| main.rs:476:23:476:23 | f | main.rs:456:9:456:9 | f | | main.rs:481:13:481:13 | x | main.rs:480:14:480:14 | x | | main.rs:482:19:482:19 | f | main.rs:479:13:479:13 | f | | main.rs:490:12:490:12 | v | main.rs:487:9:487:9 | v | @@ -514,7 +518,9 @@ variableReadAccess | main.rs:459:15:459:15 | f | main.rs:456:9:456:9 | f | | main.rs:463:9:463:9 | x | main.rs:461:10:461:10 | x | | main.rs:466:15:466:15 | f | main.rs:456:9:456:9 | f | +| main.rs:469:19:469:19 | f | main.rs:456:9:456:9 | f | | main.rs:472:17:472:17 | x | main.rs:470:14:470:14 | x | +| main.rs:476:23:476:23 | f | main.rs:456:9:456:9 | f | | main.rs:481:13:481:13 | x | main.rs:480:14:480:14 | x | | main.rs:482:19:482:19 | f | main.rs:479:13:479:13 | f | | main.rs:490:12:490:12 | v | main.rs:487:9:487:9 | v | @@ -695,5 +701,3 @@ capturedAccess | main.rs:659:13:659:16 | self | | main.rs:749:13:749:13 | x | nestedFunctionAccess -| main.rs:469:19:469:19 | f | main.rs:470:9:473:9 | fn f | -| main.rs:476:23:476:23 | f | main.rs:470:9:473:9 | fn f | From 1dc185538140d45ed6c8795ecf1ac51705be3d1b Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 20 May 2026 13:49:25 +0200 Subject: [PATCH 2/8] tc --- .../rust/elements/internal/VariableImpl.qll | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll index 977537a7c13e..7c2165e0e085 100644 --- a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll +++ b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll @@ -443,6 +443,34 @@ module Impl { ) } + private newtype TNode = + TAstNode(AstNode n) or + TVariableScope(VariableScope s) + + private AstNode getAstNode(TAstNode n) { n = TAstNode(result) } + + private predicate hasParent(TNode child, TNode parent) { + exists(AstNode c, AstNode p | + c = getAChild(p) and + p = getAstNode(parent) + | + c = getAstNode(child) and + not c instanceof VariableScope + or + child = TVariableScope(c) + ) + } + + private predicate isCandOrScope(TNode n) { + getAstNode(n) instanceof VariableAccessCand or + n = TVariableScope(_) + } + + private predicate isScope(TNode n) { getAstNode(n) instanceof VariableScope } + + private predicate hasEnclosingScope(TNode n, TNode scope) = + doublyBoundedFastTC(hasParent/2, isCandOrScope/1, isScope/1)(n, scope) + private AstNode getAnAncestorInVariableScope(AstNode n) { // ( // n instanceof Pat or @@ -460,7 +488,9 @@ module Impl { } /** Gets the immediately enclosing variable scope of `n`. */ - private VariableScope getEnclosingScope(AstNode n) { result = getAnAncestorInVariableScope(n) } + private VariableScope getEnclosingScope(AstNode n) { + hasEnclosingScope([TAstNode(n).(TNode), TVariableScope(n)], TAstNode(result)) + } /** * Get all the pattern ancestors of this variable up to an including the From 64a4467dd92b278c26032cd400995f59c9090a7c Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 20 May 2026 14:11:25 +0200 Subject: [PATCH 3/8] cleanup --- .../rust/elements/internal/VariableImpl.qll | 204 +++--------------- 1 file changed, 33 insertions(+), 171 deletions(-) diff --git a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll index 7c2165e0e085..2f4fe0e9632f 100644 --- a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll +++ b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll @@ -8,70 +8,6 @@ private import codeql.rust.elements.internal.FormatTemplateVariableAccessImpl::I private import codeql.util.DenseRank module Impl { - /** - * A variable scope. Either a block `{ ... }`, the guard/rhs - * of a match arm, or the body of a closure. - */ - abstract class VariableScope extends AstNode { } - - class BlockExprScope extends VariableScope, BlockExpr { } - - class MatchScope extends VariableScope, MatchArm { - // MatchArmExprScope() { this = any(MatchArm arm).getExpr() } - } - - // class MatchArmExprScope extends VariableScope { - // MatchArmExprScope() { this = any(MatchArm arm).getExpr() } - // } - // class MatchArmGuardScope extends VariableScope { - // MatchArmGuardScope() { this = any(MatchArm arm).getGuard() } - // } - class CallableScope extends VariableScope, Callable { - // ClosureBodyScope() { this = any(ClosureExpr ce).getBody() } - } - - class LetScope extends VariableScope, Let { } - - /** - * A scope for conditions, which may introduce variables using `let` expressions. - * - * Such variables are only available in the body guarded by the condition. - */ - class ConditionScope extends AstNode { - private AstNode parent; - private AstNode body; - - ConditionScope() { - parent = - any(IfExpr ie | - this = ie.getCondition() and - body = ie.getThen() - ) - or - parent = - any(WhileExpr we | - this = we.getCondition() and - body = we.getLoopBody() - ) - or - parent = - any(MatchArm ma | - this = ma.getGuard() and - body = ma.getExpr() - ) - } - - /** Gets the parent of this condition. */ - AstNode getParent() { result = parent } - - /** - * Gets the body in which variables introduced in this scope are available. - */ - AstNode getBody() { result = body } - - AstNode getElse() { result = any(IfExpr ie | this = ie.getCondition()).getElse() } - } - /** * A scope for conditions, which may introduce variables using `let` expressions. * @@ -401,7 +337,6 @@ module Impl { private predicate isLetParent(AstNode parent, int i, Let let) { let = getRankedChild(parent, i) } private predicate shouldBeLetChild(AstNode parent, int i, Let let, AstNode child) { - // parent.getEnclosingCallable().(Function).getName().getText() = "let_pattern3" and child = getRankedChild(parent, i) and ( isLetParent(parent, i - 1, let) @@ -411,85 +346,42 @@ module Impl { } AstNode getAChild(AstNode parent) { - // parent.getEnclosingCallable().(Function).getName().getText() = "let_pattern3" and - // result = ParentChild::getImmediateChild(parent, _) and - ( - not shouldBeLetChild(_, _, _, result) and - not result = [any(Let let).getRhs(), any(Let let).getElse()] and - // not result = [any(Case c).getPattern(_), any(Case c).getGuard(), any(Case c).getBody()] and - // not result instanceof Let and - result = getRankedChild(parent, _) - or - shouldBeLetChild(_, _, parent, result) - or - exists(Let let | let = getAChild(parent) | result = [let.getRhs(), let.getElse()]) - // or - // exists(AstNode par, Let let, int k, AstNode child | shouldBeLetChild(par, k, let, child) | - // result = child and - // parent = let - // or - // ( - // shouldBeLetChild(_, _, parent, let) - // or - // not shouldBeLetChild(_, _, _, let) and - // parent = par - // ) and - // ( - // result = let.getRhs() - // or - // result = let - // ) - // ) - ) + not shouldBeLetChild(_, _, _, result) and + not result = [any(Let let).getRhs(), any(Let let).getElse()] and + result = getRankedChild(parent, _) + or + shouldBeLetChild(_, _, parent, result) + or + exists(Let let | let = getAChild(parent) | result = [let.getRhs(), let.getElse()]) } - private newtype TNode = - TAstNode(AstNode n) or - TVariableScope(VariableScope s) - - private AstNode getAstNode(TAstNode n) { n = TAstNode(result) } - - private predicate hasParent(TNode child, TNode parent) { - exists(AstNode c, AstNode p | - c = getAChild(p) and - p = getAstNode(parent) - | - c = getAstNode(child) and - not c instanceof VariableScope - or - child = TVariableScope(c) - ) + class VariableScope extends AstNode { + VariableScope() { variableDeclInScope(_, this, _) } } - private predicate isCandOrScope(TNode n) { - getAstNode(n) instanceof VariableAccessCand or - n = TVariableScope(_) + private predicate hasChild(AstNode parent, AstNode child) { + child = getAChild(parent) and + not parent instanceof VariableScope } - private predicate isScope(TNode n) { getAstNode(n) instanceof VariableScope } - - private predicate hasEnclosingScope(TNode n, TNode scope) = - doublyBoundedFastTC(hasParent/2, isCandOrScope/1, isScope/1)(n, scope) - - private AstNode getAnAncestorInVariableScope(AstNode n) { - // ( - // n instanceof Pat or - // n instanceof VariableAccessCand or - // n instanceof LetStmt or - // n = any(LetExpr le).getScrutinee() or - // n instanceof VariableScope - // ) and - exists(AstNode n0 | n0 = getAChild(result) | - n0 = n - or - n0 = getAnAncestorInVariableScope(n) and - not n0 instanceof VariableScope - ) + private predicate isCandOrScope(AstNode n) { + n instanceof VariableAccessCand or + n instanceof VariableScope } + private predicate isScopeChild(AstNode n) { exists(VariableScope scope | n = getAChild(scope)) } + + private predicate hasEnclosingScope(AstNode scopeChild, AstNode n) = + doublyBoundedFastTC(hasChild/2, isScopeChild/1, isCandOrScope/1)(scopeChild, n) + /** Gets the immediately enclosing variable scope of `n`. */ private VariableScope getEnclosingScope(AstNode n) { - hasEnclosingScope([TAstNode(n).(TNode), TVariableScope(n)], TAstNode(result)) + exists(AstNode scopeChild | scopeChild = getAChild(result) | + n = scopeChild and + isCandOrScope(n) + or + hasEnclosingScope(scopeChild, n) + ) } /** @@ -505,60 +397,30 @@ module Impl { ) } - /** - * Holds if a parameter declares the variable `v` inside variable scope `scope`. - */ - private predicate parameterDeclInScope(Variable v, VariableScope scope) { - exists(Callable f | - v.getParameter() = f.getParamList().getAParamBase() and - scope = f // f.getBody() - ) - } - /** * Holds if `v` is named `name` and is declared inside variable scope * `scope`. The pre-order numbering of the binding site of `v`, amongst * all nodes nested under `scope`, is `ord`. */ - private predicate variableDeclInScope(Variable v, VariableScope scope, string name) { + private predicate variableDeclInScope(Variable v, AstNode scope, string name) { name = v.getText() and ( - parameterDeclInScope(v, scope) //and + v.getParameter() = scope.(Callable).getParamList().getAParamBase() or - // ord = getPreOrderNumbering(scope, scope) exists(Pat pat | pat = getAVariablePatAncestor(v) | exists(MatchArm arm | - pat = arm.getPat() //and - | - // ord = getPreOrderNumbering(scope, scope) - scope = arm //.getGuard() - // or - // not arm.hasGuard() and scope = arm.getExpr() - ) - 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 - scope = let - // ord = getLastPreOrderNumbering(scope, let) + 1 + pat = arm.getPat() and + scope = arm ) or - exists(LetExpr let, Expr scrutinee | - let.getPat() = pat and - scrutinee = let.getScrutinee() and - // scope = getEnclosingScope(scrutinee) and - // for `let` expressions, variables are bound _after_ the expression, i.e. - // not in the RHS + exists(Let let | + let.getLhs() = pat and scope = let - // ord = getLastPreOrderNumbering(scope, scrutinee) + 1 ) or exists(ForExpr fe | fe.getPat() = pat and - scope = fe.getLoopBody() //and - // ord = getPreOrderNumbering(scope, scope) + scope = fe.getLoopBody() ) ) ) From 6cb74ca8b04221ec92418c684e9093a9630b80e4 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 20 May 2026 14:41:20 +0200 Subject: [PATCH 4/8] wip --- .../rust/elements/internal/VariableImpl.qll | 73 ++++++++++--------- 1 file changed, 40 insertions(+), 33 deletions(-) diff --git a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll index 2f4fe0e9632f..7f1fe56339cc 100644 --- a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll +++ b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll @@ -102,6 +102,41 @@ module Impl { ) } + /** + * Holds if `v` is named `name` and is declared inside variable scope + * `scope`. The pre-order numbering of the binding site of `v`, amongst + * all nodes nested under `scope`, is `ord`. + */ + private predicate variableDeclInScope(AstNode definingNode, string name, AstNode scope) { + exists(Name n | variableDecl(definingNode, n, name) | + exists(SelfParam self | self = scope.(Callable).getSelfParam() and n = self.getName()) + or + exists(Pat pat, Pat pat0 | + pat = getAPatAncestor*(pat0) and + (pat0 = definingNode or pat0.(IdentPat).getName() = n) + | + exists(MatchArm arm | + pat = arm.getPat() and + scope = arm + ) + or + exists(Let let | + let.getLhs() = pat and + scope = let + ) + or + exists(ForExpr fe | + fe.getPat() = pat and + scope = fe.getLoopBody() + ) + or + exists(Param p | p = scope.(Callable).getParamList().getAParamBase() | + pat = p.(Param).getPat() + ) + ) + ) + } + /** A variable. */ class Variable extends MkVariable { private AstNode definingNode; @@ -356,7 +391,7 @@ module Impl { } class VariableScope extends AstNode { - VariableScope() { variableDeclInScope(_, this, _) } + VariableScope() { variableDeclInScope(_, _, this) } } private predicate hasChild(AstNode parent, AstNode child) { @@ -397,35 +432,6 @@ module Impl { ) } - /** - * Holds if `v` is named `name` and is declared inside variable scope - * `scope`. The pre-order numbering of the binding site of `v`, amongst - * all nodes nested under `scope`, is `ord`. - */ - private predicate variableDeclInScope(Variable v, AstNode scope, string name) { - name = v.getText() and - ( - v.getParameter() = scope.(Callable).getParamList().getAParamBase() - or - exists(Pat pat | pat = getAVariablePatAncestor(v) | - exists(MatchArm arm | - pat = arm.getPat() and - scope = arm - ) - or - exists(Let let | - let.getLhs() = pat and - scope = let - ) - or - exists(ForExpr fe | - fe.getPat() = pat and - scope = fe.getLoopBody() - ) - ) - ) - } - pragma[nomagic] private predicate lookupInScope(string name, VariableScope scope, VariableScope outer) { exists(VariableAccessCand cand | @@ -436,18 +442,19 @@ module Impl { or exists(VariableScope mid | lookupInScope(name, scope, mid) and - not variableDeclInScope(_, mid, name) and + not variableDeclInScope(_, name, mid) and outer = getEnclosingScope(mid) ) } pragma[nomagic] private predicate access(VariableAccessCand cand, string name, Variable v) { - exists(VariableScope scope, VariableScope outer | + exists(AstNode definingNode, VariableScope scope, VariableScope outer | cand.getName() = name and scope = getEnclosingScope(cand) and lookupInScope(name, scope, outer) and - variableDeclInScope(v, outer, name) + variableDeclInScope(definingNode, name, outer) and + v = MkVariable(definingNode, name) ) } From 3019922b8ba3aa709f2ec51a8353a02a2a6fec62 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 20 May 2026 15:08:11 +0200 Subject: [PATCH 5/8] share --- .../elements/internal/LocalNameBinding.qll | 220 ++++++++++++ .../rust/elements/internal/VariableImpl.qll | 316 +++++------------- 2 files changed, 296 insertions(+), 240 deletions(-) create mode 100644 rust/ql/lib/codeql/rust/elements/internal/LocalNameBinding.qll diff --git a/rust/ql/lib/codeql/rust/elements/internal/LocalNameBinding.qll b/rust/ql/lib/codeql/rust/elements/internal/LocalNameBinding.qll new file mode 100644 index 000000000000..5bcf86695df1 --- /dev/null +++ b/rust/ql/lib/codeql/rust/elements/internal/LocalNameBinding.qll @@ -0,0 +1,220 @@ +private import codeql.util.DenseRank +private import codeql.util.Location + +signature module LocalNameBindingInputSig { + /** An AST node. */ + class AstNode { + /** Gets a textual representation of this element. */ + string toString(); + + /** Gets the location of this element. */ + Location getLocation(); + } + + /** Gets the child of AST node `n` at the specified index. */ + AstNode getChild(AstNode n, int index); + + class Conditional extends AstNode { + AstNode getCondition(); + + AstNode getThen(); + + AstNode getElse(); + } + + class Let extends AstNode { + AstNode getLhs(); + + AstNode getRhs(); + + AstNode getElse(); + } + + /** + * Holds if a local declaration named `name` exists at `definingNode` inside + * the syntactic scope `scope`. + */ + predicate localDeclInScope(AstNode definingNode, string name, AstNode scope); + + predicate localAccessCand(AstNode n, string name); +} + +module LocalNameBinding Input> { + private import Input + + /** A locally declared entity, for example a variable or a parameter. */ + final class Local extends MkLocal { + private AstNode definingNode; + private string name; + + Local() { this = MkLocal(definingNode, name) } + + /** Gets the AST node that defines this local. */ + AstNode getDefiningNode() { result = definingNode } + + /** Gets the name of this local entity. */ + string getName() { result = name } + + /** Gets the location of this local entity. */ + Location getLocation() { result = definingNode.getLocation() } + + /** Gets a textual representation of this local entity. */ + string toString() { result = this.getName() } + + /** Gets an access to this local. */ + LocalAccess getAnAccess() { result.getLocal() = this } + } + + final private class AstNodeFinal = AstNode; + + /** + * A path expression that may access a local variable. These are paths that + * only consist of a simple name (i.e., without generic arguments, + * qualifiers, etc.). + */ + private class LocalAccessCand extends AstNodeFinal { + string name_; + + LocalAccessCand() { localAccessCand(this, name_) } + + string getName() { result = name_ } + } + + private module ChildDenseRankInput implements DenseRankInputSig1 { + class C = AstNode; + + class Ranked = AstNode; + + /** + * An adjusted version of `ParentChild::getImmediateChild`, which makes the following + * two adjustments: + * + * 1. For conditions like `if cond body`, instead of letting `body` be the second child + * of `if`, we make it the last child of `cond`. This ensures that variables + * introduced in the `cond` scope are available in `body`. + * + * 2. A similar adjustment is made for `while` loops: the body of the loop is made a + * child of the loop condition instead of the loop itself. + */ + int getRank(C parent, Ranked child) { + // parent.getEnclosingCallable().(Function).getName().getText() = "match_pattern14" and + child = + rank[result](AstNode res, int preOrd, int i | + res = getChild(parent, i) and + preOrd = 0 and + not exists(Conditional cond | res = [cond.getThen(), cond.getElse()]) + or + res = parent.(Conditional).getElse() and + preOrd = -1 and + i = 0 + or + exists(Conditional cond | + parent = cond.getCondition() and + res = cond.getThen() and + preOrd = 1 and + i = 0 + ) + | + res order by preOrd, i + ) + } + } + + private predicate getRankedChild = DenseRank1::denseRank/2; + + private predicate isLetParent(AstNode parent, int i, Let let) { let = getRankedChild(parent, i) } + + private predicate shouldBeLetChild(AstNode parent, int i, Let let, AstNode child) { + child = getRankedChild(parent, i) and + ( + isLetParent(parent, i - 1, let) + or + shouldBeLetChild(parent, i - 1, let, any(AstNode n | not n instanceof Let)) + ) + } + + private AstNode getAChild(AstNode parent) { + not shouldBeLetChild(_, _, _, result) and + not result = [any(Let let).getRhs(), any(Let let).getElse()] and + result = getRankedChild(parent, _) + or + shouldBeLetChild(_, _, parent, result) + or + exists(Let let | let = getAChild(parent) | result = [let.getRhs(), let.getElse()]) + } + + private class LocalScope extends AstNodeFinal { + LocalScope() { localDeclInScope(_, _, this) } + } + + private predicate hasChild(AstNode parent, AstNode child) { + child = getAChild(parent) and + not parent instanceof LocalScope + } + + private predicate isCandOrScope(AstNode n) { + n instanceof LocalAccessCand or + n instanceof LocalScope + } + + private predicate isScopeChild(AstNode n) { exists(LocalScope scope | n = getAChild(scope)) } + + private predicate hasEnclosingScope(AstNode scopeChild, AstNode n) = + doublyBoundedFastTC(hasChild/2, isScopeChild/1, isCandOrScope/1)(scopeChild, n) + + /** Gets the immediately enclosing variable scope of `n`. */ + private LocalScope getEnclosingScope(AstNode n) { + exists(AstNode scopeChild | scopeChild = getAChild(result) | + n = scopeChild and + isCandOrScope(n) + or + hasEnclosingScope(scopeChild, n) + ) + } + + pragma[nomagic] + private predicate lookupInScope(string name, LocalScope scope, LocalScope outer) { + exists(LocalAccessCand cand | + cand.getName() = name and + scope = getEnclosingScope(cand) and + outer = scope + ) + or + exists(LocalScope mid | + lookupInScope(name, scope, mid) and + not localDeclInScope(_, name, mid) and + outer = getEnclosingScope(mid) + ) + } + + /** A local access. */ + final class LocalAccess extends AstNodeFinal { + private string name; + private Local v; + + LocalAccess() { access(this, name, v) } + + /** Gets the local being accessed. */ + Local getLocal() { result = v } + } + + cached + private module Cached { + cached + newtype TLocal = + MkLocal(AstNode definingNode, string name) { localDeclInScope(definingNode, name, _) } + + cached + predicate access(LocalAccessCand cand, string name, Local v) { + exists(AstNode definingNode, LocalScope scope, LocalScope outer | + cand.getName() = name and + scope = getEnclosingScope(cand) and + lookupInScope(name, scope, outer) and + localDeclInScope(definingNode, name, outer) and + v = MkLocal(definingNode, name) + ) + } + } + + private import Cached +} diff --git a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll index 7f1fe56339cc..cececc1b9444 100644 --- a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll +++ b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll @@ -5,42 +5,74 @@ private import codeql.rust.elements.internal.generated.ParentChild as ParentChil private import codeql.rust.elements.internal.AstNodeImpl::Impl as AstNodeImpl private import codeql.rust.elements.internal.PathImpl::Impl as PathImpl private import codeql.rust.elements.internal.FormatTemplateVariableAccessImpl::Impl as FormatTemplateVariableAccessImpl -private import codeql.util.DenseRank +private import codeql.rust.elements.internal.LocalNameBinding module Impl { - /** - * A scope for conditions, which may introduce variables using `let` expressions. - * - * Such variables are only available in the body guarded by the condition. - */ - class Conditional extends AstNode { - Conditional() { - this instanceof IfExpr - or - this instanceof WhileExpr - or - this = any(MatchArm ma | ma.hasGuard()) - } + private module Input implements LocalNameBindingInputSig { + private import rust as Rust - AstNode getCondition() { - result = this.(IfExpr).getCondition() - or - result = this.(WhileExpr).getCondition() - or - result = this.(MatchArm).getGuard() + class AstNode = Rust::AstNode; + + predicate getChild = getImmediateChildAdjRust/2; + + /** + * A scope for conditions, which may introduce variables using `let` expressions. + * + * Such variables are only available in the body guarded by the condition. + */ + class Conditional extends AstNode { + Conditional() { + this instanceof IfExpr + or + this instanceof WhileExpr + or + this = any(MatchArm ma | ma.hasGuard()) + } + + AstNode getCondition() { + result = this.(IfExpr).getCondition() + or + result = this.(WhileExpr).getCondition() + or + result = this.(MatchArm).getGuard() + } + + AstNode getThen() { + result = this.(IfExpr).getThen() + or + result = this.(WhileExpr).getLoopBody() + or + result = this.(MatchArm).getExpr() + } + + AstNode getElse() { result = this.(IfExpr).getElse() } } - AstNode getThen() { - result = this.(IfExpr).getThen() - or - result = this.(WhileExpr).getLoopBody() - or - result = this.(MatchArm).getExpr() + class Let extends AstNode { + Let() { this instanceof LetStmt or this instanceof LetExpr } + + AstNode getLhs() { + result = this.(LetStmt).getPat() + or + result = this.(LetExpr).getPat() + } + + AstNode getRhs() { + result = this.(LetStmt).getInitializer() + or + result = this.(LetExpr).getScrutinee() + } + + AstNode getElse() { result = this.(LetStmt).getLetElse() } } - AstNode getElse() { result = this.(IfExpr).getElse() } + predicate localDeclInScope = variableDeclInScope/3; + + predicate localAccessCand(AstNode n, string name) { name = n.(VariableAccessCand).getName() } } + private import LocalNameBinding + private Pat getAPatAncestor(Pat p) { (p instanceof IdentPat or p instanceof OrPat) and exists(Pat p0 | result = p0.getParentPat() | @@ -120,7 +152,7 @@ module Impl { scope = arm ) or - exists(Let let | + exists(Input::Let let | let.getLhs() = pat and scope = let ) @@ -138,33 +170,22 @@ module Impl { } /** A variable. */ - class Variable extends MkVariable { - private AstNode definingNode; - private string text; - - Variable() { this = MkVariable(definingNode, text) } - - /** Gets the name of this variable as a string. */ - string getText() { result = text } - - /** Gets the location of this variable. */ - Location getLocation() { result = definingNode.getLocation() } - - /** Gets a textual representation of this variable. */ - string toString() { result = this.getText() } - + class Variable extends Local { /** Gets an access to this variable. */ VariableAccess getAnAccess() { result.getVariable() = this } + /** Gets the name of this variable. */ + string getText() { result = super.getName() } + /** * Get the name of this variable. * * Normally, the name is unique, except when introduced in an or pattern. */ - Name getName() { variableDecl(definingNode, result, text) } + Name getName() { variableDecl(this.getDefiningNode(), result, super.getName()) } /** Gets the block that encloses this variable, if any. */ - BlockExpr getEnclosingBlock() { result = definingNode.getEnclosingBlock() } + BlockExpr getEnclosingBlock() { result = this.getDefiningNode().getEnclosingBlock() } /** Gets the `self` parameter that declares this variable, if any. */ SelfParam getSelfParam() { result.getName() = this.getName() } @@ -183,11 +204,13 @@ module Impl { IdentPat getPat() { result.getName() = this.getName() } /** Gets the enclosing CFG scope for this variable declaration. */ - CfgScope getEnclosingCfgScope() { result = definingNode.getEnclosingCfgScope() } + CfgScope getEnclosingCfgScope() { result = this.getDefiningNode().getEnclosingCfgScope() } + // TODO: elaborate /** Gets the `let` statement that introduces this variable, if any. */ LetStmt getLetStmt() { this.getPat() = result.getPat() } + // TODO: elaborate /** Gets the `let` expression that introduces this variable, if any. */ LetExpr getLetExpr() { this.getPat() = result.getPat() } @@ -206,7 +229,7 @@ module Impl { Cached::ref() and result = this.getSelfParam() or - result.(Param).getPat() = getAVariablePatAncestor(this) + result.(Param).getPat() = getAPatAncestor*(this.getPat()) } /** Hold is this variable is mutable. */ @@ -235,45 +258,6 @@ module Impl { string getName() { result = name_ } } - class Let extends AstNode { - Let() { this instanceof LetStmt or this instanceof LetExpr } - - AstNode getLhs() { - result = this.(LetStmt).getPat() - or - result = this.(LetExpr).getPat() - } - - AstNode getRhs() { - result = this.(LetStmt).getInitializer() - or - result = this.(LetExpr).getScrutinee() - } - - AstNode getElse() { result = this.(LetStmt).getLetElse() } - } - - /** A case in a switch. */ - class Case extends MatchArm { - /** Gets the pattern being matched by this case at the specified (zero-based) `index`. */ - AstNode getPattern(int index) { - result = this.getPat() and - index = 0 - } - - /** Gets the guard expression of this case, if any. */ - AstNode getGuard() { result = super.getGuard() } - - /** - * Gets the body of this case, if any. - * - * A case can either have a body as a single child AST node given by this - * predicate, or it can have an implicit body given by the sequence of - * statements between this case and the next case. - */ - AstNode getBody() { result = this.getExpr() } - } - private LogicalAndExpr getALetChainAncestor(LetExpr let) { let = result.getAnOperand() or @@ -325,151 +309,15 @@ module Impl { ) } - private import codeql.util.DenseRank - - private module ChildDenseRankInput implements DenseRankInputSig1 { - class C = AstNode; - - class Ranked = AstNode; - - /** - * An adjusted version of `ParentChild::getImmediateChild`, which makes the following - * two adjustments: - * - * 1. For conditions like `if cond body`, instead of letting `body` be the second child - * of `if`, we make it the last child of `cond`. This ensures that variables - * introduced in the `cond` scope are available in `body`. - * - * 2. A similar adjustment is made for `while` loops: the body of the loop is made a - * child of the loop condition instead of the loop itself. - */ - int getRank(C parent, Ranked child) { - // parent.getEnclosingCallable().(Function).getName().getText() = "match_pattern14" and - child = - rank[result](AstNode res, int preOrd, int i | - res = getImmediateChildAdjRust(parent, i) and - preOrd = 0 and - not exists(Conditional cond | res = [cond.getThen(), cond.getElse()]) - or - res = parent.(Conditional).getElse() and - preOrd = -1 and - i = 0 - or - exists(Conditional cond | - parent = cond.getCondition() and - res = cond.getThen() and - preOrd = 1 and - i = 0 - ) - | - res order by preOrd, i - ) - } - } - - predicate getRankedChild = DenseRank1::denseRank/2; - - private predicate isLetParent(AstNode parent, int i, Let let) { let = getRankedChild(parent, i) } - - private predicate shouldBeLetChild(AstNode parent, int i, Let let, AstNode child) { - child = getRankedChild(parent, i) and - ( - isLetParent(parent, i - 1, let) - or - shouldBeLetChild(parent, i - 1, let, any(AstNode n | not n instanceof Let)) - ) - } - - AstNode getAChild(AstNode parent) { - not shouldBeLetChild(_, _, _, result) and - not result = [any(Let let).getRhs(), any(Let let).getElse()] and - result = getRankedChild(parent, _) - or - shouldBeLetChild(_, _, parent, result) - or - exists(Let let | let = getAChild(parent) | result = [let.getRhs(), let.getElse()]) - } - - class VariableScope extends AstNode { - VariableScope() { variableDeclInScope(_, _, this) } - } - - private predicate hasChild(AstNode parent, AstNode child) { - child = getAChild(parent) and - not parent instanceof VariableScope - } - - private predicate isCandOrScope(AstNode n) { - n instanceof VariableAccessCand or - n instanceof VariableScope - } - - private predicate isScopeChild(AstNode n) { exists(VariableScope scope | n = getAChild(scope)) } - - private predicate hasEnclosingScope(AstNode scopeChild, AstNode n) = - doublyBoundedFastTC(hasChild/2, isScopeChild/1, isCandOrScope/1)(scopeChild, n) - - /** Gets the immediately enclosing variable scope of `n`. */ - private VariableScope getEnclosingScope(AstNode n) { - exists(AstNode scopeChild | scopeChild = getAChild(result) | - n = scopeChild and - isCandOrScope(n) - or - hasEnclosingScope(scopeChild, n) - ) - } - - /** - * Get all the pattern ancestors of this variable up to an including the - * root of the pattern. - */ - private Pat getAVariablePatAncestor(Variable v) { - result = v.getPat() - or - exists(Pat mid | - mid = getAVariablePatAncestor(v) and - result = mid.getParentPat() - ) - } - - pragma[nomagic] - private predicate lookupInScope(string name, VariableScope scope, VariableScope outer) { - exists(VariableAccessCand cand | - cand.getName() = name and - scope = getEnclosingScope(cand) and - outer = scope - ) - or - exists(VariableScope mid | - lookupInScope(name, scope, mid) and - not variableDeclInScope(_, name, mid) and - outer = getEnclosingScope(mid) - ) - } - - pragma[nomagic] - private predicate access(VariableAccessCand cand, string name, Variable v) { - exists(AstNode definingNode, VariableScope scope, VariableScope outer | - cand.getName() = name and - scope = getEnclosingScope(cand) and - lookupInScope(name, scope, outer) and - variableDeclInScope(definingNode, name, outer) and - v = MkVariable(definingNode, name) - ) - } - /** A variable access. */ - class VariableAccess extends PathExprBase { - private string name; - private Variable v; - - VariableAccess() { access(this, name, v) } - + class VariableAccess extends LocalAccess { /** Gets the variable being accessed. */ - Variable getVariable() { result = v } + Variable getVariable() { result = super.getLocal() } /** Holds if this access is a capture. */ - predicate isCapture() { this.getEnclosingCfgScope() != v.getEnclosingCfgScope() } + predicate isCapture() { + this.getEnclosingCfgScope() != this.getVariable().getEnclosingCfgScope() + } } /** Holds if `e` occurs in the LHS of an assignment operation. */ @@ -514,7 +362,7 @@ module Impl { class NestedFunctionAccess extends PathExprBase { private Function f; - NestedFunctionAccess() { nestedFunctionAccess(_, f, this) } + NestedFunctionAccess() { none() } /** Gets the function being accessed. */ Function getFunction() { result = f } @@ -537,18 +385,6 @@ module Impl { or exists(any(Variable v).getParameter()) } - - cached - newtype TVariable = - MkVariable(AstNode definingNode, string name) { variableDecl(definingNode, _, name) } - - cached - predicate variableAccess(string name, Variable v, VariableAccessCand cand) { - access(cand, name, v) - } - - cached - predicate nestedFunctionAccess(string name, Function f, VariableAccessCand cand) { none() } } private import Cached From 9b56e77b1c7c00ba667d9544a7f6609b512e2793 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 20 May 2026 15:41:40 +0200 Subject: [PATCH 6/8] local function --- .../rust/elements/internal/VariableImpl.qll | 20 ++++++++++++++----- .../test/library-tests/variables/Ssa.expected | 4 ---- .../variables/variables.expected | 8 ++------ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll index cececc1b9444..b5135c310483 100644 --- a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll +++ b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll @@ -66,7 +66,15 @@ module Impl { AstNode getElse() { result = this.(LetStmt).getLetElse() } } - predicate localDeclInScope = variableDeclInScope/3; + predicate localDeclInScope(AstNode definingNode, string name, AstNode scope) { + variableDeclInScope(definingNode, name, scope) + or + definingNode = + any(Function f | + f = scope.(BlockExpr).getStmtList().getAStatement() and + name = f.getName().getText() + ) + } predicate localAccessCand(AstNode n, string name) { name = n.(VariableAccessCand).getName() } } @@ -171,6 +179,8 @@ module Impl { /** A variable. */ class Variable extends Local { + Variable() { not this.getDefiningNode() instanceof Function } + /** Gets an access to this variable. */ VariableAccess getAnAccess() { result.getVariable() = this } @@ -311,6 +321,8 @@ module Impl { /** A variable access. */ class VariableAccess extends LocalAccess { + VariableAccess() { this.getLocal() instanceof Variable } + /** Gets the variable being accessed. */ Variable getVariable() { result = super.getLocal() } @@ -359,13 +371,11 @@ module Impl { } /** A nested function access. */ - class NestedFunctionAccess extends PathExprBase { + class NestedFunctionAccess extends LocalAccess { private Function f; - NestedFunctionAccess() { none() } - /** Gets the function being accessed. */ - Function getFunction() { result = f } + Function getFunction() { result = super.getLocal().getDefiningNode() } } cached diff --git a/rust/ql/test/library-tests/variables/Ssa.expected b/rust/ql/test/library-tests/variables/Ssa.expected index 6ac46727548e..0acf3e94c5b5 100644 --- a/rust/ql/test/library-tests/variables/Ssa.expected +++ b/rust/ql/test/library-tests/variables/Ssa.expected @@ -322,8 +322,6 @@ read | main.rs:449:9:449:10 | n2 | main.rs:449:9:449:10 | n2 | main.rs:451:15:451:16 | n2 | | main.rs:456:9:456:9 | f | main.rs:456:9:456:9 | f | main.rs:459:15:459:15 | f | | main.rs:456:9:456:9 | f | main.rs:456:9:456:9 | f | main.rs:466:15:466:15 | f | -| main.rs:456:9:456:9 | f | main.rs:456:9:456:9 | f | main.rs:469:19:469:19 | f | -| main.rs:456:9:456:9 | f | main.rs:456:9:456:9 | f | main.rs:476:23:476:23 | f | | main.rs:457:10:457:10 | x | main.rs:457:10:457:10 | x | main.rs:458:9:458:9 | x | | main.rs:461:10:461:10 | x | main.rs:461:10:461:10 | x | main.rs:463:9:463:9 | x | | main.rs:470:14:470:14 | x | main.rs:470:14:470:14 | x | main.rs:472:17:472:17 | x | @@ -619,8 +617,6 @@ adjacentReads | main.rs:412:9:412:10 | b4 | main.rs:403:13:403:14 | b4 | main.rs:420:15:420:16 | b4 | main.rs:434:15:434:16 | b4 | | main.rs:413:9:413:11 | a10 | main.rs:402:13:402:15 | a10 | main.rs:419:15:419:17 | a10 | main.rs:433:15:433:17 | a10 | | main.rs:456:9:456:9 | f | main.rs:456:9:456:9 | f | main.rs:459:15:459:15 | f | main.rs:466:15:466:15 | f | -| main.rs:456:9:456:9 | f | main.rs:456:9:456:9 | f | main.rs:466:15:466:15 | f | main.rs:469:19:469:19 | f | -| main.rs:456:9:456:9 | f | main.rs:456:9:456:9 | f | main.rs:469:19:469:19 | f | main.rs:476:23:476:23 | f | | main.rs:497:5:497:5 | a | main.rs:496:13:496:13 | a | main.rs:498:15:498:15 | a | main.rs:499:11:499:11 | a | | main.rs:511:17:511:17 | x | main.rs:511:17:511:17 | x | main.rs:512:6:512:6 | x | main.rs:513:10:513:10 | x | | main.rs:511:17:511:17 | x | main.rs:511:17:511:17 | x | main.rs:513:10:513:10 | x | main.rs:514:10:514:10 | x | diff --git a/rust/ql/test/library-tests/variables/variables.expected b/rust/ql/test/library-tests/variables/variables.expected index bf108144e5b1..ea360357d970 100644 --- a/rust/ql/test/library-tests/variables/variables.expected +++ b/rust/ql/test/library-tests/variables/variables.expected @@ -1,6 +1,4 @@ testFailures -| main.rs:469:19:469:19 | f | Unexpected result: read_access=f1 | -| main.rs:476:23:476:23 | f | Unexpected result: read_access=f1 | variable | main.rs:5:14:5:14 | s | | main.rs:10:14:10:14 | i | @@ -279,9 +277,7 @@ variableAccess | main.rs:459:15:459:15 | f | main.rs:456:9:456:9 | f | | main.rs:463:9:463:9 | x | main.rs:461:10:461:10 | x | | main.rs:466:15:466:15 | f | main.rs:456:9:456:9 | f | -| main.rs:469:19:469:19 | f | main.rs:456:9:456:9 | f | | main.rs:472:17:472:17 | x | main.rs:470:14:470:14 | x | -| main.rs:476:23:476:23 | f | main.rs:456:9:456:9 | f | | main.rs:481:13:481:13 | x | main.rs:480:14:480:14 | x | | main.rs:482:19:482:19 | f | main.rs:479:13:479:13 | f | | main.rs:490:12:490:12 | v | main.rs:487:9:487:9 | v | @@ -518,9 +514,7 @@ variableReadAccess | main.rs:459:15:459:15 | f | main.rs:456:9:456:9 | f | | main.rs:463:9:463:9 | x | main.rs:461:10:461:10 | x | | main.rs:466:15:466:15 | f | main.rs:456:9:456:9 | f | -| main.rs:469:19:469:19 | f | main.rs:456:9:456:9 | f | | main.rs:472:17:472:17 | x | main.rs:470:14:470:14 | x | -| main.rs:476:23:476:23 | f | main.rs:456:9:456:9 | f | | main.rs:481:13:481:13 | x | main.rs:480:14:480:14 | x | | main.rs:482:19:482:19 | f | main.rs:479:13:479:13 | f | | main.rs:490:12:490:12 | v | main.rs:487:9:487:9 | v | @@ -701,3 +695,5 @@ capturedAccess | main.rs:659:13:659:16 | self | | main.rs:749:13:749:13 | x | nestedFunctionAccess +| main.rs:469:19:469:19 | f | main.rs:470:9:473:9 | fn f | +| main.rs:476:23:476:23 | f | main.rs:470:9:473:9 | fn f | From 6a00f95da5d5abf9aea1874ef2adc2c5bba3209b Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 20 May 2026 20:48:51 +0200 Subject: [PATCH 7/8] Remove tc --- .../elements/internal/LocalNameBinding.qll | 26 +++++-------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/rust/ql/lib/codeql/rust/elements/internal/LocalNameBinding.qll b/rust/ql/lib/codeql/rust/elements/internal/LocalNameBinding.qll index 5bcf86695df1..d01a035b701f 100644 --- a/rust/ql/lib/codeql/rust/elements/internal/LocalNameBinding.qll +++ b/rust/ql/lib/codeql/rust/elements/internal/LocalNameBinding.qll @@ -147,28 +147,14 @@ module LocalNameBinding LocalScope() { localDeclInScope(_, _, this) } } - private predicate hasChild(AstNode parent, AstNode child) { - child = getAChild(parent) and - not parent instanceof LocalScope - } - - private predicate isCandOrScope(AstNode n) { - n instanceof LocalAccessCand or - n instanceof LocalScope - } - - private predicate isScopeChild(AstNode n) { exists(LocalScope scope | n = getAChild(scope)) } - - private predicate hasEnclosingScope(AstNode scopeChild, AstNode n) = - doublyBoundedFastTC(hasChild/2, isScopeChild/1, isCandOrScope/1)(scopeChild, n) - /** Gets the immediately enclosing variable scope of `n`. */ private LocalScope getEnclosingScope(AstNode n) { - exists(AstNode scopeChild | scopeChild = getAChild(result) | - n = scopeChild and - isCandOrScope(n) - or - hasEnclosingScope(scopeChild, n) + n = getAChild(result) + or + exists(AstNode mid | + result = getEnclosingScope(mid) and + n = getAChild(mid) and + not mid instanceof LocalScope ) } From 46e664327e159056fd5e7f2924cb2f51a1c08f3e Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 20 May 2026 21:21:24 +0200 Subject: [PATCH 8/8] cleanup --- .../elements/internal/LocalNameBinding.qll | 212 ++++++++++++------ .../rust/elements/internal/VariableImpl.qll | 10 +- 2 files changed, 151 insertions(+), 71 deletions(-) diff --git a/rust/ql/lib/codeql/rust/elements/internal/LocalNameBinding.qll b/rust/ql/lib/codeql/rust/elements/internal/LocalNameBinding.qll index d01a035b701f..19412c827e58 100644 --- a/rust/ql/lib/codeql/rust/elements/internal/LocalNameBinding.qll +++ b/rust/ql/lib/codeql/rust/elements/internal/LocalNameBinding.qll @@ -14,76 +14,120 @@ signature module LocalNameBindingInputSig { /** Gets the child of AST node `n` at the specified index. */ AstNode getChild(AstNode n, int index); + /** + * A conditional where any local declarations in the condition are in scope + * in the then-branch but not the else-branch. + * + * Example: + * + * ```rust + * if let Some(x) = opt { + * // x is in scope here + * } else { + * // x is not in scope here + * } + * ``` + */ class Conditional extends AstNode { + /** Gets the condition of this conditional. */ AstNode getCondition(); + /** Gets the then-branch of this conditional. */ AstNode getThen(); + /** Gets the else-branch of this conditional. */ AstNode getElse(); } - class Let extends AstNode { + /** + * A shadowing declaration where any local declarations in the left-hand side + * are in scope _after_ the declaration and shadow any declarations with the + * same name preceding it. + * + * Example: + * ```rust + * let x = 1; + * let x = x + 1; // this declaration of `x` shadows the previous one, but the `x` in the right-hand side still refers to the first declaration + * println!("{}", x); // this access of `x` refers to the second declaration + * ``` + */ + class ShadowingDecl extends AstNode { + /** Gets the left-hand side of this declaration. */ AstNode getLhs(); + /** + * Gets the right-hand side of this declaration. + * + * Any local declared in the left-hand side of this declaration is _not_ in scope + * in the right-hand side. + */ AstNode getRhs(); + /** + * Gets the else-branch of this declaration, if any. + * + * Any local declared in the left-hand side of this declaration is _not_ in scope + * in the else-branch. + */ AstNode getElse(); } /** * Holds if a local declaration named `name` exists at `definingNode` inside * the syntactic scope `scope`. + * + * Note that declarations with a `definingNode` in the left-hand side of a + * `ShadowingDecl` `decl` should use `scope = decl`. */ - predicate localDeclInScope(AstNode definingNode, string name, AstNode scope); + predicate declInScope(AstNode definingNode, string name, AstNode scope); - predicate localAccessCand(AstNode n, string name); + /** + * Holds if `n` is a node that may access a local named `name`. + */ + predicate accessCand(AstNode n, string name); + + /** + * Holds the access candidate `n` should begin its lookup in `scope` instead + * of its immediately enclosing scope. + * + * For example, the `this` variable in an instance field initializer might need to be resolved + * relative to a constructor body. + * + * If `scope` declares a variable with the name of `ref`, then `scope` is guaranteed to be the + * scope that `ref` ultimately resolves to. This can thus be used to take full control of scope resolution for + * for specific types of references. + */ + default predicate lookupStartsAt(AstNode n, AstNode scope) { none() } } module LocalNameBinding Input> { private import Input - /** A locally declared entity, for example a variable or a parameter. */ - final class Local extends MkLocal { - private AstNode definingNode; - private string name; - - Local() { this = MkLocal(definingNode, name) } - - /** Gets the AST node that defines this local. */ - AstNode getDefiningNode() { result = definingNode } - - /** Gets the name of this local entity. */ - string getName() { result = name } - - /** Gets the location of this local entity. */ - Location getLocation() { result = definingNode.getLocation() } - - /** Gets a textual representation of this local entity. */ - string toString() { result = this.getName() } - - /** Gets an access to this local. */ - LocalAccess getAnAccess() { result.getLocal() = this } - } - final private class AstNodeFinal = AstNode; - /** - * A path expression that may access a local variable. These are paths that - * only consist of a simple name (i.e., without generic arguments, - * qualifiers, etc.). - */ - private class LocalAccessCand extends AstNodeFinal { - string name_; - - LocalAccessCand() { localAccessCand(this, name_) } - - string getName() { result = name_ } + private class Scope extends AstNodeFinal { + Scope() { + declInScope(_, _, this) + or + lookupStartsAt(_, this) + } } private module ChildDenseRankInput implements DenseRankInputSig1 { - class C = AstNode; + // Restrict ranking to relevant nodes for better performance + private class RelevantAstNode extends AstNodeFinal { + RelevantAstNode() { + this instanceof Scope + or + getChild(this, _).(Scope) instanceof ShadowingDecl + or + this = getChild(any(RelevantAstNode parent), _) + } + } + + class C = RelevantAstNode; - class Ranked = AstNode; + class Ranked = RelevantAstNode; /** * An adjusted version of `ParentChild::getImmediateChild`, which makes the following @@ -99,7 +143,7 @@ module LocalNameBinding int getRank(C parent, Ranked child) { // parent.getEnclosingCallable().(Function).getName().getText() = "match_pattern14" and child = - rank[result](AstNode res, int preOrd, int i | + rank[result](RelevantAstNode res, int preOrd, int i | res = getChild(parent, i) and preOrd = 0 and not exists(Conditional cond | res = [cond.getThen(), cond.getElse()]) @@ -122,57 +166,93 @@ module LocalNameBinding private predicate getRankedChild = DenseRank1::denseRank/2; - private predicate isLetParent(AstNode parent, int i, Let let) { let = getRankedChild(parent, i) } + private predicate isLetParent(AstNode parent, int i, ShadowingDecl decl) { + decl = getRankedChild(parent, i) + } - private predicate shouldBeLetChild(AstNode parent, int i, Let let, AstNode child) { + private predicate shouldBeLetChild(AstNode parent, int i, ShadowingDecl decl, AstNode child) { child = getRankedChild(parent, i) and ( - isLetParent(parent, i - 1, let) + isLetParent(parent, i - 1, decl) or - shouldBeLetChild(parent, i - 1, let, any(AstNode n | not n instanceof Let)) + shouldBeLetChild(parent, i - 1, decl, any(AstNode n | not n instanceof ShadowingDecl)) ) } private AstNode getAChild(AstNode parent) { not shouldBeLetChild(_, _, _, result) and - not result = [any(Let let).getRhs(), any(Let let).getElse()] and + not result = [any(ShadowingDecl decl).getRhs(), any(ShadowingDecl decl).getElse()] and result = getRankedChild(parent, _) or shouldBeLetChild(_, _, parent, result) or - exists(Let let | let = getAChild(parent) | result = [let.getRhs(), let.getElse()]) - } - - private class LocalScope extends AstNodeFinal { - LocalScope() { localDeclInScope(_, _, this) } + exists(ShadowingDecl decl | decl = getAChild(parent) | result = [decl.getRhs(), decl.getElse()]) } /** Gets the immediately enclosing variable scope of `n`. */ - private LocalScope getEnclosingScope(AstNode n) { + private Scope getEnclosingScope(AstNode n) { n = getAChild(result) or exists(AstNode mid | result = getEnclosingScope(mid) and n = getAChild(mid) and - not mid instanceof LocalScope + not mid instanceof Scope ) } + private class AccessCand extends AstNodeFinal { + string name_; + + AccessCand() { accessCand(this, name_) } + + string getName() { result = name_ } + + Scope getLookupScope() { + lookupStartsAt(this, result) + or + not lookupStartsAt(this, _) and + result = getEnclosingScope(this) + } + } + pragma[nomagic] - private predicate lookupInScope(string name, LocalScope scope, LocalScope outer) { - exists(LocalAccessCand cand | + private predicate lookupInScope(string name, Scope lookup, Scope scope) { + exists(AccessCand cand | cand.getName() = name and - scope = getEnclosingScope(cand) and - outer = scope + lookup = cand.getLookupScope() and + scope = lookup ) or - exists(LocalScope mid | - lookupInScope(name, scope, mid) and - not localDeclInScope(_, name, mid) and - outer = getEnclosingScope(mid) + exists(Scope mid | + lookupInScope(name, lookup, mid) and + not declInScope(_, name, mid) and + scope = getEnclosingScope(mid) ) } + /** A locally declared entity, for example a variable or a parameter. */ + final class Local extends MkLocal { + private AstNode definingNode; + private string name; + + Local() { this = MkLocal(definingNode, name) } + + /** Gets the AST node that defines this local. */ + AstNode getDefiningNode() { result = definingNode } + + /** Gets the name of this local entity. */ + string getName() { result = name } + + /** Gets the location of this local entity. */ + Location getLocation() { result = definingNode.getLocation() } + + /** Gets a textual representation of this local entity. */ + string toString() { result = this.getName() } + + /** Gets an access to this local. */ + LocalAccess getAnAccess() { result.getLocal() = this } + } + /** A local access. */ final class LocalAccess extends AstNodeFinal { private string name; @@ -188,15 +268,15 @@ module LocalNameBinding private module Cached { cached newtype TLocal = - MkLocal(AstNode definingNode, string name) { localDeclInScope(definingNode, name, _) } + MkLocal(AstNode definingNode, string name) { declInScope(definingNode, name, _) } cached - predicate access(LocalAccessCand cand, string name, Local v) { - exists(AstNode definingNode, LocalScope scope, LocalScope outer | + predicate access(AccessCand cand, string name, Local v) { + exists(AstNode definingNode, Scope lookup, Scope scope | cand.getName() = name and - scope = getEnclosingScope(cand) and - lookupInScope(name, scope, outer) and - localDeclInScope(definingNode, name, outer) and + lookup = cand.getLookupScope() and + lookupInScope(name, lookup, scope) and + declInScope(definingNode, name, scope) and v = MkLocal(definingNode, name) ) } diff --git a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll index b5135c310483..bd36bc087c10 100644 --- a/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll +++ b/rust/ql/lib/codeql/rust/elements/internal/VariableImpl.qll @@ -48,8 +48,8 @@ module Impl { AstNode getElse() { result = this.(IfExpr).getElse() } } - class Let extends AstNode { - Let() { this instanceof LetStmt or this instanceof LetExpr } + class ShadowingDecl extends AstNode { + ShadowingDecl() { this instanceof LetStmt or this instanceof LetExpr } AstNode getLhs() { result = this.(LetStmt).getPat() @@ -66,7 +66,7 @@ module Impl { AstNode getElse() { result = this.(LetStmt).getLetElse() } } - predicate localDeclInScope(AstNode definingNode, string name, AstNode scope) { + predicate declInScope(AstNode definingNode, string name, AstNode scope) { variableDeclInScope(definingNode, name, scope) or definingNode = @@ -76,7 +76,7 @@ module Impl { ) } - predicate localAccessCand(AstNode n, string name) { name = n.(VariableAccessCand).getName() } + predicate accessCand(AstNode n, string name) { name = n.(VariableAccessCand).getName() } } private import LocalNameBinding @@ -160,7 +160,7 @@ module Impl { scope = arm ) or - exists(Input::Let let | + exists(Input::ShadowingDecl let | let.getLhs() = pat and scope = let )