Merge pull request github#3459 from dbartol/github/codeql-c-analysis-team/69

C++/C#: Remove `UnmodeledUse` instruction

Author: Dave Bartolomeo

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

@@ -61,7 +61,6 @@ private newtype TOpcode =
   TReThrow() or
   TUnwind() or
   TUnmodeledDefinition() or
-  TUnmodeledUse() or
   TAliasedDefinition() or
   TInitializeNonLocal() or
   TAliasedUse() or
@@ -587,14 +586,6 @@ module Opcode {
     }
   }
 
-  class UnmodeledUse extends Opcode, TUnmodeledUse {
-    final override string toString() { result = "UnmodeledUse" }
-
-    final override predicate hasOperandInternal(OperandTag tag) {
-      tag instanceof UnmodeledUseOperandTag
-    }
-  }
-
   class AliasedDefinition extends Opcode, TAliasedDefinition {
     final override string toString() { result = "AliasedDefinition" }
 

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

@@ -31,10 +31,14 @@ class IRBlockBase extends TIRBlock {
       config.shouldEvaluateDebugStringsForFunction(this.getEnclosingFunction())
     ) and
     this =
-      rank[result + 1](IRBlock funcBlock |
-        funcBlock.getEnclosingFunction() = getEnclosingFunction()
+      rank[result + 1](IRBlock funcBlock, int sortOverride |
+        funcBlock.getEnclosingFunction() = getEnclosingFunction() and
+        // Ensure that the block containing `EnterFunction` always comes first.
+        if funcBlock.getFirstInstruction() instanceof EnterFunctionInstruction
+        then sortOverride = 0
+        else sortOverride = 1
       |
-        funcBlock order by funcBlock.getUniqueId()
+        funcBlock order by sortOverride, funcBlock.getUniqueId()
       )
   }
 

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

@@ -55,7 +55,6 @@ module InstructionConsistency {
           operand.getOperandTag() = tag
         ) and
       operandCount > 1 and
-      not tag instanceof UnmodeledUseOperandTag and
       message =
         "Instruction has " + operandCount + " operands with tag '" + tag.toString() + "'" +
           " in function '$@'." and
@@ -158,7 +157,6 @@ module InstructionConsistency {
   ) {
     exists(MemoryOperand operand, Instruction def |
       operand = instr.getAnOperand() and
-      not operand instanceof UnmodeledUseOperand and
       def = operand.getAnyDef() and
       not def.isResultModeled() and
       not def instanceof UnmodeledDefinitionInstruction and
@@ -259,7 +257,6 @@ module InstructionConsistency {
     Operand useOperand, string message, IRFunction func, string funcText
   ) {
     exists(IRBlock useBlock, int useIndex, Instruction defInstr, IRBlock defBlock, int defIndex |
-      not useOperand.getUse() instanceof UnmodeledUseInstruction and
       not defInstr instanceof UnmodeledDefinitionInstruction and
       pointOfEvaluation(useOperand, useBlock, useIndex) and
       defInstr = useOperand.getAnyDef() and

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

@@ -45,11 +45,6 @@ class IRFunction extends TIRFunction {
     result.getEnclosingIRFunction() = this
   }
 
-  pragma[noinline]
-  final UnmodeledUseInstruction getUnmodeledUseInstruction() {
-    result.getEnclosingIRFunction() = this
-  }
-
   /**
    * Gets the single return instruction for this function.
    */

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

@@ -320,8 +320,7 @@ class Instruction extends Construction::TInstruction {
   /**
    * Holds if the result of this instruction is precisely modeled in SSA. Always
    * holds for a register result. For a memory result, a modeled result is
-   * connected to its actual uses. An unmodeled result is connected to the
-   * `UnmodeledUse` instruction.
+   * connected to its actual uses. An unmodeled result has no uses.
    *
    * For example:
    * ```
@@ -1248,12 +1247,6 @@ class AliasedUseInstruction extends Instruction {
   AliasedUseInstruction() { getOpcode() instanceof Opcode::AliasedUse }
 }
 
-class UnmodeledUseInstruction extends Instruction {
-  UnmodeledUseInstruction() { getOpcode() instanceof Opcode::UnmodeledUse }
-
-  override string getOperandsString() { result = "mu*" }
-}
-
 /**
  * An instruction representing the choice of one of multiple input values based on control flow.
  *

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

@@ -19,11 +19,7 @@ private newtype TOperand =
   ) {
     defInstr = Construction::getMemoryOperandDefinition(useInstr, tag, overlap) and
     not Construction::isInCycle(useInstr) and
-    (
-      strictcount(Construction::getMemoryOperandDefinition(useInstr, tag, _)) = 1
-      or
-      tag instanceof UnmodeledUseOperandTag
-    )
+    strictcount(Construction::getMemoryOperandDefinition(useInstr, tag, _)) = 1
   } or
   TPhiOperand(
     PhiInstruction useInstr, Instruction defInstr, IRBlock predecessorBlock, Overlap overlap
@@ -327,16 +323,6 @@ class ConditionOperand extends RegisterOperand {
   override string toString() { result = "Condition" }
 }
 
-/**
- * An operand of the special `UnmodeledUse` instruction, representing a value
- * whose set of uses is unknown.
- */
-class UnmodeledUseOperand extends NonPhiMemoryOperand {
-  override UnmodeledUseOperandTag tag;
-
-  override string toString() { result = "UnmodeledUse" }
-}
-
 /**
  * The operand representing the target function of an `Call` instruction.
  */

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

@@ -247,6 +247,10 @@ private predicate resultMayReachReturn(Instruction instr) { operandMayReachRetur
 private predicate resultEscapesNonReturn(Instruction instr) {
   // The result escapes if it has at least one use that escapes.
   operandEscapesNonReturn(instr.getAUse())
+  or
+  // The result also escapes if it is not modeled in SSA, because we do not know where it might be
+  // used.
+  not instr.isResultModeled()
 }
 
 /**

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

@@ -128,23 +128,10 @@ private module Cached {
       oldOperand = oldInstruction.getAnOperand() and
       tag = oldOperand.getOperandTag() and
       (
-        (
-          if exists(Alias::getOperandMemoryLocation(oldOperand))
-          then hasMemoryOperandDefinition(oldInstruction, oldOperand, overlap, result)
-          else (
-            result = instruction.getEnclosingIRFunction().getUnmodeledDefinitionInstruction() and
-            overlap instanceof MustTotallyOverlap
-          )
-        )
-        or
-        // Connect any definitions that are not being modeled in SSA to the
-        // `UnmodeledUse` instruction.
-        exists(OldInstruction oldDefinition |
-          instruction instanceof UnmodeledUseInstruction and
-          tag instanceof UnmodeledUseOperandTag and
-          oldDefinition = oldOperand.getAnyDef() and
-          not exists(Alias::getResultMemoryLocation(oldDefinition)) and
-          result = getNewInstruction(oldDefinition) and
+        if exists(Alias::getOperandMemoryLocation(oldOperand))
+        then hasMemoryOperandDefinition(oldInstruction, oldOperand, overlap, result)
+        else (
+          result = instruction.getEnclosingIRFunction().getUnmodeledDefinitionInstruction() and
           overlap instanceof MustTotallyOverlap
         )
       )
@@ -154,13 +141,6 @@ private module Cached {
     tag instanceof ChiPartialOperandTag and
     overlap instanceof MustExactlyOverlap
     or
-    exists(IRFunction f |
-      tag instanceof UnmodeledUseOperandTag and
-      result = f.getUnmodeledDefinitionInstruction() and
-      instruction = f.getUnmodeledUseInstruction() and
-      overlap instanceof MustTotallyOverlap
-    )
-    or
     tag instanceof ChiTotalOperandTag and
     result = getChiInstructionTotalOperand(instruction) and
     overlap instanceof MustExactlyOverlap

cpp/ql/src/semmle/code/cpp/ir/implementation/internal/OperandTag.qll

@@ -15,7 +15,6 @@ private newtype TOperandTag =
   TLeftOperand() or
   TRightOperand() or
   TConditionOperand() or
-  TUnmodeledUseOperand() or
   TCallTargetOperand() or
   TThisArgumentOperand() or
   TPositionalArgumentOperand(int argIndex) { Language::hasPositionalArgIndex(argIndex) } or
@@ -165,18 +164,6 @@ class ConditionOperandTag extends RegisterOperandTag, TConditionOperand {
 
 ConditionOperandTag conditionOperand() { result = TConditionOperand() }
 
-/**
- * An operand of the special `UnmodeledUse` instruction, representing a value
- * whose set of uses is unknown.
- */
-class UnmodeledUseOperandTag extends MemoryOperandTag, TUnmodeledUseOperand {
-  final override string toString() { result = "UnmodeledUse" }
-
-  final override int getSortOrder() { result = 9 }
-}
-
-UnmodeledUseOperandTag unmodeledUseOperand() { result = TUnmodeledUseOperand() }
-
 /**
  * The operand representing the target function of an `Call` instruction.
  */

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

@@ -31,10 +31,14 @@ class IRBlockBase extends TIRBlock {
       config.shouldEvaluateDebugStringsForFunction(this.getEnclosingFunction())
     ) and
     this =
-      rank[result + 1](IRBlock funcBlock |
-        funcBlock.getEnclosingFunction() = getEnclosingFunction()
+      rank[result + 1](IRBlock funcBlock, int sortOverride |
+        funcBlock.getEnclosingFunction() = getEnclosingFunction() and
+        // Ensure that the block containing `EnterFunction` always comes first.
+        if funcBlock.getFirstInstruction() instanceof EnterFunctionInstruction
+        then sortOverride = 0
+        else sortOverride = 1
       |
-        funcBlock order by funcBlock.getUniqueId()
+        funcBlock order by sortOverride, funcBlock.getUniqueId()
       )
   }