https://github.com/Xazax-hun updated https://github.com/llvm/llvm-project/pull/204439
From 9d63534f256a25d7a63fc35c4f7080585badb1dd Mon Sep 17 00:00:00 2001 From: Gabor Horvath <[email protected]> Date: Wed, 17 Jun 2026 21:52:00 +0100 Subject: [PATCH] [LifetimeSafety] Propagate loans through the GNU binary conditional FactsGenerator only handled the ternary, so a borrow used through the GNU binary conditional `a ?: b` was silently dropped. Handle both via VisitAbstractConditionalOperator, flowing from getTrueExpr()/getFalseExpr(). For `a ?: b` getTrueExpr() is an OpaqueValueExpr, so make OpaqueValueExpr transparent in the origin manager and peel it in the arm-reachability check. Assisted-by: Claude Opus 4.8 --- .../Analyses/LifetimeSafety/FactsGenerator.h | 2 +- .../LifetimeSafety/FactsGenerator.cpp | 23 +++-- clang/lib/Analysis/LifetimeSafety/Origins.cpp | 7 ++ clang/test/Sema/LifetimeSafety/safety.cpp | 98 +++++++++++++++++++ 4 files changed, 121 insertions(+), 9 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index a021c09dcd34b..5ac67263681ac 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -47,7 +47,7 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { void VisitUnaryOperator(const UnaryOperator *UO); void VisitReturnStmt(const ReturnStmt *RS); void VisitBinaryOperator(const BinaryOperator *BO); - void VisitConditionalOperator(const ConditionalOperator *CO); + void VisitAbstractConditionalOperator(const AbstractConditionalOperator *CO); void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE); void VisitCXXFunctionalCastExpr(const CXXFunctionalCastExpr *FCE); void VisitInitListExpr(const InitListExpr *ILE); diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index d56703a4b29c4..545836cd76fb9 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -482,10 +482,13 @@ void FactsGenerator::VisitBinaryOperator(const BinaryOperator *BO) { // TODO: Handle assignments involving dereference like `*p = q`. } -void FactsGenerator::VisitConditionalOperator(const ConditionalOperator *CO) { +void FactsGenerator::VisitAbstractConditionalOperator( + const AbstractConditionalOperator *CO) { if (!hasOrigins(CO)) return; + // For the GNU binary conditional `a ?: b`, getTrueExpr() is the + // OpaqueValueExpr wrapping the common subexpression. const Expr *TrueExpr = CO->getTrueExpr(); const Expr *FalseExpr = CO->getFalseExpr(); @@ -500,13 +503,17 @@ void FactsGenerator::VisitConditionalOperator(const ConditionalOperator *CO) { case 0: return; case 1: { - TBHasEdge = llvm::any_of(**Preds.begin(), - [ExpectedStmt = TrueExpr->IgnoreParenImpCasts()]( - const CFGElement &Elt) { - if (auto CS = Elt.getAs<CFGStmt>()) - return CS->getStmt() == ExpectedStmt; - return false; - }); + // For `a ?: b`, getTrueExpr() is the OpaqueValueExpr; the common + // subexpression it wraps is what appears in the predecessor block. + const Expr *TrueArm = TrueExpr->IgnoreParenImpCasts(); + if (const auto *OVE = dyn_cast<OpaqueValueExpr>(TrueArm)) + if (const Expr *Src = OVE->getSourceExpr()) + TrueArm = Src->IgnoreParenImpCasts(); + TBHasEdge = llvm::any_of(**Preds.begin(), [TrueArm](const CFGElement &Elt) { + if (auto CS = Elt.getAs<CFGStmt>()) + return CS->getStmt() == TrueArm; + return false; + }); FBHasEdge = !TBHasEdge; break; } diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp index 55d3b36e3163a..3ff4823ca88a6 100644 --- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp @@ -222,6 +222,13 @@ OriginList *OriginManager::getOrCreateList(const Expr *E) { if (const ExprWithCleanups *EWC = dyn_cast<ExprWithCleanups>(E)) return getOrCreateList(EWC->getSubExpr()); + // An OpaqueValueExpr is a placeholder for an already-evaluated subexpression + // (e.g. the common operand of `a ?: b`) and is not itself a CFG statement, so + // reuse its source's origins rather than flowing into a fresh node. + if (const auto *OVE = dyn_cast<OpaqueValueExpr>(E)) + if (const Expr *Src = OVE->getSourceExpr()) + return getOrCreateList(Src); + if (!hasOrigins(E)) return nullptr; diff --git a/clang/test/Sema/LifetimeSafety/safety.cpp b/clang/test/Sema/LifetimeSafety/safety.cpp index 6fc275b51a9d0..7a2644e46a6e1 100644 --- a/clang/test/Sema/LifetimeSafety/safety.cpp +++ b/clang/test/Sema/LifetimeSafety/safety.cpp @@ -1287,6 +1287,88 @@ void comma_safe() { (void)*p; // no-warning } +// GNU binary conditional operator `a ?: b`. +void binary_conditional_false_unsafe(MyObj* in) { + MyObj* p; + { + MyObj temp; + p = in ?: &temp; // expected-warning {{local variable 'temp' does not live long enough}} + } // expected-note {{destroyed here}} + (void)*p; // expected-note {{later used here}} +} + +void binary_conditional_common_unsafe(MyObj* fallback) { + MyObj* p; + { + MyObj temp; + MyObj* t = &temp; // expected-warning {{local variable 'temp' does not live long enough}} + p = t ?: fallback; + } // expected-note {{destroyed here}} + (void)*p; // expected-note {{later used here}} +} + +void binary_conditional_safe(MyObj* in) { + MyObj fallback; + MyObj* p = in ?: &fallback; + (void)*p; // no-warning +} + +void binary_conditional_nested(MyObj* a, MyObj* b) { + MyObj* p; + { + MyObj temp; + p = a ?: b ?: &temp; // expected-warning {{local variable 'temp' does not live long enough}} + } // expected-note {{destroyed here}} + (void)*p; // expected-note {{later used here}} +} + +void binary_conditional_masked_by_conditional(bool cond, MyObj* in) { + MyObj safe; + MyObj* keep = &safe; + MyObj* p; + { + MyObj temp; + p = cond ? keep : (in ?: &temp); // expected-warning {{local variable 'temp' does not live long enough}} + } // expected-note {{destroyed here}} + (void)*p; // expected-note {{later used here}} +} + +void binary_conditional_use_after_free(int* in) { + int* h = new int; // expected-warning {{allocated object does not live long enough}} + int* p = in ?: h; + delete h; // expected-note {{freed here}} + (void)*p; // expected-note {{later used here}} +} + +int** binary_conditional_double_ptr(int** in) { + int* local = nullptr; + int** p = in ?: &local; // expected-warning {{stack memory associated with local variable 'local' is returned}} + return p; // expected-note {{returned here}} +} + +// A constexpr operator bool folds the condition and prunes an arm; the surviving +// value must still flow without tripping the origin shape invariant. +struct [[gsl::Pointer]] TrueView { + const int *p; + constexpr TrueView(const int &x [[clang::lifetimebound]]) : p(&x) {} + constexpr explicit operator bool() const { return true; } +}; +struct [[gsl::Pointer]] FalseView { + const int *p; + constexpr FalseView(const int &x [[clang::lifetimebound]]) : p(&x) {} + constexpr explicit operator bool() const { return false; } +}; + +TrueView binary_conditional_folded_true(TrueView fb) { + int local = 0; + return TrueView(local) ?: fb; // expected-warning {{stack memory associated with local variable 'local' is returned}} expected-note {{returned here}} +} + +FalseView binary_conditional_folded_false(FalseView fb) { + int local = 0; + return FalseView(local) ?: fb; // no-warning (result is always fb) +} + // FIXME: Diagnostic output does not handle ParenExpr correctly, causing alias // information to be missed (local variable 'p' aliases the storage of local variable 'b'). void simpleparen() { @@ -2788,6 +2870,22 @@ void nested_throw_branches(bool cond, bool cond2, int *value) { (void)(cond ? throw 1 : (cond2 ? value : throw 2)); } +// A `throw` arm of a binary conditional `a ?: b` carries no origins; flowing it +// must not crash, and the common value's loans must still flow. +int *binary_conditional_throw_select(int *in) { + return in ?: throw 1; // no-warning (in is caller storage) +} + +void binary_conditional_throw_void(int *in) { + in ?: throw 1; // no-warning +} + +int *binary_conditional_throw_dangling() { + int x; + int *p = &x; // expected-warning {{stack memory associated with local variable 'x' is returned}} + return p ?: throw 1; // expected-note {{returned here}} +} + #endif int *f(int *p [[clang::lifetimebound]]); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
