Merge pull request #15152 from MathiasVP/fix-unnecessary-evaluation-of-debug-strings

C++: Fix unnecessary evaluation of debug strings

Author: MathiasVP

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

@@ -59,6 +59,41 @@ private module Cached {
 import Cached
 private import Nodes0
 
+/**
+ * A module for calculating the number of stars (i.e., `*`s) needed for various
+ * dataflow node `toString` predicates.
+ */
+module NodeStars {
+  private int getNumberOfIndirections(Node n) {
+    result = n.(RawIndirectOperand).getIndirectionIndex()
+    or
+    result = n.(RawIndirectInstruction).getIndirectionIndex()
+    or
+    result = n.(VariableNode).getIndirectionIndex()
+    or
+    result = n.(PostUpdateNodeImpl).getIndirectionIndex()
+    or
+    result = n.(FinalParameterNode).getIndirectionIndex()
+  }
+
+  private int maxNumberOfIndirections() { result = max(getNumberOfIndirections(_)) }
+
+  private string repeatStars(int n) {
+    n = 0 and result = ""
+    or
+    n = [1 .. maxNumberOfIndirections()] and
+    result = "*" + repeatStars(n - 1)
+  }
+
+  /**
+   * Gets the number of stars (i.e., `*`s) needed to produce the `toString`
+   * output for `n`.
+   */
+  string stars(Node n) { result = repeatStars(getNumberOfIndirections(n)) }
+}
+
+import NodeStars
+
 class Node0Impl extends TIRDataFlowNode0 {
   /**
    * INTERNAL: Do not use.

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

@@ -486,47 +486,6 @@ class Node extends TIRDataFlowNode {
   }
 }
 
-private string toExprString(Node n) {
-  not isDebugMode() and
-  (
-    result = n.asExpr(0).toString()
-    or
-    not exists(n.asExpr()) and
-    result = stars(n) + n.asIndirectExpr(0, 1).toString()
-  )
-}
-
-private module NodeStars {
-  private int getNumberOfIndirections(Node n) {
-    result = n.(RawIndirectOperand).getIndirectionIndex()
-    or
-    result = n.(RawIndirectInstruction).getIndirectionIndex()
-    or
-    result = n.(VariableNode).getIndirectionIndex()
-    or
-    result = n.(PostUpdateNodeImpl).getIndirectionIndex()
-    or
-    result = n.(FinalParameterNode).getIndirectionIndex()
-  }
-
-  private int maxNumberOfIndirections() { result = max(getNumberOfIndirections(_)) }
-
-  private string repeatStars(int n) {
-    n = 0 and result = ""
-    or
-    n = [1 .. maxNumberOfIndirections()] and
-    result = "*" + repeatStars(n - 1)
-  }
-
-  /**
-   * Gets the number of stars (i.e., `*`s) needed to produce the `toString`
-   * output for `n`.
-   */
-  string stars(Node n) { result = repeatStars(getNumberOfIndirections(n)) }
-}
-
-private import NodeStars
-
 /**
  * A class that lifts pre-SSA dataflow nodes to regular dataflow nodes.
  */
@@ -589,7 +548,10 @@ Type stripPointer(Type t) {
   result = t.(FunctionPointerIshType).getBaseType()
 }
 
-private class PostUpdateNodeImpl extends PartialDefinitionNode, TPostUpdateNodeImpl {
+/**
+ * INTERNAL: Do not use.
+ */
+class PostUpdateNodeImpl extends PartialDefinitionNode, TPostUpdateNodeImpl {
   int indirectionIndex;
   Operand operand;
 

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

@@ -1,9 +1,24 @@
 /**
- * This file activates debugging mode for dataflow node printing.
+ * This file contains the class that implements the _debug_ version of
+ * `toString` for `Instruction` and `Operand` dataflow nodes.
  */
 
+private import semmle.code.cpp.ir.IR
+private import codeql.util.Unit
 private import Node0ToString
+private import DataFlowUtil
 
 private class DebugNode0ToString extends Node0ToString {
-  final override predicate isDebugMode() { any() }
+  DebugNode0ToString() {
+    // Silence warning about `this` not being bound.
+    exists(this)
+  }
+
+  override string instructionToString(Instruction i) { result = i.getDumpString() }
+
+  override string operandToString(Operand op) {
+    result = op.getDumpString() + " @ " + op.getUse().getResultId()
+  }
+
+  override string toExprString(Node n) { none() }
 }

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

@@ -1,75 +1,53 @@
 /**
- * This file contains the abstract class that serves as the base class for
- * dataflow node printing.
+ * This file imports the class that is used to construct the strings used by
+ * `Node.ToString`.
  *
- * By default, a non-debug string is produced. However, a debug-friendly
- * string can be produced by importing `DebugPrinting.qll`.
+ * Normally, this file should just import `NormalNode0ToString` to compute the
+ * efficient `toString`, but for debugging purposes one can import
+ * `DebugPrinting.qll` to better correlate the dataflow nodes with their
+ * underlying instructions and operands.
  */
 
 private import semmle.code.cpp.ir.IR
 private import codeql.util.Unit
+private import DataFlowUtil
+import NormalNode0ToString // Change this import to control which version should be used.
 
-/**
- * A class to control whether a debugging version of instructions and operands
- * should be printed as part of the `toString` output of dataflow nodes.
- *
- * To enable debug printing import the `DebugPrinting.ql` file. By default,
- * non-debug output will be used.
- */
-class Node0ToString extends Unit {
-  abstract predicate isDebugMode();
-
-  private string normalInstructionToString(Instruction i) {
-    not this.isDebugMode() and
-    if i.(InitializeParameterInstruction).getIRVariable() instanceof IRThisVariable
-    then result = "this"
-    else result = i.getAst().toString()
-  }
-
-  private string normalOperandToString(Operand op) {
-    not this.isDebugMode() and
-    if op.getDef().(InitializeParameterInstruction).getIRVariable() instanceof IRThisVariable
-    then result = "this"
-    else result = op.getDef().getAst().toString()
-  }
-
+/** An abstract class to control the behavior of `Node.toString`. */
+abstract class Node0ToString extends Unit {
   /**
-   * Gets the string that should be used by `InstructionNode.toString`
+   * Gets the string that should be used by `OperandNode.toString` to print the
+   * dataflow node whose underlying operand is `op.`
    */
-  string instructionToString(Instruction i) {
-    if this.isDebugMode()
-    then result = i.getDumpString()
-    else result = this.normalInstructionToString(i)
-  }
+  abstract string operandToString(Operand op);
 
   /**
-   * Gets the string that should be used by `OperandNode.toString`.
+   * Gets the string that should be used by `InstructionNode.toString` to print
+   * the dataflow node whose underlying instruction is `instr`.
    */
-  string operandToString(Operand op) {
-    if this.isDebugMode()
-    then result = op.getDumpString() + " @ " + op.getUse().getResultId()
-    else result = this.normalOperandToString(op)
-  }
-}
+  abstract string instructionToString(Instruction i);
 
-private class NoDebugNode0ToString extends Node0ToString {
-  final override predicate isDebugMode() { none() }
+  /**
+   * Gets the string representation of the `Expr` associated with `n`, if any.
+   */
+  abstract string toExprString(Node n);
 }
 
 /**
- * Gets the string that should be used by `OperandNode.toString`.
+ * Gets the string that should be used by `OperandNode.toString` to print the
+ * dataflow node whose underlying operand is `op.`
  */
-string operandToString(Operand op) { result = any(Node0ToString nts).operandToString(op) }
+string operandToString(Operand op) { result = any(Node0ToString s).operandToString(op) }
 
 /**
- * Gets the string that should be used by `InstructionNode.toString`
+ * Gets the string that should be used by `InstructionNode.toString` to print
+ * the dataflow node whose underlying instruction is `instr`.
  */
-string instructionToString(Instruction i) { result = any(Node0ToString nts).instructionToString(i) }
+string instructionToString(Instruction instr) {
+  result = any(Node0ToString s).instructionToString(instr)
+}
 
 /**
- * Holds if debugging mode is enabled.
- *
- * In debug mode the `toString` on dataflow nodes is more expensive to compute,
- * but gives more precise information about the different dataflow nodes.
+ * Gets the string representation of the `Expr` associated with `n`, if any.
  */
-predicate isDebugMode() { any(Node0ToString nts).isDebugMode() }
+string toExprString(Node n) { result = any(Node0ToString s).toExprString(n) }

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

@@ -0,0 +1,36 @@
+/**
+ * This file contains the class that implements the non-debug version of
+ * `toString` for `Instruction` and `Operand` dataflow nodes.
+ */
+
+private import semmle.code.cpp.ir.IR
+private import codeql.util.Unit
+private import Node0ToString
+private import DataFlowUtil
+private import DataFlowPrivate
+
+private class NormalNode0ToString extends Node0ToString {
+  NormalNode0ToString() {
+    // Silence warning about `this` not being bound.
+    exists(this)
+  }
+
+  override string instructionToString(Instruction i) {
+    if i.(InitializeParameterInstruction).getIRVariable() instanceof IRThisVariable
+    then result = "this"
+    else result = i.getAst().toString()
+  }
+
+  override string operandToString(Operand op) {
+    if op.getDef().(InitializeParameterInstruction).getIRVariable() instanceof IRThisVariable
+    then result = "this"
+    else result = op.getDef().getAst().toString()
+  }
+
+  override string toExprString(Node n) {
+    result = n.asExpr(0).toString()
+    or
+    not exists(n.asExpr()) and
+    result = stars(n) + n.asIndirectExpr(0, 1).toString()
+  }
+}

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

@@ -1,6 +1,7 @@
 private import cpp
 private import semmle.code.cpp.ir.IR
 private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
+private import semmle.code.cpp.ir.dataflow.internal.DataFlowPrivate
 private import SsaInternals as Ssa
 private import PrintIRUtilities
 
@@ -33,9 +34,9 @@ private string getNodeProperty(Node node, string key) {
   key = "flow" and
   result =
     strictconcat(string flow, boolean to, int order1, int order2 |
-      flow = getFromFlow(node, order1, order2) + "->" + starsForNode(node) + "@" and to = false
+      flow = getFromFlow(node, order1, order2) + "->" + stars(node) + "@" and to = false
       or
-      flow = starsForNode(node) + "@->" + getToFlow(node, order1, order2) and to = true
+      flow = stars(node) + "@->" + getToFlow(node, order1, order2) and to = true
     |
       flow, ", " order by to, order1, order2, flow
     )

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

@@ -7,37 +7,14 @@ private import semmle.code.cpp.ir.IR
 private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
 private import semmle.code.cpp.ir.dataflow.internal.DataFlowPrivate
 
-private string stars(int k) {
-  k =
-    [0 .. max([
-            any(RawIndirectInstruction n).getIndirectionIndex(),
-            any(RawIndirectOperand n).getIndirectionIndex()
-          ]
-      )] and
-  (if k = 0 then result = "" else result = "*" + stars(k - 1))
-}
-
-string starsForNode(Node node) {
-  exists(int indirectionIndex |
-    node.(IndirectInstruction).hasInstructionAndIndirectionIndex(_, indirectionIndex) or
-    node.(IndirectOperand).hasOperandAndIndirectionIndex(_, indirectionIndex)
-  |
-    result = stars(indirectionIndex)
-  )
-  or
-  not node instanceof IndirectInstruction and
-  not node instanceof IndirectOperand and
-  result = ""
-}
-
 private Instruction getInstruction(Node n, string stars) {
   result = [n.asInstruction(), n.(RawIndirectInstruction).getInstruction()] and
-  stars = starsForNode(n)
+  stars = stars(n)
 }
 
 private Operand getOperand(Node n, string stars) {
   result = [n.asOperand(), n.(RawIndirectOperand).getOperand()] and
-  stars = starsForNode(n)
+  stars = stars(n)
 }
 
 /**