https://github.com/Xazax-hun updated https://github.com/llvm/llvm-project/pull/204630
From e01def2e480102d2076f762b51f52a6b19ff0963 Mon Sep 17 00:00:00 2001 From: Gabor Horvath <[email protected]> Date: Thu, 18 Jun 2026 17:28:18 +0100 Subject: [PATCH] [LifetimeSafety] Catch intra-method lifetime_capture_by(this) dangles A borrow captured into the object via [[clang::lifetime_capture_by(this)]] flows into the never-expiring `this` origin, so when the capture and the captured local's expiry both happen inside one method, `this` is not otherwise live at the expiry and the dangle was missed. Add a CaptureEscapeFact, emitted for `this` at function exit, so checkExpiry sees the captured local going out of scope while still held by the object and reports it as use-after-scope. Assisted-by: Claude Opus 4.8 --- .../Analysis/Analyses/LifetimeSafety/Facts.h | 24 +++++++++++++++++ .../Analyses/LifetimeSafety/LifetimeSafety.h | 6 +++++ clang/lib/Analysis/LifetimeSafety/Checker.cpp | 9 +++++++ clang/lib/Analysis/LifetimeSafety/Facts.cpp | 7 +++++ .../LifetimeSafety/FactsGenerator.cpp | 7 +++++ .../Analysis/LifetimeSafety/LiveOrigins.cpp | 2 ++ clang/lib/Sema/SemaLifetimeSafety.h | 19 +++++++++++++ .../LifetimeSafety/noescape-violation.cpp | 12 +++++++++ clang/test/Sema/LifetimeSafety/safety.cpp | 27 +++++++++++++++++++ 9 files changed, 113 insertions(+) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index 5c671a93b149c..90c03477ac787 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -165,6 +165,8 @@ class OriginEscapesFact : public Fact { Return, /// Escapes via return statement. Field, /// Escapes via assignment to a field. Global, /// Escapes via assignment to global storage. + This, /// Escapes via the enclosing object `this`, which outlives the + /// method. } EscKind; static bool classof(const Fact *F) { @@ -234,6 +236,28 @@ class GlobalEscapeFact : public OriginEscapesFact { const OriginManager &OM) const override; }; +/// Represents an origin escaping through the enclosing object `this`, which +/// outlives the current method. Emitted for `this` at function exit so a borrow +/// still held by the object there is seen as escaping -- e.g. a borrow captured +/// via [[clang::lifetime_capture_by(this)]] that outlived the captured local is +/// reported as a use-after-scope. +class ThisEscapeFact : public OriginEscapesFact { + SourceLocation Loc; + +public: + ThisEscapeFact(OriginID OID, SourceLocation Loc) + : OriginEscapesFact(OID, EscapeKind::This), Loc(Loc) {} + + static bool classof(const Fact *F) { + return F->getKind() == Kind::OriginEscapes && + static_cast<const OriginEscapesFact *>(F)->getEscapeKind() == + EscapeKind::This; + } + SourceLocation getLoc() const { return Loc; } + void dump(llvm::raw_ostream &OS, const LoanManager &, + const OriginManager &OM) const override; +}; + class UseFact : public Fact { const Expr *UseExpr; const OriginList *OList; diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index 80a23f18baebd..6b44294cc6214 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -69,6 +69,12 @@ class LifetimeSafetySemaHelper { 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, + const Expr *MovedExpr, + SourceLocation FreeLoc) {} + virtual void reportUseAfterReturn(const Expr *IssueExpr, const Expr *ReturnExpr, const Expr *MovedExpr) {} diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 746ebbfb15c39..bcc790d54742b 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -305,6 +305,8 @@ class LifetimeChecker { Warning.InvalidatedByExpr); } else if (isa<ReturnEscapeFact>(OEF)) { // FIXME: Diagnose invalidated return escapes separately. + } else if (isa<ThisEscapeFact>(OEF)) { + // FIXME: Diagnose a `this`-held loan invalidated through `this`. } else llvm_unreachable("Unhandled OriginEscapesFact type"); } else if (const auto *RetEscape = dyn_cast<ReturnEscapeFact>(OEF)) @@ -319,6 +321,13 @@ class LifetimeChecker { // Global escape. SemaHelper->reportDanglingGlobal(IssueExpr, GlobalEscape->getGlobal(), MovedExpr, ExpiryLoc); + else if (const auto *ThisEscape = dyn_cast<ThisEscapeFact>(OEF)) + // A borrow is still held by the object (e.g. captured via + // lifetime_capture_by(this)) when the captured local goes out of + // scope; reuse the use-after-scope diagnostic at the capturing + // method. + SemaHelper->reportUseAfterScope(IssueExpr, ThisEscape->getLoc(), + MovedExpr, ExpiryLoc); else llvm_unreachable("Unhandled OriginEscapesFact type"); } else diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 3d7fbcdacc830..9dac4864e7642 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -77,6 +77,13 @@ void GlobalEscapeFact::dump(llvm::raw_ostream &OS, const LoanManager &, OS << ", via Global)\n"; } +void ThisEscapeFact::dump(llvm::raw_ostream &OS, const LoanManager &, + const OriginManager &OM) const { + OS << "OriginEscapes ("; + OM.dump(getEscapedOriginID(), OS); + OS << ", via This)\n"; +} + void UseFact::dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM) const { OS << "Use ("; diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 8358c69a5165a..70b26148305cd 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -858,6 +858,13 @@ void FactsGenerator::handleExitBlock() { FactMgr.createFact<GlobalEscapeFact>(O.ID, VD)); } } + + // A borrow captured via [[clang::lifetime_capture_by(this)]] flows into the + // never-expiring `this` origin, so it is not otherwise live at the captured + // local's expiry. Keep `this` live at exit so the dangle is caught. + if (auto ThisOrigins = FactMgr.getOriginMgr().getThisOrigins()) + EscapesInCurrentBlock.push_back(FactMgr.createFact<ThisEscapeFact>( + (*ThisOrigins)->getOuterOriginID(), AC.getDecl()->getEndLoc())); } void FactsGenerator::handleGSLPointerConstruction(const CXXConstructExpr *CCE) { diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index 69b903c813555..27a8697c823c9 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -64,6 +64,8 @@ static SourceLocation GetFactLoc(CausingFactType F) { return FieldEsc->getFieldDecl()->getLocation(); if (auto *GlobalEsc = dyn_cast<GlobalEscapeFact>(OEF)) return GlobalEsc->getGlobal()->getLocation(); + if (auto *ThisEsc = dyn_cast<ThisEscapeFact>(OEF)) + return ThisEsc->getLoc(); } llvm_unreachable("unhandled causing fact in PointerUnion"); } diff --git a/clang/lib/Sema/SemaLifetimeSafety.h b/clang/lib/Sema/SemaLifetimeSafety.h index 3b9ea9eafbec5..055effd111b7a 100644 --- a/clang/lib/Sema/SemaLifetimeSafety.h +++ b/clang/lib/Sema/SemaLifetimeSafety.h @@ -127,6 +127,25 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { << UseExpr->getSourceRange(); } + void reportUseAfterScope(const Expr *IssueExpr, SourceLocation UseLoc, + const Expr *MovedExpr, + SourceLocation FreeLoc) 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; + + S.Diag(UseLoc, diag::note_lifetime_safety_used_here); + } + void reportUseAfterReturn(const Expr *IssueExpr, const Expr *ReturnExpr, const Expr *MovedExpr) override { unsigned DiagID = MovedExpr diff --git a/clang/test/Sema/LifetimeSafety/noescape-violation.cpp b/clang/test/Sema/LifetimeSafety/noescape-violation.cpp index 048c500239b4f..367f70a66e652 100644 --- a/clang/test/Sema/LifetimeSafety/noescape-violation.cpp +++ b/clang/test/Sema/LifetimeSafety/noescape-violation.cpp @@ -192,6 +192,18 @@ int* return_spaced_brackets(int* p [ [clang::noescape] /*some comment*/ ]) { // return p; // expected-note {{returned here}} } +// FIXME: A [[clang::noescape]] parameter captured into the object via +// [[clang::lifetime_capture_by(this)]] escapes the function and should be +// diagnosed. It currently is not: the escape is modeled as a ThisEscapeFact at +// exit, which does not feed the noescape-violation machinery. +struct CaptureByThisNoescape { + const int *p; + void store(const int &x [[clang::lifetime_capture_by(this)]]); + void captures_noescape(const int &n [[clang::noescape]]) { + store(n); // FIXME-warning: parameter is marked [[clang::noescape]] but escapes + } +}; + namespace callable_wrappers { std::function<void()> escape_noescape_via_function(int &x [[clang::noescape]]) { // expected-warning {{parameter is marked [[clang::noescape]] but escapes}} diff --git a/clang/test/Sema/LifetimeSafety/safety.cpp b/clang/test/Sema/LifetimeSafety/safety.cpp index 2cbf651eb46b5..dd8adf868d829 100644 --- a/clang/test/Sema/LifetimeSafety/safety.cpp +++ b/clang/test/Sema/LifetimeSafety/safety.cpp @@ -4047,3 +4047,30 @@ void test_loop_cond_bind(bool cond) { consume_loop_cond_bind(cond ? &x : &y); // no-warning } } + +// A borrow captured into the implicit object via +// [[clang::lifetime_capture_by(this)]] that outlives the captured local is +// caught at the capturing method's exit. +struct CaptureByThis { + int member; + const int *p; + void store(const int &x [[clang::lifetime_capture_by(this)]]); + void captures_dangling_local() { + int local = 0; + store(local); // expected-warning {{local variable 'local' does not live long enough}} + } // expected-note {{local variable 'local' is destroyed here}} expected-note {{later used here}} + void captures_in_nested_scope() { + { + int local = 0; + store(local); // expected-warning {{local variable 'local' does not live long enough}} + } // expected-note {{local variable 'local' is destroyed here}} + } // expected-note {{later used here}} + // Negative: capturing a caller-scoped reference into `this` does not dangle. + void captures_caller_ref(const int &caller_ref) { + store(caller_ref); + } + // Negative: capturing a member (same lifetime as the object) does not dangle. + void captures_member() { + store(member); + } +}; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
