[all-commits] [llvm/llvm-project] d72146: Re-apply "Emit missing cleanups for stmt-expr" and...

[Utkarsh Saxena via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: d72146f47156dc645b1c0a4cb9c72f01e6d6dd0e
      https://github.com/llvm/llvm-project/commit/d72146f47156dc645b1c0a4cb9c72f01e6d6dd0e
  Author: Utkarsh Saxena <usx at google.com>
  Date:   2024-04-29 (Mon, 29 Apr 2024)

  Changed paths:
    M clang/docs/ReleaseNotes.rst
    M clang/lib/CodeGen/CGCall.cpp
    M clang/lib/CodeGen/CGCleanup.cpp
    M clang/lib/CodeGen/CGCleanup.h
    M clang/lib/CodeGen/CGDecl.cpp
    M clang/lib/CodeGen/CGExpr.cpp
    M clang/lib/CodeGen/CGExprAgg.cpp
    M clang/lib/CodeGen/CGExprCXX.cpp
    M clang/lib/CodeGen/CodeGenFunction.cpp
    M clang/lib/CodeGen/CodeGenFunction.h
    M clang/test/CodeGenCXX/blocks.cpp
    A clang/test/CodeGenCXX/control-flow-in-stmt-expr.cpp
    A clang/test/CodeGenCoroutines/coro-suspend-cleanups.cpp
    M clang/test/CodeGenObjC/arc-blocks-exceptions.m
    M clang/test/CodeGenObjC/arc-blocks.m

  Log Message:
  -----------
  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



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications