Merge pull request #9584 from swiftlang/msamak-cherrypick-vector-data

Cherrypick [-Wunsafe-buffer-usage] Emit a warning if pointer returned by vector::data and array::data is cast to larger type

Author: malavikasamak

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -12475,7 +12475,7 @@ def warn_unsafe_buffer_variable : Warning<
   InGroup<UnsafeBufferUsage>, DefaultIgnore;
 def warn_unsafe_buffer_operation : Warning<
   "%select{unsafe pointer operation|unsafe pointer arithmetic|"
-  "unsafe buffer access|function introduces unsafe buffer manipulation|unsafe invocation of span::data|"
+  "unsafe buffer access|function introduces unsafe buffer manipulation|unsafe invocation of %1|"
   "field %1 prone to unsafe buffer manipulation}0">,
   InGroup<UnsafeBufferUsage>, DefaultIgnore;
 def warn_unsafe_buffer_libc_call : Warning<

clang/lib/Analysis/UnsafeBufferUsage.cpp

@@ -1505,8 +1505,11 @@ class DataInvocationGadget : public WarningGadget {
   }
 
   static Matcher matcher() {
-    Matcher callExpr = cxxMemberCallExpr(
-        callee(cxxMethodDecl(hasName("data"), ofClass(hasName("std::span")))));
+
+    Matcher callExpr = cxxMemberCallExpr(callee(
+        cxxMethodDecl(hasName("data"),
+                      ofClass(anyOf(hasName("std::span"), hasName("std::array"),
+                                    hasName("std::vector"))))));
     return stmt(
         explicitCastExpr(anyOf(has(callExpr), has(parenExpr(has(callExpr)))))
             .bind(OpTag));

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -2279,9 +2279,18 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
         QualType srcType = ECE->getSubExpr()->getType();
         const uint64_t sSize =
             Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
+
         if (sSize >= dSize)
           return;
 
+        if (const auto *CE = dyn_cast<CXXMemberCallExpr>(
+                ECE->getSubExpr()->IgnoreParens())) {
+          D = CE->getMethodDecl();
+        }
+
+        if (!D)
+          return;
+
         MsgParam = 4;
       }
       Loc = Operation->getBeginLoc();

clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp

@@ -32,38 +32,68 @@ void foo(int *p){}
 namespace std{
   template <typename T> class span {
 
-  T *elements;
+   T *elements;
 
-  span(T *, unsigned){}
+   span(T *, unsigned){}
 
-  public:
+   public:
 
-  constexpr span<T> subspan(size_t offset, size_t count) const {
-    return span<T> (elements+offset, count); // expected-warning{{unsafe pointer arithmetic}}
-  }
+   constexpr span<T> subspan(size_t offset, size_t count) const {
+     return span<T> (elements+offset, count); // expected-warning{{unsafe pointer arithmetic}}
+   }
 
-  constexpr T* data() const noexcept {
-    return elements;
-  }
+   constexpr T* data() const noexcept {
+     return elements;
+   }
+
+   constexpr T* hello() const noexcept {
+     return elements;
+   }
+ };
+
+ template <typename T> class vector {
+
+   T *elements;
+
+   public:
+
+   vector(size_t n) {
+     elements = new T[n];
+   }
+
+   constexpr T* data() const noexcept {
+      return elements;
+   }
+
+   ~vector() {
+     delete[] elements;
+   }
+ };
+
+ template <class T, size_t N>
+ class array {
+   T elements[N];
+
+   public:
+
+   constexpr const T* data() const noexcept {
+      return elements;
+   }
+
+ };
 
- 
-  constexpr T* hello() const noexcept {
-   return elements;
-  }
-};
- 
  template <typename T> class span_duplicate {
-  span_duplicate(T *, unsigned){}
+   span_duplicate(T *, unsigned){}
 
-  T array[10];
+   T array[10];
 
-  public:
+   public:
 
-  T* data() {
-    return array;
-  }
+   T* data() {
+     return array;
+   }
 
-};
+ };
 }
 
 using namespace std;
@@ -89,21 +119,28 @@ void cast_without_data(int *ptr) {
  float *p = (float*) ptr;
 }
 
-void warned_patterns(std::span<int> span_ptr, std::span<Base> base_span, span<int> span_without_qual) {
-    A *a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of span::data}}
-    a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of span::data}}
+void warned_patterns_span(std::span<int> span_ptr, std::span<Base> base_span, span<int> span_without_qual) {
+    A *a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of 'data'}}
+    a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of 'data'}}
 
-    a1 = (A*)(span_ptr.data()); // expected-warning{{unsafe invocation of span::data}}
-    A *a2 = (A*) (span_without_qual.data()); // expected-warning{{unsafe invocation of span::data}}
+    a1 = (A*)(span_ptr.data()); // expected-warning{{unsafe invocation of 'data'}}
+    A *a2 = (A*) (span_without_qual.data()); // expected-warning{{unsafe invocation of 'data'}}
 
-    a2 = (A*) span_without_qual.data(); // expected-warning{{unsafe invocation of span::data}}
+    a2 = (A*) span_without_qual.data(); // expected-warning{{unsafe invocation of 'data'}}
 
      // TODO:: Should we warn when we cast from base to derived type?
-     Derived *b = dynamic_cast<Derived*> (base_span.data());// expected-warning{{unsafe invocation of span::data}}
+     Derived *b = dynamic_cast<Derived*> (base_span.data());// expected-warning{{unsafe invocation of 'data'}}
 
     // TODO:: This pattern is safe. We can add special handling for it, if we decide this
     // is the recommended fixit for the unsafe invocations.
-    A *a3 = (A*)span_ptr.subspan(0, sizeof(A)).data(); // expected-warning{{unsafe invocation of span::data}}
+    A *a3 = (A*)span_ptr.subspan(0, sizeof(A)).data(); // expected-warning{{unsafe invocation of 'data'}}
+}
+
+void warned_patterns_array(std::array<int, 5> array_ptr, std::array<Base, 10> base_span, span<int> span_without_qual) {
+    const A *a1 = (A*)array_ptr.data(); // expected-warning{{unsafe invocation of 'data'}}
+    a1 = (A*)array_ptr.data(); // expected-warning{{unsafe invocation of 'data'}}
+
+    a1 = (A*)(array_ptr.data()); // expected-warning{{unsafe invocation of 'data'}}
 }
 
 void not_warned_patterns(std::span<A> span_ptr, std::span<Base> base_span) {