[clang] [-Wunsafe-buffer-usage] Emit a warning if pointer returned by vector::data and array::data is cast to larger type (PR #111910)

via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 10 13:48:42 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Malavika Samak (malavikasamak)

<details>
<summary>Changes</summary>

Emit a warning when the raw pointer retrieved from std::vector and std::array instances are cast to a larger type. Such a cast followed by a field dereference to the resulting pointer could cause an OOB access. This is similar to the existing span::data warning.

(rdar://136704278)

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


4 Files Affected:

- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+1-1) 
- (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+5-2) 
- (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+9) 
- (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp (+67-30) 


``````````diff
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f4a2d4a3f0656a..301a0d46d88390 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12497,7 +12497,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<
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 97f1c4f16b8f4c..5e0ec9ecc92ea4 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1499,8 +1499,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));
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 6496a33b8f5a50..c76733e9a774b6 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/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();
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
index 08707d7ff545d8..0228e42652bd95 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
+++ b/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) {

``````````

</details>


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


More information about the cfe-commits mailing list