Merge pull request #1541 from jbj/ir-operand-exact

C++ IR: Make instruction operand getters have only exact results

Author: dave-bartolomeo

cpp/ql/src/semmle/code/cpp/ir/dataflow/TaintTracking.qll

@@ -136,7 +136,7 @@ module TaintTracking {
     // Taint can flow through expressions that alter the value but preserve
     // more than one bit of it _or_ expressions that follow data through
     // pointer indirections.
-    nodeTo.getAnOperand().getDefinitionInstruction() = nodeFrom and
+    nodeTo.getAnOperand().getAnyDef() = nodeFrom and
     (
       nodeTo instanceof ArithmeticInstruction
       or
@@ -145,6 +145,11 @@ module TaintTracking {
       nodeTo instanceof PointerArithmeticInstruction
       or
       nodeTo instanceof FieldAddressInstruction
+      or
+      // The `CopyInstruction` case is also present in non-taint data flow, but
+      // that uses `getDef` rather than `getAnyDef`. For taint, we want flow
+      // from a definition of `myStruct` to a `myStruct.myField` expression.
+      nodeTo instanceof CopyInstruction
     )
     or
     nodeTo.(LoadInstruction).getSourceAddress() = nodeFrom

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

@@ -143,7 +143,7 @@ UninitializedNode uninitializedNode(LocalVariable v) { result.getLocalVariable()
  */
 predicate localFlowStep(Node nodeFrom, Node nodeTo) {
   nodeTo.(CopyInstruction).getSourceValue() = nodeFrom or
-  nodeTo.(PhiInstruction).getAnOperand().getDefinitionInstruction() = nodeFrom or
+  nodeTo.(PhiInstruction).getAnOperand().getDef() = nodeFrom or
   // Treat all conversions as flow, even conversions between different numeric types.
   nodeTo.(ConvertInstruction).getUnary() = nodeFrom
 }

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

@@ -103,7 +103,7 @@ module InstructionSanity {
   query predicate missingOperandType(Operand operand, string message) {
     exists(Function func |
       not exists(operand.getType()) and
-      func = operand.getUseInstruction().getEnclosingFunction() and
+      func = operand.getUse().getEnclosingFunction() and
       message = "Operand missing type in function '" + getIdentityString(func) + "'."
     )
   }
@@ -158,8 +158,8 @@ module InstructionSanity {
    * a different function.
    */
   query predicate operandAcrossFunctions(Operand operand, Instruction instr, Instruction defInstr) {
-    operand.getUseInstruction() = instr and
-    operand.getDefinitionInstruction() = defInstr and
+    operand.getUse() = instr and
+    operand.getAnyDef() = defInstr and
     instr.getEnclosingIRFunction() != defInstr.getEnclosingIRFunction()
   }
 
@@ -480,17 +480,18 @@ class Instruction extends Construction::TInstruction {
   }
 
   /**
-   * Gets all direct uses of the result of this instruction.
+   * Gets all direct uses of the result of this instruction. The result can be
+   * an `Operand` for which `isDefinitionInexact` holds.
    */
   final Operand getAUse() {
-    result.getDefinitionInstruction() = this
+    result.getAnyDef() = this
   }
 
   /**
    * Gets all of this instruction's operands.
    */
   final Operand getAnOperand() {
-    result.getUseInstruction() = this
+    result.getUse() = this
   }
 
   /**
@@ -509,13 +510,22 @@ class Instruction extends Construction::TInstruction {
   }
 
   /**
-   * Returns the operand that holds the memory address to which the instruction stores its
+   * 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()`
    * is `r1`.
    */
   final AddressOperand getResultAddressOperand() {
     getResultMemoryAccess().usesAddressOperand() and
-    result.getUseInstruction() = this
+    result.getUse() = this
+  }
+
+  /**
+   * Gets the instruction that holds the exact memory address to which this instruction stores its
+   * result, if any. For example, in `m3 = Store r1, r2`, the result of `getResultAddressOperand()`
+   * is the instruction that defines `r1`.
+   */
+  final Instruction getResultAddress() {
+    result = getResultAddressOperand().getDef()
   }
 
   /**
@@ -698,7 +708,7 @@ class FieldAddressInstruction extends FieldInstruction {
   }
 
   final Instruction getObjectAddress() {
-    result = getObjectAddressOperand().getDefinitionInstruction()
+    result = getObjectAddressOperand().getDef()
   }
 }
 
@@ -763,7 +773,7 @@ class ReturnValueInstruction extends ReturnInstruction {
   }
 
   final Instruction getReturnValue() {
-    result = getReturnValueOperand().getDefinitionInstruction()
+    result = getReturnValueOperand().getDef()
   }
 }
 
@@ -777,7 +787,7 @@ class CopyInstruction extends Instruction {
   }
 
   final Instruction getSourceValue() {
-    result = getSourceValueOperand().getDefinitionInstruction()
+    result = getSourceValueOperand().getDef()
   }
 }
 
@@ -801,7 +811,7 @@ class LoadInstruction extends CopyInstruction {
   }
 
   final Instruction getSourceAddress() {
-    result = getSourceAddressOperand().getDefinitionInstruction()
+    result = getSourceAddressOperand().getDef()
   }
 
   override final LoadOperand getSourceValueOperand() {
@@ -823,7 +833,7 @@ class StoreInstruction extends CopyInstruction {
   }
 
   final Instruction getDestinationAddress() {
-    result = getDestinationAddressOperand().getDefinitionInstruction()
+    result = getDestinationAddressOperand().getDef()
   }
 
   override final StoreValueOperand getSourceValueOperand() {
@@ -841,7 +851,7 @@ class ConditionalBranchInstruction extends Instruction {
   }
 
   final Instruction getCondition() {
-    result = getConditionOperand().getDefinitionInstruction()
+    result = getConditionOperand().getDef()
   }
 
   final Instruction getTrueSuccessor() {
@@ -907,11 +917,11 @@ class BinaryInstruction extends Instruction {
   }
 
   final Instruction getLeft() {
-    result = getLeftOperand().getDefinitionInstruction()
+    result = getLeftOperand().getDef()
   }
 
   final Instruction getRight() {
-    result = getRightOperand().getDefinitionInstruction()
+    result = getRightOperand().getDef()
   }
 
   /**
@@ -1061,7 +1071,7 @@ class UnaryInstruction extends Instruction {
   }
 
   final Instruction getUnary() {
-    result = getUnaryOperand().getDefinitionInstruction()
+    result = getUnaryOperand().getDef()
   }
 }
 
@@ -1291,7 +1301,7 @@ class SwitchInstruction extends Instruction {
   }
 
   final Instruction getExpression() {
-    result = getExpressionOperand().getDefinitionInstruction()
+    result = getExpressionOperand().getDef()
   }
 
   final Instruction getACaseSuccessor() {
@@ -1326,7 +1336,7 @@ class CallInstruction extends Instruction {
    * function pointer.
    */
   final Instruction getCallTarget() {
-    result = getCallTargetOperand().getDefinitionInstruction()
+    result = getCallTargetOperand().getDef()
   }
 
   /**
@@ -1347,7 +1357,7 @@ class CallInstruction extends Instruction {
    * Gets all of the arguments of the call, including the `this` pointer, if any.
    */
   final Instruction getAnArgument() {
-    result = getAnArgumentOperand().getDefinitionInstruction()
+    result = getAnArgumentOperand().getDef()
   }
 
   /**
@@ -1361,7 +1371,7 @@ class CallInstruction extends Instruction {
    * Gets the `this` pointer argument of the call, if any.
    */
   final Instruction getThisArgument() {
-    result = getThisArgumentOperand().getDefinitionInstruction()
+    result = getThisArgumentOperand().getDef()
   }
 
   /**
@@ -1376,7 +1386,7 @@ class CallInstruction extends Instruction {
    * Gets the argument at the specified index.
    */
   final Instruction getPositionalArgument(int index) {
-    result = getPositionalArgumentOperand(index).getDefinitionInstruction()
+    result = getPositionalArgumentOperand(index).getDef()
   }
 }
 
@@ -1532,7 +1542,7 @@ class ThrowValueInstruction extends ThrowInstruction {
    * Gets the address of the exception thrown by this instruction.
    */
   final Instruction getExceptionAddress() {
-    result = getExceptionAddressOperand().getDefinitionInstruction()
+    result = getExceptionAddressOperand().getDef()
   }
 
   /**
@@ -1546,7 +1556,7 @@ class ThrowValueInstruction extends ThrowInstruction {
    * Gets the exception thrown by this instruction.
    */
   final Instruction getException() {
-    result = getExceptionOperand().getDefinitionInstruction()
+    result = getExceptionOperand().getDef()
   }
 }
 
@@ -1676,7 +1686,7 @@ class PhiInstruction extends Instruction {
    */
   pragma[noinline]
   final Instruction getAnInput() {
-    result = this.getAnInputOperand().getDefinitionInstruction()
+    result = this.getAnInputOperand().getDef()
   }
 }
 
@@ -1744,7 +1754,7 @@ class ChiInstruction extends Instruction {
    * memory write.
    */
   final Instruction getTotal() {
-    result = getTotalOperand().getDefinitionInstruction()
+    result = getTotalOperand().getDef()
   }
 
   /**
@@ -1758,7 +1768,7 @@ class ChiInstruction extends Instruction {
    * Gets the operand that represents the new value written by the memory write.
    */
   final Instruction getPartial() {
-    result = getPartialOperand().getDefinitionInstruction()
+    result = getPartialOperand().getDef()
   }
 }
 

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

@@ -27,27 +27,63 @@ class Operand extends TOperand {
   }
 
   final Location getLocation() {
-    result = getUseInstruction().getLocation()
+    result = getUse().getLocation()
   }
 
   final IRFunction getEnclosingIRFunction() {
-    result = getUseInstruction().getEnclosingIRFunction()
+    result = getUse().getEnclosingIRFunction()
   }
-  
+
   /**
    * Gets the `Instruction` that consumes this operand.
    */
-  Instruction getUseInstruction() {
+  Instruction getUse() {
     none()
   }
 
   /**
-   * Gets the `Instruction` whose result is the value of the operand.
+   * Gets the `Instruction` whose result is the value of the operand. Unlike
+   * `getDef`, this also has a result when `isDefinitionInexact` holds, which
+   * means that the resulting instruction may only _partially_ or _potentially_
+   * be the value of this operand.
    */
-  Instruction getDefinitionInstruction() {
+  Instruction getAnyDef() {
     none()
   }
 
+  /**
+   * Gets the `Instruction` whose result is the value of the operand. Unlike
+   * `getAnyDef`, this also has no result when `isDefinitionInexact` holds,
+   * which means that the resulting instruction must always be exactly the be
+   * the value of this operand.
+   */
+  final Instruction getDef() {
+    result = this.getAnyDef() and
+    getDefinitionOverlap() instanceof MustExactlyOverlap
+  }
+
+  /**
+   * DEPRECATED: renamed to `getUse`.
+   *
+   * Gets the `Instruction` that consumes this operand.
+   */
+  deprecated
+  final Instruction getUseInstruction() {
+    result = getUse()
+  }
+
+  /**
+   * DEPRECATED: use `getAnyDef` or `getDef`. The exact replacement for this
+   * predicate is `getAnyDef`, but most uses of this predicate should probably
+   * be replaced with `getDef`.
+   *
+   * Gets the `Instruction` whose result is the value of the operand.
+   */
+  deprecated
+  final Instruction getDefinitionInstruction() {
+    result = getAnyDef()
+  }
+
   /**
    * Gets the overlap relationship between the operand's definition and its use.
    */
@@ -77,7 +113,7 @@ class Operand extends TOperand {
    * For example: `this:r3_5`
    */
   final string getDumpString() {
-    result = getDumpLabel() + getInexactSpecifier() + getDefinitionInstruction().getResultId()
+    result = getDumpLabel() + getInexactSpecifier() + getAnyDef().getResultId()
   }
 
   /**
@@ -107,7 +143,7 @@ class Operand extends TOperand {
    * has been cast to a different type.
    */
   Type getType() {
-    result = getDefinitionInstruction().getResultType()
+    result = getAnyDef().getResultType()
   }
 
   /**
@@ -118,7 +154,7 @@ class Operand extends TOperand {
    * given by `getResultType()`.
    */
   predicate isGLValue() {
-    getDefinitionInstruction().isGLValue()
+    getAnyDef().isGLValue()
   }
 
   /**
@@ -158,7 +194,7 @@ class MemoryOperand extends Operand {
    */
   final AddressOperand getAddressOperand() {
     getMemoryAccess().usesAddressOperand() and
-    result.getUseInstruction() = getUseInstruction()
+    result.getUse() = getUse()
   }
 }
 
@@ -175,11 +211,11 @@ class NonPhiOperand extends Operand {
     this = TNonPhiMemoryOperand(useInstr, tag, defInstr, _)
   }
 
-  override final Instruction getUseInstruction() {
+  override final Instruction getUse() {
     result = useInstr
   }
 
-  override final Instruction getDefinitionInstruction() {
+  override final Instruction getAnyDef() {
     result = defInstr
   }
 
@@ -437,11 +473,11 @@ class PhiInputOperand extends MemoryOperand, TPhiOperand {
     result = "Phi"
   }
 
-  override final PhiInstruction getUseInstruction() {
+  override final PhiInstruction getUse() {
     result = useInstr
   }
 
-  override final Instruction getDefinitionInstruction() {
+  override final Instruction getAnyDef() {
     result = defInstr
   }