llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-analysis Author: Abhinav Pradeep (AbhinavPradeep) <details> <summary>Changes</summary> This PR allows for modelling escape of parameters to global storage, and dangling global storage. Change summary: 1. Created `GlobalEscapeFact` as a subclass of `OriginEscapesFact` 2. Emit a `GlobalEscapeFact` for all origins with global-storage that remain live at function exit. 3. Integrated into warning reporting as necessary, introducing the groups `-Wlifetime-safety-dangling-global` and `-Wlifetime-safety-dangling-global-moved` 4. Wrote sema tests for escape to a variety of global storage locations. --- Full diff: https://github.com/llvm/llvm-project/pull/181646.diff 10 Files Affected: - (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h (+21-1) - (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h (+9) - (modified) clang/include/clang/Basic/DiagnosticGroups.td (+17-1) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+12) - (modified) clang/lib/Analysis/LifetimeSafety/Checker.cpp (+10-1) - (modified) clang/lib/Analysis/LifetimeSafety/Facts.cpp (+8) - (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+10-1) - (modified) clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp (+2) - (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+28) - (modified) clang/test/Sema/warn-lifetime-safety-noescape.cpp (+22-3) ``````````diff diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index f9d55991f2e09..e4fbe07b3e9bd 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -14,6 +14,7 @@ #ifndef LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_FACTS_H #define LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_FACTS_H +#include "clang/AST/Decl.h" #include "clang/Analysis/Analyses/LifetimeSafety/Loans.h" #include "clang/Analysis/Analyses/LifetimeSafety/Origins.h" #include "clang/Analysis/Analyses/LifetimeSafety/Utils.h" @@ -151,7 +152,7 @@ class OriginEscapesFact : public Fact { enum class EscapeKind : uint8_t { Return, /// Escapes via return statement. Field, /// Escapes via assignment to a field. - // FIXME: Add support for escape to global (dangling global ptr). + Global, /// Escapes via assignment to global storage. } EscKind; static bool classof(const Fact *F) { @@ -202,6 +203,25 @@ class FieldEscapeFact : public OriginEscapesFact { const OriginManager &OM) const override; }; +/// Represents that an origin escapes via assignment to global storage. +/// Example: `global_storage = local_var;` +class GlobalEscapeFact : public OriginEscapesFact { + const VarDecl *Global; + +public: + GlobalEscapeFact(OriginID OID, const VarDecl *VDecl) + : OriginEscapesFact(OID, EscapeKind::Global), Global(VDecl) {} + + static bool classof(const Fact *F) { + return F->getKind() == Kind::OriginEscapes && + static_cast<const OriginEscapesFact *>(F)->getEscapeKind() == + EscapeKind::Global; + } + const VarDecl *getGlobal() const { return Global; }; + 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 d7aadf4cf04ca..2bcd0ddd7a16c 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -84,6 +84,11 @@ class LifetimeSafetySemaHelper { const Expr *MovedExpr, SourceLocation ExpiryLoc) {} + virtual void reportDanglingGlobal(const Expr *IssueExpr, + const VarDecl *DanglingGlobal, + const Expr *MovedExpr, + SourceLocation ExpiryLoc) {} + // Reports when a reference/iterator is used after the container operation // that invalidated it. virtual void reportUseAfterInvalidation(const Expr *IssueExpr, @@ -104,6 +109,10 @@ class LifetimeSafetySemaHelper { // Reports misuse of [[clang::noescape]] when parameter escapes through field virtual void reportNoescapeViolation(const ParmVarDecl *ParmWithNoescape, const FieldDecl *EscapeField) {} + // Reports misuse of [[clang::noescape]] when parameter escapes through + // assignment to a global variable + virtual void reportNoescapeViolation(const ParmVarDecl *ParmWithNoescape, + const VarDecl *EscapeGlobal) {} // Suggests lifetime bound annotations for implicit this. virtual void suggestLifetimeboundToImplicitThis(SuggestionScope Scope, diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 7df83d2a4011f..72a50f4141e27 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -595,6 +595,19 @@ This may contain false-positives, e.g. when the borrowed storage is potentially }]; } +def LifetimeSafetyDanglingGlobal : DiagGroup<"lifetime-safety-dangling-global"> { + code Documentation = [{ +Warning to detect dangling global references. + }]; +} + +def LifetimeSafetyDanglingGlobalMoved : DiagGroup<"lifetime-safety-dangling-global-moved"> { + code Documentation = [{ +Warning to detect dangling global references. +This may contain false-positives, e.g. when the borrowed storage is potentially moved and is not destroyed at function exit. + }]; +} + def LifetimeSafetyInvalidation : DiagGroup<"lifetime-safety-invalidation"> { code Documentation = [{ Warning to detect invalidation of references. @@ -604,11 +617,14 @@ Warning to detect invalidation of references. def LifetimeSafetyPermissive : DiagGroup<"lifetime-safety-permissive", [LifetimeSafetyUseAfterScope, LifetimeSafetyReturnStackAddr, - LifetimeSafetyDanglingField]>; + LifetimeSafetyDanglingField, + LifetimeSafetyDanglingGlobal]>; + def LifetimeSafetyStrict : DiagGroup<"lifetime-safety-strict", [LifetimeSafetyUseAfterScopeMoved, LifetimeSafetyReturnStackAddrMoved, LifetimeSafetyDanglingFieldMoved, + LifetimeSafetyDanglingGlobal, LifetimeSafetyInvalidation]>; def LifetimeSafety : DiagGroup<"lifetime-safety", diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 68016ec4d58a3..b3c44eb1a120a 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10931,6 +10931,16 @@ def warn_lifetime_safety_dangling_field_moved "Consider moving first and then aliasing later to resolve the issue">, InGroup<LifetimeSafetyDanglingFieldMoved>, DefaultIgnore; +def warn_lifetime_safety_dangling_global + : Warning<"address of stack memory escapes to a global">, + InGroup<LifetimeSafetyDanglingGlobal>, + DefaultIgnore; +def warn_lifetime_safety_dangling_global_moved + : Warning<"address of stack memory escapes to a global. " + "This could be a false positive as the storage may have been moved. " + "Consider moving first and then aliasing later to resolve the issue">, + InGroup<LifetimeSafetyDanglingGlobalMoved>, + DefaultIgnore; def note_lifetime_safety_used_here : Note<"later used here">; def note_lifetime_safety_invalidated_here : Note<"invalidated here">; @@ -10938,7 +10948,9 @@ def note_lifetime_safety_destroyed_here : Note<"destroyed here">; def note_lifetime_safety_returned_here : Note<"returned here">; def note_lifetime_safety_moved_here : Note<"potentially moved here">; def note_lifetime_safety_dangling_field_here: Note<"this field dangles">; +def note_lifetime_safety_dangling_global_here: Note<"this global dangles">; def note_lifetime_safety_escapes_to_field_here: Note<"escapes to this field">; +def note_lifetime_safety_escapes_to_global_here: Note<"escapes to this global storage">; def warn_lifetime_safety_intra_tu_param_suggestion : Warning<"parameter in intra-TU function should be marked " diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 78c2a6dba3eb6..7399fa1c2dbd2 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -25,6 +25,7 @@ #include "clang/Basic/SourceManager.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/Support/Casting.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/TimeProfiler.h" @@ -55,7 +56,8 @@ struct PendingWarning { using AnnotationTarget = llvm::PointerUnion<const ParmVarDecl *, const CXXMethodDecl *>; -using EscapingTarget = llvm::PointerUnion<const Expr *, const FieldDecl *>; +using EscapingTarget = + llvm::PointerUnion<const Expr *, const FieldDecl *, const VarDecl *>; class LifetimeChecker { private: @@ -109,6 +111,8 @@ class LifetimeChecker { NoescapeWarningsMap.try_emplace(PVD, ReturnEsc->getReturnExpr()); if (auto *FieldEsc = dyn_cast<FieldEscapeFact>(OEF)) NoescapeWarningsMap.try_emplace(PVD, FieldEsc->getFieldDecl()); + if (auto *GlobalEsc = dyn_cast<GlobalEscapeFact>(OEF)) + NoescapeWarningsMap.try_emplace(PVD, GlobalEsc->getGlobal()); return; } // Suggest lifetimebound for parameter escaping through return. @@ -270,6 +274,9 @@ class LifetimeChecker { else if (const auto *FieldEscape = dyn_cast<FieldEscapeFact>(OEF)) SemaHelper->reportDanglingField( IssueExpr, FieldEscape->getFieldDecl(), MovedExpr, ExpiryLoc); + else if (const auto *GlobalEscape = dyn_cast<GlobalEscapeFact>(OEF)) + SemaHelper->reportDanglingGlobal(IssueExpr, GlobalEscape->getGlobal(), + MovedExpr, ExpiryLoc); else llvm_unreachable("Unhandled OriginEscapesFact type"); } else @@ -342,6 +349,8 @@ class LifetimeChecker { SemaHelper->reportNoescapeViolation(PVD, E); else if (const auto *FD = EscapeTarget.dyn_cast<const FieldDecl *>()) SemaHelper->reportNoescapeViolation(PVD, FD); + else if (const auto *G = EscapeTarget.dyn_cast<const VarDecl *>()) + SemaHelper->reportNoescapeViolation(PVD, G); else llvm_unreachable("Unhandled EscapingTarget type"); } diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index c963d9c45fa9d..4ffc8b4195949 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -8,6 +8,7 @@ #include "clang/Analysis/Analyses/LifetimeSafety/Facts.h" #include "clang/AST/Decl.h" +#include "clang/AST/DeclID.h" #include "clang/Analysis/Analyses/PostOrderCFGView.h" #include "clang/Analysis/FlowSensitive/DataflowWorklist.h" #include "llvm/ADT/STLFunctionalExtras.h" @@ -68,6 +69,13 @@ void FieldEscapeFact::dump(llvm::raw_ostream &OS, const LoanManager &, OS << ", via Field)\n"; } +void GlobalEscapeFact::dump(llvm::raw_ostream &OS, const LoanManager &, + const OriginManager &OM) const { + OS << "OriginEscapes ("; + OM.dump(getEscapedOriginID(), OS); + OS << ", via Global)\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 f39d677758393..8238cf69edfcd 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -9,6 +9,7 @@ #include <cassert> #include <string> +#include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" @@ -469,11 +470,19 @@ void FactsGenerator::handleFullExprCleanup( } void FactsGenerator::handleExitBlock() { - // Creates FieldEscapeFacts for all field origins that remain live at exit. for (const Origin &O : FactMgr.getOriginMgr().getOrigins()) if (auto *FD = dyn_cast_if_present<FieldDecl>(O.getDecl())) + // Create FieldEscapeFacts for all field origins that remain live at exit. EscapesInCurrentBlock.push_back( FactMgr.createFact<FieldEscapeFact>(O.ID, FD)); + else if (auto *VD = dyn_cast_if_present<VarDecl>(O.getDecl())) { + // Create GlobalEscapeFacts for all origins with global-storage that + // remain live at exit. + if (VD->hasGlobalStorage()) { + EscapesInCurrentBlock.push_back( + FactMgr.createFact<GlobalEscapeFact>(O.ID, VD)); + } + } } void FactsGenerator::handleGSLPointerConstruction(const CXXConstructExpr *CCE) { diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index f210fb4d752d4..fe20d779669c1 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -62,6 +62,8 @@ static SourceLocation GetFactLoc(CausingFactType F) { return ReturnEsc->getReturnExpr()->getExprLoc(); if (auto *FieldEsc = dyn_cast<FieldEscapeFact>(OEF)) return FieldEsc->getFieldDecl()->getLocation(); + if (auto *GlobalEsc = dyn_cast<GlobalEscapeFact>(OEF)) + return GlobalEsc->getGlobal()->getLocation(); } llvm_unreachable("unhandled causing fact in PointerUnion"); } diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 20c41096501fb..9769a6d678dcd 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2888,6 +2888,7 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { S.Diag(ReturnExpr->getExprLoc(), diag::note_lifetime_safety_returned_here) << ReturnExpr->getSourceRange(); } + void reportDanglingField(const Expr *IssueExpr, const FieldDecl *DanglingField, const Expr *MovedExpr, @@ -2904,6 +2905,22 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { << DanglingField->getEndLoc(); } + void reportDanglingGlobal(const Expr *IssueExpr, + const VarDecl *DanglingGlobal, + const Expr *MovedExpr, + SourceLocation ExpiryLoc) override { + S.Diag(IssueExpr->getExprLoc(), + MovedExpr ? diag::warn_lifetime_safety_dangling_global_moved + : diag::warn_lifetime_safety_dangling_global) + << IssueExpr->getSourceRange(); + if (MovedExpr) + S.Diag(MovedExpr->getExprLoc(), diag::note_lifetime_safety_moved_here) + << MovedExpr->getSourceRange(); + S.Diag(DanglingGlobal->getLocation(), + diag::note_lifetime_safety_dangling_global_here) + << DanglingGlobal->getEndLoc(); + } + void reportUseAfterInvalidation(const Expr *IssueExpr, const Expr *UseExpr, const Expr *InvalidationExpr) override { S.Diag(IssueExpr->getExprLoc(), diag::warn_lifetime_safety_invalidation) @@ -3005,6 +3022,17 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { << EscapeField->getEndLoc(); } + void reportNoescapeViolation(const ParmVarDecl *ParmWithNoescape, + const VarDecl *EscapeGlobal) override { + S.Diag(ParmWithNoescape->getBeginLoc(), + diag::warn_lifetime_safety_noescape_escapes) + << ParmWithNoescape->getSourceRange(); + + S.Diag(EscapeGlobal->getLocation(), + diag::note_lifetime_safety_escapes_to_global_here) + << EscapeGlobal->getEndLoc(); + } + void addLifetimeBoundToImplicitThis(const CXXMethodDecl *MD) override { S.addLifetimeBoundToImplicitThis(const_cast<CXXMethodDecl *>(MD)); } diff --git a/clang/test/Sema/warn-lifetime-safety-noescape.cpp b/clang/test/Sema/warn-lifetime-safety-noescape.cpp index ee661add0acc8..03f4c24a0956f 100644 --- a/clang/test/Sema/warn-lifetime-safety-noescape.cpp +++ b/clang/test/Sema/warn-lifetime-safety-noescape.cpp @@ -113,12 +113,31 @@ View escape_through_unannotated_call(const MyObj& in [[clang::noescape]]) { // e return no_annotation_identity(in); // expected-note {{returned here}} } -View global_view; +View global_view; // expected-note {{escapes to this global storage}} -// FIXME: Escaping through a global variable is not detected. -void escape_through_global_var(const MyObj& in [[clang::noescape]]) { +void escape_through_global_var(const MyObj& in [[clang::noescape]]) { // expected-warning {{parameter is marked [[clang::noescape]] but escapes}} global_view = in; } +struct ObjWithStaticField { + static int *static_field; // expected-note {{escapes to this global storage}} +}; + +void escape_to_static_data_member(int *data [[clang::noescape]]) { // expected-warning {{parameter is marked [[clang::noescape]] but escapes}} + ObjWithStaticField::static_field = data; +} + + + +void escape_through_static_local(int *data [[clang::noescape]]) { // expected-warning {{parameter is marked [[clang::noescape]] but escapes}} + static int *static_local; // expected-note {{escapes to this global storage}} + static_local = data; +} + +thread_local int *thread_local_storage; // expected-note {{escapes to this global storage}} + +void escape_through_thread_local(int *data [[clang::noescape]]) { // expected-warning {{parameter is marked [[clang::noescape]] but escapes}} + thread_local_storage = data; +} struct ObjConsumer { void escape_through_member(const MyObj& in [[clang::noescape]]) { // expected-warning {{parameter is marked [[clang::noescape]] but escapes}} `````````` </details> https://github.com/llvm/llvm-project/pull/181646 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
