Merge pull request #2696 from dbartol/dbartol/Indirections

C++/C#: Alias analysis of indirect parameters

Author: rdmarsh2

config/identical-files.json

@@ -190,9 +190,14 @@
     "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstructionImports.qll",
     "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstructionImports.qll"
   ],
-  "C++ SSA AliasAnalysis": [
+  "SSA AliasAnalysis": [
     "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll",
-    "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll"
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll",
+    "csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll"
+  ],
+  "C++ SSA AliasAnalysisImports": [
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysisImports.qll",
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysisImports.qll"
   ],
   "C++ IR ValueNumberingImports": [
     "cpp/ql/src/semmle/code/cpp/ir/implementation/raw/gvn/internal/ValueNumberingImports.qll",
@@ -203,6 +208,10 @@
     "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll",
     "csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll"
   ],
+  "IR AliasConfiguration (unaliased_ssa)": [
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasConfiguration.qll",
+    "csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/AliasConfiguration.qll"
+  ],
   "IR SSA SSAConstruction": [
     "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll",
     "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll",

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

@@ -1,6 +1,7 @@
 private newtype TMemoryAccessKind =
   TIndirectMemoryAccess() or
   TBufferMemoryAccess() or
+  TEntireAllocationMemoryAccess() or
   TEscapedMemoryAccess() or
   TNonLocalMemoryAccess() or
   TPhiMemoryAccess() or
@@ -43,6 +44,16 @@ class BufferMemoryAccess extends MemoryAccessKind, TBufferMemoryAccess {
   final override predicate usesAddressOperand() { any() }
 }
 
+/**
+ * The operand or results accesses all memory in the contiguous allocation that contains the address
+ * specified by the `AddressOperand` on the same instruction.
+ */
+class EntireAllocationMemoryAccess extends MemoryAccessKind, TEntireAllocationMemoryAccess {
+  override string toString() { result = "alloc" }
+
+  final override predicate usesAddressOperand() { any() }
+}
+
 /**
  * The operand or result accesses all memory whose address has escaped.
  */

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

@@ -232,6 +232,31 @@ abstract class BufferReadOpcode extends BufferAccessOpcode {
   final override MemoryAccessKind getReadMemoryAccess() { result instanceof BufferMemoryAccess }
 }
 
+/**
+ * An opcode that access an entire memory allocation.
+ */
+abstract class EntireAllocationAccessOpcode extends Opcode {
+  final override predicate hasAddressOperand() { any() }
+}
+
+/**
+ * An opcode that write to an entire memory allocation.
+ */
+abstract class EntireAllocationWriteOpcode extends EntireAllocationAccessOpcode {
+  final override MemoryAccessKind getWriteMemoryAccess() {
+    result instanceof EntireAllocationMemoryAccess
+  }
+}
+
+/**
+ * An opcode that reads from an entire memory allocation.
+ */
+abstract class EntireAllocationReadOpcode extends EntireAllocationAccessOpcode {
+  final override MemoryAccessKind getReadMemoryAccess() {
+    result instanceof EntireAllocationMemoryAccess
+  }
+}
+
 /**
  * An opcode that accesses a memory buffer whose size is determined by a `BufferSizeOperand`.
  */
@@ -325,7 +350,7 @@ module Opcode {
     final override string toString() { result = "InitializeParameter" }
   }
 
-  class InitializeIndirection extends IndirectWriteOpcode, TInitializeIndirection {
+  class InitializeIndirection extends EntireAllocationWriteOpcode, TInitializeIndirection {
     final override string toString() { result = "InitializeIndirection" }
   }
 
@@ -349,7 +374,7 @@ module Opcode {
     final override string toString() { result = "ReturnVoid" }
   }
 
-  class ReturnIndirection extends IndirectReadOpcode, TReturnIndirection {
+  class ReturnIndirection extends EntireAllocationReadOpcode, TReturnIndirection {
     final override string toString() { result = "ReturnIndirection" }
 
     final override predicate hasOperandInternal(OperandTag tag) {

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

@@ -1,21 +1,9 @@
 private import AliasAnalysisInternal
-private import cpp
 private import InputIR
-private import semmle.code.cpp.ir.internal.IntegerConstant as Ints
-private import semmle.code.cpp.ir.implementation.IRConfiguration
-private import semmle.code.cpp.models.interfaces.Alias
+private import AliasAnalysisImports
 
 private class IntValue = Ints::IntValue;
 
-/**
- * Gets the offset of field `field` in bits.
- */
-private IntValue getFieldBitOffset(Field field) {
-  if field instanceof BitField
-  then result = Ints::add(Ints::mul(field.getByteOffset(), 8), field.(BitField).getBitOffset())
-  else result = Ints::mul(field.getByteOffset(), 8)
-}
-
 /**
  * Holds if the operand `tag` of instruction `instr` is used in a way that does
  * not result in any address held in that operand from escaping beyond the
@@ -36,7 +24,7 @@ private predicate operandIsConsumedWithoutEscaping(Operand operand) {
       instr instanceof PointerDiffInstruction
       or
       // Converting an address to a `bool` does not escape the address.
-      instr.(ConvertInstruction).getResultType() instanceof BoolType
+      instr.(ConvertInstruction).getResultIRType() instanceof IRBooleanType
     )
   )
   or
@@ -111,13 +99,10 @@ private predicate operandIsPropagated(Operand operand, IntValue bitOffset) {
       bitOffset = Ints::unknown()
       or
       // Conversion to another pointer type propagates the source address.
-      exists(ConvertInstruction convert, Type resultType |
+      exists(ConvertInstruction convert, IRType resultType |
         convert = instr and
-        resultType = convert.getResultType() and
-        (
-          resultType instanceof PointerType or
-          resultType instanceof Class //REVIEW: Remove when all glvalues are pointers
-        ) and
+        resultType = convert.getResultIRType() and
+        resultType instanceof IRAddressType and
         bitOffset = 0
       )
       or
@@ -131,7 +116,7 @@ private predicate operandIsPropagated(Operand operand, IntValue bitOffset) {
       or
       // Computing a field address from a pointer propagates the address plus the
       // offset of the field.
-      bitOffset = getFieldBitOffset(instr.(FieldAddressInstruction).getField())
+      bitOffset = Language::getFieldBitOffset(instr.(FieldAddressInstruction).getField())
       or
       // A copy propagates the source value.
       operand = instr.(CopyInstruction).getSourceValueOperand() and bitOffset = 0
@@ -212,7 +197,7 @@ private predicate operandReturned(Operand operand, IntValue bitOffset) {
 }
 
 private predicate isArgumentForParameter(CallInstruction ci, Operand operand, Instruction init) {
-  exists(Function f |
+  exists(Language::Function f |
     ci = operand.getUse() and
     f = ci.getStaticCallTarget() and
     (
@@ -223,27 +208,27 @@ private predicate isArgumentForParameter(CallInstruction ci, Operand operand, In
       init.getEnclosingFunction() = f and
       operand instanceof ThisArgumentOperand
     ) and
-    not f.isVirtual() and
-    not f instanceof AliasFunction
+    not Language::isFunctionVirtual(f) and
+    not f instanceof AliasModels::AliasFunction
   )
 }
 
 private predicate isAlwaysReturnedArgument(Operand operand) {
-  exists(AliasFunction f |
+  exists(AliasModels::AliasFunction f |
     f = operand.getUse().(CallInstruction).getStaticCallTarget() and
     f.parameterIsAlwaysReturned(operand.(PositionalArgumentOperand).getIndex())
   )
 }
 
 private predicate isOnlyEscapesViaReturnArgument(Operand operand) {
-  exists(AliasFunction f |
+  exists(AliasModels::AliasFunction f |
     f = operand.getUse().(CallInstruction).getStaticCallTarget() and
     f.parameterEscapesOnlyViaReturn(operand.(PositionalArgumentOperand).getIndex())
   )
 }
 
 private predicate isNeverEscapesArgument(Operand operand) {
-  exists(AliasFunction f |
+  exists(AliasModels::AliasFunction f |
     f = operand.getUse().(CallInstruction).getStaticCallTarget() and
     f.parameterNeverEscapes(operand.(PositionalArgumentOperand).getIndex())
   )
@@ -265,61 +250,86 @@ private predicate resultEscapesNonReturn(Instruction instr) {
 }
 
 /**
- * Holds if the address of the specified local variable or parameter escapes the
- * domain of the analysis.
+ * Holds if the address of `allocation` escapes outside the domain of the analysis. This can occur
+ * either because the allocation's address is taken within the function and escapes, or because the
+ * allocation is marked as always escaping via `alwaysEscapes()`.
  */
-private predicate automaticVariableAddressEscapes(IRAutomaticVariable var) {
-  // The variable's address escapes if the result of any
-  // VariableAddressInstruction that computes the variable's address escapes.
-  exists(VariableAddressInstruction instr |
-    instr.getIRVariable() = var and
-    resultEscapesNonReturn(instr)
+predicate allocationEscapes(Configuration::Allocation allocation) {
+  allocation.alwaysEscapes()
+  or
+  exists(IREscapeAnalysisConfiguration config |
+    config.useSoundEscapeAnalysis() and resultEscapesNonReturn(allocation.getABaseInstruction())
   )
 }
 
 /**
- * Holds if the address of the specified variable escapes the domain of the
- * analysis.
+ * Equivalent to `operandIsPropagated()`, but includes interprocedural propagation.
  */
-predicate variableAddressEscapes(IRVariable var) {
-  exists(IREscapeAnalysisConfiguration config |
-    config.useSoundEscapeAnalysis() and
-    automaticVariableAddressEscapes(var.(IRAutomaticVariable))
-  )
+private predicate operandIsPropagatedIncludingByCall(Operand operand, IntValue bitOffset) {
+  operandIsPropagated(operand, bitOffset)
   or
-  // All variables with static storage duration have their address escape, even when escape analysis
-  // is allowed to be unsound. Otherwise, we won't have a definition for any non-escaped global
-  // variable. Normally, we rely on `AliasedDefinition` to handle that.
-  not var instanceof IRAutomaticVariable
+  exists(CallInstruction call, Instruction init |
+    isArgumentForParameter(call, operand, init) and
+    resultReturned(init, bitOffset)
+  )
 }
 
 /**
- * Holds if the result of instruction `instr` points within variable `var`, at
- * bit offset `bitOffset` within the variable. If the result points within
- * `var`, but at an unknown or non-constant offset, then `bitOffset` is unknown.
+ * Holds if `addrOperand` is at offset `bitOffset` from the value of instruction `base`. The offset
+ * may be `unknown()`.
  */
-predicate resultPointsTo(Instruction instr, IRVariable var, IntValue bitOffset) {
-  // The address of a variable points to that variable, at offset 0.
-  instr.(VariableAddressInstruction).getIRVariable() = var and
-  bitOffset = 0
-  or
-  // A string literal is just a special read-only global variable.
-  instr.(StringConstantInstruction).getIRVariable() = var and
-  bitOffset = 0
+private predicate hasBaseAndOffset(AddressOperand addrOperand, Instruction base, IntValue bitOffset) {
+  base = addrOperand.getDef() and bitOffset = 0 // Base case
   or
-  exists(Operand operand, IntValue originalBitOffset, IntValue propagatedBitOffset |
-    operand = instr.getAnOperand() and
-    // If an operand is propagated, then the result points to the same variable,
-    // offset by the bit offset from the propagation.
-    resultPointsTo(operand.getAnyDef(), var, originalBitOffset) and
-    (
-      operandIsPropagated(operand, propagatedBitOffset)
-      or
-      exists(CallInstruction ci, Instruction init |
-        isArgumentForParameter(ci, operand, init) and
-        resultReturned(init, propagatedBitOffset)
-      )
-    ) and
-    bitOffset = Ints::add(originalBitOffset, propagatedBitOffset)
+  exists(
+    Instruction middle, int previousBitOffset, Operand middleOperand, IntValue additionalBitOffset
+  |
+    // We already have an offset from `middle`.
+    hasBaseAndOffset(addrOperand, middle, previousBitOffset) and
+    // `middle` is propagated from `base`.
+    middleOperand = middle.getAnOperand() and
+    operandIsPropagatedIncludingByCall(middleOperand, additionalBitOffset) and
+    base = middleOperand.getDef() and
+    bitOffset = Ints::add(previousBitOffset, additionalBitOffset)
+  )
+}
+
+/**
+ * Holds if `addrOperand` is at constant offset `bitOffset` from the value of instruction `base`.
+ * Only holds for the `base` with the longest chain of propagation to `addrOperand`.
+ */
+predicate addressOperandBaseAndConstantOffset(
+  AddressOperand addrOperand, Instruction base, int bitOffset
+) {
+  hasBaseAndOffset(addrOperand, base, bitOffset) and
+  Ints::hasValue(bitOffset) and
+  not exists(Instruction previousBase, int previousBitOffset |
+    hasBaseAndOffset(addrOperand, previousBase, previousBitOffset) and
+    previousBase = base.getAnOperand().getDef() and
+    Ints::hasValue(previousBitOffset)
+  )
+}
+
+/**
+ * Gets the allocation into which `addrOperand` points, if known.
+ */
+Configuration::Allocation getAddressOperandAllocation(AddressOperand addrOperand) {
+  addressOperandAllocationAndOffset(addrOperand, result, _)
+}
+
+/**
+ * Holds if `addrOperand` is at offset `bitOffset` from a base instruction of `allocation`. The
+ * offset may be `unknown()`.
+ */
+predicate addressOperandAllocationAndOffset(
+  AddressOperand addrOperand, Configuration::Allocation allocation, IntValue bitOffset
+) {
+  exists(Instruction base |
+    allocation.getABaseInstruction() = base and
+    hasBaseAndOffset(addrOperand, base, bitOffset) and
+    not exists(Instruction previousBase |
+      hasBaseAndOffset(addrOperand, previousBase, _) and
+      previousBase = base.getAnOperand().getDef()
+    )
   )
 }

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

@@ -0,0 +1,3 @@
+import semmle.code.cpp.ir.implementation.IRConfiguration
+import semmle.code.cpp.ir.internal.IntegerConstant as Ints
+import semmle.code.cpp.models.interfaces.Alias as AliasModels

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

@@ -1 +1,3 @@
+import semmle.code.cpp.ir.internal.IRCppLanguage as Language
 import semmle.code.cpp.ir.implementation.unaliased_ssa.IR as InputIR
+import AliasConfiguration as Configuration