Re: [Qemu-devel] [PULL 0/9] Misc patches for QEMU 2.5-rc2 (2015-11-25)

[Peter Maydell]

On 26 November 2015 at 15:40, Paolo Bonzini <address@hidden> wrote:
> On 26/11/2015 16:01, Peter Maydell wrote:
>> I'm confused by all this text about constant expressions. Does
>> -fwrapv guarantee that signed shift of << behaves as we want
>> in all situations (including constant expressions) or doesn't it?
>
> Until GCC 5, it does even without -fwrapv.
>
> Until GCC 6, the generated code is OK but you get warnings.  For (-1 <<
> 8) you have to enable them manually, for (0xFF << 28) you always get
> them.  I am working on a patch to make GCC 6 shut up if you specify
> -fwrapv, but my point is that -fwrapv is _not_ necessary because:
>
> - GCC very sensibly does not make -Wall enable -Wshift-negative-value
>
> - warnings such as 0xFF << 28 are much less contentious, and even shifts
> into the sign bit (e.g. 1 << 31 or 0xFF << 24) are not enabled by
> default either.
>
>> (And a lot of our uses of "-1 << 8" are indeed in constant expressions.)
>
> Those are okay as long as you do not use -Wextra.

This is all talking about in-practice behaviour of the compiler.
What I'm interested in is what the documented guarantees are.

> Again, the _value_ is perfectly specified by the GCC documentation (and
> as of this morning, it's promised to remain that way).  GCC leaves the
> door open for warning in constant expressions, and indeed GCC 6 warns
> more than GCC 5 in this regard.

I still don't understand why the GCC documentation can't straightforwardly
say "-fwrapv means you get 2s complement behaviour for all operations
including shifts and arithmetic, in all contexts, and we will not warn
or otherwise complain about it". If that's what they're in practice
agreeing to then why not just say so in a comprehensible way, rather
than splitting the information across two different sections of the
documentation and including a confusing bit of text about constant
expressions?

>> I don't think this gains us much as a different approach, and it's
>> still trying to do cleanup (2) before we have dealt with the main
>> issue (1).
>
> What I'm saying is that:
>
> - you were (rightly) worried about the compiler's behavior for constant
> expressions
>
> - but since we use -Werror, we do not have to worry about them.  With
> -Werror, anything that GCC 6 can compile is perfectly specified by the
> GCC documentation, including left shifts of negative values.
>
> So GCC does not even need -fwrapv, and -std=gnu89 is a better way to
> shut up ubsan because it already works.
>
> Regarding clang, we cannot be hostage of their silence.  And recalling
> that they were the ones who f***ed up their warning levels in the first
> place, and are making us waste so much time, I couldn't care less about
> their opinions.

It's been less than 10 days since you filed a bug report with them.
Why are we in such a hurry? I would much rather we took the time to
hear from the clang developers as well as the gcc developers, and then
made our decision about what the right compiler flags are. There doesn't
seem to be any sudden change here that means we need to adjust our
compiler flags for the 2.5 release.

>> > This is about implementation-defined rather than undefined behavior, but
>> > I think it's enough to not care about clang developer's silence.
>>
>> I disagree here. I think it's important to get the clang developers
>> to tell us what they mean by -fwrapv and what they want to guarantee
>> about signed shifts. That's because if they decide to say "no, we
>> don't guarantee behaviour here because the C spec says it's UB" then
>> we need to change how we deal with these in QEMU.
>
> No, we need to change our list of supported compilers (if that happens).

I would strongly favour avoiding this UB rather than dropping clang
as a supported compiler,and implicitly dropping OSX as a supported
platform. (But it doesn't seem to me very likely we'd end up having
to make that awkward choice.)

thanks
-- PMM