C++: Refactor parameter nodes into an abstract class that's easier to extend.

Author: MathiasVP

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

@@ -1247,7 +1247,7 @@ module IsUnreachableInCall {
 
   predicate isUnreachableInCall(Node n, DataFlowCall call) {
     exists(
-      DirectParameterNode paramNode, ConstantIntegralTypeArgumentNode arg,
+      InstructionDirectParameterNode paramNode, ConstantIntegralTypeArgumentNode arg,
       IntegerConstantInstruction constant, int k, Operand left, Operand right, IRBlock block
     |
       // arg flows into `paramNode`
@@ -1461,7 +1461,7 @@ private predicate getAdditionalFlowIntoCallNodeTermStep(Node node1, Node node2)
 /** Gets the `IRVariable` associated with the parameter node `p`. */
 pragma[nomagic]
 private IRVariable getIRVariableForParameterNode(ParameterNode p) {
-  result = p.(DirectParameterNode).getIRVariable()
+  result = p.(InstructionDirectParameterNode).getIRVariable()
   or
   result.getAst() = p.(IndirectParameterNode).getParameter()
 }

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

@@ -389,7 +389,7 @@ class Node extends TIRDataFlowNode {
     index = 0 and
     result = this.(ExplicitParameterNode).getParameter()
     or
-    this.(IndirectParameterNode).hasInstructionAndIndirectionIndex(_, index) and
+    this.(IndirectParameterNode).getIndirectionIndex() = index and
     result = this.(IndirectParameterNode).getParameter()
   }
 
@@ -767,42 +767,6 @@ class FlowSummaryNode extends Node, TFlowSummaryNode {
   override string toStringImpl() { result = this.getSummaryNode().toString() }
 }
 
-/**
- * INTERNAL: do not use.
- *
- * A node representing an indirection of a parameter.
- */
-class IndirectParameterNode extends Node instanceof IndirectInstruction {
-  InitializeParameterInstruction init;
-
-  IndirectParameterNode() { IndirectInstruction.super.hasInstructionAndIndirectionIndex(init, _) }
-
-  int getArgumentIndex() { init.hasIndex(result) }
-
-  /** Gets the parameter whose indirection is initialized. */
-  Parameter getParameter() { result = init.getParameter() }
-
-  override Declaration getEnclosingCallable() { result = this.getFunction() }
-
-  override Declaration getFunction() { result = init.getEnclosingFunction() }
-
-  /** Gets the underlying operand and the underlying indirection index. */
-  predicate hasInstructionAndIndirectionIndex(Instruction instr, int index) {
-    IndirectInstruction.super.hasInstructionAndIndirectionIndex(instr, index)
-  }
-
-  override Location getLocationImpl() { result = this.getParameter().getLocation() }
-
-  override string toStringImpl() {
-    exists(string prefix | prefix = stars(this) |
-      result = prefix + this.getParameter().toString()
-      or
-      not exists(this.getParameter()) and
-      result = prefix + "this"
-    )
-  }
-}
-
 /**
  * INTERNAL: do not use.
  *
@@ -1655,6 +1619,89 @@ class IndirectExprNode extends Node instanceof IndirectExprNodeBase {
   }
 }
 
+abstract private class AbstractParameterNode extends Node {
+  /**
+   * Holds if this node is the parameter of `f` at the specified position. The
+   * implicit `this` parameter is considered to have position `-1`, and
+   * pointer-indirection parameters are at further negative positions.
+   */
+  abstract predicate isParameterOf(DataFlowCallable f, ParameterPosition pos);
+
+  /** Gets the `Parameter` associated with this node, if it exists. */
+  Parameter getParameter() { none() } // overridden by subclasses
+}
+
+abstract private class AbstractIndirectParameterNode extends AbstractParameterNode {
+  abstract int getIndirectionIndex();
+}
+
+/**
+ * INTERNAL: do not use.
+ *
+ * A node representing an indirection of a parameter.
+ */
+final class IndirectParameterNode = AbstractIndirectParameterNode;
+
+pragma[noinline]
+private predicate indirectParameterNodeHasArgumentIndexAndIndex(
+  IndirectInstructionParameterNode node, int argumentIndex, int indirectionIndex
+) {
+  node.hasInstructionAndIndirectionIndex(_, indirectionIndex) and
+  node.getArgumentIndex() = argumentIndex
+}
+
+pragma[noinline]
+private predicate indirectPositionHasArgumentIndexAndIndex(
+  IndirectionPosition pos, int argumentIndex, int indirectionIndex
+) {
+  pos.getArgumentIndex() = argumentIndex and
+  pos.getIndirectionIndex() = indirectionIndex
+}
+
+private class IndirectInstructionParameterNode extends AbstractIndirectParameterNode instanceof IndirectInstruction
+{
+  InitializeParameterInstruction init;
+
+  IndirectInstructionParameterNode() {
+    IndirectInstruction.super.hasInstructionAndIndirectionIndex(init, _)
+  }
+
+  int getArgumentIndex() { init.hasIndex(result) }
+
+  override Location getLocationImpl() { result = IndirectInstruction.super.getLocationImpl() }
+
+  override string toStringImpl() {
+    exists(string prefix | prefix = stars(this) |
+      result = prefix + this.getParameter().toString()
+      or
+      not exists(this.getParameter()) and
+      result = prefix + "this"
+    )
+  }
+
+  /** Gets the parameter whose indirection is initialized. */
+  override Parameter getParameter() { result = init.getParameter() }
+
+  override Declaration getEnclosingCallable() { result = this.getFunction() }
+
+  override Declaration getFunction() { result = init.getEnclosingFunction() }
+
+  override predicate isParameterOf(DataFlowCallable f, ParameterPosition pos) {
+    this.getEnclosingCallable() = f.getUnderlyingCallable() and
+    exists(int argumentIndex, int indirectionIndex |
+      indirectPositionHasArgumentIndexAndIndex(pos, argumentIndex, indirectionIndex) and
+      indirectParameterNodeHasArgumentIndexAndIndex(this, argumentIndex, indirectionIndex)
+    )
+  }
+
+  /** Gets the underlying operand and the underlying indirection index. */
+  predicate hasInstructionAndIndirectionIndex(Instruction instr, int index) {
+    IndirectInstruction.super.hasInstructionAndIndirectionIndex(instr, index)
+  }
+
+  final override int getIndirectionIndex() { this.hasInstructionAndIndirectionIndex(init, result) }
+}
+
 /**
  * The value of a parameter at function entry, viewed as a node in a data
  * flow graph. This includes both explicit parameters such as `x` in `f(x)`
@@ -1664,42 +1711,33 @@ class IndirectExprNode extends Node instanceof IndirectExprNodeBase {
  * `ExplicitParameterNode`, `ThisParameterNode`, or
  * `ParameterIndirectionNode`.
  */
-class ParameterNode extends Node {
-  ParameterNode() {
-    // To avoid making this class abstract, we enumerate its values here
-    this.asInstruction() instanceof InitializeParameterInstruction
-    or
-    this instanceof IndirectParameterNode
-    or
-    FlowSummaryImpl::Private::summaryParameterNode(this.(FlowSummaryNode).getSummaryNode(), _)
-  }
+final class ParameterNode = AbstractParameterNode;
 
-  /**
-   * Holds if this node is the parameter of `f` at the specified position. The
-   * implicit `this` parameter is considered to have position `-1`, and
-   * pointer-indirection parameters are at further negative positions.
-   */
-  predicate isParameterOf(DataFlowCallable f, ParameterPosition pos) { none() } // overridden by subclasses
-
-  /** Gets the `Parameter` associated with this node, if it exists. */
-  Parameter getParameter() { none() } // overridden by subclasses
-}
+abstract private class AbstractDirectParameterNode extends AbstractParameterNode { }
 
 /** An explicit positional parameter, including `this`, but not `...`. */
-class DirectParameterNode extends InstructionNode {
-  override InitializeParameterInstruction instr;
+final class DirectParameterNode = AbstractDirectParameterNode;
+
+abstract class InstructionDirectParameterNode extends InstructionNode, AbstractDirectParameterNode {
+  final override InitializeParameterInstruction instr;
 
   /**
    * INTERNAL: Do not use.
    *
    * Gets the `IRVariable` that this parameter references.
    */
-  IRVariable getIRVariable() { result = instr.getIRVariable() }
+  final IRVariable getIRVariable() { result = instr.getIRVariable() }
 }
 
+abstract private class AbstractExplicitParameterNode extends AbstractDirectParameterNode { }
+
+final class ExplicitParameterNode = AbstractExplicitParameterNode;
+
 /** An explicit positional parameter, not including `this` or `...`. */
-private class ExplicitParameterNode extends ParameterNode, DirectParameterNode {
-  ExplicitParameterNode() { exists(instr.getParameter()) }
+private class ExplicitParameterInstructionNode extends AbstractExplicitParameterNode,
+  InstructionDirectParameterNode
+{
+  ExplicitParameterInstructionNode() { exists(instr.getParameter()) }
 
   override predicate isParameterOf(DataFlowCallable f, ParameterPosition pos) {
     f.getUnderlyingCallable().(Function).getParameter(pos.(DirectPosition).getIndex()) =
@@ -1712,8 +1750,10 @@ private class ExplicitParameterNode extends ParameterNode, DirectParameterNode {
 }
 
 /** An implicit `this` parameter. */
-class ThisParameterNode extends ParameterNode, DirectParameterNode {
-  ThisParameterNode() { instr.getIRVariable() instanceof IRThisVariable }
+class ThisParameterInstructionNode extends AbstractExplicitParameterNode,
+  InstructionDirectParameterNode
+{
+  ThisParameterInstructionNode() { instr.getIRVariable() instanceof IRThisVariable }
 
   override predicate isParameterOf(DataFlowCallable f, ParameterPosition pos) {
     pos.(DirectPosition).getIndex() = -1 and
@@ -1726,7 +1766,7 @@ class ThisParameterNode extends ParameterNode, DirectParameterNode {
 /**
  * A parameter node that is part of a summary.
  */
-class SummaryParameterNode extends ParameterNode, FlowSummaryNode {
+class SummaryParameterNode extends AbstractParameterNode, FlowSummaryNode {
   SummaryParameterNode() {
     FlowSummaryImpl::Private::summaryParameterNode(this.getSummaryNode(), _)
   }