Merge pull request #16455 from jketema/if-fix

C++: Ensure destructors for ifs are called after both branches and for both if and constexpr if

Author: jketema

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

@@ -842,9 +842,7 @@ class TranslatedCatchAnyHandler extends TranslatedHandler {
   }
 }
 
-class TranslatedIfStmt extends TranslatedStmt, ConditionContext {
-  override IfStmt stmt;
-
+abstract class TranslatedIfLikeStmt extends TranslatedStmt, ConditionContext {
   override Instruction getFirstInstruction(EdgeKind kind) {
     if this.hasInitialization()
     then result = this.getInitialization().getFirstInstruction(kind)
@@ -857,6 +855,8 @@ class TranslatedIfStmt extends TranslatedStmt, ConditionContext {
 
   override TranslatedElement getLastChild() { result = this.getElse() or result = this.getThen() }
 
+  override predicate handlesDestructorsExplicitly() { any() }
+
   override TranslatedElement getChildInternal(int id) {
     id = 0 and result = this.getInitialization()
     or
@@ -867,25 +867,21 @@ class TranslatedIfStmt extends TranslatedStmt, ConditionContext {
     id = 3 and result = this.getElse()
   }
 
-  private predicate hasInitialization() { exists(stmt.getInitialization()) }
+  abstract predicate hasInitialization();
 
-  private TranslatedStmt getInitialization() {
-    result = getTranslatedStmt(stmt.getInitialization())
-  }
+  abstract TranslatedStmt getInitialization();
 
-  private TranslatedCondition getCondition() {
-    result = getTranslatedCondition(stmt.getCondition().getFullyConverted())
-  }
+  abstract TranslatedCondition getCondition();
 
   private Instruction getFirstConditionInstruction(EdgeKind kind) {
     result = this.getCondition().getFirstInstruction(kind)
   }
 
-  private TranslatedStmt getThen() { result = getTranslatedStmt(stmt.getThen()) }
+  abstract TranslatedStmt getThen();
 
-  private TranslatedStmt getElse() { result = getTranslatedStmt(stmt.getElse()) }
+  abstract TranslatedStmt getElse();
 
-  private predicate hasElse() { exists(stmt.getElse()) }
+  abstract predicate hasElse();
 
   override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) { none() }
 
@@ -898,92 +894,81 @@ class TranslatedIfStmt extends TranslatedStmt, ConditionContext {
     child = this.getCondition() and
     if this.hasElse()
     then result = this.getElse().getFirstInstruction(kind)
-    else result = this.getParent().getChildSuccessor(this, kind)
+    else (
+      if this.hasAnImplicitDestructorCall()
+      then result = this.getChild(this.getFirstDestructorCallIndex()).getFirstInstruction(kind)
+      else result = this.getParent().getChildSuccessor(this, kind)
+    )
   }
 
   override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
     child = this.getInitialization() and
     result = this.getFirstConditionInstruction(kind)
     or
     (child = this.getThen() or child = this.getElse()) and
-    result = this.getParent().getChildSuccessor(this, kind)
+    (
+      if this.hasAnImplicitDestructorCall()
+      then result = this.getChild(this.getFirstDestructorCallIndex()).getFirstInstruction(kind)
+      else result = this.getParent().getChildSuccessor(this, kind)
+    )
+    or
+    exists(int destructorId |
+      destructorId >= this.getFirstDestructorCallIndex() and
+      child = this.getChild(destructorId) and
+      result = this.getChild(destructorId + 1).getFirstInstruction(kind)
+    )
+    or
+    exists(int lastDestructorIndex |
+      lastDestructorIndex =
+        max(int n | exists(this.getChild(n)) and n >= this.getFirstDestructorCallIndex()) and
+      child = this.getChild(lastDestructorIndex) and
+      result = this.getParent().getChildSuccessor(this, kind)
+    )
   }
 
   override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
     none()
   }
 }
 
-class TranslatedConstExprIfStmt extends TranslatedStmt, ConditionContext {
-  override ConstexprIfStmt stmt;
-
-  override Instruction getFirstInstruction(EdgeKind kind) {
-    if this.hasInitialization()
-    then result = this.getInitialization().getFirstInstruction(kind)
-    else result = this.getFirstConditionInstruction(kind)
-  }
-
-  override TranslatedElement getChildInternal(int id) {
-    id = 0 and result = this.getInitialization()
-    or
-    id = 1 and result = this.getCondition()
-    or
-    id = 2 and result = this.getThen()
-    or
-    id = 3 and result = this.getElse()
-  }
+class TranslatedIfStmt extends TranslatedIfLikeStmt {
+  override IfStmt stmt;
 
-  private predicate hasInitialization() { exists(stmt.getInitialization()) }
+  override predicate hasInitialization() { exists(stmt.getInitialization()) }
 
-  private TranslatedStmt getInitialization() {
+  override TranslatedStmt getInitialization() {
     result = getTranslatedStmt(stmt.getInitialization())
   }
 
-  private TranslatedCondition getCondition() {
+  override TranslatedCondition getCondition() {
     result = getTranslatedCondition(stmt.getCondition().getFullyConverted())
   }
 
-  private Instruction getFirstConditionInstruction(EdgeKind kind) {
-    result = this.getCondition().getFirstInstruction(kind)
-  }
+  override TranslatedStmt getThen() { result = getTranslatedStmt(stmt.getThen()) }
 
-  private TranslatedStmt getThen() { result = getTranslatedStmt(stmt.getThen()) }
+  override TranslatedStmt getElse() { result = getTranslatedStmt(stmt.getElse()) }
 
-  private TranslatedStmt getElse() { result = getTranslatedStmt(stmt.getElse()) }
+  override predicate hasElse() { exists(stmt.getElse()) }
+}
 
-  private predicate hasElse() { exists(stmt.getElse()) }
+class TranslatedConstExprIfStmt extends TranslatedIfLikeStmt {
+  override ConstexprIfStmt stmt;
 
-  override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) { none() }
+  override predicate hasInitialization() { exists(stmt.getInitialization()) }
 
-  override Instruction getChildTrueSuccessor(TranslatedCondition child, EdgeKind kind) {
-    child = this.getCondition() and
-    result = this.getThen().getFirstInstruction(kind)
+  override TranslatedStmt getInitialization() {
+    result = getTranslatedStmt(stmt.getInitialization())
   }
 
-  override Instruction getChildFalseSuccessor(TranslatedCondition child, EdgeKind kind) {
-    child = this.getCondition() and
-    if this.hasElse()
-    then result = this.getElse().getFirstInstruction(kind)
-    else result = this.getParent().getChildSuccessor(this, kind)
+  override TranslatedCondition getCondition() {
+    result = getTranslatedCondition(stmt.getCondition().getFullyConverted())
   }
 
-  override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
-    child = this.getInitialization() and
-    result = this.getFirstConditionInstruction(kind)
-    or
-    (child = this.getThen() or child = this.getElse()) and
-    result = this.getParent().getChildSuccessor(this, kind)
-  }
+  override TranslatedStmt getThen() { result = getTranslatedStmt(stmt.getThen()) }
 
-  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
-    none()
-  }
+  override TranslatedStmt getElse() { result = getTranslatedStmt(stmt.getElse()) }
 
-  override Instruction getALastInstructionInternal() {
-    result = this.getThen().getALastInstruction()
-    or
-    result = this.getElse().getALastInstruction()
-  }
+  override predicate hasElse() { exists(stmt.getElse()) }
 }
 
 abstract class TranslatedLoop extends TranslatedStmt, ConditionContext {