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

Author: MathiasVP

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

@@ -645,6 +645,24 @@ class GlobalLikeVariable extends Variable {
   }
 }
 
+/**
+ * Returns the smallest indirection for the type `t`.
+ *
+ * For most types this is `1`, but for `ArrayType`s (which are allocated on
+ * the stack) this is `0`
+ */
+int getMinIndirectionsForType(Type t) {
+  if t.getUnspecifiedType() instanceof Cpp::ArrayType then result = 0 else result = 1
+}
+
+private int getMinIndirectionForGlobalUse(Ssa::GlobalUse use) {
+  result = getMinIndirectionsForType(use.getUnspecifiedType())
+}
+
+private int getMinIndirectionForGlobalDef(Ssa::GlobalDef def) {
+  result = getMinIndirectionsForType(def.getUnspecifiedType())
+}
+
 /**
  * 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 +674,7 @@ predicate jumpStep(Node n1, Node n2) {
       v = globalUse.getVariable() and
       n1.(FinalGlobalValue).getGlobalUse() = globalUse
     |
-      globalUse.getIndirection() = 1 and
+      globalUse.getIndirection() = getMinIndirectionForGlobalUse(globalUse) and
       v = n2.asVariable()
       or
       v = n2.asIndirectVariable(globalUse.getIndirection())
@@ -666,7 +684,7 @@ predicate jumpStep(Node n1, Node n2) {
       v = globalDef.getVariable() and
       n2.(InitialGlobalValue).getGlobalDef() = globalDef
     |
-      globalDef.getIndirection() = 1 and
+      globalDef.getIndirection() = getMinIndirectionForGlobalDef(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,8 @@ cached
 private newtype TIRDataFlowNode =
   TNode0(Node0Impl node) { DataFlowImplCommon::forceCachingInSameStage() } or
   TVariableNode(Variable var, int indirectionIndex) {
-    indirectionIndex = [1 .. Ssa::getMaxIndirectionsForType(var.getUnspecifiedType())]
+    indirectionIndex =
+      [getMinIndirectionsForType(var.getUnspecifiedType()) .. Ssa::getMaxIndirectionsForType(var.getUnspecifiedType())]
   } or
   TPostFieldUpdateNode(FieldAddress operand, int indirectionIndex) {
     indirectionIndex =
@@ -346,15 +347,17 @@ 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() {
+    this = TVariableNode(result, getMinIndirectionsForType(result.getUnspecifiedType()))
+  }
 
   /**
    * 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
+    indirectionIndex > getMinIndirectionsForType(result.getUnspecifiedType()) and
     this = TVariableNode(result, indirectionIndex)
   }
 

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

@@ -872,7 +872,7 @@ private module Cached {
       upper = countIndirectionsForCppType(type) and
       ind = ind0 + [lower .. upper] and
       indirectionIndex = ind - (ind0 + lower) and
-      (if type.hasType(any(Cpp::ArrayType arrayType), true) then lower = 0 else lower = 1)
+      lower = getMinIndirectionsForType(any(Type t | type.hasUnspecifiedType(t, _)))
     )
   }
 

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