https://github.com/Xazax-hun updated https://github.com/llvm/llvm-project/pull/204650
From 716bfe1d63988b64b3a9a83adbe8443168cfda27 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 captured view). The analysis never sees those bodies, so model the action as a use of the object, keeping the borrow live to that point: a borrowed-from object destroyed earlier (reverse-declaration order) is now reported instead of silently dangling. The implicit use has no source expression, so UseFact gains a SourceLocation anchor and reportUseAfterScope/reportUseAfterInvalidation gain SourceLocation overloads for the "used here" note. Owners are excluded from the destructor case (their destruction frees their own storage, already modeled by the ExpireFact). Assisted-by: Claude Opus 4.8 --- .../Analysis/Analyses/LifetimeSafety/Facts.h | 14 +++++ .../Analyses/LifetimeSafety/FactsGenerator.h | 2 + .../Analyses/LifetimeSafety/LifetimeSafety.h | 14 +++++ clang/lib/Analysis/LifetimeSafety/Checker.cpp | 20 +++++- .../LifetimeSafety/FactsGenerator.cpp | 33 ++++++++++ .../Analysis/LifetimeSafety/LiveOrigins.cpp | 2 +- clang/lib/Sema/SemaLifetimeSafety.h | 47 ++++++++++++++ .../Sema/LifetimeSafety/invalidations.cpp | 50 +++++++++++++++ clang/test/Sema/LifetimeSafety/safety.cpp | 62 +++++++++++++++++++ 9 files changed, 241 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index 88b509e1b94df..612f726a1a206 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -240,16 +240,30 @@ class UseFact : public Fact { // True if this use is a write operation (e.g., left-hand side of assignment). // Write operations are exempted from use-after-free checks. bool IsWritten = false; + // For an implicit use with no source expression (a scope-exit destructor or + // cleanup callback reading a borrow): the location to anchor diagnostics at. + SourceLocation ImplicitLoc; 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(SourceLocation ImplicitLoc, const OriginList *OList) + : Fact(Kind::Use), UseExpr(nullptr), OList(OList), + ImplicitLoc(ImplicitLoc) {} const OriginList *getUsedOrigins() const { return OList; } void setUsedOrigins(const OriginList *NewList) { OList = NewList; } const Expr *getUseExpr() const { return UseExpr; } + /// True if this use has no source expression; use getImplicitLoc() instead. + bool isImplicit() const { return UseExpr == nullptr; } + SourceLocation getImplicitLoc() const { return ImplicitLoc; } + /// The location to anchor diagnostics at: the use expression's location, or + /// the explicit anchor location for an implicit use (no source expression). + SourceLocation getUseLoc() const { + return UseExpr ? UseExpr->getExprLoc() : ImplicitLoc; + } void markAsWritten() { IsWritten = true; } bool isWritten() const { return IsWritten; } diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index 5ac67263681ac..c923b99dfc743 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -84,6 +84,8 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { void handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds); + void handleCleanupFunction(const CFGCleanupFunction &CleanupFunction); + void handleFullExprCleanup(const CFGFullExprCleanup &FullExprCleanup); void handleExitBlock(); diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index 28886b826f72f..ec207be0823ff 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -65,6 +65,12 @@ class LifetimeSafetySemaHelper { const Expr *MovedExpr, SourceLocation FreeLoc, llvm::ArrayRef<const Expr *> ExprChain) {} + // Variant for an implicit use with no source expression; `UseLoc` anchors the + // "used here" note. + virtual void reportUseAfterScope(const Expr *IssueExpr, SourceLocation UseLoc, + const Expr *MovedExpr, + SourceLocation FreeLoc, + llvm::ArrayRef<const Expr *> ExprChain) {} virtual void reportUseAfterReturn(const Expr *IssueExpr, const Expr *ReturnExpr, @@ -88,6 +94,14 @@ class LifetimeSafetySemaHelper { virtual void reportUseAfterInvalidation(const ParmVarDecl *PVD, const Expr *UseExpr, const Expr *InvalidationExpr) {} + // Variants for an implicit use with no source expression; `UseLoc` anchors + // the "used here" note. + virtual void reportUseAfterInvalidation(const Expr *IssueExpr, + SourceLocation UseLoc, + const Expr *InvalidationExpr) {} + virtual void reportUseAfterInvalidation(const ParmVarDecl *PVD, + SourceLocation UseLoc, + const Expr *InvalidationExpr) {} virtual void reportInvalidatedField(const Expr *IssueExpr, const FieldDecl *Field, const Expr *InvalidationExpr) {} diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index d41d6f43f837b..269d898de0f85 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -72,7 +72,7 @@ class LifetimeChecker { static SourceLocation GetFactLoc(llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> F) { if (const auto *UF = F.dyn_cast<const UseFact *>()) - return UF->getUseExpr()->getExprLoc(); + return UF->getUseLoc(); if (const auto *OEF = F.dyn_cast<const OriginEscapesFact *>()) { if (auto *ReturnEsc = dyn_cast<ReturnEscapeFact>(OEF)) return ReturnEsc->getReturnExpr()->getExprLoc(); @@ -254,7 +254,23 @@ class LifetimeChecker { SourceLocation ExpiryLoc = Warning.ExpiryLoc; if (const auto *UF = CausingFact.dyn_cast<const UseFact *>()) { - if (Warning.InvalidatedByExpr) { + // An implicit use has no source expression; anchor diagnostics at its + // location. + if (UF->isImplicit()) { + if (Warning.InvalidatedByExpr) { + if (IssueExpr) + SemaHelper->reportUseAfterInvalidation( + IssueExpr, UF->getImplicitLoc(), Warning.InvalidatedByExpr); + else if (InvalidatedPVD) + SemaHelper->reportUseAfterInvalidation(InvalidatedPVD, + UF->getImplicitLoc(), + Warning.InvalidatedByExpr); + } else { + SemaHelper->reportUseAfterScope( + IssueExpr, UF->getImplicitLoc(), MovedExpr, ExpiryLoc, + getExprChain(LoanPropagation.buildOriginFlowChain(UF, LID))); + } + } else if (Warning.InvalidatedByExpr) { if (IssueExpr) // Use-after-invalidation of an object on stack. SemaHelper->reportUseAfterInvalidation(IssueExpr, UF->getUseExpr(), diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 3861117005752..d3f5a3d252b33 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -125,6 +125,9 @@ void FactsGenerator::run() { else if (std::optional<CFGLifetimeEnds> LifetimeEnds = Element.getAs<CFGLifetimeEnds>()) handleLifetimeEnds(*LifetimeEnds); + else if (std::optional<CFGCleanupFunction> CleanupFunction = + Element.getAs<CFGCleanupFunction>()) + handleCleanupFunction(*CleanupFunction); else if (std::optional<CFGFullExprCleanup> FullExprCleanup = Element.getAs<CFGFullExprCleanup>()) { handleFullExprCleanup(*FullExprCleanup); @@ -792,6 +795,20 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { const VarDecl *LifetimeEndsVD = LifetimeEnds.getVarDecl(); if (!LifetimeEndsVD) return; + // A non-trivial destructor at scope exit may read a borrow the object holds + // (e.g. a [[gsl::Pointer]] whose out-of-line ~T() dereferences its view). The + // analysis never sees that body, so model the destruction as a use, keeping + // the borrow live to that point so a borrowed-from object destroyed earlier + // (reverse-declaration order) is reported. Owners are excluded: their + // destruction frees their own storage (modeled by the ExpireFact), not a + // borrow into another object. + QualType VDTy = LifetimeEndsVD->getType(); + if (const CXXRecordDecl *RD = VDTy->getAsCXXRecordDecl(); + RD && RD->hasDefinition() && RD->hasNonTrivialDestructor() && + !isGslOwnerType(VDTy) && hasOrigins(VDTy)) + if (OriginList *List = getOriginsList(*LifetimeEndsVD)) + CurrentBlockFacts.push_back(FactMgr.createFact<UseFact>( + LifetimeEnds.getTriggerStmt()->getEndLoc(), List)); // Expire the origin when its variable's lifetime ends to ensure liveness // doesn't persist through loop back-edges. std::optional<OriginID> ExpiredOID; @@ -807,6 +824,22 @@ void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { ExpiredOID)); } +void FactsGenerator::handleCleanupFunction( + const CFGCleanupFunction &CleanupFunction) { + // A variable with __attribute__((cleanup(fn))) has fn(&var) called at scope + // exit; like a non-trivial destructor, that callback may read a borrow the + // variable holds, so model it as a use of the variable. Unlike the destructor + // case, owners are not excluded: the cleanup callback receives the variable's + // address explicitly and may read whatever it holds, regardless of whether + // the variable is a gsl::Owner. + const VarDecl *VD = CleanupFunction.getVarDecl(); + if (!VD || !hasOrigins(VD->getType())) + return; + if (OriginList *List = getOriginsList(*VD)) + CurrentBlockFacts.push_back( + FactMgr.createFact<UseFact>(VD->getLocation(), List)); +} + void FactsGenerator::handleFullExprCleanup( const CFGFullExprCleanup &FullExprCleanup) { for (const auto *MTE : FullExprCleanup.getExpiringMTEs()) diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index cfbcacf04b1b0..009dbf5d352ad 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -56,7 +56,7 @@ struct Lattice { static SourceLocation GetFactLoc(CausingFactType F) { if (const auto *UF = F.dyn_cast<const UseFact *>()) - return UF->getUseExpr()->getExprLoc(); + return UF->getUseLoc(); if (const auto *OEF = F.dyn_cast<const OriginEscapesFact *>()) { 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 4bde272fb40a1..370adde199a1d 100644 --- a/clang/lib/Sema/SemaLifetimeSafety.h +++ b/clang/lib/Sema/SemaLifetimeSafety.h @@ -101,6 +101,25 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { << UseExpr->getSourceRange(); } + void reportUseAfterScope(const Expr *IssueExpr, SourceLocation UseLoc, + 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; + + S.Diag(IssueExpr->getExprLoc(), DiagID) + << getDiagSubjectDescription(IssueExpr) << 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); + + reportAliasingChain(ExprChain); + + S.Diag(UseLoc, diag::note_lifetime_safety_used_here); + } + void reportUseAfterReturn(const Expr *IssueExpr, const Expr *ReturnExpr, const Expr *MovedExpr) override { unsigned DiagID = MovedExpr @@ -194,6 +213,34 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { S.Diag(UseExpr->getExprLoc(), diag::note_lifetime_safety_used_here) << UseExpr->getSourceRange(); } + void reportUseAfterInvalidation(const Expr *IssueExpr, SourceLocation UseLoc, + const Expr *InvalidationExpr) override { + auto WarnDiag = isa<CXXDeleteExpr>(InvalidationExpr) + ? diag::warn_lifetime_safety_use_after_free + : diag::warn_lifetime_safety_invalidation; + auto UseDiag = isa<CXXDeleteExpr>(InvalidationExpr) + ? diag::note_lifetime_safety_freed_here + : diag::note_lifetime_safety_invalidated_here; + S.Diag(IssueExpr->getExprLoc(), WarnDiag) + << getDiagSubjectDescription(IssueExpr) << IssueExpr->getSourceRange(); + S.Diag(InvalidationExpr->getExprLoc(), UseDiag) + << InvalidationExpr->getSourceRange(); + S.Diag(UseLoc, diag::note_lifetime_safety_used_here); + } + void reportUseAfterInvalidation(const ParmVarDecl *PVD, SourceLocation UseLoc, + const Expr *InvalidationExpr) override { + auto WarnDiag = isa<CXXDeleteExpr>(InvalidationExpr) + ? diag::warn_lifetime_safety_use_after_free + : diag::warn_lifetime_safety_invalidation; + auto UseDiag = isa<CXXDeleteExpr>(InvalidationExpr) + ? diag::note_lifetime_safety_freed_here + : diag::note_lifetime_safety_invalidated_here; + S.Diag(PVD->getSourceRange().getBegin(), WarnDiag) + << getDiagSubjectDescription(PVD) << PVD->getSourceRange(); + S.Diag(InvalidationExpr->getExprLoc(), UseDiag) + << InvalidationExpr->getSourceRange(); + S.Diag(UseLoc, diag::note_lifetime_safety_used_here); + } void reportInvalidatedField(const Expr *IssueExpr, const FieldDecl *DanglingField, diff --git a/clang/test/Sema/LifetimeSafety/invalidations.cpp b/clang/test/Sema/LifetimeSafety/invalidations.cpp index 301822f066de8..7add7e39906f3 100644 --- a/clang/test/Sema/LifetimeSafety/invalidations.cpp +++ b/clang/test/Sema/LifetimeSafety/invalidations.cpp @@ -920,3 +920,53 @@ void invalid_after_ternary_reset(bool flag) { } } // namespace unique_ptr_invalidation + +// A non-trivial destructor at scope exit is modeled as an implicit use (a +// UseFact with no source expression). The live-origins join helper reads such a +// fact's explicit location rather than dereferencing its null use-expression, so +// a function with both a tracked borrow and such an implicit use (e.g. a +// std::function local) is analyzed without crashing. +namespace implicit_use_join { +void view_and_callable() { + std::string text = "long enough heap-allocated backing string value here!"; + std::string_view tok = text; // a tracked borrow (live origin) + std::function<void()> c = [] {}; // non-trivial dtor at scope exit + (void)c; + (void)tok.size(); // no-warning (must not crash) +} + +void view_and_callable_mutation() { + std::string text = "long enough heap-allocated backing string value here!"; + std::string_view tok = text; // expected-warning {{local variable 'text' is later invalidated}} + std::function<void()> c = [] {}; + (void)c; + text.push_back('x'); // expected-note {{invalidated here}} + (void)tok.size(); // expected-note {{later used here}} +} +} // namespace implicit_use_join + +// A borrow read by a scope-exit destructor or cleanup callback (an implicit use +// with no source expression) that was freed earlier via `delete` is a +// use-after-free; the diagnostic is anchored at the implicit use's location. +namespace implicit_use_after_free { +struct [[gsl::Pointer]] Ref { + const int *p; + Ref &operator=(const int *q [[clang::lifetime_capture_by(this)]]); + ~Ref(); // non-trivial, out-of-line: may read p +}; +void cleanup_ref(Ref *r); // out-of-line: may read r->p + +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 {{freed here}} +} // expected-note {{later used here}} + +void via_cleanup() { + Ref g __attribute__((cleanup(cleanup_ref))); // expected-note {{later used here}} + int *h = new int(7); // expected-warning {{allocated object does not live long enough}} + g = h; + delete h; // expected-note {{freed here}} +} +} // namespace implicit_use_after_free diff --git a/clang/test/Sema/LifetimeSafety/safety.cpp b/clang/test/Sema/LifetimeSafety/safety.cpp index 65bfe69e854ac..9ae84db8c2018 100644 --- a/clang/test/Sema/LifetimeSafety/safety.cpp +++ b/clang/test/Sema/LifetimeSafety/safety.cpp @@ -3894,3 +3894,65 @@ struct [[gsl::Pointer()]] PtrWithInt { int x; }; PtrWithInt f() { return PtrWithInt{10}; } + +// A scope-exit destructor or cleanup callback may read a borrow the object +// holds. The analysis never sees the out-of-line body, so it is modeled as a +// use of the object: a borrowed-from object destroyed earlier (reverse- +// declaration order) is reported. +namespace ScopeExitUse { +struct [[gsl::Pointer]] Ref { + std::string_view sv; + Ref() = default; + Ref &operator=(std::string_view s [[clang::lifetime_capture_by(this)]]); + ~Ref(); // non-trivial, out-of-line: may read sv +}; + +void dtor_reverse_order() { + Ref r; // destroyed LAST + std::string backing; // destroyed FIRST + r = backing; // expected-warning {{local variable 'backing' does not live long enough}} +} // expected-note {{destroyed here}} expected-note {{later used here}} + +void dtor_safe_order() { + std::string backing; + Ref r; + r = backing; // no-warning +} + +struct [[gsl::Pointer]] TrivialRef { + std::string_view sv; + TrivialRef &operator=(std::string_view s [[clang::lifetime_capture_by(this)]]); + // trivial destructor cannot read sv +}; +void trivial_dtor_no_use() { + TrivialRef r; + std::string backing; + r = backing; // no-warning +} + +void cleanup_ref(Ref *r); // out-of-line: may read r->sv +void cleanup_reverse_order() { + Ref g __attribute__((cleanup(cleanup_ref))); // cleaned up LAST // expected-note {{later used here}} + std::string backing; // destroyed FIRST + g = backing; // expected-warning {{local variable 'backing' does not live long enough}} +} // expected-note {{destroyed here}} + +void cleanup_safe_order() { + std::string backing; + Ref g __attribute__((cleanup(cleanup_ref))); + g = backing; // no-warning +} + +// The destructor-as-use is not limited to gsl::Pointer: any origin-holding type +// with a non-trivial destructor qualifies. A lambda capturing a Ref by value is +// not itself a gsl::Pointer, but holds the Ref's origin and has a non-trivial +// destructor -- so ~closure runs ~Ref(), which reads the captured borrow. +void closure_dtor_reads_captured_borrow() { + std::function<void()> fn = [] {}; // destroyed LAST + std::string backing; // destroyed FIRST + Ref r; + r = backing; // expected-warning {{local variable 'backing' does not live long enough}} + fn = [r] {}; // expected-note {{local variable 'r' aliases the storage of local variable 'backing'}} +} // expected-note {{destroyed here}} expected-note {{later used here}} +} // namespace ScopeExitUse + _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
