Skip to content

Commit 1f4c932

Browse files
committed
unified: Share logic with type inference
1 parent 65001f0 commit 1f4c932

6 files changed

Lines changed: 41 additions & 22 deletions

File tree

‎unified/ql/lib/codeql/unified/internal/ExprPositions.qll‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,13 @@ predicate hasResultValue(Expr e) {
7979
not isInAssignmentContext(e, any(AssignExpr n)) and // non-compound assignment target
8080
not e instanceof IdentifierLabel
8181
}
82+
83+
/** Holds if `e` is returned from `callable` either because it is an operand of `return` or is implicitly returned due to language-specific semantics. */
84+
predicate isReturnedExpr(Callable callable, Expr e) {
85+
exists(ReturnExpr ret |
86+
callable = ret.getEnclosingCallable() and
87+
e = ret.getValue()
88+
)
89+
or
90+
any(ExprPositionsPlugin p).isReturnedExpr(callable, e)
91+
}

‎unified/ql/lib/codeql/unified/internal/ExprPositionsPlugin.qll‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,6 @@ private module Plugins {
77

88
class ExprPositionsPlugin extends Unit {
99
predicate isInTypeContext(Expr e) { none() }
10+
11+
predicate isReturnedExpr(Callable callable, Expr e) { none() }
1012
}

‎unified/ql/lib/codeql/unified/internal/ExprPositionsPluginSwift.qll‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,24 @@ private import ExprPositionsPlugin
33

44
private class ExprPositionsPluginSwift extends ExprPositionsPlugin {
55
override predicate isInTypeContext(Expr e) { e = any(GenericTypeExpr g).getBase() }
6+
7+
override predicate isReturnedExpr(Callable callable, Expr e) {
8+
// Short-hand return from a function body, e.g. `func foo() -> String { "42" }`
9+
// This happens when the body contains a single expression and this expression resolves to a value.
10+
// We don't detect precisely whether the expression resolves to a value, but we restrict to cases
11+
// where a spurious return would be harmless in practice.
12+
(
13+
callable instanceof FunctionDeclaration and
14+
exists(Expr ret | ret = callable.getReturnType() | not ret.(Identifier).getValue() = "Void")
15+
or
16+
callable instanceof GetterDeclaration
17+
or
18+
callable instanceof FunctionExpr // We can't tell whether closures have a return value
19+
) and
20+
exists(Block body |
21+
body = callable.getBody() and
22+
not exists(body.getStmt(1)) and // Check that only one statement exists
23+
e = body.getStmt(0)
24+
)
25+
}
626
}

‎unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
private import unified
22
private import AllDataFlow
33
private import codeql.unified.internal.LocalNameBinding
4+
private import codeql.unified.internal.ExprPositions
45

56
predicate step(BuilderNode node1, Step step, BuilderNode node2) {
67
any(DataFlowPlugin p).step(node1, step, node2)
@@ -180,10 +181,11 @@ predicate step(BuilderNode node1, Step step, BuilderNode node2) {
180181
node2.isIncomingValue(fun.getNameNode())
181182
)
182183
or
183-
exists(ReturnExpr ret |
184-
node1.isResultValue(ret.getValue()) and
184+
exists(Callable callable, Expr expr |
185+
isReturnedExpr(callable, expr) and
186+
node1.isResultValue(expr) and
185187
step.value() and
186-
node2.isReturnNode(ret.getEnclosingCallable())
188+
node2.isReturnNode(callable)
187189
)
188190
or
189191
none() // Temporarily disable compilation errors from unsatisfiable types

‎unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPluginSwift.qll‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -76,22 +76,5 @@ private class SwiftDataFlowPlugin extends DataFlowPlugin {
7676
step.taint() and
7777
node2.isResultValue(call)
7878
)
79-
or
80-
// Short-hand return from a function body, e.g. `func foo() -> String { "42" }`
81-
// This happens when the body contains a single expression and this expression resolves to a value.
82-
// We don't detect precisely whether the expression resolves to a value, but we restrict to cases
83-
// where a spurious flow edge would be harmless.
84-
exists(Callable callable, Block body |
85-
(
86-
callable instanceof FunctionExpr or
87-
callable instanceof FunctionDeclaration or
88-
callable instanceof GetterDeclaration
89-
) and
90-
body = callable.getBody() and
91-
not exists(body.getStmt(1)) and // Check that only one statement exists
92-
node1.isResultValue(body.getStmt(0)) and
93-
step.value() and
94-
node2.isReturnNode(callable)
95-
)
9679
}
9780
}

‎unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,8 +340,10 @@ module Input3 implements InputSig3 {
340340
result = M3::inferFieldAccessReceiverTypeContextualDefault(_, receiver, path)
341341
}
342342

343-
class Return extends ReturnExpr {
344-
Expr getExpr() { result = this.getValue() }
343+
class Return extends Expr {
344+
Return() { isReturnedExpr(_, this) }
345+
346+
Expr getExpr() { result = this }
345347
}
346348

347349
class Parameter extends VariableDeclarationImpl, TParameterExDeclaration {

0 commit comments

Comments
 (0)