Merge pull request #7578 from MathiasVP/store-dest-should-not-be-use

C++: Store destinations should not be uses for dataflow SSA

Author: Robert Marsh

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

@@ -170,10 +170,16 @@ private class ReturnParameterIndirection extends Use, TReturnParamIndirection {
 }
 
 private predicate isExplicitUse(Operand op) {
-  op.getDef() instanceof VariableAddressInstruction and
-  not exists(LoadInstruction load |
-    load.getSourceAddressOperand() = op and
-    load.getAUse().getUse() instanceof InitializeIndirectionInstruction
+  exists(VariableAddressInstruction vai | vai = op.getDef() |
+    // Don't include this operand as a use if it only exists to initialize the
+    // indirection of a parameter.
+    not exists(LoadInstruction load |
+      load.getSourceAddressOperand() = op and
+      load.getAUse().getUse() instanceof InitializeIndirectionInstruction
+    ) and
+    // Don't include this operand as a use if the only use of the address is for a write
+    // that definitely overrides a variable.
+    not (explicitWrite(true, _, vai) and exists(unique( | | vai.getAUse())))
   )
 }
 

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

@@ -570,6 +570,9 @@ postWithInFlow
 | test.cpp:481:24:481:30 | FieldAddress [post update] | PostUpdateNode should not be the target of local flow. |
 | test.cpp:481:24:481:30 | content [post update] | PostUpdateNode should not be the target of local flow. |
 | test.cpp:482:8:482:16 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:489:7:489:7 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:491:5:491:5 | x [post update] | PostUpdateNode should not be the target of local flow. |
+| test.cpp:494:5:494:5 | x [post update] | PostUpdateNode should not be the target of local flow. |
 | true_upon_entry.cpp:9:7:9:7 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
 | true_upon_entry.cpp:10:12:10:12 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
 | true_upon_entry.cpp:10:27:10:27 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |

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

@@ -481,4 +481,16 @@ void local_field_flow_def_by_ref_steps_with_local_flow(MyStruct * s) {
   writes_to_content(s->content);
   int* p_content = s->content;
   sink(*p_content);
-}
+}
+
+bool unknown();
+
+void regression_with_phi_flow(int clean1) {
+  int x = 0;
+  while (unknown()) {
+    x = clean1;
+    if (unknown()) { }
+    sink(x); // clean
+    x = source();
+  }
+}