CPP: Add delete/delete[] calls to the IR.

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/SideEffects.qll

@@ -130,6 +130,8 @@ class CallOrAllocationExpr extends Expr {
     this instanceof Call
     or
     this instanceof NewOrNewArrayExpr
+    or
+    this instanceof DeleteOrDeleteArrayExpr
   }
 
   /** Gets the `Function` invoked by this expression, if known. */
@@ -137,6 +139,8 @@ class CallOrAllocationExpr extends Expr {
     result = this.(Call).getTarget()
     or
     result = this.(NewOrNewArrayExpr).getAllocator()
+    or
+    result = this.(DeleteOrDeleteArrayExpr).getDeallocator()
   }
 }
 

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll

@@ -350,6 +350,9 @@ class TranslatedCallSideEffects extends TranslatedSideEffects, TTranslatedCallSi
     or
     expr instanceof NewOrNewArrayExpr and
     result = getTranslatedAllocatorCall(expr).getInstruction(CallTag())
+    or
+    expr instanceof DeleteOrDeleteArrayExpr and
+    result = getTranslatedDeallocatorCall(expr).getInstruction(CallTag())
   }
 }
 

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll

@@ -75,19 +75,18 @@ private predicate ignoreExprAndDescendants(Expr expr) {
     // REVIEW: Ignore initializers for `NewArrayExpr` until we determine how to
     // represent them.
     newExpr.getInitializer().getFullyConverted() = expr
-  )
+  ) or
+  exists(DeleteOrDeleteArrayExpr deleteExpr |
+    // Ignore the deallocator call, because we always synthesize it.
+    deleteExpr.getDeallocatorCall() = expr
+  ) 
   or
   // Do not translate input/output variables in GNU asm statements
   //  getRealParent(expr) instanceof AsmStmt
   //  or
   ignoreExprAndDescendants(getRealParent(expr)) // recursive case
   or
-  // We do not yet translate destructors properly, so for now we ignore any
-  // custom deallocator call, if present.
-  exists(DeleteExpr deleteExpr | deleteExpr.getDeallocatorCall() = expr)
-  or
-  exists(DeleteArrayExpr deleteArrayExpr | deleteArrayExpr.getDeallocatorCall() = expr)
-  or
+  // va_start doesn't evaluate its argument, so we don't need to translate it.
   exists(BuiltInVarArgsStart vaStartExpr |
     vaStartExpr.getLastNamedParameter().getFullyConverted() = expr
   )
@@ -104,6 +103,12 @@ private predicate ignoreExprOnly(Expr expr) {
     newExpr.getAllocatorCall() = expr
   )
   or
+  exists(DeleteOrDeleteArrayExpr deleteExpr |
+    // Ignore the destructor call as we don't model it yet. Don't ignore
+    // its arguments, though, as they are the arguments to the deallocator.
+    deleteExpr.getDestructorCall() = expr
+  )
+  or
   // The extractor deliberately emits an `ErrorExpr` as the first argument to
   // the allocator call, if any, of a `NewOrNewArrayExpr`. That `ErrorExpr`
   // should not be translated.
@@ -111,13 +116,6 @@ private predicate ignoreExprOnly(Expr expr) {
   or
   not translateFunction(getEnclosingFunction(expr)) and
   not Raw::varHasIRFunc(getEnclosingVariable(expr))
-  or
-  // We do not yet translate destructors properly, so for now we ignore the
-  // destructor call. We do, however, translate the expression being
-  // destructed, and that expression can be a child of the destructor call.
-  exists(DeleteExpr deleteExpr | deleteExpr.getDestructorCall() = expr)
-  or
-  exists(DeleteArrayExpr deleteArrayExpr | deleteArrayExpr.getDestructorCall() = expr)
 }
 
 /**
@@ -724,6 +722,8 @@ newtype TTranslatedElement =
   } or
   // An allocator call in a `new` or `new[]` expression
   TTranslatedAllocatorCall(NewOrNewArrayExpr newExpr) { not ignoreExpr(newExpr) } or
+  // An deallocator call in a `delete` or `delete[]` expression
+  TTranslatedDeallocatorCall(DeleteOrDeleteArrayExpr newExpr) { not ignoreExpr(newExpr) } or
   // An allocation size for a `new` or `new[]` expression
   TTranslatedAllocationSize(NewOrNewArrayExpr newExpr) { not ignoreExpr(newExpr) } or
   // The declaration/initialization part of a `ConditionDeclExpr`

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

@@ -2017,6 +2017,48 @@ TranslatedAllocatorCall getTranslatedAllocatorCall(NewOrNewArrayExpr newExpr) {
   result.getAst() = newExpr
 }
 
+/**
+ * The IR translation of a call to `operator delete` as part of a `delete` or `delete[]`
+ * expression.
+ */
+class TranslatedDeallocatorCall extends TTranslatedDeallocatorCall, TranslatedDirectCall {
+  override DeleteOrDeleteArrayExpr expr;
+
+  TranslatedDeallocatorCall() { this = TTranslatedDeallocatorCall(expr) }
+
+  final override string toString() { result = "Deallocator call for " + expr.toString() }
+
+  final override predicate producesExprResult() { none() }
+
+  override Function getInstructionFunction(InstructionTag tag) {
+    tag = CallTargetTag() and result = expr.getDeallocator()
+  }
+
+  final override Type getCallResultType() { result = expr.getType() }
+
+  final override TranslatedExpr getQualifier() { none() }
+
+  final override predicate hasArguments() {
+    // All deallocator calls have at least one argument.
+    any()
+  }
+
+  final override int getNumberOfArguments() {
+    // We ignore the other arguments for now as we would have to synthesize them.
+    result = 1
+  }
+
+  final override TranslatedExpr getArgument(int index) {
+    // The only argument we define is the pointer to be deallocated.
+    index = 0 and
+    result = getTranslatedExpr(expr.getExpr().getFullyConverted())
+  }
+}
+
+TranslatedDeallocatorCall getTranslatedDeallocatorCall(DeleteOrDeleteArrayExpr newExpr) {
+  result.getAst() = newExpr
+}
+
 /**
  * Abstract class implemented by any `TranslatedElement` that has a child
  * expression that is a call to a constructor or destructor, in order to
@@ -2955,75 +2997,60 @@ class TranslatedNewArrayExpr extends TranslatedNewOrNewArrayExpr {
 }
 
 /**
- * A placeholder for the translation of a `delete[]` expression.
- *
- * Proper translation is not yet implemented, but this stub implementation
- * ensures that code following a `delete[]` is not unreachable.
+ * The IR translation of a `delete` or `delete[]` expression.
  */
-class TranslatedDeleteArrayExprPlaceHolder extends TranslatedSingleInstructionExpr {
-  override DeleteArrayExpr expr;
+abstract class TranslatedDeleteOrDeleteArrayExpr extends TranslatedNonConstantExpr {
+  override DeleteOrDeleteArrayExpr expr;
 
-  final override Instruction getFirstInstruction() {
-    result = this.getOperand().getFirstInstruction()
+  final override TranslatedElement getChild(int id) {
+    id = 0 and result = this.getDeallocatorCall()
   }
 
-  final override TranslatedElement getChild(int id) { id = 0 and result = this.getOperand() }
+  final override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
+    tag = OnlyInstructionTag() and
+    opcode instanceof Opcode::Convert and
+    resultType = this.getResultType()
+  }
+
+  final override Instruction getFirstInstruction() {
+    result = this.getDeallocatorCall().getFirstInstruction()
+  }
+
+  final override Instruction getResult() { result = this.getInstruction(OnlyInstructionTag()) }
 
   final override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
+    kind instanceof GotoEdge and
     tag = OnlyInstructionTag() and
-    result = this.getParent().getChildSuccessor(this) and
-    kind instanceof GotoEdge
+    result = this.getParent().getChildSuccessor(this)
   }
 
   final override Instruction getChildSuccessor(TranslatedElement child) {
-    child = this.getOperand() and result = this.getInstruction(OnlyInstructionTag())
+    child = this.getDeallocatorCall() and result = this.getInstruction(OnlyInstructionTag())
   }
 
   final override Instruction getInstructionRegisterOperand(InstructionTag tag, OperandTag operandTag) {
-    none()
+    tag = OnlyInstructionTag() and
+    operandTag instanceof UnaryOperandTag and
+    result = this.getDeallocatorCall().getResult()
   }
 
-  final override Opcode getOpcode() { result instanceof Opcode::NoOp }
-
-  private TranslatedExpr getOperand() {
-    result = getTranslatedExpr(expr.getExpr().getFullyConverted())
+  private TranslatedDeallocatorCall getDeallocatorCall() {
+    result = getTranslatedDeallocatorCall(expr)
   }
 }
 
 /**
- * A placeholder for the translation of a `delete` expression.
- *
- * Proper translation is not yet implemented, but this stub implementation
- * ensures that code following a `delete` is not unreachable.
+ * The IR translation of a `delete` expression.
  */
-class TranslatedDeleteExprPlaceHolder extends TranslatedSingleInstructionExpr {
+class TranslatedDeleteExpr extends TranslatedDeleteOrDeleteArrayExpr {
   override DeleteExpr expr;
+}
 
-  final override Instruction getFirstInstruction() {
-    result = this.getOperand().getFirstInstruction()
-  }
-
-  final override TranslatedElement getChild(int id) { id = 0 and result = this.getOperand() }
-
-  final override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
-    tag = OnlyInstructionTag() and
-    result = this.getParent().getChildSuccessor(this) and
-    kind instanceof GotoEdge
-  }
-
-  final override Instruction getChildSuccessor(TranslatedElement child) {
-    child = this.getOperand() and result = this.getInstruction(OnlyInstructionTag())
-  }
-
-  final override Instruction getInstructionRegisterOperand(InstructionTag tag, OperandTag operandTag) {
-    none()
-  }
-
-  final override Opcode getOpcode() { result instanceof Opcode::NoOp }
-
-  private TranslatedExpr getOperand() {
-    result = getTranslatedExpr(expr.getExpr().getFullyConverted())
-  }
+/**
+ * The IR translation of a `delete[]` expression.
+ */
+class TranslatedDeleteArrayExpr extends TranslatedDeleteOrDeleteArrayExpr {
+  override DeleteArrayExpr expr;
 }
 
 /**