Use pushFullExprCleanup for deferred destroy

[usx95]

Instead of directly pushing the Destroy, we should use pushFullExprCleanup which handles conditional branches.
This fixes a backend crash due to 89ba7e1.

Added minimized crash reproducer.

[llvmbot]

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Utkarsh Saxena (usx95)

Changes

Instead of directly pushing the Destroy, we should use pushFullExprCleanup which handles conditional branches.
This fixes a backend crash due to 89ba7e1.

Tested:

CLANG_DIR=/usr/local/google/home/usx/build

$CLANG_DIR/bin/clang++ \
  -Itools/clang/tools/extra/clangd \
  -I/usr/local/google/home/usx/src/llvm/llvm-project/clang-tools-extra/clangd \
  -I/usr/local/google/home/usx/src/llvm/llvm-project/clang-tools-extra/clangd/../include-cleaner/include \
  -Itools/clang/tools/extra/clangd/../clang-tidy \
  -I/usr/local/google/home/usx/src/llvm/llvm-project/clang/include \
  -Itools/clang/include \
  -Iinclude \
  -I/usr/local/google/home/usx/src/llvm/llvm-project/llvm/include \
  -I/usr/local/google/home/usx/src/llvm/llvm-project/clang-tools-extra/pseudo/lib/../include \
  -isystem$CLANG_DIR/lib/clang/19/include \
  -o  /usr/local/google/home/usx/build/bootstrap/tools/clang/tools/extra/clangd/CMakeFiles/obj.clangDaemon.dir/ClangdLSPServer.cpp.o \
  -c  /usr/local/google/home/usx/src/llvm/llvm-project/clang-tools-extra/clangd/ClangdLSPServer.cpp

---

Full diff: https://github.com/llvm/llvm-project/pull/88670.diff

1 Files Affected:

• (modified) clang/lib/CodeGen/CGDecl.cpp (+5-2)

diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index 8bdafa7c569b08..3f05ebb561da57 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -2216,8 +2216,11 @@ void CodeGenFunction::pushDestroyAndDeferDeactivation(
 void CodeGenFunction::pushDestroyAndDeferDeactivation(
     CleanupKind cleanupKind, Address addr, QualType type, Destroyer *destroyer,
     bool useEHCleanupForArray) {
-  pushCleanupAndDeferDeactivation<DestroyObject>(
-      cleanupKind, addr, type, destroyer, useEHCleanupForArray);
+  llvm::Instruction *DominatingIP =
+      Builder.CreateFlagLoad(llvm::Constant::getNullValue(Int8PtrTy));
+  pushDestroy(cleanupKind, addr, type, destroyer, useEHCleanupForArray);
+  DeferredDeactivationCleanupStack.push_back(
+      {EHStack.stable_begin(), DominatingIP});
 }
 
 void CodeGenFunction::pushStackRestore(CleanupKind Kind, Address SPMem) {

[usx95]

This fix is for a wrong refactoring change in 89ba7e1 and
I would be inclined towards landing this in a few hours to unblock the build breakage (if there are no review comments).
CC: @efriedma-quic

[cor3ntin]

@usx95 Could you maybe reduce a test from the crash? Thanks!

[usx95]

@cor3ntin Added tests for crash.

[efriedma-quic]

The use of initializer_list here doesn't seem necessary; you could remove it to reduce the size of the testcase.

Can we integrate this into an existing test file?

[usx95]

Did you try this out ? I cannot reproduce the original crash without std::initializer_list.

[efriedma-quic]

class Object {
public:
  struct KV;
  Object(KV);
};

struct A { A(); ~A(); };
class Value {
public:
  Value(A);
  Value(const char *V);
  ~Value();
};

class ObjectKey {
public:
  ObjectKey(const char *S);
  ~ObjectKey();
};

struct Object::KV {
  ObjectKey K;
  Value V;
};
bool foo();
void DestroyInConditionalCleanup() {
  foo() ? Object{{"key1", A()}} : Object{{"key2", "val2"}};
}

Not really a big deal either way, though; just a minor cleanup.

[usx95]

Great. Thanks. I was able to reduce it further.

[efriedma-quic]

LGTM