Merge pull request github#12133 from d10c/swift/case-let-dataflow

Swift: `case let` dataflow

Author: MathiasVP

swift/ql/.generated.list

@@ -232,7 +232,6 @@ ql/lib/codeql/swift/elements/pattern/IsPatternConstructor.qll 209ad40227f49dfe1b
 ql/lib/codeql/swift/elements/pattern/NamedPatternConstructor.qll 437ccf0a28c204a83861babc91e0e422846630f001554a3d7764b323c8632a26 91c52727ccf6b035cc1f0c2ca1eb91482ef28fa874bca382fb34f9226c31c84e
 ql/lib/codeql/swift/elements/pattern/OptionalSomePatternConstructor.qll bc33a81415edfa4294ad9dfb57f5aa929ea4d7f21a013f145949352009a93975 9fb2afa86dc9cedd28af9f27543ea8babf431a4ba48e9941bcd484b9aa0aeab5
 ql/lib/codeql/swift/elements/pattern/ParenPatternConstructor.qll 7229439aac7010dbb82f2aaa48aedf47b189e21cc70cb926072e00faa8358369 341cfacd838185178e95a2a7bb29f198e46954098f6d13890351a82943969809
-ql/lib/codeql/swift/elements/pattern/Pattern.qll a5cee4c72446f884107acc248a10b098871dc027513452c569294aaeecbc5890 cbecbc4b2d9bea7b571b9d184a0bb8cf472f66106e96d4f70e0e98d627f269a5
 ql/lib/codeql/swift/elements/pattern/TuplePatternConstructor.qll 208fe1f6af1eb569ea4cd5f76e8fbe4dfab9a6264f6c12b762f074a237934bdc 174c55ad1bf3058059ed6c3c3502d6099cda95fbfce925cfd261705accbddbcd
 ql/lib/codeql/swift/elements/pattern/TypedPatternConstructor.qll 1befdd0455e94d4daa0332b644b74eae43f98bab6aab7491a37176a431c36c34 e574ecf38aac7d9381133bfb894da8cb96aec1c933093f4f7cc951dba8152570
 ql/lib/codeql/swift/elements/stmt/BraceStmtConstructor.qll eb2b4817d84da4063eaa0b95fe22131cc980c761dcf41f1336566e95bc28aae4 c8e5f7fecd01a7724d8f58c2cd8c845264be91252281f37e3eb20f4a6f421c72

swift/ql/lib/codeql/swift/controlflow/CfgNodes.qll

@@ -118,6 +118,16 @@ class ExprCfgNode extends CfgNode {
   Expr getExpr() { result = e }
 }
 
+/** A control-flow node that wraps a pattern. */
+class PatternCfgNode extends CfgNode {
+  Pattern p;
+
+  PatternCfgNode() { p = this.getNode().asAstNode() }
+
+  /** Gets the underlying pattern. */
+  Pattern getPattern() { result = p }
+}
+
 /** A control-flow node that wraps a property getter. */
 class PropertyGetterCfgNode extends CfgNode {
   override PropertyGetterElement n;

swift/ql/lib/codeql/swift/controlflow/internal/Completion.qll

@@ -185,7 +185,7 @@ private predicate switchMatching(SwitchStmt switch, CaseStmt c, AstNode ast) {
   (
     c.getALabel() = ast
     or
-    isSubPattern+(c.getALabel().getPattern(), ast)
+    ast.(Pattern).getImmediateEnclosingPattern+() = c.getALabel().getPattern()
   )
 }
 
@@ -216,27 +216,10 @@ predicate catchMatchingPattern(DoCatchStmt s, CaseStmt c, Pattern pattern) {
   exists(CaseLabelItem cli | catchMatching(s, c, cli) |
     cli.getPattern() = pattern
     or
-    isSubPattern+(cli.getPattern(), pattern)
+    pattern.getImmediateEnclosingPattern+() = cli.getPattern()
   )
 }
 
-/** Holds if `sub` is a subpattern of `p`. */
-private predicate isSubPattern(Pattern p, Pattern sub) {
-  sub = p.(BindingPattern).getSubPattern().getFullyUnresolved()
-  or
-  sub = p.(EnumElementPattern).getSubPattern().getFullyUnresolved()
-  or
-  sub = p.(IsPattern).getSubPattern().getFullyUnresolved()
-  or
-  sub = p.(OptionalSomePattern).getFullyUnresolved()
-  or
-  sub = p.(ParenPattern).getResolveStep()
-  or
-  sub = p.(TuplePattern).getAnElement().getFullyUnresolved()
-  or
-  sub = p.(TypedPattern).getSubPattern().getFullyUnresolved()
-}
-
 /** Gets the value of `e` if it is a constant value, disregarding conversions. */
 private string getExprValue(Expr e) {
   result = e.(IntegerLiteralExpr).getStringValue()
@@ -260,8 +243,6 @@ private string getPatternValue(Pattern p) {
 private predicate isIrrefutableMatch(Pattern p) {
   (p instanceof NamedPattern or p instanceof AnyPattern)
   or
-  isIrrefutableMatch(p.(BindingPattern).getSubPattern().getFullyUnresolved())
-  or
   isIrrefutableMatch(p.(TypedPattern).getSubPattern().getFullyUnresolved())
   or
   // A pattern hidden underneath a conversion is also an irrefutable pattern.

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

@@ -897,21 +897,12 @@ module Patterns {
     }
   }
 
-  private class BindingTree extends AstPostOrderTree {
+  private class BindingTree extends AstStandardPostOrderTree {
     override BindingPattern ast;
 
-    final override predicate propagatesAbnormal(ControlFlowElement n) {
-      n.asAstNode() = ast.getSubPattern().getFullyUnresolved()
-    }
-
-    final override predicate first(ControlFlowElement n) {
-      astFirst(ast.getSubPattern().getFullyUnresolved(), n)
-    }
-
-    override predicate succ(ControlFlowElement pred, ControlFlowElement succ, Completion c) {
-      astLast(ast.getSubPattern().getFullyUnresolved(), pred, c) and
-      c.(MatchingCompletion).isMatch() and
-      succ.asAstNode() = ast
+    final override ControlFlowElement getChildElement(int i) {
+      i = 0 and
+      result.asAstNode() = ast.getResolveStep()
     }
   }
 

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

@@ -38,9 +38,9 @@ module Ssa {
       // if let x5 = optional { ... }
       // guard let x6 = optional else { ... }
       // ```
-      exists(Pattern pattern |
+      exists(NamedPattern pattern |
         bb.getNode(i).getNode().asAstNode() = pattern and
-        v.getParentPattern() = pattern and
+        v = pattern.getVarDecl() and
         certain = true
       )
       or
@@ -153,14 +153,10 @@ module Ssa {
         value.getNode().asAstNode() = a.getSource()
       )
       or
-      exists(
-        VarDecl var, SsaInput::BasicBlock bb, int blockIndex, PatternBindingDecl pbd, Expr init
-      |
-        this.definesAt(var, bb, blockIndex) and
-        pbd.getAPattern() = bb.getNode(blockIndex).getNode().asAstNode() and
-        init = var.getParentInitializer()
-      |
-        value.getAst() = init
+      exists(SsaInput::BasicBlock bb, int blockIndex, NamedPattern np |
+        this.definesAt(_, bb, blockIndex) and
+        np = bb.getNode(blockIndex).getNode().asAstNode() and
+        value.getNode().asAstNode() = np
       )
       or
       exists(SsaInput::BasicBlock bb, int blockIndex, ConditionElement ce, Expr init |

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

@@ -40,6 +40,14 @@ private class ExprNodeImpl extends ExprNode, NodeImpl {
   override DataFlowCallable getEnclosingCallable() { result = TDataFlowFunc(n.getScope()) }
 }
 
+private class PatternNodeImpl extends PatternNode, NodeImpl {
+  override Location getLocationImpl() { result = pattern.getLocation() }
+
+  override string toStringImpl() { result = pattern.toString() }
+
+  override DataFlowCallable getEnclosingCallable() { result = TDataFlowFunc(n.getScope()) }
+}
+
 private class SsaDefinitionNodeImpl extends SsaDefinitionNode, NodeImpl {
   override Location getLocationImpl() { result = def.getLocation() }
 
@@ -63,6 +71,7 @@ private module Cached {
   cached
   newtype TNode =
     TExprNode(CfgNode n, Expr e) { hasExprNode(n, e) } or
+    TPatternNode(CfgNode n, Pattern p) { hasPatternNode(n, p) } or
     TSsaDefinitionNode(Ssa::Definition def) or
     TInoutReturnNode(ParamDecl param) { modifiableParam(param) } or
     TSummaryNode(FlowSummary::SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state) {
@@ -175,6 +184,20 @@ private module Cached {
       nodeFrom.asExpr() = ie.getBranch(_)
     )
     or
+    // flow from Expr to Pattern
+    exists(Expr e, Pattern p |
+      nodeFrom.asExpr() = e and
+      nodeTo.asPattern() = p and
+      p.getImmediateMatchingExpr() = e
+    )
+    or
+    // flow from Pattern to an identity-preserving sub-Pattern:
+    nodeTo.asPattern() =
+      [
+        nodeFrom.asPattern().(IsPattern).getSubPattern(),
+        nodeFrom.asPattern().(TypedPattern).getSubPattern()
+      ]
+    or
     // flow through a flow summary (extension of `SummaryModelCsv`)
     FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom, nodeTo, true)
   }
@@ -201,7 +224,8 @@ private module Cached {
   cached
   newtype TContent =
     TFieldContent(FieldDecl f) or
-    TTupleContent(int index) { exists(any(TupleExpr te).getElement(index)) }
+    TTupleContent(int index) { exists(any(TupleExpr te).getElement(index)) } or
+    TEnumContent(ParamDecl f) { exists(EnumElementDecl d | d.getAParam() = f) }
 }
 
 /**
@@ -230,6 +254,11 @@ private predicate hasExprNode(CfgNode n, Expr e) {
   n.(PropertyObserverCfgNode).getAssignExpr() = e
 }
 
+private predicate hasPatternNode(PatternCfgNode n, Pattern p) {
+  n.getPattern() = p and
+  p = p.resolve() // no need to turn hidden-AST patterns (`let`s, parens) into data flow nodes
+}
+
 import Cached
 
 /** Holds if `n` should be hidden from path explanations. */
@@ -558,6 +587,29 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
     c.isSingleton(any(Content::TupleContent tc | tc.getIndex() = tuple.getIndex()))
   )
   or
+  // creation of an enum `.variant(v1, v2)`
+  exists(EnumElementExpr enum, int pos |
+    node1.asExpr() = enum.getArgument(pos).getExpr() and
+    node2.asExpr() = enum and
+    c.isSingleton(any(Content::EnumContent ec | ec.getParam() = enum.getElement().getParam(pos)))
+  )
+  or
+  // creation of an optional via implicit conversion,
+  // i.e. from `f(x)` where `x: T` into `f(.some(x))` where the context `f` expects a `T?`.
+  exists(InjectIntoOptionalExpr e |
+    e.convertsFrom(node1.asExpr()) and
+    node2 = node1 and // HACK: we should ideally have a separate Node case for the (hidden) InjectIntoOptionalExpr
+    c instanceof OptionalSomeContentSet
+  )
+  or
+  // creation of an optional by returning from a failable initializer (`init?`)
+  exists(ConstructorDecl init |
+    node1.asExpr().(CallExpr).getStaticTarget() = init and
+    node2 = node1 and // HACK: again, we should ideally have a separate Node case here, and not reuse the CallExpr
+    c instanceof OptionalSomeContentSet and
+    init.isFailable()
+  )
+  or
   FlowSummaryImpl::Private::Steps::summaryStoreStep(node1, c, node2)
 }
 
@@ -578,6 +630,34 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
     node2.asExpr() = tuple and
     c.isSingleton(any(Content::TupleContent tc | tc.getIndex() = tuple.getIndex()))
   )
+  or
+  // read of an enum member via `case let .variant(v1, v2)` pattern matching
+  exists(EnumElementPattern enumPat, ParamDecl enumParam, Pattern subPat |
+    node1.asPattern() = enumPat and
+    node2.asPattern() = subPat and
+    c.isSingleton(any(Content::EnumContent ec | ec.getParam() = enumParam))
+  |
+    exists(int idx |
+      enumPat.getElement().getParam(idx) = enumParam and
+      enumPat.getSubPattern(idx) = subPat
+    )
+  )
+  or
+  // read of a tuple member via `case let (v1, v2)` pattern matching
+  exists(TuplePattern tupPat, int idx, Pattern subPat |
+    node1.asPattern() = tupPat and
+    node2.asPattern() = subPat and
+    c.isSingleton(any(Content::TupleContent tc | tc.getIndex() = idx))
+  |
+    tupPat.getElement(idx) = subPat
+  )
+  or
+  // read of an optional .some member via `let x: T = y: T?` pattern matching
+  exists(OptionalSomePattern pat |
+    node1.asPattern() = pat and
+    node2.asPattern() = pat.getSubPattern() and
+    c instanceof OptionalSomeContentSet
+  )
 }
 
 /**
@@ -595,6 +675,22 @@ predicate clearsContent(Node n, ContentSet c) {
  */
 predicate expectsContent(Node n, ContentSet c) { none() }
 
+/**
+ * The global singleton `Optional.some` content set.
+ */
+private class OptionalSomeContentSet extends ContentSet {
+  OptionalSomeContentSet() {
+    exists(EnumDecl optional, EnumElementDecl some |
+      some.getDeclaringDecl() = optional and
+      some.getName() = "some" and
+      optional.getName() = "Optional" and
+      optional.getModule().getName() = "Swift"
+    |
+      this.isSingleton(any(Content::EnumContent ec | ec.getParam() = some.getParam(0)))
+    )
+  }
+}
+
 private newtype TDataFlowType = TODO_DataFlowType()
 
 class DataFlowType extends TDataFlowType {

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

@@ -37,6 +37,11 @@ class Node extends TNode {
    */
   Expr asExpr() { none() }
 
+  /**
+   * Gets this node's underlying pattern, if any.
+   */
+  Pattern asPattern() { none() }
+
   /**
    * Gets this data flow node's corresponding control flow node.
    */
@@ -66,6 +71,20 @@ class ExprNode extends Node, TExprNode {
   override ControlFlowNode getCfgNode() { result = n }
 }
 
+/**
+ * A pattern, viewed as a node in a data flow graph.
+ */
+class PatternNode extends Node, TPatternNode {
+  CfgNode n;
+  Pattern pattern;
+
+  PatternNode() { this = TPatternNode(n, pattern) }
+
+  override Pattern asPattern() { result = pattern }
+
+  override ControlFlowNode getCfgNode() { result = n }
+}
+
 /**
  * The value of a parameter at function entry, viewed as a node in a data
  * flow graph.
@@ -171,6 +190,20 @@ module Content {
 
     override string toString() { result = "Tuple element at index " + index.toString() }
   }
+
+  /** A parameter of an enum element. */
+  class EnumContent extends Content, TEnumContent {
+    private ParamDecl p;
+
+    EnumContent() { this = TEnumContent(p) }
+
+    /** Gets the declaration of the enum parameter. */
+    ParamDecl getParam() { result = p }
+
+    override string toString() {
+      exists(EnumElementDecl d, int pos | d.getParam(pos) = p | result = d.toString() + ":" + pos)
+    }
+  }
 }
 
 /**

swift/ql/lib/codeql/swift/elements/decl/ConstructorDecl.qll

@@ -1,9 +1,17 @@
 private import codeql.swift.generated.decl.ConstructorDecl
 private import codeql.swift.elements.decl.MethodDecl
+private import codeql.swift.elements.type.FunctionType
+private import codeql.swift.elements.type.OptionalType
 
 /**
  * An initializer of a class, struct, enum or protocol.
  */
 class ConstructorDecl extends Generated::ConstructorDecl, MethodDecl {
   override string toString() { result = this.getSelfParam().getType() + "." + super.toString() }
+
+  /** Holds if this initializer returns an optional type. Failable initializers are written as `init?`. */
+  predicate isFailable() {
+    this.getInterfaceType().(FunctionType).getResult().(FunctionType).getResult() instanceof
+      OptionalType
+  }
 }