https://github.com/NeKon69 created https://github.com/llvm/llvm-project/pull/198510
Now, `handleAssignment` also propagates loans into the assignment expression (`BinaryOperator`/`CXXOperatorCallExpr`) so chained assignments can flow loans forward. This PR also fixes assigning from `gsl::Owner` to `gsl::Pointer`, where rvalue-origin handling stripped the owner storage origin and left no origin to flow from (checked in test `chained_assignment_gsl`) Closes #198497 >From 75314d7f7a0a323803e229ed702152ba46941b9f Mon Sep 17 00:00:00 2001 From: NeKon69 <[email protected]> Date: Tue, 19 May 2026 15:27:00 +0300 Subject: [PATCH] implement fix and add tests --- .../Analyses/LifetimeSafety/FactsGenerator.h | 3 +- .../LifetimeSafety/FactsGenerator.cpp | 16 ++++-- clang/test/Sema/warn-lifetime-safety.cpp | 57 +++++++++++++++++++ 3 files changed, 69 insertions(+), 7 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index f3ebd4dfa0195..6ddc05d54ec47 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -65,7 +65,8 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { void flow(OriginList *Dst, OriginList *Src, bool Kill); - void handleAssignment(const Expr *LHSExpr, const Expr *RHSExpr); + void handleAssignment(const Expr *TargetExpr, const Expr *LHSExpr, + const Expr *RHSExpr); void handlePointerArithmetic(const BinaryOperator *BO); diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 0a06548d881d1..deb0f2e1e0e85 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -373,7 +373,8 @@ void FactsGenerator::VisitReturnStmt(const ReturnStmt *RS) { } } -void FactsGenerator::handleAssignment(const Expr *LHSExpr, +void FactsGenerator::handleAssignment(const Expr *TargetExpr, + const Expr *LHSExpr, const Expr *RHSExpr) { LHSExpr = LHSExpr->IgnoreParenImpCasts(); OriginList *LHSList = nullptr; @@ -397,7 +398,9 @@ void FactsGenerator::handleAssignment(const Expr *LHSExpr, // unlike built-in assignment where LValueToRValue cast strips the outer // lvalue origin. Strip it manually to get the actual value origins being // assigned. - RHSList = getRValueOrigins(RHSExpr, RHSList); + if (!(isGslPointerType(LHSExpr->getType()) && + isGslOwnerType(RHSExpr->getType()))) + RHSList = getRValueOrigins(RHSExpr, RHSList); if (const auto *DRE_LHS = dyn_cast<DeclRefExpr>(LHSExpr)) { QualType QT = DRE_LHS->getDecl()->getType(); @@ -434,6 +437,7 @@ void FactsGenerator::handleAssignment(const Expr *LHSExpr, // Kill the old loans of the destination origin and flow the new loans // from the source origin. flow(LHSList->peelOuterOrigin(), RHSList, /*Kill=*/true); + killAndFlowOrigin(*TargetExpr, *LHSExpr); } void FactsGenerator::handlePointerArithmetic(const BinaryOperator *BO) { @@ -454,7 +458,7 @@ void FactsGenerator::VisitBinaryOperator(const BinaryOperator *BO) { handlePointerArithmetic(BO); handleUse(BO->getRHS()); if (BO->isAssignmentOp()) - handleAssignment(BO->getLHS(), BO->getRHS()); + handleAssignment(BO, BO->getLHS(), BO->getRHS()); // TODO: Handle assignments involving dereference like `*p = q`. } @@ -522,14 +526,14 @@ void FactsGenerator::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) { QualType LHSTy = OCE->getArg(0)->getType(); if (LHSTy->isPointerOrReferenceType() || isGslPointerType(LHSTy) || isGslOwnerType(LHSTy)) { - handleAssignment(OCE->getArg(0), OCE->getArg(1)); + handleAssignment(OCE, OCE->getArg(0), OCE->getArg(1)); return; } // Standard library callable wrappers (e.g., std::function) can propagate // the stored lambda's origins. if (const auto *RD = LHSTy->getAsCXXRecordDecl(); RD && isStdCallableWrapperType(RD)) { - handleAssignment(OCE->getArg(0), OCE->getArg(1)); + handleAssignment(OCE, OCE->getArg(0), OCE->getArg(1)); return; } // Other tracked types: only defaulted operator= propagates origins. @@ -537,7 +541,7 @@ void FactsGenerator::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) { if (const auto *MD = dyn_cast_or_null<CXXMethodDecl>(OCE->getDirectCallee()); MD && MD->isDefaulted()) { - handleAssignment(OCE->getArg(0), OCE->getArg(1)); + handleAssignment(OCE, OCE->getArg(0), OCE->getArg(1)); return; } } diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 30b450c333fbd..332630c56496d 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -107,6 +107,15 @@ void propagation_gsl() { v2.use(); // expected-note {{later used here}} } +void chained_assignment_gsl() { + View v1, v2; + { + MyObj s; + v2 = v1 = s; // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + v2.use(); // expected-note {{later used here}} +} + void multiple_uses_one_warning() { MyObj* p; { @@ -133,6 +142,26 @@ void multiple_pointers() { (void)*r; // expected-note {{later used here}} } +void multiple_pointers_chained() { + MyObj *p; + { + MyObj s; + MyObj* obj1, *obj2; + p = obj1 = obj2 = &s; // expected-warning {{does not live long enough}} + } // expected-note {{destroyed here}} + (void)*p; // expected-note {{later used here}} +} + +void multiple_pointers_chained_safe() { + MyObj *p; + MyObj s; + { + MyObj* obj1, *obj2; + p = obj1 = obj2 = &s; + } + (void)*p; +} + void single_pointer_multiple_loans(bool cond) { MyObj *p; if (cond){ @@ -889,6 +918,15 @@ void lifetimebound_with_pointers() { (void)*ptr; // expected-note {{later used here}} } +void chained_assignment_lifetimebound_call() { + MyObj *p, *obj; + { + MyObj s; + p = Identity(obj = &s); // expected-warning {{does not live long enough}} + } // expected-note {{destroyed here}} + (void)*p; // expected-note {{later used here}} +} + void lifetimebound_no_error_safe_usage() { MyObj obj; View v1 = Identity(obj); // No warning - obj lives long enough @@ -2363,6 +2401,16 @@ void assignment_propagation() { use(b); // expected-note {{later used here}} } +void chained_defaulted_assignment_propagation() { + S b, c; + { + std::string str{"abc"}; + S a = getS(str); // expected-warning {{object whose reference is captured does not live long enough}} + c = b = a; + } // expected-note {{destroyed here}} + use(c); // expected-note {{later used here}} +} + S getSNoAnnotation(const std::string &s); void no_annotation() { @@ -3201,6 +3249,15 @@ std::function<void()> copy_assign() { return f2; // expected-note {{returned here}} } +std::function<void()> chained_copy_assign() { + int x; + std::function<void()> f = [&x]() { (void)x; }; // expected-warning {{address of stack memory is returned later}} + std::function<void()> f2 = []() {}; + std::function<void()> f3 = []() {}; + f3 = f2 = f; + return f3; // expected-note {{returned here}} +} + // FIXME: False negative. std::move's lifetimebound handling in // `handleFunctionCall` only flows the outermost origin, missing inner origins // that carry the lambda's loans. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
