Author: Kashika Akhouri Date: 2025-11-19T17:38:02+05:30 New Revision: 5343dd92303657dc15f4038a3843ddb778760242
URL: https://github.com/llvm/llvm-project/commit/5343dd92303657dc15f4038a3843ddb778760242 DIFF: https://github.com/llvm/llvm-project/commit/5343dd92303657dc15f4038a3843ddb778760242.diff LOG: [LifetimeSafety] Detect use-after-return (#165370) Adding "use-after-return" in Lifetime Analysis. Detecting when a function returns a reference to its own stack memory: [UAR Design Doc](https://docs.google.com/document/d/1Wxjn_rJD_tuRdejP81dlb9VOckTkCq5-aE1nGcerb_o/edit?usp=sharing) Consider the following example: ```cpp std::string_view foo() { std::string_view a; std::string str = "small scoped string"; a = str; return a; } ``` The code adds a new Fact "OriginEscape" in the end of the CFG to determine any loan that is escaping the function as shown below: ``` Function: foo Block B2: End of Block Block B1: OriginFlow (Dest: 0 (Decl: a), Src: 1 (Expr: CXXConstructExpr)) OriginFlow (Dest: 2 (Expr: ImplicitCastExpr), Src: 3 (Expr: StringLiteral)) Issue (0 (Path: operator=), ToOrigin: 4 (Expr: DeclRefExpr)) OriginFlow (Dest: 5 (Expr: ImplicitCastExpr), Src: 4 (Expr: DeclRefExpr)) Use (0 (Decl: a), Write) Issue (1 (Path: str), ToOrigin: 6 (Expr: DeclRefExpr)) OriginFlow (Dest: 7 (Expr: ImplicitCastExpr), Src: 6 (Expr: DeclRefExpr)) OriginFlow (Dest: 8 (Expr: CXXMemberCallExpr), Src: 7 (Expr: ImplicitCastExpr)) OriginFlow (Dest: 9 (Expr: ImplicitCastExpr), Src: 8 (Expr: CXXMemberCallExpr)) OriginFlow (Dest: 10 (Expr: ImplicitCastExpr), Src: 9 (Expr: ImplicitCastExpr)) OriginFlow (Dest: 11 (Expr: MaterializeTemporaryExpr), Src: 10 (Expr: ImplicitCastExpr)) OriginFlow (Dest: 0 (Decl: a), Src: 11 (Expr: MaterializeTemporaryExpr)) Use (0 (Decl: a), Read) OriginFlow (Dest: 12 (Expr: ImplicitCastExpr), Src: 0 (Decl: a)) OriginFlow (Dest: 13 (Expr: CXXConstructExpr), Src: 12 (Expr: ImplicitCastExpr)) Expire (1 (Path: str)) OriginEscapes (13 (Expr: CXXConstructExpr)) End of Block Block B0: End of Block ``` The confidence of the report is determined by checking if at least one of the loans returned is not expired (strict). If all loans are expired it is considered permissive. More information [UAR Design Doc](https://docs.google.com/document/d/1Wxjn_rJD_tuRdejP81dlb9VOckTkCq5-aE1nGcerb_o/edit?usp=sharing) Added: Modified: clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Analysis/LifetimeSafety/Checker.cpp clang/lib/Analysis/LifetimeSafety/Dataflow.h clang/lib/Analysis/LifetimeSafety/Facts.cpp clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp clang/lib/Sema/AnalysisBasedWarnings.cpp clang/test/Sema/warn-lifetime-safety-dataflow.cpp clang/test/Sema/warn-lifetime-safety.cpp clang/unittests/Analysis/LifetimeSafetyTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index b9cad5340c940..b5f7f8746186a 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -42,12 +42,12 @@ class Fact { /// it. Otherwise, the source's loan set is merged into the destination's /// loan set. OriginFlow, - /// An origin escapes the function by flowing into the return value. - ReturnOfOrigin, /// An origin is used (eg. appears as l-value expression like DeclRefExpr). Use, /// A marker for a specific point in the code, for testing. TestPoint, + /// An origin that escapes the function scope (e.g., via return). + OriginEscapes, }; private: @@ -136,16 +136,19 @@ class OriginFlowFact : public Fact { const OriginManager &OM) const override; }; -class ReturnOfOriginFact : public Fact { +class OriginEscapesFact : public Fact { OriginID OID; + const Expr *EscapeExpr; public: static bool classof(const Fact *F) { - return F->getKind() == Kind::ReturnOfOrigin; + return F->getKind() == Kind::OriginEscapes; } - ReturnOfOriginFact(OriginID OID) : Fact(Kind::ReturnOfOrigin), OID(OID) {} - OriginID getReturnedOriginID() const { return OID; } + OriginEscapesFact(OriginID OID, const Expr *EscapeExpr) + : Fact(Kind::OriginEscapes), OID(OID), EscapeExpr(EscapeExpr) {} + OriginID getEscapedOriginID() const { return OID; } + const Expr *getEscapeExpr() const { return EscapeExpr; }; void dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM) const override; }; @@ -225,6 +228,9 @@ class FactManager { /// user-defined locations in the code. /// \note This is intended for testing only. llvm::StringMap<ProgramPoint> getTestPoints() const; + /// Retrieves all the facts in the block containing Program Point P. + /// \note This is intended for testing only. + llvm::ArrayRef<const Fact *> getBlockContaining(ProgramPoint P) const; unsigned getNumFacts() const { return NextFactID.Value; } diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index 4c8ab3f859a49..8ea37259c570b 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -94,6 +94,10 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { FactManager &FactMgr; AnalysisDeclContext &AC; llvm::SmallVector<Fact *> CurrentBlockFacts; + // Collect origins that escape the function in this block (OriginEscapesFact), + // appended at the end of CurrentBlockFacts to ensure they appear after + // ExpireFact entries. + llvm::SmallVector<Fact *> EscapesInCurrentBlock; // To distinguish between reads and writes for use-after-free checks, this map // stores the `UseFact` for each `DeclRefExpr`. We initially identify all // `DeclRefExpr`s as "read" uses. When an assignment is processed, the use diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index 91ffbb169f947..b34a7f18b5809 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -42,6 +42,11 @@ class LifetimeSafetyReporter { virtual void reportUseAfterFree(const Expr *IssueExpr, const Expr *UseExpr, SourceLocation FreeLoc, Confidence Confidence) {} + + virtual void reportUseAfterReturn(const Expr *IssueExpr, + const Expr *EscapeExpr, + SourceLocation ExpiryLoc, + Confidence Confidence) {} }; /// The main entry point for the analysis. diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h index c4f5f0e9ae46c..8ad17db83499d 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h @@ -31,6 +31,9 @@ namespace clang::lifetimes::internal { +using CausingFactType = + ::llvm::PointerUnion<const UseFact *, const OriginEscapesFact *>; + enum class LivenessKind : uint8_t { Dead, // Not alive Maybe, // Live on some path but not all paths (may-be-live) @@ -43,7 +46,7 @@ struct LivenessInfo { /// multiple uses along diff erent paths, this will point to the use appearing /// earlier in the translation unit. /// This is 'null' when the origin is not live. - const UseFact *CausingUseFact; + CausingFactType CausingFact; /// The kind of liveness of the origin. /// `Must`: The origin is live on all control-flow paths from the current @@ -56,17 +59,16 @@ struct LivenessInfo { /// while `Maybe`-be-alive suggests a potential one on some paths. LivenessKind Kind; - LivenessInfo() : CausingUseFact(nullptr), Kind(LivenessKind::Dead) {} - LivenessInfo(const UseFact *UF, LivenessKind K) - : CausingUseFact(UF), Kind(K) {} + LivenessInfo() : CausingFact(nullptr), Kind(LivenessKind::Dead) {} + LivenessInfo(CausingFactType CF, LivenessKind K) : CausingFact(CF), Kind(K) {} bool operator==(const LivenessInfo &Other) const { - return CausingUseFact == Other.CausingUseFact && Kind == Other.Kind; + return CausingFact == Other.CausingFact && Kind == Other.Kind; } bool operator!=(const LivenessInfo &Other) const { return !(*this == Other); } void Profile(llvm::FoldingSetNodeID &IDBuilder) const { - IDBuilder.AddPointer(CausingUseFact); + IDBuilder.AddPointer(CausingFact.getOpaqueValue()); IDBuilder.Add(Kind); } }; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index a6e60fe4692ee..c535fe2b9f241 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10744,8 +10744,19 @@ def warn_lifetime_safety_loan_expires_permissive : Warning< def warn_lifetime_safety_loan_expires_strict : Warning< "object whose reference is captured may not live long enough">, InGroup<LifetimeSafetyStrict>, DefaultIgnore; + +def warn_lifetime_safety_return_stack_addr_permissive + : Warning<"address of stack memory is returned later">, + InGroup<LifetimeSafetyPermissive>, + DefaultIgnore; +def warn_lifetime_safety_return_stack_addr_strict + : Warning<"address of stack memory may be returned later">, + InGroup<LifetimeSafetyStrict>, + DefaultIgnore; + def note_lifetime_safety_used_here : Note<"later used here">; def note_lifetime_safety_destroyed_here : Note<"destroyed here">; +def note_lifetime_safety_returned_here : Note<"returned here">; // For non-floating point, expressions of the form x == x or x != x // should result in a warning, since these always evaluate to a constant. diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index c443c3a5d4f9b..1f7c282dadac2 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -43,7 +43,7 @@ namespace { /// Struct to store the complete context for a potential lifetime violation. struct PendingWarning { SourceLocation ExpiryLoc; // Where the loan expired. - const Expr *UseExpr; // Where the origin holding this loan was used. + llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> CausingFact; Confidence ConfidenceLevel; }; @@ -68,7 +68,7 @@ class LifetimeChecker { issuePendingWarnings(); } - /// Checks for use-after-free errors when a loan expires. + /// Checks for use-after-free & use-after-return errors when a loan expires. /// /// This method examines all live origins at the expiry point and determines /// if any of them hold the expiring loan. If so, it creates a pending @@ -83,7 +83,11 @@ class LifetimeChecker { LoanID ExpiredLoan = EF->getLoanID(); LivenessMap Origins = LiveOrigins.getLiveOriginsAt(EF); Confidence CurConfidence = Confidence::None; - const UseFact *BadUse = nullptr; + // The UseFact or OriginEscapesFact most indicative of a lifetime error, + // prioritized by earlier source location. + llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> + BestCausingFact = nullptr; + for (auto &[OID, LiveInfo] : Origins) { LoanSet HeldLoans = LoanPropagation.getLoans(OID, EF); if (!HeldLoans.contains(ExpiredLoan)) @@ -92,17 +96,17 @@ class LifetimeChecker { Confidence NewConfidence = livenessKindToConfidence(LiveInfo.Kind); if (CurConfidence < NewConfidence) { CurConfidence = NewConfidence; - BadUse = LiveInfo.CausingUseFact; + BestCausingFact = LiveInfo.CausingFact; } } - if (!BadUse) + if (!BestCausingFact) return; // We have a use-after-free. Confidence LastConf = FinalWarningsMap.lookup(ExpiredLoan).ConfidenceLevel; if (LastConf >= CurConfidence) return; FinalWarningsMap[ExpiredLoan] = {/*ExpiryLoc=*/EF->getExpiryLoc(), - /*UseExpr=*/BadUse->getUseExpr(), + /*BestCausingFact=*/BestCausingFact, /*ConfidenceLevel=*/CurConfidence}; } @@ -112,8 +116,20 @@ class LifetimeChecker { for (const auto &[LID, Warning] : FinalWarningsMap) { const Loan &L = FactMgr.getLoanMgr().getLoan(LID); const Expr *IssueExpr = L.IssueExpr; - Reporter->reportUseAfterFree(IssueExpr, Warning.UseExpr, - Warning.ExpiryLoc, Warning.ConfidenceLevel); + llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> + CausingFact = Warning.CausingFact; + Confidence Confidence = Warning.ConfidenceLevel; + SourceLocation ExpiryLoc = Warning.ExpiryLoc; + + if (const auto *UF = CausingFact.dyn_cast<const UseFact *>()) + Reporter->reportUseAfterFree(IssueExpr, UF->getUseExpr(), ExpiryLoc, + Confidence); + else if (const auto *OEF = + CausingFact.dyn_cast<const OriginEscapesFact *>()) + Reporter->reportUseAfterReturn(IssueExpr, OEF->getEscapeExpr(), + ExpiryLoc, Confidence); + else + llvm_unreachable("Unhandled CausingFact type"); } } }; diff --git a/clang/lib/Analysis/LifetimeSafety/Dataflow.h b/clang/lib/Analysis/LifetimeSafety/Dataflow.h index de821bb17eb6b..05c20d6385368 100644 --- a/clang/lib/Analysis/LifetimeSafety/Dataflow.h +++ b/clang/lib/Analysis/LifetimeSafety/Dataflow.h @@ -170,8 +170,8 @@ class DataflowAnalysis { return D->transfer(In, *F->getAs<ExpireFact>()); case Fact::Kind::OriginFlow: return D->transfer(In, *F->getAs<OriginFlowFact>()); - case Fact::Kind::ReturnOfOrigin: - return D->transfer(In, *F->getAs<ReturnOfOriginFact>()); + case Fact::Kind::OriginEscapes: + return D->transfer(In, *F->getAs<OriginEscapesFact>()); case Fact::Kind::Use: return D->transfer(In, *F->getAs<UseFact>()); case Fact::Kind::TestPoint: @@ -184,7 +184,7 @@ class DataflowAnalysis { Lattice transfer(Lattice In, const IssueFact &) { return In; } Lattice transfer(Lattice In, const ExpireFact &) { return In; } Lattice transfer(Lattice In, const OriginFlowFact &) { return In; } - Lattice transfer(Lattice In, const ReturnOfOriginFact &) { return In; } + Lattice transfer(Lattice In, const OriginEscapesFact &) { return In; } Lattice transfer(Lattice In, const UseFact &) { return In; } Lattice transfer(Lattice In, const TestPointFact &) { return In; } }; diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp index 190c038f46401..0ae7111c489e8 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -43,10 +43,10 @@ void OriginFlowFact::dump(llvm::raw_ostream &OS, const LoanManager &, OS << ")\n"; } -void ReturnOfOriginFact::dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const { - OS << "ReturnOfOrigin ("; - OM.dump(getReturnedOriginID(), OS); +void OriginEscapesFact::dump(llvm::raw_ostream &OS, const LoanManager &, + const OriginManager &OM) const { + OS << "OriginEscapes ("; + OM.dump(getEscapedOriginID(), OS); OS << ")\n"; } @@ -95,4 +95,14 @@ void FactManager::dump(const CFG &Cfg, AnalysisDeclContext &AC) const { } } +llvm::ArrayRef<const Fact *> +FactManager::getBlockContaining(ProgramPoint P) const { + for (const auto &BlockToFactsVec : BlockToFacts) { + for (const Fact *F : BlockToFactsVec) + if (F == P) + return BlockToFactsVec; + } + return {}; +} + } // namespace clang::lifetimes::internal diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 381ff99aae420..cb9a202b08968 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -58,6 +58,7 @@ void FactsGenerator::run() { // initializations and destructions are processed in the correct sequence. for (const CFGBlock *Block : *AC.getAnalysis<PostOrderCFGView>()) { CurrentBlockFacts.clear(); + EscapesInCurrentBlock.clear(); for (unsigned I = 0; I < Block->size(); ++I) { const CFGElement &Element = Block->Elements[I]; if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>()) @@ -66,6 +67,8 @@ void FactsGenerator::run() { Element.getAs<CFGAutomaticObjDtor>()) handleDestructor(*DtorOpt); } + CurrentBlockFacts.append(EscapesInCurrentBlock.begin(), + EscapesInCurrentBlock.end()); FactMgr.addBlockFacts(Block, CurrentBlockFacts); } } @@ -166,7 +169,8 @@ 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)); + EscapesInCurrentBlock.push_back( + FactMgr.createFact<OriginEscapesFact>(OID, RetExpr)); } } } diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index 59f594e50fb46..57338122b4440 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -53,6 +53,14 @@ struct Lattice { } }; +static SourceLocation GetFactLoc(CausingFactType F) { + if (const auto *UF = F.dyn_cast<const UseFact *>()) + return UF->getUseExpr()->getExprLoc(); + if (const auto *OEF = F.dyn_cast<const OriginEscapesFact *>()) + return OEF->getEscapeExpr()->getExprLoc(); + llvm_unreachable("unhandled causing fact in PointerUnion"); +} + /// The analysis that tracks which origins are live, with granular information /// about the causing use fact and confidence level. This is a backward /// analysis. @@ -74,11 +82,14 @@ class AnalysisImpl /// one. Lattice join(Lattice L1, Lattice L2) const { LivenessMap Merged = L1.LiveOrigins; - // Take the earliest UseFact to make the join hermetic and commutative. - auto CombineUseFact = [](const UseFact &A, - const UseFact &B) -> const UseFact * { - return A.getUseExpr()->getExprLoc() < B.getUseExpr()->getExprLoc() ? &A - : &B; + // Take the earliest Fact to make the join hermetic and commutative. + auto CombineCausingFact = [](CausingFactType A, + CausingFactType B) -> CausingFactType { + if (!A) + return B; + if (!B) + return A; + return GetFactLoc(A) < GetFactLoc(B) ? A : B; }; auto CombineLivenessKind = [](LivenessKind K1, LivenessKind K2) -> LivenessKind { @@ -93,12 +104,11 @@ class AnalysisImpl const LivenessInfo *L2) -> LivenessInfo { assert((L1 || L2) && "unexpectedly merging 2 empty sets"); if (!L1) - return LivenessInfo(L2->CausingUseFact, LivenessKind::Maybe); + return LivenessInfo(L2->CausingFact, LivenessKind::Maybe); if (!L2) - return LivenessInfo(L1->CausingUseFact, LivenessKind::Maybe); - return LivenessInfo( - CombineUseFact(*L1->CausingUseFact, *L2->CausingUseFact), - CombineLivenessKind(L1->Kind, L2->Kind)); + return LivenessInfo(L1->CausingFact, LivenessKind::Maybe); + return LivenessInfo(CombineCausingFact(L1->CausingFact, L2->CausingFact), + CombineLivenessKind(L1->Kind, L2->Kind)); }; return Lattice(utils::join( L1.LiveOrigins, L2.LiveOrigins, Factory, CombineLivenessInfo, @@ -120,6 +130,14 @@ class AnalysisImpl LivenessInfo(&UF, LivenessKind::Must))); } + /// An escaping origin (e.g., via return) makes the origin live with definite + /// confidence, as it dominates this program point. + Lattice transfer(Lattice In, const OriginEscapesFact &OEF) { + OriginID OID = OEF.getEscapedOriginID(); + return Lattice(Factory.add(In.LiveOrigins, OID, + LivenessInfo(&OEF, LivenessKind::Must))); + } + /// Issuing a new loan to an origin kills its liveness. Lattice transfer(Lattice In, const IssueFact &IF) { return Lattice(Factory.remove(In.LiveOrigins, IF.getOriginID())); diff --git a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp index 0e6c194123df8..23ce1b78dfde2 100644 --- a/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LoanPropagation.cpp @@ -58,12 +58,10 @@ static llvm::BitVector computePersistentOrigins(const FactManager &FactMgr, CheckOrigin(OF->getSrcOriginID()); break; } - case Fact::Kind::ReturnOfOrigin: - CheckOrigin(F->getAs<ReturnOfOriginFact>()->getReturnedOriginID()); - break; case Fact::Kind::Use: CheckOrigin(F->getAs<UseFact>()->getUsedOrigin()); break; + case Fact::Kind::OriginEscapes: case Fact::Kind::Expire: case Fact::Kind::TestPoint: break; diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 41a98323450e4..0fa75a24db4f3 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2872,6 +2872,18 @@ class LifetimeSafetyReporterImpl : public LifetimeSafetyReporter { << UseExpr->getEndLoc(); } + void reportUseAfterReturn(const Expr *IssueExpr, const Expr *EscapeExpr, + SourceLocation ExpiryLoc, Confidence C) override { + S.Diag(IssueExpr->getExprLoc(), + C == Confidence::Definite + ? diag::warn_lifetime_safety_return_stack_addr_permissive + : diag::warn_lifetime_safety_return_stack_addr_strict) + << IssueExpr->getEndLoc(); + + S.Diag(EscapeExpr->getExprLoc(), diag::note_lifetime_safety_returned_here) + << EscapeExpr->getEndLoc(); + } + private: Sema &S; }; diff --git a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp index e9515b5d61006..11d3b836db3e7 100644 --- a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp +++ b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp @@ -18,8 +18,8 @@ MyObj* return_local_addr() { return p; // CHECK: Use ([[O_P]] (Decl: p), Read) // CHECK: OriginFlow (Dest: [[O_RET_VAL:[0-9]+]] (Expr: ImplicitCastExpr), Src: [[O_P]] (Decl: p)) -// CHECK: ReturnOfOrigin ([[O_RET_VAL]] (Expr: ImplicitCastExpr)) // CHECK: Expire ([[L_X]] (Path: x)) +// CHECK: OriginEscapes ([[O_RET_VAL]] (Expr: ImplicitCastExpr)) } @@ -49,8 +49,8 @@ MyObj* assign_and_return_local_addr() { return ptr2; // CHECK: Use ([[O_PTR2]] (Decl: ptr2), Read) // CHECK: OriginFlow (Dest: [[O_RET_VAL:[0-9]+]] (Expr: ImplicitCastExpr), Src: [[O_PTR2]] (Decl: ptr2)) -// CHECK: ReturnOfOrigin ([[O_RET_VAL]] (Expr: ImplicitCastExpr)) // CHECK: Expire ([[L_Y]] (Path: y)) +// CHECK: OriginEscapes ([[O_RET_VAL]] (Expr: ImplicitCastExpr)) } // Return of Non-Pointer Type diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index b9368db550805..2803e73b5aee2 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety -Wexperimental-lifetime-safety -verify %s +// RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety -Wexperimental-lifetime-safety -Wno-dangling -verify %s struct MyObj { int id; @@ -396,6 +396,131 @@ void loan_from_previous_iteration(MyObj safe, bool condition) { } // expected-note {{destroyed here}} } +//===----------------------------------------------------------------------===// +// Basic Definite Use-After-Return (Return-Stack-Address) (-W...permissive) +// These are cases where the pointer is guaranteed to be dangling at the use site. +//===----------------------------------------------------------------------===// + +MyObj* simple_return_stack_address() { + MyObj s; + MyObj* p = &s; // expected-warning {{address of stack memory is returned later}} + return p; // expected-note {{returned here}} +} + +MyObj* direct_return() { + MyObj s; + return &s; // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 {{returned here}} +} + +const MyObj* conditional_assign_unconditional_return(const MyObj& safe, bool c) { + MyObj s; + const MyObj* p = &safe; + if (c) { + p = &s; // expected-warning {{address of stack memory is returned later}} + } + return p; // expected-note {{returned here}} +} + +View conditional_assign_both_branches(const MyObj& safe, bool c) { + MyObj s; + View p; + if (c) { + p = s; // expected-warning {{address of stack memory is returned later}} + } + else { + p = safe; + } + return p; // expected-note {{returned here}} + +} + +View reassign_safe_to_local(const MyObj& safe) { + MyObj local; + View p = safe; + p = local; // expected-warning {{address of stack memory is returned later}} + return p; // expected-note {{returned here}} +} + +View pointer_chain_to_local() { + MyObj local; + View p1 = local; // expected-warning {{address of stack memory is returned later}} + View p2 = p1; + return p2; // expected-note {{returned here}} +} + +View multiple_assign_multiple_return(const MyObj& safe, bool c1, bool c2) { + MyObj local1; + MyObj local2; + View p; + if (c1) { + p = local1; // expected-warning {{address of stack memory is returned later}} + return p; // expected-note {{returned here}} + } + else if (c2) { + p = local2; // expected-warning {{address of stack memory is returned later}} + return p; // expected-note {{returned here}} + } + p = safe; + return p; +} + +View multiple_assign_single_return(const MyObj& safe, bool c1, bool c2) { + MyObj local1; + MyObj local2; + View p; + if (c1) { + p = local1; // expected-warning {{address of stack memory is returned later}} + } + else if (c2) { + p = local2; // expected-warning {{address of stack memory is returned later}} + } + else { + p = safe; + } + return p; // expected-note 2 {{returned here}} +} + +View direct_return_of_local() { + MyObj stack; + return stack; // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 {{returned here}} +} + +MyObj& reference_return_of_local() { + MyObj stack; + return stack; // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 {{returned here}} +} + +//===----------------------------------------------------------------------===// +// Use-After-Scope & Use-After-Return (Return-Stack-Address) Combined +// These are cases where the diagnostic kind is determined by location +//===----------------------------------------------------------------------===// + +MyObj* uaf_before_uar() { + MyObj* p; + { + MyObj local_obj; + p = &local_obj; // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + return p; // expected-note {{later used here}} +} + +View uar_before_uaf(const MyObj& safe, bool c) { + View p; + { + MyObj local_obj; + p = local_obj; // expected-warning {{address of stack memory is returned later}} + if (c) { + return p; // expected-note {{returned here}} + } + } + p.use(); + p = safe; + return p; +} + //===----------------------------------------------------------------------===// // No-Error Cases //===----------------------------------------------------------------------===// @@ -434,12 +559,19 @@ void no_error_loan_from_current_iteration(bool cond) { } } +View safe_return(const MyObj& safe) { + MyObj local; + View p = local; + p = safe; // p has been reassigned + return p; // This is safe +} //===----------------------------------------------------------------------===// // Lifetimebound Attribute Tests //===----------------------------------------------------------------------===// View Identity(View v [[clang::lifetimebound]]); +const MyObj& IdentityRef(const MyObj& obj [[clang::lifetimebound]]); 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]]); @@ -584,6 +716,28 @@ void lifetimebound_ctor() { (void)v; } +View lifetimebound_return_of_local() { + MyObj stack; + return Identity(stack); // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 {{returned here}} +} + +const MyObj& lifetimebound_return_ref_to_local() { + MyObj stack; + return IdentityRef(stack); // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 {{returned here}} +} + +// FIXME: Fails to diagnose UAR when a reference to a by-value param escapes via the return value. +View lifetimebound_return_of_by_value_param(MyObj stack_param) { + return Identity(stack_param); +} + +// FIXME: Fails to diagnose UAF when a reference to a by-value param escapes via an out-param. +void uaf_from_by_value_param_failing(MyObj param, View* out_p) { + *out_p = Identity(param); +} + // Conditional operator. void conditional_operator_one_unsafe_branch(bool cond) { MyObj safe; diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index 601308c53f9a9..558a22af72572 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -20,6 +20,7 @@ namespace clang::lifetimes::internal { namespace { using namespace ast_matchers; +using ::testing::Not; using ::testing::SizeIs; using ::testing::UnorderedElementsAreArray; @@ -122,6 +123,40 @@ class LifetimeTestHelper { return LID; } + // Gets the set of loans that are live at the given program point. A loan is + // considered live at point P if there is a live origin which contains this + // loan. + std::optional<LoanSet> getLiveLoansAtPoint(ProgramPoint P) const { + const auto &LiveOriginsAnalysis = Runner.getAnalysis().getLiveOrigins(); + const auto &LoanPropagation = Runner.getAnalysis().getLoanPropagation(); + + LivenessMap LiveOriginsMap = LiveOriginsAnalysis.getLiveOriginsAt(P); + + LoanSet::Factory F; + LoanSet Result = F.getEmptySet(); + + for (const auto &[OID, LI] : LiveOriginsMap) { + LoanSet Loans = LoanPropagation.getLoans(OID, P); + Result = clang::lifetimes::internal::utils::join(Result, Loans, F); + } + + if (Result.isEmpty()) + return std::nullopt; + + return Result; + } + + const ExpireFact * + getExpireFactFromAllFacts(const llvm::ArrayRef<const Fact *> &FactsInBlock, + const LoanID &loanID) { + for (const Fact *F : FactsInBlock) { + if (auto const *CurrentEF = F->getAs<ExpireFact>()) + if (CurrentEF->getLoanID() == loanID) + return CurrentEF; + } + return nullptr; + } + std::optional<LoanSet> getLoansAtPoint(OriginID OID, llvm::StringRef Annotation) { ProgramPoint PP = Runner.getProgramPoint(Annotation); @@ -141,6 +176,14 @@ class LifetimeTestHelper { return Result; } + ProgramPoint getProgramPoint(llvm::StringRef Annotation) { + return Runner.getProgramPoint(Annotation); + } + + llvm::ArrayRef<const Fact *> getBlockContaining(ProgramPoint P) { + return Runner.getAnalysis().getFactManager().getBlockContaining(P); + } + private: template <typename DeclT> DeclT *findDecl(llvm::StringRef Name) { auto &Ctx = Runner.getASTContext(); @@ -304,6 +347,43 @@ MATCHER_P2(AreLiveAtImpl, Annotation, ConfFilter, "") { return true; } +MATCHER_P2(HasLiveLoanAtExpiryImpl, HelperPtr, Annotation, "") { + llvm::StringRef VarName = arg; + LifetimeTestHelper &Helper = *HelperPtr; + + std::vector<LoanID> Loans = Helper.getLoansForVar(VarName); + if (Loans.empty()) { + *result_listener << "No loans found for variable" << VarName.str(); + return false; + } + + ProgramPoint PP = Helper.getProgramPoint(Annotation); + llvm::ArrayRef<const Fact *> AllFactsInBlock = Helper.getBlockContaining(PP); + + bool NoExpireFactLive = false; + for (const LoanID CurrentLoanID : Loans) { + const ExpireFact *EF = + Helper.getExpireFactFromAllFacts(AllFactsInBlock, CurrentLoanID); + if (!EF) { + NoExpireFactLive = true; + continue; + } + std::optional<LoanSet> LiveLoans = Helper.getLiveLoansAtPoint(EF); + if (!LiveLoans.has_value()) { + *result_listener << "No Live Loans At Expiry Location."; + continue; + } + if (LiveLoans->contains({CurrentLoanID})) + return true; + } + if (NoExpireFactLive) { + *result_listener << "No Expire Fact for loan of " << VarName.str(); + return false; + } + *result_listener << "No loans of " << VarName.str() << " are live"; + return false; +} + MATCHER_P(MustBeLiveAt, Annotation, "") { return ExplainMatchResult(AreLiveAtImpl(Annotation, LivenessKindFilter::Must), arg, result_listener); @@ -353,6 +433,10 @@ class LifetimeAnalysisTest : public ::testing::Test { return HasLoansToImpl(std::vector<std::string>(LoanVars), Annotation); } + auto HasLiveLoanAtExpiry(const char *Annotation) { + return HasLiveLoanAtExpiryImpl(Helper.get(), Annotation); + } + std::unique_ptr<LifetimeTestRunner> Runner; std::unique_ptr<LifetimeTestHelper> Helper; }; @@ -1223,5 +1307,204 @@ TEST_F(LifetimeAnalysisTest, LivenessOutsideLoop) { EXPECT_THAT(Origins({"p"}), MaybeLiveAt("p1")); } +TEST_F(LifetimeAnalysisTest, SimpleReturnStackAddress) { + SetupTest(R"( + MyObj* target() { + MyObj s; + MyObj* p = &s; + POINT(p1); + return p; + } + )"); + EXPECT_THAT("s", HasLiveLoanAtExpiry("p1")); +} + +TEST_F(LifetimeAnalysisTest, DirectReturn) { + SetupTest(R"( + MyObj* target() { + MyObj s; + POINT(P); + return &s; + } + )"); + EXPECT_THAT("s", HasLiveLoanAtExpiry("P")); +} + +TEST_F(LifetimeAnalysisTest, ConditionalAssignUnconditionalReturn) { + SetupTest(R"( + MyObj* target(bool c) { + MyObj s1; + MyObj* p = nullptr; + if (c) { + p = &s1; + } + POINT(P); + return p; + } + )"); + EXPECT_THAT("s1", HasLiveLoanAtExpiry("P")); +} + +TEST_F(LifetimeAnalysisTest, MultipleAssignments) { + SetupTest(R"( + MyObj* target() { + MyObj s; + MyObj* p1 = &s; + MyObj* p2 = &s; + POINT(P); + return p2; + } + )"); + // Test if atleast one loan to "s" is live; + EXPECT_THAT("s", HasLiveLoanAtExpiry("P")); +} + +TEST_F(LifetimeAnalysisTest, ConditionalAssignBothBranches) { + SetupTest(R"( + MyObj* target(bool c) { + MyObj s1; + static MyObj s2; + MyObj* p = nullptr; + if (c) { + p = &s1; + } else { + p = &s2; + } + POINT(P); + return p; + } + )"); + EXPECT_THAT("s1", HasLiveLoanAtExpiry("P")); +} + +TEST_F(LifetimeAnalysisTest, ReassignFromSafeToLocalThenReturn) { + SetupTest(R"( + MyObj* target() { + static MyObj safe_obj; + MyObj local_obj; + MyObj* p = &safe_obj; + + p = &local_obj; + POINT(P); + return p; + } + )"); + EXPECT_THAT("local_obj", HasLiveLoanAtExpiry("P")); +} + +TEST_F(LifetimeAnalysisTest, PointerChainToLocal) { + SetupTest(R"( + MyObj* target() { + MyObj local_obj; + MyObj* p1 = &local_obj; + MyObj* p2 = p1; + POINT(P); + return p2; + } + )"); + EXPECT_THAT("local_obj", HasLiveLoanAtExpiry("P")); +} + +TEST_F(LifetimeAnalysisTest, MultipleAssignmentMultipleReturn) { + SetupTest(R"( + MyObj* target(bool c1, bool c2) { + static MyObj global_obj; + MyObj local_obj1; + MyObj local_obj2; + MyObj* p = nullptr; + if(c1){ + p = &local_obj1; + POINT(C1); + return p; + } + else if(c2){ + p = &local_obj2; + POINT(C2); + return p; + } + p = &global_obj; + POINT(C3); + return p; + } + )"); + + EXPECT_THAT("local_obj1", HasLiveLoanAtExpiry("C1")); + EXPECT_THAT("local_obj2", HasLiveLoanAtExpiry("C2")); + + EXPECT_THAT("local_obj1", Not(HasLiveLoanAtExpiry("C3"))); + EXPECT_THAT("local_obj2", Not(HasLiveLoanAtExpiry("C3"))); +} + +TEST_F(LifetimeAnalysisTest, MultipleAssignmentsSingleReturn) { + SetupTest(R"( + MyObj* target(bool c1, bool c2) { + static MyObj global_obj; + MyObj local_obj1; + MyObj local_obj2; + MyObj* p = nullptr; + if(c1){ + p = &local_obj1; + } + else if(c2){ + p = &local_obj2; + } + else{ + p = &global_obj; + } + POINT(P); + return p; + } + )"); + EXPECT_THAT("local_obj1", HasLiveLoanAtExpiry("P")); + EXPECT_THAT("local_obj2", HasLiveLoanAtExpiry("P")); +} + +TEST_F(LifetimeAnalysisTest, UseAfterScopeThenReturn) { + SetupTest(R"( + MyObj* target() { + MyObj* p; + { + MyObj local_obj; + p = &local_obj; + POINT(p1); + } + POINT(p2); + return p; + } + )"); + EXPECT_THAT(Origin("p"), HasLoansTo({"local_obj"}, "p2")); + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p2")); + + EXPECT_THAT(Origin("p"), HasLoansTo({"local_obj"}, "p1")); + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p1")); + + EXPECT_THAT("local_obj", HasLiveLoanAtExpiry("p2")); +} + +TEST_F(LifetimeAnalysisTest, ReturnBeforeUseAfterScope) { + SetupTest(R"( + MyObj* target(bool c) { + MyObj* p; + static MyObj global_obj; + { + MyObj local_obj; + p = &local_obj; + if(c){ + POINT(p1); + return p; + } + } + POINT(p2); + return &global_obj; + } + )"); + EXPECT_THAT("local_obj", HasLiveLoanAtExpiry("p1")); + + EXPECT_THAT(NoOrigins(), AreLiveAt("p2")); + + EXPECT_THAT(Origin("p"), HasLoansTo({"local_obj"}, "p1")); + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p1")); +} + } // anonymous namespace } // namespace clang::lifetimes::internal _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
