This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2] Fix LO_HI_LONG definition
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Mon, 11 Jul 2016 14:09:47 -0700
- Subject: Re: [PATCH v2] Fix LO_HI_LONG definition
- Authentication-results: sourceware.org; auth=none
- References: <1467834756-21898-1-git-send-email-adhemerval.zanella@linaro.org>
On Wed, Jul 6, 2016 at 12:52 PM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> The p{read,write}v{64} consolidation patch [1] added a wrong guard
> for LO_HI_LONG definition. It currently uses both
> '__WORDSIZE == 64' and 'defined __ASSUME_WORDSIZE64_ILP32' to set
> the value to be passed in one argument, otherwise it will be split
> in two. Since kernel commit 601cc11d054 the non-compat
> preadv/pwritev in same order, so the LO_HI_LONG is the same regardless
> off/off64_t size or wordsize.
>
> The tests were passing on 64-bit because files were small enough so
> the high part of offset is 0 regardless. This patch also changes the
> tst-preadvwritev64 test to check reads and writes on a sparse file
> larger than 4G.
>
> Checked on x86_64, i686, x32, aarch64, armhf, and s390.
>
> * sysdeps/unix/sysv/linux/sysdep.h
> [__WORDSIZE == 64 || __ASSUME_WORDSIZE64_ILP32] (LO_HI_LONG): Remove
> guards.
> * misc/tst-preadvwritev-common.c: New file.
> * misc/tst-preadvwritev.c: Use tst-preadvwritev-common.c.
> * misc/tst-preadvwritev64.c: Use tst-preadwritev-common.c and add
> a check for files larger than 2GB.
>
> diff --git a/sysdeps/unix/sysv/linux/sysdep.h b/sysdeps/unix/sysv/linux/sysdep.h
> index 8c9e62e..a469f57 100644
> --- a/sysdeps/unix/sysv/linux/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/sysdep.h
> @@ -49,10 +49,6 @@
> #endif
>
> /* Provide a macro to pass the off{64}_t argument on p{readv,writev}{64}. */
> -#if __WORDSIZE == 64 || defined __ASSUME_WORDSIZE64_ILP32
> -# define LO_HI_LONG(val) (val)
> -#else
> -# define LO_HI_LONG(val) \
> - (long) (val), \
> - (long) (((uint64_t) (val)) >> 32)
> -#endif
> +#define LO_HI_LONG(val) \
> + (long) (val), \
> + (long) (((uint64_t) (val)) >> 32)
> --
This change is incorrect for x32. __ASSUME_WORDSIZE64_ILP32 seems
to mean different things for mips and x86-64. X86-64 always passes 64-bit
value in a 64-bit register.
--
H.J.