Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(492)

Unified Diff: webrtc/modules/audio_coding/codecs/isac/fix/source/lattice.c

Issue 2314413002: iSAC fix: Ignore overflow in signed left shift (Closed)
Patch Set: rebase Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « webrtc/common_audio/signal_processing/include/signal_processing_library.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/modules/audio_coding/codecs/isac/fix/source/lattice.c
diff --git a/webrtc/modules/audio_coding/codecs/isac/fix/source/lattice.c b/webrtc/modules/audio_coding/codecs/isac/fix/source/lattice.c
index 2e9d0664ef572d080101b16f8ec3af1ca9a54c28..2b92acb64a3d7dd268574abb57a24850b60cb3d1 100644
--- a/webrtc/modules/audio_coding/codecs/isac/fix/source/lattice.c
+++ b/webrtc/modules/audio_coding/codecs/isac/fix/source/lattice.c
@@ -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
« no previous file with comments | « webrtc/common_audio/signal_processing/include/signal_processing_library.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698