Skip to content

Commit a7663ad

Browse files
rdmarsh2rdmarsh2
authored
Swift: add flow through inout parameters
1 parent 21ba731 commit a7663ad

File tree

8 files changed

+216
-11
lines changed

8 files changed

+216
-11
lines changed

‎swift/ql/lib/codeql/swift/dataflow/Ssa.qll

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,18 @@ module Ssa {
7171
value.getNode().asAstNode() = var.getParentInitializer()
7272
)
7373
}
74+
75+
cached predicate isInoutDef(ExprCfgNode argument) {
76+
exists(
77+
CallExpr c, BasicBlock bb, int blockIndex, int argIndex, VarDecl v, InOutExpr argExpr // TODO: use CFG node for assignment expr
78+
|
79+
this.definesAt(v, bb, blockIndex) and
80+
bb.getNode(blockIndex).getNode().asAstNode() = c and
81+
c.getArgument(argIndex).getExpr() = argExpr and
82+
argExpr = argument.getNode().asAstNode() and
83+
argExpr.getSubExpr() = v.getAnAccess() // TODO: fields?
84+
)
85+
}
7486
}
7587

7688
cached

‎swift/ql/lib/codeql/swift/dataflow/internal/DataFlowDispatch.qll

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ private import swift
22
private import DataFlowPrivate
33
private import DataFlowPublic
44

5-
newtype TReturnKind = TNormalReturnKind()
5+
newtype TReturnKind =
6+
TNormalReturnKind() or
7+
TParamReturnKind(int i) { exists(ParamDecl param | param.getIndex() = i) }
68

79
/**
810
* Gets a node that can read the value returned from `call` with return kind
@@ -27,16 +29,36 @@ class NormalReturnKind extends ReturnKind, TNormalReturnKind {
2729
override string toString() { result = "return" }
2830
}
2931

32+
/**
33+
* A value returned from a callable using an `inout` parameter.
34+
*/
35+
class ParamReturnKind extends ReturnKind, TParamReturnKind {
36+
int index;
37+
38+
ParamReturnKind() { this = TParamReturnKind(index) }
39+
40+
int getIndex() {
41+
result = index
42+
}
43+
44+
override string toString() { result = "param(" + index + ")" }
45+
}
46+
3047
/**
3148
* A callable. This includes callables from source code, as well as callables
3249
* defined in library code.
3350
*/
3451
class DataFlowCallable extends TDataFlowCallable {
52+
AbstractFunctionDecl func;
53+
54+
DataFlowCallable() {
55+
this = TDataFlowFunc(func)
56+
}
3557
/** Gets a textual representation of this callable. */
36-
string toString() { none() }
58+
string toString() { result = func.toString() }
3759

3860
/** Gets the location of this callable. */
39-
Location getLocation() { none() }
61+
Location getLocation() { result = func.getLocation() }
4062
}
4163

4264
/**
@@ -47,7 +69,7 @@ class DataFlowCall extends ExprNode {
4769
DataFlowCall() { this.asExpr() instanceof CallExpr }
4870

4971
/** Gets the enclosing callable. */
50-
DataFlowCallable getEnclosingCallable() { none() }
72+
DataFlowCallable getEnclosingCallable() { result = TDataFlowFunc(this.getCfgNode().getScope()) }
5173
}
5274

5375
cached

‎swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
private import swift
22
private import DataFlowPublic
33
private import DataFlowDispatch
4+
private import codeql.swift.controlflow.ControlFlowGraph
45
private import codeql.swift.controlflow.CfgNodes
56
private import codeql.swift.dataflow.Ssa
67
private import codeql.swift.controlflow.BasicBlocks
@@ -20,7 +21,7 @@ predicate isArgumentNode(ArgumentNode arg, DataFlowCall c, ArgumentPosition pos)
2021
}
2122

2223
abstract class NodeImpl extends Node {
23-
DataFlowCallable getEnclosingCallable() { none() }
24+
abstract DataFlowCallable getEnclosingCallable();
2425

2526
/** Do not call: use `getLocation()` instead. */
2627
abstract Location getLocationImpl();
@@ -60,7 +61,20 @@ private module Cached {
6061
cached
6162
newtype TNode =
6263
TExprNode(ExprCfgNode e) or
63-
TSsaDefinitionNode(Ssa::Definition def)
64+
TSsaDefinitionNode(Ssa::Definition def) or
65+
TInoutReturnNode(ParamDecl param, ControlFlowNode exit) {
66+
param.isInout() and
67+
exit.getScope() = param.getDeclaringFunction() and
68+
exit.getNode().asAstNode() instanceof ReturnStmt
69+
} or
70+
TInOutUpdateNode(ParamDecl param, CallExpr call) {
71+
param.isInout() and
72+
call.getStaticTarget() = param.getDeclaringFunction()
73+
}
74+
75+
private predicate localSsaFlowStepUseUse(Ssa::Definition def, Node nodeFrom, Node nodeTo) {
76+
def.adjacentReadPair(nodeFrom.getCfgNode(), nodeTo.getCfgNode())
77+
}
6478

6579
private predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) {
6680
exists(Ssa::Definition def |
@@ -73,11 +87,26 @@ private module Cached {
7387
nodeTo.getCfgNode() = def.getAFirstRead()
7488
or
7589
// use-use flow
76-
def.adjacentReadPair(nodeFrom.getCfgNode(), nodeTo.getCfgNode())
90+
localSsaFlowStepUseUse(def, nodeFrom, nodeTo)
7791
or
92+
//localSsaFlowStepUseUse(def, nodeFrom.(PostUpdateNode).getPreUpdateNode(), nodeTo)
93+
//or
7894
// step from previous read to Phi node
7995
localFlowSsaInput(nodeFrom, def, nodeTo.asDefinition())
8096
)
97+
or
98+
exists(ParamReturnKind kind, ExprCfgNode arg |
99+
arg =
100+
nodeFrom
101+
.(InOutUpdateNode)
102+
.getCall(kind)
103+
.getCfgNode()
104+
.(CallExprCfgNode)
105+
.getArgument(kind.getIndex()) and
106+
nodeTo.asDefinition().(Ssa::WriteDefinition).isInoutDef(arg)
107+
)
108+
or
109+
nodeFrom.asExpr() = nodeTo.asExpr().(InOutExpr).getSubExpr()
81110
}
82111

83112
/**
@@ -172,6 +201,27 @@ private module ReturnNodes {
172201

173202
override ReturnKind getKind() { result instanceof NormalReturnKind }
174203
}
204+
205+
class InoutReturnNodeImpl extends ReturnNode, TInoutReturnNode, NodeImpl {
206+
ParamDecl param;
207+
ControlFlowNode exit;
208+
209+
InoutReturnNodeImpl() { this = TInoutReturnNode(param, exit) }
210+
211+
override ReturnKind getKind() { result.(ParamReturnKind).getIndex() = param.getIndex() }
212+
213+
override ControlFlowNode getCfgNode() { result = exit }
214+
215+
override DataFlowCallable getEnclosingCallable() {
216+
result = TDataFlowFunc(param.getDeclaringFunction())
217+
}
218+
219+
ParamDecl getParameter() { result = param }
220+
221+
override Location getLocationImpl() { result = exit.getLocation() }
222+
223+
override string toStringImpl() { result = param.toString() + "[return]" }
224+
}
175225
}
176226

177227
import ReturnNodes
@@ -188,6 +238,26 @@ private module OutNodes {
188238
result = this and kind instanceof NormalReturnKind
189239
}
190240
}
241+
242+
class InOutUpdateNode extends OutNode, TInOutUpdateNode, NodeImpl {
243+
ParamDecl param;
244+
CallExpr call;
245+
246+
InOutUpdateNode() { this = TInOutUpdateNode(param, call) }
247+
248+
override DataFlowCall getCall(ReturnKind kind) {
249+
result.asExpr() = call and
250+
kind.(ParamReturnKind).getIndex() = param.getIndex()
251+
}
252+
253+
override DataFlowCallable getEnclosingCallable() {
254+
result = TDataFlowFunc(getCall(_).getCfgNode().getScope())
255+
}
256+
257+
override Location getLocationImpl() { result = call.getLocation() }
258+
259+
override string toStringImpl() { result = param.toString() }
260+
}
191261
}
192262

193263
import OutNodes

‎swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ class SsaDefinitionNode extends Node, TSsaDefinitionNode {
8181
override Ssa::Definition asDefinition() { result = def }
8282
}
8383

84+
class InoutReturnNode extends Node instanceof InoutReturnNodeImpl {
85+
86+
ParamDecl getParameter() { result = super.getParameter() }
87+
88+
}
89+
8490
/**
8591
* A node associated with an object after an operation that might have
8692
* changed its state.

‎swift/ql/lib/codeql/swift/dataflow/internal/SsaImplSpecific.qll

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@ predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain)
2828
certain = true
2929
)
3030
or
31+
exists(CallExpr call, Argument arg |
32+
arg.getExpr().(InOutExpr).getSubExpr() = v.getAnAccess() and
33+
call.getAnArgument() = arg and
34+
call.getStaticTarget().getParam(arg.getIndex()).isInout() and
35+
bb.getNode(i).getNode().asAstNode() = call and
36+
certain = false
37+
)
38+
or
3139
v instanceof ParamDecl and
3240
bb.getNode(i).getNode().asAstNode() = v and
3341
certain = true
@@ -42,4 +50,12 @@ predicate variableRead(BasicBlock bb, int i, SourceVariable v, boolean certain)
4250
v = ref.getDecl() and
4351
certain = true
4452
)
53+
or
54+
exists(ReturnStmt return, AbstractFunctionDecl func |
55+
bb.getNode(i).getNode().asAstNode() = return and
56+
v.(ParamDecl).isInout() and
57+
func.getAParam() = v and
58+
bb.getScope() = func and
59+
certain = true
60+
)
4561
}

‎swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,20 @@ edges
1212
| test.swift:29:26:29:29 | y : | test.swift:31:15:31:15 | y |
1313
| test.swift:35:12:35:19 | call to source : | test.swift:39:15:39:29 | call to callee_source |
1414
| test.swift:43:19:43:26 | call to source : | test.swift:50:15:50:15 | t |
15+
| test.swift:54:11:54:18 | call to source : | test.swift:55:5:55:5 | arg[return] : |
16+
| test.swift:55:5:55:5 | arg[return] : | test.swift:61:5:61:24 | arg : |
17+
| test.swift:61:5:61:24 | arg : | test.swift:62:15:62:15 | x |
18+
| test.swift:65:16:65:28 | WriteDef : | test.swift:69:5:69:5 | arg1[return] : |
19+
| test.swift:65:16:65:28 | WriteDef : | test.swift:69:5:69:5 | arg2[return] : |
20+
| test.swift:65:16:65:28 | arg1 : | test.swift:69:5:69:5 | arg1[return] : |
21+
| test.swift:65:16:65:28 | arg1 : | test.swift:69:5:69:5 | arg2[return] : |
22+
| test.swift:73:18:73:25 | call to source : | test.swift:75:21:75:22 | &... : |
23+
| test.swift:75:5:75:33 | arg1 : | test.swift:76:15:76:15 | x |
24+
| test.swift:75:5:75:33 | arg2 : | test.swift:77:15:77:15 | y |
25+
| test.swift:75:21:75:22 | &... : | test.swift:65:16:65:28 | WriteDef : |
26+
| test.swift:75:21:75:22 | &... : | test.swift:65:16:65:28 | arg1 : |
27+
| test.swift:75:21:75:22 | &... : | test.swift:75:5:75:33 | arg1 : |
28+
| test.swift:75:21:75:22 | &... : | test.swift:75:5:75:33 | arg2 : |
1529
nodes
1630
| test.swift:6:19:6:26 | call to source : | semmle.label | call to source : |
1731
| test.swift:7:15:7:15 | t1 | semmle.label | t1 |
@@ -33,7 +47,27 @@ nodes
3347
| test.swift:39:15:39:29 | call to callee_source | semmle.label | call to callee_source |
3448
| test.swift:43:19:43:26 | call to source : | semmle.label | call to source : |
3549
| test.swift:50:15:50:15 | t | semmle.label | t |
50+
| test.swift:54:11:54:18 | call to source : | semmle.label | call to source : |
51+
| test.swift:55:5:55:5 | arg[return] : | semmle.label | arg[return] : |
52+
| test.swift:61:5:61:24 | arg : | semmle.label | arg : |
53+
| test.swift:62:15:62:15 | x | semmle.label | x |
54+
| test.swift:65:16:65:28 | WriteDef : | semmle.label | WriteDef : |
55+
| test.swift:65:16:65:28 | WriteDef : | semmle.label | arg1 : |
56+
| test.swift:65:16:65:28 | arg1 : | semmle.label | WriteDef : |
57+
| test.swift:65:16:65:28 | arg1 : | semmle.label | arg1 : |
58+
| test.swift:69:5:69:5 | arg1[return] : | semmle.label | arg1[return] : |
59+
| test.swift:69:5:69:5 | arg2[return] : | semmle.label | arg2[return] : |
60+
| test.swift:73:18:73:25 | call to source : | semmle.label | call to source : |
61+
| test.swift:75:5:75:33 | arg1 : | semmle.label | arg1 : |
62+
| test.swift:75:5:75:33 | arg2 : | semmle.label | arg2 : |
63+
| test.swift:75:21:75:22 | &... : | semmle.label | &... : |
64+
| test.swift:76:15:76:15 | x | semmle.label | x |
65+
| test.swift:77:15:77:15 | y | semmle.label | y |
3666
subpaths
67+
| test.swift:75:21:75:22 | &... : | test.swift:65:16:65:28 | WriteDef : | test.swift:69:5:69:5 | arg1[return] : | test.swift:75:5:75:33 | arg1 : |
68+
| test.swift:75:21:75:22 | &... : | test.swift:65:16:65:28 | WriteDef : | test.swift:69:5:69:5 | arg2[return] : | test.swift:75:5:75:33 | arg2 : |
69+
| test.swift:75:21:75:22 | &... : | test.swift:65:16:65:28 | arg1 : | test.swift:69:5:69:5 | arg1[return] : | test.swift:75:5:75:33 | arg1 : |
70+
| test.swift:75:21:75:22 | &... : | test.swift:65:16:65:28 | arg1 : | test.swift:69:5:69:5 | arg2[return] : | test.swift:75:5:75:33 | arg2 : |
3771
#select
3872
| test.swift:6:19:6:26 | call to source : | test.swift:7:15:7:15 | t1 |
3973
| test.swift:6:19:6:26 | call to source : | test.swift:9:15:9:15 | t1 |
@@ -42,3 +76,6 @@ subpaths
4276
| test.swift:26:26:26:33 | call to source : | test.swift:31:15:31:15 | y |
4377
| test.swift:35:12:35:19 | call to source : | test.swift:39:15:39:29 | call to callee_source |
4478
| test.swift:43:19:43:26 | call to source : | test.swift:50:15:50:15 | t |
79+
| test.swift:54:11:54:18 | call to source : | test.swift:62:15:62:15 | x |
80+
| test.swift:73:18:73:25 | call to source : | test.swift:76:15:76:15 | x |
81+
| test.swift:73:18:73:25 | call to source : | test.swift:77:15:77:15 | y |

‎swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,33 @@
2323
| test.swift:48:9:48:13 | WriteDef | test.swift:50:5:50:5 | Phi |
2424
| test.swift:48:13:48:13 | 1 | test.swift:48:9:48:13 | WriteDef |
2525
| test.swift:50:5:50:5 | Phi | test.swift:50:15:50:15 | t |
26-
| test.swift:58:9:58:12 | WriteDef | test.swift:59:15:59:15 | x |
27-
| test.swift:58:18:58:18 | 0 | test.swift:58:9:58:12 | WriteDef |
28-
| test.swift:59:15:59:15 | x | test.swift:60:23:60:23 | x |
29-
| test.swift:60:23:60:23 | x | test.swift:61:15:61:15 | x |
26+
| test.swift:54:5:54:18 | WriteDef | test.swift:55:5:55:5 | arg[return] |
27+
| test.swift:54:11:54:18 | call to source | test.swift:54:5:54:18 | WriteDef |
28+
| test.swift:59:9:59:12 | WriteDef | test.swift:60:15:60:15 | x |
29+
| test.swift:59:18:59:18 | 0 | test.swift:59:9:59:12 | WriteDef |
30+
| test.swift:60:15:60:15 | x | test.swift:61:23:61:23 | x |
31+
| test.swift:61:5:61:24 | WriteDef | test.swift:62:15:62:15 | x |
32+
| test.swift:61:5:61:24 | arg | test.swift:61:5:61:24 | WriteDef |
33+
| test.swift:61:23:61:23 | x | test.swift:61:22:61:23 | &... |
34+
| test.swift:65:16:65:28 | WriteDef | test.swift:66:21:66:21 | arg1 |
35+
| test.swift:65:16:65:28 | arg1 | test.swift:66:21:66:21 | arg1 |
36+
| test.swift:65:33:65:45 | WriteDef | test.swift:67:12:67:12 | arg2 |
37+
| test.swift:65:33:65:45 | arg2 | test.swift:67:12:67:12 | arg2 |
38+
| test.swift:66:9:66:15 | WriteDef | test.swift:68:12:68:12 | temp |
39+
| test.swift:66:21:66:21 | arg1 | test.swift:66:9:66:15 | WriteDef |
40+
| test.swift:67:5:67:12 | WriteDef | test.swift:69:5:69:5 | arg1[return] |
41+
| test.swift:67:5:67:12 | WriteDef | test.swift:69:5:69:5 | arg2[return] |
42+
| test.swift:67:12:67:12 | arg2 | test.swift:67:5:67:12 | WriteDef |
43+
| test.swift:68:5:68:12 | WriteDef | test.swift:69:5:69:5 | arg1[return] |
44+
| test.swift:68:5:68:12 | WriteDef | test.swift:69:5:69:5 | arg2[return] |
45+
| test.swift:68:12:68:12 | temp | test.swift:68:5:68:12 | WriteDef |
46+
| test.swift:73:9:73:12 | WriteDef | test.swift:75:22:75:22 | x |
47+
| test.swift:73:18:73:25 | call to source | test.swift:73:9:73:12 | WriteDef |
48+
| test.swift:74:9:74:12 | WriteDef | test.swift:75:32:75:32 | y |
49+
| test.swift:74:18:74:18 | 0 | test.swift:74:9:74:12 | WriteDef |
50+
| test.swift:75:5:75:33 | WriteDef | test.swift:76:15:76:15 | x |
51+
| test.swift:75:5:75:33 | WriteDef | test.swift:77:15:77:15 | y |
52+
| test.swift:75:5:75:33 | arg1 | test.swift:75:5:75:33 | WriteDef |
53+
| test.swift:75:5:75:33 | arg2 | test.swift:75:5:75:33 | WriteDef |
54+
| test.swift:75:22:75:22 | x | test.swift:75:21:75:22 | &... |
55+
| test.swift:75:32:75:32 | y | test.swift:75:31:75:32 | &... |

‎swift/ql/test/library-tests/dataflow/dataflow/test.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,27 @@ func branching(b: Bool) -> Void {
5252

5353
func inoutSource(arg: inout Int) -> Void {
5454
arg = source()
55+
return
5556
}
5657

5758
func inoutUser() {
5859
var x: Int = 0
5960
sink(arg: x)
6061
inoutSource(arg: &x)
6162
sink(arg: x)
63+
}
64+
65+
func inoutSwap(arg1: inout Int, arg2: inout Int) -> Void {
66+
var temp: Int = arg1
67+
arg1 = arg2
68+
arg2 = temp
69+
return
70+
}
71+
72+
func swapUser() {
73+
var x: Int = source()
74+
var y: Int = 0
75+
inoutSwap(arg1: &x, arg2: &y)
76+
sink(arg: x)
77+
sink(arg: y)
6278
}

0 commit comments

Comments
 (0)