aboutsummaryrefslogtreecommitdiff
path: root/clang/lib/CodeGen/CodeGenFunction.cpp
diff options
context:
space:
mode:
authorUtkarsh Saxena <usx@google.com>2024-04-29 12:33:46 +0200
committerGitHub <noreply@github.com>2024-04-29 12:33:46 +0200
commitd72146f47156dc645b1c0a4cb9c72f01e6d6dd0e (patch)
treeecb3e0ba91b124f7306dd64707d9f7b6eda59351 /clang/lib/CodeGen/CodeGenFunction.cpp
parentdf762a1643bb5b0b3c907611d118c82d4b68a39d (diff)
downloadllvm-d72146f47156dc645b1c0a4cb9c72f01e6d6dd0e.zip
llvm-d72146f47156dc645b1c0a4cb9c72f01e6d6dd0e.tar.gz
llvm-d72146f47156dc645b1c0a4cb9c72f01e6d6dd0e.tar.bz2
Re-apply "Emit missing cleanups for stmt-expr" and other commits (#89154)
Latest diff: https://github.com/llvm/llvm-project/pull/89154/files/f1ab4c2677394bbfc985d9680d5eecd7b2e6a882..adf9bc902baddb156c83ce0f8ec03c142e806d45 We address two additional bugs here: ### Problem 1: Deactivated normal cleanup still runs, leading to double-free Consider the following: ```cpp struct A { }; struct B { B(const A&); }; struct S { A a; B b; }; int AcceptS(S s); void Accept2(int x, int y); void Test() { Accept2(AcceptS({.a = A{}, .b = A{}}), ({ return; 0; })); } ``` We add cleanups as follows: 1. push dtor for field `S::a` 2. push dtor for temp `A{}` (used by ` B(const A&)` in `.b = A{}`) 3. push dtor for field `S::b` 4. Deactivate 3 `S::b`-> This pops the cleanup. 5. Deactivate 1 `S::a` -> Does not pop the cleanup as *2* is top. Should create _active flag_!! 6. push dtor for `~S()`. 7. ... It is important to deactivate **5** using active flags. Without the active flags, the `return` will fallthrough it and would run both `~S()` and dtor `S::a` leading to **double free** of `~A()`. In this patch, we unconditionally emit active flags while deactivating normal cleanups. These flags are deleted later by the `AllocaTracker` if the cleanup is not emitted. ### Problem 2: Missing cleanup for conditional lifetime extension We push 2 cleanups for lifetime-extended cleanup. The first cleanup is useful if we exit from the middle of the expression (stmt-expr/coro suspensions). This is deactivated after full-expr, and a new cleanup is pushed, extending the lifetime of the temporaries (to the scope of the reference being initialized). If this lifetime extension happens to be conditional, then we use active flags to remember whether the branch was taken and if the object was initialized. Previously, we used a **single** active flag, which was used by both cleanups. This is wrong because the first cleanup will be forced to deactivate after the full-expr and therefore this **active** flag will always be **inactive**. The dtor for the lifetime extended entity would not run as it always sees an **inactive** flag. In this patch, we solve this using two separate active flags for both cleanups. Both of them are activated if the conditional branch is taken, but only one of them is deactivated after the full-expr. --- Fixes https://github.com/llvm/llvm-project/issues/63818 Fixes https://github.com/llvm/llvm-project/issues/88478 --- Previous PR logs: 1. https://github.com/llvm/llvm-project/pull/85398 2. https://github.com/llvm/llvm-project/pull/88670 3. https://github.com/llvm/llvm-project/pull/88751 4. https://github.com/llvm/llvm-project/pull/88884
Diffstat (limited to 'clang/lib/CodeGen/CodeGenFunction.cpp')
-rw-r--r--clang/lib/CodeGen/CodeGenFunction.cpp6
1 files changed, 6 insertions, 0 deletions
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 86a6ddd8..87766a7 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -91,6 +91,8 @@ CodeGenFunction::CodeGenFunction(CodeGenModule &cgm, bool suppressNewContext)
CodeGenFunction::~CodeGenFunction() {
assert(LifetimeExtendedCleanupStack.empty() && "failed to emit a cleanup");
+ assert(DeferredDeactivationCleanupStack.empty() &&
+ "missed to deactivate a cleanup");
if (getLangOpts().OpenMP && CurFn)
CGM.getOpenMPRuntime().functionFinished(*this);
@@ -346,6 +348,10 @@ 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 of cleanups in EHStack!");
+ assert(DeferredDeactivationCleanupStack.empty() &&
+ "mismatched activate/deactivate of cleanups!");
bool OnlySimpleReturnStmts = NumSimpleReturnExprs > 0
&& NumSimpleReturnExprs == NumReturnExprs