Ruby: CFG: make RescueClause post-order

Author: aibaars

ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll

@@ -246,7 +246,7 @@ module Trees {
       )
       or
       // Last element from any matching `rescue` block continues to the `ensure` block
-      this.getRescue(_).(RescueTree).lastMatch(result, c) and
+      last(this.getRescue(_), result, c) and
       ensurable = true
       or
       // If the last `rescue` block does not match, continue to the `ensure` block
@@ -1170,80 +1170,68 @@ module Trees {
     }
   }
 
-  private class RescueTree extends PreOrderTree, RescueClause {
-    final override predicate propagatesAbnormal(AstNode child) { child = this.getAnException() }
-
-    private Expr getLastException() {
-      exists(int i | result = this.getException(i) and not exists(this.getException(i + 1)))
+  private class RescueTree extends PostOrderTree, RescueClause {
+    final override predicate propagatesAbnormal(AstNode child) {
+      child = this.getAnException() or
+      child = this.getBody()
     }
 
-    predicate lastMatch(AstNode last, Completion c) {
-      last(this.getBody(), last, c)
+    final override predicate first(AstNode first) {
+      first(this.getException(0), first)
       or
-      not exists(this.getBody()) and
+      not exists(this.getException(0)) and
       (
-        last(this.getVariableExpr(), last, c)
+        first(this.getVariableExpr(), first)
         or
         not exists(this.getVariableExpr()) and
         (
-          last(this.getAnException(), last, c) and
-          c.(MatchingCompletion).getValue() = true
+          first(this.getBody(), first)
           or
-          not exists(this.getAnException()) and
-          last = this and
-          c.isValidFor(this)
+          not exists(this.getBody()) and first = this
         )
       )
     }
 
+    private Expr getLastException() {
+      exists(int i | result = this.getException(i) and not exists(this.getException(i + 1)))
+    }
+
     predicate lastNoMatch(AstNode last, Completion c) {
       last(this.getLastException(), last, c) and
       c.(MatchingCompletion).getValue() = false
     }
 
-    final override predicate last(AstNode last, Completion c) {
-      this.lastNoMatch(last, c)
-      or
-      this.lastMatch(last, c)
-    }
-
     final override predicate succ(AstNode pred, AstNode succ, Completion c) {
-      exists(AstNode next |
-        pred = this and
-        first(next, succ) and
-        c instanceof SimpleCompletion
-      |
-        next = this.getException(0)
+      last(this.getAnException(), pred, c) and
+      c.(MatchingCompletion).getValue() = true and
+      (
+        first(this.getVariableExpr(), succ)
         or
-        not exists(this.getException(0)) and
+        not exists(this.getVariableExpr()) and
         (
-          next = this.getVariableExpr()
+          first(this.getBody(), succ)
           or
-          not exists(this.getVariableExpr()) and
-          next = this.getBody()
+          not exists(this.getBody()) and succ = this
         )
       )
       or
-      exists(AstNode next |
-        last(this.getAnException(), pred, c) and
-        first(next, succ) and
-        c.(MatchingCompletion).getValue() = true
-      |
-        next = this.getVariableExpr()
-        or
-        not exists(this.getVariableExpr()) and
-        next = this.getBody()
-      )
-      or
       exists(int i |
         last(this.getException(i), pred, c) and
         c.(MatchingCompletion).getValue() = false and
         first(this.getException(i + 1), succ)
       )
       or
       last(this.getVariableExpr(), pred, c) and
-      first(this.getBody(), succ) and
-      c instanceof NormalCompletion
+      c instanceof NormalCompletion and
+      (
+        first(this.getBody(), succ)
+        or
+        not exists(this.getBody()) and succ = this
+      )
+      or
+      last(this.getBody(), pred, c) and
+      c instanceof NormalCompletion and
+      succ = this
     }
   }
 

ruby/ql/test/library-tests/controlflow/graph/Cfg.expected

@@ -4914,7 +4914,7 @@ raise.rb:
 #-----| true -> self
 
 #   17| call to raise
-#-----| raise -> rescue ...
+#-----| raise -> ExceptionA
 
 #   17| self
 #-----|  -> ExceptionA
@@ -4923,14 +4923,14 @@ raise.rb:
 #-----|  -> call to raise
 
 #   19| rescue ...
-#-----|  -> ExceptionA
+#-----|  -> self
 
 #   19| ExceptionA
 #-----| match -> self
 #-----| raise -> exit m2 (abnormal)
 
 #   19| then ...
-#-----|  -> self
+#-----|  -> rescue ...
 
 #   20| call to puts
 #-----|  -> then ...
@@ -4972,7 +4972,7 @@ raise.rb:
 #-----| true -> self
 
 #   28| call to raise
-#-----| raise -> rescue ...
+#-----| raise -> self
 
 #   28| self
 #-----|  -> ExceptionA
@@ -4984,7 +4984,7 @@ raise.rb:
 #-----|  -> self
 
 #   30| then ...
-#-----|  -> self
+#-----|  -> rescue ...
 
 #   31| call to puts
 #-----|  -> then ...
@@ -5026,7 +5026,7 @@ raise.rb:
 #-----| true -> self
 
 #   39| call to raise
-#-----| raise -> rescue ...
+#-----| raise -> e
 
 #   39| self
 #-----|  -> ExceptionA
@@ -5035,13 +5035,13 @@ raise.rb:
 #-----|  -> call to raise
 
 #   41| rescue ...
-#-----|  -> e
+#-----|  -> self
 
 #   41| e
 #-----|  -> self
 
 #   41| then ...
-#-----|  -> self
+#-----|  -> rescue ...
 
 #   42| call to puts
 #-----|  -> then ...
@@ -5083,7 +5083,7 @@ raise.rb:
 #-----| true -> self
 
 #   50| call to raise
-#-----| raise -> rescue ...
+#-----| raise -> e
 
 #   50| self
 #-----|  -> ExceptionA
@@ -5092,10 +5092,10 @@ raise.rb:
 #-----|  -> call to raise
 
 #   52| rescue ...
-#-----|  -> e
+#-----|  -> self
 
 #   52| e
-#-----|  -> self
+#-----|  -> rescue ...
 
 #   54| call to puts
 #-----|  -> exit m5 (normal)
@@ -5131,7 +5131,7 @@ raise.rb:
 #-----| true -> self
 
 #   60| call to raise
-#-----| raise -> rescue ...
+#-----| raise -> ExceptionA
 
 #   60| self
 #-----|  -> ExceptionA
@@ -5140,7 +5140,7 @@ raise.rb:
 #-----|  -> call to raise
 
 #   62| rescue ...
-#-----|  -> ExceptionA
+#-----|  -> self
 
 #   62| ExceptionA
 #-----| no-match -> ExceptionB
@@ -5154,7 +5154,7 @@ raise.rb:
 #-----|  -> self
 
 #   62| then ...
-#-----|  -> self
+#-----|  -> rescue ...
 
 #   63| call to puts
 #-----|  -> then ...
@@ -5813,7 +5813,7 @@ raise.rb:
 #-----| true -> self
 
 #  131| call to raise
-#-----| raise -> rescue ...
+#-----| raise -> ExceptionA
 
 #  131| self
 #-----|  -> ExceptionA
@@ -5822,21 +5822,21 @@ raise.rb:
 #-----|  -> call to raise
 
 #  133| rescue ...
-#-----|  -> ExceptionA
+#-----|  -> self
 
 #  133| ExceptionA
-#-----| no-match -> rescue ...
-#-----| match -> self
+#-----| match -> rescue ...
+#-----| no-match -> ExceptionB
 
 #  134| rescue ...
-#-----|  -> ExceptionB
+#-----|  -> self
 
 #  134| ExceptionB
 #-----| match -> self
 #-----| raise -> [ensure: raise] self
 
 #  134| then ...
-#-----|  -> self
+#-----|  -> rescue ...
 
 #  135| call to puts
 #-----|  -> then ...