Merge pull request #2601 from dbartol/dbartol/OpcodeProperties

C++: Consolidate opcode properties onto `Opcode` class

Author: Robert Marsh

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

@@ -11,60 +11,12 @@ import Imports::Opcode
 private import Imports::OperandTag
 
 module InstructionSanity {
-  /**
-   * Holds if the instruction `instr` should be expected to have an operand
-   * with operand tag `tag`. Only holds for singleton operand tags. Tags with
-   * parameters, such as `PhiInputOperand` and `PositionalArgumentOperand` are handled
-   * separately in `unexpectedOperand`.
-   */
-  private predicate expectsOperand(Instruction instr, OperandTag tag) {
-    exists(Opcode opcode |
-      opcode = instr.getOpcode() and
-      (
-        opcode instanceof UnaryOpcode and tag instanceof UnaryOperandTag
-        or
-        opcode instanceof BinaryOpcode and
-        (
-          tag instanceof LeftOperandTag or
-          tag instanceof RightOperandTag
-        )
-        or
-        opcode instanceof MemoryAccessOpcode and tag instanceof AddressOperandTag
-        or
-        opcode instanceof SizedBufferAccessOpcode and tag instanceof BufferSizeOperandTag
-        or
-        opcode instanceof OpcodeWithCondition and tag instanceof ConditionOperandTag
-        or
-        opcode instanceof OpcodeWithLoad and tag instanceof LoadOperandTag
-        or
-        opcode instanceof Opcode::Store and tag instanceof StoreValueOperandTag
-        or
-        opcode instanceof Opcode::UnmodeledUse and tag instanceof UnmodeledUseOperandTag
-        or
-        opcode instanceof Opcode::Call and tag instanceof CallTargetOperandTag
-        or
-        opcode instanceof Opcode::Chi and tag instanceof ChiTotalOperandTag
-        or
-        opcode instanceof Opcode::Chi and tag instanceof ChiPartialOperandTag
-        or
-        (
-          opcode instanceof ReadSideEffectOpcode or
-          opcode instanceof Opcode::InlineAsm or
-          opcode instanceof Opcode::CallSideEffect or
-          opcode instanceof Opcode::ReturnIndirection or
-          opcode instanceof Opcode::AliasedUse
-        ) and
-        tag instanceof SideEffectOperandTag
-      )
-    )
-  }
-
   /**
    * Holds if instruction `instr` is missing an expected operand with tag `tag`.
    */
   query predicate missingOperand(Instruction instr, string message, IRFunction func, string funcText) {
     exists(OperandTag tag |
-      expectsOperand(instr, tag) and
+      instr.getOpcode().hasOperand(tag) and
       not exists(NonPhiOperand operand |
         operand = instr.getAnOperand() and
         operand.getOperandTag() = tag
@@ -84,7 +36,7 @@ module InstructionSanity {
       operand = instr.getAnOperand() and
       operand.getOperandTag() = tag
     ) and
-    not expectsOperand(instr, tag) and
+    not instr.getOpcode().hasOperand(tag) and
     not (instr instanceof CallInstruction and tag instanceof ArgumentOperandTag) and
     not (
       instr instanceof BuiltInOperationInstruction and tag instanceof PositionalArgumentOperandTag
@@ -549,7 +501,8 @@ class Instruction extends Construction::TInstruction {
    * Gets the kind of memory access performed by this instruction's result.
    * Holds only for instructions with a memory result.
    */
-  MemoryAccessKind getResultMemoryAccess() { none() }
+  pragma[inline]
+  final MemoryAccessKind getResultMemoryAccess() { result = getOpcode().getWriteMemoryAccess() }
 
   /**
    * Holds if the memory access performed by this instruction's result will not always write to
@@ -559,7 +512,8 @@ class Instruction extends Construction::TInstruction {
    * 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() }
+  pragma[inline]
+  final predicate hasResultMayMemoryAccess() { getOpcode().hasMayWriteMemoryAccess() }
 
   /**
    * Gets the operand that holds the memory address to which this instruction stores its
@@ -710,16 +664,12 @@ class InitializeParameterInstruction extends VariableInstruction {
   InitializeParameterInstruction() { getOpcode() instanceof Opcode::InitializeParameter }
 
   final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
-
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess }
 }
 
 class InitializeIndirectionInstruction extends VariableInstruction {
   InitializeIndirectionInstruction() { getOpcode() instanceof Opcode::InitializeIndirection }
 
   final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
-
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess }
 }
 
 /**
@@ -754,8 +704,6 @@ class ErrorInstruction extends Instruction {
 class UninitializedInstruction extends VariableInstruction {
   UninitializedInstruction() { getOpcode() instanceof Opcode::Uninitialized }
 
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess }
-
   /**
    * Gets the variable that is uninitialized.
    */
@@ -821,8 +769,6 @@ class LoadInstruction extends CopyInstruction {
 class StoreInstruction extends CopyInstruction {
   StoreInstruction() { getOpcode() instanceof Opcode::Store }
 
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess }
-
   final AddressOperand getDestinationAddressOperand() { result = getAnOperand() }
 
   final Instruction getDestinationAddress() { result = getDestinationAddressOperand().getDef() }
@@ -1236,10 +1182,6 @@ class SideEffectInstruction extends Instruction {
  */
 class CallSideEffectInstruction extends SideEffectInstruction {
   CallSideEffectInstruction() { getOpcode() instanceof Opcode::CallSideEffect }
-
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess }
-
-  final override predicate hasResultMayMemoryAccess() { any() }
 }
 
 /**
@@ -1303,8 +1245,6 @@ class IndirectMustWriteSideEffectInstruction extends WriteSideEffectInstruction
   IndirectMustWriteSideEffectInstruction() {
     getOpcode() instanceof Opcode::IndirectMustWriteSideEffect
   }
-
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess }
 }
 
 /**
@@ -1315,8 +1255,6 @@ class BufferMustWriteSideEffectInstruction extends WriteSideEffectInstruction {
   BufferMustWriteSideEffectInstruction() {
     getOpcode() instanceof Opcode::BufferMustWriteSideEffect
   }
-
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess }
 }
 
 /**
@@ -1328,8 +1266,6 @@ class SizedBufferMustWriteSideEffectInstruction extends WriteSideEffectInstructi
     getOpcode() instanceof Opcode::SizedBufferMustWriteSideEffect
   }
 
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess }
-
   Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() }
 }
 
@@ -1342,10 +1278,6 @@ class IndirectMayWriteSideEffectInstruction extends WriteSideEffectInstruction {
   IndirectMayWriteSideEffectInstruction() {
     getOpcode() instanceof Opcode::IndirectMayWriteSideEffect
   }
-
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess }
-
-  final override predicate hasResultMayMemoryAccess() { any() }
 }
 
 /**
@@ -1354,10 +1286,6 @@ class IndirectMayWriteSideEffectInstruction extends WriteSideEffectInstruction {
  */
 class BufferMayWriteSideEffectInstruction extends WriteSideEffectInstruction {
   BufferMayWriteSideEffectInstruction() { getOpcode() instanceof Opcode::BufferMayWriteSideEffect }
-
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess }
-
-  final override predicate hasResultMayMemoryAccess() { any() }
 }
 
 /**
@@ -1369,10 +1297,6 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio
     getOpcode() instanceof Opcode::SizedBufferMayWriteSideEffect
   }
 
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess }
-
-  final override predicate hasResultMayMemoryAccess() { any() }
-
   Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() }
 }
 
@@ -1381,10 +1305,6 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio
  */
 class InlineAsmInstruction extends Instruction {
   InlineAsmInstruction() { getOpcode() instanceof Opcode::InlineAsm }
-
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess }
-
-  final override predicate hasResultMayMemoryAccess() { any() }
 }
 
 /**
@@ -1470,19 +1390,13 @@ class CatchAnyInstruction extends CatchInstruction {
 
 class UnmodeledDefinitionInstruction extends Instruction {
   UnmodeledDefinitionInstruction() { getOpcode() instanceof Opcode::UnmodeledDefinition }
-
-  final override MemoryAccessKind getResultMemoryAccess() {
-    result instanceof UnmodeledMemoryAccess
-  }
 }
 
 /**
  * An instruction that initializes all escaped memory.
  */
 class AliasedDefinitionInstruction extends Instruction {
   AliasedDefinitionInstruction() { getOpcode() instanceof Opcode::AliasedDefinition }
-
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess }
 }
 
 /**
@@ -1511,8 +1425,6 @@ class UnmodeledUseInstruction extends Instruction {
 class PhiInstruction extends Instruction {
   PhiInstruction() { getOpcode() instanceof Opcode::Phi }
 
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof PhiMemoryAccess }
-
   /**
    * Gets all of the instruction's `PhiInputOperand`s, representing the values that flow from each predecessor block.
    */
@@ -1573,8 +1485,6 @@ class PhiInstruction extends Instruction {
 class ChiInstruction extends Instruction {
   ChiInstruction() { getOpcode() instanceof Opcode::Chi }
 
-  final override MemoryAccessKind getResultMemoryAccess() { result instanceof ChiTotalMemoryAccess }
-
   /**
    * Gets the operand that represents the previous state of all memory that might be aliased by the
    * memory write.

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

@@ -170,7 +170,7 @@ class MemoryOperand extends Operand {
   /**
    * Gets the kind of memory access performed by the operand.
    */
-  MemoryAccessKind getMemoryAccess() { none() }
+  MemoryAccessKind getMemoryAccess() { result = getUse().getOpcode().getReadMemoryAccess() }
 
   /**
    * Holds if the memory access performed by this operand will not always read from every bit in the
@@ -180,7 +180,7 @@ class MemoryOperand extends Operand {
    * 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() }
+  predicate hasMayReadMemoryAccess() { getUse().getOpcode().hasMayReadMemoryAccess() }
 
   /**
    * Returns the operand that holds the memory address from which the current operand loads its
@@ -274,8 +274,6 @@ class LoadOperand extends TypedOperand {
   override LoadOperandTag tag;
 
   override string toString() { result = "Load" }
-
-  final override MemoryAccessKind getMemoryAccess() { result instanceof IndirectMemoryAccess }
 }
 
 /**
@@ -331,8 +329,6 @@ class UnmodeledUseOperand extends NonPhiMemoryOperand {
   override UnmodeledUseOperandTag tag;
 
   override string toString() { result = "UnmodeledUse" }
-
-  final override MemoryAccessKind getMemoryAccess() { result instanceof UnmodeledMemoryAccess }
 }
 
 /**
@@ -382,50 +378,6 @@ class PositionalArgumentOperand extends ArgumentOperand {
 
 class SideEffectOperand extends TypedOperand {
   override SideEffectOperandTag tag;
-
-  override MemoryAccessKind getMemoryAccess() {
-    useInstr instanceof AliasedUseInstruction and
-    result instanceof NonLocalMemoryAccess
-    or
-    useInstr instanceof CallSideEffectInstruction and
-    result instanceof EscapedMemoryAccess
-    or
-    useInstr instanceof CallReadSideEffectInstruction and
-    result instanceof EscapedMemoryAccess
-    or
-    useInstr instanceof IndirectReadSideEffectInstruction and
-    result instanceof IndirectMemoryAccess
-    or
-    useInstr instanceof BufferReadSideEffectInstruction and
-    result instanceof BufferMemoryAccess
-    or
-    useInstr instanceof IndirectMustWriteSideEffectInstruction and
-    result instanceof IndirectMemoryAccess
-    or
-    useInstr instanceof BufferMustWriteSideEffectInstruction and
-    result instanceof BufferMemoryAccess
-    or
-    useInstr instanceof IndirectMayWriteSideEffectInstruction and
-    result instanceof IndirectMemoryAccess
-    or
-    useInstr instanceof BufferMayWriteSideEffectInstruction and
-    result instanceof BufferMemoryAccess
-    or
-    useInstr instanceof ReturnIndirectionInstruction and
-    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
-  }
 }
 
 /**

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

@@ -29,7 +29,7 @@ private predicate hasOperandMemoryAccess(
   resultPointsTo(operand.getAddressOperand().getAnyDef(), var, startBitOffset) and
   languageType = operand.getLanguageType() and
   type = languageType.getIRType() and
-  (if operand.hasMayMemoryAccess() then isMayAccess = true else isMayAccess = false) and
+  (if operand.hasMayReadMemoryAccess() then isMayAccess = true else isMayAccess = false) and
   if exists(type.getByteSize())
   then endBitOffset = Ints::add(startBitOffset, Ints::mul(type.getByteSize(), 8))
   else endBitOffset = Ints::unknown()
@@ -483,7 +483,7 @@ MemoryLocation getResultMemoryLocation(Instruction instr) {
 MemoryLocation getOperandMemoryLocation(MemoryOperand operand) {
   exists(MemoryAccessKind kind, boolean isMayAccess |
     kind = operand.getMemoryAccess() and
-    (if operand.hasMayMemoryAccess() then isMayAccess = true else isMayAccess = false) and
+    (if operand.hasMayReadMemoryAccess() then isMayAccess = true else isMayAccess = false) and
     (
       (
         kind.usesAddressOperand() and

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

@@ -0,0 +1 @@
+import semmle.code.cpp.ir.implementation.MemoryAccessKind as MemoryAccessKind