CFG: Use MatchingCompletion for patterns

2026-02-20 00:43:44 +01:00 · 2020-12-07 17:19:34 +01:00
parent 31b8d33a7c
commit 80a59a81ed
3 changed files with 43 additions and 45 deletions
--- a/ql/src/codeql_ruby/controlflow/internal/Completion.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/Completion.qll
@@ -170,7 +170,11 @@ private predicate inBooleanContext(AstNode n) {
  or
  n = any(ParenthesizedStatement parent | inBooleanContext(parent)).getChild()
  or
-  n instanceof Pattern
+  exists(Case c, When w |
+    not exists(c.getValue()) and
+    c.getChild(_) = w and
+    w.getPattern(_).getChild() = n
+  )
 }

 /**
@@ -185,7 +189,15 @@ private predicate mustHaveMatchingCompletion(AstNode n) {
 * Holds if `n` is used in a matching context. That is, whether or
 * not the value of `n` matches, determines the successor.
 */
-private predicate inMatchingContext(AstNode n) { n = any(Rescue r).getExceptions().getChild(_) }
+private predicate inMatchingContext(AstNode n) {
+  n = any(Rescue r).getExceptions().getChild(_)
+  or
+  exists(Case c, When w |
+    exists(c.getValue()) and
+    c.getChild(_) = w and
+    w.getPattern(_).getChild() = n
+  )
+}

 /**
 * A completion that represents normal evaluation of a statement or an
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -362,7 +362,7 @@ module Trees {
      exists(int i, WhenTree branch | branch = this.getChild(i) |
        last(branch.getLastPattern(), pred, c) and
        first(this.getChild(i + 1), succ) and
-        c instanceof FalseCompletion
+        c.(ConditionalCompletion).getValue() = false
      )
    }
  }
@@ -836,8 +836,10 @@ module Trees {
    final override AstNode getChildNode(int i) { result = this.getChild(i) }
  }

-  private class PatternTree extends StandardPostOrderTree, Pattern {
+  private class PatternTree extends StandardPreOrderTree, Pattern {
    final override AstNode getChildNode(int i) { result = this.getChild() and i = 0 }
+
+    final override predicate isHidden() { any() }
  }

  private class ProgramTree extends StandardPreOrderTree, Program {
@@ -1277,7 +1279,7 @@ module Trees {

    final override predicate last(AstNode last, Completion c) {
      last(this.getLastPattern(), last, c) and
-      c instanceof FalseCompletion
+      c.(ConditionalCompletion).getValue() = false
      or
      last(this.getBody(), last, c)
    }
@@ -1287,13 +1289,15 @@ module Trees {
      first(this.getPattern(0), succ) and
      c instanceof SimpleCompletion
      or
-      exists(int i, Pattern p |
+      exists(int i, Pattern p, boolean b |
        p = this.getPattern(i) and
-        last(p, pred, c)
+        last(p, pred, c) and
+        b = c.(ConditionalCompletion).getValue()
      |
-        c instanceof TrueCompletion and first(this.getBody(), succ)
+        b = true and
+        first(this.getBody(), succ)
        or
-        c instanceof FalseCompletion and
+        b = false and
        first(this.getPattern(i + 1), succ)
      )
    }
--- a/ql/test/library-tests/controlflow/graph/Cfg.expected
+++ b/ql/test/library-tests/controlflow/graph/Cfg.expected
@@ -84,7 +84,6 @@ nodes
 | case.rb:2:8:2:9 | x1 |
 | case.rb:3:5:3:42 | When |
 | case.rb:3:10:3:10 | 1 |
-| case.rb:3:10:3:10 | Pattern |
 | case.rb:3:17:3:42 | ParenthesizedStatements |
 | case.rb:3:18:3:41 | If |
 | case.rb:3:21:3:22 | x2 |
@@ -93,7 +92,6 @@ nodes
 | case.rb:3:34:3:37 | String |
 | case.rb:4:5:4:24 | When |
 | case.rb:4:10:4:10 | 2 |
-| case.rb:4:10:4:10 | Pattern |
 | case.rb:4:17:4:20 | puts |
 | case.rb:4:17:4:24 | MethodCall |
 | case.rb:4:22:4:24 | String |
@@ -177,17 +175,13 @@ nodes
 | cfg.rb:41:6:41:7 | 10 |
 | cfg.rb:42:3:42:24 | When |
 | cfg.rb:42:8:42:8 | 1 |
-| cfg.rb:42:8:42:8 | Pattern |
 | cfg.rb:42:15:42:18 | puts |
 | cfg.rb:42:15:42:24 | MethodCall |
 | cfg.rb:42:20:42:24 | String |
 | cfg.rb:43:3:43:31 | When |
 | cfg.rb:43:8:43:8 | 2 |
-| cfg.rb:43:8:43:8 | Pattern |
 | cfg.rb:43:11:43:11 | 3 |
-| cfg.rb:43:11:43:11 | Pattern |
 | cfg.rb:43:14:43:14 | 4 |
-| cfg.rb:43:14:43:14 | Pattern |
 | cfg.rb:43:21:43:24 | puts |
 | cfg.rb:43:21:43:31 | MethodCall |
 | cfg.rb:43:26:43:31 | String |
@@ -198,7 +192,6 @@ nodes
 | cfg.rb:48:3:48:29 | When |
 | cfg.rb:48:8:48:8 | b |
 | cfg.rb:48:8:48:13 | Binary |
-| cfg.rb:48:8:48:13 | Pattern |
 | cfg.rb:48:13:48:13 | 1 |
 | cfg.rb:48:20:48:23 | puts |
 | cfg.rb:48:20:48:29 | MethodCall |
@@ -206,11 +199,9 @@ nodes
 | cfg.rb:49:3:49:37 | When |
 | cfg.rb:49:8:49:8 | b |
 | cfg.rb:49:8:49:13 | Binary |
-| cfg.rb:49:8:49:13 | Pattern |
 | cfg.rb:49:13:49:13 | 0 |
 | cfg.rb:49:16:49:16 | b |
 | cfg.rb:49:16:49:20 | Binary |
-| cfg.rb:49:16:49:20 | Pattern |
 | cfg.rb:49:20:49:20 | 1 |
 | cfg.rb:49:27:49:30 | puts |
 | cfg.rb:49:27:49:37 | MethodCall |
@@ -1177,9 +1168,8 @@ edges
 | case.rb:2:3:5:5 | Case | case.rb:2:8:2:9 | x1 | semmle.label | successor |
 | case.rb:2:8:2:9 | x1 | case.rb:3:5:3:42 | When | semmle.label | successor |
 | case.rb:3:5:3:42 | When | case.rb:3:10:3:10 | 1 | semmle.label | successor |
-| case.rb:3:10:3:10 | 1 | case.rb:3:10:3:10 | Pattern | semmle.label | successor |
-| case.rb:3:10:3:10 | Pattern | case.rb:3:18:3:41 | If | semmle.label | true |
-| case.rb:3:10:3:10 | Pattern | case.rb:4:5:4:24 | When | semmle.label | false |
+| case.rb:3:10:3:10 | 1 | case.rb:3:18:3:41 | If | semmle.label | match |
+| case.rb:3:10:3:10 | 1 | case.rb:4:5:4:24 | When | semmle.label | no-match |
 | case.rb:3:17:3:42 | ParenthesizedStatements | case.rb:1:1:6:3 | exit if_in_case (normal) | semmle.label | successor |
 | case.rb:3:18:3:41 | If | case.rb:3:21:3:22 | x2 | semmle.label | successor |
 | case.rb:3:21:3:22 | x2 | case.rb:3:17:3:42 | ParenthesizedStatements | semmle.label | false |
@@ -1188,9 +1178,8 @@ edges
 | case.rb:3:29:3:37 | MethodCall | case.rb:3:17:3:42 | ParenthesizedStatements | semmle.label | successor |
 | case.rb:3:34:3:37 | String | case.rb:3:29:3:32 | puts | semmle.label | successor |
 | case.rb:4:5:4:24 | When | case.rb:4:10:4:10 | 2 | semmle.label | successor |
-| case.rb:4:10:4:10 | 2 | case.rb:4:10:4:10 | Pattern | semmle.label | successor |
-| case.rb:4:10:4:10 | Pattern | case.rb:1:1:6:3 | exit if_in_case (normal) | semmle.label | false |
-| case.rb:4:10:4:10 | Pattern | case.rb:4:22:4:24 | String | semmle.label | true |
+| case.rb:4:10:4:10 | 2 | case.rb:1:1:6:3 | exit if_in_case (normal) | semmle.label | no-match |
+| case.rb:4:10:4:10 | 2 | case.rb:4:22:4:24 | String | semmle.label | match |
 | case.rb:4:17:4:20 | puts | case.rb:4:17:4:24 | MethodCall | semmle.label | successor |
 | case.rb:4:17:4:24 | MethodCall | case.rb:1:1:6:3 | exit if_in_case (normal) | semmle.label | successor |
 | case.rb:4:22:4:24 | String | case.rb:4:17:4:20 | puts | semmle.label | successor |
@@ -1268,22 +1257,18 @@ edges
 | cfg.rb:41:1:45:3 | Case | cfg.rb:41:6:41:7 | 10 | semmle.label | successor |
 | cfg.rb:41:6:41:7 | 10 | cfg.rb:42:3:42:24 | When | semmle.label | successor |
 | cfg.rb:42:3:42:24 | When | cfg.rb:42:8:42:8 | 1 | semmle.label | successor |
-| cfg.rb:42:8:42:8 | 1 | cfg.rb:42:8:42:8 | Pattern | semmle.label | successor |
-| cfg.rb:42:8:42:8 | Pattern | cfg.rb:42:20:42:24 | String | semmle.label | true |
-| cfg.rb:42:8:42:8 | Pattern | cfg.rb:43:3:43:31 | When | semmle.label | false |
+| cfg.rb:42:8:42:8 | 1 | cfg.rb:42:20:42:24 | String | semmle.label | match |
+| cfg.rb:42:8:42:8 | 1 | cfg.rb:43:3:43:31 | When | semmle.label | no-match |
 | cfg.rb:42:15:42:18 | puts | cfg.rb:42:15:42:24 | MethodCall | semmle.label | successor |
 | cfg.rb:42:15:42:24 | MethodCall | cfg.rb:47:1:50:3 | Case | semmle.label | successor |
 | cfg.rb:42:20:42:24 | String | cfg.rb:42:15:42:18 | puts | semmle.label | successor |
 | cfg.rb:43:3:43:31 | When | cfg.rb:43:8:43:8 | 2 | semmle.label | successor |
-| cfg.rb:43:8:43:8 | 2 | cfg.rb:43:8:43:8 | Pattern | semmle.label | successor |
-| cfg.rb:43:8:43:8 | Pattern | cfg.rb:43:11:43:11 | 3 | semmle.label | false |
-| cfg.rb:43:8:43:8 | Pattern | cfg.rb:43:26:43:31 | String | semmle.label | true |
-| cfg.rb:43:11:43:11 | 3 | cfg.rb:43:11:43:11 | Pattern | semmle.label | successor |
-| cfg.rb:43:11:43:11 | Pattern | cfg.rb:43:14:43:14 | 4 | semmle.label | false |
-| cfg.rb:43:11:43:11 | Pattern | cfg.rb:43:26:43:31 | String | semmle.label | true |
-| cfg.rb:43:14:43:14 | 4 | cfg.rb:43:14:43:14 | Pattern | semmle.label | successor |
-| cfg.rb:43:14:43:14 | Pattern | cfg.rb:43:26:43:31 | String | semmle.label | true |
-| cfg.rb:43:14:43:14 | Pattern | cfg.rb:44:13:44:18 | String | semmle.label | false |
+| cfg.rb:43:8:43:8 | 2 | cfg.rb:43:11:43:11 | 3 | semmle.label | no-match |
+| cfg.rb:43:8:43:8 | 2 | cfg.rb:43:26:43:31 | String | semmle.label | match |
+| cfg.rb:43:11:43:11 | 3 | cfg.rb:43:14:43:14 | 4 | semmle.label | no-match |
+| cfg.rb:43:11:43:11 | 3 | cfg.rb:43:26:43:31 | String | semmle.label | match |
+| cfg.rb:43:14:43:14 | 4 | cfg.rb:43:26:43:31 | String | semmle.label | match |
+| cfg.rb:43:14:43:14 | 4 | cfg.rb:44:13:44:18 | String | semmle.label | no-match |
 | cfg.rb:43:21:43:24 | puts | cfg.rb:43:21:43:31 | MethodCall | semmle.label | successor |
 | cfg.rb:43:21:43:31 | MethodCall | cfg.rb:47:1:50:3 | Case | semmle.label | successor |
 | cfg.rb:43:26:43:31 | String | cfg.rb:43:21:43:24 | puts | semmle.label | successor |
@@ -1293,23 +1278,20 @@ edges
 | cfg.rb:47:1:50:3 | Case | cfg.rb:48:3:48:29 | When | semmle.label | successor |
 | cfg.rb:48:3:48:29 | When | cfg.rb:48:8:48:8 | b | semmle.label | successor |
 | cfg.rb:48:8:48:8 | b | cfg.rb:48:13:48:13 | 1 | semmle.label | successor |
-| cfg.rb:48:8:48:13 | Binary | cfg.rb:48:8:48:13 | Pattern | semmle.label | successor |
-| cfg.rb:48:8:48:13 | Pattern | cfg.rb:48:25:48:29 | String | semmle.label | true |
-| cfg.rb:48:8:48:13 | Pattern | cfg.rb:49:3:49:37 | When | semmle.label | false |
+| cfg.rb:48:8:48:13 | Binary | cfg.rb:48:25:48:29 | String | semmle.label | true |
+| cfg.rb:48:8:48:13 | Binary | cfg.rb:49:3:49:37 | When | semmle.label | false |
 | cfg.rb:48:13:48:13 | 1 | cfg.rb:48:8:48:13 | Binary | semmle.label | successor |
 | cfg.rb:48:20:48:23 | puts | cfg.rb:48:20:48:29 | MethodCall | semmle.label | successor |
 | cfg.rb:48:20:48:29 | MethodCall | cfg.rb:52:11:52:13 | String | semmle.label | successor |
 | cfg.rb:48:25:48:29 | String | cfg.rb:48:20:48:23 | puts | semmle.label | successor |
 | cfg.rb:49:3:49:37 | When | cfg.rb:49:8:49:8 | b | semmle.label | successor |
 | cfg.rb:49:8:49:8 | b | cfg.rb:49:13:49:13 | 0 | semmle.label | successor |
-| cfg.rb:49:8:49:13 | Binary | cfg.rb:49:8:49:13 | Pattern | semmle.label | successor |
-| cfg.rb:49:8:49:13 | Pattern | cfg.rb:49:16:49:16 | b | semmle.label | false |
-| cfg.rb:49:8:49:13 | Pattern | cfg.rb:49:32:49:37 | String | semmle.label | true |
+| cfg.rb:49:8:49:13 | Binary | cfg.rb:49:16:49:16 | b | semmle.label | false |
+| cfg.rb:49:8:49:13 | Binary | cfg.rb:49:32:49:37 | String | semmle.label | true |
 | cfg.rb:49:13:49:13 | 0 | cfg.rb:49:8:49:13 | Binary | semmle.label | successor |
 | cfg.rb:49:16:49:16 | b | cfg.rb:49:20:49:20 | 1 | semmle.label | successor |
-| cfg.rb:49:16:49:20 | Binary | cfg.rb:49:16:49:20 | Pattern | semmle.label | successor |
-| cfg.rb:49:16:49:20 | Pattern | cfg.rb:49:32:49:37 | String | semmle.label | true |
-| cfg.rb:49:16:49:20 | Pattern | cfg.rb:52:11:52:13 | String | semmle.label | false |
+| cfg.rb:49:16:49:20 | Binary | cfg.rb:49:32:49:37 | String | semmle.label | true |
+| cfg.rb:49:16:49:20 | Binary | cfg.rb:52:11:52:13 | String | semmle.label | false |
 | cfg.rb:49:20:49:20 | 1 | cfg.rb:49:16:49:20 | Binary | semmle.label | successor |
 | cfg.rb:49:27:49:30 | puts | cfg.rb:49:27:49:37 | MethodCall | semmle.label | successor |
 | cfg.rb:49:27:49:37 | MethodCall | cfg.rb:52:11:52:13 | String | semmle.label | successor |