Merge pull request #4558 from rdmarsh2/rdmarsh2/cpp/remove-initialize-nonlocal

Remove InitializeNonlocalInstruction

Author: Dave Bartolomeo

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

@@ -10,6 +10,7 @@ private newtype TMemoryAccessKind =
   TEntireAllocationMemoryAccess() or
   TEscapedMemoryAccess() or
   TNonLocalMemoryAccess() or
+  TEscapedInitializationMemoryAccess() or
   TPhiMemoryAccess() or
   TUnmodeledMemoryAccess() or
   TChiTotalMemoryAccess() or
@@ -76,6 +77,14 @@ class NonLocalMemoryAccess extends MemoryAccessKind, TNonLocalMemoryAccess {
   override string toString() { result = "nonlocal" }
 }
 
+/**
+ * The operand or result accesses all memory whose address has escaped and can define read-only
+ * memory (such as string constants).
+ */
+class EscapedInitializationMemoryAccess extends MemoryAccessKind, TEscapedInitializationMemoryAccess {
+  override string toString() { result = "escaped(init)" }
+}
+
 /**
  * The operand is a Phi operand, which accesses the same memory as its
  * definition.

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

@@ -979,19 +979,8 @@ module Opcode {
   class AliasedDefinition extends Opcode, TAliasedDefinition {
     final override string toString() { result = "AliasedDefinition" }
 
-    final override MemoryAccessKind getWriteMemoryAccess() { result instanceof EscapedMemoryAccess }
-  }
-
-  /**
-   * The `Opcode` for an `InitializeNonLocalInstruction`.
-   *
-   * See the `InitializeNonLocalInstruction` documentation for more details.
-   */
-  class InitializeNonLocal extends Opcode, TInitializeNonLocal {
-    final override string toString() { result = "InitializeNonLocal" }
-
     final override MemoryAccessKind getWriteMemoryAccess() {
-      result instanceof NonLocalMemoryAccess
+      result instanceof EscapedInitializationMemoryAccess
     }
   }
 

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

@@ -441,34 +441,6 @@ module InstructionConsistency {
     isOnAliasedDefinitionChain(instr.(PhiInstruction).getAnInputOperand().getAnyDef())
   }
 
-  private predicate shouldBeConflated(Instruction instr) {
-    isOnAliasedDefinitionChain(instr)
-    or
-    instr.getOpcode() instanceof Opcode::InitializeNonLocal
-  }
-
-  query predicate notMarkedAsConflated(
-    Instruction instr, string message, OptionalIRFunction irFunc, string irFuncText
-  ) {
-    shouldBeConflated(instr) and
-    not instr.isResultConflated() and
-    message =
-      "Instruction '" + instr.toString() +
-        "' should be marked as having a conflated result in function '$@'." and
-    irFunc = getInstructionIRFunction(instr, irFuncText)
-  }
-
-  query predicate wronglyMarkedAsConflated(
-    Instruction instr, string message, OptionalIRFunction irFunc, string irFuncText
-  ) {
-    instr.isResultConflated() and
-    not shouldBeConflated(instr) and
-    message =
-      "Instruction '" + instr.toString() +
-        "' should not be marked as having a conflated result in function '$@'." and
-    irFunc = getInstructionIRFunction(instr, irFuncText)
-  }
-
   query predicate invalidOverlap(
     MemoryOperand useOperand, string message, OptionalIRFunction irFunc, string irFuncText
   ) {

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

@@ -92,6 +92,11 @@ class Instruction extends Construction::TStageInstruction {
       else result = "r"
   }
 
+  private string getConflationPrefix() {
+    shouldGenerateDumpStrings() and
+    if isResultConflated() then result = "%" else result = ""
+  }
+
   /**
    * Gets the zero-based index of this instruction within its block. This is
    * used by debugging and printing code only.
@@ -143,7 +148,8 @@ class Instruction extends Construction::TStageInstruction {
    */
   final string getResultString() {
     shouldGenerateDumpStrings() and
-    result = getResultId() + "(" + getResultLanguageType().getDumpString() + ")"
+    result =
+      getConflationPrefix() + getResultId() + "(" + getResultLanguageType().getDumpString() + ")"
   }
 
   /**
@@ -584,16 +590,6 @@ class InitializeParameterInstruction extends VariableInstruction {
   final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
 }
 
-/**
- * An instruction that initializes all memory that existed before this function was called.
- *
- * This instruction provides a definition for memory that, because it was actually allocated and
- * initialized elsewhere, would not otherwise have a definition in this function.
- */
-class InitializeNonLocalInstruction extends Instruction {
-  InitializeNonLocalInstruction() { getOpcode() instanceof Opcode::InitializeNonLocal }
-}
-
 /**
  * An instruction that initializes the memory pointed to by a parameter of the enclosing function
  * with the value of that memory on entry to the function.

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

@@ -81,8 +81,11 @@ private newtype TMemoryLocation =
   TAllNonLocalMemory(IRFunction irFunc, boolean isMayAccess) {
     isMayAccess = false or isMayAccess = true
   } or
-  TAllAliasedMemory(IRFunction irFunc, boolean isMayAccess) {
-    isMayAccess = false or isMayAccess = true
+  TAllAliasedMemory(IRFunction irFunc, boolean isMayAccess, boolean canDefineReadOnly) {
+    isMayAccess = false and
+    canDefineReadOnly = [true, false]
+    or
+    isMayAccess = true and canDefineReadOnly = false
   }
 
 /**
@@ -154,7 +157,7 @@ abstract class AllocationMemoryLocation extends MemoryLocation {
 
   final override VirtualVariable getVirtualVariable() {
     if allocationEscapes(var)
-    then result = TAllAliasedMemory(var.getEnclosingIRFunction(), false)
+    then result = TAllAliasedMemory(var.getEnclosingIRFunction(), false, true)
     else result.(AllocationMemoryLocation).getAllocation() = var
   }
 
@@ -284,7 +287,9 @@ class UnknownMemoryLocation extends TUnknownMemoryLocation, MemoryLocation {
 
   final override string toStringInternal() { result = "{Unknown}" }
 
-  final override VirtualVariable getVirtualVariable() { result = TAllAliasedMemory(irFunc, false) }
+  final override VirtualVariable getVirtualVariable() {
+    result = TAllAliasedMemory(irFunc, false, true)
+  }
 
   final override Language::LanguageType getType() {
     result = any(IRUnknownType type).getCanonicalLanguageType()
@@ -325,13 +330,7 @@ class AllNonLocalMemory extends TAllNonLocalMemory, MemoryLocation {
 
   final override predicate isMayAccess() { isMayAccess = true }
 
-  override predicate canDefineReadOnly() {
-    // A "must" access that defines all non-local memory appears only on the `InitializeNonLocal`
-    // instruction, which provides the initial definition for all memory outside of the current
-    // function's stack frame. This memory includes string literals and other read-only globals, so
-    // we allow such an access to be the definition for a use of a read-only location.
-    not isMayAccess()
-  }
+  override predicate canDefineReadOnly() { none() }
 }
 
 /**
@@ -340,8 +339,9 @@ class AllNonLocalMemory extends TAllNonLocalMemory, MemoryLocation {
 class AllAliasedMemory extends TAllAliasedMemory, MemoryLocation {
   IRFunction irFunc;
   boolean isMayAccess;
+  boolean canDefineReadOnly;
 
-  AllAliasedMemory() { this = TAllAliasedMemory(irFunc, isMayAccess) }
+  AllAliasedMemory() { this = TAllAliasedMemory(irFunc, isMayAccess, canDefineReadOnly) }
 
   final override string toStringInternal() { result = "{AllAliased}" }
 
@@ -355,14 +355,18 @@ class AllAliasedMemory extends TAllAliasedMemory, MemoryLocation {
 
   final override string getUniqueId() { result = " " + toString() }
 
-  final override VirtualVariable getVirtualVariable() { result = TAllAliasedMemory(irFunc, false) }
+  final override VirtualVariable getVirtualVariable() {
+    result = TAllAliasedMemory(irFunc, false, true)
+  }
 
   final override predicate isMayAccess() { isMayAccess = true }
+
+  final override predicate canDefineReadOnly() { canDefineReadOnly = true }
 }
 
 /** A virtual variable that groups all escaped memory within a function. */
 class AliasedVirtualVariable extends AllAliasedMemory, VirtualVariable {
-  AliasedVirtualVariable() { not isMayAccess() }
+  AliasedVirtualVariable() { not isMayAccess() and canDefineReadOnly() }
 }
 
 /**
@@ -585,7 +589,10 @@ MemoryLocation getResultMemoryLocation(Instruction instr) {
           isMayAccess)
       or
       kind instanceof EscapedMemoryAccess and
-      result = TAllAliasedMemory(instr.getEnclosingIRFunction(), isMayAccess)
+      result = TAllAliasedMemory(instr.getEnclosingIRFunction(), isMayAccess, false)
+      or
+      kind instanceof EscapedInitializationMemoryAccess and
+      result = TAllAliasedMemory(instr.getEnclosingIRFunction(), false, true)
       or
       kind instanceof NonLocalMemoryAccess and
       result = TAllNonLocalMemory(instr.getEnclosingIRFunction(), isMayAccess)
@@ -616,7 +623,10 @@ MemoryLocation getOperandMemoryLocation(MemoryOperand operand) {
           isMayAccess)
       or
       kind instanceof EscapedMemoryAccess and
-      result = TAllAliasedMemory(operand.getEnclosingIRFunction(), isMayAccess)
+      result = TAllAliasedMemory(operand.getEnclosingIRFunction(), isMayAccess, false)
+      or
+      kind instanceof EscapedInitializationMemoryAccess and
+      result = TAllAliasedMemory(operand.getEnclosingIRFunction(), false, true)
       or
       kind instanceof NonLocalMemoryAccess and
       result = TAllNonLocalMemory(operand.getEnclosingIRFunction(), isMayAccess)

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

@@ -68,8 +68,6 @@ private module Cached {
   predicate hasConflatedMemoryResult(Instruction instruction) {
     instruction instanceof AliasedDefinitionInstruction
     or
-    instruction.getOpcode() instanceof Opcode::InitializeNonLocal
-    or
     // Chi instructions track virtual variables, and therefore a chi instruction is
     // conflated if it's associated with the aliased virtual variable.
     exists(OldInstruction oldInstruction | instruction = getChi(oldInstruction) |

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

@@ -441,34 +441,6 @@ module InstructionConsistency {
     isOnAliasedDefinitionChain(instr.(PhiInstruction).getAnInputOperand().getAnyDef())
   }
 
-  private predicate shouldBeConflated(Instruction instr) {
-    isOnAliasedDefinitionChain(instr)
-    or
-    instr.getOpcode() instanceof Opcode::InitializeNonLocal
-  }
-
-  query predicate notMarkedAsConflated(
-    Instruction instr, string message, OptionalIRFunction irFunc, string irFuncText
-  ) {
-    shouldBeConflated(instr) and
-    not instr.isResultConflated() and
-    message =
-      "Instruction '" + instr.toString() +
-        "' should be marked as having a conflated result in function '$@'." and
-    irFunc = getInstructionIRFunction(instr, irFuncText)
-  }
-
-  query predicate wronglyMarkedAsConflated(
-    Instruction instr, string message, OptionalIRFunction irFunc, string irFuncText
-  ) {
-    instr.isResultConflated() and
-    not shouldBeConflated(instr) and
-    message =
-      "Instruction '" + instr.toString() +
-        "' should not be marked as having a conflated result in function '$@'." and
-    irFunc = getInstructionIRFunction(instr, irFuncText)
-  }
-
   query predicate invalidOverlap(
     MemoryOperand useOperand, string message, OptionalIRFunction irFunc, string irFuncText
   ) {

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

@@ -92,6 +92,11 @@ class Instruction extends Construction::TStageInstruction {
       else result = "r"
   }
 
+  private string getConflationPrefix() {
+    shouldGenerateDumpStrings() and
+    if isResultConflated() then result = "%" else result = ""
+  }
+
   /**
    * Gets the zero-based index of this instruction within its block. This is
    * used by debugging and printing code only.
@@ -143,7 +148,8 @@ class Instruction extends Construction::TStageInstruction {
    */
   final string getResultString() {
     shouldGenerateDumpStrings() and
-    result = getResultId() + "(" + getResultLanguageType().getDumpString() + ")"
+    result =
+      getConflationPrefix() + getResultId() + "(" + getResultLanguageType().getDumpString() + ")"
   }
 
   /**
@@ -584,16 +590,6 @@ class InitializeParameterInstruction extends VariableInstruction {
   final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
 }
 
-/**
- * An instruction that initializes all memory that existed before this function was called.
- *
- * This instruction provides a definition for memory that, because it was actually allocated and
- * initialized elsewhere, would not otherwise have a definition in this function.
- */
-class InitializeNonLocalInstruction extends Instruction {
-  InitializeNonLocalInstruction() { getOpcode() instanceof Opcode::InitializeNonLocal }
-}
-
 /**
  * An instruction that initializes the memory pointed to by a parameter of the enclosing function
  * with the value of that memory on entry to the function.

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

@@ -166,8 +166,6 @@ predicate hasModeledMemoryResult(Instruction instruction) { none() }
 
 predicate hasConflatedMemoryResult(Instruction instruction) {
   instruction instanceof AliasedDefinitionInstruction
-  or
-  instruction.getOpcode() instanceof Opcode::InitializeNonLocal
 }
 
 Instruction getRegisterOperandDefinition(Instruction instruction, RegisterOperandTag tag) {

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

@@ -28,7 +28,6 @@ newtype TInstructionTag =
   ReturnTag() or
   ExitFunctionTag() or
   AliasedDefinitionTag() or
-  InitializeNonLocalTag() or
   AliasedUseTag() or
   SwitchBranchTag() or
   CallTargetTag() or
@@ -128,8 +127,6 @@ string getInstructionTagId(TInstructionTag tag) {
   or
   tag = AliasedDefinitionTag() and result = "AliasedDef"
   or
-  tag = InitializeNonLocalTag() and result = "InitNonLocal"
-  or
   tag = AliasedUseTag() and result = "AliasedUse"
   or
   tag = SwitchBranchTag() and result = "SwitchBranch"