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

When building SSA, we'll be assuming that stack variables do not escape, at least until we improve our alias analysis. I've added a new `IREscapeAnalysisConfiguration` class to allow the query to control this, and a new `UseSoundEscapeAnalysis.qll` module that can be imported to switch to the sound escape analysis. I've cloned the existing IR and SSA tests to have both sound and unsound versions. There were relatively few diffs in the IR dump tests, and the sanity tests still give the same results after one change described below.

Assuming that stack variables do not escape exposed an existing bug where we do not emit an `Uninitialized` instruction for the temporary variables used by `return` statements and `throw` expressions, even if the initializer is a constructor call or array initializer. I've refactored the code for handling elements that initialize a variable to share a common base class. I added a test case for returning an object initialized by constructor call, and ensured that the IR diffs for the existing `throw` test cases are correct.

Author: Dave Bartolomeo

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,16 @@ 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,33 @@ 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
+    result = TranslatedVariableInitialization.super.getInstructionSuccessor(tag, kind)
   }
 
-  override Instruction getChildSuccessor(TranslatedElement child) {
-    child = getInitialization() and
+  final override Instruction getInitializationSuccessor() {
     result = getInstruction(ThrowTag())
   }
 
-  override IRVariable getInstructionVariable(InstructionTag tag) {
-    tag = InitializerVariableAddressTag() and
-    result = getIRTempVariable(expr, ThrowTempVar())
-  }
-
   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 +1975,16 @@ 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.