[PATCH] D153294: [clang] Do not create ExprWithCleanups while checking immediate invocation

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Herald added a project: All.
Fznamznon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Immediate invocations do not need to run cleanups themselves and the
destructors need to run after each full-expression is processed.
Attempting to add ExprWithCleanups for each immediate invocation may cause
adding it in the wrong place and producing code with memory leaks.

Thanks @ilya-biryukov for helping reducing the reproducer and confirming
that the analysis is correct.

Fixes https://github.com/llvm/llvm-project/issues/60709


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153294

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGenCXX/consteval-cleanup.cpp
  clang/test/SemaCXX/consteval-cleanup.cpp


Index: clang/test/SemaCXX/consteval-cleanup.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/consteval-cleanup.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -Wno-unused-value -std=c++20 -ast-dump -verify %s -ast-dump | FileCheck %s
+
+// expected-no-diagnostics
+
+struct P {
+  consteval P() {}
+};
+
+struct A {
+  A(int v) { this->data = new int(v); }
+  ~A() { delete data; }
+private:
+  int *data;
+};
+
+void foo() {
+  for (;A(1), P(), false;);
+  // CHECK: foo
+  // CHECK: ExprWithCleanups
+  // CHECK-NEXT: BinaryOperator
+  // CHECK: ConstantExpr
+  // CHECK-NOT: ExprWithCleanups
+}
+
+struct B {
+  int *p = new int(38);
+  consteval int get() { return *p; }
+  constexpr ~B() { delete p; }
+};
+
+void bar() {
+  // CHECK: bar
+  // CHECK: ExprWithCleanups
+  // CHECK-NEXT: ConstantExpr
+  int k = B().get();
+}
Index: clang/test/CodeGenCXX/consteval-cleanup.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/consteval-cleanup.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -std=c++20 -Wno-unused-value -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+struct P {
+  consteval P() {}
+};
+
+struct A {
+  A(int v) { this->data = new int(v); }
+  ~A() { delete data; }
+private:
+  int *data;
+};
+
+void foo() {
+  for (;A(1), P(), false;);
+  // CHECK: foo
+  // CHECK: for.cond:
+  // CHECK: call void @_ZN1AC1Ei
+  // CHECK: call void @_ZN1AD1Ev
+  // CHECK: for.body
+}
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -18170,8 +18170,6 @@
     return E;
   }
 
-  E = MaybeCreateExprWithCleanups(E);
-
   ConstantExpr *Res = ConstantExpr::Create(
       getASTContext(), E.get(),
       ConstantExpr::getStorageKind(Decl->getReturnType().getTypePtr(),
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -520,6 +520,10 @@
   type by the default argument promotions, and thus this is UB. Clang's
   behavior now matches GCC's behavior in C++.
   (`#38717 <https://github.com/llvm/llvm-project/issues/38717>_`).
+- Fix missing destructor calls and therefore memory leaks in generated code
+  when an immediate invocation appears as a part of an expression that produces
+  temporaries.
+  (`#60709 <https://github.com/llvm/llvm-project/issues/60709>_`).
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D153294.532689.patch
Type: text/x-patch
Size: 2601 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230619/3319d147/attachment.bin>