llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-analysis Author: Yuan Suo (suoyuan666) <details> <summary>Changes</summary> Initially, I planned to make all diagnostics in Checker.cpp use `buildOriginFlowChain`. However, after thinking about it, I think the current scope is more suitable for review. --- Patch is 84.21 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/212083.diff 12 Files Affected: - (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h (+16-7) - (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h (+1-1) - (modified) clang/lib/Analysis/LifetimeSafety/Checker.cpp (+19-16) - (modified) clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp (+7-2) - (modified) clang/lib/Sema/SemaLifetimeSafety.h (+31-13) - (modified) clang/test/Sema/LifetimeSafety/annotation-suggestions.cpp (+15-8) - (modified) clang/test/Sema/LifetimeSafety/dangling-field.cpp (+18-9) - (modified) clang/test/Sema/LifetimeSafety/dangling-global.cpp (+1-1) - (modified) clang/test/Sema/LifetimeSafety/invalidations.cpp (+13-6) - (modified) clang/test/Sema/LifetimeSafety/nocfg.cpp (+65-28) - (modified) clang/test/Sema/LifetimeSafety/safety-c.c (+4-2) - (modified) clang/test/Sema/LifetimeSafety/safety.cpp (+167-85) ``````````diff diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index a51ef2f7cc0ba..8e52363a41b4f 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -71,17 +71,20 @@ class LifetimeSafetySemaHelper { virtual void reportUseAfterReturn(const Expr *IssueExpr, const Expr *ReturnExpr, - const Expr *MovedExpr) {} + const Expr *MovedExpr, + llvm::ArrayRef<const Expr *> ExprChain) {} virtual void reportDanglingField(const Expr *IssueExpr, const FieldDecl *Field, const Expr *MovedExpr, - SourceLocation ExpiryLoc) {} + SourceLocation ExpiryLoc, + llvm::ArrayRef<const Expr *> ExprChain) {} virtual void reportDanglingGlobal(const Expr *IssueExpr, const VarDecl *DanglingGlobal, const Expr *MovedExpr, - SourceLocation ExpiryLoc) {} + SourceLocation ExpiryLoc, + llvm::ArrayRef<const Expr *> ExprChain) {} // Reports when a reference/iterator is used after the container operation // that invalidated it. @@ -95,16 +98,22 @@ class LifetimeSafetySemaHelper { llvm::ArrayRef<const Expr *> ExprChain) {} virtual void reportInvalidatedField(const Expr *IssueExpr, const FieldDecl *Field, - const Expr *InvalidationExpr) {} + const Expr *InvalidationExpr, + llvm::ArrayRef<const Expr *> ExprChain) {} virtual void reportInvalidatedField(const ParmVarDecl *PVD, const FieldDecl *Field, - const Expr *InvalidationExpr) {} + const Expr *InvalidationExpr, + llvm::ArrayRef<const Expr *> ExprChain) {} virtual void reportInvalidatedGlobal(const Expr *IssueExpr, const VarDecl *Global, - const Expr *InvalidationExpr) {} + const Expr *InvalidationExpr, + llvm::ArrayRef<const Expr *> ExprChain) { + } virtual void reportInvalidatedGlobal(const ParmVarDecl *PVD, const VarDecl *Global, - const Expr *InvalidationExpr) {} + const Expr *InvalidationExpr, + llvm::ArrayRef<const Expr *> ExprChain) { + } using EscapingTarget = llvm::PointerUnion<const Expr *, const FieldDecl *, const VarDecl *>; diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h index e13442facd82d..36c5e4242262e 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h @@ -51,7 +51,7 @@ class LoanPropagationAnalysis { const LoanID TargetLoan, const CFG *Cfg) const; - llvm::SmallVector<OriginID> buildOriginFlowChain(const UseFact *UF, + llvm::SmallVector<OriginID> buildOriginFlowChain(const Fact *F, const LoanID TargetLoan, const CFG *Cfg) const; diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 53e5077131147..f099e400ee3b2 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -281,33 +281,35 @@ class LifetimeChecker { } else if (const auto *OEF = CausingFact.dyn_cast<const OriginEscapesFact *>()) { + llvm::SmallVector<const Expr *> ExprChain = + getExprChain(LoanPropagation.buildOriginFlowChain(OEF, LID, Cfg)); if (Warning.InvalidatedByExpr) { if (const auto *FieldEscape = dyn_cast<FieldEscapeFact>(OEF)) { // Invalidated object escapes to a field. if (IssueExpr) // Invalidated object on stack escapes to a field. - SemaHelper->reportInvalidatedField(IssueExpr, - FieldEscape->getFieldDecl(), - Warning.InvalidatedByExpr); + SemaHelper->reportInvalidatedField( + IssueExpr, FieldEscape->getFieldDecl(), + Warning.InvalidatedByExpr, ExprChain); else if (InvalidatedPVD) // Invalidated parameter escapes to a field. - SemaHelper->reportInvalidatedField(InvalidatedPVD, - FieldEscape->getFieldDecl(), - Warning.InvalidatedByExpr); + SemaHelper->reportInvalidatedField( + InvalidatedPVD, FieldEscape->getFieldDecl(), + Warning.InvalidatedByExpr, ExprChain); } else if (const auto *GlobalEscape = dyn_cast<GlobalEscapeFact>(OEF)) { // Invalidated object escapes to global or static storage. if (IssueExpr) // Invalidated object on stack escapes to global or static // storage. - SemaHelper->reportInvalidatedGlobal(IssueExpr, - GlobalEscape->getGlobal(), - Warning.InvalidatedByExpr); + SemaHelper->reportInvalidatedGlobal( + IssueExpr, GlobalEscape->getGlobal(), + Warning.InvalidatedByExpr, ExprChain); else if (InvalidatedPVD) // Invalidated parameter escapes to global or static storage. - SemaHelper->reportInvalidatedGlobal(InvalidatedPVD, - GlobalEscape->getGlobal(), - Warning.InvalidatedByExpr); + SemaHelper->reportInvalidatedGlobal( + InvalidatedPVD, GlobalEscape->getGlobal(), + Warning.InvalidatedByExpr, ExprChain); } else if (isa<ReturnEscapeFact>(OEF)) { // FIXME: Diagnose invalidated return escapes separately. } else @@ -315,15 +317,16 @@ class LifetimeChecker { } else if (const auto *RetEscape = dyn_cast<ReturnEscapeFact>(OEF)) // Return stack address. SemaHelper->reportUseAfterReturn( - IssueExpr, RetEscape->getReturnExpr(), MovedExpr); + IssueExpr, RetEscape->getReturnExpr(), MovedExpr, ExprChain); else if (const auto *FieldEscape = dyn_cast<FieldEscapeFact>(OEF)) // Dangling field. - SemaHelper->reportDanglingField( - IssueExpr, FieldEscape->getFieldDecl(), MovedExpr, ExpiryLoc); + SemaHelper->reportDanglingField(IssueExpr, + FieldEscape->getFieldDecl(), + MovedExpr, ExpiryLoc, ExprChain); else if (const auto *GlobalEscape = dyn_cast<GlobalEscapeFact>(OEF)) // Global escape. SemaHelper->reportDanglingGlobal(IssueExpr, GlobalEscape->getGlobal(), - MovedExpr, ExpiryLoc); + MovedExpr, ExpiryLoc, ExprChain); else llvm_unreachable("Unhandled OriginEscapesFact type"); } else diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 078892bd48c10..b2787c3abc987 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -366,7 +366,12 @@ llvm::SmallVector<OriginID> LoanPropagationAnalysis::buildOriginFlowChain( } llvm::SmallVector<OriginID> LoanPropagationAnalysis::buildOriginFlowChain( - const UseFact *UF, const LoanID TargetLoan, const CFG *Cfg) const { - return PImpl->buildOriginFlowChain(UF, TargetLoan, Cfg); + const Fact *F, const LoanID TargetLoan, const CFG *Cfg) const { + if (const auto *UF = llvm::dyn_cast<UseFact>(F)) + return PImpl->buildOriginFlowChain(UF, TargetLoan, Cfg); + if (const auto *OEF = llvm::dyn_cast<OriginEscapesFact>(F)) + return PImpl->buildOriginFlowChain(OEF, OEF->getEscapedOriginID(), + TargetLoan, Cfg); + llvm_unreachable("Unhandled Fact type"); } } // namespace clang::lifetimes::internal diff --git a/clang/lib/Sema/SemaLifetimeSafety.h b/clang/lib/Sema/SemaLifetimeSafety.h index 1d9f94be7e22d..eaa5981639e80 100644 --- a/clang/lib/Sema/SemaLifetimeSafety.h +++ b/clang/lib/Sema/SemaLifetimeSafety.h @@ -128,7 +128,8 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { } void reportUseAfterReturn(const Expr *IssueExpr, const Expr *ReturnExpr, - const Expr *MovedExpr) override { + const Expr *MovedExpr, + llvm::ArrayRef<const Expr *> ExprChain) override { unsigned DiagID = MovedExpr ? diag::warn_lifetime_safety_return_stack_addr_moved : diag::warn_lifetime_safety_return_stack_addr; @@ -139,14 +140,17 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { if (MovedExpr) S.Diag(MovedExpr->getExprLoc(), diag::note_lifetime_safety_moved_here) << MovedExpr->getSourceRange(); + + reportAliasingChain(ExprChain); + S.Diag(ReturnExpr->getExprLoc(), diag::note_lifetime_safety_returned_here) << ReturnExpr->getSourceRange(); } void reportDanglingField(const Expr *IssueExpr, const FieldDecl *DanglingField, - const Expr *MovedExpr, - SourceLocation ExpiryLoc) override { + const Expr *MovedExpr, SourceLocation ExpiryLoc, + llvm::ArrayRef<const Expr *> ExprChain) override { unsigned DiagID = MovedExpr ? diag::warn_lifetime_safety_dangling_field_moved : diag::warn_lifetime_safety_dangling_field; @@ -158,6 +162,9 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { if (MovedExpr) S.Diag(MovedExpr->getExprLoc(), diag::note_lifetime_safety_moved_here) << MovedExpr->getSourceRange(); + + reportAliasingChain(ExprChain); + S.Diag(DanglingField->getLocation(), diag::note_lifetime_safety_dangling_field_here) << DanglingField->getEndLoc(); @@ -165,8 +172,8 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { void reportDanglingGlobal(const Expr *IssueExpr, const VarDecl *DanglingGlobal, - const Expr *MovedExpr, - SourceLocation ExpiryLoc) override { + const Expr *MovedExpr, SourceLocation ExpiryLoc, + llvm::ArrayRef<const Expr *> ExprChain) override { unsigned DiagID = MovedExpr ? diag::warn_lifetime_safety_dangling_global_moved : diag::warn_lifetime_safety_dangling_global; @@ -178,6 +185,9 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { if (MovedExpr) S.Diag(MovedExpr->getExprLoc(), diag::note_lifetime_safety_moved_here) << MovedExpr->getSourceRange(); + + reportAliasingChain(ExprChain); + if (DanglingGlobal->isStaticLocal() || DanglingGlobal->isStaticDataMember()) S.Diag(DanglingGlobal->getLocation(), diag::note_lifetime_safety_dangling_static_here) @@ -223,13 +233,15 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { void reportInvalidatedField(const Expr *IssueExpr, const FieldDecl *DanglingField, - const Expr *InvalidationExpr) override { + const Expr *InvalidationExpr, + llvm::ArrayRef<const Expr *> ExprChain) override { std::string InvalidatedSubject = getDiagSubjectDescription(IssueExpr); S.Diag(IssueExpr->getExprLoc(), diag::warn_lifetime_safety_invalidated_field) << InvalidatedSubject << getDiagSubjectDescription(DanglingField) << IssueExpr->getSourceRange(); reportInvalidationSite(InvalidationExpr, InvalidatedSubject); + reportAliasingChain(ExprChain); S.Diag(DanglingField->getLocation(), diag::note_lifetime_safety_dangling_field_here) << DanglingField->getEndLoc(); @@ -237,27 +249,31 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { void reportInvalidatedField(const ParmVarDecl *PVD, const FieldDecl *DanglingField, - const Expr *InvalidationExpr) override { + const Expr *InvalidationExpr, + llvm::ArrayRef<const Expr *> ExprChain) override { std::string InvalidatedSubject = getDiagSubjectDescription(PVD); S.Diag(PVD->getSourceRange().getBegin(), diag::warn_lifetime_safety_invalidated_field) << InvalidatedSubject << getDiagSubjectDescription(DanglingField) << PVD->getSourceRange(); reportInvalidationSite(InvalidationExpr, InvalidatedSubject); + reportAliasingChain(ExprChain); S.Diag(DanglingField->getLocation(), diag::note_lifetime_safety_dangling_field_here) << DanglingField->getEndLoc(); } - void reportInvalidatedGlobal(const Expr *IssueExpr, - const VarDecl *DanglingGlobal, - const Expr *InvalidationExpr) override { + void + reportInvalidatedGlobal(const Expr *IssueExpr, const VarDecl *DanglingGlobal, + const Expr *InvalidationExpr, + llvm::ArrayRef<const Expr *> ExprChain) override { std::string InvalidatedSubject = getDiagSubjectDescription(IssueExpr); S.Diag(IssueExpr->getExprLoc(), diag::warn_lifetime_safety_invalidated_global) << InvalidatedSubject << getDiagSubjectDescription(DanglingGlobal) << IssueExpr->getSourceRange(); reportInvalidationSite(InvalidationExpr, InvalidatedSubject); + reportAliasingChain(ExprChain); if (DanglingGlobal->isStaticLocal() || DanglingGlobal->isStaticDataMember()) S.Diag(DanglingGlobal->getLocation(), diag::note_lifetime_safety_dangling_static_here) @@ -268,15 +284,17 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { << DanglingGlobal->getEndLoc(); } - void reportInvalidatedGlobal(const ParmVarDecl *PVD, - const VarDecl *DanglingGlobal, - const Expr *InvalidationExpr) override { + void + reportInvalidatedGlobal(const ParmVarDecl *PVD, const VarDecl *DanglingGlobal, + const Expr *InvalidationExpr, + llvm::ArrayRef<const Expr *> ExprChain) override { std::string InvalidatedSubject = getDiagSubjectDescription(PVD); S.Diag(PVD->getSourceRange().getBegin(), diag::warn_lifetime_safety_invalidated_global) << InvalidatedSubject << getDiagSubjectDescription(DanglingGlobal) << PVD->getSourceRange(); reportInvalidationSite(InvalidationExpr, InvalidatedSubject); + reportAliasingChain(ExprChain); if (DanglingGlobal->isStaticLocal() || DanglingGlobal->isStaticDataMember()) S.Diag(DanglingGlobal->getLocation(), diag::note_lifetime_safety_dangling_static_here) diff --git a/clang/test/Sema/LifetimeSafety/annotation-suggestions.cpp b/clang/test/Sema/LifetimeSafety/annotation-suggestions.cpp index 56ed4a6dc7106..3497598196ee5 100644 --- a/clang/test/Sema/LifetimeSafety/annotation-suggestions.cpp +++ b/clang/test/Sema/LifetimeSafety/annotation-suggestions.cpp @@ -528,7 +528,8 @@ struct CaptureRefToView { CaptureRefToView test_ref_to_view() { MyObj obj; CaptureRefToView x(obj); // expected-warning {{stack memory associated with local variable 'obj' is returned}} - return x; // expected-note {{returned here}} + return x; // expected-note {{returned here}} \ + // expected-note {{local variable 'x' aliases the storage of local variable 'obj'}} } struct CaptureRefToPtr { @@ -539,7 +540,8 @@ struct CaptureRefToPtr { CaptureRefToPtr test_ref_to_ptr() { MyObj obj; CaptureRefToPtr x(obj); // expected-warning {{stack memory associated with local variable 'obj' is returned}} - return x; // expected-note {{returned here}} + return x; // expected-note {{returned here}} \ + // expected-note {{local variable 'x' aliases the storage of local variable 'obj'}} } struct CaptureViewToView { @@ -550,8 +552,9 @@ struct CaptureViewToView { CaptureViewToView test_view_to_view() { MyObj obj; View v(obj); // expected-warning {{stack memory associated with local variable 'obj' is returned}} - CaptureViewToView x(v); - return x; // expected-note {{returned here}} + CaptureViewToView x(v); // expected-note {{local variable 'v' aliases the storage of local variable 'obj'}} + return x; // expected-note {{returned here}} \ + // expected-note {{local variable 'x' aliases the storage of local variable 'obj'}} } struct CapturePtrToPtr { @@ -562,7 +565,8 @@ struct CapturePtrToPtr { CapturePtrToPtr test_ptr_to_ptr() { MyObj obj; CapturePtrToPtr x(&obj); // expected-warning {{stack memory associated with local variable 'obj' is returned}} - return x; // expected-note {{returned here}} + return x; // expected-note {{returned here}} \ + // expected-note {{local variable 'x' aliases the storage of local variable 'obj'}} } struct CaptureRefToRef { @@ -573,7 +577,8 @@ struct CaptureRefToRef { CaptureRefToRef test_ref_to_ref() { MyObj obj; CaptureRefToRef x(obj); // expected-warning {{stack memory associated with local variable 'obj' is returned}} - return x; // expected-note {{returned here}} + return x; // expected-note {{returned here}} \ + // expected-note {{local variable 'x' aliases the storage of local variable 'obj'}} } struct BaseWithView { @@ -588,7 +593,8 @@ struct CaptureRefToBaseView : BaseWithView { CaptureRefToBaseView test_ref_to_base_view() { MyObj obj; CaptureRefToBaseView x(obj); // expected-warning {{stack memory associated with local variable 'obj' is returned}} - return x; // expected-note {{returned here}} + return x; // expected-note {{returned here}} \ + // expected-note {{local variable 'x' aliases the storage of local variable 'obj'}} } } // namespace capturing_constructor @@ -660,7 +666,8 @@ struct HasCtorField { HasCtorField test_dangling_field_ctor() { MyObj obj; HasCtorField x(obj); // expected-warning {{stack memory associated with local variable 'obj' is returned}} - return x; // expected-note {{returned here}} + return x; // expected-note {{returned here}} \ + // expected-note {{local variable 'x' aliases the storage of local variable 'obj'}} } struct HasSetterField { diff --git a/clang/test/Sema/LifetimeSafety/dangling-field.cpp b/clang/test/Sema/LifetimeSafety/dangling-field.cpp index bc73c4f7e8644..d3190d990d7a1 100644... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/212083 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
