Author: Utkarsh Saxena Date: 2026-03-13T17:18:42Z New Revision: 872ec9f8943b8d4e3cc66ed09f60f7137c28f85e
URL: https://github.com/llvm/llvm-project/commit/872ec9f8943b8d4e3cc66ed09f60f7137c28f85e DIFF: https://github.com/llvm/llvm-project/commit/872ec9f8943b8d4e3cc66ed09f60f7137c28f85e.diff LOG: [LifetimeSafety] Remove confidence tracking (#182709) Removed the confidence level tracking system from the lifetime safety analysis, simplifying the implementation by eliminating the distinction between "definite" and "maybe" lifetime violations. The confidence level system was marked as deprecated in the TODO comment and was no longer being used to differentiate between strict and permissive warnings. Removing this complexity simplifies the codebase while maintaining the core lifetime safety analysis functionality. The change eliminates the distinction between "must-be-live" and "maybe-be-live" states, treating all detected lifetime violations uniformly. Added: Modified: clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h clang/lib/Analysis/LifetimeSafety/Checker.cpp clang/lib/Sema/AnalysisBasedWarnings.cpp clang/test/Sema/warn-lifetime-safety.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index 2bcd0ddd7a16c..08038dd096685 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 -// diff erentiator 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/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 7399fa1c2dbd2..db87f511a230f 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -24,21 +24,18 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "llvm/ADT/DenseMap.h" -#include "llvm/ADT/DenseSet.h" -#include "llvm/Support/Casting.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/TimeProfiler.h" namespace clang::lifetimes::internal { -static Confidence livenessKindToConfidence(LivenessKind K) { +static bool causingFactDominatesExpiry(LivenessKind K) { switch (K) { case LivenessKind::Must: - return Confidence::Definite; + return true; case LivenessKind::Maybe: - return Confidence::Maybe; case LivenessKind::Dead: - return Confidence::None; + return false; } llvm_unreachable("unknown liveness kind"); } @@ -51,7 +48,7 @@ struct PendingWarning { llvm::PointerUnion<const UseFact *, const OriginEscapesFact *> CausingFact; const Expr *MovedExpr; const Expr *InvalidatedByExpr; - Confidence ConfidenceLevel; + bool CausingFactDominatesExpiry; }; using AnnotationTarget = @@ -71,6 +68,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, @@ -143,9 +153,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 @@ -157,34 +165,34 @@ class LifetimeChecker { MovedExpr = *ME; LivenessMap Origins = LiveOrigins.getLiveOriginsAt(EF); - Confidence CurConfidence = Confidence::None; + bool CurDomination = false; // 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 (!CurDomination || causingFactDominatesExpiry(LiveInfo.Kind)) + CausingFact = LiveInfo.CausingFact; } - if (!BestCausingFact) + if (!CausingFact) return; - // We have a use-after-free. - Confidence LastConf = FinalWarningsMap.lookup(ExpiredLoan).ConfidenceLevel; - if (LastConf >= CurConfidence) + + bool LastDomination = + FinalWarningsMap.lookup(ExpiredLoan).CausingFactDominatesExpiry; + if (LastDomination) return; - FinalWarningsMap[ExpiredLoan] = {/*ExpiryLoc=*/EF->getExpiryLoc(), - /*BestCausingFact=*/BestCausingFact, - /*MovedExpr=*/MovedExpr, - /*InvalidatedByExpr=*/nullptr, - /*ConfidenceLevel=*/CurConfidence}; + FinalWarningsMap[ExpiredLoan] = { + /*ExpiryLoc=*/EF->getExpiryLoc(), + /*CausingFact=*/CausingFact, + /*MovedExpr=*/MovedExpr, + /*InvalidatedByExpr=*/nullptr, + /*CausingFactDominatesExpiry=*/CurDomination}; } /// Checks for use-after-invalidation errors when a container is modified. @@ -220,16 +228,16 @@ 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) { + bool CurDomination = causingFactDominatesExpiry(LiveInfo.Kind); + bool LastDomination = + FinalWarningsMap.lookup(LiveLoanID).CausingFactDominatesExpiry; + if (!LastDomination) { FinalWarningsMap[LiveLoanID] = { /*ExpiryLoc=*/{}, /*CausingFact=*/LiveInfo.CausingFact, /*MovedExpr=*/nullptr, /*InvalidatedByExpr=*/IOF->getInvalidationExpr(), - /*ConfidenceLevel=*/CurConfidence}; + /*CausingFactDominatesExpiry=*/CurDomination}; } } } @@ -249,7 +257,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; @@ -264,13 +271,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/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 7ed9b43b76d9d..2824bf61526b7 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2880,8 +2880,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) @@ -2895,8 +2895,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 7034c8686b315..da901a7ff4bb3 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,13 @@ void definite_overrides_potential(bool cond) { q = &safe; } - // The use of 'p' is a definite error because it was never rescued. + // The use of 'p' dominates expiry of 's' error because it was never rescued. (void)*q; (void)*p; // expected-note {{later used here}} (void)*q; } -void potential_due_to_conditional_killing(bool cond) { +void due_to_conditional_killing(bool cond) { MyObj safe; MyObj* q; { @@ -244,7 +243,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 +262,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 +272,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 +283,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 +296,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 +309,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; @@ -329,9 +328,9 @@ void potential_multiple_expiry_of_same_loan(bool cond) { if (cond) { p = &unsafe; // expected-warning {{does not live long enough}} if (cond) - break; // expected-note {{destroyed here}} + break; } - } + } // expected-note {{destroyed here}} (void)*p; // expected-note {{later used here}} p = &safe; @@ -342,11 +341,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; @@ -355,12 +349,12 @@ void potential_multiple_expiry_of_same_loan(bool cond) { if (cond) p = &unsafe; // expected-warning {{does not live long enough}} if (cond) - break; // expected-note {{destroyed here}} - } + break; + } // expected-note {{destroyed here}} (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 +372,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 +396,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 +462,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() { @@ -716,12 +709,12 @@ View uar_before_uaf(const MyObj& safe, bool c) { View p; { MyObj local_obj; - p = local_obj; // expected-warning {{address of stack memory is returned later}} + p = local_obj; // expected-warning {{object whose reference is captured does not live long enough}} if (c) { - return p; // expected-note {{returned here}} + return p; } - } - p.use(); + } // expected-note {{destroyed here}} + p.use(); // expected-note {{later used here}} p = safe; return p; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
