From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753509Ab1AYS5v (ORCPT ); Tue, 25 Jan 2011 13:57:51 -0500 Received: from anchor-post-1.mail.demon.net ([195.173.77.132]:47192 "EHLO anchor-post-1.mail.demon.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752478Ab1AYS5u (ORCPT ); Tue, 25 Jan 2011 13:57:50 -0500 Message-ID: <4D3F1D2A.9040309@lougher.demon.co.uk> Date: Tue, 25 Jan 2011 18:57:46 +0000 From: Phillip Lougher User-Agent: Thunderbird 2.0.0.23 (X11/20090817) MIME-Version: 1.0 To: Geert Uytterhoeven CC: Linux Kernel Development , Jesper Juhl , Andrew Morton Subject: Re: [PATCH] Squashfs: Fix use of uninitialised variable in zlib & xz decompressors References: <4D3E2858.5010000@lougher.demon.co.uk> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Geert Uytterhoeven wrote: > On Tue, Jan 25, 2011 at 02:33, Phillip Lougher > wrote: >> Incidentally, on most architectures (bar Mips and Parisc), no >> uninitialised variable warning is generated by gcc, this is because >> the while condition test on continue is optimised out and not performed >> (when executing continue zlib_err has not been changed since entering the >> loop, and logically if the while condition was true previously, then it's >> still true). > > As this is a "do { ... } while (...);" construct and not a "while > (...) { ... }" construct, > the condition is not checked before doing the first iteration. Furthermore the > "continue" may happen during the first iteration (this depends on parameters > passed to the function), so the compiler cannot make any assumptions about the > value of zlib_err, except that may be uninitialized. > No, I've checked the assembly language produced - the while condition test has been optimised out in the case of continue. This is the assembly language output for x86_64, the relevant bits manually annotated by me, zlib_uncompress: .LFB1375: ... call mutex_lock_nested movl $0, 32(%r12) movl $0, 8(%r12) xorl %edx, %edx xorl %eax, %eax # jump to top of loop jmp .L12 .p2align 4,,10 .p2align 3 .L25: # whole if (stream->avail_in == 0 && k < b) { block of code moved out of # main loop, so continue can fall through to main loop # if (stream->avail_in == 0) testl %edx, %edx jne .L2 # wait_on_buffer(bh[k]); movq 32(%rsp), %rsi movq 48(%rsp), %rdx movslq %r13d,%rcx movq %rcx, 56(%rsp) movl $.LC0, %edi movl %eax, (%rsp) leaq (%rsi,%rcx,8), %rcx movl 8(%rdx), %edx movl $306, %esi movq (%rcx), %r8 movq %rcx, 8(%rsp) movl %edx, 68(%rsp) xorl %edx, %edx movq %r8, 16(%rsp) call __might_sleep movq 16(%rsp), %r8 movl (%rsp), %eax movq 8(%rsp), %rcx movq (%r8), %rdx andl $4, %edx jne .L24 .L3: # if (!buffer_uptodate(bh[k])) # goto release_mutex; movq (%rcx), %rdx movq (%rdx), %rcx andl $1, %ecx je .L4 # int avail = min(length, msblk->devblksize - offset); movl 68(%rsp), %ecx subl 44(%rsp), %ecx cmpl 28(%rsp), %ecx cmovg 28(%rsp), %ecx # length -= avail; subl %ecx, 28(%rsp) # if (avail == 0) testl %ecx, %ecx jne .L5 # put_bh(bh[k++]); addl $1, %r13d #APP # 107 "/stripe/git-trees/linux-linus-bugfix-1/arch/x86/include/asm/atomic.h" 1 .section .smp_locks,"a" .balign 4 .long 671f - . .previous 671: lock; decl 96(%rdx) # 0 "" 2 #NO_APP # offset = 0; movl $0, 44(%rsp) .p2align 4,,10 .p2align 3 # *continue* fails through to top of loop # no while condition test # top of loop .L6: movl 8(%r12), %edx # optimised case - first iteration around loop enters here as # edx doesn't need to be loaded (it already holds 0 from # stream->avail_in = 0) .L12: # if (k < b) cmpl %ebp, %r13d setl %r15b jl .L25 .L2: # if (stream->avail_out == 0 && page < pages) { cmpl 152(%rsp), %r14d jge .L7 movl 32(%r12), %ecx testl %ecx, %ecx jne .L7 movslq %r14d,%rdx movl $4096, 32(%r12) addl $1, %r14d movq (%rbx,%rdx,8), %rdx movq %rdx, 24(%r12) .L7: # if (!zlib_init) { testl %eax, %eax jne .L8 movl $15, %esi movq %r12, %rdi call zlib_inflateInit2 testl %eax, %eax jne .L26 .L8: # zlib_err = zlib_inflate(stream, Z_SYNC_FLUSH); movl $3, %esi movq %r12, %rdi call zlib_inflate # if( stream->avail_in == 0 && k < b) { testb %r15b, %r15b je .L10 movl 8(%r12), %edx testl %edx, %edx jne .L10 movq 32(%rsp), %rcx movslq %r13d,%rdx addl $1, %r13d movq (%rcx,%rdx,8), %rdx #APP # 107 "/stripe/git-trees/linux-linus-bugfix-1/arch/x86/include/asm/atomic.h" 1 .section .smp_locks,"a" .balign 4 .long 671f - . .previous 671: lock; decl 96(%rdx) # 0 "" 2 #NO_APP .L10: # } while (zlib_err == Z_OK); testl %eax, %eax jne .L11 movb $1, %al # jump to top of loop jmp .L6 .p2align 4,,10 .p2align 3 Phillip