This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] powerpc: Fix unitialized variable
- From: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>
- To: libc-alpha at sourceware dot org
- Date: Thu, 11 Dec 2014 13:29:03 -0200
- Subject: Re: [PATCH] powerpc: Fix unitialized variable
- Authentication-results: sourceware.org; auth=none
- References: <54889AAC dot 3060404 at linux dot vnet dot ibm dot com> <alpine dot DEB dot 2 dot 10 dot 1412102119230 dot 26602 at digraph dot polyomino dot org dot uk> <5488DA58 dot 9010505 at linux dot vnet dot ibm dot com> <m6bvp4$tus$1 at ger dot gmane dot org> <mvmsigmpfku dot fsf at hawking dot suse dot de> <m6c0u2$o15$1 at ger dot gmane dot org> <54898F5B dot 2000904 at linux dot vnet dot ibm dot com> <5489925D dot 5010609 at linux dot vnet dot ibm dot com> <alpine dot DEB dot 2 dot 10 dot 1412111432180 dot 19830 at digraph dot polyomino dot org dot uk>
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