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

[Steven Noonan]

On Tue, Nov 10, 2015 at 12:06 PM, Markus Armbruster <address@hidden> wrote:
> Steven Noonan <address@hidden> writes:
>
>> 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.
> [...]
>
> Misunderstanding?
>
> Clang's warning is at worst annoying, but nowhere near nasty.

Sure, it's merely annoying if you only compile your code with Clang.
But my point is that a lot of Clang's aggressive undefined behavior
warnings are there to get people to write portable code so it doesn't
break on other compilers where the "undefined behavior" depends on the
compiler implementation.

> ICC concluding that code executing a left shift of a negative value must
> be unreachable is gratuitously nasty.

I agree, that's completely insane.