Merge pull request #12004 from jketema/single-use

C++: Map operand nodes that are only used once onto the related instruction node

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

@@ -12,13 +12,21 @@ private module 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
-    TOperandNode0(Operand op) { not Ssa::ignoreOperand(op) }
+    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
@@ -66,11 +74,9 @@ class Node0Impl extends TIRDataFlowNode0 {
 /**
  * An instruction, viewed as a node in a data flow graph.
  */
-class InstructionNode0 extends Node0Impl, TInstructionNode0 {
+abstract class InstructionNode0 extends Node0Impl {
   Instruction instr;
 
-  InstructionNode0() { this = TInstructionNode0(instr) }
-
   /** Gets the instruction corresponding to this node. */
   Instruction getInstruction() { result = instr }
 
@@ -85,18 +91,35 @@ class InstructionNode0 extends Node0Impl, TInstructionNode0 {
   override string toStringImpl() {
     // This predicate is overridden in subclasses. This default implementation
     // does not use `Instruction.toString` because that's expensive to compute.
-    result = this.getInstruction().getOpcode().toString()
+    result = instr.getOpcode().toString()
+  }
+}
+
+/**
+ * An instruction without an operand that is used only once, viewed as a node in a data flow graph.
+ */
+private class InstructionInstructionNode0 extends InstructionNode0, TInstructionNode0 {
+  InstructionInstructionNode0() { this = TInstructionNode0(instr) }
+}
+
+/**
+ * An instruction with an operand that is used only once, viewed as a node in a data flow graph.
+ */
+private class SingleUseOperandInstructionNode0 extends InstructionNode0, TSingleUseOperandNode0 {
+  SingleUseOperandInstructionNode0() {
+    exists(Operand op |
+      this = TSingleUseOperandNode0(op) and
+      instr = Ssa::getIRRepresentationOfOperand(op)
+    )
   }
 }
 
 /**
  * An operand, viewed as a node in a data flow graph.
  */
-class OperandNode0 extends Node0Impl, TOperandNode0 {
+abstract class OperandNode0 extends Node0Impl {
   Operand op;
 
-  OperandNode0() { this = TOperandNode0(op) }
-
   /** Gets the operand corresponding to this node. */
   Operand getOperand() { result = op }
 
@@ -108,7 +131,21 @@ class OperandNode0 extends Node0Impl, TOperandNode0 {
 
   final override Location getLocationImpl() { result = op.getLocation() }
 
-  override string toStringImpl() { result = this.getOperand().toString() }
+  override string toStringImpl() { result = op.toString() }
+}
+
+/**
+ * An operand that is used multiple times, viewed as a node in a data flow graph.
+ */
+private class MultipleUseOperandNode0 extends OperandNode0, TMultipleUseOperandNode0 {
+  MultipleUseOperandNode0() { this = TMultipleUseOperandNode0(op) }
+}
+
+/**
+ * An operand that is used only once, viewed as a node in a data flow graph.
+ */
+private class SingleUseOperandNode0 extends OperandNode0, TSingleUseOperandNode0 {
+  SingleUseOperandNode0() { this = TSingleUseOperandNode0(op) }
 }
 
 /**

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

@@ -404,7 +404,7 @@ class OperandNode extends Node, Node0 {
   OperandNode() { op = node.getOperand() }
 
   /** Gets the operand corresponding to this node. */
-  Operand getOperand() { result = node.getOperand() }
+  Operand getOperand() { result = op }
 
   override string toStringImpl() { result = op.getDef().getAst().toString() }
 }
@@ -1439,7 +1439,13 @@ private module Cached {
     simpleInstructionLocalFlowStep(nodeFrom.asOperand(), nodeTo.asInstruction())
     or
     // Instruction -> Operand flow
-    simpleOperandLocalFlowStep(nodeFrom.asInstruction(), nodeTo.asOperand())
+    exists(Instruction iFrom, Operand opTo |
+      iFrom = nodeFrom.asInstruction() and opTo = nodeTo.asOperand()
+    |
+      simpleOperandLocalFlowStep(iFrom, opTo) and
+      // Omit when the instruction node also represents the operand.
+      not iFrom = Ssa::getIRRepresentationOfOperand(opTo)
+    )
     or
     // Phi node -> Node flow
     Ssa::fromPhiNode(nodeFrom, nodeTo)

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

@@ -719,6 +719,18 @@ private module Cached {
     )
   }
 
+  /**
+   * Holds if the underlying IR has a suitable instruction to represent a value
+   * that would otherwise need to be represented by a dedicated `OperandNode` value.
+   *
+   * Such operands do not create new `OperandNode` values, but are
+   * instead associated with the instruction returned by this predicate.
+   */
+  cached
+  Instruction getIRRepresentationOfOperand(Operand operand) {
+    operand = unique( | | result.getAUse())
+  }
+
   /**
    * Holds if the underlying IR has a suitable operand to represent a value
    * that would otherwise need to be represented by a dedicated `RawIndirectOperand` value.

cpp/ql/test/query-tests/Security/CWE/CWE-129/semmle/ImproperArrayIndexValidation/ImproperArrayIndexValidation.expected

@@ -1,23 +1,18 @@
 edges
 | test1.c:7:26:7:29 | argv | test1.c:9:9:9:9 | i |
 | test1.c:7:26:7:29 | argv | test1.c:11:9:11:9 | i |
-| test1.c:7:26:7:29 | argv | test1.c:12:9:12:9 | i |
 | test1.c:7:26:7:29 | argv | test1.c:13:9:13:9 | i |
 | test1.c:7:26:7:29 | argv indirection | test1.c:9:9:9:9 | i |
 | test1.c:7:26:7:29 | argv indirection | test1.c:9:9:9:9 | i |
 | test1.c:7:26:7:29 | argv indirection | test1.c:11:9:11:9 | i |
 | test1.c:7:26:7:29 | argv indirection | test1.c:11:9:11:9 | i |
-| test1.c:7:26:7:29 | argv indirection | test1.c:12:9:12:9 | i |
-| test1.c:7:26:7:29 | argv indirection | test1.c:12:9:12:9 | i |
 | test1.c:7:26:7:29 | argv indirection | test1.c:13:9:13:9 | i |
 | test1.c:7:26:7:29 | argv indirection | test1.c:13:9:13:9 | i |
 | test1.c:9:9:9:9 | i | test1.c:16:16:16:16 | i |
 | test1.c:11:9:11:9 | i | test1.c:32:16:32:16 | i |
-| test1.c:12:9:12:9 | i | test1.c:40:16:40:16 | i |
 | test1.c:13:9:13:9 | i | test1.c:48:16:48:16 | i |
 | test1.c:16:16:16:16 | i | test1.c:18:16:18:16 | i |
 | test1.c:32:16:32:16 | i | test1.c:33:11:33:11 | i |
-| test1.c:40:16:40:16 | i | test1.c:41:11:41:11 | i |
 | test1.c:48:16:48:16 | i | test1.c:53:15:53:15 | j |
 nodes
 | test1.c:7:26:7:29 | argv | semmle.label | argv |
@@ -25,14 +20,11 @@ nodes
 | test1.c:7:26:7:29 | argv indirection | semmle.label | argv indirection |
 | test1.c:9:9:9:9 | i | semmle.label | i |
 | test1.c:11:9:11:9 | i | semmle.label | i |
-| test1.c:12:9:12:9 | i | semmle.label | i |
 | test1.c:13:9:13:9 | i | semmle.label | i |
 | test1.c:16:16:16:16 | i | semmle.label | i |
 | test1.c:18:16:18:16 | i | semmle.label | i |
 | test1.c:32:16:32:16 | i | semmle.label | i |
 | test1.c:33:11:33:11 | i | semmle.label | i |
-| test1.c:40:16:40:16 | i | semmle.label | i |
-| test1.c:41:11:41:11 | i | semmle.label | i |
 | test1.c:48:16:48:16 | i | semmle.label | i |
 | test1.c:53:15:53:15 | j | semmle.label | j |
 subpaths
@@ -43,9 +35,6 @@ subpaths
 | test1.c:33:11:33:11 | i | test1.c:7:26:7:29 | argv | test1.c:33:11:33:11 | i | An array indexing expression depends on $@ that might be outside the bounds of the array. | test1.c:7:26:7:29 | argv | a command-line argument |
 | test1.c:33:11:33:11 | i | test1.c:7:26:7:29 | argv indirection | test1.c:33:11:33:11 | i | An array indexing expression depends on $@ that might be outside the bounds of the array. | test1.c:7:26:7:29 | argv indirection | a command-line argument |
 | test1.c:33:11:33:11 | i | test1.c:7:26:7:29 | argv indirection | test1.c:33:11:33:11 | i | An array indexing expression depends on $@ that might be outside the bounds of the array. | test1.c:7:26:7:29 | argv indirection | a command-line argument |
-| test1.c:41:11:41:11 | i | test1.c:7:26:7:29 | argv | test1.c:41:11:41:11 | i | An array indexing expression depends on $@ that might be outside the bounds of the array. | test1.c:7:26:7:29 | argv | a command-line argument |
-| test1.c:41:11:41:11 | i | test1.c:7:26:7:29 | argv indirection | test1.c:41:11:41:11 | i | An array indexing expression depends on $@ that might be outside the bounds of the array. | test1.c:7:26:7:29 | argv indirection | a command-line argument |
-| test1.c:41:11:41:11 | i | test1.c:7:26:7:29 | argv indirection | test1.c:41:11:41:11 | i | An array indexing expression depends on $@ that might be outside the bounds of the array. | test1.c:7:26:7:29 | argv indirection | a command-line argument |
 | test1.c:53:15:53:15 | j | test1.c:7:26:7:29 | argv | test1.c:53:15:53:15 | j | An array indexing expression depends on $@ that might be outside the bounds of the array. | test1.c:7:26:7:29 | argv | a command-line argument |
 | test1.c:53:15:53:15 | j | test1.c:7:26:7:29 | argv indirection | test1.c:53:15:53:15 | j | An array indexing expression depends on $@ that might be outside the bounds of the array. | test1.c:7:26:7:29 | argv indirection | a command-line argument |
 | test1.c:53:15:53:15 | j | test1.c:7:26:7:29 | argv indirection | test1.c:53:15:53:15 | j | An array indexing expression depends on $@ that might be outside the bounds of the array. | test1.c:7:26:7:29 | argv indirection | a command-line argument |

cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/globalVars/UncontrolledFormatStringThroughGlobalVar.expected

@@ -2,7 +2,6 @@ edges
 | globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy |
 | globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy |
 | globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy |
-| globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy |
 | globalVars.c:8:7:8:10 | copy | globalVars.c:30:15:30:18 | copy |
 | globalVars.c:8:7:8:10 | copy | globalVars.c:30:15:30:18 | copy |
 | globalVars.c:8:7:8:10 | copy | globalVars.c:30:15:30:18 | copy |
@@ -12,7 +11,6 @@ edges
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 |
-| globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:41:15:41:19 | copy2 |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:41:15:41:19 | copy2 |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:41:15:41:19 | copy2 |
@@ -20,7 +18,6 @@ edges
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 |
-| globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 |
 | globalVars.c:11:22:11:25 | argv | globalVars.c:8:7:8:10 | copy |
 | globalVars.c:11:22:11:25 | argv | globalVars.c:12:2:12:15 | ... = ... |
 | globalVars.c:12:2:12:15 | ... = ... | globalVars.c:8:7:8:10 | copy |
@@ -29,26 +26,12 @@ edges
 | globalVars.c:16:2:16:12 | ... = ... | globalVars.c:9:7:9:11 | copy2 |
 | globalVars.c:24:11:24:14 | argv | globalVars.c:11:22:11:25 | argv |
 | globalVars.c:24:11:24:14 | argv | globalVars.c:11:22:11:25 | argv |
-| globalVars.c:27:9:27:12 | copy | globalVars.c:27:9:27:12 | copy |
-| globalVars.c:27:9:27:12 | copy | globalVars.c:27:9:27:12 | copy |
-| globalVars.c:27:9:27:12 | copy | globalVars.c:27:9:27:12 | copy |
-| globalVars.c:27:9:27:12 | copy | globalVars.c:30:15:30:18 | copy |
-| globalVars.c:27:9:27:12 | copy | globalVars.c:30:15:30:18 | copy |
-| globalVars.c:27:9:27:12 | copy | globalVars.c:35:11:35:14 | copy |
 | globalVars.c:30:15:30:18 | copy | globalVars.c:30:15:30:18 | copy |
 | globalVars.c:30:15:30:18 | copy | globalVars.c:30:15:30:18 | copy |
 | globalVars.c:30:15:30:18 | copy | globalVars.c:35:11:35:14 | copy |
 | globalVars.c:33:15:33:18 | copy | globalVars.c:35:11:35:14 | copy |
 | globalVars.c:35:11:35:14 | copy | globalVars.c:15:21:15:23 | val |
 | globalVars.c:35:11:35:14 | copy | globalVars.c:35:11:35:14 | copy |
-| globalVars.c:38:9:38:13 | copy2 | globalVars.c:38:9:38:13 | copy2 |
-| globalVars.c:38:9:38:13 | copy2 | globalVars.c:38:9:38:13 | copy2 |
-| globalVars.c:38:9:38:13 | copy2 | globalVars.c:38:9:38:13 | copy2 |
-| globalVars.c:38:9:38:13 | copy2 | globalVars.c:41:15:41:19 | copy2 |
-| globalVars.c:38:9:38:13 | copy2 | globalVars.c:41:15:41:19 | copy2 |
-| globalVars.c:38:9:38:13 | copy2 | globalVars.c:50:9:50:13 | copy2 |
-| globalVars.c:38:9:38:13 | copy2 | globalVars.c:50:9:50:13 | copy2 |
-| globalVars.c:38:9:38:13 | copy2 | globalVars.c:50:9:50:13 | copy2 |
 | globalVars.c:41:15:41:19 | copy2 | globalVars.c:41:15:41:19 | copy2 |
 | globalVars.c:41:15:41:19 | copy2 | globalVars.c:41:15:41:19 | copy2 |
 | globalVars.c:41:15:41:19 | copy2 | globalVars.c:50:9:50:13 | copy2 |
@@ -57,9 +40,6 @@ edges
 | globalVars.c:44:15:44:19 | copy2 | globalVars.c:50:9:50:13 | copy2 |
 | globalVars.c:44:15:44:19 | copy2 | globalVars.c:50:9:50:13 | copy2 |
 | globalVars.c:44:15:44:19 | copy2 | globalVars.c:50:9:50:13 | copy2 |
-| globalVars.c:50:9:50:13 | copy2 | globalVars.c:50:9:50:13 | copy2 |
-| globalVars.c:50:9:50:13 | copy2 | globalVars.c:50:9:50:13 | copy2 |
-| globalVars.c:50:9:50:13 | copy2 | globalVars.c:50:9:50:13 | copy2 |
 subpaths
 nodes
 | globalVars.c:8:7:8:10 | copy | semmle.label | copy |
@@ -73,7 +53,6 @@ nodes
 | globalVars.c:27:9:27:12 | copy | semmle.label | copy |
 | globalVars.c:27:9:27:12 | copy | semmle.label | copy |
 | globalVars.c:27:9:27:12 | copy | semmle.label | copy |
-| globalVars.c:27:9:27:12 | copy | semmle.label | copy |
 | globalVars.c:30:15:30:18 | copy | semmle.label | copy |
 | globalVars.c:30:15:30:18 | copy | semmle.label | copy |
 | globalVars.c:30:15:30:18 | copy | semmle.label | copy |
@@ -83,7 +62,6 @@ nodes
 | globalVars.c:38:9:38:13 | copy2 | semmle.label | copy2 |
 | globalVars.c:38:9:38:13 | copy2 | semmle.label | copy2 |
 | globalVars.c:38:9:38:13 | copy2 | semmle.label | copy2 |
-| globalVars.c:38:9:38:13 | copy2 | semmle.label | copy2 |
 | globalVars.c:41:15:41:19 | copy2 | semmle.label | copy2 |
 | globalVars.c:41:15:41:19 | copy2 | semmle.label | copy2 |
 | globalVars.c:41:15:41:19 | copy2 | semmle.label | copy2 |
@@ -91,7 +69,6 @@ nodes
 | globalVars.c:50:9:50:13 | copy2 | semmle.label | copy2 |
 | globalVars.c:50:9:50:13 | copy2 | semmle.label | copy2 |
 | globalVars.c:50:9:50:13 | copy2 | semmle.label | copy2 |
-| globalVars.c:50:9:50:13 | copy2 | semmle.label | copy2 |
 #select
 | globalVars.c:27:9:27:12 | copy | globalVars.c:24:11:24:14 | argv | globalVars.c:27:9:27:12 | copy | The value of this argument may come from $@ and is being used as a formatting argument to printf(format). | globalVars.c:24:11:24:14 | argv | argv |
 | globalVars.c:30:15:30:18 | copy | globalVars.c:24:11:24:14 | argv | globalVars.c:30:15:30:18 | copy | The value of this argument may come from $@ and is being used as a formatting argument to printWrapper(str), which calls printf(format). | globalVars.c:24:11:24:14 | argv | argv |

cpp/ql/test/query-tests/Security/CWE/CWE-326/InsufficientKeySize.expected

@@ -1,13 +1,7 @@
 edges
-| test.cpp:34:45:34:48 | 1024 | test.cpp:34:45:34:48 | 1024 |
-| test.cpp:35:49:35:52 | 1024 | test.cpp:35:49:35:52 | 1024 |
-| test.cpp:37:43:37:46 | 1024 | test.cpp:37:43:37:46 | 1024 |
 nodes
 | test.cpp:34:45:34:48 | 1024 | semmle.label | 1024 |
-| test.cpp:34:45:34:48 | 1024 | semmle.label | 1024 |
 | test.cpp:35:49:35:52 | 1024 | semmle.label | 1024 |
-| test.cpp:35:49:35:52 | 1024 | semmle.label | 1024 |
-| test.cpp:37:43:37:46 | 1024 | semmle.label | 1024 |
 | test.cpp:37:43:37:46 | 1024 | semmle.label | 1024 |
 subpaths
 #select