Skip to content

Commit 89aa86a

Browse files
MathiasVPMathiasVP
authored
Merge pull request github#13741 from rdmarsh2/rdmarsh2/swift/array-content-flow
Swift: add DataFlow::Content for arrays
2 parents ff5409f + 22ae430 commit 89aa86a

File tree

23 files changed

+801
-550
lines changed

23 files changed

+801
-550
lines changed

‎swift/ql/lib/change-notes/2023-07-18-array-content.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: majorAnalysis
3+
---
4+
* Added `DataFlow::ArrayContent`, which will provide more accurate flow through arrays.

‎swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,14 +1235,6 @@ module Exprs {
12351235
}
12361236
}
12371237

1238-
private class InOutTree extends AstStandardPostOrderTree {
1239-
override InOutExpr ast;
1240-
1241-
final override ControlFlowElement getChildElement(int i) {
1242-
i = 0 and result.asAstNode() = ast.getSubExpr().getFullyConverted()
1243-
}
1244-
}
1245-
12461238
private class SubscriptTree extends AstControlFlowTree {
12471239
override SubscriptExpr ast;
12481240

@@ -1749,7 +1741,8 @@ module Exprs {
17491741

17501742
module Conversions {
17511743
class ConversionOrIdentity =
1752-
Synth::TIdentityExpr or Synth::TExplicitCastExpr or Synth::TImplicitConversionExpr;
1744+
Synth::TIdentityExpr or Synth::TExplicitCastExpr or Synth::TImplicitConversionExpr or
1745+
Synth::TInOutExpr;
17531746

17541747
abstract class ConversionOrIdentityTree extends AstStandardPostOrderTree {
17551748
ConversionOrIdentityTree() { ast instanceof ConversionOrIdentity }
@@ -1780,6 +1773,12 @@ module Exprs {
17801773

17811774
override predicate convertsFrom(Expr e) { ast.convertsFrom(e) }
17821775
}
1776+
1777+
private class InOutTree extends ConversionOrIdentityTree {
1778+
override InOutExpr ast;
1779+
1780+
override predicate convertsFrom(Expr e) { ast.convertsFrom(e) }
1781+
}
17831782
}
17841783
}
17851784

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -486,8 +486,12 @@ private predicate parseEnum(AccessPathToken c, Content::EnumContent f) {
486486

487487
/** Holds if the specification component parses as a `Content`. */
488488
predicate parseContent(AccessPathToken component, Content content) {
489-
parseField(component, content) or
489+
parseField(component, content)
490+
or
490491
parseEnum(component, content)
492+
or
493+
component.getName() = "ArrayElement" and
494+
content instanceof Content::ArrayContent
491495
}
492496

493497
cached

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

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ private import codeql.swift.controlflow.BasicBlocks
88
private import codeql.swift.dataflow.FlowSummary as FlowSummary
99
private import codeql.swift.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
1010
private import codeql.swift.frameworks.StandardLibrary.PointerTypes
11+
private import codeql.swift.frameworks.StandardLibrary.Array
1112

1213
/** Gets the callable in which this node occurs. */
1314
DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.(NodeImpl).getEnclosingCallable() }
@@ -110,7 +111,8 @@ private module Cached {
110111
hasExprNode(n,
111112
[
112113
any(Argument arg | modifiable(arg)).getExpr(), any(MemberRefExpr ref).getBase(),
113-
any(ApplyExpr apply).getQualifier(), any(TupleElementExpr te).getSubExpr()
114+
any(ApplyExpr apply).getQualifier(), any(TupleElementExpr te).getSubExpr(),
115+
any(SubscriptExpr se).getBase()
114116
])
115117
}
116118

@@ -166,6 +168,10 @@ private module Cached {
166168
// flow through `&` (inout argument)
167169
nodeFrom.asExpr() = nodeTo.asExpr().(InOutExpr).getSubExpr()
168170
or
171+
// reverse flow through `&` (inout argument)
172+
nodeFrom.(PostUpdateNode).getPreUpdateNode().asExpr().(InOutExpr).getSubExpr() =
173+
nodeTo.(PostUpdateNode).getPreUpdateNode().asExpr()
174+
or
169175
// flow through `try!` and similar constructs
170176
nodeFrom.asExpr() = nodeTo.asExpr().(AnyTryExpr).getSubExpr()
171177
or
@@ -249,7 +255,8 @@ private module Cached {
249255
newtype TContent =
250256
TFieldContent(FieldDecl f) or
251257
TTupleContent(int index) { exists(any(TupleExpr te).getElement(index)) } or
252-
TEnumContent(ParamDecl f) { exists(EnumElementDecl d | d.getAParam() = f) }
258+
TEnumContent(ParamDecl f) { exists(EnumElementDecl d | d.getAParam() = f) } or
259+
TArrayContent()
253260
}
254261

255262
/**
@@ -695,6 +702,22 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
695702
init.isFailable()
696703
)
697704
or
705+
// creation of an array `[v1,v2]`
706+
exists(ArrayExpr arr |
707+
node1.asExpr() = arr.getAnElement() and
708+
node2.asExpr() = arr and
709+
c.isSingleton(any(Content::ArrayContent ac))
710+
)
711+
or
712+
// array assignment `a[n] = x`
713+
exists(AssignExpr assign, SubscriptExpr subscript |
714+
node1.asExpr() = assign.getSource() and
715+
node2.(PostUpdateNode).getPreUpdateNode().asExpr() = subscript.getBase() and
716+
subscript = assign.getDest() and
717+
subscript.getBase().getType() instanceof ArrayType and
718+
c.isSingleton(any(Content::ArrayContent ac))
719+
)
720+
or
698721
FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), c,
699722
node2.(FlowSummaryNode).getSummaryNode())
700723
}
@@ -750,10 +773,14 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
750773
)
751774
or
752775
// read of a component in a key-path expression chain
753-
exists(KeyPathComponent component, FieldDecl f |
776+
exists(KeyPathComponent component |
754777
component = node1.(KeyPathComponentNodeImpl).getComponent() and
755-
f = component.getDeclRef() and
756-
c.isSingleton(any(Content::FieldContent ct | ct.getField() = f))
778+
(
779+
c.isSingleton(any(Content::FieldContent ct | ct.getField() = component.getDeclRef()))
780+
or
781+
c.isSingleton(any(Content::ArrayContent ac)) and
782+
component.isSubscript()
783+
)
757784
|
758785
// the next node is either the next element in the chain
759786
node2.(KeyPathComponentNodeImpl).getComponent() = component.getNextComponent()
@@ -762,6 +789,14 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
762789
not exists(component.getNextComponent()) and
763790
node2.(KeyPathReturnNodeImpl).getKeyPathExpr() = component.getKeyPathExpr()
764791
)
792+
or
793+
// read of an array member via subscript operator
794+
exists(SubscriptExpr subscript |
795+
subscript.getBase() = node1.asExpr() and
796+
subscript = node2.asExpr() and
797+
subscript.getBase().getType() instanceof ArrayType and
798+
c.isSingleton(any(Content::ArrayContent ac))
799+
)
765800
}
766801

767802
/**
@@ -863,7 +898,7 @@ class DataFlowExpr = Expr;
863898
* Holds if access paths with `c` at their head always should be tracked at high
864899
* precision. This disables adaptive access path precision for such access paths.
865900
*/
866-
predicate forceHighPrecision(Content c) { none() }
901+
predicate forceHighPrecision(Content c) { c instanceof Content::ArrayContent }
867902

868903
/**
869904
* Holds if the node `n` is unreachable when the call context is `call`.

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,11 @@ module Content {
219219

220220
override string toString() { result = this.getSignature() }
221221
}
222+
223+
/** An element of an array at an unknown index */
224+
class ArrayContent extends Content, TArrayContent {
225+
override string toString() { result = "Array element" }
226+
}
222227
}
223228

224229
/**

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,11 @@ private string getContentSpecific(ContentSet cs) {
109109
cs.isSingleton(c) and
110110
result = "Field[" + c.getField().getName() + "]"
111111
)
112+
or
113+
exists(Content::ArrayContent c |
114+
cs.isSingleton(c) and
115+
result = "ArrayElement"
116+
)
112117
}
113118

114119
/** Gets the textual representation of a summary component in the format used for MaD models. */

‎swift/ql/lib/codeql/swift/elements/expr/InOutExpr.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,6 @@ private import codeql.swift.generated.expr.InOutExpr
22

33
class InOutExpr extends Generated::InOutExpr {
44
override string toString() { result = "&..." }
5+
6+
override predicate convertsFrom(Expr e) { e = this.getImmediateSubExpr() }
57
}

‎swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Array.qll

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* Provides models for `Array` and related Swift classes.
3+
*/
4+
5+
import swift
6+
private import codeql.swift.dataflow.ExternalFlow
7+
8+
/**
9+
* An instance of the `Array` type.
10+
*/
11+
class ArrayType extends Type {
12+
ArrayType() { this.getName().matches("Array<%") or this.getName().matches("[%]") }
13+
}
14+
15+
/**
16+
* A model for `Array` and related class members that permit data flow.
17+
*/
18+
private class ArraySummaries extends SummaryModelCsv {
19+
override predicate row(string row) {
20+
row =
21+
[
22+
";Array;true;insert(_:at:);;;Argument[0];Argument[-1].ArrayElement;value",
23+
";Array;true;insert(_:at:);;;Argument[1];Argument[-1];taint"
24+
]
25+
}
26+
}

‎swift/ql/lib/codeql/swift/frameworks/StandardLibrary/StandardLibrary.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* This file imports all models related to the Swift standard library.
33
*/
44

5+
private import Array
56
private import Collection
67
private import CustomUrlSchemes
78
private import Data

‎swift/ql/test/extractor-tests/generated/decl/CapturedDecl/PrintAst.expected

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ closures.swift:
7272
# 17| getBase(): [TypeExpr] Int.Type
7373
# 17| getTypeRepr(): [TypeRepr] Int
7474
# 17| getMethodRef(): [DeclRefExpr] +=(_:_:)
75-
# 17| getArgument(0): [Argument] : &...
76-
# 17| getExpr(): [InOutExpr] &...
77-
# 17| getSubExpr(): [DeclRefExpr] x
75+
# 17| getArgument(0): [Argument] : x
76+
# 17| getExpr(): [DeclRefExpr] x
77+
# 17| getExpr().getFullyConverted(): [InOutExpr] &...
7878
# 17| getArgument(1): [Argument] : 1
7979
# 17| getExpr(): [IntegerLiteralExpr] 1
8080
# 18| getElement(1): [CallExpr] call to print(_:separator:terminator:)
@@ -292,9 +292,9 @@ closures.swift:
292292
# 52| getBase(): [TypeExpr] Int.Type
293293
# 52| getTypeRepr(): [TypeRepr] Int
294294
# 52| getMethodRef(): [DeclRefExpr] +=(_:_:)
295-
# 52| getArgument(0): [Argument] : &...
296-
# 52| getExpr(): [InOutExpr] &...
297-
# 52| getSubExpr(): [DeclRefExpr] x
295+
# 52| getArgument(0): [Argument] : x
296+
# 52| getExpr(): [DeclRefExpr] x
297+
# 52| getExpr().getFullyConverted(): [InOutExpr] &...
298298
# 52| getArgument(1): [Argument] : y
299299
# 52| getExpr(): [DeclRefExpr] y
300300
# 52| getCapture(0): [CapturedDecl] x
@@ -304,9 +304,9 @@ closures.swift:
304304
# 53| getBase(): [TypeExpr] Int.Type
305305
# 53| getTypeRepr(): [TypeRepr] Int
306306
# 53| getMethodRef(): [DeclRefExpr] +=(_:_:)
307-
# 53| getArgument(0): [Argument] : &...
308-
# 53| getExpr(): [InOutExpr] &...
309-
# 53| getSubExpr(): [DeclRefExpr] x
307+
# 53| getArgument(0): [Argument] : x
308+
# 53| getExpr(): [DeclRefExpr] x
309+
# 53| getExpr().getFullyConverted(): [InOutExpr] &...
310310
# 53| getArgument(1): [Argument] : 40
311311
# 53| getExpr(): [IntegerLiteralExpr] 40
312312
# 54| getElement(3): [PatternBindingDecl] var ... = ...
@@ -351,9 +351,9 @@ closures.swift:
351351
# 61| getBase(): [TypeExpr] Int.Type
352352
# 61| getTypeRepr(): [TypeRepr] Int
353353
# 61| getMethodRef(): [DeclRefExpr] +=(_:_:)
354-
# 61| getArgument(0): [Argument] : &...
355-
# 61| getExpr(): [InOutExpr] &...
356-
# 61| getSubExpr(): [DeclRefExpr] x
354+
# 61| getArgument(0): [Argument] : x
355+
# 61| getExpr(): [DeclRefExpr] x
356+
# 61| getExpr().getFullyConverted(): [InOutExpr] &...
357357
# 61| getArgument(1): [Argument] : y
358358
# 61| getExpr(): [DeclRefExpr] y
359359
# 61| getCapture(0): [CapturedDecl] x
@@ -363,9 +363,9 @@ closures.swift:
363363
# 62| getBase(): [TypeExpr] Int.Type
364364
# 62| getTypeRepr(): [TypeRepr] Int
365365
# 62| getMethodRef(): [DeclRefExpr] +=(_:_:)
366-
# 62| getArgument(0): [Argument] : &...
367-
# 62| getExpr(): [InOutExpr] &...
368-
# 62| getSubExpr(): [DeclRefExpr] x
366+
# 62| getArgument(0): [Argument] : x
367+
# 62| getExpr(): [DeclRefExpr] x
368+
# 62| getExpr().getFullyConverted(): [InOutExpr] &...
369369
# 62| getArgument(1): [Argument] : 40
370370
# 62| getExpr(): [IntegerLiteralExpr] 40
371371
# 63| getElement(3): [PatternBindingDecl] var ... = ...
@@ -417,9 +417,9 @@ closures.swift:
417417
# 71| getBase(): [TypeExpr] Int.Type
418418
# 71| getTypeRepr(): [TypeRepr] Int
419419
# 71| getMethodRef(): [DeclRefExpr] +=(_:_:)
420-
# 71| getArgument(0): [Argument] : &...
421-
# 71| getExpr(): [InOutExpr] &...
422-
# 71| getSubExpr(): [DeclRefExpr] x
420+
# 71| getArgument(0): [Argument] : x
421+
# 71| getExpr(): [DeclRefExpr] x
422+
# 71| getExpr().getFullyConverted(): [InOutExpr] &...
423423
# 71| getArgument(1): [Argument] : y
424424
# 71| getExpr(): [DeclRefExpr] y
425425
# 71| getCapture(0): [CapturedDecl] x
@@ -429,9 +429,9 @@ closures.swift:
429429
# 72| getBase(): [TypeExpr] Int.Type
430430
# 72| getTypeRepr(): [TypeRepr] Int
431431
# 72| getMethodRef(): [DeclRefExpr] +=(_:_:)
432-
# 72| getArgument(0): [Argument] : &...
433-
# 72| getExpr(): [InOutExpr] &...
434-
# 72| getSubExpr(): [DeclRefExpr] x
432+
# 72| getArgument(0): [Argument] : x
433+
# 72| getExpr(): [DeclRefExpr] x
434+
# 72| getExpr().getFullyConverted(): [InOutExpr] &...
435435
# 72| getArgument(1): [Argument] : 40
436436
# 72| getExpr(): [IntegerLiteralExpr] 40
437437
# 73| getElement(3): [PatternBindingDecl] var ... = ...
@@ -502,9 +502,9 @@ closures.swift:
502502
# 86| getBase(): [TypeExpr] Int.Type
503503
# 86| getTypeRepr(): [TypeRepr] Int
504504
# 86| getMethodRef(): [DeclRefExpr] -=(_:_:)
505-
# 86| getArgument(0): [Argument] : &...
506-
# 86| getExpr(): [InOutExpr] &...
507-
# 86| getSubExpr(): [DeclRefExpr] x
505+
# 86| getArgument(0): [Argument] : x
506+
# 86| getExpr(): [DeclRefExpr] x
507+
# 86| getExpr().getFullyConverted(): [InOutExpr] &...
508508
# 86| getArgument(1): [Argument] : 1
509509
# 86| getExpr(): [IntegerLiteralExpr] 1
510510
# 87| getElement(2): [IfStmt] if ... then { ... }
@@ -563,9 +563,9 @@ closures.swift:
563563
# 94| getBase(): [TypeExpr] Int.Type
564564
# 94| getTypeRepr(): [TypeRepr] Int
565565
# 94| getMethodRef(): [DeclRefExpr] -=(_:_:)
566-
# 94| getArgument(0): [Argument] : &...
567-
# 94| getExpr(): [InOutExpr] &...
568-
# 94| getSubExpr(): [DeclRefExpr] x
566+
# 94| getArgument(0): [Argument] : x
567+
# 94| getExpr(): [DeclRefExpr] x
568+
# 94| getExpr().getFullyConverted(): [InOutExpr] &...
569569
# 94| getArgument(1): [Argument] : 1
570570
# 94| getExpr(): [IntegerLiteralExpr] 1
571571
# 95| getElement(2): [IfStmt] if ... then { ... }
@@ -629,9 +629,9 @@ closures.swift:
629629
# 111| getBase(): [TypeExpr] Int.Type
630630
# 111| getTypeRepr(): [TypeRepr] Int
631631
# 111| getMethodRef(): [DeclRefExpr] +=(_:_:)
632-
# 111| getArgument(0): [Argument] : &...
633-
# 111| getExpr(): [InOutExpr] &...
634-
# 111| getSubExpr(): [DeclRefExpr] x
632+
# 111| getArgument(0): [Argument] : x
633+
# 111| getExpr(): [DeclRefExpr] x
634+
# 111| getExpr().getFullyConverted(): [InOutExpr] &...
635635
# 111| getArgument(1): [Argument] : 1
636636
# 111| getExpr(): [IntegerLiteralExpr] 1
637637
# 111| getCapture(0): [CapturedDecl] x

0 commit comments

Comments
 (0)