https://github.com/ilovepi updated https://github.com/llvm/llvm-project/pull/175817
>From 07880f92e2a62f495ca5e946f2a25bb3cf5f2668 Mon Sep 17 00:00:00 2001 From: Paul Kirth <[email protected]> Date: Mon, 12 Jan 2026 16:14:15 -0800 Subject: [PATCH 1/3] [clang] Use uniform lifetime bounds under exceptions To do this we have to slightly modify how some expressions are handled in Sema. Principally, we need to ensure that calls to new for non-trivial types still have their destructors run. Generally this isn't an issue, since these just get sunk into the surrounding scope. With more lifetime annotations being produced for the expressions, we found that some calls to `new` in an unreachable switch arm would not be wrapped in ExprWithCleanups. As a result, they remain on the EhStack when processing the default label, and since the dead arm doesn't dominate the default label, we can end up with a case where the def-use chain is broken (e.g. the def doesn't dominate all uses). Technically this path would be impossible to reach due to the active bit, but it still failed to satisfy a dominance relationship. With that in place, we can remove the constraint on only using tighter lifetimes when exceptions are disabled. --- clang/lib/CodeGen/CGCall.cpp | 6 ++-- clang/lib/Sema/SemaExprCXX.cpp | 16 ++++++++- .../CodeGenCXX/aggregate-lifetime-invoke.cpp | 36 +++++++++++++++++++ 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index 0829603aa2012..f480c0179c4c0 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -5075,10 +5075,8 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E, // For arguments with aggregate type, create an alloca to store // the value. If the argument's type has a destructor, that destructor // will run at the end of the full-expression; emit matching lifetime - // markers. - // - // FIXME: For types which don't have a destructor, consider using a - // narrower lifetime bound. + // markers. For types which don't have a destructor, we use a narrower + // lifetime bound. if (hasAggregateEvaluationKind(E->getType())) { RawAddress ArgSlotAlloca = Address::invalid(); ArgSlot = CreateAggTemp(E->getType(), "agg.tmp", &ArgSlotAlloca); diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index e90311d51bbd3..1cf00ab1e32be 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -6720,8 +6720,22 @@ Expr *Sema::MaybeCreateExprWithCleanups(Expr *SubExpr) { assert(ExprCleanupObjects.size() >= FirstCleanup); assert(Cleanup.exprNeedsCleanups() || ExprCleanupObjects.size() == FirstCleanup); - if (!Cleanup.exprNeedsCleanups()) + if (!Cleanup.exprNeedsCleanups()) { + // If we have a 'new' expression with a non-trivial destructor, we need to + // wrap it in an ExprWithCleanups to ensure that the destructor is called + // if the constructor throws. + if (auto *NE = dyn_cast<CXXNewExpr>(SubExpr)) { + if (NE->getOperatorDelete() && + !NE->getOperatorDelete()->isReservedGlobalPlacementOperator()) { + auto Cleanups = llvm::ArrayRef<ExprWithCleanups::CleanupObject>(); + auto *E = ExprWithCleanups::Create( + Context, SubExpr, Cleanup.cleanupsHaveSideEffects(), Cleanups); + DiscardCleanupsInEvaluationContext(); + return E; + } + } return SubExpr; + } auto Cleanups = llvm::ArrayRef(ExprCleanupObjects.begin() + FirstCleanup, ExprCleanupObjects.size() - FirstCleanup); diff --git a/clang/test/CodeGenCXX/aggregate-lifetime-invoke.cpp b/clang/test/CodeGenCXX/aggregate-lifetime-invoke.cpp index a08d9e5c047c0..2cfe7f7c6c5f3 100644 --- a/clang/test/CodeGenCXX/aggregate-lifetime-invoke.cpp +++ b/clang/test/CodeGenCXX/aggregate-lifetime-invoke.cpp @@ -14,6 +14,42 @@ struct Trivial { void func_that_throws(Trivial t); +// TIGHT-LABEL: define dso_local void @test( +// TIGHT-SAME: ) local_unnamed_addr #[[ATTR0:[0-9]+]] personality ptr @__gxx_personality_v0 { +// TIGHT-NEXT: [[ENTRY:.*:]] +// TIGHT-NEXT: [[AGG_TMP:%.*]] = alloca [[STRUCT_TRIVIAL:%.*]], align 8 +// TIGHT-NEXT: [[AGG_TMP2:%.*]] = alloca [[STRUCT_TRIVIAL]], align 8 +// TIGHT-NEXT: call void @llvm.lifetime.start.p0(ptr nonnull [[AGG_TMP]]) #[[ATTR4:[0-9]+]] +// TIGHT-NEXT: call void @llvm.memset.p0.i64(ptr noundef nonnull align 8 dereferenceable(400) [[AGG_TMP]], i8 0, i64 400, i1 false) +// TIGHT-NEXT: invoke void @func_that_throws(ptr noundef nonnull byval([[STRUCT_TRIVIAL]]) align 8 [[AGG_TMP]]) +// TIGHT-NEXT: to label %[[INVOKE_CONT:.*]] unwind label %[[LPAD1:.*]] +// TIGHT: [[INVOKE_CONT]]: +// TIGHT-NEXT: call void @llvm.lifetime.end.p0(ptr nonnull [[AGG_TMP]]) #[[ATTR4]] +// TIGHT-NEXT: call void @llvm.lifetime.start.p0(ptr nonnull [[AGG_TMP2]]) #[[ATTR4]] +// TIGHT-NEXT: call void @llvm.memset.p0.i64(ptr noundef nonnull align 8 dereferenceable(400) [[AGG_TMP2]], i8 0, i64 400, i1 false) +// TIGHT-NEXT: invoke void @func_that_throws(ptr noundef nonnull byval([[STRUCT_TRIVIAL]]) align 8 [[AGG_TMP2]]) +// TIGHT-NEXT: to label %[[INVOKE_CONT5:.*]] unwind label %[[LPAD4:.*]] +// TIGHT: [[INVOKE_CONT5]]: +// TIGHT-NEXT: call void @llvm.lifetime.end.p0(ptr nonnull [[AGG_TMP2]]) #[[ATTR4]] +// TIGHT-NEXT: br label %[[TRY_CONT:.*]] +// TIGHT: [[LPAD1]]: +// TIGHT-NEXT: [[TMP0:%.*]] = landingpad { ptr, i32 } +// TIGHT-NEXT: catch ptr null +// TIGHT-NEXT: br label %[[EHCLEANUP:.*]] +// TIGHT: [[LPAD4]]: +// TIGHT-NEXT: [[TMP1:%.*]] = landingpad { ptr, i32 } +// TIGHT-NEXT: catch ptr null +// TIGHT-NEXT: call void @llvm.lifetime.end.p0(ptr nonnull [[AGG_TMP2]]) #[[ATTR4]] +// TIGHT-NEXT: br label %[[EHCLEANUP]] +// TIGHT: [[EHCLEANUP]]: +// TIGHT-NEXT: [[DOTPN:%.*]] = phi { ptr, i32 } [ [[TMP1]], %[[LPAD4]] ], [ [[TMP0]], %[[LPAD1]] ] +// TIGHT-NEXT: [[EXN_SLOT_0:%.*]] = extractvalue { ptr, i32 } [[DOTPN]], 0 +// TIGHT-NEXT: call void @llvm.lifetime.end.p0(ptr nonnull [[AGG_TMP]]) #[[ATTR4]] +// TIGHT-NEXT: [[TMP2:%.*]] = tail call ptr @__cxa_begin_catch(ptr [[EXN_SLOT_0]]) #[[ATTR4]] +// TIGHT-NEXT: tail call void @__cxa_end_catch() +// TIGHT-NEXT: br label %[[TRY_CONT]] +// TIGHT: [[TRY_CONT]]: +// TIGHT-NEXT: ret void // CHECK-LABEL: define dso_local void @test( // CHECK-SAME: ) local_unnamed_addr #[[ATTR0:[0-9]+]] personality ptr @__gxx_personality_v0 { // CHECK-NEXT: [[ENTRY:.*:]] >From e508dd3b5ade5e8103e977c7aa3d5dcc4ba60398 Mon Sep 17 00:00:00 2001 From: Paul Kirth <[email protected]> Date: Tue, 13 Jan 2026 17:44:39 -0800 Subject: [PATCH 2/3] Use setExprNeedsCleanups in BuildCXXNew and avoid breaking c++98 This approach is much cleaner, but broke checkICE reporting in c++98. Stepping through a debugger shows that this happend because the static_assert test didn not recognize ExprWithCleanups as transparent to constant evaluation. To addresse this, we update CheckICE to recurse into the sub-expression, and keep the old behavior. --- clang/lib/AST/ExprConstant.cpp | 5 ++++- clang/lib/Sema/SemaExprCXX.cpp | 16 +--------------- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index b06233423db4d..71922914ea8ce 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -21049,7 +21049,6 @@ static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) { case Expr::CXXInheritedCtorInitExprClass: case Expr::CXXStdInitializerListExprClass: case Expr::CXXBindTemporaryExprClass: - case Expr::ExprWithCleanupsClass: case Expr::CXXTemporaryObjectExprClass: case Expr::CXXUnresolvedConstructExprClass: case Expr::CXXDependentScopeMemberExprClass: @@ -21090,6 +21089,10 @@ static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) { case Expr::HLSLOutArgExprClass: return ICEDiag(IK_NotICE, E->getBeginLoc()); + case Expr::ExprWithCleanupsClass: + // ExprWithCleanups is just a wrapper, so check the wrapped expression. + return CheckICE(cast<ExprWithCleanups>(E)->getSubExpr(), Ctx); + case Expr::InitListExprClass: { // C++03 [dcl.init]p13: If T is a scalar type, then a declaration of the // form "T x = { a };" is equivalent to "T x = a;". diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 1cf00ab1e32be..e90311d51bbd3 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -6720,22 +6720,8 @@ Expr *Sema::MaybeCreateExprWithCleanups(Expr *SubExpr) { assert(ExprCleanupObjects.size() >= FirstCleanup); assert(Cleanup.exprNeedsCleanups() || ExprCleanupObjects.size() == FirstCleanup); - if (!Cleanup.exprNeedsCleanups()) { - // If we have a 'new' expression with a non-trivial destructor, we need to - // wrap it in an ExprWithCleanups to ensure that the destructor is called - // if the constructor throws. - if (auto *NE = dyn_cast<CXXNewExpr>(SubExpr)) { - if (NE->getOperatorDelete() && - !NE->getOperatorDelete()->isReservedGlobalPlacementOperator()) { - auto Cleanups = llvm::ArrayRef<ExprWithCleanups::CleanupObject>(); - auto *E = ExprWithCleanups::Create( - Context, SubExpr, Cleanup.cleanupsHaveSideEffects(), Cleanups); - DiscardCleanupsInEvaluationContext(); - return E; - } - } + if (!Cleanup.exprNeedsCleanups()) return SubExpr; - } auto Cleanups = llvm::ArrayRef(ExprCleanupObjects.begin() + FirstCleanup, ExprCleanupObjects.size() - FirstCleanup); >From ecc5e7a88bc186e2bc7c39cf57d8c6d0f70ef314 Mon Sep 17 00:00:00 2001 From: Paul Kirth <[email protected]> Date: Tue, 13 Jan 2026 18:55:28 -0800 Subject: [PATCH 3/3] Use more restrictive condition for adding ExprWithCleanups --- clang/lib/AST/ExprConstant.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 71922914ea8ce..b06233423db4d 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -21049,6 +21049,7 @@ static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) { case Expr::CXXInheritedCtorInitExprClass: case Expr::CXXStdInitializerListExprClass: case Expr::CXXBindTemporaryExprClass: + case Expr::ExprWithCleanupsClass: case Expr::CXXTemporaryObjectExprClass: case Expr::CXXUnresolvedConstructExprClass: case Expr::CXXDependentScopeMemberExprClass: @@ -21089,10 +21090,6 @@ static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) { case Expr::HLSLOutArgExprClass: return ICEDiag(IK_NotICE, E->getBeginLoc()); - case Expr::ExprWithCleanupsClass: - // ExprWithCleanups is just a wrapper, so check the wrapped expression. - return CheckICE(cast<ExprWithCleanups>(E)->getSubExpr(), Ctx); - case Expr::InitListExprClass: { // C++03 [dcl.init]p13: If T is a scalar type, then a declaration of the // form "T x = { a };" is equivalent to "T x = a;". _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
