Merge pull request #13965 from MathiasVP/better-certain-writes-and-invalid-ptr-deref-prep

C++: Remove more dataflow FPs after frontend upgrade

Author: MathiasVP

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

@@ -1520,6 +1520,25 @@ private module Cached {
     )
   }
 
+  /**
+   * Holds if `operand.getDef() = instr`, but there exists a `StoreInstruction` that
+   * writes to an address that is equivalent to the value computed by `instr` in
+   * between `instr` and `operand`, and therefore there should not be flow from `*instr`
+   * to `*operand`.
+   */
+  pragma[nomagic]
+  private predicate isStoredToBetween(Instruction instr, Operand operand) {
+    simpleOperandLocalFlowStep(pragma[only_bind_into](instr), pragma[only_bind_into](operand)) and
+    exists(StoreInstruction store, IRBlock block, int storeIndex, int instrIndex, int operandIndex |
+      store.getDestinationAddress() = instr and
+      block.getInstruction(storeIndex) = store and
+      block.getInstruction(instrIndex) = instr and
+      block.getInstruction(operandIndex) = operand.getUse() and
+      instrIndex < storeIndex and
+      storeIndex < operandIndex
+    )
+  }
+
   private predicate indirectionInstructionFlow(
     RawIndirectInstruction nodeFrom, IndirectOperand nodeTo
   ) {
@@ -1529,7 +1548,8 @@ private module Cached {
       simpleOperandLocalFlowStep(pragma[only_bind_into](instr), pragma[only_bind_into](operand))
     |
       hasOperandAndIndex(nodeTo, operand, pragma[only_bind_into](indirectionIndex)) and
-      hasInstructionAndIndex(nodeFrom, instr, pragma[only_bind_into](indirectionIndex))
+      hasInstructionAndIndex(nodeFrom, instr, pragma[only_bind_into](indirectionIndex)) and
+      not isStoredToBetween(instr, operand)
     )
   }
 

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

@@ -6,6 +6,7 @@ private import DataFlowImplCommon as DataFlowImplCommon
 private import DataFlowUtil
 private import semmle.code.cpp.models.interfaces.PointerWrapper
 private import DataFlowPrivate
+private import semmle.code.cpp.ir.ValueNumbering
 
 /**
  * Holds if `operand` is an operand that is not used by the dataflow library.
@@ -864,7 +865,7 @@ private module Cached {
    * to a specific address.
    */
   private predicate isCertainAddress(Operand operand) {
-    operand.getDef() instanceof VariableAddressInstruction
+    valueNumberOfOperand(operand).getAnInstruction() instanceof VariableAddressInstruction
     or
     operand.getType() instanceof Cpp::ReferenceType
   }

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

@@ -732,7 +732,7 @@ void test_does_not_write_source_to_dereference()
 {
   int x;
   does_not_write_source_to_dereference(&x);
-  sink(x); // $ ast,ir=733:7 SPURIOUS: ast,ir=726:11
+  sink(x); // $ ast=733:7 ir SPURIOUS: ast=726:11
 }
 
 void sometimes_calls_sink_eq(int x, int n) {

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

@@ -134,7 +134,7 @@ void pointer_test() {
 	sink(*p3); // $ ast,ir
 
 	*p3 = 0;
-	sink(*p3); // $ SPURIOUS: ast,ir
+	sink(*p3); // $ SPURIOUS: ast
 }
 
 // --- return values ---