Add comments and add release note.

Author: hokein

clang/docs/ReleaseNotes.rst

@@ -278,6 +278,8 @@ Improvements to Clang's diagnostics
 - The lifetimebound and GSL analysis in clang are coherent, allowing clang to
   detect more use-after-free bugs. (#GH100549).
 
+- Clang now diagnoses cases where a dangling `GSLOwner<GSLPointer>`` object is constructed, e.g. `std::vector<string_view> v = {std::string}` (#GH100526).
+
 Improvements to Clang's time-trace
 ----------------------------------
 

clang/include/clang/Basic/AttrDocs.td

@@ -6690,6 +6690,20 @@ When the Owner's lifetime ends, it will consider the Pointer to be dangling.
     P.getInt(); // P is dangling
   }
 
+If a template class is annotated with [[gsl::Owner]], and the first instantiated
+template argument is a [[gsl::Pointer]] type, the analysis will consider the
+instantiated class as a container of the pointer. When constructing such an
+object from a GSL owner object, the analysis will assume that the container
+holds a pointer to the owner object. Consequently, when the owner object is
+destroyed, the pointer will be considered dangling.
+
+.. code-block:: c++
+
+   int f() {
+     std::vector<std::string_view> v = {std::string();}; // v holds a dangling pointer.
+     std::optional<std::string_view> o = std::string(); // o holds a dangling pointer.
+   }
+
 }];
 }
 

clang/lib/Sema/CheckExprLifetime.cpp

@@ -361,12 +361,21 @@ static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
     if (ATL.getAttrAs<LifetimeBoundAttr>())
       return true;
   }
+
   return isNormalAsisgnmentOperator(FD);
 }
 
-bool isFirstTemplateArgumentGSLPointer(const TemplateArgumentList &TAs) {
-  return TAs.size() > 0 && TAs[0].getKind() == TemplateArgument::Type &&
-         isRecordWithAttr<PointerAttr>(TAs[0].getAsType());
+// Returns true if the given Record decl is a form of `GSLOwner<GSLPointer>`
+// type, e.g. std::vector<string_view>, std::optional<string_view>.
+static bool isContainerOfGSLPointer(const CXXRecordDecl *Container) {
+  if (const auto *CTSD = dyn_cast<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<PointerAttr>(TAs[0].getAsType());
+  }
+  return false;
 }
 
 // Visit lifetimebound or gsl-pointer arguments.
@@ -471,19 +480,10 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
                            !Callee->getReturnType()->isReferenceType());
       } else if (auto *Ctor = dyn_cast<CXXConstructExpr>(Call)) {
         const auto *ClassD = Ctor->getConstructor()->getParent();
-        // Constructing the Container<GSLPointer> case (e.g.
-        // std::optional<string_view>) case.
-        if (const auto *CTSD =
-                dyn_cast<ClassTemplateSpecializationDecl>(ClassD)) {
-          if (isFirstTemplateArgumentGSLPointer(CTSD->getTemplateArgs()) &&
-              CTSD->hasAttr<OwnerAttr>()) {
-            VisitGSLPointerArg(Ctor->getConstructor()->getParamDecl(0), Args[0],
-                               true);
-            return;
-          }
-        }
-        // Constructing the GSLPointer (e.g. std::string_view) case.
-        if (ClassD->hasAttr<PointerAttr>())
+        // Two cases:
+        //  a GSL pointer, e.g. std::string_view
+        //  a container of GSL pointer, e.g. std::vector<string_view>
+        if (ClassD->hasAttr<PointerAttr>() || isContainerOfGSLPointer(ClassD))
           VisitGSLPointerArg(Ctor->getConstructor()->getParamDecl(0), Args[0],
                              true);
       }

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

@@ -547,19 +547,22 @@ void test() {
 
 namespace GH100526 {
 void test() {
-  std::vector<std::string_view> t1 = {std::string()}; // expected-warning {{object backing the pointer will be destroyed at the end}}
-  std::optional<std::string_view> t2 = std::string(); // expected-warning {{object backing the pointer}}
+  std::vector<std::string_view> v1({std::string()}); // expected-warning {{object backing the pointer will be destroyed at the end}}
+  std::vector<std::string_view> v2({std::string(), std::string_view()}); // expected-warning {{object backing the pointer will be destroyed at the end}}
+  std::vector<std::string_view> v3({std::string_view(), std::string()}); // expected-warning {{object backing the pointer will be destroyed at the end}}
+
+  std::optional<std::string_view> o1 = std::string(); // expected-warning {{object backing the pointer}}
 
   std::string s;
   // This is a tricky use-after-free case, what it does:
   //   1. make_optional creates a temporary "optional<string>"" object
   //   2. the temporary object owns the underlying string which is copied from s.
   //   3. the t3 object holds the view to the underlying string of the temporary object.
-  std::optional<std::string_view> t3 = std::make_optional(s); // expected-warning {{object backing the pointer}}
+  std::optional<std::string_view> o2 = std::make_optional(s); // expected-warning {{object backing the pointer}}
 
   // FIXME: should work for assignment cases
-  t1 = {std::string()};
-  t2 = std::string();
+  v1 = {std::string()};
+  o1 = std::string();
 
   // no warning on copying pointers.
   std::vector<std::string_view> n1 = {std::string_view()};