C# IR: Object creation refactoring

The way object creation was translated has been changed: now creations are treated as expressions.
The main motivation for this was the inability to have creation expressions as arguments to
function calls (a test case has been added to showcase this).
All code that dealt with creation expressions has been moved from `TranslatedInitialization.qll` to `TranslatedExpr.qll`.
Some light refactoring has also been done, mainly removing code that was useless after the changes mentioned above.

Author: AndreiDiaconu1

csharp/ql/src/semmle/code/csharp/ir/implementation/raw/internal/TranslatedElement.qll

@@ -169,6 +169,10 @@ newtype TTranslatedElement =
     not isNativeCondition(expr) and
     not isFlexibleCondition(expr)
   } or
+  // A creation expression
+  TTranslatedCreationExpr(Expr expr) {
+    not ignoreExpr(expr)
+  } or
   // A separate element to handle the lvalue-to-rvalue conversion step of an
   // expression.
   TTranslatedLoad(Expr expr) {
@@ -219,32 +223,21 @@ newtype TTranslatedElement =
       // we deal with all the types of initialization separately.
       // First only simple local variable initialization (ie. `int x = 0`)
       exists(LocalVariableDeclAndInitExpr lvInit |
-        lvInit.getInitializer() = expr and
-        not expr instanceof ArrayCreation and
-        not expr instanceof ObjectCreation and
-        not expr instanceof DelegateCreation
+        lvInit.getInitializer() = expr
       )
       or
       // Then treat more complex ones
-      expr instanceof ObjectCreation
-      or
-      expr instanceof DelegateCreation
-      or
       expr instanceof ArrayInitializer
       or
       expr instanceof ObjectInitializer
       or
-      expr = any(ThrowExpr throw).getExpr()
+      expr = any(ThrowStmt throwStmt).getExpr()
       or
       expr = any(CollectionInitializer colInit).getAnElementInitializer()
       or
       expr = any(ReturnStmt returnStmt).getExpr()
       or
       expr = any(ArrayInitializer arrInit).getAnElement()
-      or
-      expr = any(LambdaExpr lambda).getSourceDeclaration()
-      or
-      expr = any(AnonymousMethodExpr anonMethExpr).getSourceDeclaration()
     )
   } or
   // The initialization of an array element via a member of an initializer list.

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

@@ -24,7 +24,11 @@ private import semmle.code.csharp.ir.internal.IRCSharpLanguage as Language
  */
 TranslatedExpr getTranslatedExpr(Expr expr) {
   result.getExpr() = expr and
-  result.producesExprResult()
+  result.producesExprResult() and
+  // When a constructor call is needed, we fetch it manually.
+  // This is because of how we translate object creations: the translated expression
+  // and the translated constructor call are attached to the same element.
+  (expr instanceof ObjectCreation implies not result instanceof TranslatedConstructorCall)
 }
 
 /**
@@ -1918,3 +1922,91 @@ class TranslatedDelegateCall extends TranslatedNonConstantExpr {
     result = DelegateElements::getInvoke(expr)
   }
 }
+
+/**
+ * Represents the IR translation of creation expression. Can be the translation of an
+ * `ObjectCreation` or a `DelegateCreation`.
+ * The `NewObj` instruction denotes the fact that during initialization a new
+ * object is allocated, which is then initialized by the constructor.
+ */
+abstract class TranslatedCreation extends TranslatedCoreExpr, TTranslatedCreationExpr,
+  ConstructorCallContext {
+  TranslatedCreation() { this = TTranslatedCreationExpr(expr) }
+
+  override TranslatedElement getChild(int id) {
+    id = 0 and result = this.getConstructorCall()
+    or
+    id = 1 and result = this.getInitializerExpr()
+  }
+
+  override predicate hasInstruction(
+    Opcode opcode, InstructionTag tag, Type resultType, boolean isLValue
+  ) {
+    // Instruction that allocated space for a new object,
+    // and returns its address
+    tag = NewObjTag() and
+    opcode instanceof Opcode::NewObj and
+    resultType = expr.getType() and
+    isLValue = false
+  }
+
+  final override Instruction getFirstInstruction() { result = this.getInstruction(NewObjTag()) }
+
+  override Instruction getResult() { result = getInstruction(NewObjTag()) }
+
+  override Instruction getReceiver() { result = getInstruction(NewObjTag()) }
+
+  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
+    kind instanceof GotoEdge and
+    tag = NewObjTag() and
+    result = this.getConstructorCall().getFirstInstruction()
+  }
+
+  override Instruction getChildSuccessor(TranslatedElement child) {
+    (
+      child = this.getConstructorCall() and
+      if exists(this.getInitializerExpr())
+      then result = this.getInitializerExpr().getFirstInstruction()
+      else result = this.getParent().getChildSuccessor(this)
+    )
+    or
+    child = this.getInitializerExpr() and
+    result = this.getParent().getChildSuccessor(this)
+  }
+
+  abstract TranslatedElement getConstructorCall();
+
+  abstract TranslatedExpr getInitializerExpr();
+}
+
+/**
+ * Represents the IR translation of an `ObjectCreation`.
+ */
+class TranslatedObjectCreation extends TranslatedCreation {
+  override ObjectCreation expr;
+
+  override TranslatedExpr getInitializerExpr() { result = getTranslatedExpr(expr.getInitializer()) }
+
+  override TranslatedExpr getConstructorCall() {
+    // Since calls are also expressions, we can't
+    // use the predicate getTranslatedExpr (since that would
+    // also return `this`).
+    exists(TranslatedConstructorCall cc |
+      cc.getAST() = this.getAST() and
+      result = cc
+    )
+  }
+}
+
+/**
+ * Represents the IR translation of a `DelegateCreation`.
+ */
+class TranslatedDelegateCreation extends TranslatedCreation {
+  override DelegateCreation expr;
+
+  override TranslatedExpr getInitializerExpr() { none() }
+
+  override TranslatedElement getConstructorCall() {
+    result = DelegateElements::getConstructor(expr)
+  }
+}

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

@@ -123,13 +123,9 @@ class TranslatedArrayListInitialization extends TranslatedListInitialization {
  */
 class TranslatedDirectInitialization extends TranslatedInitialization {
   TranslatedDirectInitialization() {
-    // TODO: Make sure this is complete and correct
     not expr instanceof ArrayInitializer and
     not expr instanceof ObjectInitializer and
-    not expr instanceof DelegateCreation and
-    not expr instanceof CollectionInitializer and
-    not expr instanceof ObjectCreation and
-    not expr instanceof StringLiteral
+    not expr instanceof CollectionInitializer
   }
 
   override TranslatedElement getChild(int id) { id = 0 and result = this.getInitializer() }
@@ -145,64 +141,6 @@ class TranslatedDirectInitialization extends TranslatedInitialization {
     opcode instanceof Opcode::Store and
     resultType = this.getContext().getTargetType() and
     isLValue = false
-  }
-
-  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
-    tag = InitializerStoreTag() and
-    result = this.getParent().getChildSuccessor(this) and
-    kind instanceof GotoEdge
-  }
-
-  override Instruction getChildSuccessor(TranslatedElement child) {
-    child = this.getInitializer() and result = this.getInstruction(InitializerStoreTag())
-  }
-
-  override Instruction getInstructionOperand(InstructionTag tag, OperandTag operandTag) {
-    tag = InitializerStoreTag() and
-    (
-      operandTag instanceof AddressOperandTag and
-      result = this.getContext().getTargetAddress()
-      or
-      operandTag instanceof StoreValueOperandTag and
-      result = this.getInitializer().getResult()
-    )
-  }
-
-  TranslatedExpr getInitializer() { result = getTranslatedExpr(expr) }
-}
-
-/**
- * Represents the IR translation of an initialization from a constructor.
- * The `NewObj` instruction denotes the fact that during initialization a new
- * object of type `expr.getType()` is allocated, which is then initialized by the
- * constructor.
- */
-class TranslatedObjectInitialization extends TranslatedInitialization, ConstructorCallContext {
-  override ObjectCreation expr;
-
-  override TranslatedElement getChild(int id) {
-    id = 0 and result = this.getConstructorCall()
-    or
-    id = 1 and result = this.getInitializerExpr()
-  }
-
-  override predicate hasInstruction(
-    Opcode opcode, InstructionTag tag, Type resultType, boolean isLValue
-  ) {
-    // Instruction that allocated space for a new object,
-    // and returns its address
-    tag = NewObjTag() and
-    opcode instanceof Opcode::NewObj and
-    resultType = expr.getType() and
-    isLValue = false
-    or
-    // Store op used to assign the variable that
-    // is initialized the address of the newly allocated
-    // object
-    tag = InitializerStoreTag() and
-    opcode instanceof Opcode::Store and
-    resultType = expr.getType() and
-    isLValue = false
     or
     needsConversion() and
     tag = AssignmentConvertRightTag() and
@@ -214,39 +152,22 @@ class TranslatedObjectInitialization extends TranslatedInitialization, Construct
     isLValue = false
   }
 
-  final override Instruction getFirstInstruction() { result = this.getInstruction(NewObjTag()) }
-
   override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
-    kind instanceof GotoEdge and
-    (
-      tag = NewObjTag() and
-      result = this.getConstructorCall().getFirstInstruction()
-      or
-      tag = InitializerStoreTag() and
-      result = this.getParent().getChildSuccessor(this)
-      or
-      tag = AssignmentConvertRightTag() and
-      result = this.getInstruction(InitializerStoreTag())
-    )
+    tag = InitializerStoreTag() and
+    result = this.getParent().getChildSuccessor(this) and
+    kind instanceof GotoEdge
+    or
+    needsConversion() and
+    tag = AssignmentConvertRightTag() and
+    result = getInstruction(InitializerStoreTag()) and
+    kind instanceof GotoEdge
   }
 
   override Instruction getChildSuccessor(TranslatedElement child) {
-    (
-      child = this.getConstructorCall() and
-      if exists(this.getInitializerExpr())
-      then result = this.getInitializerExpr().getFirstInstruction()
-      else
-        if this.needsConversion()
-        then result = this.getInstruction(AssignmentConvertRightTag())
-        else result = this.getInstruction(InitializerStoreTag())
-    )
-    or
-    (
-      child = this.getInitializerExpr() and
-      if this.needsConversion()
-      then result = this.getInstruction(AssignmentConvertRightTag())
-      else result = this.getInstruction(InitializerStoreTag())
-    )
+    child = this.getInitializer() and
+    if this.needsConversion()
+    then result = this.getInstruction(AssignmentConvertRightTag())
+    else result = this.getInstruction(InitializerStoreTag())
   }
 
   override Instruction getInstructionOperand(InstructionTag tag, OperandTag operandTag) {
@@ -256,33 +177,28 @@ class TranslatedObjectInitialization extends TranslatedInitialization, Construct
       result = this.getParent().(InitializationContext).getTargetAddress()
       or
       (
+        operandTag instanceof AddressOperandTag and
+        result = this.getContext().getTargetAddress()
+        or
         operandTag instanceof StoreValueOperandTag and
-        if needsConversion()
-        then result = this.getInstruction(AssignmentConvertRightTag())
-        else result = this.getInstruction(NewObjTag())
+        result = this.getInitializer().getResult()
       )
     )
     or
     tag = AssignmentConvertRightTag() and
     operandTag instanceof UnaryOperandTag and
+    result = this.getInitializer().getResult()
+    or
+    tag = AssignmentConvertRightTag() and
+    operandTag instanceof UnaryOperandTag and
     result = this.getInstruction(NewObjTag())
   }
 
-  private TranslatedExpr getConstructorCall() { result = getTranslatedExpr(expr) }
-
-  private TranslatedExpr getInitializerExpr() { result = getTranslatedExpr(expr.getInitializer()) }
-
-  override Instruction getReceiver() {
-    // The newly allocated object will be the target of the constructor call
-    result = this.getInstruction(NewObjTag())
-  }
+  TranslatedExpr getInitializer() { result = getTranslatedExpr(expr) }
 
   private predicate needsConversion() { expr.getType() != this.getContext().getTargetType() }
 }
 
-//private string getZeroValue(Type type) {
-//  if type instanceof FloatingPointType then result = "0.0" else result = "0"
-//}
 /**
  * Represents the IR translation of the initialization of an array element from
  * an element of an initializer list.
@@ -424,7 +340,7 @@ class TranslatedConstructorInitializer extends TranslatedConstructorCallFromCons
   TTranslatedConstructorInitializer {
   TranslatedConstructorInitializer() { this = TTranslatedConstructorInitializer(call) }
 
-  override string toString() { result = "constuructor init: " + call.toString() }
+  override string toString() { result = "constructor init: " + call.toString() }
 
   override Instruction getFirstInstruction() {
     if needsConversion()
@@ -472,58 +388,3 @@ class TranslatedConstructorInitializer extends TranslatedConstructorCallFromCons
     derivedClass = this.getFunction().getDeclaringType()
   }
 }
-
-/**
- * Represents the IR translation of a delegate creation.
- */
-class TranslatedDelegateCreation extends TranslatedInitialization, ConstructorCallContext {
-  override DelegateCreation expr;
-
-  override TranslatedElement getChild(int id) { id = 0 and result = getConstructorCall() }
-
-  override predicate hasInstruction(
-    Opcode opcode, InstructionTag tag, Type resultType, boolean isLValue
-  ) {
-    tag = NewObjTag() and
-    opcode instanceof Opcode::NewObj and
-    resultType = expr.getType() and
-    isLValue = false
-    or
-    tag = InitializerStoreTag() and
-    opcode instanceof Opcode::Store and
-    resultType = expr.getType() and
-    isLValue = false
-  }
-
-  final override Instruction getFirstInstruction() { result = getInstruction(NewObjTag()) }
-
-  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
-    tag = NewObjTag() and
-    result = getConstructorCall().getFirstInstruction() and
-    kind instanceof GotoEdge
-    or
-    tag = InitializerStoreTag() and
-    result = getParent().getChildSuccessor(this) and
-    kind instanceof GotoEdge
-  }
-
-  override Instruction getChildSuccessor(TranslatedElement child) {
-    child = getConstructorCall() and
-    result = getInstruction(InitializerStoreTag())
-  }
-
-  override Instruction getInstructionOperand(InstructionTag tag, OperandTag operandTag) {
-    tag = InitializerStoreTag() and
-    (
-      operandTag instanceof AddressOperandTag and
-      result = getParent().(InitializationContext).getTargetAddress()
-      or
-      operandTag instanceof StoreValueOperandTag and
-      result = getInstruction(NewObjTag())
-    )
-  }
-
-  private TranslatedElement getConstructorCall() { result = DelegateElements::getConstructor(expr) }
-
-  override Instruction getReceiver() { result = getInstruction(NewObjTag()) }
-}