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


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
>


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