Merge pull request #4142 from MathiasVP/mathiasvp/read-step-without-memory-operands

C++: Use IR alias analysis for field flow

Author: jbj

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

@@ -144,8 +144,23 @@ OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) {
  */
 predicate jumpStep(Node n1, Node n2) { none() }
 
+/**
+ * Gets a field corresponding to the bit range `[startBit..endBit)` of class `c`, if any.
+ */
+private Field getAField(Class c, int startBit, int endBit) {
+  result.getDeclaringType() = c and
+  startBit = 8 * result.getByteOffset() and
+  endBit = 8 * result.getType().getSize() + startBit
+  or
+  exists(Field f, Class cInner |
+    f = c.getAField() and
+    cInner = f.getUnderlyingType() and
+    result = getAField(cInner, startBit - 8 * f.getByteOffset(), endBit - 8 * f.getByteOffset())
+  )
+}
+
 private newtype TContent =
-  TFieldContent(Field f) or
+  TFieldContent(Class c, int startBit, int endBit) { exists(getAField(c, startBit, endBit)) } or
   TCollectionContent() or
   TArrayContent()
 
@@ -163,17 +178,18 @@ class Content extends TContent {
 }
 
 private class FieldContent extends Content, TFieldContent {
-  Field f;
+  Class c;
+  int startBit;
+  int endBit;
 
-  FieldContent() { this = TFieldContent(f) }
+  FieldContent() { this = TFieldContent(c, startBit, endBit) }
 
-  Field getField() { result = f }
+  // Ensure that there's just 1 result for `toString`.
+  override string toString() { result = min(Field f | f = getAField() | f.toString()) }
 
-  override string toString() { result = f.toString() }
+  predicate hasOffset(Class cl, int start, int end) { cl = c and start = startBit and end = endBit }
 
-  override predicate hasLocationInfo(string path, int sl, int sc, int el, int ec) {
-    f.getLocation().hasLocationInfo(path, sl, sc, el, ec)
-  }
+  Field getAField() { result = getAField(c, startBit, endBit) }
 }
 
 private class CollectionContent extends Content, TCollectionContent {
@@ -185,20 +201,38 @@ private class ArrayContent extends Content, TArrayContent {
 }
 
 private predicate storeStepNoChi(Node node1, Content f, PostUpdateNode node2) {
-  exists(FieldAddressInstruction fa, StoreInstruction store |
+  exists(StoreInstruction store, Class c |
     store = node2.asInstruction() and
-    store.getDestinationAddress() = fa and
     store.getSourceValue() = node1.asInstruction() and
-    f.(FieldContent).getField() = fa.getField()
+    getWrittenField(store, f.(FieldContent).getAField(), c) and
+    f.(FieldContent).hasOffset(c, _, _)
+  )
+}
+
+pragma[noinline]
+private predicate getWrittenField(StoreInstruction store, Field f, Class c) {
+  exists(FieldAddressInstruction fa |
+    fa = store.getDestinationAddress() and
+    f = fa.getField() and
+    c = f.getDeclaringType()
   )
 }
 
 private predicate storeStepChi(Node node1, Content f, PostUpdateNode node2) {
-  exists(FieldAddressInstruction fa, StoreInstruction store |
+  exists(StoreInstruction store, ChiInstruction chi |
     node1.asInstruction() = store and
-    store.getDestinationAddress() = fa and
-    node2.asInstruction().(ChiInstruction).getPartial() = store and
-    f.(FieldContent).getField() = fa.getField()
+    node2.asInstruction() = chi and
+    chi.getPartial() = store and
+    exists(Class c |
+      c = chi.getResultType() and
+      exists(int startBit, int endBit |
+        chi.getUpdatedInterval(startBit, endBit) and
+        f.(FieldContent).hasOffset(c, startBit, endBit)
+      )
+      or
+      getWrittenField(store, f.(FieldContent).getAField(), c) and
+      f.(FieldContent).hasOffset(c, _, _)
+    )
   )
 }
 
@@ -212,17 +246,37 @@ predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
   storeStepChi(node1, f, node2)
 }
 
+bindingset[result, i]
+private int unbindInt(int i) { i <= result and i >= result }
+
+pragma[noinline]
+private predicate getLoadedField(LoadInstruction load, Field f, Class c) {
+  exists(FieldAddressInstruction fa |
+    fa = load.getSourceAddress() and
+    f = fa.getField() and
+    c = f.getDeclaringType()
+  )
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` via a read of `f`.
  * Thus, `node1` references an object with a field `f` whose value ends up in
  * `node2`.
  */
 predicate readStep(Node node1, Content f, Node node2) {
-  exists(FieldAddressInstruction fa, LoadInstruction load |
-    load.getSourceAddress() = fa and
+  exists(LoadInstruction load |
+    node2.asInstruction() = load and
     node1.asInstruction() = load.getSourceValueOperand().getAnyDef() and
-    fa.getField() = f.(FieldContent).getField() and
-    load = node2.asInstruction()
+    exists(Class c |
+      c = load.getSourceValueOperand().getAnyDef().getResultType() and
+      exists(int startBit, int endBit |
+        load.getSourceValueOperand().getUsedInterval(unbindInt(startBit), unbindInt(endBit)) and
+        f.(FieldContent).hasOffset(c, startBit, endBit)
+      )
+      or
+      getLoadedField(load, f.(FieldContent).getAField(), c) and
+      f.(FieldContent).hasOffset(c, _, _)
+    )
   )
 }
 

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

@@ -335,12 +335,14 @@ abstract private class PartialDefinitionNode extends PostUpdateNode {
 
 private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
   override ChiInstruction instr;
-  FieldAddressInstruction field;
+  StoreInstruction store;
 
   ExplicitFieldStoreQualifierNode() {
     not instr.isResultConflated() and
-    exists(StoreInstruction store |
-      instr.getPartial() = store and field = store.getDestinationAddress()
+    instr.getPartial() = store and
+    (
+      instr.getUpdatedInterval(_, _) or
+      store.getDestinationAddress() instanceof FieldAddressInstruction
     )
   }
 
@@ -351,7 +353,12 @@ private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
   override Node getPreUpdateNode() { result.asOperand() = instr.getTotalOperand() }
 
   override Expr getDefinedExpr() {
-    result = field.getObjectAddress().getUnconvertedResultExpression()
+    result =
+      store
+          .getDestinationAddress()
+          .(FieldAddressInstruction)
+          .getObjectAddress()
+          .getUnconvertedResultExpression()
   }
 }
 
@@ -363,17 +370,22 @@ private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
  */
 private class ExplicitSingleFieldStoreQualifierNode extends PartialDefinitionNode {
   override StoreInstruction instr;
-  FieldAddressInstruction field;
 
   ExplicitSingleFieldStoreQualifierNode() {
-    field = instr.getDestinationAddress() and
-    not exists(ChiInstruction chi | chi.getPartial() = instr)
+    not exists(ChiInstruction chi | chi.getPartial() = instr) and
+    // Without this condition any store would create a `PostUpdateNode`.
+    instr.getDestinationAddress() instanceof FieldAddressInstruction
   }
 
   override Node getPreUpdateNode() { none() }
 
   override Expr getDefinedExpr() {
-    result = field.getObjectAddress().getUnconvertedResultExpression()
+    result =
+      instr
+          .getDestinationAddress()
+          .(FieldAddressInstruction)
+          .getObjectAddress()
+          .getUnconvertedResultExpression()
   }
 }
 

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

@@ -1962,6 +1962,13 @@ class ChiInstruction extends Instruction {
    * Gets the operand that represents the new value written by the memory write.
    */
   final Instruction getPartial() { result = getPartialOperand().getDef() }
+
+  /**
+   * Gets the bit range `[startBit, endBit)` updated by the partial operand of this `ChiInstruction`, relative to the start address of the total operand.
+   */
+  final predicate getUpdatedInterval(int startBit, int endBit) {
+    Construction::getIntervalUpdatedByChi(this, startBit, endBit)
+  }
 }
 
 /**

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

@@ -328,6 +328,14 @@ class NonPhiMemoryOperand extends NonPhiOperand, MemoryOperand, NonPhiMemoryOper
     not Construction::isInCycle(useInstr) and
     strictcount(Construction::getMemoryOperandDefinition(useInstr, tag, _)) = 1
   }
+
+  /**
+   * Holds if the operand totally overlaps with its definition and consumes the
+   * bit range `[startBitOffset, endBitOffset)` relative to the start address of the definition.
+   */
+  predicate getUsedInterval(int startBitOffset, int endBitOffset) {
+    Construction::getUsedInterval(this, startBitOffset, endBitOffset)
+  }
 }
 
 /**

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll

@@ -617,3 +617,9 @@ MemoryLocation getOperandMemoryLocation(MemoryOperand operand) {
     )
   )
 }
+
+/** Gets the start bit offset of a `MemoryLocation`, if any. */
+int getStartBitOffset(VariableMemoryLocation location) { result = location.getStartBitOffset() }
+
+/** Gets the end bit offset of a `MemoryLocation`, if any. */
+int getEndBitOffset(VariableMemoryLocation location) { result = location.getEndBitOffset() }

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll

@@ -149,6 +149,34 @@ private module Cached {
     )
   }
 
+  /**
+   * Holds if the partial operand of this `ChiInstruction` updates the bit range
+   * `[startBitOffset, endBitOffset)` of the total operand.
+   */
+  cached
+  predicate getIntervalUpdatedByChi(ChiInstruction chi, int startBitOffset, int endBitOffset) {
+    exists(Alias::MemoryLocation location, OldInstruction oldInstruction |
+      oldInstruction = getOldInstruction(chi.getPartial()) and
+      location = Alias::getResultMemoryLocation(oldInstruction) and
+      startBitOffset = Alias::getStartBitOffset(location) and
+      endBitOffset = Alias::getEndBitOffset(location)
+    )
+  }
+
+  /**
+   * Holds if `operand` totally overlaps with its definition and consumes the bit range
+   * `[startBitOffset, endBitOffset)`.
+   */
+  cached
+  predicate getUsedInterval(NonPhiMemoryOperand operand, int startBitOffset, int endBitOffset) {
+    exists(Alias::MemoryLocation location, OldIR::NonPhiMemoryOperand oldOperand |
+      oldOperand = operand.getUse().(OldInstruction).getAnOperand() and
+      location = Alias::getOperandMemoryLocation(oldOperand) and
+      startBitOffset = Alias::getStartBitOffset(location) and
+      endBitOffset = Alias::getEndBitOffset(location)
+    )
+  }
+
   /**
    * Holds if `instr` is part of a cycle in the operand graph that doesn't go
    * through a phi instruction and therefore should be impossible.

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

@@ -1962,6 +1962,13 @@ class ChiInstruction extends Instruction {
    * Gets the operand that represents the new value written by the memory write.
    */
   final Instruction getPartial() { result = getPartialOperand().getDef() }
+
+  /**
+   * Gets the bit range `[startBit, endBit)` updated by the partial operand of this `ChiInstruction`, relative to the start address of the total operand.
+   */
+  final predicate getUpdatedInterval(int startBit, int endBit) {
+    Construction::getIntervalUpdatedByChi(this, startBit, endBit)
+  }
 }
 
 /**

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

@@ -328,6 +328,14 @@ class NonPhiMemoryOperand extends NonPhiOperand, MemoryOperand, NonPhiMemoryOper
     not Construction::isInCycle(useInstr) and
     strictcount(Construction::getMemoryOperandDefinition(useInstr, tag, _)) = 1
   }
+
+  /**
+   * Holds if the operand totally overlaps with its definition and consumes the
+   * bit range `[startBitOffset, endBitOffset)` relative to the start address of the definition.
+   */
+  predicate getUsedInterval(int startBitOffset, int endBitOffset) {
+    Construction::getUsedInterval(this, startBitOffset, endBitOffset)
+  }
 }
 
 /**

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll

@@ -182,6 +182,18 @@ Instruction getMemoryOperandDefinition(
   none()
 }
 
+/**
+ * Holds if the partial operand of this `ChiInstruction` updates the bit range
+ * `[startBitOffset, endBitOffset)` of the total operand.
+ */
+predicate getIntervalUpdatedByChi(ChiInstruction chi, int startBit, int endBit) { none() }
+
+/**
+ * Holds if the operand totally overlaps with its definition and consumes the
+ * bit range `[startBitOffset, endBitOffset)`.
+ */
+predicate getUsedInterval(Operand operand, int startBit, int endBit) { none() }
+
 /** Gets a non-phi instruction that defines an operand of `instr`. */
 private Instruction getNonPhiOperandDef(Instruction instr) {
   result = getRegisterOperandDefinition(instr, _)

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

@@ -1962,6 +1962,13 @@ class ChiInstruction extends Instruction {
    * Gets the operand that represents the new value written by the memory write.
    */
   final Instruction getPartial() { result = getPartialOperand().getDef() }
+
+  /**
+   * Gets the bit range `[startBit, endBit)` updated by the partial operand of this `ChiInstruction`, relative to the start address of the total operand.
+   */
+  final predicate getUpdatedInterval(int startBit, int endBit) {
+    Construction::getIntervalUpdatedByChi(this, startBit, endBit)
+  }
 }
 
 /**