This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/5] Fix {INLINE,INTERNAL}_SYSCALL macros for x32
- 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>, Adhemerval Zanella <adhemerval dot zanella at linaro dot com>
- Date: Sat, 1 Jul 2017 09:53:20 -0700
- Subject: Re: [PATCH 1/5] Fix {INLINE,INTERNAL}_SYSCALL macros for x32
- Authentication-results: sourceware.org; auth=none
- References: <1495563960-669-1-git-send-email-adhemerval.zanella@linaro.org> <1495563960-669-2-git-send-email-adhemerval.zanella@linaro.org>
On Tue, May 23, 2017 at 11:25 AM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> From: Adhemerval Zanella <adhemerval.zanella@linaro.com>
>
> The problem for x32 is the {INTERNAL,INLINE}_SYSCALL C macros explicit
> cast the arguments to 'long int', thus passing as 32 bits arguments
> that should be passed to 64 bits.
>
> Previous x32 implementation uses the auto-generated syscalls from
> assembly macros (syscalls.list), so the {INTERNAL,INLINE}_SYSCALL
> macros are never used with 64 bit argument in x32 (which are
> internally broken for this ILP).
>
> To fix it I used a strategy similar to MIPS64n32 (although both
> ABI differs for some syscalls on how top pass 64-bits arguments)
> where argument types for kernel call are defined using GCC extension
> 'typeof' with a arithmetic operation. This allows 64-bits arguments
> to be defined while 32-bits argument will still passed as 32-bits.
>
> I also cleanup the {INLINE,INTERNAL}_SYSCALL definition by defining
> 'inline_syscallX' instead of constructing the argument passing using
> macros (it adds some readability) and removed the ununsed
> INTERNAL_SYSCALL_NCS_TYPES define (since the patch idea is exactly to
> avoid requiric explicit types passing).
>
> Tested on x86_64 and x32.
>
> * sysdeps/unix/sysv/linux/x86_64/sysdep.h
> (INTERNAL_SYSCALL_NCS_TYPES): Remove define.
> (LOAD_ARGS_0): Likewise.
> (LOAD_ARGS_1): Likewise.
> (LOAD_ARGS_2): Likewise.
> (LOAD_ARGS_3): Likewise.
> (LOAD_ARGS_4): Likewise.
> (LOAD_ARGS_5): Likewise.
> (LOAD_ARGS_6): Likewise.
> (LOAD_REGS_0): Likewise.
> (LOAD_REGS_1): Likewise.
> (LOAD_REGS_2): Likewise.
> (LOAD_REGS_3): Likewise.
> (LOAD_REGS_4): Likewise.
> (LOAD_REGS_5): Likewise.
> (LOAD_REGS_6): Likewise.
> (ASM_ARGS_0): Likewise.
> (ASM_ARGS_1): Likewise.
> (ASM_ARGS_2): Likewise.
> (ASM_ARGS_3): Likewise.
> (ASM_ARGS_4): Likewise.
> (ASM_ARGS_5): Likewise.
> (ASM_ARGS_6): Likewise.
> (LOAD_ARGS_TYPES_1): Likewise.
> (LOAD_ARGS_TYPES_2): Likewise.
> (LOAD_ARGS_TYPES_3): Likewise.
> (LOAD_ARGS_TYPES_4): Likewise.
> (LOAD_ARGS_TYPES_5): Likewise.
> (LOAD_ARGS_TYPES_6): Likewise.
> (LOAD_REGS_TYPES_1): Likewise.
> (LOAD_REGS_TYPES_2): Likewise.
> (LOAD_REGS_TYPES_3): Likewise.
> (LOAD_REGS_TYPES_4): Likewise.
> (LOAD_REGS_TYPES_5): Likewise.
> (LOAD_REGS_TYPES_6): Likewise.
> (TYPEFY): New define.
> (ARGIFY): Likewise.
> (internal_syscall0): Likewise.
> (internal_syscall1): Likewise.
> (internal_syscall2): Likewise.
> (internal_syscall3): Likewise.
> (internal_syscall4): Likewise.
> (internal_syscall5): Likewise.
> (internal_syscall6): Likewise.
> * sysdeps/unix/sysv/linux/x86_64/x32/times.c
> (INTERNAL_SYSCALL_NCS): Remove define.
> (internal_syscall1): Add define.
> ---
> ChangeLog | 50 ++++++
> sysdeps/unix/sysv/linux/x86_64/sysdep.h | 251 ++++++++++++++++-------------
> sysdeps/unix/sysv/linux/x86_64/x32/times.c | 24 +--
> 3 files changed, 205 insertions(+), 120 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> index 7b8bd79..6d0a6f4 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> @@ -221,33 +221,148 @@
> /* Registers clobbered by syscall. */
> # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx"
>
> -# define INTERNAL_SYSCALL_NCS(name, err, nr, args...) \
> - ({ \
> - unsigned long int resultvar; \
> - LOAD_ARGS_##nr (args) \
> - LOAD_REGS_##nr \
> - asm volatile ( \
> - "syscall\n\t" \
> - : "=a" (resultvar) \
> - : "0" (name) ASM_ARGS_##nr : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \
> - (long int) resultvar; })
> -# undef INTERNAL_SYSCALL
> -# define INTERNAL_SYSCALL(name, err, nr, args...) \
> - INTERNAL_SYSCALL_NCS (__NR_##name, err, nr, ##args)
> -
> -# define INTERNAL_SYSCALL_NCS_TYPES(name, err, nr, args...) \
> - ({ \
> - unsigned long int resultvar; \
> - LOAD_ARGS_TYPES_##nr (args) \
> - LOAD_REGS_TYPES_##nr (args) \
> - asm volatile ( \
> - "syscall\n\t" \
> - : "=a" (resultvar) \
> - : "0" (name) ASM_ARGS_##nr : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \
> - (long int) resultvar; })
> -# undef INTERNAL_SYSCALL_TYPES
> -# define INTERNAL_SYSCALL_TYPES(name, err, nr, args...) \
> - INTERNAL_SYSCALL_NCS_TYPES (__NR_##name, err, nr, ##args)
> +/* Create a variable 'name' based on type 'X' to avoid explicit types.
> + This is mainly used set use 64-bits arguments in x32. */
> +#define TYPEFY(X, name) __typeof__ ((X) - (X)) name
> +/* Explicit cast the argument to avoid integer from pointer warning on
> + x32. */
> +#define ARGIFY(X) ((__typeof__ ((X) - (X))) (X))
> +
I am a little bit concerned about this. The current macros cast to long int
by default on purpose. 64-bit return and arguments are treated as
special cases. With your change, this code in loadlocale.c:
if (filedata != NULL)
{
off_t to_read = st.st_size;
ssize_t nread;
char *p = (char *) filedata;
while (to_read > 0)
{
nread = read_not_cancel (fd, p, to_read);
if (__builtin_expect (nread, 1) <= 0)
{
free (filedata);
if (nread == 0)
__set_errno (EINVAL); /* Bizarreness going on. */
goto puntfd;
}
p += nread;
is generated differently. On x32, ssize_t is 32 bits and offset is 64 bits.
Before your patch, to_read is passed as 32 bit value. With your change,
__typeof__ ((X) - (X)) turns to_read into 64 bits. It may be OK for this
particular case. But I am not certain that your change is 100% safe in
all cases.
H.J.