[clang] [clang] Don't consider the lifetimeboundCall when analyzing the gsl pointer construction. (PR #114044)

[Haojian Wu via cfe-commits]

https://github.com/hokein created https://github.com/llvm/llvm-project/pull/114044

This fixes a bug raised in https://github.com/llvm/llvm-project/pull/112751#issuecomment-2443566707.




>From 32d808a6c30cb572c7905f93b9c14b5d8a25599d Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Tue, 29 Oct 2024 13:53:35 +0100
Subject: [PATCH] [clang] Don't consider the lifetimeboundCall when analyzing
 the gsl pointer construction.

---
 clang/lib/Sema/CheckExprLifetime.cpp          | 23 +++++++++++-
 .../Sema/warn-lifetime-analysis-nocfg.cpp     | 37 ++++++++++++++++---
 clang/test/SemaCXX/attr-lifetimebound.cpp     |  2 +-
 3 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index aa0a2e223e708f..b19b320fe30228 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -1094,6 +1094,24 @@ static bool pathOnlyHandlesGslPointer(IndirectLocalPath &Path) {
   return false;
 }
 
+static bool
+isLifetimeboundInterleaveInGSL(llvm::ArrayRef<IndirectLocalPathEntry> PathRef) {
+  if (auto FirstGSLPointer = llvm::find_if(
+          PathRef,
+          [](const IndirectLocalPathEntry &Path) {
+            return Path.Kind == IndirectLocalPathEntry::GslPointerInit ||
+                   Path.Kind == IndirectLocalPathEntry::GslPointerAssignment;
+          });
+      FirstGSLPointer != PathRef.end()) {
+    return llvm::find_if(
+               PathRef.drop_front(FirstGSLPointer - PathRef.begin() + 1),
+               [](const IndirectLocalPathEntry &Path) {
+                 return Path.Kind == IndirectLocalPathEntry::LifetimeBoundCall;
+               }) != PathRef.end();
+  }
+  return false;
+}
+
 static bool isAssignmentOperatorLifetimeBound(CXXMethodDecl *CMD) {
   if (!CMD)
     return false;
@@ -1141,7 +1159,8 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
         //   someContainer.add(std::move(localUniquePtr));
         //   return p;
         IsLocalGslOwner = isRecordWithAttr<OwnerAttr>(L->getType());
-        if (pathContainsInit(Path) || !IsLocalGslOwner)
+        if (pathContainsInit(Path) || !IsLocalGslOwner ||
+            isLifetimeboundInterleaveInGSL(Path))
           return false;
       } else {
         IsGslPtrValueFromGslTempOwner =
@@ -1152,6 +1171,8 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
         // a local or temporary owner or the address of a local variable/param.
         if (!IsGslPtrValueFromGslTempOwner)
           return true;
+        if (isLifetimeboundInterleaveInGSL(Path))
+          return false;
       }
     }
 
diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
index 688f55edfe84df..b97e34840c0d9b 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -714,8 +714,8 @@ 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}}
-  return StatusOr<std::string_view>().valueLB(); // expected-warning {{returning address of local temporary}}
+  std::string_view a = StatusOr<std::string_view>().valueLB();
+  return StatusOr<std::string_view>().valueLB();
 
   // No dangling diagnostics on non-lifetimebound methods.
   std::string_view b = StatusOr<std::string_view>().valueNoLB();
@@ -746,7 +746,7 @@ std::vector<int*> test8(StatusOr<std::vector<int*>> aa) {
 
 // Pointer<Pointer> from Owner<Owner<Pointer>>
 Span<int*> test9(StatusOr<std::vector<int*>> aa) {
-  return aa.valueLB(); // expected-warning {{address of stack memory associated}}
+  return aa.valueLB(); //
   return aa.valueNoLB(); // OK.
 }
 
@@ -754,7 +754,7 @@ Span<int*> test9(StatusOr<std::vector<int*>> aa) {
 
 // Pointer<Owner>> from Owner<Owner>
 Span<std::string> test10(StatusOr<std::vector<std::string>> aa) {
-  return aa.valueLB(); // expected-warning {{address of stack memory}}
+  return aa.valueLB(); //
   return aa.valueNoLB(); // OK.
 }
 
@@ -762,7 +762,7 @@ Span<std::string> test10(StatusOr<std::vector<std::string>> aa) {
 
 // Pointer<Owner>> from Owner<Pointer<Owner>>
 Span<std::string> test11(StatusOr<Span<std::string>> aa) {
-  return aa.valueLB(); // expected-warning {{address of stack memory}}
+  return aa.valueLB(); //
   return aa.valueNoLB(); // OK.
 }
 
@@ -780,3 +780,30 @@ void test13() {
 }
 
 } // namespace GH100526
+
+namespace test {
+
+struct [[gsl::Pointer]] FooOwner {};
+struct [[gsl::Pointer]] FooPointer {
+  FooPointer(const FooOwner&);
+};
+
+template<typename T>
+struct [[gsl::Owner]] Container {
+  const T& get() const [[clang::lifetimebound]];
+};
+
+FooPointer func() {
+  Container<FooPointer> foo;
+
+  FooPointer p = Container<FooPointer>().get();
+  p = Container<FooPointer>().get();
+  return foo.get();
+
+  FooPointer p2 = Container<FooOwner>().get();
+  p2 = Container<FooOwner>().get();
+  Container<FooOwner> foo2;
+  return foo2.get();
+}
+
+}
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 804d61fb62ca40..70a80affceda54 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -153,7 +153,7 @@ namespace p0936r0_examples {
   void f() {
     std::string_view sv = "hi";
     std::string_view sv2 = sv + sv; // expected-warning {{temporary}}
-    sv2 = sv + sv; // expected-warning {{object backing the pointer}}
+    sv2 = sv + sv; // FIXME: warn here too.
   }
 
   struct X { int a, b; };