From patchwork Thu Mar 9 16:30:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 19502 Received: (qmail 118651 invoked by alias); 9 Mar 2017 16:30:35 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 118330 invoked by uid 89); 9 Mar 2017 16:30:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=579, Bye, 580 X-HELO: mail-wm0-f52.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=G1mmZJQmiw+C5HW94xjPFkmqZmAw5aEpsMRbC0Ic07s=; b=Ble160zuBplF6RFiw/ITfe+50wq8L84exvHKm2JvE3FjXpB5CxxjCiysmqyTwrvZFE CxA+ojYkr5YfvRKT7UJvURGgT41PGhMyjxO4K5OEqNn6flUtR7W3ysXi5WWVqF+uFcko HbWK5LHRg/NyoYFCi4ufZNW6mv/I+JWfjKGAFrq0/XDQ37hKNeunao0Y8zMjMdNZi3KW WJFKBHjy2GjFnbqZEfXkIBxsYDZ9I+38EWYJ8XXg0auuu5VDLXYvghoacEfa4rLSFL15 TzZ0+pCY42PCRSnzNsp5Zwh6I+xPqfnhDCooVdWlnUdAdzehzy8v/yVwGUV8T+iQTtDs xoig== X-Gm-Message-State: AMke39naesYRvvomBlSMJR3cgPVULYwT2sz5JkN3rldBEhbCxgLwkRcp6iZzfkbarz2kZNPG X-Received: by 10.28.165.147 with SMTP id o141mr11327147wme.67.1489077006338; Thu, 09 Mar 2017 08:30:06 -0800 (PST) Subject: Re: [PATCH v2] Test errno setup To: libc-alpha@sourceware.org References: <1487411274-21877-1-git-send-email-ynorov@caviumnetworks.com> <20170307024254.GB7273@yury-N73SV> From: Adhemerval Zanella Message-ID: Date: Thu, 9 Mar 2017 17:30:03 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: 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: [1] http://pubs.opengroup.org/onlinepubs/9699919799/ > Bye > Stefan > 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);