C++: Hide that IR DataFlow::Node is Instruction

We haven't come to a conclusion on whether these two types will remain
identical forever. To make sure we're able to change it in the future,
this change makes it impossible to cast between the two types. Callers
must use the `asInstruction` member predicate to convert.

Author: jbj

cpp/ql/src/semmle/code/cpp/ir/dataflow/TaintTracking.qll

@@ -133,6 +133,14 @@ module TaintTracking {
     // Taint can flow into using ordinary data flow.
     DataFlow::localFlowStep(nodeFrom, nodeTo)
     or
+    localInstructionTaintStep(nodeFrom.asInstruction(), nodeTo.asInstruction())
+  }
+
+  /**
+   * Holds if taint propagates from `nodeFrom` to `nodeTo` in exactly one local
+   * (intra-procedural) step.
+   */
+  private predicate localInstructionTaintStep(Instruction nodeFrom, Instruction nodeTo) {
     // Taint can flow through expressions that alter the value but preserve
     // more than one bit of it _or_ expressions that follow data through
     // pointer indirections.

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

@@ -8,16 +8,16 @@ private import DataFlowDispatch
  * to the callable. Instance arguments (`this` pointer) are also included.
  */
 class ArgumentNode extends Node {
-  ArgumentNode() { exists(CallInstruction call | this = call.getAnArgument()) }
+  ArgumentNode() { exists(CallInstruction call | this.asInstruction() = call.getAnArgument()) }
 
   /**
    * Holds if this argument occurs at the given position in the given call.
    * The instance argument is considered to have index `-1`.
    */
   predicate argumentOf(DataFlowCall call, int pos) {
-    this = call.getPositionalArgument(pos)
+    this.asInstruction() = call.getPositionalArgument(pos)
     or
-    this = call.getThisArgument() and pos = -1
+    this.asInstruction() = call.getThisArgument() and pos = -1
   }
 
   /** Gets the call in which this node is an argument. */
@@ -37,24 +37,26 @@ class ReturnKind extends TReturnKind {
 
 /** A data flow node that occurs as the result of a `ReturnStmt`. */
 class ReturnNode extends Node {
-  ReturnNode() { exists(ReturnValueInstruction ret | this = ret.getReturnValue()) }
+  ReturnNode() { exists(ReturnValueInstruction ret | this.asInstruction() = ret.getReturnValue()) }
 
   /** Gets the kind of this returned value. */
   ReturnKind getKind() { result = TNormalReturnKind() }
 }
 
 /** A data flow node that represents the output of a call. */
-class OutNode extends Node, CallInstruction {
+class OutNode extends Node {
+  override CallInstruction instr;
+
   /** Gets the underlying call. */
-  DataFlowCall getCall() { result = this }
+  DataFlowCall getCall() { result = instr }
 }
 
 /**
  * Gets a node that can read the value returned from `call` with return kind
  * `kind`.
  */
 OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) {
-  result = call and
+  result.getCall() = call and
   kind = TNormalReturnKind()
 }
 
@@ -192,11 +194,13 @@ class DataFlowType = Type;
 class DataFlowLocation = Location;
 
 /** A function call relevant for data flow. */
-class DataFlowCall extends CallInstruction, Node {
+class DataFlowCall extends CallInstruction {
   /**
    * Gets the nth argument for this call.
    *
    * The range of `n` is from `0` to `getNumberOfArguments() - 1`.
    */
-  Node getArgument(int n) { result = this.getPositionalArgument(n) }
+  Node getArgument(int n) { result.asInstruction() = this.getPositionalArgument(n) }
+
+  Function getEnclosingCallable() { result = this.getEnclosingFunction() }
 }

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

@@ -6,21 +6,36 @@ private import cpp
 private import semmle.code.cpp.ir.IR
 private import semmle.code.cpp.controlflow.IRGuards
 
+/**
+ * A newtype wrapper to prevent accidental casts between `Node` and
+ * `Instruction`. This ensures we can add `Node`s that are not `Instruction`s
+ * in the future.
+ */
+private newtype TIRDataFlowNode = MkIRDataFlowNode(Instruction i)
+
 /**
  * A node in a data flow graph.
  *
  * A node can be either an expression, a parameter, or an uninitialized local
  * variable. Such nodes are created with `DataFlow::exprNode`,
  * `DataFlow::parameterNode`, and `DataFlow::uninitializedNode` respectively.
  */
-class Node extends Instruction {
+class Node extends TIRDataFlowNode {
+  Instruction instr;
+
+  Node() { this = MkIRDataFlowNode(instr) }
+
   /**
    * INTERNAL: Do not use. Alternative name for `getFunction`.
    */
-  Function getEnclosingCallable() { result = this.getEnclosingFunction() }
+  Function getEnclosingCallable() { result = this.getFunction() }
+
+  Function getFunction() { result = instr.getEnclosingFunction() }
 
   /** Gets the type of this node. */
-  Type getType() { result = this.getResultType() }
+  Type getType() { result = instr.getResultType() }
+
+  Instruction asInstruction() { this = MkIRDataFlowNode(result) }
 
   /**
    * Gets the non-conversion expression corresponding to this node, if any. If
@@ -29,30 +44,33 @@ class Node extends Instruction {
    * expression.
    */
   Expr asExpr() {
-    result.getConversion*() = this.getConvertedResultExpression() and
+    result.getConversion*() = instr.getConvertedResultExpression() and
     not result instanceof Conversion
   }
 
   /**
    * Gets the expression corresponding to this node, if any. The returned
    * expression may be a `Conversion`.
    */
-  Expr asConvertedExpr() { result = this.getConvertedResultExpression() }
+  Expr asConvertedExpr() { result = instr.getConvertedResultExpression() }
 
   /** Gets the parameter corresponding to this node, if any. */
-  Parameter asParameter() { result = this.(InitializeParameterInstruction).getParameter() }
+  Parameter asParameter() { result = instr.(InitializeParameterInstruction).getParameter() }
 
   /**
    * Gets the uninitialized local variable corresponding to this node, if
    * any.
    */
-  LocalVariable asUninitialized() { result = this.(UninitializedInstruction).getLocalVariable() }
+  LocalVariable asUninitialized() { result = instr.(UninitializedInstruction).getLocalVariable() }
 
   /**
    * Gets an upper bound on the type of this node.
    */
   Type getTypeBound() { result = getType() }
 
+  /** Gets the location of this element. */
+  Location getLocation() { result = instr.getLocation() }
+
   /**
    * Holds if this element is at the specified location.
    * The location spans column `startcolumn` of line `startline` to
@@ -63,8 +81,10 @@ class Node extends Instruction {
   predicate hasLocationInfo(
     string filepath, int startline, int startcolumn, int endline, int endcolumn
   ) {
-    getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
   }
+
+  string toString() { result = instr.toString() }
 }
 
 /**
@@ -92,19 +112,27 @@ class ExprNode extends Node {
  * The value of a parameter at function entry, viewed as a node in a data
  * flow graph.
  */
-class ParameterNode extends Node, InitializeParameterInstruction {
+class ParameterNode extends Node {
+  override InitializeParameterInstruction instr;
+
   /**
    * Holds if this node is the parameter of `c` at the specified (zero-based)
    * position. The implicit `this` parameter is considered to have index `-1`.
    */
-  predicate isParameterOf(Function f, int i) { f.getParameter(i) = getParameter() }
+  predicate isParameterOf(Function f, int i) { f.getParameter(i) = instr.getParameter() }
+
+  Parameter getParameter() { result = instr.getParameter() }
 }
 
 /**
  * The value of an uninitialized local variable, viewed as a node in a data
  * flow graph.
  */
-class UninitializedNode extends Node, UninitializedInstruction { }
+class UninitializedNode extends Node {
+  override UninitializedInstruction instr;
+
+  LocalVariable getLocalVariable() { result = instr.getLocalVariable() }
+}
 
 /**
  * A node associated with an object after an operation that might have
@@ -166,10 +194,14 @@ predicate localFlowStep(Node nodeFrom, Node nodeTo) {
  * data flow. It may have less flow than the `localFlowStep` predicate.
  */
 predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
-  nodeTo.(CopyInstruction).getSourceValue() = nodeFrom or
-  nodeTo.(PhiInstruction).getAnOperand().getDef() = nodeFrom or
+  simpleInstructionLocalFlowStep(nodeFrom.asInstruction(), nodeTo.asInstruction())
+}
+
+private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction iTo) {
+  iTo.(CopyInstruction).getSourceValue() = iFrom or
+  iTo.(PhiInstruction).getAnOperand().getDef() = iFrom or
   // Treat all conversions as flow, even conversions between different numeric types.
-  nodeTo.(ConvertInstruction).getUnary() = nodeFrom
+  iTo.(ConvertInstruction).getUnary() = iFrom
 }
 
 /**