avoid GCC 10 test suite warnings (BZ #25219)
Commit Message
The attached patch suppresses a few GCC 10 true positive warnings
in tests that exercise failure modes by making calls with invalid
arguments. With the patch, GCC 10 compiles all tests without
an error.
Martin
PS I'm not sure the readlink test is quite correct. If its purpose
is to verify that readlink fails for "/" because it's not a symbolic
link then it shouldn't also pass an excessive size as the last
argument, because it makes the call undefined. If, on the other
hand, its purpose is to verify the function fails with EINVAL when
"bufsiz is not positive" (as documented on the Linux man page(*)
but not in the Glibc manual) then that failure mode should be
tested separately from the first. That said, I'm not sure POSIX
allows the function to fail with EINVAL in this case because that
would make the two failures indistinguishable.
http://man7.org/linux/man-pages/man2/readlink.2.html
Comments
* Martin Sebor via Libc-alpha:
> +
> +#if __GNUC_PREREQ (7, 0)
> + DIAG_PUSH_NEEDS_COMMENT;
> + /* Avoid warnings about the second (size) argument being excessive. */
> + DIAG_IGNORE_NEEDS_COMMENT (10.1, "-Wstringop-overflow");
> +#endif
> fails |= test_wrp (EINVAL, readlink, "/", buf, -1);
> +#if __GNUC_PREREQ (7, 0)
> + DIAG_POP_NEEDS_COMMENT;
> +#endif
I'd suggest to use
fails |= test_wrp (EINVAL, readlink, "/", buf, sizeof (buf));
here, for the reason you explained.
Thanks,
Florian
On 5/8/20 10:04 AM, Florian Weimer wrote:
> * Martin Sebor via Libc-alpha:
>
>> +
>> +#if __GNUC_PREREQ (7, 0)
>> + DIAG_PUSH_NEEDS_COMMENT;
>> + /* Avoid warnings about the second (size) argument being excessive. */
>> + DIAG_IGNORE_NEEDS_COMMENT (10.1, "-Wstringop-overflow");
>> +#endif
>> fails |= test_wrp (EINVAL, readlink, "/", buf, -1);
>> +#if __GNUC_PREREQ (7, 0)
>> + DIAG_POP_NEEDS_COMMENT;
>> +#endif
>
> I'd suggest to use
>
> fails |= test_wrp (EINVAL, readlink, "/", buf, sizeof (buf));
>
> here, for the reason you explained.
Sounds good. I will commit the attached patch unless there are more
suggestions for changes.
Martin
* Martin Sebor:
> On 5/8/20 10:04 AM, Florian Weimer wrote:
>> * Martin Sebor via Libc-alpha:
>>
>>> +
>>> +#if __GNUC_PREREQ (7, 0)
>>> + DIAG_PUSH_NEEDS_COMMENT;
>>> + /* Avoid warnings about the second (size) argument being excessive. */
>>> + DIAG_IGNORE_NEEDS_COMMENT (10.1, "-Wstringop-overflow");
>>> +#endif
>>> fails |= test_wrp (EINVAL, readlink, "/", buf, -1);
>>> +#if __GNUC_PREREQ (7, 0)
>>> + DIAG_POP_NEEDS_COMMENT;
>>> +#endif
>>
>> I'd suggest to use
>>
>> fails |= test_wrp (EINVAL, readlink, "/", buf, sizeof (buf));
>>
>> here, for the reason you explained.
>
> Sounds good. I will commit the attached patch unless there are more
> suggestions for changes.
Thanks, this patch looks good to me.
Florian
@@ -23,6 +23,7 @@
#include <string.h>
#include <unistd.h>
#include <sys/param.h>
+#include <libc-diag.h>
#define TEST_FUNCTION do_test ()
@@ -58,7 +59,13 @@ do_test (void)
bufs[i] = (char *) malloc (sbs);
}
+ /* Avoid warnings about the first argument being null when the second
+ is nonzero. */
+ DIAG_PUSH_NEEDS_COMMENT;
+ DIAG_IGNORE_NEEDS_COMMENT (10.1, "-Wnonnull");
bufs[i] = getcwd (NULL, sbs);
+ DIAG_POP_NEEDS_COMMENT;
+
lens[i] = sbs;
if (bufs[i] == NULL)
{
@@ -96,12 +103,17 @@ getcwd (NULL, sbs) = \"%s\", getcwd (thepath, sizeof thepath) = \"%s\"\n",
free (bufs[i]);
/* Test whether the function signals success despite the buffer
- being too small. */
+ being too small.
+ Avoid warnings about the first argument being null when the second
+ is nonzero. */
+ DIAG_PUSH_NEEDS_COMMENT;
+ DIAG_IGNORE_NEEDS_COMMENT (10.1, "-Wnonnull");
if (getcwd (NULL, len) != NULL)
{
puts ("getcwd (NULL, len) didn't failed");
return 1;
}
+ DIAG_POP_NEEDS_COMMENT;
bufs[0] = malloc (len);
bufs[1] = malloc (len);
@@ -132,13 +144,18 @@ getcwd (NULL, sbs) = \"%s\", getcwd (thepath, sizeof thepath) = \"%s\"\n",
return 1;
}
- /* Now test handling of correctly sized buffers. */
+ /* Now test handling of correctly sized buffers.
+ Again. avoid warnings about the first argument being null when
+ the second is nonzero. */
+ DIAG_PUSH_NEEDS_COMMENT;
+ DIAG_IGNORE_NEEDS_COMMENT (10.1, "-Wnonnull");
bufs[0] = getcwd (NULL, len + 1);
if (bufs[0] == NULL)
{
puts ("getcwd (NULL, len + 1) failed");
return 1;
}
+ DIAG_POP_NEEDS_COMMENT;
free (bufs[0]);
memset (thepath, '\xff', sizeof thepath);
@@ -34,6 +34,7 @@
#include <sys/uio.h>
#include <unistd.h>
#include <netinet/in.h>
+#include <libc-diag.h>
/* This is not an exhaustive test: only system calls that can be
persuaded to fail with a consistent error code and no side effects
@@ -119,7 +120,16 @@ do_test (void)
fails |= test_wrp (EBADF, fstatfs, -1, &sfs);
fails |= test_wrp (EBADF, fsync, -1);
fails |= test_wrp (EBADF, ftruncate, -1, 0);
+
+#if __GNUC_PREREQ (7, 0)
+ DIAG_PUSH_NEEDS_COMMENT;
+ /* Avoid warnings about the second (size) argument being negative. */
+ DIAG_IGNORE_NEEDS_COMMENT (10.1, "-Wstringop-overflow");
+#endif
fails |= test_wrp (EINVAL, getgroups, -1, 0);
+#if __GNUC_PREREQ (7, 0)
+ DIAG_POP_NEEDS_COMMENT;
+#endif
fails |= test_wrp (EBADF, getpeername, -1, &sa, &sl);
fails |= test_wrp (EBADF, getsockname, -1, &sa, &sl);
fails |= test_wrp (EBADF, getsockopt, -1, 0, 0, buf, &sl);
@@ -134,7 +144,16 @@ do_test (void)
fails |= test_wrp (EINVAL, munmap, (void *) -1, 0);
fails |= test_wrp (EISDIR, open, "/bin", EISDIR, O_WRONLY);
fails |= test_wrp (EBADF, read, -1, buf, 1);
+
+#if __GNUC_PREREQ (7, 0)
+ DIAG_PUSH_NEEDS_COMMENT;
+ /* Avoid warnings about the second (size) argument being excessive. */
+ DIAG_IGNORE_NEEDS_COMMENT (10.1, "-Wstringop-overflow");
+#endif
fails |= test_wrp (EINVAL, readlink, "/", buf, -1);
+#if __GNUC_PREREQ (7, 0)
+ DIAG_POP_NEEDS_COMMENT;
+#endif
fails |= test_wrp (EBADF, readv, -1, iov, 1);
fails |= test_wrp (EBADF, recv, -1, buf, 1, 0);
fails |= test_wrp (EBADF, recvfrom, -1, buf, 1, 0, &sa, &sl);