Re: [PATCH][4.15] crypto: adjust rijndaelEncrypt() prototype for gcc11

[Julien Grall <julien@xxxxxxx>]

On 04/03/2021 08:06, Jan Beulich wrote:
> On 03.03.2021 20:09, Julien Grall wrote:
> > On 01/03/2021 07:57, Jan Beulich wrote:
> > > The upcoming release complains, not entirely unreasonably:
> > >
> > > In file included from rijndael.c:33:
> > > .../xen/include/crypto/rijndael.h:55:53: note: previously declared as 'const 
> > > unsigned char[]'
> > >      55 | void    rijndaelEncrypt(const unsigned int [], int, const unsigned 
> > > char [],
> > >         |                                                     
> > > ^~~~~~~~~~~~~~~~~~~~~~
> > > rijndael.c:865:8: error: argument 4 of type 'u8[16]' {aka 'unsigned char[16]'} 
> > > with mismatched bound [-Werror=array-parameter=]
> > >     865 |     u8 ct[16])
> > >         |     ~~~^~~~~~
> > > In file included from rijndael.c:33:
> > > .../xen/include/crypto/rijndael.h:56:13: note: previously declared as 'unsigned 
> > > char[]'
> > >      56 |             unsigned char []);
> > >         |             ^~~~~~~~~~~~~~~~
> > >
> > > While it's not really clear to me why it would complain only for arg 4,
> > > the adjustment to make is obvious and riskfree also for arg 3: Simply
> > > declare the correct array dimension right away. This then allows
> > > compilers to apply checking at call sites, which seems desirable anyway.
> > I am a bit confused, if GCC is not complaining for arg3, then what is
> > the following error message for:
> >
> >   > In file included from rijndael.c:33:
> >   > .../xen/include/crypto/rijndael.h:55:53: note: previously declared as
> > 'const unsigned char[]'
> >   >     55 | void    rijndaelEncrypt(const unsigned int [], int, const
> > unsigned char [],
> >   >        |
> > ^~~~~~~~~~~~~~~~~~~~~~
> Hmm, good point. I didn't observe this myself, and simply copied the
> part of the error message that I was handed. I suppose there was an
> "error: argument 3 of type ..." there then as well. Charles - any
> chance you could confirm this, and perhaps even quote the full set
> of error messages in our internal patch?
>
> I'll adjust the wording of the description in any event.
With the description adjusted:

Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

> > > There are quite a few more issues with gcc11, but from my brief initial
> > > inspection I'm suspecting (hoping) it'll rather be the compiler which
> > > will get further changed by the time their release gets finalized. Just
> > > one example:
> > >
> > > .../xen/include/xen/string.h:101:27: error: '__builtin_memcmp' specified bound 
> > > 4 exceeds source size 0 [-Werror=stringop-overread]
> > >     101 | #define memcmp(s1, s2, n) __builtin_memcmp(s1, s2, n)
> > >         |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > mpparse.c:722:13: note: in expansion of macro 'memcmp'
> > >     722 |         if (memcmp(mpf->mpf_signature, "_MP_", 4) == 0 &&
> > >         |             ^~~~~~
> > >
> > > Clearly neither the 1st nor the 2nd argument have a "source size" of 0.
> > It looks like there is a report on the redhat bug tracker for it [1]. Do
> > you know if there is a bug report on the GCC tracker as well?
> I have no idea, to be honest.
I had a look and couldn't find any. It might be worth to fill one in 
case they are not aware.
Cheers,

--
Julien Grall