[clang] [-Wunsafe-buffer-usage] Support span creation from std::initializer_list and begin/end pairs (PR #145311)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Ziqing Luo (ziqingluo-90)

<details>
<summary>Changes</summary>

Support safe construction of `std::span` from `begin` and `end` calls on hardened containers or views or `std::initializer_list`s.

For example, the following code is safe:
```
void create(std::initializer_list<int> il) {
    std::span<int> input{ il.begin(), il.end() }; // no warn
}
```

rdar://152637380

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


2 Files Affected:

- (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+34-5) 
- (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp (+60) 


``````````diff
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 631a546b45ff4..259db3f092c92 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -115,6 +115,10 @@ class MatchResult {
 };
 } // namespace
 
+#define SIZED_CONTAINER_OR_VIEW_LIST                                           \
+  "span", "array", "vector", "basic_string_view", "basic_string",              \
+      "initializer_list",
+
 // A `RecursiveASTVisitor` that traverses all descendants of a given node "n"
 // except for those belonging to a different callable of "n".
 class MatchDescendantVisitor : public DynamicRecursiveASTVisitor {
@@ -463,6 +467,8 @@ static bool areEqualIntegers(const Expr *E1, const Expr *E2, ASTContext &Ctx) {
 //       `N, M` are parameter indexes to the allocating element number and size.
 //        Sometimes, there is only one parameter index representing the total
 //        size.
+//   7. `std::span<T>{x.begin(), x.end()}` where `x` is an object in the
+//      SIZED_CONTAINER_OR_VIEW_LIST.
 static bool isSafeSpanTwoParamConstruct(const CXXConstructExpr &Node,
                                         ASTContext &Ctx) {
   assert(Node.getNumArgs() == 2 &&
@@ -560,6 +566,31 @@ static bool isSafeSpanTwoParamConstruct(const CXXConstructExpr &Node,
         }
     }
   }
+  // Check form 7:
+  auto IsMethodCallToSizedObject = [](const Stmt *Node, StringRef MethodName) {
+    if (const auto *MC = dyn_cast<CXXMemberCallExpr>(Node)) {
+      const auto *MD = MC->getMethodDecl();
+      const auto *RD = MC->getRecordDecl();
+
+      if (auto *II = RD->getDeclName().getAsIdentifierInfo();
+          II && RD->isInStdNamespace())
+        return llvm::is_contained({SIZED_CONTAINER_OR_VIEW_LIST},
+                                  II->getName()) &&
+               MD->getName() == MethodName;
+    }
+    return false;
+  };
+
+  if (IsMethodCallToSizedObject(Arg0, "begin") &&
+      IsMethodCallToSizedObject(Arg1, "end"))
+    return AreSameDRE(
+        // We know Arg0 and Arg1 are `CXXMemberCallExpr`s:
+        cast<CXXMemberCallExpr>(Arg0)
+            ->getImplicitObjectArgument()
+            ->IgnoreParenImpCasts(),
+        cast<CXXMemberCallExpr>(Arg1)
+            ->getImplicitObjectArgument()
+            ->IgnoreParenImpCasts());
   return false;
 }
 
@@ -1058,8 +1089,7 @@ static bool hasUnsafeSnprintfBuffer(const CallExpr &Node,
     return false; // not an snprintf call
 
   // Pattern 1:
-  static StringRef SizedObjs[] = {"span", "array", "vector",
-                                  "basic_string_view", "basic_string"};
+  static StringRef SizedObjs[] = {SIZED_CONTAINER_OR_VIEW_LIST};
   Buf = Buf->IgnoreParenImpCasts();
   Size = Size->IgnoreParenImpCasts();
   if (auto *MCEPtr = dyn_cast<CXXMemberCallExpr>(Buf))
@@ -1826,9 +1856,8 @@ class DataInvocationGadget : public WarningGadget {
     auto *method = cast<CXXMethodDecl>(callee);
     if (method->getNameAsString() == "data" &&
         method->getParent()->isInStdNamespace() &&
-        (method->getParent()->getName() == "span" ||
-         method->getParent()->getName() == "array" ||
-         method->getParent()->getName() == "vector"))
+        llvm::is_contained({SIZED_CONTAINER_OR_VIEW_LIST},
+                           method->getParent()->getName()))
       return true;
     return false;
   }
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
index e287f00a5453b..1e7855517207e 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -std=c++20  -Wno-all  -Wunsafe-buffer-usage-in-container -verify %s
 
+typedef unsigned int size_t;
+
 namespace std {
   template <class T> class span {
   public:
@@ -16,6 +18,9 @@ namespace std {
 
     template<class R>
     constexpr span(R && range){};
+
+    T* begin() noexcept;
+    T* end() noexcept;
   };
 
 
@@ -27,6 +32,37 @@ namespace std {
     return &__x;
   }
 
+  template <typename T, size_t N>
+  struct array {
+    T* begin() noexcept;
+    const T* begin() const noexcept;
+    T* end() noexcept;
+    const T* end() const noexcept;
+    size_t size() const noexcept;
+    T * data() const noexcept;
+    T& operator[](size_t n);
+  };
+
+  template<class T>
+  class initializer_list {
+  public:
+    size_t size() const noexcept;
+    const T* begin() const noexcept;
+    const T* end() const noexcept;
+    T * data() const noexcept;
+  };
+
+  template<typename T>
+  struct basic_string {
+    T *c_str() const noexcept;
+    T *data()  const noexcept;
+    unsigned size();
+    const T* begin() const noexcept;
+    const T* end() const noexcept;
+  };
+
+  typedef basic_string<char> string;
+  typedef basic_string<wchar_t> wstring;
 }
 
 namespace irrelevant_constructors {
@@ -232,3 +268,27 @@ struct HoldsStdSpanAndNotInitializedInCtor {
       : Ptr(P), Size(S)
   {}
 };
+
+namespace test_begin_end {
+  struct Object {
+    int * begin();
+    int * end();
+  };
+  void safe_cases(std::span<int> Sp, std::array<int, 10> Arr, std::string Str, std::initializer_list<Object> Il) {
+    std::span<int>{Sp.begin(), Sp.end()};
+    std::span<int>{Arr.begin(), Arr.end()};
+    std::span<char>{Str.begin(), Str.end()};
+    std::span<Object>{Il.begin(), Il.end()};
+  }
+
+  void unsafe_cases(std::span<int> Sp, std::array<int, 10> Arr, std::string Str, std::initializer_list<Object> Il,
+		    Object Obj) {
+    std::span<int>{Obj.begin(), Obj.end()}; // expected-warning {{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
+    std::span<int>{Sp.end(), Sp.begin()};   // expected-warning {{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
+    std::span<int>{Sp.begin(), Arr.end()};   // expected-warning {{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
+  }
+
+  void unsupport_cases(std::array<Object, 10> Arr) {
+    std::span<int>{Arr[0].begin(), Arr[0].end()}; // expected-warning {{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
+  }
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/145311