This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] powerpc: Fix unitialized variable


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]