https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/124133
>From cec28dbf72a389b80bead9e1184d2bc8b1c1e894 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Fri, 7 Feb 2025 17:08:56 +0000 Subject: [PATCH] CFG-based lifetime analysis using existing annotations --- .../Analysis/Analyses/DanglingReference.h | 27 + clang/include/clang/Basic/DiagnosticGroups.td | 7 + .../clang/Basic/DiagnosticSemaKinds.td | 19 + clang/lib/Analysis/CMakeLists.txt | 1 + clang/lib/Analysis/DanglingReference.cpp | 878 ++++++++++++++++++ clang/lib/Sema/AnalysisBasedWarnings.cpp | 49 + clang/test/Sema/Inputs/lifetime-analysis.h | 1 + .../test/Sema/warn-lifetime-analysis-cfg.cpp | 480 ++++++++++ .../Sema/warn-lifetime-analysis-nocfg.cpp | 104 ++- 9 files changed, 1534 insertions(+), 32 deletions(-) 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..2a6f5c88ed7341f --- /dev/null +++ b/clang/include/clang/Analysis/Analyses/DanglingReference.h @@ -0,0 +1,27 @@ +#ifndef LLVM_CLANG_ANALYSIS_ANALYSES_DANGLING_REFERENCE_H +#define LLVM_CLANG_ANALYSIS_ANALYSES_DANGLING_REFERENCE_H +#include "clang/AST/Decl.h" +#include "clang/AST/DeclBase.h" +#include "clang/Analysis/AnalysisDeclContext.h" +#include "clang/Analysis/CFG.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) {} + virtual void ReportDanglingReference(const VarDecl *VD) {} + virtual void SuggestLifetimebound(const ParmVarDecl *PVD, + const Expr *RetExpr) {} +}; + +void runDanglingReferenceAnalysis(const DeclContext &dc, const CFG &cfg, + AnalysisDeclContext &ac, + DanglingReferenceReporter *reporter); + +} // 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..600dca0e4a96441 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -472,6 +472,13 @@ def Dangling : DiagGroup<"dangling", [DanglingAssignment, DanglingInitializerList, DanglingGsl, ReturnStackAddress]>; +def ReturnStackAddressCFG : DiagGroup<"return-stack-address-cfg">; +def SuggestLifetimeboundCFG : DiagGroup<"suggest-lifetimebound-cfg">; +def DanglingReferenceCFG : DiagGroup<"dangling-reference-cfg">; +def DanglingCFG + : DiagGroup<"dangling-cfg", [ReturnStackAddressCFG, DanglingReferenceCFG, + SuggestLifetimeboundCFG]>; + 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..62da2db4a1c66eb 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10169,6 +10169,25 @@ 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; +def warn_dangling_reference_cfg : Warning<"reference to local object dangles">, + InGroup<DanglingReferenceCFG>, + DefaultIgnore; // TODO: add note on loc of +def warn_suggest_lifetimebound_cfg + : Warning<"param should be marked lifetimebound">, + InGroup<SuggestLifetimeboundCFG>, + 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..8d10bbf21c2e7bb --- /dev/null +++ b/clang/lib/Analysis/DanglingReference.cpp @@ -0,0 +1,878 @@ +#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/Analyses/LiveVariables.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/FoldingSet.h" +#include "llvm/ADT/ImmutableMap.h" +#include "llvm/ADT/ImmutableSet.h" +#include "llvm/Support/Casting.h" +#include "llvm/Support/raw_ostream.h" +#include <cassert> +#include <memory> +#include <sstream> +#include <stdbool.h> +#include <string> + +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>(); + // TODO: Should we consider attribute from base class ? + if (RD->hasDefinition()) + for (auto B : RD->bases()) + Result |= isRecordWithAttr<T>(B.getType()); + return Result; +} + +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(QualType Q) { + return Q->isNullPtrType() || Q->isPointerType() || + isRecordWithAttr<PointerAttr>(Q); +} +bool isPointer(const Expr *E) { return isPointer(E->getType()); } +bool isPointer(const Decl *D) { + 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; +} + +// Returns true if the given Record decl is a form of `GSLOwner<Pointer>` +// type, e.g. std::vector<string_view>, std::optional<string_view>. +static bool isContainerOfPointer(const RecordDecl *Container) { + if (const auto *CTSD = + dyn_cast_if_present<ClassTemplateSpecializationDecl>(Container)) { + if (!CTSD->hasAttr<OwnerAttr>()) // Container must be a GSL owner type. + return false; + const auto &TAs = CTSD->getTemplateArgs(); + return TAs.size() > 0 && TAs[0].getKind() == TemplateArgument::Type && + isPointer(TAs[0].getAsType()); + } + return false; +} +static bool isContainerOfOwner(const RecordDecl *Container) { + const auto *CTSD = + dyn_cast_if_present<ClassTemplateSpecializationDecl>(Container); + if (!CTSD) + return false; + if (!CTSD->hasAttr<OwnerAttr>()) // Container must be a GSL owner type. + return false; + const auto &TAs = CTSD->getTemplateArgs(); + return TAs.size() > 0 && TAs[0].getKind() == TemplateArgument::Type && + isRecordWithAttr<OwnerAttr>(TAs[0].getAsType()); +} + +// Returns true if the given Record is `std::initializer_list<pointer>`. +static bool isStdInitializerListOfPointer(const RecordDecl *RD) { + if (const auto *CTSD = + dyn_cast_if_present<ClassTemplateSpecializationDecl>(RD)) { + const auto &TAs = CTSD->getTemplateArgs(); + return isInStlNamespace(RD) && RD->getIdentifier() && + RD->getName() == "initializer_list" && TAs.size() > 0 && + TAs[0].getKind() == TemplateArgument::Type && + isPointer(TAs[0].getAsType()); + } + return false; +} + +// Returns true if the given constructor is a copy-like constructor, such as +// `Ctor(Owner<U>&&)` or `Ctor(const Owner<U>&)`. +static bool isCopyLikeConstructor(const CXXConstructorDecl *Ctor) { + if (!Ctor || Ctor->param_size() != 1) + return false; + const auto *ParamRefType = + Ctor->getParamDecl(0)->getType()->getAs<ReferenceType>(); + if (!ParamRefType) + return false; + + // Check if the first parameter type is "Owner<U>". + if (const auto *TST = + ParamRefType->getPointeeType()->getAs<TemplateSpecializationType>()) + return TST->getTemplateName() + .getAsTemplateDecl() + ->getTemplatedDecl() + ->hasAttr<OwnerAttr>(); + return false; +} + +// Returns true if we should perform the GSL analysis on the first argument for +// the given constructor. +static bool +shouldTrackFirstArgumentForConstructor(const CXXConstructExpr *Ctor) { + if (Ctor->getNumArgs() == 0) + return false; + const auto *LHSRD = Ctor->getConstructor()->getParent(); + auto RHSArgType = Ctor->getArg(0)->getType(); + + // Case 1, construct a GSL pointer, e.g. std::string_view + // Always inspect when LHS is a pointer. + if (LHSRD->hasAttr<PointerAttr>()) + return isPointer(RHSArgType) || isOwner(RHSArgType); + if (Ctor->getConstructor()->param_empty() || !isContainerOfPointer(LHSRD)) + return false; + + // Now, the LHS is an Owner<Pointer> type, e.g., std::vector<string_view>. + // + // At a high level, we cannot precisely determine what the nested pointer + // owns. However, by analyzing the RHS owner type, we can use heuristics to + // infer ownership information. These heuristics are designed to be + // conservative, minimizing false positives while still providing meaningful + // diagnostics. + // + // While this inference isn't perfect, it helps catch common use-after-free + // patterns. + const auto *RHSRD = RHSArgType->getAsRecordDecl(); + // LHS is constructed from an intializer_list. + // + // std::initializer_list is a proxy object that provides access to the backing + // array. We perform analysis on it to determine if there are any dangling + // temporaries in the backing array. + // E.g. std::vector<string_view> abc = {string()}; + if (isStdInitializerListOfPointer(RHSRD)) + return true; + + // RHS can be a Pointer. + if (isPointer(RHSArgType)) + return true; + + // RHS must be an owner. + if (!isOwner(RHSArgType)) + return false; + + // Bail out if the RHS is Owner<Pointer>. + // + // We cannot reliably determine what the LHS nested pointer owns -- it could + // be the entire RHS or the nested pointer in RHS. To avoid false positives, + // we skip this case, such as: + // std::stack<std::string_view> s(std::deque<std::string_view>{}); + // + // TODO: this also has a false negative, it doesn't catch the case like: + // std::optional<span<int*>> os = std::vector<int*>{} + if (isContainerOfPointer(RHSRD) && LHSRD != RHSRD) + return false; + + // Assume that the nested Pointer is constructed from the nested Owner. + // E.g. std::optional<string_view> sv = std::optional<string>(s); + if (isContainerOfOwner(RHSRD)) + return true; + + // Now, the LHS is an Owner<Pointer> and the RHS is an Owner<X>, where X is + // neither an `Owner` nor a `Pointer`. + // + // Use the constructor's signature as a hint. If it is a copy-like constructor + // `Owner1<Pointer>(Owner2<X>&&)`, we assume that the nested pointer is + // constructed from X. In such cases, we do not diagnose, as `X` is not an + // owner, e.g. + // std::optional<string_view> sv = std::optional<Foo>(); + if (const auto *PrimaryCtorTemplate = + Ctor->getConstructor()->getPrimaryTemplate(); + PrimaryCtorTemplate && + isCopyLikeConstructor(dyn_cast_if_present<CXXConstructorDecl>( + PrimaryCtorTemplate->getTemplatedDecl()))) { + return false; + } + // Assume that the nested pointer is constructed from the whole RHS. + // E.g. optional<string_view> s = std::string(); + return true; +} + +bool shouldTrackAsPointer(QualType QT) { + if (isPointer(QT)) + return true; + if (auto *RD = QT->getAsRecordDecl()) + return isContainerOfPointer(RD); + return false; +} + +bool shouldTrackAsPointer(const Decl *D) { + auto *VD = dyn_cast<VarDecl>(D); + if (!VD) + return false; + return shouldTrackAsPointer(VD->getType()); +} + +bool doesParmPointsToArgument(const ParmVarDecl *PVD) { + // Prefer only pointer args. Skip containers. + // Default args live as long as the function call expr. Ignore. + return isRecordWithAttr<PointerAttr>(PVD->getType()) && !PVD->hasDefaultArg(); +} + +struct MemoryLoc { + enum Storage { + EMPTY, // Pointer is null. + STACK, // Pointer points to something on stack. + UNKNOWN_ARGUMENT, // Pointer points to an argument provided to the function. + 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 IsArgument() { return Loc == UNKNOWN_ARGUMENT; } + bool IsUnknown() { return Loc == UNKNOWN || IsArgument(); } + + const Decl *getDecl() { return D; } + const Expr *getExpr() { return MTE; } + + static MemoryLoc Argument(const Decl *D) { + return {UNKNOWN_ARGUMENT, D, nullptr}; + } + 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() const { + std::ostringstream os; + switch (Loc) { + case EMPTY: + os << "Empty"; + break; + case UNKNOWN_ARGUMENT: + os << "Argument(unknown)"; + if (auto *VD = dyn_cast_or_null<VarDecl>(D)) + os << " \"" << VD->getName().str() << "\""; + 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(); + } + + 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) = default; + + PointsToSet &operator=(const PointsToSet &P) = default; + + 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; +}; + +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(const clang::CFGBlock *B, PointsToSet Incoming, + const DeclContext &DC, LiveVariables *LV, + DanglingReferenceReporter *Reporter) + : B(B), PointsTo(Incoming), DC(DC), LV(LV), Reporter(Reporter) {} + void Handle() { + for (CFGBlock::const_iterator BI = B->begin(), BE = B->end(); BI != BE; + ++BI) { + BI->dump(); + 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 = ResolveExpr(E); + // llvm::errs() << "Points To : " << Loc.str() << "\n"; + // } + + if (auto *RS = dyn_cast<ReturnStmt>(stmt)) + DiagnoseReturnStmt(RS); + } + if (auto dtor = BI->getAs<CFGAutomaticObjDtor>()) { + DiagnoseDanglingReference(dtor->getVarDecl()); + } + } + } + PointsToSet getOutgoing() { return PointsTo; } + +private: + void DiagnoseDanglingReference(const VarDecl *VD) { + llvm::errs() << "Got destructor!\n"; + VD->dumpColor(); + for (const auto &PT : PointsTo.DeclPointsTo) { + MemoryLoc Pointee = PT.second; + if (Pointee.getDecl() != VD) + continue; + const VarDecl *Pointer = dyn_cast_or_null<VarDecl>(PT.first); + if (!Pointer) + continue; + if (LV && LV->isLive(B, Pointer)) { + // We have a use-after-free! + // TODO: Show more helpful diagnostics. + Reporter->ReportDanglingReference(VD); + return; + } + } + } + + // Diagnose possible return of stack variable. + void DiagnoseReturnStmt(const ReturnStmt *RS) { + if (!RS) + return; + const Expr *RetExpr = RS->getRetValue(); + if (!RetExpr) + return; + if (!shouldTrackAsPointer(RetExpr->getType())) + 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); + } else if (RetPointee.IsArgument()) { + auto *PVD = dyn_cast<ParmVarDecl>(RetPointee.getDecl()); + // TODO: This can be split in more granular diagnostics based on the type + // of function. Eg. Visibility of a function (private member fn, static, + // anonymous namespace Vs. public members fn, fn in headers). + // TODO: Templates should not be suggested. + // TODO: Add an infered annotation so that it is available atleast in the + // same TU. + if (!PVD->hasAttr<LifetimeBoundAttr>()) + Reporter->SuggestLifetimebound(PVD, RetExpr); + } + } + + void Handle(const Stmt *S) { + if (auto *E = dyn_cast<Expr>(S); !PointsTo.ContainsExprPointsTo(E)) + 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 (shouldTrackAsPointer(VD->getType())) { + if (PointsTo.ContainsDeclPointsTo(D)) + return; + // Pointer params are always unknown. + if (auto *PVD = dyn_cast<ParmVarDecl>(VD)) { + UpdatePointer(VD, doesParmPointsToArgument(PVD) + ? MemoryLoc::Argument(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); + } + +public: + 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 (shouldTrackAsPointer(MTE->getType())) { + Handle(MTE->getSubExpr()); + SetExprPointer(MTE, MTE->getSubExpr()); + return; + } + // We have a temporary on stack. + AddToStack(MTE); + } + + void VisitImplicitCastExpr(const ImplicitCastExpr *E) { + auto *SE = E->IgnoreImpCasts(); + Handle(SE); + SetExprPointer(E, SE); + } + + void VisitExprWithCleanups(const ExprWithCleanups *E) { + SetExprPointer(E, E->getSubExpr()); + } + 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()))) { + // FIXME: Gives a false positive: + // std::vector<int*> a = StatusOr<std::vector<int*>>{}.value(); + // 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; + } + } + } + + void VisitCXXConstructExpr(const CXXConstructExpr *CCE) { + if (shouldTrackFirstArgumentForConstructor(CCE)) { + 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_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 (!shouldTrackAsPointer(A->getType())) + return; + + if (const Decl *PointerDecl = DeclReferencedBy(A); + PointerDecl && shouldTrackAsPointer(PointerDecl)) + UpdatePointer(PointerDecl, B); + } + + // Update the contents of a Pointer. + void UpdatePointer(const Decl *PointerD, MemoryLoc ML) { + assert(shouldTrackAsPointer(PointerD)); + PointsTo.SetDeclPointer(PointerD, ML); + } + void UpdatePointer(const Decl *PointerD, const Expr *A) { + UpdatePointer(PointerD, ResolveExpr(A)); + } + + void SetExprPointer(const Expr *E, MemoryLoc ML) { + assert(!PointsTo.ContainsExprPointsTo(E)); + PointsTo.SetExprPointer(E, ML); + } + void SetExprPointer(const Expr *E, const Expr *PointeeE) { + SetExprPointer(E, ResolveExpr(PointeeE)); + } + void SetExprPointer(const Expr *E, const Decl *D) { + SetExprPointer(E, ResolveDecl(D)); + } + + MemoryLoc ResolveExpr(const Expr *E) { + if (!E) + return MemoryLoc::Empty(); + if (PointsTo.ContainsExprPointsTo(E)) { + return PointsTo.GetExprPointsTo(E); + } + 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, + // returns the memory pointed to by the pointer. + MemoryLoc ResolveDecl(const Decl *D) { + MaybeInitaliseDecl(D); + if (shouldTrackAsPointer(D)) + return ResolvePointer(D); + return ResolveNonPointer(D); + } + + MemoryLoc ResolvePointer(const Decl *D) { + auto *VD = dyn_cast<VarDecl>(D); + // TODO: Handle other decls like field. + if (!VD || !VD->hasLocalStorage()) + return MemoryLoc::Unknown(); + assert(shouldTrackAsPointer(D)); + return PointsTo.GetDeclPointsTo(D); + } + + MemoryLoc ResolveNonPointer(const Decl *D) { + assert(!shouldTrackAsPointer(D)); + if (IsOnStack(D)) { + return MemoryLoc::VarOnStack(D); + } + return MemoryLoc::Unknown(); + } + + void AddToStack(const Decl *D) { PointsTo.AddToStack(D); } + void AddToStack(const Expr *E) { + assert(isa<MaterializeTemporaryExpr>(E)); + assert(!isPointer(E)); + PointsTo.AddToStack(E); + // Add a self edge. + assert(!PointsTo.ContainsExprPointsTo(E)); + PointsTo.SetExprPointer(E, MemoryLoc::Temporary(E)); + } + bool IsOnStack(const Decl *D) { return PointsTo.IsOnStack(D); } + bool IsOnStack(const Expr *E) { return PointsTo.IsOnStack(E); } + + const clang::CFGBlock *B; + PointsToSet PointsTo; + const DeclContext &DC; + LiveVariables *LV; + DanglingReferenceReporter *Reporter; +}; + +class DanglingReferenceAnalyzer { +public: + DanglingReferenceAnalyzer(const DeclContext &DC, const CFG &cfg, + AnalysisDeclContext &AC, LiveVariables *LV, + DanglingReferenceReporter *Reporter) + : DC(DC), cfg(cfg), AC(AC), LV(LV), Reporter(Reporter) {} + void RunAnalysis() { + cfg.dump(AC.getASTContext().getLangOpts(), true); + 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(Block, IncomingPointsTo, DC, LV, Reporter); + BV.Handle(); + PointsToSets.insert({Block, BV.getOutgoing()}); + + worklist.enqueueSuccessors(Block); + Visited[BlockID] = true; + } + } + + 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()); + return Result; + } + + PointsToFactory Factory; + [[maybe_unused]] const DeclContext &DC; + const CFG &cfg; + AnalysisDeclContext &AC; + LiveVariables *LV; + DanglingReferenceReporter *Reporter; + llvm::DenseMap<const CFGBlock *, PointsToSet> PointsToSets; +}; +} // namespace + +void runDanglingReferenceAnalysis(const DeclContext &DC, const CFG &cfg, + AnalysisDeclContext &AC, + DanglingReferenceReporter *Reporter) { + std::unique_ptr<LiveVariables> LV = + LiveVariables::computeLiveness(AC, /*killAtAssign=*/false); + DanglingReferenceAnalyzer DRA(DC, cfg, AC, LV.get(), Reporter); + DRA.RunAnalysis(); +} + +} // namespace clang diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 589869d0186575b..6e931a1a3edfe81 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" @@ -2637,6 +2638,46 @@ 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(); + 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()); + } + + void ReportDanglingReference(const VarDecl *VD) override { + S.Diag(VD->getLocation(), diag::warn_dangling_reference_cfg) + << VD->getLocation(); + llvm::errs() << "Debug: Return temporary expr:\n"; + VD->getLocation().dump(S.getSourceManager()); + } + + void SuggestLifetimebound(const ParmVarDecl *PVD, + const Expr *RetExpr) override { + S.Diag(PVD->getLocation(), diag::warn_suggest_lifetimebound_cfg) + << PVD->getLocation(); + } + +private: + Sema &S; +}; +} // namespace void clang::sema::AnalysisBasedWarnings::IssueWarnings( sema::AnalysisBasedWarnings::Policy P, sema::FunctionScopeInfo *fscope, const Decl *D, QualType BlockType) { @@ -2826,6 +2867,14 @@ 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()) { + DanglingReferenceReporterImpl Reporter(S); + runDanglingReferenceAnalysis(*cast<DeclContext>(D), *cfg, AC, &Reporter); + } + } // 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/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 new file mode 100644 index 000000000000000..7f6b42be4ad44e0 --- /dev/null +++ b/clang/test/Sema/warn-lifetime-analysis-cfg.cpp @@ -0,0 +1,480 @@ +// 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(); + +int return_value(int t) { return t; } + +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; +} + +int* return_null_ptr() { + auto T = nullptr; + return T; +} + +// 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}} +} + +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::optional<std::string_view> containerOfPointers(bool cond) { + std::string s; // expected-note 5 {{reference to this stack variable is returned}} + std::string_view sv; + std::optional<std::string_view> osv1; + std::optional<std::string_view> osv2; + sv = s; + osv1 = sv; + if (cond) + return s; // expected-warning {{returning reference to a stack variable}} + if (cond) + return sv; // expected-warning {{returning reference to a stack variable}} + if (cond) + return osv1; // expected-warning {{returning reference to a stack variable}} + if (cond) + return osv2; + osv2 = osv1; + if (cond) + return osv2; // expected-warning {{returning reference to a stack variable}} + return s; // expected-warning {{returning reference to a stack variable}} +} + +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. +} + +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. +} + +namespace ViewConstructedFromNonPointerNonOwner { + const std::string &getstring(); + +struct KeyView { + KeyView(const std::string &c [[clang::lifetimebound]]); + KeyView(); +}; +struct [[gsl::Pointer]] SetView { + SetView(const KeyView &c); +}; +SetView getstringLB(const std::string &s [[clang::lifetimebound]]); + +SetView foo() { return KeyView(); } +} // namespace ViewConstructedFromNonPointerNonOwner + +namespace SuggestLifetimebound { +std::string_view return_param(std::string_view s) { // expected-warning {{param should be marked lifetimebound}} + return s; +} +std::string_view getLB(std::string_view s [[clang::lifetimebound]]); +std::string_view return_lifetimebound_call(std::string_view s) { // expected-warning {{param should be marked lifetimebound}} + return getLB(s); +} +std::string_view controlflow(std::string_view s) { // expected-warning {{param should be marked lifetimebound}} + std::string_view result = s; + result = getLB(result); + return result; +} +} // namespace SuggestLifetimebound + +namespace DanglingReferences { +void foo(bool cond) { + std::string_view live_pointer; + while (cond) { + std::string scope_local; // expected-warning {{reference to local object dangles}} + live_pointer = scope_local; + } + while (cond) { + std::string_view dead_pointer; + std::string scope_local; + dead_pointer = scope_local; + } +} +} // namespace DanglingReferences \ 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..94e8f782d080948 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() { @@ -366,7 +379,9 @@ std::vector<int>::iterator doNotInterferWithUnannotated2() { return value; } -std::vector<int>::iterator supportDerefAddrofChain(int a, std::vector<int>::iterator value) { +std::vector<int>::iterator supportDerefAddrofChain( + int a, + std::vector<int>::iterator value) { // cfg-warning {{param should be marked lifetimebound}} switch (a) { default: return value; @@ -405,12 +420,13 @@ int *supportDerefAddrofChain3(int a, std::vector<int>::iterator value) { } } -MyIntPointer handleDerivedToBaseCast1(MySpecialIntPointer ptr) { +MyIntPointer handleDerivedToBaseCast1(MySpecialIntPointer ptr) { // cfg-warning {{param should be marked lifetimebound}} return ptr; } -MyIntPointer handleDerivedToBaseCast2(MyOwnerIntPointer ptr) { +MyIntPointer handleDerivedToBaseCast2(MyOwnerIntPointer ptr) { // cfg-warning {{param should be marked lifetimebound}} 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() { @@ -569,15 +585,19 @@ struct [[gsl::Owner]] Container2 { }; std::optional<std::string_view> test3(int i) { - std::string s; + std::string s; // cfg-note {{reference to this stack variable is returned}} std::string_view sv; if (i) return s; // expected-warning {{address of stack memory associated}} - return sv; // fine + // cfg-warning@-1 {{returning reference to a stack variable}} + if (i) + return sv; // fine Container2<std::string_view> c1 = Container1<Foo>(); // no diagnostic as Foo is not an Owner. Container2<std::string_view> c2 = Container1<FooOwner>(); // expected-warning {{object backing the pointer will be destroyed}} - return GetFoo(); // fine, we don't know Foo is owner or not, be conservative. + if (i) + return GetFoo(); // fine, we don't know Foo is owner or not, be conservative. return GetFooOwner(); // expected-warning {{returning address of local temporary object}} + // cfg-warning@-1 {{returning reference to a temporary object}} } std::optional<int*> test4(int a) { @@ -607,6 +627,7 @@ std::string_view test5() { // The Owner<Pointer> doesn't own the object which its inner pointer points to. std::string_view a = StatusOr<std::string_view>().valueLB(); // OK return StatusOr<std::string_view>().valueLB(); // OK + // FIXME(cfg): Detect this! // No dangling diagnostics on non-lifetimebound methods. std::string_view b = StatusOr<std::string_view>().valueNoLB(); @@ -615,8 +636,10 @@ 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) { Span<int *> dangling = std::vector<int*>(); // expected-warning {{object backing the pointer}} + if(cond) return dangling; + dangling = std::vector<int*>(); // expected-warning {{object backing the pointer}} return v; // expected-warning {{address of stack memory}} } @@ -630,22 +653,25 @@ int* test7(StatusOr<StatusOr<int*>> aa) { } // Owner<Pointer> from Owner<Owner<Pointer>> -std::vector<int*> test8(StatusOr<std::vector<int*>> aa) { +std::vector<int*> test8(StatusOr<std::vector<int*>> aa) { // cfg-note {{reference to this stack variable is returned}} return aa.valueLB(); // OK, no pointer being construct on this case. + // FIXME(cfg): False positive! cfg-warning@-1 {{returning reference to a stack variable}} return aa.valueNoLB(); } // 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 +689,13 @@ 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}} + // cfg-warning@-1 {{returning reference to a temporary object}} + return t; } } // namespace GH100526 @@ -690,8 +718,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 +733,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 +763,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 +789,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): Track lifetimebound constructors } template<typename T> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits