https://github.com/ilovepi updated https://github.com/llvm/llvm-project/pull/175817
>From cb0ff14a8c50570ac497689cdee123d40a1e9937 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 52326d0bb80ad..afced9d5a89be 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -4968,10 +4968,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 3c6b8b7d5bb57..00a5a6bc6ed29 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -6690,8 +6690,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 efdc085a562ce1a2a8354b4c39e519eebbe479d3 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 44629b8bae194..54a14e221e9d4 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -21029,7 +21029,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: @@ -21070,6 +21069,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 00a5a6bc6ed29..3c6b8b7d5bb57 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -6690,22 +6690,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 a99d52325aaeea3b42d93ee6e9f260ee86533f7f 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 54a14e221e9d4..44629b8bae194 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -21029,6 +21029,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: @@ -21069,10 +21070,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
