diff mbox series

avoid GCC 10 test suite warnings (BZ #25219)

Message ID aa77e06f-5420-1c48-b1f8-3c689560e98f@gmail.com
State Committed
Headers
Series avoid GCC 10 test suite warnings (BZ #25219) |

Commit Message

Martin Sebor May 7, 2020, 5:58 p.m. UTC 
  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

Florian Weimer May 8, 2020, 4:04 p.m. UTC | #1 
* 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
Martin Sebor May 11, 2020, 3:33 p.m. UTC | #2 
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
Florian Weimer May 11, 2020, 3:41 p.m. UTC | #3 
* 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
diff mbox series

Patch

diff --git a/io/tst-getcwd.c b/io/tst-getcwd.c
index 75ecd2c7b9..c9c4713fc3 100644
--- a/io/tst-getcwd.c
+++ b/io/tst-getcwd.c
@@ -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);
diff --git a/posix/test-errno.c b/posix/test-errno.c
index 6afadcd102..e62efe2638 100644
--- a/posix/test-errno.c
+++ b/posix/test-errno.c
@@ -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);