Skip to content

Commit e7a3050

Browse files
committed
Improve the modelling of self variables.
We model `self` variables by inserting a write at the start of every method body. We then treat them as local variables that are alive for the extent of the method body.
1 parent 3f396ac commit e7a3050

File tree

5 files changed

+57
-4
lines changed

5 files changed

+57
-4
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ class ClassVariable extends Variable instanceof ClassVariableImpl {
7171
final override ClassVariableAccess getAnAccess() { result.getVariable() = this }
7272
}
7373

74+
/** The `self` variable */
75+
class SelfVariable extends Variable instanceof SelfVariableImpl { }
76+
7477
/** An access to a variable. */
7578
class VariableAccess extends Expr instanceof VariableAccessImpl {
7679
/** Gets the variable this identifier refers to. */
@@ -185,3 +188,8 @@ class ClassVariableWriteAccess extends ClassVariableAccess, VariableWriteAccess
185188

186189
/** An access to a class variable where the value is read. */
187190
class ClassVariableReadAccess extends ClassVariableAccess, VariableReadAccess { }
191+
192+
/** An access to the `self` variable */
193+
class SelfVariableAccess extends VariableAccess instanceof SelfVariableAccessImpl {
194+
final override string getAPrimaryQlClass() { result = "SelfVariableAccess" }
195+
}

ruby/ql/lib/codeql/ruby/ast/internal/AST.qll

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ private import TreeSitter
33
private import codeql.ruby.ast.internal.Call
44
private import codeql.ruby.ast.internal.Parameter
55
private import codeql.ruby.ast.internal.Variable
6+
private import codeql.ruby.ast.internal.Scope
67
private import codeql.ruby.AST as AST
78
private import Synthesis
89

@@ -236,6 +237,7 @@ private module Cached {
236237
} or
237238
TSelfReal(Ruby::Self g) or
238239
TSelfSynth(AST::AstNode parent, int i) { mkSynthChild(SelfKind(), parent, i) } or
240+
TSelfVariableAccessReal(Ruby::Self self, MethodBase::Range scope) { scopeOf(self) = scope } or
239241
TSimpleParameter(Ruby::Identifier g) { g instanceof Parameter::Range } or
240242
TSimpleSymbolLiteral(Ruby::SimpleSymbol g) or
241243
TSingletonClass(Ruby::SingletonClass g) or
@@ -693,7 +695,8 @@ class TNamedParameter =
693695
class TTuplePattern = TTuplePatternParameter or TDestructuredLeftAssignment or TLeftAssignmentList;
694696

695697
class TVariableAccess =
696-
TLocalVariableAccess or TGlobalVariableAccess or TInstanceVariableAccess or TClassVariableAccess;
698+
TLocalVariableAccess or TGlobalVariableAccess or TInstanceVariableAccess or
699+
TClassVariableAccess or TSelfVariableAccess;
697700

698701
class TLocalVariableAccess = TLocalVariableAccessReal or TLocalVariableAccessSynth;
699702

@@ -702,3 +705,5 @@ class TGlobalVariableAccess = TGlobalVariableAccessReal or TGlobalVariableAccess
702705
class TInstanceVariableAccess = TInstanceVariableAccessReal or TInstanceVariableAccessSynth;
703706

704707
class TClassVariableAccess = TClassVariableAccessReal or TClassVariableAccessSynth;
708+
709+
class TSelfVariableAccess = TSelfVariableAccessReal;

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

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ private module Cached {
133133
not scopeDefinesParameterVariable(scope, name, _) and
134134
not inherits(scope, name, _)
135135
} or
136+
TSelfVariable(MethodBase::Range scope) or
136137
TLocalVariableSynth(AstNode n, int i) { any(Synthesis s).localVariable(n, i) }
137138

138139
// Db types that can be vcalls
@@ -374,9 +375,10 @@ abstract class VariableImpl extends TVariable {
374375
abstract Location getLocationImpl();
375376
}
376377

377-
class TVariableReal = TGlobalVariable or TClassVariable or TInstanceVariable or TLocalVariableReal;
378+
class TVariableReal =
379+
TGlobalVariable or TClassVariable or TInstanceVariable or TLocalVariableReal or TSelfVariable;
378380

379-
class TLocalVariable = TLocalVariableReal or TLocalVariableSynth;
381+
class TLocalVariable = TLocalVariableReal or TLocalVariableSynth or TSelfVariable;
380382

381383
/**
382384
* This class only exists to avoid negative recursion warnings. Ideally,
@@ -475,6 +477,18 @@ class ClassVariableImpl extends VariableReal, TClassVariable {
475477
final override Scope::Range getDeclaringScopeImpl() { result = scope }
476478
}
477479

480+
class SelfVariableImpl extends VariableReal, TSelfVariable {
481+
private MethodBase::Range scope;
482+
483+
SelfVariableImpl() { this = TSelfVariable(scope) }
484+
485+
final override string getNameImpl() { result = "self" }
486+
487+
final override Location getLocationImpl() { result = scope.getLocation() }
488+
489+
final override Scope::Range getDeclaringScopeImpl() { result = scope }
490+
}
491+
478492
abstract class VariableAccessImpl extends Expr, TVariableAccess {
479493
abstract VariableImpl getVariableImpl();
480494
}
@@ -602,3 +616,18 @@ private class ClassVariableAccessSynth extends ClassVariableAccessRealImpl,
602616

603617
final override string toString() { result = v.getName() }
604618
}
619+
620+
abstract class SelfVariableAccessImpl extends VariableAccessImpl, TSelfVariableAccess { }
621+
622+
private class SelfVariableAccessReal extends SelfVariableAccessImpl, TSelfVariableAccessReal {
623+
private Ruby::Self self;
624+
private SelfVariable var;
625+
626+
SelfVariableAccessReal() {
627+
exists(MethodBase::Range scope |
628+
var = TSelfVariable(scope) and this = TSelfVariableAccessReal(self, scope)
629+
)
630+
}
631+
632+
final override SelfVariable getVariableImpl() { result = var }
633+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ class ExitNode extends CfgNode, TExitNode {
5959
/**
6060
* A node for an AST node.
6161
*
62-
* Each AST node maps to zero or more `AstCfgNode`s: zero when the node in unreachable
62+
* Each AST node maps to zero or more `AstCfgNode`s: zero when the node is unreachable
6363
* (dead) code or not important for control flow, and multiple when there are different
6464
* splits for the AST node.
6565
*/

ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImplSpecific.qll

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,19 @@ class ExitBasicBlock = BasicBlocks::ExitBasicBlock;
1717

1818
class SourceVariable = LocalVariable;
1919

20+
/**
21+
* Holds if the statement at index `i` of basic block `bb` contains a write to variable `v`.
22+
* `certain` is true if the write definitely occurs.
23+
*/
2024
predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) {
2125
(
26+
// We consider the `self` variable to have a single write at the entry to a method block.
27+
exists(SelfVariableAccess access |
28+
access.getCfgScope() = bb.getScope() and
29+
access.getVariable() = v and
30+
i = 0
31+
)
32+
or
2233
SsaImpl::uninitializedWrite(bb, i, v)
2334
or
2435
SsaImpl::capturedEntryWrite(bb, i, v)

0 commit comments

Comments
 (0)