Skip to content

Commit 21c6340

Browse files
committed
C++: Fix performance of unchecked leap year query
This query used `getASuccessor()` on the CFG, which worked in many cases but became quadratic on certain projects including PostgreSQL and MySQL. The problem was that there was just enough context for magic to apply to the transitive closure, but the use of magic meant that the fast transitive closure algorithm wasn't used. In projects where the magic had little effect, that led to the `#ControlFlowGraph::ControlFlowNode::getASuccessor_dispred#bfPlus` predicate taking quadratic time and space. This commit changes the query to use basic blocks to find successors, which is much faster because (1) there are many more `ControlFlowNode`s than `BasicBlocks`, and (2) the optimizer does not apply magic but uses fast transitive closure instead. Behavior changes slightly in the `isUsedInCorrectLeapYearCheck` case: we now accept a `yfacheck` that comes _before_ `yfa` if they are in the same basic block. I don't think that matters in practice.
1 parent 46d7792 commit 21c6340

File tree

1 file changed

+3
-3
lines changed

1 file changed

+3
-3
lines changed

cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ where
2424
exists(LeapYearFieldAccess yfacheck |
2525
yfacheck.getQualifier() = var.getAnAccess() and
2626
yfacheck.isUsedInCorrectLeapYearCheck() and
27-
yfacheck = yfa.getASuccessor*()
27+
yfacheck.getBasicBlock() = yfa.getBasicBlock().getASuccessor*()
2828
)
2929
or
3030
// If there is a data flow from the variable that was modified to a function that seems to check for leap year
@@ -50,8 +50,8 @@ where
5050
mfa.getQualifier() = var.getAnAccess() and
5151
mfa.isModified() and
5252
(
53-
mfa = yfa.getASuccessor*() or
54-
yfa = mfa.getASuccessor*()
53+
mfa.getBasicBlock() = yfa.getBasicBlock().getASuccessor*() or
54+
yfa.getBasicBlock() = mfa.getBasicBlock().getASuccessor+()
5555
) and
5656
ae = mfa.getEnclosingElement() and
5757
ae.getAnOperand().getValue().toInt() = 1

0 commit comments

Comments
 (0)