This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2] Test errno setup



On 09/03/2017 16:00, Stefan Liebler wrote:
> On 03/08/2017 05:20 PM, Zack Weinberg wrote:
>> On 03/06/2017 09:42 PM, Yury Norov wrote:
>>> On Mon, Mar 06, 2017 at 05:49:19PM -0300, Wainer dos Santos Moschetta wrote:
>>>> LGTM.
>>> Thanks. I don't have the write access to the glibc repo. Could you
>>> (someone else) apply the patch?
>>
>> I have committed the patch.
>>
>> zw
>>
>>
> Hi,
> 
> on s390 (31bit), I get the following fails:
> FAIL: misc/test-errno:
> FAIL: mlock: errno is: 12 (Cannot allocate memory) expected: 22 (Invalid argument)
> 
> FAIL: posix/test-errno:
> FAIL: mlock: errno is: 12 (Cannot allocate memory) expected: 22 (Invalid argument)
> 
> Is it intended, that the same test is run twice?

It is since although they have the same name they are different test
in fact: posix/test-errno.c only uses POSIX defined interfaces while
sysdeps/unix/sysv/linux/test-errno.c check for Linux specific ones.

> Both are compiled with sysdeps/unix/sysv/linux/test-errno.c.

But this indeed the issue and it need to be fixed. I am not sure if
glibc Makefile system can handle test with same name in multiple
paths (and I personally not compiling to actually debug if it is
the case), so I would recommend to just rename Linux specific one
to test-errno-linux.c.

> Or should there two different tests, one compiled
> with posix/test-errno.c
> and the other with sysdeps/unix/sysv/linux/test-errno.c?
> 
> Why is the test-errno added to tests in sysdeps/unix/sysv/linux/Makefile with:
> ifeq ($(subdir),misc)
> tests += test-errno
> endif

Afaik it is basically to put the resulting objects/binaries on misc folder
(where usually Linux-only tests are placed).

> 
> 
> 
> Regarding mlock-syscall:
> If the compat mlock syscall is used, it returns 12 (ENOMEM).
> This is also observable if you compile and run the testcase with -m32 on a x86_64 system.
> 
> 
> I've compiled and run posix/test-errno.c on my s390x system and
> get the following error:
> FAIL: setsockopt: errno is: 22 (Invalid argument) expected: 9 (Bad file descriptor)
> 
> sl=0xfdfa9170 before setsockopt syscall.
> The test succeeds if I sl is initialized to zero.
> 

POSIX [1] states that mlock should may fail with EINVAL only if
the addr argument is not a multiple of {PAGESIZE}.  Linux does
not return this issue, since it aligns the resulting address
to pagesize:

* mm/mlock.c:

666 static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t flags)
667 {
[...]
676 
677         len = PAGE_ALIGN(len + (offset_in_page(start)));
678         start &= PAGE_MASK;

EINVAL is only returned on 'apply_vma_lock_flags':

578 static int apply_vma_lock_flags(unsigned long start, size_t len,
579                                 vm_flags_t flags)
580 {
[...]
587         end = start + len;
588         if (end < start)
589                 return -EINVAL;

But if you runs 32 binaries on 64 bits kernel overflow won't happen.
It is documented in man pages, so I think from kernel side it should
be consistent for 32 bit binaries on 64 bit kernel.

For glibc side, I think we should do something like:

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-linux.c b/sysdeps/unix/sysv/linux/test-errno-linux.c
index ab3735f..c3facd5 100644
--- a/sysdeps/unix/sysv/linux/test-errno-linux.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 kernel, 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);

[1] http://pubs.opengroup.org/onlinepubs/9699919799/

> Bye
> Stefan
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]