Merge pull request github#12181 from MathiasVP/fix-node-type

C++: Fix node types

Author: MathiasVP

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

@@ -69,6 +69,31 @@ class Node0Impl extends TIRDataFlowNode0 {
 
   /** Gets a textual representation of this node. */
   final string toString() { result = this.toStringImpl() }
+
+  /** Holds if the value of this node is a glvalue */
+  predicate isGLValue() { none() } // overridden in subclasses
+}
+
+/**
+ * Gets the type of the operand `op`.
+ *
+ * The boolean `isGLValue` is true if the operand represents a glvalue. In that case,
+ * the returned type should be thought of as a pointer type whose base type is given
+ * by this predicate.
+ */
+DataFlowType getOperandType(Operand op, boolean isGLValue) {
+  Ssa::getLanguageType(op).hasType(result, isGLValue)
+}
+
+/**
+ * Gets the type of the instruction `instr`.
+ *
+ * The boolean `isGLValue` is true if the operand represents a glvalue. In that case,
+ * the returned type should be thought of as a pointer type whose base type is given
+ * by this predicate.
+ */
+DataFlowType getInstructionType(Instruction instr, boolean isGLValue) {
+  Ssa::getResultLanguageType(instr).hasType(result, isGLValue)
 }
 
 /**
@@ -84,7 +109,7 @@ abstract class InstructionNode0 extends Node0Impl {
 
   override Declaration getFunction() { result = instr.getEnclosingFunction() }
 
-  override DataFlowType getType() { result = instr.getResultType() }
+  override DataFlowType getType() { result = getInstructionType(instr, _) }
 
   final override Location getLocationImpl() { result = instr.getLocation() }
 
@@ -93,6 +118,8 @@ abstract class InstructionNode0 extends Node0Impl {
     // does not use `Instruction.toString` because that's expensive to compute.
     result = instr.getOpcode().toString()
   }
+
+  final override predicate isGLValue() { exists(getInstructionType(instr, true)) }
 }
 
 /**
@@ -127,11 +154,13 @@ abstract class OperandNode0 extends Node0Impl {
 
   override Declaration getFunction() { result = op.getUse().getEnclosingFunction() }
 
-  override DataFlowType getType() { result = op.getType() }
+  override DataFlowType getType() { result = getOperandType(op, _) }
 
   final override Location getLocationImpl() { result = op.getLocation() }
 
   override string toStringImpl() { result = op.toString() }
+
+  final override predicate isGLValue() { exists(getOperandType(op, true)) }
 }
 
 /**

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

@@ -143,10 +143,13 @@ class Node extends TIRDataFlowNode {
   /** Gets the function to which this node belongs, if any. */
   Declaration getFunction() { none() } // overridden in subclasses
 
+  /** Holds if this node represents a glvalue. */
+  predicate isGLValue() { none() }
+
   /**
    * Gets the type of this node.
    *
-   * If `asInstruction().isGLValue()` holds, then the type of this node
+   * If `isGLValue()` holds, then the type of this node
    * should be thought of as "pointer to `getType()`".
    */
   DataFlowType getType() { none() } // overridden in subclasses
@@ -394,6 +397,8 @@ private class Node0 extends Node, TNode0 {
     // does not use `Instruction.toString` because that's expensive to compute.
     result = node.toStringImpl()
   }
+
+  override predicate isGLValue() { node.isGLValue() }
 }
 
 /**
@@ -497,7 +502,7 @@ class SsaPhiNode extends Node, TSsaPhiNode {
 
   override Declaration getFunction() { result = phi.getBasicBlock().getEnclosingFunction() }
 
-  override DataFlowType getType() { result = this.getAnInput().getType() }
+  override DataFlowType getType() { result = this.getAnInput().getType().getUnspecifiedType() }
 
   final override Location getLocationImpl() { result = phi.getBasicBlock().getLocation() }
 
@@ -591,10 +596,14 @@ class InitialGlobalValue extends Node, TInitialGlobalValue {
 
   override Declaration getFunction() { result = globalDef.getIRFunction().getFunction() }
 
+  final override predicate isGLValue() { globalDef.getIndirectionIndex() = 0 }
+
   override DataFlowType getType() {
-    exists(int indirectionIndex |
-      indirectionIndex = globalDef.getIndirectionIndex() and
-      result = getTypeImpl(globalDef.getUnspecifiedType(), indirectionIndex)
+    exists(DataFlowType type |
+      type = globalDef.getUnspecifiedType() and
+      if this.isGLValue()
+      then result = type
+      else result = getTypeImpl(type, globalDef.getIndirectionIndex() - 1)
     )
   }
 
@@ -843,8 +852,11 @@ class RawIndirectOperand extends Node, TRawIndirectOperand {
   override Declaration getEnclosingCallable() { result = this.getFunction() }
 
   override DataFlowType getType() {
-    exists(int sub | if operand.isGLValue() then sub = 1 else sub = 0 |
-      result = getTypeImpl(operand.getType().getUnspecifiedType(), indirectionIndex - sub)
+    exists(int sub, DataFlowType type, boolean isGLValue |
+      type = getOperandType(operand, isGLValue) and
+      if isGLValue = true then sub = 1 else sub = 0
+    |
+      result = getTypeImpl(type.getUnspecifiedType(), indirectionIndex - sub)
     )
   }
 
@@ -938,8 +950,11 @@ class RawIndirectInstruction extends Node, TRawIndirectInstruction {
   override Declaration getEnclosingCallable() { result = this.getFunction() }
 
   override DataFlowType getType() {
-    exists(int sub | if instr.isGLValue() then sub = 1 else sub = 0 |
-      result = getTypeImpl(instr.getResultType().getUnspecifiedType(), indirectionIndex - sub)
+    exists(int sub, DataFlowType type, boolean isGLValue |
+      type = getInstructionType(instr, isGLValue) and
+      if isGLValue = true then sub = 1 else sub = 0
+    |
+      result = getTypeImpl(type.getUnspecifiedType(), indirectionIndex - sub)
     )
   }
 

cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected

@@ -0,0 +1,3 @@
+failures
+astTypeBugs
+irTypeBugs

cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.ql

@@ -0,0 +1,30 @@
+import TestUtilities.InlineExpectationsTest
+import cpp
+
+module AstTest {
+  private import semmle.code.cpp.dataflow.internal.DataFlowUtil
+
+  query predicate astTypeBugs(Location location, Node node) {
+    exists(int n |
+      n = count(node.getType()) and
+      location = node.getLocation() and
+      n != 1
+    )
+  }
+}
+
+import AstTest
+
+module IrTest {
+  private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
+
+  query predicate irTypeBugs(Location location, Node node) {
+    exists(int n |
+      n = count(node.getType()) and
+      location = node.getLocation() and
+      n != 1
+    )
+  }
+}
+
+import IrTest