[clang] [LifetimeSafety] Fix handling of reference-type DeclRefExpr (PR #176728)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: Utkarsh Saxena (usx95)

<details>
<summary>Changes</summary>

Fix handling of reference-typed DeclRefExpr in lifetime analysis

Fixes https://github.com/llvm/llvm-project/issues/176399

This PR fixes a bug in the lifetime analysis where reference-typed DeclRefExpr nodes were incorrectly handled. The analysis was incorrectly removing the outer layer of origin for reference types, which led to missing some dangling reference warnings.

The fix adds a check to only remove the outer layer of origin when the declaration is not a reference type.

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


4 Files Affected:

- (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+4-3) 
- (modified) clang/test/Sema/Inputs/lifetime-analysis.h (+2-1) 
- (modified) clang/test/Sema/warn-lifetime-analysis-nocfg.cpp (+13-5) 
- (modified) clang/test/Sema/warn-lifetime-safety.cpp (+23) 


``````````diff
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index 54e19b58bd7d5..5d1afd15c795d 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -589,9 +589,10 @@ void FactsGenerator::handleUse(const DeclRefExpr *DRE) {
   OriginList *List = getOriginsList(*DRE);
   if (!List)
     return;
-  // Remove the outer layer of origin which borrows from the decl directly. This
-  // is a use of the underlying decl.
-  List = getRValueOrigins(DRE, List);
+  // Remove the outer layer of origin which borrows from the decl directly
+  // (e.g., when this is not a reference). This is a use of the underlying decl.
+  if (!DRE->getDecl()->getType()->isReferenceType())
+    List = getRValueOrigins(DRE, List);
   // Skip if there is no inner origin (e.g., when it is not a pointer type).
   if (!List)
     return;
diff --git a/clang/test/Sema/Inputs/lifetime-analysis.h b/clang/test/Sema/Inputs/lifetime-analysis.h
index 918547dfcec51..b47fe78bdf0fb 100644
--- a/clang/test/Sema/Inputs/lifetime-analysis.h
+++ b/clang/test/Sema/Inputs/lifetime-analysis.h
@@ -49,7 +49,8 @@ struct vector {
   template<typename InputIterator>
 	vector(InputIterator first, InputIterator __last);
 
-  T &at(int n);
+  T &  at(int n) &;
+  T && at(int n) &&;
 
   void push_back(const T&);
   void push_back(T&&);
diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
index 7fdc493dbd17a..fac5eb68d1f50 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -280,13 +280,21 @@ std::string_view danglingRefToOptionalFromTemp4() {
 }
 
 void danglingReferenceFromTempOwner() {
+  int &&r = *std::optional<int>();          // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \
+                                            // cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}}
   // FIXME: Detect this using the CFG-based lifetime analysis.
   //        https://github.com/llvm/llvm-project/issues/175893
-  int &&r = *std::optional<int>();          // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
   int &&r2 = *std::optional<int>(5);        // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+
+  // FIXME: Detect this using the CFG-based lifetime analysis.
+  //        https://github.com/llvm/llvm-project/issues/175893
   int &&r3 = std::optional<int>(5).value(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
-  int &r4 = std::vector<int>().at(3);       // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
-  use(r, r2, r3, r4);
+
+  const int &r4 = std::vector<int>().at(3); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \
+                                            // cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}}
+  int &&r5 = std::vector<int>().at(3);      // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \
+                                            // cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}}
+  use(r, r2, r3, r4, r5);                   // cfg-note 3 {{later used here}}
 
   std::string_view sv = *getTempOptStr();  // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \
                                            // cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}}
@@ -299,8 +307,8 @@ std::optional<std::vector<int>> getTempOptVec();
 void testLoops() {
   for (auto i : getTempVec()) // ok
     ;
-  // FIXME: Detect this using the CFG-based lifetime analysis.
-  for (auto i : *getTempOptVec()) // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  for (auto i : *getTempOptVec()) // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} \
+                                  // cfg-warning {{object whose reference is captured does not live long enough}} cfg-note {{destroyed here}} cfg-note {{later used here}}
     ;
 }
 
diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp
index 0b1962b7cb651..655cdf64046f3 100644
--- a/clang/test/Sema/warn-lifetime-safety.cpp
+++ b/clang/test/Sema/warn-lifetime-safety.cpp
@@ -1431,3 +1431,26 @@ void not_silenced_via_conditional(bool cond) {
   (void)v;  // expected-note 2 {{later used here}}
 }
 } // namespace do_not_warn_on_std_move
+
+namespace reference_type_decl_ref_expr {
+struct S {
+  S();
+  ~S();
+  const std::string& x() const [[clang::lifetimebound]];
+};
+
+void test_temporary() {
+  const std::string& x = S().x(); // expected-warning {{object whose reference is captured does not live long enough}} expected-note {{destroyed here}}
+  (void)x; // expected-note {{later used here}}
+}
+
+void test_lifetime_extension_ok() {
+  const S& x = S();
+  (void)x;
+}
+
+const std::string& test_return() {
+  const std::string& x = S().x(); // expected-warning {{object whose reference is captured does not live long enough}} expected-note {{destroyed here}}
+  return x; // expected-note {{later used here}}
+}
+} // namespace reference_type_decl_ref_expr

``````````

</details>


https://github.com/llvm/llvm-project/pull/176728