[LifetimeSafety] Add support for conditional operators

[usx95]

Added support for conditional operators in the lifetime safety analysis.

Added a VisitConditionalOperator method to the FactsGenerator class to handle the ternary operator (?:) in lifetime safety analysis.

Fixes #157108

[usx95]

• [LifetimeSafety] Ignore parentheses when tracking expressions #167245
• [LifetimeSafety] Add support for conditional operators #167240 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

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

@llvm/pr-subscribers-clang-analysis

Author: Utkarsh Saxena (usx95)

Changes

Added support for conditional operators in the lifetime safety analysis.

Added a VisitConditionalOperator method to the FactsGenerator class to handle the ternary operator (?:) in lifetime safety analysis.

Fixes #157108

---

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

4 Files Affected:

• (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h (+1)
• (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+7)
• (modified) clang/test/Sema/warn-lifetime-safety-dataflow.cpp (+19)
• (modified) clang/test/Sema/warn-lifetime-safety.cpp (+53)

diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
index 5e58abee2bbb3..4c8ab3f859a49 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
@@ -43,6 +43,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 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 bec8e1dabb0b5..fbb993f5ed6b2 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -176,6 +176,13 @@ void FactsGenerator::VisitBinaryOperator(const BinaryOperator *BO) {
     handleAssignment(BO->getLHS(), BO->getRHS());
 }
 
+void FactsGenerator::VisitConditionalOperator(const ConditionalOperator *CO) {
+  if (hasOrigin(CO)) {
+    killAndFlowOrigin(*CO, *CO->getTrueExpr());
+    flowOrigin(*CO, *CO->getFalseExpr());
+  }
+}
+
 void FactsGenerator::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) {
   // Assignment operators have special "kill-then-propagate" semantics
   // and are handled separately.
diff --git a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
index 31148b990d6bd..1568289d148db 100644
--- a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
+++ b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
@@ -414,3 +414,22 @@ void test_use_lifetimebound_call() {
 // CHECK:   Expire ([[L_Y]] (Path: y))
 // CHECK:   Expire ([[L_X]] (Path: x))
 }
+// CHECK-LABEL: Function: test_conditional_operator
+void test_conditional_operator(bool cond) {
+  MyObj x, y;
+  MyObj *p = cond ? &x : &y;
+// CHECK: Block B{{[0-9]+}}:
+// CHECK:   Issue ([[L_COND:[0-9]+]] (Path: cond), ToOrigin: {{[0-9]+}} (Expr: DeclRefExpr))
+// CHECK: Block B{{[0-9]+}}:
+// CHECK:   Issue ([[L_X:[0-9]+]] (Path: x), ToOrigin: [[O_DRE_X:[0-9]+]] (Expr: DeclRefExpr))
+// CHECK:   OriginFlow (Dest: [[O_ADDR_X:[0-9]+]] (Expr: UnaryOperator), Src: [[O_DRE_X]] (Expr: DeclRefExpr))
+// CHECK: Block B{{[0-9]+}}:
+// CHECK:   Issue ([[L_Y:[0-9]+]] (Path: y), ToOrigin: [[O_DRE_Y:[0-9]+]] (Expr: DeclRefExpr))
+// CHECK:   OriginFlow (Dest: [[O_ADDR_Y:[0-9]+]] (Expr: UnaryOperator), Src: [[O_DRE_Y]] (Expr: DeclRefExpr))
+// CHECK: Block B{{[0-9]+}}:
+// CHECK:   OriginFlow (Dest: [[O_COND_OP:[0-9]+]] (Expr: ConditionalOperator), Src: [[O_ADDR_X]] (Expr: UnaryOperator))
+// CHECK:   OriginFlow (Dest: [[O_COND_OP]] (Expr: ConditionalOperator), Src: [[O_ADDR_Y]] (Expr: UnaryOperator), Merge)
+// CHECK:   OriginFlow (Dest: [[O_P:[0-9]+]] (Decl: p), Src: [[O_COND_OP]] (Expr: ConditionalOperator))
+// CHECK:   Expire ([[L_Y]] (Path: y))
+// CHECK:   Expire ([[L_X]] (Path: x))
+}
diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp
index 4f234f0ac6e2d..3057ac9385736 100644
--- a/clang/test/Sema/warn-lifetime-safety.cpp
+++ b/clang/test/Sema/warn-lifetime-safety.cpp
@@ -440,6 +440,7 @@ void no_error_loan_from_current_iteration(bool cond) {
 //===----------------------------------------------------------------------===//
 
 View Identity(View v [[clang::lifetimebound]]);
+MyObj* Identity(MyObj* v [[clang::lifetimebound]]);
 View Choose(bool cond, View a [[clang::lifetimebound]], View b [[clang::lifetimebound]]);
 MyObj* GetPointer(const MyObj& obj [[clang::lifetimebound]]);
 
@@ -582,3 +583,55 @@ void lifetimebound_ctor() {
   }
   (void)v;
 }
+
+// Conditional operator.
+void conditional_operator(bool cond) {
+  MyObj safe;
+  MyObj* p = &safe;
+  {
+    MyObj temp;
+    p = cond ? &temp  // expected-warning {{object whose reference is captured may not live long enough}}
+             : &safe;
+  }  // expected-note {{destroyed here}}
+  if (cond) p = &safe;
+  (void)*p;  // expected-note {{later used here}}
+
+  {
+    MyObj a, b;
+    p = cond ? &a   // expected-warning {{object whose reference is captured does not live long enough}}
+             : &b;  // expected-warning {{object whose reference is captured does not live long enough}}
+  }  // expected-note 2 {{destroyed here}}
+  (void)*p;  // expected-note 2 {{later used here}}
+
+  {
+    MyObj a, b, c, d;
+    p = cond ? cond ? &a    // expected-warning {{object whose reference is captured does not live long enough}}.
+                    : &b    // expected-warning {{object whose reference is captured does not live long enough}}.
+             : cond ? &c    // expected-warning {{object whose reference is captured does not live long enough}}.
+                    : &d;   // expected-warning {{object whose reference is captured does not live long enough}}.
+  }  // expected-note 4 {{destroyed here}}
+  (void)*p;  // expected-note 4 {{later used here}}
+
+  {
+    MyObj a, b;
+    p = Identity(cond ? &a    // expected-warning {{object whose reference is captured does not live long enough}}
+                      : &b);  // expected-warning {{object whose reference is captured does not live long enough}}
+  }  // expected-note 2 {{destroyed here}}
+  (void)*p;  // expected-note 2 {{later used here}}
+
+  {
+    MyObj a, b;
+    p = Identity(cond ? Identity(&a)    // expected-warning {{object whose reference is captured does not live long enough}}
+                      : Identity(&b));  // expected-warning {{object whose reference is captured does not live long enough}}
+  }  // expected-note 2 {{destroyed here}}
+  (void)*p;  // expected-note 2 {{later used here}}
+
+  {
+    MyObj a, b, c, d;
+    p = Identity(cond ? Identity(cond ? &a     // expected-warning {{object whose reference is captured does not live long enough}}
+                                      : &b)    // expected-warning {{object whose reference is captured does not live long enough}}
+                      : Identity(cond ? &c     // expected-warning {{object whose reference is captured does not live long enough}}
+                                      : &d));  // expected-warning {{object whose reference is captured does not live long enough}}
+  }  // expected-note 4 {{destroyed here}}
+  (void)*p;  // expected-note 4 {{later used here}}
+}

[ymand]

maybe comment why we only kill for the true branch?

[usx95]

we kill to clear the initial state and merge (flow) both origins into it.

[ymand]

Does this trigger in all modes or only pessimitic/conservative? I vaguely remember discussing this but can't remember where we landed.

[usx95]

This triggers in only strict mode. Confidence is driven by liveness now and p is not live on all paths from here so this is maybe case.