Merge pull request #14577 from MathiasVP/capture-flow-swift

Swift: Add variable-capture flow

swift/ql/lib/change-notes/2023-10-27-variable-capture.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Improved support for flow through captured variables that properly adheres to inter-procedural control flow.

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

@@ -173,7 +173,7 @@ class ApplyExprCfgNode extends ExprCfgNode {
 
  Callable getStaticTarget() { result = e.getStaticTarget() }
 
- Expr getFunction() { result = e.getFunction() }
+ CfgNode getFunction() { result.getAst() = e.getFunction() }
 }
 
 class CallExprCfgNode extends ApplyExprCfgNode {

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

@@ -10,6 +10,7 @@ private import codeql.swift.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
 private import codeql.swift.frameworks.StandardLibrary.PointerTypes
 private import codeql.swift.frameworks.StandardLibrary.Array
 private import codeql.swift.frameworks.StandardLibrary.Dictionary
+private import codeql.dataflow.VariableCapture as VariableCapture
 
 /** Gets the callable in which this node occurs. */
 DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.(NodeImpl).getEnclosingCallable() }
@@ -98,6 +99,16 @@ private class SsaDefinitionNodeImpl extends SsaDefinitionNode, NodeImpl {
  }
 }
 
+private class CaptureNodeImpl extends CaptureNode, NodeImpl {
+ override Location getLocationImpl() { result = this.getSynthesizedCaptureNode().getLocation() }
+
+ override string toStringImpl() { result = this.getSynthesizedCaptureNode().toString() }
+
+ override DataFlowCallable getEnclosingCallable() {
+ result.asSourceCallable() = this.getSynthesizedCaptureNode().getEnclosingCallable()
+ }
+}
+
 private predicate localFlowSsaInput(Node nodeFrom, Ssa::Definition def, Ssa::Definition next) {
  exists(BasicBlock bb, int i | def.lastRefRedef(bb, i, next) |
  def.definesAt(_, bb, i) and
@@ -140,19 +151,22 @@ private module Cached {
  [
  any(Argument arg | modifiable(arg)).getExpr(), any(MemberRefExpr ref).getBase(),
  any(ApplyExpr apply).getQualifier(), any(TupleElementExpr te).getSubExpr(),
- any(SubscriptExpr se).getBase()
+ any(SubscriptExpr se).getBase(),
+ any(ApplyExpr apply | not exists(apply.getStaticTarget())).getFunction()
  ])
  } or
  TDictionarySubscriptNode(SubscriptExpr e) {
  e.getBase().getType().getCanonicalType() instanceof CanonicalDictionaryType
- }
+ } or
+ TCaptureNode(CaptureFlow::SynthesizedCaptureNode cn) or
+ TClosureSelfParameterNode(ClosureExpr closure)
 
  private predicate localSsaFlowStepUseUse(Ssa::Definition def, Node nodeFrom, Node nodeTo) {
  def.adjacentReadPair(nodeFrom.getCfgNode(), nodeTo.getCfgNode()) and
  (
- nodeTo instanceof InoutReturnNode
+ nodeTo instanceof InoutReturnNodeImpl
  implies
- nodeTo.(InoutReturnNode).getParameter() = def.getSourceVariable().asVarDecl()
+ nodeTo.(InoutReturnNodeImpl).getParameter() = def.getSourceVariable().asVarDecl()
  )
  }
 
@@ -167,7 +181,7 @@ private module Cached {
  * Holds if `nodeFrom` is a parameter node, and `nodeTo` is a corresponding SSA node.
  */
  private predicate localFlowSsaParamInput(Node nodeFrom, Node nodeTo) {
- nodeTo = getParameterDefNode(nodeFrom.(ParameterNode).getParameter())
+ nodeTo = getParameterDefNode(nodeFrom.asParameter())
  }
 
  private predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) {
@@ -180,9 +194,9 @@ private module Cached {
  nodeFrom.asDefinition() = def and
  nodeTo.getCfgNode() = def.getAFirstRead() and
  (
- nodeTo instanceof InoutReturnNode
+ nodeTo instanceof InoutReturnNodeImpl
  implies
- nodeTo.(InoutReturnNode).getParameter() = def.getSourceVariable().asVarDecl()
+ nodeTo.(InoutReturnNodeImpl).getParameter() = def.getSourceVariable().asVarDecl()
  )
  or
  // use-use flow
@@ -279,6 +293,9 @@ private module Cached {
  // flow through a flow summary (extension of `SummaryModelCsv`)
  FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom.(FlowSummaryNode).getSummaryNode(),
  nodeTo.(FlowSummaryNode).getSummaryNode(), true)
+ or
+ // flow step according to the closure capture library
+ captureValueStep(nodeFrom, nodeTo)
  }
 
  /**
@@ -305,7 +322,8 @@ private module Cached {
  TFieldContent(FieldDecl f) or
  TTupleContent(int index) { exists(any(TupleExpr te).getElement(index)) } or
  TEnumContent(ParamDecl f) { exists(EnumElementDecl d | d.getAParam() = f) } or
- TCollectionContent()
+ TCollectionContent() or
+ TCapturedVariableContent(CapturedVariable v)
 }
 
 /**
@@ -342,7 +360,9 @@ private predicate hasPatternNode(PatternCfgNode n, Pattern p) {
 import Cached
 
 /** Holds if `n` should be hidden from path explanations. */
-predicate nodeIsHidden(Node n) { n instanceof FlowSummaryNode }
+predicate nodeIsHidden(Node n) {
+ n instanceof FlowSummaryNode or n instanceof ClosureSelfParameterNode
+}
 
 /**
  * The intermediate node for a dictionary subscript operation `dict[key]`. In a write, this is used
@@ -396,6 +416,25 @@ private module ParameterNodes {
  override ParamDecl getParameter() { result = param }
  }
 
+ class ClosureSelfParameterNode extends ParameterNodeImpl, TClosureSelfParameterNode {
+ ClosureExpr closure;
+
+ ClosureSelfParameterNode() { this = TClosureSelfParameterNode(closure) }
+
+ override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
+ c.asSourceCallable() = closure and
+ pos instanceof TThisParameter
+ }
+
+ override Location getLocationImpl() { result = closure.getLocation() }
+
+ override string toStringImpl() { result = "closure self parameter" }
+
+ override DataFlowCallable getEnclosingCallable() { this.isParameterOf(result, _) }
+
+ ClosureExpr getClosure() { result = closure }
+ }
+
  class SummaryParameterNode extends ParameterNodeImpl, FlowSummaryNode {
  SummaryParameterNode() {
  FlowSummaryImpl::Private::summaryParameterNode(this.getSummaryNode(), _)
@@ -610,6 +649,18 @@ private module ArgumentNodes {
  pos = TPositionalArgument(0)
  }
  }
+
+ class SelfClosureArgumentNode extends ExprNode, ArgumentNode {
+ ApplyExprCfgNode apply;
+
+ SelfClosureArgumentNode() { n = apply.getFunction() }
+
+ override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
+ apply = call.asCall() and
+ not exists(apply.getStaticTarget()) and
+ pos instanceof ThisArgumentPosition
+ }
+ }
 }
 
 import ArgumentNodes
@@ -785,6 +836,175 @@ private module OutNodes {
 
 import OutNodes
 
+/**
+ * Holds if there is a data flow step from `e1` to `e2` that only steps from
+ * child to parent in the AST.
+ */
+private predicate simpleAstFlowStep(Expr e1, Expr e2) {
+ e2.(IfExpr).getBranch(_) = e1
+ or
+ e2.(AssignExpr).getSource() = e1
+ or
+ e2.(ArrayExpr).getAnElement() = e1
+}
+
+private predicate closureFlowStep(CaptureInput::Expr e1, CaptureInput::Expr e2) {
+ simpleAstFlowStep(e1, e2)
+ or
+ exists(Ssa::WriteDefinition def |
+ def.getARead().getNode().asAstNode() = e2 and
+ def.assigns(any(CfgNode cfg | cfg.getNode().asAstNode() = e1))
+ )
+ or
+ e2.(Pattern).getImmediateMatchingExpr() = e1
+}
+
+private module CaptureInput implements VariableCapture::InputSig {
+ private import swift as S
+ private import codeql.swift.controlflow.BasicBlocks as B
+
+ class Location = S::Location;
+
+ class BasicBlock instanceof B::BasicBlock {
+ string toString() { result = super.toString() }
+
+ Callable getEnclosingCallable() { result = super.getScope() }
+
+ Location getLocation() { result = super.getLocation() }
+ }
+
+ BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) {
+ result.(B::BasicBlock).immediatelyDominates(bb)
+ }
+
+ BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.(B::BasicBlock).getASuccessor() }
+
+ class CapturedVariable instanceof S::VarDecl {
+ CapturedVariable() {
+ any(S::CapturedDecl capturedDecl).getDecl() = this and
+ exists(this.getEnclosingCallable())
+ }
+
+ string toString() { result = super.toString() }
+
+ Callable getCallable() { result = super.getEnclosingCallable() }
+
+ Location getLocation() { result = super.getLocation() }
+ }
+
+ class CapturedParameter extends CapturedVariable instanceof S::ParamDecl { }
+
+ class Expr instanceof S::AstNode {
+ string toString() { result = super.toString() }
+
+ Location getLocation() { result = super.getLocation() }
+
+ predicate hasCfgNode(BasicBlock bb, int i) {
+ this = bb.(B::BasicBlock).getNode(i).getNode().asAstNode()
+ }
+ }
+
+ class VariableWrite extends Expr {
+ CapturedVariable variable;
+ Expr source;
+
+ VariableWrite() {
+ exists(S::Assignment a | this = a |
+ a.getDest().(DeclRefExpr).getDecl() = variable and
+ source = a.getSource()
+ )
+ or
+ exists(S::NamedPattern np | this = np |
+ variable = np.getVarDecl() and
+ source = np.getMatchingExpr()
+ )
+ }
+
+ CapturedVariable getVariable() { result = variable }
+
+ Expr getSource() { result = source }
+ }
+
+ class VariableRead extends Expr instanceof S::DeclRefExpr {
+ CapturedVariable v;
+
+ VariableRead() { this.getDecl() = v and not isLValue(this) }
+
+ CapturedVariable getVariable() { result = v }
+ }
+
+ class ClosureExpr extends Expr instanceof S::Callable {
+ ClosureExpr() { any(S::CapturedDecl c).getScope() = this }
+
+ predicate hasBody(Callable body) { this = body }
+
+ predicate hasAliasedAccess(Expr f) { closureFlowStep+(this, f) and not closureFlowStep(f, _) }
+ }
+
+ class Callable extends S::Callable {
+ predicate isConstructor() {
+ // A class declaration cannot capture a variable in Swift. Consider this hypothetical example:
+ // ```
+ // protocol Interface { }
+ // func foo() -> Interface {
+ // let y = 42
+ // class Impl : Interface {
+ // let x : Int
+ // init() {
+ // x = y
+ // }
+ // }
+ // let object = Impl()
+ // return object
+ // }
+ // ```
+ // The Swift compiler will reject this with an error message such as
+ // ```
+ // error: class declaration cannot close over value 'y' defined in outer scope
+ // x = y
+ // ^
+ // ```
+ none()
+ }
+ }
+}
+
+class CapturedVariable = CaptureInput::CapturedVariable;
+
+class CapturedParameter = CaptureInput::CapturedParameter;
+
+module CaptureFlow = VariableCapture::Flow<CaptureInput>;
+
+private CaptureFlow::ClosureNode asClosureNode(Node n) {
+ result = n.(CaptureNode).getSynthesizedCaptureNode()
+ or
+ result.(CaptureFlow::ExprNode).getExpr() = n.asExpr()
+ or
+ result.(CaptureFlow::ExprPostUpdateNode).getExpr() =
+ n.(PostUpdateNode).getPreUpdateNode().asExpr()
+ or
+ result.(CaptureFlow::ParameterNode).getParameter() = n.asParameter()
+ or
+ result.(CaptureFlow::ThisParameterNode).getCallable() = n.(ClosureSelfParameterNode).getClosure()
+ or
+ exists(CaptureInput::VariableWrite write |
+ result.(CaptureFlow::VariableWriteSourceNode).getVariableWrite() = write and
+ n.asExpr() = write.getSource()
+ )
+}
+
+private predicate captureStoreStep(Node node1, Content::CapturedVariableContent c, Node node2) {
+ CaptureFlow::storeStep(asClosureNode(node1), c.getVariable(), asClosureNode(node2))
+}
+
+private predicate captureReadStep(Node node1, Content::CapturedVariableContent c, Node node2) {
+ CaptureFlow::readStep(asClosureNode(node1), c.getVariable(), asClosureNode(node2))
+}
+
+predicate captureValueStep(Node node1, Node node2) {
+ CaptureFlow::localFlowStep(asClosureNode(node1), asClosureNode(node2))
+}
+
 predicate jumpStep(Node pred, Node succ) {
  FlowSummaryImpl::Private::Steps::summaryJumpStep(pred.(FlowSummaryNode).getSummaryNode(),
  succ.(FlowSummaryNode).getSummaryNode())
@@ -897,6 +1117,8 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
  or
  FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), c,
  node2.(FlowSummaryNode).getSummaryNode())
+ or
+ captureStoreStep(node1, any(Content::CapturedVariableContent cvc | c.isSingleton(cvc)), node2)
 }
 
 predicate isLValue(Expr e) { any(AssignExpr assign).getDest() = e }
@@ -1001,6 +1223,8 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
  or
  FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), c,
  node2.(FlowSummaryNode).getSummaryNode())
+ or
+ captureReadStep(node1, any(Content::CapturedVariableContent cvc | c.isSingleton(cvc)), node2)
 }
 
 /**
@@ -1094,6 +1318,17 @@ private module PostUpdateNodes {
  result.(FlowSummaryNode).getSummaryNode())
  }
  }
+
+ class CapturePostUpdateNode extends PostUpdateNodeImpl, CaptureNode {
+ private CaptureNode pre;
+
+ CapturePostUpdateNode() {
+ CaptureFlow::capturePostUpdateNode(this.getSynthesizedCaptureNode(),
+ pre.getSynthesizedCaptureNode())
+ }
+
+ override Node getPreUpdateNode() { result = pre }
+ }
 }
 
 private import PostUpdateNodes
@@ -1152,7 +1387,12 @@ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preserves
  * One example would be to allow flow like `p.foo = p.bar;`, which is disallowed
  * by default as a heuristic.
  */
-predicate allowParameterReturnInSelf(ParameterNode p) { none() }
+predicate allowParameterReturnInSelf(ParameterNode p) {
+ exists(Callable c |
+ c = p.(ParameterNodeImpl).getEnclosingCallable().asSourceCallable() and
+ CaptureFlow::heuristicAllowInstanceParameterReturnInSelf(c)
+ )
+}
 
 /** An approximated `Content`. */
 class ContentApprox = Unit;