C++/C#: Consistent handling of "may" vs. "must" memory accesses

In the IR, some memory accesses are "must" accesses (the entire memory location is always read or written), and some are "may" accesses (some, all, or none of the bits in the location are written). We previously had to special case specific "may" accesses in a few places. This change regularizes our handling of "may" accesses.

The `MemoryAccessKind` enumeration now describes only the extent of the access (the set of locations potentially accessed), but does not distinguish "must" from "may". The new predicates `Operand.hasMayMemoryAccess()` and `Instruction.hasResultMayMemoryAccess()` hold when the access is a "may" access.

Unaliased SSA now correctly ignores variables that are ever accessed via a "may" access.

Aliased SSA now distinguishes `MemoryLocation`s for "may" and "must" accesses. I've refactored `getOverlap()` into the core `getExtentOverlap()`, which considers only the extent, but not the "may" vs. "must", and `getOverlap()`, which tweaks the result of `getExtentOverlap()` based on "may" vs. "must" and read-only locations.

When determining the overlap between a `Phi` operand and its definition, we now use the result of the defining `Chi` instruction, if one exists. This gives exact definitions for `Phi` operands for virtual variables.

Author: Dave Bartolomeo

cpp/ql/src/semmle/code/cpp/ir/implementation/MemoryAccessKind.qll

@@ -1,11 +1,8 @@
 private newtype TMemoryAccessKind =
   TIndirectMemoryAccess() or
-  TIndirectMayMemoryAccess() or
   TBufferMemoryAccess() or
-  TBufferMayMemoryAccess() or
   TEscapedMemoryAccess() or
-  TEscapedMayMemoryAccess() or
-  TNonLocalMayMemoryAccess() or
+  TNonLocalMemoryAccess() or
   TPhiMemoryAccess() or
   TUnmodeledMemoryAccess() or
   TChiTotalMemoryAccess() or
@@ -35,16 +32,6 @@ class IndirectMemoryAccess extends MemoryAccessKind, TIndirectMemoryAccess {
   final override predicate usesAddressOperand() { any() }
 }
 
-/**
- * The operand or result may access some, all, or none of the memory at the address specified by the
- * `AddressOperand` on the same instruction.
- */
-class IndirectMayMemoryAccess extends MemoryAccessKind, TIndirectMayMemoryAccess {
-  override string toString() { result = "indirect(may)" }
-
-  final override predicate usesAddressOperand() { any() }
-}
-
 /**
  * The operand or result accesses memory starting at the address specified by the `AddressOperand`
  * on the same instruction, accessing a number of consecutive elements given by the
@@ -56,17 +43,6 @@ class BufferMemoryAccess extends MemoryAccessKind, TBufferMemoryAccess {
   final override predicate usesAddressOperand() { any() }
 }
 
-/**
- * The operand or result may access some, all, or none of the memory starting at the address
- * specified by the `AddressOperand` on the same instruction, accessing a number of consecutive
- * elements given by the `BufferSizeOperand`.
- */
-class BufferMayMemoryAccess extends MemoryAccessKind, TBufferMayMemoryAccess {
-  override string toString() { result = "buffer(may)" }
-
-  final override predicate usesAddressOperand() { any() }
-}
-
 /**
  * The operand or result accesses all memory whose address has escaped.
  */
@@ -75,18 +51,11 @@ class EscapedMemoryAccess extends MemoryAccessKind, TEscapedMemoryAccess {
 }
 
 /**
- * The operand or result may access all memory whose address has escaped.
- */
-class EscapedMayMemoryAccess extends MemoryAccessKind, TEscapedMayMemoryAccess {
-  override string toString() { result = "escaped(may)" }
-}
-
-/**
- * The operand or result may access all memory whose address has escaped, other than data on the
- * stack frame of the current function.
+ * The operand or result access all memory whose address has escaped, other than data on the stack
+ * frame of the current function.
  */
-class NonLocalMayMemoryAccess extends MemoryAccessKind, TNonLocalMayMemoryAccess {
-  override string toString() { result = "nonlocal(may)" }
+class NonLocalMemoryAccess extends MemoryAccessKind, TNonLocalMemoryAccess {
+  override string toString() { result = "nonlocal" }
 }
 
 /**

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll

@@ -550,6 +550,16 @@ class Instruction extends Construction::TInstruction {
    */
   MemoryAccessKind getResultMemoryAccess() { none() }
 
+  /**
+   * Holds if the memory access performed by this instruction's result will not always write to
+   * every bit in the memory location. This is most commonly used for memory accesses that may or
+   * may not actually occur depending on runtime state (for example, the write side effect of an
+   * output parameter that is not written to on all paths), or for accesses where the memory
+   * location is a conservative estimate of the memory that might actually be accessed at runtime
+   * (for example, the global side effects of a function call).
+   */
+  predicate hasResultMayMemoryAccess() { none() }
+
   /**
    * Gets the operand that holds the memory address to which this instruction stores its
    * result, if any. For example, in `m3 = Store r1, r2`, the result of `getResultAddressOperand()`
@@ -1206,9 +1216,9 @@ class SideEffectInstruction extends Instruction {
 class CallSideEffectInstruction extends SideEffectInstruction {
   CallSideEffectInstruction() { getOpcode() instanceof Opcode::CallSideEffect }
 
-  final override MemoryAccessKind getResultMemoryAccess() {
-    result instanceof EscapedMayMemoryAccess
-  }
+  final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess }
+
+  final override predicate hasResultMayMemoryAccess() { any() }
 }
 
 /**
@@ -1306,9 +1316,9 @@ class IndirectMayWriteSideEffectInstruction extends WriteSideEffectInstruction {
     getOpcode() instanceof Opcode::IndirectMayWriteSideEffect
   }
 
-  final override MemoryAccessKind getResultMemoryAccess() {
-    result instanceof IndirectMayMemoryAccess
-  }
+  final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess }
+
+  final override predicate hasResultMayMemoryAccess() { any() }
 }
 
 /**
@@ -1318,9 +1328,9 @@ class IndirectMayWriteSideEffectInstruction extends WriteSideEffectInstruction {
 class BufferMayWriteSideEffectInstruction extends WriteSideEffectInstruction {
   BufferMayWriteSideEffectInstruction() { getOpcode() instanceof Opcode::BufferMayWriteSideEffect }
 
-  final override MemoryAccessKind getResultMemoryAccess() {
-    result instanceof BufferMayMemoryAccess
-  }
+  final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess }
+
+  final override predicate hasResultMayMemoryAccess() { any() }
 }
 
 /**
@@ -1332,9 +1342,9 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio
     getOpcode() instanceof Opcode::SizedBufferMayWriteSideEffect
   }
 
-  final override MemoryAccessKind getResultMemoryAccess() {
-    result instanceof BufferMayMemoryAccess
-  }
+  final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess }
+
+  final override predicate hasResultMayMemoryAccess() { any() }
 
   Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() }
 }
@@ -1345,9 +1355,9 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio
 class InlineAsmInstruction extends Instruction {
   InlineAsmInstruction() { getOpcode() instanceof Opcode::InlineAsm }
 
-  final override MemoryAccessKind getResultMemoryAccess() {
-    result instanceof EscapedMayMemoryAccess
-  }
+  final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess }
+
+  final override predicate hasResultMayMemoryAccess() { any() }
 }
 
 /**

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Operand.qll

@@ -194,6 +194,16 @@ class MemoryOperand extends Operand {
    */
   MemoryAccessKind getMemoryAccess() { none() }
 
+  /**
+   * Holds if the memory access performed by this operand will not always read from every bit in the
+   * memory location. This is most commonly used for memory accesses that may or may not actually
+   * occur depending on runtime state (for example, the write side effect of an output parameter
+   * that is not written to on all paths), or for accesses where the memory location is a
+   * conservative estimate of the memory that might actually be accessed at runtime (for example,
+   * the global side effects of a function call).
+   */
+  predicate hasMayMemoryAccess() { none() }
+
   /**
    * Returns the operand that holds the memory address from which the current operand loads its
    * value, if any. For example, in `r3 = Load r1, m2`, the result of `getAddressOperand()` for `m2`
@@ -397,13 +407,13 @@ class SideEffectOperand extends TypedOperand {
 
   override MemoryAccessKind getMemoryAccess() {
     useInstr instanceof AliasedUseInstruction and
-    result instanceof NonLocalMayMemoryAccess
+    result instanceof NonLocalMemoryAccess
     or
     useInstr instanceof CallSideEffectInstruction and
-    result instanceof EscapedMayMemoryAccess
+    result instanceof EscapedMemoryAccess
     or
     useInstr instanceof CallReadSideEffectInstruction and
-    result instanceof EscapedMayMemoryAccess
+    result instanceof EscapedMemoryAccess
     or
     useInstr instanceof IndirectReadSideEffectInstruction and
     result instanceof IndirectMemoryAccess
@@ -418,10 +428,22 @@ class SideEffectOperand extends TypedOperand {
     result instanceof BufferMemoryAccess
     or
     useInstr instanceof IndirectMayWriteSideEffectInstruction and
-    result instanceof IndirectMayMemoryAccess
+    result instanceof IndirectMemoryAccess
     or
     useInstr instanceof BufferMayWriteSideEffectInstruction and
-    result instanceof BufferMayMemoryAccess
+    result instanceof BufferMemoryAccess
+  }
+
+  final override predicate hasMayMemoryAccess() {
+    useInstr instanceof AliasedUseInstruction
+    or
+    useInstr instanceof CallSideEffectInstruction
+    or
+    useInstr instanceof CallReadSideEffectInstruction
+    or
+    useInstr instanceof IndirectMayWriteSideEffectInstruction
+    or
+    useInstr instanceof BufferMayWriteSideEffectInstruction
   }
 }