Fix a newly-introcuded false negative.

And add more tests per the comment. The new false negative was caused by
replacing the instances of "!isRecordWithAttr<OwnerAttr>" in `TemporaryVisitor`
with "isGSLOwner".

Author: hokein

clang/lib/Sema/CheckExprLifetime.cpp

@@ -298,11 +298,6 @@ static bool isStdInitializerListOfPointer(const RecordDecl *RD) {
   return false;
 }
 
-static bool isGSLOwner(QualType T) {
-  return isRecordWithAttr<OwnerAttr>(T) &&
-         !isContainerOfPointer(T->getAsRecordDecl());
-}
-
 static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
   if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee))
     if (isRecordWithAttr<PointerAttr>(Conv->getConversionType()))
@@ -311,7 +306,7 @@ static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
     return false;
   if (!isRecordWithAttr<PointerAttr>(
           Callee->getFunctionObjectParameterType()) &&
-      !isGSLOwner(Callee->getFunctionObjectParameterType()))
+      !isRecordWithAttr<OwnerAttr>(Callee->getFunctionObjectParameterType()))
     return false;
   if (isPointerLikeType(Callee->getReturnType())) {
     if (!Callee->getIdentifier())
@@ -367,22 +362,29 @@ static bool
 shouldTrackFirstArgumentForConstructor(const CXXConstructExpr *Ctor) {
   const auto *ClassD = Ctor->getConstructor()->getParent();
 
-  auto FirstArgType = Ctor->getArg(0)->getType();
   // Case 1, construct a GSL pointer, e.g. std::string_view
   if (ClassD->hasAttr<PointerAttr>())
     return true;
 
-  // case 2: construct a container of pointer (std::vector<std::string_view>)
-  // from an owner or a std::initilizer_list.
-  //
-  // std::initializer_list is a proxy object that provides access to the backing
-  // array. We perform analysis on it to determine if there are any dangling
-  // temporaries in the backing array.
+  auto FirstArgType = Ctor->getArg(0)->getType();
+  // Case 2, construct a container of pointer (std::vector<std::string_view>)
+  // from a std::initilizer_list or an GSL owner
   if (Ctor->getConstructor()->getNumParams() != 1 ||
       !isContainerOfPointer(ClassD))
     return false;
-  return isGSLOwner(FirstArgType) ||
-         isStdInitializerListOfPointer(FirstArgType->getAsRecordDecl());
+
+  // For the typical case: `std::vector<std::string_view> abc = {std::string()};`
+  // std::initializer_list is a proxy object that provides access to the backing
+  // array. We perform analysis on it to determine if there are any dangling
+  // temporaries in the backing array.
+  if (isStdInitializerListOfPointer(FirstArgType->getAsRecordDecl()))
+    return true;
+  // For the case: `std::optional<std::string_view> abc = std::string();`
+  // When constructing from a container of pointers, it's less likely to result
+  // in a dangling pointer. Therefore, we try to be conservative to not track
+  // the argument futher to avoid false positives.
+  return isRecordWithAttr<OwnerAttr>(FirstArgType) &&
+         !isContainerOfPointer(FirstArgType->getAsRecordDecl());
 }
 
 // Return true if this is an "normal" assignment operator.
@@ -472,7 +474,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
     // Once we initialized a value with a non gsl-owner reference, it can no
     // longer dangle.
     if (ReturnType->isReferenceType() &&
-        !isGSLOwner(ReturnType->getPointeeType())) {
+        !isRecordWithAttr<OwnerAttr>(ReturnType->getPointeeType())) {
       for (const IndirectLocalPathEntry &PE : llvm::reverse(Path)) {
         if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit ||
             PE.Kind == IndirectLocalPathEntry::LifetimeBoundCall)
@@ -1044,12 +1046,13 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
         //   int &p = *localUniquePtr;
         //   someContainer.add(std::move(localUniquePtr));
         //   return p;
-        IsLocalGslOwner = isGSLOwner(L->getType());
+        IsLocalGslOwner = isRecordWithAttr<OwnerAttr>(L->getType());
         if (pathContainsInit(Path) || !IsLocalGslOwner)
           return false;
       } else {
         IsGslPtrValueFromGslTempOwner =
-            MTE && !MTE->getExtendingDecl() && isGSLOwner(MTE->getType());
+            MTE && !MTE->getExtendingDecl() &&
+            isRecordWithAttr<OwnerAttr>(MTE->getType());
         // Skipping a chain of initializing gsl::Pointer annotated objects.
         // We are looking only for the final source to find out if it was
         // a local or temporary owner or the address of a local variable/param.

clang/test/Sema/warn-lifetime-analysis-nocfg.cpp

@@ -172,7 +172,7 @@ struct vector {
   const T *data() const;
   vector();
   vector(initializer_list<T> __l);
-  
+
   template<typename InputIterator>
 	vector(InputIterator first, InputIterator __last);
 
@@ -218,10 +218,10 @@ struct optional {
   optional(const T&);
 
   template<typename U = T>
-	optional(U&& t);
+  optional(U&& t);
 
   template<typename U>
-	optional(optional<U>&& __t);
+  optional(optional<U>&& __t);
 
   T &operator*() &;
   T &&operator*() &&;
@@ -598,7 +598,7 @@ void test() {
   //   3. the t3 object holds the view to the underlying string of the temporary object.
   std::optional<std::string_view> o2 = std::make_optional(s); // expected-warning {{object backing the pointer}}
   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); 
+  std::optional<std::string_view> o4 = std::optional<std::string_view>(s);
 
   // FIXME: should work for assignment cases
   v1 = {std::string()};
@@ -633,12 +633,81 @@ std::optional<int*> test4(int a) {
   return std::make_optional(nullptr); // fine
 }
 
+
 template <typename T>
 struct [[gsl::Owner]] StatusOr {
-  const T &value() [[clang::lifetimebound]];
+  const T &valueLB() const [[clang::lifetimebound]];
+  const T &valueNoLB() const;
 };
-std::vector<int*> test5(StatusOr<std::vector<int*>> aa) {
-  return aa.value(); // fine
+
+template<typename T>
+struct [[gsl::Pointer]] Span {
+  Span(const std::vector<T> &V);
+
+  const int& getFieldLB() const [[clang::lifetimebound]];
+  const int& getFieldNoLB() const;
+};
+
+
+/////// From Owner<Pointer> ///////
+
+// 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}}
+
+  // No dangling diagnostics on non-lifetimebound methods.
+  std::string_view b = StatusOr<std::string_view>().valueNoLB();
+  return StatusOr<std::string_view>().valueNoLB();
+}
+
+// Pointer<Pointer> from Owner<Pointer>
+// Prevent regression GH108463
+Span<int*> test6(std::vector<int*> v) {
+  Span<int *> dangling = std::vector<int*>(); // expected-warning {{object backing the pointer}}
+  return v; // expected-warning {{address of stack memory}}
+}
+
+/////// From Owner<Owner<Pointer>> ///////
+
+// Pointer from Owner<Owner<Pointer>>
+int* test7(StatusOr<StatusOr<int*>> aa) {
+  // No dangling diagnostic on pointer.
+  return aa.valueLB().valueLB(); // OK.
+}
+
+// Owner<Pointer> from Owner<Owner<Pointer>>
+std::vector<int*> test8(StatusOr<std::vector<int*>> aa) {
+  return aa.valueLB(); // OK, no pointer being construct on this case.
+  return aa.valueNoLB();
+}
+
+// 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.valueNoLB(); // OK.
+}
+
+/////// From Owner<Owner> ///////
+
+// 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.valueNoLB(); // OK.
+}
+
+/////// From Owner<Pointer<Owner>> ///////
+
+// 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.valueNoLB(); // OK.
+}
+
+// Lifetimebound and gsl::Pointer.
+const int& test12(Span<int> a) {
+  return a.getFieldLB(); // expected-warning {{reference to stack memory associated}}
+  return a.getFieldNoLB(); // OK.
 }
 
 } // namespace GH100526