[Qemu-devel] clang 3.5.0 errors

[Qemu-devel] clang 3.5.0 errors

[John Snow]

Since we seem to be rejecting patches that introduce clang warnings on 
Peter Maydell's OSX configuration, I felt like I should detail the long 
list of errors we currently have that prevent me from thoroughly testing 
with clang in 3.5.0.

There are five classes of errors that currently plague our fair project:


-Wunknown-attributes currently complains about the many instances of 
__alloc_size__ used about the code, because clang ignores this 
attribute. We can stifle this warning with -Wno-unknown-attributes as 
Stefan tried to, but this apparently causes further warnings and errors 
in versions of clang/gcc that do not recognize this option.


-Wunused-command-line-argument currently complains about the many 
include flags passed to each CC incantation -- presumably this is not 
really fixable, because we'd have to fix our Makefile to be a lot 
smarter than it is. We probably need to stifle these warnings 
conditionally like unknown-attributes, above.


-Wparentheses-equality complains about functions like our QLIST 
functions, because once values get wrapped by a few macros, they have 
many unnecessary parentheses. Since that's "good practice" with macros, 
I think there's no way around needing to disable this warning.


-Wself-assign complains about many functions within softmmu_template.h, 
because of our TGT_LE macro which is a no-op on my machine. Clang gets 
upset about "x = (x)" as a worthless self assignment. Maybe we can avoid 
these errors by using a standard macro instead of one we rolled 
ourselves, but otherwise this error needs to get quieted, too.


-Wtautological-compare also comes up a few times, because we 
occasionally use macros to check if one symbolic value is equivalent to 
another. When both values are bound at preprocessor time to the same 
symbol, runtime conditionals become tautological comparisons.

In qemu/target-mips/translate.c:1965:166, there's an instance of 
FOP_CONDNS being expanded with ifmt = 'FMT_D', so when we get to this 
conditional: 'if (ifmt == FMT_D)', we trigger the tautological 
comparison error.

This is a particular error we probably don't want to disable, but we'll 
need to rework some of these macros to avoid runtime checking of 
compile-time constructs.



Input on which errors should be quieted and which errors should be fixed 
in code is welcome. My hunch is to ignore #1, #2, #3. We might be able 
to fix #4 in code, but it should also be safe to ignore. #5 I suspect we 
really want to fix in code, because the tautological comparison is a 
good warning for type mismatches and we wouldn't want to miss those.

Thanks,
--js

Re: [Qemu-devel] clang 3.5.0 errors

[Peter Maydell]

On 17 March 2015 at 19:30, John Snow <jsnow@redhat.com> wrote:
> -Wunused-command-line-argument currently complains about the many include
> flags passed to each CC incantation -- presumably this is not really
> fixable, because we'd have to fix our Makefile to be a lot smarter than it
> is. We probably need to stifle these warnings conditionally like
> unknown-attributes, above.

Do you see this with really just --cc=clang ? I see these warnings
if I try to use clang with ccache, but I believe that's a ccache bug.

-- PMM

Re: [Qemu-devel] clang 3.5.0 errors

[John Snow]

On 03/17/2015 03:34 PM, Peter Maydell wrote:
> On 17 March 2015 at 19:30, John Snow <jsnow@redhat.com> wrote:
>> -Wunused-command-line-argument currently complains about the many include
>> flags passed to each CC incantation -- presumably this is not really
>> fixable, because we'd have to fix our Makefile to be a lot smarter than it
>> is. We probably need to stifle these warnings conditionally like
>> unknown-attributes, above.
>
> Do you see this with really just --cc=clang ? I see these warnings
> if I try to use clang with ccache, but I believe that's a ccache bug.
>
> -- PMM
>

Not /intentionally/ invoking ccache:

../../configure --cc=clang --host-cc=clang --enable-debug 
--extra-cflags="-Werror -Wno-unknown-attributes 
-Wno-parentheses-equality -Wno-self-assign -Wno-tautological-compare"

But that's because:

which clang
/usr/lib64/ccache/clang

Which is definitely not of my own doing -- seems to be a default in 
Fedora 21.

Re: [Qemu-devel] clang 3.5.0 errors

[Peter Maydell]

On 17 March 2015 at 19:59, John Snow <jsnow@redhat.com> wrote:
> On 03/17/2015 03:34 PM, Peter Maydell wrote:
>> On 17 March 2015 at 19:30, John Snow <jsnow@redhat.com> wrote:
>>> -Wunused-command-line-argument currently complains about the
>>> many include flags passed to each CC incantation

>> Do you see this with really just --cc=clang ? I see these warnings
>> if I try to use clang with ccache, but I believe that's a ccache bug.

> Not /intentionally/ invoking ccache:
>
> ../../configure --cc=clang --host-cc=clang --enable-debug
> --extra-cflags="-Werror -Wno-unknown-attributes -Wno-parentheses-equality
> -Wno-self-assign -Wno-tautological-compare"
>
> But that's because:
>
> which clang
> /usr/lib64/ccache/clang
>
> Which is definitely not of my own doing -- seems to be a default in Fedora
> 21.

Brave of them :-)

Supposedly this bug is fixed in ccache 3.2:
https://bugzilla.samba.org/show_bug.cgi?id=8118

You can probably work around it by telling QEMU's configure
--extra-cflags=-Qunused-arguments or something similar.

-- PMM

Re: [Qemu-devel] clang 3.5.0 errors

[John Snow]

On 03/17/2015 07:07 PM, Peter Maydell wrote:
> On 17 March 2015 at 19:59, John Snow <jsnow@redhat.com> wrote:
>> On 03/17/2015 03:34 PM, Peter Maydell wrote:
>>> On 17 March 2015 at 19:30, John Snow <jsnow@redhat.com> wrote:
>>>> -Wunused-command-line-argument currently complains about the
>>>> many include flags passed to each CC incantation
>
>>> Do you see this with really just --cc=clang ? I see these warnings
>>> if I try to use clang with ccache, but I believe that's a ccache bug.
>
>> Not /intentionally/ invoking ccache:
>>
>> ../../configure --cc=clang --host-cc=clang --enable-debug
>> --extra-cflags="-Werror -Wno-unknown-attributes -Wno-parentheses-equality
>> -Wno-self-assign -Wno-tautological-compare"
>>
>> But that's because:
>>
>> which clang
>> /usr/lib64/ccache/clang
>>
>> Which is definitely not of my own doing -- seems to be a default in Fedora
>> 21.
>
> Brave of them :-)
>
> Supposedly this bug is fixed in ccache 3.2:
> https://bugzilla.samba.org/show_bug.cgi?id=8118
>
> You can probably work around it by telling QEMU's configure
> --extra-cflags=-Qunused-arguments or something similar.
>
> -- PMM
>

Ah, yes; many of these wind up being ccache problems, but not all of them.

There's one case of error here that's interesting that ccache unearths:

we use a gnu extension to give return values to compound statement 
blocks, then wrap these blocks into macros as if they were functions.

The practical outcome here is that these blocks have return codes that 
we often don't check, so clang will spit out "unused value" warnings if 
we compile these after preprocessing, like ccache will tend to do.

This warning is potentially valid: if these calls can fail, we should 
probably either be asserting that a failure did not occur OR we should 
switch to a variant without a return code, if failure is impossible in 
these locations.

An example of this is in linux-user/elfload.c where we define the 
NEW_AUX_ENT() macro which in turn uses the put_user_ual(val, sp) macro. 
When this is expanded, it turns into a compound statement where we 
discard the expression result, so clang whines.

Of course, this all goes away if you disable ccache, but is it worth 
adjusting this particular usage anyway?

--js

Re: [Qemu-devel] clang 3.5.0 errors

[Peter Maydell]

On 18 March 2015 at 19:22, John Snow <jsnow@redhat.com> wrote:
> There's one case of error here that's interesting that ccache unearths:
>
> we use a gnu extension to give return values to compound statement blocks,
> then wrap these blocks into macros as if they were functions.
>
> The practical outcome here is that these blocks have return codes that we
> often don't check, so clang will spit out "unused value" warnings if we
> compile these after preprocessing, like ccache will tend to do.
>
> This warning is potentially valid: if these calls can fail, we should
> probably either be asserting that a failure did not occur OR we should
> switch to a variant without a return code, if failure is impossible in these
> locations.
>
> An example of this is in linux-user/elfload.c where we define the
> NEW_AUX_ENT() macro which in turn uses the put_user_ual(val, sp) macro. When
> this is expanded, it turns into a compound statement where we discard the
> expression result, so clang whines.
>
> Of course, this all goes away if you disable ccache, but is it worth
> adjusting this particular usage anyway?

I agree that ideally we wouldn't do that, but why is this
only visible via ccache? If ccache creates warnings that
aren't visible when directly running the c compiler then
that's a bug in ccache even if the warnings are in theory
interesting.

-- PMM

Re: [Qemu-devel] clang 3.5.0 errors

[John Snow]

On 03/18/2015 04:28 PM, Peter Maydell wrote:
> On 18 March 2015 at 19:22, John Snow <jsnow@redhat.com> wrote:
>> There's one case of error here that's interesting that ccache unearths:
>>
>> we use a gnu extension to give return values to compound statement blocks,
>> then wrap these blocks into macros as if they were functions.
>>
>> The practical outcome here is that these blocks have return codes that we
>> often don't check, so clang will spit out "unused value" warnings if we
>> compile these after preprocessing, like ccache will tend to do.
>>
>> This warning is potentially valid: if these calls can fail, we should
>> probably either be asserting that a failure did not occur OR we should
>> switch to a variant without a return code, if failure is impossible in these
>> locations.
>>
>> An example of this is in linux-user/elfload.c where we define the
>> NEW_AUX_ENT() macro which in turn uses the put_user_ual(val, sp) macro. When
>> this is expanded, it turns into a compound statement where we discard the
>> expression result, so clang whines.
>>
>> Of course, this all goes away if you disable ccache, but is it worth
>> adjusting this particular usage anyway?
>
> I agree that ideally we wouldn't do that, but why is this
> only visible via ccache? If ccache creates warnings that
> aren't visible when directly running the c compiler then
> that's a bug in ccache even if the warnings are in theory
> interesting.
>

You're asking the hard questions.

My hunch is that when the macro is functionaized like it is, clang's 
static analysis treats it as such and decides not to warn about not 
checking the return code, much like it won't for real functions.

When it's flattened, it loses its semantic nature as a function and it 
becomes more of a "useless statement" warning.

It's definitely a bug in ccache, clang or both.

I just found this particular instance of a ccache-provoked warning 
interesting because it does have some validity to it and I wanted to 
raise the issue.

If nobody cares, then, well. Nobody cares.

More mechanical, less-interesting clang fixes will hit the list shortly.

--js

Re: [Qemu-devel] clang 3.5.0 errors

[Peter Maydell]

On 18 March 2015 at 22:22, John Snow <jsnow@redhat.com> wrote:
> I just found this particular instance of a ccache-provoked warning
> interesting because it does have some validity to it and I wanted to raise
> the issue.
>
> If nobody cares, then, well. Nobody cares.

Well, I do sort of care, but on the other hand there are a
lot of issues with linux-user that are more immediately
interesting, so the temptation is to handwave this away,
especially since I don't see the warning on *my* configs :-)

If you want to provide patches to fix them (it genuinely
is a bug that we don't check the return values here) I'm
happy to review them.

I do seem to recall having fixed some issues where clang
warned about this in the past, eg commit f296c0d17.

-- PMM