Merge pull request #82 from dave-bartolomeo/dave/NewDelete2

C++: IR generation for `new` and `new[]`

Author: jbj

cpp/ql/src/semmle/code/cpp/exprs/Expr.qll

@@ -581,24 +581,13 @@ class ReferenceDereferenceExpr extends Conversion, @ref_indirect {
 }
 
 /**
- * A C++ `new` (non-array) expression.
+ * A C++ `new` or `new[]` expression.
  */
-class NewExpr extends Expr, @new_expr {
-  override string toString() { result = "new" }
-
+class NewOrNewArrayExpr extends Expr, @any_new_expr {
   override int getPrecedence() { result = 15 }
 
   /**
-   * Gets the type that is being allocated.
-   *
-   * For example, for `new int` the result is `int`.
-   */
-  Type getAllocatedType() {
-    new_allocated_type(underlyingElement(this), unresolveElement(result))
-  }
-
-  /**
-   * Gets the `operator new` that allocates storage.
+   * Gets the `operator new` or `operator new[]` that allocates storage.
    */
   Function getAllocator() {
     expr_allocator(underlyingElement(this), unresolveElement(result), _)
@@ -612,6 +601,21 @@ class NewExpr extends Expr, @new_expr {
     expr_allocator(underlyingElement(this), _, 1)
   }
 
+  /**
+   * Gets the alignment argument passed to the allocation function, if any.
+   */
+  Expr getAlignmentArgument() {
+    hasAlignedAllocation() and
+    (
+      // If we have an allocator call, the alignment is the second argument to
+      // that call.
+      result = getAllocatorCall().getArgument(1) or
+      // Otherwise, the alignment winds up as child number 3 of the `new`
+      // itself.
+      result = getChild(3)
+    )
+  }
+
   /**
    * Gets the call to a non-default `operator new` that allocates storage, if any.
    *
@@ -652,6 +656,30 @@ class NewExpr extends Expr, @new_expr {
     )
   }
 
+  /**
+   * Gets the type that is being allocated.
+   *
+   * For example, for `new int` the result is `int`.
+   * For `new int[5]` the result is `int[5]`.
+   */
+  abstract Type getAllocatedType();
+}
+
+/**
+ * A C++ `new` (non-array) expression.
+ */
+class NewExpr extends NewOrNewArrayExpr, @new_expr {
+  override string toString() { result = "new" }
+
+  /**
+   * Gets the type that is being allocated.
+   *
+   * For example, for `new int` the result is `int`.
+   */
+  override Type getAllocatedType() {
+    new_allocated_type(underlyingElement(this), unresolveElement(result))
+  }
+
   /**
    * Gets the call or expression that initializes the allocated object, if any.
    *
@@ -664,17 +692,15 @@ class NewExpr extends Expr, @new_expr {
 /**
  * A C++ `new[]` (array) expression.
  */
-class NewArrayExpr extends Expr, @new_array_expr {
+class NewArrayExpr extends NewOrNewArrayExpr, @new_array_expr {
   override string toString() { result = "new[]" }
 
-  override int getPrecedence() { result = 15 }
-
   /**
    * Gets the type that is being allocated.
    *
    * For example, for `new int[5]` the result is `int[5]`.
    */
-  Type getAllocatedType() {
+  override Type getAllocatedType() {
     new_array_allocated_type(underlyingElement(this), unresolveElement(result))
   }
 
@@ -685,56 +711,6 @@ class NewArrayExpr extends Expr, @new_array_expr {
     result = getType().getUnderlyingType().(PointerType).getBaseType()
   }
 
-  /**
-   * Gets the `operator new[]` that allocates storage.
-   */
-  Function getAllocator() {
-    expr_allocator(underlyingElement(this), unresolveElement(result), _)
-  }
-
-  /**
-   * Holds if the allocation function is the version that expects an alignment
-   * argument of type `std::align_val_t`.
-   */
-  predicate hasAlignedAllocation() {
-    expr_allocator(underlyingElement(this), _, 1)
-  }
-
-  /**
-   * Gets the call to a non-default `operator new[]` that allocates storage for the array, if any.
-   *
-   * If the default `operator new[]` is used, then there will be no call.
-   */
-  FunctionCall getAllocatorCall() { result = this.getChild(0) }
-
-  /**
-   * Gets the `operator delete` that deallocates storage if the initialization
-   * throws an exception, if any.
-   */
-  Function getDeallocator() {
-    expr_deallocator(underlyingElement(this), unresolveElement(result), _)
-  }
-
-  /**
-   * Holds if the deallocation function expects a size argument.
-   */
-  predicate hasSizedDeallocation() {
-    exists(int form |
-      expr_deallocator(underlyingElement(this), _, form) and
-      form.bitAnd(1) != 0  // Bit zero is the "size" bit
-    )
-  }
-
-  /**
-   * Holds if the deallocation function expects an alignment argument.
-   */
-  predicate hasAlignedDeallocation() {
-    exists(int form |
-      expr_deallocator(underlyingElement(this), _, form) and
-      form.bitAnd(2) != 0  // Bit one is the "alignment" bit
-    )
-  }
-
   /**
    * Gets the call or expression that initializes the first element of the array, if any.
    *

cpp/ql/src/semmle/code/cpp/ir/internal/Instruction.qll

@@ -195,8 +195,11 @@ class Instruction extends Construction::TInstruction {
   private string getResultTypeString() {
     exists(string valcat |
       valcat = getValueCategoryString(resultType.toString()) and
-      if resultType instanceof UnknownType and exists(getResultSize()) then
+      if (resultType instanceof UnknownType and
+          not isGLValue() and
+          exists(getResultSize())) then (
         result = valcat + "[" + getResultSize().toString() + "]"
+      )
       else
         result = valcat
     )

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

@@ -43,6 +43,9 @@ newtype TInstructionTag =
   SwitchBranchTag() or
   CallTargetTag() or
   CallTag() or
+  AllocationSizeTag() or
+  AllocationElementSizeTag() or
+  AllocationExtentConvertTag() or
   ValueConditionConditionalBranchTag() or
   ConditionValueTrueTempAddressTag() or
   ConditionValueTrueConstantTag() or
@@ -88,11 +91,15 @@ string getInstructionTagId(TInstructionTag tag) {
   tag = OnlyInstructionTag() and result = "Only" or  // Single instruction (not including implicit Load)
   tag = InitializerVariableAddressTag() and result = "InitVarAddr" or
   tag = InitializerStoreTag() and result = "InitStore" or
+  tag = ZeroPadStringConstantTag() and result = "ZeroPadConst" or
+  tag = ZeroPadStringElementIndexTag() and result = "ZeroPadElemIndex" or
+  tag = ZeroPadStringElementAddressTag() and result = "ZeroPadElemAddr" or
+  tag = ZeroPadStringStoreTag() and result = "ZeroPadStore" or
   tag = AssignOperationLoadTag() and result = "AssignOpLoad" or
   tag = AssignOperationConvertLeftTag() and result = "AssignOpConvLeft" or
   tag = AssignOperationOpTag() and result = "AssignOpOp" or
   tag = AssignOperationConvertResultTag() and result = "AssignOpConvRes" or
-  tag = AssignmentStoreTag() and result = "AssigStore" or
+  tag = AssignmentStoreTag() and result = "AssignStore" or
   tag = CrementLoadTag() and result = "CrementLoad" or
   tag = CrementConstantTag() and result = "CrementConst" or
   tag = CrementOpTag() and result = "CrementOp" or
@@ -106,6 +113,9 @@ string getInstructionTagId(TInstructionTag tag) {
   tag = SwitchBranchTag() and result = "SwitchBranch" or
   tag = CallTargetTag() and result = "CallTarget" or
   tag = CallTag() and result = "Call" or
+  tag = AllocationSizeTag() and result = "AllocSize" or
+  tag = AllocationElementSizeTag() and result = "AllocElemSize" or
+  tag = AllocationExtentConvertTag() and result = "AllocExtConv" or
   tag = ValueConditionConditionalBranchTag() and result = "ValCondCondBranch" or
   tag = ConditionValueTrueTempAddressTag() and result = "CondValTrueTempAddr" or
   tag = ConditionValueTrueConstantTag() and result = "CondValTrueConst" or

cpp/ql/src/semmle/code/cpp/ir/internal/TranslatedElement.qll

@@ -47,12 +47,12 @@ private Element getRealParent(Expr expr) {
 }
 
 /**
- * Holds if `expr` should be ignored for the purposes of code generation due to
- * some property of `expr` itself. Unlike `ignoreExpr()`, this predicate does
- * not ignore an expression solely because it is a descendant of an ignored
- * element.
+ * Holds if `expr` and all of its descendants should be ignored for the purposes
+ * of IR generation due to some property of `expr` itself. Unlike
+ * `ignoreExpr()`, this predicate does not ignore an expression solely because
+ * it is a descendant of an ignored element.
  */
-private predicate ignoreExprLocal(Expr expr) {
+private predicate ignoreExprAndDescendants(Expr expr) {
   // Ignore parentless expressions
   not exists(getRealParent(expr)) or
   // Ignore the constants in SwitchCase, since their values are embedded in the
@@ -65,23 +65,32 @@ private predicate ignoreExprLocal(Expr expr) {
   // node as its qualifier, but that `FieldAccess` does not have a child of its own.
   // We'll ignore that `FieldAccess`, and supply the receiver as part of the calling
   // context, much like we do with constructor calls.
-  expr.getParent().(DestructorCall).getParent() instanceof DestructorFieldDestruction
+  expr.getParent().(DestructorCall).getParent() instanceof DestructorFieldDestruction or
+  exists(NewArrayExpr newExpr |
+    // REVIEW: Ignore initializers for `NewArrayExpr` until we determine how to
+    // represent them.
+    newExpr.getInitializer().getFullyConverted() = expr
+  )
 }
 
 /**
- * Holds if `expr` should be ignored for the purposes of IR generation.
+ * Holds if `expr` (not including its descendants) should be ignored for the
+ * purposes of IR generation.
  */
-private predicate ignoreExpr(Expr expr) {
-  ignoreExprLocal(expr) or
-  // Ignore all descendants of ignored elements as well.
-  ignoreElement(getRealParent(expr))
+private predicate ignoreExprOnly(Expr expr) {
+  exists(NewOrNewArrayExpr newExpr |
+    // Ignore the allocator call, because we always synthesize it. Don't ignore
+    // its arguments, though, because we use them as part of the synthesis.
+    newExpr.getAllocatorCall() = expr
+  )
 }
 
 /**
- * Holds if `element` should be ignored for the purposes of IR generation.
+ * Holds if `expr` should be ignored for the purposes of IR generation.
  */
-private predicate ignoreElement(Element element) {
-  ignoreExpr(element.(Expr))
+private predicate ignoreExpr(Expr expr) {
+  ignoreExprOnly(expr) or
+  ignoreExprAndDescendants(getRealParent*(expr))
 }
 
 /**
@@ -155,7 +164,7 @@ predicate ignoreLoad(Expr expr) {
 
 newtype TTranslatedElement =
   // An expression that is not being consumed as a condition
-  TTranslatedNonLoadExpr(Expr expr) {
+  TTranslatedValueExpr(Expr expr) {
     not ignoreExpr(expr) and
     not isNativeCondition(expr) and
     not isFlexibleCondition(expr)
@@ -216,6 +225,9 @@ newtype TTranslatedElement =
       exists(ConstructorFieldInit fieldInit |
         fieldInit.getExpr().getFullyConverted() = expr
       ) or
+      exists(NewExpr newExpr |
+        newExpr.getInitializer().getFullyConverted() = expr
+      ) or
       exists(ThrowExpr throw |
         throw.getExpr().getFullyConverted() = expr
       )
@@ -298,6 +310,14 @@ newtype TTranslatedElement =
     exists(DeclStmt declStmt |
       declStmt.getADeclarationEntry() = entry
     )
+  } or
+  // An allocator call in a `new` or `new[]` expression
+  TTranslatedAllocatorCall(NewOrNewArrayExpr newExpr) {
+    not ignoreExpr(newExpr)
+  } or
+  // An allocation size for a `new` or `new[]` expression
+  TTranslatedAllocationSize(NewOrNewArrayExpr newExpr) {
+    not ignoreExpr(newExpr)
   }
 
 /**