Introduce more helpful FX_*_STRUCT classes.upstream-main

Avoid buf[3]-style indexing and unsafe buffer warning suppression as
a result in CPDF_DeviceCS.

-- avoid some else-clauses while at it.
-- provide a truncating_reinterpret_span<>() function as a fudge
   for callers that have fixed-size buffers which may not be
   multiples of the cast-to type's size.
-- Fix CPDF_ColorSpace to know actual number of components it holds
   to avoid length assertion of size 16 modulo 3 (size of RGB) during
   reinterpret_cast<>.

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

4 files changed, 142 insertions, 79 deletions

diff --git a/core/fpdfapi/page/cpdf_devicecs.cpp b/core/fpdfapi/page/cpdf_devicecs.cpp
index 3a59f2dad..73390746e 100644
--- a/core/fpdfapi/page/cpdf_devicecs.cpp
+++ b/core/fpdfapi/page/cpdf_devicecs.cpp
@@ -4,11 +4,6 @@
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
-#if defined(UNSAFE_BUFFERS_BUILD)
-// TODO(crbug.com/pdfium/2154): resolve buffer safety issues.
-#pragma allow_unsafe_buffers
-#endif
-
 #include "core/fpdfapi/page/cpdf_devicecs.h"
 
 #include <algorithm>
@@ -52,31 +47,41 @@ bool CPDF_DeviceCS::GetRGB(pdfium::span<const float> pBuf,
                            float* G,
                            float* B) const {
   switch (GetFamily()) {
-    case Family::kDeviceGray:
-      *R = NormalizeChannel(pBuf[0]);
-      *G = *R;
-      *B = *R;
+    case Family::kDeviceGray: {
+      const float pix = NormalizeChannel(pBuf.front());
+      *R = pix;
+      *G = pix;
+      *B = pix;
       return true;
-    case Family::kDeviceRGB:
-      *R = NormalizeChannel(pBuf[0]);
-      *G = NormalizeChannel(pBuf[1]);
-      *B = NormalizeChannel(pBuf[2]);
+    }
+    case Family::kDeviceRGB: {
+      auto rgb =
+          fxcrt::truncating_reinterpret_span<const FX_RGB_STRUCT<float>>(pBuf);
+      *R = NormalizeChannel(rgb.front().red);
+      *G = NormalizeChannel(rgb.front().green);
+      *B = NormalizeChannel(rgb.front().blue);
       return true;
-    case Family::kDeviceCMYK:
+    }
+    case Family::kDeviceCMYK: {
+      auto cmyk =
+          fxcrt::truncating_reinterpret_span<const FX_CMYK_STRUCT<float>>(pBuf);
       if (IsStdConversionEnabled()) {
-        float k = pBuf[3];
-        *R = 1.0f - std::min(1.0f, pBuf[0] + k);
-        *G = 1.0f - std::min(1.0f, pBuf[1] + k);
-        *B = 1.0f - std::min(1.0f, pBuf[2] + k);
-      } else {
-        FX_RGB_STRUCT<float> rgb = AdobeCMYK_to_sRGB(
-            NormalizeChannel(pBuf[0]), NormalizeChannel(pBuf[1]),
-            NormalizeChannel(pBuf[2]), NormalizeChannel(pBuf[3]));
-        *R = rgb.red;
-        *G = rgb.green;
-        *B = rgb.blue;
+        float k = cmyk.front().key;
+        *R = 1.0f - std::min(1.0f, cmyk.front().cyan + k);
+        *G = 1.0f - std::min(1.0f, cmyk.front().magenta + k);
+        *B = 1.0f - std::min(1.0f, cmyk.front().yellow + k);
+        return true;
       }
+      FX_RGB_STRUCT<float> rgb =
+          AdobeCMYK_to_sRGB(NormalizeChannel(cmyk.front().cyan),
+                            NormalizeChannel(cmyk.front().magenta),
+                            NormalizeChannel(cmyk.front().yellow),
+                            NormalizeChannel(cmyk.front().key));
+      *R = rgb.red;
+      *G = rgb.green;
+      *B = rgb.blue;
       return true;
+    }
     default:
       NOTREACHED_NORETURN();
   }
@@ -88,69 +93,63 @@ void CPDF_DeviceCS::TranslateImageLine(pdfium::span<uint8_t> dest_span,
                                        int image_width,
                                        int image_height,
                                        bool bTransMask) const {
-  uint8_t* pDestBuf = dest_span.data();
-  const uint8_t* pSrcBuf = src_span.data();
+  auto rgb_out =
+      fxcrt::truncating_reinterpret_span<FX_RGB_STRUCT<uint8_t>>(dest_span);
   switch (GetFamily()) {
     case Family::kDeviceGray:
-      CHECK(!bTransMask);  // Only applies to CMYK colorspaces.
-
-      for (int i = 0; i < pixels; i++) {
-        // Compiler can not conclude that src/dest don't overlap, avoid
-        // duplicate loads.
-        const uint8_t pix = pSrcBuf[i];
-        *pDestBuf++ = pix;
-        *pDestBuf++ = pix;
-        *pDestBuf++ = pix;
+      CHECK(!bTransMask);  // bTransMask only allowed for CMYK colorspaces.
+      // Compiler can't conclude src/dest don't overlap, avoid interleaved
+      // loads and stores by not using an auto& reference here.
+      for (const auto pix : src_span.first(pixels)) {
+        rgb_out.front().red = pix;
+        rgb_out.front().green = pix;
+        rgb_out.front().blue = pix;
+        rgb_out = rgb_out.subspan(1);
       }
       break;
     case Family::kDeviceRGB:
-      CHECK(!bTransMask);  // Only applies to CMYK colorspaces.
-
-      fxcodec::ReverseRGB(pDestBuf, pSrcBuf, pixels);
+      CHECK(!bTransMask);  // bTransMask only allowed for CMYK colorspaces.
+      fxcodec::ReverseRGB(dest_span.data(), src_span.data(), pixels);
       break;
-    case Family::kDeviceCMYK:
+    case Family::kDeviceCMYK: {
+      auto cmyk_in =
+          fxcrt::truncating_reinterpret_span<const FX_CMYK_STRUCT<uint8_t>>(
+              src_span);
       if (bTransMask) {
-        for (int i = 0; i < pixels; i++) {
-          // Compiler can't conclude src/dest don't overlap, avoid interleaved
-          // loads and stores.
-          const uint8_t s0 = pSrcBuf[0];
-          const uint8_t s1 = pSrcBuf[1];
-          const uint8_t s2 = pSrcBuf[2];
-          const int k = 255 - pSrcBuf[3];
-          pDestBuf[0] = ((255 - s0) * k) / 255;
-          pDestBuf[1] = ((255 - s1) * k) / 255;
-          pDestBuf[2] = ((255 - s2) * k) / 255;
-          pDestBuf += 3;
-          pSrcBuf += 4;
+        // Compiler can't conclude src/dest don't overlap, avoid interleaved
+        // loads and stores by not using an auto& reference here.
+        for (const auto cmyk : cmyk_in.first(pixels)) {
+          const int k = 255 - cmyk.key;
+          rgb_out.front().red = ((255 - cmyk.cyan) * k) / 255;
+          rgb_out.front().green = ((255 - cmyk.magenta) * k) / 255;
+          rgb_out.front().blue = ((255 - cmyk.yellow) * k) / 255;
+          rgb_out = rgb_out.subspan(1);
         }
-      } else {
-        if (IsStdConversionEnabled()) {
-          for (int i = 0; i < pixels; i++) {
-            // Compiler can't conclude src/dest don't overlap, avoid
-            // interleaved loads and stores.
-            const uint8_t s0 = pSrcBuf[0];
-            const uint8_t s1 = pSrcBuf[1];
-            const uint8_t s2 = pSrcBuf[2];
-            const uint8_t k = pSrcBuf[3];
-            pDestBuf[2] = 255 - std::min(255, s0 + k);
-            pDestBuf[1] = 255 - std::min(255, s1 + k);
-            pDestBuf[0] = 255 - std::min(255, s2 + k);
-            pSrcBuf += 4;
-            pDestBuf += 3;
-          }
-        } else {
-          for (int i = 0; i < pixels; i++) {
-            FX_RGB_STRUCT<uint8_t> rgb = AdobeCMYK_to_sRGB1(
-                pSrcBuf[0], pSrcBuf[1], pSrcBuf[2], pSrcBuf[3]);
-            pDestBuf[0] = rgb.blue;
-            pDestBuf[1] = rgb.green;
-            pDestBuf[2] = rgb.red;
-            pSrcBuf += 4;
-            pDestBuf += 3;
-          }
+        break;
+      }
+      if (IsStdConversionEnabled()) {
+        // Compiler can't conclude src/dest don't overlap, avoid interleaved
+        // loads and stores by not using am auto& reference here,
+        for (const auto cmyk : cmyk_in.first(pixels)) {
+          const uint8_t k = cmyk.key;
+          rgb_out.front().blue = 255 - std::min(255, cmyk.cyan + k);
+          rgb_out.front().green = 255 - std::min(255, cmyk.magenta + k);
+          rgb_out.front().red = 255 - std::min(255, cmyk.yellow + k);
+          rgb_out = rgb_out.subspan(1);
         }
+        break;
+      }
+      for (const auto& cmyk : cmyk_in.first(pixels)) {
+        // TODO(tsepez): maybe this is a FX_BGR_STRUCT in reality?
+        FX_RGB_STRUCT<uint8_t> rgb =
+            AdobeCMYK_to_sRGB1(cmyk.cyan, cmyk.magenta, cmyk.yellow, cmyk.key);
+        rgb_out.front().red = rgb.blue;
+        rgb_out.front().green = rgb.green;
+        rgb_out.front().blue = rgb.red;
+        rgb_out = rgb_out.subspan(1);
       }
       break;
+    }
     default:
       NOTREACHED_NORETURN();
   }
diff --git a/core/fxcrt/span_util.h b/core/fxcrt/span_util.h
index 4dedfe8c9..c6611ad1e 100644
--- a/core/fxcrt/span_util.h
+++ b/core/fxcrt/span_util.h
@@ -174,14 +174,22 @@ template <typename T,
           typename U,
           typename = typename std::enable_if_t<std::is_const_v<T> ||
                                                !std::is_const_v<U>>>
-inline pdfium::span<T> reinterpret_span(pdfium::span<U> s) noexcept {
-  CHECK_EQ(s.size_bytes() % sizeof(T), 0u);
+inline pdfium::span<T> truncating_reinterpret_span(pdfium::span<U> s) noexcept {
   CHECK_EQ(reinterpret_cast<uintptr_t>(s.data()) % alignof(T), 0u);
   // SAFETY: relies on correct conversion of size_bytes() result.
   return UNSAFE_BUFFERS(pdfium::make_span(reinterpret_cast<T*>(s.data()),
                                           s.size_bytes() / sizeof(T)));
 }
 
+template <typename T,
+          typename U,
+          typename = typename std::enable_if_t<std::is_const_v<T> ||
+                                               !std::is_const_v<U>>>
+inline pdfium::span<T> reinterpret_span(pdfium::span<U> s) noexcept {
+  CHECK_EQ(s.size_bytes() % sizeof(T), 0u);
+  return truncating_reinterpret_span<T, U>(s);
+}
+
 }  // namespace fxcrt
 
 #endif  // CORE_FXCRT_SPAN_UTIL_H_
diff --git a/core/fxge/dib/fx_dib.cpp b/core/fxge/dib/fx_dib.cpp
index c8f9833a5..c989b5582 100644
--- a/core/fxge/dib/fx_dib.cpp
+++ b/core/fxge/dib/fx_dib.cpp
@@ -20,6 +20,15 @@ static_assert(sizeof(FX_COLORREF) == sizeof(COLORREF),
               "FX_COLORREF vs. COLORREF mismatch");
 #endif
 
+// Assert that FX_*_STRUCTS are packed.
+static_assert(sizeof(FX_RGB_STRUCT<uint8_t>) == 3u);
+static_assert(sizeof(FX_BGR_STRUCT<uint8_t>) == 3u);
+static_assert(sizeof(FX_ARGB_STRUCT<uint8_t>) == 4u);
+static_assert(sizeof(FX_ABGR_STRUCT<uint8_t>) == 4u);
+static_assert(sizeof(FX_RGBA_STRUCT<uint8_t>) == 4u);
+static_assert(sizeof(FX_BGRA_STRUCT<uint8_t>) == 4u);
+static_assert(sizeof(FX_CMYK_STRUCT<uint8_t>) == 4u);
+
 FXDIB_Format MakeRGBFormat(int bpp) {
   switch (bpp) {
     case 1:
diff --git a/core/fxge/dib/fx_dib.h b/core/fxge/dib/fx_dib.h
index e7b099cb7..901b9046a 100644
--- a/core/fxge/dib/fx_dib.h
+++ b/core/fxge/dib/fx_dib.h
@@ -44,6 +44,53 @@ struct FX_RGB_STRUCT {
   T blue;
 };
 
+template <typename T>
+struct FX_BGR_STRUCT {
+  T blue;
+  T green;
+  T red;
+};
+
+template <typename T>
+struct FX_ARGB_STRUCT {
+  T alpha;
+  T red;
+  T green;
+  T blue;
+};
+
+template <typename T>
+struct FX_ABGR_STRUCT {
+  T alpha;
+  T blue;
+  T green;
+  T red;
+};
+
+template <typename T>
+struct FX_RGBA_STRUCT {
+  T red;
+  T green;
+  T blue;
+  T alpha;
+};
+
+template <typename T>
+struct FX_BGRA_STRUCT {
+  T blue;
+  T green;
+  T red;
+  T alpha;
+};
+
+template <typename T>
+struct FX_CMYK_STRUCT {
+  T cyan;
+  T magenta;
+  T yellow;
+  T key;
+};
+
 struct FXDIB_ResampleOptions {
   FXDIB_ResampleOptions();