iSAC Decode: Prevent Memcheck from complaining about uninitialized value

Without this patch, Valgrind's Memcheck was complaining that the test
for whether we should return -1 following the call to
WebRtcIsac_DecodeLb made a conditional branch or move based on the
value of numSamplesLB, which was uninitialized if WebRtcIsac_DecodeLb
failed.

However, as can be seen in the source, the control flow only depends
on the value of numSamplesLB if numDecodedBytesLB >= 0; i.e., if
WebRtcIsac_DecodeLb returned successfully, in which case numSamplesLB
is always initialized. The discrepancy is due to the fact that
Valgrind works on the generated machine code, which contains spurious
such dependencies. The generated code for this test:

  if ((numDecodedBytesLB < 0) || (numDecodedBytesLB > lenEncodedLBBytes) ||
      (numSamplesLB > MAX_FRAMESAMPLES)) {

looks like this:

  95:   0f bf 45 d6             movswl -0x2a(%rbp),%eax
  99:   3d c0 03 00 00          cmp    $0x3c0,%eax
  9e:   0f 8f 45 01 00 00       jg     1e9 <Decode+0x1e9>
  a4:   44 89 f0                mov    %r14d,%eax
  a7:   c1 e0 10                shl    $0x10,%eax
  aa:   0f 88 39 01 00 00       js     1e9 <Decode+0x1e9>
  b0:   41 0f bf ce             movswl %r14w,%ecx
  b4:   89 8d 98 e1 ff ff       mov    %ecx,-0x1e68(%rbp)
  ba:   41 0f bf c7             movswl %r15w,%eax
  be:   39 c1                   cmp    %eax,%ecx
  c0:   0f 8f 23 01 00 00       jg     1e9 <Decode+0x1e9>

Note how the compiler has seemingly ignored the C language's guarantee
that the arguments to || must be evaluated in left-to-right order, and
compares numSamplesLB (%eax) with MAX_FRAMESAMPLES (0x3c0, a.k.a. 960)
before the other two conditions! If the uninitialized value in
numSamplesLB happens to be greater than 960, we'll jump to
Decode+0x1e9 (where we'll return -1) without even looking at the other
two conditions. Has the compiler generated broken code?

Well, no. If numDecodedBytesLB is < 0 so that numSamplesLB is
uninitialized, we'll end up jumping to 1e9 whether that value is
greater than 960 or not; we'll just do it with different jump
instructions. This is entirely invisible as far as the C language is
concerned, but the dependency on the uninitialized value is visible at
the machine code level, which is why Memcheck complains.

This patch solves the problem by pragmatically initializing
numSamplesLB before the call even though it isn't necessary other than
for placating Memcheck.

BUG=4143
R=henrik.lundin@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/36309004

Cr-Commit-Position: refs/heads/master@{#8492}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8492 4adac7df-926f-26a2-2b94-8c16560cd09d

tools/valgrind-webrtc/memcheck/suppressions.txt

@@ -612,20 +612,6 @@
    fun:_ZN12_GLOBAL__N_118StatsCollectorTest22TestCertificateReportsERKN3rtc18FakeSSLCertificateERKSt6vectorISsSaISsEES4_S9_
    fun:_ZN12_GLOBAL__N_156StatsCollectorTest_ChainedCertificateReportsCreated_Test8TestBodyEv
 }
-{
-   bug_4143
-   Memcheck:Uninitialized
-   fun:Decode
-   fun:WebRtcIsac_Decode
-   fun:_ZN6webrtc9IsacFloat6DecodeEP16WebRtcISACStructPKhsPsS5_
-   fun:_ZN6webrtc24AudioEncoderDecoderIsacTINS_9IsacFloatEE6DecodeEPKhmPsPNS_12AudioDecoder10SpeechTypeE
-   fun:_ZThn8_N6webrtc24AudioEncoderDecoderIsacTINS_9IsacFloatEE6DecodeEPKhmPsPNS_12AudioDecoder10SpeechTypeE
-   fun:_ZN6webrtc9NetEqImpl10DecodeLoopEPSt4listIPNS_6PacketESaIS3_EEPNS_10OperationsEPNS_12AudioDecoderEPiPNS9_10SpeechTypeE
-   fun:_ZN6webrtc9NetEqImpl6DecodeEPSt4listIPNS_6PacketESaIS3_EEPNS_10OperationsEPiPNS_12AudioDecoder10SpeechTypeE
-   fun:_ZN6webrtc9NetEqImpl16GetAudioInternalEmPsPiS2_
-   fun:_ZN6webrtc9NetEqImpl8GetAudioEmPsPiS2_PNS_15NetEqOutputTypeE
-   fun:_ZN6webrtc35NetEqDecodingTest_DecoderError_Test8TestBodyEv
-}
 {
    bug_4147_1
    Memcheck:Unaddressable

webrtc/modules/audio_coding/codecs/isac/main/source/isac.c

@@ -1096,6 +1096,13 @@ static int16_t Decode(ISACStruct* ISAC_main_inst,
   memcpy(instISAC->instLB.ISACdecLB_obj.bitstr_obj.stream, encoded,
          lenEncodedLBBytes);
 
+  /* We need to initialize numSamplesLB to something; otherwise, in the test
+     for whether we should return -1 below, the compiler might generate code
+     that fools Memcheck (Valgrind) into thinking that the control flow depends
+     on the uninitialized value in numSamplesLB (since WebRtcIsac_DecodeLb will
+     not fill it in if it fails and returns -1). */
+  numSamplesLB = 0;
+
   /* Regardless of that the current codec is setup to work in
    * wideband or super-wideband, the decoding of the lower-band
    * has to be performed. */