| Message ID | 5489B83F.6070007@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
>> * Do you actually need this here, or only around the use of the variable? > > Yes, in my testing we need on both places to silent the compiler. On s390x, too. > + by "*thisresplenp = rlen;" will be executed so that subsequent > + times round the loop resplen has been initialized. So this is After "So this is" is a whitespace. > + a false-positive. > + */ > +#if __GNUC_PREREQ (4, 7) > + DIAG_PUSH_NEEDS_COMMENT; tested on s390x with gcc 4.8 4.9, gcc head. Thanks.
Ping, this is the remaining fix to enable powerpc build with -werror. PS: I fix the the comment trialing whitespace noted by Stefan. On 11-12-2014 13:29, Adhemerval Zanella wrote: > On 11-12-2014 12:40, Joseph Myers wrote: >> On Thu, 11 Dec 2014, Adhemerval Zanella wrote: >> >>> + int truncating, connreset, n; >>> + /* There is the following warning on some architectures: >>> + 'resplen' may be used uninitialized in this function >>> + [-Wmaybe-uninitialized] >>> + This is a false positive according to: >>> + https://www.sourceware.org/ml/libc-alpha/2014-12/msg00323.html >>> + */ >>> + DIAG_PUSH_NEEDS_COMMENT; >>> + DIAG_IGNORE_NEEDS_COMMENT (4.7, "-Wmaybe-uninitialized"); >>> + int resplen; >>> + DIAG_POP_NEEDS_COMMENT; >> * Do you actually need this here, or only around the use of the variable? > Yes, in my testing we need on both places to silent the compiler. > >> * An actual analysis of why the variable can't be used uninitialized would >> be better than a URL. I.e., if buf2 == NULL then this code won't be >> executed; if buf2 != NULL, then first time round the loop recvresp1 and >> recvresp2 will be 0 so this code won't be executed but "thisresplenp = >> &resplen;" followed by "*thisresplenp = rlen;" will be executed so that >> subsequent times round the loop resplen has been initialized. > I will replace the ULR pointer with a comment. > >> * The version number in DIAG_IGNORE_NEEDS_COMMENT is the most recent GCC >> version with which the issue has been observed, not the oldest. > Right, I thought I saw it failing by using something different that the compiler > used (GCC 4.6 in my tests), but it was a mistake from my part. Changed to 5. > >> * A conditional __GNUC_PREREQ (4, 7) is needed around the >> DIAG_IGNORE_NEEDS_COMMENT call because 4.6 doesn't have >> -Wmaybe-uninitialized (if the warnings appear with 4.6, a #else case to >> use -Wuninitialized instead with 4.6 will be needed). >> > I added the guards. I checked with GCC 4.6 and building with it does not > shows the warnings. > > What about now: > > -- > > 2014-12-11 Stefan Liebler <stli@linux.vnet.ibm.com> > Adhemerval Zanella <azanella@linux.vnet.ibm.com> > > * resolv/res_send.c (send_vc): Disable warning resplen may > be used uninitialized. > > -- > > diff --git a/resolv/res_send.c b/resolv/res_send.c > index af42b8a..9ec951a 100644 > --- a/resolv/res_send.c > +++ b/resolv/res_send.c > @@ -96,6 +96,7 @@ static const char rcsid[] = "$BINDId: res_send.c,v 8.38 2000/03/30 20:16:51 vixi > #include <string.h> > #include <unistd.h> > #include <kernel-features.h> > +#include <libc-internal.h> > > #if PACKETSZ > 65536 > #define MAXPACKET PACKETSZ > @@ -668,7 +669,24 @@ send_vc(res_state statp, > // int anssiz = *anssizp; > HEADER *anhp = (HEADER *) ans; > struct sockaddr_in6 *nsap = EXT(statp).nsaddrs[ns]; > - int truncating, connreset, resplen, n; > + int truncating, connreset, n; > + /* On some architectures compiler might emit a warning indicating > + 'resplen' may be used uninitialized. However if buf2 == NULL > + then this code won't be executed; if buf2 != NULL, then first > + time round the loop recvresp1 and recvresp2 will be 0 so this > + code won't be executed but "thisresplenp = &resplen;" followed > + by "*thisresplenp = rlen;" will be executed so that subsequent > + times round the loop resplen has been initialized. So this is > + a false-positive. > + */ > +#if __GNUC_PREREQ (4, 7) > + DIAG_PUSH_NEEDS_COMMENT; > + DIAG_IGNORE_NEEDS_COMMENT (5, "-Wmaybe-uninitialized"); > +#endif > + int resplen; > +#if __GNUC_PREREQ (4, 7) > + DIAG_POP_NEEDS_COMMENT; > +#endif > struct iovec iov[4]; > u_short len; > u_short len2; > @@ -787,6 +805,10 @@ send_vc(res_state statp, > /* No buffer allocated for the first > reply. We can try to use the rest > of the user-provided buffer. */ > +#if __GNUC_PREREQ (4, 7) > + DIAG_PUSH_NEEDS_COMMENT; > + DIAG_IGNORE_NEEDS_COMMENT (5, "-Wmaybe-uninitialized"); > +#endif > #if _STRING_ARCH_unaligned > *anssizp2 = orig_anssizp - resplen; > *ansp2 = *ansp + resplen; > @@ -797,6 +819,9 @@ send_vc(res_state statp, > *anssizp2 = orig_anssizp - aligned_resplen; > *ansp2 = *ansp + aligned_resplen; > #endif > +#if __GNUC_PREREQ (4, 7) > + DIAG_POP_NEEDS_COMMENT; > +#endif > } else { > /* The first reply did not fit into the > user-provided buffer. Maybe the second >
Looks OK
diff --git a/resolv/res_send.c b/resolv/res_send.c index af42b8a..9ec951a 100644 --- a/resolv/res_send.c +++ b/resolv/res_send.c @@ -96,6 +96,7 @@ static const char rcsid[] = "$BINDId: res_send.c,v 8.38 2000/03/30 20:16:51 vixi #include <string.h> #include <unistd.h> #include <kernel-features.h> +#include <libc-internal.h> #if PACKETSZ > 65536 #define MAXPACKET PACKETSZ @@ -668,7 +669,24 @@ send_vc(res_state statp, // int anssiz = *anssizp; HEADER *anhp = (HEADER *) ans; struct sockaddr_in6 *nsap = EXT(statp).nsaddrs[ns]; - int truncating, connreset, resplen, n; + int truncating, connreset, n; + /* On some architectures compiler might emit a warning indicating + 'resplen' may be used uninitialized. However if buf2 == NULL + then this code won't be executed; if buf2 != NULL, then first + time round the loop recvresp1 and recvresp2 will be 0 so this + code won't be executed but "thisresplenp = &resplen;" followed + by "*thisresplenp = rlen;" will be executed so that subsequent + times round the loop resplen has been initialized. So this is + a false-positive. + */ +#if __GNUC_PREREQ (4, 7) + DIAG_PUSH_NEEDS_COMMENT; + DIAG_IGNORE_NEEDS_COMMENT (5, "-Wmaybe-uninitialized"); +#endif + int resplen; +#if __GNUC_PREREQ (4, 7) + DIAG_POP_NEEDS_COMMENT; +#endif struct iovec iov[4]; u_short len; u_short len2; @@ -787,6 +805,10 @@ send_vc(res_state statp, /* No buffer allocated for the first reply. We can try to use the rest of the user-provided buffer. */ +#if __GNUC_PREREQ (4, 7) + DIAG_PUSH_NEEDS_COMMENT; + DIAG_IGNORE_NEEDS_COMMENT (5, "-Wmaybe-uninitialized"); +#endif #if _STRING_ARCH_unaligned *anssizp2 = orig_anssizp - resplen; *ansp2 = *ansp + resplen; @@ -797,6 +819,9 @@ send_vc(res_state statp, *anssizp2 = orig_anssizp - aligned_resplen; *ansp2 = *ansp + aligned_resplen; #endif +#if __GNUC_PREREQ (4, 7) + DIAG_POP_NEEDS_COMMENT; +#endif } else { /* The first reply did not fit into the user-provided buffer. Maybe the second