[v2] Fix test-errno issues

Submitted by Adhemerval Zanella on March 14, 2017, 8:26 p.m.

Details

Message ID 505e6f6a-e22a-bdc3-062d-92e2a99744a6@linaro.org
State New
Headers show

Commit Message

Adhemerval Zanella March 14, 2017, 8:26 p.m.
And I did not used the -C -M option on this one... the following one is
essentially the same patch formatted with -C -M:


On 14/03/2017 17:24, Adhemerval Zanella wrote:
> Changes from previous version:
> 
>   - Change open tests from EINVAL to EISDIR.
> 
> --
> 
> This patch fixes multiple issues of test-errno.c (9a56f8718341):
> 
>   - Rename Linux test-errno.c to test-errno-linux.c to avoid build
>     the same source for both tests.
> 
>   - Add a mlock check for 32 bits build running on 64 bits kernels.
>     Althuough man pages states that mlock fails with EINVAL if final
>     address overflows, kernels does not return it for aforementioned
>     condition (it returns ENOMEM instead).  Although it seems to be
>     a kernel issue for compat syscall handling, I think it is worth
>     to still check syscall return and document the behavior.
> 
>   - Initialize option length for setsockopt check.
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu (running on 64 bits
> kernel).
> 
> 	* posix/test-errno.c (do_test): Initialize setsockopt optlen.
> 	* sysdeps/unix/sysv/linux/test-errno.c: Move to ...
> 	* sysdeps/unix/sysv/linux/test-errno-linux.c: ... here.
> 	(do_test): Handle mlock return on 64 bits kernels with 32 bits
> 	binaries and change open test.

Comments

Mike Frysinger March 15, 2017, 7:15 a.m.
On 14 Mar 2017 17:26, Adhemerval Zanella wrote:
> +    if (ret == (rtype) -1 && (err == experr1 || err == experr2))	\

not a new issue exactly, but should be parens around experr1 & experr2

> +      fail = 0;								\
> +    else								\
> +      {									\
> +        fail = 1;							\
> +        if (ret != (rtype) -1)						\
> +          printf ("FAIL: " #syscall ": didn't fail as expected"		\
> +               " (return "prtype")\n", ret);				\

shouldn't these be indented with tabs ?

> +        else if (err == 0xdead)						\
> +          puts("FAIL: " #syscall ": didn't update errno\n");		\

missing parens before the (

don't need the \n since it's a puts
-mike
Adhemerval Zanella March 15, 2017, 8:10 p.m.
On 15/03/2017 04:15, Mike Frysinger wrote:
> On 14 Mar 2017 17:26, Adhemerval Zanella wrote:
>> +    if (ret == (rtype) -1 && (err == experr1 || err == experr2))	\
> 
> not a new issue exactly, but should be parens around experr1 & experr2
> 
>> +      fail = 0;								\
>> +    else								\
>> +      {									\
>> +        fail = 1;							\
>> +        if (ret != (rtype) -1)						\
>> +          printf ("FAIL: " #syscall ": didn't fail as expected"		\
>> +               " (return "prtype")\n", ret);				\
> 
> shouldn't these be indented with tabs ?
> 
>> +        else if (err == 0xdead)						\
>> +          puts("FAIL: " #syscall ": didn't update errno\n");		\
> 
> missing parens before the (
> 
> don't need the \n since it's a puts
> -mike

I fixed the format issues and pushed as b36a65e5ca.

Patch hide | download patch | download mbox

diff --git a/posix/test-errno.c b/posix/test-errno.c
index 98df344..c2bfd8a 100644
--- a/posix/test-errno.c
+++ b/posix/test-errno.c
@@ -131,7 +131,7 @@  do_test (void)
   fails |= test_wrp (EINVAL, mprotect, (void *) -1, pagesize, -1);
   fails |= test_wrp (EINVAL, msync, (void *) -1, pagesize, -1);
   fails |= test_wrp (EINVAL, munmap, (void *) -1, 0);
-  fails |= test_wrp (EINVAL, open, "/bin/sh", -1, 0);
+  fails |= test_wrp (EISDIR, open, "/bin", EISDIR, O_WRONLY);
   fails |= test_wrp (EBADF, read, -1, buf, 1);
   fails |= test_wrp (EINVAL, readlink, "/", buf, -1);
   fails |= test_wrp (EBADF, readv, -1, iov, 1);
@@ -142,7 +142,7 @@  do_test (void)
   fails |= test_wrp (EBADF, send, -1, buf, 1, 0);
   fails |= test_wrp (EBADF, sendmsg, -1, &msg, 0);
   fails |= test_wrp (EBADF, sendto, -1, buf, 1, 0, &sa, sl);
-  fails |= test_wrp (EBADF, setsockopt, -1, 0, 0, buf, sl);
+  fails |= test_wrp (EBADF, setsockopt, -1, 0, 0, buf, sizeof (*buf));
   fails |= test_wrp (EBADF, shutdown, -1, SHUT_RD);
   fails |= test_wrp (EBADF, write, -1, "Hello", sizeof ("Hello") );
   fails |= test_wrp (EBADF, writev, -1, iov, 1 );
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 6b7aa3f..1872cdb 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -43,7 +43,7 @@  sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
 		  bits/mman-linux.h
 
 tests += tst-clone tst-clone2 tst-fanotify tst-personality tst-quota \
-	 tst-sync_file_range test-errno
+	 tst-sync_file_range test-errno-linux
 
 # Generate the list of SYS_* macros for the system calls (__NR_* macros).
 
diff --git a/sysdeps/unix/sysv/linux/test-errno.c b/sysdeps/unix/sysv/linux/test-errno-linux.c
similarity index 77%
rename from sysdeps/unix/sysv/linux/test-errno.c
rename to sysdeps/unix/sysv/linux/test-errno-linux.c
index ab3735f..b00d14e 100644
--- a/sysdeps/unix/sysv/linux/test-errno.c
+++ b/sysdeps/unix/sysv/linux/test-errno-linux.c
@@ -1,4 +1,5 @@ 
 /* Test that failing system calls do set errno to the correct value.
+   Linux sycalls version.
 
    Copyright (C) 2017 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
@@ -90,9 +91,37 @@ 
     fail;							\
   }))
 
+#define test_wrp_rv2(rtype, prtype, experr1, experr2, syscall, ...) 	\
+  (__extension__ ({							\
+    errno = 0xdead;							\
+    rtype ret = syscall (__VA_ARGS__);					\
+    int err = errno;							\
+    int fail;								\
+    if (ret == (rtype) -1 && (err == experr1 || err == experr2))	\
+      fail = 0;								\
+    else								\
+      {									\
+        fail = 1;							\
+        if (ret != (rtype) -1)						\
+          printf ("FAIL: " #syscall ": didn't fail as expected"		\
+               " (return "prtype")\n", ret);				\
+        else if (err == 0xdead)						\
+          puts("FAIL: " #syscall ": didn't update errno\n");		\
+        else if (err != experr1 && err != experr2)			\
+          printf ("FAIL: " #syscall					\
+               ": errno is: %d (%s) expected: %d (%s) or %d (%s)\n",	\
+               err, strerror (err), experr1, strerror (experr1),	\
+               experr2, strerror (experr2));				\
+      }									\
+    fail;								\
+  }))
+
 #define test_wrp(experr, syscall, ...)				\
   test_wrp_rv(int, "%d", experr, syscall, __VA_ARGS__)
 
+#define test_wrp2(experr1, experr2, syscall, ...)		\
+  test_wrp_rv2(int, "%d", experr1, experr2, syscall, __VA_ARGS__)
+
 static int
 do_test (void)
 {
@@ -120,7 +149,12 @@  do_test (void)
   fails |= test_wrp (ESRCH, getpgid, -1);
   fails |= test_wrp (EINVAL, inotify_add_watch, -1, "/", 0);
   fails |= test_wrp (EINVAL, mincore, (void *) -1, 0, vec);
-  fails |= test_wrp (EINVAL, mlock, (void *) -1, 1); // different errors
+  /* mlock fails if the result of the addition addr+len was less than addr
+     (which indicates final address overflow), however on 32 bits binaries
+     running on 64 bits kernels, internal syscall address check won't result
+     in an invalid address and thus syscalls fails later in vma
+     allocation.  */
+  fails |= test_wrp2 (EINVAL, ENOMEM, mlock, (void *) -1, 1);
   fails |= test_wrp (EINVAL, nanosleep, &ts, &ts);
   fails |= test_wrp (EINVAL, poll, &pollfd, -1, 0);
   fails |= test_wrp (ENODEV, quotactl, Q_GETINFO, NULL, -1, (caddr_t) &dqblk);