Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -3683,6 +3683,16 @@ module Make1<LocationSig Location, InputSig1<Location> Input1> {
pragma[nomagic]
Type inferType(AstNode n) { result = inferType(n, TypePath::nil()) }

/** Gets the unique non-pseudo type inferred at `path` for `n`, if any. */
Type inferTypeUnique(AstNode n, TypePath path) {
result = unique(Type t | t = inferType(n, path) and not t instanceof PseudoType | t)
}

/**
* Gets the unique non-pseudo type inferred at the root for `n`, if any.
*/
Type inferTypeUnique(AstNode n) { result = inferTypeUnique(n, TypePath::nil()) }

/**
* The cached stage of this module.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,16 @@ class NameBindingPluginSwift extends NameBindingPlugin {
or
not m instanceof GeneratedConstructor and
result =
concat(int i, Parameter p |
p = m.(Callable).getParameter(i)
concat(int i, Parameter p, string name |
p = m.(Callable).getParameter(i) and
(
name = p.getExternalNameNode().getValue()
or
not exists(p.getExternalNameNode()) and
name = "_"
Comment on lines +94 to +97

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we're OK with a spurious setter call

)
|
p.getExternalNameNode().getValue(), "," order by i
name, "," order by i
)
}

Expand Down
21 changes: 2 additions & 19 deletions unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,9 @@ private import codeql.unified.internal.ExprPositions
DataFlowCallable viableCallable(DataFlowCall c) {
result.asSourceCallable() = T::resolveCallTarget(c.asExplicitCall())
or
result.asSourceCallable() = resolveAccessor(c.asGetterCall()).(GetterDeclaration)
result.asSourceCallable() = T::resolveAccessorAccess(c.asGetterCall()).(GetterDeclaration)
or
result.asSourceCallable() = resolveAccessor(c.asSetterCall()).(SetterDeclaration)
result.asSourceCallable() = T::resolveAccessorAccess(c.asSetterCall()).(SetterDeclaration)
or
result.asSummarizedCallable().matchesCall(c.asExplicitCall())
}

bindingset[access]
pragma[inline_late]
private N::NamespaceNode resolveBase(MemberAccessExpr access) {
result.isInstanceMemberNamespace(T::inferType(access.getBase()).asClass())
}

private AccessorDeclaration resolveAccessor(Expr access) {
result.getNameNode() = N::getStaticBindingTargetFromRef(access)
or
not exists(N::getStaticBindingTargetFromRef(access)) and
exists(MemberAccessExpr m |
access = m and
// TODO: Make type inference own accessor-resolution. It also needs it internally.
resolveBase(m).getMember(m.getMemberName()).isMember(result)
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,15 @@ private import unified
private import AllDataFlow
private import codeql.unified.internal.ExprPositions
private import codeql.unified.internal.NameBinding as N
private import codeql.unified.internal.typeinference.TypeInference as T

private newtype TDataFlowCall =
TExplicitCall(CallExpr call) {
not isInBindingContext(call, _) // ignore constructor patterns
} or
TGetterCall(Expr expr) {
hasResultValue(expr) and
(
N::getStaticBindingTargetFromRef(expr) = any(GetterDeclaration g).getNameNode()
or
// Assume a qualified member accesses might resolve to a getter.
expr instanceof MemberAccessExpr
)
T::resolveAccessorAccess(expr) instanceof GetterDeclaration
} or
TSetterCall(Expr expr) {
(
Expand All @@ -24,12 +20,7 @@ private newtype TDataFlowCall =
hasPostUpdateAfterCallOrAssign(expr, _) and
mayWriteBackViaSetterCall(expr)
) and
(
N::getStaticBindingTargetFromRef(expr) = any(SetterDeclaration g).getNameNode()
or
// Assume a qualified member accesses might resolve to a setter.
expr instanceof MemberAccessExpr
)
T::resolveAccessorAccess(expr) instanceof SetterDeclaration
} or
TSummaryCall(SummarizedCallable callable, SummaryNode callee) {
FlowSummaryOutput::summaryCallbackRange(callable, callee)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,23 @@ private import AllDataFlow
private import codeql.unified.internal.ExprPositions
private import codeql.unified.internal.typeinference.TypeInference as T

private predicate isReferenceType(T::Type t) {
// todo: type parameters with type constraints
t.asClass().hasModifier(["actor", "class"])
}

private class SwiftDataFlowPlugin extends DataFlowPlugin {
// Note: For now we assume all code is Swift, but in the future we must restrict these rules to Swift-files
override predicate bypassContentSet(ContentSet contents) {
contents.asSingleton().asNamedMember() = "some.0"
}

// Swift writes back mutated values of computed properties via their setter
// Swift writes back mutated values of computed value-type properties via their setter
bindingset[expr]
override predicate mayWriteBackViaSetterCall(Expr expr) {
isInWriteBackPosition(expr)
// TODO: Check that 'expr' has a value type. Reference types don't cause writebacks.
exists(boolean direct | isInWriteBackPosition(expr, direct) |
if direct = false then not isReferenceType(T::inferTypeUnique(expr)) else any()
)
}

override predicate exprStep(Expr expr1, Expr expr2, boolean mustFlow) {
Expand Down Expand Up @@ -100,25 +106,29 @@ private class SwiftDataFlowPlugin extends DataFlowPlugin {
* Holds if `expr` is target of an assignment or 'inout' argument,
* or the base of such an expression, e.g. `x` in `x.f1.f2 = foo()`
*/
private predicate isInWriteBackPosition(Expr expr) {
isInAssignmentContext(expr, _)
private predicate isInWriteBackPosition(Expr expr, boolean direct) {
isInAssignmentContext(expr, _) and
direct = false
or
// Arguments to an 'inout' parameter must be prefixed with '&'
exists(UnaryExpr unary |
unary.getOperator().(PrefixOperator).getValue() = "&" and
unary.getOperand() = expr
// TODO: support custom operators with an 'inout' parameter, which lack the '&' indicator
)
) and
direct = true
or
// The receiver of a call to a 'mutating' function counts as an 'inout' argument
exists(CallExpr call, Callable target |
target = T::resolveCallTarget(call) and
target.hasModifier("mutating") and
expr = call.getCallee().(MemberAccessExpr).getBase()
)
) and
direct = true
or
exists(MemberAccessExpr member |
isInWriteBackPosition(member) and
expr = member.getBase()
isInWriteBackPosition(member, _) and
expr = member.getBase() and
direct = false
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ module Input3 implements InputSig3 {
predicate cacheRevRef() {
(exists(resolveFieldAccess(_)) implies any())
or
(exists(resolveAccessorAccess(_)) implies any())
or
(exists(resolveCallTarget(_)) implies any())
}

Expand Down Expand Up @@ -252,20 +254,38 @@ module Input3 implements InputSig3 {
override Location getLocation() { result = decl.getLocation() }
}

class Field extends AstVariableDeclaration {
final class Field = FieldImpl;

abstract private class FieldImpl extends DeclarationImpl {
abstract AstNode getAstNode();
}

private class VariableDeclarationField extends FieldImpl, AstVariableDeclaration {
ClassLikeDeclaration cls;

Field() { decl = cls.getAMember() }
VariableDeclarationField() { decl = cls.getAMember() }

override AstNode getAstNode() { result = this.getDecl() }

override TypeMention getDeclaringType() { result = getClassLikeDeclarationTypeMention(cls) }
}

private class EnumField_ extends Field {
private class EnumField_ extends VariableDeclarationField {
override EnumField decl;

override TypeMention getType() { result = decl.getEnum().getNameNode() }
}

private class AccessorField extends FieldImpl, Callable {
Comment thread
hvitved marked this conversation as resolved.
Resolved
private AccessorDeclaration accessor;

AccessorField() { accessor = this.getCallable() }

override TypeMention getType() { result = accessor.getReturnType() }

override AstNode getAstNode() { result = accessor }
}

abstract class FieldAccess extends Expr {
abstract Expr getReceiver();

Expand All @@ -275,7 +295,7 @@ module Input3 implements InputSig3 {
pragma[nomagic]
private predicate lookupMember0(MemberAccessExpr mae, ClassLikeDeclaration cls, string name) {
not exists(getStaticBindingTargetFromRef(mae)) and // if a static member can be resolved, use it
cls = inferType(mae.getBase()).(ClassLikeDeclarationType).getClassLikeDeclaration() and
cls = inferType(mae.getBase()).asClass() and
name = mae.getMemberName()
}

Expand Down Expand Up @@ -303,7 +323,7 @@ module Input3 implements InputSig3 {
// also mutual recursion
exists(TupleType tuple, string name |
this.isTupleProjection(tuple, name) and
result.getDecl() = tuple.getField(name)
result.getAstNode() = tuple.getField(name)
)
or
// no mutual recursion; can be resolved directly with static name binding
Expand All @@ -325,6 +345,18 @@ module Input3 implements InputSig3 {
override Field getField() { result.getNameNode() = this.getTarget() }
}

private class LocalAccessorFieldAccess extends FieldAccess, LocalNameAccess {
AccessorDeclaration target;

LocalAccessorFieldAccess() {
target = getStaticBindingTargetFromIdentifier(this).getDeclaration()
}

override Expr getReceiver() { none() }

override Field getField() { result.getAstNode() = target }
}

Type inferFieldAccessReceiverType(FieldAccess fa, TypePath path) {
// instance field with explicit receiver
result = inferType(fa.getReceiver(), path)
Expand Down Expand Up @@ -506,17 +538,21 @@ module Input3 implements InputSig3 {
}

Callable getTargetImpl(boolean isFunctionExprInvoke) {
// mutual recursion; call with explicit receiver: `obj.foo(42)`
result = lookupInstanceMember(this.getCallee()) and
isFunctionExprInvoke = false
or
// mutual recursion; call to a function expression: `callback(42)`
result.asCallable() = this.getInvokeTarget(_) and
isFunctionExprInvoke = true
or
// no mutual recursion; can be resolved directly with static name binding
result = this.getTargetViaStaticNameBinding() and
isFunctionExprInvoke = false
(
// mutual recursion; call with explicit receiver: `obj.foo(42)`
result = lookupInstanceMember(this.getCallee()) and
isFunctionExprInvoke = false
or
// mutual recursion; call to a function expression: `callback(42)`
result.asCallable() = this.getInvokeTarget(_) and
isFunctionExprInvoke = true
or
// no mutual recursion; can be resolved directly with static name binding
result = this.getTargetViaStaticNameBinding() and
isFunctionExprInvoke = false
) and
// handled as a field access
not result.asCallable() instanceof AccessorDeclaration
}

Callable getTarget(InvocationResolutionContext c) {
Expand Down Expand Up @@ -820,6 +856,10 @@ predicate inferType = M3::inferType/1;

predicate inferType = M3::inferType/2;

predicate inferTypeUnique = M3::inferTypeUnique/1;

predicate inferTypeUnique = M3::inferTypeUnique/2;

predicate inferTypeCertain = M3::inferTypeCertain/2;

module Consistency = M3::Consistency;
Expand All @@ -835,7 +875,23 @@ module CachedStage = M3::CachedStage;
cached
VariableDeclaration resolveFieldAccess(Expr access) {
CachedStage::ref() and
result = access.(Input3::FieldAccess).getField().getDecl()
result = access.(Input3::FieldAccess).getField().getAstNode()
}

/**
* Gets the accessor that is accessed by the given expression.
*
* Note that we do not attempt to distinguish between different kinds of
* accessors; all accessors belonging to the same declaration will be
* returned (for example both the getter and setter of a property).
*
* `access` is either a `MemberAccessExpr` or an `Identifier` (when
* the field is accessed with an implicit receiver).
*/
cached
AccessorDeclaration resolveAccessorAccess(Expr access) {
CachedStage::ref() and
result = access.(Input3::FieldAccess).getField().getAstNode()
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ abstract private class TypeMentionImpl extends AstNode {
final class ExprTypeMention = ExprTypeMentionImpl;

abstract private class ExprTypeMentionImpl extends TypeMentionImpl, Expr {
ExprTypeMentionImpl() { isInTypeContext(this) and not this instanceof GenericTypeExpr }
ExprTypeMentionImpl() {
isInTypeContext(this) and
not this instanceof GenericTypeExpr and
not this instanceof ModifiedTypeExpr
}

pragma[nomagic]
abstract TypePath getTypeArgumentPath(int i);
Expand Down Expand Up @@ -191,6 +195,14 @@ private class ClassLikeDeclarationTypeMention extends TypeMentionImpl, Identifie
}
}

private class ModifiedTypeExprMention extends TypeMentionImpl instanceof ModifiedTypeExpr {
Comment thread
hvitved marked this conversation as resolved.
Resolved
private TypeMention base;

ModifiedTypeExprMention() { base = ModifiedTypeExpr.super.getType() }

override Type getTypeAt(TypePath path) { result = base.getTypeAt(path) }
}

TypeMention getClassLikeDeclarationTypeMention(ClassLikeDeclaration c) {
result = c.getNameNode()
or
Expand Down
13 changes: 10 additions & 3 deletions unified/ql/test/library-tests/dataflow/accessors_instance.swift
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,18 @@ func t11() {
class C {
var p: (String, String) {
get { ("safe", "safe") }
set { sink(newValue.0) } // $ hasValueFlow=t11.1
set { sink(newValue.0) } // $ hasValueFlow=t11.1 hasValueFlow=t11.2
}
var q: C {
get { C() }
set { sink(newValue.p.0) }
}
}
let c = C()
// `p` has value type, so this calls `p`'s setter
c.p.0 = source("t11.1")
// `q` has reference type, so this calls `q`'s getter followed by `p`'s setter
c.q.p = (source("t11.2"), source("t11.3"))
}

func t12() {
Expand All @@ -146,9 +153,9 @@ func t12() {
class C {
var s: S {
get { S() }
set { sink(newValue.field) } // $ MISSING: hasValueFlow=t12.1
set { sink(newValue.field) } // $ hasValueFlow=t12.1
}
}
let c = C()
c.s.update() // MISSING: the call to 'update' is not resolved
c.s.update()
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,7 @@ func t12() {
}
var p: S {
get { S() }
set { sink(newValue.field) } // $ MISSING: hasValueFlow=t12.1
set { sink(newValue.field) } // $ hasValueFlow=t12.1
}
// TODO: test fails because the call to 'mutate' is not yet resolved
// At the time of writing, the test passes if the call graph is manually patched
p.mutate()
}
Loading
Loading