Message ID | 20190916221536.18500-1-alistair.francis@wdc.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 18742 invoked by alias); 16 Sep 2019 22:19:46 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 18692 invoked by uid 89); 16 Sep 2019 22:19:45 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT autolearn=ham version=3.3.1 spammy=filed, diag_*, Francis, DIAG_* X-HELO: esa4.hgst.iphmx.com DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=wdc.com; i=@wdc.com; q=dns/txt; s=dkim.wdc.com; t=1568672384; x=1600208384; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=Bi/lxYPudxVNjp5Wn13pRebCLXSRWCmgKiuc7CI5BUQ=; b=XJs7Lan4UY//XNGZrD5ikHpJrSOYioKPGtT2I+MuwZvFKVu6/jMZn/+P dja2q7r2AsQfM4rj7PP/e9AtfBiiaolWWApgT7ATSDlN9f7XNfLWxRIWy HyjLSD2oCXHmKpJ6gYGDKR3DyJ0zavJ2u3MyXC3oH7sZEvK0CeXJYPmzW a8RLyQGCfxEyn9l7Y+dZujBBvYOcT5ccOH6Cca1ZyK3upTYKZEHZxWy1t oNH9hPwAjhLEC9eaBelpEyBLghVblEgXBdA4ecP9Oo6w5ESwgYhQoDqd/ MG0j8WLfb0gA+df3RpsI/e3btX71madHurmi/ZXPFjqS9iM7IZuLhYQjg w==; IronPort-SDR: dvdTLW18x/Ajg8OViOhAZMwTsiZKJnXKjXrPDTi4Zrx8UHWBrZk630ak+mxOoDhiYrc8iuXHHq Ehf3xHaWTp+wAszkZdWSfladNy8JIye1h1qtMonVE9CG28dn68F3fZcdGTf00JxO9qsMlq8cSw caY4uBbuLCidyWRLT2j0Ed1d5W0eX9BIs2AIqv4cNWRhAGnQMDyWdMDQLPaHPdff5aduU6UlJb hnBDLEOz3YXgIpX2KYXMmN3COfBzDaogMIGPFkf572n5H+IOoL2U39U6l0bycTVU3cr6LvTQex lSw= IronPort-SDR: LbKoRigmtjq2ldxvjRWekeMtUGcHkZfgmxeVlepyxCWpKdezqH3X2K9NERyOpqn6RhD/YibN1u um0d+p2OGfbknicUHVO4o86Y5KOOwH6Quq1dFoAZ+/Y4R89CL/xbxi9YgI44DBz/cmaefsnCPF C+gH0d9uZadaOCU+LG6Se62NZeJlORVq0Ex09ujv0vRu9vGIxh9FawrHsRbN2S6UNGFi5P0RHn 5fwApnkD85yWUP5hU+SEDSzCk2NOykQmC5g+pqyp1wDCBA8p4w3GojmyCDizsz+2aWGVbJggn6 CiFGa2t4ysRMnppca+W41rX4 IronPort-SDR: T38ankBPAhOCHZllP7okB24v8t5rH0wnZ3A9PMoyO0llnCPao0OR//0qCwOZIaG1WNqFCybLBF NOPWRMxi6b7ts2HrTH1AVxDPL9OoYBhN0XgksGSDhWTmepR0fwotUkRN4rxL9Tc2cLZP8VkFZD hfaFh7PTolEL2rGtII/TKHt6qpBg8KP3k0Yc9EWJbjOuzVz+0xrgAE6OTo5Dcn/0Tuh/13uJqO vvM7UmSJSb7HB8briURbedHVMdU0ubLO7/hMGtlb6cnLTimczmwTBoUzsPer0kirZSH9Ic8KIV AVQ= WDCIronportException: Internal From: Alistair Francis <alistair.francis@wdc.com> To: libc-alpha@sourceware.org Cc: law@redhat.com, joseph@codesourcery.com, zackw@panix.com, alistair.francis@wdc.com, macro@wdc.com, alistair23@gmail.com Subject: [PATCH] inet/net-internal.h: Fix uninitalised clntudp_call() variable Date: Mon, 16 Sep 2019 15:15:36 -0700 Message-Id: <20190916221536.18500-1-alistair.francis@wdc.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit |
Commit Message
Alistair Francis
Sept. 16, 2019, 10:15 p.m. UTC
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 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(+)
Comments
On Mon, Sep 16, 2019 at 3:19 PM Alistair Francis <alistair.francis@wdc.com> 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 > > 2019-09-16 Alistair Francis <alistair.francis@wdc.com> > > * inet/net-internal.h: Fix uninitalised clntudp_call() variable Ping! Is this patch ok to go in? Alistair > --- > 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, > -- > 2.23.0 >
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. 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, >
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.
On Tue, 24 Sep 2019, Alistair Francis wrote: > > 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 :) I have committed it on your behalf, with a couple of minor formatting nits fixed in the commit message (double space after a full stop) and the ChangeLog entry (full stop at the end of a sentence). Thanks for your contribution. Maciej
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,