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.

Author: hmac

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

@@ -71,6 +71,9 @@ class ClassVariable extends Variable instanceof ClassVariableImpl {
   final override ClassVariableAccess getAnAccess() { result.getVariable() = this }
 }
 
+/** The `self` variable */
+class SelfVariable extends Variable instanceof SelfVariableImpl { }
+
 /** An access to a variable. */
 class VariableAccess extends Expr instanceof VariableAccessImpl {
   /** Gets the variable this identifier refers to. */
@@ -185,3 +188,8 @@ class ClassVariableWriteAccess extends ClassVariableAccess, VariableWriteAccess
 
 /** An access to a class variable where the value is read. */
 class ClassVariableReadAccess extends ClassVariableAccess, VariableReadAccess { }
+
+/** An access to the `self` variable */
+class SelfVariableAccess extends VariableAccess instanceof SelfVariableAccessImpl {
+  final override string getAPrimaryQlClass() { result = "SelfVariableAccess" }
+}

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

@@ -3,6 +3,7 @@ private import TreeSitter
 private import codeql.ruby.ast.internal.Call
 private import codeql.ruby.ast.internal.Parameter
 private import codeql.ruby.ast.internal.Variable
+private import codeql.ruby.ast.internal.Scope
 private import codeql.ruby.AST as AST
 private import Synthesis
 
@@ -236,6 +237,7 @@ private module Cached {
     } or
     TSelfReal(Ruby::Self g) or
     TSelfSynth(AST::AstNode parent, int i) { mkSynthChild(SelfKind(), parent, i) } or
+    TSelfVariableAccessReal(Ruby::Self self, MethodBase::Range scope) { scopeOf(self) = scope } or
     TSimpleParameter(Ruby::Identifier g) { g instanceof Parameter::Range } or
     TSimpleSymbolLiteral(Ruby::SimpleSymbol g) or
     TSingletonClass(Ruby::SingletonClass g) or
@@ -693,7 +695,8 @@ class TNamedParameter =
 class TTuplePattern = TTuplePatternParameter or TDestructuredLeftAssignment or TLeftAssignmentList;
 
 class TVariableAccess =
-  TLocalVariableAccess or TGlobalVariableAccess or TInstanceVariableAccess or TClassVariableAccess;
+  TLocalVariableAccess or TGlobalVariableAccess or TInstanceVariableAccess or
+      TClassVariableAccess or TSelfVariableAccess;
 
 class TLocalVariableAccess = TLocalVariableAccessReal or TLocalVariableAccessSynth;
 
@@ -702,3 +705,5 @@ class TGlobalVariableAccess = TGlobalVariableAccessReal or TGlobalVariableAccess
 class TInstanceVariableAccess = TInstanceVariableAccessReal or TInstanceVariableAccessSynth;
 
 class TClassVariableAccess = TClassVariableAccessReal or TClassVariableAccessSynth;
+
+class TSelfVariableAccess = TSelfVariableAccessReal;

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

@@ -133,6 +133,7 @@ private module Cached {
       not scopeDefinesParameterVariable(scope, name, _) and
       not inherits(scope, name, _)
     } or
+    TSelfVariable(MethodBase::Range scope) or
     TLocalVariableSynth(AstNode n, int i) { any(Synthesis s).localVariable(n, i) }
 
   // Db types that can be vcalls
@@ -374,9 +375,10 @@ abstract class VariableImpl extends TVariable {
   abstract Location getLocationImpl();
 }
 
-class TVariableReal = TGlobalVariable or TClassVariable or TInstanceVariable or TLocalVariableReal;
+class TVariableReal =
+  TGlobalVariable or TClassVariable or TInstanceVariable or TLocalVariableReal or TSelfVariable;
 
-class TLocalVariable = TLocalVariableReal or TLocalVariableSynth;
+class TLocalVariable = TLocalVariableReal or TLocalVariableSynth or TSelfVariable;
 
 /**
  * This class only exists to avoid negative recursion warnings. Ideally,
@@ -475,6 +477,18 @@ class ClassVariableImpl extends VariableReal, TClassVariable {
   final override Scope::Range getDeclaringScopeImpl() { result = scope }
 }
 
+class SelfVariableImpl extends VariableReal, TSelfVariable {
+  private MethodBase::Range scope;
+
+  SelfVariableImpl() { this = TSelfVariable(scope) }
+
+  final override string getNameImpl() { result = "self" }
+
+  final override Location getLocationImpl() { result = scope.getLocation() }
+
+  final override Scope::Range getDeclaringScopeImpl() { result = scope }
+}
+
 abstract class VariableAccessImpl extends Expr, TVariableAccess {
   abstract VariableImpl getVariableImpl();
 }
@@ -602,3 +616,18 @@ private class ClassVariableAccessSynth extends ClassVariableAccessRealImpl,
 
   final override string toString() { result = v.getName() }
 }
+
+abstract class SelfVariableAccessImpl extends VariableAccessImpl, TSelfVariableAccess { }
+
+private class SelfVariableAccessReal extends SelfVariableAccessImpl, TSelfVariableAccessReal {
+  private Ruby::Self self;
+  private SelfVariable var;
+
+  SelfVariableAccessReal() {
+    exists(MethodBase::Range scope |
+      var = TSelfVariable(scope) and this = TSelfVariableAccessReal(self, scope)
+    )
+  }
+
+  final override SelfVariable getVariableImpl() { result = var }
+}

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

@@ -59,7 +59,7 @@ class ExitNode extends CfgNode, TExitNode {
 /**
  * A node for an AST node.
  *
- * Each AST node maps to zero or more `AstCfgNode`s: zero when the node in unreachable
+ * Each AST node maps to zero or more `AstCfgNode`s: zero when the node is unreachable
  * (dead) code or not important for control flow, and multiple when there are different
  * splits for the AST node.
  */

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

@@ -17,8 +17,19 @@ class ExitBasicBlock = BasicBlocks::ExitBasicBlock;
 
 class SourceVariable = LocalVariable;
 
+/**
+ * Holds if the statement at index `i` of basic block `bb` contains a write to variable `v`.
+ * `certain` is true if the write definitely occurs.
+ */
 predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) {
   (
+    // We consider the `self` variable to have a single write at the entry to a method block.
+    exists(SelfVariableAccess access |
+      access.getCfgScope() = bb.getScope() and
+      access.getVariable() = v and
+      i = 0
+    )
+    or
     SsaImpl::uninitializedWrite(bb, i, v)
     or
     SsaImpl::capturedEntryWrite(bb, i, v)