https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/182709
>From 29ca3552e763c75a4785db9586dd3f8024688e34 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <[email protected]> Date: Sat, 21 Feb 2026 22:43:23 +0000 Subject: [PATCH] remove-confidence --- .../Analyses/LifetimeSafety/LifetimeSafety.h | 18 +--- .../Analyses/LifetimeSafety/LiveOrigins.h | 24 +---- clang/lib/Analysis/LifetimeSafety/Checker.cpp | 85 +++++++-------- .../Analysis/LifetimeSafety/LiveOrigins.cpp | 53 +++------- clang/lib/Sema/AnalysisBasedWarnings.cpp | 8 +- clang/test/Sema/warn-lifetime-safety.cpp | 62 +++++------ .../unittests/Analysis/LifetimeSafetyTest.cpp | 100 ++++++------------ 7 files changed, 119 insertions(+), 231 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index d7aadf4cf04ca..095b8409d753d 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -28,20 +28,11 @@ #include "clang/Analysis/Analyses/LifetimeSafety/MovedLoans.h" #include "clang/Analysis/Analyses/LifetimeSafety/Origins.h" #include "clang/Analysis/AnalysisDeclContext.h" -#include <cstdint> +#include <cstddef> #include <memory> namespace clang::lifetimes { -// TODO: Deprecate and remove Confidence as this is no more used as a -// differentiator between strict and permissive warnings. -/// Enum to track the confidence level of a potential error. -enum class Confidence : uint8_t { - None, - Maybe, // Reported as a potential error (-Wlifetime-safety-strict) - Definite // Reported as a definite error (-Wlifetime-safety-permissive) -}; - struct LifetimeSafetyOpts { /// Maximum number of CFG blocks to analyze. Functions with larger CFGs will /// be skipped. @@ -70,14 +61,13 @@ class LifetimeSafetySemaHelper { virtual ~LifetimeSafetySemaHelper() = default; virtual void reportUseAfterFree(const Expr *IssueExpr, const Expr *UseExpr, - const Expr *MovedExpr, SourceLocation FreeLoc, - Confidence Confidence) {} + const Expr *MovedExpr, + SourceLocation FreeLoc) {} virtual void reportUseAfterReturn(const Expr *IssueExpr, const Expr *ReturnExpr, const Expr *MovedExpr, - SourceLocation ExpiryLoc, - Confidence Confidence) {} + SourceLocation ExpiryLoc) {} virtual void reportDanglingField(const Expr *IssueExpr, const FieldDecl *Field, diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h index 35b4224883cce..fa9deed9f3423 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h @@ -34,12 +34,6 @@ 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) - Must // Live on all paths (must-be-live) -}; - /// Information about why an origin is live at a program point. struct LivenessInfo { /// The use that makes the origin live. If liveness is propagated from @@ -48,28 +42,16 @@ struct LivenessInfo { /// This is 'null' when the origin is not live. CausingFactType CausingFact; - /// The kind of liveness of the origin. - /// `Must`: The origin is live on all control-flow paths from the current - /// point to the function's exit (i.e. the current point is dominated by a set - /// of uses). - /// `Maybe`: indicates it is live on some but not all paths. - /// - /// This determines the diagnostic's confidence level. - /// `Must`-be-alive at expiration implies a definite use-after-free, - /// while `Maybe`-be-alive suggests a potential one on some paths. - LivenessKind Kind; - - LivenessInfo() : CausingFact(nullptr), Kind(LivenessKind::Dead) {} - LivenessInfo(CausingFactType CF, LivenessKind K) : CausingFact(CF), Kind(K) {} + LivenessInfo() : CausingFact(nullptr) {} + LivenessInfo(CausingFactType CF) : CausingFact(CF) {} bool operator==(const LivenessInfo &Other) const { - return CausingFact == Other.CausingFact && Kind == Other.Kind; + return CausingFact == Other.CausingFact; } bool operator!=(const LivenessInfo &Other) const { return !(*this == Other); } void Profile(llvm::FoldingSetNodeID &IDBuilder) const { IDBuilder.AddPointer(CausingFact.getOpaqueValue()); - IDBuilder.Add(Kind); } }; diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 78c2a6dba3eb6..22a69d2b719f3 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -24,24 +24,11 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "llvm/ADT/DenseMap.h" -#include "llvm/ADT/DenseSet.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/TimeProfiler.h" namespace clang::lifetimes::internal { -static Confidence livenessKindToConfidence(LivenessKind K) { - switch (K) { - case LivenessKind::Must: - return Confidence::Definite; - case LivenessKind::Maybe: - return Confidence::Maybe; - case LivenessKind::Dead: - return Confidence::None; - } - llvm_unreachable("unknown liveness kind"); -} - namespace { /// Struct to store the complete context for a potential lifetime violation. @@ -50,7 +37,6 @@ struct PendingWarning { llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> CausingFact; const Expr *MovedExpr; const Expr *InvalidatedByExpr; - Confidence ConfidenceLevel; }; using AnnotationTarget = @@ -69,6 +55,19 @@ class LifetimeChecker { LifetimeSafetySemaHelper *SemaHelper; ASTContext &AST; + static SourceLocation + GetFactLoc(llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> F) { + if (const auto *UF = F.dyn_cast<const UseFact *>()) + return UF->getUseExpr()->getExprLoc(); + if (const auto *OEF = F.dyn_cast<const OriginEscapesFact *>()) { + if (auto *ReturnEsc = dyn_cast<ReturnEscapeFact>(OEF)) + return ReturnEsc->getReturnExpr()->getExprLoc(); + if (auto *FieldEsc = dyn_cast<FieldEscapeFact>(OEF)) + return FieldEsc->getFieldDecl()->getLocation(); + } + llvm_unreachable("unhandled causing fact in PointerUnion"); + } + public: LifetimeChecker(const LoanPropagationAnalysis &LoanPropagation, const MovedLoansAnalysis &MovedLoans, @@ -139,9 +138,7 @@ class LifetimeChecker { /// /// 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 - /// warning with the appropriate confidence level based on the liveness - /// information. The confidence reflects whether the origin is definitely - /// or maybe live at this point. + /// warning. /// /// Note: This implementation considers only the confidence of origin /// liveness. Future enhancements could also consider the confidence of loan @@ -153,34 +150,31 @@ class LifetimeChecker { MovedExpr = *ME; LivenessMap Origins = LiveOrigins.getLiveOriginsAt(EF); - Confidence CurConfidence = Confidence::None; // The UseFact or OriginEscapesFact most indicative of a lifetime error, // prioritized by earlier source location. - llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> - BestCausingFact = nullptr; + llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> CausingFact = + nullptr; for (auto &[OID, LiveInfo] : Origins) { LoanSet HeldLoans = LoanPropagation.getLoans(OID, EF); if (!HeldLoans.contains(ExpiredLoan)) continue; - // Loan is defaulted. - Confidence NewConfidence = livenessKindToConfidence(LiveInfo.Kind); - if (CurConfidence < NewConfidence) { - CurConfidence = NewConfidence; - BestCausingFact = LiveInfo.CausingFact; - } + + if (!CausingFact || + GetFactLoc(LiveInfo.CausingFact) < GetFactLoc(CausingFact)) + CausingFact = LiveInfo.CausingFact; } - if (!BestCausingFact) - return; - // We have a use-after-free. - Confidence LastConf = FinalWarningsMap.lookup(ExpiredLoan).ConfidenceLevel; - if (LastConf >= CurConfidence) + if (!CausingFact) return; - FinalWarningsMap[ExpiredLoan] = {/*ExpiryLoc=*/EF->getExpiryLoc(), - /*BestCausingFact=*/BestCausingFact, - /*MovedExpr=*/MovedExpr, - /*InvalidatedByExpr=*/nullptr, - /*ConfidenceLevel=*/CurConfidence}; + + auto It = FinalWarningsMap.find(ExpiredLoan); + if (It == FinalWarningsMap.end() || + GetFactLoc(CausingFact) < GetFactLoc(It->second.CausingFact)) { + FinalWarningsMap[ExpiredLoan] = {/*ExpiryLoc=*/EF->getExpiryLoc(), + CausingFact, + /*MovedExpr=*/MovedExpr, + /*InvalidatedByExpr=*/nullptr}; + } } /// Checks for use-after-invalidation errors when a container is modified. @@ -216,16 +210,15 @@ class LifetimeChecker { LoanSet HeldLoans = LoanPropagation.getLoans(OID, IOF); for (LoanID LiveLoanID : HeldLoans) if (IsInvalidated(FactMgr.getLoanMgr().getLoan(LiveLoanID))) { - Confidence CurConfidence = livenessKindToConfidence(LiveInfo.Kind); - Confidence LastConf = - FinalWarningsMap.lookup(LiveLoanID).ConfidenceLevel; - if (LastConf < CurConfidence) { + auto It = FinalWarningsMap.find(LiveLoanID); + if (It == FinalWarningsMap.end() || + GetFactLoc(LiveInfo.CausingFact) < + GetFactLoc(It->second.CausingFact)) { FinalWarningsMap[LiveLoanID] = { /*ExpiryLoc=*/{}, /*CausingFact=*/LiveInfo.CausingFact, /*MovedExpr=*/nullptr, - /*InvalidatedByExpr=*/IOF->getInvalidationExpr(), - /*ConfidenceLevel=*/CurConfidence}; + /*InvalidatedByExpr=*/IOF->getInvalidationExpr()}; } } } @@ -245,7 +238,6 @@ class LifetimeChecker { InvalidatedPVD = PL->getParmVarDecl(); llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> CausingFact = Warning.CausingFact; - Confidence Confidence = Warning.ConfidenceLevel; const Expr *MovedExpr = Warning.MovedExpr; SourceLocation ExpiryLoc = Warning.ExpiryLoc; @@ -260,13 +252,12 @@ class LifetimeChecker { } else SemaHelper->reportUseAfterFree(IssueExpr, UF->getUseExpr(), MovedExpr, - ExpiryLoc, Confidence); + ExpiryLoc); } else if (const auto *OEF = CausingFact.dyn_cast<const OriginEscapesFact *>()) { if (const auto *RetEscape = dyn_cast<ReturnEscapeFact>(OEF)) - SemaHelper->reportUseAfterReturn(IssueExpr, - RetEscape->getReturnExpr(), - MovedExpr, ExpiryLoc, Confidence); + SemaHelper->reportUseAfterReturn( + IssueExpr, RetEscape->getReturnExpr(), MovedExpr, ExpiryLoc); else if (const auto *FieldEscape = dyn_cast<FieldEscapeFact>(OEF)) SemaHelper->reportDanglingField( IssueExpr, FieldEscape->getFieldDecl(), MovedExpr, ExpiryLoc); diff --git a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp index f210fb4d752d4..b18999fbbbef4 100644 --- a/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LiveOrigins.cpp @@ -35,21 +35,9 @@ struct Lattice { OS << " <empty>\n"; for (const auto &Entry : LiveOrigins) { OriginID OID = Entry.first; - const LivenessInfo &Info = Entry.second; OS << " "; OM.dump(OID, OS); - OS << " is "; - switch (Info.Kind) { - case LivenessKind::Must: - OS << "definitely"; - break; - case LivenessKind::Maybe: - OS << "maybe"; - break; - case LivenessKind::Dead: - llvm_unreachable("liveness kind of live origins should not be dead."); - } - OS << " live at this point\n"; + OS << " is live at this point\n"; } } }; @@ -67,8 +55,7 @@ static SourceLocation GetFactLoc(CausingFactType F) { } /// The analysis that tracks which origins are live, with granular information -/// about the causing use fact and confidence level. This is a backward -/// analysis. +/// about the causing use fact. This is a backward analysis. class AnalysisImpl : public DataflowAnalysis<AnalysisImpl, Lattice, Direction::Backward> { @@ -83,8 +70,6 @@ class AnalysisImpl Lattice getInitialState() { return Lattice(Factory.getEmptyMap()); } /// Merges two lattices by combining liveness information. - /// When the same origin has different confidence levels, we take the lower - /// one. Lattice join(Lattice L1, Lattice L2) const { LivenessMap Merged = L1.LiveOrigins; // Take the earliest Fact to make the join hermetic and commutative. @@ -96,34 +81,24 @@ class AnalysisImpl return A; return GetFactLoc(A) < GetFactLoc(B) ? A : B; }; - auto CombineLivenessKind = [](LivenessKind K1, - LivenessKind K2) -> LivenessKind { - assert(K1 != LivenessKind::Dead && "LivenessKind should not be dead."); - assert(K2 != LivenessKind::Dead && "LivenessKind should not be dead."); - // Only return "Must" if both paths are "Must", otherwise Maybe. - if (K1 == LivenessKind::Must && K2 == LivenessKind::Must) - return LivenessKind::Must; - return LivenessKind::Maybe; - }; auto CombineLivenessInfo = [&](const LivenessInfo *L1, const LivenessInfo *L2) -> LivenessInfo { assert((L1 || L2) && "unexpectedly merging 2 empty sets"); if (!L1) - return LivenessInfo(L2->CausingFact, LivenessKind::Maybe); + return LivenessInfo(L2->CausingFact); if (!L2) - return LivenessInfo(L1->CausingFact, LivenessKind::Maybe); - return LivenessInfo(CombineCausingFact(L1->CausingFact, L2->CausingFact), - CombineLivenessKind(L1->Kind, L2->Kind)); + return LivenessInfo(L1->CausingFact); + return LivenessInfo(CombineCausingFact(L1->CausingFact, L2->CausingFact)); }; return Lattice(utils::join( L1.LiveOrigins, L2.LiveOrigins, Factory, CombineLivenessInfo, // A symmetric join is required here. If an origin is live on one - // branch but not the other, its confidence must be demoted to `Maybe`. + // branch but not the other, it is live in the joined state. utils::JoinKind::Symmetric)); } - /// A read operation makes the origin live with definite confidence, as it - /// dominates this program point. A write operation kills the liveness of + /// A read operation makes the origin live, as it dominates this program + /// point. A write operation kills the liveness of /// the origin since it overwrites the value. Lattice transfer(Lattice In, const UseFact &UF) { Lattice Out = In; @@ -134,21 +109,17 @@ class AnalysisImpl if (UF.isWritten()) { Out = Lattice(Factory.remove(Out.LiveOrigins, OID)); } else { - // Read makes origin live with definite confidence (dominates this - // point). - Out = Lattice(Factory.add(Out.LiveOrigins, OID, - LivenessInfo(&UF, LivenessKind::Must))); + // Read makes origin live. + Out = Lattice(Factory.add(Out.LiveOrigins, OID, LivenessInfo(&UF))); } } return Out; } - /// An escaping origin (e.g., via return) makes the origin live with definite - /// confidence, as it dominates this program point. + /// An escaping origin (e.g., via return) makes the origin live. Lattice transfer(Lattice In, const OriginEscapesFact &OEF) { OriginID OID = OEF.getEscapedOriginID(); - return Lattice(Factory.add(In.LiveOrigins, OID, - LivenessInfo(&OEF, LivenessKind::Must))); + return Lattice(Factory.add(In.LiveOrigins, OID, LivenessInfo(&OEF))); } /// Issuing a new loan to an origin kills its liveness. diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 20c41096501fb..e3dbaec6cc3a9 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2861,8 +2861,8 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { LifetimeSafetySemaHelperImpl(Sema &S) : S(S) {} void reportUseAfterFree(const Expr *IssueExpr, const Expr *UseExpr, - const Expr *MovedExpr, SourceLocation FreeLoc, - Confidence) override { + const Expr *MovedExpr, + SourceLocation FreeLoc) override { S.Diag(IssueExpr->getExprLoc(), MovedExpr ? diag::warn_lifetime_safety_use_after_scope_moved : diag::warn_lifetime_safety_use_after_scope) @@ -2876,8 +2876,8 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { } void reportUseAfterReturn(const Expr *IssueExpr, const Expr *ReturnExpr, - const Expr *MovedExpr, SourceLocation ExpiryLoc, - Confidence) override { + const Expr *MovedExpr, + SourceLocation ExpiryLoc) override { S.Diag(IssueExpr->getExprLoc(), MovedExpr ? diag::warn_lifetime_safety_return_stack_addr_moved : diag::warn_lifetime_safety_return_stack_addr) diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 097f3279d8e54..f5d1871ec36a1 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -46,11 +46,10 @@ View construct_view(const MyObj &obj [[clang::lifetimebound]]) { void use(View); //===----------------------------------------------------------------------===// -// Basic Definite Use-After-Free (-W...permissive) -// These are cases where the pointer is guaranteed to be dangling at the use site. +// Basic Use-After-Free //===----------------------------------------------------------------------===// -void definite_simple_case() { +void simple_case() { MyObj* p; { MyObj s; @@ -59,7 +58,7 @@ void definite_simple_case() { (void)*p; // expected-note {{later used here}} } -void definite_simple_case_gsl() { +void simple_case_gsl() { View v; { MyObj s; @@ -86,7 +85,7 @@ void no_use_no_error_gsl() { // 'v' is dangling here, but since it is never used, no warning is issued. } -void definite_pointer_chain() { +void pointer_chain() { MyObj* p; MyObj* q; { @@ -97,7 +96,7 @@ void definite_pointer_chain() { (void)*q; // expected-note {{later used here}} } -void definite_propagation_gsl() { +void propagation_gsl() { View v1, v2; { MyObj s; @@ -107,7 +106,7 @@ void definite_propagation_gsl() { v2.use(); // expected-note {{later used here}} } -void definite_multiple_uses_one_warning() { +void multiple_uses_one_warning() { MyObj* p; { MyObj s; @@ -120,7 +119,7 @@ void definite_multiple_uses_one_warning() { (void)*q; } -void definite_multiple_pointers() { +void multiple_pointers() { MyObj *p, *q, *r; { MyObj s; @@ -133,7 +132,7 @@ void definite_multiple_pointers() { (void)*r; // expected-note {{later used here}} } -void definite_single_pointer_multiple_loans(bool cond) { +void single_pointer_multiple_loans(bool cond) { MyObj *p; if (cond){ MyObj s; @@ -146,7 +145,7 @@ void definite_single_pointer_multiple_loans(bool cond) { (void)*p; // expected-note 2 {{later used here}} } -void definite_single_pointer_multiple_loans_gsl(bool cond) { +void single_pointer_multiple_loans_gsl(bool cond) { View v; if (cond){ MyObj s; @@ -159,7 +158,7 @@ void definite_single_pointer_multiple_loans_gsl(bool cond) { v.use(); // expected-note 2 {{later used here}} } -void definite_if_branch(bool cond) { +void if_branch(bool cond) { MyObj safe; MyObj* p = &safe; if (cond) { @@ -169,7 +168,7 @@ void definite_if_branch(bool cond) { (void)*p; // expected-note {{later used here}} } -void potential_if_branch(bool cond) { +void if_branch_potential(bool cond) { MyObj safe; MyObj* p = &safe; if (cond) { @@ -182,7 +181,7 @@ void potential_if_branch(bool cond) { p = &safe; } -void definite_if_branch_gsl(bool cond) { +void if_branch_gsl(bool cond) { MyObj safe; View v = safe; if (cond) { @@ -192,7 +191,7 @@ void definite_if_branch_gsl(bool cond) { v.use(); // expected-note {{later used here}} } -void definite_potential_together(bool cond) { +void potential_together(bool cond) { MyObj safe; MyObj* p_maybe = &safe; MyObj* p_definite = nullptr; @@ -209,7 +208,7 @@ void definite_potential_together(bool cond) { (void)*p_maybe; // expected-note {{later used here}} } -void definite_overrides_potential(bool cond) { +void overrides_potential(bool cond) { MyObj safe; MyObj* p; MyObj* q; @@ -224,13 +223,12 @@ void definite_overrides_potential(bool cond) { q = &safe; } - // The use of 'p' is a definite error because it was never rescued. - (void)*q; - (void)*p; // expected-note {{later used here}} + (void)*q; // expected-note {{later used here}} + (void)*p; (void)*q; } -void potential_due_to_conditional_killing(bool cond) { +void due_to_conditional_killing(bool cond) { MyObj safe; MyObj* q; { @@ -244,7 +242,7 @@ void potential_due_to_conditional_killing(bool cond) { (void)*q; // expected-note {{later used here}} } -void potential_for_loop_use_after_loop_body(MyObj safe) { +void for_loop_use_after_loop_body(MyObj safe) { MyObj* p = &safe; for (int i = 0; i < 1; ++i) { MyObj s; @@ -263,7 +261,7 @@ void safe_for_loop_gsl() { } } -void potential_for_loop_gsl() { +void for_loop_gsl() { MyObj safe; View v = safe; for (int i = 0; i < 1; ++i) { @@ -273,7 +271,7 @@ void potential_for_loop_gsl() { v.use(); // expected-note {{later used here}} } -void potential_for_loop_use_before_loop_body(MyObj safe) { +void for_loop_use_before_loop_body(MyObj safe) { MyObj* p = &safe; // Prefer the earlier use for diagnsotics. for (int i = 0; i < 1; ++i) { @@ -284,7 +282,7 @@ void potential_for_loop_use_before_loop_body(MyObj safe) { (void)*p; } -void definite_loop_with_break(bool cond) { +void loop_with_break(bool cond) { MyObj safe; MyObj* p = &safe; for (int i = 0; i < 10; ++i) { @@ -297,7 +295,7 @@ void definite_loop_with_break(bool cond) { (void)*p; // expected-note {{later used here}} } -void definite_loop_with_break_gsl(bool cond) { +void loop_with_break_gsl(bool cond) { MyObj safe; View v = safe; for (int i = 0; i < 10; ++i) { @@ -310,7 +308,7 @@ void definite_loop_with_break_gsl(bool cond) { v.use(); // expected-note {{later used here}} } -void potential_multiple_expiry_of_same_loan(bool cond) { +void multiple_expiry_of_same_loan(bool cond) { // Choose the last expiry location for the loan (e.g., through scope-ends and break statements). MyObj safe; MyObj* p = &safe; @@ -342,11 +340,6 @@ void potential_multiple_expiry_of_same_loan(bool cond) { break; // expected-note {{destroyed here}} } } - - // TODO: This can be argued to be a "maybe" warning. This is because - // we only check for confidence of liveness and not the confidence of - // the loan contained in an origin. To deal with this, we can introduce - // a confidence in loan propagation analysis as well like liveness. (void)*p; // expected-note {{later used here}} p = &safe; @@ -360,7 +353,7 @@ void potential_multiple_expiry_of_same_loan(bool cond) { (void)*p; // expected-note {{later used here}} } -void potential_switch(int mode) { +void switch_potential(int mode) { MyObj safe; MyObj* p = &safe; switch (mode) { @@ -378,7 +371,7 @@ void potential_switch(int mode) { (void)*p; // expected-note {{later used here}} } -void definite_switch(int mode) { +void switch_uaf(int mode) { MyObj safe; MyObj* p = &safe; // A use domintates all the loan expires --> all definite error. @@ -402,7 +395,7 @@ void definite_switch(int mode) { (void)*p; // expected-note 3 {{later used here}} } -void definite_switch_gsl(int mode) { +void switch_gsl(int mode) { View v; switch (mode) { case 1: { @@ -468,8 +461,7 @@ void small_scope_reference_var_no_error() { } //===----------------------------------------------------------------------===// -// 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. +// Basic Use-After-Return (Return-Stack-Address) //===----------------------------------------------------------------------===// MyObj* simple_return_stack_address() { diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index a27f746fffb60..437ef9d4aae83 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -178,21 +178,21 @@ class LifetimeTestHelper { } std::optional<LoanSet> getLoansAtPoint(OriginID OID, - llvm::StringRef Annotation) { + llvm::StringRef Annotation) const { ProgramPoint PP = Runner.getProgramPoint(Annotation); if (!PP) return std::nullopt; return Analysis.getLoanPropagation().getLoans(OID, PP); } - std::optional<std::vector<std::pair<OriginID, LivenessKind>>> - getLiveOriginsAtPoint(llvm::StringRef Annotation) { + std::optional<std::vector<OriginID>> + getLiveOriginsAtPoint(llvm::StringRef Annotation) const { ProgramPoint PP = Runner.getProgramPoint(Annotation); if (!PP) return std::nullopt; - std::vector<std::pair<OriginID, LivenessKind>> Result; + std::vector<OriginID> Result; for (auto &[OID, Info] : Analysis.getLiveOrigins().getLiveOriginsAt(PP)) - Result.push_back({OID, Info.Kind}); + Result.push_back(OID); return Result; } @@ -316,10 +316,8 @@ MATCHER_P2(HasLoansToImpl, LoanVars, Annotation, "") { ActualLoans, result_listener); } -enum class LivenessKindFilter { Maybe, Must, All }; - /// Matcher to verify the complete set of live origins at a program point. -MATCHER_P2(AreLiveAtImpl, Annotation, ConfFilter, "") { +MATCHER_P(AreLiveAtImpl, Annotation, "") { const OriginsInfo &Info = arg; auto &Helper = Info.Helper; auto ActualLiveSetOpt = Helper.getLiveOriginsAtPoint(Annotation); @@ -328,17 +326,7 @@ MATCHER_P2(AreLiveAtImpl, Annotation, ConfFilter, "") { << Annotation << "'"; return false; } - std::vector<OriginID> ActualLiveOrigins; - for (const auto [OID, ActualConfidence] : ActualLiveSetOpt.value()) { - if (ConfFilter == LivenessKindFilter::All) - ActualLiveOrigins.push_back(OID); - if (ActualConfidence == LivenessKind::Maybe && - ConfFilter == LivenessKindFilter::Maybe) - ActualLiveOrigins.push_back(OID); - if (ActualConfidence == LivenessKind::Must && - ConfFilter == LivenessKindFilter::Must) - ActualLiveOrigins.push_back(OID); - } + std::vector<OriginID> ActualLiveOrigins = ActualLiveSetOpt.value(); std::vector<OriginID> ExpectedLiveOrigins; for (const auto &VarName : Info.OriginVars) { @@ -404,20 +392,8 @@ MATCHER_P2(HasLiveLoanAtExpiryImpl, HelperPtr, Annotation, "") { return false; } -MATCHER_P(MustBeLiveAt, Annotation, "") { - return ExplainMatchResult(AreLiveAtImpl(Annotation, LivenessKindFilter::Must), - arg, result_listener); -} - -MATCHER_P(MaybeLiveAt, Annotation, "") { - return ExplainMatchResult( - AreLiveAtImpl(Annotation, LivenessKindFilter::Maybe), arg, - result_listener); -} - MATCHER_P(AreLiveAt, Annotation, "") { - return ExplainMatchResult(AreLiveAtImpl(Annotation, LivenessKindFilter::All), - arg, result_listener); + return ExplainMatchResult(AreLiveAtImpl(Annotation), arg, result_listener); } MATCHER_P(HasLoanToATemporary, Annotation, "") { @@ -1216,7 +1192,7 @@ TEST_F(LifetimeAnalysisTest, LivenessSimpleReturn) { return p; } )"); - EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p1")); + EXPECT_THAT(Origins({"p"}), AreLiveAt("p1")); } TEST_F(LifetimeAnalysisTest, LivenessKilledByReassignment) { @@ -1230,7 +1206,7 @@ TEST_F(LifetimeAnalysisTest, LivenessKilledByReassignment) { return p; } )"); - EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p2")); + EXPECT_THAT(Origins({"p"}), AreLiveAt("p2")); EXPECT_THAT(NoOrigins(), AreLiveAt("p1")); } @@ -1250,8 +1226,8 @@ TEST_F(LifetimeAnalysisTest, LivenessAcrossBranches) { return p; } )"); - EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p2")); - EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p3")); + EXPECT_THAT(Origins({"p"}), AreLiveAt("p2")); + EXPECT_THAT(Origins({"p"}), AreLiveAt("p3")); // Before the `if`, the value of `p` (`nullptr`) is always overwritten before. EXPECT_THAT(NoOrigins(), AreLiveAt("p1")); } @@ -1274,15 +1250,10 @@ TEST_F(LifetimeAnalysisTest, LivenessInLoop) { } )"); - EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p4")); - EXPECT_THAT(NoOrigins(), MaybeLiveAt("p4")); - - EXPECT_THAT(Origins({"p", "q"}), MaybeLiveAt("p3")); - - EXPECT_THAT(Origins({"q"}), MustBeLiveAt("p2")); - EXPECT_THAT(NoOrigins(), MaybeLiveAt("p2")); - - EXPECT_THAT(Origins({"p", "q"}), MaybeLiveAt("p1")); + EXPECT_THAT(Origins({"p"}), AreLiveAt("p4")); + EXPECT_THAT(Origins({"p", "q"}), AreLiveAt("p3")); + EXPECT_THAT(Origins({"q"}), AreLiveAt("p2")); + EXPECT_THAT(Origins({"p", "q"}), AreLiveAt("p1")); } TEST_F(LifetimeAnalysisTest, LivenessInLoopAndIf) { @@ -1309,9 +1280,9 @@ TEST_F(LifetimeAnalysisTest, LivenessInLoopAndIf) { } )"); EXPECT_THAT(NoOrigins(), AreLiveAt("p5")); - EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p4")); + EXPECT_THAT(Origins({"p"}), AreLiveAt("p4")); EXPECT_THAT(NoOrigins(), AreLiveAt("p3")); - EXPECT_THAT(Origins({"p"}), MaybeLiveAt("p2")); + EXPECT_THAT(Origins({"p"}), AreLiveAt("p2")); EXPECT_THAT(NoOrigins(), AreLiveAt("p1")); } @@ -1341,21 +1312,12 @@ TEST_F(LifetimeAnalysisTest, LivenessInLoopAndIf2) { } } )"); - EXPECT_THAT(Origins({"q"}), MaybeLiveAt("p6")); - EXPECT_THAT(NoOrigins(), MustBeLiveAt("p6")); - - EXPECT_THAT(Origins({"p", "q"}), MustBeLiveAt("p5")); - - EXPECT_THAT(Origins({"p", "q"}), MustBeLiveAt("p4")); - - EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p3")); - EXPECT_THAT(Origins({"q"}), MaybeLiveAt("p3")); - - EXPECT_THAT(Origins({"q"}), MaybeLiveAt("p2")); - EXPECT_THAT(NoOrigins(), MustBeLiveAt("p2")); - - EXPECT_THAT(Origins({"q"}), MaybeLiveAt("p1")); - EXPECT_THAT(NoOrigins(), MustBeLiveAt("p1")); + EXPECT_THAT(Origins({"q"}), AreLiveAt("p6")); + EXPECT_THAT(Origins({"p", "q"}), AreLiveAt("p5")); + EXPECT_THAT(Origins({"p", "q"}), AreLiveAt("p4")); + EXPECT_THAT(Origins({"p", "q"}), AreLiveAt("p3")); + EXPECT_THAT(Origins({"q"}), AreLiveAt("p2")); + EXPECT_THAT(Origins({"q"}), AreLiveAt("p1")); } TEST_F(LifetimeAnalysisTest, LivenessOutsideLoop) { @@ -1371,8 +1333,8 @@ TEST_F(LifetimeAnalysisTest, LivenessOutsideLoop) { (void)*p; } )"); - EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p2")); - EXPECT_THAT(Origins({"p"}), MaybeLiveAt("p1")); + EXPECT_THAT(Origins({"p"}), AreLiveAt("p2")); + EXPECT_THAT(Origins({"p"}), AreLiveAt("p1")); } TEST_F(LifetimeAnalysisTest, TrivialDestructorsUAF) { @@ -1388,7 +1350,7 @@ TEST_F(LifetimeAnalysisTest, TrivialDestructorsUAF) { } )"); EXPECT_THAT(Origin("ptr"), HasLoansTo({"s"}, "p1")); - EXPECT_THAT(Origins({"ptr"}), MustBeLiveAt("p1")); + EXPECT_THAT(Origins({"ptr"}), AreLiveAt("p1")); } TEST_F(LifetimeAnalysisTest, TrivialClassDestructorsUAF) { @@ -1408,7 +1370,7 @@ TEST_F(LifetimeAnalysisTest, TrivialClassDestructorsUAF) { } )"); EXPECT_THAT(Origin("ptr"), HasLoansTo({"s"}, "p1")); - EXPECT_THAT(Origins({"ptr"}), MustBeLiveAt("p1")); + EXPECT_THAT(Origins({"ptr"}), AreLiveAt("p1")); } TEST_F(LifetimeAnalysisTest, SimpleReturnStackAddress) { @@ -1577,10 +1539,10 @@ TEST_F(LifetimeAnalysisTest, UseAfterScopeThenReturn) { } )"); EXPECT_THAT(Origin("p"), HasLoansTo({"local_obj"}, "p2")); - EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p2")); + EXPECT_THAT(Origins({"p"}), AreLiveAt("p2")); EXPECT_THAT(Origin("p"), HasLoansTo({"local_obj"}, "p1")); - EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p1")); + EXPECT_THAT(Origins({"p"}), AreLiveAt("p1")); EXPECT_THAT("local_obj", HasLiveLoanAtExpiry("p2")); } @@ -1607,7 +1569,7 @@ TEST_F(LifetimeAnalysisTest, ReturnBeforeUseAfterScope) { EXPECT_THAT(NoOrigins(), AreLiveAt("p2")); EXPECT_THAT(Origin("p"), HasLoansTo({"local_obj"}, "p1")); - EXPECT_THAT(Origins({"p"}), MustBeLiveAt("p1")); + EXPECT_THAT(Origins({"p"}), AreLiveAt("p1")); } TEST_F(LifetimeAnalysisTest, TrivialDestructorsUAR) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
