https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/80698
>From 28d90711ff8e4924135d4bd4e5f252d96ac41b93 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 23 Jan 2024 20:18:25 +0000 Subject: [PATCH 01/12] [misc-coroutine-hostile-raii] Use getOperand instead of getCommonExpr --- .../misc/CoroutineHostileRAIICheck.cpp | 2 +- .../checkers/misc/coroutine-hostile-raii.cpp | 34 ++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp index a0e8700b0522bc..360335b86c6418 100644 --- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp @@ -56,7 +56,7 @@ AST_MATCHER_P(Stmt, forEachPrevStmt, ast_matchers::internal::Matcher<Stmt>, // Matches the expression awaited by the `co_await`. AST_MATCHER_P(CoawaitExpr, awaitable, ast_matchers::internal::Matcher<Expr>, InnerMatcher) { - if (Expr *E = Node.getCommonExpr()) + if (Expr *E = Node.getOperand()) return InnerMatcher.matches(*E, Finder, Builder); return false; } diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/coroutine-hostile-raii.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/coroutine-hostile-raii.cpp index 55a7e4b8f2954a..c23c355dac1b2a 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/coroutine-hostile-raii.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/coroutine-hostile-raii.cpp @@ -1,7 +1,7 @@ // RUN: %check_clang_tidy -std=c++20 %s misc-coroutine-hostile-raii %t \ // RUN: -config="{CheckOptions: {\ // RUN: misc-coroutine-hostile-raii.RAIITypesList: 'my::Mutex; ::my::other::Mutex', \ -// RUN: misc-coroutine-hostile-raii.AllowedAwaitablesList: 'safe::awaitable; ::my::other::awaitable' \ +// RUN: misc-coroutine-hostile-raii.AllowedAwaitablesList: 'safe::awaitable; ::transformable::awaitable' \ // RUN: }}" namespace std { @@ -136,6 +136,9 @@ ReturnObject scopedLockableTest() { absl::Mutex no_warning_5; } +// ================================================================================ +// Safe awaitable +// ================================================================================ namespace safe { struct awaitable { bool await_ready() noexcept { return false; } @@ -150,6 +153,32 @@ ReturnObject RAIISafeSuspendTest() { co_await other{}; } +// ================================================================================ +// Safe transformable awaitable +// ================================================================================ +struct transformable { struct awaitable{}; }; +using alias_transformable_awaitable = transformable::awaitable; +struct UseTransformAwaitable { + struct promise_type { + UseTransformAwaitable get_return_object() { return {}; } + std::suspend_always initial_suspend() { return {}; } + std::suspend_always final_suspend() noexcept { return {}; } + void unhandled_exception() {} + std::suspend_always await_transform(transformable::awaitable) { return {}; } + }; +}; + +auto retAwaitable() { return transformable::awaitable{}; } +UseTransformAwaitable RAIISafeSuspendTest2() { + absl::Mutex a; + co_await retAwaitable(); + co_await transformable::awaitable{}; + co_await alias_transformable_awaitable{}; +} + +// ================================================================================ +// Lambdas +// ================================================================================ void lambda() { absl::Mutex no_warning; auto lambda = []() -> ReturnObject { @@ -164,6 +193,9 @@ void lambda() { absl::Mutex no_warning_2; } +// ================================================================================ +// Denylisted RAII +// ================================================================================ template<class T> ReturnObject raii_in_template(){ T a; >From 442809ce6047f8f83c9f3f54b17182c18535bb03 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Mon, 5 Feb 2024 15:52:56 +0000 Subject: [PATCH 02/12] [codegen] Emit cleanups for lifetime-extended temporaries when statment-expression has control-flow --- clang/lib/CodeGen/CGCleanup.cpp | 31 +++++-- clang/lib/CodeGen/CodeGenFunction.h | 4 + .../return-in-stmt-expr-cleanup.cpp | 37 +++++++++ .../coro-suspend-in-agg-init.cpp | 82 +++++++++++++++++++ 4 files changed, 145 insertions(+), 9 deletions(-) create mode 100644 clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp create mode 100644 clang/test/CodeGenCoroutines/coro-suspend-in-agg-init.cpp diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp index f87caf050eeaa7..da0528b271aa37 100644 --- a/clang/lib/CodeGen/CGCleanup.cpp +++ b/clang/lib/CodeGen/CGCleanup.cpp @@ -488,16 +488,11 @@ 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 +514,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 +1108,13 @@ 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); + AddLifetimeExtendedCleanups(0); + // Create the branch. llvm::BranchInst *BI = Builder.CreateBr(Dest.getBlock()); diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 143ad64e8816b1..ac55e84034bc60 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1143,6 +1143,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. diff --git a/clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp b/clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp new file mode 100644 index 00000000000000..214becd81e61af --- /dev/null +++ b/clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp @@ -0,0 +1,37 @@ +// 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() { + Printies p2{ + // CHECK: store ptr %ref.tmp + Printy(), + ({ + if(foo()) { + // CHECK-LABEL: if.then: + // CHECK-NEXT: call void @_ZN6PrintyD1Ev + // CHECK-NEXT: br label %return + return; + } + // CHECK-LABEL: if.end: + // CHECK-NEXT: store ptr %ref.tmp1 + Printy(); + })}; + // CHECK-NEXT: call void @_ZN8PrintiesD1Ev + // CHECK-NEXT: call void @_ZN6PrintyD1Ev + // CHECK-NEXT: call void @_ZN6PrintyD1Ev + // CHECK-NEXT: br label %return + return; +} + diff --git a/clang/test/CodeGenCoroutines/coro-suspend-in-agg-init.cpp b/clang/test/CodeGenCoroutines/coro-suspend-in-agg-init.cpp new file mode 100644 index 00000000000000..362e212b7fba39 --- /dev/null +++ b/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: +} >From 41258086773829db7f17afa9df192d6ca56b2bfa Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Mon, 5 Feb 2024 16:52:25 +0000 Subject: [PATCH 03/12] format --- clang/lib/CodeGen/CGCleanup.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp index da0528b271aa37..39c65c0b07c6f6 100644 --- a/clang/lib/CodeGen/CGCleanup.cpp +++ b/clang/lib/CodeGen/CGCleanup.cpp @@ -489,7 +489,8 @@ void CodeGenFunction::PopCleanupBlocks( } /// Adds deferred lifetime-extended cleanups onto the EH stack. -void CodeGenFunction::AddLifetimeExtendedCleanups(size_t OldLifetimeExtendedSize) { +void CodeGenFunction::AddLifetimeExtendedCleanups( + size_t OldLifetimeExtendedSize) { for (size_t I = OldLifetimeExtendedSize, E = LifetimeExtendedCleanupStack.size(); I != E;) { >From 35437116444d9ac2827634456a449c19394434d7 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 6 Feb 2024 14:27:40 +0000 Subject: [PATCH 04/12] Emit not-all but only necessary lifetime-extended cleanups for each branch --- clang/lib/CodeGen/CGCleanup.cpp | 3 +- clang/lib/CodeGen/CGStmt.cpp | 2 +- clang/lib/CodeGen/CodeGenFunction.h | 17 ++++- .../control-flow-in-stmt-expr-cleanup.cpp | 68 +++++++++++++++++++ .../return-in-stmt-expr-cleanup.cpp | 37 ---------- 5 files changed, 85 insertions(+), 42 deletions(-) create mode 100644 clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp delete mode 100644 clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp index 39c65c0b07c6f6..a5a49917d108f9 100644 --- a/clang/lib/CodeGen/CGCleanup.cpp +++ b/clang/lib/CodeGen/CGCleanup.cpp @@ -1114,7 +1114,8 @@ void CodeGenFunction::EmitBranchThroughCleanup(JumpDest Dest) { // EHStack. Do not remove them from lifetime-extended stack, they need to be // emitted again after the expression completes. RunCleanupsScope LifetimeExtendedCleanups(*this); - AddLifetimeExtendedCleanups(0); + if(Dest.isValid()) + AddLifetimeExtendedCleanups(Dest.getLifetimeExtendedDepth()); // Create the branch. llvm::BranchInst *BI = Builder.CreateBr(Dest.getBlock()); diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index beff0ad9da2709..8d7e2616c168c0 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/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; } diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index ac55e84034bc60..78bc45dca2936c 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -238,15 +238,20 @@ 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 +261,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; }; @@ -1163,6 +1173,7 @@ class CodeGenFunction : public CodeGenTypeCache { JumpDest getJumpDestInCurrentScope(llvm::BasicBlock *Target) { return JumpDest(Target, EHStack.getInnermostNormalCleanup(), + LifetimeExtendedCleanupStack.size(), NextCleanupDestIndex++); } diff --git a/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp b/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp new file mode 100644 index 00000000000000..a53b0fcf040927 --- /dev/null +++ b/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 +} + diff --git a/clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp b/clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp deleted file mode 100644 index 214becd81e61af..00000000000000 --- a/clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp +++ /dev/null @@ -1,37 +0,0 @@ -// 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() { - Printies p2{ - // CHECK: store ptr %ref.tmp - Printy(), - ({ - if(foo()) { - // CHECK-LABEL: if.then: - // CHECK-NEXT: call void @_ZN6PrintyD1Ev - // CHECK-NEXT: br label %return - return; - } - // CHECK-LABEL: if.end: - // CHECK-NEXT: store ptr %ref.tmp1 - Printy(); - })}; - // CHECK-NEXT: call void @_ZN8PrintiesD1Ev - // CHECK-NEXT: call void @_ZN6PrintyD1Ev - // CHECK-NEXT: call void @_ZN6PrintyD1Ev - // CHECK-NEXT: br label %return - return; -} - >From d23cdb3fb5998cab7e7102ac715eca8e0bfe57c0 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 6 Feb 2024 14:34:25 +0000 Subject: [PATCH 05/12] format --- clang/lib/CodeGen/CGCleanup.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp index a5a49917d108f9..e97441310e34c6 100644 --- a/clang/lib/CodeGen/CGCleanup.cpp +++ b/clang/lib/CodeGen/CGCleanup.cpp @@ -1114,7 +1114,7 @@ void CodeGenFunction::EmitBranchThroughCleanup(JumpDest Dest) { // 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()) + if (Dest.isValid()) AddLifetimeExtendedCleanups(Dest.getLifetimeExtendedDepth()); // Create the branch. >From dc3659a4f061ab05fd51ed39d673c5a2710cd41f Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 6 Feb 2024 14:35:24 +0000 Subject: [PATCH 06/12] format --- clang/lib/CodeGen/CodeGenFunction.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 78bc45dca2936c..11e4c6481776f7 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -249,9 +249,7 @@ class CodeGenFunction : public CodeGenTypeCache { llvm::BasicBlock *getBlock() const { return Block; } EHScopeStack::stable_iterator getScopeDepth() const { return ScopeDepth; } unsigned getDestIndex() const { return Index; } - unsigned getLifetimeExtendedDepth() const { - return LifetimeExtendedDepth; - } + unsigned getLifetimeExtendedDepth() const { return LifetimeExtendedDepth; } // This should be used cautiously. void setScopeDepth(EHScopeStack::stable_iterator depth) { @@ -1171,8 +1169,7 @@ 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++); } >From ddf7c244e6d142cf61c7277d06ead2e0fb79f4a3 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 6 Feb 2024 14:52:44 +0000 Subject: [PATCH 07/12] format --- clang/lib/CodeGen/CodeGenFunction.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 11e4c6481776f7..ec9f0de0c03385 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -242,8 +242,7 @@ class CodeGenFunction : public CodeGenTypeCache { JumpDest(llvm::BasicBlock *Block, EHScopeStack::stable_iterator Depth, unsigned LifetimeExtendedScopeDepth, unsigned Index) : Block(Block), ScopeDepth(Depth), - LifetimeExtendedDepth(LifetimeExtendedScopeDepth), Index(Index) { - } + LifetimeExtendedDepth(LifetimeExtendedScopeDepth), Index(Index) {} bool isValid() const { return Block != nullptr; } llvm::BasicBlock *getBlock() const { return Block; } >From a58284b872ca4118426d0e9339118c025ef8a0d3 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Fri, 9 Feb 2024 20:03:27 +0000 Subject: [PATCH 08/12] Add BranchInExpr cleanups kind to handle more missing dtor calls --- clang/lib/CodeGen/CGCleanup.cpp | 40 +++--- clang/lib/CodeGen/CGDecl.cpp | 15 ++- clang/lib/CodeGen/CGExprAgg.cpp | 19 ++- clang/lib/CodeGen/CGStmt.cpp | 6 +- clang/lib/CodeGen/CodeGenFunction.cpp | 3 + clang/lib/CodeGen/CodeGenFunction.h | 117 ++++++++++++----- clang/lib/CodeGen/EHScopeStack.h | 10 ++ .../control-flow-in-stmt-expr-cleanup.cpp | 119 +++++++++++++++++- 8 files changed, 260 insertions(+), 69 deletions(-) diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp index e97441310e34c6..af787440f8847b 100644 --- a/clang/lib/CodeGen/CGCleanup.cpp +++ b/clang/lib/CodeGen/CGCleanup.cpp @@ -18,6 +18,8 @@ #include "CGCleanup.h" #include "CodeGenFunction.h" +#include "EHScopeStack.h" +#include "llvm/Support/ErrorHandling.h" #include "llvm/Support/SaveAndRestore.h" using namespace clang; @@ -488,29 +490,22 @@ void CodeGenFunction::PopCleanupBlocks( } } -/// 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;) { +void CodeGenFunction::AddDeferredCleanups(DeferredCleanupStack &Cleanups, + size_t OldSize) { + for (size_t I = OldSize, E = Cleanups.size(); I != E;) { // Alignment should be guaranteed by the vptrs in the individual cleanups. - assert((I % alignof(LifetimeExtendedCleanupHeader) == 0) && + assert((I % alignof(DeferredCleanupHeader) == 0) && "misaligned cleanup stack entry"); - LifetimeExtendedCleanupHeader &Header = - reinterpret_cast<LifetimeExtendedCleanupHeader&>( - LifetimeExtendedCleanupStack[I]); + DeferredCleanupHeader &Header = + reinterpret_cast<DeferredCleanupHeader &>(Cleanups[I]); I += sizeof(Header); - EHStack.pushCopyOfCleanup(Header.getKind(), - &LifetimeExtendedCleanupStack[I], - Header.getSize()); + EHStack.pushCopyOfCleanup(Header.getKind(), &Cleanups[I], Header.getSize()); I += Header.getSize(); if (Header.isConditional()) { - Address ActiveFlag = - reinterpret_cast<Address &>(LifetimeExtendedCleanupStack[I]); + Address ActiveFlag = reinterpret_cast<Address &>(Cleanups[I]); initFullExprCleanupWithFlag(ActiveFlag); I += sizeof(ActiveFlag); } @@ -524,8 +519,8 @@ void CodeGenFunction::PopCleanupBlocks( std::initializer_list<llvm::Value **> ValuesToReload) { PopCleanupBlocks(Old, ValuesToReload); - // Move our deferred cleanups onto the EH stack. - AddLifetimeExtendedCleanups(OldLifetimeExtendedSize); + // Move our deferred lifetime-extended cleanups onto the EH stack. + AddDeferredCleanups(LifetimeExtendedCleanupStack, OldLifetimeExtendedSize); LifetimeExtendedCleanupStack.resize(OldLifetimeExtendedSize); } @@ -1109,13 +1104,12 @@ 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 we are emitting a branch in a partial expression, add deferred cleanups + // to EHStack, which would otherwise have only been emitted after the full + // expression. + RunCleanupsScope BranchInExprCleanups(*this); if (Dest.isValid()) - AddLifetimeExtendedCleanups(Dest.getLifetimeExtendedDepth()); + AddDeferredCleanups(BranchInExprCleanupStack, Dest.getBranchInExprDepth()); // Create the branch. llvm::BranchInst *BI = Builder.CreateBr(Dest.getBlock()); diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp index bbe14ef4c17244..66df9186155d9c 100644 --- a/clang/lib/CodeGen/CGDecl.cpp +++ b/clang/lib/CodeGen/CGDecl.cpp @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +#include "Address.h" #include "CGBlocks.h" #include "CGCXXABI.h" #include "CGCleanup.h" @@ -19,6 +20,7 @@ #include "CodeGenFunction.h" #include "CodeGenModule.h" #include "ConstantEmitter.h" +#include "EHScopeStack.h" #include "PatternInit.h" #include "TargetInfo.h" #include "clang/AST/ASTContext.h" @@ -2209,8 +2211,11 @@ void CodeGenFunction::pushLifetimeExtendedDestroy(CleanupKind cleanupKind, static_cast<CleanupKind>(cleanupKind & ~NormalCleanup), addr, type, destroyer, useEHCleanupForArray); - return pushCleanupAfterFullExprWithActiveFlag<DestroyObject>( - cleanupKind, Address::invalid(), addr, type, destroyer, useEHCleanupForArray); + pushDestroy(BranchInExprCleanup, addr, type, destroyer, + useEHCleanupForArray); + return pushDeferredCleanup<DestroyObject>( + LifetimeExtendedCleanupStack, cleanupKind, Address::invalid(), addr, + type, destroyer, useEHCleanupForArray); } // Otherwise, we should only destroy the object if it's been initialized. @@ -2232,9 +2237,9 @@ void CodeGenFunction::pushLifetimeExtendedDestroy(CleanupKind cleanupKind, initFullExprCleanupWithFlag(ActiveFlag); } - pushCleanupAfterFullExprWithActiveFlag<ConditionalCleanupType>( - cleanupKind, ActiveFlag, SavedAddr, type, destroyer, - useEHCleanupForArray); + pushDeferredCleanup<ConditionalCleanupType>( + LifetimeExtendedCleanupStack, cleanupKind, ActiveFlag, SavedAddr, type, + destroyer, useEHCleanupForArray); } /// emitDestroy - Immediately perform the destruction of the given diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index 22f55fe9aac904..bd2e9bc0f82a91 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -15,6 +15,7 @@ #include "CodeGenFunction.h" #include "CodeGenModule.h" #include "ConstantEmitter.h" +#include "EHScopeStack.h" #include "TargetInfo.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Attr.h" @@ -553,6 +554,7 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, Address endOfInit = Address::invalid(); EHScopeStack::stable_iterator cleanup; llvm::Instruction *cleanupDominator = nullptr; + auto myDtorKind = dtorKind; if (CGF.needsEHCleanup(dtorKind)) { // In principle we could tell the cleanup where we are more // directly, but the control flow can get so varied here that it @@ -580,6 +582,7 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, // elements have been initialized. llvm::Value *element = begin; + CodeGenFunction::RestoreBranchInExpr branchInExpr(CGF); // Emit the explicit initializers. for (uint64_t i = 0; i != NumInitElements; ++i) { // Advance to the next element. @@ -592,10 +595,14 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, // observed to be unnecessary. if (endOfInit.isValid()) Builder.CreateStore(element, endOfInit); } - - LValue elementLV = CGF.MakeAddrLValue( - Address(element, llvmElementType, elementAlign), elementType); + Address address = Address(element, llvmElementType, elementAlign); + LValue elementLV = CGF.MakeAddrLValue(address, elementType); EmitInitializationToLValue(Args[i], elementLV); + // Schedule to emit element cleanup if we see a branch in the array + // initialisation statement. + if (CGF.needsBranchCleanup(myDtorKind)) + CGF.pushDestroy(BranchInExprCleanup, address, elementType, + CGF.getDestroyer(myDtorKind), false /*oder true ?*/); } // Check whether there's a non-trivial array-fill expression. @@ -1754,7 +1761,7 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr( return; } - + CodeGenFunction::RestoreBranchInExpr branchInExpr(CGF); // Here we iterate over the fields; this makes it simpler to both // default-initialize fields and skip over unnamed fields. for (const auto *field : record->fields()) { @@ -1793,6 +1800,10 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr( if (QualType::DestructionKind dtorKind = field->getType().isDestructedType()) { assert(LV.isSimple()); + if(CGF.needsBranchCleanup(dtorKind)) { + CGF.pushDestroy(BranchInExprCleanup, LV.getAddress(CGF), + field->getType(), CGF.getDestroyer(dtorKind), false); + } if (CGF.needsEHCleanup(dtorKind)) { CGF.pushDestroy(EHCleanup, LV.getAddress(CGF), field->getType(), CGF.getDestroyer(dtorKind), false); diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index 8d7e2616c168c0..9f1ab1e7817179 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -627,9 +627,9 @@ CodeGenFunction::getJumpDestForLabel(const LabelDecl *D) { if (Dest.isValid()) return Dest; // Create, but don't insert, the new block. - Dest = JumpDest(createBasicBlock(D->getName()), - EHScopeStack::stable_iterator::invalid(), 0, - NextCleanupDestIndex++); + Dest = JumpDest( + createBasicBlock(D->getName()), EHScopeStack::stable_iterator::invalid(), + EHScopeStack::stable_iterator::invalid(), 0, NextCleanupDestIndex++); return Dest; } diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 1ad905078d349c..7bff6c89de563c 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -331,6 +331,9 @@ static void EmitIfUsed(CodeGenFunction &CGF, llvm::BasicBlock *BB) { void CodeGenFunction::FinishFunction(SourceLocation EndLoc) { assert(BreakContinueStack.empty() && "mismatched push/pop in break/continue stack!"); + assert(LifetimeExtendedCleanupStack.empty() && + "mismatched push/pop in stack!"); + assert(BranchInExprCleanupStack.empty() && "mismatched push/pop in stack!"); bool OnlySimpleReturnStmts = NumSimpleReturnExprs > 0 && NumSimpleReturnExprs == NumReturnExprs diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index ec9f0de0c03385..5310ec8440fd5a 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -238,17 +238,19 @@ 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), LifetimeExtendedDepth(0), Index(0) {} + JumpDest() : Block(nullptr), BranchInExprDepth(0), Index(0) {} JumpDest(llvm::BasicBlock *Block, EHScopeStack::stable_iterator Depth, - unsigned LifetimeExtendedScopeDepth, unsigned Index) - : Block(Block), ScopeDepth(Depth), - LifetimeExtendedDepth(LifetimeExtendedScopeDepth), Index(Index) {} + EHScopeStack::stable_iterator EHScopeDepth, + unsigned BranchInExprDepth, unsigned Index) + : Block(Block), ScopeDepth(Depth), EHScopeDepth(EHScopeDepth), + BranchInExprDepth(BranchInExprDepth), Index(Index) {} bool isValid() const { return Block != nullptr; } llvm::BasicBlock *getBlock() const { return Block; } EHScopeStack::stable_iterator getScopeDepth() const { return ScopeDepth; } + EHScopeStack::stable_iterator getEHScopeDepth() const { return EHScopeDepth; } unsigned getDestIndex() const { return Index; } - unsigned getLifetimeExtendedDepth() const { return LifetimeExtendedDepth; } + unsigned getBranchInExprDepth() const { return BranchInExprDepth; } // This should be used cautiously. void setScopeDepth(EHScopeStack::stable_iterator depth) { @@ -258,11 +260,12 @@ class CodeGenFunction : public CodeGenTypeCache { private: llvm::BasicBlock *Block; EHScopeStack::stable_iterator ScopeDepth; + EHScopeStack::stable_iterator EHScopeDepth; - // 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; + // Size of the branch-in-expr cleanup stack in destination scope. + // All cleanups beyond this depth would be emitted on encountering a branch + // to this destination inside an expression. + unsigned BranchInExprDepth; unsigned Index; }; @@ -633,7 +636,33 @@ class CodeGenFunction : public CodeGenTypeCache { llvm::DenseMap<const VarDecl *, llvm::Value *> NRVOFlags; EHScopeStack EHStack; - llvm::SmallVector<char, 256> LifetimeExtendedCleanupStack; + + using DeferredCleanupStack = llvm::SmallVector<char, 256>; + + // Deferred cleanups for lifetime-extended temporaries. Such cleanups are + // deferred until the end of the full expression, after which these are added + // to the EHStack. + DeferredCleanupStack LifetimeExtendedCleanupStack; + + // Branch-in-expression cleanups include the cleanups which are not yet added + // to the EHStack while building an expression. + // Cleanups from this stack are only emitted when encountering a branch while + // building an expression (eg: branches in stmt-expr or coroutine + // suspensions). Otherwise, these should be cleared the end of the expression and + // added separately to the EHStack. + DeferredCleanupStack BranchInExprCleanupStack; + + class RestoreBranchInExpr { + public: + RestoreBranchInExpr(CodeGenFunction &CGF) + : CGF(CGF), OldSize(CGF.BranchInExprCleanupStack.size()) {} + ~RestoreBranchInExpr() { CGF.BranchInExprCleanupStack.resize(OldSize); } + + private: + CodeGenFunction &CGF; + size_t OldSize; + }; + llvm::SmallVector<const JumpDest *, 2> SEHTryEpilogueStack; llvm::Instruction *CurrentFuncletPad = nullptr; @@ -653,8 +682,8 @@ class CodeGenFunction : public CodeGenTypeCache { } }; - /// Header for data within LifetimeExtendedCleanupStack. - struct LifetimeExtendedCleanupHeader { + /// Header for data within deferred cleanup stacks. + struct DeferredCleanupHeader { /// The size of the following cleanup object. unsigned Size; /// The kind of cleanup to push: a value from the CleanupKind enumeration. @@ -784,6 +813,14 @@ class CodeGenFunction : public CodeGenTypeCache { /// we're currently inside a conditionally-evaluated expression. template <class T, class... As> void pushFullExprCleanup(CleanupKind kind, As... A) { + if (kind & BranchInExprCleanup) { + // Defer BranchInExprCleanup as a NormalCleanup (emitted only if we see + // a branch). Do not add these to the EHStack as they should be added + // separately with a different CleanupKind. + pushDeferredCleanup<T>(BranchInExprCleanupStack, NormalCleanup, + Address::invalid(), A...); + return; + } // If we're not in a conditional branch, or if none of the // arguments requires saving, then use the unconditional cleanup. if (!isInConditionalBranch()) @@ -803,8 +840,8 @@ class CodeGenFunction : public CodeGenTypeCache { template <class T, class... As> void pushCleanupAfterFullExpr(CleanupKind Kind, As... A) { if (!isInConditionalBranch()) - return pushCleanupAfterFullExprWithActiveFlag<T>(Kind, Address::invalid(), - A...); + return pushDeferredCleanup<T>(LifetimeExtendedCleanupStack, Kind, + Address::invalid(), A...); Address ActiveFlag = createCleanupActiveFlag(); assert(!DominatingValue<Address>::needsSaving(ActiveFlag) && @@ -814,24 +851,23 @@ class CodeGenFunction : public CodeGenTypeCache { SavedTuple Saved{saveValueInCond(A)...}; typedef EHScopeStack::ConditionalCleanup<T, As...> CleanupType; - pushCleanupAfterFullExprWithActiveFlag<CleanupType>(Kind, ActiveFlag, Saved); + pushDeferredCleanup<CleanupType>(LifetimeExtendedCleanupStack, Kind, + ActiveFlag, Saved); } template <class T, class... As> - void pushCleanupAfterFullExprWithActiveFlag(CleanupKind Kind, - Address ActiveFlag, As... A) { - LifetimeExtendedCleanupHeader Header = {sizeof(T), Kind, - ActiveFlag.isValid()}; + void pushDeferredCleanup(DeferredCleanupStack &Cleanups, CleanupKind Kind, + Address ActiveFlag, As... A) { + DeferredCleanupHeader Header = {sizeof(T), Kind, ActiveFlag.isValid()}; - size_t OldSize = LifetimeExtendedCleanupStack.size(); - LifetimeExtendedCleanupStack.resize( - LifetimeExtendedCleanupStack.size() + sizeof(Header) + Header.Size + - (Header.IsConditional ? sizeof(ActiveFlag) : 0)); + size_t OldSize = Cleanups.size(); + Cleanups.resize(Cleanups.size() + sizeof(Header) + Header.Size + + (Header.IsConditional ? sizeof(ActiveFlag) : 0)); static_assert(sizeof(Header) % alignof(T) == 0, "Cleanup will be allocated on misaligned address"); - char *Buffer = &LifetimeExtendedCleanupStack[OldSize]; - new (Buffer) LifetimeExtendedCleanupHeader(Header); + char *Buffer = &Cleanups[OldSize]; + new (Buffer) DeferredCleanupHeader(Header); new (Buffer + sizeof(Header)) T(A...); if (Header.IsConditional) new (Buffer + sizeof(Header) + sizeof(T)) Address(ActiveFlag); @@ -888,6 +924,8 @@ class CodeGenFunction : public CodeGenTypeCache { class RunCleanupsScope { EHScopeStack::stable_iterator CleanupStackDepth, OldCleanupScopeDepth; size_t LifetimeExtendedCleanupStackSize; + // size_t BranchInExprCleanupStackSize; + RestoreBranchInExpr RestoreBranchInExpr; bool OldDidCallStackSave; protected: bool PerformCleanup; @@ -902,11 +940,11 @@ class CodeGenFunction : public CodeGenTypeCache { public: /// Enter a new cleanup scope. explicit RunCleanupsScope(CodeGenFunction &CGF) - : PerformCleanup(true), CGF(CGF) - { + : RestoreBranchInExpr(CGF), PerformCleanup(true), CGF(CGF) { CleanupStackDepth = CGF.EHStack.stable_begin(); LifetimeExtendedCleanupStackSize = CGF.LifetimeExtendedCleanupStack.size(); + // BranchInExprCleanupStackSize = CGF.BranchInExprCleanupStack.size(); OldDidCallStackSave = CGF.DidCallStackSave; CGF.DidCallStackSave = false; OldCleanupScopeDepth = CGF.CurrentCleanupScopeDepth; @@ -1150,9 +1188,11 @@ 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); + /// Adds deferred cleanups from the given position to the EHStack. + /// These could be lifetime-extended cleanups or branch-in-expr cleanups. + /// (does not remove the cleanups from the original stack). + void AddDeferredCleanups(DeferredCleanupStack &DeferredCleanupsStack, + size_t OldSize); /// Takes the old cleanup stack size and emits the cleanup blocks /// that have been added, then adds all lifetime-extended cleanups from @@ -1169,8 +1209,8 @@ class CodeGenFunction : public CodeGenTypeCache { /// to which we can perform this jump later. JumpDest getJumpDestInCurrentScope(llvm::BasicBlock *Target) { return JumpDest(Target, EHStack.getInnermostNormalCleanup(), - LifetimeExtendedCleanupStack.size(), - NextCleanupDestIndex++); + EHStack.getInnermostEHScope(), + BranchInExprCleanupStack.size(), NextCleanupDestIndex++); } /// The given basic block lies in the current EH scope, but may be a @@ -2163,6 +2203,19 @@ class CodeGenFunction : public CodeGenTypeCache { llvm_unreachable("bad destruction kind"); } + bool needsBranchCleanup(QualType::DestructionKind kind) { + switch (kind) { + case QualType::DK_none: + return false; + case QualType::DK_cxx_destructor: + case QualType::DK_objc_weak_lifetime: + case QualType::DK_nontrivial_c_struct: + case QualType::DK_objc_strong_lifetime: + return true; + } + llvm_unreachable("bad destruction kind"); + } + CleanupKind getCleanupKind(QualType::DestructionKind kind) { return (needsEHCleanup(kind) ? NormalAndEHCleanup : NormalCleanup); } diff --git a/clang/lib/CodeGen/EHScopeStack.h b/clang/lib/CodeGen/EHScopeStack.h index 0c667e80bb6d8c..feffa37c87f6b1 100644 --- a/clang/lib/CodeGen/EHScopeStack.h +++ b/clang/lib/CodeGen/EHScopeStack.h @@ -85,6 +85,13 @@ enum CleanupKind : unsigned { NormalAndEHCleanup = EHCleanup | NormalCleanup, + // Denotes a deferred cleanup while building an expression. These cleanups are + // emitted on seeing a branch in an partially built expression (eg. branches + // in stmt-expr and coroutine suspensions). + // This cleanup type should not be used with other types. Cleanups of other + // types should be added separately to the EHStack. + BranchInExprCleanup = 0x4, + LifetimeMarker = 0x8, NormalEHLifetimeMarker = LifetimeMarker | NormalAndEHCleanup, }; @@ -244,6 +251,9 @@ class EHScopeStack { /// The innermost EH scope on the stack. stable_iterator InnermostEHScope; + /// The innermost EH scope on the stack. + stable_iterator InnermostBranchExprCleanup; + /// The CGF this Stack belong to CodeGenFunction* CGF; diff --git a/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp b/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp index a53b0fcf040927..ee380a167e3fbe 100644 --- a/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp +++ b/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s +// RUN: %clang_cc1 --std=c++20 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s // Context: GH63818 struct Printy { @@ -13,6 +13,9 @@ struct Printies { bool foo(); +// ==================================== +// Init with lifetime extensions +// ==================================== void bar() { // CHECK: define dso_local void @_Z3barv() // CHECK-NOT: call void @_ZN6PrintyD1Ev @@ -37,7 +40,9 @@ void bar() { // CHECK-NOT: call void @_ZN6PrintyD1Ev } - +// ==================================== +// Break in stmt-expr +// ==================================== void test_break() { // CHECK: define dso_local void @_Z10test_breakv() // CHECK-NOT: call void @_ZN6PrintyD1Ev @@ -66,3 +71,113 @@ void test_break() { // CHECK-NOT: call void @_ZN6PrintyD1Ev } +// ============================================= +// Initialisation without lifetime-extension +// ============================================= +void test_init_with_no_ref_binding() { + // CHECK: define dso_local void @_Z29test_init_with_no_ref_bindingv() + struct PrintiesCopy { + Printy a; + Printy b; + Printy c; + }; + PrintiesCopy ps(Printy(), ({ + if (foo()) { + // CHECK: if.then: + // CHECK-NEXT: call void @_ZN6PrintyD1Ev + // CHECK-NEXT: br label %return + return; + } + Printy(); + }), + ({ + if (foo()) { + // CHECK: if.then2: + // CHECK-NEXT: call void @_ZN6PrintyD1Ev + // CHECK-NEXT: call void @_ZN6PrintyD1Ev + // CHECK-NEXT: br label %return + return; + } + Printy(); + })); +} + +// ==================================== +// Array init +// ==================================== +void test_array_init() { + // CHECK: define dso_local void @_Z15test_array_initv() + Printy arr[] = { + Printy(), + ({ + if (foo()) { + // CHECK: if.then: + // CHECK-NEXT: call void @_ZN6PrintyD1Ev + // CHECK-NEXT: br label %return + return; + } + Printy(); + }), + ({ + if (foo()) { + return; + // CHECK: if.then3: + // CHECK-NEXT: call void @_ZN6PrintyD1Ev + // CHECK-NEXT: call void @_ZN6PrintyD1Ev + // CHECK-NEXT: br label %return + } + Printy(); + })}; + return; +} + +// ==================================== +// Arrays as subobjects +// ==================================== +void arrays_as_subobjects() { + // CHECK: define dso_local void @_Z20arrays_as_subobjectsv() + struct S { + Printy arr1[2]; + Printy arr2[2]; + Printy p; + }; + S s{{Printy(), Printy()}, + {Printy(), ({ + if (foo()) { + /** One dtor followed by an array destruction **/ + // CHECK: if.then: + // CHECK: call void @_ZN6PrintyD1Ev + // CHECK: br label %arraydestroy.body + + // CHECK: arraydestroy.body: + // CHECK: call void @_ZN6PrintyD1Ev + + // CHECK: arraydestroy.done{{.*}}: + // CHECK: br label %return + return; + } + Printy(); + })}, + ({ + if (foo()) { + /** Two array destructions **/ + //CHECK: if.then{{.*}}: + //CHECK: br label %arraydestroy.body{{.*}} + + //CHECK: arraydestroy.body{{.*}}: + //CHECK: call void @_ZN6PrintyD1Ev + + //CHECK: arraydestroy.done{{.*}}: + //CHECK: br label %arraydestroy.body{{.*}} + + //CHECK: arraydestroy.body{{.*}}: + //CHECK: call void @_ZN6PrintyD1Ev + + //CHECK: arraydestroy.done{{.*}}: + //CHECK: br label %return + return; + } + Printy(); + })}; +} + >From cf5024cd8a371e6a00cbe85fb0e91f476906f566 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Fri, 9 Feb 2024 20:18:04 +0000 Subject: [PATCH 09/12] remove dead code --- clang/lib/CodeGen/CGStmt.cpp | 6 +++--- clang/lib/CodeGen/CodeGenFunction.h | 9 ++------- clang/lib/CodeGen/EHScopeStack.h | 3 --- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index 9f1ab1e7817179..8d7e2616c168c0 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -627,9 +627,9 @@ CodeGenFunction::getJumpDestForLabel(const LabelDecl *D) { if (Dest.isValid()) return Dest; // Create, but don't insert, the new block. - Dest = JumpDest( - createBasicBlock(D->getName()), EHScopeStack::stable_iterator::invalid(), - EHScopeStack::stable_iterator::invalid(), 0, NextCleanupDestIndex++); + Dest = JumpDest(createBasicBlock(D->getName()), + EHScopeStack::stable_iterator::invalid(), 0, + NextCleanupDestIndex++); return Dest; } diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 5310ec8440fd5a..1713636b554adf 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -240,15 +240,13 @@ class CodeGenFunction : public CodeGenTypeCache { struct JumpDest { JumpDest() : Block(nullptr), BranchInExprDepth(0), Index(0) {} JumpDest(llvm::BasicBlock *Block, EHScopeStack::stable_iterator Depth, - EHScopeStack::stable_iterator EHScopeDepth, unsigned BranchInExprDepth, unsigned Index) - : Block(Block), ScopeDepth(Depth), EHScopeDepth(EHScopeDepth), - BranchInExprDepth(BranchInExprDepth), Index(Index) {} + : Block(Block), ScopeDepth(Depth), BranchInExprDepth(BranchInExprDepth), + Index(Index) {} bool isValid() const { return Block != nullptr; } llvm::BasicBlock *getBlock() const { return Block; } EHScopeStack::stable_iterator getScopeDepth() const { return ScopeDepth; } - EHScopeStack::stable_iterator getEHScopeDepth() const { return EHScopeDepth; } unsigned getDestIndex() const { return Index; } unsigned getBranchInExprDepth() const { return BranchInExprDepth; } @@ -260,7 +258,6 @@ class CodeGenFunction : public CodeGenTypeCache { private: llvm::BasicBlock *Block; EHScopeStack::stable_iterator ScopeDepth; - EHScopeStack::stable_iterator EHScopeDepth; // Size of the branch-in-expr cleanup stack in destination scope. // All cleanups beyond this depth would be emitted on encountering a branch @@ -944,7 +941,6 @@ class CodeGenFunction : public CodeGenTypeCache { CleanupStackDepth = CGF.EHStack.stable_begin(); LifetimeExtendedCleanupStackSize = CGF.LifetimeExtendedCleanupStack.size(); - // BranchInExprCleanupStackSize = CGF.BranchInExprCleanupStack.size(); OldDidCallStackSave = CGF.DidCallStackSave; CGF.DidCallStackSave = false; OldCleanupScopeDepth = CGF.CurrentCleanupScopeDepth; @@ -1209,7 +1205,6 @@ class CodeGenFunction : public CodeGenTypeCache { /// to which we can perform this jump later. JumpDest getJumpDestInCurrentScope(llvm::BasicBlock *Target) { return JumpDest(Target, EHStack.getInnermostNormalCleanup(), - EHStack.getInnermostEHScope(), BranchInExprCleanupStack.size(), NextCleanupDestIndex++); } diff --git a/clang/lib/CodeGen/EHScopeStack.h b/clang/lib/CodeGen/EHScopeStack.h index feffa37c87f6b1..2077eb5e315fbd 100644 --- a/clang/lib/CodeGen/EHScopeStack.h +++ b/clang/lib/CodeGen/EHScopeStack.h @@ -251,9 +251,6 @@ class EHScopeStack { /// The innermost EH scope on the stack. stable_iterator InnermostEHScope; - /// The innermost EH scope on the stack. - stable_iterator InnermostBranchExprCleanup; - /// The CGF this Stack belong to CodeGenFunction* CGF; >From db43f0271b18784b5d138a442f91ef59b1c42afe Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Fri, 9 Feb 2024 21:58:54 +0000 Subject: [PATCH 10/12] format --- clang/lib/CodeGen/CGExprAgg.cpp | 2 +- clang/lib/CodeGen/CodeGenFunction.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index bd2e9bc0f82a91..9ee0b94705ba06 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -1800,7 +1800,7 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr( if (QualType::DestructionKind dtorKind = field->getType().isDestructedType()) { assert(LV.isSimple()); - if(CGF.needsBranchCleanup(dtorKind)) { + if (CGF.needsBranchCleanup(dtorKind)) { CGF.pushDestroy(BranchInExprCleanup, LV.getAddress(CGF), field->getType(), CGF.getDestroyer(dtorKind), false); } diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 1713636b554adf..74317c3815a3f0 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -645,8 +645,8 @@ class CodeGenFunction : public CodeGenTypeCache { // to the EHStack while building an expression. // Cleanups from this stack are only emitted when encountering a branch while // building an expression (eg: branches in stmt-expr or coroutine - // suspensions). Otherwise, these should be cleared the end of the expression and - // added separately to the EHStack. + // suspensions). Otherwise, these should be cleared the end of the expression + // and added separately to the EHStack. DeferredCleanupStack BranchInExprCleanupStack; class RestoreBranchInExpr { >From 034ebda4cfd501796b82f0fe0d17141927f0a86e Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Mon, 12 Feb 2024 11:29:43 +0000 Subject: [PATCH 11/12] add branch-in-expr cleanup for lambda init --- clang/lib/CodeGen/CGExprAgg.cpp | 4 ++++ clang/lib/CodeGen/CodeGenFunction.h | 1 - .../control-flow-in-stmt-expr-cleanup.cpp | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index 9ee0b94705ba06..9c2596d0f30095 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -1367,6 +1367,7 @@ AggExprEmitter::VisitLambdaExpr(LambdaExpr *E) { SmallVector<EHScopeStack::stable_iterator, 16> Cleanups; llvm::Instruction *CleanupDominator = nullptr; + CodeGenFunction::RestoreBranchInExpr RestoreBranchInExpr(CGF); CXXRecordDecl::field_iterator CurField = E->getLambdaClass()->field_begin(); for (LambdaExpr::const_capture_init_iterator i = E->capture_init_begin(), e = E->capture_init_end(); @@ -1384,6 +1385,9 @@ AggExprEmitter::VisitLambdaExpr(LambdaExpr *E) { if (QualType::DestructionKind DtorKind = CurField->getType().isDestructedType()) { assert(LV.isSimple()); + if (CGF.needsBranchCleanup(DtorKind)) + CGF.pushDestroy(BranchInExprCleanup, LV.getAddress(CGF), + CurField->getType(), CGF.getDestroyer(DtorKind), false); if (CGF.needsEHCleanup(DtorKind)) { if (!CleanupDominator) CleanupDominator = CGF.Builder.CreateAlignedLoad( diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 74317c3815a3f0..176441a12d4db0 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -921,7 +921,6 @@ class CodeGenFunction : public CodeGenTypeCache { class RunCleanupsScope { EHScopeStack::stable_iterator CleanupStackDepth, OldCleanupScopeDepth; size_t LifetimeExtendedCleanupStackSize; - // size_t BranchInExprCleanupStackSize; RestoreBranchInExpr RestoreBranchInExpr; bool OldDidCallStackSave; protected: diff --git a/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp b/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp index ee380a167e3fbe..fe839316021e3b 100644 --- a/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp +++ b/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp @@ -181,3 +181,18 @@ void arrays_as_subobjects() { })}; } +// ==================================== +// Lambda capture initialisation +// ==================================== +void lambda_init() { + // CHECK: define dso_local void @_Z11lambda_initv() + auto S = [a = Printy(), b = ({ + if (foo()) { + return; + // CHECK: if.then: + // CHECK: call void @_ZN6PrintyD1Ev + // CHECK: br label %return + } + Printy(); + })]() {}; +} >From b902c61fbcba8a4c3a3400b73c898d01eeeb115f Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Mon, 12 Feb 2024 12:31:25 +0000 Subject: [PATCH 12/12] fix var name --- clang/lib/CodeGen/CGExprAgg.cpp | 6 +++--- clang/lib/CodeGen/CodeGenFunction.h | 16 ++++++++++------ 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index 9c2596d0f30095..81412de97fa46d 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -582,7 +582,7 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, // elements have been initialized. llvm::Value *element = begin; - CodeGenFunction::RestoreBranchInExpr branchInExpr(CGF); + CodeGenFunction::RestoreBranchInExprRAII branchInExpr(CGF); // Emit the explicit initializers. for (uint64_t i = 0; i != NumInitElements; ++i) { // Advance to the next element. @@ -1367,7 +1367,7 @@ AggExprEmitter::VisitLambdaExpr(LambdaExpr *E) { SmallVector<EHScopeStack::stable_iterator, 16> Cleanups; llvm::Instruction *CleanupDominator = nullptr; - CodeGenFunction::RestoreBranchInExpr RestoreBranchInExpr(CGF); + CodeGenFunction::RestoreBranchInExprRAII RestoreBranchInExpr(CGF); CXXRecordDecl::field_iterator CurField = E->getLambdaClass()->field_begin(); for (LambdaExpr::const_capture_init_iterator i = E->capture_init_begin(), e = E->capture_init_end(); @@ -1765,7 +1765,7 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr( return; } - CodeGenFunction::RestoreBranchInExpr branchInExpr(CGF); + CodeGenFunction::RestoreBranchInExprRAII branchInExpr(CGF); // Here we iterate over the fields; this makes it simpler to both // default-initialize fields and skip over unnamed fields. for (const auto *field : record->fields()) { diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 176441a12d4db0..a8ca5b99b6e44c 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -645,15 +645,19 @@ class CodeGenFunction : public CodeGenTypeCache { // to the EHStack while building an expression. // Cleanups from this stack are only emitted when encountering a branch while // building an expression (eg: branches in stmt-expr or coroutine - // suspensions). Otherwise, these should be cleared the end of the expression - // and added separately to the EHStack. + // suspensions). + // Otherwise, these should be cleared the end of the expression and added + // separately to the EHStack. DeferredCleanupStack BranchInExprCleanupStack; - class RestoreBranchInExpr { + // RAII for restoring BranchInExprCleanupStack. + // All cleanups added to this stack during its scope are simply deleted. These + // cleanups should be added to the EHStack only on emitting a branch. + class RestoreBranchInExprRAII { public: - RestoreBranchInExpr(CodeGenFunction &CGF) + RestoreBranchInExprRAII(CodeGenFunction &CGF) : CGF(CGF), OldSize(CGF.BranchInExprCleanupStack.size()) {} - ~RestoreBranchInExpr() { CGF.BranchInExprCleanupStack.resize(OldSize); } + ~RestoreBranchInExprRAII() { CGF.BranchInExprCleanupStack.resize(OldSize); } private: CodeGenFunction &CGF; @@ -921,7 +925,7 @@ class CodeGenFunction : public CodeGenTypeCache { class RunCleanupsScope { EHScopeStack::stable_iterator CleanupStackDepth, OldCleanupScopeDepth; size_t LifetimeExtendedCleanupStackSize; - RestoreBranchInExpr RestoreBranchInExpr; + RestoreBranchInExprRAII RestoreBranchInExpr; bool OldDidCallStackSave; protected: bool PerformCleanup; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits