From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 76748 invoked by alias); 1 Jul 2017 16:53:25 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 76737 invoked by uid 89); 1 Jul 2017 16:53:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.8 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-oi0-f66.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=hd9BCI9It/GTRHGBYBjQOZ+X7HFGo84yYIItuiFm0P0=; b=Ud9mf49nl94TWqLhsb7pGqRMXJ2FSG/dCZShfuMfYhMNtMG5JVdda8fKEgDTNCN4Qr AZBls/wFZuONJ6bAHDdMFCmHi/jEy8lGKCZfXumWv5CQnfTOTYGOvjvprGSJdrXMLOMs 1/xNleTMj84aZa4KP9JZV+dyYo5HQT1BmqIMLe1qc03XrwV4rvupzDM2aQsYCnsYnyX8 6g8SUVi5kcfrsknvGcrpnOormdmHA30hg3C+igUNR1hOB/0AxYhagYGCDB53jyENeovk toneRHgeko5V5LeNFa5o+sBVLJBkThL462ApV4P83E2iAw/OBQspr8vbSuFAnituR6Ld b5xw== X-Gm-Message-State: AKS2vOzzY3cP8Du/28spbARZR1dpp76kJZDqN46GV1zwMZTup6eKRP69 BB5HRpfbK15CQg84r5pXnMtptG6qkA== X-Received: by 10.202.84.214 with SMTP id i205mr12724132oib.158.1498928001379; Sat, 01 Jul 2017 09:53:21 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1495563960-669-2-git-send-email-adhemerval.zanella@linaro.org> References: <1495563960-669-1-git-send-email-adhemerval.zanella@linaro.org> <1495563960-669-2-git-send-email-adhemerval.zanella@linaro.org> From: "H.J. Lu" Date: Sat, 01 Jul 2017 16:53:00 -0000 Message-ID: Subject: Re: [PATCH 1/5] Fix {INLINE,INTERNAL}_SYSCALL macros for x32 To: Adhemerval Zanella Cc: GNU C Library , Adhemerval Zanella Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2017-07/txt/msg00013.txt.bz2 On Tue, May 23, 2017 at 11:25 AM, Adhemerval Zanella wrote: > From: Adhemerval Zanella > > 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.