https://github.com/kashika0112 updated https://github.com/llvm/llvm-project/pull/165370
>From 2ab23aa98033c9f14cc6910786b6041c125b0726 Mon Sep 17 00:00:00 2001 From: Kashika Akhouri <[email protected]> Date: Tue, 28 Oct 2025 10:59:48 +0000 Subject: [PATCH 1/6] Adding use-after-return in Lifetime Analysis --- .../Analysis/Analyses/LifetimeSafety/Facts.h | 19 + .../Analyses/LifetimeSafety/FactsGenerator.h | 1 + .../Analyses/LifetimeSafety/LifetimeSafety.h | 5 + .../clang/Basic/DiagnosticSemaKinds.td | 1 + clang/lib/Analysis/LifetimeSafety/Checker.cpp | 77 +++- clang/lib/Analysis/LifetimeSafety/Dataflow.h | 3 + clang/lib/Analysis/LifetimeSafety/Facts.cpp | 8 + .../LifetimeSafety/FactsGenerator.cpp | 6 +- clang/lib/Sema/AnalysisBasedWarnings.cpp | 415 +++++++++--------- 9 files changed, 329 insertions(+), 206 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index 6a90aeb01e638..712e1d42e2966 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -44,6 +44,8 @@ class Fact { Use, /// A marker for a specific point in the code, for testing. TestPoint, + OriginEscapes, + // An origin that stores a loan escapes the function. }; private: @@ -142,6 +144,23 @@ class ReturnOfOriginFact : public Fact { const OriginManager &OM) const override; }; +class OriginEscapesFact : public Fact { + OriginID OID; + const Stmt *EscapeSource; + +public: + static bool classof(const Fact *F) { + return F->getKind() == Kind::OriginEscapes; + } + + OriginEscapesFact(OriginID OID, const Stmt *Source) + : Fact(Kind::OriginEscapes), OID(OID), EscapeSource(Source) {} + OriginID getEscapedOriginID() const { return OID; } + const Stmt *getEscapeSource() const { return EscapeSource; } + void dump(llvm::raw_ostream &OS, const LoanManager &, + const OriginManager &OM) const override; +}; + class UseFact : public Fact { const Expr *UseExpr; // True if this use is a write operation (e.g., left-hand side of assignment). diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index 5e58abee2bbb3..ec3c130bb6c66 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -93,6 +93,7 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { FactManager &FactMgr; AnalysisDeclContext &AC; llvm::SmallVector<Fact *> CurrentBlockFacts; + 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..f5b1b3d9b6564 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 Stmt *EscapeStmt, + SourceLocation ExpiryLoc, + Confidence Confidence) {} }; /// The main entry point for the analysis. diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 5ff4cc4b833d9..6ea5e34cab291 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10730,6 +10730,7 @@ def warn_lifetime_safety_loan_expires_strict : Warning< 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..ae5c62ef04d72 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -21,6 +21,7 @@ #include "clang/Analysis/AnalysisDeclContext.h" #include "clang/Basic/SourceLocation.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/SmallSet.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/TimeProfiler.h" @@ -61,10 +62,23 @@ class LifetimeChecker { AnalysisDeclContext &ADC, LifetimeSafetyReporter *Reporter) : LoanPropagation(LoanPropagation), LiveOrigins(LiveOrigins), FactMgr(FM), Reporter(Reporter) { - for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>()) - for (const Fact *F : FactMgr.getFacts(B)) - if (const auto *EF = F->getAs<ExpireFact>()) + for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>()) { + llvm::SmallVector<const ExpireFact *> BlockExpires; + llvm::SmallVector<const OriginEscapesFact *> BlockEscapes; + for (const Fact *F : FactMgr.getFacts(B)) { + if (const auto *EF = F->getAs<ExpireFact>()) { checkExpiry(EF); + BlockExpires.push_back(EF); + } else if (const auto *OEF = F->getAs<OriginEscapesFact>()) { + BlockEscapes.push_back(OEF); + } + } + if (Reporter) { + for (const OriginEscapesFact *OEF : BlockEscapes) { + checkEscape(OEF, BlockExpires); + } + } + } issuePendingWarnings(); } @@ -106,6 +120,63 @@ class LifetimeChecker { /*ConfidenceLevel=*/CurConfidence}; } + void checkEscape(const OriginEscapesFact *OEF, + llvm::ArrayRef<const ExpireFact *> BlockExpires) { + + if (!Reporter) { + return; + } + + OriginID returnedOID = OEF->getEscapedOriginID(); + ProgramPoint PP = OEF; + + LoanSet HeldLoans = LoanPropagation.getLoans(returnedOID, PP); + if (HeldLoans.isEmpty()) { + return; + } + + llvm::SmallSet<LoanID, 4> ExpiredLoansInBlock; + llvm::DenseMap<LoanID, SourceLocation> ExpiryLocs; + + for (const ExpireFact *EF : BlockExpires) { + ExpiredLoansInBlock.insert(EF->getLoanID()); + ExpiryLocs[EF->getLoanID()] = EF->getExpiryLoc(); + } + + bool hasExpiredDependency = false; + bool allHeldLoansExpired = true; + LoanID exampleExpiredLoan = LoanID(); + + for (LoanID heldLoan : HeldLoans) { + if (ExpiredLoansInBlock.count(heldLoan)) { + hasExpiredDependency = true; + if (exampleExpiredLoan.Value == LoanID().Value) { + exampleExpiredLoan = heldLoan; + } + } else { + allHeldLoansExpired = false; + } + } + + if (!hasExpiredDependency) { + return; + } + + Confidence FinalConfidence; + if (allHeldLoansExpired) { + FinalConfidence = Confidence::Definite; + } else { + FinalConfidence = Confidence::Maybe; + } + + const Loan &L = FactMgr.getLoanMgr().getLoan(exampleExpiredLoan); + SourceLocation ExpiryLoc = ExpiryLocs[exampleExpiredLoan]; + const Stmt *EscapeSource = OEF->getEscapeSource(); + + Reporter->reportUseAfterReturn(L.IssueExpr, EscapeSource, ExpiryLoc, + FinalConfidence); + } + void issuePendingWarnings() { if (!Reporter) return; diff --git a/clang/lib/Analysis/LifetimeSafety/Dataflow.h b/clang/lib/Analysis/LifetimeSafety/Dataflow.h index 2f7bcb6e5dc81..37720ffa03618 100644 --- a/clang/lib/Analysis/LifetimeSafety/Dataflow.h +++ b/clang/lib/Analysis/LifetimeSafety/Dataflow.h @@ -168,6 +168,8 @@ class DataflowAnalysis { 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: @@ -181,6 +183,7 @@ class DataflowAnalysis { 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 1aea64f864367..29c75959ba0fe 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -50,6 +50,14 @@ void ReturnOfOriginFact::dump(llvm::raw_ostream &OS, const LoanManager &, OS << ")\n"; } +void OriginEscapesFact::dump(llvm::raw_ostream &OS, const LoanManager &, + const OriginManager &OM) const { + OS << "OriginEscapes ("; + OM.dump(getEscapedOriginID(), OS); + OS << ")\n"; +} + + void UseFact::dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM) const { OS << "Use ("; diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 9b68de107e314..762ed07eb5bb8 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, RS)); } } } diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 140b709dbb651..879793575e0b4 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -65,61 +65,61 @@ using namespace clang; //===----------------------------------------------------------------------===// namespace { - class UnreachableCodeHandler : public reachable_code::Callback { - Sema &S; - SourceRange PreviousSilenceableCondVal; - - public: - UnreachableCodeHandler(Sema &s) : S(s) {} - - void HandleUnreachable(reachable_code::UnreachableKind UK, SourceLocation L, - SourceRange SilenceableCondVal, SourceRange R1, - SourceRange R2, bool HasFallThroughAttr) override { - // If the diagnosed code is `[[fallthrough]];` and - // `-Wunreachable-code-fallthrough` is enabled, suppress `code will never - // be executed` warning to avoid generating diagnostic twice - if (HasFallThroughAttr && - !S.getDiagnostics().isIgnored(diag::warn_unreachable_fallthrough_attr, - SourceLocation())) - return; +class UnreachableCodeHandler : public reachable_code::Callback { + Sema &S; + SourceRange PreviousSilenceableCondVal; - // Avoid reporting multiple unreachable code diagnostics that are - // triggered by the same conditional value. +public: + UnreachableCodeHandler(Sema &s) : S(s) {} + + void HandleUnreachable(reachable_code::UnreachableKind UK, SourceLocation L, + SourceRange SilenceableCondVal, SourceRange R1, + SourceRange R2, bool HasFallThroughAttr) override { + // If the diagnosed code is `[[fallthrough]];` and + // `-Wunreachable-code-fallthrough` is enabled, suppress `code will never + // be executed` warning to avoid generating diagnostic twice + if (HasFallThroughAttr && + !S.getDiagnostics().isIgnored(diag::warn_unreachable_fallthrough_attr, + SourceLocation())) + return; + + // Avoid reporting multiple unreachable code diagnostics that are + // triggered by the same conditional value. if (PreviousSilenceableCondVal.isValid() && SilenceableCondVal.isValid() && - PreviousSilenceableCondVal == SilenceableCondVal) - return; - PreviousSilenceableCondVal = SilenceableCondVal; + PreviousSilenceableCondVal == SilenceableCondVal) + return; + PreviousSilenceableCondVal = SilenceableCondVal; - unsigned diag = diag::warn_unreachable; - switch (UK) { - case reachable_code::UK_Break: - diag = diag::warn_unreachable_break; - break; - case reachable_code::UK_Return: - diag = diag::warn_unreachable_return; - break; - case reachable_code::UK_Loop_Increment: - diag = diag::warn_unreachable_loop_increment; - break; - case reachable_code::UK_Other: - break; - } + unsigned diag = diag::warn_unreachable; + switch (UK) { + case reachable_code::UK_Break: + diag = diag::warn_unreachable_break; + break; + case reachable_code::UK_Return: + diag = diag::warn_unreachable_return; + break; + case reachable_code::UK_Loop_Increment: + diag = diag::warn_unreachable_loop_increment; + break; + case reachable_code::UK_Other: + break; + } - S.Diag(L, diag) << R1 << R2; + S.Diag(L, diag) << R1 << R2; - SourceLocation Open = SilenceableCondVal.getBegin(); - if (Open.isValid()) { - SourceLocation Close = SilenceableCondVal.getEnd(); - Close = S.getLocForEndOfToken(Close); - if (Close.isValid()) { - S.Diag(Open, diag::note_unreachable_silence) + SourceLocation Open = SilenceableCondVal.getBegin(); + if (Open.isValid()) { + SourceLocation Close = SilenceableCondVal.getEnd(); + Close = S.getLocForEndOfToken(Close); + if (Close.isValid()) { + S.Diag(Open, diag::note_unreachable_silence) << FixItHint::CreateInsertion(Open, "/* DISABLES CODE */ (") << FixItHint::CreateInsertion(Close, ")"); - } } } - }; + } +}; } // anonymous namespace /// CheckUnreachable - Check for unreachable code. @@ -388,9 +388,9 @@ static void checkThrowInNonThrowingFunc(Sema &S, const FunctionDecl *FD, if (BodyCFG->getExit().pred_empty()) return; visitReachableThrows(BodyCFG, [&](const CXXThrowExpr *Throw, CFGBlock &Block) { - if (throwEscapes(S, Throw, Block, BodyCFG)) - EmitDiagForCXXThrowInNonThrowingFunc(S, Throw->getThrowLoc(), FD); - }); + if (throwEscapes(S, Throw, Block, BodyCFG)) + EmitDiagForCXXThrowInNonThrowingFunc(S, Throw->getThrowLoc(), FD); + }); } static bool isNoexcept(const FunctionDecl *FD) { @@ -803,7 +803,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, } else if (isa<BlockDecl>(D)) { if (const FunctionType *FT = - BlockType->getPointeeType()->getAs<FunctionType>()) { + BlockType->getPointeeType()->getAs<FunctionType>()) { if (FT->getReturnType()->isVoidType()) ReturnsVoid = true; if (FT->getNoReturnAttr()) @@ -815,7 +815,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, // Short circuit for compilation speed. if (CD.checkDiagnostics(Diags, ReturnsVoid, HasNoReturn)) - return; + return; SourceLocation LBrace = Body->getBeginLoc(), RBrace = Body->getEndLoc(); // cpu_dispatch functions permit empty function bodies for ICC compatibility. @@ -892,7 +892,7 @@ class ContainsReference : public ConstEvaluatedExprVisitor<ContainsReference> { typedef ConstEvaluatedExprVisitor<ContainsReference> Inherited; ContainsReference(ASTContext &Context, const DeclRefExpr *Needle) - : Inherited(Context), FoundReference(false), Needle(Needle) {} + : Inherited(Context), FoundReference(false), Needle(Needle) {} void VisitExpr(const Expr *E) { // Stop evaluating if we already have a reference. @@ -1016,7 +1016,7 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use, int RemoveDiagKind = -1; const char *FixitStr = S.getLangOpts().CPlusPlus ? (I->Output ? "true" : "false") - : (I->Output ? "1" : "0"); + : (I->Output ? "1" : "0"); FixItHint Fixit1, Fixit2; switch (Term ? Term->getStmtClass() : Stmt::DeclStmtClass) { @@ -1124,7 +1124,7 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use, << IsCapturedByBlock << User->getSourceRange(); if (RemoveDiagKind != -1) S.Diag(Fixit1.RemoveRange.getBegin(), diag::note_uninit_fixit_remove_cond) - << RemoveDiagKind << Str << I->Output << Fixit1 << Fixit2; + << RemoveDiagKind << Str << I->Output << Fixit1 << Fixit2; Diagnosed = true; } @@ -1294,32 +1294,32 @@ class FallthroughMapper : public DynamicRecursiveASTVisitor { // Don't care about other unreachable statements. } } - // If there are no unreachable statements, this may be a special - // case in CFG: - // case X: { - // A a; // A has a destructor. - // break; - // } - // // <<<< This place is represented by a 'hanging' CFG block. - // case Y: - continue; + // If there are no unreachable statements, this may be a special + // case in CFG: + // case X: { + // A a; // A has a destructor. + // break; + // } + // // <<<< This place is represented by a 'hanging' CFG block. + // case Y: + continue; } - const Stmt *LastStmt = getLastStmt(*P); - if (const AttributedStmt *AS = asFallThroughAttr(LastStmt)) { - markFallthroughVisited(AS); - ++AnnotatedCnt; - continue; // Fallthrough annotation, good. - } + const Stmt *LastStmt = getLastStmt(*P); + if (const AttributedStmt *AS = asFallThroughAttr(LastStmt)) { + markFallthroughVisited(AS); + ++AnnotatedCnt; + continue; // Fallthrough annotation, good. + } - if (!LastStmt) { // This block contains no executable statements. - // Traverse its predecessors. - std::copy(P->pred_begin(), P->pred_end(), - std::back_inserter(BlockQueue)); - continue; - } + if (!LastStmt) { // This block contains no executable statements. + // Traverse its predecessors. + std::copy(P->pred_begin(), P->pred_end(), + std::back_inserter(BlockQueue)); + continue; + } - ++UnannotatedCnt; + ++UnannotatedCnt; } return !!UnannotatedCnt; } @@ -1335,48 +1335,48 @@ class FallthroughMapper : public DynamicRecursiveASTVisitor { return true; } - // We don't want to traverse local type declarations. We analyze their - // methods separately. - bool TraverseDecl(Decl *D) override { return true; } + // We don't want to traverse local type declarations. We analyze their + // methods separately. + bool TraverseDecl(Decl *D) override { return true; } - // We analyze lambda bodies separately. Skip them here. - bool TraverseLambdaExpr(LambdaExpr *LE) override { - // Traverse the captures, but not the body. - for (const auto C : zip(LE->captures(), LE->capture_inits())) - TraverseLambdaCapture(LE, &std::get<0>(C), std::get<1>(C)); - return true; - } + // We analyze lambda bodies separately. Skip them here. + bool TraverseLambdaExpr(LambdaExpr *LE) override { + // Traverse the captures, but not the body. + for (const auto C : zip(LE->captures(), LE->capture_inits())) + TraverseLambdaCapture(LE, &std::get<0>(C), std::get<1>(C)); + return true; + } - private: +private: - static const AttributedStmt *asFallThroughAttr(const Stmt *S) { - if (const AttributedStmt *AS = dyn_cast_or_null<AttributedStmt>(S)) { - if (hasSpecificAttr<FallThroughAttr>(AS->getAttrs())) - return AS; - } - return nullptr; + static const AttributedStmt *asFallThroughAttr(const Stmt *S) { + if (const AttributedStmt *AS = dyn_cast_or_null<AttributedStmt>(S)) { + if (hasSpecificAttr<FallThroughAttr>(AS->getAttrs())) + return AS; } + return nullptr; + } - static const Stmt *getLastStmt(const CFGBlock &B) { - if (const Stmt *Term = B.getTerminatorStmt()) - return Term; - for (const CFGElement &Elem : llvm::reverse(B)) - if (std::optional<CFGStmt> CS = Elem.getAs<CFGStmt>()) - return CS->getStmt(); - // Workaround to detect a statement thrown out by CFGBuilder: - // case X: {} case Y: - // case X: ; case Y: - if (const SwitchCase *SW = dyn_cast_or_null<SwitchCase>(B.getLabel())) - if (!isa<SwitchCase>(SW->getSubStmt())) - return SW->getSubStmt(); + static const Stmt *getLastStmt(const CFGBlock &B) { + if (const Stmt *Term = B.getTerminatorStmt()) + return Term; + for (const CFGElement &Elem : llvm::reverse(B)) + if (std::optional<CFGStmt> CS = Elem.getAs<CFGStmt>()) + return CS->getStmt(); + // Workaround to detect a statement thrown out by CFGBuilder: + // case X: {} case Y: + // case X: ; case Y: + if (const SwitchCase *SW = dyn_cast_or_null<SwitchCase>(B.getLabel())) + if (!isa<SwitchCase>(SW->getSubStmt())) + return SW->getSubStmt(); - return nullptr; - } + return nullptr; + } - bool FoundSwitchStatements; - AttrStmts FallthroughStmts; - Sema &S; - llvm::SmallPtrSet<const CFGBlock *, 16> ReachableBlocks; + bool FoundSwitchStatements; + AttrStmts FallthroughStmts; + Sema &S; + llvm::SmallPtrSet<const CFGBlock *, 16> ReachableBlocks; }; } // anonymous namespace @@ -1384,7 +1384,7 @@ static StringRef getFallthroughAttrSpelling(Preprocessor &PP, SourceLocation Loc) { TokenValue FallthroughTokens[] = { tok::l_square, tok::l_square, - PP.getIdentifierInfo("fallthrough"), + PP.getIdentifierInfo("fallthrough"), tok::r_square, tok::r_square }; @@ -1513,7 +1513,7 @@ static void diagnoseRepeatedUseOfWeak(Sema &S, typedef sema::FunctionScopeInfo::WeakObjectUseMap WeakObjectUseMap; typedef sema::FunctionScopeInfo::WeakUseVector WeakUseVector; typedef std::pair<const Stmt *, WeakObjectUseMap::const_iterator> - StmtUsesPair; + StmtUsesPair; ASTContext &Ctx = S.getASTContext(); @@ -1977,7 +1977,7 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { : getNotes(); } - public: +public: ThreadSafetyReporter(Sema &S, SourceLocation FL, SourceLocation FEL) : S(S), FunLocation(FL), FunEndLocation(FEL), CurrentFunction(nullptr), Verbose(false) {} @@ -2075,18 +2075,18 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { bool ReentrancyMismatch) override { unsigned DiagID = 0; switch (LEK) { - case LEK_LockedSomePredecessors: - DiagID = diag::warn_lock_some_predecessors; - break; - case LEK_LockedSomeLoopIterations: - DiagID = diag::warn_expecting_lock_held_on_loop; - break; - case LEK_LockedAtEndOfFunction: - DiagID = diag::warn_no_unlock; - break; - case LEK_NotLockedAtEndOfFunction: - DiagID = diag::warn_expecting_locked; - break; + case LEK_LockedSomePredecessors: + DiagID = diag::warn_lock_some_predecessors; + break; + case LEK_LockedSomeLoopIterations: + DiagID = diag::warn_expecting_lock_held_on_loop; + break; + case LEK_LockedAtEndOfFunction: + DiagID = diag::warn_no_unlock; + break; + case LEK_NotLockedAtEndOfFunction: + DiagID = diag::warn_expecting_locked; + break; } if (LocEndOfScope.isInvalid()) LocEndOfScope = FunEndLocation; @@ -2132,7 +2132,7 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { break; } PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID) - << D << getLockKindFromAccessKind(AK)); + << D << getLockKindFromAccessKind(AK)); Warnings.emplace_back(std::move(Warning), getNotes()); } @@ -2143,39 +2143,39 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { unsigned DiagID = 0; if (PossibleMatch) { switch (POK) { - case POK_VarAccess: - DiagID = diag::warn_variable_requires_lock_precise; - break; - case POK_VarDereference: - DiagID = diag::warn_var_deref_requires_lock_precise; - break; - case POK_FunctionCall: - DiagID = diag::warn_fun_requires_lock_precise; - break; - case POK_PassByRef: - DiagID = diag::warn_guarded_pass_by_reference; - break; - case POK_PtPassByRef: - DiagID = diag::warn_pt_guarded_pass_by_reference; - break; - case POK_ReturnByRef: - DiagID = diag::warn_guarded_return_by_reference; - break; - case POK_PtReturnByRef: - DiagID = diag::warn_pt_guarded_return_by_reference; - break; - case POK_PassPointer: - DiagID = diag::warn_guarded_pass_pointer; - break; - case POK_PtPassPointer: - DiagID = diag::warn_pt_guarded_pass_pointer; - break; - case POK_ReturnPointer: - DiagID = diag::warn_guarded_return_pointer; - break; - case POK_PtReturnPointer: - DiagID = diag::warn_pt_guarded_return_pointer; - break; + case POK_VarAccess: + DiagID = diag::warn_variable_requires_lock_precise; + break; + case POK_VarDereference: + DiagID = diag::warn_var_deref_requires_lock_precise; + break; + case POK_FunctionCall: + DiagID = diag::warn_fun_requires_lock_precise; + break; + case POK_PassByRef: + DiagID = diag::warn_guarded_pass_by_reference; + break; + case POK_PtPassByRef: + DiagID = diag::warn_pt_guarded_pass_by_reference; + break; + case POK_ReturnByRef: + DiagID = diag::warn_guarded_return_by_reference; + break; + case POK_PtReturnByRef: + DiagID = diag::warn_pt_guarded_return_by_reference; + break; + case POK_PassPointer: + DiagID = diag::warn_guarded_pass_pointer; + break; + case POK_PtPassPointer: + DiagID = diag::warn_pt_guarded_pass_pointer; + break; + case POK_ReturnPointer: + DiagID = diag::warn_guarded_return_pointer; + break; + case POK_PtReturnPointer: + DiagID = diag::warn_pt_guarded_return_pointer; + break; } PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID) << Kind << D @@ -2191,39 +2191,39 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { Warnings.emplace_back(std::move(Warning), getNotes(Note)); } else { switch (POK) { - case POK_VarAccess: - DiagID = diag::warn_variable_requires_lock; - break; - case POK_VarDereference: - DiagID = diag::warn_var_deref_requires_lock; - break; - case POK_FunctionCall: - DiagID = diag::warn_fun_requires_lock; - break; - case POK_PassByRef: - DiagID = diag::warn_guarded_pass_by_reference; - break; - case POK_PtPassByRef: - DiagID = diag::warn_pt_guarded_pass_by_reference; - break; - case POK_ReturnByRef: - DiagID = diag::warn_guarded_return_by_reference; - break; - case POK_PtReturnByRef: - DiagID = diag::warn_pt_guarded_return_by_reference; - break; - case POK_PassPointer: - DiagID = diag::warn_guarded_pass_pointer; - break; - case POK_PtPassPointer: - DiagID = diag::warn_pt_guarded_pass_pointer; - break; - case POK_ReturnPointer: - DiagID = diag::warn_guarded_return_pointer; - break; - case POK_PtReturnPointer: - DiagID = diag::warn_pt_guarded_return_pointer; - break; + case POK_VarAccess: + DiagID = diag::warn_variable_requires_lock; + break; + case POK_VarDereference: + DiagID = diag::warn_var_deref_requires_lock; + break; + case POK_FunctionCall: + DiagID = diag::warn_fun_requires_lock; + break; + case POK_PassByRef: + DiagID = diag::warn_guarded_pass_by_reference; + break; + case POK_PtPassByRef: + DiagID = diag::warn_pt_guarded_pass_by_reference; + break; + case POK_ReturnByRef: + DiagID = diag::warn_guarded_return_by_reference; + break; + case POK_PtReturnByRef: + DiagID = diag::warn_pt_guarded_return_by_reference; + break; + case POK_PassPointer: + DiagID = diag::warn_guarded_pass_pointer; + break; + case POK_PtPassPointer: + DiagID = diag::warn_pt_guarded_pass_pointer; + break; + case POK_ReturnPointer: + DiagID = diag::warn_guarded_return_pointer; + break; + case POK_PtReturnPointer: + DiagID = diag::warn_pt_guarded_return_pointer; + break; } PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID) << Kind << D @@ -2241,7 +2241,7 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { SourceLocation Loc) override { PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_acquire_requires_negative_cap) - << Kind << LockName << Neg); + << Kind << LockName << Neg); Warnings.emplace_back(std::move(Warning), getNotes()); } @@ -2423,7 +2423,7 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler { public: UnsafeBufferUsageReporter(Sema &S, bool SuggestSuggestions) - : S(S), SuggestSuggestions(SuggestSuggestions) {} + : S(S), SuggestSuggestions(SuggestSuggestions) {} void handleUnsafeOperation(const Stmt *Operation, bool IsRelatedToDecl, ASTContext &Ctx) override { @@ -2808,6 +2808,17 @@ class LifetimeSafetyReporterImpl : public LifetimeSafetyReporter { << UseExpr->getEndLoc(); } + void reportUseAfterReturn(const Expr *IssueExpr, const Stmt *EscapeStmt, + SourceLocation ExpiryLoc, Confidence C) override { + S.Diag(IssueExpr->getExprLoc(), + C == Confidence::Definite + ? diag::warn_lifetime_safety_loan_expires_permissive + : diag::warn_lifetime_safety_loan_expires_strict) + << IssueExpr->getEndLoc(); + + S.Diag(EscapeStmt->getBeginLoc(), diag::note_lifetime_safety_returned_here); + } + private: Sema &S; }; @@ -2815,7 +2826,7 @@ class LifetimeSafetyReporterImpl : public LifetimeSafetyReporter { } // namespace clang::lifetimes void clang::sema::AnalysisBasedWarnings::IssueWarnings( - TranslationUnitDecl *TU) { + TranslationUnitDecl *TU) { if (!TU) return; // This is unexpected, give up quietly. @@ -2928,13 +2939,13 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( AC.getCFGBuildOptions().setAllAlwaysAdd(); } else { AC.getCFGBuildOptions() - .setAlwaysAdd(Stmt::BinaryOperatorClass) - .setAlwaysAdd(Stmt::CompoundAssignOperatorClass) - .setAlwaysAdd(Stmt::BlockExprClass) - .setAlwaysAdd(Stmt::CStyleCastExprClass) - .setAlwaysAdd(Stmt::DeclRefExprClass) - .setAlwaysAdd(Stmt::ImplicitCastExprClass) - .setAlwaysAdd(Stmt::UnaryOperatorClass); + .setAlwaysAdd(Stmt::BinaryOperatorClass) + .setAlwaysAdd(Stmt::CompoundAssignOperatorClass) + .setAlwaysAdd(Stmt::BlockExprClass) + .setAlwaysAdd(Stmt::CStyleCastExprClass) + .setAlwaysAdd(Stmt::DeclRefExprClass) + .setAlwaysAdd(Stmt::ImplicitCastExprClass) + .setAlwaysAdd(Stmt::UnaryOperatorClass); } // Install the logical handler. >From 0ba83e2a940795f3427f7141c20822853c51e63a Mon Sep 17 00:00:00 2001 From: Kashika Akhouri <[email protected]> Date: Tue, 28 Oct 2025 10:59:48 +0000 Subject: [PATCH 2/6] Adding use-after-return in Lifetime Analysis --- .../Analysis/Analyses/LifetimeSafety/Facts.h | 26 +- .../Analyses/LifetimeSafety/FactsGenerator.h | 2 + .../Analyses/LifetimeSafety/LifetimeSafety.h | 2 +- .../Analyses/LifetimeSafety/LiveOrigins.h | 11 +- .../clang/Basic/DiagnosticSemaKinds.td | 10 + clang/lib/Analysis/LifetimeSafety/Checker.cpp | 91 ++----- clang/lib/Analysis/LifetimeSafety/Dataflow.h | 3 - clang/lib/Analysis/LifetimeSafety/Facts.cpp | 7 - .../LifetimeSafety/FactsGenerator.cpp | 5 +- .../Analysis/LifetimeSafety/LiveOrigins.cpp | 48 +++- clang/lib/Sema/AnalysisBasedWarnings.cpp | 8 +- .../Sema/warn-lifetime-safety-dataflow.cpp | 4 +- clang/test/Sema/warn-lifetime-safety.cpp | 118 +++++++++ .../unittests/Analysis/LifetimeSafetyTest.cpp | 237 ++++++++++++++++++ 14 files changed, 446 insertions(+), 126 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index 712e1d42e2966..1b87eee4a8d17 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -39,13 +39,11 @@ class Fact { /// 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, + // Indicates that an origin escapes the function scope (e.g., via return). OriginEscapes, - // An origin that stores a loan escapes the function. }; private: @@ -130,33 +128,19 @@ class OriginFlowFact : public Fact { const OriginManager &OM) const override; }; -class ReturnOfOriginFact : public Fact { - OriginID OID; - -public: - static bool classof(const Fact *F) { - return F->getKind() == Kind::ReturnOfOrigin; - } - - ReturnOfOriginFact(OriginID OID) : Fact(Kind::ReturnOfOrigin), OID(OID) {} - OriginID getReturnedOriginID() const { return OID; } - void dump(llvm::raw_ostream &OS, const LoanManager &, - const OriginManager &OM) const override; -}; - class OriginEscapesFact : public Fact { OriginID OID; - const Stmt *EscapeSource; + const Expr *EscapeExpr; public: static bool classof(const Fact *F) { return F->getKind() == Kind::OriginEscapes; } - OriginEscapesFact(OriginID OID, const Stmt *Source) - : Fact(Kind::OriginEscapes), OID(OID), EscapeSource(Source) {} + OriginEscapesFact(OriginID OID, const Expr *EscapeExpr) + : Fact(Kind::OriginEscapes), OID(OID), EscapeExpr(EscapeExpr) {} OriginID getEscapedOriginID() const { return OID; } - const Stmt *getEscapeSource() const { return EscapeSource; } + const Expr *getEscapeExpr() const { return EscapeExpr; }; void dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM) const override; }; diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index ec3c130bb6c66..21d160493289e 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -93,6 +93,8 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { FactManager &FactMgr; AnalysisDeclContext &AC; llvm::SmallVector<Fact *> CurrentBlockFacts; + // Collect origins that escape the function in this block. These are handled + // at the end of the block to ensure `OEFs` appear after `Expire` facts. 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 diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index f5b1b3d9b6564..b34a7f18b5809 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -44,7 +44,7 @@ class LifetimeSafetyReporter { Confidence Confidence) {} virtual void reportUseAfterReturn(const Expr *IssueExpr, - const Stmt *EscapeStmt, + const Expr *EscapeExpr, SourceLocation ExpiryLoc, Confidence Confidence) {} }; diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h index c4f5f0e9ae46c..dae97501c0b53 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h @@ -43,7 +43,7 @@ struct LivenessInfo { /// multiple uses along different 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; + const Fact *CausingFact; /// The kind of liveness of the origin. /// `Must`: The origin is live on all control-flow paths from the current @@ -56,17 +56,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(const Fact *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); IDBuilder.Add(Kind); } }; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 6ea5e34cab291..4eb7263715f4a 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10728,6 +10728,16 @@ 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<"returning reference to stack allocated object">, + InGroup<LifetimeSafetyPermissive>, + DefaultIgnore; +def warn_lifetime_safety_return_stack_addr_strict + : Warning<"may be returning reference to stack allocated object">, + 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">; diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index ae5c62ef04d72..64ca5439cc264 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -44,7 +44,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. + const Fact *CausingFact; // If the use is a UseFact or OriginEscapeFact Confidence ConfidenceLevel; }; @@ -64,25 +64,17 @@ class LifetimeChecker { Reporter(Reporter) { for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>()) { llvm::SmallVector<const ExpireFact *> BlockExpires; - llvm::SmallVector<const OriginEscapesFact *> BlockEscapes; for (const Fact *F : FactMgr.getFacts(B)) { if (const auto *EF = F->getAs<ExpireFact>()) { checkExpiry(EF); BlockExpires.push_back(EF); - } else if (const auto *OEF = F->getAs<OriginEscapesFact>()) { - BlockEscapes.push_back(OEF); - } - } - if (Reporter) { - for (const OriginEscapesFact *OEF : BlockEscapes) { - checkEscape(OEF, BlockExpires); } } } 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 @@ -97,7 +89,8 @@ class LifetimeChecker { LoanID ExpiredLoan = EF->getLoanID(); LivenessMap Origins = LiveOrigins.getLiveOriginsAt(EF); Confidence CurConfidence = Confidence::None; - const UseFact *BadUse = nullptr; + const Fact *BestCausingFact = nullptr; + for (auto &[OID, LiveInfo] : Origins) { LoanSet HeldLoans = LoanPropagation.getLoans(OID, EF); if (!HeldLoans.contains(ExpiredLoan)) @@ -106,85 +99,37 @@ 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}; } - void checkEscape(const OriginEscapesFact *OEF, - llvm::ArrayRef<const ExpireFact *> BlockExpires) { - - if (!Reporter) { - return; - } - - OriginID returnedOID = OEF->getEscapedOriginID(); - ProgramPoint PP = OEF; - - LoanSet HeldLoans = LoanPropagation.getLoans(returnedOID, PP); - if (HeldLoans.isEmpty()) { - return; - } - - llvm::SmallSet<LoanID, 4> ExpiredLoansInBlock; - llvm::DenseMap<LoanID, SourceLocation> ExpiryLocs; - - for (const ExpireFact *EF : BlockExpires) { - ExpiredLoansInBlock.insert(EF->getLoanID()); - ExpiryLocs[EF->getLoanID()] = EF->getExpiryLoc(); - } - - bool hasExpiredDependency = false; - bool allHeldLoansExpired = true; - LoanID exampleExpiredLoan = LoanID(); - - for (LoanID heldLoan : HeldLoans) { - if (ExpiredLoansInBlock.count(heldLoan)) { - hasExpiredDependency = true; - if (exampleExpiredLoan.Value == LoanID().Value) { - exampleExpiredLoan = heldLoan; - } - } else { - allHeldLoansExpired = false; - } - } - - if (!hasExpiredDependency) { - return; - } - - Confidence FinalConfidence; - if (allHeldLoansExpired) { - FinalConfidence = Confidence::Definite; - } else { - FinalConfidence = Confidence::Maybe; - } - - const Loan &L = FactMgr.getLoanMgr().getLoan(exampleExpiredLoan); - SourceLocation ExpiryLoc = ExpiryLocs[exampleExpiredLoan]; - const Stmt *EscapeSource = OEF->getEscapeSource(); - - Reporter->reportUseAfterReturn(L.IssueExpr, EscapeSource, ExpiryLoc, - FinalConfidence); - } - void issuePendingWarnings() { if (!Reporter) return; 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); + const Fact *CausingFact = Warning.CausingFact; + Confidence Confidence = Warning.ConfidenceLevel; + SourceLocation ExpiryLoc = Warning.ExpiryLoc; + + if (const auto *UF = CausingFact->getAs<UseFact>()) { + Reporter->reportUseAfterFree(IssueExpr, UF->getUseExpr(), ExpiryLoc, + Confidence); + } else if (const auto *OEF = CausingFact->getAs<OriginEscapesFact>()) { + Reporter->reportUseAfterReturn(IssueExpr, OEF->getEscapeExpr(), + ExpiryLoc, Confidence); + } } } }; diff --git a/clang/lib/Analysis/LifetimeSafety/Dataflow.h b/clang/lib/Analysis/LifetimeSafety/Dataflow.h index 37720ffa03618..23ef9dbcdcb86 100644 --- a/clang/lib/Analysis/LifetimeSafety/Dataflow.h +++ b/clang/lib/Analysis/LifetimeSafety/Dataflow.h @@ -166,8 +166,6 @@ 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: @@ -182,7 +180,6 @@ 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 29c75959ba0fe..45fce4bcf8ba3 100644 --- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp @@ -43,13 +43,6 @@ 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); - OS << ")\n"; -} - void OriginEscapesFact::dump(llvm::raw_ostream &OS, const LoanManager &, const OriginManager &OM) const { OS << "OriginEscapes ("; diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 762ed07eb5bb8..72a1b52786d54 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -59,6 +59,7 @@ void FactsGenerator::run() { for (const CFGBlock *Block : *AC.getAnalysis<PostOrderCFGView>()) { CurrentBlockFacts.clear(); EscapesInCurrentBlock.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>()) @@ -67,6 +68,8 @@ void FactsGenerator::run() { Element.getAs<CFGAutomaticObjDtor>()) handleDestructor(*DtorOpt); } + CurrentBlockFacts.append(EscapesInCurrentBlock.begin(), + EscapesInCurrentBlock.end()); CurrentBlockFacts.append(EscapesInCurrentBlock.begin(), EscapesInCurrentBlock.end()); FactMgr.addBlockFacts(Block, CurrentBlockFacts); @@ -170,7 +173,7 @@ void FactsGenerator::VisitReturnStmt(const ReturnStmt *RS) { if (hasOrigin(RetExpr)) { OriginID OID = FactMgr.getOriginMgr().getOrCreate(*RetExpr); EscapesInCurrentBlock.push_back( - FactMgr.createFact<OriginEscapesFact>(OID, RS)); + FactMgr.createFact<OriginEscapesFact>(OID, RetExpr)); } } } diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index cddb3f3ce4c1c..02a079184723c 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -53,6 +53,16 @@ struct Lattice { } }; +static SourceLocation GetFactLoc(const Fact &F) { + if (const auto *UF = F.getAs<UseFact>()) { + return UF->getUseExpr()->getExprLoc(); + } + if (const auto *OEF = F.getAs<OriginEscapesFact>()) { + return OEF->getEscapeExpr()->getExprLoc(); + } + return SourceLocation(); // Invalid SourceLocation +} + /// 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 +84,25 @@ 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 = [](const Fact &A, const Fact &B) -> const Fact * { + SourceLocation LocA = GetFactLoc(A); + SourceLocation LocB = GetFactLoc(B); + + bool aValid = LocA.isValid(); + bool bValid = LocB.isValid(); + + if (aValid && bValid) { + if (LocA < LocB) + return &A; + if (LocB < LocA) + return &B; + } else if (aValid) { + return &A; + } else if (bValid) { + return &B; + } + return &A < &B ? &A : &B; }; auto CombineLivenessKind = [](LivenessKind K1, LivenessKind K2) -> LivenessKind { @@ -93,11 +117,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(L1->CausingFact, LivenessKind::Maybe); return LivenessInfo( - CombineUseFact(*L1->CausingUseFact, *L2->CausingUseFact), + CombineCausingFact(*L1->CausingFact, *L2->CausingFact), CombineLivenessKind(L1->Kind, L2->Kind)); }; return Lattice(utils::join( @@ -120,6 +144,14 @@ class AnalysisImpl LivenessInfo(&UF, LivenessKind::Must))); } + // A return operation 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/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 879793575e0b4..08b621f39441c 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2808,15 +2808,15 @@ class LifetimeSafetyReporterImpl : public LifetimeSafetyReporter { << UseExpr->getEndLoc(); } - void reportUseAfterReturn(const Expr *IssueExpr, const Stmt *EscapeStmt, + void reportUseAfterReturn(const Expr *IssueExpr, const Expr *EscapeExpr, SourceLocation ExpiryLoc, Confidence C) override { S.Diag(IssueExpr->getExprLoc(), C == Confidence::Definite - ? diag::warn_lifetime_safety_loan_expires_permissive - : diag::warn_lifetime_safety_loan_expires_strict) + ? diag::warn_lifetime_safety_return_stack_addr_permissive + : diag::warn_lifetime_safety_return_stack_addr_strict) << IssueExpr->getEndLoc(); - S.Diag(EscapeStmt->getBeginLoc(), diag::note_lifetime_safety_returned_here); + S.Diag(EscapeExpr->getBeginLoc(), diag::note_lifetime_safety_returned_here); } private: diff --git a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp index 31148b990d6bd..32796c2ae43c4 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 4f234f0ac6e2d..55bf6253a8c82 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -396,6 +396,118 @@ 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 {{returning reference to stack allocated object}} + return p; // expected-note {{returned here}} +} + +MyObj* conditional_assign_unconditional_return(MyObj safe, bool c){ + MyObj s; + MyObj* p = &safe; + if(c){ + p = &s; // expected-warning {{returning reference to stack allocated object}} + } + return p; // expected-note {{returned here}} +} + +MyObj* conditional_assign_both_branches(MyObj safe, bool c){ + + MyObj s; + MyObj* p = nullptr; + if (c) { + p = &s; // expected-warning {{returning reference to stack allocated object}} + } else { + p = &safe; + } + return p; // expected-note {{returned here}} + +} + +MyObj* reassign_safe_to_local(MyObj safe){ + MyObj local; + MyObj* p = &safe; + + p = &local; // expected-warning {{returning reference to stack allocated object}} + return p; // expected-note {{returned here}} +} + +MyObj* pointer_chain_to_local(){ + MyObj local; + MyObj* p1 = &local; // expected-warning {{returning reference to stack allocated object}} + + MyObj* p2 = p1; + + return p2; // expected-note {{returned here}} +} + +MyObj* multiple_assign_multiple_return(MyObj safe, bool c1, bool c2){ + MyObj local1; + MyObj local2; + MyObj* p = nullptr; + if(c1){ + p = &local1; // expected-warning {{returning reference to stack allocated object}} + return p; // expected-note {{returned here}} + } + else if(c2){ + p = &local2; // expected-warning {{returning reference to stack allocated object}} + return p; // expected-note {{returned here}} + } + p = &safe; + return p; +} + +MyObj* multiple_assign_single_return(MyObj safe, bool c1, bool c2){ + MyObj local1; + MyObj local2; + MyObj* p = nullptr; + if(c1){ + p = &local1; // expected-warning {{returning reference to stack allocated object}} + } + else if(c2){ + p = &local2; // expected-warning {{returning reference to stack allocated object}} + } + else{ + p = &safe; + } + + return p; // expected-note {{returned here}} // expected-note {{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}} +} + +MyObj* uar_before_uaf(MyObj safe, bool c){ + MyObj* p; + { + MyObj local_obj; + p = &local_obj; // expected-warning {{returning reference to stack allocated object}} + if(c){ + return p; // expected-note {{returned here}} + } + + } + (void)*p; + p = &safe; + return p; +} + //===----------------------------------------------------------------------===// // No-Error Cases //===----------------------------------------------------------------------===// @@ -434,6 +546,12 @@ void no_error_loan_from_current_iteration(bool cond) { } } +MyObj* safe_return(MyObj safe){ + MyObj local; + MyObj *p = &local; + p = &safe; // p has been reassigned + return p; // This is safe +} //===----------------------------------------------------------------------===// // Lifetimebound Attribute Tests diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index 0c051847f4d47..5370cde779a8a 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -1202,5 +1202,242 @@ 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(Origin("p"), HasLoansTo({"s"}, "p1")); + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p1")); +} + +TEST_F(LifetimeAnalysisTest, ConditionalAssignUnconditionalReturn) { + SetupTest(R"( + MyObj* target(bool c) { + MyObj s1; + MyObj* p = nullptr; + POINT(before_if); + if (c) { + p = &s1; + POINT(after_assign); + } + POINT(after_if); + return p; + } + )"); + EXPECT_THAT(Origin("p"), HasLoansTo({"s1"}, "after_if")); + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("after_if")); + + EXPECT_THAT(Origin("p"), HasLoansTo({"s1"}, "after_assign")); + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("after_assign")); + + EXPECT_THAT(Origin("p"), HasLoansTo({}, "before_if")); + EXPECT_THAT(Origins({"p"}), MaybeLiveAt("before_if")); +} + +TEST_F(LifetimeAnalysisTest, ConditionalAssignBothBranches) { + SetupTest(R"( + MyObj* target(bool c) { + MyObj s1; + static MyObj s2; + MyObj* p = nullptr; + POINT(before_if); + if (c) { + p = &s1; + POINT(after_assign_if); + } else { + p = &s2; + POINT(after_assign_else); + } + POINT(after_if); + return p; + } + )"); + EXPECT_THAT(Origin("p"), HasLoansTo({"s1", "s2"}, "after_if")); + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("after_if")); + + EXPECT_THAT(Origin("p"), HasLoansTo({"s1"}, "after_assign_if")); + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("after_assign_if")); + + EXPECT_THAT(Origin("p"), HasLoansTo({"s2"}, "after_assign_else")); + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("after_assign_else")); + + EXPECT_THAT(Origin("p"), HasLoansTo({}, "before_if")); + EXPECT_THAT(NoOrigins(), AreLiveAt("before_if")); +} + +TEST_F(LifetimeAnalysisTest, ReassignFromSafeToLocalThenReturn) { + SetupTest(R"( + MyObj* target() { + static MyObj safe_obj; + MyObj local_obj; + MyObj* p = &safe_obj; + POINT(p1); + + p = &local_obj; + POINT(p2); + return p; + } + )"); + + EXPECT_THAT(Origin("p"), HasLoansTo({"local_obj"}, "p2")); + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p2")); + + EXPECT_THAT(Origin("p"), HasLoansTo({"safe_obj"}, "p1")); + EXPECT_THAT(NoOrigins(), AreLiveAt("p1")); +} + +TEST_F(LifetimeAnalysisTest, PointerChainToLocal) { + SetupTest(R"( + MyObj* target() { + MyObj local_obj; + MyObj* p1 = &local_obj; + POINT(p_p1); + MyObj* p2 = p1; + POINT(p_p2); + return p2; + } + )"); + EXPECT_THAT(Origin("p2"), HasLoansTo({"local_obj"}, "p_p2")); + EXPECT_THAT(Origins({"p2"}), MustBeLiveAt("p_p2")); + EXPECT_THAT(Origins({"p1"}), testing::Not(AreLiveAt("p_p2"))); + + EXPECT_THAT(Origin("p1"), HasLoansTo({"local_obj"}, "p_p1")); + EXPECT_THAT(Origins({"p1"}), MustBeLiveAt("p_p1")); +} + +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; + POINT(before_cond); + if(c1){ + p = &local_obj1; + POINT(after_c1); + return p; + } + else if(c2){ + p = &local_obj2; + POINT(after_c2); + return p; + } + p = &global_obj; + POINT(after_cond); + return p; + } + )"); + // Expect two permissive warnings here in each block + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("after_cond")); + EXPECT_THAT(Origin("p"), HasLoansTo({"global_obj"}, "after_cond")); + + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("after_c2")); + EXPECT_THAT(Origin("p"), HasLoansTo({"local_obj2"}, "after_c2")); + + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("after_c1")); + EXPECT_THAT(Origin("p"), HasLoansTo({"local_obj1"}, "after_c1")); + + EXPECT_THAT(Origin("p"), HasLoansTo({}, "before_cond")); + EXPECT_THAT(NoOrigins(), AreLiveAt("before_cond")); +} + +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; + POINT(before_cond); + if(c1){ + p = &local_obj1; + POINT(after_c1); + } + else if(c2){ + p = &local_obj2; + POINT(after_c2); + } + else{ + p = &global_obj; + POINT(after_else); + } + + POINT(after_cond); + return p; + } + )"); + // Expect permissive warning at return statement with both local_obj1 and + // local_obj2 as culprit loans + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("after_cond")); + EXPECT_THAT( + Origin("p"), + HasLoansTo({"global_obj", "local_obj2", "local_obj1"}, "after_cond")); + + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("after_else")); + EXPECT_THAT(Origin("p"), HasLoansTo({"global_obj"}, "after_else")); + + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("after_c2")); + EXPECT_THAT(Origin("p"), HasLoansTo({"local_obj2"}, "after_c2")); + + EXPECT_THAT(Origins({"p"}), MustBeLiveAt("after_c1")); + EXPECT_THAT(Origin("p"), HasLoansTo({"local_obj1"}, "after_c1")); + + EXPECT_THAT(Origin("p"), HasLoansTo({}, "before_cond")); + EXPECT_THAT(NoOrigins(), AreLiveAt("before_cond")); +} + +TEST_F(LifetimeAnalysisTest, UseAfterScopeThenReturn) { + SetupTest(R"( + MyObj* target() { + MyObj* p; + { + MyObj local_obj; + p = &local_obj; + POINT(p1); + } + POINT(p2); + return p; + } + )"); + // Expect a Use-after-scope warning because `p` in `return p` is a use even + // before return + 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")); +} + +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 a return stack address warning as return is before use after scope + 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 >From 1e51cd0de2c302e9d752ab0aaa0778957ff3d82a Mon Sep 17 00:00:00 2001 From: Kashika Akhouri <[email protected]> Date: Tue, 28 Oct 2025 10:59:48 +0000 Subject: [PATCH 3/6] Adding use-after-return in Lifetime Analysis --- clang/lib/Sema/AnalysisBasedWarnings.cpp | 404 +++++++++++------------ 1 file changed, 202 insertions(+), 202 deletions(-) diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 08b621f39441c..b12bceca44df9 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -65,61 +65,61 @@ using namespace clang; //===----------------------------------------------------------------------===// namespace { -class UnreachableCodeHandler : public reachable_code::Callback { - Sema &S; - SourceRange PreviousSilenceableCondVal; - -public: - UnreachableCodeHandler(Sema &s) : S(s) {} - - void HandleUnreachable(reachable_code::UnreachableKind UK, SourceLocation L, - SourceRange SilenceableCondVal, SourceRange R1, - SourceRange R2, bool HasFallThroughAttr) override { - // If the diagnosed code is `[[fallthrough]];` and - // `-Wunreachable-code-fallthrough` is enabled, suppress `code will never - // be executed` warning to avoid generating diagnostic twice - if (HasFallThroughAttr && - !S.getDiagnostics().isIgnored(diag::warn_unreachable_fallthrough_attr, - SourceLocation())) - return; + class UnreachableCodeHandler : public reachable_code::Callback { + Sema &S; + SourceRange PreviousSilenceableCondVal; + + public: + UnreachableCodeHandler(Sema &s) : S(s) {} + + void HandleUnreachable(reachable_code::UnreachableKind UK, SourceLocation L, + SourceRange SilenceableCondVal, SourceRange R1, + SourceRange R2, bool HasFallThroughAttr) override { + // If the diagnosed code is `[[fallthrough]];` and + // `-Wunreachable-code-fallthrough` is enabled, suppress `code will never + // be executed` warning to avoid generating diagnostic twice + if (HasFallThroughAttr && + !S.getDiagnostics().isIgnored(diag::warn_unreachable_fallthrough_attr, + SourceLocation())) + return; - // Avoid reporting multiple unreachable code diagnostics that are - // triggered by the same conditional value. + // Avoid reporting multiple unreachable code diagnostics that are + // triggered by the same conditional value. if (PreviousSilenceableCondVal.isValid() && SilenceableCondVal.isValid() && - PreviousSilenceableCondVal == SilenceableCondVal) - return; - PreviousSilenceableCondVal = SilenceableCondVal; + PreviousSilenceableCondVal == SilenceableCondVal) + return; + PreviousSilenceableCondVal = SilenceableCondVal; - unsigned diag = diag::warn_unreachable; - switch (UK) { - case reachable_code::UK_Break: - diag = diag::warn_unreachable_break; - break; - case reachable_code::UK_Return: - diag = diag::warn_unreachable_return; - break; - case reachable_code::UK_Loop_Increment: - diag = diag::warn_unreachable_loop_increment; - break; - case reachable_code::UK_Other: - break; - } + unsigned diag = diag::warn_unreachable; + switch (UK) { + case reachable_code::UK_Break: + diag = diag::warn_unreachable_break; + break; + case reachable_code::UK_Return: + diag = diag::warn_unreachable_return; + break; + case reachable_code::UK_Loop_Increment: + diag = diag::warn_unreachable_loop_increment; + break; + case reachable_code::UK_Other: + break; + } - S.Diag(L, diag) << R1 << R2; + S.Diag(L, diag) << R1 << R2; - SourceLocation Open = SilenceableCondVal.getBegin(); - if (Open.isValid()) { - SourceLocation Close = SilenceableCondVal.getEnd(); - Close = S.getLocForEndOfToken(Close); - if (Close.isValid()) { - S.Diag(Open, diag::note_unreachable_silence) + SourceLocation Open = SilenceableCondVal.getBegin(); + if (Open.isValid()) { + SourceLocation Close = SilenceableCondVal.getEnd(); + Close = S.getLocForEndOfToken(Close); + if (Close.isValid()) { + S.Diag(Open, diag::note_unreachable_silence) << FixItHint::CreateInsertion(Open, "/* DISABLES CODE */ (") << FixItHint::CreateInsertion(Close, ")"); + } } } - } -}; + }; } // anonymous namespace /// CheckUnreachable - Check for unreachable code. @@ -388,9 +388,9 @@ static void checkThrowInNonThrowingFunc(Sema &S, const FunctionDecl *FD, if (BodyCFG->getExit().pred_empty()) return; visitReachableThrows(BodyCFG, [&](const CXXThrowExpr *Throw, CFGBlock &Block) { - if (throwEscapes(S, Throw, Block, BodyCFG)) - EmitDiagForCXXThrowInNonThrowingFunc(S, Throw->getThrowLoc(), FD); - }); + if (throwEscapes(S, Throw, Block, BodyCFG)) + EmitDiagForCXXThrowInNonThrowingFunc(S, Throw->getThrowLoc(), FD); + }); } static bool isNoexcept(const FunctionDecl *FD) { @@ -803,7 +803,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, } else if (isa<BlockDecl>(D)) { if (const FunctionType *FT = - BlockType->getPointeeType()->getAs<FunctionType>()) { + BlockType->getPointeeType()->getAs<FunctionType>()) { if (FT->getReturnType()->isVoidType()) ReturnsVoid = true; if (FT->getNoReturnAttr()) @@ -815,7 +815,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, // Short circuit for compilation speed. if (CD.checkDiagnostics(Diags, ReturnsVoid, HasNoReturn)) - return; + return; SourceLocation LBrace = Body->getBeginLoc(), RBrace = Body->getEndLoc(); // cpu_dispatch functions permit empty function bodies for ICC compatibility. @@ -892,7 +892,7 @@ class ContainsReference : public ConstEvaluatedExprVisitor<ContainsReference> { typedef ConstEvaluatedExprVisitor<ContainsReference> Inherited; ContainsReference(ASTContext &Context, const DeclRefExpr *Needle) - : Inherited(Context), FoundReference(false), Needle(Needle) {} + : Inherited(Context), FoundReference(false), Needle(Needle) {} void VisitExpr(const Expr *E) { // Stop evaluating if we already have a reference. @@ -1016,7 +1016,7 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use, int RemoveDiagKind = -1; const char *FixitStr = S.getLangOpts().CPlusPlus ? (I->Output ? "true" : "false") - : (I->Output ? "1" : "0"); + : (I->Output ? "1" : "0"); FixItHint Fixit1, Fixit2; switch (Term ? Term->getStmtClass() : Stmt::DeclStmtClass) { @@ -1124,7 +1124,7 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use, << IsCapturedByBlock << User->getSourceRange(); if (RemoveDiagKind != -1) S.Diag(Fixit1.RemoveRange.getBegin(), diag::note_uninit_fixit_remove_cond) - << RemoveDiagKind << Str << I->Output << Fixit1 << Fixit2; + << RemoveDiagKind << Str << I->Output << Fixit1 << Fixit2; Diagnosed = true; } @@ -1294,32 +1294,32 @@ class FallthroughMapper : public DynamicRecursiveASTVisitor { // Don't care about other unreachable statements. } } - // If there are no unreachable statements, this may be a special - // case in CFG: - // case X: { - // A a; // A has a destructor. - // break; - // } - // // <<<< This place is represented by a 'hanging' CFG block. - // case Y: - continue; + // If there are no unreachable statements, this may be a special + // case in CFG: + // case X: { + // A a; // A has a destructor. + // break; + // } + // // <<<< This place is represented by a 'hanging' CFG block. + // case Y: + continue; } - const Stmt *LastStmt = getLastStmt(*P); - if (const AttributedStmt *AS = asFallThroughAttr(LastStmt)) { - markFallthroughVisited(AS); - ++AnnotatedCnt; - continue; // Fallthrough annotation, good. - } + const Stmt *LastStmt = getLastStmt(*P); + if (const AttributedStmt *AS = asFallThroughAttr(LastStmt)) { + markFallthroughVisited(AS); + ++AnnotatedCnt; + continue; // Fallthrough annotation, good. + } - if (!LastStmt) { // This block contains no executable statements. - // Traverse its predecessors. - std::copy(P->pred_begin(), P->pred_end(), - std::back_inserter(BlockQueue)); - continue; - } + if (!LastStmt) { // This block contains no executable statements. + // Traverse its predecessors. + std::copy(P->pred_begin(), P->pred_end(), + std::back_inserter(BlockQueue)); + continue; + } - ++UnannotatedCnt; + ++UnannotatedCnt; } return !!UnannotatedCnt; } @@ -1335,48 +1335,48 @@ class FallthroughMapper : public DynamicRecursiveASTVisitor { return true; } - // We don't want to traverse local type declarations. We analyze their - // methods separately. - bool TraverseDecl(Decl *D) override { return true; } + // We don't want to traverse local type declarations. We analyze their + // methods separately. + bool TraverseDecl(Decl *D) override { return true; } - // We analyze lambda bodies separately. Skip them here. - bool TraverseLambdaExpr(LambdaExpr *LE) override { - // Traverse the captures, but not the body. - for (const auto C : zip(LE->captures(), LE->capture_inits())) - TraverseLambdaCapture(LE, &std::get<0>(C), std::get<1>(C)); - return true; - } + // We analyze lambda bodies separately. Skip them here. + bool TraverseLambdaExpr(LambdaExpr *LE) override { + // Traverse the captures, but not the body. + for (const auto C : zip(LE->captures(), LE->capture_inits())) + TraverseLambdaCapture(LE, &std::get<0>(C), std::get<1>(C)); + return true; + } -private: + private: - static const AttributedStmt *asFallThroughAttr(const Stmt *S) { - if (const AttributedStmt *AS = dyn_cast_or_null<AttributedStmt>(S)) { - if (hasSpecificAttr<FallThroughAttr>(AS->getAttrs())) - return AS; + static const AttributedStmt *asFallThroughAttr(const Stmt *S) { + if (const AttributedStmt *AS = dyn_cast_or_null<AttributedStmt>(S)) { + if (hasSpecificAttr<FallThroughAttr>(AS->getAttrs())) + return AS; + } + return nullptr; } - return nullptr; - } - static const Stmt *getLastStmt(const CFGBlock &B) { - if (const Stmt *Term = B.getTerminatorStmt()) - return Term; - for (const CFGElement &Elem : llvm::reverse(B)) - if (std::optional<CFGStmt> CS = Elem.getAs<CFGStmt>()) - return CS->getStmt(); - // Workaround to detect a statement thrown out by CFGBuilder: - // case X: {} case Y: - // case X: ; case Y: - if (const SwitchCase *SW = dyn_cast_or_null<SwitchCase>(B.getLabel())) - if (!isa<SwitchCase>(SW->getSubStmt())) - return SW->getSubStmt(); + static const Stmt *getLastStmt(const CFGBlock &B) { + if (const Stmt *Term = B.getTerminatorStmt()) + return Term; + for (const CFGElement &Elem : llvm::reverse(B)) + if (std::optional<CFGStmt> CS = Elem.getAs<CFGStmt>()) + return CS->getStmt(); + // Workaround to detect a statement thrown out by CFGBuilder: + // case X: {} case Y: + // case X: ; case Y: + if (const SwitchCase *SW = dyn_cast_or_null<SwitchCase>(B.getLabel())) + if (!isa<SwitchCase>(SW->getSubStmt())) + return SW->getSubStmt(); - return nullptr; - } + return nullptr; + } - bool FoundSwitchStatements; - AttrStmts FallthroughStmts; - Sema &S; - llvm::SmallPtrSet<const CFGBlock *, 16> ReachableBlocks; + bool FoundSwitchStatements; + AttrStmts FallthroughStmts; + Sema &S; + llvm::SmallPtrSet<const CFGBlock *, 16> ReachableBlocks; }; } // anonymous namespace @@ -1384,7 +1384,7 @@ static StringRef getFallthroughAttrSpelling(Preprocessor &PP, SourceLocation Loc) { TokenValue FallthroughTokens[] = { tok::l_square, tok::l_square, - PP.getIdentifierInfo("fallthrough"), + PP.getIdentifierInfo("fallthrough"), tok::r_square, tok::r_square }; @@ -1513,7 +1513,7 @@ static void diagnoseRepeatedUseOfWeak(Sema &S, typedef sema::FunctionScopeInfo::WeakObjectUseMap WeakObjectUseMap; typedef sema::FunctionScopeInfo::WeakUseVector WeakUseVector; typedef std::pair<const Stmt *, WeakObjectUseMap::const_iterator> - StmtUsesPair; + StmtUsesPair; ASTContext &Ctx = S.getASTContext(); @@ -1977,7 +1977,7 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { : getNotes(); } -public: + public: ThreadSafetyReporter(Sema &S, SourceLocation FL, SourceLocation FEL) : S(S), FunLocation(FL), FunEndLocation(FEL), CurrentFunction(nullptr), Verbose(false) {} @@ -2075,18 +2075,18 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { bool ReentrancyMismatch) override { unsigned DiagID = 0; switch (LEK) { - case LEK_LockedSomePredecessors: - DiagID = diag::warn_lock_some_predecessors; - break; - case LEK_LockedSomeLoopIterations: - DiagID = diag::warn_expecting_lock_held_on_loop; - break; - case LEK_LockedAtEndOfFunction: - DiagID = diag::warn_no_unlock; - break; - case LEK_NotLockedAtEndOfFunction: - DiagID = diag::warn_expecting_locked; - break; + case LEK_LockedSomePredecessors: + DiagID = diag::warn_lock_some_predecessors; + break; + case LEK_LockedSomeLoopIterations: + DiagID = diag::warn_expecting_lock_held_on_loop; + break; + case LEK_LockedAtEndOfFunction: + DiagID = diag::warn_no_unlock; + break; + case LEK_NotLockedAtEndOfFunction: + DiagID = diag::warn_expecting_locked; + break; } if (LocEndOfScope.isInvalid()) LocEndOfScope = FunEndLocation; @@ -2132,7 +2132,7 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { break; } PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID) - << D << getLockKindFromAccessKind(AK)); + << D << getLockKindFromAccessKind(AK)); Warnings.emplace_back(std::move(Warning), getNotes()); } @@ -2143,39 +2143,39 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { unsigned DiagID = 0; if (PossibleMatch) { switch (POK) { - case POK_VarAccess: - DiagID = diag::warn_variable_requires_lock_precise; - break; - case POK_VarDereference: - DiagID = diag::warn_var_deref_requires_lock_precise; - break; - case POK_FunctionCall: - DiagID = diag::warn_fun_requires_lock_precise; - break; - case POK_PassByRef: - DiagID = diag::warn_guarded_pass_by_reference; - break; - case POK_PtPassByRef: - DiagID = diag::warn_pt_guarded_pass_by_reference; - break; - case POK_ReturnByRef: - DiagID = diag::warn_guarded_return_by_reference; - break; - case POK_PtReturnByRef: - DiagID = diag::warn_pt_guarded_return_by_reference; - break; - case POK_PassPointer: - DiagID = diag::warn_guarded_pass_pointer; - break; - case POK_PtPassPointer: - DiagID = diag::warn_pt_guarded_pass_pointer; - break; - case POK_ReturnPointer: - DiagID = diag::warn_guarded_return_pointer; - break; - case POK_PtReturnPointer: - DiagID = diag::warn_pt_guarded_return_pointer; - break; + case POK_VarAccess: + DiagID = diag::warn_variable_requires_lock_precise; + break; + case POK_VarDereference: + DiagID = diag::warn_var_deref_requires_lock_precise; + break; + case POK_FunctionCall: + DiagID = diag::warn_fun_requires_lock_precise; + break; + case POK_PassByRef: + DiagID = diag::warn_guarded_pass_by_reference; + break; + case POK_PtPassByRef: + DiagID = diag::warn_pt_guarded_pass_by_reference; + break; + case POK_ReturnByRef: + DiagID = diag::warn_guarded_return_by_reference; + break; + case POK_PtReturnByRef: + DiagID = diag::warn_pt_guarded_return_by_reference; + break; + case POK_PassPointer: + DiagID = diag::warn_guarded_pass_pointer; + break; + case POK_PtPassPointer: + DiagID = diag::warn_pt_guarded_pass_pointer; + break; + case POK_ReturnPointer: + DiagID = diag::warn_guarded_return_pointer; + break; + case POK_PtReturnPointer: + DiagID = diag::warn_pt_guarded_return_pointer; + break; } PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID) << Kind << D @@ -2191,39 +2191,39 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { Warnings.emplace_back(std::move(Warning), getNotes(Note)); } else { switch (POK) { - case POK_VarAccess: - DiagID = diag::warn_variable_requires_lock; - break; - case POK_VarDereference: - DiagID = diag::warn_var_deref_requires_lock; - break; - case POK_FunctionCall: - DiagID = diag::warn_fun_requires_lock; - break; - case POK_PassByRef: - DiagID = diag::warn_guarded_pass_by_reference; - break; - case POK_PtPassByRef: - DiagID = diag::warn_pt_guarded_pass_by_reference; - break; - case POK_ReturnByRef: - DiagID = diag::warn_guarded_return_by_reference; - break; - case POK_PtReturnByRef: - DiagID = diag::warn_pt_guarded_return_by_reference; - break; - case POK_PassPointer: - DiagID = diag::warn_guarded_pass_pointer; - break; - case POK_PtPassPointer: - DiagID = diag::warn_pt_guarded_pass_pointer; - break; - case POK_ReturnPointer: - DiagID = diag::warn_guarded_return_pointer; - break; - case POK_PtReturnPointer: - DiagID = diag::warn_pt_guarded_return_pointer; - break; + case POK_VarAccess: + DiagID = diag::warn_variable_requires_lock; + break; + case POK_VarDereference: + DiagID = diag::warn_var_deref_requires_lock; + break; + case POK_FunctionCall: + DiagID = diag::warn_fun_requires_lock; + break; + case POK_PassByRef: + DiagID = diag::warn_guarded_pass_by_reference; + break; + case POK_PtPassByRef: + DiagID = diag::warn_pt_guarded_pass_by_reference; + break; + case POK_ReturnByRef: + DiagID = diag::warn_guarded_return_by_reference; + break; + case POK_PtReturnByRef: + DiagID = diag::warn_pt_guarded_return_by_reference; + break; + case POK_PassPointer: + DiagID = diag::warn_guarded_pass_pointer; + break; + case POK_PtPassPointer: + DiagID = diag::warn_pt_guarded_pass_pointer; + break; + case POK_ReturnPointer: + DiagID = diag::warn_guarded_return_pointer; + break; + case POK_PtReturnPointer: + DiagID = diag::warn_pt_guarded_return_pointer; + break; } PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID) << Kind << D @@ -2241,7 +2241,7 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { SourceLocation Loc) override { PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_acquire_requires_negative_cap) - << Kind << LockName << Neg); + << Kind << LockName << Neg); Warnings.emplace_back(std::move(Warning), getNotes()); } @@ -2423,7 +2423,7 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler { public: UnsafeBufferUsageReporter(Sema &S, bool SuggestSuggestions) - : S(S), SuggestSuggestions(SuggestSuggestions) {} + : S(S), SuggestSuggestions(SuggestSuggestions) {} void handleUnsafeOperation(const Stmt *Operation, bool IsRelatedToDecl, ASTContext &Ctx) override { @@ -2826,7 +2826,7 @@ class LifetimeSafetyReporterImpl : public LifetimeSafetyReporter { } // namespace clang::lifetimes void clang::sema::AnalysisBasedWarnings::IssueWarnings( - TranslationUnitDecl *TU) { + TranslationUnitDecl *TU) { if (!TU) return; // This is unexpected, give up quietly. @@ -2939,13 +2939,13 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( AC.getCFGBuildOptions().setAllAlwaysAdd(); } else { AC.getCFGBuildOptions() - .setAlwaysAdd(Stmt::BinaryOperatorClass) - .setAlwaysAdd(Stmt::CompoundAssignOperatorClass) - .setAlwaysAdd(Stmt::BlockExprClass) - .setAlwaysAdd(Stmt::CStyleCastExprClass) - .setAlwaysAdd(Stmt::DeclRefExprClass) - .setAlwaysAdd(Stmt::ImplicitCastExprClass) - .setAlwaysAdd(Stmt::UnaryOperatorClass); + .setAlwaysAdd(Stmt::BinaryOperatorClass) + .setAlwaysAdd(Stmt::CompoundAssignOperatorClass) + .setAlwaysAdd(Stmt::BlockExprClass) + .setAlwaysAdd(Stmt::CStyleCastExprClass) + .setAlwaysAdd(Stmt::DeclRefExprClass) + .setAlwaysAdd(Stmt::ImplicitCastExprClass) + .setAlwaysAdd(Stmt::UnaryOperatorClass); } // Install the logical handler. >From 9411d9c6acb0e36d432d2334183b665c8d528483 Mon Sep 17 00:00:00 2001 From: Kashika Akhouri <[email protected]> Date: Tue, 28 Oct 2025 10:59:48 +0000 Subject: [PATCH 4/6] Adding use-after-return in Lifetime Analysis --- clang/test/Sema/warn-lifetime-safety.cpp | 66 ++++++++++++++++-------- 1 file changed, 45 insertions(+), 21 deletions(-) diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 55bf6253a8c82..adeace7c37b9b 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -416,67 +416,73 @@ MyObj* conditional_assign_unconditional_return(MyObj safe, bool c){ return p; // expected-note {{returned here}} } -MyObj* conditional_assign_both_branches(MyObj safe, bool c){ +View conditional_assign_both_branches(MyObj safe, bool c){ MyObj s; - MyObj* p = nullptr; + View p; if (c) { - p = &s; // expected-warning {{returning reference to stack allocated object}} + p = s; // expected-warning {{returning reference to stack allocated object}} } else { - p = &safe; + p = safe; } return p; // expected-note {{returned here}} } -MyObj* reassign_safe_to_local(MyObj safe){ +View reassign_safe_to_local(MyObj safe){ MyObj local; - MyObj* p = &safe; + View p = safe; - p = &local; // expected-warning {{returning reference to stack allocated object}} + p = local; // expected-warning {{returning reference to stack allocated object}} return p; // expected-note {{returned here}} } -MyObj* pointer_chain_to_local(){ +View pointer_chain_to_local(){ MyObj local; - MyObj* p1 = &local; // expected-warning {{returning reference to stack allocated object}} + View p1 = local; // expected-warning {{returning reference to stack allocated object}} - MyObj* p2 = p1; + View p2 = p1; return p2; // expected-note {{returned here}} } -MyObj* multiple_assign_multiple_return(MyObj safe, bool c1, bool c2){ +View multiple_assign_multiple_return(MyObj safe, bool c1, bool c2){ MyObj local1; MyObj local2; - MyObj* p = nullptr; + View p; if(c1){ - p = &local1; // expected-warning {{returning reference to stack allocated object}} + p = local1; // expected-warning {{returning reference to stack allocated object}} return p; // expected-note {{returned here}} } else if(c2){ - p = &local2; // expected-warning {{returning reference to stack allocated object}} + p = local2; // expected-warning {{returning reference to stack allocated object}} return p; // expected-note {{returned here}} } - p = &safe; + p = safe; return p; } -MyObj* multiple_assign_single_return(MyObj safe, bool c1, bool c2){ +View multiple_assign_single_return(MyObj safe, bool c1, bool c2){ MyObj local1; MyObj local2; - MyObj* p = nullptr; + View p; if(c1){ - p = &local1; // expected-warning {{returning reference to stack allocated object}} + p = local1; // expected-warning {{returning reference to stack allocated object}} } else if(c2){ - p = &local2; // expected-warning {{returning reference to stack allocated object}} + p = local2; // expected-warning {{returning reference to stack allocated object}} } else{ - p = &safe; + p = safe; } - return p; // expected-note {{returned here}} // expected-note {{returned here}} + return p; // expected-note 2 {{returned here}} +} + +View direct_return_of_local(){ + MyObj stack; + return stack; // expected-warning {{returning reference to stack allocated object}} + // expected-note@-1 {{returned here}} } //===----------------------------------------------------------------------===// @@ -700,3 +706,21 @@ void lifetimebound_ctor() { } (void)v; } + +View lifetimebound_return_of_local(){ + MyObj stack; + return Identity(stack); // expected-warning {{returning reference to stack allocated object}} + // expected-note@-1 {{returned here}} +} + +// FIXME: The analysis does not currently model the lifetime of by-value +// parameters, so it fails to diagnose this UAR violation. +View lifetimebound_return_of_by_value_param(MyObj stack_param) { + return Identity(stack_param); +} + +// FIXME: The analysis does not currently model the lifetime of by-value +// parameters, so it fails to diagnose this UAF violation. +void uaf_from_by_value_param_failing(MyObj param, View* out_p) { + *out_p = Identity(param); +} \ No newline at end of file >From 90a831154dd61729aba2015c53b5e9fd4e7f83b1 Mon Sep 17 00:00:00 2001 From: Kashika Akhouri <[email protected]> Date: Tue, 28 Oct 2025 10:59:48 +0000 Subject: [PATCH 5/6] Adding use-after-return in Lifetime Analysis --- clang/lib/Analysis/LifetimeSafety/Checker.cpp | 2 -- clang/test/Sema/warn-lifetime-safety.cpp | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 64ca5439cc264..5dc605fbf407b 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -63,11 +63,9 @@ class LifetimeChecker { : LoanPropagation(LoanPropagation), LiveOrigins(LiveOrigins), FactMgr(FM), Reporter(Reporter) { for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>()) { - llvm::SmallVector<const ExpireFact *> BlockExpires; for (const Fact *F : FactMgr.getFacts(B)) { if (const auto *EF = F->getAs<ExpireFact>()) { checkExpiry(EF); - BlockExpires.push_back(EF); } } } diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index adeace7c37b9b..a07bfe61f3d26 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -485,6 +485,13 @@ View direct_return_of_local(){ // expected-note@-1 {{returned here}} } +MyObj& reference_return_of_local(){ + MyObj stack; + return stack; // expected-warning {{returning reference to stack allocated object}} + // expected-note@-1 {{returned here}} + // expected-warning@-2 {{reference to stack memory associated with local variable 'stack' returned}} +} + //===----------------------------------------------------------------------===// // Use-After-Scope & Use-After-Return (Return-Stack-Address) Combined // These are cases where the diagnostic kind is determined by location @@ -564,6 +571,7 @@ MyObj* safe_return(MyObj safe){ //===----------------------------------------------------------------------===// View Identity(View v [[clang::lifetimebound]]); +const MyObj& IdentityRef(const MyObj& obj [[clang::lifetimebound]]); View Choose(bool cond, View a [[clang::lifetimebound]], View b [[clang::lifetimebound]]); MyObj* GetPointer(const MyObj& obj [[clang::lifetimebound]]); @@ -713,6 +721,13 @@ View lifetimebound_return_of_local(){ // expected-note@-1 {{returned here}} } +const MyObj& lifetimebound_return_ref_to_local() { + MyObj stack; + return IdentityRef(stack); // expected-warning {{returning reference to stack allocated object}} + // expected-note@-1 {{returned here}} + // expected-warning@-2 {{reference to stack memory associated with local variable 'stack' returned}} +} + // FIXME: The analysis does not currently model the lifetime of by-value // parameters, so it fails to diagnose this UAR violation. View lifetimebound_return_of_by_value_param(MyObj stack_param) { >From 58fd6c81428c0e0fd269c2aa6684cdfdd9089728 Mon Sep 17 00:00:00 2001 From: Kashika Akhouri <[email protected]> Date: Tue, 28 Oct 2025 10:59:48 +0000 Subject: [PATCH 6/6] Adding use-after-return in Lifetime Analysis --- clang/lib/Analysis/LifetimeSafety/Checker.cpp | 11 ++++------- clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp | 3 --- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 5dc605fbf407b..92ab1bb8f9e33 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -62,13 +62,10 @@ class LifetimeChecker { AnalysisDeclContext &ADC, LifetimeSafetyReporter *Reporter) : LoanPropagation(LoanPropagation), LiveOrigins(LiveOrigins), FactMgr(FM), Reporter(Reporter) { - for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>()) { - for (const Fact *F : FactMgr.getFacts(B)) { - if (const auto *EF = F->getAs<ExpireFact>()) { + for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>()) + for (const Fact *F : FactMgr.getFacts(B)) + if (const auto *EF = F->getAs<ExpireFact>()) checkExpiry(EF); - } - } - } issuePendingWarnings(); } @@ -107,7 +104,7 @@ class LifetimeChecker { if (LastConf >= CurConfidence) return; FinalWarningsMap[ExpiredLoan] = {/*ExpiryLoc=*/EF->getExpiryLoc(), - /*BestCausingFact*/ BestCausingFact, + /*BestCausingFact=*/BestCausingFact, /*ConfidenceLevel=*/CurConfidence}; } diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 72a1b52786d54..39e7882655711 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -59,7 +59,6 @@ void FactsGenerator::run() { for (const CFGBlock *Block : *AC.getAnalysis<PostOrderCFGView>()) { CurrentBlockFacts.clear(); EscapesInCurrentBlock.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>()) @@ -68,8 +67,6 @@ void FactsGenerator::run() { Element.getAs<CFGAutomaticObjDtor>()) handleDestructor(*DtorOpt); } - CurrentBlockFacts.append(EscapesInCurrentBlock.begin(), - EscapesInCurrentBlock.end()); CurrentBlockFacts.append(EscapesInCurrentBlock.begin(), EscapesInCurrentBlock.end()); FactMgr.addBlockFacts(Block, CurrentBlockFacts); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
