Introduce fxcrt::reinterpret_span<>()
Convert between different span types performing automatic size
adjustment. Try it in PDFium before arguing to inflict it elsewhere.
-- Introduce some bounds checks while at it.
Change-Id: I6ff7b92238b89eb3047c5f9d2cc2dd9cd7e33bf4
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/114951
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/edit/cpdf_stringarchivestream.cpp b/core/fpdfapi/edit/cpdf_stringarchivestream.cpp
index 48033b0..41cd8cd 100644
--- a/core/fpdfapi/edit/cpdf_stringarchivestream.cpp
+++ b/core/fpdfapi/edit/cpdf_stringarchivestream.cpp
@@ -6,6 +6,7 @@
#include <sstream>
+#include "core/fxcrt/span_util.h"
#include "third_party/base/notreached.h"
CPDF_StringArchiveStream::CPDF_StringArchiveStream(fxcrt::ostringstream* stream)
@@ -18,6 +19,7 @@
}
bool CPDF_StringArchiveStream::WriteBlock(pdfium::span<const uint8_t> buffer) {
- stream_->write(reinterpret_cast<const char*>(buffer.data()), buffer.size());
+ auto chars = fxcrt::reinterpret_span<const char>(buffer);
+ stream_->write(chars.data(), chars.size());
return true;
}
diff --git a/core/fpdfapi/parser/fpdf_parser_utility.cpp b/core/fpdfapi/parser/fpdf_parser_utility.cpp
index f2e3e36..e31131e 100644
--- a/core/fpdfapi/parser/fpdf_parser_utility.cpp
+++ b/core/fpdfapi/parser/fpdf_parser_utility.cpp
@@ -20,6 +20,7 @@
#include "core/fpdfapi/parser/fpdf_parser_decode.h"
#include "core/fxcrt/fx_extension.h"
#include "core/fxcrt/fx_stream.h"
+#include "core/fxcrt/span_util.h"
#include "third_party/base/check.h"
// Indexed by 8-bit character code, contains either:
@@ -247,8 +248,8 @@
buf << p->GetDict().Get() << "stream\r\n";
auto pAcc = pdfium::MakeRetain<CPDF_StreamAcc>(std::move(p));
pAcc->LoadAllDataRaw();
- pdfium::span<const uint8_t> span = pAcc->GetSpan();
- buf.write(reinterpret_cast<const char*>(span.data()), span.size());
+ auto span = fxcrt::reinterpret_span<const char>(pAcc->GetSpan());
+ buf.write(span.data(), span.size());
buf << "\r\nendstream";
break;
}
diff --git a/core/fpdfapi/render/cpdf_rendershading.cpp b/core/fpdfapi/render/cpdf_rendershading.cpp
index 4991d76..470a3db 100644
--- a/core/fpdfapi/render/cpdf_rendershading.cpp
+++ b/core/fpdfapi/render/cpdf_rendershading.cpp
@@ -134,7 +134,8 @@
CFX_Matrix matrix = mtObject2Bitmap.GetInverse();
for (int row = 0; row < height; row++) {
uint32_t* dib_buf =
- reinterpret_cast<uint32_t*>(pBitmap->GetWritableScanline(row).data());
+ fxcrt::reinterpret_span<uint32_t>(pBitmap->GetWritableScanline(row))
+ .data();
for (int column = 0; column < width; column++) {
CFX_PointF pos = matrix.Transform(
CFX_PointF(static_cast<float>(column), static_cast<float>(row)));
@@ -207,7 +208,8 @@
CFX_Matrix matrix = mtObject2Bitmap.GetInverse();
for (int row = 0; row < height; row++) {
uint32_t* dib_buf =
- reinterpret_cast<uint32_t*>(pBitmap->GetWritableScanline(row).data());
+ fxcrt::reinterpret_span<uint32_t>(pBitmap->GetWritableScanline(row))
+ .data();
for (int column = 0; column < width; column++) {
CFX_PointF pos = matrix.Transform(
CFX_PointF(static_cast<float>(column), static_cast<float>(row)));
@@ -288,7 +290,8 @@
std::vector<float> result_array(total_results);
for (int row = 0; row < height; ++row) {
uint32_t* dib_buf =
- reinterpret_cast<uint32_t*>(pBitmap->GetWritableScanline(row).data());
+ fxcrt::reinterpret_span<uint32_t>(pBitmap->GetWritableScanline(row))
+ .data();
for (int column = 0; column < width; column++) {
CFX_PointF pos = matrix.Transform(
CFX_PointF(static_cast<float>(column), static_cast<float>(row)));
diff --git a/core/fpdfapi/render/cpdf_rendertiling.cpp b/core/fpdfapi/render/cpdf_rendertiling.cpp
index ea83a3f..caab8f1 100644
--- a/core/fpdfapi/render/cpdf_rendertiling.cpp
+++ b/core/fpdfapi/render/cpdf_rendertiling.cpp
@@ -18,6 +18,7 @@
#include "core/fpdfapi/render/cpdf_renderoptions.h"
#include "core/fpdfapi/render/cpdf_renderstatus.h"
#include "core/fxcrt/fx_safe_types.h"
+#include "core/fxcrt/span_util.h"
#include "core/fxge/cfx_defaultrenderdevice.h"
#include "core/fxge/dib/cfx_dibitmap.h"
@@ -231,8 +232,8 @@
uint32_t* dest_buf = reinterpret_cast<uint32_t*>(
pScreen->GetWritableScanline(start_y).subspan(start_x * 4).data());
if (pPattern->colored()) {
- const auto* src_buf32 =
- reinterpret_cast<const uint32_t*>(src_buf.data());
+ const uint32_t* src_buf32 =
+ fxcrt::reinterpret_span<const uint32_t>(src_buf).data();
*dest_buf = *src_buf32;
} else {
*dest_buf = (*(src_buf.data()) << 24) | (fill_argb & 0xffffff);
diff --git a/core/fxcodec/fax/faxmodule.cpp b/core/fxcodec/fax/faxmodule.cpp
index df97129..60a7786 100644
--- a/core/fxcodec/fax/faxmodule.cpp
+++ b/core/fxcodec/fax/faxmodule.cpp
@@ -19,6 +19,7 @@
#include "core/fxcrt/data_vector.h"
#include "core/fxcrt/fx_2d_size.h"
#include "core/fxcrt/fx_memory.h"
+#include "core/fxcrt/span_util.h"
#include "core/fxge/calculate_pitch.h"
#include "third_party/base/check.h"
#include "third_party/base/check_op.h"
@@ -26,7 +27,6 @@
#include "third_party/base/numerics/safe_conversions.h"
#if BUILDFLAG(IS_WIN)
-#include "core/fxcrt/span_util.h"
#include "core/fxge/dib/cfx_dibbase.h"
#endif
@@ -584,11 +584,11 @@
}
void FaxDecoder::InvertBuffer() {
- DCHECK_EQ(m_Pitch, m_ScanlineBuf.size());
- DCHECK_EQ(m_Pitch % 4, 0u);
- uint32_t* data = reinterpret_cast<uint32_t*>(m_ScanlineBuf.data());
- for (size_t i = 0; i < m_ScanlineBuf.size() / 4; ++i)
- data[i] = ~data[i];
+ auto byte_span = pdfium::make_span(m_ScanlineBuf);
+ auto data = fxcrt::reinterpret_span<uint32_t>(byte_span);
+ for (auto& datum : data) {
+ datum = ~datum;
+ }
}
} // namespace
diff --git a/core/fxcodec/gif/cfx_gifcontext.cpp b/core/fxcodec/gif/cfx_gifcontext.cpp
index 9811759..c094f84 100644
--- a/core/fxcodec/gif/cfx_gifcontext.cpp
+++ b/core/fxcodec/gif/cfx_gifcontext.cpp
@@ -369,10 +369,10 @@
return GifDecoder::Status::kError;
bc_index_ = lsd.bc_index;
- uint32_t palette_size = palette_count * sizeof(CFX_GifPalette);
std::vector<CFX_GifPalette> palette(palette_count);
- if (!ReadAllOrNone(reinterpret_cast<uint8_t*>(palette.data()),
- palette_size)) {
+ auto bytes = pdfium::as_writable_bytes(pdfium::make_span(palette));
+ if (!ReadAllOrNone(bytes.data(),
+ pdfium::base::checked_cast<uint32_t>(bytes.size()))) {
// Roll back the read for the LSD
input_buffer_->Seek(read_marker);
return GifDecoder::Status::kUnfinished;
@@ -476,8 +476,9 @@
gif_image->local_palette_exp = gif_img_info_lf->pal_bits;
uint32_t loc_pal_count = unsigned(2 << gif_img_info_lf->pal_bits);
std::vector<CFX_GifPalette> loc_pal(loc_pal_count);
- if (!ReadAllOrNone(reinterpret_cast<uint8_t*>(loc_pal.data()),
- loc_pal_count * sizeof(CFX_GifPalette))) {
+ auto bytes = pdfium::as_writable_bytes(pdfium::make_span(loc_pal));
+ if (!ReadAllOrNone(bytes.data(),
+ pdfium::base::checked_cast<uint32_t>(bytes.size()))) {
input_buffer_->Seek(read_marker);
return GifDecoder::Status::kUnfinished;
}
diff --git a/core/fxcrt/span_util.h b/core/fxcrt/span_util.h
index 2a34980..3b29f5c 100644
--- a/core/fxcrt/span_util.h
+++ b/core/fxcrt/span_util.h
@@ -5,6 +5,8 @@
#ifndef CORE_FXCRT_SPAN_UTIL_H_
#define CORE_FXCRT_SPAN_UTIL_H_
+#include <stdint.h>
+
#include "core/fxcrt/fx_memcpy_wrappers.h"
#include "third_party/base/check_op.h"
#include "third_party/base/containers/span.h"
@@ -41,6 +43,16 @@
FXSYS_memset(dst.data(), 0, dst.size_bytes());
}
+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);
+ CHECK_EQ(reinterpret_cast<uintptr_t>(s.data()) % alignof(T), 0u);
+ return {reinterpret_cast<T*>(s.data()), s.size_bytes() / sizeof(T)};
+}
+
} // namespace fxcrt
#endif // CORE_FXCRT_SPAN_UTIL_H_
diff --git a/core/fxcrt/span_util_unittest.cpp b/core/fxcrt/span_util_unittest.cpp
index 5718ed8..577e2f7 100644
--- a/core/fxcrt/span_util_unittest.cpp
+++ b/core/fxcrt/span_util_unittest.cpp
@@ -89,3 +89,33 @@
span = pdfium::make_span(src);
EXPECT_EQ(span.size(), 2u);
}
+
+TEST(ReinterpretSpan, Empty) {
+ pdfium::span<uint8_t> empty;
+ pdfium::span<uint32_t> converted = fxcrt::reinterpret_span<uint32_t>(empty);
+ EXPECT_EQ(converted.data(), nullptr);
+ EXPECT_EQ(converted.size(), 0u);
+}
+
+TEST(ReinterpretSpan, LegalConversions) {
+ uint8_t aaaabbbb[8] = {0x61, 0x61, 0x61, 0x61, 0x62, 0x62, 0x62, 0x62};
+ pdfium::span<uint8_t> original = pdfium::make_span(aaaabbbb);
+ pdfium::span<uint32_t> converted =
+ fxcrt::reinterpret_span<uint32_t>(original);
+ ASSERT_NE(converted.data(), nullptr);
+ ASSERT_EQ(converted.size(), 2u);
+ EXPECT_EQ(converted[0], 0x61616161u);
+ EXPECT_EQ(converted[1], 0x62626262u);
+}
+
+TEST(ReinterpretSpan, BadLength) {
+ uint8_t ab[2] = {0x61, 0x62};
+ EXPECT_DEATH(fxcrt::reinterpret_span<uint32_t>(pdfium::make_span(ab)), "");
+}
+
+TEST(ReinterpretSpan, BadAlignment) {
+ uint8_t abcabc[6] = {0x61, 0x62, 0x63, 0x61, 0x62, 0x63};
+ EXPECT_DEATH(fxcrt::reinterpret_span<uint32_t>(
+ pdfium::make_span(abcabc).subspan(1, 4)),
+ "");
+}
diff --git a/core/fxge/dib/cfx_dibbase.cpp b/core/fxge/dib/cfx_dibbase.cpp
index f80c19e..cd00ede 100644
--- a/core/fxge/dib/cfx_dibbase.cpp
+++ b/core/fxge/dib/cfx_dibbase.cpp
@@ -659,13 +659,15 @@
int right_shift = 32 - left_shift;
int dword_count = pNewBitmap->m_Pitch / 4;
for (int row = rect.top; row < rect.bottom; ++row) {
+ auto src_span = fxcrt::reinterpret_span<const uint32_t>(GetScanline(row));
+ auto dst_span = fxcrt::reinterpret_span<uint32_t>(
+ pNewBitmap->GetWritableScanline(row - rect.top));
+ // Bounds check for free with first/subspan.
const uint32_t* src_scan =
- reinterpret_cast<const uint32_t*>(GetScanline(row).data()) +
- rect.left / 32;
- uint32_t* dest_scan = reinterpret_cast<uint32_t*>(
- pNewBitmap->GetWritableScanline(row - rect.top).data());
+ src_span.subspan(rect.left / 32, dword_count + 1).data();
+ uint32_t* dst_scan = dst_span.first(dword_count).data();
for (int i = 0; i < dword_count; ++i) {
- dest_scan[i] =
+ dst_scan[i] =
(src_scan[i] << left_shift) | (src_scan[i + 1] >> right_shift);
}
}
@@ -1011,8 +1013,9 @@
uint8_t* dest_scan = dest_span.subspan(dest_offset).data();
if (nBytes == 4) {
const uint32_t* src_scan =
- reinterpret_cast<const uint32_t*>(GetScanline(row).data()) +
- col_start;
+ fxcrt::reinterpret_span<const uint32_t>(GetScanline(row))
+ .subspan(col_start)
+ .data();
for (int col = col_start; col < col_end; ++col) {
uint32_t* dest_scan32 = reinterpret_cast<uint32_t*>(dest_scan);
*dest_scan32 = *src_scan++;
diff --git a/core/fxge/win32/cfx_psrenderer.cpp b/core/fxge/win32/cfx_psrenderer.cpp
index ff3bd9f..cb9e069 100644
--- a/core/fxge/win32/cfx_psrenderer.cpp
+++ b/core/fxge/win32/cfx_psrenderer.cpp
@@ -904,7 +904,8 @@
DataVector<uint8_t> encoded_data = m_pEncoderIface->pA85EncodeFunc(data);
pdfium::span<const uint8_t> result =
encoded_data.empty() ? data : encoded_data;
- m_Output.write(reinterpret_cast<const char*>(result.data()), result.size());
+ auto chars = fxcrt::reinterpret_span<const char>(result);
+ m_Output.write(chars.data(), chars.size());
}
void CFX_PSRenderer::WriteStream(fxcrt::ostringstream& stream) {
diff --git a/testing/string_write_stream.cpp b/testing/string_write_stream.cpp
index 1a99d9a..feb11f9 100644
--- a/testing/string_write_stream.cpp
+++ b/testing/string_write_stream.cpp
@@ -5,6 +5,7 @@
#include "testing/string_write_stream.h"
#include "core/fxcrt/bytestring.h"
+#include "core/fxcrt/span_util.h"
#include "core/fxcrt/widestring.h"
StringWriteStream::StringWriteStream() = default;
@@ -12,6 +13,7 @@
StringWriteStream::~StringWriteStream() = default;
bool StringWriteStream::WriteBlock(pdfium::span<const uint8_t> buffer) {
- stream_.write(reinterpret_cast<const char*>(buffer.data()), buffer.size());
+ auto chars = fxcrt::reinterpret_span<const char>(buffer);
+ stream_.write(chars.data(), chars.size());
return true;
}
diff --git a/xfa/fgas/graphics/cfgas_gegraphics.cpp b/xfa/fgas/graphics/cfgas_gegraphics.cpp
index 5b5da61..082b75a 100644
--- a/xfa/fgas/graphics/cfgas_gegraphics.cpp
+++ b/xfa/fgas/graphics/cfgas_gegraphics.cpp
@@ -299,7 +299,8 @@
float axis_len_square = (x_span * x_span) + (y_span * y_span);
for (int32_t row = 0; row < height; row++) {
uint32_t* dib_buf =
- reinterpret_cast<uint32_t*>(bmp->GetWritableScanline(row).data());
+ fxcrt::reinterpret_span<uint32_t>(bmp->GetWritableScanline(row))
+ .data();
for (int32_t column = 0; column < width; column++) {
float scale = 0.0f;
if (axis_len_square) {
@@ -333,7 +334,8 @@
((start_r - end_r) * (start_r - end_r));
for (int32_t row = 0; row < height; row++) {
uint32_t* dib_buf =
- reinterpret_cast<uint32_t*>(bmp->GetWritableScanline(row).data());
+ fxcrt::reinterpret_span<uint32_t>(bmp->GetWritableScanline(row))
+ .data();
for (int32_t column = 0; column < width; column++) {
float x = (float)(column);
float y = (float)(row);
