C++: Fix bad join order.

aschackmull · MathiasVP · commit 738b7d1599b6 · 2025-03-13T16:54:59.000Z
Before:

[2025-03-12 10:27:53] Evaluated non-recursive predicate SsaInternals::UseImpl.hasIndexInBlock/2#dispred#1e34a5af@e87543ui in 935ms (size: 8905695).
Evaluated relational algebra for predicate SsaInternals::UseImpl.hasIndexInBlock/2#dispred#1e34a5af@e87543ui with tuple counts:
                          {3} r1 = SsaInternals::DirectUseImpl#a58aae88 AND NOT `_ArithmeticOperation::PostfixCrementOperation#17623ada_Expr::UnaryOperation.getOperand/0#dispred#990__#antijoin_rhs`(FIRST 3)
         8579337   ~4%    {2}    | SCAN OUTPUT In.1, In.0
         8579337   ~0%    {2}    | JOIN WITH `Operand::Operand.getUse/0#dispred#427b49d0` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
         8579337   ~0%    {3}    | JOIN WITH `IRBlock::Cached::getInstruction/2#627f9c61_201#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Rhs.2

           48215   ~2%    {2} r2 = SCAN SsaInternals::GlobalUse#9cd323b4 OUTPUT In.2, In.0
        35467318   ~3%    {2}    | JOIN WITH `SSAConstruction::getInstructionEnclosingIRFunction/1#5443f355_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1

           48189   ~0%    {2} r3 = JOIN r2 WITH Instruction::ReturnInstruction#28bfb7eb ON FIRST 1 OUTPUT Lhs.0, Lhs.1

           12332   ~0%    {2} r4 = JOIN r2 WITH Instruction::UnreachedInstruction#774c7a34 ON FIRST 1 OUTPUT Lhs.0, Lhs.1

           60521   ~0%    {2} r5 = r3 UNION r4
           60521   ~2%    {3}    | JOIN WITH `IRBlock::Cached::getInstruction/2#627f9c61_201#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Rhs.2

           39316   ~0%    {2} r6 = JOIN SsaInternals::FinalParameterUse#c1f84700_10#join_rhs WITH `Parameter::Parameter.getFunction/0#dispred#803faca2` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
        43821265   ~0%    {2}    | JOIN WITH `Instruction::Instruction.getEnclosingFunction/0#dispred#cb8ccc56_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1

           39194   ~0%    {2} r7 = JOIN r6 WITH Instruction::ReturnInstruction#28bfb7eb ON FIRST 1 OUTPUT Lhs.0, Lhs.1

           21255   ~2%    {2} r8 = JOIN r6 WITH Instruction::UnreachedInstruction#774c7a34 ON FIRST 1 OUTPUT Lhs.0, Lhs.1

           60449   ~0%    {2} r9 = r7 UNION r8
           60449   ~3%    {3}    | JOIN WITH `IRBlock::Cached::getInstruction/2#627f9c61_201#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Rhs.2

         8784725   ~1%    {5} r10 = JOIN `_SsaInternals::DirectUseImpl#a58aae88_SsaInternals::DirectUseImpl.getBase/0#dispred#4b8c43d0_SsaInte__#shared` WITH `SsaInternals::DirectUseImpl.getBase/0#dispred#4b8c43d0` ON FIRST 1 OUTPUT Rhs.1, Lhs.0, Lhs.1, Lhs.2, Lhs.3
         8784725   ~0%    {5}    | JOIN WITH `cached_SSAConstruction::getInstructionAst/1#d0d95b50` ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2, Lhs.3, Lhs.4
          210435   ~4%    {5}    | JOIN WITH `Expr::UnaryOperation.getOperand/0#dispred#990de484#bf_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2, Lhs.3, Lhs.4
          205388   ~0%    {4}    | JOIN WITH ArithmeticOperation::PostfixCrementOperation#17623ada ON FIRST 1 OUTPUT Lhs.4, Lhs.3, Lhs.2, Lhs.1
          205388   ~4%    {3}    | JOIN WITH `__IRBlock::Cached::getInstruction/2#627f9c61_201#join_rhs__ArithmeticOperation::PostfixCrementOperat__#join_rhs` ON FIRST 3 OUTPUT Rhs.4, Lhs.3, Rhs.3
          205388   ~0%    {3}    | JOIN WITH `Operand::Operand.getUse/0#dispred#427b49d0` ON FIRST 1 OUTPUT Lhs.2, Rhs.1, Lhs.1
          205388   ~1%    {3}    | JOIN WITH `IRBlock::Cached::getInstruction/2#627f9c61_021#join_rhs` ON FIRST 2 OUTPUT Lhs.2, Lhs.0, Rhs.2

         8905695   ~0%    {3} r11 = r1 UNION r5 UNION r9 UNION r10
                          return r11

After:

[2025-03-12 11:12:48] Evaluated non-recursive predicate SsaInternals::hasReturnPosition/3#02f7eab8@bc405c4l in 3ms (size: 49368).
Evaluated relational algebra for predicate SsaInternals::hasReturnPosition/3#02f7eab8@bc405c4l with tuple counts:
        49368  ~3%    {1} r1 = Instruction::ReturnInstruction#28bfb7eb UNION Instruction::UnreachedInstruction#774c7a34
        49368  ~0%    {2}    | JOIN WITH `cached_SSAConstruction::getInstructionEnclosingIRFunction/1#5443f355` ON FIRST 1 OUTPUT Lhs.0, Rhs.1
        49368  ~2%    {3}    | JOIN WITH `IRBlock::Cached::getInstruction/2#627f9c61_201#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Rhs.2
                      return r1

[2025-03-12 11:12:54] Evaluated non-recursive predicate SsaInternals::UseImpl.hasIndexInBlock/2#dispred#1e34a5af@6e30cduo in 549ms (size: 8905695).
Evaluated relational algebra for predicate SsaInternals::UseImpl.hasIndexInBlock/2#dispred#1e34a5af@6e30cduo with tuple counts:
          48215   ~2%    {2} r1 = SCAN SsaInternals::GlobalUse#9cd323b4 OUTPUT In.2, In.0
          60521   ~2%    {3}    | JOIN WITH `SsaInternals::hasReturnPosition/3#02f7eab8` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Rhs.2

          50725   ~0%    {2} r2 = JOIN `IRFunctionBase::IRFunctionBase.getFunction/0#dispred#b024672e_10#join_rhs` WITH `Parameter::Parameter.getFunction/0#dispred#803faca2_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
          39231   ~2%    {2}    | JOIN WITH SsaInternals::FinalParameterUse#c1f84700_10#join_rhs ON FIRST 1 OUTPUT Lhs.1, Rhs.1
          60449   ~3%    {3}    | JOIN WITH `SsaInternals::hasReturnPosition/3#02f7eab8` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Rhs.2

                         {3} r3 = SsaInternals::DirectUseImpl#a58aae88 AND NOT `_ArithmeticOperation::PostfixCrementOperation#17623ada_Expr::UnaryOperation.getOperand/0#dispred#990__#antijoin_rhs`(FIRST 3)
        8579337   ~1%    {2}    | SCAN OUTPUT In.1, In.0
        8579337   ~0%    {2}    | JOIN WITH `Operand::Operand.getUse/0#dispred#427b49d0` ON FIRST 1 OUTPUT Rhs.1, Lhs.1
        8579337   ~1%    {3}    | JOIN WITH `IRBlock::Cached::getInstruction/2#627f9c61_201#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Rhs.2

        8784725   ~0%    {5} r4 = JOIN `_SsaInternals::DirectUseImpl#a58aae88_SsaInternals::DirectUseImpl.getBase/0#dispred#4b8c43d0_SsaInte__#shared` WITH `SsaInternals::DirectUseImpl.getBase/0#dispred#4b8c43d0` ON FIRST 1 OUTPUT Rhs.1, Lhs.0, Lhs.1, Lhs.2, Lhs.3
        8784725   ~0%    {5}    | JOIN WITH `cached_SSAConstruction::getInstructionAst/1#d0d95b50` ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2, Lhs.3, Lhs.4
         210435   ~0%    {5}    | JOIN WITH `Expr::UnaryOperation.getOperand/0#dispred#990de484#bf_10#join_rhs` ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2, Lhs.3, Lhs.4
         205388   ~2%    {4}    | JOIN WITH ArithmeticOperation::PostfixCrementOperation#17623ada ON FIRST 1 OUTPUT Lhs.4, Lhs.3, Lhs.2, Lhs.1
         205388   ~0%    {3}    | JOIN WITH `__IRBlock::Cached::getInstruction/2#627f9c61_201#join_rhs__ArithmeticOperation::PostfixCrementOperat__#join_rhs` ON FIRST 3 OUTPUT Rhs.4, Lhs.3, Rhs.3
         205388   ~0%    {3}    | JOIN WITH `Operand::Operand.getUse/0#dispred#427b49d0` ON FIRST 1 OUTPUT Lhs.2, Rhs.1, Lhs.1
         205388   ~0%    {3}    | JOIN WITH `IRBlock::Cached::getInstruction/2#627f9c61_021#join_rhs` ON FIRST 2 OUTPUT Lhs.2, Lhs.0, Rhs.2

        8905695   ~0%    {3} r5 = r1 UNION r2 UNION r3 UNION r4
                         return r5
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
@@ -465,6 +465,17 @@ private predicate finalParameterNodeHasParameterAndIndex(
   n.getIndirectionIndex() = indirectionIndex
 }
 
+pragma[nomagic]
+private predicate hasReturnPosition(IRFunction f, IRBlock block, int index) {
+  exists(Instruction return |
+    return instanceof ReturnInstruction or
+    return instanceof UnreachedInstruction
+  |
+    block.getInstruction(index) = return and
+    return.getEnclosingIRFunction() = f
+  )
+}
+
 class FinalParameterUse extends UseImpl, TFinalParameterUse {
   Parameter p;
 
@@ -493,12 +504,9 @@ class FinalParameterUse extends UseImpl, TFinalParameterUse {
     // `UnreachedInstruction`. If that's the case this predicate will
     // return multiple results. I don't think this is detrimental to
     // performance, however.
-    exists(Instruction return |
-      return instanceof ReturnInstruction or
-      return instanceof UnreachedInstruction
-    |
-      block.getInstruction(index) = return and
-      return.getEnclosingFunction() = p.getFunction()
+    exists(IRFunction f |
+      hasReturnPosition(f, block, index) and
+      f.getFunction() = p.getFunction()
     )
   }
 
@@ -588,13 +596,7 @@ class GlobalUse extends UseImpl, TGlobalUse {
     // globals at any exit so that we can flow out of non-returning functions.
     // Obviously this isn't correct as we can't actually flow but the global flow
     // requires this if we want to flow into children.
-    exists(Instruction return |
-      return instanceof ReturnInstruction or
-      return instanceof UnreachedInstruction
-    |
-      block.getInstruction(index) = return and
-      return.getEnclosingIRFunction() = f
-    )
+    hasReturnPosition(f, block, index)
   }
 
   override BaseSourceVariable getBaseSourceVariable() {
