From patchwork Tue Dec 15 13:52:18 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 10016 Received: (qmail 7390 invoked by alias); 15 Dec 2015 13:52:24 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 7365 invoked by uid 89); 15 Dec 2015 13:52:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-oi0-f43.google.com MIME-Version: 1.0 X-Received: by 10.202.211.68 with SMTP id k65mr12907260oig.9.1450187539042; Tue, 15 Dec 2015 05:52:19 -0800 (PST) In-Reply-To: <20151215052534.GA1512@altlinux.org> References: <20151215032733.GA14426@gmail.com> <20151215052534.GA1512@altlinux.org> Date: Tue, 15 Dec 2015 05:52:18 -0800 Message-ID: Subject: Re: [PATCH] [BZ #19363] Use INTERNAL_SYSCALL_TIMES for Linux times From: "H.J. Lu" To: GNU C Library On Mon, Dec 14, 2015 at 9:25 PM, Dmitry V. Levin wrote: > On Mon, Dec 14, 2015 at 07:27:33PM -0800, H.J. Lu wrote: >> The Linux times function, which returns clock_t, is implemented with >> INTERNAL_SYSCALL. Since INTERNAL_SYSCALL returns 32-bit integer and >> and clock_t is 64-bit on x32, there is mismatch on x32. times is the >> only such function since there is lseek.S for x32. This patch replaces >> INTERNAL_SYSCALL in Linux times.c with INTERNAL_SYSCALL_TIMES which is >> default to INTERNAL_SYSCALL and provides x32 times.c with proper >> INTERNAL_SYSCALL_TIMES. >> >> There is no code change on times for i686 nor x86-64. For x32, before >> this patch, there are >> >> 0000000 <__times>: >> 0: b8 64 00 00 40 mov $0x40000064,%eax >> 5: 0f 05 syscall >> 7: 48 63 d0 movslq %eax,%rdx >> ^^^^^^^^^^ Incorrect signed extension >> a: 48 83 fa f2 cmp $0xfffffffffffffff2,%rdx >> e: 75 07 jne 17 <__times+0x17> >> 10: 3d 00 f0 ff ff cmp $0xfffff000,%eax >> 15: 77 11 ja 28 <__times+0x28> >> 17: 48 83 fa ff cmp $0xffffffffffffffff,%rdx >> 1b: b8 00 00 00 00 mov $0x0,%eax >> 20: 48 0f 45 c2 cmovne %rdx,%rax >> 24: c3 retq >> >> After this patch, there are >> >> 00000000 <__times>: >> 0: b8 64 00 00 40 mov $0x40000064,%eax >> 5: 0f 05 syscall >> 7: 48 83 f8 f2 cmp $0xfffffffffffffff2,%rax >> b: 75 07 jne 14 <__times+0x14> >> d: 3d 00 f0 ff ff cmp $0xfffff000,%eax > > Looks like there is another truncation remaining here that comes > from INTERNAL_SYSCALL_ERROR_P. > You are right. Here is the updated patch with all INTERNAL_SYSCALL* macros replaced by INTERNAL_SYSCALL_TIMES* macros. There is no code change on i686 nor x86-64. OK for master? Thanks. From e31f1cb4a363db73fe09d99f800907950aec7122 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Mon, 14 Dec 2015 19:09:13 -0800 Subject: [PATCH] Use INTERNAL_SYSCALL_TIMES* macros for Linux times The Linux times function, which returns clock_t, is implemented with INTERNAL_SYSCALL_DECL, INTERNAL_SYSCALL, INTERNAL_SYSCALL_ERROR_P and INTERNAL_SYSCALL_ERRNO. Since INTERNAL_SYSCALL* macros use 32-bit integer and clock_t is 64-bit on x32, this is a mismatch on x32. All system calls returning 64-bit integer, which are lseek, time and times, must be handled specially for x32. lseek is handled by x32 lseek.S and time doesn't check syscall return. times is the only missed one. This patch replaces INTERNAL_SYSCALL* macros in Linux times.c with INTERNAL_SYSCALL_TIMES* macros which are default to INTERNAL_SYSCALL* macros and provides x32 times.c with proper INTERNAL_SYSCALL_TIMES* macros. There is no code change on times for i686 nor x86-64. For x32, before this patch, there are 0000000 <__times>: 0: b8 64 00 00 40 mov $0x40000064,%eax 5: 0f 05 syscall 7: 48 63 d0 movslq %eax,%rdx ^^^^^^^^^^ Incorrect signed extension a: 48 83 fa f2 cmp $0xfffffffffffffff2,%rdx e: 75 07 jne 17 <__times+0x17> 10: 3d 00 f0 ff ff cmp $0xfffff000,%eax ^^^^^^^^^^^^^^^^^^^^^ 32-bit compare 15: 77 11 ja 28 <__times+0x28> 17: 48 83 fa ff cmp $0xffffffffffffffff,%rdx 1b: b8 00 00 00 00 mov $0x0,%eax 20: 48 0f 45 c2 cmovne %rdx,%rax 24: c3 retq After this patch, there are 00000000 <__times>: 0: b8 64 00 00 40 mov $0x40000064,%eax 5: 0f 05 syscall 7: 48 83 f8 f2 cmp $0xfffffffffffffff2,%rax b: 75 08 jne 15 <__times+0x15> d: 48 3d 00 f0 ff ff cmp $0xfffffffffffff000,%rax 13: 77 13 ja 28 <__times+0x28> 15: 48 83 f8 ff cmp $0xffffffffffffffff,%rax 19: ba 00 00 00 00 mov $0x0,%edx 1e: 48 0f 44 c2 cmove %rdx,%rax 22: c3 retq The incorrect signed extension and 32-bit compare are gone. [BZ #19363] * sysdeps/unix/sysv/linux/times.c (INTERNAL_SYSCALL_TIMES_DECL): New. (INTERNAL_SYSCALL_TIMES): Likewise. (INTERNAL_SYSCALL_TIMES_ERROR_P): Likewise. (INTERNAL_SYSCALL_TIMES_ERRNO): Likewise. (__times): Replace INTERNAL_SYSCALL* macros with INTERNAL_SYSCALL_TIMES* macros. * sysdeps/unix/sysv/linux/x86_64/x32/times.c: New file. --- sysdeps/unix/sysv/linux/times.c | 28 ++++++++++++++++++++---- sysdeps/unix/sysv/linux/x86_64/x32/times.c | 34 ++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/times.c diff --git a/sysdeps/unix/sysv/linux/times.c b/sysdeps/unix/sysv/linux/times.c index 19b77cf..d2e9c23 100644 --- a/sysdeps/unix/sysv/linux/times.c +++ b/sysdeps/unix/sysv/linux/times.c @@ -19,14 +19,34 @@ #include #include +#ifndef INTERNAL_SYSCALL_TIMES_DECL +# define INTERNAL_SYSCALL_TIMES_DECL(err) \ + INTERNAL_SYSCALL_DECL (err) +#endif + +#ifndef INTERNAL_SYSCALL_TIMES +# define INTERNAL_SYSCALL_TIMES(err, buf) \ + INTERNAL_SYSCALL (times, err, 1, buf) +#endif + +#ifndef INTERNAL_SYSCALL_TIMES_ERROR_P +# define INTERNAL_SYSCALL_TIMES_ERROR_P(ret, err) \ + INTERNAL_SYSCALL_ERROR_P (ret, err) +#endif + +#ifndef INTERNAL_SYSCALL_TIMES_ERRNO +# define INTERNAL_SYSCALL_TIMES_ERRNO(ret, err) \ + INTERNAL_SYSCALL_ERRNO (ret, err) +#endif clock_t __times (struct tms *buf) { - INTERNAL_SYSCALL_DECL (err); - clock_t ret = INTERNAL_SYSCALL (times, err, 1, buf); - if (INTERNAL_SYSCALL_ERROR_P (ret, err) - && __builtin_expect (INTERNAL_SYSCALL_ERRNO (ret, err) == EFAULT, 0) + INTERNAL_SYSCALL_TIMES_DECL (err); + clock_t ret = INTERNAL_SYSCALL_TIMES (err, buf); + if (INTERNAL_SYSCALL_TIMES_ERROR_P (ret, err) + && (__builtin_expect (INTERNAL_SYSCALL_TIMES_ERRNO (ret, err) + == EFAULT, 0)) && buf) { /* This might be an error or not. For architectures which have diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/times.c b/sysdeps/unix/sysv/linux/x86_64/x32/times.c new file mode 100644 index 0000000..fa0b8e4 --- /dev/null +++ b/sysdeps/unix/sysv/linux/x86_64/x32/times.c @@ -0,0 +1,34 @@ +/* Linux times. X32 version. + Copyright (C) 2015 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +/* Inline Linux times system call. */ +#define INTERNAL_SYSCALL_TIMES(err, buf) \ + ({ \ + unsigned long long int resultvar; \ + LOAD_ARGS_1 (buf) \ + LOAD_REGS_1 \ + asm volatile ( \ + "syscall\n\t" \ + : "=a" (resultvar) \ + : "0" (__NR_times) ASM_ARGS_1 : "memory", "cc", "r11", "cx"); \ + (long long int) resultvar; }) + +#define INTERNAL_SYSCALL_TIMES_ERROR_P(val, err) \ + ((unsigned long long int) (val) >= -4095LL) + +#include -- 2.5.0