C++: Fix global variable flow for array types.

Author: MathiasVP

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

@@ -645,6 +645,26 @@ class GlobalLikeVariable extends Variable {
   }
 }
 
+/**
+ * Returns the smallest indirection for `use`.
+ *
+ * For most types this is `1`, but for `ArrayType`s (which are allocated on
+ * the stack) this is `0`
+ */
+private int getLowerForGlobalUse(Ssa::GlobalUse use) {
+  if use.getUnspecifiedType() instanceof Cpp::ArrayType then result = 0 else result = 1
+}
+
+/**
+ * Returns the smallest indirection for `def`.
+ *
+ * For most types this is `1`, but for `ArrayType`s (which are allocated on
+ * the stack) this is `0`
+ */
+private int getLowerForGlobalDef(Ssa::GlobalDef def) {
+  if def.getUnspecifiedType() instanceof Cpp::ArrayType then result = 0 else result = 1
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` in a way that loses the
  * calling context. For example, this would happen with flow through a
@@ -656,7 +676,7 @@ predicate jumpStep(Node n1, Node n2) {
       v = globalUse.getVariable() and
       n1.(FinalGlobalValue).getGlobalUse() = globalUse
     |
-      globalUse.getIndirection() = 1 and
+      globalUse.getIndirection() = getLowerForGlobalUse(globalUse) and
       v = n2.asVariable()
       or
       v = n2.asIndirectVariable(globalUse.getIndirection())
@@ -666,7 +686,7 @@ predicate jumpStep(Node n1, Node n2) {
       v = globalDef.getVariable() and
       n2.(InitialGlobalValue).getGlobalDef() = globalDef
     |
-      globalDef.getIndirection() = 1 and
+      globalDef.getIndirection() = getLowerForGlobalDef(globalDef) and
       v = n1.asVariable()
       or
       v = n1.asIndirectVariable(globalDef.getIndirection())

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

@@ -34,7 +34,11 @@ cached
 private newtype TIRDataFlowNode =
   TNode0(Node0Impl node) { DataFlowImplCommon::forceCachingInSameStage() } or
   TVariableNode(Variable var, int indirectionIndex) {
-    indirectionIndex = [1 .. Ssa::getMaxIndirectionsForType(var.getUnspecifiedType())]
+    exists(int lower |
+      if var.getUnspecifiedType() instanceof ArrayType then lower = 0 else lower = 1
+    |
+      indirectionIndex = [lower .. Ssa::getMaxIndirectionsForType(var.getUnspecifiedType())]
+    )
   } or
   TPostFieldUpdateNode(FieldAddress operand, int indirectionIndex) {
     indirectionIndex =
@@ -346,15 +350,23 @@ class Node extends TIRDataFlowNode {
    * Gets the variable corresponding to this node, if any. This can be used for
    * modeling flow in and out of global variables.
    */
-  Variable asVariable() { this = TVariableNode(result, 1) }
+  Variable asVariable() {
+    if result.getUnspecifiedType() instanceof ArrayType
+    then this = TVariableNode(result, 0)
+    else this = TVariableNode(result, 1)
+  }
 
   /**
    * Gets the `indirectionIndex`'th indirection of this node's underlying variable, if any.
    *
    * This can be used for modeling flow in and out of global variables.
    */
   Variable asIndirectVariable(int indirectionIndex) {
-    indirectionIndex > 1 and
+    (
+      if result.getUnspecifiedType() instanceof ArrayType
+      then indirectionIndex > 0
+      else indirectionIndex > 1
+    ) and
     this = TVariableNode(result, indirectionIndex)
   }
 

cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected

@@ -286,9 +286,11 @@ irFlow
 | test.cpp:853:55:853:62 | call to source | test.cpp:854:10:854:36 | * ... |
 | test.cpp:860:54:860:59 | call to source | test.cpp:861:10:861:37 | static_local_pointer_dynamic |
 | test.cpp:872:46:872:51 | call to source | test.cpp:875:10:875:31 | global_pointer_dynamic |
+| test.cpp:880:64:880:83 | indirect_source(1) indirection | test.cpp:883:10:883:45 | static_local_array_static_indirect_1 |
 | test.cpp:881:64:881:83 | indirect_source(2) indirection | test.cpp:886:19:886:54 | static_local_array_static_indirect_2 indirection |
 | test.cpp:890:54:890:61 | source | test.cpp:893:10:893:36 | static_local_pointer_static |
 | test.cpp:891:65:891:84 | indirect_source(1) indirection | test.cpp:895:19:895:56 | static_local_pointer_static_indirect_1 indirection |
+| test.cpp:901:56:901:75 | indirect_source(1) indirection | test.cpp:907:10:907:39 | global_array_static_indirect_1 |
 | test.cpp:902:56:902:75 | indirect_source(2) indirection | test.cpp:911:19:911:48 | global_array_static_indirect_2 indirection |
 | test.cpp:914:46:914:53 | source | test.cpp:919:10:919:30 | global_pointer_static |
 | test.cpp:915:57:915:76 | indirect_source(1) indirection | test.cpp:921:19:921:50 | global_pointer_static_indirect_1 indirection |

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

@@ -880,7 +880,7 @@ namespace GlobalArrays {
     static const char static_local_array_static_indirect_1[] = "indirect_source(1)";
     static const char static_local_array_static_indirect_2[] = "indirect_source(2)";
     sink(static_local_array_static); // clean
-    sink(static_local_array_static_indirect_1); // $ MISSING: ast,ir
+    sink(static_local_array_static_indirect_1); // $ ir MISSING: ast
     indirect_sink(static_local_array_static_indirect_1); // clean
     sink(static_local_array_static_indirect_2); // clean
     indirect_sink(static_local_array_static_indirect_2); // $ ir MISSING: ast
@@ -904,7 +904,7 @@ namespace GlobalArrays {
   void test7() {
     sink(global_array_static); // clean
     sink(*global_array_static); // clean
-    sink(global_array_static_indirect_1); // $ MISSING: ir,ast
+    sink(global_array_static_indirect_1); // $ ir MISSING: ast
     sink(*global_array_static_indirect_1); // clean
     indirect_sink(global_array_static); // clean
     indirect_sink(global_array_static_indirect_1); // clean