Ruby: Fix SSA inconsistency

Author: hvitved

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

@@ -143,23 +143,23 @@ private predicate hasCapturedRead(Variable v, Cfg::CfgScope scope) {
 }
 
 /**
- * Holds if `v` is written inside basic block `bb`, which is in the immediate
- * outer scope of `scope`.
+ * Holds if `v` is written inside basic block `bb` at index `i`, which is in
+ * the immediate outer scope of `scope`.
  */
 pragma[noinline]
-private predicate variableWriteInOuterScope(Cfg::BasicBlock bb, LocalVariable v, Cfg::CfgScope scope) {
-  SsaInput::variableWrite(bb, _, v, _) and
+private predicate variableWriteInOuterScope(
+  Cfg::BasicBlock bb, int i, LocalVariable v, Cfg::CfgScope scope
+) {
+  SsaInput::variableWrite(bb, i, v, _) and
   scope.getOuterCfgScope() = bb.getScope()
 }
 
 pragma[noinline]
-private predicate proceedsVariableWriteWithCapturedRead(
-  Cfg::BasicBlock bb, LocalVariable v, Cfg::CfgScope scope
+private predicate hasVariableWriteWithCapturedRead(
+  Cfg::BasicBlock bb, int i, LocalVariable v, Cfg::CfgScope scope
 ) {
   hasCapturedRead(v, scope) and
-  variableWriteInOuterScope(bb, v, scope)
-  or
-  proceedsVariableWriteWithCapturedRead(bb.getAPredecessor(), v, scope)
+  variableWriteInOuterScope(bb, i, v, scope)
 }
 
 /**
@@ -168,7 +168,10 @@ private predicate proceedsVariableWriteWithCapturedRead(
  */
 private predicate capturedCallRead(CallCfgNode call, Cfg::BasicBlock bb, int i, LocalVariable v) {
   exists(Cfg::CfgScope scope |
-    proceedsVariableWriteWithCapturedRead(bb, v, scope) and
+    (
+      hasVariableWriteWithCapturedRead(bb, any(int j | j < i), v, scope) or
+      hasVariableWriteWithCapturedRead(bb.getAPredecessor+(), _, v, scope)
+    ) and
     call = bb.getNode(i)
   |
     // If the read happens inside a block, we restrict to the call that
@@ -199,23 +202,23 @@ private predicate hasCapturedWrite(Variable v, Cfg::CfgScope scope) {
 }
 
 /**
- * Holds if `v` is read inside basic block `bb`, which is in the immediate
- * outer scope of `scope`.
+ * Holds if `v` is read inside basic block `bb` at index `i`, which is in the
+ * immediate outer scope of `scope`.
  */
 pragma[noinline]
 private predicate variableReadActualInOuterScope(
-  Cfg::BasicBlock bb, LocalVariable v, Cfg::CfgScope scope
+  Cfg::BasicBlock bb, int i, LocalVariable v, Cfg::CfgScope scope
 ) {
-  variableReadActual(bb, _, v) and
+  variableReadActual(bb, i, v) and
   bb.getScope() = scope.getOuterCfgScope()
 }
 
 pragma[noinline]
 private predicate hasVariableReadWithCapturedWrite(
-  Cfg::BasicBlock bb, LocalVariable v, Cfg::CfgScope scope
+  Cfg::BasicBlock bb, int i, LocalVariable v, Cfg::CfgScope scope
 ) {
   hasCapturedWrite(v, scope) and
-  variableReadActualInOuterScope(bb, v, scope)
+  variableReadActualInOuterScope(bb, i, v, scope)
 }
 
 pragma[noinline]
@@ -324,7 +327,11 @@ private module Cached {
   cached
   predicate capturedCallWrite(CallCfgNode call, Cfg::BasicBlock bb, int i, LocalVariable v) {
     exists(Cfg::CfgScope scope |
-      hasVariableReadWithCapturedWrite(bb.getASuccessor*(), v, scope) and
+      (
+        hasVariableReadWithCapturedWrite(bb, any(int j | j > i), v, scope)
+        or
+        hasVariableReadWithCapturedWrite(bb.getASuccessor+(), _, v, scope)
+      ) and
       call = bb.getNode(i)
     |
       // If the write happens inside a block, we restrict to the call that

ruby/ql/test/library-tests/variables/ssa.expected

@@ -90,7 +90,6 @@ definition
 | scopes.rb:2:9:6:3 | <captured entry> self | scopes.rb:1:1:49:4 | self |
 | scopes.rb:4:4:4:4 | a | scopes.rb:4:4:4:4 | a |
 | scopes.rb:7:1:7:1 | a | scopes.rb:7:1:7:1 | a |
-| scopes.rb:9:1:18:3 | <captured exit> a | scopes.rb:7:1:7:1 | a |
 | scopes.rb:9:9:18:3 | <captured entry> a | scopes.rb:7:1:7:1 | a |
 | scopes.rb:9:9:18:3 | <captured entry> self | scopes.rb:1:1:49:4 | self |
 | scopes.rb:11:4:11:4 | a | scopes.rb:7:1:7:1 | a |