Skip to content

Commit 336bd15

Browse files
committed
Override isCapturedAccess for self variables
Many `self` reads are synthesised from method calls with an implicit `self` receiver. Synthesised nodes have no `toGenerated` result, which the default definition of `isCapturedAccess` uses to determine if a variable's scope matches the access's scope. Hence we override the definition to properly identify accesses like the call `puts` (below) as captured reads of a `self` variable defined in a parent scope. In other words, `puts x` is short for `self.puts x` and the `self` refers to its value in the scope of the module `Foo`. ```ruby module Foo MY_PROC = -> (x) { puts x } end ``` We also have to update the SSA `SelfDefinition` to exclude captured `self` variables.
1 parent f1add38 commit 336bd15

File tree

2 files changed

+15
-4
lines changed

2 files changed

+15
-4
lines changed

ruby/ql/lib/codeql/ruby/ast/Variable.qll

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ class LocalVariableAccess extends VariableAccess instanceof LocalVariableAccessI
147147
* the access to `x` in the first `puts x` is a captured access, while
148148
* the access to `x` in the second `puts x` is not.
149149
*/
150-
final predicate isCapturedAccess() { isCapturedAccess(this) }
150+
predicate isCapturedAccess() { isCapturedAccess(this) }
151151
}
152152

153153
/** An access to a local variable where the value is updated. */
@@ -195,4 +195,10 @@ class SelfVariableAccess extends LocalVariableAccess instanceof SelfVariableAcce
195195
}
196196

197197
/** An access to the `self` variable where the value is read. */
198-
class SelfVariableReadAccess extends SelfVariableAccess, VariableReadAccess { }
198+
class SelfVariableReadAccess extends SelfVariableAccess, VariableReadAccess {
199+
// We override the definition in `LocalVariableAccess` because it gives the
200+
// wrong result for synthesised `self` variables.
201+
override predicate isCapturedAccess() {
202+
this.getVariable().getDeclaringScope() != this.getCfgScope()
203+
}
204+
}

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,12 +221,17 @@ module Ssa {
221221
}
222222

223223
/**
224-
* An SSA definition that corresponds to the value of `self` upon method entry.
224+
* An SSA definition that corresponds to the value of `self` upon entry to a method, class or module.
225225
*/
226226
class SelfDefinition extends Definition, SsaImplCommon::WriteDefinition {
227227
private SelfVariable v;
228228

229-
SelfDefinition() { this.definesAt(v, _, _) }
229+
SelfDefinition() {
230+
exists(BasicBlock bb, int i |
231+
this.definesAt(v, bb, i) and
232+
not SsaImpl::capturedEntryWrite(bb, i, v)
233+
)
234+
}
230235

231236
final override string toString() { result = "self (" + v.getDeclaringScope() + ")" }
232237

0 commit comments

Comments
 (0)