hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 11:15:13 +02:00
Code Issues Packages Projects Releases Wiki Activity

C++: Fix join order in reachesWithoutAssignment

Browse Source 
The negation in this predicate did not get pulled into an
`#antijoin_rhs` predicate but got materialized as part of each
iteration, which meant that the temporary `ControlFlowNode` column did
not get projected away. The tuple counts looked like this on
kamailio/kamailio (iteration 20):

    5724      ~13%      {3} r9 = JOIN r8 WITH BasicBlocks::Cached::bb_successor_cached#ff@staged_ext AS R ON FIRST 2 OUTPUT r8.<2>, r8.<3>, r8.<1>
    5724      ~12%      {3} r10 = JOIN r8 WITH BasicBlocks::Cached::bb_successor_cached#ff@staged_ext AS R ON FIRST 2 OUTPUT r8.<3>, r8.<2>, r8.<1>
    124717061 ~11%      {4} r11 = JOIN r10 WITH project#FlowVar::FlowVar_internal::assignmentLikeOperation#ffff_10#join_rhs AS R ON FIRST 1 OUTPUT R.<1>, r10.<2>, r10.<1>, r10.<0>
    66        ~0%       {3} r12 = JOIN r11 WITH project#BasicBlocks::Cached::basic_block_member AS R ON FIRST 2 OUTPUT r11.<2>, r11.<3>, r11.<1>
    66                  {3} r13 = MATERIALIZE r12 AS antijoin_rhs
    5658      ~14%      {3} r14 = r9 AND NOT r13(r9.<0>, r9.<1>, r9.<2>)

After manually pulling out the join inside the negation, the time per
iteration drops from ~30 to <1s. The pipeline above is replaced with

    892394  ~0%      {4} r6 = r5 AND NOT FlowVar::FlowVar_internal::assignsToVar#fb AS R(r5.<3>, r5.<2>)
    892394  ~0%      {4} r7 = SCAN r6 OUTPUT r6.<1>, r6.<3>, r6.<0>, r6.<2>
    5658    ~11%     {3} r8 = JOIN r7 WITH BasicBlocks::Cached::bb_successor_cached#ff@staged_ext AS R ON FIRST 2 OUTPUT r7.<2>, r7.<1>, r7.<3>
This commit is contained in:
Jonas Jensen
2020-08-27 09:39:47 +02:00
parent c9e22ab271
commit 2b720b332b
1 changed files with 8 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
cpp/ql/src/semmle/code/cpp/dataflow/internal/FlowVar.qll
View File

@@ -427,7 +427,7 @@ module FlowVar_internal {
/**
* Gets a variable that is assigned in this loop and read outside the loop.
*/
private Variable getARelevantVariable() {
Variable getARelevantVariable() {
result = this.getAVariableAssignedInLoop() and
exists(VariableAccess va |
va.getTarget() = result and
@@ -472,10 +472,16 @@ module FlowVar_internal {
reachesWithoutAssignment(bb.getAPredecessor(), v) and
this.bbInLoop(bb)
) and
not assignmentLikeOperation(bb.getANode(), v, _, _)
not assignsToVar(bb, v)
}
}
pragma[noinline]
private predicate assignsToVar(BasicBlock bb, Variable v) {
assignmentLikeOperation(bb.getANode(), v, _, _) and
exists(AlwaysTrueUponEntryLoop loop | v = loop.getARelevantVariable())
}
/**
* Holds if `loop` always assigns to `v` before leaving through an edge
* from `bbInside` in its condition to `bbOutside` outside the loop. Also,