llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-temporal-safety Author: Gábor Horváth (Xazax-hun) <details> <summary>Changes</summary> 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 --- Full diff: https://github.com/llvm/llvm-project/pull/204630.diff 8 Files Affected: - (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h (+26-3) - (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h (+7) - (modified) clang/lib/Analysis/LifetimeSafety/Checker.cpp (+8) - (modified) clang/lib/Analysis/LifetimeSafety/Facts.cpp (+7) - (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+7) - (modified) clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp (+2) - (modified) clang/lib/Sema/SemaLifetimeSafety.h (+19) - (modified) clang/test/Sema/LifetimeSafety/safety.cpp (+27) ``````````diff diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index 88b509e1b94df..289b89ac8854e 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -162,9 +162,10 @@ class OriginEscapesFact : public Fact { public: /// The way an origin can escape the current scope. enum class EscapeKind : uint8_t { - Return, /// Escapes via return statement. - Field, /// Escapes via assignment to a field. - Global, /// Escapes via assignment to global storage. + Return, /// Escapes via return statement. + Field, /// Escapes via assignment to a field. + Global, /// Escapes via assignment to global storage. + Capture, /// Captured into the object via lifetime_capture_by(this). } EscKind; static bool classof(const Fact *F) { @@ -234,6 +235,28 @@ class GlobalEscapeFact : public OriginEscapesFact { const OriginManager &OM) const override; }; +/// Represents that an origin is captured into the implicit object via +/// [[clang::lifetime_capture_by(this)]]. The capture flows into the whole-object +/// `this` origin (we do not know which member receives it), so this is emitted +/// for `this` at function exit to keep it live: a borrow still held there spans +/// to exit and dangles if it outlived the captured local. +class CaptureEscapeFact : public OriginEscapesFact { + SourceLocation Loc; + +public: + CaptureEscapeFact(OriginID OID, SourceLocation Loc) + : OriginEscapesFact(OID, EscapeKind::Capture), Loc(Loc) {} + + static bool classof(const Fact *F) { + return F->getKind() == Kind::OriginEscapes && + static_cast<const OriginEscapesFact *>(F)->getEscapeKind() == + EscapeKind::Capture; + } + 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 28886b826f72f..e1581b502487f 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -66,6 +66,13 @@ 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, + llvm::ArrayRef<const Expr *> ExprChain) {} + 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 d41d6f43f837b..bfb5cf542088d 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -301,6 +301,8 @@ class LifetimeChecker { Warning.InvalidatedByExpr); } else if (isa<ReturnEscapeFact>(OEF)) { // FIXME: Diagnose invalidated return escapes separately. + } else if (isa<CaptureEscapeFact>(OEF)) { + // FIXME: Diagnose a capture-held loan invalidated through `this`. } else llvm_unreachable("Unhandled OriginEscapesFact type"); } else if (const auto *RetEscape = dyn_cast<ReturnEscapeFact>(OEF)) @@ -315,6 +317,12 @@ class LifetimeChecker { // Global escape. SemaHelper->reportDanglingGlobal(IssueExpr, GlobalEscape->getGlobal(), MovedExpr, ExpiryLoc); + else if (const auto *CaptureEscape = dyn_cast<CaptureEscapeFact>(OEF)) + // A borrow is still held by the object (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, CaptureEscape->getLoc(), + MovedExpr, ExpiryLoc, /*ExprChain=*/{}); 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..0e37851e72743 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 CaptureEscapeFact::dump(llvm::raw_ostream &OS, const LoanManager &, + const OriginManager &OM) const { + OS << "OriginEscapes ("; + OM.dump(getEscapedOriginID(), OS); + OS << ", via Capture)\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 545836cd76fb9..4714498cd7642 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -790,6 +790,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<CaptureEscapeFact>( + (*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 cfbcacf04b1b0..de0e082a117b9 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 *CaptureEsc = dyn_cast<CaptureEscapeFact>(OEF)) + return CaptureEsc->getLoc(); } llvm_unreachable("unhandled causing fact in PointerUnion"); } diff --git a/clang/lib/Sema/SemaLifetimeSafety.h b/clang/lib/Sema/SemaLifetimeSafety.h index a8bde363e3397..fb7d65d574194 100644 --- a/clang/lib/Sema/SemaLifetimeSafety.h +++ b/clang/lib/Sema/SemaLifetimeSafety.h @@ -100,6 +100,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 diff --git a/clang/test/Sema/LifetimeSafety/safety.cpp b/clang/test/Sema/LifetimeSafety/safety.cpp index 7a2644e46a6e1..c7a0eceb25b7d 100644 --- a/clang/test/Sema/LifetimeSafety/safety.cpp +++ b/clang/test/Sema/LifetimeSafety/safety.cpp @@ -3866,3 +3866,30 @@ struct [[gsl::Pointer()]] PtrWithInt { int x; }; PtrWithInt f() { return PtrWithInt{10}; } + +// 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 {{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 {{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); + } +}; `````````` </details> https://github.com/llvm/llvm-project/pull/204630 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
