Re: [PATCH] inet/net-internal.h: Fix uninitalised clntudp_call() variable

[Alistair Francis <alistair23@gmail.com>]

On Tue, Sep 24, 2019 at 1:21 PM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 9/16/19 6:15 PM, Alistair Francis wrote:
> > The total_deadline variable inside the clntudp_call() function inside
> > sunrpc/clnt_udp.c can cause uninitalised variable warnings when building
> > with GCC 8.3 or 9.2 on a platform with a 64-bit tv_nsec on a 32-bit
> > architecture. To fix the warning let's use the DIAG_* macros to hide the
> > warning.
> >
> > A GCC bug case has also been submitted:
> >     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91691
> >
>
> This looks good to me. If you're seeing this using the released 8.3 or 9.2
> then others will see it too. You also reference the compiler version that
> is the latest with the problem e.g. 9. I don't think we should fix this
> by zeroing out the entire field since this looks like an optimization
> or compiler related issue.
>
> OK for master.

Thanks!

I don't have commit access yet, I'm going to start the process of
requesting it. I named you as the person to approve my account
creation Carlos.

While that is happening if anyone else would like to apply this patch
please feel free to :)

Alistair

>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
> > 2019-09-16  Alistair Francis  <alistair.francis@wdc.com>
> >
> >       * inet/net-internal.h: Fix uninitalised clntudp_call() variable
> > ---
> >  inet/net-internal.h | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/inet/net-internal.h b/inet/net-internal.h
> > index 2f522eef555..c774de2b78a 100644
> > --- a/inet/net-internal.h
> > +++ b/inet/net-internal.h
> > @@ -23,6 +23,7 @@
> >  #include <stdbool.h>
> >  #include <stdint.h>
> >  #include <sys/time.h>
> > +#include <libc-diag.h>
> >
> >  int __inet6_scopeid_pton (const struct in6_addr *address,
> >                            const char *scope, uint32_t *result);
> > @@ -96,6 +97,16 @@ __deadline_is_infinite (struct deadline deadline)
> >    return deadline.absolute.tv_nsec < 0;
> >  }
> >
> > +/* GCC 8.3 and 9.2 both incorrectly report total_deadline
> > + * (from sunrpc/clnt_udp.c) as maybe-uninitialized when tv_sec is 8 bytes
> > + * (64-bits) wide on 32-bit systems. We have to set -Wmaybe-uninitialized
> > + * here as it won't fix the error in sunrpc/clnt_udp.c.
> > + * A GCC bug has been filed here:
> > + *    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91691
> > + */
> > +DIAG_PUSH_NEEDS_COMMENT;
> > +DIAG_IGNORE_NEEDS_COMMENT (9, "-Wmaybe-uninitialized");
> > +
> >  /* Return true if the current time is at the deadline or past it.  */
> >  static inline bool
> >  __deadline_elapsed (struct deadline_current_time current,
> > @@ -120,6 +131,8 @@ __deadline_first (struct deadline left, struct deadline right)
> >      return right;
> >  }
> >
> > +DIAG_POP_NEEDS_COMMENT;
> > +
> >  /* Add TV to the current time and return it.  Returns a special
> >     infinite absolute deadline on overflow.  */
> >  struct deadline __deadline_from_timeval (struct deadline_current_time,
> >
>
>
> --
> Cheers,
> Carlos.