C++: Enforce unique enclosing callable

jbj · jbj · commit d4338473b05a · 2020-04-06T12:04:39.000+02:00
Every data-flow node should have a unique enclosing function (_callable_
in the terminology of the data-flow library), but this was not evident
for the optimizer, and it led to a bad join order in `pathStep`. This
commit fixes the join order for C++ AST data flow. All other copies of
data flow seem to be fine.

These are the tuple counts for OpenJDK before this commit:

    (231s) Tuple counts for DataFlowImplLocal::pathStep#fffff#cur_delta:
    5882       ~0%       {6} r1 = SCAN DataFlowImplLocal::PathNodeMid#class#ffffff#prev_delta AS I OUTPUT I.&lt;2&gt;, I.&lt;0&gt;, I.&lt;1&gt;, I.&lt;3&gt;, I.&lt;4&gt;, I.&lt;5&gt;
    1063406780 ~0%       {7} r2 = JOIN r1 WITH DataFlowImplCommon::CallContext::relevantFor_dispred#ff AS R ON FIRST 1 OUTPUT r1.&lt;2&gt;, R.&lt;1&gt;, r1.&lt;1&gt;, r1.&lt;0&gt;, r1.&lt;3&gt;, r1.&lt;4&gt;, r1.&lt;5&gt;
    5882       ~1%       {6} r3 = JOIN r2 WITH DataFlowUtil::Node::getFunction_dispred#ff AS R ON FIRST 2 OUTPUT r2.&lt;0&gt;, r2.&lt;6&gt;, r2.&lt;2&gt;, r2.&lt;3&gt;, r2.&lt;4&gt;, r2.&lt;5&gt;
    105        ~0%       {5} r4 = JOIN r3 WITH project#DataFlowImplLocal::LocalFlowBigStep::localFlowBigStep#ffffff_021#join_rhs AS R ON FIRST 2 OUTPUT r3.&lt;2&gt;, r3.&lt;3&gt;, r3.&lt;4&gt;, r3.&lt;5&gt;, R.&lt;2&gt;
    5882       ~1%       {6} r5 = JOIN r2 WITH DataFlowUtil::Node::getFunction_dispred#ff AS R ON FIRST 2 OUTPUT r2.&lt;5&gt;, r2.&lt;2&gt;, r2.&lt;0&gt;, r2.&lt;3&gt;, r2.&lt;4&gt;, r2.&lt;6&gt;
    5882       ~0%       {6} r6 = JOIN r5 WITH DataFlowImplLocal::TNil#ff_1#join_rhs AS R ON FIRST 1 OUTPUT r5.&lt;2&gt;, false, r5.&lt;5&gt;, r5.&lt;1&gt;, r5.&lt;3&gt;, r5.&lt;4&gt;
    0          ~0%       {5} r7 = JOIN r6 WITH DataFlowImplLocal::LocalFlowBigStep::localFlowBigStep#ffffff_02413#join_rhs AS R ON FIRST 3 OUTPUT R.&lt;4&gt;, r6.&lt;3&gt;, r6.&lt;4&gt;, r6.&lt;5&gt;, R.&lt;3&gt;
    0          ~0%       {5} r8 = JOIN r7 WITH DataFlowImplLocal::TNil#ff AS R ON FIRST 1 OUTPUT r7.&lt;1&gt;, r7.&lt;2&gt;, r7.&lt;3&gt;, R.&lt;1&gt;, r7.&lt;4&gt;
    105        ~0%       {5} r9 = r4 \/ r8

The problem is that `DataFlowUtil::Node::getFunction_dispred#ff`
(`getEnclosingCallable`) is joined too late.

After this commit, the tuple counts look like this:

    (13s) Tuple counts for DataFlowImplLocal::pathStep#fffff#cur_delta:
    5882    ~1%       {6} r1 = SCAN DataFlowImplLocal::PathNodeMid#class#ffffff#prev_delta AS I OUTPUT I.&lt;1&gt;, I.&lt;0&gt;, I.&lt;2&gt;, I.&lt;3&gt;, I.&lt;4&gt;, I.&lt;5&gt;
    5882    ~3%       {7} r2 = JOIN r1 WITH DataFlowUtil::Node::getEnclosingCallable_dispred#ff AS R ON FIRST 1 OUTPUT r1.&lt;2&gt;, R.&lt;1&gt;, r1.&lt;1&gt;, r1.&lt;0&gt;, r1.&lt;3&gt;, r1.&lt;4&gt;, r1.&lt;5&gt;
    5882    ~1%       {6} r3 = JOIN r2 WITH DataFlowImplCommon::CallContext::relevantFor_dispred#ff AS R ON FIRST 2 OUTPUT r2.&lt;3&gt;, r2.&lt;6&gt;, r2.&lt;2&gt;, r2.&lt;0&gt;, r2.&lt;4&gt;, r2.&lt;5&gt;
    105     ~0%       {5} r4 = JOIN r3 WITH project#DataFlowImplLocal::LocalFlowBigStep::localFlowBigStep#ffffff_021#join_rhs AS R ON FIRST 2 OUTPUT r3.&lt;2&gt;, r3.&lt;3&gt;, r3.&lt;4&gt;, r3.&lt;5&gt;, R.&lt;2&gt;
    5882    ~1%       {6} r5 = JOIN r2 WITH DataFlowImplCommon::CallContext::relevantFor_dispred#ff AS R ON FIRST 2 OUTPUT r2.&lt;5&gt;, r2.&lt;2&gt;, r2.&lt;3&gt;, r2.&lt;0&gt;, r2.&lt;4&gt;, r2.&lt;6&gt;
    5882    ~0%       {6} r6 = JOIN r5 WITH DataFlowImplLocal::TNil#ff_1#join_rhs AS R ON FIRST 1 OUTPUT r5.&lt;2&gt;, false, r5.&lt;5&gt;, r5.&lt;1&gt;, r5.&lt;3&gt;, r5.&lt;4&gt;
    0       ~0%       {5} r7 = JOIN r6 WITH DataFlowImplLocal::LocalFlowBigStep::localFlowBigStep#ffffff_02413#join_rhs AS R ON FIRST 3 OUTPUT R.&lt;4&gt;, r6.&lt;3&gt;, r6.&lt;4&gt;, r6.&lt;5&gt;, R.&lt;3&gt;
    0       ~0%       {5} r8 = JOIN r7 WITH DataFlowImplLocal::TNil#ff AS R ON FIRST 1 OUTPUT r7.&lt;1&gt;, r7.&lt;2&gt;, r7.&lt;3&gt;, R.&lt;1&gt;, r7.&lt;4&gt;
    105     ~0%       {5} r9 = r4 \/ r8

There is a slight slowdown coming from the introduction of a new
predicate `DataFlowImplLocal::pathStep#fffff#join_rhs`, which is used
only in the standard order:

    (12s) Tuple counts for DataFlowImplLocal::pathStep#fffff#join_rhs:
    282057  ~0%     {2} r1 = SCAN DataFlowImplCommon::CallContext::relevantFor_dispred#ff AS I OUTPUT I.&lt;1&gt;, I.&lt;0&gt;
    9159890 ~1%     {2} r2 = JOIN r1 WITH DataFlowUtil::Node::getEnclosingCallable_dispred#ff_10#join_rhs AS R ON FIRST 1 OUTPUT R.&lt;1&gt;, r1.&lt;1&gt;
                    return r2

The evaluation of `unique` is cheap but not free:

    DataFlowUtil::Node::getEnclosingCallable_dispred#ff .............. 3.9s
    DataFlowUtil::Node::getEnclosingCallable_dispred#ff_10#join_rhs .. 3.5s

The first of these two predicates evaluates `unique`, and the second
simply reorders columns. They take about the same time, which suggests
that `unique` is about as fast as it can be, given the number of tuples
it needs to push around. Note that the column reordering predicate is
only needed because of the standard order.
diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll
@@ -43,7 +43,7 @@ class Node extends TNode {
   /**
    * INTERNAL: Do not use. Alternative name for `getFunction`.
    */
-  Function getEnclosingCallable() { result = this.getFunction() }
+  final Function getEnclosingCallable() { result = unique( | | this.getFunction()) }
 
   /** Gets the type of this node. */
   Type getType() { none() } // overridden in subclasses
