Skip to content

Commit afb22cb

Browse files
hvitvedhvitved
committed
Rust: Untangle argument/parameter data flow matching from call graph
1 parent b0dc48e commit afb22cb

File tree

3 files changed

+148
-41
lines changed

3 files changed

+148
-41
lines changed

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

Lines changed: 100 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -90,15 +90,12 @@ final class DataFlowCall extends TDataFlowCall {
9090

9191
/**
9292
* The position of a parameter in a function.
93-
*
94-
* In Rust there is a 1-to-1 correspondence between parameter positions and
95-
* arguments positions, so we use the same underlying type for both.
9693
*/
9794
final class ParameterPosition extends TParameterPosition {
9895
/** Gets the underlying integer position, if any. */
99-
int getPosition() { this = TPositionalParameterPosition(result) }
96+
int getPosition(boolean inMethod) { this = TPositionalParameterPosition(result, inMethod) }
10097

101-
predicate hasPosition() { exists(this.getPosition()) }
98+
predicate hasPosition(boolean inMethod) { exists(this.getPosition(inMethod)) }
10299

103100
/** Holds if this position represents the `self` position. */
104101
predicate isSelf() { this = TSelfParameterPosition() }
@@ -111,32 +108,88 @@ final class ParameterPosition extends TParameterPosition {
111108

112109
/** Gets a textual representation of this position. */
113110
string toString() {
114-
result = this.getPosition().toString()
111+
exists(boolean inMethod, string suffix |
112+
inMethod = true and suffix = " (method)"
113+
or
114+
inMethod = false and suffix = ""
115+
|
116+
result = this.getPosition(inMethod).toString() + suffix
117+
)
115118
or
116119
result = "self" and this.isSelf()
117120
or
118121
result = "closure self" and this.isClosureSelf()
119122
}
120123

121124
ParamBase getParameterIn(ParamList ps) {
122-
result = ps.getParam(this.getPosition())
125+
exists(boolean inMethod |
126+
result = ps.getParam(this.getPosition(inMethod)) and
127+
if ps.hasSelfParam() then inMethod = true else inMethod = false
128+
)
123129
or
124130
result = ps.getSelfParam() and this.isSelf()
125131
}
126132
}
127133

128134
/**
129135
* The position of an argument in a call.
130-
*
131-
* In Rust there is a 1-to-1 correspondence between parameter positions and
132-
* arguments positions, so we use the same underlying type for both.
133136
*/
134-
final class ArgumentPosition extends ParameterPosition {
137+
final class ArgumentPosition extends TArgumentPosition {
135138
/** Gets the argument of `call` at this position, if any. */
136139
Expr getArgument(Call call) {
137-
result = call.getPositionalArgument(this.getPosition())
140+
this.isReceiver() and
141+
(
142+
result = call.(MethodCallExpr).getReceiver()
143+
or
144+
result = call.(Operation).getOperand(0)
145+
or
146+
result = call.(IndexExpr).getBase()
147+
)
148+
or
149+
exists(boolean inMethodCall, int pos | this.getPosition(inMethodCall) = pos |
150+
result = call.(CallExpr).getArg(pos) and
151+
inMethodCall = false
152+
or
153+
result = call.(MethodCallExpr).getArg(pos) and
154+
inMethodCall = true
155+
or
156+
result = call.(Operation).getOperand(pos + 1) and
157+
pos >= 0 and
158+
inMethodCall = true
159+
or
160+
result = call.(IndexExpr).getIndex() and
161+
pos = 1 and
162+
inMethodCall = true
163+
)
164+
}
165+
166+
/** Gets the underlying integer position, if any. */
167+
int getPosition(boolean inMethodCall) { this = TPositionalArgumentPosition(result, inMethodCall) }
168+
169+
predicate hasPosition(boolean inMethodCall) { exists(this.getPosition(inMethodCall)) }
170+
171+
/** Holds if this position represents a receiver in a method call. */
172+
predicate isReceiver() { this = TReceiverArgumentPosition() }
173+
174+
/**
175+
* Holds if this position represents a reference to a closure itself. Only
176+
* used for tracking flow through captured variables.
177+
*/
178+
predicate isClosureReceiver() { this = TClosureReceiverArgumentPosition() }
179+
180+
/** Gets a textual representation of this position. */
181+
string toString() {
182+
exists(boolean inMethodCall, string suffix |
183+
inMethodCall = true and suffix = " (method call)"
184+
or
185+
inMethodCall = false and suffix = ""
186+
|
187+
result = this.getPosition(inMethodCall).toString() + suffix
188+
)
138189
or
139-
result = call.getReceiver() and this.isSelf()
190+
result = "receiver" and this.isReceiver()
191+
or
192+
result = "closure receiver" and this.isClosureReceiver()
140193
}
141194
}
142195

@@ -146,14 +199,11 @@ final class ArgumentPosition extends ParameterPosition {
146199
* Note that this does not hold for the receiever expression of a method call
147200
* as the synthetic `ReceiverNode` is the argument for the `self` parameter.
148201
*/
149-
predicate isArgumentForCall(ExprCfgNode arg, CallCfgNode call, ParameterPosition pos) {
202+
predicate isArgumentForCall(ExprCfgNode arg, CallCfgNode call, ArgumentPosition pos) {
150203
// TODO: Handle index expressions as calls in data flow.
151204
not call.getCall() instanceof IndexExpr and
152-
(
153-
call.getPositionalArgument(pos.getPosition()) = arg
154-
or
155-
call.getReceiver() = arg and pos.isSelf() and not call.getCall().receiverImplicitlyBorrowed()
156-
)
205+
arg.getExpr() = pos.getArgument(call.getCall()) and
206+
not (pos.isReceiver() and call.getCall().receiverImplicitlyBorrowed())
157207
}
158208

159209
/** Provides logic related to SSA. */
@@ -465,7 +515,21 @@ module RustDataFlow implements InputSig<Location> {
465515
* Holds if the parameter position `ppos` matches the argument position
466516
* `apos`.
467517
*/
468-
predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos = apos }
518+
pragma[nomagic]
519+
predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
520+
exists(boolean inMethod | ppos.getPosition(inMethod) = apos.getPosition(inMethod))
521+
or
522+
// `Vec::push(vec, value))`: match `value` against parameter after `self`
523+
ppos.getPosition(true) = apos.getPosition(false) - 1
524+
or
525+
// `vec.push(value)`: match `vec` against `self` parameter
526+
ppos.isSelf() and apos.isReceiver()
527+
or
528+
// `Vec::push(vec, value))`: match `vec` against `self` parameter
529+
ppos.isSelf() and apos.getPosition(false) = 0
530+
or
531+
ppos.isClosureSelf() and apos.isClosureReceiver()
532+
}
469533

470534
/**
471535
* Holds if there is a simple local flow step from `node1` to `node2`. These
@@ -712,7 +776,7 @@ module RustDataFlow implements InputSig<Location> {
712776
or
713777
// Store in function argument
714778
exists(DataFlowCall call, int i |
715-
isArgumentNode(node1, call, TPositionalParameterPosition(i)) and
779+
isArgumentNode(node1, call, TPositionalArgumentPosition(i, false)) and
716780
lambdaCall(call, _, node2.(PostUpdateNode).getPreUpdateNode()) and
717781
c.(FunctionCallArgumentContent).getPosition() = i
718782
)
@@ -1021,16 +1085,26 @@ private module Cached {
10211085

10221086
cached
10231087
newtype TParameterPosition =
1024-
TPositionalParameterPosition(int i) {
1025-
i in [0 .. max([any(ParamList l).getNumberOfParams(), any(ArgList l).getNumberOfArgs()]) - 1]
1088+
TPositionalParameterPosition(int i, Boolean inMethod) {
1089+
i in [0 .. any(ParamList l).getNumberOfParams() - 1]
10261090
or
1027-
FlowSummaryImpl::ParsePositions::isParsedArgumentPosition(_, i)
1028-
or
1029-
FlowSummaryImpl::ParsePositions::isParsedParameterPosition(_, i)
1091+
FlowSummaryImpl::ParsePositions::isParsedArgumentPosition(_, i) and
1092+
inMethod = false
10301093
} or
10311094
TClosureSelfParameterPosition() or
10321095
TSelfParameterPosition()
10331096

1097+
cached
1098+
newtype TArgumentPosition =
1099+
TPositionalArgumentPosition(int i, Boolean inMethodCall) {
1100+
i in [0 .. any(ArgList l).getNumberOfArgs() - 1]
1101+
or
1102+
FlowSummaryImpl::ParsePositions::isParsedParameterPosition(_, i) and
1103+
inMethodCall = false
1104+
} or
1105+
TClosureReceiverArgumentPosition() or
1106+
TReceiverArgumentPosition()
1107+
10341108
cached
10351109
newtype TReturnKind = TNormalReturnKind()
10361110

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

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,24 @@ module Input implements InputSig<Location, RustDataFlow> {
2626
final predicate callResolvesTo(string path) {
2727
path = this.getCall().getStaticTarget().(Addressable).getCanonicalPath()
2828
}
29+
30+
Expr getArgument(ParameterPosition pos) {
31+
exists(ParameterPosition pos0, ArgumentPosition apos |
32+
result = apos.getArgument(this.getCall()) and
33+
RustDataFlow::parameterMatch(pos0, apos)
34+
|
35+
not pos.hasPosition(_) and
36+
pos0 = pos
37+
or
38+
// We parse all positions as if they do not belong to a method; here we adjust if needed
39+
exists(boolean inMethod |
40+
pos0.getPosition(inMethod) = pos.getPosition(_) and
41+
if this.getCall().getStaticTarget().(Function).hasSelfParam()
42+
then inMethod = true
43+
else inMethod = false
44+
)
45+
)
46+
}
2947
}
3048

3149
abstract class SourceBase extends SourceSinkBase { }
@@ -48,14 +66,18 @@ module Input implements InputSig<Location, RustDataFlow> {
4866
override MethodCallExpr getCall() { result = call }
4967
}
5068

51-
RustDataFlow::ArgumentPosition callbackSelfParameterPosition() { result.isClosureSelf() }
69+
RustDataFlow::ArgumentPosition callbackSelfParameterPosition() { result.isClosureReceiver() }
5270

5371
ReturnKind getStandardReturnValueKind() { result = TNormalReturnKind() }
5472

55-
string encodeParameterPosition(ParameterPosition pos) { result = pos.toString() }
73+
string encodeParameterPosition(ParameterPosition pos) {
74+
result = pos.toString() and
75+
not pos.hasPosition(true)
76+
}
5677

5778
string encodeArgumentPosition(RustDataFlow::ArgumentPosition pos) {
58-
result = encodeParameterPosition(pos)
79+
result = pos.toString() and
80+
not pos.hasPosition(true)
5981
}
6082

6183
string encodeContent(ContentSet cs, string arg) {
@@ -112,14 +134,14 @@ module Input implements InputSig<Location, RustDataFlow> {
112134
ParameterPosition decodeUnknownParameterPosition(AccessPath::AccessPathTokenBase token) {
113135
// needed to support `Argument[x..y]` ranges
114136
token.getName() = "Argument" and
115-
result.getPosition() = AccessPath::parseInt(token.getAnArgument())
137+
result.getPosition(false) = AccessPath::parseInt(token.getAnArgument())
116138
}
117139

118140
bindingset[token]
119141
RustDataFlow::ArgumentPosition decodeUnknownArgumentPosition(AccessPath::AccessPathTokenBase token) {
120142
// needed to support `Parameter[x..y]` ranges
121143
token.getName() = "Parameter" and
122-
result.getPosition() = AccessPath::parseInt(token.getAnArgument())
144+
result.getPosition(false) = AccessPath::parseInt(token.getAnArgument())
123145
}
124146

125147
bindingset[token]
@@ -138,9 +160,9 @@ private module StepsInput implements Impl::Private::StepsInputSig {
138160

139161
/** Gets the argument of `source` described by `sc`, if any. */
140162
private Expr getSourceNodeArgument(Input::SourceBase source, Impl::Private::SummaryComponent sc) {
141-
exists(ArgumentPosition pos |
163+
exists(ParameterPosition pos |
142164
sc = Impl::Private::SummaryComponent::argument(pos) and
143-
result = pos.getArgument(source.getCall())
165+
result = source.getArgument(pos)
144166
)
145167
}
146168

@@ -169,19 +191,18 @@ private module StepsInput implements Impl::Private::StepsInputSig {
169191
exists(ArgumentPosition pos, Expr arg |
170192
s.head() = Impl::Private::SummaryComponent::parameter(pos) and
171193
arg = getSourceNodeArgument(source, s.tail().headOfSingleton()) and
172-
result.asParameter() = getCallable(arg).getParam(pos.getPosition())
194+
result.asParameter() = getCallable(arg).getParam(pos.getPosition(_))
173195
)
174196
or
175197
result.(RustDataFlow::PostUpdateNode).getPreUpdateNode().asExpr().getExpr() =
176198
getSourceNodeArgument(source, s.headOfSingleton())
177199
}
178200

179201
RustDataFlow::Node getSinkNode(Input::SinkBase sink, Impl::Private::SummaryComponent sc) {
180-
exists(CallExprBase call, Expr arg, ArgumentPosition pos |
202+
exists(Expr arg, ParameterPosition ppos |
181203
result.asExpr().getExpr() = arg and
182-
sc = Impl::Private::SummaryComponent::argument(pos) and
183-
call = sink.getCall() and
184-
arg = pos.getArgument(call)
204+
sc = Impl::Private::SummaryComponent::argument(ppos) and
205+
arg = sink.getArgument(ppos)
185206
)
186207
}
187208
}

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,19 @@ final class SummaryParameterNode extends ParameterNode, FlowSummaryNode {
190190
private ParameterPosition pos_;
191191

192192
SummaryParameterNode() {
193-
FlowSummaryImpl::Private::summaryParameterNode(this.getSummaryNode(), pos_)
193+
exists(ParameterPosition pos0 |
194+
FlowSummaryImpl::Private::summaryParameterNode(this.getSummaryNode(), pos0)
195+
|
196+
not pos0.hasPosition(_) and
197+
pos_ = pos0
198+
or
199+
// We parse all positions as if they do not belong to a method; here we adjust if needed
200+
exists(SummarizedCallable sc, boolean inMethod |
201+
sc = this.getSummarizedCallable() and
202+
pos_.getPosition(inMethod) = pos0.getPosition(_) and
203+
if sc.hasSelfParam() then inMethod = true else inMethod = false
204+
)
205+
)
194206
}
195207

196208
override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
@@ -247,7 +259,7 @@ final class ReceiverNode extends ArgumentNode, TReceiverNode {
247259
MethodCallExprCfgNode getMethodCall() { result = n }
248260

249261
override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
250-
call.asCallCfgNode() = n and pos = TSelfParameterPosition()
262+
call.asCallCfgNode() = n and pos = TReceiverArgumentPosition()
251263
}
252264

253265
override CfgScope getCfgScope() { result = n.getAstNode().getEnclosingCfgScope() }
@@ -281,7 +293,7 @@ final class ClosureArgumentNode extends ArgumentNode, ExprNode {
281293

282294
override predicate isArgumentOf(DataFlowCall call, RustDataFlow::ArgumentPosition pos) {
283295
call.asCallCfgNode() = call_ and
284-
pos.isClosureSelf()
296+
pos.isClosureReceiver()
285297
}
286298
}
287299

0 commit comments

Comments
 (0)