Merge pull request #2667 from dbartol/dbartol/NoEscape

C++/C#: Make escape analysis unsound by default

Author: Robert Marsh

config/identical-files.json

@@ -82,6 +82,14 @@
     "cpp/ql/src/semmle/code/cpp/ir/implementation/IRType.qll",
     "csharp/ql/src/semmle/code/csharp/ir/implementation/IRType.qll"
   ],
+  "IR IRConfiguration": [
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/IRConfiguration.qll",
+    "csharp/ql/src/semmle/code/csharp/ir/implementation/IRConfiguration.qll"
+  ],
+  "IR UseSoundEscapeAnalysis": [
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/UseSoundEscapeAnalysis.qll",
+    "csharp/ql/src/semmle/code/csharp/ir/implementation/UseSoundEscapeAnalysis.qll"
+  ],
   "IR Operand Tag": [
     "cpp/ql/src/semmle/code/cpp/ir/implementation/internal/OperandTag.qll",
     "csharp/ql/src/semmle/code/csharp/ir/implementation/internal/OperandTag.qll"

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

@@ -1,3 +1,7 @@
+/**
+ * Module used to configure the IR generation process.
+ */
+
 private import internal.IRConfigurationInternal
 
 private newtype TIRConfiguration = MkIRConfiguration()
@@ -13,3 +17,18 @@ class IRConfiguration extends TIRConfiguration {
    */
   predicate shouldCreateIRForFunction(Language::Function func) { any() }
 }
+
+private newtype TIREscapeAnalysisConfiguration = MkIREscapeAnalysisConfiguration()
+
+/**
+ * The query can extend this class to control what escape analysis is used when generating SSA.
+ */
+class IREscapeAnalysisConfiguration extends TIREscapeAnalysisConfiguration {
+  string toString() { result = "IREscapeAnalysisConfiguration" }
+
+  /**
+   * Holds if the escape analysis done by SSA construction should be sound. By default, the SSA is
+   * built assuming that no variable's address ever escapes.
+   */
+  predicate useSoundEscapeAnalysis() { none() }
+}

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

@@ -0,0 +1,9 @@
+import IRConfiguration
+
+/**
+ * Overrides the default IR configuration to use sound escape analysis, instead of assuming that
+ * variable addresses never escape.
+ */
+class SoundEscapeAnalysisConfiguration extends IREscapeAnalysisConfiguration {
+  override predicate useSoundEscapeAnalysis() { any() }
+}

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

@@ -2,6 +2,7 @@ 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 class IntValue = Ints::IntValue;
@@ -277,9 +278,14 @@ private predicate automaticVariableAddressEscapes(IRAutomaticVariable var) {
  * analysis.
  */
 predicate variableAddressEscapes(IRVariable var) {
-  automaticVariableAddressEscapes(var.(IRAutomaticVariable))
+  exists(IREscapeAnalysisConfiguration config |
+    config.useSoundEscapeAnalysis() and
+    automaticVariableAddressEscapes(var.(IRAutomaticVariable))
+  )
   or
-  // All variables with static storage duration have their address escape.
+  // 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
 }
 

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

@@ -107,7 +107,7 @@ class PropertyProvider extends IRPropertyProvider {
     exists(
       MemoryLocation useLocation, IRBlock predBlock, IRBlock defBlock, int defIndex, Overlap overlap
     |
-      hasPhiOperandDefinition(_, useLocation, block, predBlock, defBlock, defIndex, overlap) and
+      hasPhiOperandDefinition(_, useLocation, block, predBlock, defBlock, defIndex) and
       key = "PhiUse[" + useLocation.toString() + " from " + predBlock.getDisplayIndex().toString() +
           "]" and
       result = defBlock.getDisplayIndex().toString() + "_" + defIndex + " (" + overlap.toString() +

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

@@ -43,93 +43,34 @@ abstract class TranslatedDeclarationEntry extends TranslatedElement, TTranslated
  * Represents the IR translation of the declaration of a local variable,
  * including its initialization, if any.
  */
-abstract class TranslatedVariableDeclaration extends TranslatedElement, InitializationContext {
+abstract class TranslatedLocalVariableDeclaration extends TranslatedVariableInitialization {
   /**
    * Gets the local variable being declared.
    */
   abstract LocalVariable getVariable();
 
-  override TranslatedElement getChild(int id) { id = 0 and result = getInitialization() }
+  final override Type getTargetType() { result = getVariableType(getVariable()) }
 
-  override Instruction getFirstInstruction() {
-    result = getInstruction(InitializerVariableAddressTag())
-  }
-
-  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
-    tag = InitializerVariableAddressTag() and
-    opcode instanceof Opcode::VariableAddress and
-    resultType = getTypeForGLValue(getVariableType(getVariable()))
-    or
-    hasUninitializedInstruction() and
-    tag = InitializerStoreTag() and
-    opcode instanceof Opcode::Uninitialized and
-    resultType = getTypeForPRValue(getVariableType(getVariable()))
-  }
-
-  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
-    (
-      tag = InitializerVariableAddressTag() and
-      kind instanceof GotoEdge and
-      if hasUninitializedInstruction()
-      then result = getInstruction(InitializerStoreTag())
-      else result = getInitialization().getFirstInstruction()
-    )
-    or
-    hasUninitializedInstruction() and
-    kind instanceof GotoEdge and
-    tag = InitializerStoreTag() and
-    (
-      result = getInitialization().getFirstInstruction()
-      or
-      not exists(getInitialization()) and result = getParent().getChildSuccessor(this)
-    )
-  }
-
-  override Instruction getChildSuccessor(TranslatedElement child) {
-    child = getInitialization() and result = getParent().getChildSuccessor(this)
-  }
-
-  override IRVariable getInstructionVariable(InstructionTag tag) {
-    (
-      tag = InitializerVariableAddressTag()
-      or
-      hasUninitializedInstruction() and tag = InitializerStoreTag()
-    ) and
-    result = getIRUserVariable(getFunction(), getVariable())
-  }
-
-  override Instruction getInstructionOperand(InstructionTag tag, OperandTag operandTag) {
-    hasUninitializedInstruction() and
-    tag = InitializerStoreTag() and
-    operandTag instanceof AddressOperandTag and
-    result = getInstruction(InitializerVariableAddressTag())
-  }
-
-  override Instruction getTargetAddress() {
-    result = getInstruction(InitializerVariableAddressTag())
-  }
-
-  override Type getTargetType() { result = getVariableType(getVariable()) }
-
-  private TranslatedInitialization getInitialization() {
+  final override TranslatedInitialization getInitialization() {
     result = getTranslatedInitialization(getVariable()
             .getInitializer()
             .getExpr()
             .getFullyConverted())
   }
 
-  private predicate hasUninitializedInstruction() {
-    not exists(getInitialization()) or
-    getInitialization() instanceof TranslatedListInitialization or
-    getInitialization() instanceof TranslatedConstructorInitialization or
-    getInitialization().(TranslatedStringLiteralInitialization).zeroInitRange(_, _)
+  final override Instruction getInitializationSuccessor() {
+    result = getParent().getChildSuccessor(this)
+  }
+
+  final override IRVariable getIRVariable() {
+    result = getIRUserVariable(getFunction(), getVariable())
   }
 }
 
 /**
  * Represents the IR translation of a local variable declaration within a declaration statement.
  */
-class TranslatedVariableDeclarationEntry extends TranslatedVariableDeclaration,
+class TranslatedVariableDeclarationEntry extends TranslatedLocalVariableDeclaration,
   TranslatedDeclarationEntry {
   LocalVariable var;
 
@@ -151,7 +92,7 @@ TranslatedRangeBasedForVariableDeclaration getTranslatedRangeBasedForVariableDec
 /**
  * Represents the IR translation of a compiler-generated variable in a range-based `for` loop.
  */
-class TranslatedRangeBasedForVariableDeclaration extends TranslatedVariableDeclaration,
+class TranslatedRangeBasedForVariableDeclaration extends TranslatedLocalVariableDeclaration,
   TTranslatedRangeBasedForVariableDeclaration {
   RangeBasedForStmt forStmt;
   LocalVariable var;
@@ -181,7 +122,7 @@ TranslatedConditionDecl getTranslatedConditionDecl(ConditionDeclExpr expr) {
  * }
  * ```
  */
-class TranslatedConditionDecl extends TranslatedVariableDeclaration, TTranslatedConditionDecl {
+class TranslatedConditionDecl extends TranslatedLocalVariableDeclaration, TTranslatedConditionDecl {
   ConditionDeclExpr conditionDeclExpr;
 
   TranslatedConditionDecl() { this = TTranslatedConditionDecl(conditionDeclExpr) }

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

@@ -1932,47 +1932,31 @@ abstract class TranslatedThrowExpr extends TranslatedNonConstantExpr {
  * IR translation of a `throw` expression with an argument
  * (e.g. `throw std::bad_alloc()`).
  */
-class TranslatedThrowValueExpr extends TranslatedThrowExpr, InitializationContext {
+class TranslatedThrowValueExpr extends TranslatedThrowExpr, TranslatedVariableInitialization {
   TranslatedThrowValueExpr() { not expr instanceof ReThrowExpr }
 
-  override TranslatedElement getChild(int id) { id = 0 and result = getInitialization() }
-
-  override Instruction getFirstInstruction() {
-    result = getInstruction(InitializerVariableAddressTag())
-  }
-
   override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
     TranslatedThrowExpr.super.hasInstruction(opcode, tag, resultType)
     or
-    tag = InitializerVariableAddressTag() and
-    opcode instanceof Opcode::VariableAddress and
-    resultType = getTypeForGLValue(getExceptionType())
+    TranslatedVariableInitialization.super.hasInstruction(opcode, tag, resultType)
   }
 
   override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
     result = TranslatedThrowExpr.super.getInstructionSuccessor(tag, kind)
     or
-    tag = InitializerVariableAddressTag() and
-    result = getInitialization().getFirstInstruction() and
-    kind instanceof GotoEdge
-  }
-
-  override Instruction getChildSuccessor(TranslatedElement child) {
-    child = getInitialization() and
-    result = getInstruction(ThrowTag())
+    result = TranslatedVariableInitialization.super.getInstructionSuccessor(tag, kind)
   }
 
-  override IRVariable getInstructionVariable(InstructionTag tag) {
-    tag = InitializerVariableAddressTag() and
-    result = getIRTempVariable(expr, ThrowTempVar())
-  }
+  final override Instruction getInitializationSuccessor() { result = getInstruction(ThrowTag()) }
 
   final override predicate hasTempVariable(TempVariableTag tag, CppType type) {
     tag = ThrowTempVar() and
     type = getTypeForPRValue(getExceptionType())
   }
 
   final override Instruction getInstructionOperand(InstructionTag tag, OperandTag operandTag) {
+    result = TranslatedVariableInitialization.super.getInstructionOperand(tag, operandTag)
+    or
     tag = ThrowTag() and
     (
       operandTag instanceof AddressOperandTag and
@@ -1989,16 +1973,14 @@ class TranslatedThrowValueExpr extends TranslatedThrowExpr, InitializationContex
     result = getTypeForPRValue(getExceptionType())
   }
 
-  override Instruction getTargetAddress() {
-    result = getInstruction(InitializerVariableAddressTag())
-  }
-
   override Type getTargetType() { result = getExceptionType() }
 
-  TranslatedInitialization getInitialization() {
+  final override TranslatedInitialization getInitialization() {
     result = getTranslatedInitialization(expr.getExpr().getFullyConverted())
   }
 
+  final override IRVariable getIRVariable() { result = getIRTempVariable(expr, ThrowTempVar()) }
+
   final override Opcode getThrowOpcode() { result instanceof Opcode::ThrowValue }
 
   private Type getExceptionType() { result = expr.getType() }

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

@@ -30,6 +30,98 @@ abstract class InitializationContext extends TranslatedElement {
   abstract Type getTargetType();
 }
 
+/**
+ * Base class for any element that initializes a stack variable. Examples include local variable
+ * declarations, `return` statements, and `throw` expressions.
+ */
+abstract class TranslatedVariableInitialization extends TranslatedElement, InitializationContext {
+  final override TranslatedElement getChild(int id) { id = 0 and result = getInitialization() }
+
+  final override Instruction getFirstInstruction() {
+    result = getInstruction(InitializerVariableAddressTag())
+  }
+
+  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
+    tag = InitializerVariableAddressTag() and
+    opcode instanceof Opcode::VariableAddress and
+    resultType = getTypeForGLValue(getTargetType())
+    or
+    hasUninitializedInstruction() and
+    tag = InitializerStoreTag() and
+    opcode instanceof Opcode::Uninitialized and
+    resultType = getTypeForPRValue(getTargetType())
+  }
+
+  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
+    (
+      tag = InitializerVariableAddressTag() and
+      kind instanceof GotoEdge and
+      if hasUninitializedInstruction()
+      then result = getInstruction(InitializerStoreTag())
+      else result = getInitialization().getFirstInstruction()
+    )
+    or
+    hasUninitializedInstruction() and
+    kind instanceof GotoEdge and
+    tag = InitializerStoreTag() and
+    (
+      result = getInitialization().getFirstInstruction()
+      or
+      not exists(getInitialization()) and result = getInitializationSuccessor()
+    )
+  }
+
+  final override Instruction getChildSuccessor(TranslatedElement child) {
+    child = getInitialization() and result = getInitializationSuccessor()
+  }
+
+  override Instruction getInstructionOperand(InstructionTag tag, OperandTag operandTag) {
+    hasUninitializedInstruction() and
+    tag = InitializerStoreTag() and
+    operandTag instanceof AddressOperandTag and
+    result = getInstruction(InitializerVariableAddressTag())
+  }
+
+  final override IRVariable getInstructionVariable(InstructionTag tag) {
+    (
+      tag = InitializerVariableAddressTag()
+      or
+      hasUninitializedInstruction() and tag = InitializerStoreTag()
+    ) and
+    result = getIRVariable()
+  }
+
+  final override Instruction getTargetAddress() {
+    result = getInstruction(InitializerVariableAddressTag())
+  }
+
+  /**
+   * Get the initialization for the variable.
+   */
+  abstract TranslatedInitialization getInitialization();
+
+  /**
+   * Get the `IRVariable` to be initialized. This may be an `IRTempVariable`.
+   */
+  abstract IRVariable getIRVariable();
+
+  /**
+   * Gets the `Instruction` to be executed immediately after the initialization.
+   */
+  abstract Instruction getInitializationSuccessor();
+
+  /**
+   * Holds if this initialization requires an `Uninitialized` instruction to be emitted before
+   * evaluating the initializer.
+   */
+  final predicate hasUninitializedInstruction() {
+    not exists(getInitialization()) or
+    getInitialization() instanceof TranslatedListInitialization or
+    getInitialization() instanceof TranslatedConstructorInitialization or
+    getInitialization().(TranslatedStringLiteralInitialization).zeroInitRange(_, _)
+  }
+}
+
 /**
  * Represents the IR translation of any initialization, whether from an
  * initializer list or from a direct initializer.