Skip to content

Commit 91cef39

Browse files
committed
Ruby: Reimplement flow through captured variables using field flow
1 parent 91a809f commit 91cef39

28 files changed

+790
-377
lines changed

ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -727,16 +727,30 @@ module ExprNodes {
727727

728728
override VariableAccess e;
729729

730-
final override VariableAccess getExpr() { result = ExprCfgNode.super.getExpr() }
730+
override VariableAccess getExpr() { result = ExprCfgNode.super.getExpr() }
731+
732+
/** Gets the variable that is being accessed. */
733+
Variable getVariable() { result = this.getExpr().getVariable() }
731734
}
732735

733736
/** A control-flow node that wraps a `VariableReadAccess` AST expression. */
734-
class VariableReadAccessCfgNode extends ExprCfgNode {
737+
class VariableReadAccessCfgNode extends VariableAccessCfgNode {
735738
override string getAPrimaryQlClass() { result = "VariableReadAccessCfgNode" }
736739

737740
override VariableReadAccess e;
738741

739-
final override VariableReadAccess getExpr() { result = ExprCfgNode.super.getExpr() }
742+
override VariableReadAccess getExpr() { result = VariableAccessCfgNode.super.getExpr() }
743+
}
744+
745+
/** A control-flow node that wraps a `LocalVariableReadAccess` AST expression. */
746+
class LocalVariableReadAccessCfgNode extends VariableReadAccessCfgNode {
747+
override string getAPrimaryQlClass() { result = "LocalVariableReadAccessCfgNode" }
748+
749+
override LocalVariableReadAccess e;
750+
751+
final override LocalVariableReadAccess getExpr() { result = super.getExpr() }
752+
753+
final override LocalVariable getVariable() { result = super.getVariable() }
740754
}
741755

742756
private class InstanceVariableAccessMapping extends ExprChildMapping, InstanceVariableAccess {
@@ -762,21 +776,32 @@ module ExprNodes {
762776
}
763777

764778
/** A control-flow node that wraps a `SelfVariableAccess` AST expression. */
765-
class SelfVariableAccessCfgNode extends ExprCfgNode {
779+
class SelfVariableAccessCfgNode extends VariableAccessCfgNode {
766780
final override string getAPrimaryQlClass() { result = "SelfVariableAccessCfgNode" }
767781

768782
override SelfVariableAccessMapping e;
769783

770-
override SelfVariableAccess getExpr() { result = ExprCfgNode.super.getExpr() }
784+
override SelfVariableAccess getExpr() { result = VariableAccessCfgNode.super.getExpr() }
771785
}
772786

773787
/** A control-flow node that wraps a `VariableWriteAccess` AST expression. */
774-
class VariableWriteAccessCfgNode extends ExprCfgNode {
788+
class VariableWriteAccessCfgNode extends VariableAccessCfgNode {
775789
override string getAPrimaryQlClass() { result = "VariableWriteAccessCfgNode" }
776790

777791
override VariableWriteAccess e;
778792

779-
final override VariableWriteAccess getExpr() { result = ExprCfgNode.super.getExpr() }
793+
override VariableWriteAccess getExpr() { result = VariableAccessCfgNode.super.getExpr() }
794+
}
795+
796+
/** A control-flow node that wraps a `LocalVariableWriteAccess` AST expression. */
797+
class LocalVariableWriteAccessCfgNode extends VariableWriteAccessCfgNode {
798+
override string getAPrimaryQlClass() { result = "LocalVariableWriteAccessCfgNode" }
799+
800+
override LocalVariableWriteAccess e;
801+
802+
final override LocalVariableWriteAccess getExpr() { result = super.getExpr() }
803+
804+
final override LocalVariable getVariable() { result = super.getVariable() }
780805
}
781806

782807
/** A control-flow node that wraps a `ConstantReadAccess` AST expression. */

ruby/ql/lib/codeql/ruby/dataflow/SSA.qll

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,9 @@ module Ssa {
178178

179179
/** Gets the location of this SSA definition. */
180180
Location getLocation() { result = this.getControlFlowNode().getLocation() }
181+
182+
/** Gets the scope of this SSA definition. */
183+
CfgScope getScope() { result = this.getBasicBlock().getScope() }
181184
}
182185

183186
/**
@@ -289,7 +292,7 @@ module Ssa {
289292
)
290293
}
291294

292-
final override string toString() { result = "<captured> " + this.getSourceVariable() }
295+
final override string toString() { result = "<captured entry> " + this.getSourceVariable() }
293296

294297
override Location getLocation() { result = this.getBasicBlock().getLocation() }
295298
}
@@ -324,7 +327,7 @@ module Ssa {
324327
*/
325328
final Definition getPriorDefinition() { result = SsaImpl::uncertainWriteDefinitionInput(this) }
326329

327-
override string toString() { result = this.getControlFlowNode().toString() }
330+
override string toString() { result = "<captured exit> " + this.getSourceVariable() }
328331
}
329332

330333
/**
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
private import codeql.ssa.Ssa as SsaImplCommon
2+
private import codeql.ruby.AST
3+
private import codeql.ruby.CFG as Cfg
4+
private import codeql.ruby.controlflow.internal.ControlFlowGraphImplShared as ControlFlowGraphImplShared
5+
private import codeql.ruby.dataflow.SSA
6+
private import codeql.ruby.ast.Variable
7+
private import Cfg::CfgNodes::ExprNodes
8+
9+
class CapturedVariableAccess extends LocalVariableAccess {
10+
CapturedVariableAccess() { this.isCapturedAccess() }
11+
}
12+
13+
class CapturedVariable extends LocalVariable {
14+
CapturedVariable() { this.isCaptured() }
15+
}
16+
17+
class CapturingCallable extends Callable {
18+
CapturingCallable() { any(CapturedVariableAccess access).getCfgScope() = this }
19+
20+
CapturedVariable getACapturedVariable() { result.getAnAccess().getCfgScope() = this }
21+
}
22+
23+
private predicate entryWrite(Cfg::EntryBasicBlock bb, int i, CapturingCallable v) {
24+
i = -1 and
25+
(
26+
bb.getScope() = v
27+
or
28+
bb.getScope() = v.getACapturedVariable().getDeclaringScope()
29+
)
30+
}
31+
32+
private predicate captureRead1(
33+
Cfg::BasicBlock bb, int i, CapturedVariable var, VariableAccessCfgNode access, CapturingCallable v
34+
) {
35+
bb.getNode(i) = access and
36+
access.getNode() = var.getAnAccess() and
37+
var = v.getACapturedVariable() and
38+
(bb.getScope() = v or bb.getScope() = var.getDefiningAccess().getCfgScope())
39+
}
40+
41+
private predicate captureRead2(Cfg::BasicBlock bb, int i, CapturingCallable v) {
42+
bb.getNode(i).getNode() = v
43+
}
44+
45+
newtype TCaptureAccess =
46+
TEntryDef(Cfg::EntryBasicBlock bb, int i, CapturingCallable v) { entryWrite(bb, i, v) } or
47+
TVariableAccess(VariableAccessCfgNode access, CapturingCallable v) {
48+
captureRead1(_, _, _, access, v)
49+
} or
50+
TLambdaAccess(Cfg::BasicBlock bb, int i, CapturingCallable v) { captureRead2(bb, i, v) }
51+
52+
class CaptureAccess extends TCaptureAccess {
53+
string toString() { result = "<captured variable>" }
54+
55+
Location getLocation() {
56+
exists(Cfg::EntryBasicBlock bb, int i, CapturingCallable v |
57+
this = TEntryDef(bb, i, v) and
58+
result = bb.getLocation()
59+
)
60+
or
61+
exists(VariableAccessCfgNode access | this = TVariableAccess(access, _) |
62+
result = access.getLocation()
63+
)
64+
or
65+
exists(CapturingCallable c |
66+
this = TLambdaAccess(_, _, c) and
67+
result = c.getLocation()
68+
)
69+
}
70+
71+
Cfg::CfgScope getCfgScope() {
72+
exists(Cfg::EntryBasicBlock bb, int i, CapturingCallable v |
73+
this = TEntryDef(bb, i, v) and
74+
result = bb.getScope()
75+
)
76+
or
77+
exists(VariableAccessCfgNode access | this = TVariableAccess(access, _) |
78+
result = access.getScope()
79+
)
80+
or
81+
exists(CapturingCallable c |
82+
this = TLambdaAccess(_, _, c) and
83+
result = c.getCfgScope()
84+
)
85+
}
86+
87+
predicate isLambdaAccess(Cfg::BasicBlock bb, int i, CapturingCallable v) {
88+
captureRead2(bb, i, v) and
89+
this = TLambdaAccess(bb, i, v)
90+
}
91+
92+
predicate isRead(Cfg::BasicBlock bb, int i, CapturingCallable v) {
93+
exists(VariableAccessCfgNode acc |
94+
captureRead1(bb, i, _, acc, v) and
95+
this = TVariableAccess(acc, v)
96+
)
97+
or
98+
this.isLambdaAccess(bb, i, v)
99+
}
100+
101+
predicate isWrite(Cfg::BasicBlock bb, int i, CapturingCallable v) {
102+
entryWrite(bb, i, v) and
103+
this = TEntryDef(bb, i, v)
104+
}
105+
106+
predicate isReadOrWrite(Cfg::BasicBlock bb, int i, CapturingCallable v) {
107+
this.isRead(bb, i, v) or
108+
this.isWrite(bb, i, v)
109+
}
110+
}
111+
112+
private module SsaInput implements SsaImplCommon::InputSig {
113+
private import codeql.ruby.controlflow.BasicBlocks as BasicBlocks
114+
115+
class BasicBlock = BasicBlocks::BasicBlock;
116+
117+
BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() }
118+
119+
BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() }
120+
121+
class ExitBasicBlock = BasicBlocks::ExitBasicBlock;
122+
123+
class SourceVariable = CapturingCallable;
124+
125+
/**
126+
* Holds if the statement at index `i` of basic block `bb` contains a write to variable `v`.
127+
* `certain` is true if the write definitely occurs.
128+
*/
129+
predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) {
130+
any(CaptureAccess access).isWrite(bb, i, v) and
131+
certain = true
132+
}
133+
134+
predicate variableRead(BasicBlock bb, int i, SourceVariable v, boolean certain) {
135+
any(CaptureAccess access).isRead(bb, i, v) and
136+
certain = true
137+
}
138+
}
139+
140+
private import SsaImplCommon::Make<SsaInput> as Impl
141+
142+
class Definition = Impl::Definition;
143+
144+
predicate adjacentStep(CaptureAccess access1, CaptureAccess access2) {
145+
exists(
146+
Definition def, SsaInput::BasicBlock bb1, int i1, SsaInput::BasicBlock bb2, int i2,
147+
SsaInput::SourceVariable v
148+
|
149+
Impl::adjacentDefRead(def, bb1, i1, bb2, i2) and
150+
v = def.getSourceVariable() and
151+
access1.isReadOrWrite(bb1, i1, v) and
152+
access2.isReadOrWrite(bb2, i2, v)
153+
)
154+
}

0 commit comments

Comments
 (0)