[clang] [clang] Detect dangling assignment for "Container<Pointer>" case. (PR #108205)

[Haojian Wu via cfe-commits]

https://github.com/hokein updated https://github.com/llvm/llvm-project/pull/108205

>From ff2f955965fd5f8f17ae76a11e495217aa4f02fd Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Wed, 2 Oct 2024 13:26:44 +0200
Subject: [PATCH] [clang] Detect dangling assignment for "Container<Pointer>"
 case.

---
 clang/lib/Sema/CheckExprLifetime.cpp          | 18 ++++++++++++++--
 .../Sema/warn-lifetime-analysis-nocfg.cpp     | 21 ++++++++++++++++---
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 009b8d000e6b0e..26f49bc559dc55 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -1096,7 +1096,8 @@ static bool shouldRunGSLAssignmentAnalysis(const Sema &SemaRef,
       diag::warn_dangling_lifetime_pointer_assignment, SourceLocation());
   return (EnableGSLAssignmentWarnings &&
           (isRecordWithAttr<PointerAttr>(Entity.LHS->getType()) ||
-           isAssignmentOperatorLifetimeBound(Entity.AssignmentOperator)));
+           isAssignmentOperatorLifetimeBound(Entity.AssignmentOperator) ||
+           isContainerOfPointer(Entity.LHS->getType()->getAsRecordDecl())));
 }
 
 static void checkExprLifetimeImpl(Sema &SemaRef,
@@ -1391,8 +1392,21 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
   };
 
   llvm::SmallVector<IndirectLocalPathEntry, 8> Path;
-  if (LK == LK_Assignment && shouldRunGSLAssignmentAnalysis(SemaRef, *AEntity))
+  if (LK == LK_Assignment &&
+      shouldRunGSLAssignmentAnalysis(SemaRef, *AEntity)) {
+    if (isContainerOfPointer(AEntity->LHS->getType()->getAsRecordDecl())) {
+      // Bail out for user-defined assignment operators, as their contract is
+      // unknown.
+      if (!AEntity->AssignmentOperator->isDefaulted())
+        return;
+      // Skip the top MaterializeTemoraryExpr because it is temporary object of
+      // the containerOfPointer itself.
+      if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Init))
+        Init = MTE->getSubExpr();
+    }
+
     Path.push_back({IndirectLocalPathEntry::GslPointerAssignment, Init});
+  }
 
   if (Init->isGLValue())
     visitLocalsRetainedByReferenceBinding(Path, Init, RK_ReferenceBinding,
diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
index 731639ab16a735..db53a434789812 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -634,18 +634,27 @@ void test() {
   std::optional<std::string_view> o3 = std::optional<std::string>(s); // expected-warning {{object backing the pointer}}
   std::optional<std::string_view> o4 = std::optional<std::string_view>(s);
 
-  // FIXME: should work for assignment cases
-  v1 = {std::string()};
-  o1 = std::string();
+  v1 = {std::string()}; // expected-warning {{object backing the pointer}}
+  o1 = std::string(); // expected-warning {{object backing the pointer}}
 
   // no warning on copying pointers.
   std::vector<std::string_view> n1 = {std::string_view()};
+  n1 = {std::string_view()};
   std::optional<std::string_view> n2 = {std::string_view()};
+  n2 = {std::string_view()};
   std::optional<std::string_view> n3 = std::string_view();
+  n3 = std::string_view();
   std::optional<std::string_view> n4 = std::make_optional(std::string_view());
+  n4 = std::make_optional(std::string_view());
   const char* b = "";
   std::optional<std::string_view> n5 = std::make_optional(b);
+  n5 = std::make_optional(b);
   std::optional<std::string_view> n6 = std::make_optional("test");
+  n6 = std::make_optional("test");
+  // Deeper nested containers are not supported; only first-level nesting is
+  // supported.
+  std::vector<std::vector<std::string_view>> n7 = {{std::string()}};
+  n7 = {{std::string()}};
 }
 
 std::vector<std::string_view> test2(int i) {
@@ -683,7 +692,9 @@ std::optional<std::string_view> test3(int i) {
    return s; // expected-warning {{address of stack memory associated}}
   return sv; // fine
   Container2<std::string_view> c1 = Container1<Foo>(); // no diagnostic as Foo is not an Owner.
+  c1 = Container1<Foo>();
   Container2<std::string_view> c2 = Container1<FooOwner>(); // expected-warning {{object backing the pointer will be destroyed}}
+  c2 = Container1<FooOwner>(); // expected-warning {{object backing the pointer}}
   return GetFoo(); // fine, we don't know Foo is owner or not, be conservative.
   return GetFooOwner(); // expected-warning {{returning address of local temporary object}}
 }
@@ -713,10 +724,12 @@ struct [[gsl::Pointer]] Span {
 // Pointer from Owner<Pointer>
 std::string_view test5() {
   std::string_view a = StatusOr<std::string_view>().valueLB(); // expected-warning {{object backing the pointer will be dest}}
+  a = StatusOr<std::string_view>().valueLB(); // expected-warning {{object backing the pointer}}
   return StatusOr<std::string_view>().valueLB(); // expected-warning {{returning address of local temporary}}
 
   // No dangling diagnostics on non-lifetimebound methods.
   std::string_view b = StatusOr<std::string_view>().valueNoLB();
+  b = StatusOr<std::string_view>().valueNoLB();
   return StatusOr<std::string_view>().valueNoLB();
 }
 
@@ -773,8 +786,10 @@ const int& test12(Span<int> a) {
 void test13() {
   // FIXME: RHS is Owner<Pointer>, we skip this case to avoid false positives.
   std::optional<Span<int*>> abc = std::vector<int*>{};
+  abc = std::vector<int*> {};
 
   std::optional<Span<int>> t = std::vector<int> {}; // expected-warning {{object backing the pointer will be destroyed}}
+  t = std::vector<int> {}; // expected-warning {{object backing the pointer}}
 }
 
 } // namespace GH100526