https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/182709
>From 615ac03ef2f0678fbea9e3ff04f0f615f0387611 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <[email protected]> Date: Fri, 13 Mar 2026 16:51:31 +0000 Subject: [PATCH] [LifetimeSafety] Remove confidence tracking --- .../Analyses/LifetimeSafety/LifetimeSafety.h | 18 +---- clang/lib/Analysis/LifetimeSafety/Checker.cpp | 80 ++++++++++--------- clang/lib/Sema/AnalysisBasedWarnings.cpp | 8 +- clang/test/Sema/warn-lifetime-safety.cpp | 75 ++++++++--------- 4 files changed, 85 insertions(+), 96 deletions(-) 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 -// 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/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
