Message ID | 1472041476-14664-1-git-send-email-ynorov@caviumnetworks.com |
---|---|
State | New |
Headers | show |
On 24/08/2016 09:24, Yury Norov wrote: > Hi Adhemerval, > > Current __lseek() implementation under > sysdeps/unix/sysv/linux/generic/wordsize-32/lseek.c > is broken if off_t is 64-bit type. In this patch it is redirected to 64-bit > __llseek() version. It helps to avoid introducing platform implementation for > __lseek() under aarch64/ilp32. > > Notice that using SYSCALL_LL64() makes LTP hang for me (ltp-pan itself), > so I don't use it. I am also not sure this patch does not affect other > ports, and that there's no alternative implementations in glibc that we'd > cleenup. So I marked it as RFC. Please take a look. If you find it helpful, > I'll handle SYSCALL_LL64() issue and resend it. > I think the patch is incomplete in the sense that my idea is to consolidate all the implementation at 'sysdeps/unix/sysv/linux' instead of generic one. Also, based on kernel lseek/llseek syscall definition: arch/mips/kernel/linux32.c:99:SYSCALL_DEFINE5(32_llseek, unsigned int, fd, unsigned int, offset_high, arch/tile/kernel/compat.c:90:COMPAT_SYSCALL_DEFINE5(llseek, unsigned int, fd, unsigned int, offset_high, arch/s390/kernel/compat_wrapper.c:97:COMPAT_SYSCALL_WRAP5(llseek, unsigned int, fd, unsigned long, high, unsigned long, low, loff_t __user *, result, unsigned int, whence); fs/read_write.c:305:SYSCALL_DEFINE3(lseek, unsigned int, fd, off_t, offset, unsigned int, whence) fs/read_write.c:324:COMPAT_SYSCALL_DEFINE3(lseek, unsigned int, fd, compat_off_t, offset, unsigned int, whence) fs/read_write.c:331:SYSCALL_DEFINE5(llseek, unsigned int, fd, unsigned long, offset_high, I follows the preadv/pwritev of explicit splitting the off_t argument instead of relying on ABI. So SYSCALL_LL{64} won't work on this, we should use LO_HI_LONG instead. My initial prototype on how to consolidate lseek/lseek64/llseek would be something like the below. I haven't tested it yet and it will prop require some work: - lseek: ABIs with __OFF_T_MATCHES_OFF64_T != 1 will preferable use __NR__llseek if kernel supports it, otherwise it will use __NR_lseek. - lseek64: ABIs with __OFF_T_MATCHES_OFF64_T == 1 will use __NR_lseek. - _llseek: files will be removed and alias to lseek64. In this way ABI with __OFF_T_MATCHES_OFF64_T == 1 will have a empty lseek.c and use __NR_lseek for lseek64.c (with correct alias for _llseek). One possible optimization that I did not added in snippet below, but occurred to me now is for __OFF_T_MATCHES_OFF64_T != 1 where _NR__llseek is defined we can just skip lseek.c compilation and alias the lseek64 to all required lseek symbols. If it is the case where _NR__llseek is defined on all supported arches for minimum current kernel I think we can simplify the strategy below. * sysdeps/unix/sysv/linux/lseek.c #ifndef __OFF_T_MATCHES_OFF64_T off_t __lseek (int fd, off_t offset, int whence) { #ifdef _NR__llseek loff_t res; int rc = INLINE_SYSCALL_CALL (_llseek, fd, LO_HI_LONG (offset), &res, whence); return rc ?: lseek_overflow (res); #else return INLINE_SYSCALL_CALL (lseek, fd, offset, whence); #endif } libc_hidden_def (__lseek) weak_alias (__lseek, lseek) strong_alias (__lseek, __libc_lseek) #endif * lseek64.c off64_t __lseek64 (int fd, off64_t offset, int whence) { #if __OFF_T_MATCHES_OFF64_T return INLINE_SYSCALL_CALL (lseek, fd, offset, whence); #else loff_t res; int rc = INLINE_SYSCALL_CALL (_llseek, fd, LO_HI_LONG (offset), &res, whence); return rc ?: lseek_overflow (res); #endif } #ifndef __OFF_T_MATCHES_OFF64_T weak_alias (__lseek64, lseek) weak_alias (__lseek64, __lseek) strong_alias (__lseek64, __libc_lseek) libc_hidden_def (__lseek) #endif strong_alias (__lseek64, __libc_lseek64) strong_alias (__lseek64, __lseek64) weak_alias (__lseek64, lseek64) /* llseek doesn't have a prototype. Since the second parameter is a 64bit type, this results in wrong behaviour if no prototype is provided. */ strong_alias (__lseek64, _llseek) link_warning (llseek, "\ the `llseek' function may be dangerous; use `lseek64' instead.") llseek.c /* Remove file. */ What do you think? > Yury. > > Signed-off-by: Yury Norov <ynorov@caviumnetworks.com> > --- > sysdeps/unix/sysv/linux/generic/wordsize-32/llseek.c | 7 +++++++ > sysdeps/unix/sysv/linux/generic/wordsize-32/lseek.c | 2 ++ > 2 files changed, 9 insertions(+) > > diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/llseek.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/llseek.c > index 458964c..3352ffd 100644 > --- a/sysdeps/unix/sysv/linux/generic/wordsize-32/llseek.c > +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/llseek.c > @@ -39,6 +39,13 @@ strong_alias (__llseek, __libc_lseek64) > strong_alias (__llseek, __lseek64) > weak_alias (__llseek, lseek64) > > +#ifdef __OFF_T_MATCHES_OFF64_T > +strong_alias (__llseek, __lseek) > +libc_hidden_ver (__llseek,__lseek) > +weak_alias (__llseek, lseek) > +strong_alias (__llseek, __libc_lseek) > +#endif > + > /* llseek doesn't have a prototype. Since the second parameter is a > 64bit type, this results in wrong behaviour if no prototype is > provided. */ > diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/lseek.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/lseek.c > index dbf0b26..c953e79 100644 > --- a/sysdeps/unix/sysv/linux/generic/wordsize-32/lseek.c > +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/lseek.c > @@ -23,6 +23,7 @@ > #include <sysdep.h> > #include <sys/syscall.h> > > +#ifndef __OFF_T_MATCHES_OFF64_T > #include "overflow.h" > > off_t > @@ -36,3 +37,4 @@ __lseek (int fd, off_t offset, int whence) > libc_hidden_def (__lseek) > weak_alias (__lseek, lseek) > strong_alias (__lseek, __libc_lseek) > +#endif >
diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/llseek.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/llseek.c index 458964c..3352ffd 100644 --- a/sysdeps/unix/sysv/linux/generic/wordsize-32/llseek.c +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/llseek.c @@ -39,6 +39,13 @@ strong_alias (__llseek, __libc_lseek64) strong_alias (__llseek, __lseek64) weak_alias (__llseek, lseek64) +#ifdef __OFF_T_MATCHES_OFF64_T +strong_alias (__llseek, __lseek) +libc_hidden_ver (__llseek,__lseek) +weak_alias (__llseek, lseek) +strong_alias (__llseek, __libc_lseek) +#endif + /* llseek doesn't have a prototype. Since the second parameter is a 64bit type, this results in wrong behaviour if no prototype is provided. */ diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/lseek.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/lseek.c index dbf0b26..c953e79 100644 --- a/sysdeps/unix/sysv/linux/generic/wordsize-32/lseek.c +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/lseek.c @@ -23,6 +23,7 @@ #include <sysdep.h> #include <sys/syscall.h> +#ifndef __OFF_T_MATCHES_OFF64_T #include "overflow.h" off_t @@ -36,3 +37,4 @@ __lseek (int fd, off_t offset, int whence) libc_hidden_def (__lseek) weak_alias (__lseek, lseek) strong_alias (__lseek, __libc_lseek) +#endif
Hi Adhemerval, Current __lseek() implementation under sysdeps/unix/sysv/linux/generic/wordsize-32/lseek.c is broken if off_t is 64-bit type. In this patch it is redirected to 64-bit __llseek() version. It helps to avoid introducing platform implementation for __lseek() under aarch64/ilp32. Notice that using SYSCALL_LL64() makes LTP hang for me (ltp-pan itself), so I don't use it. I am also not sure this patch does not affect other ports, and that there's no alternative implementations in glibc that we'd cleenup. So I marked it as RFC. Please take a look. If you find it helpful, I'll handle SYSCALL_LL64() issue and resend it. Yury. Signed-off-by: Yury Norov <ynorov@caviumnetworks.com> --- sysdeps/unix/sysv/linux/generic/wordsize-32/llseek.c | 7 +++++++ sysdeps/unix/sysv/linux/generic/wordsize-32/lseek.c | 2 ++ 2 files changed, 9 insertions(+)