Skip to content

Swift: use shared capture flow library #14078

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions swift/ql/lib/change-notes/2023-08-28-shared-capture-flow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: majorAnalysis
---
* Introduced support for flow through captured variables that properly adheres to inter-procedural control flow.
175 changes: 172 additions & 3 deletions swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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() }
Expand Down Expand Up @@ -76,6 +77,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
Expand Down Expand Up @@ -116,6 +127,7 @@ private module Cached {
any(SubscriptExpr se).getBase()
])
} or
TCaptureNode(CaptureFlow::SynthesizedCaptureNode cn) or
TDictionarySubscriptNode(SubscriptExpr e) {
e.getBase().getType().getCanonicalType() instanceof CanonicalDictionaryType
}
Expand Down Expand Up @@ -234,6 +246,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)
}

/**
Expand Down Expand Up @@ -261,7 +276,8 @@ private module Cached {
TTupleContent(int index) { exists(any(TupleExpr te).getElement(index)) } or
TEnumContent(ParamDecl f) { exists(EnumElementDecl d | d.getAParam() = f) } or
TArrayContent() or
TCollectionContent()
TCollectionContent() or
TCapturedVariableContent(CapturedVariable v)
}

/**
Expand Down Expand Up @@ -327,7 +343,7 @@ private module ParameterNodes {
predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { none() }

/** Gets the parameter associated with this node, if any. */
ParamDecl getParameter() { none() }
override ParamDecl getParameter() { none() }
}

class SourceParameterNode extends ParameterNodeImpl, TSourceParameterNode {
Expand Down Expand Up @@ -550,7 +566,7 @@ private module ReturnNodes {
result = TDataFlowFunc(param.getDeclaringFunction())
}

ParamDecl getParameter() { result = param }
override ParamDecl getParameter() { result = param }

override Location getLocationImpl() { result = exit.getLocation() }

Expand Down Expand Up @@ -677,6 +693,144 @@ 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(Expr e1, 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))
)
}

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() }

//TODO: support capture of `this` in lambdas
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::PatternBindingDecl pbd, S::NamedPattern np | this = pbd and pbd.getAPattern() = np |
np.getVarDecl() = variable and
source = np.getMatchingExpr()
)
// TODO: support multiple variables in LHS of =, in both of above cases.
}

CapturedVariable getVariable() { result = variable }

Expr getSource() { result = source }
}

class VariableRead extends Expr instanceof S::DeclRefExpr {
CapturedVariable v;

VariableRead() { this.getDecl() = v /* TODO: this should be an R-value only. */ }

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() { this instanceof S::Initializer }
}
}

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.getParameter() or
result.(CaptureFlow::ThisParameterNode).getCallable().getSelfParam() = n.getParameter() or
result.(CaptureFlow::MallocNode).getClosureExpr() = n.getCfgNode().getNode().asAstNode() // TODO: figure out why the java version had PostUpdateNode logic here
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a constructor call new Foo(x), the corresponding expression node is the value after the constructor has returned. And in order for a constructor to work as a setter (say if new Foo(x) assigns x to some field of Foo), then the constructor expression needs to be a post-update node. That's why we generally (not just in this context) synthesise a node that we call "malloc" to be the pre-update for a constructor call. That way the constructor becomes just another instance method taking the "malloc" as the input to the this parameter and returning the value of this after potentially updating some field on it.

The variable capture library needs to refer to this "malloc" node, which is the pre-update for a constructor call, in case the closure expression has a constructor that's being called when the closure is created. This usually isn't the case for lambdas, as lambdas don't have constructors, but if Swift allows the definition of other capturing objects, say local class definitions or anonymous class objects, then this becomes relevant. See e.g. the Java test cases, which also includes some examples of local and anonymous classes where the constructor captures a variable.

}

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())
Expand Down Expand Up @@ -789,6 +943,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 }
Expand Down Expand Up @@ -891,6 +1047,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)
}

/**
Expand Down Expand Up @@ -982,6 +1140,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
Expand Down
34 changes: 30 additions & 4 deletions swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPublic.qll
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ class Node extends TNode {
* Gets this node's underlying SSA definition, if any.
*/
Ssa::Definition asDefinition() { none() }

/**
* Gets the parameter that corresponds to this node, if any.
*/
ParamDecl getParameter() { none() }
}

/**
Expand Down Expand Up @@ -96,7 +101,7 @@ class ParameterNode extends Node instanceof ParameterNodeImpl {
result = this.(ParameterNodeImpl).getEnclosingCallable()
}

ParamDecl getParameter() { result = this.(ParameterNodeImpl).getParameter() }
override ParamDecl getParameter() { result = this.(ParameterNodeImpl).getParameter() }
}

/**
Expand All @@ -109,9 +114,7 @@ class SsaDefinitionNode extends Node, TSsaDefinitionNode {
override Ssa::Definition asDefinition() { result = def }
}

class InoutReturnNode extends Node instanceof InoutReturnNodeImpl {
ParamDecl getParameter() { result = super.getParameter() }
}
class InoutReturnNode extends Node instanceof InoutReturnNodeImpl { }

/**
* A node associated with an object after an operation that might have
Expand All @@ -129,6 +132,18 @@ class PostUpdateNode extends Node instanceof PostUpdateNodeImpl {
Node getPreUpdateNode() { result = super.getPreUpdateNode() }
}

/**
* A synthesized data flow node representing a closure object that tracks
* captured variables.
*/
class CaptureNode extends Node, TCaptureNode {
private CaptureFlow::SynthesizedCaptureNode cn;

CaptureNode() { this = TCaptureNode(cn) }

CaptureFlow::SynthesizedCaptureNode getSynthesizedCaptureNode() { result = cn }
}

/**
* Gets a node corresponding to expression `e`.
*/
Expand Down Expand Up @@ -234,6 +249,17 @@ module Content {
class CollectionContent extends Content, TCollectionContent {
override string toString() { result = "Collection element" }
}

/** A captured variable. */
class CapturedVariableContent extends Content, TCapturedVariableContent {
CapturedVariable v;

CapturedVariableContent() { this = TCapturedVariableContent(v) }

CapturedVariable getVariable() { result = v }

override string toString() { result = v.toString() }
}
}

/**
Expand Down
Loading