Skip to content

Commit d4e14c3

Browse files
committed
Java: Add proper support for variable capture flow.
1 parent b572974 commit d4e14c3

File tree

15 files changed

+754
-28
lines changed

15 files changed

+754
-28
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: majorAnalysis
3+
---
4+
* Improved support for flow through captured variables that properly adheres to inter-procedural control flow.

java/ql/lib/ext/java.lang.model.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,6 @@ extensions:
176176
- ["java.lang", "Object", "getClass", "()", "summary", "manual"]
177177
- ["java.lang", "Object", "hashCode", "()", "summary", "manual"]
178178
- ["java.lang", "Object", "toString", "()", "summary", "manual"]
179-
- ["java.lang", "Runnable", "run", "()", "summary", "manual"]
180179
- ["java.lang", "Runtime", "getRuntime", "()", "summary", "manual"]
181180
- ["java.lang", "String", "compareTo", "(String)", "summary", "manual"]
182181
- ["java.lang", "String", "contains", "(CharSequence)", "summary", "manual"]

java/ql/lib/qlpack.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ extractor: java
66
library: true
77
upgrades: upgrades
88
dependencies:
9+
codeql/dataflow: ${workspace}
910
codeql/regex: ${workspace}
1011
codeql/tutorial: ${workspace}
1112
codeql/typetracking: ${workspace}

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ private module DispatchImpl {
156156

157157
private module Unification = MkUnification<unificationTargetLeft/1, unificationTargetRight/1>;
158158

159-
private int parameterPosition() { result in [-1, any(Parameter p).getPosition()] }
159+
private int parameterPosition() { result in [-2, -1, any(Parameter p).getPosition()] }
160160

161161
/** A parameter position represented by an integer. */
162162
class ParameterPosition extends int {

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,9 @@ private module Cached {
5555
)
5656
} or
5757
TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn) or
58-
TFieldValueNode(Field f)
58+
TFieldValueNode(Field f) or
59+
TParameterPostUpdate(CapturedParameter p) or
60+
TClosureNode(CaptureFlow::ClosureNode cn)
5961

6062
cached
6163
newtype TContent =
@@ -64,6 +66,7 @@ private module Cached {
6466
TCollectionContent() or
6567
TMapKeyContent() or
6668
TMapValueContent() or
69+
TClosureContent(CapturedVariable v) or
6770
TSyntheticFieldContent(SyntheticField s)
6871

6972
cached
@@ -73,6 +76,7 @@ private module Cached {
7376
TCollectionContentApprox() or
7477
TMapKeyContentApprox() or
7578
TMapValueContentApprox() or
79+
TClosureContentApprox(CapturedVariable v) or
7680
TSyntheticFieldApproxContent()
7781
}
7882

@@ -127,6 +131,8 @@ module Public {
127131
or
128132
result = this.(ImplicitPostUpdateNode).getPreUpdateNode().getType()
129133
or
134+
result = this.(ClosureNode).getTypeImpl()
135+
or
130136
result = this.(FieldValueNode).getField().getType()
131137
}
132138

@@ -359,6 +365,10 @@ private class ImplicitExprPostUpdate extends ImplicitPostUpdateNode, TImplicitEx
359365
}
360366
}
361367

368+
private class ParameterPostUpdate extends ImplicitPostUpdateNode, TParameterPostUpdate {
369+
override Node getPreUpdateNode() { this = TParameterPostUpdate(result.asParameter()) }
370+
}
371+
362372
module Private {
363373
private import DataFlowDispatch
364374

@@ -372,6 +382,7 @@ module Private {
372382
result.asCallable() = n.(MallocNode).getClassInstanceExpr().getEnclosingCallable() or
373383
result = nodeGetEnclosingCallable(n.(ImplicitPostUpdateNode).getPreUpdateNode()) or
374384
result.asSummarizedCallable() = n.(FlowSummaryNode).getSummarizedCallable() or
385+
result = n.(ClosureNode).getCaptureFlowNode().getEnclosingCallable() or
375386
result.asFieldScope() = n.(FieldValueNode).getField()
376387
}
377388

@@ -400,6 +411,8 @@ module Private {
400411
this = getInstanceArgument(_)
401412
or
402413
this.(FlowSummaryNode).isArgumentOf(_, _)
414+
or
415+
this.(ClosureNode).isArgumentOf(_, _)
403416
}
404417

405418
/**
@@ -417,6 +430,8 @@ module Private {
417430
pos = -1 and this = getInstanceArgument(call.asCall())
418431
or
419432
this.(FlowSummaryNode).isArgumentOf(call, pos)
433+
or
434+
this.(ClosureNode).isArgumentOf(call, pos)
420435
}
421436

422437
/** Gets the call in which this node is an argument. */
@@ -491,6 +506,38 @@ module Private {
491506
c.asSummarizedCallable() = this.getSummarizedCallable() and pos = this.getPosition()
492507
}
493508
}
509+
510+
/**
511+
* A synthesized data flow node representing a closure object that tracks
512+
* captured variables.
513+
*/
514+
class ClosureNode extends Node, TClosureNode {
515+
CaptureFlow::ClosureNode getCaptureFlowNode() { this = TClosureNode(result) }
516+
517+
override Location getLocation() { result = this.getCaptureFlowNode().getLocation() }
518+
519+
override string toString() { result = this.getCaptureFlowNode().toString() }
520+
521+
predicate isParameter(DataFlowCallable c) {
522+
this.getCaptureFlowNode().isParameter(c)
523+
}
524+
525+
predicate isArgumentOf(DataFlowCall call, int pos) {
526+
this.getCaptureFlowNode().isArgument(call) and pos = -2
527+
}
528+
529+
Type getTypeImpl() { result instanceof TypeObject }
530+
}
531+
532+
class ClosureParameterNode extends ParameterNode, ClosureNode {
533+
ClosureParameterNode() {
534+
this.isParameter(_)
535+
}
536+
537+
override predicate isParameterOf(DataFlowCallable c, int pos) {
538+
this.isParameter(c) and pos = -2
539+
}
540+
}
494541
}
495542

496543
private import Private
@@ -520,3 +567,13 @@ private class SummaryPostUpdateNode extends FlowSummaryNode, PostUpdateNode {
520567

521568
override Node getPreUpdateNode() { result = pre }
522569
}
570+
571+
private class ClosurePostUpdateNode extends PostUpdateNode, ClosureNode {
572+
private ClosureNode pre;
573+
574+
ClosurePostUpdateNode() {
575+
CaptureFlow::closurePostUpdateNode(this.getCaptureFlowNode(), pre.getCaptureFlowNode())
576+
}
577+
578+
override Node getPreUpdateNode() { result = pre }
579+
}

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll

Lines changed: 109 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ private import semmle.code.java.dataflow.FlowSummary
1010
private import FlowSummaryImpl as FlowSummaryImpl
1111
private import DataFlowImplConsistency
1212
private import DataFlowNodes
13+
private import codeql.dataflow.VariableCapture as VariableCapture
1314
import DataFlowNodes::Private
1415

1516
private newtype TReturnKind = TNormalReturnKind()
@@ -51,37 +52,119 @@ private predicate staticFieldStep(Node node1, Node node2) {
5152
)
5253
}
5354

54-
/**
55-
* Holds if data can flow from `node1` to `node2` through variable capture.
56-
*/
57-
private predicate variableCaptureStep(Node node1, ExprNode node2) {
58-
exists(SsaImplicitInit closure, SsaVariable captured |
59-
closure.captures(captured) and
60-
node2.getExpr() = closure.getAFirstUse()
55+
private module CaptureInput implements VariableCapture::InputSig {
56+
private import java as J
57+
58+
class Location = J::Location;
59+
60+
class BasicBlock instanceof J::BasicBlock {
61+
string toString() { result = super.toString() }
62+
63+
DataFlowCallable getEnclosingCallable() { result.asCallable() = super.getEnclosingCallable() }
64+
}
65+
66+
BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { bbIDominates(result, bb) }
67+
68+
BasicBlock getABasicBlockSuccessor(BasicBlock bb) {
69+
result = bb.(J::BasicBlock).getABBSuccessor()
70+
}
71+
72+
//TODO: support capture of `this` in lambdas
73+
class CapturedVariable instanceof LocalScopeVariable {
74+
CapturedVariable() {
75+
2 <=
76+
strictcount(J::Callable c |
77+
c = this.getCallable() or c = this.getAnAccess().getEnclosingCallable()
78+
)
79+
}
80+
81+
string toString() { result = super.toString() }
82+
83+
DataFlowCallable getCallable() { result.asCallable() = super.getCallable() }
84+
}
85+
86+
class CapturedParameter extends CapturedVariable instanceof Parameter { }
87+
88+
additional predicate capturedVarUpdate(
89+
J::BasicBlock bb, int i, CapturedVariable v, VariableUpdate upd
90+
) {
91+
upd.getDestVar() = v and bb.getNode(i) = upd
92+
}
93+
94+
additional predicate capturedVarRead(J::BasicBlock bb, int i, CapturedVariable v, RValue rv) {
95+
v.(LocalScopeVariable).getAnAccess() = rv and bb.getNode(i) = rv
96+
}
97+
98+
predicate variableWrite(BasicBlock bb, int i, CapturedVariable v, Location loc) {
99+
exists(VariableUpdate upd | capturedVarUpdate(bb, i, v, upd) and loc = upd.getLocation())
100+
}
101+
102+
predicate variableRead(BasicBlock bb, int i, CapturedVariable v, Location loc) {
103+
exists(RValue rv | capturedVarRead(bb, i, v, rv) and loc = rv.getLocation())
104+
}
105+
106+
class Callable = DataFlowCallable;
107+
108+
class Call instanceof DataFlowCall {
109+
string toString() { result = super.toString() }
110+
111+
Location getLocation() { result = super.getLocation() }
112+
113+
DataFlowCallable getEnclosingCallable() { result = super.getEnclosingCallable() }
114+
115+
predicate hasCfgNode(BasicBlock bb, int i) { super.asCall() = bb.(J::BasicBlock).getNode(i) }
116+
}
117+
}
118+
119+
class CapturedVariable = CaptureInput::CapturedVariable;
120+
121+
class CapturedParameter = CaptureInput::CapturedParameter;
122+
123+
module CaptureFlow = VariableCapture::Flow<CaptureInput>;
124+
125+
private predicate captureStoreStep(Node node1, ClosureContent c, Node node2) {
126+
exists(BasicBlock bb, int i, CaptureInput::CapturedVariable v, VariableUpdate upd |
127+
upd.(VariableAssign).getSource() = node1.asExpr() or
128+
upd.(AssignOp) = node1.asExpr()
61129
|
62-
node1.asExpr() = captured.getAUse()
63-
or
64-
not exists(captured.getAUse()) and
65-
exists(SsaVariable capturedDef | capturedDef = captured.getAnUltimateDefinition() |
66-
capturedDef.(SsaImplicitInit).isParameterDefinition(node1.asParameter()) or
67-
capturedDef.(SsaExplicitUpdate).getDefiningExpr().(VariableAssign).getSource() =
68-
node1.asExpr() or
69-
capturedDef.(SsaExplicitUpdate).getDefiningExpr().(AssignOp) = node1.asExpr()
70-
)
130+
CaptureInput::capturedVarUpdate(bb, i, v, upd) and
131+
c.getVariable() = v and
132+
CaptureFlow::storeStep(bb, i, v, node2.(ClosureNode).getCaptureFlowNode())
133+
)
134+
or
135+
exists(Parameter p |
136+
node1.asParameter() = p and
137+
c.getVariable() = p and
138+
CaptureFlow::parameterStoreStep(p, node2.(ClosureNode).getCaptureFlowNode())
71139
)
72140
}
73141

142+
private predicate captureReadStep(Node node1, ClosureContent c, Node node2) {
143+
exists(BasicBlock bb, int i, CaptureInput::CapturedVariable v |
144+
CaptureFlow::readStep(node1.(ClosureNode).getCaptureFlowNode(), bb, i, v) and
145+
c.getVariable() = v and
146+
CaptureInput::capturedVarRead(bb, i, v, node2.asExpr())
147+
)
148+
or
149+
exists(Parameter p |
150+
CaptureFlow::parameterReadStep(node1.(ClosureNode).getCaptureFlowNode(), p) and
151+
c.getVariable() = p and
152+
node2.(PostUpdateNode).getPreUpdateNode().asParameter() = p
153+
)
154+
}
155+
156+
predicate captureValueStep(Node node1, Node node2) {
157+
CaptureFlow::localFlowStep(node1.(ClosureNode).getCaptureFlowNode(),
158+
node2.(ClosureNode).getCaptureFlowNode())
159+
}
160+
74161
/**
75162
* Holds if data can flow from `node1` to `node2` through a static field or
76163
* variable capture.
77164
*/
78165
predicate jumpStep(Node node1, Node node2) {
79166
staticFieldStep(node1, node2)
80167
or
81-
variableCaptureStep(node1, node2)
82-
or
83-
variableCaptureStep(node1.(PostUpdateNode).getPreUpdateNode(), node2)
84-
or
85168
any(AdditionalValueStep a).step(node1, node2) and
86169
node1.getEnclosingCallable() != node2.getEnclosingCallable()
87170
or
@@ -117,6 +200,8 @@ predicate storeStep(Node node1, Content f, Node node2) {
117200
or
118201
FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), f,
119202
node2.(FlowSummaryNode).getSummaryNode())
203+
or
204+
captureStoreStep(node1, f, node2)
120205
}
121206

122207
/**
@@ -149,6 +234,8 @@ predicate readStep(Node node1, Content f, Node node2) {
149234
or
150235
FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), f,
151236
node2.(FlowSummaryNode).getSummaryNode())
237+
or
238+
captureReadStep(node1, f, node2)
152239
}
153240

154241
/**
@@ -465,6 +552,8 @@ ContentApprox getContentApprox(Content c) {
465552
or
466553
c instanceof MapValueContent and result = TMapValueContentApprox()
467554
or
555+
exists(CapturedVariable v | c = TClosureContent(v) and result = TClosureContentApprox(v))
556+
or
468557
c instanceof SyntheticFieldContent and result = TSyntheticFieldApproxContent()
469558
}
470559

0 commit comments

Comments
 (0)