Refine the heuristic on inferring the nested pointer onwnership.

Fix another new false positive

Author: hokein

clang/lib/Sema/CheckExprLifetime.cpp

@@ -285,6 +285,18 @@ static bool isContainerOfPointer(const RecordDecl *Container) {
   }
   return false;
 }
+static bool isContainerOfOwner(const RecordDecl *Container) {
+  if (const auto *CTSD =
+          dyn_cast_if_present<ClassTemplateSpecializationDecl>(Container)) {
+    if (!CTSD->hasAttr<OwnerAttr>()) // Container must be a GSL owner type.
+      return false;
+    const auto &TAs = CTSD->getTemplateArgs();
+    return TAs.size() > 0 && TAs[0].getKind() == TemplateArgument::Type &&
+           isRecordWithAttr<OwnerAttr>(TAs[0].getAsType());
+  }
+  return false;
+}
+
 // Returns true if the given Record is `std::initializer_list<pointer>`.
 static bool isStdInitializerListOfPointer(const RecordDecl *RD) {
   if (const auto *CTSD =
@@ -357,35 +369,101 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
   return false;
 }
 
+// Returns true if the given constructor is a copy-like constructor, such as
+// `Ctor(Owner<U>&&)` or `Ctor(const Owner<U>&)`.
+static bool isCopyLikeConstructor(const CXXConstructorDecl *Ctor) {
+  if (!Ctor || Ctor->param_size() != 1)
+    return false;
+  const auto *ParamRefType =
+      Ctor->getParamDecl(0)->getType()->getAs<ReferenceType>();
+  if (!ParamRefType)
+    return false;
+
+  // Check if the first parameter type "Owner<U>".
+  if (const auto *TST =
+          ParamRefType->getPointeeType()->getAs<TemplateSpecializationType>())
+    return TST->getTemplateName()
+        .getAsTemplateDecl()
+        ->getTemplatedDecl()
+        ->hasAttr<OwnerAttr>();
+  return false;
+}
+
 // Returns true if we should perform the GSL analysis on the first argument for
 // the given constructor.
 static bool
 shouldTrackFirstArgumentForConstructor(const CXXConstructExpr *Ctor) {
-  const auto *ClassD = Ctor->getConstructor()->getParent();
+  const auto *LHSRecordDecl = Ctor->getConstructor()->getParent();
 
   // Case 1, construct a GSL pointer, e.g. std::string_view
-  if (ClassD->hasAttr<PointerAttr>())
+  // Always inspect when LHS is a pointer.
+  if (LHSRecordDecl->hasAttr<PointerAttr>())
     return true;
 
-  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))
+      !isContainerOfPointer(LHSRecordDecl))
     return false;
 
-  // For the typical case: `std::vector<std::string_view> abc = {std::string()};`
+  // Now, the LHS is an Owner<Pointer> type, e.g., std::vector<string_view>.
+  //
+  // At a high level, we cannot precisely determine what the nested pointer
+  // owns. However, by analyzing the RHS owner type, we can use heuristics to
+  // infer ownership information. These heuristics are designed to be
+  // conservative, minimizing false positives while still providing meaningful
+  // diagnostics.
+  //
+  // While this inference isn't perfect, it helps catch common use-after-free
+  // patterns.
+  auto RHSArgType = Ctor->getArg(0)->getType();
+  const auto *RHSRD = RHSArgType->getAsRecordDecl();
+  // LHS is constructed from an intializer_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.
-  if (isStdInitializerListOfPointer(FirstArgType->getAsRecordDecl()))
+  // E.g. std::vector<string_view> abc = {string()};
+  if (isStdInitializerListOfPointer(RHSRD))
+    return true;
+
+  // RHS must be an owner.
+  if (!isRecordWithAttr<OwnerAttr>(RHSArgType))
+    return false;
+
+  // Bail out if the RHS is Owner<Pointer>.
+  //
+  // We cannot reliably determine what the LHS nested pointer owns -- it could
+  // be the entire RHS or the nested pointer in RHS. To avoid false positives,
+  // we skip this case, such as:
+  //   std::stack<std::string_view> s(std::deque<std::string_view>{});
+  //
+  // TODO: this also has a false negative, it doesn't catch the case like:
+  //   std::optional<span<int*>> os = std::vector<int*>{}
+  if (isContainerOfPointer(RHSRD))
+    return false;
+
+  // Assume that the nested Pointer is constructed from the nested Owner.
+  // E.g. std::optional<string_view> sv = std::optional<string>(s);
+  if (isContainerOfOwner(RHSRD))
     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());
+
+  // Now, the LHS is an Owner<Pointer> and the RHS is an Owner<X>,  where X is
+  // neither an `Owner` nor a `Pointer`.
+  //
+  // Use the constructor's signature as a hint. If it is a copy-like constructor
+  // `Owner1<Pointer>(Owner2<X>&&)`, we assume that the nested pointer is
+  // constructed from X. In such cases, we do not diagnose, as `X` is not an
+  // owner, e.g.
+  //   std::optional<string_view> sv = std::optional<Foo>();
+  if (const auto *PrimaryCtorTemplate =
+          Ctor->getConstructor()->getPrimaryTemplate();
+      PrimaryCtorTemplate &&
+      isCopyLikeConstructor(dyn_cast_if_present<CXXConstructorDecl>(
+          PrimaryCtorTemplate->getTemplatedDecl()))) {
+    return false;
+  }
+  // Assume that the nested pointer is constructed from the whole RHS.
+  // E.g. optional<string_view> s = std::string();
+  return true;
 }
 
 // Return true if this is an "normal" assignment operator.

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

@@ -655,12 +655,37 @@ std::vector<std::string_view> test2(int i) {
   return std::vector<std::string_view>(t.begin(), t.end());
 }
 
+class Foo {
+  public:
+   operator std::string_view() const { return ""; }
+};
+class [[gsl::Owner]] FooOwner {
+  public:
+   operator std::string_view() const { return ""; }
+};
+std::optional<Foo> GetFoo();
+std::optional<FooOwner> GetFooOwner();
+
+template <typename T>
+struct [[gsl::Owner]] Container1 {
+   Container1();
+};
+template <typename T>
+struct [[gsl::Owner]] Container2 {
+  template<typename U>
+  Container2(const Container1<U>& C2);
+};
+
 std::optional<std::string_view> test3(int i) {
   std::string s;
   std::string_view sv;
   if (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.
+  Container2<std::string_view> c2 = Container1<FooOwner>(); // expected-warning {{object backing the pointer will be destroyed}}
+  return GetFoo(); // fine, we don't know Foo is owner or not, be conservative.
+  return GetFooOwner(); // expected-warning {{returning address of local temporary object}}
 }
 
 std::optional<int*> test4(int a) {