C++: Add implicit destructors for named variables to the IR

cpp/ql/lib/semmle/code/cpp/Enclosing.qll

@@ -60,4 +60,6 @@ Element exprEnclosingElement(Expr e) {
       )
     else result = de.getDeclaration()
   )
+  or
+  result.(Stmt).getAnImplicitDestructorCall() = e
 }

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

@@ -11,6 +11,7 @@
 private import TranslatedCondition
 private import TranslatedElement
 private import TranslatedExpr
+private import TranslatedCall
 private import TranslatedStmt
 private import TranslatedFunction
 private import TranslatedGlobalVar

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

@@ -85,10 +85,14 @@ newtype TInstructionTag =
   // The next three cases handle generation of branching for __except handling.
   TryExceptCompareNegativeOneBranch() or
   TryExceptCompareZeroBranch() or
-  TryExceptCompareOneBranch()
+  TryExceptCompareOneBranch() or
+  ImplicitDestructorTag(int index) {
+    exists(Expr e | exists(e.getImplicitDestructorCall(index))) or
+    exists(Stmt s | exists(s.getImplicitDestructorCall(index)))
+  }
 
 class InstructionTag extends TInstructionTag {
-  final string toString() { result = "Tag" }
+  final string toString() { result = getInstructionTagId(this) }
 }
 
 /**
@@ -255,4 +259,8 @@ string getInstructionTagId(TInstructionTag tag) {
   tag = TryExceptCompareZeroBranch() and result = "TryExceptCompareZeroBranch"
   or
   tag = TryExceptCompareOneBranch() and result = "TryExceptCompareOneBranch"
+  or
+  exists(int index |
+    tag = ImplicitDestructorTag(index) and result = "ImplicitDestructor(" + index + ")"
+  )
 }

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

@@ -27,7 +27,7 @@ private CallInstruction getTranslatedCallInstruction(Call call) {
  * of a higher-level constructor (e.g. the allocator call in a `NewExpr`).
  */
 abstract class TranslatedCall extends TranslatedExpr {
-  final override TranslatedElement getChild(int id) {
+  final override TranslatedElement getChildInternal(int id) {
     // We choose the child's id in the order of evaluation.
     // The qualifier is evaluated before the call target, because the value of
     // the call target may depend on the value of the qualifier for virtual
@@ -47,13 +47,19 @@ abstract class TranslatedCall extends TranslatedExpr {
     else result = this.getFirstCallTargetInstruction(kind)
   }
 
+  override Instruction getALastInstructionInternal() {
+    result = this.getSideEffects().getALastInstruction()
+  }
+
+  override TranslatedElement getLastChild() { result = this.getSideEffects() }
+
   override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
     tag = CallTag() and
     opcode instanceof Opcode::Call and
     resultType = getTypeForPRValue(this.getCallResultType())
   }
 
-  override Instruction getChildSuccessor(TranslatedElement child, EdgeKind kind) {
+  override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
     child = this.getQualifier() and
     result = this.getFirstCallTargetInstruction(kind)
     or
@@ -87,7 +93,7 @@ abstract class TranslatedCall extends TranslatedExpr {
     )
   }
 
-  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
+  override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
     tag = CallTag() and
     result = this.getSideEffects().getFirstInstruction(kind)
   }
@@ -225,7 +231,7 @@ abstract class TranslatedSideEffects extends TranslatedElement {
       )
   }
 
-  final override Instruction getChildSuccessor(TranslatedElement te, EdgeKind kind) {
+  final override Instruction getChildSuccessorInternal(TranslatedElement te, EdgeKind kind) {
     exists(int i |
       this.getChild(i) = te and
       if exists(this.getChild(i + 1))
@@ -234,6 +240,10 @@ abstract class TranslatedSideEffects extends TranslatedElement {
     )
   }
 
+  override TranslatedElement getLastChild() {
+    result = this.getChild(max(int i | exists(this.getChild(i))))
+  }
+
   final override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType type) {
     none()
   }
@@ -246,7 +256,18 @@ abstract class TranslatedSideEffects extends TranslatedElement {
     result = this.getParent().getChildSuccessor(this, kind)
   }
 
-  final override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) { none() }
+  override Instruction getALastInstructionInternal() {
+    if exists(this.getAChild())
+    then result = this.getChild(max(int i | exists(this.getChild(i)))).getALastInstruction()
+    else
+      // If there are no side effects, the "last" instruction should be the parent call's last
+      // instruction, so that implicit destructors can be inserted in the right place.
+      result = this.getParent().getInstruction(CallTag())
+  }
+
+  final override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
+    none()
+  }
 
   /** Gets the primary instruction to be associated with each side effect instruction. */
   abstract Instruction getPrimaryInstruction();
@@ -273,8 +294,8 @@ abstract class TranslatedDirectCall extends TranslatedCall {
     resultType = getFunctionGLValueType()
   }
 
-  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
-    result = TranslatedCall.super.getInstructionSuccessor(tag, kind)
+  override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
+    result = TranslatedCall.super.getInstructionSuccessorInternal(tag, kind)
     or
     tag = CallTargetTag() and
     result = this.getFirstArgumentOrCallInstruction(kind)
@@ -367,6 +388,16 @@ class TranslatedStructorCall extends TranslatedFunctionCall {
       context = this.getParent() and
       result = context.getReceiver()
     )
+    or
+    exists(Stmt parent |
+      expr = parent.getAnImplicitDestructorCall() and
+      result = getTranslatedExpr(expr.getQualifier().getFullyConverted()).getResult()
+    )
+    or
+    exists(Expr parent |
+      expr = parent.getAnImplicitDestructorCall() and
+      result = getTranslatedExpr(expr.getQualifier().getFullyConverted()).getResult()
+    )
   }
 
   override predicate hasQualifier() { any() }
@@ -416,19 +447,25 @@ private int initializeAllocationGroup() { result = 3 }
 abstract class TranslatedSideEffect extends TranslatedElement {
   final override TranslatedElement getChild(int n) { none() }
 
-  final override Instruction getChildSuccessor(TranslatedElement child, EdgeKind kind) { none() }
+  final override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
+    none()
+  }
 
   final override Instruction getFirstInstruction(EdgeKind kind) {
     result = this.getInstruction(OnlyInstructionTag()) and
     kind instanceof GotoEdge
   }
 
+  override Instruction getALastInstructionInternal() {
+    result = this.getInstruction(OnlyInstructionTag())
+  }
+
   final override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType type) {
     tag = OnlyInstructionTag() and
     this.sideEffectInstruction(opcode, type)
   }
 
-  final override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
+  final override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
     result = this.getParent().getChildSuccessor(this, kind) and
     tag = OnlyInstructionTag()
   }

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

@@ -50,19 +50,29 @@ abstract class TranslatedFlexibleCondition extends TranslatedCondition, Conditio
 {
   TranslatedFlexibleCondition() { this = TTranslatedFlexibleCondition(expr) }
 
+  final override predicate handlesDestructorsExplicitly() { none() } // TODO: this needs to be revisted when we get unnamed destructors
+
   final override TranslatedElement getChild(int id) { id = 0 and result = this.getOperand() }
 
   final override Instruction getFirstInstruction(EdgeKind kind) {
     result = this.getOperand().getFirstInstruction(kind)
   }
 
+  final override Instruction getALastInstructionInternal() {
+    result = this.getOperand().getALastInstruction()
+  }
+
   final override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
     none()
   }
 
-  final override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) { none() }
+  final override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
+    none()
+  }
 
-  final override Instruction getChildSuccessor(TranslatedElement child, EdgeKind kind) { none() }
+  final override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
+    none()
+  }
 
   abstract TranslatedCondition getOperand();
 }
@@ -88,12 +98,16 @@ class TranslatedParenthesisCondition extends TranslatedFlexibleCondition {
 abstract class TranslatedNativeCondition extends TranslatedCondition, TTranslatedNativeCondition {
   TranslatedNativeCondition() { this = TTranslatedNativeCondition(expr) }
 
-  final override Instruction getChildSuccessor(TranslatedElement child, EdgeKind kind) { none() }
+  final override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
+    none()
+  }
 }
 
 abstract class TranslatedBinaryLogicalOperation extends TranslatedNativeCondition, ConditionContext {
   override BinaryLogicalOperation expr;
 
+  final override predicate handlesDestructorsExplicitly() { none() } // TODO: this needs to be revisted when we get unnamed destructors
+
   final override TranslatedElement getChild(int id) {
     id = 0 and result = this.getLeftOperand()
     or
@@ -104,11 +118,19 @@ abstract class TranslatedBinaryLogicalOperation extends TranslatedNativeConditio
     result = this.getLeftOperand().getFirstInstruction(kind)
   }
 
+  final override Instruction getALastInstructionInternal() {
+    result = this.getLeftOperand().getALastInstruction()
+    or
+    result = this.getRightOperand().getALastInstruction()
+  }
+
   final override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
     none()
   }
 
-  final override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) { none() }
+  final override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
+    none()
+  }
 
   final TranslatedCondition getLeftOperand() {
     result = getTranslatedCondition(expr.getLeftOperand().getFullyConverted())
@@ -162,19 +184,25 @@ class TranslatedValueCondition extends TranslatedCondition, TTranslatedValueCond
     result = this.getValueExpr().getFirstInstruction(kind)
   }
 
+  override Instruction getALastInstructionInternal() {
+    result = this.getInstruction(ValueConditionConditionalBranchTag())
+  }
+
+  final override predicate handlesDestructorsExplicitly() { none() } // TODO: this needs to be revisted when we get unnamed destructors
+
   override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
     tag = ValueConditionConditionalBranchTag() and
     opcode instanceof Opcode::ConditionalBranch and
     resultType = getVoidType()
   }
 
-  override Instruction getChildSuccessor(TranslatedElement child, EdgeKind kind) {
+  override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
     child = this.getValueExpr() and
     result = this.getInstruction(ValueConditionConditionalBranchTag()) and
     kind instanceof GotoEdge
   }
 
-  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
+  override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
     tag = ValueConditionConditionalBranchTag() and
     (
       kind instanceof TrueEdge and

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

@@ -60,6 +60,10 @@ abstract class TranslatedLocalVariableDeclaration extends TranslatedVariableInit
    */
   abstract LocalVariable getVariable();
 
+  final override TranslatedElement getChild(int id) {
+    result = TranslatedVariableInitialization.super.getChildInternal(id)
+  }
+
   final override Type getTargetType() { result = getVariableType(this.getVariable()) }
 
   final override TranslatedInitialization getInitialization() {
@@ -152,7 +156,13 @@ class TranslatedStaticLocalVariableDeclarationEntry extends TranslatedDeclaratio
     kind instanceof GotoEdge
   }
 
-  final override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
+  final override Instruction getALastInstructionInternal() {
+    result = this.getInstruction(DynamicInitializationConditionalBranchTag())
+    or
+    result = this.getInstruction(DynamicInitializationFlagStoreTag())
+  }
+
+  final override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
     tag = DynamicInitializationFlagAddressTag() and
     kind instanceof GotoEdge and
     result = this.getInstruction(DynamicInitializationFlagLoadTag())
@@ -178,7 +188,7 @@ class TranslatedStaticLocalVariableDeclarationEntry extends TranslatedDeclaratio
     result = this.getParent().getChildSuccessor(this, kind)
   }
 
-  final override Instruction getChildSuccessor(TranslatedElement child, EdgeKind kind) {
+  final override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
     child = this.getInitialization() and
     result = this.getInstruction(DynamicInitializationFlagConstantTag()) and
     kind instanceof GotoEdge