From patchwork Sat Jun 3 01:47:11 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 20765 Received: (qmail 4085 invoked by alias); 3 Jun 2017 01:47:12 -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 4070 invoked by uid 89); 3 Jun 2017 01:47:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.1 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, 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= X-HELO: mail-qk0-f196.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=iIzRxb+s6AH5gD9gjQiNXobvZmrC6MRDWx9YlthqNKM=; b=bbb5rtrpD1mtdFnWpbbOcU23v8SonfQZw0JBNpf3+X2r0r5MM+KUuBIQ4e9RQ64zed 4iC9mnopmkPJtoFGqcvda8WKbovY9X0zHaMOu5ikhR5Y/ugbaoiqYytVvlKhUI9wcjAK tZx6qaf8f45vvVaWWAIed4qvNz9OLD4A/QQcGPerxX88Z/czs/MnxYXle3bcTZYYtDTA 5GcfA++QulJt/NTHJ+3/1j8yawtfFoueo7AJ6xl2VCeR08dD3npHZCibtHYlVZ9F3J/h GObA6zWw/wWtbfeJZDXg9ELKhEnis7FdJFy4RohTFtiR9ScBCMNlRb11EQsyAlL64Qml ydng== X-Gm-Message-State: AKS2vOzJG+kWscgL+nuXRG8hh8/XoSjv7p80xMcnfbnkpN5+SBmYUck/ iDk5CNF4F1342BQev6bCMZ6A/KzGHg== X-Received: by 10.55.33.207 with SMTP id f76mr11982960qki.69.1496454431877; Fri, 02 Jun 2017 18:47:11 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <77f9ac84-620a-a408-c448-14a6978f5d2e@linaro.org> References: <1493901791-13438-1-git-send-email-adhemerval.zanella@linaro.org> <1493901791-13438-2-git-send-email-adhemerval.zanella@linaro.org> <8ceeaabf-110f-31fe-516b-18f2fe6710f8@gotplt.org> <7412360d-c776-532b-e22c-dd3f0048cfba@linaro.org> <7f07b067-a074-d670-88b1-3da917451c71@redhat.com> <77f9ac84-620a-a408-c448-14a6978f5d2e@linaro.org> From: "H.J. Lu" Date: Fri, 2 Jun 2017 18:47:11 -0700 Message-ID: Subject: Re: [PATCH v3 2/2] posix: Implement preadv2 and pwritev2 To: Adhemerval Zanella Cc: Florian Weimer , Zack Weinberg , Siddhesh Poyarekar , GNU C Library On Fri, Jun 2, 2017 at 12:46 PM, Adhemerval Zanella wrote: > > > On 02/06/2017 16:02, Florian Weimer wrote: >> On 06/02/2017 08:19 PM, Adhemerval Zanella wrote: >>> I am even more confident this is indeed a miscompilation from GCC7 branch. >>> Using a previous built x86_64-linux-gnu with GCC6 branch I saw no issue, however >>> using GCC7 there is indeed the test failure. And it seems to show only for kernels >>> with support for p{read,write}v2 syscall, which means it stress the default >>> SYSCALL_CANCEL path. >>> >>> However, running a GCC6 built testcase with a GCC7 build glibc (with testrun.sh) >>> I saw no issue. The GCC7 built test also fails with a GCC6 built glibc. I will >>> check with GCC master to see if this is something only on GCC7 branch or if it >>> is still on master. >>> >>> Also, I seems that for GCC7 only x86_64-linux-gnu is affect on x86. >> >> I see it with GCC 6.3 on Fedora 24, x86-64 as well. It could be a >> broken GCC 7 patch that was backported, or perhaps our constraints for >> the syscall instruction are off. > > My GCC 6.3 was built with build-many-glibcs.py, so I assume it contains > no backports. I just checked with GCC trunk (gcc version 8.0.0 20170602) > built also with build-many-glibcs.py and it does not trigger the issue. The kernel interface for p{readv,writev}{64}v is (unsigned long fd, {const }struct iovec *iov, unsigned long vlen, unsigned long pos_l, unsigned long pos_h) The LO_HI_LONG macro is used to pass offset to the pos_l and pos_h pair. Since pos_h is ignored when size of offset == sizeof of pos_l, x86-64 has #define LO_HI_LONG(val) (val) But the kernel interface for p{readv,writev}{64}v2 is (unsigned long fd, {const }struct iovec *iov, unsigned long vlen, unsigned long pos_l, unsigned long pos_h, int flags) Except for targets which define __ARCH_WANT_COMPAT_SYS_PREADV64V2 and __ARCH_WANT_COMPAT_SYS_PWRITEV64V2, (unsigned long fd, {const }struct iovec *iov, unsigned long vlen, unsigned long pos, int flags) will be used for p{readv,writev}{64}v2. X32 is the only such target. The x86-64 LO_HI_LONG can't be used for p{readv,writev}{64}v2. Add a new macro, LO_HI_LONG_FLAGS, to pass the off{64}_t and flags arguments. Please test it on other aches. Thanks. From 5750b7e3f971e02e1d5c676484eb10c779c6ac13 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Fri, 2 Jun 2017 18:31:16 -0700 Subject: [PATCH] Use LO_HI_LONG_FLAGS for p{readv,writev}{64}v2 The kernel interface for p{readv,writev}{64}v is (unsigned long fd, {const }struct iovec *iov, unsigned long vlen, unsigned long pos_l, unsigned long pos_h) The LO_HI_LONG macro is used to pass offset to the pos_l and pos_h pair. Since pos_h is ignored when size of offset == sizeof of pos_l, x86-64 has #define LO_HI_LONG(val) (val) But the kernel interface for p{readv,writev}{64}v2 is (unsigned long fd, {const }struct iovec *iov, unsigned long vlen, unsigned long pos_l, unsigned long pos_h, int flags) Except for targets which define __ARCH_WANT_COMPAT_SYS_PREADV64V2 and __ARCH_WANT_COMPAT_SYS_PWRITEV64V2, (unsigned long fd, {const }struct iovec *iov, unsigned long vlen, unsigned long pos, int flags) will be used for p{readv,writev}{64}v2. X32 is the only such target. The x86-64 LO_HI_LONG can't be used for p{readv,writev}{64}v2. Add a new macro, LO_HI_LONG_FLAGS, to pass the off{64}_t and flags arguments. Tested on i686, x32 and x86-64. * sysdeps/unix/sysv/linux/preadv2.c (preadv2): Replace LO_HI_LONG with LO_HI_LONG_FLAGS. * sysdeps/unix/sysv/linux/preadv64v2.c (preadv64v2): Likewise. * sysdeps/unix/sysv/linux/pwritev2.c (pwritev2): Likewise. * sysdeps/unix/sysv/linux/pwritev64v2.c (pwritev64v2): Likewise. * sysdeps/unix/sysv/linux/sysdep.h (LO_HI_LONG_FLAGS): New. * sysdeps/unix/sysv/linux/x86_64/sysdep.h (LO_HI_LONG_FLAGS): Likewise. * sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h (LO_HI_LONG_FLAGS): Likewise. --- sysdeps/unix/sysv/linux/preadv2.c | 2 +- sysdeps/unix/sysv/linux/preadv64v2.c | 2 +- sysdeps/unix/sysv/linux/pwritev2.c | 2 +- sysdeps/unix/sysv/linux/pwritev64v2.c | 2 +- sysdeps/unix/sysv/linux/sysdep.h | 7 +++++++ sysdeps/unix/sysv/linux/x86_64/sysdep.h | 5 +++++ sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h | 5 +++++ 7 files changed, 21 insertions(+), 4 deletions(-) diff --git a/sysdeps/unix/sysv/linux/preadv2.c b/sysdeps/unix/sysv/linux/preadv2.c index 11fe85e..51f7d2c 100644 --- a/sysdeps/unix/sysv/linux/preadv2.c +++ b/sysdeps/unix/sysv/linux/preadv2.c @@ -31,7 +31,7 @@ preadv2 (int fd, const struct iovec *vector, int count, off_t offset, { # ifdef __NR_preadv2 ssize_t result = SYSCALL_CANCEL (preadv2, fd, vector, count, - LO_HI_LONG (offset), flags); + LO_HI_LONG_FLAGS (offset, flags)); if (result >= 0 || errno != ENOSYS) return result; # endif diff --git a/sysdeps/unix/sysv/linux/preadv64v2.c b/sysdeps/unix/sysv/linux/preadv64v2.c index 9d7f8c9..c1960de 100644 --- a/sysdeps/unix/sysv/linux/preadv64v2.c +++ b/sysdeps/unix/sysv/linux/preadv64v2.c @@ -29,7 +29,7 @@ preadv64v2 (int fd, const struct iovec *vector, int count, off64_t offset, { #ifdef __NR_preadv64v2 ssize_t result = SYSCALL_CANCEL (preadv64v2, fd, vector, count, - LO_HI_LONG (offset), flags); + LO_HI_LONG_FLAGS (offset, flags)); if (result >= 0 || errno != ENOSYS) return result; #endif diff --git a/sysdeps/unix/sysv/linux/pwritev2.c b/sysdeps/unix/sysv/linux/pwritev2.c index 72f0471..82685a4 100644 --- a/sysdeps/unix/sysv/linux/pwritev2.c +++ b/sysdeps/unix/sysv/linux/pwritev2.c @@ -27,7 +27,7 @@ pwritev2 (int fd, const struct iovec *vector, int count, off_t offset, { # ifdef __NR_pwritev2 ssize_t result = SYSCALL_CANCEL (pwritev2, fd, vector, count, - LO_HI_LONG (offset), flags); + LO_HI_LONG_FLAGS (offset, flags)); if (result >= 0 || errno != ENOSYS) return result; # endif diff --git a/sysdeps/unix/sysv/linux/pwritev64v2.c b/sysdeps/unix/sysv/linux/pwritev64v2.c index def9a0b..18336cc 100644 --- a/sysdeps/unix/sysv/linux/pwritev64v2.c +++ b/sysdeps/unix/sysv/linux/pwritev64v2.c @@ -29,7 +29,7 @@ pwritev64v2 (int fd, const struct iovec *vector, int count, off64_t offset, { #ifdef __NR_pwritev64v2 ssize_t result = SYSCALL_CANCEL (pwritev64v2, fd, vector, count, - LO_HI_LONG (offset), flags); + LO_HI_LONG_FLAGS (offset, flags)); if (result >= 0 || errno != ENOSYS) return result; #endif diff --git a/sysdeps/unix/sysv/linux/sysdep.h b/sysdeps/unix/sysv/linux/sysdep.h index 1c24766..1b9ad3a 100644 --- a/sysdeps/unix/sysv/linux/sysdep.h +++ b/sysdeps/unix/sysv/linux/sysdep.h @@ -63,6 +63,13 @@ (long) (val), \ (long) (((uint64_t) (val)) >> 32) +/* Provide a macro to pass the off{64}_t and flags arguments on + p{readv,writev}{64}v2. */ +#define LO_HI_LONG_FLAGS(val, flags) \ + (long) (val), \ + (long) (((uint64_t) (val)) >> 32), \ + (int) (flags) + /* Exports the __send symbol on send.c linux implementation (some ABI have it missing due the usage of a old generic version without it). */ #define HAVE_INTERNAL_SEND_SYMBOL 1 diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h index 7b8bd79..a3fe2fa 100644 --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h @@ -389,4 +389,9 @@ #undef LO_HI_LONG #define LO_HI_LONG(val) (val) +/* Provide a macro to pass the off{64}_t and flags arguments on + p{readv,writev}{64}v2. */ +#undef LO_HI_LONG_FLAGS +#define LO_HI_LONG_FLAGS(val, flags) (val), 0, (flags) + #endif /* linux/x86_64/sysdep.h */ diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h index f90fcfa..b694202 100644 --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h @@ -22,4 +22,9 @@ #include #include +/* Provide a macro to pass the off{64}_t and flags arguments on + p{readv,writev}{64}v2. */ +#undef LO_HI_LONG_FLAGS +#define LO_HI_LONG_FLAGS(val, flags) (val), (flags) + #endif /* linux/x86_64/x32/sysdep.h */ -- 2.9.4