https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/168344
>From 5b9efab397d193c64374483d0bc7939c8a7a8326 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <[email protected]> Date: Mon, 17 Nov 2025 10:53:05 +0000 Subject: [PATCH] lifetime-safety-multi-origin --- .../Analysis/Analyses/LifetimeSafety/Facts.h | 9 +- .../Analyses/LifetimeSafety/FactsGenerator.h | 19 +- .../LifetimeSafety/LifetimeAnnotations.h | 5 + .../Analyses/LifetimeSafety/Origins.h | 96 +++++- clang/lib/Analysis/LifetimeSafety/Facts.cpp | 5 +- .../LifetimeSafety/FactsGenerator.cpp | 316 +++++++++++++----- .../LifetimeSafety/LifetimeAnnotations.cpp | 31 ++ .../Analysis/LifetimeSafety/LiveOrigins.cpp | 20 +- .../LifetimeSafety/LoanPropagation.cpp | 3 +- clang/lib/Analysis/LifetimeSafety/Origins.cpp | 128 ++++--- clang/lib/Sema/CheckExprLifetime.cpp | 69 ++-- clang/lib/Sema/CheckExprLifetime.h | 3 - clang/lib/Sema/SemaAttr.cpp | 4 +- clang/test/Sema/warn-lifetime-safety.cpp | 127 +++++-- .../unittests/Analysis/LifetimeSafetyTest.cpp | 5 +- 15 files changed, 582 insertions(+), 258 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index b9cad5340c940..3ae2458ebd239 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -152,7 +152,8 @@ class ReturnOfOriginFact : 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; @@ -160,10 +161,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; } diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index 4c8ab3f859a49..bf19f87f9e0b7 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 handleDestructor(const CFGAutomaticObjDtor &DtorOpt); void handleGSLPointerConstruction(const CXXConstructExpr *CCE); @@ -64,32 +69,24 @@ 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). void handleUse(const DeclRefExpr *DRE); - void markUseAsWrite(const DeclRefExpr *DRE); + void markUseAsWrite(const Expr *E); FactManager &FactMgr; AnalysisDeclContext &AC; diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h index f02969e0a9563..1a16fb82f9a84 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h @@ -38,6 +38,11 @@ bool isAssignmentOperatorLifetimeBound(const CXXMethodDecl *CMD); /// method or because it's a normal assignment operator. bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD); +// Tells whether the type is annotated with [[gsl::Pointer]]. +bool isGslPointerType(QualType QT); +// Tells whether the type is annotated with [[gsl::Owner]]. +bool isGslOwnerType(QualType QT); + } // namespace clang::lifetimes #endif // LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMEANNOTATIONS_H diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h index 56b9010f41fa2..171719785e77e 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h @@ -52,28 +52,77 @@ 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); + /// 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); + /// 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 3). + /// + /// \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, ASTContext &Ctx); 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 +130,27 @@ 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); + 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 190c038f46401..4c97577dac6db 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -53,7 +53,10 @@ void ReturnOfOriginFact::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 381ff99aae420..a6a1a3498d370 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -6,6 +6,10 @@ // //===----------------------------------------------------------------------===// +#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" @@ -15,28 +19,34 @@ namespace clang::lifetimes::internal { using llvm::isa_and_present; -static bool isGslPointerType(QualType QT) { - if (const auto *RD = QT->getAsCXXRecordDecl()) { - // We need to check the template definition for specializations. - if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) - return CTSD->getSpecializedTemplate() - ->getTemplatedDecl() - ->hasAttr<PointerAttr>(); - return RD->hasAttr<PointerAttr>(); - } - return false; +// Check if a type has an origin. +static bool hasOrigin(const Expr *E) { + return E->isGLValue() || isPointerLikeType(E->getType()); } -static bool isPointerType(QualType QT) { - return QT->isPointerOrReferenceType() || isGslPointerType(QT); +OriginTree *FactsGenerator::getTree(const ValueDecl &D) { + return FactMgr.getOriginMgr().getOrCreateTree(&D); } -// Check if a type has an origin. -static bool hasOrigin(const Expr *E) { - return E->isGLValue() || isPointerType(E->getType()); +OriginTree *FactsGenerator::getTree(const Expr &E) { + return FactMgr.getOriginMgr().getOrCreateTree(&E, AC.getASTContext()); } -static bool hasOrigin(const VarDecl *VD) { - return isPointerType(VD->getType()); +void FactsGenerator::flow(OriginTree *Dst, OriginTree *Src, bool Kill) { + if (!Dst) + return; + DEBUG_WITH_TYPE("multi", llvm::errs() + << "Dst depth = " << Dst->getDepth() << "\n"); + assert(Src); + DEBUG_WITH_TYPE("multi", llvm::errs() + << "Src depth = " << Src->getDepth() << "\n"); + assert(Dst->getDepth() == Src->getDepth() && "Trees must be 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. @@ -53,6 +63,19 @@ static const Loan *createLoan(FactManager &FactMgr, const DeclRefExpr *DRE) { } void FactsGenerator::run() { + + if (const Decl *D = AC.getDecl()) { + if (const auto *ND = dyn_cast<NamedDecl>(D)) { + DEBUG_WITH_TYPE("multi", + llvm::errs() + << "=============================================\n"); + DEBUG_WITH_TYPE("multi", llvm::dbgs() + << "Function: " + << ND->getQualifiedNameAsString() << "\n"); + // DEBUG_WITH_TYPE("multi", AC.getBody()->dumpColor()); + } + } + llvm::TimeTraceScope TimeProfile("FactGenerator"); // Iterate through the CFG blocks in reverse post-order to ensure that // initializations and destructions are processed in the correct sequence. @@ -60,10 +83,12 @@ void FactsGenerator::run() { CurrentBlockFacts.clear(); for (unsigned I = 0; I < Block->size(); ++I) { const CFGElement &Element = Block->Elements[I]; - if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>()) + if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>()) { + DEBUG_WITH_TYPE("multi", llvm::errs() << "Processing: \n"); + DEBUG_WITH_TYPE("multi", CS->getStmt()->dumpColor()); Visit(CS->getStmt()); - else if (std::optional<CFGAutomaticObjDtor> DtorOpt = - Element.getAs<CFGAutomaticObjDtor>()) + } else if (std::optional<CFGAutomaticObjDtor> DtorOpt = + Element.getAs<CFGAutomaticObjDtor>()) handleDestructor(*DtorOpt); } FactMgr.addBlockFacts(Block, CurrentBlockFacts); @@ -73,12 +98,33 @@ 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); // Dst + if (!VDTree) + continue; + OriginTree *InitTree = getTree(*InitExpr); // Src + if (!InitTree) + continue; + // TODO: Document escape hatch. + // e.g. Ranges&& constructed from move. + if (VD->getType()->isRValueReferenceType() && InitExpr->isXValue()) { + // If the variable is an rvalue reference or + // the initializer is an + flow(VDTree->Pointee, InitTree->Pointee, /*Kill=*/true); + continue; + } + // DS->dumpColor(); + // llvm::errs() << "VD: " << VD->getName() << "\n"; + // llvm::errs() << "isGslPointerType(VD->getType())" + // << isGslPointerType(VD->getType()) << "\n"; + // llvm::errs() << "VDTree depth = " << VDTree->getDepth() << "\n"; + flow(VDTree, InitTree, /*Kill=*/true); + } } void FactsGenerator::VisitDeclRefExpr(const DeclRefExpr *DRE) { + 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. @@ -90,12 +136,14 @@ void FactsGenerator::VisitDeclRefExpr(const DeclRefExpr *DRE) { // 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)); - } + if (!isPointerLikeType(DRE->getType()) && + !DRE->getDecl()->getType()->isReferenceType()) { + const Loan *L = createLoan(FactMgr, DRE); + assert(L); + OriginTree *tree = getTree(*DRE); + assert(tree && "lvalue DRE of non-pointer type should have an origin tree"); + CurrentBlockFacts.push_back( + FactMgr.createFact<IssueFact>(L->ID, tree->OID)); } } @@ -109,12 +157,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 @@ -136,15 +186,41 @@ 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) { @@ -152,7 +228,7 @@ void FactsGenerator::VisitUnaryOperator(const UnaryOperator *UO) { 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())) + if (isPointerLikeType(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 @@ -164,19 +240,34 @@ 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); - CurrentBlockFacts.push_back(FactMgr.createFact<ReturnOfOriginFact>(OID)); - } + if (OriginTree *Tree = getTree(*RetExpr)) + CurrentBlockFacts.push_back( + FactMgr.createFact<ReturnOfOriginFact>(Tree->OID)); } } 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. @@ -188,8 +279,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(), @@ -218,11 +327,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::handleDestructor(const CFGAutomaticObjDtor &DtorOpt) { @@ -251,13 +373,23 @@ 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 (hasOrigin(CCE->getArg(0))) { + // TODO: Add code example here. + OriginTree *ArgTree = getTree(*CCE->getArg(0)); + assert(ArgTree && "GSL pointer argument should have an origin tree"); + // GSL pointer is constructed from another gsl pointer. + // TODO: Add proper assertions. + 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 @@ -268,8 +400,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; @@ -282,22 +415,39 @@ 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; + } + // 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. @@ -321,36 +471,32 @@ 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; + if (!isPointerLikeType(DRE->getType())) + return; + OriginTree *tree = getTree(*DRE); + 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())) +void FactsGenerator::markUseAsWrite(const Expr *E) { + auto *DRE = dyn_cast<DeclRefExpr>(E); + if (!DRE) + return; + if (!isPointerLikeType(DRE->getType())) return; assert(UseFacts.contains(DRE)); UseFacts[DRE]->markAsWritten(); diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp index ad61a42c0eaeb..54e343fc2ee5e 100644 --- a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp @@ -10,6 +10,7 @@ #include "clang/AST/Attr.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" #include "clang/AST/Type.h" #include "clang/AST/TypeLoc.h" @@ -70,4 +71,34 @@ bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) { return isNormalAssignmentOperator(FD); } +template <typename T> static bool isRecordWithAttr(QualType Type) { + auto *RD = Type->getAsCXXRecordDecl(); + if (!RD) + return false; + // Generally, if a primary template class declaration is annotated with an + // attribute, all its specializations generated from template instantiations + // should inherit the attribute. + // + // However, since lifetime analysis occurs during parsing, we may encounter + // cases where a full definition of the specialization is not required. In + // such cases, the specialization declaration remains incomplete and lacks the + // attribute. Therefore, we fall back to checking the primary template class. + // + // Note: it is possible for a specialization declaration to have an attribute + // even if the primary template does not. + // + // FIXME: What if the primary template and explicit specialization + // declarations have conflicting attributes? We should consider diagnosing + // this scenario. + bool Result = RD->hasAttr<T>(); + + if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) + Result |= CTSD->getSpecializedTemplate()->getTemplatedDecl()->hasAttr<T>(); + + return Result; +} + +bool isGslPointerType(QualType QT) { return isRecordWithAttr<PointerAttr>(QT); } +bool isGslOwnerType(QualType QT) { return isRecordWithAttr<OwnerAttr>(QT); } + } // namespace clang::lifetimes diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index 59f594e50fb46..eb054a3cc6761 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -111,13 +111,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; } /// Issuing a new loan to an origin kills its liveness. diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 0e6c194123df8..4511572215bd6 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -62,7 +62,8 @@ static llvm::BitVector computePersistentOrigins(const FactManager &FactMgr, CheckOrigin(F->getAs<ReturnOfOriginFact>()->getReturnedOriginID()); break; case Fact::Kind::Use: - CheckOrigin(F->getAs<UseFact>()->getUsedOrigin()); + for (OriginID OID : F->getAs<UseFact>()->getUsedOrigins()) + CheckOrigin(OID); break; case Fact::Kind::Expire: case Fact::Kind::TestPoint: diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp index 0f2eaa94a5987..279e406da2de0 100644 --- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp @@ -7,70 +7,102 @@ //===----------------------------------------------------------------------===// #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 { +bool isPointerLikeType(QualType QT) { + return QT->isPointerOrReferenceType() || isGslPointerType(QT); +} + +static bool hasOrigins(QualType QT) { return isPointerLikeType(QT); } + +// TODO: Doc. +// Rvalues of non-pointer type do not have origins. +static bool hasOrigins(const Expr *E) { + return E->isGLValue() || hasOrigins(E->getType()); +} + void OriginManager::dump(OriginID OID, llvm::raw_ostream &OS) const { OS << OID << " ("; Origin O = getOrigin(OID); - if (const ValueDecl *VD = O.getDecl()) + if (const ValueDecl *VD = O.getDecl()) { OS << "Decl: " << VD->getNameAsString(); - else if (const Expr *E = O.getExpr()) + } else if (const Expr *E = O.getExpr()) { OS << "Expr: " << E->getStmtClassName(); - else + if (auto *DRE = dyn_cast<DeclRefExpr>(E)) { + if (const ValueDecl *VD = DRE->getDecl()) + OS << "(" << VD->getNameAsString() << ")"; + } + } else { OS << "Unknown"; + } OS << ")"; } -Origin &OriginManager::addOrigin(OriginID ID, const clang::ValueDecl &D) { - AllOrigins.emplace_back(ID, &D); - return AllOrigins.back(); -} - -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(isPointerLikeType(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 (isPointerLikeType(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 (!isPointerLikeType(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, ASTContext &Ctx) { + if (auto *ParenIgnored = E->IgnoreParens(); ParenIgnored != E) + return getOrCreateTree(ParenIgnored, Ctx); - 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(); + + // TODO: Doc about DeclRefExpr. + // Why does reference type does not + if (auto *DRE = dyn_cast<DeclRefExpr>(E)) { + OriginTree *Root = nullptr; + if (DRE->getDecl()->getType()->isReferenceType()) + Root = getOrCreateTree(DRE->getDecl()); + else { + // Create outer origin. Use inner tree from the underlying decl. + // This also ensures that the underlying decl's origins are always reused! + Root = createNode(DRE); + Root->Pointee = 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 = Ctx.getLValueReferenceType(Type); + ExprToTree[E] = buildTreeForType(Type, E); + return ExprToTree[E]; } const Origin &OriginManager::getOrigin(OriginID ID) const { @@ -78,14 +110,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/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index f9665b5e59831..c91ca751984c9 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -17,6 +17,9 @@ #include "llvm/ADT/PointerIntPair.h" namespace clang::sema { +using lifetimes::isGslOwnerType; +using lifetimes::isGslPointerType; + namespace { enum LifetimeKind { /// The lifetime of a temporary bound to this entity ends at the end of the @@ -257,38 +260,8 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, Expr *Init, ReferenceKind RK, LocalVisitor Visit); -template <typename T> static bool isRecordWithAttr(QualType Type) { - auto *RD = Type->getAsCXXRecordDecl(); - if (!RD) - return false; - // Generally, if a primary template class declaration is annotated with an - // attribute, all its specializations generated from template instantiations - // should inherit the attribute. - // - // However, since lifetime analysis occurs during parsing, we may encounter - // cases where a full definition of the specialization is not required. In - // such cases, the specialization declaration remains incomplete and lacks the - // attribute. Therefore, we fall back to checking the primary template class. - // - // Note: it is possible for a specialization declaration to have an attribute - // even if the primary template does not. - // - // FIXME: What if the primary template and explicit specialization - // declarations have conflicting attributes? We should consider diagnosing - // this scenario. - bool Result = RD->hasAttr<T>(); - - if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) - Result |= CTSD->getSpecializedTemplate()->getTemplatedDecl()->hasAttr<T>(); - - return Result; -} - -// Tells whether the type is annotated with [[gsl::Pointer]]. -bool isGLSPointerType(QualType QT) { return isRecordWithAttr<PointerAttr>(QT); } - static bool isPointerLikeType(QualType QT) { - return isGLSPointerType(QT) || QT->isPointerType() || QT->isNullPtrType(); + return isGslPointerType(QT) || QT->isPointerType() || QT->isNullPtrType(); } // Decl::isInStdNamespace will return false for iterators in some STL @@ -331,7 +304,7 @@ static bool isContainerOfOwner(const RecordDecl *Container) { return false; const auto &TAs = CTSD->getTemplateArgs(); return TAs.size() > 0 && TAs[0].getKind() == TemplateArgument::Type && - isRecordWithAttr<OwnerAttr>(TAs[0].getAsType()); + isGslOwnerType(TAs[0].getAsType()); } // Returns true if the given Record is `std::initializer_list<pointer>`. @@ -349,14 +322,13 @@ static bool isStdInitializerListOfPointer(const RecordDecl *RD) { static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) { if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee)) - if (isRecordWithAttr<PointerAttr>(Conv->getConversionType()) && + if (isGslPointerType(Conv->getConversionType()) && Callee->getParent()->hasAttr<OwnerAttr>()) return true; if (!isInStlNamespace(Callee->getParent())) return false; - if (!isRecordWithAttr<PointerAttr>( - Callee->getFunctionObjectParameterType()) && - !isRecordWithAttr<OwnerAttr>(Callee->getFunctionObjectParameterType())) + if (!isGslPointerType(Callee->getFunctionObjectParameterType()) && + !isGslOwnerType(Callee->getFunctionObjectParameterType())) return false; if (isPointerLikeType(Callee->getReturnType())) { if (!Callee->getIdentifier()) @@ -393,7 +365,7 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) { if (!RD->hasAttr<PointerAttr>() && !RD->hasAttr<OwnerAttr>()) return false; if (FD->getReturnType()->isPointerType() || - isRecordWithAttr<PointerAttr>(FD->getReturnType())) { + isGslPointerType(FD->getReturnType())) { return llvm::StringSwitch<bool>(FD->getName()) .Cases({"begin", "rbegin", "cbegin", "crbegin"}, true) .Cases({"end", "rend", "cend", "crend"}, true) @@ -465,7 +437,7 @@ shouldTrackFirstArgumentForConstructor(const CXXConstructExpr *Ctor) { return true; // RHS must be an owner. - if (!isRecordWithAttr<OwnerAttr>(RHSArgType)) + if (!isGslOwnerType(RHSArgType)) return false; // Bail out if the RHS is Owner<Pointer>. @@ -547,7 +519,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, // Once we initialized a value with a non gsl-owner reference, it can no // longer dangle. if (ReturnType->isReferenceType() && - !isRecordWithAttr<OwnerAttr>(ReturnType->getPointeeType())) { + !isGslOwnerType(ReturnType->getPointeeType())) { for (const IndirectLocalPathEntry &PE : llvm::reverse(Path)) { if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit || PE.Kind == IndirectLocalPathEntry::LifetimeBoundCall) @@ -1158,8 +1130,7 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path, // auto p2 = Temp().owner; // Here p2 is dangling. if (const auto *FD = llvm::dyn_cast_or_null<FieldDecl>(E.D); FD && !FD->getType()->isReferenceType() && - isRecordWithAttr<OwnerAttr>(FD->getType()) && - LK != LK_MemInitializer) { + isGslOwnerType(FD->getType()) && LK != LK_MemInitializer) { return Report; } return Abandon; @@ -1191,10 +1162,9 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path, // const GSLOwner& func(const Foo& foo [[clang::lifetimebound]]) // GSLOwner* func(cosnt Foo& foo [[clang::lifetimebound]]) // GSLPointer func(const Foo& foo [[clang::lifetimebound]]) - if (FD && - ((FD->getReturnType()->isPointerOrReferenceType() && - isRecordWithAttr<OwnerAttr>(FD->getReturnType()->getPointeeType())) || - isGLSPointerType(FD->getReturnType()))) + if (FD && ((FD->getReturnType()->isPointerOrReferenceType() && + isGslOwnerType(FD->getReturnType()->getPointeeType())) || + isGslPointerType(FD->getReturnType()))) return Report; return Abandon; @@ -1206,7 +1176,7 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path, // int &p = *localUniquePtr; // someContainer.add(std::move(localUniquePtr)); // return p; - if (!pathContainsInit(Path) && isRecordWithAttr<OwnerAttr>(L->getType())) + if (!pathContainsInit(Path) && isGslOwnerType(L->getType())) return Report; return Abandon; } @@ -1215,8 +1185,7 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path, auto *MTE = dyn_cast<MaterializeTemporaryExpr>(L); bool IsGslPtrValueFromGslTempOwner = - MTE && !MTE->getExtendingDecl() && - isRecordWithAttr<OwnerAttr>(MTE->getType()); + MTE && !MTE->getExtendingDecl() && isGslOwnerType(MTE->getType()); // Skipping a chain of initializing gsl::Pointer annotated objects. // We are looking only for the final source to find out if it was // a local or temporary owner or the address of a local @@ -1231,7 +1200,7 @@ static bool shouldRunGSLAssignmentAnalysis(const Sema &SemaRef, bool EnableGSLAssignmentWarnings = !SemaRef.getDiagnostics().isIgnored( diag::warn_dangling_lifetime_pointer_assignment, SourceLocation()); return (EnableGSLAssignmentWarnings && - (isRecordWithAttr<PointerAttr>(Entity.LHS->getType()) || + (isGslPointerType(Entity.LHS->getType()) || lifetimes::isAssignmentOperatorLifetimeBound( Entity.AssignmentOperator))); } @@ -1400,7 +1369,7 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity, // Suppress false positives for code like the one below: // Ctor(unique_ptr<T> up) : pointer(up.get()), owner(move(up)) {} // FIXME: move this logic to analyzePathForGSLPointer. - if (DRE && isRecordWithAttr<OwnerAttr>(DRE->getType())) + if (DRE && isGslOwnerType(DRE->getType())) return false; auto *VD = DRE ? dyn_cast<VarDecl>(DRE->getDecl()) : nullptr; diff --git a/clang/lib/Sema/CheckExprLifetime.h b/clang/lib/Sema/CheckExprLifetime.h index 16595d0ca1b36..38b7061988dc7 100644 --- a/clang/lib/Sema/CheckExprLifetime.h +++ b/clang/lib/Sema/CheckExprLifetime.h @@ -18,9 +18,6 @@ namespace clang::sema { -// Tells whether the type is annotated with [[gsl::Pointer]]. -bool isGLSPointerType(QualType QT); - /// Describes an entity that is being assigned. struct AssignedEntity { // The left-hand side expression of the assignment. diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 8411a3da8322d..7729c113e422e 100644 --- a/clang/lib/Sema/SemaAttr.cpp +++ b/clang/lib/Sema/SemaAttr.cpp @@ -11,11 +11,11 @@ // //===----------------------------------------------------------------------===// -#include "CheckExprLifetime.h" #include "clang/AST/ASTConsumer.h" #include "clang/AST/Attr.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/Expr.h" +#include "clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h" #include "clang/Basic/TargetInfo.h" #include "clang/Lex/Preprocessor.h" #include "clang/Sema/Lookup.h" @@ -289,7 +289,7 @@ void Sema::inferLifetimeCaptureByAttribute(FunctionDecl *FD) { // We only apply the lifetime_capture_by attribute to parameters of // pointer-like reference types (`const T&`, `T&&`). if (PVD->getType()->isReferenceType() && - sema::isGLSPointerType(PVD->getType().getNonReferenceType())) { + lifetimes::isGslPointerType(PVD->getType().getNonReferenceType())) { int CaptureByThis[] = {LifetimeCaptureByAttr::This}; PVD->addAttr( LifetimeCaptureByAttr::CreateImplicit(Context, CaptureByThis, 1)); diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index b9368db550805..d8b166336ea0f 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 -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 { @@ -220,6 +224,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; @@ -444,13 +458,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; { @@ -497,25 +504,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() { @@ -556,18 +572,18 @@ void lifetimebound_partial_safety(bool cond) { v.use(); // expected-note {{later used here}} } -// FIXME: Creating reference from lifetimebound call doesn't propagate loans. +// TODO:Extensively test references types. 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; - } - (void)*ptr; + } // expected-note {{destroyed here}} + (void)*ptr; // expected-note {{later used here}} } // FIXME: No warning for non gsl::Pointer types. Origin tracking is only supported for pointer types. @@ -575,6 +591,7 @@ struct LifetimeBoundCtor { LifetimeBoundCtor(); LifetimeBoundCtor(const MyObj& obj [[clang::lifetimebound]]); }; + void lifetimebound_ctor() { LifetimeBoundCtor v; { @@ -686,3 +703,67 @@ 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 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 601308c53f9a9..b63ddbb7367ac 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -101,8 +101,9 @@ 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); + // return const_cast<OriginManager &>(Analysis.getFactManager().getOriginMgr()) + // .get(*VD); + return std::nullopt; } std::vector<LoanID> getLoansForVar(llvm::StringRef VarName) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
