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: Fri, 12 Dec 2014 09:35:28 -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> <5489B83F dot 6070007 at linux dot vnet dot ibm dot com>
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
>