From 265852058d61a84e7898be60177d8408371e23b9 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Fri, 5 Oct 2018 14:17:44 +0200 Subject: [PATCH 1/2] C++: Faster implementation of BB entry node The existing implementation of `primitive_basic_block_entry_node` was "cleverly" computing two properties about `node` with a single `strictcount`: whether `node` had multiple predecessors and whether any of those predecessors had more than once successor. This was fast enough on most snapshots, but on the snapshot of our own code it took 37 seconds to compute `primitive_basic_block_entry_node` and its auxiliary predicates. This is likely to have affected other large snapshots too. With this change, the property is computed like in our other languages, and it brings the run time down to 4 seconds. --- .../code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll b/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll index babed5d10e15..f25ae5928f52 100644 --- a/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll +++ b/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll @@ -31,8 +31,11 @@ private cached module Cached { // or the node's predecessor has more than one successor, // then the node is the start of a new primitive basic block. or - strictcount (Node pred, Node other - | successors_extended(pred,node) and successors_extended(pred,other)) > 1 + strictcount(Node pred | successors_extended(pred, node)) > 1 + or + exists(ControlFlowNode pred | successors_extended(pred, node) | + strictcount(ControlFlowNode other | successors_extended(pred, other)) > 1 + ) // If the node has zero predecessors then it is the start of // a BB. However, the C++ AST contains many nodes with zero From 11e03b31611d3ef6a5ddece7c10e868951ac5e69 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Fri, 5 Oct 2018 14:26:04 +0200 Subject: [PATCH 2/2] C++: Fix primitive_basic_block_member join order This predicate looked like a join of two already-computed predicates, but it was a bit more complicated because the `*` operator expands into two cases: the reflexive case and the transitive case. The join order for the transitive case placed the `PrimitiveBasicBlock` charpred call _after_ the `member_step+` call, which means that all the tuples of `member_step+` passed through the pipeline. This commit changes the implementation by fully writing out the expansion of `*` into two cases, where the base case is manually specialised to make sure the join orderer doesn't get tempted into reusing the same strategy for both cases. This speeds up the predicate from 2m38s to 1s on a snapshot of our own C/C++ code. --- .../cpp/controlflow/internal/PrimitiveBasicBlocks.qll | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll b/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll index f25ae5928f52..ddbbf8bd61bd 100644 --- a/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll +++ b/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll @@ -66,8 +66,14 @@ private cached module Cached { /** Holds if `node` is the `pos`th control-flow node in primitive basic block `bb`. */ cached predicate primitive_basic_block_member(Node node, PrimitiveBasicBlock bb, int pos) { - pos = getMemberIndex(node) and - member_step*(bb, node) + primitive_basic_block_entry_node(bb) and + ( + pos = 0 and + node = bb + or + pos = getMemberIndex(node) and + member_step+(bb, node) + ) } /** Gets the number of control-flow nodes in the primitive basic block `bb`. */