Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized

[Philippe Mathieu-Daudé]

On 22/12/22 11:07, Eric Auger wrote:
> Hi Philippe,
>
> On 12/22/22 10:01, Philippe Mathieu-Daudé wrote:
> > On 22/12/22 09:18, Paolo Bonzini wrote:
> > > On 12/21/22 17:36, Eric Auger wrote:
> > > > To avoid compilation errors when -Werror=maybe-uninitialized is used,
> > > > replace 'case 3' by 'default'.
> > > >
> > > > Otherwise we get:
> > > >
> > > > ../target/i386/ops_sse.h: In function ‘helper_vpermdq_ymm’:
> > > > ../target/i386/ops_sse.h:2495:13: error: ‘r3’ may be used
> > > > uninitialized in this function [-Werror=maybe-uninitialized]
> > > >      2495 |     d->Q(3) = r3;
> > > >           |     ~~~~~~~~^~~~
> > > > ../target/i386/ops_sse.h:2494:13: error: ‘r2’ may be used
> > > > uninitialized in this function [-Werror=maybe-uninitialized]
> > > >      2494 |     d->Q(2) = r2;
> > > >           |     ~~~~~~~~^~~~
> > > > ../target/i386/ops_sse.h:2493:13: error: ‘r1’ may be used
> > > > uninitialized in this function [-Werror=maybe-uninitialized]
> > > >      2493 |     d->Q(1) = r1;
> > > >           |     ~~~~~~~~^~~~
> > > > ../target/i386/ops_sse.h:2492:13: error: ‘r0’ may be used
> > > > uninitialized in this function [-Werror=maybe-uninitialized]
> > > >      2492 |     d->Q(0) = r0;
> > > >           |     ~~~~~~~~^~~~
> >
> > With what compiler? Is that a supported one?
> 3aab489e-9d90-c1ad-0b6b-b2b5d80db723@redhat.com/">https://lore.kernel.org/qemu-devel/3aab489e-9d90-c1ad-0b6b-b2b5d80db723@redhat.com/

Adding the compiler version in the commit description would help:

--
Using GCC 11.3.1 "cc (GCC) 11.3.1 20220421 (Red Hat 11.3.1-2)" we get:
--

> > > > Signed-off-by: Eric Auger <eric.auger@redhat.com>
> > > > Suggested-by: Stefan Weil <sw@weilnetz.de>
> > > > Fixes: 790684776861 ("target/i386: reimplement 0x0f 0x3a, add AVX")
> > > > ---
> > > >    target/i386/ops_sse.h | 4 ++--
> > > >    1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h
> > > > index 3cbc36a59d..c442c8c10c 100644
> > > > --- a/target/i386/ops_sse.h
> > > > +++ b/target/i386/ops_sse.h
> > > > @@ -2466,7 +2466,7 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg
> > > > *s, uint32_t order)
> > > >            r0 = s->Q(0);
> > > >            r1 = s->Q(1);
> > > >            break;
> > > > -    case 3:
> > > > +    default:
> > > >            r0 = s->Q(2);
> > > >            r1 = s->Q(3);
> > > >            break;
> > > > @@ -2484,7 +2484,7 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg
> > > > *s, uint32_t order)
> > > >            r2 = s->Q(0);
> > > >            r3 = s->Q(1);
> > > >            break;
> > > > -    case 3:
> > > > +    default:
> > > >            r2 = s->Q(2);
> > > >            r3 = s->Q(3);
> > > >            break;
> > >
> > > Queued, but this compiler sucks. :)
> >
> > Can't we simply add a dumb 'default' case? So when reviewing we don't
> > have to evaluate 'default' means 3 here.
> >
> > -- >8 --
> > --- a/target/i386/ops_sse.h
> > +++ b/target/i386/ops_sse.h
> > @@ -2470,6 +2470,8 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg *s,
> > uint32_t order)
> >           r0 = s->Q(2);
> >           r1 = s->Q(3);
> >           break;
> > +    default:
> > +        qemu_build_not_reached();
> >       }
> >       switch ((order >> 4) & 3) {
> >       case 0:
> > @@ -2488,6 +2490,8 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg *s,
> > uint32_t order)
> >           r2 = s->Q(2);
> >           r3 = s->Q(3);
> >           break;
> > +    default:
> > +        qemu_build_not_reached();
> >       }
> I guess this won't fix the fact r0, r1, r2, r3 are not initialized, will it?

Well my compiler (Apple clang version 14.0.0 (clang-1400.0.29.202))
doesn't display the warning, I don't have yours handy to test it :)