[clang] [LifetimeSafety] Propagate loans through the GNU binary conditional (PR #204439)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 17 13:53:36 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Gábor Horváth (Xazax-hun)
<details>
<summary>Changes</summary>
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; guard against flowing a void (e.g. throw) arm.
Assisted-by: Claude Opus 4.8
---
Full diff: https://github.com/llvm/llvm-project/pull/204439.diff
5 Files Affected:
- (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h (+1-1)
- (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+18-8)
- (modified) clang/lib/Analysis/LifetimeSafety/Origins.cpp (+6)
- (added) clang/test/Sema/LifetimeSafety/binary-conditional-throw.cpp (+19)
- (modified) clang/test/Sema/LifetimeSafety/safety.cpp (+82)
``````````diff
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
index 2c961bd305fac..fed6531aa9097 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
@@ -45,7 +45,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 be0577b0f3f8f..8fa80c729d111 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -460,10 +460,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();
@@ -478,13 +481,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;
}
@@ -501,6 +508,9 @@ void FactsGenerator::VisitConditionalOperator(const ConditionalOperator *CO) {
bool FirstFlow = true;
auto HandleFlow = [&](const Expr *E) {
+ // An arm with no origins (e.g. a `throw`/void arm) carries no loans.
+ if (!getOriginsList(*E))
+ return;
if (FirstFlow) {
killAndFlowOrigin(*CO, *E);
FirstFlow = false;
diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp
index 55d3b36e3163a..460a04b3b1494 100644
--- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp
@@ -222,6 +222,12 @@ 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`); reuse its source's origins.
+ 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/binary-conditional-throw.cpp b/clang/test/Sema/LifetimeSafety/binary-conditional-throw.cpp
new file mode 100644
index 0000000000000..4b15f13b3b0e3
--- /dev/null
+++ b/clang/test/Sema/LifetimeSafety/binary-conditional-throw.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -Wlifetime-safety -Wno-dangling -fcxx-exceptions -verify %s
+
+// A `throw` arm of a GNU binary conditional `a ?: b` collapses the CFG to a
+// single predecessor and carries no origins; flowing it must not crash, and the
+// common value's loans must still flow.
+
+int *select_throw(int *in) {
+ return in ?: throw 1; // no-warning (in is caller storage)
+}
+
+void void_context(int *in) {
+ in ?: throw 1; // no-warning
+}
+
+int *dangling_common() {
+ int x;
+ int *p = &x; // expected-warning {{stack memory associated with local variable 'x' is returned}}
+ return p ?: throw 1; // expected-note {{returned here}}
+}
diff --git a/clang/test/Sema/LifetimeSafety/safety.cpp b/clang/test/Sema/LifetimeSafety/safety.cpp
index 3c954cebda820..619cae086a25f 100644
--- a/clang/test/Sema/LifetimeSafety/safety.cpp
+++ b/clang/test/Sema/LifetimeSafety/safety.cpp
@@ -1250,6 +1250,88 @@ void conditional_operator_lifetimebound_nested_deep(bool cond) {
(void)*p; // expected-note 4 {{later used here}}
}
+// 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() {
``````````
</details>
https://github.com/llvm/llvm-project/pull/204439
More information about the cfe-commits
mailing list