Re: [Qemu-devel] [PULL 2/9] QEMU does not care about left shifts of signed negative values

[Peter Maydell]

On 25 November 2015 at 21:05, Paolo Bonzini <address@hidden> wrote:
>
>
> On 25/11/2015 20:54, Peter Maydell wrote:
>> > > Your latest patch at 
>> > > https://gcc.gnu.org/ml/gcc-patches/2015-11/msg03055.html
>> > > doesn't seem to touch the documentation of -fwrapv at all, so I
>> > > don't think it is sufficient to allow users of the compiler
>> > > to say "-fwrapv means signed behaviour for shifts".
>> >
>> > GCC *always* does signed behavior for shifts, even without -fwrapv.
>> > I'll commit tomorrow the patch that promises that for the future.
>> >
>> > GCC does not need -fwrapv at all.
>>
>> Yes it does, because without -fwrapv it still wants to warn
>> about them. We need to tell the compiler that we really do
>> want a C dialect where they have specific behaviour and so no
>> warnings are ever correct. By default (as documented, even
>> with your patch) GCC just promises that it won't actually
>> do undefined behaviour for signed negative shifts. (It doesn't
>> even actually say what impdef semantics it does provide,
>
> It says it above the text I changed:
>
>      GCC supports only two's complement integer types, and all bit
>      patterns are ordinary values.
>      [...]
>      Bitwise operators act on the representation of the value including
>      both the sign and value bits, where the sign bit is considered
>      immediately above the highest-value value bit.

Oops, yes. I misread the doc there.

>> and in practice the impdef semantics include "warn about
>> this", which we don't want.)
>
> No, it doesn't warn with the commonly used options.

"They are also diagnosed where constant expressions are required."
strongly implies "we feel free to warn about this usage". Constant
expressions are required all over the place...

>  Only with -pedantic, which is documented as
>
>     Issue all the warnings demanded by strict ISO C and ISO C++;
>     reject all programs that use forbidden extensions, and some
>     other programs that do not follow ISO C and ISO C++.
>
> So the combination of -fwrapv and -pedantic is not particularly
> interesting.  Even then the warning is "initializer element is not
> a constant expression"; nothing to do with overflow.  For example:
>
>     #define INT_MIN ((int)-0x80000000)
>     int y = INT_MIN - 1;
>     int z = -1 << 2;
>     int w = INT_MIN << 1;
>     int u = 1 << 31;
>
> $ gcc f.c -std=c11 -Wall -pedantic
> f.c:2:1: warning: overflow in constant expression [-Woverflow]
> f.c:3:9: warning: initializer element is not a constant expression 
> [-Wpedantic]
> f.c:4:9: warning: initializer element is not a constant expression 
> [-Wpedantic]
> f.c:5:9: warning: initializer element is not a constant expression 
> [-Wpedantic]
>
> The first warning is activated by -Wall, the others aren't.

Unless the C standard specifically requires the warning text
as written, I'd say these are pretty terrible warnings, since
they don't actually diagnose the problem with the code. But
we don't use -pedantic so I don't care about them specifically.
I just care that the documentation should say clearly
"we guarantee these semantics; we won't warn unless you
specifically ask us to warn about their use".

thanks
-- PMM