Add UNSAFE_BUFFER annotations to fx_dib.h But stop well short of actually enforcing these during any builds. Convert some macros to inline functions to allow propagating the unsafe requirement to callers. -- Fix const issue in skia test hidden by c-style cast in macro. Change-Id: Ic9c4338eba4807cf0c25e2d993b9684617d6eedc Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/116551 Reviewed-by: Thomas Sepez <tsepez@google.com> Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/render/cpdf_rendershading.cpp b/core/fpdfapi/render/cpdf_rendershading.cpp index 5575205..8f4d6cb 100644 --- a/core/fpdfapi/render/cpdf_rendershading.cpp +++ b/core/fpdfapi/render/cpdf_rendershading.cpp
@@ -409,7 +409,7 @@ r_result += r_unit; g_result += g_unit; b_result += b_unit; - FXARGB_SETDIB(dib_buf, ArgbEncode(alpha, static_cast<int>(r_result * 255), + FXARGB_SetDIB(dib_buf, ArgbEncode(alpha, static_cast<int>(r_result * 255), static_cast<int>(g_result * 255), static_cast<int>(b_result * 255))); dib_span = dib_span.subspan(4);
diff --git a/core/fxcodec/progressive_decoder.cpp b/core/fxcodec/progressive_decoder.cpp index 669949e..5e3c695 100644 --- a/core/fxcodec/progressive_decoder.cpp +++ b/core/fxcodec/progressive_decoder.cpp
@@ -430,7 +430,7 @@ } case 12: { for (int col = 0; col < sizeX; col++) { - FXARGB_SETDIB(pScanline, argb); + FXARGB_SetDIB(pScanline, argb); pScanline += 4; } break;
diff --git a/core/fxge/agg/fx_agg_driver.cpp b/core/fxge/agg/fx_agg_driver.cpp index 0be44eb..a380a78 100644 --- a/core/fxge/agg/fx_agg_driver.cpp +++ b/core/fxge/agg/fx_agg_driver.cpp
@@ -99,7 +99,7 @@ for (int col = 0; col < width; col++) { uint8_t back_alpha = dest_scan[3]; if (back_alpha == 0) { - FXARGB_SETRGBORDERDIB(dest_scan, argb); + FXARGB_SetRGBOrderDIB(dest_scan, argb); dest_scan += 4; continue; } @@ -162,7 +162,7 @@ pSrcBitmap->GetScanline(src_top + row).subspan(src_x_offset).data(); if (Bpp == 4) { for (int col = 0; col < width; col++) { - FXARGB_SETRGBORDERDIB(dest_scan, + FXARGB_SetRGBOrderDIB(dest_scan, *reinterpret_cast<const uint32_t*>(src_scan)); dest_scan += 4; src_scan += 4; @@ -212,7 +212,7 @@ const uint8_t* src_scan = pSrcBitmap->GetScanline(src_top + row).subspan(src_x_offset).data(); for (int col = 0; col < width; col++) { - FXARGB_SETDIB(dest_scan, + FXARGB_SetDIB(dest_scan, ArgbEncode(0xff, src_scan[0], src_scan[1], src_scan[2])); dest_scan += 4; src_scan += 3; @@ -230,7 +230,7 @@ const uint8_t* src_scan = pSrcBitmap->GetScanline(src_top + row).subspan(src_x_offset).data(); for (int col = 0; col < width; col++) { - FXARGB_SETDIB(dest_scan, + FXARGB_SetDIB(dest_scan, ArgbEncode(0xff, src_scan[0], src_scan[1], src_scan[2])); src_scan += 4; dest_scan += 4;
diff --git a/core/fxge/cfx_renderdevice.cpp b/core/fxge/cfx_renderdevice.cpp index 4dc06a8..8e96719 100644 --- a/core/fxge/cfx_renderdevice.cpp +++ b/core/fxge/cfx_renderdevice.cpp
@@ -160,7 +160,7 @@ int src_alpha) { uint8_t back_alpha = dest[3]; if (back_alpha == 0) - FXARGB_SETDIB(dest, ArgbEncode(src_alpha, r, g, b)); + FXARGB_SetDIB(dest, ArgbEncode(src_alpha, r, g, b)); else if (src_alpha != 0) ApplyDestAlpha(back_alpha, src_alpha, r, g, b, dest); }
diff --git a/core/fxge/dib/cfx_dibitmap.cpp b/core/fxge/dib/cfx_dibitmap.cpp index ec91d70..aa1996a 100644 --- a/core/fxge/dib/cfx_dibitmap.cpp +++ b/core/fxge/dib/cfx_dibitmap.cpp
@@ -430,9 +430,9 @@ : ArgbEncode(0xff, *pos, *pos, *pos); case FXDIB_Format::kRgb: case FXDIB_Format::kRgb32: - return FXARGB_GETDIB(pos) | 0xff000000; + return FXARGB_GetDIB(pos) | 0xff000000; case FXDIB_Format::kArgb: - return FXARGB_GETDIB(pos); + return FXARGB_GetDIB(pos); default: break; } @@ -794,7 +794,7 @@ for (int col = 0; col < width; col++) { uint8_t back_alpha = dest_scan[3]; if (back_alpha == 0) { - FXARGB_SETDIB(dest_scan, ArgbEncode(src_alpha, color_p[2], color_p[1], + FXARGB_SetDIB(dest_scan, ArgbEncode(src_alpha, color_p[2], color_p[1], color_p[0])); dest_scan += 4; continue;
diff --git a/core/fxge/dib/cfx_dibitmap_unittest.cpp b/core/fxge/dib/cfx_dibitmap_unittest.cpp index 5473ce0..2c7de3e 100644 --- a/core/fxge/dib/cfx_dibitmap_unittest.cpp +++ b/core/fxge/dib/cfx_dibitmap_unittest.cpp
@@ -121,7 +121,7 @@ TEST(CFX_DIBitmap, UnPreMultiply_FromCleared) { auto bitmap = pdfium::MakeRetain<CFX_DIBitmap>(); ASSERT_TRUE(bitmap->Create(1, 1, FXDIB_Format::kArgb)); - FXARGB_SETDIB(bitmap->GetBuffer().data(), 0x7f'7f'7f'7f); + FXARGB_SetDIB(bitmap->GetWritableBuffer().data(), 0x7f'7f'7f'7f); bitmap->UnPreMultiply(); @@ -132,7 +132,7 @@ auto bitmap = pdfium::MakeRetain<CFX_DIBitmap>(); ASSERT_TRUE(bitmap->Create(1, 1, FXDIB_Format::kArgb)); bitmap->ForcePreMultiply(); - FXARGB_SETDIB(bitmap->GetBuffer().data(), 0x7f'7f'7f'7f); + FXARGB_SetDIB(bitmap->GetWritableBuffer().data(), 0x7f'7f'7f'7f); bitmap->UnPreMultiply(); @@ -143,7 +143,7 @@ auto bitmap = pdfium::MakeRetain<CFX_DIBitmap>(); ASSERT_TRUE(bitmap->Create(1, 1, FXDIB_Format::kArgb)); bitmap->UnPreMultiply(); - FXARGB_SETDIB(bitmap->GetBuffer().data(), 0x7f'ff'ff'ff); + FXARGB_SetDIB(bitmap->GetWritableBuffer().data(), 0x7f'ff'ff'ff); bitmap->UnPreMultiply();
diff --git a/core/fxge/dib/cfx_scanlinecompositor.cpp b/core/fxge/dib/cfx_scanlinecompositor.cpp index b15cedc..4cb5b91 100644 --- a/core/fxge/dib/cfx_scanlinecompositor.cpp +++ b/core/fxge/dib/cfx_scanlinecompositor.cpp
@@ -246,8 +246,8 @@ uint8_t src_alpha = GetAlpha(src_scan[3], clip_scan, col); if (back_alpha == 0) { if (clip_scan) { - FXARGB_SETDIB(dest_scan, - (FXARGB_GETDIB(src_scan) & 0xffffff) | (src_alpha << 24)); + FXARGB_SetDIB(dest_scan, + (FXARGB_GetDIB(src_scan) & 0xffffff) | (src_alpha << 24)); } else { memcpy(dest_scan, src_scan, 4); } @@ -298,9 +298,9 @@ uint8_t back_alpha = *dest_alpha; if (back_alpha == 0) { if (src_Bpp == 4) { - FXARGB_SETDIB(dest_scan, 0xff000000 | FXARGB_GETDIB(src_scan)); + FXARGB_SetDIB(dest_scan, 0xff000000 | FXARGB_GetDIB(src_scan)); } else { - FXARGB_SETDIB(dest_scan, + FXARGB_SetDIB(dest_scan, ArgbEncode(0xff, src_scan[2], src_scan[1], src_scan[0])); } dest_scan += 4; @@ -416,9 +416,9 @@ const uint8_t* src_scan = src_span.data(); for (int col = 0; col < width; col++) { if (src_Bpp == 4) { - FXARGB_SETDIB(dest_scan, 0xff000000 | FXARGB_GETDIB(src_scan)); + FXARGB_SetDIB(dest_scan, 0xff000000 | FXARGB_GetDIB(src_scan)); } else { - FXARGB_SETDIB(dest_scan, + FXARGB_SetDIB(dest_scan, ArgbEncode(0xff, src_scan[2], src_scan[1], src_scan[0])); } dest_scan += 4; @@ -902,7 +902,7 @@ int src_alpha = GetAlphaWithSrc(mask_alpha, clip_scan, src_scan, col); uint8_t back_alpha = dest_scan[3]; if (back_alpha == 0) { - FXARGB_SETDIB(dest_scan, ArgbEncode(src_alpha, src_r, src_g, src_b)); + FXARGB_SetDIB(dest_scan, ArgbEncode(src_alpha, src_r, src_g, src_b)); dest_scan += 4; continue; } @@ -1055,7 +1055,7 @@ FX_ARGB argb = ArgbEncode(0xff, src_r, src_g, src_b); for (int col = 0; col < pixel_count; col++) { if (src_scan[(src_left + col) / 8] & (1 << (7 - (src_left + col) % 8))) { - FXARGB_SETDIB(dest_scan, argb); + FXARGB_SetDIB(dest_scan, argb); } dest_scan += 4; } @@ -1069,7 +1069,7 @@ int src_alpha = GetAlpha(mask_alpha, clip_scan, col); uint8_t back_alpha = dest_scan[3]; if (back_alpha == 0) { - FXARGB_SETDIB(dest_scan, ArgbEncode(src_alpha, src_r, src_g, src_b)); + FXARGB_SetDIB(dest_scan, ArgbEncode(src_alpha, src_r, src_g, src_b)); dest_scan += 4; continue; } @@ -1301,9 +1301,9 @@ uint8_t back_alpha = dest_scan[3]; if (back_alpha == 0) { if (src_Bpp == 4) { - FXARGB_SETRGBORDERDIB(dest_scan, 0xff000000 | FXARGB_GETDIB(src_scan)); + FXARGB_SetRGBOrderDIB(dest_scan, 0xff000000 | FXARGB_GetDIB(src_scan)); } else { - FXARGB_SETRGBORDERDIB( + FXARGB_SetRGBOrderDIB( dest_scan, ArgbEncode(0xff, src_scan[2], src_scan[1], src_scan[0])); } dest_scan += 4; @@ -1382,9 +1382,9 @@ const uint8_t* src_scan = src_span.data(); for (int col = 0; col < width; col++) { if (src_Bpp == 4) { - FXARGB_SETRGBORDERDIB(dest_scan, 0xff000000 | FXARGB_GETDIB(src_scan)); + FXARGB_SetRGBOrderDIB(dest_scan, 0xff000000 | FXARGB_GetDIB(src_scan)); } else { - FXARGB_SETRGBORDERDIB( + FXARGB_SetRGBOrderDIB( dest_scan, ArgbEncode(0xff, src_scan[2], src_scan[1], src_scan[0])); } dest_scan += 4; @@ -1851,7 +1851,7 @@ int src_alpha = GetAlphaWithSrc(mask_alpha, clip_scan, src_scan, col); uint8_t back_alpha = dest_scan[3]; if (back_alpha == 0) { - FXARGB_SETRGBORDERDIB(dest_scan, + FXARGB_SetRGBOrderDIB(dest_scan, ArgbEncode(src_alpha, src_r, src_g, src_b)); dest_scan += 4; continue; @@ -1964,7 +1964,7 @@ FX_ARGB argb = ArgbEncode(0xff, src_r, src_g, src_b); for (int col = 0; col < pixel_count; col++) { if (src_scan[(src_left + col) / 8] & (1 << (7 - (src_left + col) % 8))) { - FXARGB_SETRGBORDERDIB(dest_scan, argb); + FXARGB_SetRGBOrderDIB(dest_scan, argb); } dest_scan += 4; } @@ -1978,7 +1978,7 @@ int src_alpha = GetAlpha(mask_alpha, clip_scan, col); uint8_t back_alpha = dest_scan[3]; if (back_alpha == 0) { - FXARGB_SETRGBORDERDIB(dest_scan, + FXARGB_SetRGBOrderDIB(dest_scan, ArgbEncode(src_alpha, src_r, src_g, src_b)); dest_scan += 4; continue;
diff --git a/core/fxge/dib/fx_dib.h b/core/fxge/dib/fx_dib.h index bd458f9..895b453 100644 --- a/core/fxge/dib/fx_dib.h +++ b/core/fxge/dib/fx_dib.h
@@ -12,6 +12,8 @@ #include <tuple> #include <utility> +#include "third_party/base/compiler_specific.h" + // Encoding: // - Bits-per-pixel: value & 0xFF // - Is mask: value & 0x100 @@ -102,24 +104,22 @@ FXDIB_Format MakeRGBFormat(int bpp); -constexpr FX_CMYK CmykEncode(uint32_t c, uint32_t m, uint32_t y, uint32_t k) { - return (c << 24) | (m << 16) | (y << 8) | k; -} - // Returns (a, r, g, b) std::tuple<int, int, int, int> ArgbDecode(FX_ARGB argb); // Returns (a, FX_COLORREF) std::pair<int, FX_COLORREF> ArgbToAlphaAndColorRef(FX_ARGB argb); -// Returns FX_COLORREF. FX_COLORREF ArgbToColorRef(FX_ARGB argb); +FX_ARGB AlphaAndColorRefToArgb(int a, FX_COLORREF colorref); constexpr FX_ARGB ArgbEncode(uint32_t a, uint32_t r, uint32_t g, uint32_t b) { return (a << 24) | (r << 16) | (g << 8) | b; } -FX_ARGB AlphaAndColorRefToArgb(int a, FX_COLORREF colorref); +constexpr FX_CMYK CmykEncode(uint32_t c, uint32_t m, uint32_t y, uint32_t k) { + return (c << 24) | (m << 16) | (y << 8) | k; +} #define FXARGB_A(argb) ((uint8_t)((argb) >> 24)) #define FXARGB_R(argb) ((uint8_t)((argb) >> 16)) @@ -131,19 +131,7 @@ #define FXRGB2GRAY(r, g, b) (((b)*11 + (g)*59 + (r)*30) / 100) #define FXDIB_ALPHA_MERGE(backdrop, source, source_alpha) \ (((backdrop) * (255 - (source_alpha)) + (source) * (source_alpha)) / 255) -#define FXARGB_GETDIB(p) \ - ((((uint8_t*)(p))[0]) | (((uint8_t*)(p))[1] << 8) | \ - (((uint8_t*)(p))[2] << 16) | (((uint8_t*)(p))[3] << 24)) -#define FXARGB_SETDIB(p, argb) \ - ((uint8_t*)(p))[0] = (uint8_t)(argb), \ - ((uint8_t*)(p))[1] = (uint8_t)((argb) >> 8), \ - ((uint8_t*)(p))[2] = (uint8_t)((argb) >> 16), \ - ((uint8_t*)(p))[3] = (uint8_t)((argb) >> 24) -#define FXARGB_SETRGBORDERDIB(p, argb) \ - ((uint8_t*)(p))[3] = (uint8_t)(argb >> 24), \ - ((uint8_t*)(p))[0] = (uint8_t)((argb) >> 16), \ - ((uint8_t*)(p))[1] = (uint8_t)((argb) >> 8), \ - ((uint8_t*)(p))[2] = (uint8_t)(argb) + #define FXCMYK_TODIB(cmyk) \ ((uint8_t)((cmyk) >> 24) | ((uint8_t)((cmyk) >> 16)) << 8 | \ ((uint8_t)((cmyk) >> 8)) << 16 | ((uint8_t)(cmyk) << 24)) @@ -151,10 +139,35 @@ ((uint8_t)(argb >> 16) | ((uint8_t)(argb >> 8)) << 8 | \ ((uint8_t)(argb)) << 16 | ((uint8_t)(argb >> 24) << 24)) -inline void ReverseCopy3Bytes(uint8_t* dest, const uint8_t* src) { - dest[2] = src[0]; - dest[1] = src[1]; - dest[0] = src[2]; +// SAFETY: Caller must ensure 4 valid bytes at `p`. +UNSAFE_BUFFER_USAGE inline FX_ARGB FXARGB_GetDIB(const uint8_t* p) { + return ArgbEncode(UNSAFE_BUFFERS(p[3]), UNSAFE_BUFFERS(p[2]), + UNSAFE_BUFFERS(p[1]), UNSAFE_BUFFERS(p[0])); +} + +// SAFETY: Caller must ensure 4 valid bytes at `p`. +UNSAFE_BUFFER_USAGE inline void FXARGB_SetDIB(uint8_t* p, uint32_t argb) { + UNSAFE_BUFFERS(p[0]) = FXARGB_B(argb); + UNSAFE_BUFFERS(p[1]) = FXARGB_G(argb); + UNSAFE_BUFFERS(p[2]) = FXARGB_R(argb); + UNSAFE_BUFFERS(p[3]) = FXARGB_A(argb); +} + +// SAFETY: Caller must ensure 4 valid bytes at `p`. +UNSAFE_BUFFER_USAGE inline void FXARGB_SetRGBOrderDIB(uint8_t* p, + uint32_t argb) { + UNSAFE_BUFFERS(p[0]) = FXARGB_R(argb); + UNSAFE_BUFFERS(p[1]) = FXARGB_G(argb); + UNSAFE_BUFFERS(p[2]) = FXARGB_B(argb); + UNSAFE_BUFFERS(p[3]) = FXARGB_A(argb); +} + +// SAFETY: Caller must ensure 3 valid bytes at `dest` and `src`. +UNSAFE_BUFFER_USAGE inline void ReverseCopy3Bytes(uint8_t* dest, + const uint8_t* src) { + UNSAFE_BUFFERS(dest[2] = src[0]); + UNSAFE_BUFFERS(dest[1] = src[1]); + UNSAFE_BUFFERS(dest[0] = src[2]); } #endif // CORE_FXGE_DIB_FX_DIB_H_