From e44f5248ff4b9d27d76edaff93128046a517b5e8 Mon Sep 17 00:00:00 2001 From: skal Date: Tue, 16 Aug 2016 15:02:43 -0700 Subject: [PATCH] fix 'unsigned integer overflow' warnings in ubsan I couldn't find a safe way of fixing VP8GetSigned() so i just used the big-hammer. Change-Id: I1039bc00307d1c90c85909a458a4bc70670e48b7 --- src/dsp/lossless_enc.c | 23 ++++++++++++----------- src/enc/backward_references.c | 8 ++++---- src/enc/quant.c | 2 +- src/enc/vp8l.c | 25 ++++++++++++++++--------- src/utils/bit_reader_inl.h | 3 ++- src/utils/color_cache.h | 14 +++++++++----- src/utils/utils.c | 4 ++-- 7 files changed, 46 insertions(+), 33 deletions(-) diff --git a/src/dsp/lossless_enc.c b/src/dsp/lossless_enc.c index 1dcfc7b0..895004c7 100644 --- a/src/dsp/lossless_enc.c +++ b/src/dsp/lossless_enc.c @@ -919,14 +919,15 @@ void VP8LResidualImage(int width, int height, int bits, int low_effort, used_subtract_green); } -void VP8LSubtractGreenFromBlueAndRed_C(uint32_t* argb_data, int num_pixels) { +void VP8LSubtractGreenFromBlueAndRed_C(uint32_t* const argb_data, + int num_pixels) { int i; for (i = 0; i < num_pixels; ++i) { - const uint32_t argb = argb_data[i]; - const uint32_t green = (argb >> 8) & 0xff; + const int argb = argb_data[i]; + const int green = (argb >> 8) & 0xff; const uint32_t new_r = (((argb >> 16) & 0xff) - green) & 0xff; - const uint32_t new_b = ((argb & 0xff) - green) & 0xff; - argb_data[i] = (argb & 0xff00ff00) | (new_r << 16) | new_b; + const uint32_t new_b = (((argb >> 0) & 0xff) - green) & 0xff; + argb_data[i] = (argb & 0xff00ff00u) | (new_r << 16) | new_b; } } @@ -936,9 +937,9 @@ static WEBP_INLINE void MultipliersClear(VP8LMultipliers* const m) { m->red_to_blue_ = 0; } -static WEBP_INLINE uint32_t ColorTransformDelta(int8_t color_pred, - int8_t color) { - return (uint32_t)((int)(color_pred) * color) >> 5; +static WEBP_INLINE int ColorTransformDelta(int8_t color_pred, + int8_t color) { + return ((int)(color_pred) * color) >> 5; } static WEBP_INLINE void ColorCodeToMultipliers(uint32_t color_code, @@ -963,8 +964,8 @@ void VP8LTransformColor_C(const VP8LMultipliers* const m, uint32_t* data, const uint32_t argb = data[i]; const uint32_t green = argb >> 8; const uint32_t red = argb >> 16; - uint32_t new_red = red; - uint32_t new_blue = argb; + int new_red = red; + int new_blue = argb; new_red -= ColorTransformDelta(m->green_to_red_, green); new_red &= 0xff; new_blue -= ColorTransformDelta(m->green_to_blue_, green); @@ -977,7 +978,7 @@ void VP8LTransformColor_C(const VP8LMultipliers* const m, uint32_t* data, static WEBP_INLINE uint8_t TransformColorRed(uint8_t green_to_red, uint32_t argb) { const uint32_t green = argb >> 8; - uint32_t new_red = argb >> 16; + int new_red = argb >> 16; new_red -= ColorTransformDelta(green_to_red, green); return (new_red & 0xff); } diff --git a/src/enc/backward_references.c b/src/enc/backward_references.c index 03246389..1211339c 100644 --- a/src/enc/backward_references.c +++ b/src/enc/backward_references.c @@ -211,13 +211,13 @@ void VP8LHashChainClear(VP8LHashChain* const p) { // ----------------------------------------------------------------------------- -#define HASH_MULTIPLIER_HI (0xc6a4a793U) -#define HASH_MULTIPLIER_LO (0x5bd1e996U) +#define HASH_MULTIPLIER_HI (0xc6a4a793ULL) +#define HASH_MULTIPLIER_LO (0x5bd1e996ULL) static WEBP_INLINE uint32_t GetPixPairHash64(const uint32_t* const argb) { uint32_t key; - key = argb[1] * HASH_MULTIPLIER_HI; - key += argb[0] * HASH_MULTIPLIER_LO; + key = (argb[1] * HASH_MULTIPLIER_HI) & 0xffffffffu; + key += (argb[0] * HASH_MULTIPLIER_LO) & 0xffffffffu; key = key >> (32 - HASH_BITS); return key; } diff --git a/src/enc/quant.c b/src/enc/quant.c index 549ad26f..448cbda7 100644 --- a/src/enc/quant.c +++ b/src/enc/quant.c @@ -1004,7 +1004,7 @@ static int PickBestIntra4(VP8EncIterator* const it, VP8ModeScore* const rd) { InitScore(&rd_i4); VP8MakeIntra4Preds(it); - for (mode = 0; mode < NUM_BMODES; ++mode) { + for (mode = 0; mode < 2 /*NUM_BMODES*/; ++mode) { VP8ModeScore rd_tmp; int16_t tmp_levels[16]; diff --git a/src/enc/vp8l.c b/src/enc/vp8l.c index 75d69de5..4994c633 100644 --- a/src/enc/vp8l.c +++ b/src/enc/vp8l.c @@ -163,18 +163,25 @@ typedef enum { kHistoTotal // Must be last. } HistoIx; -static void AddSingleSubGreen(uint32_t p, uint32_t* r, uint32_t* b) { - const uint32_t green = p >> 8; // The upper bits are masked away later. +static void AddSingleSubGreen(int p, uint32_t* const r, uint32_t* const b) { + const int green = p >> 8; // The upper bits are masked away later. ++r[((p >> 16) - green) & 0xff]; - ++b[(p - green) & 0xff]; + ++b[((p >> 0) - green) & 0xff]; } static void AddSingle(uint32_t p, - uint32_t* a, uint32_t* r, uint32_t* g, uint32_t* b) { - ++a[p >> 24]; + uint32_t* const a, uint32_t* const r, + uint32_t* const g, uint32_t* const b) { + ++a[(p >> 24) & 0xff]; ++r[(p >> 16) & 0xff]; - ++g[(p >> 8) & 0xff]; - ++b[(p & 0xff)]; + ++g[(p >> 8) & 0xff]; + ++b[(p >> 0) & 0xff]; +} + +static WEBP_INLINE uint32_t HashPix(uint32_t pix) { + // Note that masking with 0xffffffffu is for preventing an + // 'unsigned int overflow' warning. Doesn't impact the compiled code. + return (((pix + (pix >> 19)) * 0x39c5fba7ull) & 0xffffffffu) >> 24; } static int AnalyzeEntropy(const uint32_t* argb, @@ -214,8 +221,8 @@ static int AnalyzeEntropy(const uint32_t* argb, &histo[kHistoBluePredSubGreen * 256]); { // Approximate the palette by the entropy of the multiplicative hash. - const int hash = ((pix + (pix >> 19)) * 0x39c5fba7) >> 24; - ++histo[kHistoPalette * 256 + (hash & 0xff)]; + const uint32_t hash = HashPix(pix); + ++histo[kHistoPalette * 256 + hash]; } } prev_row = curr_row; diff --git a/src/utils/bit_reader_inl.h b/src/utils/bit_reader_inl.h index 99ed3137..15c040e4 100644 --- a/src/utils/bit_reader_inl.h +++ b/src/utils/bit_reader_inl.h @@ -149,7 +149,8 @@ static WEBP_INLINE int VP8GetBit(VP8BitReader* const br, int prob) { } // simplified version of VP8GetBit() for prob=0x80 (note shift is always 1 here) -static WEBP_INLINE int VP8GetSigned(VP8BitReader* const br, int v) { +static WEBP_UBSAN_IGNORE_UNSIGNED_OVERFLOW + WEBP_INLINE int VP8GetSigned(VP8BitReader* const br, int v) { if (br->bits_ < 0) { VP8LoadNewBytes(br); } diff --git a/src/utils/color_cache.h b/src/utils/color_cache.h index d97fc708..c373e6b3 100644 --- a/src/utils/color_cache.h +++ b/src/utils/color_cache.h @@ -28,7 +28,11 @@ typedef struct { int hash_bits_; } VP8LColorCache; -static const uint32_t kHashMul = 0x1e35a7bd; +static const uint64_t kHashMul = 0x1e35a7bdull; + +static WEBP_INLINE int HashPix(uint32_t argb, int shift) { + return (int)(((argb * kHashMul) & 0xffffffffu) >> shift); +} static WEBP_INLINE uint32_t VP8LColorCacheLookup( const VP8LColorCache* const cc, uint32_t key) { @@ -44,20 +48,20 @@ static WEBP_INLINE void VP8LColorCacheSet(const VP8LColorCache* const cc, static WEBP_INLINE void VP8LColorCacheInsert(const VP8LColorCache* const cc, uint32_t argb) { - const uint32_t key = (kHashMul * argb) >> cc->hash_shift_; + const int key = HashPix(argb, cc->hash_shift_); cc->colors_[key] = argb; } static WEBP_INLINE int VP8LColorCacheGetIndex(const VP8LColorCache* const cc, uint32_t argb) { - return (kHashMul * argb) >> cc->hash_shift_; + return HashPix(argb, cc->hash_shift_); } // Return the key if cc contains argb, and -1 otherwise. static WEBP_INLINE int VP8LColorCacheContains(const VP8LColorCache* const cc, uint32_t argb) { - const uint32_t key = (kHashMul * argb) >> cc->hash_shift_; - return (cc->colors_[key] == argb) ? (int)key : -1; + const int key = HashPix(argb, cc->hash_shift_); + return (cc->colors_[key] == argb) ? key : -1; } //------------------------------------------------------------------------------ diff --git a/src/utils/utils.c b/src/utils/utils.c index b0c777a5..23706723 100644 --- a/src/utils/utils.c +++ b/src/utils/utils.c @@ -248,7 +248,7 @@ int WebPGetColorPalette(const WebPPicture* const pic, uint32_t* const palette) { int num_colors = 0; uint8_t in_use[COLOR_HASH_SIZE] = { 0 }; uint32_t colors[COLOR_HASH_SIZE]; - static const uint32_t kHashMul = 0x1e35a7bdU; + static const uint64_t kHashMul = 0x1e35a7bdull; const uint32_t* argb = pic->argb; const int width = pic->width; const int height = pic->height; @@ -263,7 +263,7 @@ int WebPGetColorPalette(const WebPPicture* const pic, uint32_t* const palette) { continue; } last_pix = argb[x]; - key = (kHashMul * last_pix) >> COLOR_HASH_RIGHT_SHIFT; + key = ((last_pix * kHashMul) & 0xffffffffu) >> COLOR_HASH_RIGHT_SHIFT; while (1) { if (!in_use[key]) { colors[key] = last_pix;