Author: Kashika Akhouri Date: 2026-01-26T11:32:52+01:00 New Revision: 7d887bccc9bc5d493f8aeb558275ce24f520f2c2
URL: https://github.com/llvm/llvm-project/commit/7d887bccc9bc5d493f8aeb558275ce24f520f2c2 DIFF: https://github.com/llvm/llvm-project/commit/7d887bccc9bc5d493f8aeb558275ce24f520f2c2.diff LOG: [LifetimeSafety] Add suggestion and inference for implicit this (#176703) This PR implements support for automatically suggesting and inferring `[[clang::lifetimebound]]` annotations for the implicit `this`. The analysis now models the implicit this object as an implicit parameter that carries a "placeholder loan" from the caller's scope. By tracking this loan through the method body, the analysis can identify if this reaches an escape point, such as a return statement. Key Changes: - Updated `PlaceholderLoan` to hold a union of `ParmVarDecl*` and `CXXMethodDecl*` - Extended `OriginManager` to handle `CXXThisExpr` and create a dedicated origin list for the implicit this parameter - Modified `FactsGenerator::issuePlaceholderLoans` to create a placeholder loan for this at the entry of non-static member functions - Updated `implicitObjectParamIsLifetimeBound` in `LifetimeAnnotations.cpp` to check for the attribute on the method declaration itself - Added logic to skip implicit methods and standard assignment operators to avoid redundant warnings on boilerplate code Example: ```cpp struct ReturnsSelf { const ReturnsSelf& get() const { return *this; } }; void test_get_on_temporary() { const ReturnsSelf& s_ref = ReturnsSelf().get(); (void)s_ref; } ``` Warning: ``` a7.cpp:5:33: warning: implict this in intra-TU function should be marked [[clang::lifetimebound]] [-Wexperimental-lifetime-safety-intra-tu-suggestions] 5 | const ReturnsSelf& get() const { | ~~~ ^ | [[clang::lifetimebound]] a7.cpp:6:12: note: param returned here 6 | return *this; | ^~~~~ a7.cpp:11:30: warning: temporary bound to local reference 's_ref' will be destroyed at the end of the full-expression [-Wdangling] 11 | const ReturnsSelf& s_ref = ReturnsSelf().get(); | ^~~~~~~~~~~~~ ``` Fixes https://github.com/llvm/llvm-project/issues/169941 Added: Modified: clang/include/clang/Analysis/Analyses/LifetimeSafety/Checker.h clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Analysis/LifetimeSafety/Checker.cpp clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp clang/lib/Analysis/LifetimeSafety/Origins.cpp clang/lib/Sema/AnalysisBasedWarnings.cpp clang/lib/Sema/SemaDecl.cpp clang/test/Sema/warn-lifetime-safety-suggestions.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Checker.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Checker.h index 03636be7d00c3..5c631c92c0167 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Checker.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Checker.h @@ -28,7 +28,7 @@ namespace clang::lifetimes::internal { void runLifetimeChecker(const LoanPropagationAnalysis &LoanPropagation, const LiveOriginsAnalysis &LiveOrigins, const FactManager &FactMgr, AnalysisDeclContext &ADC, - LifetimeSafetyReporter *Reporter); + LifetimeSafetySemaHelper *SemaHelper); } // namespace clang::lifetimes::internal diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index a66925b7302ca..1bb34e6986857 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -195,7 +195,7 @@ class TestPointFact : public Fact { class FactManager { public: FactManager(const AnalysisDeclContext &AC, const CFG &Cfg) - : OriginMgr(AC.getASTContext()) { + : OriginMgr(AC.getASTContext(), AC.getDecl()) { BlockToFacts.resize(Cfg.getNumBlockIDs()); } diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index 9c91355355233..25d97b4af1ed5 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -42,10 +42,20 @@ enum class SuggestionScope { IntraTU // For suggestions on definitions local to a Translation Unit. }; -class LifetimeSafetyReporter { +/// Abstract interface for operations requiring Sema access. +/// +/// This class exists to break a circular dependency: the LifetimeSafety +/// analysis target cannot directly depend on clangSema (which would create the +/// cycle: clangSema -> clangAnalysis -> clangAnalysisLifetimeSafety -> +/// clangSema). +/// +/// Instead, this interface is implemented in AnalysisBasedWarnings.cpp (part of +/// clangSema), allowing the analysis to report diagnostics and modify the AST +/// through Sema without introducing a circular dependency. +class LifetimeSafetySemaHelper { public: - LifetimeSafetyReporter() = default; - virtual ~LifetimeSafetyReporter() = default; + LifetimeSafetySemaHelper() = default; + virtual ~LifetimeSafetySemaHelper() = default; virtual void reportUseAfterFree(const Expr *IssueExpr, const Expr *UseExpr, SourceLocation FreeLoc, @@ -56,15 +66,24 @@ class LifetimeSafetyReporter { SourceLocation ExpiryLoc, Confidence Confidence) {} - // Suggests lifetime bound annotations for function paramters - virtual void suggestAnnotation(SuggestionScope Scope, - const ParmVarDecl *ParmToAnnotate, - const Expr *EscapeExpr) {} + // Suggests lifetime bound annotations for function paramters. + virtual void suggestLifetimeboundToParmVar(SuggestionScope Scope, + const ParmVarDecl *ParmToAnnotate, + const Expr *EscapeExpr) {} + + // Suggests lifetime bound annotations for implicit this. + virtual void suggestLifetimeboundToImplicitThis(SuggestionScope Scope, + const CXXMethodDecl *MD, + const Expr *EscapeExpr) {} + + // Adds inferred lifetime bound attribute for implicit this to its + // TypeSourceInfo. + virtual void addLifetimeBoundToImplicitThis(const CXXMethodDecl *MD) {} }; /// The main entry point for the analysis. void runLifetimeSafetyAnalysis(AnalysisDeclContext &AC, - LifetimeSafetyReporter *Reporter, + LifetimeSafetySemaHelper *SemaHelper, LifetimeSafetyStats &Stats, bool CollectStats); namespace internal { @@ -85,7 +104,7 @@ struct LifetimeFactory { class LifetimeSafetyAnalysis { public: LifetimeSafetyAnalysis(AnalysisDeclContext &AC, - LifetimeSafetyReporter *Reporter); + LifetimeSafetySemaHelper *SemaHelper); void run(); @@ -98,7 +117,7 @@ class LifetimeSafetyAnalysis { private: AnalysisDeclContext &AC; - LifetimeSafetyReporter *Reporter; + LifetimeSafetySemaHelper *SemaHelper; LifetimeFactory Factory; std::unique_ptr<FactManager> FactMgr; std::unique_ptr<LiveOriginsAnalysis> LiveOrigins; diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h index ee1634a6f5ea2..a366e3e811cbf 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h @@ -15,6 +15,7 @@ #define LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_LOANS_H #include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" #include "clang/AST/ExprCXX.h" #include "clang/Analysis/Analyses/LifetimeSafety/Utils.h" #include "llvm/Support/raw_ostream.h" @@ -100,10 +101,11 @@ class PathLoan : public Loan { static bool classof(const Loan *L) { return L->getKind() == Kind::Path; } }; -/// A placeholder loan held by a function parameter, representing a borrow from -/// the caller's scope. +/// A placeholder loan held by a function parameter or an implicit 'this' +/// object, representing a borrow from the caller's scope. /// -/// Created at function entry for each pointer or reference parameter with an +/// Created at function entry for each pointer or reference parameter or for +/// the implicit 'this' parameter of instance methods, with an /// origin. Unlike PathLoan, placeholder loans: /// - Have no IssueExpr (created at function entry, not at a borrow site) /// - Have no AccessPath (the borrowed object is not visible to the function) @@ -111,17 +113,27 @@ class PathLoan : public Loan { /// invalidations (e.g., vector::push_back) /// /// When a placeholder loan escapes the function (e.g., via return), it -/// indicates the parameter should be marked [[clang::lifetimebound]], enabling -/// lifetime annotation suggestions. +/// indicates the parameter or method should be marked [[clang::lifetimebound]], +/// enabling lifetime annotation suggestions. class PlaceholderLoan : public Loan { - /// The function parameter that holds this placeholder loan. - const ParmVarDecl *PVD; + /// The function parameter or method (representing 'this') that holds this + /// placeholder loan. + llvm::PointerUnion<const ParmVarDecl *, const CXXMethodDecl *> ParamOrMethod; public: PlaceholderLoan(LoanID ID, const ParmVarDecl *PVD) - : Loan(Kind::Placeholder, ID), PVD(PVD) {} + : Loan(Kind::Placeholder, ID), ParamOrMethod(PVD) {} - const ParmVarDecl *getParmVarDecl() const { return PVD; } + PlaceholderLoan(LoanID ID, const CXXMethodDecl *MD) + : Loan(Kind::Placeholder, ID), ParamOrMethod(MD) {} + + const ParmVarDecl *getParmVarDecl() const { + return ParamOrMethod.dyn_cast<const ParmVarDecl *>(); + } + + const CXXMethodDecl *getMethodDecl() const { + return ParamOrMethod.dyn_cast<const CXXMethodDecl *>(); + } void dump(llvm::raw_ostream &OS) const override; diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h index 690faae996f0e..8c638bdcace3f 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h @@ -15,6 +15,7 @@ #define LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_ORIGINS_H #include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" #include "clang/AST/Expr.h" #include "clang/AST/TypeBase.h" #include "clang/Analysis/Analyses/LifetimeSafety/LifetimeStats.h" @@ -124,7 +125,7 @@ bool doesDeclHaveStorage(const ValueDecl *D); /// variables and expressions. class OriginManager { public: - explicit OriginManager(ASTContext &AST) : AST(AST) {} + explicit OriginManager(ASTContext &AST, const Decl *D); /// Gets or creates the OriginList for a given ValueDecl. /// @@ -144,6 +145,10 @@ class OriginManager { /// \returns The OriginList, or nullptr for non-pointer rvalues. OriginList *getOrCreateList(const Expr *E); + /// Returns the OriginList for the implicit 'this' parameter if the current + /// declaration is an instance method. + std::optional<OriginList *> getThisOrigins() const { return ThisOrigins; } + const Origin &getOrigin(OriginID ID) const; llvm::ArrayRef<Origin> getOrigins() const { return AllOrigins; } @@ -172,6 +177,7 @@ class OriginManager { llvm::BumpPtrAllocator ListAllocator; llvm::DenseMap<const clang::ValueDecl *, OriginList *> DeclToList; llvm::DenseMap<const clang::Expr *, OriginList *> ExprToList; + std::optional<OriginList *> ThisOrigins; }; } // namespace clang::lifetimes::internal diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 33461284e11dd..afe37ab88c5c8 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10830,18 +10830,30 @@ def note_lifetime_safety_used_here : Note<"later used here">; def note_lifetime_safety_destroyed_here : Note<"destroyed here">; def note_lifetime_safety_returned_here : Note<"returned here">; -def warn_lifetime_safety_intra_tu_suggestion +def warn_lifetime_safety_intra_tu_param_suggestion : Warning<"parameter in intra-TU function should be marked " "[[clang::lifetimebound]]">, InGroup<LifetimeSafetyIntraTUSuggestions>, DefaultIgnore; -def warn_lifetime_safety_cross_tu_suggestion +def warn_lifetime_safety_cross_tu_param_suggestion : Warning<"parameter in cross-TU function should be marked " "[[clang::lifetimebound]]">, InGroup<LifetimeSafetyCrossTUSuggestions>, DefaultIgnore; +def warn_lifetime_safety_intra_tu_this_suggestion + : Warning<"implicit this in intra-TU function should be marked " + "[[clang::lifetimebound]]">, + InGroup<LifetimeSafetyIntraTUSuggestions>, + DefaultIgnore; + +def warn_lifetime_safety_cross_tu_this_suggestion + : Warning<"implicit this in cross-TU function should be marked " + "[[clang::lifetimebound]]">, + InGroup<LifetimeSafetyCrossTUSuggestions>, + DefaultIgnore; + def note_lifetime_safety_suggestion_returned_here : Note<"param returned here">; // For non-floating point, expressions of the form x == x or x != x diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 48803fae9d837..7b3479bbc3677 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -33,6 +33,7 @@ #include "clang/AST/StmtCXX.h" #include "clang/AST/Type.h" #include "clang/AST/TypeLoc.h" +#include "clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h" #include "clang/Basic/AttrSubjectMatchRules.h" #include "clang/Basic/Builtins.h" #include "clang/Basic/CapturedStmt.h" @@ -1256,6 +1257,10 @@ class Sema final : public SemaBase { bool ForceComplain = false, bool (*IsPlausibleResult)(QualType) = nullptr); + // Adds implicit lifetime bound attribute for implicit this to its + // TypeSourceInfo. + void addLifetimeBoundToImplicitThis(CXXMethodDecl *MD); + /// Figure out if an expression could be turned into a call. /// /// Use this when trying to recover from an error where the programmer may diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index f7383126fac38..91fbcc0a98650 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -49,22 +49,26 @@ struct PendingWarning { Confidence ConfidenceLevel; }; +using AnnotationTarget = + llvm::PointerUnion<const ParmVarDecl *, const CXXMethodDecl *>; + class LifetimeChecker { private: llvm::DenseMap<LoanID, PendingWarning> FinalWarningsMap; - llvm::DenseMap<const ParmVarDecl *, const Expr *> AnnotationWarningsMap; + llvm::DenseMap<AnnotationTarget, const Expr *> AnnotationWarningsMap; const LoanPropagationAnalysis &LoanPropagation; const LiveOriginsAnalysis &LiveOrigins; const FactManager &FactMgr; - LifetimeSafetyReporter *Reporter; + LifetimeSafetySemaHelper *SemaHelper; ASTContext &AST; public: LifetimeChecker(const LoanPropagationAnalysis &LoanPropagation, const LiveOriginsAnalysis &LiveOrigins, const FactManager &FM, - AnalysisDeclContext &ADC, LifetimeSafetyReporter *Reporter) + AnalysisDeclContext &ADC, + LifetimeSafetySemaHelper *SemaHelper) : LoanPropagation(LoanPropagation), LiveOrigins(LiveOrigins), FactMgr(FM), - Reporter(Reporter), AST(ADC.getASTContext()) { + SemaHelper(SemaHelper), AST(ADC.getASTContext()) { for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>()) for (const Fact *F : FactMgr.getFacts(B)) if (const auto *EF = F->getAs<ExpireFact>()) @@ -88,10 +92,14 @@ class LifetimeChecker { for (LoanID LID : EscapedLoans) { const Loan *L = FactMgr.getLoanMgr().getLoan(LID); if (const auto *PL = dyn_cast<PlaceholderLoan>(L)) { - const ParmVarDecl *PVD = PL->getParmVarDecl(); - if (PVD->hasAttr<LifetimeBoundAttr>()) - continue; - AnnotationWarningsMap.try_emplace(PVD, OEF->getEscapeExpr()); + if (const auto *PVD = PL->getParmVarDecl()) { + if (PVD->hasAttr<LifetimeBoundAttr>()) + continue; + AnnotationWarningsMap.try_emplace(PVD, OEF->getEscapeExpr()); + } else if (const auto *MD = PL->getMethodDecl()) { + if (!implicitObjectParamIsLifetimeBound(MD)) + AnnotationWarningsMap.try_emplace(MD, OEF->getEscapeExpr()); + } } } } @@ -139,7 +147,7 @@ class LifetimeChecker { } void issuePendingWarnings() { - if (!Reporter) + if (!SemaHelper) return; for (const auto &[LID, Warning] : FinalWarningsMap) { const Loan *L = FactMgr.getLoanMgr().getLoan(LID); @@ -151,12 +159,12 @@ class LifetimeChecker { SourceLocation ExpiryLoc = Warning.ExpiryLoc; if (const auto *UF = CausingFact.dyn_cast<const UseFact *>()) - Reporter->reportUseAfterFree(IssueExpr, UF->getUseExpr(), ExpiryLoc, - Confidence); + SemaHelper->reportUseAfterFree(IssueExpr, UF->getUseExpr(), ExpiryLoc, + Confidence); else if (const auto *OEF = CausingFact.dyn_cast<const OriginEscapesFact *>()) - Reporter->reportUseAfterReturn(IssueExpr, OEF->getEscapeExpr(), - ExpiryLoc, Confidence); + SemaHelper->reportUseAfterReturn(IssueExpr, OEF->getEscapeExpr(), + ExpiryLoc, Confidence); else llvm_unreachable("Unhandled CausingFact type"); } @@ -164,50 +172,81 @@ class LifetimeChecker { /// Returns the declaration of a function that is visible across translation /// units, if such a declaration exists and is diff erent from the definition. - static const FunctionDecl *getCrossTUDecl(const ParmVarDecl &PVD, + static const FunctionDecl *getCrossTUDecl(const FunctionDecl &FD, SourceManager &SM) { - const auto *FD = dyn_cast<FunctionDecl>(PVD.getDeclContext()); - if (!FD) + if (!FD.isExternallyVisible()) return nullptr; - if (!FD->isExternallyVisible()) - return nullptr; - const FileID DefinitionFile = SM.getFileID(FD->getLocation()); - for (const FunctionDecl *Redecl : FD->redecls()) + const FileID DefinitionFile = SM.getFileID(FD.getLocation()); + for (const FunctionDecl *Redecl : FD.redecls()) if (SM.getFileID(Redecl->getLocation()) != DefinitionFile) return Redecl; return nullptr; } + static const FunctionDecl *getCrossTUDecl(const ParmVarDecl &PVD, + SourceManager &SM) { + if (const auto *FD = dyn_cast<FunctionDecl>(PVD.getDeclContext())) + return getCrossTUDecl(*FD, SM); + return nullptr; + } + + static void suggestWithScopeForParmVar(LifetimeSafetySemaHelper *SemaHelper, + const ParmVarDecl *PVD, + SourceManager &SM, + const Expr *EscapeExpr) { + if (const FunctionDecl *CrossTUDecl = getCrossTUDecl(*PVD, SM)) + SemaHelper->suggestLifetimeboundToParmVar( + SuggestionScope::CrossTU, + CrossTUDecl->getParamDecl(PVD->getFunctionScopeIndex()), EscapeExpr); + else + SemaHelper->suggestLifetimeboundToParmVar(SuggestionScope::IntraTU, PVD, + EscapeExpr); + } + + static void + suggestWithScopeForImplicitThis(LifetimeSafetySemaHelper *SemaHelper, + const CXXMethodDecl *MD, SourceManager &SM, + const Expr *EscapeExpr) { + if (const FunctionDecl *CrossTUDecl = getCrossTUDecl(*MD, SM)) + SemaHelper->suggestLifetimeboundToImplicitThis( + SuggestionScope::CrossTU, cast<CXXMethodDecl>(CrossTUDecl), + EscapeExpr); + else + SemaHelper->suggestLifetimeboundToImplicitThis(SuggestionScope::IntraTU, + MD, EscapeExpr); + } + void suggestAnnotations() { - if (!Reporter) + if (!SemaHelper) return; SourceManager &SM = AST.getSourceManager(); - for (const auto &[PVD, EscapeExpr] : AnnotationWarningsMap) { - if (const FunctionDecl *CrossTUDecl = getCrossTUDecl(*PVD, SM)) - Reporter->suggestAnnotation( - SuggestionScope::CrossTU, - CrossTUDecl->getParamDecl(PVD->getFunctionScopeIndex()), - EscapeExpr); - else - Reporter->suggestAnnotation(SuggestionScope::IntraTU, PVD, EscapeExpr); + for (auto [Target, EscapeExpr] : AnnotationWarningsMap) { + if (const auto *PVD = Target.dyn_cast<const ParmVarDecl *>()) + suggestWithScopeForParmVar(SemaHelper, PVD, SM, EscapeExpr); + else if (const auto *MD = Target.dyn_cast<const CXXMethodDecl *>()) + suggestWithScopeForImplicitThis(SemaHelper, MD, SM, EscapeExpr); } } void inferAnnotations() { - for (const auto &[ConstPVD, EscapeExpr] : AnnotationWarningsMap) { - ParmVarDecl *PVD = const_cast<ParmVarDecl *>(ConstPVD); - const auto *FD = dyn_cast<FunctionDecl>(PVD->getDeclContext()); - if (!FD) - continue; - // Propagates inferred attributes via the most recent declaration to - // ensure visibility for callers in post-order analysis. - FD = getDeclWithMergedLifetimeBoundAttrs(FD); - ParmVarDecl *InferredPVD = const_cast<ParmVarDecl *>( - FD->getParamDecl(PVD->getFunctionScopeIndex())); - if (!InferredPVD->hasAttr<LifetimeBoundAttr>()) - InferredPVD->addAttr( - LifetimeBoundAttr::CreateImplicit(AST, PVD->getLocation())); + for (auto [Target, EscapeExpr] : AnnotationWarningsMap) { + if (const auto *MD = Target.dyn_cast<const CXXMethodDecl *>()) { + if (!implicitObjectParamIsLifetimeBound(MD)) + SemaHelper->addLifetimeBoundToImplicitThis(cast<CXXMethodDecl>(MD)); + } else if (const auto *PVD = Target.dyn_cast<const ParmVarDecl *>()) { + const auto *FD = dyn_cast<FunctionDecl>(PVD->getDeclContext()); + if (!FD) + continue; + // Propagates inferred attributes via the most recent declaration to + // ensure visibility for callers in post-order analysis. + FD = getDeclWithMergedLifetimeBoundAttrs(FD); + ParmVarDecl *InferredPVD = const_cast<ParmVarDecl *>( + FD->getParamDecl(PVD->getFunctionScopeIndex())); + if (!InferredPVD->hasAttr<LifetimeBoundAttr>()) + InferredPVD->addAttr( + LifetimeBoundAttr::CreateImplicit(AST, PVD->getLocation())); + } } } }; @@ -216,9 +255,9 @@ class LifetimeChecker { void runLifetimeChecker(const LoanPropagationAnalysis &LP, const LiveOriginsAnalysis &LO, const FactManager &FactMgr, AnalysisDeclContext &ADC, - LifetimeSafetyReporter *Reporter) { + LifetimeSafetySemaHelper *SemaHelper) { llvm::TimeTraceScope TimeProfile("LifetimeChecker"); - LifetimeChecker Checker(LP, LO, FactMgr, ADC, Reporter); + LifetimeChecker Checker(LP, LO, FactMgr, ADC, SemaHelper); } } // namespace clang::lifetimes::internal diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 5aad51728054f..c982b255d54ea 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -633,6 +633,13 @@ llvm::SmallVector<Fact *> FactsGenerator::issuePlaceholderLoans() { return {}; llvm::SmallVector<Fact *> PlaceholderLoanFacts; + if (const auto *MD = dyn_cast<CXXMethodDecl>(FD); MD && MD->isInstance()) { + OriginList *List = *FactMgr.getOriginMgr().getThisOrigins(); + const PlaceholderLoan *L = + FactMgr.getLoanMgr().createLoan<PlaceholderLoan>(MD); + PlaceholderLoanFacts.push_back( + FactMgr.createFact<IssueFact>(L->getID(), List->getOuterOriginID())); + } for (const ParmVarDecl *PVD : FD->parameters()) { OriginList *List = getOriginsList(*PVD); if (!List) diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp index be0d405bd3086..bf9a65254e8fa 100644 --- a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp @@ -48,9 +48,9 @@ static void DebugOnlyFunction(AnalysisDeclContext &AC, const CFG &Cfg, } #endif -LifetimeSafetyAnalysis::LifetimeSafetyAnalysis(AnalysisDeclContext &AC, - LifetimeSafetyReporter *Reporter) - : AC(AC), Reporter(Reporter) {} +LifetimeSafetyAnalysis::LifetimeSafetyAnalysis( + AnalysisDeclContext &AC, LifetimeSafetySemaHelper *SemaHelper) + : AC(AC), SemaHelper(SemaHelper) {} void LifetimeSafetyAnalysis::run() { llvm::TimeTraceScope TimeProfile("LifetimeSafetyAnalysis"); @@ -90,7 +90,7 @@ void LifetimeSafetyAnalysis::run() { DEBUG_WITH_TYPE("LiveOrigins", LiveOrigins->dump(llvm::dbgs(), FactMgr->getTestPoints())); - runLifetimeChecker(*LoanPropagation, *LiveOrigins, *FactMgr, AC, Reporter); + runLifetimeChecker(*LoanPropagation, *LiveOrigins, *FactMgr, AC, SemaHelper); } void collectLifetimeStats(AnalysisDeclContext &AC, OriginManager &OM, @@ -103,9 +103,9 @@ void collectLifetimeStats(AnalysisDeclContext &AC, OriginManager &OM, } // namespace internal void runLifetimeSafetyAnalysis(AnalysisDeclContext &AC, - LifetimeSafetyReporter *Reporter, + LifetimeSafetySemaHelper *SemaHelper, LifetimeSafetyStats &Stats, bool CollectStats) { - internal::LifetimeSafetyAnalysis Analysis(AC, Reporter); + internal::LifetimeSafetyAnalysis Analysis(AC, SemaHelper); Analysis.run(); if (CollectStats) collectLifetimeStats(AC, Analysis.getFactManager().getOriginMgr(), Stats); diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp index ca933f612eb08..bf539303695b1 100644 --- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp @@ -86,6 +86,12 @@ bool doesDeclHaveStorage(const ValueDecl *D) { return !D->getType()->isReferenceType(); } +OriginManager::OriginManager(ASTContext &AST, const Decl *D) : AST(AST) { + if (const auto *MD = llvm::dyn_cast_or_null<CXXMethodDecl>(D); + MD && MD->isInstance()) + ThisOrigins = buildListForType(MD->getThisType(), MD); +} + OriginList *OriginManager::createNode(const ValueDecl *D, QualType QT) { OriginID NewID = getNextOriginID(); AllOrigins.emplace_back(NewID, D, QT.getTypePtrOrNull()); @@ -137,6 +143,12 @@ OriginList *OriginManager::getOrCreateList(const Expr *E) { return It->second; QualType Type = E->getType(); + // Special handling for 'this' expressions to share origins with the method's + // implicit object parameter. + if (llvm::isa<CXXThisExpr>(E)) { + assert(ThisOrigins && "origins for 'this' should be set for a method decl"); + return *ThisOrigins; + } // Special handling for DeclRefExpr to share origins with the underlying decl. if (auto *DRE = dyn_cast<DeclRefExpr>(E)) { diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 03d84fc935b8e..39995992eb717 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -13,6 +13,7 @@ //===----------------------------------------------------------------------===// #include "clang/Sema/AnalysisBasedWarnings.h" +#include "TypeLocBuilder.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" @@ -30,6 +31,7 @@ #include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h" #include "clang/Analysis/Analyses/CalledOnceCheck.h" #include "clang/Analysis/Analyses/Consumed.h" +#include "clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h" #include "clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h" #include "clang/Analysis/Analyses/ReachableCode.h" #include "clang/Analysis/Analyses/ThreadSafety.h" @@ -2872,10 +2874,10 @@ class CallableVisitor : public DynamicRecursiveASTVisitor { namespace clang::lifetimes { namespace { -class LifetimeSafetyReporterImpl : public LifetimeSafetyReporter { +class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { public: - LifetimeSafetyReporterImpl(Sema &S) : S(S) {} + LifetimeSafetySemaHelperImpl(Sema &S) : S(S) {} void reportUseAfterFree(const Expr *IssueExpr, const Expr *UseExpr, SourceLocation FreeLoc, Confidence C) override { @@ -2901,32 +2903,47 @@ class LifetimeSafetyReporterImpl : public LifetimeSafetyReporter { << EscapeExpr->getSourceRange(); } - void suggestAnnotation(SuggestionScope Scope, - const ParmVarDecl *ParmToAnnotate, - const Expr *EscapeExpr) override { - unsigned DiagID; - switch (Scope) { - case SuggestionScope::CrossTU: - DiagID = diag::warn_lifetime_safety_cross_tu_suggestion; - break; - case SuggestionScope::IntraTU: - DiagID = diag::warn_lifetime_safety_intra_tu_suggestion; - break; - } - + void suggestLifetimeboundToParmVar(SuggestionScope Scope, + const ParmVarDecl *ParmToAnnotate, + const Expr *EscapeExpr) override { + unsigned DiagID = + (Scope == SuggestionScope::CrossTU) + ? diag::warn_lifetime_safety_cross_tu_param_suggestion + : diag::warn_lifetime_safety_intra_tu_param_suggestion; SourceLocation InsertionPoint = Lexer::getLocForEndOfToken( ParmToAnnotate->getEndLoc(), 0, S.getSourceManager(), S.getLangOpts()); - S.Diag(ParmToAnnotate->getBeginLoc(), DiagID) << ParmToAnnotate->getSourceRange() << FixItHint::CreateInsertion(InsertionPoint, " [[clang::lifetimebound]]"); + S.Diag(EscapeExpr->getBeginLoc(), + diag::note_lifetime_safety_suggestion_returned_here) + << EscapeExpr->getSourceRange(); + } + void suggestLifetimeboundToImplicitThis(SuggestionScope Scope, + const CXXMethodDecl *MD, + const Expr *EscapeExpr) override { + unsigned DiagID = (Scope == SuggestionScope::CrossTU) + ? diag::warn_lifetime_safety_cross_tu_this_suggestion + : diag::warn_lifetime_safety_intra_tu_this_suggestion; + SourceLocation InsertionPoint; + InsertionPoint = Lexer::getLocForEndOfToken( + MD->getTypeSourceInfo()->getTypeLoc().getEndLoc(), 0, + S.getSourceManager(), S.getLangOpts()); + S.Diag(InsertionPoint, DiagID) + << MD->getNameInfo().getSourceRange() + << FixItHint::CreateInsertion(InsertionPoint, + " [[clang::lifetimebound]]"); S.Diag(EscapeExpr->getBeginLoc(), diag::note_lifetime_safety_suggestion_returned_here) << EscapeExpr->getSourceRange(); } + void addLifetimeBoundToImplicitThis(const CXXMethodDecl *MD) override { + S.addLifetimeBoundToImplicitThis(const_cast<CXXMethodDecl *>(MD)); + } + private: Sema &S; }; @@ -2939,7 +2956,7 @@ LifetimeSafetyTUAnalysis(Sema &S, TranslationUnitDecl *TU, llvm::TimeTraceScope TimeProfile("LifetimeSafetyTUAnalysis"); CallGraph CG; CG.addToCallGraph(TU); - lifetimes::LifetimeSafetyReporterImpl Reporter(S); + lifetimes::LifetimeSafetySemaHelperImpl SemaHelper(S); for (auto *Node : llvm::post_order(&CG)) { const clang::FunctionDecl *CanonicalFD = dyn_cast_or_null<clang::FunctionDecl>(Node->getDecl()); @@ -2955,7 +2972,7 @@ LifetimeSafetyTUAnalysis(Sema &S, TranslationUnitDecl *TU, AC.getCFGBuildOptions().AddTemporaryDtors = true; AC.getCFGBuildOptions().setAllAlwaysAdd(); if (AC.getCFG()) - runLifetimeSafetyAnalysis(AC, &Reporter, LSStats, S.CollectStats); + runLifetimeSafetyAnalysis(AC, &SemaHelper, LSStats, S.CollectStats); } } @@ -3190,9 +3207,9 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( // stable. if (EnableLifetimeSafetyAnalysis && S.getLangOpts().CPlusPlus) { if (AC.getCFG()) { - lifetimes::LifetimeSafetyReporterImpl LifetimeSafetyReporter(S); - lifetimes::runLifetimeSafetyAnalysis(AC, &LifetimeSafetyReporter, LSStats, - S.CollectStats); + lifetimes::LifetimeSafetySemaHelperImpl LifetimeSafetySemaHelper(S); + lifetimes::runLifetimeSafetyAnalysis(AC, &LifetimeSafetySemaHelper, + LSStats, S.CollectStats); } } // Check for violations of "called once" parameter properties. diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 8a6d1617151a7..0b3416a12f9f4 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -14772,6 +14772,22 @@ StmtResult Sema::ActOnCXXForRangeIdentifier(Scope *S, SourceLocation IdentLoc, : IdentLoc); } +void Sema::addLifetimeBoundToImplicitThis(CXXMethodDecl *MD) { + if (!MD || lifetimes::implicitObjectParamIsLifetimeBound(MD)) + return; + auto *Attr = LifetimeBoundAttr::CreateImplicit(Context, MD->getLocation()); + QualType MethodType = MD->getType(); + QualType AttributedType = + Context.getAttributedType(Attr, MethodType, MethodType); + TypeLocBuilder TLB; + if (TypeSourceInfo *TSI = MD->getTypeSourceInfo()) + TLB.pushFullCopy(TSI->getTypeLoc()); + AttributedTypeLoc TyLoc = TLB.push<AttributedTypeLoc>(AttributedType); + TyLoc.setAttr(Attr); + MD->setType(AttributedType); + MD->setTypeSourceInfo(TLB.getTypeSourceInfo(Context, AttributedType)); +} + void Sema::CheckCompleteVariableDeclaration(VarDecl *var) { if (var->isInvalidDecl()) return; diff --git a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp index 974ea06e39651..cfae34420baa7 100644 --- a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp +++ b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp @@ -12,6 +12,8 @@ struct View; struct [[gsl::Owner]] MyObj { int id; + MyObj(int i) : id(i) {} + MyObj() {} ~MyObj() {} // Non-trivial destructor MyObj operator+(MyObj); @@ -46,6 +48,15 @@ inline View redeclared_in_header(View a) { // expected-warning {{parameter in i return a; // expected-note {{param returned here}} } +struct ReturnThis { + const ReturnThis& get() const; // expected-warning {{implicit this in cross-TU function should be marked [[clang::lifetimebound]]}}. +}; + +struct ReturnThisPointer { + const ReturnThisPointer* get() const; // expected-warning {{implicit this in cross-TU function should be marked [[clang::lifetimebound]]}}. +}; + + #endif // TEST_HEADER_H //--- test_source.cpp @@ -161,35 +172,86 @@ static View return_view_static(View a) { // expected-warning {{parameter in int return a; // expected-note {{param returned here}} } -//===----------------------------------------------------------------------===// -// FIXME Test Cases -//===----------------------------------------------------------------------===// +const ReturnThis& ReturnThis::get() const { + return *this; // expected-note {{param returned here}} +} + +const ReturnThisPointer* ReturnThisPointer::get() const { + return this; // expected-note {{param returned here}} +} struct ReturnsSelf { - const ReturnsSelf& get() const { - return *this; + ReturnsSelf() {} + ~ReturnsSelf() {} + const ReturnsSelf& get() const { // expected-warning {{implicit this in intra-TU function should be marked [[clang::lifetimebound]]}}. + return *this; // expected-note {{param returned here}} } }; +struct ReturnThisAnnotated { + const ReturnThisAnnotated& get() [[clang::lifetimebound]] { return *this; } +}; + struct ViewProvider { + ViewProvider(int d) : data(d) {} + ~ViewProvider() {} MyObj data; - View getView() const { - return data; + View getView() const { // expected-warning {{implicit this in intra-TU function should be marked [[clang::lifetimebound]]}}. + return data; // expected-note {{param returned here}} } }; -// FIXME: Fails to generate lifetime suggestions for the implicit 'this' parameter, as this feature is not yet implemented. -void test_get_on_temporary() { - const ReturnsSelf& s_ref = ReturnsSelf().get(); - (void)s_ref; +View return_view_field(const ViewProvider& v) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. + return v.data; // expected-note {{param returned here}} +} + +void test_get_on_temporary_pointer() { + const ReturnsSelf* s_ref = &ReturnsSelf().get(); // expected-warning {{object whose reference is captured does not live long enough}}. + // expected-note@-1 {{destroyed here}} + (void)s_ref; // expected-note {{later used here}} +} + +void test_get_on_temporary_ref() { + const ReturnsSelf& s_ref = ReturnsSelf().get(); // expected-warning {{object whose reference is captured does not live long enough}}. + // expected-note@-1 {{destroyed here}} + (void)s_ref; // expected-note {{later used here}} } -// FIXME: Fails to generate lifetime suggestions for the implicit 'this' parameter, as this feature is not yet implemented. void test_getView_on_temporary() { - View sv = ViewProvider{1}.getView(); - (void)sv; + View sv = ViewProvider{1}.getView(); // expected-warning {{object whose reference is captured does not live long enough}}. + // expected-note@-1 {{destroyed here}} + (void)sv; // expected-note {{later used here}} } +void test_get_on_temporary_copy() { + ReturnsSelf copy = ReturnsSelf().get(); + (void)copy; +} + +struct MemberReturn { + MyObj data; + + MyObj& getRef() { // expected-warning {{implicit this in intra-TU function should be marked [[clang::lifetimebound]]}}. + return data; // expected-note {{param returned here}} + } + + MyObj& getRefExplicit() { // expected-warning {{implicit this in intra-TU function should be marked [[clang::lifetimebound]]}}. + return this->data; // expected-note {{param returned here}} + } + + MyObj& getRefDereference() { // expected-warning {{implicit this in intra-TU function should be marked [[clang::lifetimebound]]}}. + return (*this).data; // expected-note {{param returned here}} + } + + const MyObj* getPtr() { // expected-warning {{implicit this in intra-TU function should be marked [[clang::lifetimebound]]}}. + return &data; // expected-note {{param returned here}} + } + + const MyObj* getPtrExplicit() { // expected-warning {{implicit this in intra-TU function should be marked [[clang::lifetimebound]]}}. + return &(this->data); // expected-note {{param returned here}} + } +}; + //===----------------------------------------------------------------------===// // Annotation Inference Test Cases //===----------------------------------------------------------------------===// _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
