Merge pull request github#12532 from MathiasVP/local-flow-for-getAdditionalFlowIntoCallNodeTerm

C++: Use local flow instead of GVN in `getAdditionalFlowIntoCallNodeTerm`

Author: MathiasVP

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

@@ -5,33 +5,58 @@ private import DataFlowDispatch
 private import DataFlowImplConsistency
 private import semmle.code.cpp.ir.internal.IRCppLanguage
 private import SsaInternals as Ssa
-private import DataFlowImplCommon
-private import semmle.code.cpp.ir.ValueNumbering
+private import DataFlowImplCommon as DataFlowImplCommon
 
 cached
 private module Cached {
   cached
-  newtype TIRDataFlowNode0 =
-    TInstructionNode0(Instruction i) {
-      not Ssa::ignoreInstruction(i) and
-      not exists(Operand op |
-        not Ssa::ignoreOperand(op) and i = Ssa::getIRRepresentationOfOperand(op)
-      ) and
-      // We exclude `void`-typed instructions because they cannot contain data.
-      // However, if the instruction is a glvalue, and their type is `void`, then the result
-      // type of the instruction is really `void*`, and thus we still want to have a dataflow
-      // node for it.
-      (not i.getResultType() instanceof VoidType or i.isGLValue())
-    } or
-    TMultipleUseOperandNode0(Operand op) {
-      not Ssa::ignoreOperand(op) and not exists(Ssa::getIRRepresentationOfOperand(op))
-    } or
-    TSingleUseOperandNode0(Operand op) {
-      not Ssa::ignoreOperand(op) and exists(Ssa::getIRRepresentationOfOperand(op))
-    }
-}
-
-private import Cached
+  module Nodes0 {
+    cached
+    newtype TIRDataFlowNode0 =
+      TInstructionNode0(Instruction i) {
+        not Ssa::ignoreInstruction(i) and
+        not exists(Operand op |
+          not Ssa::ignoreOperand(op) and i = Ssa::getIRRepresentationOfOperand(op)
+        ) and
+        // We exclude `void`-typed instructions because they cannot contain data.
+        // However, if the instruction is a glvalue, and their type is `void`, then the result
+        // type of the instruction is really `void*`, and thus we still want to have a dataflow
+        // node for it.
+        (not i.getResultType() instanceof VoidType or i.isGLValue())
+      } or
+      TMultipleUseOperandNode0(Operand op) {
+        not Ssa::ignoreOperand(op) and not exists(Ssa::getIRRepresentationOfOperand(op))
+      } or
+      TSingleUseOperandNode0(Operand op) {
+        not Ssa::ignoreOperand(op) and exists(Ssa::getIRRepresentationOfOperand(op))
+      }
+  }
+
+  /**
+   * Gets an additional term that is added to the `join` and `branch` computations to reflect
+   * an additional forward or backwards branching factor that is not taken into account
+   * when calculating the (virtual) dispatch cost.
+   *
+   * Argument `arg` is part of a path from a source to a sink, and `p` is the target parameter.
+   */
+  pragma[nomagic]
+  cached
+  int getAdditionalFlowIntoCallNodeTerm(ArgumentNode arg, ParameterNode p) {
+    DataFlowImplCommon::forceCachingInSameStage() and
+    exists(
+      ParameterNode switchee, SwitchInstruction switch, ConditionOperand op, DataFlowCall call
+    |
+      DataFlowImplCommon::viableParamArg(call, p, arg) and
+      DataFlowImplCommon::viableParamArg(call, switchee, _) and
+      switch.getExpressionOperand() = op and
+      getAdditionalFlowIntoCallNodeTermStep+(switchee, operandNode(op)) and
+      result = countNumberOfBranchesUsingParameter(switch, p)
+    )
+  }
+}
+
+import Cached
+private import Nodes0
 
 class Node0Impl extends TIRDataFlowNode0 {
   /**
@@ -898,23 +923,54 @@ IRBlock getBasicBlock(Node node) {
 }
 
 /**
- * Gets an additional term that is added to the `join` and `branch` computations to reflect
- * an additional forward or backwards branching factor that is not taken into account
- * when calculating the (virtual) dispatch cost.
- *
- * Argument `arg` is part of a path from a source to a sink, and `p` is the target parameter.
+ * A local flow relation that includes both local steps, read steps and
+ * argument-to-return flow through summarized functions.
  */
-pragma[nomagic]
-int getAdditionalFlowIntoCallNodeTerm(ArgumentNode arg, ParameterNode p) {
-  exists(ParameterNode switchee, SwitchInstruction switch, ConditionOperand op, DataFlowCall call |
-    viableParamArg(call, p, arg) and
-    viableParamArg(call, switchee, _) and
-    switch.getExpressionOperand() = op and
-    valueNumber(switchee.asInstruction()).getAUse() = op and
-    result = countNumberOfBranchesUsingParameter(switch, p)
+private predicate localFlowStepWithSummaries(Node node1, Node node2) {
+  localFlowStep(node1, node2)
+  or
+  readStep(node1, _, node2)
+  or
+  DataFlowImplCommon::argumentValueFlowsThrough(node1, _, node2)
+}
+
+/** Holds if `node` flows to a node that is used in a `SwitchInstruction`. */
+private predicate localStepsToSwitch(Node node) {
+  node.asOperand() = any(SwitchInstruction switch).getExpressionOperand()
+  or
+  exists(Node succ |
+    localStepsToSwitch(succ) and
+    localFlowStepWithSummaries(node, succ)
   )
 }
 
+/**
+ * Holds if `node` is part of a path from a `ParameterNode` to an operand
+ * of a `SwitchInstruction`.
+ */
+private predicate localStepsFromParameterToSwitch(Node node) {
+  localStepsToSwitch(node) and
+  (
+    node instanceof ParameterNode
+    or
+    exists(Node prev |
+      localStepsFromParameterToSwitch(prev) and
+      localFlowStepWithSummaries(prev, node)
+    )
+  )
+}
+
+/**
+ * The local flow relation `localFlowStepWithSummaries` pruned to only
+ * include steps that are part of a path from a `ParameterNode` to an
+ * operand of a `SwitchInstruction`.
+ */
+private predicate getAdditionalFlowIntoCallNodeTermStep(Node node1, Node node2) {
+  localStepsFromParameterToSwitch(node1) and
+  localStepsFromParameterToSwitch(node2) and
+  localFlowStepWithSummaries(node1, node2)
+}
+
 /** Gets the `IRVariable` associated with the parameter node `p`. */
 pragma[nomagic]
 private IRVariable getIRVariableForParameterNode(ParameterNode p) {
@@ -943,7 +999,7 @@ private EdgeKind caseOrDefaultEdge() {
 /**
  * Gets the number of switch branches that that read from (or write to) the parameter `p`.
  */
-int countNumberOfBranchesUsingParameter(SwitchInstruction switch, ParameterNode p) {
+private int countNumberOfBranchesUsingParameter(SwitchInstruction switch, ParameterNode p) {
   exists(Ssa::SourceVariable sv |
     parameterNodeHasSourceVariable(p, sv) and
     // Count the number of cases that use the parameter. We do this by finding the phi node