Skip to content

Commit 2465cc2

Browse files
MathiasVPMathiasVP
committed
Swift: Don't define 'ClosureSelfParameterNode' as the expression node of the closure.
1 parent 11194e5 commit 2465cc2

File tree

7 files changed

+25
-26
lines changed

7 files changed

+25
-26
lines changed

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

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,8 @@ private module Cached {
158158
TDictionarySubscriptNode(SubscriptExpr e) {
159159
e.getBase().getType().getCanonicalType() instanceof CanonicalDictionaryType
160160
} or
161-
TCaptureNode(CaptureFlow::SynthesizedCaptureNode cn)
161+
TCaptureNode(CaptureFlow::SynthesizedCaptureNode cn) or
162+
TClosureSelfParameterNode(ClosureExpr closure)
162163

163164
private predicate localSsaFlowStepUseUse(Ssa::Definition def, Node nodeFrom, Node nodeTo) {
164165
def.adjacentReadPair(nodeFrom.getCfgNode(), nodeTo.getCfgNode()) and
@@ -359,7 +360,9 @@ private predicate hasPatternNode(PatternCfgNode n, Pattern p) {
359360
import Cached
360361

361362
/** Holds if `n` should be hidden from path explanations. */
362-
predicate nodeIsHidden(Node n) { n instanceof FlowSummaryNode }
363+
predicate nodeIsHidden(Node n) {
364+
n instanceof FlowSummaryNode or n instanceof ClosureSelfParameterNode
365+
}
363366

364367
/**
365368
* The intermediate node for a dictionary subscript operation `dict[key]`. In a write, this is used
@@ -383,12 +386,6 @@ private class DictionarySubscriptNode extends NodeImpl, TDictionarySubscriptNode
383386
SubscriptExpr getExpr() { result = expr }
384387
}
385388

386-
private class ClosureSelfReferenceNode extends ExprNodeImpl {
387-
override ClosureExpr expr;
388-
389-
ClosureExpr getClosure() { result = expr }
390-
}
391-
392389
private module ParameterNodes {
393390
abstract class ParameterNodeImpl extends NodeImpl {
394391
predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { none() }
@@ -419,11 +416,23 @@ private module ParameterNodes {
419416
override ParamDecl getParameter() { result = param }
420417
}
421418

422-
class ClosureSelfParameterNode extends ParameterNodeImpl, ClosureSelfReferenceNode {
419+
class ClosureSelfParameterNode extends ParameterNodeImpl, TClosureSelfParameterNode {
420+
ClosureExpr closure;
421+
422+
ClosureSelfParameterNode() { this = TClosureSelfParameterNode(closure) }
423+
423424
override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
424-
c.asSourceCallable() = this.getClosure() and
425+
c.asSourceCallable() = closure and
425426
pos instanceof TThisParameter
426427
}
428+
429+
override Location getLocationImpl() { result = closure.getLocation() }
430+
431+
override string toStringImpl() { result = "closure self parameter" }
432+
433+
override DataFlowCallable getEnclosingCallable() { this.isParameterOf(result, _) }
434+
435+
ClosureExpr getClosure() { result = closure }
427436
}
428437

429438
class SummaryParameterNode extends ParameterNodeImpl, FlowSummaryNode {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ func captureList() {
1111
let y: Int = source("captureList", 123);
1212
{ [x = hello()] () in
1313
sink(x) // $ MISSING: hasValueFlow=hello
14-
sink(y) // $ hasValueFlow=captureList
14+
sink(y) // $ MISSING: hasValueFlow=captureList
1515
}()
1616
}
1717

@@ -31,7 +31,7 @@ var escape: (() -> Int)? = nil
3131
func setEscape() {
3232
let x = source("setEscape", 0)
3333
escape = {
34-
sink(x) // $ hasValueFlow=setEscape
34+
sink(x) // $ MISSING: hasValueFlow=setEscape
3535
return x + 1
3636
}
3737
}

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ edges
4747
| test.swift:113:14:113:19 | arg | test.swift:114:19:114:19 | arg |
4848
| test.swift:113:14:113:19 | arg | test.swift:114:19:114:19 | arg |
4949
| test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg |
50-
| test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg |
5150
| test.swift:114:19:114:19 | arg | test.swift:114:12:114:22 | call to ... |
5251
| test.swift:114:19:114:19 | arg | test.swift:114:12:114:22 | call to ... |
5352
| test.swift:114:19:114:19 | arg | test.swift:123:10:123:13 | i |
@@ -59,9 +58,6 @@ edges
5958
| test.swift:122:31:122:38 | call to source() | test.swift:113:14:113:19 | arg |
6059
| test.swift:122:31:122:38 | call to source() | test.swift:122:18:125:6 | call to forward(arg:lambda:) |
6160
| test.swift:123:10:123:13 | i | test.swift:124:16:124:16 | i |
62-
| test.swift:128:22:131:6 | call to forward(arg:lambda:) | test.swift:132:15:132:15 | clean |
63-
| test.swift:128:35:128:42 | call to source() | test.swift:113:14:113:19 | arg |
64-
| test.swift:128:35:128:42 | call to source() | test.swift:128:22:131:6 | call to forward(arg:lambda:) |
6561
| test.swift:142:10:142:13 | i | test.swift:143:16:143:16 | i |
6662
| test.swift:145:23:145:30 | call to source() | test.swift:142:10:142:13 | i |
6763
| test.swift:145:23:145:30 | call to source() | test.swift:145:15:145:31 | call to ... |
@@ -697,9 +693,6 @@ nodes
697693
| test.swift:123:10:123:13 | i | semmle.label | i |
698694
| test.swift:124:16:124:16 | i | semmle.label | i |
699695
| test.swift:126:15:126:15 | z | semmle.label | z |
700-
| test.swift:128:22:131:6 | call to forward(arg:lambda:) | semmle.label | call to forward(arg:lambda:) |
701-
| test.swift:128:35:128:42 | call to source() | semmle.label | call to source() |
702-
| test.swift:132:15:132:15 | clean | semmle.label | clean |
703696
| test.swift:138:19:138:26 | call to source() | semmle.label | call to source() |
704697
| test.swift:142:10:142:13 | i | semmle.label | i |
705698
| test.swift:143:16:143:16 | i | semmle.label | i |
@@ -1264,11 +1257,9 @@ nodes
12641257
subpaths
12651258
| test.swift:75:22:75:22 | x | test.swift:65:16:65:28 | arg1 | test.swift:65:1:70:1 | arg2[return] | test.swift:75:32:75:32 | [post] y |
12661259
| test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg | test.swift:110:12:110:12 | arg | test.swift:114:12:114:22 | call to ... |
1267-
| test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg | test.swift:110:12:110:12 | arg | test.swift:114:12:114:22 | call to ... |
12681260
| test.swift:114:19:114:19 | arg | test.swift:123:10:123:13 | i | test.swift:124:16:124:16 | i | test.swift:114:12:114:22 | call to ... |
12691261
| test.swift:119:31:119:31 | x | test.swift:113:14:113:19 | arg | test.swift:114:12:114:22 | call to ... | test.swift:119:18:119:44 | call to forward(arg:lambda:) |
12701262
| test.swift:122:31:122:38 | call to source() | test.swift:113:14:113:19 | arg | test.swift:114:12:114:22 | call to ... | test.swift:122:18:125:6 | call to forward(arg:lambda:) |
1271-
| test.swift:128:35:128:42 | call to source() | test.swift:113:14:113:19 | arg | test.swift:114:12:114:22 | call to ... | test.swift:128:22:131:6 | call to forward(arg:lambda:) |
12721263
| test.swift:145:23:145:30 | call to source() | test.swift:142:10:142:13 | i | test.swift:143:16:143:16 | i | test.swift:145:15:145:31 | call to ... |
12731264
| test.swift:170:9:170:9 | value | test.swift:163:7:163:7 | value | file://:0:0:0:0 | [post] self [x] | test.swift:170:5:170:5 | [post] self [x] |
12741265
| test.swift:174:12:174:12 | self [x] | test.swift:163:7:163:7 | self [x] | file://:0:0:0:0 | .x | test.swift:174:12:174:12 | .x |
@@ -1347,7 +1338,6 @@ subpaths
13471338
| test.swift:105:19:105:19 | x | test.swift:89:15:89:22 | call to source() | test.swift:105:19:105:19 | x | result |
13481339
| test.swift:120:15:120:15 | y | test.swift:118:18:118:25 | call to source() | test.swift:120:15:120:15 | y | result |
13491340
| test.swift:126:15:126:15 | z | test.swift:122:31:122:38 | call to source() | test.swift:126:15:126:15 | z | result |
1350-
| test.swift:132:15:132:15 | clean | test.swift:128:35:128:42 | call to source() | test.swift:132:15:132:15 | clean | result |
13511341
| test.swift:138:19:138:26 | call to source() | test.swift:138:19:138:26 | call to source() | test.swift:138:19:138:26 | call to source() | result |
13521342
| test.swift:145:15:145:31 | call to ... | test.swift:145:23:145:30 | call to source() | test.swift:145:15:145:31 | call to ... | result |
13531343
| test.swift:151:15:151:28 | call to ... | test.swift:149:16:149:23 | call to source() | test.swift:151:15:151:28 | call to ... | result |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1134,7 +1134,7 @@
11341134
| test.swift:888:9:888:9 | stream | test.swift:888:9:888:9 | SSA def(stream) |
11351135
| test.swift:888:18:896:6 | call to AsyncStream<Element>.init(_:bufferingPolicy:_:) | test.swift:888:9:888:9 | stream |
11361136
| test.swift:889:9:889:9 | continuation | test.swift:890:27:895:13 | continuation |
1137-
| test.swift:890:27:895:13 | { ... } | test.swift:891:17:891:17 | phi(this) |
1137+
| test.swift:890:27:895:13 | closure self parameter | test.swift:891:17:891:17 | phi(this) |
11381138
| test.swift:891:17:891:17 | $generator | test.swift:891:17:891:17 | &... |
11391139
| test.swift:891:17:891:17 | &... | test.swift:891:17:891:17 | $generator |
11401140
| test.swift:891:17:891:17 | [post] $generator | test.swift:891:17:891:17 | &... |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ func forwarder() {
129129
(i: Int) -> Int in
130130
return 0
131131
})
132-
sink(arg: clean) // $ SPURIOUS: flow=128
132+
sink(arg: clean) // clean
133133
}
134134

135135
func lambdaFlows() {

‎swift/ql/test/library-tests/dataflow/taint/core/TaintInline.expected‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
failures
21
testFailures
2+
failures

‎swift/ql/test/library-tests/dataflow/taint/libraries/url.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ func taintThroughURL() {
287287
let _ = clean.withCString({
288288
ptrClean in
289289
sink(arg: URL(fileURLWithFileSystemRepresentation: ptrClean, isDirectory: false, relativeTo: nil))
290-
sink(arg: URL(fileURLWithFileSystemRepresentation: ptrClean, isDirectory: false, relativeTo: urlTainted)) // $ tainted=210
290+
sink(arg: URL(fileURLWithFileSystemRepresentation: ptrClean, isDirectory: false, relativeTo: urlTainted)) // $ MISSING: tainted=210
291291
});
292292
sink(arg: URL(fileURLWithFileSystemRepresentation: 0 as! UnsafePointer<Int8>, isDirectory: false, relativeTo: urlTainted)) // $ tainted=210
293293
let _ = tainted.withCString({

0 commit comments

Comments
 (0)