https://github.com/Xazax-hun updated https://github.com/llvm/llvm-project/pull/204650
From c0293b761699a65d78a0e0355248280c0f745904 Mon Sep 17 00:00:00 2001 From: Gabor Horvath <[email protected]> Date: Thu, 18 Jun 2026 18:39:11 +0100 Subject: [PATCH] [LifetimeSafety] Model scope-exit destructor and cleanup callback as a use A non-trivial destructor or __attribute__((cleanup(fn))) callback running at scope exit may read a borrow the object holds (e.g. a [[gsl::Pointer]] whose out-of-line ~T() or cleanup function dereferences its view). The analysis never sees those bodies, so model them as an implicit use of the object at the end of its scope: a borrowed-from object destroyed earlier is now reported. Destructor uses are reported under the new, experimental -Wlifetime-safety-experimental-destructor-use, since destructors that never read the borrow (e.g. std::function's) are also reported. Owners are excluded, as their destruction frees their own storage. Cleanup callbacks receive the variable's address explicitly and are reported under the existing groups. A UseSite describes where and how an origin is used, so the diagnostics name the destructor or cleanup function instead of a bare "later used here" at the closing brace. It replaces the expression and location overloads of the use-after-scope and use-after-invalidation reporting functions. Assisted-by: Claude Opus 4.8 --- clang/docs/LifetimeSafety.md | 1 + clang/docs/ReleaseNotes.md | 3 + .../Analysis/Analyses/LifetimeSafety/Facts.h | 73 ++++++++++- .../Analyses/LifetimeSafety/LifetimeSafety.h | 23 +--- clang/include/clang/Basic/DiagnosticGroups.td | 13 +- .../clang/Basic/DiagnosticSemaKinds.td | 12 ++ clang/lib/Analysis/LifetimeSafety/Checker.cpp | 23 ++-- .../LifetimeSafety/FactsGenerator.cpp | 21 +++- .../Analysis/LifetimeSafety/LiveOrigins.cpp | 2 +- clang/lib/Sema/SemaLifetimeSafety.h | 115 +++++++++--------- .../Sema/LifetimeSafety/invalidations.cpp | 37 +++++- clang/test/Sema/LifetimeSafety/safety.cpp | 71 ++++++++++- 12 files changed, 293 insertions(+), 101 deletions(-) diff --git a/clang/docs/LifetimeSafety.md b/clang/docs/LifetimeSafety.md index ea2364deaee618..13f137e28e3367 100644 --- a/clang/docs/LifetimeSafety.md +++ b/clang/docs/LifetimeSafety.md @@ -524,6 +524,7 @@ enables only the high-confidence subset of these checks. - `-Wlifetime-safety-experimental-invalidation`: Warns when a pointer, reference, iterator or view is used after an operation that may invalidate it, such as container mutation or explicit destruction (e.g., `std::unique_ptr::reset`, `std::destroy_at`). - `-Wlifetime-safety-experimental-field-origin-escape`: Warns when a member function returns a value derived from a pointer, reference, or view field rather than from `*this` (e.g., `Span::data()`). Owned pointer fields (e.g., `Vector::data()`) are also reported; mark such methods `[[clang::lifetimebound]]` to silence the warning. + - `-Wlifetime-safety-experimental-destructor-use`: Treats a non-trivial destructor run at scope exit as a use of the borrows the object holds, so an object destroyed after the storage it borrows from is reported. Destructors that never read the borrow (e.g., `std::function`'s) are also reported. ## Limitations diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 53d688d2e11e7c..f1a929c7dc37ee 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -587,6 +587,9 @@ documentation](CIR/index.md) for more information regarding ClangIR's status and - `-Wlifetime-safety-experimental-field-origin-escape`, which warns when a member function returns a value derived from a pointer, reference, or view field rather than from `*this`. + - `-Wlifetime-safety-experimental-destructor-use`, which warns when an object + whose non-trivial destructor may read a borrow is destroyed after the + storage it borrows from. - Lifetime safety analysis is now enabled for C by default. The `-fexperimental-lifetime-safety-c` flag is renamed to `-flifetime-safety-c`. Use `-fno-lifetime-safety-c` to disable it. diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index a78d22a80b527e..7e086f8c0f1a71 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -22,6 +22,7 @@ #include "clang/Analysis/CFG.h" #include "llvm/ADT/BitVector.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/PointerUnion.h" #include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Support/Debug.h" @@ -29,6 +30,64 @@ #include <cstdint> #include <optional> +namespace clang::lifetimes { + +/// How an origin is used: by an expression, or implicitly at scope exit by a +/// destructor or a cleanup callback that may read the borrows it holds. +enum class UseKind { Explicit, Destructor, Cleanup }; + +/// Where and how an origin is used, for diagnostics. +class UseSite { + /// The use expression for an explicit use, or the variable destroyed or + /// cleaned up for an implicit one. Null for an explicit use known only by + /// its location. + llvm::PointerUnion<const Expr *, const VarDecl *> ExprOrVD; + /// Used when there is no expression. + SourceLocation Loc; + UseKind Kind = UseKind::Explicit; + + UseSite(llvm::PointerUnion<const Expr *, const VarDecl *> ExprOrVD, + SourceLocation Loc, UseKind Kind) + : ExprOrVD(ExprOrVD), Loc(Loc), Kind(Kind) {} + +public: + static UseSite fromExpr(const Expr &E) { + return UseSite(&E, SourceLocation(), UseKind::Explicit); + } + static UseSite at(SourceLocation Loc) { + return UseSite(nullptr, Loc, UseKind::Explicit); + } + static UseSite scopeExit(SourceLocation Loc, const VarDecl &VD, + UseKind Kind) { + assert((Kind == UseKind::Destructor || Kind == UseKind::Cleanup) && + "scope-exit uses are destructor or cleanup uses"); + return UseSite(&VD, Loc, Kind); + } + + UseKind getKind() const { return Kind; } + /// Null for an implicit use or one known only by its location. + const Expr *getExpr() const { + return dyn_cast_if_present<const Expr *>(ExprOrVD); + } + /// The variable destroyed or cleaned up, for implicit uses. + const VarDecl *getScopeExitVD() const { + return dyn_cast_if_present<const VarDecl *>(ExprOrVD); + } + SourceLocation getLoc() const { + if (const Expr *E = getExpr()) + return E->getExprLoc(); + return Loc; + } + /// Empty unless there is a use expression. + SourceRange getRange() const { + if (const Expr *E = getExpr()) + return E->getSourceRange(); + return SourceRange(); + } +}; + +} // namespace clang::lifetimes + namespace clang::lifetimes::internal { class FactsGenerator; @@ -268,17 +327,23 @@ class ThisEscapeFact : public OriginEscapesFact { }; class UseFact : public Fact { - const Expr *UseExpr; + UseSite Site; const OriginList *OList; public: static bool classof(const Fact *F) { return F->getKind() == Kind::Use; } - UseFact(const Expr *UseExpr, const OriginList *OList) - : Fact(Kind::Use), UseExpr(UseExpr), OList(OList) {} + UseFact(const Expr &UseExpr, const OriginList *OList) + : Fact(Kind::Use), Site(UseSite::fromExpr(UseExpr)), OList(OList) {} + UseFact(const UseSite &ImplicitSite, const OriginList *OList) + : Fact(Kind::Use), Site(ImplicitSite), OList(OList) { + assert(Site.getKind() != UseKind::Explicit); + } const OriginList *getUsedOrigins() const { return OList; } - const Expr *getUseExpr() const { return UseExpr; } + /// Null for an implicit use. + const Expr *getUseExpr() const { return Site.getExpr(); } + const UseSite &getUseSite() const { return Site; } void dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM, const LoanPropagationAnalysis *LPA = nullptr) const override; diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index eb227eb1566868..76fa8baaed4120 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -47,6 +47,8 @@ struct LifetimeSafetyOpts { bool CheckMisplacedLifetimebound = false; bool CheckInapplicableLifetimebound = false; bool CheckFieldOriginEscape = false; + /// Whether to model non-trivial destructors at scope exit as uses. + bool CheckDestructorUses = false; }; /// Enum to track functions visible across or within TU. @@ -70,14 +72,7 @@ class LifetimeSafetySemaHelper { LifetimeSafetySemaHelper() = default; virtual ~LifetimeSafetySemaHelper() = default; - virtual void reportUseAfterScope(const Expr *IssueExpr, const Expr *UseExpr, - const Expr *MovedExpr, - SourceLocation FreeLoc, - llvm::ArrayRef<const Expr *> ExprChain) {} - - // Overload for a use with only a location and no expression (e.g. a borrow - // captured into the object and still held at the capturing method's exit). - virtual void reportUseAfterScope(const Expr *IssueExpr, SourceLocation UseLoc, + virtual void reportUseAfterScope(const Expr *IssueExpr, const UseSite &Use, const Expr *MovedExpr, SourceLocation FreeLoc, llvm::ArrayRef<const Expr *> ExprChain) {} @@ -104,19 +99,11 @@ class LifetimeSafetySemaHelper { // Reports when a reference/iterator is used after the container operation // that invalidated it. virtual void - reportUseAfterInvalidation(const Expr *IssueExpr, const Expr *UseExpr, - const Expr *InvalidationExpr, - llvm::ArrayRef<const Expr *> ExprChain) {} - virtual void - reportUseAfterInvalidation(const ParmVarDecl *PVD, const Expr *UseExpr, - const Expr *InvalidationExpr, - llvm::ArrayRef<const Expr *> ExprChain) {} - virtual void - reportUseAfterInvalidation(const Expr *IssueExpr, SourceLocation UseLoc, + reportUseAfterInvalidation(const Expr *IssueExpr, const UseSite &Use, const Expr *InvalidationExpr, llvm::ArrayRef<const Expr *> ExprChain) {} virtual void - reportUseAfterInvalidation(const ParmVarDecl *PVD, SourceLocation UseLoc, + reportUseAfterInvalidation(const ParmVarDecl *PVD, const UseSite &Use, const Expr *InvalidationExpr, llvm::ArrayRef<const Expr *> ExprChain) {} virtual void reportInvalidatedField(const Expr *IssueExpr, diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index b3bb2fb1b53fdc..eaf87106a2fa31 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -754,10 +754,21 @@ method ``[[clang::lifetimebound]]``. }]; } +def LifetimeSafetyDestructorUse + : DiagGroup<"lifetime-safety-experimental-destructor-use"> { + code Documentation = [{ +Experimental. Treats a non-trivial destructor running at scope exit as a use of +the borrows the object holds, since the analysis cannot see whether the +destructor reads them. Detects objects destroyed after the storage they borrow +from. May report destructors that never read the borrow (e.g. ``std::function``). + }]; +} + def LifetimeSafetyExperimental : DiagGroup<"lifetime-safety-experimental", [LifetimeSafetyInvalidation, - LifetimeSafetyFieldOriginEscape]> { + LifetimeSafetyFieldOriginEscape, + LifetimeSafetyDestructorUse]> { code Documentation = [{ Experimental lifetime safety warnings. These may have known false positives and their behavior may change. Not enabled by -Wlifetime-safety-all. diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 56cc9721781063..671bdd63af6861 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11110,6 +11110,10 @@ def warn_lifetime_safety_inapplicable_lifetimebound DefaultIgnore; def note_lifetime_safety_used_here : Note<"later used here">; +def note_lifetime_safety_used_by_destructor + : Note<"later used by the destructor of %0 at scope exit">; +def note_lifetime_safety_used_by_cleanup + : Note<"later used by the cleanup function of %0 at scope exit">; def note_lifetime_safety_invalidated_here : Note<"%0 is invalidated here">; def note_lifetime_safety_destroyed_here : Note<"%0 is destroyed here">; def note_lifetime_safety_freed_here : Note<"%0 is freed here">; @@ -11173,6 +11177,14 @@ def warn_lifetime_safety_field_origin_escape : Warning<"return value depends on the object referenced by %0, not on '*this'">, InGroup<LifetimeSafetyFieldOriginEscape>, DefaultIgnore; +def warn_lifetime_safety_destructor_use_after_scope + : Warning<"%0 does not live long enough">, + InGroup<LifetimeSafetyDestructorUse>, + DefaultIgnore; +def warn_lifetime_safety_destructor_use_after_invalidation + : Warning<"%0 is later invalidated">, + InGroup<LifetimeSafetyDestructorUse>, + DefaultIgnore; def note_lifetime_safety_owned_field_suggestion : Note<"if %0 is owned by the class, mark the method [[clang::lifetimebound]]">; diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index ae0297af25d8d5..2b03a51e66b615 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -303,22 +303,21 @@ class LifetimeChecker { if (const auto *UF = dyn_cast<const UseFact *>(CausingFact)) { llvm::SmallVector<const Expr *> ExprChain = getExprChain(LoanPropagation.buildOriginFlowChain(UF, LID, Cfg)); + const UseSite &Use = UF->getUseSite(); if (Warning.InvalidatedByExpr) { if (IssueExpr) // Use-after-invalidation of an object on stack. - SemaHelper->reportUseAfterInvalidation(IssueExpr, UF->getUseExpr(), - Warning.InvalidatedByExpr, - ExprChain); + SemaHelper->reportUseAfterInvalidation( + IssueExpr, Use, Warning.InvalidatedByExpr, ExprChain); else if (InvalidatedPVD) // Use-after-invalidation of a parameter. SemaHelper->reportUseAfterInvalidation( - InvalidatedPVD, UF->getUseExpr(), Warning.InvalidatedByExpr, - ExprChain); + InvalidatedPVD, Use, Warning.InvalidatedByExpr, ExprChain); } else // Scope-based expiry (use-after-scope). - SemaHelper->reportUseAfterScope(IssueExpr, UF->getUseExpr(), - MovedExpr, ExpiryLoc, ExprChain); + SemaHelper->reportUseAfterScope(IssueExpr, Use, MovedExpr, ExpiryLoc, + ExprChain); } else if (const auto *OEF = dyn_cast<const OriginEscapesFact *>(CausingFact)) { @@ -356,14 +355,13 @@ class LifetimeChecker { // invalidated. llvm::SmallVector<const Expr *> ExprChain = getExprChain( LoanPropagation.buildOriginFlowChain(OEF, LID, Cfg)); + UseSite Use = UseSite::at(ThisEscape->getLoc()); if (IssueExpr) SemaHelper->reportUseAfterInvalidation( - IssueExpr, ThisEscape->getLoc(), Warning.InvalidatedByExpr, - ExprChain); + IssueExpr, Use, Warning.InvalidatedByExpr, ExprChain); else if (InvalidatedPVD) SemaHelper->reportUseAfterInvalidation( - InvalidatedPVD, ThisEscape->getLoc(), - Warning.InvalidatedByExpr, ExprChain); + InvalidatedPVD, Use, Warning.InvalidatedByExpr, ExprChain); } else llvm_unreachable("Unhandled OriginEscapesFact type"); } else if (const auto *RetEscape = dyn_cast<ReturnEscapeFact>(OEF)) { @@ -392,7 +390,8 @@ class LifetimeChecker { // scope; reuse the use-after-scope diagnostic at the capturing // method. SemaHelper->reportUseAfterScope( - IssueExpr, ThisEscape->getLoc(), MovedExpr, ExpiryLoc, + IssueExpr, UseSite::at(ThisEscape->getLoc()), MovedExpr, + ExpiryLoc, getExprChain( LoanPropagation.buildOriginFlowChain(OEF, LID, Cfg))); else diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index f3b29ca63fe158..52e76b930e7f16 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -819,6 +819,21 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { const VarDecl *LifetimeEndsVD = LifetimeEnds.getVarDecl(); if (!LifetimeEndsVD) return; + // A cleanup callback or a non-trivial destructor (other than an owner's) run + // at scope exit may read the borrows the variable holds. + QualType Ty = LifetimeEndsVD->getType(); + bool HasCleanup = LifetimeEndsVD->hasAttr<CleanupAttr>(); + bool IsDestructorUse = + LSOpts.CheckDestructorUses && + Ty.isDestructedType() == QualType::DK_cxx_destructor && + !isGslOwnerType(AC.getASTContext().getBaseElementType(Ty)); + if (HasCleanup || IsDestructorUse) + if (OriginList *List = getOriginsList(*LifetimeEndsVD)) + CurrentBlockFacts.push_back(FactMgr.createFact<UseFact>( + UseSite::scopeExit( + LifetimeEnds.getTriggerStmt()->getEndLoc(), *LifetimeEndsVD, + HasCleanup ? UseKind::Cleanup : UseKind::Destructor), + List)); // Expire the origin when its variable's lifetime ends to ensure liveness // doesn't persist through loop back-edges. std::optional<OriginID> ExpiredOID; @@ -1003,7 +1018,7 @@ void FactsGenerator::handleImplicitObjectFieldUses(const Expr *Call, for (const auto *Field : RD->fields()) if (auto *FieldList = getOriginsList(*Field)) CurrentBlockFacts.push_back( - FactMgr.createFact<UseFact>(Call, FieldList)); + FactMgr.createFact<UseFact>(*Call, FieldList)); }; UseFields(ClassDecl); @@ -1206,7 +1221,7 @@ void FactsGenerator::handleAccess(const Expr *E) { if (!List || namesDeclStorage(List)) return; CurrentBlockFacts.push_back(FactMgr.createFact<UseFact>( - E, + *E, FactMgr.getOriginMgr().createSingleOriginList(List->getOuterOriginID()))); } @@ -1215,7 +1230,7 @@ void FactsGenerator::handleUse(const Expr *E) { if (List && namesDeclStorage(List)) List = List->peelOuterOrigin(); if (List) - CurrentBlockFacts.push_back(FactMgr.createFact<UseFact>(E, List)); + CurrentBlockFacts.push_back(FactMgr.createFact<UseFact>(*E, List)); } // Issues a placeholder loan (this->field) for each origin-carrying field of the diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index 91dabe362fdb76..ed3d0ff1a30794 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -61,7 +61,7 @@ struct Lattice { static SourceLocation GetFactLoc(CausingFactType F) { if (const auto *UF = dyn_cast<const UseFact *>(F)) - return UF->getUseExpr()->getExprLoc(); + return UF->getUseSite().getLoc(); if (const auto *OEF = dyn_cast<const OriginEscapesFact *>(F)) { if (auto *ReturnEsc = dyn_cast<ReturnEscapeFact>(OEF)) return ReturnEsc->getReturnExpr()->getExprLoc(); diff --git a/clang/lib/Sema/SemaLifetimeSafety.h b/clang/lib/Sema/SemaLifetimeSafety.h index bd7cf68277152d..70b100213a2dbc 100644 --- a/clang/lib/Sema/SemaLifetimeSafety.h +++ b/clang/lib/Sema/SemaLifetimeSafety.h @@ -79,6 +79,17 @@ inline bool ShouldCheckFieldOriginEscape(Sema &S, const Decl *D) { diag::warn_lifetime_safety_field_origin_escape, D->getBeginLoc()); } +inline bool ShouldCheckDestructorUses(Sema &S, const Decl *D) { + DiagnosticsEngine &Diags = S.getDiagnostics(); + constexpr unsigned DiagIDs[] = { + diag::warn_lifetime_safety_destructor_use_after_scope, + diag::warn_lifetime_safety_destructor_use_after_invalidation}; + for (unsigned DiagID : DiagIDs) + if (!Diags.isIgnored(DiagID, D->getBeginLoc())) + return true; + return false; +} + inline bool ShouldSuggestLifetimeAnnotations(Sema &S, const Decl *D) { DiagnosticsEngine &Diags = S.getDiagnostics(); constexpr unsigned DiagIDs[] = { @@ -122,6 +133,7 @@ inline bool IsLifetimeSafetyEnabled(Sema &S, const Decl *D) { ShouldCheckMisplacedLifetimebound(S, D) || ShouldCheckInapplicableLifetimebound(S, D) || ShouldCheckFieldOriginEscape(S, D) || + ShouldCheckDestructorUses(S, D) || ShouldSuggestLifetimeAnnotations(S, D); } @@ -136,6 +148,7 @@ inline LifetimeSafetyOpts GetLifetimeSafetyOpts(Sema &S, const Decl *D) { LSOpts.CheckInapplicableLifetimebound = ShouldCheckInapplicableLifetimebound(S, D); LSOpts.CheckFieldOriginEscape = ShouldCheckFieldOriginEscape(S, D); + LSOpts.CheckDestructorUses = ShouldCheckDestructorUses(S, D); return LSOpts; } @@ -144,34 +157,15 @@ class LifetimeSafetySemaHelperImpl final : public LifetimeSafetySemaHelper { public: LifetimeSafetySemaHelperImpl(Sema &S) : S(S) {} - void reportUseAfterScope(const Expr *IssueExpr, const Expr *UseExpr, - const Expr *MovedExpr, SourceLocation FreeLoc, - llvm::ArrayRef<const Expr *> ExprChain) override { - unsigned DiagID = MovedExpr - ? diag::warn_lifetime_safety_use_after_scope_moved - : diag::warn_lifetime_safety_use_after_scope; - std::string DestroyedSubject = getDiagSubjectDescription(IssueExpr); - - S.Diag(IssueExpr->getExprLoc(), DiagID) - << DestroyedSubject << IssueExpr->getSourceRange(); - if (MovedExpr) - S.Diag(MovedExpr->getExprLoc(), diag::note_lifetime_safety_moved_here) - << MovedExpr->getSourceRange(); - S.Diag(FreeLoc, diag::note_lifetime_safety_destroyed_here) - << DestroyedSubject; - - reportAliasingChain(ExprChain); - - S.Diag(UseExpr->getExprLoc(), diag::note_lifetime_safety_used_here) - << UseExpr->getSourceRange(); - } - - void reportUseAfterScope(const Expr *IssueExpr, SourceLocation UseLoc, + void reportUseAfterScope(const Expr *IssueExpr, const UseSite &Use, const Expr *MovedExpr, SourceLocation FreeLoc, llvm::ArrayRef<const Expr *> ExprChain) override { - unsigned DiagID = MovedExpr - ? diag::warn_lifetime_safety_use_after_scope_moved - : diag::warn_lifetime_safety_use_after_scope; + unsigned DiagID; + if (Use.getKind() == UseKind::Destructor) + DiagID = diag::warn_lifetime_safety_destructor_use_after_scope; + else + DiagID = MovedExpr ? diag::warn_lifetime_safety_use_after_scope_moved + : diag::warn_lifetime_safety_use_after_scope; std::string DestroyedSubject = getDiagSubjectDescription(IssueExpr); S.Diag(IssueExpr->getExprLoc(), DiagID) @@ -184,7 +178,7 @@ class LifetimeSafetySemaHelperImpl final : public LifetimeSafetySemaHelper { reportAliasingChain(ExprChain); - S.Diag(UseLoc, diag::note_lifetime_safety_used_here); + reportUseNote(Use); } void reportUseAfterReturn(const Expr *IssueExpr, const Expr *ReturnExpr, @@ -260,40 +254,20 @@ class LifetimeSafetySemaHelperImpl final : public LifetimeSafetySemaHelper { } void - reportUseAfterInvalidation(const Expr *IssueExpr, const Expr *UseExpr, - const Expr *InvalidationExpr, - llvm::ArrayRef<const Expr *> ExprChain) override { - reportUseAfterInvalidationImpl( - IssueExpr->getExprLoc(), IssueExpr->getSourceRange(), - getDiagSubjectDescription(IssueExpr), InvalidationExpr, ExprChain, - UseExpr->getExprLoc(), UseExpr->getSourceRange()); - } - void - reportUseAfterInvalidation(const ParmVarDecl *PVD, const Expr *UseExpr, - const Expr *InvalidationExpr, - llvm::ArrayRef<const Expr *> ExprChain) override { - reportUseAfterInvalidationImpl( - PVD->getSourceRange().getBegin(), PVD->getSourceRange(), - getDiagSubjectDescription(PVD), InvalidationExpr, ExprChain, - UseExpr->getExprLoc(), UseExpr->getSourceRange()); - } - void - reportUseAfterInvalidation(const Expr *IssueExpr, SourceLocation UseLoc, + reportUseAfterInvalidation(const Expr *IssueExpr, const UseSite &Use, const Expr *InvalidationExpr, llvm::ArrayRef<const Expr *> ExprChain) override { reportUseAfterInvalidationImpl( IssueExpr->getExprLoc(), IssueExpr->getSourceRange(), - getDiagSubjectDescription(IssueExpr), InvalidationExpr, ExprChain, - UseLoc, SourceRange()); + getDiagSubjectDescription(IssueExpr), InvalidationExpr, ExprChain, Use); } void - reportUseAfterInvalidation(const ParmVarDecl *PVD, SourceLocation UseLoc, + reportUseAfterInvalidation(const ParmVarDecl *PVD, const UseSite &Use, const Expr *InvalidationExpr, llvm::ArrayRef<const Expr *> ExprChain) override { reportUseAfterInvalidationImpl( PVD->getSourceRange().getBegin(), PVD->getSourceRange(), - getDiagSubjectDescription(PVD), InvalidationExpr, ExprChain, UseLoc, - SourceRange()); + getDiagSubjectDescription(PVD), InvalidationExpr, ExprChain, Use); } void reportInvalidatedField(const Expr *IssueExpr, @@ -634,15 +608,40 @@ class LifetimeSafetySemaHelperImpl final : public LifetimeSafetySemaHelper { StringRef InvalidatedSubject, const Expr *InvalidationExpr, llvm::ArrayRef<const Expr *> ExprChain, - SourceLocation UseLoc, - SourceRange UseRange) { - auto WarnDiag = isa<CXXDeleteExpr>(InvalidationExpr) - ? diag::warn_lifetime_safety_use_after_free - : diag::warn_lifetime_safety_invalidation; - S.Diag(IssueLoc, WarnDiag) << InvalidatedSubject << IssueRange; + const UseSite &Use) { + S.Diag(IssueLoc, getInvalidationWarnDiag( + isa<CXXDeleteExpr>(InvalidationExpr), Use.getKind())) + << InvalidatedSubject << IssueRange; reportInvalidationSite(InvalidationExpr, InvalidatedSubject); reportAliasingChain(ExprChain); - S.Diag(UseLoc, diag::note_lifetime_safety_used_here) << UseRange; + reportUseNote(Use); + } + + static unsigned getInvalidationWarnDiag(bool IsFree, UseKind Kind) { + if (Kind == UseKind::Destructor) + return IsFree + ? diag::warn_lifetime_safety_destructor_use_after_scope + : diag::warn_lifetime_safety_destructor_use_after_invalidation; + return IsFree ? diag::warn_lifetime_safety_use_after_free + : diag::warn_lifetime_safety_invalidation; + } + + void reportUseNote(const UseSite &Use) { + switch (Use.getKind()) { + case UseKind::Explicit: + S.Diag(Use.getLoc(), diag::note_lifetime_safety_used_here) + << Use.getRange(); + return; + case UseKind::Destructor: + S.Diag(Use.getLoc(), diag::note_lifetime_safety_used_by_destructor) + << Use.getScopeExitVD(); + return; + case UseKind::Cleanup: + S.Diag(Use.getLoc(), diag::note_lifetime_safety_used_by_cleanup) + << Use.getScopeExitVD(); + return; + } + llvm_unreachable("unknown UseKind"); } std::string getLifetimeBoundFixItText(SourceLocation Loc, bool LeadingSpace, diff --git a/clang/test/Sema/LifetimeSafety/invalidations.cpp b/clang/test/Sema/LifetimeSafety/invalidations.cpp index d9955ec42a2080..4e17f4096354e8 100644 --- a/clang/test/Sema/LifetimeSafety/invalidations.cpp +++ b/clang/test/Sema/LifetimeSafety/invalidations.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -Wlifetime-safety -Wlifetime-safety-experimental-invalidation -Wno-dangling -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wlifetime-safety -Wlifetime-safety-experimental-invalidation -Wlifetime-safety-experimental-destructor-use -Wno-dangling -verify %s #include "Inputs/lifetime-analysis.h" @@ -1009,3 +1009,38 @@ struct S { } // expected-note {{later used here}} }; } // namespace CapturedByThis + +namespace implicit_use_after_free { +struct [[gsl::Pointer]] Ref { + const int *p; + Ref &operator=(const int *q [[clang::lifetime_capture_by_this]]); + ~Ref(); +}; +void cleanup_ref(Ref *r); + +void via_destructor() { + Ref r; + int *h = new int(7); // expected-warning {{allocated object does not live long enough}} + r = h; + delete h; // expected-note {{allocated object is freed here}} +} // expected-note {{later used by the destructor of 'r' at scope exit}} + +void via_cleanup() { + Ref g __attribute__((cleanup(cleanup_ref))); + int *h = new int(7); // expected-warning {{allocated object does not live long enough}} + g = h; + delete h; // expected-note {{allocated object is freed here}} +} // expected-note {{later used by the cleanup function of 'g' at scope exit}} + +struct [[gsl::Pointer]] ViewRef { + std::string_view sv; + ViewRef &operator=(std::string_view s [[clang::lifetime_capture_by_this]]); + ~ViewRef(); +}; +void via_destructor_invalidated() { + std::string s; + ViewRef r; + r = s; // expected-warning {{local variable 's' is later invalidated}} + s.push_back('x'); // expected-note {{local variable 's' is invalidated here}} +} // expected-note {{later used by the destructor of 'r' at scope exit}} +} // namespace implicit_use_after_free diff --git a/clang/test/Sema/LifetimeSafety/safety.cpp b/clang/test/Sema/LifetimeSafety/safety.cpp index ada2ebf3e35171..18c8950b825019 100644 --- a/clang/test/Sema/LifetimeSafety/safety.cpp +++ b/clang/test/Sema/LifetimeSafety/safety.cpp @@ -1,6 +1,6 @@ -// RUN: %clang_cc1 -fsyntax-only -Wlifetime-safety -Wlifetime-safety-annotation-placement -Wno-dangling -verify=expected,function %s -// RUN: %clang_cc1 -fsyntax-only -flifetime-safety-inference -fexperimental-lifetime-safety-tu-analysis -Wlifetime-safety -Wno-dangling -verify=expected,tu %s -// RUN: %clang_cc1 -fsyntax-only -Wlifetime-safety -Wno-dangling -fcxx-exceptions -verify=expected,function %s +// RUN: %clang_cc1 -fsyntax-only -Wlifetime-safety -Wlifetime-safety-experimental-destructor-use -Wlifetime-safety-annotation-placement -Wno-dangling -verify=expected,function %s +// RUN: %clang_cc1 -fsyntax-only -flifetime-safety-inference -fexperimental-lifetime-safety-tu-analysis -Wlifetime-safety -Wlifetime-safety-experimental-destructor-use -Wno-dangling -verify=expected,tu %s +// RUN: %clang_cc1 -fsyntax-only -Wlifetime-safety -Wlifetime-safety-experimental-destructor-use -Wno-dangling -fcxx-exceptions -verify=expected,function %s #include "Inputs/lifetime-analysis.h" @@ -4914,3 +4914,68 @@ struct CaptureByThis { store(local); } }; + +// A destructor or cleanup callback run at scope exit may read the borrows the +// object holds. +namespace ScopeExitUse { +struct [[gsl::Pointer]] Ref { + std::string_view sv; + Ref() = default; + Ref &operator=(std::string_view s [[clang::lifetime_capture_by_this]]); + ~Ref(); +}; + +void dtor_reverse_order() { + Ref r; + std::string backing; + r = backing; // expected-warning {{local variable 'backing' does not live long enough}} +} // expected-note {{local variable 'backing' is destroyed here}} expected-note {{later used by the destructor of 'r' at scope exit}} + +void dtor_safe_order() { + std::string backing; + Ref r; + r = backing; +} + +struct [[gsl::Pointer]] TrivialRef { + std::string_view sv; + TrivialRef &operator=(std::string_view s [[clang::lifetime_capture_by_this]]); +}; +void trivial_dtor_no_use() { + TrivialRef r; + std::string backing; + r = backing; +} + +void cleanup_ref(Ref *r); +void cleanup_reverse_order() { + Ref g __attribute__((cleanup(cleanup_ref))); + std::string backing; + g = backing; // expected-warning {{local variable 'backing' does not live long enough}} +} // expected-note {{local variable 'backing' is destroyed here}} expected-note {{later used by the cleanup function of 'g' at scope exit}} + +void cleanup_safe_order() { + std::string backing; + Ref g __attribute__((cleanup(cleanup_ref))); + g = backing; +} + +// The closure is not a gsl::Pointer, but destroying it runs ~Ref(). +void closure_dtor_reads_captured_borrow() { + std::function<void()> fn = [] {}; + std::string backing; + Ref r; + r = backing; // expected-warning {{local variable 'backing' does not live long enough}} + fn = [r] {}; +} // expected-note {{local variable 'backing' is destroyed here}} expected-note {{later used by the destructor of 'fn' at scope exit}} + +void use(const Ref &); +// The explicit use and the destructor use of `r` meet after the if. +void explicit_or_destructor_use(bool b) { + Ref r; + std::string backing; + r = backing; // expected-warning {{local variable 'backing' does not live long enough}} + if (b) + use(r); +} // expected-note {{local variable 'backing' is destroyed here}} expected-note {{later used by the destructor of 'r' at scope exit}} +} // namespace ScopeExitUse _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
