C++: Fix ValueNumbering for CopyInstruction

Querying for overlap type wasn't possible when this library was first
written. This change fixes FPs in `RedundantNullCheckSimple.ql` on
Wireshark and other real-world projects.

Author: jbj

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/gvn/ValueNumbering.qll

@@ -105,19 +105,10 @@ class ValueNumber extends TValueNumber {
  * definition because it accesses the exact same memory.
  * The use of `p.x` on line 3 is linked to the definition of `p` on line 1 as well, but is not
  * congruent to that definition because `p.x` accesses only a subset of the memory defined by `p`.
- *
- * This concept should probably be exposed in the public IR API.
  */
 private class CongruentCopyInstruction extends CopyInstruction {
   CongruentCopyInstruction() {
-    exists(Instruction def |
-      def = this.getSourceValue() and
-      (
-        def.getResultMemoryAccess() instanceof IndirectMemoryAccess or
-        def.getResultMemoryAccess() instanceof PhiMemoryAccess or
-        not def.hasMemoryResult()
-      )
-    )
+    this.getSourceValueOperand().getDefinitionOverlap() instanceof MustExactlyOverlap
   }
 }
 

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/gvn/ValueNumbering.qll

@@ -105,19 +105,10 @@ class ValueNumber extends TValueNumber {
  * definition because it accesses the exact same memory.
  * The use of `p.x` on line 3 is linked to the definition of `p` on line 1 as well, but is not
  * congruent to that definition because `p.x` accesses only a subset of the memory defined by `p`.
- *
- * This concept should probably be exposed in the public IR API.
  */
 private class CongruentCopyInstruction extends CopyInstruction {
   CongruentCopyInstruction() {
-    exists(Instruction def |
-      def = this.getSourceValue() and
-      (
-        def.getResultMemoryAccess() instanceof IndirectMemoryAccess or
-        def.getResultMemoryAccess() instanceof PhiMemoryAccess or
-        not def.hasMemoryResult()
-      )
-    )
+    this.getSourceValueOperand().getDefinitionOverlap() instanceof MustExactlyOverlap
   }
 }
 

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/gvn/ValueNumbering.qll

@@ -105,19 +105,10 @@ class ValueNumber extends TValueNumber {
  * definition because it accesses the exact same memory.
  * The use of `p.x` on line 3 is linked to the definition of `p` on line 1 as well, but is not
  * congruent to that definition because `p.x` accesses only a subset of the memory defined by `p`.
- *
- * This concept should probably be exposed in the public IR API.
  */
 private class CongruentCopyInstruction extends CopyInstruction {
   CongruentCopyInstruction() {
-    exists(Instruction def |
-      def = this.getSourceValue() and
-      (
-        def.getResultMemoryAccess() instanceof IndirectMemoryAccess or
-        def.getResultMemoryAccess() instanceof PhiMemoryAccess or
-        not def.hasMemoryResult()
-      )
-    )
+    this.getSourceValueOperand().getDefinitionOverlap() instanceof MustExactlyOverlap
   }
 }
 

cpp/ql/test/query-tests/Likely Bugs/RedundantNullCheckSimple/RedundantNullCheckSimple.cpp

@@ -105,7 +105,7 @@ struct S {
   long **pplong;
 
   void test_phi() {
-    while (*pplong != nullptr) { // GOOD [FALSE POSITIVE]
+    while (*pplong != nullptr) { // GOOD
       pplong++;
     }
   }

cpp/ql/test/query-tests/Likely Bugs/RedundantNullCheckSimple/RedundantNullCheckSimple.expected

@@ -3,4 +3,3 @@
 | RedundantNullCheckSimple.cpp:48:12:48:12 | Load: p | This null check is redundant because the value is $@ in any case | RedundantNullCheckSimple.cpp:51:10:51:11 | Load: * ... | dereferenced here |
 | RedundantNullCheckSimple.cpp:79:7:79:9 | Load: * ... | This null check is redundant because the value is $@ in any case | RedundantNullCheckSimple.cpp:78:7:78:10 | Load: * ... | dereferenced here |
 | RedundantNullCheckSimple.cpp:93:13:93:13 | Load: p | This null check is redundant because the value is $@ in any case | RedundantNullCheckSimple.cpp:92:13:92:18 | Load: * ... | dereferenced here |
-| RedundantNullCheckSimple.cpp:108:12:108:18 | Load: * ... | This null check is redundant because the value is $@ in any case | RedundantNullCheckSimple.cpp:108:12:108:18 | Load: * ... | dereferenced here |