diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -549,6 +549,10 @@ (`#48512 `_). - Fixed a failing assertion when parsing incomplete destructor. (`#63503 `_) +- 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 `_). Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -6911,9 +6911,11 @@ /// MaybeCreateExprWithCleanups - If the current full-expression /// requires any cleanups, surround it with a ExprWithCleanups node. /// Otherwise, just returns the passed-in expression. - Expr *MaybeCreateExprWithCleanups(Expr *SubExpr); + Expr *MaybeCreateExprWithCleanups(Expr *SubExpr, + bool IsImmediateInvocation = false); Stmt *MaybeCreateStmtWithCleanups(Stmt *SubStmt); - ExprResult MaybeCreateExprWithCleanups(ExprResult SubExpr); + ExprResult MaybeCreateExprWithCleanups(ExprResult SubExpr, + bool IsImmediateInvocation = false); MaterializeTemporaryExpr * CreateMaterializeTemporaryExpr(QualType T, Expr *Temporary, diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -18183,7 +18183,7 @@ return E; } - E = MaybeCreateExprWithCleanups(E); + E = MaybeCreateExprWithCleanups(E, /*IsImmediateInvocation=*/true); ConstantExpr *Res = ConstantExpr::Create( getASTContext(), E.get(), diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -7346,15 +7346,16 @@ return Bind; } -ExprResult -Sema::MaybeCreateExprWithCleanups(ExprResult SubExpr) { +ExprResult Sema::MaybeCreateExprWithCleanups(ExprResult SubExpr, + bool IsImmediateInvocation) { if (SubExpr.isInvalid()) return ExprError(); - return MaybeCreateExprWithCleanups(SubExpr.get()); + return MaybeCreateExprWithCleanups(SubExpr.get(), IsImmediateInvocation); } -Expr *Sema::MaybeCreateExprWithCleanups(Expr *SubExpr) { +Expr *Sema::MaybeCreateExprWithCleanups(Expr *SubExpr, + bool IsImmediateInvocation) { assert(SubExpr && "subexpression can't be null!"); CleanupVarDeclMarking(); @@ -7371,8 +7372,22 @@ auto *E = ExprWithCleanups::Create( Context, SubExpr, Cleanup.cleanupsHaveSideEffects(), Cleanups); - DiscardCleanupsInEvaluationContext(); + if (IsImmediateInvocation) { + // Do not discard cleanups in case we're processing an immediate invocation + // since an immediate invocation is a full expression itself - it requires + // an additional ExprWithCleanups node, but it can participate to a bigger + // full expression which actually requires cleanups to be run after. + + // ExprCleanupObjects contain BlockDecl or CompoundLiteralExpr nodes which + // can't end up attached to a ExprWithCleanups created for an immediate + // invocation, because BlockDecl is ObjC construct and compound literals + // in C++ are just temporaries, so it is ok to not care about them. + assert(LangOpts.CPlusPlus && "Immediate invocation outside of C++?"); + return E; + } + + DiscardCleanupsInEvaluationContext(); return E; } diff --git a/clang/test/CodeGenCXX/consteval-cleanup.cpp b/clang/test/CodeGenCXX/consteval-cleanup.cpp new file mode 100644 --- /dev/null +++ b/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 +} diff --git a/clang/test/SemaCXX/consteval-cleanup.cpp b/clang/test/SemaCXX/consteval-cleanup.cpp new file mode 100644 --- /dev/null +++ b/clang/test/SemaCXX/consteval-cleanup.cpp @@ -0,0 +1,42 @@ +// 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 {{.*}} 'bool' ',' + // CHECK-NEXT: BinaryOperator {{.*}} 'P':'P' ',' + // CHECK-NEXT: CXXFunctionalCastExpr {{.*}} 'A':'A' + // CHECK-NEXT: CXXBindTemporaryExpr {{.*}} 'A':'A' + // CHECK-NEXT: CXXConstructExpr {{.*}} 'A':'A' + // CHECK: ConstantExpr {{.*}} 'P':'P' + // CHECK-NEXT: value: + // CHECK-NEXT: ExprWithCleanups +} + +struct B { + int *p = new int(38); + consteval int get() { return *p; } + constexpr ~B() { delete p; } +}; + +void bar() { + // CHECK: bar + // CHECK: ConstantExpr + // CHECK-NEXT: value: + // CHECK-NEXT: ExprWithCleanups + int k = B().get(); +}