Re: [Qemu-devel] [PATCH] gic: avoid a warning from clang

[Blue Swirl]

On Sat, Sep 29, 2012 at 11:44 AM, Peter Maydell
<address@hidden> wrote:
> On 29 September 2012 12:25, Blue Swirl <address@hidden> wrote:
>> On Mon, Sep 24, 2012 at 2:56 PM, Peter Maydell <address@hidden> wrote:
>>> On 24 September 2012 14:22, Peter Maydell <address@hidden> wrote:
>>>> On 23 September 2012 17:33, Blue Swirl <address@hidden> wrote:
>>>>> Avoid this warning:
>>>>>   CC    arm-softmmu/hw/arm/../arm_gic.o
>>>>> /src/qemu/hw/arm/../arm_gic.c:432:17: error: implicit truncation from 
>>>>> 'unsigned int' to bitfield changes value from 4294967040 to 0 
>>>>> [-Werror,-Wconstant-conversion]
>>>>>                 GIC_CLEAR_PENDING(irq + i, ALL_CPU_MASK);
>>>>>                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>> /src/qemu/hw/arm/../arm_gic_internal.h:43:62: note: expanded from:
>>>>> #define GIC_CLEAR_PENDING(irq, cm) s->irq_state[irq].pending &= ~(cm)
>>>>>                                                              ^  ~~~~~
>>>>>
>>>>> 4294967040 is 0xffffff00 and field 'pending' is effectively 8 bits
>>>>> wide, so the masking has no effect except for avoiding the warning.
>>>>
>>>> foo &= ~SOME_FLAGS; is an entirely legitimate and common C idiom,
>>>> and I think clang is being overexuberant in warning here: we should
>>>> disable this warning instead of working around it in the code.
>>
>> This is the only warning generated from QEMU sources, related to
>> -Wconstant-conversion (enabled by -Wall). It would be nice to work
>> around it. How about changing the macros to functions? The use of 's'
>> in the macros look bad and there's no do {} while(0) either to protect
>> the assignment.
>
> Using inline functions would be cleaner code, I think, so if that
> happens to fix the warning that's OK I guess.
>
> I still think clang is actively wrong here, though:
>  foo.bar &= ~(cm);
> where foo.bar is an 8 bit unsigned bitfield means that we do the
> standard integer promotions, so we do the & on 'unsigned int'
> types. So we're not truncating 4294967040 at all, and provably
> the value that goes back into bar has the top 24 bits clear
> anyway. Clang appears to think it's doing the & operation on
> an 8-bit-wide type?

Probably. I tried to change the type of the expression with casts, but
then GCC would complain. I guess the problem is linked with the use of
bitfields, with uint8_t or uint32_t I would not expect problems. That
would limit the number of CPUs though.

>
> -- PMM