[Qemu-devel] clang warnings too noisy

[Qemu-devel] clang warnings too noisy

[Stefan Hajnoczi]

Hi Peter,
You are rejecting pull requests that produce warnings under clang.

clang 3.5.0 on Fedora 21 produces so much noise that it's extremely
tedious and error-prone to try finding relevant new warnings.

Are you using a different clang version which produces fewer warnings?

Is anyone working on making QEMU build cleanly under clang?  Under gcc
people regularly submit patches to keep the build clean (e.g. recent
gcc 5 fixes).

Currently I'm not happy wrangling with clang when very few people seem
to use it or care enough to make QEMU build cleanly.

Examples of noise produced by clang:

1. It complains about glib headers:
In file included from /home/stefanha/qemu/include/net/eth.h:32:
In file included from /home/stefanha/qemu/include/qemu/iov.h:17:
In file included from /home/stefanha/qemu/include/qemu-common.h:43:
In file included from /home/stefanha/qemu/include/glib-compat.h:19:
In file included from /usr/include/glib-2.0/glib.h:50:
In file included from /usr/include/glib-2.0/glib/ghash.h:33:
In file included from /usr/include/glib-2.0/glib/glist.h:32:
/usr/include/glib-2.0/glib/gmem.h:76:78: warning: unknown attribute
'__alloc_size__' ignored [-Wunknown-attributes]
gpointer g_malloc (gsize n_bytes) __attribute__((__malloc__))
__attribute__((__alloc_size__(1)));
                                                                             ^
/usr/include/glib-2.0/glib/gmem.h:78:79: warning: unknown attribute
'__alloc_size__' ignored [-Wunknown-attributes]

2. It complains about the 'return !strcmp(s, "?") || !strcmp(s,
"help")' in qemu-common.h:
/home/stefanha/qemu/include/qemu-common.h:150:1916: warning: array
index 3 is past the end of the array (which contains 2 elements)
[-Warray-bounds]
    return !__extension__ ({ size_t __s1_len, __s2_len;
(__builtin_constant_p (s) && __builtin_constant_p ("?") && (__s1_len =
strlen (s), __s2_len = strlen ("?"), (!((size_t)(const void *)((s) +
1) - (size_t)(const void *)(s) == 1) || __s1_len >= 4) &&
(!((size_t)(const void *)(("?") + 1) - (size_t)(const void *)("?") ==
1) || __s2_len >= 4)) ? __builtin_strcmp (s, "?") :
(__builtin_constant_p (s) && ((size_t)(const void *)((s) + 1) -
(size_t)(const void *)(s) == 1) && (__s1_len = strlen (s), __s1_len <
4) ? (__builtin_constant_p ("?") && ((size_t)(const void *)(("?") + 1)
- (size_t)(const void *)("?") == 1) ? __builtin_strcmp (s, "?") : ...

3. It complains about unused -I paths:
clang: warning: argument unused during compilation: '-I /home/stefanha/qemu/tcg'
clang: warning: argument unused during compilation: '-I
/home/stefanha/qemu/tcg/i386'
clang: warning: argument unused during compilation: '-I
/home/stefanha/qemu/linux-headers'

Stefan

Re: [Qemu-devel] clang warnings too noisy

[Peter Maydell]

On 7 March 2015 at 04:06, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> Hi Peter,
> You are rejecting pull requests that produce warnings under clang.
>
> clang 3.5.0 on Fedora 21 produces so much noise that it's extremely
> tedious and error-prone to try finding relevant new warnings.
>
> Are you using a different clang version which produces fewer warnings?

I use the stock clangs for OSX 10.9.5 and Ubuntu trusty:
Apple LLVM version 6.0 (clang-600.0.56) (based on LLVM 3.5svn)
Target: x86_64-apple-darwin13.4.0
Thread model: posix

Ubuntu clang version 3.4-1ubuntu3 (tags/RELEASE_34/final) (based on LLVM 3.4)
Target: x86_64-pc-linux-gnu
Thread model: posix

> Is anyone working on making QEMU build cleanly under clang?  Under gcc
> people regularly submit patches to keep the build clean (e.g. recent
> gcc 5 fixes).

Yes, I have been working on making QEMU build cleanly under clang,
why do you think I've been rejecting pulls which add warnings and
submitting patches which fix existing warnings?
It currently builds totally cleanly on the clang I use. I have my
config set up with -Werror now and would like to keep it that way.

(The OSX build has warnings about deprecated audio APIs still but
is otherwise clean.)

> Currently I'm not happy wrangling with clang when very few people seem
> to use it or care enough to make QEMU build cleanly.
>
> Examples of noise produced by clang:
>
> 1. It complains about glib headers:
> In file included from /home/stefanha/qemu/include/net/eth.h:32:
> In file included from /home/stefanha/qemu/include/qemu/iov.h:17:
> In file included from /home/stefanha/qemu/include/qemu-common.h:43:
> In file included from /home/stefanha/qemu/include/glib-compat.h:19:
> In file included from /usr/include/glib-2.0/glib.h:50:
> In file included from /usr/include/glib-2.0/glib/ghash.h:33:
> In file included from /usr/include/glib-2.0/glib/glist.h:32:
> /usr/include/glib-2.0/glib/gmem.h:76:78: warning: unknown attribute
> '__alloc_size__' ignored [-Wunknown-attributes]
> gpointer g_malloc (gsize n_bytes) __attribute__((__malloc__))
> __attribute__((__alloc_size__(1)));
>                                                                              ^
> /usr/include/glib-2.0/glib/gmem.h:78:79: warning: unknown attribute
> '__alloc_size__' ignored [-Wunknown-attributes]

I think this is a glib bug...

> 2. It complains about the 'return !strcmp(s, "?") || !strcmp(s,
> "help")' in qemu-common.h:
> /home/stefanha/qemu/include/qemu-common.h:150:1916: warning: array
> index 3 is past the end of the array (which contains 2 elements)
> [-Warray-bounds]
>     return !__extension__ ({ size_t __s1_len, __s2_len;
> (__builtin_constant_p (s) && __builtin_constant_p ("?") && (__s1_len =
> strlen (s), __s2_len = strlen ("?"), (!((size_t)(const void *)((s) +
> 1) - (size_t)(const void *)(s) == 1) || __s1_len >= 4) &&
> (!((size_t)(const void *)(("?") + 1) - (size_t)(const void *)("?") ==
> 1) || __s2_len >= 4)) ? __builtin_strcmp (s, "?") :
> (__builtin_constant_p (s) && ((size_t)(const void *)((s) + 1) -
> (size_t)(const void *)(s) == 1) && (__s1_len = strlen (s), __s1_len <
> 4) ? (__builtin_constant_p ("?") && ((size_t)(const void *)(("?") + 1)
> - (size_t)(const void *)("?") == 1) ? __builtin_strcmp (s, "?") : ...
>
> 3. It complains about unused -I paths:
> clang: warning: argument unused during compilation: '-I /home/stefanha/qemu/tcg'
> clang: warning: argument unused during compilation: '-I
> /home/stefanha/qemu/tcg/i386'
> clang: warning: argument unused during compilation: '-I
> /home/stefanha/qemu/linux-headers'

Are you sure 3 isn't because you're running clang under ccache?

-- PMM

Re: [Qemu-devel] clang warnings too noisy

[Stefan Weil]

Am 06.03.2015 um 20:06 schrieb Stefan Hajnoczi:
> Hi Peter,
> You are rejecting pull requests that produce warnings under clang.
>
> clang 3.5.0 on Fedora 21 produces so much noise that it's extremely
> tedious and error-prone to try finding relevant new warnings.
>
> Are you using a different clang version which produces fewer warnings?
>
> Is anyone working on making QEMU build cleanly under clang?  Under gcc
> people regularly submit patches to keep the build clean (e.g. recent
> gcc 5 fixes).
>
> Currently I'm not happy wrangling with clang when very few people seem
> to use it or care enough to make QEMU build cleanly.
>

Compilation with different compilers is still very important to detect 
hidden bugs. Here is the latest example which I found because of your 
e-mail:

block/iscsi.c:1329:20: warning: comparison of array 'iscsi_url->user' 
not equal to a null pointer is always true [-Wtautological-pointer-compare]

The code is wrong (I'll send a patch), and the code where it was copied 
from is wrong, too (see https://github.com/sahlberg/libiscsi/pull/146).

I use clang on Mac from time to time and used it some time ago on Linux.

Regards
Stefan

Re: [Qemu-devel] clang warnings too noisy

[Stefan Weil]

Am 07.03.2015 um 10:04 schrieb Stefan Weil:
> Am 06.03.2015 um 20:06 schrieb Stefan Hajnoczi:
>> Hi Peter,
>> You are rejecting pull requests that produce warnings under clang.
>>
>> clang 3.5.0 on Fedora 21 produces so much noise that it's extremely
>> tedious and error-prone to try finding relevant new warnings.
>>
>> Are you using a different clang version which produces fewer warnings?
>>
>> Is anyone working on making QEMU build cleanly under clang? Under gcc
>> people regularly submit patches to keep the build clean (e.g. recent
>> gcc 5 fixes).
>>
>> Currently I'm not happy wrangling with clang when very few people seem
>> to use it or care enough to make QEMU build cleanly.
>>
>
> Compilation with different compilers is still very important to detect 
> hidden bugs. Here is the latest example which I found because of your 
> e-mail:
>
> block/iscsi.c:1329:20: warning: comparison of array 'iscsi_url->user' 
> not equal to a null pointer is always true 
> [-Wtautological-pointer-compare]
>
> The code is wrong (I'll send a patch), and the code where it was 
> copied from is wrong, too (see 
> https://github.com/sahlberg/libiscsi/pull/146).
>
> I use clang on Mac from time to time and used it some time ago on Linux.
>
> Regards
> Stefan

My clang build (Debian Jessie clang version 3.5.0-10 
(tags/RELEASE_350/final) (based on LLVM 3.5.0))
just found one more bug and also an unused variable (patches already 
sent to qemu-devel), therefore
yes, we need it.

Stefan

Re: [Qemu-devel] clang warnings too noisy

[Paolo Bonzini]

On 07/03/2015 00:47, Peter Maydell wrote:
> > 1. It complains about glib headers:
> > In file included from /home/stefanha/qemu/include/net/eth.h:32:
> > In file included from /home/stefanha/qemu/include/qemu/iov.h:17:
> > In file included from /home/stefanha/qemu/include/qemu-common.h:43:
> > In file included from /home/stefanha/qemu/include/glib-compat.h:19:
> > In file included from /usr/include/glib-2.0/glib.h:50:
> > In file included from /usr/include/glib-2.0/glib/ghash.h:33:
> > In file included from /usr/include/glib-2.0/glib/glist.h:32:
> > /usr/include/glib-2.0/glib/gmem.h:76:78: warning: unknown attribute
> > '__alloc_size__' ignored [-Wunknown-attributes]
> > gpointer g_malloc (gsize n_bytes) __attribute__((__malloc__))
> > __attribute__((__alloc_size__(1)));
> >                                                                              ^
> > /usr/include/glib-2.0/glib/gmem.h:78:79: warning: unknown attribute
> > '__alloc_size__' ignored [-Wunknown-attributes]
> 
> I think this is a glib bug...

If clang supports fewer attributes than GCC, the right course of action
is disabling the warning on clang.  There are very few attributes that
are used commonly and that affect ABI; most of them are just annotations.

Paolo

Re: [Qemu-devel] clang warnings too noisy

[Stefan Weil]

Am 07.03.2015 um 12:14 schrieb Paolo Bonzini:
> On 07/03/2015 00:47, Peter Maydell wrote:
>>> 1. It complains about glib headers:
>>> In file included from /home/stefanha/qemu/include/net/eth.h:32:
>>> In file included from /home/stefanha/qemu/include/qemu/iov.h:17:
>>> In file included from /home/stefanha/qemu/include/qemu-common.h:43:
>>> In file included from /home/stefanha/qemu/include/glib-compat.h:19:
>>> In file included from /usr/include/glib-2.0/glib.h:50:
>>> In file included from /usr/include/glib-2.0/glib/ghash.h:33:
>>> In file included from /usr/include/glib-2.0/glib/glist.h:32:
>>> /usr/include/glib-2.0/glib/gmem.h:76:78: warning: unknown attribute
>>> '__alloc_size__' ignored [-Wunknown-attributes]
>>> gpointer g_malloc (gsize n_bytes) __attribute__((__malloc__))
>>> __attribute__((__alloc_size__(1)));
>>>                                                                               ^
>>> /usr/include/glib-2.0/glib/gmem.h:78:79: warning: unknown attribute
>>> '__alloc_size__' ignored [-Wunknown-attributes]
>> I think this is a glib bug...
> If clang supports fewer attributes than GCC, the right course of action
> is disabling the warning on clang.  There are very few attributes that
> are used commonly and that affect ABI; most of them are just annotations.
>
> Paolo

On Debian Jessie I don't get any of those warnings with clang.

I think we can focus clang support on those environmentswhich are
really needed: Mac OS X and Linux with sufficiently new distributions.
Then no action is necessary.

Stefan

Re: [Qemu-devel] clang warnings too noisy

[Stefan Hajnoczi]

On Fri, Mar 6, 2015 at 11:47 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 7 March 2015 at 04:06, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> Is anyone working on making QEMU build cleanly under clang?  Under gcc
>> people regularly submit patches to keep the build clean (e.g. recent
>> gcc 5 fixes).
>
> Yes, I have been working on making QEMU build cleanly under clang,
> why do you think I've been rejecting pulls which add warnings and
> submitting patches which fix existing warnings?
> It currently builds totally cleanly on the clang I use. I have my
> config set up with -Werror now and would like to keep it that way.

Okay, that's a dramatically different experience from what I get on
Fedora 21 with clang 3.5.0.

I'll look into fixing these warnings because I now have hope it will
build cleanly if it already works for you.

Stefan