https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/124133
>From 22990789b61e9f9d22e88a6b008eb3166fd1cb56 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Thu, 23 Jan 2025 15:47:39 +0000 Subject: [PATCH 1/3] [experimental] Detect return-stack-addr using CFG --- .../Analysis/Analyses/DanglingReference.h | 14 + clang/include/clang/Basic/DiagnosticGroups.td | 3 + .../clang/Basic/DiagnosticSemaKinds.td | 8 + clang/lib/Analysis/CMakeLists.txt | 1 + clang/lib/Analysis/DanglingReference.cpp | 351 ++++++++++++++++++ clang/lib/Sema/AnalysisBasedWarnings.cpp | 8 + .../test/Sema/warn-lifetime-analysis-cfg.cpp | 136 +++++++ 7 files changed, 521 insertions(+) create mode 100644 clang/include/clang/Analysis/Analyses/DanglingReference.h create mode 100644 clang/lib/Analysis/DanglingReference.cpp create mode 100644 clang/test/Sema/warn-lifetime-analysis-cfg.cpp diff --git a/clang/include/clang/Analysis/Analyses/DanglingReference.h b/clang/include/clang/Analysis/Analyses/DanglingReference.h new file mode 100644 index 000000000000000..c9f5753eed070e6 --- /dev/null +++ b/clang/include/clang/Analysis/Analyses/DanglingReference.h @@ -0,0 +1,14 @@ +#ifndef LLVM_CLANG_ANALYSIS_ANALYSES_DANGLING_REFERENCE_H +#define LLVM_CLANG_ANALYSIS_ANALYSES_DANGLING_REFERENCE_H +#include "clang/AST/DeclBase.h" +#include "clang/Analysis/AnalysisDeclContext.h" +#include "clang/Analysis/CFG.h" +#include "clang/Sema/Sema.h" + +namespace clang { +void runDanglingReferenceAnalysis(const DeclContext &dc, const CFG &cfg, + AnalysisDeclContext &ac, Sema &S); + +} // namespace clang + +#endif // LLVM_CLANG_ANALYSIS_ANALYSES_DANGLING_REFERENCE_H diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 594e99a19b64d61..eeddd6eb82a3019 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -472,6 +472,9 @@ def Dangling : DiagGroup<"dangling", [DanglingAssignment, DanglingInitializerList, DanglingGsl, ReturnStackAddress]>; +def ReturnStackAddressCFG : DiagGroup<"return-stack-address-cfg">; +def DanglingCFG : DiagGroup<"dangling-cfg", [ReturnStackAddressCFG]>; + def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">; def DllexportExplicitInstantiationDecl : DiagGroup<"dllexport-explicit-instantiation-decl">; def ExcessInitializers : DiagGroup<"excess-initializers">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 8be4f946dce1ccb..846cf6b3d45f8a8 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10169,6 +10169,14 @@ def err_lifetimebound_implicit_object_parameter_void_return_type : Error< "parameter of a function that returns void; " "did you mean 'lifetime_capture_by(X)'">; +// CFG based lifetime analysis. +def warn_ret_stack_variable_ref_cfg : Warning< + "returning reference to a stack variable">, InGroup<ReturnStackAddressCFG>, DefaultIgnore; +def note_local_variable_declared_here : Note<"reference to this stack variable is returned">; + +def warn_ret_stack_temporary_ref_cfg : Warning< + "returning reference to a temporary object">, InGroup<ReturnStackAddressCFG>, DefaultIgnore; + // CHECK: returning address/reference of stack memory def warn_ret_stack_addr_ref : Warning< "%select{address of|reference to}0 stack memory associated with " diff --git a/clang/lib/Analysis/CMakeLists.txt b/clang/lib/Analysis/CMakeLists.txt index 7914c12d429ef95..d6ea1e907e7f09b 100644 --- a/clang/lib/Analysis/CMakeLists.txt +++ b/clang/lib/Analysis/CMakeLists.txt @@ -16,6 +16,7 @@ add_clang_library(clangAnalysis ConstructionContext.cpp Consumed.cpp CodeInjector.cpp + DanglingReference.cpp Dominators.cpp ExprMutationAnalyzer.cpp IntervalPartition.cpp diff --git a/clang/lib/Analysis/DanglingReference.cpp b/clang/lib/Analysis/DanglingReference.cpp new file mode 100644 index 000000000000000..2602cc597a36b86 --- /dev/null +++ b/clang/lib/Analysis/DanglingReference.cpp @@ -0,0 +1,351 @@ +#include "clang/Analysis/Analyses/DanglingReference.h" +#include "clang/AST/Attrs.inc" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclVisitor.h" +#include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" +#include "clang/AST/Stmt.h" +#include "clang/AST/StmtVisitor.h" +#include "clang/AST/Type.h" +#include "clang/Analysis/CFG.h" +#include "clang/Basic/DiagnosticSema.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/Support/raw_ostream.h" +#include <sstream> + +namespace clang { +namespace { + +template <typename T> static bool isRecordWithAttr(QualType Type) { + auto *RD = Type->getAsCXXRecordDecl(); + if (!RD) + return false; + bool Result = RD->hasAttr<T>(); + + if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) + Result |= CTSD->getSpecializedTemplate()->getTemplatedDecl()->hasAttr<T>(); + + return Result; +} +bool isOwner(const Expr *E) { + return isRecordWithAttr<OwnerAttr>(E->getType()); +} +bool isOwner(const Decl *D) { + return isa<ValueDecl>(D) && + isRecordWithAttr<OwnerAttr>(dyn_cast<ValueDecl>(D)->getType()); +} +bool isPointer(const Expr *E) { + return isRecordWithAttr<PointerAttr>(E->getType()); +} +bool isPointer(const Decl *D) { + return isa<ValueDecl>(D) && + isRecordWithAttr<PointerAttr>(dyn_cast<ValueDecl>(D)->getType()); +} + +struct MemoryLoc { + enum MemoryType { + EMPTY, // Pointer is null. + STACK, // Pointer points to something on stack. + UNKNOWN, // Pointer points to an unknown entity. + } Loc; + // Details of stack location. + const Decl *D = nullptr; + const Expr *MTE = nullptr; + + bool IsEmpty() { return Loc == EMPTY; } + bool IsOnStack() { return Loc == STACK; } + bool IsUnkown() { return Loc == UNKNOWN; } + + const Decl *getDecl() { return D; } + const Expr *getExpr() { return MTE; } + + static MemoryLoc Unknown() { return {UNKNOWN, nullptr, nullptr}; } + static MemoryLoc Empty() { return {EMPTY, nullptr, nullptr}; } + static MemoryLoc VarOnStack(const Decl *D) { return {STACK, D, nullptr}; } + static MemoryLoc Temporary(const Expr *MTE) { return {STACK, nullptr, MTE}; } + + std::string str() { + std::ostringstream os; + switch (Loc) { + case EMPTY: + os << "Empty"; + break; + case UNKNOWN: + os << "Unknown"; + break; + case STACK: + os << "Stack"; + if (auto *VD = dyn_cast_or_null<VarDecl>(D)) + os << " \"" << VD->getName().str() << "\""; + if (MTE) + os << " (temporary)"; + break; + } + return os.str(); + } +}; + +class PointsToTracker : public ConstStmtVisitor<PointsToTracker> { +public: + void Handle(const Stmt *S) { + if (auto *E = dyn_cast<Expr>(S); + E && ExprPointsTo.find(E) != ExprPointsTo.end()) + return; + Visit(S); + } + + void MaybeInitaliseDecl(const Decl *D) { + if (!D) + return; + auto *VD = dyn_cast<VarDecl>(D); + if (!VD) + return; + // Initialise the pointer if we are seeing it for the first time. + if (isPointer(VD)) { + if (DeclPointsTo.find(D) == DeclPointsTo.end()) + UpdatePointer(VD, ResolveExpr(VD->getInit())); + } + if (isOwner(VD)) { + if (StackDecls.find(D) == StackDecls.end() && VD->hasLocalStorage()) + AddToStack(VD); + } + } + + // Merge above and below in VisitVarDecl !! + void VisitDeclStmt(const DeclStmt *DS) { + MaybeInitaliseDecl(DS->getSingleDecl()); + } + + void VisitDeclRefExpr(const DeclRefExpr *DRE) { + SetExprPointer(DRE, DRE->getDecl()); + } + + void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *MTE) { + // Ignore MTE of pointer types. + if (isPointer(MTE)) { + Handle(MTE->getSubExpr()); + SetExprPointer(MTE, MTE->getSubExpr()); + } + if (isOwner(MTE)) { + // We have a temporary owner on stack. + AddToStack(MTE); + } + } + + void VisitImplicitCastExpr(const ImplicitCastExpr *E) { + auto *SE = E->IgnoreImpCasts(); + Handle(SE); + SetExprPointer(E, SE); + } + + void VisitExprWithCleanups(const ExprWithCleanups *E) { + // Handle(E->getSubExpr()); + SetExprPointer(E, E->getSubExpr()); + } + + void VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) { + // Conversion from Owner to a Pointer. + const Expr *ConversionFrom = MCE->IgnoreConversionOperatorSingleStep(); + if (ConversionFrom != MCE) { + if (isOwner(ConversionFrom) && isPointer(MCE)) { + SetExprPointer(MCE, ConversionFrom); + } + } + } + + void VisitCXXConstructExpr(const CXXConstructExpr *CCE) { + if (!isPointer(CCE)) + return; + if (CCE->getNumArgs() == 1) + SetExprPointer(CCE, CCE->getArg(0)); + } + + void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) { + if (OCE->isAssignmentOp()) { + assert(OCE->getNumArgs() == 2); + Handle(OCE->getArg(0)); + Handle(OCE->getArg(1)); + HandleAssignment(OCE->getArg(0), OCE->getArg(1)); + } + } + +private: + // Returns the Decl that is aliased by this expression. + const Decl *DeclReferencedBy(const Expr *E) { + if (auto *DRE = dyn_cast<DeclRefExpr>(E)) { + return DRE->getDecl(); + } + return nullptr; + } + + void HandleAssignment(const Expr *A, const Expr *B) { + if (!isPointer(A)) + return; + + if (const Decl *PointerDecl = DeclReferencedBy(A)) + UpdatePointer(PointerDecl, B); + } + + // Update the contents of a Pointer. + void UpdatePointer(const Decl *PointerD, MemoryLoc ML) { + assert(isPointer(PointerD)); + DeclPointsTo[PointerD] = ML; + } + void UpdatePointer(const Decl *PointerD, const Expr *A) { + UpdatePointer(PointerD, ResolveExpr(A)); + } + + void SetExprPointer(const Expr *E, MemoryLoc ML) { + assert(ExprPointsTo.insert({E, ML}).second); + } + void SetExprPointer(const Expr *E, const Expr *PointeeE) { + SetExprPointer(E, ResolveExpr(PointeeE)); + } + void SetExprPointer(const Expr *E, const Decl *D) { + SetExprPointer(E, ResolveDecl(D)); + } + +public: + MemoryLoc ResolveExpr(const Expr *E) { + if (!E) + return MemoryLoc::Empty(); + Handle(E); + auto Res = ExprPointsTo.find(E); + if (Res != ExprPointsTo.end()) { + return Res->getSecond(); + } + return ExprPointsTo[E] = MemoryLoc::Unknown(); + } + + // Returns the memory location pointed to by D. If D is a pointer-type, + // returns the memory pointed to by the pointer. + MemoryLoc ResolveDecl(const Decl *D) { + MaybeInitaliseDecl(D); + if (isPointer(D)) + return ResolvePointer(D); + if (isOwner(D)) + return ResolveOwner(D); + return MemoryLoc::Unknown(); + } + + MemoryLoc ResolvePointer(const Decl *D) { + auto *VD = dyn_cast<VarDecl>(D); + assert(VD); + if (!VD->hasLocalStorage()) + return MemoryLoc::Unknown(); + assert(isPointer(D)); + auto Res = DeclPointsTo.find(D); + assert(Res != DeclPointsTo.end()); + return Res->getSecond(); + } + + MemoryLoc ResolveOwner(const Decl *D) { + assert(isOwner(D)); + if (IsOnStack(D)) + return MemoryLoc::VarOnStack(D); + return MemoryLoc::Unknown(); + } + + void AddToStack(const Decl *D) { + assert(isOwner(D)); + StackDecls.insert(D); + } + void AddToStack(const Expr *E) { + assert(isa<MaterializeTemporaryExpr>(E)); + assert(isOwner(E)); + StackExprs.insert(E); + // Add a self edge. + assert(ExprPointsTo.insert({E, MemoryLoc::Temporary(E)}).second); + } + bool IsOnStack(const Decl *D) { return StackDecls.contains(D); } + bool IsOnStack(const Expr *E) { return StackExprs.contains(E); } + + // ExpressionResolver &ExprResolver; + // Map from an expression of View type to its pointee and Owner type to the + // reference<TODO simplify>. This should follow single assignment because + // Expr* cannot be reassigned in the program. + llvm::DenseMap<const Expr *, MemoryLoc> ExprPointsTo; + // Map from a decl of View type to it pointee. This can be reassigned at + // various points in the program due to transfer functions. + llvm::DenseMap<const Decl *, MemoryLoc> DeclPointsTo; + + // Collection of Expr* and Decl* stored on stack. + llvm::DenseSet<const Expr *> StackExprs; + llvm::DenseSet<const Decl *> StackDecls; +}; +class DanglingReferenceAnalyzer { +public: + DanglingReferenceAnalyzer(const DeclContext &DC, const CFG &cfg, + AnalysisDeclContext &AC, Sema &S) + : DC(DC), cfg(cfg), AC(AC), S(S) {} + void RunAnalysis() { + + // For simplicity in protoytpe, avoid joins and stick to functions without + // branches. + if (!cfg.isLinear()) + return; + cfg.dump(AC.getASTContext().getLangOpts(), true); + for (auto I = cfg.begin(), E = cfg.end(); I != E; ++I) { + for (CFGBlock::const_iterator BI = (*I)->begin(), BE = (*I)->end(); + BI != BE; ++BI) { + if (auto cfgstmt = BI->getAs<CFGStmt>()) { + auto *stmt = cfgstmt->getStmt(); + llvm::errs() << "================================================\n"; + cfgstmt->dump(); + if (auto *E = dyn_cast<Expr>(stmt)) + E->dumpColor(); + PointsTo.Handle(stmt); + if (auto *E = dyn_cast<Expr>(stmt)) { + llvm::errs() << "Points To : " << PointsTo.ResolveExpr(E).str() + << "\n"; + } + if (auto *RS = dyn_cast<ReturnStmt>(stmt)) + HandleReturnStmt(RS); + } + } + } + } + +private: + void HandleReturnStmt(const ReturnStmt *RS) { + // Diagnose possible return of stack variable. + if (!RS) + return; + const Expr *RetExpr = RS->getRetValue(); + if (!RetExpr || !isPointer(RetExpr)) + return; + PointsTo.Handle(RetExpr); + auto RetPointee = PointsTo.ResolveExpr(RetExpr); + // RetExpr->dumpColor(); + // llvm::errs() << "Returning pointer to " << RetPointee.str() << "\n"; + if (RetPointee.IsOnStack()) { + // This points to something on stack!! + if (auto *D = RetPointee.getDecl()) { + S.Diag(RetExpr->getExprLoc(), diag::warn_ret_stack_variable_ref_cfg) + << RetExpr->getExprLoc(); + S.Diag(D->getLocation(), diag::note_local_variable_declared_here) + << D->getLocation(); + } + if (auto *E = RetPointee.getExpr()) + S.Diag(E->getExprLoc(), diag::warn_ret_stack_temporary_ref_cfg) + << E->getExprLoc(); + } + } + + [[maybe_unused]] const DeclContext &DC; + const CFG &cfg; + [[maybe_unused]] AnalysisDeclContext &AC; + Sema &S; + // ExpressionResolver ExprResolver; + PointsToTracker PointsTo; +}; +} // namespace + +void runDanglingReferenceAnalysis(const DeclContext &DC, const CFG &cfg, + AnalysisDeclContext &AC, Sema &S) { + DanglingReferenceAnalyzer DRA(DC, cfg, AC, S); + DRA.RunAnalysis(); +} + +} // namespace clang diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 589869d0186575b..721d4bdba3db545 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -29,6 +29,7 @@ #include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h" #include "clang/Analysis/Analyses/CalledOnceCheck.h" #include "clang/Analysis/Analyses/Consumed.h" +#include "clang/Analysis/Analyses/DanglingReference.h" #include "clang/Analysis/Analyses/ReachableCode.h" #include "clang/Analysis/Analyses/ThreadSafety.h" #include "clang/Analysis/Analyses/UninitializedValues.h" @@ -2826,6 +2827,13 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( } } + if (S.getLangOpts().CPlusPlus && + !Diags.isIgnored(diag::warn_ret_stack_variable_ref_cfg, + D->getBeginLoc())) { + if (CFG *cfg = AC.getCFG()) { + runDanglingReferenceAnalysis(*cast<DeclContext>(D), *cfg, AC, S); + } + } // Check for violations of "called once" parameter properties. if (S.getLangOpts().ObjC && !S.getLangOpts().CPlusPlus && shouldAnalyzeCalledOnceParameters(Diags, D->getBeginLoc())) { diff --git a/clang/test/Sema/warn-lifetime-analysis-cfg.cpp b/clang/test/Sema/warn-lifetime-analysis-cfg.cpp new file mode 100644 index 000000000000000..e337c75f8344d2f --- /dev/null +++ b/clang/test/Sema/warn-lifetime-analysis-cfg.cpp @@ -0,0 +1,136 @@ +// RUN: %clang_cc1 -Wno-dangling -Wdangling-cfg -verify -std=c++11 %s +#include "Inputs/lifetime-analysis.h" + +std::string_view unknown(std::string_view s); +std::string_view unknown(); +std::string temporary(); + +std::string_view simple_with_ctor() { + std::string s1; // expected-note {{reference to this stack variable is returned}} + std::string_view cs1 = s1; + return cs1; // expected-warning {{returning reference to a stack variable}} +} + +std::string_view simple_with_assignment() { + std::string s1; // expected-note {{reference to this stack variable is returned}} + std::string_view cs1; + cs1 = s1; + return cs1; // expected-warning {{returning reference to a stack variable}} +} + +std::string_view use_unknown() { + std::string s1; + std::string_view cs1 = s1; + cs1 = unknown(); + return cs1; // ok. +} + +std::string_view overwrite_unknown() { + std::string s1; // expected-note {{reference to this stack variable is returned}} + std::string_view cs1 = s1; + cs1 = unknown(); + cs1 = s1; + return cs1; // expected-warning {{returning reference to a stack variable}} +} + +std::string_view overwrite_with_unknown() { + std::string s1; + std::string_view cs1 = s1; + cs1 = s1; + cs1 = unknown(); + return cs1; // Ok. +} + +std::string_view return_unknown() { + std::string s1; + std::string_view cs1 = s1; + return unknown(cs1); +} + +std::string_view multiple_assignments() { + std::string s1; + std::string s2 = s1; // expected-note {{reference to this stack variable is returned}} + std::string_view cs1 = s1; + std::string_view cs2 = s1; + s2 = s1; // Ignore owner transfers. + cs1 = s1; + cs2 = s2; + cs1 = cs2; + cs2 = s1; + return cs1; // expected-warning {{returning reference to a stack variable}} +} + +std::string_view global_view; +std::string_view ignore_global_view() { + std::string s; + global_view = s; // TODO: We can still track writes to static storage! + return global_view; // Ok. +} +std::string global_owner; +std::string_view ignore_global_owner() { + std::string_view sv; + sv = global_owner; + return sv; // Ok. +} + +std::string_view return_temporary() { + return + temporary(); // expected-warning {{returning reference to a temporary object}} +} + +std::string_view store_temporary() { + std::string_view sv = + temporary(); // expected-warning {{returning reference to a temporary object}} + return sv; +} + +std::string_view return_local() { + std::string local; // expected-note {{reference to this stack variable is returned}} + return // expected-warning {{returning reference to a stack variable}} + local; +} + +// Parameters +std::string_view params_owner_and_views( + std::string_view sv, + std::string s) { // expected-note {{reference to this stack variable is returned}} + sv = s; + return sv; // expected-warning {{returning reference to a stack variable}} +} + +std::string_view param_owner(std::string s) { // expected-note {{reference to this stack variable is returned}} + return s; // expected-warning {{returning reference to a stack variable}} +} +std::string_view param_owner_ref(const std::string& s) { + return s; +} + +std::string& get_str_ref(); +std::string_view ref_to_global_str() { + const std::string& s = get_str_ref(); + std::string_view sv = s; + return sv; +} +std::string_view view_to_global_str() { + std::string_view s = get_str_ref(); + std::string_view sv = s; + sv = s; + return sv; +} +std::string_view copy_of_global_str() { + std::string s = get_str_ref(); // expected-note {{reference to this stack variable is returned}} + std::string_view sv = s; + return sv; // expected-warning {{returning reference to a stack variable}} +} + +// TODO: Use lifetimebound in function calls. Use Pointer to Owner of pointer +// std::string_view containerOfString() { +// std::vector<std::string> local; +// return local.at(0); +// } + +// std::string_view containerOfViewMultistmt() { +// std::vector<std::string> local; +// std::string_view sv = local.at(0); +// return sv; +// } >From 46c76ead9a224d5f6e7bc04fafc194f352392151 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Thu, 23 Jan 2025 16:16:23 +0000 Subject: [PATCH 2/3] Move out Sema from Analysis --- .../Analysis/Analyses/DanglingReference.h | 15 +++++-- .../clang/Basic/DiagnosticSemaKinds.td | 17 +++++--- clang/lib/Analysis/DanglingReference.cpp | 41 ++++++++----------- clang/lib/Sema/AnalysisBasedWarnings.cpp | 25 ++++++++++- 4 files changed, 65 insertions(+), 33 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/DanglingReference.h b/clang/include/clang/Analysis/Analyses/DanglingReference.h index c9f5753eed070e6..f49f967686600d2 100644 --- a/clang/include/clang/Analysis/Analyses/DanglingReference.h +++ b/clang/include/clang/Analysis/Analyses/DanglingReference.h @@ -3,11 +3,20 @@ #include "clang/AST/DeclBase.h" #include "clang/Analysis/AnalysisDeclContext.h" #include "clang/Analysis/CFG.h" -#include "clang/Sema/Sema.h" - namespace clang { +class DanglingReferenceReporter { +public: + DanglingReferenceReporter() = default; + virtual ~DanglingReferenceReporter() = default; + + virtual void ReportReturnLocalVar(const Expr *RetExpr, + const Decl *LocalDecl) {} + virtual void ReportReturnTemporaryExpr(const Expr *TemporaryExpr) {} +}; + void runDanglingReferenceAnalysis(const DeclContext &dc, const CFG &cfg, - AnalysisDeclContext &ac, Sema &S); + AnalysisDeclContext &ac, + DanglingReferenceReporter *reporter); } // namespace clang diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 846cf6b3d45f8a8..15f45971b4b2d47 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10170,12 +10170,17 @@ def err_lifetimebound_implicit_object_parameter_void_return_type : Error< "did you mean 'lifetime_capture_by(X)'">; // CFG based lifetime analysis. -def warn_ret_stack_variable_ref_cfg : Warning< - "returning reference to a stack variable">, InGroup<ReturnStackAddressCFG>, DefaultIgnore; -def note_local_variable_declared_here : Note<"reference to this stack variable is returned">; - -def warn_ret_stack_temporary_ref_cfg : Warning< - "returning reference to a temporary object">, InGroup<ReturnStackAddressCFG>, DefaultIgnore; +def warn_ret_stack_variable_ref_cfg + : Warning<"returning reference to a stack variable">, + InGroup<ReturnStackAddressCFG>, + DefaultIgnore; +def note_local_variable_declared_here + : Note<"reference to this stack variable is returned">; + +def warn_ret_stack_temporary_ref_cfg + : Warning<"returning reference to a temporary object">, + InGroup<ReturnStackAddressCFG>, + DefaultIgnore; // CHECK: returning address/reference of stack memory def warn_ret_stack_addr_ref : Warning< diff --git a/clang/lib/Analysis/DanglingReference.cpp b/clang/lib/Analysis/DanglingReference.cpp index 2602cc597a36b86..1ee79d26a843a8f 100644 --- a/clang/lib/Analysis/DanglingReference.cpp +++ b/clang/lib/Analysis/DanglingReference.cpp @@ -1,17 +1,14 @@ #include "clang/Analysis/Analyses/DanglingReference.h" #include "clang/AST/Attrs.inc" #include "clang/AST/Decl.h" -#include "clang/AST/DeclVisitor.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/Stmt.h" #include "clang/AST/StmtVisitor.h" #include "clang/AST/Type.h" #include "clang/Analysis/CFG.h" -#include "clang/Basic/DiagnosticSema.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" -#include "llvm/Support/raw_ostream.h" #include <sstream> namespace clang { @@ -277,28 +274,30 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> { class DanglingReferenceAnalyzer { public: DanglingReferenceAnalyzer(const DeclContext &DC, const CFG &cfg, - AnalysisDeclContext &AC, Sema &S) - : DC(DC), cfg(cfg), AC(AC), S(S) {} + AnalysisDeclContext &AC, + DanglingReferenceReporter *Reporter) + : DC(DC), cfg(cfg), AC(AC), Reporter(Reporter) {} void RunAnalysis() { // For simplicity in protoytpe, avoid joins and stick to functions without // branches. if (!cfg.isLinear()) return; - cfg.dump(AC.getASTContext().getLangOpts(), true); + // cfg.dump(AC.getASTContext().getLangOpts(), true); for (auto I = cfg.begin(), E = cfg.end(); I != E; ++I) { for (CFGBlock::const_iterator BI = (*I)->begin(), BE = (*I)->end(); BI != BE; ++BI) { if (auto cfgstmt = BI->getAs<CFGStmt>()) { auto *stmt = cfgstmt->getStmt(); - llvm::errs() << "================================================\n"; - cfgstmt->dump(); - if (auto *E = dyn_cast<Expr>(stmt)) - E->dumpColor(); + // llvm::errs() << + // "================================================\n"; + // cfgstmt->dump(); + // if (auto *E = dyn_cast<Expr>(stmt)) + // E->dumpColor(); PointsTo.Handle(stmt); if (auto *E = dyn_cast<Expr>(stmt)) { - llvm::errs() << "Points To : " << PointsTo.ResolveExpr(E).str() - << "\n"; + // llvm::errs() << "Points To : " << PointsTo.ResolveExpr(E).str() + // << "\n"; } if (auto *RS = dyn_cast<ReturnStmt>(stmt)) HandleReturnStmt(RS); @@ -321,30 +320,26 @@ class DanglingReferenceAnalyzer { // llvm::errs() << "Returning pointer to " << RetPointee.str() << "\n"; if (RetPointee.IsOnStack()) { // This points to something on stack!! - if (auto *D = RetPointee.getDecl()) { - S.Diag(RetExpr->getExprLoc(), diag::warn_ret_stack_variable_ref_cfg) - << RetExpr->getExprLoc(); - S.Diag(D->getLocation(), diag::note_local_variable_declared_here) - << D->getLocation(); - } + if (auto *D = RetPointee.getDecl()) + Reporter->ReportReturnLocalVar(RetExpr, D); if (auto *E = RetPointee.getExpr()) - S.Diag(E->getExprLoc(), diag::warn_ret_stack_temporary_ref_cfg) - << E->getExprLoc(); + Reporter->ReportReturnTemporaryExpr(E); } } [[maybe_unused]] const DeclContext &DC; const CFG &cfg; [[maybe_unused]] AnalysisDeclContext &AC; - Sema &S; + DanglingReferenceReporter *Reporter; // ExpressionResolver ExprResolver; PointsToTracker PointsTo; }; } // namespace void runDanglingReferenceAnalysis(const DeclContext &DC, const CFG &cfg, - AnalysisDeclContext &AC, Sema &S) { - DanglingReferenceAnalyzer DRA(DC, cfg, AC, S); + AnalysisDeclContext &AC, + DanglingReferenceReporter *Reporter) { + DanglingReferenceAnalyzer DRA(DC, cfg, AC, Reporter); DRA.RunAnalysis(); } diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 721d4bdba3db545..618b00734ccb2ac 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2638,6 +2638,28 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( } } +namespace { + +class DanglingReferenceReporterImpl : public DanglingReferenceReporter { +public: + DanglingReferenceReporterImpl(Sema &S) : S(S) {} + + void ReportReturnLocalVar(const Expr *RetExpr, + const Decl *LocalDecl) override { + S.Diag(RetExpr->getExprLoc(), diag::warn_ret_stack_variable_ref_cfg) + << RetExpr->getExprLoc(); + S.Diag(LocalDecl->getLocation(), diag::note_local_variable_declared_here) + << LocalDecl->getLocation(); + } + void ReportReturnTemporaryExpr(const Expr *TemporaryExpr) override { + S.Diag(TemporaryExpr->getExprLoc(), diag::warn_ret_stack_temporary_ref_cfg) + << TemporaryExpr->getExprLoc(); + } + +private: + Sema &S; +}; +} // namespace void clang::sema::AnalysisBasedWarnings::IssueWarnings( sema::AnalysisBasedWarnings::Policy P, sema::FunctionScopeInfo *fscope, const Decl *D, QualType BlockType) { @@ -2831,7 +2853,8 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( !Diags.isIgnored(diag::warn_ret_stack_variable_ref_cfg, D->getBeginLoc())) { if (CFG *cfg = AC.getCFG()) { - runDanglingReferenceAnalysis(*cast<DeclContext>(D), *cfg, AC, S); + DanglingReferenceReporterImpl Reporter(S); + runDanglingReferenceAnalysis(*cast<DeclContext>(D), *cfg, AC, &Reporter); } } // Check for violations of "called once" parameter properties. >From fcdadc7d0cfe456d9def8bea1f99cd2d46f42fa4 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Fri, 24 Jan 2025 14:45:17 +0000 Subject: [PATCH 3/3] Add equivalence for no-cfg tests --- clang/lib/Analysis/DanglingReference.cpp | 579 ++++++++++++++---- clang/lib/Sema/AnalysisBasedWarnings.cpp | 4 + clang/test/Sema/Inputs/lifetime-analysis.h | 1 + .../test/Sema/warn-lifetime-analysis-cfg.cpp | 296 ++++++++- .../Sema/warn-lifetime-analysis-nocfg.cpp | 83 ++- 5 files changed, 809 insertions(+), 154 deletions(-) diff --git a/clang/lib/Analysis/DanglingReference.cpp b/clang/lib/Analysis/DanglingReference.cpp index 1ee79d26a843a8f..89b718052c8b91d 100644 --- a/clang/lib/Analysis/DanglingReference.cpp +++ b/clang/lib/Analysis/DanglingReference.cpp @@ -1,15 +1,28 @@ #include "clang/Analysis/Analyses/DanglingReference.h" #include "clang/AST/Attrs.inc" #include "clang/AST/Decl.h" +#include "clang/AST/DeclBase.h" +#include "clang/AST/DeclTemplate.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/Stmt.h" #include "clang/AST/StmtVisitor.h" #include "clang/AST/Type.h" +#include "clang/AST/TypeLoc.h" +#include "clang/Analysis/AnalysisDeclContext.h" #include "clang/Analysis/CFG.h" +#include "clang/Analysis/FlowSensitive/DataflowWorklist.h" +#include "clang/Basic/LLVM.h" +#include "llvm/ADT/BitVector.h" #include "llvm/ADT/DenseMap.h" -#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/FoldingSet.h" +#include "llvm/ADT/ImmutableMap.h" +#include "llvm/ADT/ImmutableSet.h" +#include "llvm/Support/raw_ostream.h" +#include <cassert> #include <sstream> +#include <stdbool.h> +#include <string> namespace clang { namespace { @@ -22,26 +35,98 @@ template <typename T> static bool isRecordWithAttr(QualType Type) { if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) Result |= CTSD->getSpecializedTemplate()->getTemplatedDecl()->hasAttr<T>(); - + if (RD->hasDefinition()) + for (auto B : RD->bases()) + Result |= isRecordWithAttr<T>(B.getType()); return Result; } -bool isOwner(const Expr *E) { - return isRecordWithAttr<OwnerAttr>(E->getType()); -} + +bool isOwner(QualType Q) { return isRecordWithAttr<OwnerAttr>(Q); } +bool isOwner(const Expr *E) { return isOwner(E->getType()); } bool isOwner(const Decl *D) { return isa<ValueDecl>(D) && isRecordWithAttr<OwnerAttr>(dyn_cast<ValueDecl>(D)->getType()); } -bool isPointer(const Expr *E) { - return isRecordWithAttr<PointerAttr>(E->getType()); +bool isPointer(QualType Q) { + return Q->isNullPtrType() || Q->isPointerType() || + isRecordWithAttr<PointerAttr>(Q); } +bool isPointer(const Expr *E) { return isPointer(E->getType()); } bool isPointer(const Decl *D) { - return isa<ValueDecl>(D) && - isRecordWithAttr<PointerAttr>(dyn_cast<ValueDecl>(D)->getType()); + auto *VD = dyn_cast<ValueDecl>(D); + return VD && isPointer(VD->getType()); +} + +static bool isInStlNamespace(const Decl *D) { + const DeclContext *DC = D->getDeclContext(); + if (!DC) + return false; + if (const auto *ND = dyn_cast<NamespaceDecl>(DC)) + if (const IdentifierInfo *II = ND->getIdentifier()) { + StringRef Name = II->getName(); + if (Name.size() >= 2 && Name.front() == '_' && + (Name[1] == '_' || isUppercase(Name[1]))) + return true; + } + + return DC->isStdNamespace(); +} + +static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) { + if (!Callee) + return false; + if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee)) + if (isRecordWithAttr<PointerAttr>(Conv->getConversionType()) && + Callee->getParent()->hasAttr<OwnerAttr>()) + return true; + if (!isInStlNamespace(Callee->getParent())) + return false; + if (!isRecordWithAttr<PointerAttr>( + Callee->getFunctionObjectParameterType()) && + !isRecordWithAttr<OwnerAttr>(Callee->getFunctionObjectParameterType())) + return false; + if (isPointer(Callee->getReturnType())) { + if (!Callee->getIdentifier()) + return false; + return llvm::StringSwitch<bool>(Callee->getName()) + .Cases("begin", "rbegin", "cbegin", "crbegin", true) + .Cases("end", "rend", "cend", "crend", true) + .Cases("c_str", "data", "get", true) + // Map and set types. + .Cases("find", "equal_range", "lower_bound", "upper_bound", true) + .Default(false); + } + if (Callee->getReturnType()->isReferenceType()) { + if (!Callee->getIdentifier()) { + auto OO = Callee->getOverloadedOperator(); + if (!Callee->getParent()->hasAttr<OwnerAttr>()) + return false; + return OO == OverloadedOperatorKind::OO_Subscript || + OO == OverloadedOperatorKind::OO_Star; + } + return llvm::StringSwitch<bool>(Callee->getName()) + .Cases("front", "back", "at", "top", "value", true) + .Default(false); + } + return false; +} + +bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) { + const TypeSourceInfo *TSI = FD->getTypeSourceInfo(); + if (!TSI) + return false; + AttributedTypeLoc ATL; + for (TypeLoc TL = TSI->getTypeLoc(); + (ATL = TL.getAsAdjusted<AttributedTypeLoc>()); + TL = ATL.getModifiedLoc()) { + if (ATL.getAttrAs<LifetimeBoundAttr>()) + return true; + } + return false; } struct MemoryLoc { - enum MemoryType { + enum Storage { EMPTY, // Pointer is null. STACK, // Pointer points to something on stack. UNKNOWN, // Pointer points to an unknown entity. @@ -62,7 +147,7 @@ struct MemoryLoc { static MemoryLoc VarOnStack(const Decl *D) { return {STACK, D, nullptr}; } static MemoryLoc Temporary(const Expr *MTE) { return {STACK, nullptr, MTE}; } - std::string str() { + std::string str() const { std::ostringstream os; switch (Loc) { case EMPTY: @@ -81,13 +166,229 @@ struct MemoryLoc { } return os.str(); } + + static inline void Profile(llvm::FoldingSetNodeID &ID, const MemoryLoc &M) { + ID.AddInteger(M.Loc); + ID.AddPointer(M.D); + ID.AddPointer(M.MTE); + } + + inline void Profile(llvm::FoldingSetNodeID &ID) const { + return Profile(ID, *this); + } +}; +inline bool operator==(const MemoryLoc &LHS, const MemoryLoc &RHS) { + return LHS.Loc == RHS.Loc && LHS.D == RHS.D && LHS.MTE == RHS.MTE; +} +inline bool operator!=(const MemoryLoc &LHS, const MemoryLoc &RHS) { + return !(LHS == RHS); +} + +class PointsToFactory { +public: + PointsToFactory() + : ESetFact(false), DSetFact(false), EPointsToFact(false), + DPointsToFact(false) {} + llvm::ImmutableSet<const Expr *>::Factory ESetFact; + llvm::ImmutableSet<const Decl *>::Factory DSetFact; + llvm::ImmutableMap<const Expr *, MemoryLoc>::Factory EPointsToFact; + llvm::ImmutableMap<const Decl *, MemoryLoc>::Factory DPointsToFact; +}; + +class PointsToSet { + +public: + explicit PointsToSet(PointsToFactory &Factory) + : Factory(Factory), ExprPointsTo(Factory.EPointsToFact.getEmptyMap()), + DeclPointsTo(Factory.DPointsToFact.getEmptyMap()), + StackExprs(Factory.ESetFact.getEmptySet()), + StackDecls(Factory.DSetFact.getEmptySet()), + UnknownDecls(Factory.DSetFact.getEmptySet()) {} + + PointsToSet(const PointsToSet &P) + : Factory(P.Factory), ExprPointsTo(P.ExprPointsTo), + DeclPointsTo(P.DeclPointsTo), StackExprs(P.StackExprs), + StackDecls(P.StackDecls), UnknownDecls(P.UnknownDecls) {} + + PointsToSet &operator=(const PointsToSet &P) { + ExprPointsTo = P.ExprPointsTo; + DeclPointsTo = P.DeclPointsTo; + StackExprs = P.StackExprs; + StackDecls = P.StackDecls; + UnknownDecls = P.UnknownDecls; + return *this; + } + + bool ContainsExprPointsTo(const Expr *E) const { + return E && ExprPointsTo.contains(E); + } + bool ContainsDeclPointsTo(const Decl *D) const { + return D && DeclPointsTo.contains(D); + } + MemoryLoc GetExprPointsTo(const Expr *E) const { + assert(ContainsExprPointsTo(E)); + return *ExprPointsTo.lookup(E); + } + MemoryLoc GetDeclPointsTo(const Decl *D) const { + assert(ContainsDeclPointsTo(D)); + return *DeclPointsTo.lookup(D); + } + MemoryLoc SetExprPointer(const Expr *E, MemoryLoc M) { + ExprPointsTo = Factory.EPointsToFact.add(ExprPointsTo, E, M); + return M; + } + MemoryLoc SetDeclPointer(const Decl *D, MemoryLoc M) { + if (DeclPointsTo.contains(D)) + DeclPointsTo = Factory.DPointsToFact.remove(DeclPointsTo, D); + DeclPointsTo = Factory.DPointsToFact.add(DeclPointsTo, D, M); + return M; + } + + void AddToStack(const Decl *D) { + StackDecls = Factory.DSetFact.add(StackDecls, D); + } + void MarkAsUnknown(const Decl *D) { + UnknownDecls = Factory.DSetFact.add(UnknownDecls, D); + } + void AddToStack(const Expr *E) { + StackExprs = Factory.ESetFact.add(StackExprs, E); + } + + bool IsOnStack(const Decl *D) const { + return StackDecls.contains(D) && !UnknownDecls.contains(D); + } + bool IsOnStack(const Expr *E) const { return StackExprs.contains(E); } + + int size() const { + return ExprPointsTo.getHeight() + DeclPointsTo.getHeight() + + StackExprs.getHeight() + StackDecls.getHeight() + + UnknownDecls.getHeight(); + } + void dump() { + std::ostringstream os; + for (const auto &x : DeclPointsTo) { + auto *VD = dyn_cast<VarDecl>(x.first); + if (VD) { + llvm::errs() << VD->getNameAsString() << " ----> Points to ---> " + << x.second.str() << "\n"; + } + } + for (const auto &x : UnknownDecls) { + auto *VD = dyn_cast<VarDecl>(x); + if (VD) { + llvm::errs() << "\tunknown: " << VD->getNameAsString() << "\n"; + } + } + } + +private: + PointsToFactory &Factory; + + // TODO: Make private. +public: + // TODO: We only need decl information. not expr! + + // Map from an expression of View type to its pointee and Owner type to the + // reference<TODO simplify>. This should follow single assignment because + // Expr* cannot be reassigned in the program. + llvm::ImmutableMap<const Expr *, MemoryLoc> ExprPointsTo; + // Map from a decl of View type to it pointee. This can be reassigned at + // various points in the program due to transfer functions. + llvm::ImmutableMap<const Decl *, MemoryLoc> DeclPointsTo; + + // Collection of Expr* and Decl* stored on stack. + llvm::ImmutableSet<const Expr *> StackExprs; + llvm::ImmutableSet<const Decl *> StackDecls; + // Decl was previously on stack but was then moved to some unknown storage. + // For example: std::unique_ptr<> P can be moved to unknown storage using + // std::move(P) or P.release(); + llvm::ImmutableSet<const Decl *> UnknownDecls; }; -class PointsToTracker : public ConstStmtVisitor<PointsToTracker> { +PointsToSet Merge(const PointsToSet a, const PointsToSet b) { + if (a.size() < b.size()) + return Merge(b, a); + PointsToSet res = a; + for (const auto &v : b.ExprPointsTo) { + if (!res.ContainsExprPointsTo(v.first)) + res.SetExprPointer(v.first, v.second); + else if (res.GetExprPointsTo(v.first) != v.second) + res.SetExprPointer(v.first, MemoryLoc::Unknown()); + } + for (const auto &v : b.DeclPointsTo) { + if (!res.ContainsDeclPointsTo(v.first)) + res.SetDeclPointer(v.first, v.second); + else if (res.GetDeclPointsTo(v.first) != v.second) + res.SetDeclPointer(v.first, MemoryLoc::Unknown()); + } + for (const auto *D : b.StackDecls) { + res.AddToStack(D); + } + for (const auto *E : b.StackExprs) { + res.AddToStack(E); + } + for (const auto *D : b.UnknownDecls) { + res.MarkAsUnknown(D); + } + return res; +} + +class BlockVisitor : public ConstStmtVisitor<BlockVisitor> { public: + BlockVisitor(PointsToSet Incoming, const DeclContext &DC, + DanglingReferenceReporter *Reporter) + : PointsTo(Incoming), DC(DC), Reporter(Reporter) {} + void Handle(const clang::CFGBlock *B) { + for (CFGBlock::const_iterator BI = B->begin(), BE = B->end(); BI != BE; + ++BI) { + if (auto cfgstmt = BI->getAs<CFGStmt>()) { + // llvm::errs() << "================================================\n"; + // cfgstmt->dump(); + auto *stmt = cfgstmt->getStmt(); + // if (auto *E = dyn_cast<Expr>(stmt)) + // E->dumpColor(); + + Handle(stmt); + + // if (auto *E = dyn_cast<Expr>(stmt)) { + // E->dumpColor(); + // auto Loc = PointsTo.ResolveExpr(E); + // llvm::errs() << "Points To : " << Loc.str() << "\n"; + // } + + if (auto *RS = dyn_cast<ReturnStmt>(stmt)) + DiagnoseReturnStmt(RS); + } + } + } + PointsToSet getOutgoing() { return PointsTo; } + +private: + // Diagnose possible return of stack variable. + void DiagnoseReturnStmt(const ReturnStmt *RS) { + if (!RS) + return; + const Expr *RetExpr = RS->getRetValue(); + if (!RetExpr || !isPointer(RetExpr)) + return; + Handle(RetExpr); + auto RetPointee = ResolveExpr(RetExpr); + // RetExpr->dumpColor(); + // llvm::errs() << "Returning pointer to " << RetPointee.str() << "\n"; + if (RetPointee.IsOnStack()) { + llvm::errs() << "=================================================\n"; + llvm::errs() << "Returning pointer to " << RetPointee.str() << "\n\n"; + RetExpr->dumpColor(); + // This points to something on stack!! + if (auto *D = RetPointee.getDecl(); D && PointsTo.IsOnStack(D)) + Reporter->ReportReturnLocalVar(RetExpr, D); + if (auto *E = RetPointee.getExpr()) + Reporter->ReportReturnTemporaryExpr(E); + } + } + void Handle(const Stmt *S) { - if (auto *E = dyn_cast<Expr>(S); - E && ExprPointsTo.find(E) != ExprPointsTo.end()) + if (auto *E = dyn_cast<Expr>(S); PointsTo.ContainsExprPointsTo(E)) return; Visit(S); } @@ -100,16 +401,27 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> { return; // Initialise the pointer if we are seeing it for the first time. if (isPointer(VD)) { - if (DeclPointsTo.find(D) == DeclPointsTo.end()) - UpdatePointer(VD, ResolveExpr(VD->getInit())); - } - if (isOwner(VD)) { - if (StackDecls.find(D) == StackDecls.end() && VD->hasLocalStorage()) - AddToStack(VD); + if (PointsTo.ContainsDeclPointsTo(D)) + return; + // Pointer params are always unknown. + if (isa<ParmVarDecl>(VD)) { + UpdatePointer(VD, MemoryLoc::Unknown()); + return; + } + UpdatePointer(VD, ResolveExpr(VD->getInit())); + return; } + // Ignore non-stack variable. + // TODO: Track reference variables. + if (!DC.containsDecl(const_cast<Decl *>(D)) || !VD->hasLocalStorage() || + VD->getType()->isReferenceType()) + return; + + if (!PointsTo.IsOnStack(D)) + AddToStack(VD); } - // Merge above and below in VisitVarDecl !! +public: void VisitDeclStmt(const DeclStmt *DS) { MaybeInitaliseDecl(DS->getSingleDecl()); } @@ -123,11 +435,10 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> { if (isPointer(MTE)) { Handle(MTE->getSubExpr()); SetExprPointer(MTE, MTE->getSubExpr()); + return; } - if (isOwner(MTE)) { - // We have a temporary owner on stack. - AddToStack(MTE); - } + // We have a temporary on stack. + AddToStack(MTE); } void VisitImplicitCastExpr(const ImplicitCastExpr *E) { @@ -137,16 +448,51 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> { } void VisitExprWithCleanups(const ExprWithCleanups *E) { - // Handle(E->getSubExpr()); SetExprPointer(E, E->getSubExpr()); } - - void VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) { - // Conversion from Owner to a Pointer. - const Expr *ConversionFrom = MCE->IgnoreConversionOperatorSingleStep(); - if (ConversionFrom != MCE) { - if (isOwner(ConversionFrom) && isPointer(MCE)) { - SetExprPointer(MCE, ConversionFrom); + bool IsUniquePtrRelease(const CXXMemberCallExpr *MCE) { + if (!MCE || !MCE->getCalleeDecl()) + return false; + auto *FD = MCE->getCalleeDecl()->getAsFunction(); + if (!FD) + return false; + return FD->getIdentifier() && FD->getName() == "release"; + } + void VisitCallExpr(const CallExpr *CE) { + if (CE->isCallToStdMove()) { + assert(CE->getNumArgs() == 1); + MarkDeclAsUnknown(CE->getArg(0)); + return; + } + // TODO: Can be merged with above std::move. + if (auto *MCE = dyn_cast<CXXMemberCallExpr>(CE)) { + if (IsUniquePtrRelease(MCE)) + MarkDeclAsUnknown(MCE->getImplicitObjectArgument()); + } + // Lifetimebound function calls. + auto *FD = dyn_cast_or_null<FunctionDecl>(CE->getCalleeDecl()); + if (!FD) + return; + // FIXME: This should only be done for GSL pointer args and not all args! + QualType RetType = FD->getReturnType(); + if (RetType->isReferenceType() && !isOwner(RetType->getPointeeType())) + return; + Expr *ObjectArg = nullptr; + if (auto *MCE = dyn_cast<CXXMemberCallExpr>(CE)) { + ObjectArg = MCE->getImplicitObjectArgument(); + if (ObjectArg && (implicitObjectParamIsLifetimeBound(FD) || + shouldTrackImplicitObjectArg(MCE->getMethodDecl()))) { + // TODO: Track more args. Not just the first one! + SetExprPointer(CE, ObjectArg); + return; + } + } + for (unsigned I = 0; I < FD->getNumParams(); ++I) { + const ParmVarDecl *PVD = FD->getParamDecl(I); + if (CE->getArg(I) && PVD->hasAttr<LifetimeBoundAttr>()) { + // TODO: Track more args. Not just the first one! + SetExprPointer(CE, CE->getArg(I)); + return; } } } @@ -155,7 +501,8 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> { if (!isPointer(CCE)) return; if (CCE->getNumArgs() == 1) - SetExprPointer(CCE, CCE->getArg(0)); + if (isOwner(CCE->getArg(0)) || isPointer(CCE->getArg(0))) + SetExprPointer(CCE, CCE->getArg(0)); } void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) { @@ -170,31 +517,41 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> { private: // Returns the Decl that is aliased by this expression. const Decl *DeclReferencedBy(const Expr *E) { - if (auto *DRE = dyn_cast<DeclRefExpr>(E)) { + if (auto *DRE = dyn_cast_or_null<DeclRefExpr>(E)) { return DRE->getDecl(); } return nullptr; } + void MarkDeclAsUnknown(const Expr *E) { + if (!E) + return; + if (const Decl *D = DeclReferencedBy(E)) { + PointsTo.MarkAsUnknown(D); + } + } + void HandleAssignment(const Expr *A, const Expr *B) { if (!isPointer(A)) return; - if (const Decl *PointerDecl = DeclReferencedBy(A)) + if (const Decl *PointerDecl = DeclReferencedBy(A); + PointerDecl && isPointer(PointerDecl)) UpdatePointer(PointerDecl, B); } // Update the contents of a Pointer. void UpdatePointer(const Decl *PointerD, MemoryLoc ML) { assert(isPointer(PointerD)); - DeclPointsTo[PointerD] = ML; + PointsTo.SetDeclPointer(PointerD, ML); } void UpdatePointer(const Decl *PointerD, const Expr *A) { UpdatePointer(PointerD, ResolveExpr(A)); } void SetExprPointer(const Expr *E, MemoryLoc ML) { - assert(ExprPointsTo.insert({E, ML}).second); + assert(!PointsTo.ContainsExprPointsTo(E)); + PointsTo.SetExprPointer(E, ML); } void SetExprPointer(const Expr *E, const Expr *PointeeE) { SetExprPointer(E, ResolveExpr(PointeeE)); @@ -203,16 +560,16 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> { SetExprPointer(E, ResolveDecl(D)); } -public: MemoryLoc ResolveExpr(const Expr *E) { if (!E) return MemoryLoc::Empty(); - Handle(E); - auto Res = ExprPointsTo.find(E); - if (Res != ExprPointsTo.end()) { - return Res->getSecond(); + if (PointsTo.ContainsExprPointsTo(E)) { + return PointsTo.GetExprPointsTo(E); } - return ExprPointsTo[E] = MemoryLoc::Unknown(); + Handle(E); + if (PointsTo.ContainsExprPointsTo(E)) + return PointsTo.GetExprPointsTo(E); + return PointsTo.SetExprPointer(E, MemoryLoc::Unknown()); } // Returns the memory location pointed to by D. If D is a pointer-type, @@ -221,56 +578,43 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> { MaybeInitaliseDecl(D); if (isPointer(D)) return ResolvePointer(D); - if (isOwner(D)) - return ResolveOwner(D); - return MemoryLoc::Unknown(); + return ResolveNonPointer(D); } MemoryLoc ResolvePointer(const Decl *D) { auto *VD = dyn_cast<VarDecl>(D); - assert(VD); - if (!VD->hasLocalStorage()) + // TODO: Handle other decls like field. + if (!VD || !VD->hasLocalStorage()) return MemoryLoc::Unknown(); assert(isPointer(D)); - auto Res = DeclPointsTo.find(D); - assert(Res != DeclPointsTo.end()); - return Res->getSecond(); + return PointsTo.GetDeclPointsTo(D); } - MemoryLoc ResolveOwner(const Decl *D) { - assert(isOwner(D)); - if (IsOnStack(D)) + MemoryLoc ResolveNonPointer(const Decl *D) { + assert(!isPointer(D)); + if (IsOnStack(D)) { return MemoryLoc::VarOnStack(D); + } return MemoryLoc::Unknown(); } - void AddToStack(const Decl *D) { - assert(isOwner(D)); - StackDecls.insert(D); - } + void AddToStack(const Decl *D) { PointsTo.AddToStack(D); } void AddToStack(const Expr *E) { assert(isa<MaterializeTemporaryExpr>(E)); - assert(isOwner(E)); - StackExprs.insert(E); + assert(!isPointer(E)); + PointsTo.AddToStack(E); // Add a self edge. - assert(ExprPointsTo.insert({E, MemoryLoc::Temporary(E)}).second); + assert(!PointsTo.ContainsExprPointsTo(E)); + PointsTo.SetExprPointer(E, MemoryLoc::Temporary(E)); } - bool IsOnStack(const Decl *D) { return StackDecls.contains(D); } - bool IsOnStack(const Expr *E) { return StackExprs.contains(E); } + bool IsOnStack(const Decl *D) { return PointsTo.IsOnStack(D); } + bool IsOnStack(const Expr *E) { return PointsTo.IsOnStack(E); } - // ExpressionResolver &ExprResolver; - // Map from an expression of View type to its pointee and Owner type to the - // reference<TODO simplify>. This should follow single assignment because - // Expr* cannot be reassigned in the program. - llvm::DenseMap<const Expr *, MemoryLoc> ExprPointsTo; - // Map from a decl of View type to it pointee. This can be reassigned at - // various points in the program due to transfer functions. - llvm::DenseMap<const Decl *, MemoryLoc> DeclPointsTo; - - // Collection of Expr* and Decl* stored on stack. - llvm::DenseSet<const Expr *> StackExprs; - llvm::DenseSet<const Decl *> StackDecls; + PointsToSet PointsTo; + const DeclContext &DC; + DanglingReferenceReporter *Reporter; }; + class DanglingReferenceAnalyzer { public: DanglingReferenceAnalyzer(const DeclContext &DC, const CFG &cfg, @@ -278,61 +622,58 @@ class DanglingReferenceAnalyzer { DanglingReferenceReporter *Reporter) : DC(DC), cfg(cfg), AC(AC), Reporter(Reporter) {} void RunAnalysis() { - // For simplicity in protoytpe, avoid joins and stick to functions without // branches. - if (!cfg.isLinear()) - return; + // if (!cfg.isLinear()) + // return; // cfg.dump(AC.getASTContext().getLangOpts(), true); - for (auto I = cfg.begin(), E = cfg.end(); I != E; ++I) { - for (CFGBlock::const_iterator BI = (*I)->begin(), BE = (*I)->end(); - BI != BE; ++BI) { - if (auto cfgstmt = BI->getAs<CFGStmt>()) { - auto *stmt = cfgstmt->getStmt(); - // llvm::errs() << - // "================================================\n"; - // cfgstmt->dump(); - // if (auto *E = dyn_cast<Expr>(stmt)) - // E->dumpColor(); - PointsTo.Handle(stmt); - if (auto *E = dyn_cast<Expr>(stmt)) { - // llvm::errs() << "Points To : " << PointsTo.ResolveExpr(E).str() - // << "\n"; - } - if (auto *RS = dyn_cast<ReturnStmt>(stmt)) - HandleReturnStmt(RS); - } - } + ForwardDataflowWorklist worklist(cfg, AC); + worklist.enqueueSuccessors(&cfg.getEntry()); + PointsToSets.insert({&cfg.getEntry(), PointsToSet(Factory)}); + + llvm::BitVector Visited(cfg.getNumBlockIDs()); + + while (const CFGBlock *Block = worklist.dequeue()) { + unsigned BlockID = Block->getBlockID(); + if (Visited[BlockID]) + continue; + // llvm::errs() << "====================================\n"; + // Block->dump(); + PointsToSet IncomingPointsTo = MergePredecessors(Block); + BlockVisitor BV(IncomingPointsTo, DC, Reporter); + BV.Handle(Block); + PointsToSets.insert({Block, BV.getOutgoing()}); + + // llvm::errs() << "Incoming points to:\n"; + // IncomingPointsTo.dump(); + // llvm::errs() << "Outgoing points to:\n"; + // BV.getOutgoing().dump(); + + worklist.enqueueSuccessors(Block); + Visited[BlockID] = true; } } -private: - void HandleReturnStmt(const ReturnStmt *RS) { - // Diagnose possible return of stack variable. - if (!RS) - return; - const Expr *RetExpr = RS->getRetValue(); - if (!RetExpr || !isPointer(RetExpr)) - return; - PointsTo.Handle(RetExpr); - auto RetPointee = PointsTo.ResolveExpr(RetExpr); - // RetExpr->dumpColor(); - // llvm::errs() << "Returning pointer to " << RetPointee.str() << "\n"; - if (RetPointee.IsOnStack()) { - // This points to something on stack!! - if (auto *D = RetPointee.getDecl()) - Reporter->ReportReturnLocalVar(RetExpr, D); - if (auto *E = RetPointee.getExpr()) - Reporter->ReportReturnTemporaryExpr(E); + PointsToSet MergePredecessors(const CFGBlock *Block) { + PointsToSet Result(Factory); + for (const auto &Pred : Block->preds()) { + if (PointsToSets.contains(Pred)) { + Result = Merge(Result, PointsToSets.find(Pred)->getSecond()); + // llvm::errs() << "Merging:\n"; + // PointsToSets.find(Pred)->getSecond().dump(); + } } + // llvm::errs() << "Merged result:\n"; + // Result.dump(); + return Result; } + PointsToFactory Factory; [[maybe_unused]] const DeclContext &DC; const CFG &cfg; - [[maybe_unused]] AnalysisDeclContext &AC; + AnalysisDeclContext &AC; DanglingReferenceReporter *Reporter; - // ExpressionResolver ExprResolver; - PointsToTracker PointsTo; + llvm::DenseMap<const CFGBlock *, PointsToSet> PointsToSets; }; } // namespace diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 618b00734ccb2ac..3fc98b4388ee941 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2650,10 +2650,14 @@ class DanglingReferenceReporterImpl : public DanglingReferenceReporter { << RetExpr->getExprLoc(); S.Diag(LocalDecl->getLocation(), diag::note_local_variable_declared_here) << LocalDecl->getLocation(); + llvm::errs() << "Debug: Return local var:\n"; + RetExpr->getExprLoc().dump(S.getSourceManager()); } void ReportReturnTemporaryExpr(const Expr *TemporaryExpr) override { S.Diag(TemporaryExpr->getExprLoc(), diag::warn_ret_stack_temporary_ref_cfg) << TemporaryExpr->getExprLoc(); + llvm::errs() << "Debug: Return temporary expr:\n"; + TemporaryExpr->getExprLoc().dump(S.getSourceManager()); } private: diff --git a/clang/test/Sema/Inputs/lifetime-analysis.h b/clang/test/Sema/Inputs/lifetime-analysis.h index f888e6ab94bb6aa..f176a4636f94144 100644 --- a/clang/test/Sema/Inputs/lifetime-analysis.h +++ b/clang/test/Sema/Inputs/lifetime-analysis.h @@ -87,6 +87,7 @@ template<typename T> struct unique_ptr { T &operator*(); T *get() const; + T *release(); }; template<typename T> diff --git a/clang/test/Sema/warn-lifetime-analysis-cfg.cpp b/clang/test/Sema/warn-lifetime-analysis-cfg.cpp index e337c75f8344d2f..824cdf4510f1123 100644 --- a/clang/test/Sema/warn-lifetime-analysis-cfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-cfg.cpp @@ -90,6 +90,11 @@ std::string_view return_local() { local; } +int* return_null_ptr() { + auto T = nullptr; + return T; +} + // Parameters std::string_view params_owner_and_views( std::string_view sv, @@ -123,14 +128,285 @@ std::string_view copy_of_global_str() { return sv; // expected-warning {{returning reference to a stack variable}} } -// TODO: Use lifetimebound in function calls. Use Pointer to Owner of pointer -// std::string_view containerOfString() { -// std::vector<std::string> local; -// return local.at(0); -// } +struct Struct { std::string s; }; +std::string_view field() { + Struct s; + std::string_view sv; + sv = s.s; + return sv; // FIXME. +} + +std::string_view loopReturnFor(int n) { + for (int i = 0; i < n; ++i) { + std::string local = "inside-loop"; // expected-note {{reference to this stack variable is returned}} + return local; // expected-warning {{returning reference to a stack variable}} + } + return "safe"; // Safe return. +} + +std::string_view loopReturnWhile(bool condition) { + while (condition) { + std::string local = "inside-loop"; // expected-note {{reference to this stack variable is returned}} + return local; // expected-warning {{returning reference to a stack variable}} + } + return "safe"; // Safe return. +} + +std::string_view loopReturnDoWhile() { + do { + std::string local = "inside-loop"; // expected-note {{reference to this stack variable is returned}} + return local; // expected-warning {{returning reference to a stack variable}} + } while (false); +} + +std::string_view nestedConditionals(bool a, bool b) { + std::string local = "nested"; // expected-note {{reference to this stack variable is returned}} + if (a) { + if (b) { + return local; // expected-warning {{returning reference to a stack variable}} + } + } + return "safe"; +} + +std::string_view nestedLoops(int n) { + std::string local = "loop"; // expected-note {{reference to this stack variable is returned}} + for (int i = 0; i < n; ++i) { + for (int j = 0; j < n; ++j) { + return local; // expected-warning {{returning reference to a stack variable}} + } + } + return "safe"; +} + +namespace lifetimebound { + +std::string_view func_lb_sv(std::string_view sv [[clang::lifetimebound]]); +std::string_view use_func_lb_sv() { + std::string s; // expected-note {{reference to this stack variable is returned}} + std::string_view sv = func_lb_sv(s); + sv = func_lb_sv(s); + std::string_view sv2; + sv2 = sv; + return sv2; // expected-warning {{returning reference to a stack variable}} +} + +std::string_view use_string_func_lb_sv() { + std::string s; // expected-note {{reference to this stack variable is returned}} + std::string_view sv = s; + std::string_view sv_lb = func_lb_sv(sv); + return sv_lb; // expected-warning {{returning reference to a stack variable}} +} + +std::string_view direct_return() { + std::string s; // expected-note {{reference to this stack variable is returned}} + std::string_view sv = s; + return func_lb_sv(sv); // expected-warning {{returning reference to a stack variable}} +} + +std::string_view if_branches(std::string param, bool cond) { // expected-note 4 {{reference to this stack variable is returned}} + std::string_view view; + view = func_lb_sv(param); + view = func_lb_sv(view); + std::string_view stack = view; + if (cond) view = func_lb_sv(view); + if (cond) view = func_lb_sv(view); + if (cond) + return view; // expected-warning {{returning reference to a stack variable}} + if (cond) + view = unknown(); + // Now it is potentially safe! + if (cond) + return view; // Ok. We can't know for sure. + if (cond) { + view = stack; + return view; // expected-warning {{returning reference to a stack variable}} + } + // Unconditionally restore to stack. + view = stack; + if (cond) + return view; // expected-warning {{returning reference to a stack variable}} + return view; // expected-warning {{returning reference to a stack variable}} +} + +std::string_view nested_early_return(std::string param, bool a, bool b) { // expected-note 2 {{reference to this stack variable is returned}} + std::string_view view; + view = func_lb_sv(param); + view = func_lb_sv(view); + std::string_view stack = view; + + if (a) { + if (b) { + return view; // expected-warning {{returning reference to a stack variable}} + } + view = unknown(); + } else { + if (b) { + view = stack; + return view; // expected-warning {{returning reference to a stack variable}} + } + } + return view; // Ok, because one branch set it to unknown. +} + +std::string_view func_with_branches(std::string param, bool cond1, bool cond2) { // expected-note 2 {{reference to this stack variable is returned}} + std::string_view view; + view = func_lb_sv(param); + view = func_lb_sv(view); + std::string_view stack = view; + + if (cond1) + view = func_lb_sv(view); + + if (cond2) + view = unknown(); + else if (cond1) + return view; // expected-warning {{returning reference to a stack variable}} + + if (cond1 && cond2) + return view; // Ok. We can't be sure. + + view = stack; + return view; // expected-warning {{returning reference to a stack variable}} +} + + +namespace MemberFunctions { +struct S { + std::string return_string() const; + const std::string& return_string_ref() const [[clang::lifetimebound]]; +}; + +std::string_view use_return_string() { + S s; + return s.return_string(); // expected-warning {{returning reference to a temporary object}} +} +std::string_view use_return_string_ref() { + S s; // expected-note {{reference to this stack variable is returned}} + return s.return_string_ref(); // expected-warning {{returning reference to a stack variable}} +} +std::string_view use_return_string_ref(const S* s) { + return s->return_string_ref(); +} +std::string_view use_return_string_ref_temporary() { + return S{}.return_string_ref(); // expected-warning {{returning reference to a temporary object}} +} +} // namespace MemberFunctions + +namespace Optional { +std::optional<std::string_view> getOptional(std::string_view); +std::string_view usOptional(std::string_view s) { + return getOptional(s).value(); +} +} // namespace Optional +struct ThisIsLB { +std::string_view get() [[clang::lifetimebound]]; +}; + +std::string_view use_lifetimebound_member_fn() { + ThisIsLB obj; // expected-note {{reference to this stack variable is returned}} + return obj.get(); // expected-warning {{returning reference to a stack variable}} +} + +std::string_view return_temporary_get() { + return ThisIsLB{}.get(); // expected-warning {{returning reference to a temporary object}} +} + +std::string_view store_temporary_get() { + // FIXME: Move this diagnostic to the return loc!! + std::string_view sv1 = ThisIsLB{}.get(); // expected-warning {{returning reference to a temporary object}} + std::string_view sv2 = sv1; + std::string_view sv3 = func_lb_sv(sv2); + return sv3; +} + +std::string_view multiple_lifetimebound_calls() { + std::string s; // expected-note {{reference to this stack variable is returned}} + std::string_view sv = func_lb_sv(func_lb_sv(func_lb_sv(s))); + sv = func_lb_sv(func_lb_sv(func_lb_sv(sv))); + return sv; // expected-warning {{returning reference to a stack variable}} +} + +} // namespace lifetimebound + +std::string_view return_char_star() { + const char* key; + key = "foo"; + return key; +} + +void lambda() { + std::string s; + auto l1 = [&s]() { + std::string_view sv = s; + return sv; + }; + auto l2 = []() { + std::string s; // expected-note {{reference to this stack variable is returned}} + std::string_view sv = s; + return sv; // expected-warning {{returning reference to a stack variable}} + }; +} + +std::string_view default_arg(std::string_view sv = std::string()) { + return sv; // Ok! +} +std::string_view default_arg_overwritten(std::string_view sv = std::string()) { + std::string s; // expected-note {{reference to this stack variable is returned}} + sv = s; + return sv; // expected-warning {{returning reference to a stack variable}} +} + +std::string_view containerOfString(bool cond) { + if (cond) { + return std::vector<std::string>{"one"}.at(0); // expected-warning {{returning reference to a temporary object}} + } + std::vector<std::string> local; // expected-note 2 {{reference to this stack variable is returned}} + if (cond) { + return local.at(0); // expected-warning {{returning reference to a stack variable}} + } + std::string_view sv = local.at(0); + if (cond) { + return sv; // expected-warning {{returning reference to a stack variable}} + } + return unknown(); +} + +std::string_view conditionalReturn(bool flag) { + std::string local1 = "if-branch"; // expected-note {{reference to this stack variable is returned}} + std::string local2 = "else-branch"; // expected-note {{reference to this stack variable is returned}} + if (flag) + return local1; // expected-warning {{returning reference to a stack variable}} + else + return local2; // expected-warning {{returning reference to a stack variable}} +} + +std::string_view conditionalReturnMixed(bool flag, const std::string& external) { + std::string local = "stack-var"; // expected-note {{reference to this stack variable is returned}} + if (flag) + return local; // expected-warning {{returning reference to a stack variable}} + else + return external; // OK, returning reference to external. +} -// std::string_view containerOfViewMultistmt() { -// std::vector<std::string> local; -// std::string_view sv = local.at(0); -// return sv; -// } +void move_to_arena(std::unique_ptr<int>); +void move_to_arena(int*); +int* move_unique_ptr(bool cond) { + std::unique_ptr<int> local; // expected-note 3 {{reference to this stack variable is returned}} + int* pointer = local.get(); + if (cond) + return local.get(); // expected-warning {{returning reference to a stack variable}} + if (cond) + return pointer; // expected-warning {{returning reference to a stack variable}} + if (cond) { + // 'local' definitely moved to unknown. + move_to_arena(std::move(local)); + return pointer; + } + if(cond) + return pointer; // expected-warning {{returning reference to a stack variable}} + // Maybe 'local' is moved to unknown. + if (cond) + move_to_arena(local.release()); + return pointer; // Ok. +} \ No newline at end of file diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index 4c19367bb7f3ddc..38ff380edf99021 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -fsyntax-only -Wdangling -Wdangling-field -Wreturn-stack-address -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wno-dangling -Wdangling-cfg -verify=cfg %s #include "Inputs/lifetime-analysis.h" struct [[gsl::Owner(int)]] MyIntOwner { MyIntOwner(); @@ -88,27 +89,32 @@ MyIntPointer returningLocalPointer() { } MyIntPointer daglingGslPtrFromLocalOwner() { - MyIntOwner localOwner; + MyIntOwner localOwner; // cfg-note {{reference to this stack variable is returned}} return localOwner; // expected-warning {{address of stack memory associated with local variable 'localOwner' returned}} + // cfg-warning@-1 {{returning reference to a stack variable}} } MyLongPointerFromConversion daglingGslPtrFromLocalOwnerConv() { - MyLongOwnerWithConversion localOwner; + MyLongOwnerWithConversion localOwner; // cfg-note {{reference to this stack variable is returned}} return localOwner; // expected-warning {{address of stack memory associated with local variable 'localOwner' returned}} + // cfg-warning@-1 {{returning reference to a stack variable}} } MyIntPointer danglingGslPtrFromTemporary() { return MyIntOwner{}; // expected-warning {{returning address of local temporary object}} + // cfg-warning@-1 {{returning reference to a temporary object}} } MyIntOwner makeTempOwner(); MyIntPointer danglingGslPtrFromTemporary2() { return makeTempOwner(); // expected-warning {{returning address of local temporary object}} + // cfg-warning@-1 {{returning reference to a temporary object}} } MyLongPointerFromConversion danglingGslPtrFromTemporaryConv() { return MyLongOwnerWithConversion{}; // expected-warning {{returning address of local temporary object}} + // cfg-warning@-1 {{returning reference to a temporary object}} } int *noFalsePositive(MyIntOwner &o) { @@ -144,6 +150,7 @@ void modelIterators() { std::vector<int>::iterator modelIteratorReturn() { return std::vector<int>().begin(); // expected-warning {{returning address of local temporary object}} + // cfg-warning@-1{{returning reference to a temporary object}} } const int *modelFreeFunctions() { @@ -163,8 +170,9 @@ int modelAnyCast3() { } const char *danglingRawPtrFromLocal() { - std::basic_string<char> s; + std::basic_string<char> s; // cfg-note {{reference to this stack variable is returned}} return s.c_str(); // expected-warning {{address of stack memory associated with local variable 's' returned}} + // cfg-warning@-1 {{returning reference to a stack variable}} } int &danglingRawPtrFromLocal2() { @@ -185,8 +193,9 @@ std::string_view containerWithAnnotatedElements() { // no warning on constructing from gsl-pointer std::string_view c2 = std::vector<std::string_view>().at(0); - std::vector<std::string> local; + std::vector<std::string> local; // cfg-note {{reference to this stack variable is returned}} return local.at(0); // expected-warning {{address of stack memory associated with local variable}} + // cfg-warning@-1 {{returning reference to a stack variable}} } std::string_view localUniquePtr(int i) { @@ -198,25 +207,29 @@ std::string_view localUniquePtr(int i) { } std::string_view localOptional(int i) { - std::optional<std::string> o; + std::optional<std::string> o; // cfg-note {{reference to this stack variable is returned}} if (i) return o.value(); // expected-warning {{address of stack memory associated with local variable}} + // cfg-warning@-1{{returning reference to a stack variable}} std::optional<std::string_view> abc; return abc.value(); // expect no warning } const char *danglingRawPtrFromTemp() { return std::basic_string<char>().c_str(); // expected-warning {{returning address of local temporary object}} + // cfg-warning@-1{{returning reference to a temporary object}} } std::unique_ptr<int> getUniquePtr(); int *danglingUniquePtrFromTemp() { return getUniquePtr().get(); // expected-warning {{returning address of local temporary object}} + // cfg-warning@-1{{returning reference to a temporary object}} } int *danglingUniquePtrFromTemp2() { return std::unique_ptr<int>().get(); // expected-warning {{returning address of local temporary object}} + // cfg-warning@-1{{returning reference to a temporary object}} } void danglingReferenceFromTempOwner() { @@ -300,7 +313,7 @@ void danglingStringviewAssignment(std::string_view a1, std::string_view a2) { std::reference_wrapper<int> danglingPtrFromNonOwnerLocal() { int i = 5; - return i; // TODO + return i; } std::reference_wrapper<int> danglingPtrFromNonOwnerLocal2() { @@ -411,6 +424,7 @@ MyIntPointer handleDerivedToBaseCast1(MySpecialIntPointer ptr) { MyIntPointer handleDerivedToBaseCast2(MyOwnerIntPointer ptr) { return ptr; // expected-warning {{address of stack memory associated with parameter 'ptr' returned}} + // FIXME(cfg): Detect this! we are traversing the type hierarchy for the attr. } std::vector<int>::iterator noFalsePositiveWithVectorOfPointers() { @@ -615,10 +629,14 @@ std::string_view test5() { // Pointer<Pointer> from Owner<Pointer> // Prevent regression GH108463 -Span<int*> test6(std::vector<int*> v) { +Span<int*> test6(std::vector<int*> v, bool cond) { // cfg-note {{reference to this stack variable is returned}} Span<int *> dangling = std::vector<int*>(); // expected-warning {{object backing the pointer}} + // cfg-warning@-1 {{returning reference to a temporary object}} + if(cond) return dangling; + dangling = std::vector<int*>(); // expected-warning {{object backing the pointer}} return v; // expected-warning {{address of stack memory}} + // cfg-warning@-1 {{returning reference to a stack variable}} } /////// From Owner<Owner<Pointer>> /////// @@ -636,16 +654,18 @@ std::vector<int*> test8(StatusOr<std::vector<int*>> aa) { } // Pointer<Pointer> from Owner<Owner<Pointer>> -Span<int*> test9(StatusOr<std::vector<int*>> aa) { +Span<int*> test9(StatusOr<std::vector<int*>> aa) { // cfg-note {{reference to this stack variable is returned}} return aa.valueLB(); // expected-warning {{address of stack memory associated}} + // cfg-warning@-1 {{returning reference to a stack variable}} return aa.valueNoLB(); // OK. } /////// From Owner<Owner> /////// // Pointer<Owner>> from Owner<Owner> -Span<std::string> test10(StatusOr<std::vector<std::string>> aa) { +Span<std::string> test10(StatusOr<std::vector<std::string>> aa) { // cfg-note {{reference to this stack variable is returned}} return aa.valueLB(); // expected-warning {{address of stack memory}} + // cfg-warning@-1 {{returning reference to a stack variable}} return aa.valueNoLB(); // OK. } @@ -663,11 +683,12 @@ const int& test12(Span<int> a) { return a.getFieldNoLB(); // OK. } -void test13() { +auto test13() { // FIXME: RHS is Owner<Pointer>, we skip this case to avoid false positives. std::optional<Span<int*>> abc = std::vector<int*>{}; std::optional<Span<int>> t = std::vector<int> {}; // expected-warning {{object backing the pointer will be destroyed}} + return t; } } // namespace GH100526 @@ -690,8 +711,10 @@ class set { } // namespace std namespace GH118064{ -void test() { +auto test() { auto y = std::set<int>{}.begin(); // expected-warning {{object backing the pointer}} + // cfg-warning@-1{{returning reference to a temporary object}} + return y; } } // namespace GH118064 @@ -703,20 +726,26 @@ std::string_view TakeSv(std::string_view abc [[clang::lifetimebound]]); std::string_view TakeStrRef(const std::string& abc [[clang::lifetimebound]]); std::string_view TakeStr(std::string abc [[clang::lifetimebound]]); +bool foo(); std::string_view test1() { - std::string_view t1 = Ref(std::string()); // expected-warning {{object backing}} - t1 = Ref(std::string()); // expected-warning {{object backing}} - return Ref(std::string()); // expected-warning {{returning address}} - - std::string_view t2 = TakeSv(std::string()); // expected-warning {{object backing}} - t2 = TakeSv(std::string()); // expected-warning {{object backing}} - return TakeSv(std::string()); // expected-warning {{returning address}} - - std::string_view t3 = TakeStrRef(std::string()); // expected-warning {{temporary}} - t3 = TakeStrRef(std::string()); // expected-warning {{object backing}} - return TakeStrRef(std::string()); // expected-warning {{returning address}} - - + if(foo()) { + std::string_view t1 = Ref(std::string()); // expected-warning {{object backing}} + t1 = Ref(std::string()); // expected-warning {{object backing}} + return Ref(std::string()); // expected-warning {{returning address}} + // cfg-warning@-1 {{returning reference to a temporary object}} + } + if(foo()) { + std::string_view t2 = TakeSv(std::string()); // expected-warning {{object backing}} + t2 = TakeSv(std::string()); // expected-warning {{object backing}} + return TakeSv(std::string()); // expected-warning {{returning address}} + // cfg-warning@-1 {{returning reference to a temporary object}} + } + if(foo()) { + std::string_view t3 = TakeStrRef(std::string()); // expected-warning {{temporary}} + t3 = TakeStrRef(std::string()); // expected-warning {{object backing}} + return TakeStrRef(std::string()); // expected-warning {{returning address}} + // cfg-warning@-1 {{returning reference to a temporary object}} + } std::string_view t4 = TakeStr(std::string()); t4 = TakeStr(std::string()); return TakeStr(std::string()); @@ -727,10 +756,13 @@ struct Foo { const T& get() const [[clang::lifetimebound]]; const T& getNoLB() const; }; -std::string_view test2(Foo<std::string> r1, Foo<std::string_view> r2) { +std::string_view test2( + Foo<std::string> r1, // cfg-note {{reference to this stack variable is returned}} + Foo<std::string_view> r2) { std::string_view t1 = Foo<std::string>().get(); // expected-warning {{object backing}} t1 = Foo<std::string>().get(); // expected-warning {{object backing}} return r1.get(); // expected-warning {{address of stack}} + // cfg-warning@-1 {{returning reference to a stack variable}} std::string_view t2 = Foo<std::string_view>().get(); t2 = Foo<std::string_view>().get(); @@ -750,6 +782,7 @@ Pointer test3(Bar bar) { Pointer p = Pointer(Bar()); // expected-warning {{temporary}} p = Pointer(Bar()); // expected-warning {{object backing}} return bar; // expected-warning {{address of stack}} + // FIXME(cfg): Handle Lifetimebound constructors. } template<typename T> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits