https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/168344
>From c59ed89c06edc5e664b88f787e1c4521aa0c749a Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <[email protected]> Date: Wed, 26 Nov 2025 08:33:33 +0000 Subject: [PATCH] Multi-origin changes --- .../Analysis/Analyses/LifetimeSafety/Facts.h | 13 +- .../Analyses/LifetimeSafety/FactsGenerator.h | 17 +- .../Analyses/LifetimeSafety/LifetimeSafety.h | 4 +- .../Analyses/LifetimeSafety/Origins.h | 113 ++++-- clang/lib/Analysis/LifetimeSafety/Facts.cpp | 5 +- .../LifetimeSafety/FactsGenerator.cpp | 303 +++++++++----- .../LifetimeSafety/LifetimeSafety.cpp | 34 +- .../Analysis/LifetimeSafety/LiveOrigins.cpp | 20 +- .../LifetimeSafety/LoanPropagation.cpp | 3 +- clang/lib/Analysis/LifetimeSafety/Origins.cpp | 171 +++++--- clang/lib/Sema/AnalysisBasedWarnings.cpp | 3 + clang/test/Sema/warn-lifetime-safety.cpp | 368 ++++++++++++++++-- .../unittests/Analysis/LifetimeSafetyTest.cpp | 86 ++-- 13 files changed, 881 insertions(+), 259 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index b5f7f8746186a..908d2a5b8cc76 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -155,7 +155,8 @@ class OriginEscapesFact : public Fact { class UseFact : public Fact { const Expr *UseExpr; - OriginID OID; + // The origins of the expression being used. + llvm::SmallVector<OriginID, 1> OIDs; // True if this use is a write operation (e.g., left-hand side of assignment). // Write operations are exempted from use-after-free checks. bool IsWritten = false; @@ -163,10 +164,10 @@ class UseFact : public Fact { public: static bool classof(const Fact *F) { return F->getKind() == Kind::Use; } - UseFact(const Expr *UseExpr, OriginManager &OM) - : Fact(Kind::Use), UseExpr(UseExpr), OID(OM.get(*UseExpr)) {} + UseFact(const Expr *UseExpr, llvm::ArrayRef<OriginID> OIDs) + : Fact(Kind::Use), UseExpr(UseExpr), OIDs(OIDs.begin(), OIDs.end()) {} - OriginID getUsedOrigin() const { return OID; } + llvm::ArrayRef<OriginID> getUsedOrigins() const { return OIDs; } const Expr *getUseExpr() const { return UseExpr; } void markAsWritten() { IsWritten = true; } bool isWritten() const { return IsWritten; } @@ -194,8 +195,8 @@ class TestPointFact : public Fact { class FactManager { public: - void init(const CFG &Cfg) { - assert(BlockToFacts.empty() && "FactManager already initialized"); + FactManager(const AnalysisDeclContext &AC, const CFG &Cfg) + : OriginMgr(AC.getASTContext()) { BlockToFacts.resize(Cfg.getNumBlockIDs()); } diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index 878cb90b685f9..939f421505463 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -50,6 +50,11 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *MTE); private: + OriginTree *getTree(const ValueDecl &D); + OriginTree *getTree(const Expr &E); + + void flow(OriginTree *Dst, OriginTree *Src, bool Kill); + void handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds); void handleGSLPointerConstruction(const CXXConstructExpr *CCE); @@ -64,26 +69,18 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { template <typename Destination, typename Source> void flowOrigin(const Destination &D, const Source &S) { - OriginID DestOID = FactMgr.getOriginMgr().getOrCreate(D); - OriginID SrcOID = FactMgr.getOriginMgr().get(S); - CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>( - DestOID, SrcOID, /*KillDest=*/false)); + flow(getTree(D), getTree(S), /*Kill=*/false); } template <typename Destination, typename Source> void killAndFlowOrigin(const Destination &D, const Source &S) { - OriginID DestOID = FactMgr.getOriginMgr().getOrCreate(D); - OriginID SrcOID = FactMgr.getOriginMgr().get(S); - CurrentBlockFacts.push_back( - FactMgr.createFact<OriginFlowFact>(DestOID, SrcOID, /*KillDest=*/true)); + flow(getTree(D), getTree(S), /*Kill=*/true); } /// Checks if the expression is a `void("__lifetime_test_point_...")` cast. /// If so, creates a `TestPointFact` and returns true. bool handleTestPoint(const CXXFunctionalCastExpr *FCE); - void handleAssignment(const Expr *LHSExpr, const Expr *RHSExpr); - // A DeclRefExpr will be treated as a use of the referenced decl. It will be // checked for use-after-free unless it is later marked as being written to // (e.g. on the left-hand side of an assignment). diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index b34a7f18b5809..a8d6e2551aab5 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -76,13 +76,13 @@ class LifetimeSafetyAnalysis { return *LoanPropagation; } LiveOriginsAnalysis &getLiveOrigins() const { return *LiveOrigins; } - FactManager &getFactManager() { return FactMgr; } + FactManager &getFactManager() { return *FactMgr; } private: AnalysisDeclContext &AC; LifetimeSafetyReporter *Reporter; LifetimeFactory Factory; - FactManager FactMgr; + std::unique_ptr<FactManager> FactMgr; std::unique_ptr<LiveOriginsAnalysis> LiveOrigins; std::unique_ptr<LoanPropagationAnalysis> LoanPropagation; }; diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h index 56b9010f41fa2..31074535979d6 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h @@ -28,12 +28,10 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, OriginID ID) { /// An Origin is a symbolic identifier that represents the set of possible /// loans a pointer-like object could hold at any given time. -/// TODO: Enhance the origin model to handle complex types, pointer -/// indirection and reborrowing. The plan is to move from a single origin per -/// variable/expression to a "list of origins" governed by the Type. -/// For example, the type 'int**' would have two origins. -/// See discussion: -/// https://github.com/llvm/llvm-project/pull/142313/commits/0cd187b01e61b200d92ca0b640789c1586075142#r2137644238 +/// +/// Each Origin corresponds to a single level of indirection. For complex types +/// with multiple levels of indirection (e.g., `int**`), multiple Origins are +/// organized into an OriginTree structure (see below). struct Origin { OriginID ID; /// A pointer to the AST node that this origin represents. This union @@ -52,28 +50,82 @@ struct Origin { } }; +/// A tree of origins representing levels of indirection for pointer-like types. +/// +/// Each node in the tree contains an OriginID representing a level of +/// indirection. The tree structure captures the multi-level nature of +/// pointer and reference types in the lifetime analysis. +/// +/// Examples: +/// - For `int& x`, the tree has depth 2: +/// * Root: origin for the reference storage itself (the lvalue `x`) +/// * Pointee: origin for what `x` refers to +/// +/// - For `int* p`, the tree has depth 2: +/// * Root: origin for the pointer variable `p` +/// * Pointee: origin for what `p` points to +/// +/// - For `View v` (where View is gsl::Pointer), the tree has depth 2: +/// * Root: origin for the view object itself +/// * Pointee: origin for what the view refers to +/// +/// - For `int** pp`, the tree has depth 3: +/// * Root: origin for `pp` itself +/// * Pointee: origin for `*pp` (what `pp` points to) +/// * Pointee->Pointee: origin for `**pp` (what `*pp` points to) +/// +/// The tree structure enables the analysis to track how loans flow through +/// different levels of indirection when assignments and dereferences occur. +struct OriginTree { + OriginID OID; + OriginTree *Pointee = nullptr; + + OriginTree(OriginID OID) : OID(OID) {} + + size_t getDepth() const { + size_t Depth = 1; + const OriginTree *T = this; + while (T->Pointee) { + T = T->Pointee; + Depth++; + } + return Depth; + } +}; + +bool isPointerLikeType(QualType QT); +/// Returns true if the declaration has its own storage that can be borrowed. +/// References have no storage - they are aliases to other storage. +bool doesDeclHaveStorage(const ValueDecl *D); + /// Manages the creation, storage, and retrieval of origins for pointer-like /// variables and expressions. class OriginManager { public: - OriginManager() = default; - - Origin &addOrigin(OriginID ID, const clang::ValueDecl &D); - Origin &addOrigin(OriginID ID, const clang::Expr &E); - - // TODO: Mark this method as const once we remove the call to getOrCreate. - OriginID get(const Expr &E); - - OriginID get(const ValueDecl &D); - - OriginID getOrCreate(const Expr &E); + explicit OriginManager(ASTContext &AST) : AST(AST) {} + + /// Gets or creates the OriginTree for a given ValueDecl. + /// + /// Creates a tree structure mirroring the levels of indirection in the + /// declaration's type (e.g., `int** p` creates depth 2). + /// + /// \returns The OriginTree, or nullptr if the type is not pointer-like. + OriginTree *getOrCreateTree(const ValueDecl *D); + + /// Gets or creates the OriginTree for a given Expr. + /// + /// Creates a tree based on the expression's type and value category: + /// - Lvalues get an implicit reference level (modeling addressability) + /// - Rvalues of non-pointer type return nullptr (no trackable origin) + /// - DeclRefExpr may reuse the underlying declaration's tree + /// + /// \returns The OriginTree, or nullptr for non-pointer rvalues. + OriginTree *getOrCreateTree(const Expr *E); const Origin &getOrigin(OriginID ID) const; llvm::ArrayRef<Origin> getOrigins() const { return AllOrigins; } - OriginID getOrCreate(const ValueDecl &D); - unsigned getNumOrigins() const { return NextOriginID.Value; } void dump(OriginID OID, llvm::raw_ostream &OS) const; @@ -81,12 +133,29 @@ class OriginManager { private: OriginID getNextOriginID() { return NextOriginID++; } + OriginTree *createNode(const ValueDecl *D) { + OriginID NewID = getNextOriginID(); + AllOrigins.emplace_back(NewID, D); + return new (TreeAllocator.Allocate<OriginTree>()) OriginTree(NewID); + } + + OriginTree *createNode(const Expr *E) { + OriginID NewID = getNextOriginID(); + AllOrigins.emplace_back(NewID, E); + return new (TreeAllocator.Allocate<OriginTree>()) OriginTree(NewID); + } + + template <typename T> + OriginTree *buildTreeForType(QualType QT, const T *Node); + + ASTContext &AST; OriginID NextOriginID{0}; - /// TODO(opt): Profile and evaluate the usefullness of small buffer + /// TODO(opt): Profile and evaluate the usefulness of small buffer /// optimisation. llvm::SmallVector<Origin> AllOrigins; - llvm::DenseMap<const clang::ValueDecl *, OriginID> DeclToOriginID; - llvm::DenseMap<const clang::Expr *, OriginID> ExprToOriginID; + llvm::BumpPtrAllocator TreeAllocator; + llvm::DenseMap<const clang::ValueDecl *, OriginTree *> DeclToTree; + llvm::DenseMap<const clang::Expr *, OriginTree *> ExprToTree; }; } // namespace clang::lifetimes::internal diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 0ae7111c489e8..99cefbd716c58 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -53,7 +53,10 @@ void OriginEscapesFact::dump(llvm::raw_ostream &OS, const LoanManager &, void UseFact::dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM) const { OS << "Use ("; - OM.dump(getUsedOrigin(), OS); + for (OriginID OID : getUsedOrigins()) { + OM.dump(OID, OS); + OS << ", "; + } OS << ", " << (isWritten() ? "Write" : "Read") << ")\n"; } diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 00870c3fd4086..d6229caa7929a 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -6,25 +6,44 @@ // //===----------------------------------------------------------------------===// +#include <cassert> +#include <string> + +#include "clang/AST/OperationKinds.h" #include "clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h" #include "clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h" #include "clang/Analysis/Analyses/PostOrderCFGView.h" #include "llvm/Support/Casting.h" +#include "llvm/Support/Signals.h" #include "llvm/Support/TimeProfiler.h" namespace clang::lifetimes::internal { using llvm::isa_and_present; -static bool isPointerType(QualType QT) { - return QT->isPointerOrReferenceType() || isGslPointerType(QT); -} // Check if a type has an origin. static bool hasOrigin(const Expr *E) { - return E->isGLValue() || isPointerType(E->getType()); + return E->isGLValue() || isPointerLikeType(E->getType()); +} + +OriginTree *FactsGenerator::getTree(const ValueDecl &D) { + return FactMgr.getOriginMgr().getOrCreateTree(&D); +} +OriginTree *FactsGenerator::getTree(const Expr &E) { + return FactMgr.getOriginMgr().getOrCreateTree(&E); } -static bool hasOrigin(const VarDecl *VD) { - return isPointerType(VD->getType()); +void FactsGenerator::flow(OriginTree *Dst, OriginTree *Src, bool Kill) { + if (!Dst) + return; + assert(Dst->getDepth() == Src->getDepth() && + "Trees must have the same shape"); + + while (Dst && Src) { + CurrentBlockFacts.push_back( + FactMgr.createFact<OriginFlowFact>(Dst->OID, Src->OID, Kill)); + Dst = Dst->Pointee; + Src = Src->Pointee; + } } /// Creates a loan for the storage path of a given declaration reference. @@ -64,29 +83,43 @@ void FactsGenerator::run() { void FactsGenerator::VisitDeclStmt(const DeclStmt *DS) { for (const Decl *D : DS->decls()) if (const auto *VD = dyn_cast<VarDecl>(D)) - if (hasOrigin(VD)) - if (const Expr *InitExpr = VD->getInit()) - killAndFlowOrigin(*VD, *InitExpr); + if (const Expr *InitExpr = VD->getInit()) { + OriginTree *VDTree = getTree(*VD); + if (!VDTree) + continue; + OriginTree *InitTree = getTree(*InitExpr); + assert(InitTree && "VarDecl had origins but InitExpr did not"); + // Special handling for rvalue references initialized with xvalues. + // For declarations like `Ranges&& r = std::move(ranges);`, the rvalue + // reference should directly refer to the object being moved from, + // rather than creating a new indirection level. We skip the outer + // reference level and flow the pointee origins directly. + if (VD->getType()->isRValueReferenceType() && InitExpr->isXValue()) { + flow(VDTree->Pointee, InitTree->Pointee, /*Kill=*/true); + continue; + } + flow(VDTree, InitTree, /*Kill=*/true); + } } void FactsGenerator::VisitDeclRefExpr(const DeclRefExpr *DRE) { + // Skip function references and PR values. + if (DRE->getFoundDecl()->isFunctionOrFunctionTemplate() || !DRE->isGLValue()) + return; handleUse(DRE); - // For non-pointer/non-view types, a reference to the variable's storage - // is a borrow. We create a loan for it. - // For pointer/view types, we stick to the existing model for now and do - // not create an extra origin for the l-value expression itself. - - // TODO: A single origin for a `DeclRefExpr` for a pointer or view type is - // not sufficient to model the different levels of indirection. The current - // single-origin model cannot distinguish between a loan to the variable's - // storage and a loan to what it points to. A multi-origin model would be - // required for this. - if (!isPointerType(DRE->getType())) { - if (const Loan *L = createLoan(FactMgr, DRE)) { - OriginID ExprOID = FactMgr.getOriginMgr().getOrCreate(*DRE); - CurrentBlockFacts.push_back( - FactMgr.createFact<IssueFact>(L->ID, ExprOID)); - } + // For pointer/view types, handleUse tracks all levels of indirection through + // the OriginTree structure. + // + // For non-pointer/non-reference types (e.g., `int x`), taking the address + // creates a borrow of the variable's storage. We issue a loan for this case. + if (doesDeclHaveStorage(DRE->getDecl())) { + const Loan *L = createLoan(FactMgr, DRE); + assert(L); + OriginTree *tree = getTree(*DRE); + assert(tree && + "gl-value DRE of non-pointer type should have an origin tree"); + CurrentBlockFacts.push_back( + FactMgr.createFact<IssueFact>(L->ID, tree->OID)); } } @@ -100,12 +133,14 @@ void FactsGenerator::VisitCXXConstructExpr(const CXXConstructExpr *CCE) { void FactsGenerator::VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) { // Specifically for conversion operators, // like `std::string_view p = std::string{};` - if (isGslPointerType(MCE->getType()) && - isa_and_present<CXXConversionDecl>(MCE->getCalleeDecl())) { + if (isa_and_present<CXXConversionDecl>(MCE->getCalleeDecl()) && + isGslPointerType(MCE->getType()) && + isGslOwnerType(MCE->getImplicitObjectArgument()->getType())) { // The argument is the implicit object itself. handleFunctionCall(MCE, MCE->getMethodDecl(), {MCE->getImplicitObjectArgument()}, /*IsGslConstruction=*/true); + return; } if (const CXXMethodDecl *Method = MCE->getMethodDecl()) { // Construct the argument list, with the implicit 'this' object as the @@ -127,24 +162,46 @@ void FactsGenerator::VisitCXXNullPtrLiteralExpr( const CXXNullPtrLiteralExpr *N) { /// TODO: Handle nullptr expr as a special 'null' loan. Uninitialized /// pointers can use the same type of loan. - FactMgr.getOriginMgr().getOrCreate(*N); + getTree(*N); } void FactsGenerator::VisitImplicitCastExpr(const ImplicitCastExpr *ICE) { - if (!hasOrigin(ICE)) + OriginTree *Dest = getTree(*ICE); + if (!Dest) return; - // An ImplicitCastExpr node itself gets an origin, which flows from the - // origin of its sub-expression (after stripping its own parens/casts). - killAndFlowOrigin(*ICE, *ICE->getSubExpr()); + OriginTree *SrcTree = getTree(*ICE->getSubExpr()); + + if (ICE->getCastKind() == CK_LValueToRValue) { + // TODO: Decide what to do for x-values here. + if (!ICE->getSubExpr()->isLValue()) + return; + + assert(SrcTree && "LValue being cast to RValue has no origin tree"); + // The result of an LValue-to-RValue cast on a reference-to-pointer like + // has the inner origin. Get rid of the outer origin. + flow(getTree(*ICE), SrcTree->Pointee, /*Kill=*/true); + return; + } + if (ICE->getCastKind() == CK_NullToPointer) { + getTree(*ICE); + // TODO: Flow into them a null origin. + return; + } + if (ICE->getCastKind() == CK_NoOp || + ICE->getCastKind() == CK_ConstructorConversion || + ICE->getCastKind() == CK_UserDefinedConversion) + flow(Dest, SrcTree, /*Kill=*/true); + if (ICE->getCastKind() == CK_FunctionToPointerDecay || + ICE->getCastKind() == CK_BuiltinFnToFnPtr || + ICE->getCastKind() == CK_ArrayToPointerDecay) { + // Ignore function-to-pointer decays. + return; + } } void FactsGenerator::VisitUnaryOperator(const UnaryOperator *UO) { if (UO->getOpcode() == UO_AddrOf) { const Expr *SubExpr = UO->getSubExpr(); - // Taking address of a pointer-type expression is not yet supported and - // will be supported in multi-origin model. - if (isPointerType(SubExpr->getType())) - return; // The origin of an address-of expression (e.g., &x) is the origin of // its sub-expression (x). This fact will cause the dataflow analysis // to propagate any loans held by the sub-expression's origin to the @@ -155,20 +212,35 @@ void FactsGenerator::VisitUnaryOperator(const UnaryOperator *UO) { void FactsGenerator::VisitReturnStmt(const ReturnStmt *RS) { if (const Expr *RetExpr = RS->getRetValue()) { - if (hasOrigin(RetExpr)) { - OriginID OID = FactMgr.getOriginMgr().getOrCreate(*RetExpr); - EscapesInCurrentBlock.push_back( - FactMgr.createFact<OriginEscapesFact>(OID, RetExpr)); - } + if (OriginTree *Tree = getTree(*RetExpr)) + for (OriginTree *T = Tree; T; T = T->Pointee) + EscapesInCurrentBlock.push_back( + FactMgr.createFact<OriginEscapesFact>(T->OID, RetExpr)); } } void FactsGenerator::VisitBinaryOperator(const BinaryOperator *BO) { - if (BO->isAssignmentOp()) - handleAssignment(BO->getLHS(), BO->getRHS()); + if (BO->isCompoundAssignmentOp()) + return; + if (BO->isAssignmentOp()) { + const Expr *LHSExpr = BO->getLHS(); + const Expr *RHSExpr = BO->getRHS(); + + if (const auto *DRE_LHS = + dyn_cast<DeclRefExpr>(LHSExpr->IgnoreParenImpCasts())) { + OriginTree *LHSTree = getTree(*DRE_LHS); + OriginTree *RHSTree = getTree(*RHSExpr); + // TODO: Handle reference types. + markUseAsWrite(DRE_LHS); + // Kill the old loans of the destination origin and flow the new loans + // from the source origin. + flow(LHSTree->Pointee, RHSTree, /*Kill=*/true); + } + } } void FactsGenerator::VisitConditionalOperator(const ConditionalOperator *CO) { + if (hasOrigin(CO)) { // Merge origins from both branches of the conditional operator. // We kill to clear the initial state and merge both origins into it. @@ -180,8 +252,26 @@ void FactsGenerator::VisitConditionalOperator(const ConditionalOperator *CO) { void FactsGenerator::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) { // Assignment operators have special "kill-then-propagate" semantics // and are handled separately. - if (OCE->isAssignmentOp() && OCE->getNumArgs() == 2) { - handleAssignment(OCE->getArg(0), OCE->getArg(1)); + if (OCE->getOperator() == OO_Equal && OCE->getNumArgs() == 2) { + + const Expr *LHSExpr = OCE->getArg(0); + const Expr *RHSExpr = OCE->getArg(1); + + if (const auto *DRE_LHS = + dyn_cast<DeclRefExpr>(LHSExpr->IgnoreParenImpCasts())) { + OriginTree *LHSTree = getTree(*DRE_LHS); + OriginTree *RHSTree = getTree(*RHSExpr); + + // TODO: Doc why. + // Construction of GSL! View &(const View &). + if (RHSExpr->isGLValue()) + RHSTree = RHSTree->Pointee; + // TODO: Handle reference types. + markUseAsWrite(DRE_LHS); + // Kill the old loans of the destination origin and flow the new loans + // from the source origin. + flow(LHSTree->Pointee, RHSTree, /*Kill=*/true); + } return; } handleFunctionCall(OCE, OCE->getDirectCallee(), @@ -210,11 +300,24 @@ void FactsGenerator::VisitInitListExpr(const InitListExpr *ILE) { void FactsGenerator::VisitMaterializeTemporaryExpr( const MaterializeTemporaryExpr *MTE) { - if (!hasOrigin(MTE)) + OriginTree *MTETree = getTree(*MTE); + OriginTree *SubExprTree = getTree(*MTE->getSubExpr()); + if (!MTETree) return; - // A temporary object's origin is the same as the origin of the - // expression that initializes it. - killAndFlowOrigin(*MTE, *MTE->getSubExpr()); + if (MTE->isGLValue()) { + assert(!SubExprTree || + MTETree->getDepth() == SubExprTree->getDepth() + 1 && "todo doc."); + // Issue a loan to the MTE. + // const Loan *L = createLoan(FactMgr, MTE); + // CurrentBlockFacts.push_back( + // FactMgr.createFact<IssueFact>(L->ID, MTETree->OID)); + if (SubExprTree) + flow(MTETree->Pointee, SubExprTree, /*Kill=*/true); + } else { + assert(MTE->isXValue()); + flow(MTETree, SubExprTree, /*Kill=*/true); + } + // TODO: MTE top level origin should contain a loan to the MTE itself. } void FactsGenerator::handleLifetimeEnds(const CFGLifetimeEnds &LifetimeEnds) { @@ -237,13 +340,24 @@ void FactsGenerator::handleGSLPointerConstruction(const CXXConstructExpr *CCE) { assert(isGslPointerType(CCE->getType())); if (CCE->getNumArgs() != 1) return; - if (hasOrigin(CCE->getArg(0))) - killAndFlowOrigin(*CCE, *CCE->getArg(0)); - else + + if (isGslPointerType(CCE->getArg(0)->getType())) { + OriginTree *ArgTree = getTree(*CCE->getArg(0)); + assert(ArgTree && "GSL pointer argument should have an origin tree"); + // GSL pointer is constructed from another gsl pointer. + // Example: + // View(View v); + // View(const View &v); + if (ArgTree->getDepth() == 2) + ArgTree = ArgTree->Pointee; + flow(getTree(*CCE), ArgTree, /*Kill=*/true); + } else { // This could be a new borrow. + // TODO: Add code example here. handleFunctionCall(CCE, CCE->getConstructor(), {CCE->getArgs(), CCE->getNumArgs()}, /*IsGslConstruction=*/true); + } } /// Checks if a call-like expression creates a borrow by passing a value to a @@ -254,8 +368,9 @@ void FactsGenerator::handleFunctionCall(const Expr *Call, const FunctionDecl *FD, ArrayRef<const Expr *> Args, bool IsGslConstruction) { + OriginTree *CallTree = getTree(*Call); // Ignore functions returning values with no origin. - if (!FD || !hasOrigin(Call)) + if (!FD || !CallTree) return; auto IsArgLifetimeBound = [FD](unsigned I) -> bool { const ParmVarDecl *PVD = nullptr; @@ -268,22 +383,41 @@ void FactsGenerator::handleFunctionCall(const Expr *Call, // For explicit arguments, find the corresponding parameter // declaration. PVD = Method->getParamDecl(I - 1); - } else if (I < FD->getNumParams()) + } else if (I < FD->getNumParams()) { // For free functions or static methods. PVD = FD->getParamDecl(I); + } return PVD ? PVD->hasAttr<clang::LifetimeBoundAttr>() : false; }; if (Args.empty()) return; - bool killedSrc = false; - for (unsigned I = 0; I < Args.size(); ++I) - if (IsGslConstruction || IsArgLifetimeBound(I)) { - if (!killedSrc) { - killedSrc = true; - killAndFlowOrigin(*Call, *Args[I]); - } else - flowOrigin(*Call, *Args[I]); + bool KillSrc = true; + for (unsigned I = 0; I < Args.size(); ++I) { + OriginTree *ArgTree = getTree(*Args[I]); + if (!ArgTree) + continue; + if (IsGslConstruction) { + // TODO: document with code example. + // std::string_view(const std::string_view& from) + if (isGslPointerType(Args[I]->getType()) && Args[I]->isGLValue()) { + assert(ArgTree->getDepth() >= 2); + ArgTree = ArgTree->Pointee; + } + if (isGslOwnerType(Args[I]->getType())) { + // GSL construction creates a view that borrows from arguments. + // This implies flowing origins through the tree structure. + flow(CallTree, ArgTree, KillSrc); + KillSrc = false; + } + } else if (IsArgLifetimeBound(I)) { + // Lifetimebound on a non-GSL-ctor function means the returned + // pointer/reference itself must not outlive the arguments. This + // only constraints the top-level origin. + CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>( + CallTree->OID, ArgTree->OID, KillSrc)); + KillSrc = false; } + } } /// Checks if the expression is a `void("__lifetime_test_point_...")` cast. @@ -307,39 +441,34 @@ bool FactsGenerator::handleTestPoint(const CXXFunctionalCastExpr *FCE) { return false; } -void FactsGenerator::handleAssignment(const Expr *LHSExpr, - const Expr *RHSExpr) { - if (!hasOrigin(LHSExpr)) - return; - // Find the underlying variable declaration for the left-hand side. - if (const auto *DRE_LHS = - dyn_cast<DeclRefExpr>(LHSExpr->IgnoreParenImpCasts())) { - markUseAsWrite(DRE_LHS); - if (const auto *VD_LHS = dyn_cast<ValueDecl>(DRE_LHS->getDecl())) { - // Kill the old loans of the destination origin and flow the new loans - // from the source origin. - killAndFlowOrigin(*VD_LHS, *RHSExpr); - } - } -} - // A DeclRefExpr will be treated as a use of the referenced decl. It will be // checked for use-after-free unless it is later marked as being written to // (e.g. on the left-hand side of an assignment). void FactsGenerator::handleUse(const DeclRefExpr *DRE) { - if (isPointerType(DRE->getType())) { - UseFact *UF = FactMgr.createFact<UseFact>(DRE, FactMgr.getOriginMgr()); - CurrentBlockFacts.push_back(UF); - assert(!UseFacts.contains(DRE)); - UseFacts[DRE] = UF; + OriginTree *Tree = getTree(*DRE); + if (!Tree) + return; + // Remove the outer layer of origin which borrows from the decl directly. This + // is a use of the underlying decl. + Tree = Tree->Pointee; + // Skip if there is no inner origin (e.g., when it is not a pointer type). + if (!Tree) + return; + llvm::SmallVector<OriginID, 1> UsedOrigins; + OriginTree *T = Tree; + while (T) { + UsedOrigins.push_back(T->OID); + T = T->Pointee; } + UseFact *UF = FactMgr.createFact<UseFact>(DRE, UsedOrigins); + CurrentBlockFacts.push_back(UF); + assert(!UseFacts.contains(DRE)); + UseFacts[DRE] = UF; } void FactsGenerator::markUseAsWrite(const DeclRefExpr *DRE) { - if (!isPointerType(DRE->getType())) - return; - assert(UseFacts.contains(DRE)); - UseFacts[DRE]->markAsWritten(); + if (UseFacts.contains(DRE)) + UseFacts[DRE]->markAsWritten(); } } // namespace clang::lifetimes::internal diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp index a51ba4280f284..50111f8ef1fba 100644 --- a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp @@ -31,6 +31,19 @@ namespace clang::lifetimes { namespace internal { +static void DebugOnlyFunction(AnalysisDeclContext &AC, const CFG &Cfg, + FactManager &FactMgr) { + std::string Name; + if (const Decl *D = AC.getDecl()) { + if (const auto *ND = dyn_cast<NamedDecl>(D)) + Name = ND->getQualifiedNameAsString(); + }; + DEBUG_WITH_TYPE(Name.c_str(), AC.getDecl()->dumpColor()); + DEBUG_WITH_TYPE(Name.c_str(), Cfg.dump(AC.getASTContext().getLangOpts(), + /*ShowColors=*/true)); + DEBUG_WITH_TYPE(Name.c_str(), FactMgr.dump(Cfg, AC)); +} + LifetimeSafetyAnalysis::LifetimeSafetyAnalysis(AnalysisDeclContext &AC, LifetimeSafetyReporter *Reporter) : AC(AC), Reporter(Reporter) {} @@ -41,11 +54,18 @@ void LifetimeSafetyAnalysis::run() { const CFG &Cfg = *AC.getCFG(); DEBUG_WITH_TYPE("PrintCFG", Cfg.dump(AC.getASTContext().getLangOpts(), /*ShowColors=*/true)); - FactMgr.init(Cfg); - FactsGenerator FactGen(FactMgr, AC); + FactMgr = std::make_unique<FactManager>(AC, Cfg); + + FactsGenerator FactGen(*FactMgr, AC); FactGen.run(); - DEBUG_WITH_TYPE("LifetimeFacts", FactMgr.dump(Cfg, AC)); + + DEBUG_WITH_TYPE("LifetimeFacts", FactMgr->dump(Cfg, AC)); + + // Debug print facts for a specific function using + // -debug-only=EnableFilterByFunctionName,YourFunctionNameFoo + DEBUG_WITH_TYPE("EnableFilterByFunctionName", + DebugOnlyFunction(AC, Cfg, *FactMgr)); /// TODO(opt): Consider optimizing individual blocks before running the /// dataflow analysis. @@ -59,14 +79,14 @@ void LifetimeSafetyAnalysis::run() { /// 3. Collapse ExpireFacts belonging to same source location into a single /// Fact. LoanPropagation = std::make_unique<LoanPropagationAnalysis>( - Cfg, AC, FactMgr, Factory.OriginMapFactory, Factory.LoanSetFactory); + Cfg, AC, *FactMgr, Factory.OriginMapFactory, Factory.LoanSetFactory); LiveOrigins = std::make_unique<LiveOriginsAnalysis>( - Cfg, AC, FactMgr, Factory.LivenessMapFactory); + Cfg, AC, *FactMgr, Factory.LivenessMapFactory); DEBUG_WITH_TYPE("LiveOrigins", - LiveOrigins->dump(llvm::dbgs(), FactMgr.getTestPoints())); + LiveOrigins->dump(llvm::dbgs(), FactMgr->getTestPoints())); - runLifetimeChecker(*LoanPropagation, *LiveOrigins, FactMgr, AC, Reporter); + runLifetimeChecker(*LoanPropagation, *LiveOrigins, *FactMgr, AC, Reporter); } } // namespace internal diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index 57338122b4440..e167d21260b36 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -121,13 +121,19 @@ class AnalysisImpl /// dominates this program point. A write operation kills the liveness of /// the origin since it overwrites the value. Lattice transfer(Lattice In, const UseFact &UF) { - OriginID OID = UF.getUsedOrigin(); - // Write kills liveness. - if (UF.isWritten()) - return Lattice(Factory.remove(In.LiveOrigins, OID)); - // Read makes origin live with definite confidence (dominates this point). - return Lattice(Factory.add(In.LiveOrigins, OID, - LivenessInfo(&UF, LivenessKind::Must))); + Lattice Out = In; + for (OriginID OID : UF.getUsedOrigins()) { + // Write kills liveness. + if (UF.isWritten()) { + Out = Lattice(Factory.remove(Out.LiveOrigins, OID)); + } else { + // Read makes origin live with definite confidence (dominates this + // point). + Out = Lattice(Factory.add(Out.LiveOrigins, OID, + LivenessInfo(&UF, LivenessKind::Must))); + } + } + return Out; } /// An escaping origin (e.g., via return) makes the origin live with definite diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 23ce1b78dfde2..e86ea7e2731b0 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -59,7 +59,8 @@ static llvm::BitVector computePersistentOrigins(const FactManager &FactMgr, break; } case Fact::Kind::Use: - CheckOrigin(F->getAs<UseFact>()->getUsedOrigin()); + for (OriginID OID : F->getAs<UseFact>()->getUsedOrigins()) + CheckOrigin(OID); break; case Fact::Kind::OriginEscapes: case Fact::Kind::Expire: diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp index 0f2eaa94a5987..b4f70efc6b5bb 100644 --- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp @@ -7,70 +7,135 @@ //===----------------------------------------------------------------------===// #include "clang/Analysis/Analyses/LifetimeSafety/Origins.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Attr.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/TypeBase.h" +#include "clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h" namespace clang::lifetimes::internal { -void OriginManager::dump(OriginID OID, llvm::raw_ostream &OS) const { - OS << OID << " ("; - Origin O = getOrigin(OID); - if (const ValueDecl *VD = O.getDecl()) - OS << "Decl: " << VD->getNameAsString(); - else if (const Expr *E = O.getExpr()) - OS << "Expr: " << E->getStmtClassName(); - else - OS << "Unknown"; - OS << ")"; +bool isPointerLikeType(QualType QT) { + return QT->isPointerOrReferenceType() || isGslPointerType(QT); +} + +static bool hasOrigins(QualType QT) { return isPointerLikeType(QT); } + +/// Determines if an expression has origins that need to be tracked. +/// +/// An expression has origins if: +/// - It's a glvalue (has addressable storage), OR +/// - Its type is pointer-like (pointer, reference, or gsl::Pointer) +/// +/// Examples: +/// - `int x; x` : has origin (glvalue) +/// - `int* p; p` : has 2 origins (1 for glvalue and 1 for pointer type) +/// - `std::string_view{}` : has 1 origin (prvalue of pointer type) +/// - `42` : no origin (prvalue of non-pointer type) +/// - `x + y` : (where x, y are int) → no origin (prvalue of non-pointer type) +static bool hasOrigins(const Expr *E) { + return E->isGLValue() || hasOrigins(E->getType()); } -Origin &OriginManager::addOrigin(OriginID ID, const clang::ValueDecl &D) { - AllOrigins.emplace_back(ID, &D); - return AllOrigins.back(); +/// Returns true if the declaration has its own storage that can be borrowed. +/// +/// References generally have no storage - they are aliases to other storage. +/// For example: +/// int x; // has storage (can issue loans to x's storage) +/// int& r = x; // no storage (r is an alias to x's storage) +/// int* p; // has storage (the pointer variable p itself has storage) +/// +/// TODO: Handle lifetime extension. References initialized by temporaries +/// can have storage when the temporary's lifetime is extended: +/// const int& r = 42; // temporary has storage, lifetime extended +/// Foo&& f = Foo{}; // temporary has storage, lifetime extended +/// Currently, this function returns false for all reference types. +bool doesDeclHaveStorage(const ValueDecl *D) { + return !D->getType()->isReferenceType(); } -Origin &OriginManager::addOrigin(OriginID ID, const clang::Expr &E) { - AllOrigins.emplace_back(ID, &E); - return AllOrigins.back(); +template <typename T> +OriginTree *OriginManager::buildTreeForType(QualType QT, const T *Node) { + assert(hasOrigins(QT) && "buildTreeForType called for non-pointer type"); + OriginTree *Root = createNode(Node); + if (QT->isPointerOrReferenceType()) { + QualType PointeeTy = QT->getPointeeType(); + // We recurse if the pointee type is pointer-like, to build the next + // level in the origin tree. E.g., for T*& / View&. + if (hasOrigins(PointeeTy)) + Root->Pointee = buildTreeForType(PointeeTy, Node); + } + return Root; } -// TODO: Mark this method as const once we remove the call to getOrCreate. -OriginID OriginManager::get(const Expr &E) { - if (auto *ParenIgnored = E.IgnoreParens(); ParenIgnored != &E) - return get(*ParenIgnored); - auto It = ExprToOriginID.find(&E); - if (It != ExprToOriginID.end()) +OriginTree *OriginManager::getOrCreateTree(const ValueDecl *D) { + if (!hasOrigins(D->getType())) + return nullptr; + auto It = DeclToTree.find(D); + if (It != DeclToTree.end()) return It->second; - // If the expression itself has no specific origin, and it's a reference - // to a declaration, its origin is that of the declaration it refers to. - // For pointer types, where we don't pre-emptively create an origin for the - // DeclRefExpr itself. - if (const auto *DRE = dyn_cast<DeclRefExpr>(&E)) - return get(*DRE->getDecl()); - // TODO: This should be an assert(It != ExprToOriginID.end()). The current - // implementation falls back to getOrCreate to avoid crashing on - // yet-unhandled pointer expressions, creating an empty origin for them. - return getOrCreate(E); + return DeclToTree[D] = buildTreeForType(D->getType(), D); } -OriginID OriginManager::get(const ValueDecl &D) { - auto It = DeclToOriginID.find(&D); - // TODO: This should be an assert(It != DeclToOriginID.end()). The current - // implementation falls back to getOrCreate to avoid crashing on - // yet-unhandled pointer expressions, creating an empty origin for them. - if (It == DeclToOriginID.end()) - return getOrCreate(D); +OriginTree *OriginManager::getOrCreateTree(const Expr *E) { + if (auto *ParenIgnored = E->IgnoreParens(); ParenIgnored != E) + return getOrCreateTree(ParenIgnored); - return It->second; -} + if (!hasOrigins(E)) + return nullptr; -OriginID OriginManager::getOrCreate(const Expr &E) { - auto It = ExprToOriginID.find(&E); - if (It != ExprToOriginID.end()) + auto It = ExprToTree.find(E); + if (It != ExprToTree.end()) return It->second; - OriginID NewID = getNextOriginID(); - addOrigin(NewID, E); - ExprToOriginID[&E] = NewID; - return NewID; + QualType Type = E->getType(); + + // Special handling for DeclRefExpr to share origins with the underlying decl. + // + // For reference-typed declarations (e.g., `int& r`), the DeclRefExpr + // directly reuses the declaration's tree since references don't add an + // extra level of indirection at the expression level. + // + // For non-reference declarations (e.g., `int* p`), the DeclRefExpr is an + // lvalue (addressable), so we create an outer origin for the lvalue itself, + // with the pointee being the declaration's tree. This models taking the + // address: `&p` borrows the storage of `p`, not what `p` points to. + // + // This ensures origin sharing: multiple DeclRefExprs to the same declaration + // share the same underlying origins. + if (auto *DRE = dyn_cast<DeclRefExpr>(E)) { + OriginTree *Root = nullptr; + if (doesDeclHaveStorage(DRE->getDecl())) { + Root = createNode(DRE); + Root->Pointee = getOrCreateTree(DRE->getDecl()); + } else + Root = getOrCreateTree(DRE->getDecl()); + return ExprToTree[E] = Root; + } + // If E is an lvalue , it refers to storage. We model this storage as the + // first level of origin tree, as if it were a reference, because l-values are + // addressable. + if (E->isGLValue() && !Type->isReferenceType()) + Type = AST.getLValueReferenceType(Type); + return ExprToTree[E] = buildTreeForType(Type, E); +} + +void OriginManager::dump(OriginID OID, llvm::raw_ostream &OS) const { + OS << OID << " ("; + Origin O = getOrigin(OID); + if (const ValueDecl *VD = O.getDecl()) { + OS << "Decl: " << VD->getNameAsString(); + } else if (const Expr *E = O.getExpr()) { + OS << "Expr: " << E->getStmtClassName(); + if (auto *DRE = dyn_cast<DeclRefExpr>(E)) { + if (const ValueDecl *VD = DRE->getDecl()) + OS << "(Decl: " << VD->getNameAsString() << ")"; + } + } else { + OS << "Unknown"; + } + OS << ")"; } const Origin &OriginManager::getOrigin(OriginID ID) const { @@ -78,14 +143,4 @@ const Origin &OriginManager::getOrigin(OriginID ID) const { return AllOrigins[ID.Value]; } -OriginID OriginManager::getOrCreate(const ValueDecl &D) { - auto It = DeclToOriginID.find(&D); - if (It != DeclToOriginID.end()) - return It->second; - OriginID NewID = getNextOriginID(); - addOrigin(NewID, D); - DeclToOriginID[&D] = NewID; - return NewID; -} - } // namespace clang::lifetimes::internal diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 43d2b9a829545..44e3503a3b2ac 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -3015,6 +3015,9 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( .setAlwaysAdd(Stmt::ImplicitCastExprClass) .setAlwaysAdd(Stmt::UnaryOperatorClass); } + if (EnableLifetimeSafetyAnalysis) { + AC.getCFGBuildOptions().AddLifetime = true; + } // Install the logical handler. std::optional<LogicalErrorHandler> LEH; diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 1191469e23df1..09a619e48f450 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -1,9 +1,13 @@ // RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety -Wexperimental-lifetime-safety -Wno-dangling -verify %s -struct MyObj { +struct View; + +struct [[gsl::Owner]] MyObj { int id; ~MyObj() {} // Non-trivial destructor MyObj operator+(MyObj); + + View getView() const [[clang::lifetimebound]]; }; struct [[gsl::Pointer()]] View { @@ -224,6 +228,16 @@ void potential_for_loop_use_after_loop_body(MyObj safe) { (void)*p; // expected-note {{later used here}} } +void safe_for_loop_gsl() { + MyObj safe; + View v = safe; + for (int i = 0; i < 1; ++i) { + MyObj s; + v = s; + v.use(); + } +} + void potential_for_loop_gsl() { MyObj safe; View v = safe; @@ -539,11 +553,28 @@ int* return_pointer_to_parameter(int a) { // expected-note@-1 {{returned here}} } -const int& return_reference_to_parameter(int a) -{ - const int &b = a; - return b; // expected-warning {{address of stack memory is returned later}} - // expected-note@-1 {{returned here}} +const int& return_reference_to_parameter(int a) { + const int &b = a; // expected-warning {{address of stack memory is returned later}} + return b; // expected-note {{returned here}} +} +int return_reference_to_parameter_no_error(int a) { + const int &b = a; + return b; +} + +const int& reference_via_conditional(int a, int b, bool cond) { + const int &c = (cond ? ((a)) : (b)); // expected-warning 2 {{address of stack memory is returned later}} + return c; // expected-note 2 {{returned here}} +} +const int* return_pointer_to_parameter_via_reference(int a, int b, bool cond) { + const int &c = cond ? a : b; // expected-warning 2 {{address of stack memory is returned later}} + const int* d = &c; + return d; // expected-note 2 {{returned here}} +} +// FIXME: Dereference of a pointer does not track the reference. +const int& return_pointer_to_parameter_via_reference_1(int a) { + const int* d = &a; + return *d; } const int& get_ref_to_local() { @@ -552,6 +583,71 @@ const int& get_ref_to_local() { // expected-note@-1 {{returned here}} } +void test_view_pointer() { + View* vp; + { + View v; + vp = &v; // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + vp->use(); // expected-note {{later used here}} +} + +void test_view_double_pointer() { + View** vpp; + { + View* vp = nullptr; + vpp = &vp; // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + (**vpp).use(); // expected-note {{later used here}} +} + +struct PtrHolder { + int* ptr; + int* const& getRef() const [[clang::lifetimebound]] { return ptr; } +}; + +int* const& test_ref_to_ptr() { + PtrHolder a; + int *const &ref = a.getRef(); // expected-warning {{address of stack memory is returned later}} + return ref; // expected-note {{returned here}} +} +int* const test_ref_to_ptr_no_error() { + PtrHolder a; + int *const &ref = a.getRef(); + return ref; +} + +int** return_inner_ptr_addr(int*** ppp [[clang::lifetimebound]]); +void test_lifetimebound_multi_level() { + int** result; + { + int* p = nullptr; + int** pp = &p; + int*** ppp = &pp; // expected-warning {{object whose reference is captured does not live long enough}} + result = return_inner_ptr_addr(ppp); + } // expected-note {{destroyed here}} + (void)**result; // expected-note {{used here}} +} + +// FIXME: Assignment does not track the dereference of a pointer. +void test_assign_through_double_ptr() { + int a = 1, b = 2; + int* p = &a; + int** pp = &p; + { + int c = 3; + *pp = &c; + } + (void)**pp; +} + +int** test_ternary_double_ptr(bool cond) { + int a = 1, b = 2; + int* pa = &a; // expected-warning {{address of stack memory is returned later}} + int* pb = &b; // expected-warning {{address of stack memory is returned later}} + int** result = cond ? &pa : &pb; // expected-warning 2 {{address of stack memory is returned later}} + return result; // expected-note 4 {{returned here}} +} //===----------------------------------------------------------------------===// // Use-After-Scope & Use-After-Return (Return-Stack-Address) Combined // These are cases where the diagnostic kind is determined by location @@ -635,13 +731,6 @@ MyObj* Identity(MyObj* v [[clang::lifetimebound]]); View Choose(bool cond, View a [[clang::lifetimebound]], View b [[clang::lifetimebound]]); MyObj* GetPointer(const MyObj& obj [[clang::lifetimebound]]); -struct [[gsl::Pointer()]] LifetimeBoundView { - LifetimeBoundView(); - LifetimeBoundView(const MyObj& obj [[clang::lifetimebound]]); - LifetimeBoundView pass() [[clang::lifetimebound]] { return *this; } - operator View() const [[clang::lifetimebound]]; -}; - void lifetimebound_simple_function() { View v; { @@ -688,25 +777,34 @@ void lifetimebound_mixed_args() { v.use(); // expected-note {{later used here}} } +struct LifetimeBoundMember { + LifetimeBoundMember(); + View get() const [[clang::lifetimebound]]; + operator View() const [[clang::lifetimebound]]; +}; + void lifetimebound_member_function() { - LifetimeBoundView lbv, lbv2; + View v; { MyObj obj; - lbv = obj; // expected-warning {{object whose reference is captured does not live long enough}} - lbv2 = lbv.pass(); - } // expected-note {{destroyed here}} - View v = lbv2; // expected-note {{later used here}} - v.use(); + v = obj.getView(); // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + v.use(); // expected-note {{later used here}} } +struct LifetimeBoundConversionView { + LifetimeBoundConversionView(); + ~LifetimeBoundConversionView(); + operator View() const [[clang::lifetimebound]]; +}; + void lifetimebound_conversion_operator() { View v; { - MyObj obj; - LifetimeBoundView lbv = obj; // expected-warning {{object whose reference is captured does not live long enough}} - v = lbv; // Conversion operator is lifetimebound - } // expected-note {{destroyed here}} - v.use(); // expected-note {{later used here}} + LifetimeBoundConversionView obj; + v = obj; // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + v.use(); // expected-note {{later used here}} } void lifetimebound_chained_calls() { @@ -747,17 +845,15 @@ void lifetimebound_partial_safety(bool cond) { v.use(); // expected-note {{later used here}} } -// FIXME: Warning should be on the 'GetObject' call, not the assignment to 'ptr'. -// The loan from the lifetimebound argument is not propagated to the call expression itself. const MyObj& GetObject(View v [[clang::lifetimebound]]); void lifetimebound_return_reference() { View v; const MyObj* ptr; { MyObj obj; - View temp_v = obj; + View temp_v = obj; // expected-warning {{object whose reference is captured does not live long enough}} const MyObj& ref = GetObject(temp_v); - ptr = &ref; // expected-warning {{object whose reference is captured does not live long enough}} + ptr = &ref; } // expected-note {{destroyed here}} (void)*ptr; // expected-note {{later used here}} } @@ -767,6 +863,7 @@ struct LifetimeBoundCtor { LifetimeBoundCtor(); LifetimeBoundCtor(const MyObj& obj [[clang::lifetimebound]]); }; + void lifetimebound_ctor() { LifetimeBoundCtor v; { @@ -943,3 +1040,218 @@ void parentheses(bool cond) { } // expected-note 4 {{destroyed here}} (void)*p; // expected-note 4 {{later used here}} } + +namespace GH162834 { +template <class T> +struct StatusOr { + ~StatusOr() {} + const T& value() const& [[clang::lifetimebound]] { return data; } + + private: + T data; +}; + +StatusOr<View> getViewOr(); +StatusOr<MyObj> getStringOr(); +StatusOr<MyObj*> getPointerOr(); + +void foo() { + View view; + { + StatusOr<View> view_or = getViewOr(); + view = view_or.value(); + } + (void)view; +} + +void bar() { + MyObj* pointer; + { + StatusOr<MyObj*> pointer_or = getPointerOr(); + pointer = pointer_or.value(); + } + (void)*pointer; +} + +void foobar() { + View view; + { + StatusOr<MyObj> string_or = getStringOr(); + view = string_or. // expected-warning {{object whose reference is captured does not live long enough}} + value(); + } // expected-note {{destroyed here}} + (void)view; // expected-note {{later used here}} +} +} // namespace GH162834 + +namespace RangeBasedForLoop { +struct MyObjStorage { + MyObj objs[1]; + MyObjStorage() {} + ~MyObjStorage() {} + const MyObj *begin() const [[clang::lifetimebound]] { return objs; } + const MyObj *end() const { return objs + 1; } +}; + +// FIXME: Detect use-after-scope. Dereference pointer does not propagate the origins. +void range_based_for_use_after_scope() { + View v; + { + MyObjStorage s; + for (const MyObj &o : s) { + v = o; + } + } + v.use(); +} +// FIXME: Detect use-after-return. Dereference pointer does not propagate the origins. +View range_based_for_use_after_return() { + MyObjStorage s; + for (const MyObj &o : s) { + return o; + } + return *s.begin(); +} + +void range_based_for_not_reference() { + View v; + { + MyObjStorage s; + for (MyObj o : s) { // expected-note {{destroyed here}} + v = o; // expected-warning {{object whose reference is captured may not live long enough}} + } + } + v.use(); // expected-note {{later used here}} +} + +void range_based_for_no_error() { + View v; + MyObjStorage s; + for (const MyObj &o : s) { + v = o; + } + v.use(); +} + +} // namespace RangeBaseForLoop + +namespace structured_binding { +struct Pair { + MyObj a; + MyObj b; + Pair() {} + ~Pair() {} +}; + +// FIXME: Detect this. +void structured_binding_use_after_scope() { + View v; + { + Pair p; + auto &[a_ref, b_ref] = p; + v = a_ref; + } + v.use(); +} +} +namespace MaxFnLifetimeBound { + +template<class T> +T&& MaxT(T&& a [[clang::lifetimebound]], T&& b [[clang::lifetimebound]]); + +const MyObj& call_max_with_obj() { + MyObj oa, ob; + return MaxT(oa, // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 2 {{returned here}} + ob); // expected-warning {{address of stack memory is returned later}} + +} + +MyObj* call_max_with_obj_error() { + MyObj oa, ob; + return &MaxT(oa, // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 2 {{returned here}} + ob); // expected-warning {{address of stack memory is returned later}} + +} + +MyObj call_max_with_obj_no_error() { + MyObj oa, ob; + return MaxT(oa, ob); +} + +const View& call_max_with_view_with_error() { + View va, vb; + return MaxT(va, // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 2 {{returned here}} + vb); // expected-warning {{address of stack memory is returned later}} +} + +struct [[gsl::Pointer]] NonTrivialPointer { ~NonTrivialPointer(); }; + +const NonTrivialPointer& call_max_with_non_trivial_view_with_error() { + NonTrivialPointer va, vb; + return MaxT(va, // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 2 {{returned here}} + vb); // expected-warning {{address of stack memory is returned later}} +} + +namespace MultiPointerTypes { +int** return_2p() { + int a = 1; + int* b = &a; // expected-warning {{address of stack memory is returned later}} + int** c = &b; // expected-warning {{address of stack memory is returned later}} + return c; // expected-note 2 {{returned here}} +} + +int** return_2p_one_is_safe(int& a) { + int* b = &a; + int** c = &b; // expected-warning {{address of stack memory is returned later}} + return c; // expected-note {{returned here}} +} + +int*** return_3p() { + int a = 1; + int* b = &a; // expected-warning {{address of stack memory is returned later}} + int** c = &b; // expected-warning {{address of stack memory is returned later}} + int*** d = &c; // expected-warning {{address of stack memory is returned later}} + return d; // expected-note 3 {{returned here}} +} + +View** return_view_p() { + MyObj a; + View b = a; // expected-warning {{address of stack memory is returned later}} + View* c = &b; // expected-warning {{address of stack memory is returned later}} + View** d = &c; // expected-warning {{address of stack memory is returned later}} + return d; // expected-note 3 {{returned here}} +} + +} // namespace MultiPointerTypes + +View call_max_with_view_without_error() { + View va, vb; + return MaxT(va, vb); +} + +} // namespace StdMaxStyleLifetimeBound + +namespace CppCoverage { + +int getInt(); + +void ReferenceParam(unsigned Value, unsigned &Ref) { + Value = getInt(); + Ref = getInt(); +} + +inline void normalize(int &exponent, int &mantissa) { + const int shift = 1; + exponent -= shift; + mantissa <<= shift; +} + +void add(int c, MyObj* node) { + MyObj* arr[10]; + arr[4] = node; +} +} // namespace CppCoverage diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index a895475013c98..892100111c768 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -32,7 +32,7 @@ class LifetimeTestRunner { std::string FullCode = R"( #define POINT(name) void("__lifetime_test_point_" #name) - struct MyObj { ~MyObj() {} int i; }; + struct [[gsl::Owner]] MyObj { ~MyObj() {} int i; }; struct [[gsl::Pointer()]] View { View(const MyObj&); @@ -103,8 +103,14 @@ class LifetimeTestHelper { // This assumes the OriginManager's `get` can find an existing origin. // We might need a `find` method on OriginManager to avoid `getOrCreate` // logic in a const-query context if that becomes an issue. - return const_cast<OriginManager &>(Analysis.getFactManager().getOriginMgr()) - .get(*VD); + OriginTree *Tree = + const_cast<OriginManager &>(Analysis.getFactManager().getOriginMgr()) + .getOrCreateTree(VD); + if (!Tree) { + ADD_FAILURE() << "No origin tree found for Var '" << VarName << "'"; + return std::nullopt; + } + return Tree->OID; } std::vector<LoanID> getLoansForVar(llvm::StringRef VarName) { @@ -826,7 +832,7 @@ TEST_F(LifetimeAnalysisTest, GslPointerWithConstAndAuto) { )"); EXPECT_THAT(Origin("v1"), HasLoansTo({"a"}, "p1")); EXPECT_THAT(Origin("v2"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("v3"), HasLoansTo({"a"}, "p1")); + EXPECT_THAT(Origin("v3"), HasLoansTo({"v2"}, "p1")); } TEST_F(LifetimeAnalysisTest, GslPointerPropagation) { @@ -879,7 +885,7 @@ TEST_F(LifetimeAnalysisTest, GslPointerConversionOperator) { StringView() = default; }; - struct String { + struct [[gsl::Owner]] String { ~String() {} operator StringView() const; }; @@ -915,24 +921,38 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundSimple) { EXPECT_THAT(Origin("v3"), HasLoansTo({"b"}, "p2")); } -TEST_F(LifetimeAnalysisTest, LifetimeboundMemberFunction) { +TEST_F(LifetimeAnalysisTest, LifetimeboundMemberFunctionOfAView) { SetupTest(R"( struct [[gsl::Pointer()]] MyView { MyView(const MyObj& o) {} - MyView pass() [[clang::lifetimebound]] { return *this; } + // + MyView& pass() [[clang::lifetimebound]] { return *this; } }; void target() { MyObj o; MyView v1 = o; POINT(p1); - MyView v2 = v1.pass(); + MyView* v2 = &v1.pass(); POINT(p2); } )"); EXPECT_THAT(Origin("v1"), HasLoansTo({"o"}, "p1")); - // The call v1.pass() is bound to 'v1'. The origin of v2 should get the loans - // from v1. - EXPECT_THAT(Origin("v2"), HasLoansTo({"o"}, "p2")); + // The call v1.pass() is bound to 'v1'. + EXPECT_THAT(Origin("v2"), HasLoansTo({"v1"}, "p2")); +} + +TEST_F(LifetimeAnalysisTest, LifetimeboundMemberFunction) { + SetupTest(R"( + struct LifetimeboundMember { + View get() [[clang::lifetimebound]]; + }; + void target() { + LifetimeboundMember o; + View v1 = o.get(); + POINT(p1); + } + )"); + EXPECT_THAT(Origin("v1"), HasLoansTo({"o"}, "p1")); } TEST_F(LifetimeAnalysisTest, LifetimeboundMultipleArgs) { @@ -1019,7 +1039,6 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundRawPointerParameter) { EXPECT_THAT(Origin("v2"), HasLoansTo({"c"}, "p3")); } -// FIXME: This can be controversial and may be revisited in the future. TEST_F(LifetimeAnalysisTest, LifetimeboundConstRefViewParameter) { SetupTest(R"( View Identity(const View& v [[clang::lifetimebound]]); @@ -1030,7 +1049,8 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundConstRefViewParameter) { POINT(p1); } )"); - EXPECT_THAT(Origin("v2"), HasLoansTo({"o"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"o"}, "p1")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"v1"}, "p1")); } TEST_F(LifetimeAnalysisTest, LifetimeboundConstRefObjParam) { @@ -1067,13 +1087,12 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundReturnReference) { EXPECT_THAT(Origin("v1"), HasLoansTo({"a"}, "p1")); EXPECT_THAT(Origin("v2"), HasLoansTo({"a"}, "p2")); - // FIXME: Handle reference types. 'v3' should have loan to 'a' instead of 'b'. - EXPECT_THAT(Origin("v3"), HasLoansTo({"b"}, "p2")); + EXPECT_THAT(Origin("v3"), HasLoansTo({"a"}, "p2")); EXPECT_THAT(Origin("v4"), HasLoansTo({"c"}, "p3")); } -TEST_F(LifetimeAnalysisTest, LifetimeboundTemplateFunction) { +TEST_F(LifetimeAnalysisTest, LifetimeboundTemplateFunctionReturnRef) { SetupTest(R"( template <typename T> const T& Identity(T&& v [[clang::lifetimebound]]); @@ -1083,32 +1102,39 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundTemplateFunction) { POINT(p1); View v2 = Identity(v1); - const View& v3 = Identity(v1); + const View& v3 = Identity(v2); POINT(p2); } )"); EXPECT_THAT(Origin("v1"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("v2"), HasLoansTo({"a"}, "p2")); - EXPECT_THAT(Origin("v3"), HasLoansTo({"a"}, "p2")); + EXPECT_THAT(Origin("v2"), HasLoansTo({}, "p2")); + EXPECT_THAT(Origin("v3"), HasLoansTo({"v2"}, "p2")); } -TEST_F(LifetimeAnalysisTest, LifetimeboundTemplateClass) { +TEST_F(LifetimeAnalysisTest, LifetimeboundTemplateFunctionReturnVal) { SetupTest(R"( - template<typename T> - struct [[gsl::Pointer()]] MyTemplateView { - MyTemplateView(const T& o) {} - MyTemplateView pass() [[clang::lifetimebound]] { return *this; } - }; + template <typename T> + T Identity(const T& v [[clang::lifetimebound]]); + void target() { - MyObj o; - MyTemplateView<MyObj> v1 = o; + MyObj a; + // FIXME: Captures a reference to temporary MyObj returned by Identity. + View v1 = Identity(a); POINT(p1); - MyTemplateView<MyObj> v2 = v1.pass(); + + MyObj b; + View v2 = b; + View v3 = Identity(v2); + const View& v4 = Identity(v3); POINT(p2); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"o"}, "p1")); - EXPECT_THAT(Origin("v2"), HasLoansTo({"o"}, "p2")); + EXPECT_THAT(Origin("v1"), HasLoansTo({}, "p1")); + + EXPECT_THAT(Origin("v2"), HasLoansTo({"b"}, "p2")); + EXPECT_THAT(Origin("v3"), HasLoansTo({"v2"}, "p2")); + // View temporary on RHS is lifetime-extended. + EXPECT_THAT(Origin("v4"), HasLoansTo({}, "p2")); } TEST_F(LifetimeAnalysisTest, LifetimeboundConversionOperator) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
