qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [PATCH 0/7] clean build - eliminate warnings


From: Jan Kiszka
Subject: [Qemu-devel] Re: [PATCH 0/7] clean build - eliminate warnings
Date: Sun, 22 Feb 2009 11:39:06 +0100
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666

Laurent Desnogues wrote:
> On Sat, Feb 21, 2009 at 9:09 PM, Jan Kiszka <address@hidden> wrote:
>> Laurent Desnogues wrote:
>>> On Sat, Feb 21, 2009 at 8:00 PM, Jan Kiszka <address@hidden> wrote:
>>>> When working on larger or intrusive changes like the monitor rework, the
>>>> number of warnings a normal build generates (here: x86-64 host, gcc 4.3)
>>>> is still too high. And sometimes these warnings are not just of cosmetic
>>>> nature, see (reposted) patch 3.
>>>>
>>>> This series reduces the number of warnings significantly, still not to
>>>> zero (someone would have to look into the NetWinder stuff), but almost:
>>>>
>>>> Warning summary for 2009-02-21 (changes since 2009-02-21-base)
>>>>  generic          0    (-1)
>>>>  softmmu          0   (-39)
>>>>    x86            0     (0)
>>>>    arm            0   (-10)
>>> This means that after applying your patch there should be no more
>>> warning for the ARM target?
>> At least for softmmu, at least with my compiler (depending on the
>> precise version / distro patches, you may have different warnings
>> enabled by default): yes.
> 
> I built softmmu and my Makefile has no other warning than the
> default.
> 
>>> On my machine (x86_64, gcc 4.1.2), I still get these:
>>>
>>>   CC    arm-softmmu/neon_helper.o
>>> /home/ldesnogu/work/Emu/qemu/svn-ref/target-arm/neon_helper.c: In
>>> function ‘helper_neon_rshl_s8’:
>>> /home/ldesnogu/work/Emu/qemu/svn-ref/target-arm/neon_helper.c:469:
>>> warning: ‘vdest.v1’ is used uninitialized in this function
>>> /home/ldesnogu/work/Emu/qemu/svn-ref/target-arm/neon_helper.c:469:
>>> warning: ‘vdest.v2’ is used uninitialized in this function
>>> /home/ldesnogu/work/Emu/qemu/svn-ref/target-arm/neon_helper.c:469:
>>> warning: ‘vdest.v3’ is used uninitialized in this function
>>> /home/ldesnogu/work/Emu/qemu/svn-ref/target-arm/neon_helper.c:469:
>>> warning: ‘vdest.v4’ is used uninitialized in this function
>>> /home/ldesnogu/work/Emu/qemu/svn-ref/target-arm/neon_helper.c: In
>>> function ‘helper_neon_rshl_s16’:
>>> /home/ldesnogu/work/Emu/qemu/svn-ref/target-arm/neon_helper.c:470:
>>> warning: ‘vdest.v1’ is used uninitialized in this function
>>> /home/ldesnogu/work/Emu/qemu/svn-ref/target-arm/neon_helper.c:470:
>>> warning: ‘vdest.v2’ is used uninitialized in this function
>>> /home/ldesnogu/work/Emu/qemu/svn-ref/target-arm/neon_helper.c: In
>>> function ‘helper_neon_rshl_s32’:
>>> /home/ldesnogu/work/Emu/qemu/svn-ref/target-arm/neon_helper.c:471:
>>> warning: ‘vdest.v1’ is used uninitialized in this function
>> Has this been identified as a real issue or just compiler blindness (my
>> series contains one "fix" for such blindness, see cris patch)? I'm
>> currently a bit lost in those macros...
> 
> Yes, it's a real bug:  dest has not been given a value.  You can
> look at preprocessed code :-)

Yeah, preprocessed and Lindent'ed, this looks really buggy.

> BTW, can gcc really say a value
> is used uninitialized?  I have seen it pretending "may be used
> uninitialized" though it is, but it was always right when it says
> "is used uninitialized".

Obviously, detecting uninitialized variables with gcc still leaves room
for improvement. Version 4.3 actually seem to have regressed in this case.

> 
>>> Note a patch has been proposed in the past (by Aurélien IIRC).
>> Do you have a reference at hand?
> 
> Try this:
> 
> http://article.gmane.org/gmane.comp.emulators.qemu/31206

OK, that makes sense, partially. My feeling is that there are some more
typos/thinkos in this code. First, the macro for 8/16/32 bit checks for
a shift width of -8/-16/-32, but the 64-bit version uses -63?! And then
we have this (for signed rshl):

 dest = src >> (width - 1);
 dest++;
 dest >>= 1;

Looks like nothing else than dest = 0, no? Paul?

Jan

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]