[clang] Add lifetimebound attr to std::span/std::string_view constructor

[hokein]

With this patch, clang now automatically adds [[clang::lifetimebound]] to the parameters of std::span, std::string_view constructors, this enables Clang to capture more cases where the returned reference outlives the object.

Fixes #100567

[llvmbot]

@llvm/pr-subscribers-clang

Author: Haojian Wu (hokein)

Changes

With this patch, clang now automatically adds [[clang::lifetimebound]] to the parameters of std::span, std::string_view constructors, this enables Clang to capture more cases where the returned reference outlives the object.

Fixes #100567

---

Full diff: https://github.com/llvm/llvm-project/pull/103716.diff

5 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+5)
• (modified) clang/include/clang/Sema/Sema.h (+3)
• (modified) clang/lib/Sema/SemaAttr.cpp (+53)
• (modified) clang/lib/Sema/SemaDecl.cpp (+1-19)
• (modified) clang/test/SemaCXX/attr-lifetimebound.cpp (+21)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 39e1b0fcb09bbd..63ba5c4494b93e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -151,6 +151,11 @@ Attribute Changes in Clang
 - The ``hybrid_patchable`` attribute is now supported on ARM64EC targets. It can be used to specify
   that a function requires an additional x86-64 thunk, which may be patched at runtime.
 
+- Clang now automatically adds ``[[clang::lifetimebound]]`` to the parameters of
+  ``std::span, std::string_view`` constructors, this enables Clang to capture
+  more cases where the returned reference outlives the object.
+  (#GH100567)
+
 Improvements to Clang's diagnostics
 -----------------------------------
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 25cb6c8fbf6104..a4e6c98ec66884 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1827,6 +1827,9 @@ class Sema final : public SemaBase {
   /// Add [[gsl::Owner]] and [[gsl::Pointer]] attributes for std:: types.
   void inferGslOwnerPointerAttribute(CXXRecordDecl *Record);
 
+  /// Add [[clang:::lifetimebound]] attr for std:: functions and methods.
+  void inferLifetimeBoundAttribute(FunctionDecl *FD);
+
   /// Add [[gsl::Pointer]] attributes for std:: types.
   void inferGslPointerAttribute(TypedefNameDecl *TD);
 
diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp
index b0c239678d0b01..fb83d56c2273ea 100644
--- a/clang/lib/Sema/SemaAttr.cpp
+++ b/clang/lib/Sema/SemaAttr.cpp
@@ -216,6 +216,59 @@ void Sema::inferGslOwnerPointerAttribute(CXXRecordDecl *Record) {
   inferGslPointerAttribute(Record, Record);
 }
 
+void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) {
+  if (FD->getNumParams() == 0)
+    return;
+
+  if (unsigned BuiltinID = FD->getBuiltinID()) {
+    // Add lifetime attribute to std::move, std::fowrard et al.
+    switch (BuiltinID) {
+    case Builtin::BIaddressof:
+    case Builtin::BI__addressof:
+    case Builtin::BI__builtin_addressof:
+    case Builtin::BIas_const:
+    case Builtin::BIforward:
+    case Builtin::BIforward_like:
+    case Builtin::BImove:
+    case Builtin::BImove_if_noexcept:
+      if (ParmVarDecl *P = FD->getParamDecl(0u);
+          !P->hasAttr<LifetimeBoundAttr>())
+        P->addAttr(
+            LifetimeBoundAttr::CreateImplicit(Context, FD->getLocation()));
+      break;
+    default:
+      break;
+    }
+    return;
+  }
+  if (auto *CMD = dyn_cast<CXXMethodDecl>(FD)) {
+    const auto *CRD = CMD->getParent();
+    if (!CRD->isInStdNamespace() || !CRD->getIdentifier())
+      return;
+
+    if (isa<CXXConstructorDecl>(CMD)) {
+      auto *Param = CMD->getParamDecl(0);
+      if (Param->hasAttr<LifetimeBoundAttr>())
+        return;
+      if (CRD->getName() == "basic_string_view" &&
+          Param->getType()->isPointerType()) {
+        // construct from a char array pointed by a pointer.
+        //   basic_string_view(const CharT* s);
+        //   basic_string_view(const CharT* s, size_type count);
+        Param->addAttr(
+            LifetimeBoundAttr::CreateImplicit(Context, FD->getLocation()));
+      } else if (CRD->getName() == "span") {
+        // construct from a reference of array.
+        //   span(std::type_identity_t<element_type> (&arr)[N]);
+        const auto *LRT = Param->getType()->getAs<LValueReferenceType>();
+        if (LRT && LRT->getPointeeType().IgnoreParens()->isArrayType())
+          Param->addAttr(
+              LifetimeBoundAttr::CreateImplicit(Context, FD->getLocation()));
+      }
+    }
+  }
+}
+
 void Sema::inferNullableClassAttribute(CXXRecordDecl *CRD) {
   static const llvm::StringSet<> Nullable{
       "auto_ptr",         "shared_ptr", "unique_ptr",         "exception_ptr",
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 641b180527da55..6a79231fb3bd71 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16593,27 +16593,9 @@ void Sema::AddKnownFunctionAttributes(FunctionDecl *FD) {
     default:
       break;
     }
-
-    // Add lifetime attribute to std::move, std::fowrard et al.
-    switch (BuiltinID) {
-    case Builtin::BIaddressof:
-    case Builtin::BI__addressof:
-    case Builtin::BI__builtin_addressof:
-    case Builtin::BIas_const:
-    case Builtin::BIforward:
-    case Builtin::BIforward_like:
-    case Builtin::BImove:
-    case Builtin::BImove_if_noexcept:
-      if (ParmVarDecl *P = FD->getParamDecl(0u);
-          !P->hasAttr<LifetimeBoundAttr>())
-        P->addAttr(
-            LifetimeBoundAttr::CreateImplicit(Context, FD->getLocation()));
-      break;
-    default:
-      break;
-    }
   }
 
+  inferLifetimeBoundAttribute(FD);
   AddKnownFunctionAttributesForReplaceableGlobalAllocationFunction(FD);
 
   // If C++ exceptions are enabled but we are told extern "C" functions cannot
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 7db0a4d64d2596..d10e876fe57526 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -237,6 +237,16 @@ template <class T> T *addressof(T &arg) {
         &const_cast<char &>(reinterpret_cast<const volatile char &>(arg)));
 }
 
+template<typename T>
+struct basic_string_view {
+  basic_string_view(const T *);
+};
+
+template <class T> struct span {
+  template<size_t _ArrayExtent>
+	span(T (&__arr)[_ArrayExtent]) noexcept;
+};
+
 } // namespace foo
 } // namespace std
 
@@ -265,3 +275,14 @@ namespace move_forward_et_al_examples {
   S *AddressOfOk = std::addressof(X);
 } // namespace move_forward_et_al_examples
 
+namespace ctor_cases {
+std::basic_string_view<char> test1() {
+  char abc[10];
+  return abc;  // expected-warning {{address of stack memory associated with local variable}}
+}
+
+std::span<int> test2() {
+  int abc[10];
+  return abc; // expected-warning {{address of stack memory associated with local variable}}
+}
+} // namespace ctor_cases

[Xazax-hun]

Looks great, thanks!

[Xazax-hun]

I start to have the feeling we are writing code like this over and over again.
Some examples:

llvm-project/clang/lib/Analysis/BodyFarm.cpp

Line 718 in df57833

case Builtin::BIas_const:

llvm-project/clang/lib/AST/ExprConstant.cpp

Line 8763 in df57833

case Builtin::BIas_const:

But there are more, and I always wonder if they can get out of sync. I wonder if this is time to introduce some helper functions that can be reused.

[hokein]

(The latter two cases are identical, we could potentially abstract a common function.)

I think this should be handled on a case-by-case basis, depending on the specific logic involved. It seems fine to me have this small amount of duplication.

[nikic]

Just as a FYI this inference introduces a small amount of overhead: http://llvm-compile-time-tracker.com/compare.php?from=866bec7d3ff8803b68e9972939c1a76ccf5fdc62&to=902b2a26ab9e1e78dfb66b52fba4512c91472e09&stat=instructions:u