From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 114585 invoked by alias); 3 Jul 2017 19:44:38 -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 114572 invoked by uid 89); 3 Jul 2017 19:44:37 -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=ypm9fq+QBo4fMmpbYvY21afzS4Q51tETvQS+9/dtt/Q=; b=eOap49KaiL5AhU+oS8sje7rJcIREJjVKonqPCivv5akZ/hkmgOglAQ9oLNnrG0q7mm /gIz5SrBV0Tu4d6N/51c3pc2ofP+e6lSHE6C9MbK7FamUv9F6VDvO1G7C64dXEhi21v/ i7AqRWjSLmE0kT/eH2IyV3QOAVr9E3yVTWD2PJ0iE9nof8cnI/xcWYbqXSoOvY/gAlQV poaK36IGS4dftrPbXIu+929epHgZ+3FCzz3qGi8E5eGLh1xyE1hKF2stUiRHRuAAy5wC rqvibYyr7P6m0+aRFBEniRGGlvcQbuNZVCuVLhMWORNprhqtn4iujRegHN7VJn67P9Rl TRdQ== X-Gm-Message-State: AKS2vOyAOfkfGMGfdeKCEu/t+Fq3Uns25LxCGNf21RRPSypcowop8Ls4 Wiu3fNa6TArS6pmm5oqfqiJeTnunXA== X-Received: by 10.202.93.215 with SMTP id r206mr18017800oib.2.1499111073798; Mon, 03 Jul 2017 12:44:33 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <044c5243-5e6c-69a7-b92e-c6d290cd9ce6@linaro.org> References: <1495563960-669-1-git-send-email-adhemerval.zanella@linaro.org> <1495563960-669-2-git-send-email-adhemerval.zanella@linaro.org> <044c5243-5e6c-69a7-b92e-c6d290cd9ce6@linaro.org> From: "H.J. Lu" Date: Mon, 03 Jul 2017 19:44: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/msg00028.txt.bz2 On Mon, Jul 3, 2017 at 4:52 AM, Adhemerval Zanella wrote: > > > On 01/07/2017 13:53, H.J. Lu wrote: >> 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. > > I think the problem is we define 'read_not_cancel' as a macro rather than > an static inline with expected arguments types. For a QoI I think creating > wrappers around it would be better. That is fine with me. > I would like to make the syscall internal macros to work seamlessly across > all architectures and avoid to create specialized assembly macros for > specific syscalls (such as lseek{64} for x32 witch I think it is not > avoidable). Also x32 is the only architecture afaik preventing the > syscall-cancel.h cleanup currently. There is fine with me as long as the implementation is correct for x32. But the current one isn't correct for x32. Thanks. -- H.J.