zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

iSAC fix: Ignore overflow in signed left shift

Browse Source 
A left shift by 10 was assumed to never overflow, since "[s]imulation
of the 25 files shows that maximum value in the vector gain_lo_hiQ17[]
is 441344, which means that it is log2((2^31)/441344) = 12.2 shifting
bits from saturation." However, a fuzzer test succeeded in provoking
an overflow, which we ignore in this CL on the theory that only
"abnormal" inputs cause overflow.

Also had to replace a "foo << 1" with "foo * (1 << 1)" in
WEBRTC_SPL_MUL_16_32_RSFT15 because foo could be negative; this
problem showed up as soon as I'd asked UBSan to ignore the overflow
discussed above.

BUG=chromium:615819

Review-Url: https://codereview.webrtc.org/2314413002
Cr-Commit-Position: refs/heads/master@{#14162}
This commit is contained in:
kwiberg
2016-09-09 05:51:33 -07:00
committed by Commit bot
parent 53cec04f5c
commit 2b1b7a83ad
2 changed files with 19 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
webrtc/modules/audio_coding/codecs/isac/fix/source/lattice.c
View File

@ -17,6 +17,7 @@
#include "codec.h"
#include "settings.h"
#include "webrtc/base/sanitizer.h"
#define LATTICE_MUL_32_32_RSFT16(a32a, a32b, b32) \
((int32_t)(WEBRTC_SPL_MUL(a32a, b32) + (WEBRTC_SPL_MUL_16_32_RSFT16(a32b, b32))))
@ -205,9 +206,13 @@ void WebRtcIsacfix_NormLatticeFilterMa(size_t orderCoef,
return;
}
// Left shift of an int32_t 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 OverflowingLShiftS32(int32_t x, int shift)
RTC_NO_SANITIZE("shift") {
return x << shift;
}
/* ----------------AR filter-------------------------*/
/* filter the signal using normalized lattice filter */
@ -252,13 +257,14 @@ void WebRtcIsacfix_NormLatticeFilterAr(size_t orderCoef,
WebRtcSpl_SqrtOfOneMinusXSquared(sthQ15, orderCoef, cthQ15);
/* Simulation of the 25 files shows that maximum value in
the vector gain_lo_hiQ17[] is 441344, which means that
it is log2((2^31)/441344) = 12.2 shifting bits from
saturation. Therefore, it should be safe to use Q27 instead
of Q17. */
tmp32 = gain_lo_hiQ17[temp3] << 10; // Q27
// Originally, this line was assumed to never overflow, since "[s]imulation
// of the 25 files shows that maximum value in the vector gain_lo_hiQ17[]
// is 441344, which means that it is log2((2^31)/441344) = 12.2 shifting
// bits from saturation. Therefore, it should be safe to use Q27 instead of
// Q17." However, a fuzzer test succeeded in provoking an overflow here,
// which we ignore on the theory that only "abnormal" inputs cause
// overflow.
tmp32 = OverflowingLShiftS32(gain_lo_hiQ17[temp3], 10); // Q27
for (k=0;k<orderCoef;k++) {
tmp32 = WEBRTC_SPL_MUL_16_32_RSFT15(cthQ15[k], tmp32); // Q15*Q27>>15 = Q27