From dfe1a7e2f01519adf61f6b9e7eb80da9fac1cfda Mon Sep 17 00:00:00 2001
From: Jonas Jensen <jbj@github.com>
Date: Fri, 28 Feb 2020 10:48:23 +0100
Subject: [PATCH] C++: Avoid `iDominates*` in Overflow.qll

The `iDominates` relation is directly on control-flow nodes, and its
transitive closure is far too large. It got compiled into a recursion
rather than `fastTC`, and I've observed that recursion to take about an
hour on a medium-size customer snapshot.

The fix is to check for dominance at the basic-block level.
---
 .../src/semmle/code/cpp/security/Overflow.qll | 21 +++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/cpp/ql/src/semmle/code/cpp/security/Overflow.qll b/cpp/ql/src/semmle/code/cpp/security/Overflow.qll
index ba98f42d096..be9dcf650cb 100644
--- a/cpp/ql/src/semmle/code/cpp/security/Overflow.qll
+++ b/cpp/ql/src/semmle/code/cpp/security/Overflow.qll
@@ -13,19 +13,32 @@ predicate guardedAbs(Operation e, Expr use) {
   )
 }
 
+pragma[inline]
+private predicate stmtDominates(Stmt dominator, Stmt dominated) {
+  // In same block
+  exists(BasicBlock block, int dominatorIndex, int dominatedIndex |
+    block.getNode(dominatorIndex) = dominator and
+    block.getNode(dominatedIndex) = dominated and
+    dominatedIndex >= dominatorIndex
+  )
+  or
+  // In (possibly) different blocks
+  bbStrictlyDominates(dominator.getBasicBlock(), dominated.getBasicBlock())
+}
+
 /** is the size of this use guarded to be less than something? */
 pragma[nomagic]
 predicate guardedLesser(Operation e, Expr use) {
   exists(IfStmt c, RelationalOperation guard |
     use = guard.getLesserOperand().getAChild*() and
     guard = c.getControllingExpr().getAChild*() and
-    iDominates*(c.getThen(), e.getEnclosingStmt())
+    stmtDominates(c.getThen(), e.getEnclosingStmt())
   )
   or
   exists(Loop c, RelationalOperation guard |
     use = guard.getLesserOperand().getAChild*() and
     guard = c.getControllingExpr().getAChild*() and
-    iDominates*(c.getStmt(), e.getEnclosingStmt())
+    stmtDominates(c.getStmt(), e.getEnclosingStmt())
   )
   or
   exists(ConditionalExpr c, RelationalOperation guard |
@@ -43,13 +56,13 @@ predicate guardedGreater(Operation e, Expr use) {
   exists(IfStmt c, RelationalOperation guard |
     use = guard.getGreaterOperand().getAChild*() and
     guard = c.getControllingExpr().getAChild*() and
-    iDominates*(c.getThen(), e.getEnclosingStmt())
+    stmtDominates(c.getThen(), e.getEnclosingStmt())
   )
   or
   exists(Loop c, RelationalOperation guard |
     use = guard.getGreaterOperand().getAChild*() and
     guard = c.getControllingExpr().getAChild*() and
-    iDominates*(c.getStmt(), e.getEnclosingStmt())
+    stmtDominates(c.getStmt(), e.getEnclosingStmt())
   )
   or
   exists(ConditionalExpr c, RelationalOperation guard |