Remove or reduce scope of UNSAFE_TODOs in dib code
Mostly reduce the scope. Remove a couple of simple ones and replace them
with fxcrt::Zip() / using std::array. Unlike a C-array, std::array can
do bounds checks when hardened.
Bug: 42271176
Change-Id: I8bd5b7ffc26a863f1fa539f0b61892e2fb55e456
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/121558
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Tom Sepez <tsepez@google.com>
diff --git a/core/fxge/dib/cfx_dibbase.cpp b/core/fxge/dib/cfx_dibbase.cpp
index 325ec0d..5f7b423 100644
--- a/core/fxge/dib/cfx_dibbase.cpp
+++ b/core/fxge/dib/cfx_dibbase.cpp
@@ -7,6 +7,7 @@
#include "core/fxge/dib/cfx_dibbase.h"
#include <algorithm>
+#include <array>
#include <utility>
#include <vector>
@@ -22,6 +23,7 @@
#include "core/fxcrt/span.h"
#include "core/fxcrt/span_util.h"
#include "core/fxcrt/stl_util.h"
+#include "core/fxcrt/zip.h"
#include "core/fxge/calculate_pitch.h"
#include "core/fxge/cfx_cliprgn.h"
#include "core/fxge/dib/cfx_bitmapstorer.h"
@@ -63,10 +65,10 @@
int bpp = pBitmap->GetBPP() / 8;
int width = pBitmap->GetWidth();
int height = pBitmap->GetHeight();
- UNSAFE_TODO({
- for (int row = 0; row < height; ++row) {
- pdfium::span<const uint8_t> scan_line = pBitmap->GetScanline(row);
- for (int col = 0; col < width; ++col) {
+ for (int row = 0; row < height; ++row) {
+ pdfium::span<const uint8_t> scan_line = pBitmap->GetScanline(row);
+ for (int col = 0; col < width; ++col) {
+ UNSAFE_TODO({
const uint8_t* src_port =
scan_line.subspan(Fx2DSizeOrDie(col, bpp)).data();
uint32_t b = src_port[0] & 0xf0;
@@ -74,22 +76,24 @@
uint32_t r = src_port[2] & 0xf0;
uint32_t index = (r << 4) + g + (b >> 4);
++m_Luts[index].first;
- }
+ });
}
- // Move non-zeros to the front and count them
- for (uint32_t row = 0; row < m_Luts.size(); ++row) {
+ }
+ // Move non-zeros to the front and count them
+ for (uint32_t row = 0; row < m_Luts.size(); ++row) {
+ UNSAFE_TODO({
if (m_Luts[row].first != 0) {
m_Luts[m_lut].first = m_Luts[row].first;
m_Luts[m_lut].second = row;
++m_lut;
}
- }
- pdfium::span<LutsData> lut_span = pdfium::make_span(m_Luts).first(m_lut);
- std::sort(lut_span.begin(), lut_span.end(),
- [](const LutsData& arg1, const LutsData& arg2) {
- return arg1.first < arg2.first;
- });
- });
+ });
+ }
+ pdfium::span<LutsData> lut_span = pdfium::make_span(m_Luts).first(m_lut);
+ std::sort(lut_span.begin(), lut_span.end(),
+ [](const LutsData& arg1, const LutsData& arg2) {
+ return arg1.first < arg2.first;
+ });
ObtainPalette();
}
@@ -213,26 +217,22 @@
int dest_pitch,
int width,
int height,
- const RetainPtr<const CFX_DIBBase>& pSrcBitmap,
+ const RetainPtr<const CFX_DIBBase>& src,
int src_left,
int src_top) {
- pdfium::span<const uint32_t> src_palette = pSrcBitmap->GetPaletteSpan();
- uint8_t gray[256];
- UNSAFE_TODO({
- for (size_t i = 0; i < std::size(gray); ++i) {
- gray[i] = FXRGB2GRAY(FXARGB_R(src_palette[i]), FXARGB_G(src_palette[i]),
- FXARGB_B(src_palette[i]));
+ pdfium::span<const uint32_t> src_palette = src->GetPaletteSpan();
+ CHECK_EQ(256u, src_palette.size());
+ std::array<uint8_t, 256> gray;
+ for (auto [input, output] : fxcrt::Zip(src_palette, gray)) {
+ output = FXRGB2GRAY(FXARGB_R(input), FXARGB_G(input), FXARGB_B(input));
+ }
+ for (int row = 0; row < height; ++row) {
+ auto dest_scan = dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch));
+ auto src_scan = src->GetScanline(src_top + row).subspan(src_left, width);
+ for (auto [input, output] : fxcrt::Zip(src_scan, dest_scan)) {
+ output = gray[input];
}
- for (int row = 0; row < height; ++row) {
- uint8_t* dest_scan =
- dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch)).data();
- const uint8_t* src_scan =
- pSrcBitmap->GetScanline(src_top + row).subspan(src_left).data();
- for (int col = 0; col < width; ++col) {
- *dest_scan++ = gray[*src_scan++];
- }
- }
- });
+ }
}
void ConvertBuffer_Rgb2Gray(pdfium::span<uint8_t> dest_buf,
@@ -438,6 +438,7 @@
int src_left,
int src_top) {
pdfium::span<const uint32_t> src_palette = pSrcBitmap->GetPaletteSpan();
+ CHECK_EQ(256u, src_palette.size());
uint8_t dst_palette[768];
UNSAFE_TODO({
for (int i = 0; i < 256; ++i) {
@@ -445,19 +446,21 @@
dst_palette[3 * i + 1] = FXARGB_G(src_palette[i]);
dst_palette[3 * i + 2] = FXARGB_R(src_palette[i]);
}
- int comps = GetCompsFromFormat(dest_format);
- for (int row = 0; row < height; ++row) {
- uint8_t* dest_scan =
- dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch)).data();
- const uint8_t* src_scan =
- pSrcBitmap->GetScanline(src_top + row).subspan(src_left).data();
- for (int col = 0; col < width; ++col) {
+ });
+ const int comps = GetCompsFromFormat(dest_format);
+ for (int row = 0; row < height; ++row) {
+ uint8_t* dest_scan =
+ dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch)).data();
+ const uint8_t* src_scan =
+ pSrcBitmap->GetScanline(src_top + row).subspan(src_left).data();
+ for (int col = 0; col < width; ++col) {
+ UNSAFE_TODO({
uint8_t* src_pixel = dst_palette + 3 * (*src_scan++);
FXSYS_memcpy(dest_scan, src_pixel, 3);
dest_scan += comps;
- }
+ });
}
- });
+ }
}
void ConvertBuffer_24bppRgb2Rgb24(
diff --git a/core/fxge/dib/cfx_dibitmap.cpp b/core/fxge/dib/cfx_dibitmap.cpp
index e687048..6d4be64 100644
--- a/core/fxge/dib/cfx_dibitmap.cpp
+++ b/core/fxge/dib/cfx_dibitmap.cpp
@@ -443,9 +443,9 @@
}
return;
}
- UNSAFE_TODO({
- if (forecolor == 0 && backcolor == 0xffffff) {
- for (int row = 0; row < GetHeight(); ++row) {
+ if (forecolor == 0 && backcolor == 0xffffff) {
+ for (int row = 0; row < GetHeight(); ++row) {
+ UNSAFE_TODO({
uint8_t* scanline = m_pBuffer.Get() + row * GetPitch();
int gap = GetBPP() / 8 - 2;
for (int col = 0; col < GetWidth(); ++col) {
@@ -455,10 +455,12 @@
*scanline = gray;
scanline += gap;
}
- }
- return;
+ });
}
- for (int row = 0; row < GetHeight(); ++row) {
+ return;
+ }
+ for (int row = 0; row < GetHeight(); ++row) {
+ UNSAFE_TODO({
uint8_t* scanline = m_pBuffer.Get() + row * GetPitch();
int gap = GetBPP() / 8 - 2;
for (int col = 0; col < GetWidth(); ++col) {
@@ -468,8 +470,8 @@
*scanline = br + (fr - br) * gray / 255;
scanline += gap;
}
- }
- });
+ });
+ }
}
bool CFX_DIBitmap::ConvertColorScale(uint32_t forecolor, uint32_t backcolor) {
@@ -690,8 +692,8 @@
width = rect.Width();
uint32_t dst_color = color;
uint8_t* color_p = reinterpret_cast<uint8_t*>(&dst_color);
- UNSAFE_TODO({
- if (GetBPP() == 8) {
+ if (GetBPP() == 8) {
+ UNSAFE_TODO({
uint8_t gray =
IsMaskFormat()
? 255
@@ -707,9 +709,11 @@
}
}
}
- return true;
- }
- if (GetBPP() == 1) {
+ });
+ return true;
+ }
+ if (GetBPP() == 1) {
+ UNSAFE_TODO({
int left_shift = rect.left % 8;
int right_shift = rect.right % 8;
int new_width = rect.right / 8 - rect.left / 8;
@@ -747,19 +751,21 @@
}
}
}
- return true;
- }
+ });
+ return true;
+ }
- CHECK_GE(GetBPP(), 24);
- color_p[3] = static_cast<uint8_t>(src_alpha);
- int Bpp = GetBPP() / 8;
- const bool bAlpha = IsAlphaFormat();
- if (bAlpha) {
- // Other formats with alpha have already been handled above.
- DCHECK_EQ(GetFormat(), FXDIB_Format::kArgb);
- }
- if (src_alpha == 255) {
- for (int row = rect.top; row < rect.bottom; row++) {
+ CHECK_GE(GetBPP(), 24);
+ UNSAFE_TODO({ color_p[3] = static_cast<uint8_t>(src_alpha); });
+ int Bpp = GetBPP() / 8;
+ const bool bAlpha = IsAlphaFormat();
+ if (bAlpha) {
+ // Other formats with alpha have already been handled above.
+ DCHECK_EQ(GetFormat(), FXDIB_Format::kArgb);
+ }
+ if (src_alpha == 255) {
+ for (int row = rect.top; row < rect.bottom; row++) {
+ UNSAFE_TODO({
uint8_t* dest_scan =
m_pBuffer.Get() + row * GetPitch() + rect.left * Bpp;
if (Bpp == 4) {
@@ -774,10 +780,12 @@
*dest_scan++ = color_p[2];
}
}
- }
- return true;
+ });
}
- for (int row = rect.top; row < rect.bottom; row++) {
+ return true;
+ }
+ for (int row = rect.top; row < rect.bottom; row++) {
+ UNSAFE_TODO({
uint8_t* dest_scan = m_pBuffer.Get() + row * GetPitch() + rect.left * Bpp;
if (bAlpha) {
for (int col = 0; col < width; col++) {
@@ -812,8 +820,8 @@
}
}
}
- }
- });
+ });
+ }
return true;
}