Skip to content

Commit d068588

Browse files
hvitvedhvitved
committed
Rust: Remove borrow logic from Call classes
1 parent 2a9e97b commit d068588

7 files changed

Lines changed: 61 additions & 81 deletions

File tree

‎rust/ql/lib/codeql/rust/dataflow/internal/DataFlowConsistency.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ private module Input implements InputSig<Location, RustDataFlow> {
2121
or
2222
// We allow flow into post-update node for receiver expressions (from the
2323
// synthetic post receiever node).
24-
n.(Node::PostUpdateNode).getPreUpdateNode().asExpr() = any(Node::ReceiverNode r).getReceiver()
24+
n.(Node::PostUpdateNode).getPreUpdateNode().asExpr() = any(Node::ArgBorrowNode r).getArg()
2525
or
2626
n.(Node::PostUpdateNode).getPreUpdateNode().asExpr() = getPostUpdateReverseStep(_, _)
2727
or

‎rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll‎

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ final class ArgumentPosition extends TArgumentPosition {
157157
inMethodCall = true
158158
or
159159
result = call.(IndexExpr).getIndex() and
160-
pos = 1 and
160+
pos = 0 and
161161
inMethodCall = true
162162
)
163163
}
@@ -201,8 +201,7 @@ final class ArgumentPosition extends TArgumentPosition {
201201
predicate isArgumentForCall(Expr arg, Call call, ArgumentPosition pos) {
202202
// TODO: Handle index expressions as calls in data flow.
203203
not call instanceof IndexExpr and
204-
arg = pos.getArgument(call) and
205-
not (pos.isReceiver() and call.receiverImplicitlyBorrowed())
204+
arg = pos.getArgument(call)
206205
}
207206

208207
/** Provides logic related to SSA. */
@@ -333,14 +332,6 @@ module LocalFlow {
333332
or
334333
nodeFrom.asPat().(OrPat).getAPat() = nodeTo.asPat()
335334
or
336-
// Simple value step from receiver expression to receiver node, in case
337-
// there is no implicit deref or borrow operation.
338-
nodeFrom.asExpr() = nodeTo.(ReceiverNode).getReceiver()
339-
or
340-
// The dual step of the above, for the post-update nodes.
341-
nodeFrom.(PostUpdateNode).getPreUpdateNode().(ReceiverNode).getReceiver() =
342-
nodeTo.(PostUpdateNode).getPreUpdateNode().asExpr()
343-
or
344335
nodeTo.(PostUpdateNode).getPreUpdateNode().asExpr() =
345336
getPostUpdateReverseStep(nodeFrom.(PostUpdateNode).getPreUpdateNode().asExpr(), true)
346337
}
@@ -430,7 +421,7 @@ module RustDataFlow implements InputSig<Location> {
430421
node.(FlowSummaryNode).getSummaryNode().isHidden() or
431422
node instanceof CaptureNode or
432423
node instanceof ClosureParameterNode or
433-
node instanceof ReceiverNode or
424+
node instanceof ArgBorrowNode or
434425
node.asExpr() instanceof ParenExpr or
435426
nodeIsHidden(node.(PostUpdateNode).getPreUpdateNode())
436427
}
@@ -584,16 +575,16 @@ module RustDataFlow implements InputSig<Location> {
584575
}
585576

586577
pragma[nomagic]
587-
private predicate implicitDerefToReceiver(Node node1, ReceiverNode node2, ReferenceContent c) {
578+
private predicate implicitDerefToReceiver(Node node1, ArgBorrowNode node2, ReferenceContent c) {
588579
TypeInference::receiverHasImplicitDeref(node1.asExpr()) and
589-
node1.asExpr() = node2.getReceiver() and
580+
node1.asExpr() = node2.getArg() and
590581
exists(c)
591582
}
592583

593584
pragma[nomagic]
594-
private predicate implicitBorrowToReceiver(Node node1, ReceiverNode node2, ReferenceContent c) {
595-
TypeInference::receiverHasImplicitBorrow(node1.asExpr()) and
596-
node1.asExpr() = node2.getReceiver() and
585+
private predicate implicitBorrowToReceiver(Node node1, ArgBorrowNode node2, ReferenceContent c) {
586+
TypeInference::argumentHasImplicitBorrow(node1.asExpr()) and
587+
node1.asExpr() = node2.getArg() and
597588
exists(c)
598589
}
599590

‎rust/ql/lib/codeql/rust/dataflow/internal/FlowSummaryImpl.qll‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ module Input implements InputSig<Location, RustDataFlow> {
2727
}
2828

2929
Expr getArgument(ParameterPosition pos) {
30-
exists(ParameterPosition pos0, ArgumentPosition apos |
31-
result = apos.getArgument(this.getCall()) and
30+
exists(ParameterPosition pos0, ArgumentPosition apos, CallExprBase call |
31+
call = this.getCall() and
32+
result = apos.getArgument(call) and
3233
RustDataFlow::parameterMatch(pos0, apos)
3334
|
3435
not pos.hasPosition(_) and

‎rust/ql/lib/codeql/rust/dataflow/internal/Node.qll‎

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ private import codeql.rust.controlflow.ControlFlowGraph
1414
private import codeql.rust.controlflow.CfgNodes
1515
private import codeql.rust.dataflow.Ssa
1616
private import codeql.rust.dataflow.FlowSummary
17+
private import codeql.rust.internal.TypeInference as TypeInference
1718
private import Node as Node
1819
private import DataFlowImpl
1920
private import FlowSummaryImpl as FlowSummaryImpl
@@ -238,35 +239,37 @@ final class ExprArgumentNode extends ArgumentNode, ExprNode {
238239
private Call call_;
239240
private RustDataFlow::ArgumentPosition pos_;
240241

241-
ExprArgumentNode() { isArgumentForCall(n, call_, pos_) }
242+
ExprArgumentNode() {
243+
isArgumentForCall(n, call_, pos_) and
244+
not TypeInference::argumentHasImplicitBorrow(n) and
245+
not TypeInference::receiverHasImplicitDeref(n)
246+
}
242247

243248
override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
244249
call.asCall() = call_ and pos = pos_
245250
}
246251
}
247252

248253
/**
249-
* The receiver of a method call _after_ any implicit borrow or dereferencing
254+
* The argument of a call _after_ any implicit borrow or dereferencing
250255
* has taken place.
251256
*/
252-
final class ReceiverNode extends ArgumentNode, TReceiverNode {
253-
private Call n;
254-
255-
ReceiverNode() { this = TReceiverNode(n, false) }
257+
final class ArgBorrowNode extends ArgumentNode, TArgBorrowNode {
258+
private Expr arg;
256259

257-
Expr getReceiver() { result = n.getReceiver() }
260+
ArgBorrowNode() { this = TArgBorrowNode(arg, false) }
258261

259-
MethodCallExpr getMethodCall() { result = n }
262+
Expr getArg() { result = arg }
260263

261264
override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
262-
call.asCall() = n and pos = TReceiverArgumentPosition()
265+
isArgumentForCall(arg, call.asCall(), pos)
263266
}
264267

265-
override CfgScope getCfgScope() { result = n.getEnclosingCfgScope() }
268+
override CfgScope getCfgScope() { result = arg.getEnclosingCfgScope() }
266269

267-
override Location getLocation() { result = this.getReceiver().getLocation() }
270+
override Location getLocation() { result = arg.getLocation() }
268271

269-
override string toString() { result = "receiver for " + this.getReceiver() }
272+
override string toString() { result = "receiver for " + arg }
270273
}
271274

272275
final class SummaryArgumentNode extends FlowSummaryNode, ArgumentNode {
@@ -414,16 +417,16 @@ final class ExprPostUpdateNode extends PostUpdateNode, TExprPostUpdateNode {
414417
override Location getLocation() { result = e.getLocation() }
415418
}
416419

417-
final class ReceiverPostUpdateNode extends PostUpdateNode, TReceiverNode {
418-
private Call call;
420+
final class ArgBorrowPostUpdateNode extends PostUpdateNode, TArgBorrowNode {
421+
private Expr arg;
419422

420-
ReceiverPostUpdateNode() { this = TReceiverNode(call, true) }
423+
ArgBorrowPostUpdateNode() { this = TArgBorrowNode(arg, true) }
421424

422-
override Node getPreUpdateNode() { result = TReceiverNode(call, false) }
425+
override Node getPreUpdateNode() { result = TArgBorrowNode(arg, false) }
423426

424-
override CfgScope getCfgScope() { result = call.getEnclosingCfgScope() }
427+
override CfgScope getCfgScope() { result = arg.getEnclosingCfgScope() }
425428

426-
override Location getLocation() { result = call.getReceiver().getLocation() }
429+
override Location getLocation() { result = arg.getLocation() }
427430
}
428431

429432
final class SummaryPostUpdateNode extends FlowSummaryNode, PostUpdateNode {
@@ -486,11 +489,13 @@ newtype TNode =
486489
]
487490
)
488491
} or
489-
TReceiverNode(Call call, Boolean isPost) {
490-
call.hasEnclosingCfgScope() and
491-
call.receiverImplicitlyBorrowed() and
492+
TArgBorrowNode(Expr arg, Boolean isPost) {
492493
// TODO: Handle index expressions as calls in data flow.
493-
not call instanceof IndexExpr
494+
not arg = any(IndexExpr ie).getBase() and
495+
(
496+
TypeInference::argumentHasImplicitBorrow(arg) or
497+
TypeInference::receiverHasImplicitDeref(arg)
498+
)
494499
} or
495500
TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) or
496501
TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn) or

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

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,6 @@ module Impl {
3434
* called in Rust.
3535
*/
3636
abstract class Call extends ExprImpl::Expr {
37-
/** Holds if the receiver of this call is implicitly borrowed. */
38-
predicate receiverImplicitlyBorrowed() { this.implicitBorrowAt(TSelfArgumentPosition(), _) }
39-
4037
/** Gets the trait targeted by this call, if any. */
4138
abstract Trait getTrait();
4239

@@ -49,9 +46,6 @@ module Impl {
4946
/** Gets the argument at the given position, if any. */
5047
abstract Expr getArgument(ArgumentPosition pos);
5148

52-
/** Holds if the argument at `pos` might be implicitly borrowed. */
53-
abstract predicate implicitBorrowAt(ArgumentPosition pos, boolean certain);
54-
5549
/** Gets the number of arguments _excluding_ any `self` argument. */
5650
int getNumberOfArguments() { result = count(this.getArgument(TPositionalArgumentPosition(_))) }
5751

@@ -115,8 +109,6 @@ module Impl {
115109

116110
override Trait getTrait() { callHasTraitQualifier(this, result) }
117111

118-
override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) { none() }
119-
120112
override Expr getArgument(ArgumentPosition pos) {
121113
result = super.getArgList().getArg(pos.asPosition())
122114
}
@@ -139,8 +131,6 @@ module Impl {
139131

140132
override Trait getTrait() { callHasTraitQualifier(this, result) }
141133

142-
override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) { none() }
143-
144134
override Expr getArgument(ArgumentPosition pos) {
145135
pos.isSelf() and result = super.getArgList().getArg(0)
146136
or
@@ -153,10 +143,6 @@ module Impl {
153143

154144
override Trait getTrait() { none() }
155145

156-
override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) {
157-
pos.isSelf() and certain = false
158-
}
159-
160146
override Expr getArgument(ArgumentPosition pos) {
161147
pos.isSelf() and result = this.(MethodCallExpr).getReceiver()
162148
or
@@ -175,15 +161,6 @@ module Impl {
175161

176162
override Trait getTrait() { result = trait }
177163

178-
override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) {
179-
(
180-
pos.isSelf() and borrows >= 1
181-
or
182-
pos.asPosition() = 0 and borrows = 2
183-
) and
184-
certain = true
185-
}
186-
187164
override Expr getArgument(ArgumentPosition pos) {
188165
pos.isSelf() and result = super.getOperand(0)
189166
or
@@ -196,10 +173,6 @@ module Impl {
196173

197174
override Trait getTrait() { result.getCanonicalPath() = "core::ops::index::Index" }
198175

199-
override predicate implicitBorrowAt(ArgumentPosition pos, boolean certain) {
200-
pos.isSelf() and certain = true
201-
}
202-
203176
override Expr getArgument(ArgumentPosition pos) {
204177
pos.isSelf() and result = super.getBase()
205178
or

‎rust/ql/lib/codeql/rust/internal/TypeInference.qll‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1590,9 +1590,9 @@ private module MethodResolution {
15901590
receiver = this.getArgument(CallImpl::TSelfArgumentPosition())
15911591
}
15921592

1593-
predicate receiverHasImplicitBorrow(AstNode receiver) {
1593+
predicate argumentHasImplicitBorrow(AstNode arg) {
15941594
exists(this.resolveCallTarget(_, "", true)) and
1595-
receiver = this.getArgument(CallImpl::TSelfArgumentPosition())
1595+
arg = this.getArgument(CallImpl::TSelfArgumentPosition())
15961596
}
15971597
}
15981598

@@ -1683,8 +1683,16 @@ private module MethodResolution {
16831683

16841684
override Expr getArgument(ArgumentPosition pos) { result = this.(Call).getArgument(pos) }
16851685

1686+
private predicate implicitBorrowAt(ArgumentPosition pos) {
1687+
exists(int borrows | this.isOverloaded(_, _, borrows) |
1688+
pos.isSelf() and borrows >= 1
1689+
or
1690+
pos.asPosition() = 0 and borrows = 2
1691+
)
1692+
}
1693+
16861694
override Type getArgumentTypeAt(ArgumentPosition pos, TypePath path) {
1687-
if this.(Call).implicitBorrowAt(pos, true)
1695+
if this.implicitBorrowAt(pos)
16881696
then
16891697
result instanceof RefType and
16901698
path.isEmpty()
@@ -1696,10 +1704,10 @@ private module MethodResolution {
16961704
else result = inferType(this.getArgument(pos), path)
16971705
}
16981706

1699-
override predicate receiverHasImplicitBorrow(AstNode receiver) {
1707+
override predicate argumentHasImplicitBorrow(AstNode arg) {
17001708
exists(ArgumentPosition pos |
1701-
this.(Call).implicitBorrowAt(pos, true) and
1702-
receiver = this.getArgument(pos)
1709+
this.implicitBorrowAt(pos) and
1710+
arg = this.getArgument(pos)
17031711
)
17041712
}
17051713

@@ -3415,10 +3423,10 @@ private module Cached {
34153423
any(MethodResolution::MethodCall mc).receiverHasImplicitDeref(receiver)
34163424
}
34173425

3418-
/** Holds if `receiver` is the receiver of a method call with an implicit borrow. */
3426+
/** Holds if `arg` is the argument of a call with an implicit borrow. */
34193427
cached
3420-
predicate receiverHasImplicitBorrow(AstNode receiver) {
3421-
any(MethodResolution::MethodCall mc).receiverHasImplicitBorrow(receiver)
3428+
predicate argumentHasImplicitBorrow(AstNode arg) {
3429+
any(MethodResolution::MethodCall mc).argumentHasImplicitBorrow(arg)
34223430
}
34233431

34243432
/** Gets an item (function or tuple struct/variant) that `call` resolves to, if any. */

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,12 @@ edges
130130
| main.rs:244:13:244:13 | a [MyInt] | main.rs:244:13:244:17 | ... + ... [MyInt] | provenance | |
131131
| main.rs:244:13:244:17 | ... + ... [MyInt] | main.rs:244:9:244:9 | c [MyInt] | provenance | |
132132
| main.rs:245:10:245:10 | c [MyInt] | main.rs:245:10:245:16 | c.value | provenance | |
133-
| main.rs:252:9:252:9 | a [MyInt] | main.rs:220:12:220:15 | SelfParam [MyInt] | provenance | |
134-
| main.rs:252:9:252:9 | a [MyInt] | main.rs:254:13:254:20 | a.add(...) [MyInt] | provenance | |
133+
| main.rs:252:9:252:9 | a [MyInt] | main.rs:254:13:254:13 | a [MyInt] | provenance | |
135134
| main.rs:252:13:252:38 | MyInt {...} [MyInt] | main.rs:252:9:252:9 | a [MyInt] | provenance | |
136135
| main.rs:252:28:252:36 | source(...) | main.rs:252:13:252:38 | MyInt {...} [MyInt] | provenance | |
137136
| main.rs:254:9:254:9 | d [MyInt] | main.rs:255:10:255:10 | d [MyInt] | provenance | |
137+
| main.rs:254:13:254:13 | a [MyInt] | main.rs:220:12:220:15 | SelfParam [MyInt] | provenance | |
138+
| main.rs:254:13:254:13 | a [MyInt] | main.rs:254:13:254:20 | a.add(...) [MyInt] | provenance | |
138139
| main.rs:254:13:254:20 | a.add(...) [MyInt] | main.rs:254:9:254:9 | d [MyInt] | provenance | |
139140
| main.rs:255:10:255:10 | d [MyInt] | main.rs:255:10:255:16 | d.value | provenance | |
140141
| main.rs:259:9:259:9 | b [MyInt] | main.rs:261:35:261:35 | b [MyInt] | provenance | |
@@ -329,6 +330,7 @@ nodes
329330
| main.rs:252:13:252:38 | MyInt {...} [MyInt] | semmle.label | MyInt {...} [MyInt] |
330331
| main.rs:252:28:252:36 | source(...) | semmle.label | source(...) |
331332
| main.rs:254:9:254:9 | d [MyInt] | semmle.label | d [MyInt] |
333+
| main.rs:254:13:254:13 | a [MyInt] | semmle.label | a [MyInt] |
332334
| main.rs:254:13:254:20 | a.add(...) [MyInt] | semmle.label | a.add(...) [MyInt] |
333335
| main.rs:255:10:255:10 | d [MyInt] | semmle.label | d [MyInt] |
334336
| main.rs:255:10:255:16 | d.value | semmle.label | d.value |
@@ -394,7 +396,7 @@ subpaths
394396
| main.rs:194:38:194:38 | a | main.rs:112:27:112:32 | ...: i64 | main.rs:112:42:114:5 | { ... } | main.rs:194:13:194:39 | ...::data_through(...) |
395397
| main.rs:212:24:212:33 | source(...) | main.rs:206:12:206:17 | ...: i64 | main.rs:206:28:208:5 | { ... } [MyInt] | main.rs:212:13:212:34 | ...::new(...) [MyInt] |
396398
| main.rs:244:13:244:13 | a [MyInt] | main.rs:220:12:220:15 | SelfParam [MyInt] | main.rs:220:42:223:5 | { ... } [MyInt] | main.rs:244:13:244:17 | ... + ... [MyInt] |
397-
| main.rs:252:9:252:9 | a [MyInt] | main.rs:220:12:220:15 | SelfParam [MyInt] | main.rs:220:42:223:5 | { ... } [MyInt] | main.rs:254:13:254:20 | a.add(...) [MyInt] |
399+
| main.rs:254:13:254:13 | a [MyInt] | main.rs:220:12:220:15 | SelfParam [MyInt] | main.rs:220:42:223:5 | { ... } [MyInt] | main.rs:254:13:254:20 | a.add(...) [MyInt] |
398400
| main.rs:261:35:261:35 | b [MyInt] | main.rs:227:30:227:39 | ...: MyInt [MyInt] | main.rs:227:19:227:27 | SelfParam [Return] [&ref, MyInt] | main.rs:261:27:261:32 | [post] &mut a [&ref, MyInt] |
399401
| main.rs:272:27:272:28 | &a [&ref, MyInt] | main.rs:235:14:235:18 | SelfParam [&ref, MyInt] | main.rs:235:38:237:5 | { ... } [&ref] | main.rs:272:14:272:29 | ...::deref(...) [&ref] |
400402
| main.rs:301:50:301:50 | a [MyInt] | main.rs:289:18:289:21 | SelfParam [MyInt] | main.rs:289:48:291:5 | { ... } [MyInt] | main.rs:301:30:301:54 | ...::take_self(...) [MyInt] |

0 commit comments

Comments
 (0)