From 8ad0e589e7ec1bbb126d118b551c7bda7a9de278 Mon Sep 17 00:00:00 2001 From: oprypin Date: Tue, 5 Sep 2017 03:00:37 -0700 Subject: [PATCH] Move UBSan warnings from a blacklist to the source This is done to make UBSan testing more convenient in integration with projects using WebRTC Some blacklist entries were obsolete so don't need a replacement Also fix one of the warnings (thanks, kwiberg@) BUG=webrtc:8189, webrtc:5486, webrtc:5491 Review-Url: https://codereview.webrtc.org/3009123002 Cr-Commit-Position: refs/heads/master@{#19682} --- tools_webrtc/ubsan/blacklist.txt | 27 +++---------------- .../signal_processing/division_operations.c | 5 +++- .../signal_processing/levinson_durbin.c | 8 ++++-- .../resample_by_2_internal.c | 26 ++++++++++++------ .../signal_processing_unittest.cc | 1 - .../isac/fix/source/filterbanks_unittest.cc | 7 +++-- .../audio_processing/aecm/aecm_core_c.cc | 13 +++++---- webrtc/rtc_base/ipaddress_unittest.cc | 8 +----- webrtc/rtc_base/random.cc | 9 +++---- webrtc/rtc_base/random_unittest.cc | 8 +----- webrtc/rtc_base/sanitizer.h | 2 ++ 11 files changed, 53 insertions(+), 61 deletions(-) diff --git a/tools_webrtc/ubsan/blacklist.txt b/tools_webrtc/ubsan/blacklist.txt index a65b1600c4..3f4c80ec1b 100644 --- a/tools_webrtc/ubsan/blacklist.txt +++ b/tools_webrtc/ubsan/blacklist.txt @@ -1,9 +1,9 @@ ############################################################################# # UBSan blacklist. -# Please think twice before you add or remove these rules. - -# This is a stripped down copy of Chromium's blacklist.txt, to enable -# adding WebRTC-specific blacklist entries. +# +# This is a WebRTC-specific replacement of Chromium's blacklist.txt. +# Only exceptions for third party libraries go here. WebRTC's code should use +# the RTC_NO_SANITIZE macro. Please think twice before adding new exceptions. ############################################################################# # YASM does some funny things that UBsan doesn't like. @@ -17,22 +17,3 @@ src:*/third_party/openh264/* ############################################################################# # Ignore system libraries. src:*/usr/* - -############################################################################# -# https://bugs.chromium.org/p/webrtc/issues/detail?id=5513 -fun:*FilterBanksTest*CalculateResidualEnergyTester* - -############################################################################# -# https://bugs.chromium.org/p/webrtc/issues/detail?id=8200 -fun:*WebRtcAecm_ProcessBlock* - -############################################################################# -# Ignore errors in common_audio. -# https://bugs.chromium.org/p/webrtc/issues/detail?id=5486 -src:*/webrtc/common_audio/signal_processing/signal_processing_unittest.cc -src:*/webrtc/common_audio/signal_processing/resample_by_2_internal.c -fun:*WebRtcSpl_AddSatW32* -fun:*WebRtcSpl_SubSatW32* -fun:*WebRtcSpl_DivW32HiLow* -fun:*WebRtcSpl_LevinsonDurbin* -fun:*GmmProbability* diff --git a/webrtc/common_audio/signal_processing/division_operations.c b/webrtc/common_audio/signal_processing/division_operations.c index eaa06a1ff9..2d420525ba 100644 --- a/webrtc/common_audio/signal_processing/division_operations.c +++ b/webrtc/common_audio/signal_processing/division_operations.c @@ -22,6 +22,7 @@ */ #include "webrtc/common_audio/signal_processing/include/signal_processing_library.h" +#include "webrtc/rtc_base/sanitizer.h" uint32_t WebRtcSpl_DivU32U16(uint32_t num, uint16_t den) { @@ -97,7 +98,8 @@ int32_t WebRtcSpl_DivResultInQ31(int32_t num, int32_t den) return div; } -int32_t WebRtcSpl_DivW32HiLow(int32_t num, int16_t den_hi, int16_t den_low) +int32_t RTC_NO_SANITIZE("signed-integer-overflow") // bugs.webrtc.org/5486 +WebRtcSpl_DivW32HiLow(int32_t num, int16_t den_hi, int16_t den_low) { int16_t approx, tmp_hi, tmp_low, num_hi, num_low; int32_t tmpW32; @@ -110,6 +112,7 @@ int32_t WebRtcSpl_DivW32HiLow(int32_t num, int16_t den_hi, int16_t den_low) // tmpW32 = den * approx tmpW32 = (int32_t)0x7fffffffL - tmpW32; // result in Q30 (tmpW32 = 2.0-(den*approx)) + // UBSan: 2147483647 - -2 cannot be represented in type 'int' // Store tmpW32 in hi and low format tmp_hi = (int16_t)(tmpW32 >> 16); diff --git a/webrtc/common_audio/signal_processing/levinson_durbin.c b/webrtc/common_audio/signal_processing/levinson_durbin.c index c8e58ba455..3e16b23f8b 100644 --- a/webrtc/common_audio/signal_processing/levinson_durbin.c +++ b/webrtc/common_audio/signal_processing/levinson_durbin.c @@ -16,11 +16,13 @@ */ #include "webrtc/common_audio/signal_processing/include/signal_processing_library.h" +#include "webrtc/rtc_base/sanitizer.h" #define SPL_LEVINSON_MAXORDER 20 -int16_t WebRtcSpl_LevinsonDurbin(const int32_t* R, int16_t* A, int16_t* K, - size_t order) +int16_t RTC_NO_SANITIZE("signed-integer-overflow") // bugs.webrtc.org/5486 +WebRtcSpl_LevinsonDurbin(const int32_t* R, int16_t* A, int16_t* K, + size_t order) { size_t i, j; // Auto-correlation coefficients in high precision @@ -44,6 +46,8 @@ int16_t WebRtcSpl_LevinsonDurbin(const int32_t* R, int16_t* A, int16_t* K, for (i = 0; i <= order; ++i) { temp1W32 = R[i] * (1 << norm); + // UBSan: 12 * 268435456 cannot be represented in type 'int' + // Put R in hi and low format R_hi[i] = (int16_t)(temp1W32 >> 16); R_low[i] = (int16_t)((temp1W32 - ((int32_t)R_hi[i] * 65536)) >> 1); diff --git a/webrtc/common_audio/signal_processing/resample_by_2_internal.c b/webrtc/common_audio/signal_processing/resample_by_2_internal.c index 085069c835..72bc0f92b1 100644 --- a/webrtc/common_audio/signal_processing/resample_by_2_internal.c +++ b/webrtc/common_audio/signal_processing/resample_by_2_internal.c @@ -15,6 +15,7 @@ */ #include "webrtc/common_audio/signal_processing/resample_by_2_internal.h" +#include "webrtc/rtc_base/sanitizer.h" // allpass filter coefficients. static const int16_t kResampleAllpass[2][3] = { @@ -28,8 +29,9 @@ static const int16_t kResampleAllpass[2][3] = { // output: int16_t (saturated) (of length len/2) // state: filter state array; length = 8 -void WebRtcSpl_DownBy2IntToShort(int32_t *in, int32_t len, int16_t *out, - int32_t *state) +void RTC_NO_SANITIZE("signed-integer-overflow") // bugs.webrtc.org/5486 +WebRtcSpl_DownBy2IntToShort(int32_t *in, int32_t len, int16_t *out, + int32_t *state) { int32_t tmp0, tmp1, diff; int32_t i; @@ -41,6 +43,8 @@ void WebRtcSpl_DownBy2IntToShort(int32_t *in, int32_t len, int16_t *out, { tmp0 = in[i << 1]; diff = tmp0 - state[1]; + // UBSan: -1771017321 - 999586185 cannot be represented in type 'int' + // scale down and round diff = (diff + (1 << 13)) >> 14; tmp1 = state[0] + diff * kResampleAllpass[1][0]; @@ -121,10 +125,11 @@ void WebRtcSpl_DownBy2IntToShort(int32_t *in, int32_t len, int16_t *out, // output: int32_t (shifted 15 positions to the left, + offset 16384) (of length len/2) // state: filter state array; length = 8 -void WebRtcSpl_DownBy2ShortToInt(const int16_t *in, - int32_t len, - int32_t *out, - int32_t *state) +void RTC_NO_SANITIZE("signed-integer-overflow") // bugs.webrtc.org/5486 +WebRtcSpl_DownBy2ShortToInt(const int16_t *in, + int32_t len, + int32_t *out, + int32_t *state) { int32_t tmp0, tmp1, diff; int32_t i; @@ -141,6 +146,8 @@ void WebRtcSpl_DownBy2ShortToInt(const int16_t *in, tmp1 = state[0] + diff * kResampleAllpass[1][0]; state[0] = tmp0; diff = tmp1 - state[2]; + // UBSan: -1379909682 - 834099714 cannot be represented in type 'int' + // scale down and truncate diff = diff >> 14; if (diff < 0) @@ -549,8 +556,9 @@ void WebRtcSpl_LPBy2ShortToInt(const int16_t* in, int32_t len, int32_t* out, // input: int32_t (shifted 15 positions to the left, + offset 16384) // output: int32_t (normalized, not saturated) // state: filter state array; length = 8 -void WebRtcSpl_LPBy2IntToInt(const int32_t* in, int32_t len, int32_t* out, - int32_t* state) +void RTC_NO_SANITIZE("signed-integer-overflow") // bugs.webrtc.org/5486 +WebRtcSpl_LPBy2IntToInt(const int32_t* in, int32_t len, int32_t* out, + int32_t* state) { int32_t tmp0, tmp1, diff; int32_t i; @@ -594,6 +602,8 @@ void WebRtcSpl_LPBy2IntToInt(const int32_t* in, int32_t len, int32_t* out, { tmp0 = in[i << 1]; diff = tmp0 - state[5]; + // UBSan: -794814117 - 1566149201 cannot be represented in type 'int' + // scale down and round diff = (diff + (1 << 13)) >> 14; tmp1 = state[4] + diff * kResampleAllpass[0][0]; diff --git a/webrtc/common_audio/signal_processing/signal_processing_unittest.cc b/webrtc/common_audio/signal_processing/signal_processing_unittest.cc index da6197ef32..52c4390c44 100644 --- a/webrtc/common_audio/signal_processing/signal_processing_unittest.cc +++ b/webrtc/common_audio/signal_processing/signal_processing_unittest.cc @@ -41,7 +41,6 @@ TEST_F(SplTest, MacroTest) { EXPECT_EQ(3, WEBRTC_SPL_ABS_W32(a)); EXPECT_EQ(-63, WEBRTC_SPL_MUL(a, B)); - EXPECT_EQ(-2147483645, WEBRTC_SPL_MUL(a, b)); EXPECT_EQ(2147483651u, WEBRTC_SPL_UMUL(a, b)); b = WEBRTC_SPL_WORD16_MAX >> 1; EXPECT_EQ(4294918147u, WEBRTC_SPL_UMUL_32_16(a, b)); diff --git a/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_unittest.cc b/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_unittest.cc index ce96e2c631..c8e6ac87ad 100644 --- a/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_unittest.cc +++ b/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_unittest.cc @@ -12,6 +12,7 @@ #include "webrtc/modules/audio_coding/codecs/isac/fix/source/filterbank_internal.h" #include "webrtc/modules/audio_coding/codecs/isac/fix/source/filterbank_tables.h" #include "webrtc/modules/audio_coding/codecs/isac/fix/source/settings.h" +#include "webrtc/rtc_base/sanitizer.h" #include "webrtc/system_wrappers/include/cpu_features_wrapper.h" #include "webrtc/test/gtest.h" #include "webrtc/typedefs.h" @@ -19,8 +20,9 @@ class FilterBanksTest : public testing::Test { protected: // Pass a function pointer to the Tester function. - void CalculateResidualEnergyTester(AllpassFilter2FixDec16 - AllpassFilter2FixDec16Function) { + void RTC_NO_SANITIZE("signed-integer-overflow") // bugs.webrtc.org/5513 + CalculateResidualEnergyTester(AllpassFilter2FixDec16 + AllpassFilter2FixDec16Function) { const int kSamples = QLOOKAHEAD; const int kState = 2; int16_t data_ch1[kSamples] = {0}; @@ -41,6 +43,7 @@ class FilterBanksTest : public testing::Test { sign *= -1; data_ch1[i] = sign * WEBRTC_SPL_WORD32_MAX / (i * i + 1); data_ch2[i] = sign * WEBRTC_SPL_WORD32_MIN / (i * i + 1); + // UBSan: -1 * -2147483648 cannot be represented in type 'int' }; AllpassFilter2FixDec16Function(data_ch1, diff --git a/webrtc/modules/audio_processing/aecm/aecm_core_c.cc b/webrtc/modules/audio_processing/aecm/aecm_core_c.cc index 32f26569f8..bf26277e38 100644 --- a/webrtc/modules/audio_processing/aecm/aecm_core_c.cc +++ b/webrtc/modules/audio_processing/aecm/aecm_core_c.cc @@ -24,6 +24,7 @@ extern "C" { } #include "webrtc/rtc_base/checks.h" +#include "webrtc/rtc_base/sanitizer.h" #include "webrtc/typedefs.h" // Square root of Hanning window in Q14. @@ -276,11 +277,12 @@ static int TimeToFrequencyDomain(AecmCore* aecm, return time_signal_scaling; } -int WebRtcAecm_ProcessBlock(AecmCore* aecm, - const int16_t* farend, - const int16_t* nearendNoisy, - const int16_t* nearendClean, - int16_t* output) { +int RTC_NO_SANITIZE("signed-integer-overflow") // bugs.webrtc.org/8200 +WebRtcAecm_ProcessBlock(AecmCore* aecm, + const int16_t* farend, + const int16_t* nearendNoisy, + const int16_t* nearendClean, + int16_t* output) { int i; uint32_t xfaSum; @@ -453,6 +455,7 @@ int WebRtcAecm_ProcessBlock(AecmCore* aecm, // How much can we shift right to preserve resolution tmp32no1 = echoEst32[i] - aecm->echoFilt[i]; aecm->echoFilt[i] += (tmp32no1 * 50) >> 8; + // UBSan: 72293096 * 50 cannot be represented in type 'int' zeros32 = WebRtcSpl_NormW32(aecm->echoFilt[i]) + 1; zeros16 = WebRtcSpl_NormW16(supGain) + 1; diff --git a/webrtc/rtc_base/ipaddress_unittest.cc b/webrtc/rtc_base/ipaddress_unittest.cc index 3389fef082..61a067f283 100644 --- a/webrtc/rtc_base/ipaddress_unittest.cc +++ b/webrtc/rtc_base/ipaddress_unittest.cc @@ -672,13 +672,7 @@ TEST(IPAddressTest, TestAsIPv6Address) { EXPECT_EQ(addr, addr2); } -// Disabled for UBSan: https://bugs.chromium.org/p/webrtc/issues/detail?id=5491 -#ifdef UNDEFINED_SANITIZER -#define MAYBE_TestCountIPMaskBits DISABLED_TestCountIPMaskBits -#else -#define MAYBE_TestCountIPMaskBits TestCountIPMaskBits -#endif -TEST(IPAddressTest, MAYBE_TestCountIPMaskBits) { +TEST(IPAddressTest, TestCountIPMaskBits) { IPAddress mask; // IPv4 on byte boundaries EXPECT_PRED2(CheckMaskCount, "255.255.255.255", 32); diff --git a/webrtc/rtc_base/random.cc b/webrtc/rtc_base/random.cc index c0cf7fea71..f6bfc08a3e 100644 --- a/webrtc/rtc_base/random.cc +++ b/webrtc/rtc_base/random.cc @@ -12,6 +12,7 @@ #include #include "webrtc/rtc_base/checks.h" +#include "webrtc/rtc_base/safe_conversions.h" namespace webrtc { @@ -40,11 +41,9 @@ uint32_t Random::Rand(uint32_t low, uint32_t high) { int32_t Random::Rand(int32_t low, int32_t high) { RTC_DCHECK(low <= high); - // We rely on subtraction (and addition) to be the same for signed and - // unsigned numbers in two-complement representation. Thus, although - // high - low might be negative as an int, it is the correct difference - // when interpreted as an unsigned. - return Rand(high - low) + low; + const int64_t low_i64{low}; + return rtc::dchecked_cast( + Rand(rtc::dchecked_cast(high - low_i64)) + low_i64); } template <> diff --git a/webrtc/rtc_base/random_unittest.cc b/webrtc/rtc_base/random_unittest.cc index 0af2733faa..12d17ad340 100644 --- a/webrtc/rtc_base/random_unittest.cc +++ b/webrtc/rtc_base/random_unittest.cc @@ -199,13 +199,7 @@ TEST(RandomNumberGeneratorTest, UniformUnsignedInterval) { BucketTestUnsignedInterval(1000, 1000000, 0, 2147483999, 4, &prng); } -// Disabled for UBSan: https://bugs.chromium.org/p/webrtc/issues/detail?id=5491 -#ifdef UNDEFINED_SANITIZER -#define MAYBE_UniformSignedInterval DISABLED_UniformSignedInterval -#else -#define MAYBE_UniformSignedInterval UniformSignedInterval -#endif -TEST(RandomNumberGeneratorTest, MAYBE_UniformSignedInterval) { +TEST(RandomNumberGeneratorTest, UniformSignedInterval) { Random prng(66260695729ull); BucketTestSignedInterval(2, 100000, 0, 1, 3, &prng); BucketTestSignedInterval(7, 100000, -2, 4, 3, &prng); diff --git a/webrtc/rtc_base/sanitizer.h b/webrtc/rtc_base/sanitizer.h index 49dc670fd1..73099ac6fa 100644 --- a/webrtc/rtc_base/sanitizer.h +++ b/webrtc/rtc_base/sanitizer.h @@ -11,6 +11,8 @@ #ifndef WEBRTC_RTC_BASE_SANITIZER_H_ #define WEBRTC_RTC_BASE_SANITIZER_H_ +#include // for size_t + #if defined(__has_feature) #if __has_feature(address_sanitizer) #define RTC_HAS_ASAN 1