Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift

[Richard Henderson]

On 09/30/2015 11:27 PM, Paolo Bonzini wrote:
> On 29/09/2015 22:34, Eduardo Habkost wrote:
> > Fix undefined behavior detected by clang runtime check:
> >
> >    qemu/target-i386/cpu.c:1494:15: runtime error:
> >      left shift of 1 by 31 places cannot be represented in type 'int'
> >
> > While doing that, add extra parenthesis for clarity.
> >
> > Reported-by: Peter Maydell <address@hidden>
> > Signed-off-by: Eduardo Habkost <address@hidden>
> > ---
> >   target-i386/cpu.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 2b914b2..6af6db9 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1491,7 +1491,7 @@ static void report_unavailable_features(FeatureWord w, 
> > uint32_t mask)
> >       int i;
> >
> >       for (i = 0; i < 32; ++i) {
> > -        if (1 << i & mask) {
> > +        if ((1UL << i) & mask) {
>
> 1U is enough.
>
> Paolo
>
> ps: Ego ceterum censeo that these warnings are useless and uglify the
> code unnecessarily.  But it looks like I'm in a minority so the patch is
> okay.

I totally agree.  There are no ones-compliment machines anymore, and so the 
whole point of that "undefined" in the C standard is moot.  Let's all accept 
that shifts of signed quantities do exactly what we expect.

Without looking, I don't suppose either compiler has a switch to disable just 
the shift part of ubsan?


r~