hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-18 01:33:15 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix a bad join-order caused by the optimizer not seeing that

Browse Source 
`Node.getEnclosingCallable` is functional. This is fixed in the same way
as in Java's DataFlowUtil: We make create a non-virtual dispatching
`getEnclosingCallableImpl`, and implement `getEnclosingCallable` as a
wrapper that uses the `unique` aggregate to tell the compiler that there
is exactly 1 result.

Tuple numbers from an arbitrarily chosen iteration of
DataFlowImpl3::Stage4::fwdFlow0#fffff.

Before:
4569      ~5%        {5} r24 = SCAN DataFlowImpl3::Stage4::fwdFlow#fffff#prev_delta AS I OUTPUT I.<0>, I.<4> 'config', I.<1>, I.<2>, I.<3>
2876      ~1%        {5} r25 = JOIN r24 WITH DataFlowImpl3::LocalFlowBigStep::localFlowEntry#ff AS R ON FIRST 2 OUTPUT r24.<2> 'cc', r24.<0>, r24.<3> 'argAp', r24.<4>, r24.<1> 'config'
409404537 ~0%        {6} r26 = JOIN r25 WITH DataFlowImplCommon::getLocalCallContext#cpe#12#ff AS R ON FIRST 1 OUTPUT r25.<1>, R.<1>, r25.<0> 'cc', r25.<2> 'argAp', r25.<3>, r25.<4> 'config'
2876      ~0%        {6} r27 = JOIN r26 WITH DataFlowUtil::Node::getEnclosingCallable_dispred#ff AS R ON FIRST 2 OUTPUT r26.<0>, true, r26.<5> 'config', r26.<2> 'cc', r26.<3> 'argAp', r26.<4>
79821     ~1781%     {5} r28 = JOIN r27 WITH DataFlowImpl3::Stage4::localStep#ffbfff_0241#join_rhs AS R ON FIRST 3 OUTPUT r27.<3> 'cc', r27.<4> 'argAp', r27.<2> 'config', r27.<5> 'ap', R.<3> 'node'
2876      ~0%        {5} r29 = JOIN r26 WITH DataFlowUtil::Node::getEnclosingCallable_dispred#ff AS R ON FIRST 2 OUTPUT r26.<4>, r26.<0>, r26.<2> 'cc', r26.<3> 'argAp', r26.<5> 'config'
0         ~0%        {5} r30 = JOIN r29 WITH DataFlowImpl3::TNil#ff_1#join_rhs AS R ON FIRST 1 OUTPUT r29.<1>, false, r29.<4> 'config', r29.<2> 'cc', r29.<3> 'argAp'
0         ~0%        {5} r31 = JOIN r30 WITH DataFlowImpl3::Stage4::localStep#ffbfff_02413#join_rhs AS R ON FIRST 3 OUTPUT r30.<3> 'cc', r30.<4> 'argAp', r30.<2> 'config', R.<4> 'ap', R.<3> 'node'
4569      ~157%      {6} r32 = JOIN DataFlowImpl3::Stage4::fwdFlow0#fffff#join_rhs AS L WITH DataFlowImpl3::Stage4::fwdFlow#fffff#prev_delta AS R CARTESIAN PRODUCT OUTPUT R.<3>, L.<0> 'ap', L.<1> 'cc', L.<2> 'argAp', R.<0>, R.<4> 'config'
0         ~0%        {5} r33 = JOIN r32 WITH DataFlowImpl3::TNil#ff_1#join_rhs AS R ON FIRST 1 OUTPUT r32.<4>, r32.<5> 'config', r32.<1> 'ap', r32.<2> 'cc', r32.<3> 'argAp'
0         ~0%        {5} r34 = JOIN r33 WITH DataFlowImpl3::additionalJumpStep#fff_021#join_rhs AS R ON FIRST 2 OUTPUT R.<2> 'node', r33.<2> 'ap', r33.<3> 'cc', r33.<4> 'argAp', r33.<1> 'config'
0         ~0%        {5} r35 = JOIN r34 WITH DataFlowUtil::TIRDataFlowNode#f@staged_ext AS R ON FIRST 1 OUTPUT r34.<0> 'node', r34.<1> 'ap', r34.<2> 'cc', r34.<3> 'argAp', r34.<4> 'config'
0         ~0%        {6} r36 = JOIN r35 WITH project#DataFlowImpl3::Stage3::revFlow#fffff#12 AS R ON FIRST 1 OUTPUT r35.<1> 'ap', r35.<2> 'cc', r35.<3> 'argAp', r35.<4> 'config', r35.<0> 'node', R.<1>

After:
4569     ~0%        {5} r24 = SCAN DataFlowImpl3::Stage4::fwdFlow#fffff#prev_delta AS I OUTPUT I.<0>, I.<4> 'config', I.<1>, I.<2>, I.<3>
2876     ~0%        {5} r25 = JOIN r24 WITH DataFlowImpl3::LocalFlowBigStep::localFlowEntry#ff AS R ON FIRST 2 OUTPUT r24.<0>, r24.<2> 'cc', r24.<3> 'argAp', r24.<4>, r24.<1> 'config'
2876     ~0%        {6} r26 = JOIN r25 WITH DataFlowUtil::Node::getEnclosingCallable_dispred#ff AS R ON FIRST 1 OUTPUT r25.<1> 'cc', R.<1>, r25.<0>, r25.<2> 'argAp', r25.<3>, r25.<4> 'config'
2876     ~0%        {6} r27 = JOIN r26 WITH DataFlowImplCommon::getLocalCallContext#cpe#12#fb AS R ON FIRST 2 OUTPUT r26.<2>, true, r26.<5> 'config', r26.<0> 'cc', r26.<3> 'argAp', r26.<4>
79821    ~1862%     {5} r28 = JOIN r27 WITH DataFlowImpl3::Stage4::localStep#ffbfff_0241#join_rhs AS R ON FIRST 3 OUTPUT r27.<3> 'cc', r27.<4> 'argAp', r27.<2> 'config', r27.<5> 'ap', R.<3> 'node'
2876     ~0%        {5} r29 = JOIN r26 WITH DataFlowImplCommon::getLocalCallContext#cpe#12#fb AS R ON FIRST 2 OUTPUT r26.<4>, r26.<2>, r26.<0> 'cc', r26.<3> 'argAp', r26.<5> 'config'
0        ~0%        {5} r30 = JOIN r29 WITH DataFlowImpl3::TNil#ff_1#join_rhs AS R ON FIRST 1 OUTPUT r29.<1>, false, r29.<4> 'config', r29.<2> 'cc', r29.<3> 'argAp'
0        ~0%        {5} r31 = JOIN r30 WITH DataFlowImpl3::Stage4::localStep#ffbfff_02413#join_rhs AS R ON FIRST 3 OUTPUT r30.<3> 'cc', r30.<4> 'argAp', r30.<2> 'config', R.<4> 'ap', R.<3> 'node'
4569     ~148%      {6} r32 = JOIN DataFlowImpl3::Stage4::fwdFlow0#fffff#join_rhs AS L WITH DataFlowImpl3::Stage4::fwdFlow#fffff#prev_delta AS R CARTESIAN PRODUCT OUTPUT R.<3>, L.<0> 'ap', L.<1> 'cc', L.<2> 'argAp', R.<0>, R.<4> 'config'
0        ~0%        {5} r33 = JOIN r32 WITH DataFlowImpl3::TNil#ff_1#join_rhs AS R ON FIRST 1 OUTPUT r32.<4>, r32.<5> 'config', r32.<1> 'ap', r32.<2> 'cc', r32.<3> 'argAp'
0        ~0%        {5} r34 = JOIN r33 WITH DataFlowImpl3::additionalJumpStep#fff_021#join_rhs AS R ON FIRST 2 OUTPUT R.<2> 'node', r33.<2> 'ap', r33.<3> 'cc', r33.<4> 'argAp', r33.<1> 'config'
0        ~0%        {5} r35 = JOIN r34 WITH DataFlowUtil::TIRDataFlowNode#f@staged_ext AS R ON FIRST 1 OUTPUT r34.<0> 'node', r34.<1> 'ap', r34.<2> 'cc', r34.<3> 'argAp', r34.<4> 'config'
0        ~0%        {6} r36 = JOIN r35 WITH project#DataFlowImpl3::Stage3::revFlow#fffff#12 AS R ON FIRST 1 OUTPUT r35.<1> 'ap', r35.<2> 'cc', r35.<3> 'argAp', r35.<4> 'config', r35.<0> 'node', R.<1>
This commit is contained in:
Mathias Vorreiter Pedersen
2020-12-05 15:26:24 +01:00
parent f5a2603cc1
commit 145ab17f6b
1 changed files with 16 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
View File

@@ -32,7 +32,22 @@ class Node extends TIRDataFlowNode {
/**
* INTERNAL: Do not use.
*/
Declaration getEnclosingCallable() { none() } // overridden in subclasses
final Declaration getEnclosingCallable() {
result = unique(Declaration d | d = this.getEnclosingCallableImpl() | d)
}
final private Declaration getEnclosingCallableImpl() {
result = this.asInstruction().getEnclosingFunction() or
result = this.asOperand().getUse().getEnclosingFunction() or
// When flow crosses from one _enclosing callable_ to another, the
// interprocedural data-flow library discards call contexts and inserts a
// node in the big-step relation used for human-readable path explanations.
// Therefore we want a distinct enclosing callable for each `VariableNode`,
// and that can be the `Variable` itself.
result = this.asVariable() or
result = this.(FieldNode).getFieldInstruction().getEnclosingFunction() or
result = this.(PartialDefinitionNode).getPreUpdateNode().getFunction()
}
/** Gets the function to which this node belongs, if any. */
Function getFunction() { none() } // overridden in subclasses
@@ -137,8 +152,6 @@ class InstructionNode extends Node, TInstructionNode {
/** Gets the instruction corresponding to this node. */
Instruction getInstruction() { result = instr }
override Declaration getEnclosingCallable() { result = this.getFunction() }
override Function getFunction() { result = instr.getEnclosingFunction() }
override IRType getType() { result = instr.getResultIRType() }
@@ -163,8 +176,6 @@ class OperandNode extends Node, TOperandNode {
/** Gets the operand corresponding to this node. */
Operand getOperand() { result = op }
override Declaration getEnclosingCallable() { result = this.getFunction() }
override Function getFunction() { result = op.getUse().getEnclosingFunction() }
override IRType getType() { result = op.getIRType() }
@@ -599,15 +610,6 @@ class VariableNode extends Node, TVariableNode {
override Function getFunction() { none() }
override Declaration getEnclosingCallable() {
// When flow crosses from one _enclosing callable_ to another, the
// interprocedural data-flow library discards call contexts and inserts a
// node in the big-step relation used for human-readable path explanations.
// Therefore we want a distinct enclosing callable for each `VariableNode`,
// and that can be the `Variable` itself.
result = v
}
override IRType getType() { result.getCanonicalLanguageType().hasUnspecifiedType(v.getType(), _) }
override Location getLocation() { result = v.getLocation() }