[clang] [LifetimeSafety] Propagate loans through the comma operator (PR #204379)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-temporal-safety

Author: Gábor Horváth (Xazax-hun)

<details>
<summary>Changes</summary>

VisitBinaryOperator had no comma case, so a comma expression carried none of its right operand's loans and a borrow used via a comma result (e.g. `g = (f(), p)`) was silently dropped. Flow the RHS's origin into the result.

Assisted-by: Claude Opus 4.8

---
Full diff: https://github.com/llvm/llvm-project/pull/204379.diff


3 Files Affected:

- (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+4) 
- (modified) clang/test/Sema/LifetimeSafety/dangling-global.cpp (+7-1) 
- (modified) clang/test/Sema/LifetimeSafety/safety.cpp (+37) 


``````````diff
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index be0577b0f3f8f..10339a9f2e3c5 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -450,6 +450,10 @@ void FactsGenerator::handlePointerArithmetic(const BinaryOperator *BO) {
 }
 
 void FactsGenerator::VisitBinaryOperator(const BinaryOperator *BO) {
+  if (BO->getOpcode() == BO_Comma) {
+    killAndFlowOrigin(*BO, *BO->getRHS());
+    return;
+  }
   if (BO->isCompoundAssignmentOp())
     return;
   if (BO->getType()->isPointerType() && BO->isAdditiveOp())
diff --git a/clang/test/Sema/LifetimeSafety/dangling-global.cpp b/clang/test/Sema/LifetimeSafety/dangling-global.cpp
index 98f8905d0e2da..f419ff4416023 100644
--- a/clang/test/Sema/LifetimeSafety/dangling-global.cpp
+++ b/clang/test/Sema/LifetimeSafety/dangling-global.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -Wlifetime-safety -Wno-dangling -verify %s
 
-int *global; // expected-note {{this global dangles}}
+int *global; // expected-note 2 {{this global dangles}}
 int *global_backup; // expected-note {{this global dangles}}
 
 struct ObjWithStaticField {
@@ -32,6 +32,12 @@ void store_local_in_global() {
   global = &local; // expected-warning {{stack memory associated with local variable 'local' escapes to the global variable 'global' which will dangle}}
 }
 
+int side();
+void store_local_in_global_via_comma() {
+  int local;
+  global = (side(), &local); // expected-warning {{stack memory associated with local variable 'local' escapes to the global variable 'global' which will dangle}}
+}
+
 void store_then_clear() {
   int local;
   global = &local;
diff --git a/clang/test/Sema/LifetimeSafety/safety.cpp b/clang/test/Sema/LifetimeSafety/safety.cpp
index 3c954cebda820..6fc275b51a9d0 100644
--- a/clang/test/Sema/LifetimeSafety/safety.cpp
+++ b/clang/test/Sema/LifetimeSafety/safety.cpp
@@ -1250,6 +1250,43 @@ void conditional_operator_lifetimebound_nested_deep(bool cond) {
   (void)*p;  // expected-note 4 {{later used here}}
 }
 
+// Comma operator.
+int side();
+void comma_use_after_scope() {
+  MyObj* p;
+  {
+    MyObj temp;
+    p = (side(), &temp);  // expected-warning {{local variable 'temp' does not live long enough}}
+  }                       // expected-note {{destroyed here}}
+  (void)*p;               // expected-note {{later used here}}
+}
+
+void comma_nested() {
+  MyObj* p;
+  {
+    MyObj temp;
+    p = (side(), (side(), &temp));  // expected-warning {{local variable 'temp' does not live long enough}}
+  }                                 // expected-note {{destroyed here}}
+  (void)*p;                         // expected-note {{later used here}}
+}
+
+void comma_masked_by_conditional(bool cond) {
+  MyObj safe;
+  MyObj* keep = &safe;
+  MyObj* p;
+  {
+    MyObj temp;
+    p = cond ? keep : (side(), &temp);  // expected-warning {{local variable 'temp' does not live long enough}}
+  }                                     // expected-note {{destroyed here}}
+  (void)*p;                             // expected-note {{later used here}}
+}
+
+void comma_safe() {
+  MyObj safe;
+  MyObj* p = (side(), &safe);
+  (void)*p;  // no-warning
+}
+
 // 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/204379