Fix UBSan errors (signed integer overflow)
WebRtcSpl_CrossCorrelation and WebRtcSpl_DotProductWithScale compute
the int32 sum of pairwise products from two int16 arrays. So as to
avoid overflow (which could otherwise happen when as little as two
products were summed), the products are right-shifted by an amount
specified by the caller.
This CL changes WebRtcIlbcfix_MyCorr and WebRtcIlbcfix_Smooth to give
sufficient right-shift amounts, instead of ones that may be too small
and cause overflow.
BUG=chromium:601787
Review-Url: https://codereview.webrtc.org/2014033002
Cr-Original-Commit-Position: refs/heads/master@{#13066}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: a10740239dd0f40f54d65288055e128496e2da3f
diff --git a/modules/audio_coding/codecs/ilbc/my_corr.c b/modules/audio_coding/codecs/ilbc/my_corr.c
index bd6ff56..80847b6 100644
--- a/modules/audio_coding/codecs/ilbc/my_corr.c
+++ b/modules/audio_coding/codecs/ilbc/my_corr.c
@@ -29,25 +29,26 @@
const int16_t* seq2, /* (i) second sequence */
size_t dim2 /* (i) dimension seq2 */
){
- int16_t max;
+ uint32_t max1, max2;
size_t loops;
- int scale;
+ int right_shift;
- /* Calculate correlation between the two sequences. Scale the
- result of the multiplcication to maximum 26 bits in order
- to avoid overflow */
- max=WebRtcSpl_MaxAbsValueW16(seq1, dim1);
- scale=WebRtcSpl_GetSizeInBits(max);
-
- scale = 2 * scale - 26;
- if (scale<0) {
- scale=0;
+ // Calculate a right shift that will let us sum dim2 pairwise products of
+ // values from the two sequences without overflowing an int32_t. (The +1 in
+ // max1 and max2 are because WebRtcSpl_MaxAbsValueW16 will return 2**15 - 1
+ // if the input array contains -2**15.)
+ max1 = WebRtcSpl_MaxAbsValueW16(seq1, dim1) + 1;
+ max2 = WebRtcSpl_MaxAbsValueW16(seq2, dim2) + 1;
+ right_shift =
+ (64 - 31) - WebRtcSpl_CountLeadingZeros64((max1 * max2) * (uint64_t)dim2);
+ if (right_shift < 0) {
+ right_shift = 0;
}
loops=dim1-dim2+1;
/* Calculate the cross correlations */
- WebRtcSpl_CrossCorrelation(corr, seq2, seq1, dim2, loops, scale, 1);
+ WebRtcSpl_CrossCorrelation(corr, seq2, seq1, dim2, loops, right_shift, 1);
return;
}
diff --git a/modules/audio_coding/codecs/ilbc/smooth.c b/modules/audio_coding/codecs/ilbc/smooth.c
index 58b37ee..269331c 100644
--- a/modules/audio_coding/codecs/ilbc/smooth.c
+++ b/modules/audio_coding/codecs/ilbc/smooth.c
@@ -31,7 +31,7 @@
int16_t *surround /* (i) The approximation from the
surrounding sequences */
) {
- int16_t maxtot, scale, scale1, scale2;
+ int16_t scale, scale1, scale2;
int16_t A, B, C, denomW16;
int32_t B_W32, denom, num;
int32_t errs;
@@ -40,18 +40,21 @@
int16_t w11prim;
int16_t bitsw00, bitsw10, bitsw11;
int32_t w11w00, w10w10, w00w00;
- int16_t max1, max2;
+ uint32_t max1, max2, max12;
/* compute some inner products (ensure no overflow by first calculating proper scale factor) */
w00 = w10 = w11 = 0;
- max1=WebRtcSpl_MaxAbsValueW16(current, ENH_BLOCKL);
- max2=WebRtcSpl_MaxAbsValueW16(surround, ENH_BLOCKL);
- maxtot=WEBRTC_SPL_MAX(max1, max2);
-
- scale=WebRtcSpl_GetSizeInBits(maxtot);
- scale = (int16_t)(2 * scale) - 26;
+ // Calculate a right shift that will let us sum ENH_BLOCKL pairwise products
+ // of values from the two sequences without overflowing an int32_t. (The +1
+ // in max1 and max2 are because WebRtcSpl_MaxAbsValueW16 will return 2**15 -
+ // 1 if the input array contains -2**15.)
+ max1 = WebRtcSpl_MaxAbsValueW16(current, ENH_BLOCKL) + 1;
+ max2 = WebRtcSpl_MaxAbsValueW16(surround, ENH_BLOCKL) + 1;
+ max12 = WEBRTC_SPL_MAX(max1, max2);
+ scale = (64 - 31) -
+ WebRtcSpl_CountLeadingZeros64((max12 * max12) * (uint64_t)ENH_BLOCKL);
scale=WEBRTC_SPL_MAX(0, scale);
w00=WebRtcSpl_DotProductWithScale(current,current,ENH_BLOCKL,scale);
