This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v3 2/2] posix: Implement preadv2 and pwritev2
On Fri, Jun 2, 2017 at 12:46 PM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> 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.
--
H.J.
From 5750b7e3f971e02e1d5c676484eb10c779c6ac13 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
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 <sysdeps/unix/sysv/linux/x86_64/sysdep.h>
#include <sysdeps/x86_64/x32/sysdep.h>
+/* 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