From dafd044e4aea529f88d899247678d4549f776388 Mon Sep 17 00:00:00 2001 From: herb Date: Thu, 17 Dec 2015 14:22:34 -0800 Subject: [PATCH] Fix UB function problems for shaders and mask. BUG=skia:4634 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1530743002 Review URL: https://codereview.chromium.org/1530743002 --- include/core/SkShader.h | 6 ++++- src/core/SkBitmapProcShader.cpp | 4 +-- src/core/SkBitmapProcState.cpp | 24 ++++++++++------- src/core/SkBitmapProcState.h | 6 ++--- src/core/SkBitmapProcState_shaderproc.h | 5 ++-- src/core/SkBlitMask.h | 2 +- src/core/SkBlitMask_D32.cpp | 36 ++++++++++++------------- src/core/SkBlitter_ARGB32.cpp | 4 +-- 8 files changed, 47 insertions(+), 40 deletions(-) diff --git a/include/core/SkShader.h b/include/core/SkShader.h index 34cb648022..9b48697d20 100644 --- a/include/core/SkShader.h +++ b/include/core/SkShader.h @@ -149,7 +149,11 @@ public: */ virtual void shadeSpan(int x, int y, SkPMColor[], int count) = 0; - typedef void (*ShadeProc)(void* ctx, int x, int y, SkPMColor[], int count); + /** + * The const void* ctx is only const because all the implementations are const. + * This can be changed to non-const if a new shade proc needs to change the ctx. + */ + typedef void (*ShadeProc)(const void* ctx, int x, int y, SkPMColor[], int count); virtual ShadeProc asAShadeProc(void** ctx); /** diff --git a/src/core/SkBitmapProcShader.cpp b/src/core/SkBitmapProcShader.cpp index dd97380bbd..6c9410ee32 100644 --- a/src/core/SkBitmapProcShader.cpp +++ b/src/core/SkBitmapProcShader.cpp @@ -174,7 +174,7 @@ void SkBitmapProcShader::BitmapProcShaderContext::shadeSpan(int x, int y, SkPMCo int count) { const SkBitmapProcState& state = *fState; if (state.getShaderProc32()) { - state.getShaderProc32()(state, x, y, dstC, count); + state.getShaderProc32()(&state, x, y, dstC, count); return; } @@ -225,7 +225,7 @@ void SkBitmapProcShader::BitmapProcShaderContext::shadeSpan16(int x, int y, uint int count) { const SkBitmapProcState& state = *fState; if (state.getShaderProc16()) { - state.getShaderProc16()(state, x, y, dstC, count); + state.getShaderProc16()(&state, x, y, dstC, count); return; } diff --git a/src/core/SkBitmapProcState.cpp b/src/core/SkBitmapProcState.cpp index 487bd80402..c85a5fb992 100644 --- a/src/core/SkBitmapProcState.cpp +++ b/src/core/SkBitmapProcState.cpp @@ -24,14 +24,14 @@ extern const SkBitmapProcState::SampleProc16 gSkBitmapProcStateSample16_neon[]; extern const SkBitmapProcState::SampleProc32 gSkBitmapProcStateSample32_neon[]; extern void S16_D16_filter_DX_neon(const SkBitmapProcState&, const uint32_t*, int, uint16_t*); -extern void Clamp_S16_D16_filter_DX_shaderproc_neon(const SkBitmapProcState&, int, int, uint16_t*, int); -extern void Repeat_S16_D16_filter_DX_shaderproc_neon(const SkBitmapProcState&, int, int, uint16_t*, int); +extern void Clamp_S16_D16_filter_DX_shaderproc_neon(const void *, int, int, uint16_t*, int); +extern void Repeat_S16_D16_filter_DX_shaderproc_neon(const void *, int, int, uint16_t*, int); extern void SI8_opaque_D32_filter_DX_neon(const SkBitmapProcState&, const uint32_t*, int, SkPMColor*); -extern void SI8_opaque_D32_filter_DX_shaderproc_neon(const SkBitmapProcState&, int, int, uint32_t*, int); -extern void Clamp_SI8_opaque_D32_filter_DX_shaderproc_neon(const SkBitmapProcState&, int, int, uint32_t*, int); +extern void SI8_opaque_D32_filter_DX_shaderproc_neon(const void *, int, int, uint32_t*, int); +extern void Clamp_SI8_opaque_D32_filter_DX_shaderproc_neon(const void*, int, int, uint32_t*, int); #endif -extern void Clamp_S32_opaque_D32_nofilter_DX_shaderproc(const SkBitmapProcState&, int, int, uint32_t*, int); +extern void Clamp_S32_opaque_D32_nofilter_DX_shaderproc(const void*, int, int, uint32_t*, int); #define NAME_WRAP(x) x #include "SkBitmapProcState_filter.h" @@ -391,10 +391,11 @@ bool SkBitmapProcState::chooseScanlineProcs(bool trivialMatrix, bool clampClamp, return true; } -static void Clamp_S32_D32_nofilter_trans_shaderproc(const SkBitmapProcState& s, +static void Clamp_S32_D32_nofilter_trans_shaderproc(const void* sIn, int x, int y, SkPMColor* SK_RESTRICT colors, int count) { + const SkBitmapProcState& s = *static_cast(sIn); SkASSERT(((s.fInvType & ~SkMatrix::kTranslate_Mask)) == 0); SkASSERT(s.fInvKy == 0); SkASSERT(count > 0 && colors != nullptr); @@ -465,10 +466,11 @@ static inline int sk_int_mirror(int x, int n) { return x; } -static void Repeat_S32_D32_nofilter_trans_shaderproc(const SkBitmapProcState& s, +static void Repeat_S32_D32_nofilter_trans_shaderproc(const void* sIn, int x, int y, SkPMColor* SK_RESTRICT colors, int count) { + const SkBitmapProcState& s = *static_cast(sIn); SkASSERT(((s.fInvType & ~SkMatrix::kTranslate_Mask)) == 0); SkASSERT(s.fInvKy == 0); SkASSERT(count > 0 && colors != nullptr); @@ -505,10 +507,11 @@ static void Repeat_S32_D32_nofilter_trans_shaderproc(const SkBitmapProcState& s, } } -static void S32_D32_constX_shaderproc(const SkBitmapProcState& s, +static void S32_D32_constX_shaderproc(const void* sIn, int x, int y, SkPMColor* SK_RESTRICT colors, int count) { + const SkBitmapProcState& s = *static_cast(sIn); SkASSERT((s.fInvType & ~(SkMatrix::kTranslate_Mask | SkMatrix::kScale_Mask)) == 0); SkASSERT(s.fInvKy == 0); SkASSERT(count > 0 && colors != nullptr); @@ -618,7 +621,7 @@ static void S32_D32_constX_shaderproc(const SkBitmapProcState& s, sk_memset32(colors, color, count); } -static void DoNothing_shaderproc(const SkBitmapProcState&, int x, int y, +static void DoNothing_shaderproc(const void*, int x, int y, SkPMColor* SK_RESTRICT colors, int count) { // if we get called, the matrix is too tricky, so we just draw nothing sk_memset32(colors, 0, count); @@ -811,8 +814,9 @@ int SkBitmapProcState::maxCountForBufferSize(size_t bufferSize) const { /////////////////////// -void Clamp_S32_opaque_D32_nofilter_DX_shaderproc(const SkBitmapProcState& s, int x, int y, +void Clamp_S32_opaque_D32_nofilter_DX_shaderproc(const void* sIn, int x, int y, SkPMColor* SK_RESTRICT dst, int count) { + const SkBitmapProcState& s = *static_cast(sIn); SkASSERT((s.fInvType & ~(SkMatrix::kTranslate_Mask | SkMatrix::kScale_Mask)) == 0); diff --git a/src/core/SkBitmapProcState.h b/src/core/SkBitmapProcState.h index e6e7a3f393..4d7e7f32dc 100644 --- a/src/core/SkBitmapProcState.h +++ b/src/core/SkBitmapProcState.h @@ -43,11 +43,9 @@ struct SkBitmapProcState { SkBitmapProcState(const SkBitmap&, SkShader::TileMode tmx, SkShader::TileMode tmy); ~SkBitmapProcState(); - typedef void (*ShaderProc32)(const SkBitmapProcState&, int x, int y, - SkPMColor[], int count); + typedef void (*ShaderProc32)(const void* ctx, int x, int y, SkPMColor[], int count); - typedef void (*ShaderProc16)(const SkBitmapProcState&, int x, int y, - uint16_t[], int count); + typedef void (*ShaderProc16)(const void* ctx, int x, int y, uint16_t[], int count); typedef void (*MatrixProc)(const SkBitmapProcState&, uint32_t bitmapXY[], diff --git a/src/core/SkBitmapProcState_shaderproc.h b/src/core/SkBitmapProcState_shaderproc.h index 94b2d3b48c..b1fdc5f056 100644 --- a/src/core/SkBitmapProcState_shaderproc.h +++ b/src/core/SkBitmapProcState_shaderproc.h @@ -12,11 +12,12 @@ // Can't be static in the general case because some of these implementations // will be defined and referenced in different object files. -void SCALE_FILTER_NAME(const SkBitmapProcState& s, int x, int y, +void SCALE_FILTER_NAME(const void* sIn, int x, int y, DSTTYPE* SK_RESTRICT colors, int count); -void SCALE_FILTER_NAME(const SkBitmapProcState& s, int x, int y, +void SCALE_FILTER_NAME(const void* sIn, int x, int y, DSTTYPE* SK_RESTRICT colors, int count) { + const SkBitmapProcState& s = *static_cast(sIn); SkASSERT((s.fInvType & ~(SkMatrix::kTranslate_Mask | SkMatrix::kScale_Mask)) == 0); SkASSERT(s.fInvKy == 0); diff --git a/src/core/SkBlitMask.h b/src/core/SkBlitMask.h index f36f9f3d82..b53ff7dfd8 100644 --- a/src/core/SkBlitMask.h +++ b/src/core/SkBlitMask.h @@ -44,7 +44,7 @@ public: * onto a row of dst colors. The RowFactory that returns this function ptr * will have been told the formats for the mask and the dst. */ - typedef void (*RowProc)(void* dst, const void* mask, + typedef void (*RowProc)(SkPMColor* dst, const void* mask, const SkPMColor* src, int width); /** diff --git a/src/core/SkBlitMask_D32.cpp b/src/core/SkBlitMask_D32.cpp index 3cc791af4e..5a1ad2813e 100644 --- a/src/core/SkBlitMask_D32.cpp +++ b/src/core/SkBlitMask_D32.cpp @@ -76,9 +76,9 @@ bool SkBlitMask::BlitColor(const SkPixmap& device, const SkMask& mask, /////////////////////////////////////////////////////////////////////////////// /////////////////////////////////////////////////////////////////////////////// -static void BW_RowProc_Blend(SkPMColor* SK_RESTRICT dst, - const uint8_t* SK_RESTRICT mask, - const SkPMColor* SK_RESTRICT src, int count) { +static void BW_RowProc_Blend( + SkPMColor* SK_RESTRICT dst, const void* maskIn, const SkPMColor* SK_RESTRICT src, int count) { + const uint8_t* SK_RESTRICT mask = static_cast(maskIn); int i, octuple = (count + 7) >> 3; for (i = 0; i < octuple; ++i) { int m = *mask++; @@ -105,9 +105,9 @@ static void BW_RowProc_Blend(SkPMColor* SK_RESTRICT dst, } } -static void BW_RowProc_Opaque(SkPMColor* SK_RESTRICT dst, - const uint8_t* SK_RESTRICT mask, - const SkPMColor* SK_RESTRICT src, int count) { +static void BW_RowProc_Opaque( + SkPMColor* SK_RESTRICT dst, const void* maskIn, const SkPMColor* SK_RESTRICT src, int count) { + const uint8_t* SK_RESTRICT mask = static_cast(maskIn); int i, octuple = (count + 7) >> 3; for (i = 0; i < octuple; ++i) { int m = *mask++; @@ -134,9 +134,9 @@ static void BW_RowProc_Opaque(SkPMColor* SK_RESTRICT dst, } } -static void A8_RowProc_Blend(SkPMColor* SK_RESTRICT dst, - const uint8_t* SK_RESTRICT mask, - const SkPMColor* SK_RESTRICT src, int count) { +static void A8_RowProc_Blend( + SkPMColor* SK_RESTRICT dst, const void* maskIn, const SkPMColor* SK_RESTRICT src, int count) { + const uint8_t* SK_RESTRICT mask = static_cast(maskIn); for (int i = 0; i < count; ++i) { if (mask[i]) { dst[i] = SkBlendARGB32(src[i], dst[i], mask[i]); @@ -153,9 +153,9 @@ static void A8_RowProc_Blend(SkPMColor* SK_RESTRICT dst, #define EXPAND1(v, m, s) (((v) >> 8) & (m)) * (s) #define COMBINE(e0, e1, m) ((((e0) >> 8) & (m)) | ((e1) & ~(m))) -static void A8_RowProc_Opaque(SkPMColor* SK_RESTRICT dst, - const uint8_t* SK_RESTRICT mask, - const SkPMColor* SK_RESTRICT src, int count) { +static void A8_RowProc_Opaque( + SkPMColor* SK_RESTRICT dst, const void* maskIn, const SkPMColor* SK_RESTRICT src, int count) { + const uint8_t* SK_RESTRICT mask = static_cast(maskIn); for (int i = 0; i < count; ++i) { int m = mask[i]; if (m) { @@ -188,9 +188,9 @@ static int src_alpha_blend(int src, int dst, int srcA, int mask) { return dst + SkAlphaMul(src - SkAlphaMul(srcA, dst), mask); } -static void LCD16_RowProc_Blend(SkPMColor* SK_RESTRICT dst, - const uint16_t* SK_RESTRICT mask, - const SkPMColor* SK_RESTRICT src, int count) { +static void LCD16_RowProc_Blend( + SkPMColor* SK_RESTRICT dst, const void* maskIn, const SkPMColor* SK_RESTRICT src, int count) { + const uint8_t* SK_RESTRICT mask = static_cast(maskIn); for (int i = 0; i < count; ++i) { uint16_t m = mask[i]; if (0 == m) { @@ -231,9 +231,9 @@ static void LCD16_RowProc_Blend(SkPMColor* SK_RESTRICT dst, } } -static void LCD16_RowProc_Opaque(SkPMColor* SK_RESTRICT dst, - const uint16_t* SK_RESTRICT mask, - const SkPMColor* SK_RESTRICT src, int count) { +static void LCD16_RowProc_Opaque( + SkPMColor* SK_RESTRICT dst, const void* maskIn, const SkPMColor* SK_RESTRICT src, int count) { + const uint8_t* SK_RESTRICT mask = static_cast(maskIn); for (int i = 0; i < count; ++i) { uint16_t m = mask[i]; if (0 == m) { diff --git a/src/core/SkBlitter_ARGB32.cpp b/src/core/SkBlitter_ARGB32.cpp index e44ad02da5..a00ed86d8e 100644 --- a/src/core/SkBlitter_ARGB32.cpp +++ b/src/core/SkBlitter_ARGB32.cpp @@ -570,7 +570,7 @@ void SkARGB32_Shader_Blitter::blitMask(const SkMask& mask, const SkIRect& clip) SkXfermode* xfer = fXfermode; do { shaderContext->shadeSpan(x, y, span, width); - xfer->xfer32((SkPMColor*)dstRow, span, width, maskRow); + xfer->xfer32(reinterpret_cast(dstRow), span, width, maskRow); dstRow += dstRB; maskRow += maskRB; y += 1; @@ -578,7 +578,7 @@ void SkARGB32_Shader_Blitter::blitMask(const SkMask& mask, const SkIRect& clip) } else { do { shaderContext->shadeSpan(x, y, span, width); - proc(dstRow, maskRow, span, width); + proc(reinterpret_cast(dstRow), maskRow, span, width); dstRow += dstRB; maskRow += maskRB; y += 1;