d4dfba8ea1
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