Skip to content

Commit dfe1a7e

Browse files
committed
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.
1 parent 4ca57db commit dfe1a7e

File tree

1 file changed

+17
-4
lines changed

1 file changed

+17
-4
lines changed

cpp/ql/src/semmle/code/cpp/security/Overflow.qll

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,32 @@ predicate guardedAbs(Operation e, Expr use) {
1313
)
1414
}
1515

16+
pragma[inline]
17+
private predicate stmtDominates(Stmt dominator, Stmt dominated) {
18+
// In same block
19+
exists(BasicBlock block, int dominatorIndex, int dominatedIndex |
20+
block.getNode(dominatorIndex) = dominator and
21+
block.getNode(dominatedIndex) = dominated and
22+
dominatedIndex >= dominatorIndex
23+
)
24+
or
25+
// In (possibly) different blocks
26+
bbStrictlyDominates(dominator.getBasicBlock(), dominated.getBasicBlock())
27+
}
28+
1629
/** is the size of this use guarded to be less than something? */
1730
pragma[nomagic]
1831
predicate guardedLesser(Operation e, Expr use) {
1932
exists(IfStmt c, RelationalOperation guard |
2033
use = guard.getLesserOperand().getAChild*() and
2134
guard = c.getControllingExpr().getAChild*() and
22-
iDominates*(c.getThen(), e.getEnclosingStmt())
35+
stmtDominates(c.getThen(), e.getEnclosingStmt())
2336
)
2437
or
2538
exists(Loop c, RelationalOperation guard |
2639
use = guard.getLesserOperand().getAChild*() and
2740
guard = c.getControllingExpr().getAChild*() and
28-
iDominates*(c.getStmt(), e.getEnclosingStmt())
41+
stmtDominates(c.getStmt(), e.getEnclosingStmt())
2942
)
3043
or
3144
exists(ConditionalExpr c, RelationalOperation guard |
@@ -43,13 +56,13 @@ predicate guardedGreater(Operation e, Expr use) {
4356
exists(IfStmt c, RelationalOperation guard |
4457
use = guard.getGreaterOperand().getAChild*() and
4558
guard = c.getControllingExpr().getAChild*() and
46-
iDominates*(c.getThen(), e.getEnclosingStmt())
59+
stmtDominates(c.getThen(), e.getEnclosingStmt())
4760
)
4861
or
4962
exists(Loop c, RelationalOperation guard |
5063
use = guard.getGreaterOperand().getAChild*() and
5164
guard = c.getControllingExpr().getAChild*() and
52-
iDominates*(c.getStmt(), e.getEnclosingStmt())
65+
stmtDominates(c.getStmt(), e.getEnclosingStmt())
5366
)
5467
or
5568
exists(ConditionalExpr c, RelationalOperation guard |

0 commit comments

Comments
 (0)