Skip to content

Commit 79edfb2

Browse files
aschackmullaschackmull
committed
Java: Redesign and reimplement variable capture flow.
1 parent 7ec54f6 commit 79edfb2

File tree

23 files changed

+1003
-437
lines changed

23 files changed

+1003
-437
lines changed

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1222,18 +1222,14 @@ module Private {
12221222
node.asNode().(PostUpdateNode).getPreUpdateNode().(ArgNode).argumentOf(mid.asCall(), apos) and
12231223
parameterMatch(ppos, apos)
12241224
|
1225-
c = "Argument" and not heapParameter(ppos)
1226-
or
1227-
parseArg(c, ppos)
1225+
c = "Argument" or parseArg(c, ppos)
12281226
)
12291227
or
12301228
exists(ArgumentPosition apos, ParameterPosition ppos |
12311229
node.asNode().(ParamNode).isParameterOf(mid.asCallable(), ppos) and
12321230
parameterMatch(ppos, apos)
12331231
|
1234-
c = "Parameter" and not heapParameter(ppos)
1235-
or
1236-
parseParam(c, apos)
1232+
c = "Parameter" or parseParam(c, apos)
12371233
)
12381234
or
12391235
c = "ReturnValue" and
@@ -1263,9 +1259,7 @@ module Private {
12631259
node.asNode().(ArgNode).argumentOf(mid.asCall(), apos) and
12641260
parameterMatch(ppos, apos)
12651261
|
1266-
c = "Argument" and not heapParameter(ppos)
1267-
or
1268-
parseArg(c, ppos)
1262+
c = "Argument" or parseArg(c, ppos)
12691263
)
12701264
or
12711265
exists(ReturnNodeExt ret |

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImplSpecific.qll

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -300,12 +300,6 @@ predicate interpretInputSpecific(string c, InterpretNode mid, InterpretNode n) {
300300
)
301301
}
302302

303-
/**
304-
* Holds if `pos` is the position of the `heap` parameter, and thus should not
305-
* be included by models that specify "any argument" or "any parameter".
306-
*/
307-
predicate heapParameter(ParameterPosition pos) { none() }
308-
309303
/** Gets the argument position obtained by parsing `X` in `Parameter[X]`. */
310304
bindingset[s]
311305
ArgumentPosition parseParamBody(string s) {

‎go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1222,18 +1222,14 @@ module Private {
12221222
node.asNode().(PostUpdateNode).getPreUpdateNode().(ArgNode).argumentOf(mid.asCall(), apos) and
12231223
parameterMatch(ppos, apos)
12241224
|
1225-
c = "Argument" and not heapParameter(ppos)
1226-
or
1227-
parseArg(c, ppos)
1225+
c = "Argument" or parseArg(c, ppos)
12281226
)
12291227
or
12301228
exists(ArgumentPosition apos, ParameterPosition ppos |
12311229
node.asNode().(ParamNode).isParameterOf(mid.asCallable(), ppos) and
12321230
parameterMatch(ppos, apos)
12331231
|
1234-
c = "Parameter" and not heapParameter(ppos)
1235-
or
1236-
parseParam(c, apos)
1232+
c = "Parameter" or parseParam(c, apos)
12371233
)
12381234
or
12391235
c = "ReturnValue" and
@@ -1263,9 +1259,7 @@ module Private {
12631259
node.asNode().(ArgNode).argumentOf(mid.asCall(), apos) and
12641260
parameterMatch(ppos, apos)
12651261
|
1266-
c = "Argument" and not heapParameter(ppos)
1267-
or
1268-
parseArg(c, ppos)
1262+
c = "Argument" or parseArg(c, ppos)
12691263
)
12701264
or
12711265
exists(ReturnNodeExt ret |

‎go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImplSpecific.qll

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -251,12 +251,6 @@ predicate interpretInputSpecific(string c, InterpretNode mid, InterpretNode n) {
251251
)
252252
}
253253

254-
/**
255-
* Holds if `pos` is the position of the `heap` parameter, and thus should not
256-
* be included by models that specify "any argument" or "any parameter".
257-
*/
258-
predicate heapParameter(ParameterPosition pos) { none() }
259-
260254
/**
261255
* Holds if specification component `c` parses as return value `n` or a range
262256
* containing `n`.

‎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 [-2, -1, any(Parameter p).getPosition()] }
159+
private int parameterPosition() { result in [-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: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,7 @@ private module Cached {
5656
} or
5757
TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn) or
5858
TFieldValueNode(Field f) or
59-
TParameterPostUpdate(CapturedParameter p) or
60-
TClosureNode(CaptureFlow::ClosureNode cn)
59+
TCaptureNode(CaptureFlow::SynthesizedCaptureNode cn)
6160

6261
cached
6362
newtype TContent =
@@ -131,7 +130,7 @@ module Public {
131130
or
132131
result = this.(ImplicitPostUpdateNode).getPreUpdateNode().getType()
133132
or
134-
result = this.(ClosureNode).getTypeImpl()
133+
result = this.(CaptureNode).getTypeImpl()
135134
or
136135
result = this.(FieldValueNode).getField().getType()
137136
}
@@ -365,10 +364,6 @@ private class ImplicitExprPostUpdate extends ImplicitPostUpdateNode, TImplicitEx
365364
}
366365
}
367366

368-
private class ParameterPostUpdate extends ImplicitPostUpdateNode, TParameterPostUpdate {
369-
override Node getPreUpdateNode() { this = TParameterPostUpdate(result.asParameter()) }
370-
}
371-
372367
module Private {
373368
private import DataFlowDispatch
374369

@@ -382,7 +377,7 @@ module Private {
382377
result.asCallable() = n.(MallocNode).getClassInstanceExpr().getEnclosingCallable() or
383378
result = nodeGetEnclosingCallable(n.(ImplicitPostUpdateNode).getPreUpdateNode()) or
384379
result.asSummarizedCallable() = n.(FlowSummaryNode).getSummarizedCallable() or
385-
result = n.(ClosureNode).getCaptureFlowNode().getEnclosingCallable() or
380+
result.asCallable() = n.(CaptureNode).getSynthesizedCaptureNode().getEnclosingCallable() or
386381
result.asFieldScope() = n.(FieldValueNode).getField()
387382
}
388383

@@ -411,8 +406,6 @@ module Private {
411406
this = getInstanceArgument(_)
412407
or
413408
this.(FlowSummaryNode).isArgumentOf(_, _)
414-
or
415-
this.(ClosureNode).isArgumentOf(_, _)
416409
}
417410

418411
/**
@@ -430,8 +423,6 @@ module Private {
430423
pos = -1 and this = getInstanceArgument(call.asCall())
431424
or
432425
this.(FlowSummaryNode).isArgumentOf(call, pos)
433-
or
434-
this.(ClosureNode).isArgumentOf(call, pos)
435426
}
436427

437428
/** Gets the call in which this node is an argument. */
@@ -511,29 +502,16 @@ module Private {
511502
* A synthesized data flow node representing a closure object that tracks
512503
* captured variables.
513504
*/
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() }
505+
class CaptureNode extends Node, TCaptureNode {
506+
CaptureFlow::SynthesizedCaptureNode getSynthesizedCaptureNode() { this = TCaptureNode(result) }
520507

521-
predicate isParameter(DataFlowCallable c) { this.getCaptureFlowNode().isParameter(c) }
508+
override Location getLocation() { result = this.getSynthesizedCaptureNode().getLocation() }
522509

523-
predicate isArgumentOf(DataFlowCall call, int pos) {
524-
this.getCaptureFlowNode().isArgument(call) and pos = -2
525-
}
510+
override string toString() { result = this.getSynthesizedCaptureNode().toString() }
526511

512+
// TODO: expose hasTypeProxy(var / callable)
527513
Type getTypeImpl() { result instanceof TypeObject }
528514
}
529-
530-
class ClosureParameterNode extends ParameterNode, ClosureNode {
531-
ClosureParameterNode() { this.isParameter(_) }
532-
533-
override predicate isParameterOf(DataFlowCallable c, int pos) {
534-
this.isParameter(c) and pos = -2
535-
}
536-
}
537515
}
538516

539517
private import Private
@@ -564,11 +542,12 @@ private class SummaryPostUpdateNode extends FlowSummaryNode, PostUpdateNode {
564542
override Node getPreUpdateNode() { result = pre }
565543
}
566544

567-
private class ClosurePostUpdateNode extends PostUpdateNode, ClosureNode {
568-
private ClosureNode pre;
545+
private class CapturePostUpdateNode extends PostUpdateNode, CaptureNode {
546+
private CaptureNode pre;
569547

570-
ClosurePostUpdateNode() {
571-
CaptureFlow::closurePostUpdateNode(this.getCaptureFlowNode(), pre.getCaptureFlowNode())
548+
CapturePostUpdateNode() {
549+
CaptureFlow::capturePostUpdateNode(this.getSynthesizedCaptureNode(),
550+
pre.getSynthesizedCaptureNode())
572551
}
573552

574553
override Node getPreUpdateNode() { result = pre }

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

Lines changed: 67 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,16 @@ private predicate staticFieldStep(Node node1, Node node2) {
5252
)
5353
}
5454

55+
private predicate closureFlowStep(Expr e1, Expr e2) {
56+
simpleAstFlowStep(e1, e2)
57+
or
58+
exists(SsaVariable v |
59+
v.getAUse() = e2 and
60+
v.getAnUltimateDefinition().(SsaExplicitUpdate).getDefiningExpr().(VariableAssign).getSource() =
61+
e1
62+
)
63+
}
64+
5565
private module CaptureInput implements VariableCapture::InputSig {
5666
private import java as J
5767

@@ -60,7 +70,9 @@ private module CaptureInput implements VariableCapture::InputSig {
6070
class BasicBlock instanceof J::BasicBlock {
6171
string toString() { result = super.toString() }
6272

63-
DataFlowCallable getEnclosingCallable() { result.asCallable() = super.getEnclosingCallable() }
73+
Callable getEnclosingCallable() { result = super.getEnclosingCallable() }
74+
75+
Location getLocation() { result = super.getLocation() }
6476
}
6577

6678
BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { bbIDominates(result, bb) }
@@ -80,39 +92,59 @@ private module CaptureInput implements VariableCapture::InputSig {
8092

8193
string toString() { result = super.toString() }
8294

83-
DataFlowCallable getCallable() { result.asCallable() = super.getCallable() }
95+
Callable getCallable() { result = super.getCallable() }
96+
97+
Location getLocation() { result = super.getLocation() }
8498
}
8599

86100
class CapturedParameter extends CapturedVariable instanceof Parameter { }
87101

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-
}
102+
class Expr instanceof J::Expr {
103+
string toString() { result = super.toString() }
104+
105+
Location getLocation() { result = super.getLocation() }
93106

94-
additional predicate capturedVarRead(J::BasicBlock bb, int i, CapturedVariable v, RValue rv) {
95-
v.(LocalScopeVariable).getAnAccess() = rv and bb.getNode(i) = rv
107+
predicate hasCfgNode(BasicBlock bb, int i) { this = bb.(J::BasicBlock).getNode(i) }
96108
}
97109

98-
predicate variableWrite(BasicBlock bb, int i, CapturedVariable v, Location loc) {
99-
exists(VariableUpdate upd | capturedVarUpdate(bb, i, v, upd) and loc = upd.getLocation())
110+
class VariableWrite extends Expr instanceof VariableUpdate {
111+
CapturedVariable v;
112+
113+
VariableWrite() { super.getDestVar() = v }
114+
115+
CapturedVariable getVariable() { result = v }
116+
117+
Expr getSource() {
118+
result = this.(VariableAssign).getSource() or
119+
result = this.(AssignOp)
120+
}
100121
}
101122

102-
predicate variableRead(BasicBlock bb, int i, CapturedVariable v, Location loc) {
103-
exists(RValue rv | capturedVarRead(bb, i, v, rv) and loc = rv.getLocation())
123+
class VariableRead extends Expr instanceof RValue {
124+
CapturedVariable v;
125+
126+
VariableRead() { super.getVariable() = v }
127+
128+
CapturedVariable getVariable() { result = v }
104129
}
105130

106-
class Callable = DataFlowCallable;
131+
class ClosureExpr extends Expr instanceof ClassInstanceExpr {
132+
NestedClass nc;
107133

108-
class Call instanceof DataFlowCall {
109-
string toString() { result = super.toString() }
134+
ClosureExpr() {
135+
nc.(AnonymousClass).getClassInstanceExpr() = this
136+
or
137+
nc instanceof LocalClass and
138+
super.getConstructedType().getASourceSupertype*().getSourceDeclaration() = nc
139+
}
110140

111-
Location getLocation() { result = super.getLocation() }
141+
predicate hasBody(Callable body) { nc.getACallable() = body }
112142

113-
DataFlowCallable getEnclosingCallable() { result = super.getEnclosingCallable() }
143+
predicate hasAliasedAccess(Expr f) { closureFlowStep+(this, f) and not closureFlowStep(f, _) }
144+
}
114145

115-
predicate hasCfgNode(BasicBlock bb, int i) { super.asCall() = bb.(J::BasicBlock).getNode(i) }
146+
class Callable extends J::Callable {
147+
predicate isConstructor() { this instanceof Constructor }
116148
}
117149
}
118150

@@ -122,40 +154,27 @@ class CapturedParameter = CaptureInput::CapturedParameter;
122154

123155
module CaptureFlow = VariableCapture::Flow<CaptureInput>;
124156

157+
CaptureFlow::ClosureNode asClosureNode(Node n) {
158+
result = n.(CaptureNode).getSynthesizedCaptureNode() or
159+
result.(CaptureFlow::ExprNode).getExpr() = n.asExpr() or
160+
result.(CaptureFlow::ExprPostUpdateNode).getExpr() =
161+
n.(PostUpdateNode).getPreUpdateNode().asExpr() or
162+
result.(CaptureFlow::ParameterNode).getParameter() = n.asParameter() or
163+
result.(CaptureFlow::ThisParameterNode).getCallable() = n.(InstanceParameterNode).getCallable() or
164+
exprNode(result.(CaptureFlow::MallocNode).getClosureExpr()).(PostUpdateNode).getPreUpdateNode() =
165+
n
166+
}
167+
125168
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()
129-
|
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())
139-
)
169+
CaptureFlow::storeStep(asClosureNode(node1), c.getVariable(), asClosureNode(node2))
140170
}
141171

142172
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-
)
173+
CaptureFlow::readStep(asClosureNode(node1), c.getVariable(), asClosureNode(node2))
154174
}
155175

156176
predicate captureValueStep(Node node1, Node node2) {
157-
CaptureFlow::localFlowStep(node1.(ClosureNode).getCaptureFlowNode(),
158-
node2.(ClosureNode).getCaptureFlowNode())
177+
CaptureFlow::localFlowStep(asClosureNode(node1), asClosureNode(node2))
159178
}
160179

161180
/**
@@ -517,6 +536,8 @@ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preserves
517536
*/
518537
predicate allowParameterReturnInSelf(ParameterNode p) {
519538
FlowSummaryImpl::Private::summaryAllowParameterReturnInSelf(p)
539+
or
540+
CaptureFlow::heuristicAllowInstanceParameterReturnInSelf(p.(InstanceParameterNode).getCallable())
520541
}
521542

522543
/** An approximated `Content`. */

0 commit comments

Comments
 (0)