Merge pull request #7090 from hvitved/ruby/perf

Ruby: Cache more predicates

Author: hvitved

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

@@ -157,6 +157,7 @@ class ErbDirective extends TDirectiveNode, ErbAstNode {
    * Gets a statement that starts in directive that is not a child of any other
    * statement starting in this directive.
    */
+  cached
   Stmt getAChildStmt() {
     this.containsAstNodeStart(result) and
     not this.containsAstNodeStart(result.getParent())

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

@@ -309,6 +309,17 @@ private module Cached {
         jbp order by JoinBlockPredecessors::getId(jbp), JoinBlockPredecessors::getSplitString(jbp)
       )
   }
+
+  cached
+  predicate immediatelyControls(ConditionBlock cb, BasicBlock succ, BooleanSuccessor s) {
+    succ = cb.getASuccessor(s) and
+    forall(BasicBlock pred | pred = succ.getAPredecessor() and pred != cb | succ.dominates(pred))
+  }
+
+  cached
+  predicate controls(ConditionBlock cb, BasicBlock controlled, BooleanSuccessor s) {
+    exists(BasicBlock succ | cb.immediatelyControls(succ, s) | succ.dominates(controlled))
+  }
 }
 
 private import Cached
@@ -395,18 +406,14 @@ class ConditionBlock extends BasicBlock {
    * successor of this block, and `succ` can only be reached from
    * the callable entry point by going via the `s` edge out of this basic block.
    */
-  pragma[nomagic]
   predicate immediatelyControls(BasicBlock succ, BooleanSuccessor s) {
-    succ = this.getASuccessor(s) and
-    forall(BasicBlock pred | pred = succ.getAPredecessor() and pred != this | succ.dominates(pred))
+    immediatelyControls(this, succ, s)
   }
 
   /**
    * Holds if basic block `controlled` is controlled by this basic block with
    * conditional value `s`. That is, `controlled` can only be reached from
    * the callable entry point by going via the `s` edge out of this basic block.
    */
-  predicate controls(BasicBlock controlled, BooleanSuccessor s) {
-    exists(BasicBlock succ | this.immediatelyControls(succ, s) | succ.dominates(controlled))
-  }
+  predicate controls(BasicBlock controlled, BooleanSuccessor s) { controls(this, controlled, s) }
 }

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

@@ -463,18 +463,7 @@ predicate mayBenefitFromCallContext(DataFlowCall call, DataFlowCallable c) { non
  */
 DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) { none() }
 
-/**
- * Holds if `e` is an `ExprNode` that may be returned by a call to `c`.
- */
-predicate exprNodeReturnedFrom(DataFlow::ExprNode e, Callable c) {
-  exists(ReturningNode r |
-    nodeGetEnclosingCallable(r).asCallable() = c and
-    (
-      r.(ExplicitReturnNode).getReturningNode().getReturnedValueNode() = e.asExpr() or
-      r.(ExprReturnNode) = e
-    )
-  )
-}
+predicate exprNodeReturnedFrom = exprNodeReturnedFromCached/2;
 
 /** A parameter position. */
 class ParameterPosition extends TParameterPosition {

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

@@ -297,6 +297,20 @@ private module Cached {
     TKnownArrayElementContent(int i) { i in [0 .. 10] } or
     TUnknownArrayElementContent() or
     TAnyArrayElementContent()
+
+  /**
+   * Holds if `e` is an `ExprNode` that may be returned by a call to `c`.
+   */
+  cached
+  predicate exprNodeReturnedFromCached(ExprNode e, Callable c) {
+    exists(ReturningNode r |
+      nodeGetEnclosingCallable(r).asCallable() = c and
+      (
+        r.(ExplicitReturnNode).getReturningNode().getReturnedValueNode() = e.asExpr() or
+        r.(ExprReturnNode) = e
+      )
+    )
+  }
 }
 
 class TArrayElementContent = TKnownArrayElementContent or TUnknownArrayElementContent;

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

@@ -18,11 +18,11 @@ class Node extends TNode {
   Parameter asParameter() { result = this.(ParameterNode).getParameter() }
 
   /** Gets a textual representation of this node. */
-  // TODO: cache
+  cached
   final string toString() { result = this.(NodeImpl).toStringImpl() }
 
   /** Gets the location of this node. */
-  // TODO: cache
+  cached
   final Location getLocation() { result = this.(NodeImpl).getLocationImpl() }
 
   /**
@@ -121,7 +121,8 @@ class LocalSourceNode extends Node {
   LocalSourceNode backtrack(TypeBackTracker t2, TypeBackTracker t) { t2 = t.step(result, this) }
 }
 
-predicate hasLocalSource(Node sink, Node source) {
+cached
+private predicate hasLocalSource(Node sink, Node source) {
   // Declaring `source` to be a `SourceNode` currently causes a redundant check in the
   // recursive case, so instead we check it explicitly here.
   source = sink and

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

@@ -20,15 +20,6 @@ private predicate hasCapturedVariableRead(BasicBlock bb, LocalVariable v) {
   )
 }
 
-/**
- * Holds if an entry definition is needed for captured variable `v` at index
- * `i` in entry block `bb`.
- */
-predicate capturedEntryWrite(EntryBasicBlock bb, int i, LocalVariable v) {
-  hasCapturedVariableRead(bb.getASuccessor*(), v) and
-  i = -1
-}
-
 /** Holds if `bb` contains a caputured write to variable `v`. */
 pragma[noinline]
 private predicate writesCapturedVariable(BasicBlock bb, LocalVariable v) {
@@ -132,6 +123,16 @@ private predicate hasVariableReadWithCapturedWrite(BasicBlock bb, LocalVariable
 
 cached
 private module Cached {
+  /**
+   * Holds if an entry definition is needed for captured variable `v` at index
+   * `i` in entry block `bb`.
+   */
+  cached
+  predicate capturedEntryWrite(EntryBasicBlock bb, int i, LocalVariable v) {
+    hasCapturedVariableRead(bb.getASuccessor*(), v) and
+    i = -1
+  }
+
   /**
    * Holds if the call at index `i` in basic block `bb` may reach a callable
    * that writes captured variable `v`.