commit 8331612b9e125c331009a3556b8dd6b238d71e6e
author Lei Zhang <thestig@chromium.org> Mon Dec 02 23:49:48 2024 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Dec 02 23:49:48 2024 +0000
tree 258751dab002d736b9934b12592d46f410087a13
parent 70cd51bf6412d0bfb1ca2437c97515e8b3eb8148

Add more UNSAFE_BUFFERS() annotations

Add UNSAFE_BUFFERS() to span, ByteString and WideString ctors. An
upcoming Clang update will complain these are unsafe.

Change-Id: Ibd2c9d27553a003a143f8e79f651478bc23f1de7
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/126890
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>

core/fxcrt/bytestring.cpp

diff --git a/core/fxcrt/bytestring.cpp b/core/fxcrt/bytestring.cpp
index 49fc738..7db94bc 100644
--- a/core/fxcrt/bytestring.cpp
+++ b/core/fxcrt/bytestring.cpp
@@ -93,7 +93,8 @@
 }
 
 ByteString::ByteString(const uint8_t* pStr, size_t nLen)
-    : ByteString(reinterpret_cast<const char*>(pStr), nLen) {}
+    // SAFETY: caller ensures `pStr` points to at least `nLen` chars.
+    : UNSAFE_BUFFERS(ByteString(reinterpret_cast<const char*>(pStr), nLen)) {}
 
 ByteString::ByteString(char ch) {
   m_pData = StringData::Create(1);
@@ -101,7 +102,8 @@
 }
 
 ByteString::ByteString(const char* ptr)
-    : ByteString(ptr, ptr ? strlen(ptr) : 0) {}
+    // SAFETY: caller ensures `ptr` is NUL-terminated.
+    : UNSAFE_BUFFERS(ByteString(ptr, ptr ? strlen(ptr) : 0)) {}
 
 ByteString::ByteString(ByteStringView bstrc) {
   if (!bstrc.IsEmpty()) {

core/fxcrt/span.h

diff --git a/core/fxcrt/span.h b/core/fxcrt/span.h
index 6b5c112..672e196 100644
--- a/core/fxcrt/span.h
+++ b/core/fxcrt/span.h
@@ -208,18 +208,23 @@
 
   // TODO(dcheng): Implement construction from a |begin| and |end| pointer.
   template <size_t N>
-  constexpr span(T (&array)[N]) noexcept : span(array, N) {
+  constexpr span(T (&array)[N]) noexcept
+      // SAFETY: The type signature guarantees `array` contains `N` elements.
+      : UNSAFE_BUFFERS(span(array, N)) {
     static_assert(Extent == dynamic_extent || Extent == N);
   }
 
   template <size_t N>
-  constexpr span(std::array<T, N>& array) noexcept : span(array.data(), N) {
+  constexpr span(std::array<T, N>& array) noexcept
+      // SAFETY: The type signature guarantees `array` contains `N` elements.
+      : UNSAFE_BUFFERS(span(array.data(), N)) {
     static_assert(Extent == dynamic_extent || Extent == N);
   }
 
   template <size_t N>
   constexpr span(const std::array<std::remove_cv_t<T>, N>& array) noexcept
-      : span(array.data(), N) {
+      // SAFETY: The type signature guarantees `array` contains `N` elements.
+      : UNSAFE_BUFFERS(span(array.data(), N)) {
     static_assert(Extent == dynamic_extent || Extent == N);
   }
 
@@ -231,18 +236,26 @@
   template <typename Container,
             typename = internal::EnableIfSpanCompatibleContainer<Container, T>>
   constexpr span(Container& container)
-      : span(container.size() ? container.data() : nullptr, container.size()) {}
+      // SAFETY: `size()` is the number of elements that can be safely accessed
+      // at `data()`, if non-empty.
+      : UNSAFE_BUFFERS(span(container.size() ? container.data() : nullptr,
+                            container.size())) {}
 #else
   template <typename Container,
             typename = internal::EnableIfSpanCompatibleContainer<Container, T>>
   constexpr span(Container& container)
-      : span(container.data(), container.size()) {}
+      // SAFETY: `size()` is the number of elements that can be safely accessed
+      // at `data()`.
+      : UNSAFE_BUFFERS(span(container.data(), container.size())) {}
 #endif
 
   template <
       typename Container,
       typename = internal::EnableIfConstSpanCompatibleContainer<Container, T>>
-  span(const Container& container) : span(container.data(), container.size()) {}
+  span(const Container& container)
+      // SAFETY: `size()` is exactly the number of elements in the initializer
+      // list, so accessing that many will be safe.
+      : UNSAFE_BUFFERS(span(container.data(), container.size())) {}
 
   constexpr span(const span& other) noexcept = default;
 
@@ -253,7 +266,9 @@
             typename R,
             typename = internal::EnableIfLegalSpanConversion<U, T>>
   constexpr span(const span<U, M, R>& other)
-      : span(other.data(), other.size()) {}
+      // SAFETY: `size()` is the number of elements that can be safely accessed
+      // at `data()`.
+      : UNSAFE_BUFFERS(span(other.data(), other.size())) {}
 
   span& operator=(const span& other) noexcept = default;
   span& operator=(span&& other) noexcept = default;

core/fxcrt/widestring.cpp

diff --git a/core/fxcrt/widestring.cpp b/core/fxcrt/widestring.cpp
index 1a5081e..0909cc6 100644
--- a/core/fxcrt/widestring.cpp
+++ b/core/fxcrt/widestring.cpp
@@ -410,7 +410,8 @@
 }
 
 WideString::WideString(const wchar_t* ptr)
-    : WideString(ptr, ptr ? wcslen(ptr) : 0) {}
+    // SAFETY: caller ensures `ptr` is NUL-terminated.
+    : UNSAFE_BUFFERS(WideString(ptr, ptr ? wcslen(ptr) : 0)) {}
 
 WideString::WideString(WideStringView stringSrc) {
   if (!stringSrc.IsEmpty()) {