From 27c7b8ff115205653c84631fb2650f03b7fda33a Mon Sep 17 00:00:00 2001 From: kwiberg Date: Fri, 9 Sep 2016 02:04:38 -0700 Subject: [PATCH] VadCore: Allow signed multiplication overflow that we don't know how to fix The right thing to do would be to ensure that the multiplication can't overflow, but that'd be a major change bordering on a rewrite, and would take too much time. So instead, we just instruct UBSan to look the other way. BUG=chromium:634834 Review-Url: https://codereview.webrtc.org/2318083002 Cr-Commit-Position: refs/heads/master@{#14154} --- webrtc/base/sanitizer.h | 9 +++++++++ webrtc/common_audio/vad/vad_core.c | 12 +++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/webrtc/base/sanitizer.h b/webrtc/base/sanitizer.h index 3327e08f7d..e27a692164 100644 --- a/webrtc/base/sanitizer.h +++ b/webrtc/base/sanitizer.h @@ -33,6 +33,15 @@ #include #endif +#ifdef __has_attribute +#if __has_attribute(no_sanitize) +#define RTC_NO_SANITIZE(what) __attribute__((no_sanitize(what))) +#endif +#endif +#ifndef RTC_NO_SANITIZE +#define RTC_NO_SANITIZE(what) +#endif + // Ask ASan to mark the memory range [ptr, ptr + element_size * num_elements) // as being unaddressable, so that reads and writes are not allowed. ASan may // narrow the range to the nearest alignment boundaries. diff --git a/webrtc/common_audio/vad/vad_core.c b/webrtc/common_audio/vad/vad_core.c index 51797eed54..0340165eb5 100644 --- a/webrtc/common_audio/vad/vad_core.c +++ b/webrtc/common_audio/vad/vad_core.c @@ -10,6 +10,7 @@ #include "webrtc/common_audio/vad/vad_core.h" +#include "webrtc/base/sanitizer.h" #include "webrtc/common_audio/signal_processing/include/signal_processing_library.h" #include "webrtc/common_audio/vad/vad_filterbank.h" #include "webrtc/common_audio/vad/vad_gmm.h" @@ -110,6 +111,15 @@ static int32_t WeightedAverage(int16_t* data, int16_t offset, return weighted_average; } +// An s16 x s32 -> s32 multiplication that's allowed to overflow. (It's still +// undefined behavior, so not a good idea; this just makes UBSan ignore the +// violation, so that our old code can continue to do what it's always been +// doing.) +static inline int32_t OverflowingMulS16ByS32ToS32(int16_t a, int32_t b) + RTC_NO_SANITIZE("signed-integer-overflow") { + return a * b; +} + // Calculates the probabilities for both speech and background noise using // Gaussian Mixture Models (GMM). A hypothesis-test is performed to decide which // type of signal is most probable. @@ -378,7 +388,7 @@ static int16_t GmmProbability(VadInstT* self, int16_t* features, // (Q14 >> 2) * Q12 = Q24. tmp_s16 = (ngprvec[gaussian] + 2) >> 2; - tmp2_s32 = tmp_s16 * tmp1_s32; + tmp2_s32 = OverflowingMulS16ByS32ToS32(tmp_s16, tmp1_s32); // Q20 * approx 0.001 (2^-10=0.0009766), hence, // (Q24 >> 14) = (Q24 >> 4) / 2^10 = Q20. tmp1_s32 = tmp2_s32 >> 14;