From 600bdb4adc89a0f326c4d3ec37b840378e989f26 Mon Sep 17 00:00:00 2001 From: Alex Loiko Date: Thu, 25 Jan 2018 11:42:43 +0100 Subject: [PATCH] Undefined shifts. This change * replaces a left shift with multiplication, because the shiftee can be negative. * replaces a right shift (a >> b) with the expression (b >= 32 ? 0 : a >> b) because a is a 32-bit value, and b can be >= 32. cppreference quote relating to the second change: "In any case, if the value of the right operand is negative or is greater or equal to the number of bits in the promoted left operand, the behavior is undefined." Bug: chromium:805832 chromium:803078 Change-Id: I67db0c3fedb0af197b2205d424414a84f8fde474 Reviewed-on: https://webrtc-review.googlesource.com/43761 Reviewed-by: Oskar Sundbom Commit-Queue: Alex Loiko Cr-Commit-Position: refs/heads/master@{#21760} --- modules/audio_processing/aecm/aecm_core.cc | 8 +++++++- modules/audio_processing/aecm/aecm_core_c.cc | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/modules/audio_processing/aecm/aecm_core.cc b/modules/audio_processing/aecm/aecm_core.cc index d69dc1ce76..3d9facab4a 100644 --- a/modules/audio_processing/aecm/aecm_core.cc +++ b/modules/audio_processing/aecm/aecm_core.cc @@ -928,8 +928,14 @@ void WebRtcAecm_UpdateChannel(AecmCore* aecm, { // We need to shift down before multiplication shiftChFar = 32 - zerosCh - zerosFar; + // If zerosCh == zerosFar == 0, shiftChFar is 32. A + // right shift of 32 is undefined. To avoid that, we + // do this check. tmpU32no1 = rtc::dchecked_cast( - aecm->channelAdapt32[i] >> shiftChFar) * far_spectrum[i]; + shiftChFar >= 32 + ? 0 + : aecm->channelAdapt32[i] >> shiftChFar) * + far_spectrum[i]; } // Determine Q-domain of numerator zerosNum = WebRtcSpl_NormU32(tmpU32no1); diff --git a/modules/audio_processing/aecm/aecm_core_c.cc b/modules/audio_processing/aecm/aecm_core_c.cc index 99ddac58c2..effe04851c 100644 --- a/modules/audio_processing/aecm/aecm_core_c.cc +++ b/modules/audio_processing/aecm/aecm_core_c.cc @@ -491,7 +491,7 @@ WebRtcAecm_ProcessBlock(AecmCore* aecm, RTC_DCHECK_GE(zeros16, 0); // |zeros16| is a norm, hence non-negative. dfa_clean_q_domain_diff = aecm->dfaCleanQDomain - aecm->dfaCleanQDomainOld; if (zeros16 < dfa_clean_q_domain_diff && aecm->nearFilt[i]) { - tmp16no1 = aecm->nearFilt[i] << zeros16; + tmp16no1 = aecm->nearFilt[i] * (1 << zeros16); qDomainDiff = zeros16 - dfa_clean_q_domain_diff; tmp16no2 = ptrDfaClean[i] >> -qDomainDiff; } else {