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

Issue 2740423003: Revert of Multiply in 64 bits to avoid overflow (Closed)

Created:
3 years, 9 months ago by tommi
Modified:
3 years, 9 months ago
CC:
webrtc-reviews_webrtc.org, AleBzk, peah-webrtc, tlegrand-webrtc, tterriberry_mozilla.com, audio-team_agora.io, hlundin-webrtc, kwiberg-webrtc, minyue-webrtc
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Revert of Multiply in 64 bits to avoid overflow (patchset #1 id:1 of https://codereview.webrtc.org/2729573002/ ) Reason for revert: Looks like this caused isac_fix_test to start failing: https://build.chromium.org/p/client.webrtc.perf/builders/Linux%20Trusty?numbuilds=200 https://build.chromium.org/p/client.webrtc.perf/builders/Linux%20Trusty/builds/1501 Original issue's description: > Multiply in 64 bits to avoid overflow > > A fuzzer run caused the operands of this multiplication to be 512 and > 5000000, resulting in a product about 20% too large for int32_t. So > change this from a 16x32->32 to a 16x32->64 multiplication. Since we > right shift by 2 at the end, the end result will still fit in int32_t. > > I also had to fix a few follow-on add/sub overflows found by the same > fuzzer input once the multiplication was fixed. I chose to saturate > these, since it wasn't just an intermediate value that overflowed. > > BUG=chromium:693868 > > Review-Url: https://codereview.webrtc.org/2729573002 > Cr-Commit-Position: refs/heads/master@{#17003} > Committed: https://chromium.googlesource.com/external/webrtc/+/3a2c803dc341a2bc266effb07df9863c14a7aeaa TBR=henrik.lundin@webrtc.org,kwiberg@webrtc.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=chromium:693868

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -18 lines) Patch
M webrtc/modules/audio_coding/codecs/isac/fix/source/entropy_coding.c View 5 chunks +8 lines, -18 lines 0 comments Download

Messages

Total messages: 5 (2 generated)
tommi
Created Revert of Multiply in 64 bits to avoid overflow
3 years, 9 months ago (2017-03-11 16:00:50 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/2740423003/1
3 years, 9 months ago (2017-03-11 16:00:56 UTC) #3
tommi
3 years, 9 months ago (2017-03-11 16:02:07 UTC) #5
On 2017/03/11 16:00:56, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

Not landing the revert since the chromium issue the CL fixed, has been verified.

Powered by Google App Engine
This is Rietveld 408576698