[codegen] Emit missing cleanups when an expression contains a branch

clang/lib/CodeGen/CGCleanup.cpp

@@ -488,16 +488,12 @@ void CodeGenFunction::PopCleanupBlocks(
   }
 }
 
-/// Pops cleanup blocks until the given savepoint is reached, then add the
-/// cleanups from the given savepoint in the lifetime-extended cleanups stack.
-void CodeGenFunction::PopCleanupBlocks(
-    EHScopeStack::stable_iterator Old, size_t OldLifetimeExtendedSize,
-    std::initializer_list<llvm::Value **> ValuesToReload) {
-  PopCleanupBlocks(Old, ValuesToReload);
-
-  // Move our deferred cleanups onto the EH stack.
+/// Adds deferred lifetime-extended cleanups onto the EH stack.
+void CodeGenFunction::AddLifetimeExtendedCleanups(
+    size_t OldLifetimeExtendedSize) {
   for (size_t I = OldLifetimeExtendedSize,
-              E = LifetimeExtendedCleanupStack.size(); I != E; /**/) {
+              E = LifetimeExtendedCleanupStack.size();
+       I != E;) {
     // Alignment should be guaranteed by the vptrs in the individual cleanups.
     assert((I % alignof(LifetimeExtendedCleanupHeader) == 0) &&
            "misaligned cleanup stack entry");
@@ -519,6 +515,17 @@ void CodeGenFunction::PopCleanupBlocks(
       I += sizeof(ActiveFlag);
     }
   }
+}
+
+/// Pops cleanup blocks until the given savepoint is reached, then add the
+/// cleanups from the given savepoint in the lifetime-extended cleanups stack.
+void CodeGenFunction::PopCleanupBlocks(
+    EHScopeStack::stable_iterator Old, size_t OldLifetimeExtendedSize,
+    std::initializer_list<llvm::Value **> ValuesToReload) {
+  PopCleanupBlocks(Old, ValuesToReload);
+
+  // Move our deferred cleanups onto the EH stack.
+  AddLifetimeExtendedCleanups(OldLifetimeExtendedSize);
   LifetimeExtendedCleanupStack.resize(OldLifetimeExtendedSize);
 }
 
@@ -1102,6 +1109,14 @@ void CodeGenFunction::EmitBranchThroughCleanup(JumpDest Dest) {
   if (!HaveInsertPoint())
     return;
 
+  // If we have lifetime-extended (LE) cleanups, then we must be emitting a
+  // branch within an expression. Emit all the LE cleanups by adding them to the
+  // EHStack. Do not remove them from lifetime-extended stack, they need to be
+  // emitted again after the expression completes.
+  RunCleanupsScope LifetimeExtendedCleanups(*this);
+  if (Dest.isValid())
+    AddLifetimeExtendedCleanups(Dest.getLifetimeExtendedDepth());
+
   // Create the branch.
   llvm::BranchInst *BI = Builder.CreateBr(Dest.getBlock());
 

clang/lib/CodeGen/CGStmt.cpp

@@ -628,7 +628,7 @@ CodeGenFunction::getJumpDestForLabel(const LabelDecl *D) {
 
   // Create, but don't insert, the new block.
   Dest = JumpDest(createBasicBlock(D->getName()),
-                  EHScopeStack::stable_iterator::invalid(),
+                  EHScopeStack::stable_iterator::invalid(), 0,
                   NextCleanupDestIndex++);
   return Dest;
 }

clang/lib/CodeGen/CodeGenFunction.h

@@ -238,15 +238,17 @@ class CodeGenFunction : public CodeGenTypeCache {
   /// A jump destination is an abstract label, branching to which may
   /// require a jump out through normal cleanups.
   struct JumpDest {
-    JumpDest() : Block(nullptr), Index(0) {}
+    JumpDest() : Block(nullptr), LifetimeExtendedDepth(0), Index(0) {}
     JumpDest(llvm::BasicBlock *Block, EHScopeStack::stable_iterator Depth,
-             unsigned Index)
-        : Block(Block), ScopeDepth(Depth), Index(Index) {}
+             unsigned LifetimeExtendedScopeDepth, unsigned Index)
+        : Block(Block), ScopeDepth(Depth),
+          LifetimeExtendedDepth(LifetimeExtendedScopeDepth), Index(Index) {}
 
     bool isValid() const { return Block != nullptr; }
     llvm::BasicBlock *getBlock() const { return Block; }
     EHScopeStack::stable_iterator getScopeDepth() const { return ScopeDepth; }
     unsigned getDestIndex() const { return Index; }
+    unsigned getLifetimeExtendedDepth() const { return LifetimeExtendedDepth; }
 
     // This should be used cautiously.
     void setScopeDepth(EHScopeStack::stable_iterator depth) {
@@ -256,6 +258,11 @@ class CodeGenFunction : public CodeGenTypeCache {
   private:
     llvm::BasicBlock *Block;
     EHScopeStack::stable_iterator ScopeDepth;
+
+    // Size of the lifetime-extended cleanup stack in destination scope.
+    // This can only occur when nested stmt-expr's contains branches.
+    // This is useful to emit only the necessary lifeitme-extended cleanups.
+    unsigned LifetimeExtendedDepth;
     unsigned Index;
   };
 
@@ -1143,6 +1150,10 @@ class CodeGenFunction : public CodeGenTypeCache {
   PopCleanupBlocks(EHScopeStack::stable_iterator OldCleanupStackSize,
                    std::initializer_list<llvm::Value **> ValuesToReload = {});
 
+  /// Adds lifetime-extended cleanups from the given position to the stack.
+  /// (does not remove the cleanups from lifetime extended stack).
+  void AddLifetimeExtendedCleanups(size_t OldLifetimeExtendedSize);
+
   /// Takes the old cleanup stack size and emits the cleanup blocks
   /// that have been added, then adds all lifetime-extended cleanups from
   /// the given position to the stack.
@@ -1157,8 +1168,8 @@ class CodeGenFunction : public CodeGenTypeCache {
   /// target of a potentially scope-crossing jump; get a stable handle
   /// to which we can perform this jump later.
   JumpDest getJumpDestInCurrentScope(llvm::BasicBlock *Target) {
-    return JumpDest(Target,
-                    EHStack.getInnermostNormalCleanup(),
+    return JumpDest(Target, EHStack.getInnermostNormalCleanup(),
+                    LifetimeExtendedCleanupStack.size(),
                     NextCleanupDestIndex++);
   }
 

clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp

@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+// Context: GH63818
+struct Printy {
+  ~Printy() { }
+};
+
+struct Printies {
+  const Printy &a;
+  const Printy &b;
+  ~Printies() {}
+};
+
+bool foo();
+
+void bar() {
+// CHECK: define dso_local void @_Z3barv()
+// CHECK-NOT: call void @_ZN6PrintyD1Ev
+  Printies p2{
+    Printy(), 
+    ({
+      if(foo()) {
+        // CHECK: if.then:
+        // CHECK-NEXT:    call void @_ZN6PrintyD1Ev
+        // CHECK-NEXT:    br label %return
+        // CHECK-NOT: call void @_ZN6PrintyD1Ev
+        return;
+      }
+      Printy();
+    })};
+  // CHECK:       if.end:
+  // CHECK:         call void @_ZN8PrintiesD1Ev
+  // CHECK:         call void @_ZN6PrintyD1Ev
+  // CHECK:         call void @_ZN6PrintyD1Ev
+  // CHECK-NEXT:    br label %return
+  return;
+  // CHECK-NOT: call void @_ZN6PrintyD1Ev
+}
+
+
+void test_break() {
+// CHECK: define dso_local void @_Z10test_breakv()
+// CHECK-NOT: call void @_ZN6PrintyD1Ev
+  Printies p2{Printy(), ({
+                for (;;) {
+                  Printies p3{Printy(), ({
+                                if(foo()) {
+                                  break;
+                                  // CHECK:       if.then:
+                                  // CHECK-NEXT:    call void @_ZN6PrintyD1Ev
+                                  // CHECK-NEXT:    br label %for.end
+                                  // CHECK-NOT:   call void @_ZN6PrintyD1Ev
+                                }
+                                Printy();
+                              })};
+                  // CHECK:         if.end:
+                  // CHECK:           call void @_ZN6PrintyD1Ev
+                  // CHECK:           call void @_ZN6PrintyD1Ev
+                  // CHECK-NOT:     call void @_ZN6PrintyD1Ev
+                }
+                Printy();
+              })};
+  // CHECK:         for.end:
+  // CHECK-COUNT-2:   call void @_ZN6PrintyD1Ev
+  // CHECK-NEXT:      ret void
+  // CHECK-NOT:     call void @_ZN6PrintyD1Ev
+}
+

clang/test/CodeGenCoroutines/coro-suspend-in-agg-init.cpp

@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 --std=c++20 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+// Context: GH63818
+
+#include "Inputs/coroutine.h"
+
+struct coroutine {
+  struct promise_type;
+  std::coroutine_handle<promise_type> handle;
+};
+
+struct coroutine::promise_type {
+  coroutine get_return_object() {
+    return {std::coroutine_handle<promise_type>::from_promise(*this)};
+  }
+  std::suspend_never initial_suspend() noexcept { return {}; }
+  std::suspend_always final_suspend() noexcept { return {}; }
+  void return_void() {}
+  void unhandled_exception() {}
+};
+
+struct Printy { ~Printy(); };
+
+struct Printies {
+  const Printy &a;
+  const Printy &b;
+  const Printy &c;
+};
+
+struct Awaiter : std::suspend_always {
+  Printy await_resume() { return {}; }
+};
+
+// CHECK: define dso_local ptr @_Z5test1v()
+coroutine test1() {
+  // CHECK-NOT: @_ZN6PrintyD1Ev
+  Printies p1{
+    Printy(),
+    co_await Awaiter{},
+    // CHECK:       await.cleanup:
+    // CHECK-NEXT:    call void @_ZN6PrintyD1Ev
+    // CHECK-NEXT:    br label %cleanup{{.*}}.from.await.cleanup
+    // CHECK-NOT: @_ZN6PrintyD1Ev
+
+    co_await Awaiter{}
+    // CHECK:       await2.cleanup:
+    // CHECK-NEXT:    call void @_ZN6PrintyD1Ev
+    // CHECK-NEXT:    call void @_ZN6PrintyD1Ev
+    // CHECK-NEXT:    br label %cleanup{{.*}}.from.await2.cleanup
+    // CHECK-NOT: @_ZN6PrintyD1Ev
+  };
+
+  // CHECK-COUNT-3:       call void @_ZN6PrintyD1Ev
+  // CHECK-NEXT:          br label
+
+  // CHECK-NOT: @_ZN6PrintyD1Ev
+
+  // CHECK: unreachable:
+}
+
+void bar(const Printy& a, const Printy& b);
+
+// CHECK: define dso_local ptr @_Z5test2v()
+coroutine test2() {
+  // CHECK-NOT: @_ZN6PrintyD1Ev
+  bar(
+    Printy(),
+    co_await Awaiter{}
+    // CHECK:       await.cleanup:
+    // CHECK-NEXT:    br label %cleanup{{.*}}.from.await.cleanup
+    // CHECK-NOT: @_ZN6PrintyD1Ev
+  );
+  // CHECK: await.ready:
+  // CHECK:   call void @_ZN6PrintyD1Ev
+  // CHECK-NOT: @_ZN6PrintyD1Ev
+
+  // CHECK: cleanup{{.*}}:
+  // CHECK:   call void @_ZN6PrintyD1Ev
+  // CHECK-NOT: @_ZN6PrintyD1Ev
+
+  // CHECK: unreachable:
+}