powerpc: Fix unitialized variable
Commit Message
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.
--
Comments
>> * 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
>
@@ -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