Skip to content

Commit 142e500

Browse files
MathiasVPMathiasVP
authored
Merge pull request github#10967 from MathiasVP/fix-swift-summary
Swift: Fix flow out of summarized callables
2 parents ce441ad + 062a0ab commit 142e500

File tree

8 files changed

+55
-79
lines changed

8 files changed

+55
-79
lines changed

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

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,17 +53,19 @@ class ParamReturnKind extends ReturnKind, TParamReturnKind {
5353
* defined in library code.
5454
*/
5555
class DataFlowCallable extends TDataFlowCallable {
56-
CfgScope scope;
57-
58-
DataFlowCallable() { this = TDataFlowFunc(scope) }
56+
/** Gets the location of this callable. */
57+
Location getLocation() { none() } // overridden in subclasses
5958

6059
/** Gets a textual representation of this callable. */
61-
string toString() { result = scope.toString() }
60+
string toString() { none() } // overridden in subclasses
6261

63-
/** Gets the location of this callable. */
64-
Location getLocation() { result = scope.getLocation() }
62+
CfgScope asSourceCallable() { this = TDataFlowFunc(result) }
63+
64+
FlowSummary::SummarizedCallable asSummarizedCallable() { this = TSummarizedCallable(result) }
6565

66-
Callable::TypeRange getUnderlyingCallable() { result = scope }
66+
Callable::TypeRange getUnderlyingCallable() {
67+
result = this.asSummarizedCallable() or result = this.asSourceCallable()
68+
}
6769
}
6870

6971
cached
@@ -230,7 +232,7 @@ cached
230232
private module Cached {
231233
cached
232234
newtype TDataFlowCallable =
233-
TDataFlowFunc(CfgScope scope) or
235+
TDataFlowFunc(CfgScope scope) { not scope instanceof FlowSummary::SummarizedCallable } or
234236
TSummarizedCallable(FlowSummary::SummarizedCallable c)
235237

236238
/** Gets a viable run-time target for the call `call`. */
@@ -247,6 +249,26 @@ private module Cached {
247249
result = TSummarizedCallable(call.asCall().getStaticTarget())
248250
}
249251

252+
private class SourceCallable extends DataFlowCallable, TDataFlowFunc {
253+
CfgScope scope;
254+
255+
SourceCallable() { this = TDataFlowFunc(scope) }
256+
257+
override string toString() { result = scope.toString() }
258+
259+
override Location getLocation() { result = scope.getLocation() }
260+
}
261+
262+
private class SummarizedCallable extends DataFlowCallable, TSummarizedCallable {
263+
FlowSummary::SummarizedCallable sc;
264+
265+
SummarizedCallable() { this = TSummarizedCallable(sc) }
266+
267+
override string toString() { result = sc.toString() }
268+
269+
override Location getLocation() { result = sc.getLocation() }
270+
}
271+
250272
cached
251273
newtype TArgumentPosition =
252274
TThisArgument() or

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ class SummaryNode extends NodeImpl, TSummaryNode {
269269

270270
SummaryNode() { this = TSummaryNode(c, state) }
271271

272-
override DataFlowCallable getEnclosingCallable() { result = TDataFlowFunc(c) }
272+
override DataFlowCallable getEnclosingCallable() { result.asSummarizedCallable() = c }
273273

274274
override UnknownLocation getLocationImpl() { any() }
275275

@@ -430,6 +430,8 @@ private module OutNodes {
430430
}
431431

432432
class SummaryOutNode extends OutNode, SummaryNode {
433+
SummaryOutNode() { FlowSummaryImpl::Private::summaryOutNode(_, this, _) }
434+
433435
override DataFlowCall getCall(ReturnKind kind) {
434436
FlowSummaryImpl::Private::summaryOutNode(result, this, kind)
435437
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ ExprNode exprNode(DataFlowExpr e) { result.asExpr() = e }
116116
/**
117117
* Gets the node corresponding to the value of parameter `p` at function entry.
118118
*/
119-
ParameterNode parameterNode(DataFlowParameter p) { none() }
119+
ParameterNode parameterNode(DataFlowParameter p) { result.getParameter() = p }
120120

121121
/**
122122
* Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ private import codeql.swift.controlflow.CfgNodes
1515

1616
class SummarizedCallableBase = AbstractFunctionDecl;
1717

18-
DataFlowCallable inject(SummarizedCallable c) { result = TDataFlowFunc(c) }
18+
DataFlowCallable inject(SummarizedCallable c) { result.getUnderlyingCallable() = c }
1919

2020
/** Gets the parameter position of the instance parameter. */
2121
ArgumentPosition instanceParameterPosition() { result instanceof ThisArgumentPosition }

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
edges
2+
| file://:0:0:0:0 | [summary param] this in signum() : | file://:0:0:0:0 | [summary] to write: return (return) in signum() : |
23
| file://:0:0:0:0 | self [a, x] : | file://:0:0:0:0 | .a [x] : |
34
| file://:0:0:0:0 | self [x] : | file://:0:0:0:0 | .x : |
45
| file://:0:0:0:0 | value : | file://:0:0:0:0 | [post] self [x] : |
@@ -100,10 +101,18 @@ edges
100101
| test.swift:225:14:225:21 | call to source() : | test.swift:238:13:238:15 | .source_value |
101102
| test.swift:259:12:259:19 | call to source() : | test.swift:263:13:263:28 | call to optionalSource() : |
102103
| test.swift:263:13:263:28 | call to optionalSource() : | test.swift:264:15:264:16 | ...! |
104+
| test.swift:263:13:263:28 | call to optionalSource() : | test.swift:266:15:266:16 | ...? : |
105+
| test.swift:265:15:265:22 | call to source() : | file://:0:0:0:0 | [summary param] this in signum() : |
106+
| test.swift:265:15:265:22 | call to source() : | test.swift:265:15:265:31 | call to signum() |
107+
| test.swift:266:15:266:16 | ...? : | file://:0:0:0:0 | [summary param] this in signum() : |
108+
| test.swift:266:15:266:16 | ...? : | test.swift:266:15:266:25 | call to signum() : |
109+
| test.swift:266:15:266:25 | call to signum() : | test.swift:266:15:266:25 | OptionalEvaluationExpr |
103110
nodes
104111
| file://:0:0:0:0 | .a [x] : | semmle.label | .a [x] : |
105112
| file://:0:0:0:0 | .x : | semmle.label | .x : |
106113
| file://:0:0:0:0 | [post] self [x] : | semmle.label | [post] self [x] : |
114+
| file://:0:0:0:0 | [summary param] this in signum() : | semmle.label | [summary param] this in signum() : |
115+
| file://:0:0:0:0 | [summary] to write: return (return) in signum() : | semmle.label | [summary] to write: return (return) in signum() : |
107116
| file://:0:0:0:0 | self [a, x] : | semmle.label | self [a, x] : |
108117
| file://:0:0:0:0 | self [x] : | semmle.label | self [x] : |
109118
| file://:0:0:0:0 | value : | semmle.label | value : |
@@ -213,6 +222,11 @@ nodes
213222
| test.swift:259:12:259:19 | call to source() : | semmle.label | call to source() : |
214223
| test.swift:263:13:263:28 | call to optionalSource() : | semmle.label | call to optionalSource() : |
215224
| test.swift:264:15:264:16 | ...! | semmle.label | ...! |
225+
| test.swift:265:15:265:22 | call to source() : | semmle.label | call to source() : |
226+
| test.swift:265:15:265:31 | call to signum() | semmle.label | call to signum() |
227+
| test.swift:266:15:266:16 | ...? : | semmle.label | ...? : |
228+
| test.swift:266:15:266:25 | OptionalEvaluationExpr | semmle.label | OptionalEvaluationExpr |
229+
| test.swift:266:15:266:25 | call to signum() : | semmle.label | call to signum() : |
216230
subpaths
217231
| test.swift:75:21:75:22 | &... : | test.swift:65:16:65:28 | arg1 : | test.swift:65:1:70:1 | arg2[return] : | test.swift:75:31:75:32 | [post] &... : |
218232
| 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 ... : |
@@ -239,6 +253,8 @@ subpaths
239253
| test.swift:218:11:218:18 | call to source() : | test.swift:169:12:169:22 | value : | test.swift:170:5:170:5 | [post] self [x] : | test.swift:218:3:218:5 | [post] getter for .a [x] : |
240254
| test.swift:219:13:219:13 | b [a, x] : | test.swift:185:7:185:7 | self [a, x] : | file://:0:0:0:0 | .a [x] : | test.swift:219:13:219:15 | .a [x] : |
241255
| test.swift:219:13:219:15 | .a [x] : | test.swift:163:7:163:7 | self [x] : | file://:0:0:0:0 | .x : | test.swift:219:13:219:17 | .x |
256+
| test.swift:265:15:265:22 | call to source() : | file://:0:0:0:0 | [summary param] this in signum() : | file://:0:0:0:0 | [summary] to write: return (return) in signum() : | test.swift:265:15:265:31 | call to signum() |
257+
| test.swift:266:15:266:16 | ...? : | file://:0:0:0:0 | [summary param] this in signum() : | file://:0:0:0:0 | [summary] to write: return (return) in signum() : | test.swift:266:15:266:25 | call to signum() : |
242258
#select
243259
| test.swift:7:15:7:15 | t1 | test.swift:6:19:6:26 | call to source() : | test.swift:7:15:7:15 | t1 | result |
244260
| test.swift:9:15:9:15 | t1 | test.swift:6:19:6:26 | call to source() : | test.swift:9:15:9:15 | t1 | result |
@@ -269,3 +285,5 @@ subpaths
269285
| test.swift:235:13:235:15 | .source_value | test.swift:225:14:225:21 | call to source() : | test.swift:235:13:235:15 | .source_value | result |
270286
| test.swift:238:13:238:15 | .source_value | test.swift:225:14:225:21 | call to source() : | test.swift:238:13:238:15 | .source_value | result |
271287
| test.swift:264:15:264:16 | ...! | test.swift:259:12:259:19 | call to source() : | test.swift:264:15:264:16 | ...! | result |
288+
| test.swift:265:15:265:31 | call to signum() | test.swift:265:15:265:22 | call to source() : | test.swift:265:15:265:31 | call to signum() | result |
289+
| test.swift:266:15:266:25 | OptionalEvaluationExpr | test.swift:259:12:259:19 | call to source() : | test.swift:266:15:266:25 | OptionalEvaluationExpr | result |

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,8 @@ func optionalSource() -> Int? {
262262
func test_optionals() {
263263
let x = optionalSource()
264264
sink(arg: x!) // $ flow=259
265-
sink(arg: source().signum()) // $ MISSING: flow=259
266-
sink(opt: x?.signum()) // $ MISSING: flow=259
265+
sink(arg: source().signum()) // $ flow=265
266+
sink(opt: x?.signum()) // $ flow=259
267267
sink(arg: x ?? 0) // $ MISSING: flow=259
268268
if let y = x {
269269
sink(arg: y) // $ MISSING: flow=259

0 commit comments

Comments
 (0)