Skip to content

Commit 10f5513

Browse files
hvitvedhvitved
authored
Merge pull request #18482 from hvitved/rust/nested-functions
Rust: Take nested functions into account when resolving variables
2 parents 4311553 + 1bbb3fd commit 10f5513

File tree

9 files changed

+1539
-1279
lines changed

9 files changed

+1539
-1279
lines changed

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

Lines changed: 109 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -397,20 +397,23 @@ module Impl {
397397
)
398398
}
399399

400-
private newtype TVariableOrAccessCand =
401-
TVariableOrAccessCandVariable(Variable v) or
402-
TVariableOrAccessCandVariableAccessCand(VariableAccessCand va)
400+
private newtype TDefOrAccessCand =
401+
TDefOrAccessCandNestedFunction(Function f, BlockExprScope scope) {
402+
f = scope.getStmtList().getAStatement()
403+
} or
404+
TDefOrAccessCandVariable(Variable v) or
405+
TDefOrAccessCandVariableAccessCand(VariableAccessCand va)
403406

404407
/**
405-
* A variable declaration or variable access candidate.
408+
* A nested function declaration, variable declaration, or variable (or function)
409+
* access candidate.
406410
*
407-
* In order to determine whether a candidate is an actual variable access,
408-
* we rank declarations and candidates by their position in source code.
411+
* In order to determine whether a candidate is an actual variable/function access,
412+
* we rank declarations and candidates by their position in the AST.
409413
*
410-
* The ranking must take variable names into account, but also variable scopes;
411-
* below a comment `rank(scope, name, i)` means that the declaration/access on
412-
* the given line has rank `i` amongst all declarations/accesses inside variable
413-
* scope `scope`, for variable name `name`:
414+
* The ranking must take names into account, but also variable scopes; below a comment
415+
* `rank(scope, name, i)` means that the declaration/access on the given line has rank
416+
* `i` amongst all declarations/accesses inside variable scope `scope`, for name `name`:
414417
*
415418
* ```rust
416419
* fn f() { // scope0
@@ -430,8 +433,8 @@ module Impl {
430433
* }
431434
* ```
432435
*
433-
* Variable declarations are only ranked in the scope that they bind into, while
434-
* accesses candidates propagate outwards through scopes, as they may access
436+
* Function/variable declarations are only ranked in the scope that they bind into,
437+
* while accesses candidates propagate outwards through scopes, as they may access
435438
* declarations from outer scopes.
436439
*
437440
* For an access candidate with ranks `{ rank(scope_i, name, rnk_i) | i in I }` and
@@ -448,41 +451,80 @@ module Impl {
448451
* i.e., its the nearest declaration before the access in the same (or outer) scope
449452
* as the access.
450453
*/
451-
private class VariableOrAccessCand extends TVariableOrAccessCand {
452-
Variable asVariable() { this = TVariableOrAccessCandVariable(result) }
454+
abstract private class DefOrAccessCand extends TDefOrAccessCand {
455+
abstract string toString();
453456

454-
VariableAccessCand asVariableAccessCand() {
455-
this = TVariableOrAccessCandVariableAccessCand(result)
456-
}
457+
abstract Location getLocation();
457458

458-
string toString() {
459-
result = this.asVariable().toString() or result = this.asVariableAccessCand().toString()
460-
}
459+
pragma[nomagic]
460+
abstract predicate rankBy(string name, VariableScope scope, int ord, int kind);
461+
}
461462

462-
Location getLocation() {
463-
result = this.asVariable().getLocation() or result = this.asVariableAccessCand().getLocation()
464-
}
463+
abstract private class NestedFunctionOrVariable extends DefOrAccessCand { }
465464

466-
pragma[nomagic]
467-
predicate rankBy(string name, VariableScope scope, int ord, int kind) {
468-
variableDeclInScope(this.asVariable(), scope, name, ord) and
465+
private class DefOrAccessCandNestedFunction extends NestedFunctionOrVariable,
466+
TDefOrAccessCandNestedFunction
467+
{
468+
private Function f;
469+
private BlockExprScope scope_;
470+
471+
DefOrAccessCandNestedFunction() { this = TDefOrAccessCandNestedFunction(f, scope_) }
472+
473+
override string toString() { result = f.toString() }
474+
475+
override Location getLocation() { result = f.getLocation() }
476+
477+
override predicate rankBy(string name, VariableScope scope, int ord, int kind) {
478+
// nested functions behave as if they are defined at the beginning of the scope
479+
name = f.getName().getText() and
480+
scope = scope_ and
481+
ord = 0 and
469482
kind = 0
470-
or
471-
variableAccessCandInScope(this.asVariableAccessCand(), scope, name, _, ord) and
483+
}
484+
}
485+
486+
private class DefOrAccessCandVariable extends NestedFunctionOrVariable, TDefOrAccessCandVariable {
487+
private Variable v;
488+
489+
DefOrAccessCandVariable() { this = TDefOrAccessCandVariable(v) }
490+
491+
override string toString() { result = v.toString() }
492+
493+
override Location getLocation() { result = v.getLocation() }
494+
495+
override predicate rankBy(string name, VariableScope scope, int ord, int kind) {
496+
variableDeclInScope(v, scope, name, ord) and
472497
kind = 1
473498
}
474499
}
475500

501+
private class DefOrAccessCandVariableAccessCand extends DefOrAccessCand,
502+
TDefOrAccessCandVariableAccessCand
503+
{
504+
private VariableAccessCand va;
505+
506+
DefOrAccessCandVariableAccessCand() { this = TDefOrAccessCandVariableAccessCand(va) }
507+
508+
override string toString() { result = va.toString() }
509+
510+
override Location getLocation() { result = va.getLocation() }
511+
512+
override predicate rankBy(string name, VariableScope scope, int ord, int kind) {
513+
variableAccessCandInScope(va, scope, name, _, ord) and
514+
kind = 2
515+
}
516+
}
517+
476518
private module DenseRankInput implements DenseRankInputSig2 {
477519
class C1 = VariableScope;
478520

479521
class C2 = string;
480522

481-
class Ranked = VariableOrAccessCand;
523+
class Ranked = DefOrAccessCand;
482524

483-
int getRank(VariableScope scope, string name, VariableOrAccessCand v) {
525+
int getRank(VariableScope scope, string name, DefOrAccessCand v) {
484526
v =
485-
rank[result](VariableOrAccessCand v0, int ord, int kind |
527+
rank[result](DefOrAccessCand v0, int ord, int kind |
486528
v0.rankBy(name, scope, ord, kind)
487529
|
488530
v0 order by ord, kind
@@ -494,7 +536,7 @@ module Impl {
494536
* Gets the rank of `v` amongst all other declarations or access candidates
495537
* to a variable named `name` in the variable scope `scope`.
496538
*/
497-
private int rankVariableOrAccess(VariableScope scope, string name, VariableOrAccessCand v) {
539+
private int rankVariableOrAccess(VariableScope scope, string name, DefOrAccessCand v) {
498540
v = DenseRank2<DenseRankInput>::denseRank(scope, name, result + 1)
499541
}
500542

@@ -512,25 +554,38 @@ module Impl {
512554
* the declaration at rank 0 can only reach the access at rank 1, while the declaration
513555
* at rank 2 can only reach the access at rank 3.
514556
*/
515-
private predicate variableReachesRank(VariableScope scope, string name, Variable v, int rnk) {
516-
rnk = rankVariableOrAccess(scope, name, TVariableOrAccessCandVariable(v))
557+
private predicate variableReachesRank(
558+
VariableScope scope, string name, NestedFunctionOrVariable v, int rnk
559+
) {
560+
rnk = rankVariableOrAccess(scope, name, v)
517561
or
518562
variableReachesRank(scope, name, v, rnk - 1) and
519-
rnk = rankVariableOrAccess(scope, name, TVariableOrAccessCandVariableAccessCand(_))
563+
rnk = rankVariableOrAccess(scope, name, TDefOrAccessCandVariableAccessCand(_))
520564
}
521565

522566
private predicate variableReachesCand(
523-
VariableScope scope, string name, Variable v, VariableAccessCand cand, int nestLevel
567+
VariableScope scope, string name, NestedFunctionOrVariable v, VariableAccessCand cand,
568+
int nestLevel
524569
) {
525570
exists(int rnk |
526571
variableReachesRank(scope, name, v, rnk) and
527-
rnk = rankVariableOrAccess(scope, name, TVariableOrAccessCandVariableAccessCand(cand)) and
572+
rnk = rankVariableOrAccess(scope, name, TDefOrAccessCandVariableAccessCand(cand)) and
528573
variableAccessCandInScope(cand, scope, name, nestLevel, _)
529574
)
530575
}
531576

577+
pragma[nomagic]
578+
predicate access(string name, NestedFunctionOrVariable v, VariableAccessCand cand) {
579+
v =
580+
min(NestedFunctionOrVariable v0, int nestLevel |
581+
variableReachesCand(_, name, v0, cand, nestLevel)
582+
|
583+
v0 order by nestLevel
584+
)
585+
}
586+
532587
/** A variable access. */
533-
class VariableAccess extends PathExprBaseImpl::PathExprBase instanceof VariableAccessCand {
588+
class VariableAccess extends PathExprBaseImpl::PathExprBase {
534589
private string name;
535590
private Variable v;
536591

@@ -574,6 +629,16 @@ module Impl {
574629
}
575630
}
576631

632+
/** A nested function access. */
633+
class NestedFunctionAccess extends PathExprBaseImpl::PathExprBase {
634+
private Function f;
635+
636+
NestedFunctionAccess() { nestedFunctionAccess(_, f, this) }
637+
638+
/** Gets the function being accessed. */
639+
Function getFunction() { result = f }
640+
}
641+
577642
cached
578643
private module Cached {
579644
cached
@@ -582,12 +647,12 @@ module Impl {
582647

583648
cached
584649
predicate variableAccess(string name, Variable v, VariableAccessCand cand) {
585-
v =
586-
min(Variable v0, int nestLevel |
587-
variableReachesCand(_, name, v0, cand, nestLevel)
588-
|
589-
v0 order by nestLevel
590-
)
650+
access(name, TDefOrAccessCandVariable(v), cand)
651+
}
652+
653+
cached
654+
predicate nestedFunctionAccess(string name, Function f, VariableAccessCand cand) {
655+
access(name, TDefOrAccessCandNestedFunction(f, _), cand)
591656
}
592657
}
593658

‎rust/ql/test/library-tests/dataflow/global/inline-flow.expected‎

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,28 @@ edges
2020
| main.rs:41:26:44:5 | { ... } | main.rs:30:17:30:22 | ...: i64 | provenance | |
2121
| main.rs:41:26:44:5 | { ... } | main.rs:41:13:44:6 | pass_through(...) | provenance | |
2222
| main.rs:43:9:43:18 | source(...) | main.rs:41:26:44:5 | { ... } | provenance | |
23-
| main.rs:56:23:56:28 | ...: i64 | main.rs:57:14:57:14 | n | provenance | |
24-
| main.rs:59:31:65:5 | { ... } | main.rs:77:13:77:25 | mn.get_data(...) | provenance | |
25-
| main.rs:63:13:63:21 | source(...) | main.rs:59:31:65:5 | { ... } | provenance | |
26-
| main.rs:66:28:66:33 | ...: i64 | main.rs:66:43:72:5 | { ... } | provenance | |
27-
| main.rs:77:9:77:9 | a | main.rs:78:10:78:10 | a | provenance | |
28-
| main.rs:77:13:77:25 | mn.get_data(...) | main.rs:77:9:77:9 | a | provenance | |
29-
| main.rs:83:9:83:9 | a | main.rs:84:16:84:16 | a | provenance | |
30-
| main.rs:83:13:83:21 | source(...) | main.rs:83:9:83:9 | a | provenance | |
31-
| main.rs:84:16:84:16 | a | main.rs:56:23:56:28 | ...: i64 | provenance | |
32-
| main.rs:89:9:89:9 | a | main.rs:90:29:90:29 | a | provenance | |
33-
| main.rs:89:13:89:21 | source(...) | main.rs:89:9:89:9 | a | provenance | |
34-
| main.rs:90:9:90:9 | b | main.rs:91:10:91:10 | b | provenance | |
35-
| main.rs:90:13:90:30 | mn.data_through(...) | main.rs:90:9:90:9 | b | provenance | |
36-
| main.rs:90:29:90:29 | a | main.rs:66:28:66:33 | ...: i64 | provenance | |
37-
| main.rs:90:29:90:29 | a | main.rs:90:13:90:30 | mn.data_through(...) | provenance | |
23+
| main.rs:49:9:49:9 | a | main.rs:55:26:55:26 | a | provenance | |
24+
| main.rs:49:13:49:22 | source(...) | main.rs:49:9:49:9 | a | provenance | |
25+
| main.rs:51:21:51:26 | ...: i64 | main.rs:51:36:53:5 | { ... } | provenance | |
26+
| main.rs:55:9:55:9 | b | main.rs:56:10:56:10 | b | provenance | |
27+
| main.rs:55:13:55:27 | pass_through(...) | main.rs:55:9:55:9 | b | provenance | |
28+
| main.rs:55:26:55:26 | a | main.rs:51:21:51:26 | ...: i64 | provenance | |
29+
| main.rs:55:26:55:26 | a | main.rs:55:13:55:27 | pass_through(...) | provenance | |
30+
| main.rs:67:23:67:28 | ...: i64 | main.rs:68:14:68:14 | n | provenance | |
31+
| main.rs:70:31:76:5 | { ... } | main.rs:88:13:88:25 | mn.get_data(...) | provenance | |
32+
| main.rs:74:13:74:21 | source(...) | main.rs:70:31:76:5 | { ... } | provenance | |
33+
| main.rs:77:28:77:33 | ...: i64 | main.rs:77:43:83:5 | { ... } | provenance | |
34+
| main.rs:88:9:88:9 | a | main.rs:89:10:89:10 | a | provenance | |
35+
| main.rs:88:13:88:25 | mn.get_data(...) | main.rs:88:9:88:9 | a | provenance | |
36+
| main.rs:94:9:94:9 | a | main.rs:95:16:95:16 | a | provenance | |
37+
| main.rs:94:13:94:21 | source(...) | main.rs:94:9:94:9 | a | provenance | |
38+
| main.rs:95:16:95:16 | a | main.rs:67:23:67:28 | ...: i64 | provenance | |
39+
| main.rs:100:9:100:9 | a | main.rs:101:29:101:29 | a | provenance | |
40+
| main.rs:100:13:100:21 | source(...) | main.rs:100:9:100:9 | a | provenance | |
41+
| main.rs:101:9:101:9 | b | main.rs:102:10:102:10 | b | provenance | |
42+
| main.rs:101:13:101:30 | mn.data_through(...) | main.rs:101:9:101:9 | b | provenance | |
43+
| main.rs:101:29:101:29 | a | main.rs:77:28:77:33 | ...: i64 | provenance | |
44+
| main.rs:101:29:101:29 | a | main.rs:101:13:101:30 | mn.data_through(...) | provenance | |
3845
nodes
3946
| main.rs:12:28:14:1 | { ... } | semmle.label | { ... } |
4047
| main.rs:13:5:13:13 | source(...) | semmle.label | source(...) |
@@ -59,34 +66,44 @@ nodes
5966
| main.rs:41:26:44:5 | { ... } | semmle.label | { ... } |
6067
| main.rs:43:9:43:18 | source(...) | semmle.label | source(...) |
6168
| main.rs:45:10:45:10 | a | semmle.label | a |
62-
| main.rs:56:23:56:28 | ...: i64 | semmle.label | ...: i64 |
63-
| main.rs:57:14:57:14 | n | semmle.label | n |
64-
| main.rs:59:31:65:5 | { ... } | semmle.label | { ... } |
65-
| main.rs:63:13:63:21 | source(...) | semmle.label | source(...) |
66-
| main.rs:66:28:66:33 | ...: i64 | semmle.label | ...: i64 |
67-
| main.rs:66:43:72:5 | { ... } | semmle.label | { ... } |
68-
| main.rs:77:9:77:9 | a | semmle.label | a |
69-
| main.rs:77:13:77:25 | mn.get_data(...) | semmle.label | mn.get_data(...) |
70-
| main.rs:78:10:78:10 | a | semmle.label | a |
71-
| main.rs:83:9:83:9 | a | semmle.label | a |
72-
| main.rs:83:13:83:21 | source(...) | semmle.label | source(...) |
73-
| main.rs:84:16:84:16 | a | semmle.label | a |
74-
| main.rs:89:9:89:9 | a | semmle.label | a |
75-
| main.rs:89:13:89:21 | source(...) | semmle.label | source(...) |
76-
| main.rs:90:9:90:9 | b | semmle.label | b |
77-
| main.rs:90:13:90:30 | mn.data_through(...) | semmle.label | mn.data_through(...) |
78-
| main.rs:90:29:90:29 | a | semmle.label | a |
79-
| main.rs:91:10:91:10 | b | semmle.label | b |
69+
| main.rs:49:9:49:9 | a | semmle.label | a |
70+
| main.rs:49:13:49:22 | source(...) | semmle.label | source(...) |
71+
| main.rs:51:21:51:26 | ...: i64 | semmle.label | ...: i64 |
72+
| main.rs:51:36:53:5 | { ... } | semmle.label | { ... } |
73+
| main.rs:55:9:55:9 | b | semmle.label | b |
74+
| main.rs:55:13:55:27 | pass_through(...) | semmle.label | pass_through(...) |
75+
| main.rs:55:26:55:26 | a | semmle.label | a |
76+
| main.rs:56:10:56:10 | b | semmle.label | b |
77+
| main.rs:67:23:67:28 | ...: i64 | semmle.label | ...: i64 |
78+
| main.rs:68:14:68:14 | n | semmle.label | n |
79+
| main.rs:70:31:76:5 | { ... } | semmle.label | { ... } |
80+
| main.rs:74:13:74:21 | source(...) | semmle.label | source(...) |
81+
| main.rs:77:28:77:33 | ...: i64 | semmle.label | ...: i64 |
82+
| main.rs:77:43:83:5 | { ... } | semmle.label | { ... } |
83+
| main.rs:88:9:88:9 | a | semmle.label | a |
84+
| main.rs:88:13:88:25 | mn.get_data(...) | semmle.label | mn.get_data(...) |
85+
| main.rs:89:10:89:10 | a | semmle.label | a |
86+
| main.rs:94:9:94:9 | a | semmle.label | a |
87+
| main.rs:94:13:94:21 | source(...) | semmle.label | source(...) |
88+
| main.rs:95:16:95:16 | a | semmle.label | a |
89+
| main.rs:100:9:100:9 | a | semmle.label | a |
90+
| main.rs:100:13:100:21 | source(...) | semmle.label | source(...) |
91+
| main.rs:101:9:101:9 | b | semmle.label | b |
92+
| main.rs:101:13:101:30 | mn.data_through(...) | semmle.label | mn.data_through(...) |
93+
| main.rs:101:29:101:29 | a | semmle.label | a |
94+
| main.rs:102:10:102:10 | b | semmle.label | b |
8095
subpaths
8196
| main.rs:36:26:36:26 | a | main.rs:30:17:30:22 | ...: i64 | main.rs:30:32:32:1 | { ... } | main.rs:36:13:36:27 | pass_through(...) |
8297
| main.rs:41:26:44:5 | { ... } | main.rs:30:17:30:22 | ...: i64 | main.rs:30:32:32:1 | { ... } | main.rs:41:13:44:6 | pass_through(...) |
83-
| main.rs:90:29:90:29 | a | main.rs:66:28:66:33 | ...: i64 | main.rs:66:43:72:5 | { ... } | main.rs:90:13:90:30 | mn.data_through(...) |
98+
| main.rs:55:26:55:26 | a | main.rs:51:21:51:26 | ...: i64 | main.rs:51:36:53:5 | { ... } | main.rs:55:13:55:27 | pass_through(...) |
99+
| main.rs:101:29:101:29 | a | main.rs:77:28:77:33 | ...: i64 | main.rs:77:43:83:5 | { ... } | main.rs:101:13:101:30 | mn.data_through(...) |
84100
testFailures
85101
#select
86102
| main.rs:18:10:18:10 | a | main.rs:13:5:13:13 | source(...) | main.rs:18:10:18:10 | a | $@ | main.rs:13:5:13:13 | source(...) | source(...) |
87103
| main.rs:22:10:22:10 | n | main.rs:26:13:26:21 | source(...) | main.rs:22:10:22:10 | n | $@ | main.rs:26:13:26:21 | source(...) | source(...) |
88104
| main.rs:37:10:37:10 | b | main.rs:35:13:35:21 | source(...) | main.rs:37:10:37:10 | b | $@ | main.rs:35:13:35:21 | source(...) | source(...) |
89105
| main.rs:45:10:45:10 | a | main.rs:43:9:43:18 | source(...) | main.rs:45:10:45:10 | a | $@ | main.rs:43:9:43:18 | source(...) | source(...) |
90-
| main.rs:57:14:57:14 | n | main.rs:83:13:83:21 | source(...) | main.rs:57:14:57:14 | n | $@ | main.rs:83:13:83:21 | source(...) | source(...) |
91-
| main.rs:78:10:78:10 | a | main.rs:63:13:63:21 | source(...) | main.rs:78:10:78:10 | a | $@ | main.rs:63:13:63:21 | source(...) | source(...) |
92-
| main.rs:91:10:91:10 | b | main.rs:89:13:89:21 | source(...) | main.rs:91:10:91:10 | b | $@ | main.rs:89:13:89:21 | source(...) | source(...) |
106+
| main.rs:56:10:56:10 | b | main.rs:49:13:49:22 | source(...) | main.rs:56:10:56:10 | b | $@ | main.rs:49:13:49:22 | source(...) | source(...) |
107+
| main.rs:68:14:68:14 | n | main.rs:94:13:94:21 | source(...) | main.rs:68:14:68:14 | n | $@ | main.rs:94:13:94:21 | source(...) | source(...) |
108+
| main.rs:89:10:89:10 | a | main.rs:74:13:74:21 | source(...) | main.rs:89:10:89:10 | a | $@ | main.rs:74:13:74:21 | source(...) | source(...) |
109+
| main.rs:102:10:102:10 | b | main.rs:100:13:100:21 | source(...) | main.rs:102:10:102:10 | b | $@ | main.rs:100:13:100:21 | source(...) | source(...) |

‎rust/ql/test/library-tests/dataflow/global/main.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,17 @@ fn block_expression_as_argument() {
4545
sink(a); // $ hasValueFlow=14
4646
}
4747

48+
fn data_through_nested_function() {
49+
let a = source(15);
50+
51+
fn pass_through(i: i64) -> i64 {
52+
i
53+
}
54+
55+
let b = pass_through(a);
56+
sink(b); // $ hasValueFlow=15
57+
}
58+
4859
// -----------------------------------------------------------------------------
4960
// Data flow in, out, and through method.
5061

@@ -127,6 +138,7 @@ fn main() {
127138
data_out_of_call();
128139
data_in_to_call();
129140
data_through_call();
141+
data_through_nested_function();
130142

131143
data_out_of_method();
132144
data_in_to_method_call();

0 commit comments

Comments
 (0)