Re: [Qemu-devel] [PATCH 3/3] disas/arm: avoid clang shifting negative signed warning

[Steven Noonan]

On Tue, Nov 10, 2015 at 10:52 AM, Markus Armbruster <address@hidden> wrote:
> Peter Maydell <address@hidden> writes:
>
>> On 10 November 2015 at 17:33, Paolo Bonzini <address@hidden> wrote:
>>>
>>>
>>> On 10/11/2015 16:57, Stefan Hajnoczi wrote:
>>>> clang 3.7.0 on x86_64 warns about the following:
>>>>
>>>>   disas/arm.c:1782:17: warning: shifting a negative signed value is 
>>>> undefined [-Wshift-negative-value]
>>>>     imm |= (-1 << 7);
>>>>             ~~ ^
>>>>
>>>> Note that this patch preserves the tab indent in this source file
>>>> because the surrounding code still uses tabs.
>>>>
>>>> Signed-off-by: Stefan Hajnoczi <address@hidden>
>>>
>>> I would like to know a case where (except with ubsan) clang actually
>>> uses the optimization.
>>>
>>> If not, this is just error message theatre (which is not news for clang)
>>> and shouldn't have been part of -Wall.
>>
>> It could be they're attempting to warn us now about the possibility
>> that in a future version of clang they will start using this UB
>> to optimize with.
>>
>> http://stackoverflow.com/questions/22883790/left-shift-of-negative-values
>> reports that Intel's ICC will use this in dead-code-elimination
>> optimization. One day clang might do that too.
>
> Nice example of a compiler being gratuitously nasty.
>

I don't read this warning as "clang will do crazy things with your
code eventually". Clang has always been very verbose when it comes to
undefined behavior, and I don't think that's really a bad thing to do.
Even if clang does emit sane code for it, all bets are off for other
compilers -- so it's more of a portability warning. And I know some
other compilers *won't* warn before doing crazy things in the name of
undefined behavior. The ICC example is a fine one...

In my experience fixing the warnings produced by clang has actually
eliminated bugs that were present but undiscovered on other platforms.