C++: Global variable flow without explicit SSA definitions

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

@@ -780,9 +780,7 @@ private predicate numberOfLoadsFromOperandRec(
  * Holds if `operandFrom` flows to `operandTo` using a sequence of conversion-like
  * operations and exactly `n` `LoadInstruction` operations.
  */
-private predicate numberOfLoadsFromOperand(
-  Operand operandFrom, Operand operandTo, int n, boolean certain
-) {
+predicate numberOfLoadsFromOperand(Operand operandFrom, Operand operandTo, int n, boolean certain) {
   numberOfLoadsFromOperandRec(operandFrom, operandTo, n, certain)
   or
   not Ssa::isDereference(_, operandFrom, _) and

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

@@ -146,13 +146,57 @@ private newtype TDefOrUseImpl =
     )
   }
 
+/**
+ * Holds if `fa` flows to a the address of a `StoreInstruction`, or flows to
+ * the qualifier of another field address that transitively flows to a `StoreInstruction`.
+ */
+private predicate fieldFlowsToStore(FieldAddress fa) {
+  numberOfLoadsFromOperand(fa, any(StoreInstruction store).getDestinationAddressOperand(), _, _)
+  or
+  exists(FieldAddress mid |
+    numberOfLoadsFromOperand(fa, mid.getObjectAddressOperand(), _, _) and
+    fieldFlowsToStore(mid)
+  )
+}
+
+private predicate isGlobalUseIndirectDefCand(GlobalLikeVariable v, IRFunction f, CppType type) {
+  exists(VariableAddressInstruction vai, Operand op |
+    vai.getEnclosingIRFunction() = f and
+    vai.getAstVariable() = v and
+    numberOfLoadsFromOperand(vai.getAUse(), op, _, _) and
+    type = getResultLanguageType(vai)
+  |
+    // Either this operand is used as the qualifier of a field that flows to
+    // a `StoreInstruction`
+    op = any(FieldAddress fa | fieldFlowsToStore(fa)).getObjectAddressOperand()
+    or
+    // Or the operand is potentially modified by a function call
+    isModifiableByCall(op, _)
+  )
+}
+
 private predicate isGlobalUse(
   GlobalLikeVariable v, IRFunction f, int indirection, int indirectionIndex
 ) {
-  exists(VariableAddressInstruction vai |
-    vai.getEnclosingIRFunction() = f and
-    vai.getAstVariable() = v and
-    isDef(_, _, _, vai, indirection, indirectionIndex)
+  // Generate a "global use" at the end of the function body if there's a
+  // direct definition somewhere in the body of the function
+  indirection =
+    min(int cand, VariableAddressInstruction vai |
+      vai.getEnclosingIRFunction() = f and
+      vai.getAstVariable() = v and
+      isDef(_, _, _, vai, cand, indirectionIndex)
+    |
+      cand
+    )
+  or
+  // Generate a "global use" at the end of the function body if the
+  // global variable is used for field-flow, or is passed as an argument
+  // to a function that may change its value.
+  exists(CppType type, int upper |
+    isGlobalUseIndirectDefCand(v, f, type) and
+    upper = countIndirectionsForCppType(type) and
+    indirection = [1 .. upper] and
+    indirectionIndex = indirection - 1
   )
 }
 

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

@@ -32,6 +32,10 @@ argHasPostUpdate
 | test.cpp:67:29:67:35 | source1 | ArgumentNode is missing PostUpdateNode. |
 | test.cpp:813:19:813:35 | * ... | ArgumentNode is missing PostUpdateNode. |
 | test.cpp:848:23:848:25 | rpx | ArgumentNode is missing PostUpdateNode. |
+| test.cpp:972:19:972:37 | * ... | ArgumentNode is missing PostUpdateNode. |
+| test.cpp:973:10:973:28 | * ... | ArgumentNode is missing PostUpdateNode. |
+| test.cpp:990:19:990:37 | * ... | ArgumentNode is missing PostUpdateNode. |
+| test.cpp:991:10:991:28 | * ... | ArgumentNode is missing PostUpdateNode. |
 postWithInFlow
 | BarrierGuard.cpp:49:6:49:6 | x [post update] | PostUpdateNode should not be the target of local flow. |
 | BarrierGuard.cpp:60:7:60:7 | x [post update] | PostUpdateNode should not be the target of local flow. |
@@ -159,6 +163,10 @@ postWithInFlow
 | test.cpp:808:5:808:21 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
 | test.cpp:808:6:808:21 | global_indirect1 [inner post update] | PostUpdateNode should not be the target of local flow. |
 | test.cpp:832:5:832:17 | global_direct [post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:931:5:931:6 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:931:6:931:6 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:961:5:961:6 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:961:6:961:6 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
 viableImplInCallContextTooLarge
 uniqueParameterNodeAtPosition
 uniqueParameterNodePosition

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

@@ -123,6 +123,11 @@ astFlow
 | test.cpp:842:11:842:16 | call to source | test.cpp:844:8:844:8 | y |
 | test.cpp:846:13:846:27 | call to indirect_source | test.cpp:848:23:848:25 | rpx |
 | test.cpp:860:54:860:59 | call to source | test.cpp:861:10:861:37 | static_local_pointer_dynamic |
+| test.cpp:931:10:931:15 | call to source | test.cpp:936:19:936:32 | global_int_ptr |
+| test.cpp:931:10:931:15 | call to source | test.cpp:1000:19:1000:34 | global_int_array |
+| test.cpp:931:10:931:15 | call to source | test.cpp:1005:10:1005:26 | * ... |
+| test.cpp:961:10:961:24 | call to indirect_source | test.cpp:966:19:966:36 | global_int_ptr_ptr |
+| test.cpp:961:10:961:24 | call to indirect_source | test.cpp:967:10:967:27 | global_int_ptr_ptr |
 | true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x |
 | true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x |
 | true_upon_entry.cpp:33:11:33:16 | call to source | true_upon_entry.cpp:39:8:39:8 | x |
@@ -300,6 +305,20 @@ irFlow
 | test.cpp:902:56:902:75 | *indirect_source(2) | test.cpp:911:19:911:48 | *global_array_static_indirect_2 |
 | 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) | test.cpp:921:19:921:50 | *global_pointer_static_indirect_1 |
+| test.cpp:931:10:931:15 | call to source | test.cpp:936:19:936:32 | *global_int_ptr |
+| test.cpp:931:10:931:15 | call to source | test.cpp:941:10:941:24 | * ... |
+| test.cpp:931:10:931:15 | call to source | test.cpp:950:19:950:32 | *global_int_ptr |
+| test.cpp:931:10:931:15 | call to source | test.cpp:955:10:955:24 | * ... |
+| test.cpp:931:10:931:15 | call to source | test.cpp:1000:19:1000:34 | *global_int_array |
+| test.cpp:931:10:931:15 | call to source | test.cpp:1005:10:1005:26 | * ... |
+| test.cpp:931:10:931:15 | call to source | test.cpp:1014:19:1014:34 | *global_int_array |
+| test.cpp:931:10:931:15 | call to source | test.cpp:1019:10:1019:26 | * ... |
+| test.cpp:961:10:961:24 | *call to indirect_source | test.cpp:966:19:966:36 | **global_int_ptr_ptr |
+| test.cpp:961:10:961:24 | *call to indirect_source | test.cpp:972:19:972:37 | ** ... |
+| test.cpp:961:10:961:24 | *call to indirect_source | test.cpp:975:10:975:29 | * ... |
+| test.cpp:961:10:961:24 | *call to indirect_source | test.cpp:984:19:984:36 | **global_int_ptr_ptr |
+| test.cpp:961:10:961:24 | *call to indirect_source | test.cpp:990:19:990:37 | ** ... |
+| test.cpp:961:10:961:24 | *call to indirect_source | test.cpp:993:10:993:29 | * ... |
 | true_upon_entry.cpp:9:11:9:16 | call to source | true_upon_entry.cpp:13:8:13:8 | x |
 | true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x |
 | true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x |

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

@@ -922,4 +922,100 @@ namespace GlobalArrays {
     sink(global_pointer_static_indirect_2); // clean: global_pointer_static_indirect_2 does not have 2 indirections
     indirect_sink(global_pointer_static_indirect_2); // clean: global_pointer_static_indirect_2 does not have 2 indirections
   }
+}
+
+namespace globals_without_explicit_def {
+  int* global_int_ptr;
+
+  void set(int* p) { // $ ast-def=p ir-def=*p
+    *p = source();
+  }
+
+  void test1() {
+    set(global_int_ptr);
+    indirect_sink(global_int_ptr); // $ ir,ast
+  }
+
+  void test2() {
+    set(global_int_ptr);
+    sink(*global_int_ptr); // $ ir MISSING: ast
+  }
+
+  void calls_set() {
+    set(global_int_ptr);
+  }
+
+  void test3() {
+    calls_set();
+    indirect_sink(global_int_ptr); // $ ir MISSING: ast
+  }
+
+  void test4() {
+    calls_set();
+    sink(*global_int_ptr); // $ ir MISSING: ast
+  }
+
+  int** global_int_ptr_ptr;
+
+  void set_indirect(int** p) { // $ ast-def=p ir-def=*p ir-def=**p
+    *p = indirect_source();
+  }
+
+  void test5() {
+    set_indirect(global_int_ptr_ptr);
+    indirect_sink(global_int_ptr_ptr); // $ ir,ast
+    sink(global_int_ptr_ptr); // $ SPURIOUS: ast
+  }
+
+  void test6() {
+    set_indirect(global_int_ptr_ptr);
+    indirect_sink(*global_int_ptr_ptr); // $ ir MISSING: ast
+    sink(*global_int_ptr_ptr);
+    indirect_sink(**global_int_ptr_ptr);
+    sink(**global_int_ptr_ptr); // $ ir
+  }
+
+  void calls_set_indirect() {
+    set_indirect(global_int_ptr_ptr);
+  }
+
+  void test7() {
+    calls_set_indirect();
+    indirect_sink(global_int_ptr_ptr); // $ ir MISSING: ast
+    sink(global_int_ptr_ptr); // $ MISSING: ast
+  }
+
+  void test8() {
+    calls_set_indirect();
+    indirect_sink(*global_int_ptr_ptr); // $ ir MISSING: ast
+    sink(*global_int_ptr_ptr);
+    indirect_sink(**global_int_ptr_ptr);
+    sink(**global_int_ptr_ptr); // $ ir MISSING: ast
+  }
+
+  int global_int_array[10];
+
+  void test9() {
+    set(global_int_array);
+    indirect_sink(global_int_array); // $ ir,ast
+  }
+
+  void test10() {
+    set(global_int_array);
+    sink(*global_int_array); // $ ir,ast
+  }
+
+  void calls_set_array() {
+    set(global_int_array);
+  }
+
+  void test11() {
+    calls_set_array();
+    indirect_sink(global_int_array); // $ ir MISSING: ast
+  }
+
+  void test12() {
+    calls_set_array();
+    sink(*global_int_array); // $ ir MISSING: ast
+  }
 }

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

@@ -204,4 +204,64 @@ void deep_member_field_arrow(S2 *ps2) {
 void deep_member_field_arrow_different_fields(S2 *ps2) {
   taint_a_ptr(&ps2->s.m1);
   sink(ps2->s.m2);
+}
+
+namespace GlobalFieldFlow {
+  S global_s;
+  S2 global_s2;
+
+  void set_field() {
+    global_s.m1 = user_input();
+  }
+
+  void read_field() {
+    sink(global_s.m1); // $ ir MISSING: ast
+  }
+
+  void set_nested_field() {
+    global_s2.s.m1 = user_input();
+  }
+
+  void read_nested_field() {
+    sink(global_s2.s.m1); // $ ir MISSING: ast
+  }
+
+  S* global_s_ptr;
+  S2* global_s2_ptr;
+
+  void set_field_ptr() {
+    global_s_ptr->m1 = user_input();
+  }
+
+  void read_field_ptr() {
+    sink(global_s_ptr->m1); // $ ir MISSING: ast
+  }
+
+  void set_nested_field_ptr() {
+    global_s2_ptr->s.m1 = user_input();
+  }
+
+  void read_nested_field_ptr() {
+    sink(global_s2_ptr->s.m1); // $ ir MISSING: ast
+  }
+
+  S_with_pointer global_s_with_pointer;
+
+  void set_field_indirect() {
+    *global_s_with_pointer.data = user_input();
+  }
+
+  void read_field_indirect() {
+    sink(*global_s_with_pointer.data); // $ ir MISSING: ast
+  }
+
+  S_with_array global_s_with_array;
+
+  void set_field_array() {
+    *global_s_with_array.data = user_input();
+  }
+
+  void read_field_array() {
+    sink(*global_s_with_array.data); // $ ir MISSING: ast
+  }
 }

cpp/ql/test/library-tests/dataflow/fields/dataflow-consistency.expected

@@ -95,6 +95,14 @@ postWithInFlow
 | aliasing.cpp:194:21:194:22 | m1 [inner post update] | PostUpdateNode should not be the target of local flow. |
 | aliasing.cpp:200:23:200:24 | m1 [inner post update] | PostUpdateNode should not be the target of local flow. |
 | aliasing.cpp:205:23:205:24 | m1 [inner post update] | PostUpdateNode should not be the target of local flow. |
+| aliasing.cpp:214:14:214:15 | m1 [post update] | PostUpdateNode should not be the target of local flow. |
+| aliasing.cpp:222:17:222:18 | m1 [post update] | PostUpdateNode should not be the target of local flow. |
+| aliasing.cpp:233:19:233:20 | m1 [post update] | PostUpdateNode should not be the target of local flow. |
+| aliasing.cpp:241:22:241:23 | m1 [post update] | PostUpdateNode should not be the target of local flow. |
+| aliasing.cpp:251:5:251:31 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
+| aliasing.cpp:251:28:251:31 | data [inner post update] | PostUpdateNode should not be the target of local flow. |
+| aliasing.cpp:261:5:261:29 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
+| aliasing.cpp:261:26:261:29 | data [inner post update] | PostUpdateNode should not be the target of local flow. |
 | arrays.cpp:6:3:6:5 | arr [inner post update] | PostUpdateNode should not be the target of local flow. |
 | arrays.cpp:6:3:6:8 | access to array [post update] | PostUpdateNode should not be the target of local flow. |
 | arrays.cpp:15:3:15:10 | * ... [post update] | PostUpdateNode should not be the target of local flow. |