Re: [Qemu-devel] [PATCH] slirp: Gcc 9 -O3 fix

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Samuel Thibault (samuel.thibault@gnu.org) wrote:
> Hello,
> 
> Dr. David Alan Gilbert (git), le ven. 05 avril 2019 19:46:48 +0100, a ecrit:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Gcc 9 needs some convincing that sopreprbuf really is going to fill
> > in iov in the call from soreadbuf, even though the failure case
> > shouldn't happen; so swing the check around initialising the fields.
> 
> While I can understand that setting iov[0].iov_len may help a compiler,
> I don't see why moving if (len <= 0) return 0; down?

The original errors are:
/home/dgilbert/git/qemu/slirp/src/socket.c: In function ‘soread’:
/home/dgilbert/git/qemu/slirp/src/socket.c:188:7: error: ‘iov.iov_base’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  188 |  nn = recv(so->s, iov[0].iov_base, iov[0].iov_len,0);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/dgilbert/git/qemu/slirp/src/socket.c:188:7: error: ‘iov.iov_len’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
/home/dgilbert/git/qemu/slirp/src/socket.c:232:5: error: ‘n’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  232 |  if (n == 2 && nn == iov[0].iov_len) {
      |     ^
/home/dgilbert/git/qemu/slirp/src/socket.c:234:19: error: ‘*((void *)&iov+16).iov_len’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

(and a few more along the same idea).
So the problem is actually in soread not sopreprbuf itself.

'soread' has the comment:

        /*
         * No need to check if there's enough room to read.
         * soread wouldn't have been called if there weren't
         */
        sopreprbuf(so, iov, &n);

the compiler doesn't realise that, and is moaning about the case
where the if (len <=0) return happens and the following 
code tries to use iov.

Dave

> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  slirp/src/socket.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/slirp/src/socket.c b/slirp/src/socket.c
> > index 4a3c935e25..4a2222a95f 100644
> > --- a/slirp/src/socket.c
> > +++ b/slirp/src/socket.c
> > @@ -113,12 +113,14 @@ size_t sopreprbuf(struct socket *so, struct iovec *iov, int *np)
> >  	DEBUG_CALL("sopreprbuf");
> >  	DEBUG_ARG("so = %p", so);
> >  
> > -	if (len <= 0)
> > -		return 0;
> > -
> >  	iov[0].iov_base = sb->sb_wptr;
> > +        iov[0].iov_len = 0;
> >          iov[1].iov_base = NULL;
> >          iov[1].iov_len = 0;
> > +
> > +	if (len <= 0)
> > +		return 0;
> > +
> >  	if (sb->sb_wptr < sb->sb_rptr) {
> >  		iov[0].iov_len = sb->sb_rptr - sb->sb_wptr;
> >  		/* Should never succeed, but... */
> > -- 
> > 2.21.0
> > 
> 
> -- 
> Samuel
>  FYLG> Tiens, vlà une URL qui va bien :
>  FYLG> ftp://127.0.0.1/WaReZ/NiouZeS/WinDoZe/NeWSMoNGeR/SuPeR
>  c'est gentil sauf que l'adresse ne fonctionne pas sa me fais une erreur
>  -+- Furtif in Guide du Neuneu Usenet : <MODE CERVEAU OFF> -+-
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK