Skip to content

Commit 03122d7

Browse files
committed
Swift: fix a bunch of MISSING dataflow test cases
Optional content flow through constructors remains.
1 parent 6a12726 commit 03122d7

File tree

7 files changed

+548
-218
lines changed

7 files changed

+548
-218
lines changed

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ module Ssa {
3838
// if let x5 = optional { ... }
3939
// guard let x6 = optional else { ... }
4040
// ```
41-
exists(Pattern pattern |
41+
exists(NamedPattern pattern |
4242
bb.getNode(i).getNode().asAstNode() = pattern and
43-
v.getParentPattern() = pattern and
43+
v = pattern.getVarDecl() and
4444
certain = true
4545
)
4646
or
@@ -153,14 +153,10 @@ module Ssa {
153153
value.getNode().asAstNode() = a.getSource()
154154
)
155155
or
156-
exists(
157-
VarDecl var, SsaInput::BasicBlock bb, int blockIndex, PatternBindingDecl pbd, Expr init
158-
|
159-
this.definesAt(var, bb, blockIndex) and
160-
pbd.getAPattern() = bb.getNode(blockIndex).getNode().asAstNode() and
161-
init = var.getParentInitializer()
162-
|
163-
value.getAst() = init
156+
exists(SsaInput::BasicBlock bb, int blockIndex, NamedPattern np |
157+
this.definesAt(_, bb, blockIndex) and
158+
np = bb.getNode(blockIndex).getNode().asAstNode() and
159+
value.getNode().asAstNode() = np
164160
)
165161
or
166162
exists(SsaInput::BasicBlock bb, int blockIndex, ConditionElement ce, Expr init |

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

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,16 @@ private module Cached {
184184
nodeFrom.asExpr() = ie.getBranch(_)
185185
)
186186
or
187+
// flow from Expr to Pattern
188+
exists(Expr e, Pattern p |
189+
nodeFrom.asExpr() = e and
190+
nodeTo.asPattern() = p and
191+
p.getImmediateMatchingExpr() = e
192+
)
193+
or
194+
// flow from Pattern to an identity-preserving sub-Pattern:
195+
nodeFrom.asPattern() = nodeTo.asPattern().getIdentityPreservingEnclosingPattern()
196+
or
187197
// flow through a flow summary (extension of `SummaryModelCsv`)
188198
FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom, nodeTo, true)
189199
}
@@ -580,6 +590,13 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
580590
c.isSingleton(any(Content::EnumContent ec | ec.getParam() = enum.getElement().getParam(pos)))
581591
)
582592
or
593+
// creation of an optional via implicit conversion
594+
exists(InjectIntoOptionalExpr e, OptionalSomeDecl someDecl |
595+
e.convertsFrom(node1.asExpr()) and
596+
node2 = node1 and // HACK: we should ideally have a separate Node case for the (hidden) InjectIntoOptionalExpr
597+
c.isSingleton(any(Content::EnumContent ec | ec.getParam() = someDecl.getParam(0)))
598+
)
599+
or
583600
FlowSummaryImpl::Private::Steps::summaryStoreStep(node1, c, node2)
584601
}
585602

@@ -602,18 +619,32 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
602619
)
603620
or
604621
// read of an enum member via `case let .variant(v1, v2)` pattern matching
605-
exists(Expr enumExpr, ParamDecl enumParam, VarDecl boundVar |
606-
node1.asExpr() = enumExpr and
607-
node2.asDefinition().getSourceVariable() = boundVar and
622+
exists(EnumElementPattern enumPat, ParamDecl enumParam, Pattern subPat |
623+
node1.asPattern() = enumPat and
624+
node2.asPattern() = subPat and
608625
c.isSingleton(any(Content::EnumContent ec | ec.getParam() = enumParam))
609626
|
610-
exists(EnumElementPattern enumPat, NamedPattern namePat, int idx |
611-
enumPat.getMatchingExpr() = enumExpr and
627+
exists(int idx |
612628
enumPat.getElement().getParam(idx) = enumParam and
613-
namePat.getImmediateIdentityPreservingEnclosingPattern*() = enumPat.getSubPattern(idx) and
614-
namePat.getVarDecl() = boundVar
629+
enumPat.getSubPattern(idx) = subPat
615630
)
616631
)
632+
or
633+
// read of a tuple member via `case let (v1, v2)` pattern matching
634+
exists(TuplePattern tupPat, int idx, Pattern subPat |
635+
node1.asPattern() = tupPat and
636+
node2.asPattern() = subPat and
637+
c.isSingleton(any(Content::TupleContent tc | tc.getIndex() = idx))
638+
|
639+
tupPat.getElement(idx) = subPat
640+
)
641+
or
642+
// read of an optional .some member via `let x: T = y: T?` pattern matching
643+
exists(OptionalSomePattern pat, OptionalSomeDecl someDecl |
644+
node1.asPattern() = pat and
645+
node2.asPattern() = pat.getSubPattern() and
646+
c.isSingleton(any(Content::EnumContent ec | ec.getParam() = someDecl.getParam(0)))
647+
)
617648
}
618649

619650
/**
@@ -631,6 +662,20 @@ predicate clearsContent(Node n, ContentSet c) {
631662
*/
632663
predicate expectsContent(Node n, ContentSet c) { none() }
633664

665+
/**
666+
* The global singleton `Optional.some` enum element.
667+
*/
668+
private class OptionalSomeDecl extends EnumElementDecl {
669+
OptionalSomeDecl() {
670+
exists(EnumDecl enum |
671+
this.getName() = "some" and
672+
this.getDeclaringDecl() = enum and
673+
enum.getName() = "Optional" and
674+
enum.getModule().getName() = "Swift"
675+
)
676+
}
677+
}
678+
634679
private newtype TDataFlowType = TODO_DataFlowType()
635680

636681
class DataFlowType extends TDataFlowType {

swift/ql/lib/codeql/swift/elements/pattern/Pattern.qll

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ private import codeql.swift.generated.ParentChild
2020

2121
class Pattern extends Generated::Pattern {
2222
/**
23-
* Gets the expression that this pattern is matched against, if any.
23+
* Gets the expression that this top-level pattern is matched against, if any.
2424
*
2525
* For example, in `switch e { case p: ... }`, the pattern `p`
26-
* is matched against the expression `e`.
26+
* is immediately matched against the expression `e`.
2727
*/
28-
Expr getMatchingExpr() {
28+
Expr getImmediateMatchingExpr() {
2929
exists(ConditionElement c |
3030
c.getPattern() = this and
3131
result = c.getInitializer()
@@ -40,6 +40,18 @@ class Pattern extends Generated::Pattern {
4040
v.getPattern(i) = this and
4141
result = v.getInit(i)
4242
)
43+
}
44+
45+
/**
46+
* Gets the expression that this pattern is matched against, if any.
47+
* The expression and the pattern need not be top-level children of
48+
* a pattern-matching construct, but they must match each other syntactically.
49+
*
50+
* For example, in `switch .some(e) { case let .some(p): ... }`, the pattern `p`
51+
* is matched against the expression `e`.
52+
*/
53+
Expr getMatchingExpr() {
54+
result = this.getImmediateMatchingExpr()
4355
or
4456
exists(Pattern p | p.getMatchingExpr() = result |
4557
this = p.(IsPattern).getSubPattern()

0 commit comments

Comments
 (0)