Patchwork [BZ,#19363] Use INTERNAL_SYSCALL_TIMES for Linux times

login
register
mail settings
Submitter H.J. Lu
Date Dec. 15, 2015, 3:27 a.m.
Message ID <20151215032733.GA14426@gmail.com>
Download mbox | patch
Permalink /patch/10009/
State New
Headers show

Comments

H.J. Lu - Dec. 15, 2015, 3:27 a.m.
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
  12:	77 14                	ja     28 <__times+0x28>
  14:	48 83 f8 ff          	cmp    $0xffffffffffffffff,%rax
  18:	ba 00 00 00 00       	mov    $0x0,%edx
  1d:	48 0f 44 c2          	cmove  %rdx,%rax
  21:	c3                   	retq

The incorrect signed extension is gone.

Tested on i686, x86-64 and x32.  OK for master?

H.J.
---
	[BZ #19363]
	* sysdeps/unix/sysv/linux/times.c (INTERNAL_SYSCALL_TIMES): New.
	(__times): Replace INTERNAL_SYSCALL with INTERNAL_SYSCALL_TIMES.
	* sysdeps/unix/sysv/linux/x86_64/x32/times.c: New file.
---
 sysdeps/unix/sysv/linux/times.c            |  6 +++++-
 sysdeps/unix/sysv/linux/x86_64/x32/times.c | 31 ++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/times.c
Dmitry Levin - Dec. 15, 2015, 3:47 a.m.
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
>   12:	77 14                	ja     28 <__times+0x28>
>   14:	48 83 f8 ff          	cmp    $0xffffffffffffffff,%rax
>   18:	ba 00 00 00 00       	mov    $0x0,%edx
>   1d:	48 0f 44 c2          	cmove  %rdx,%rax
>   21:	c3                   	retq
> 
> The incorrect signed extension is gone.

Great.  

> 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..c32324f
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/times.c
> @@ -0,0 +1,31 @@
> +/* Linux times.  X32 version.
[...]
> +/* Incline Linux times system calls.  */

Please fix spelling issues here.
Mike Frysinger - Dec. 15, 2015, 3:59 a.m.
On 14 Dec 2015 19:27, H.J. Lu wrote:
> +  clock_t ret = INTERNAL_SYSCALL_TIMES(err, buf);

needs a space before the (

> +/* Incline Linux times system calls.  */

"incline" ?  i don't understand what you mean.

> +# define INTERNAL_SYSCALL_TIMES(err, buf)				\

no space after the #

> +  ({									\
> +    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");	\

should the cc/r11/cx be made into a sysdep define ?
-mike
Dmitry Levin - Dec. 15, 2015, 5:25 a.m.
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.

>   12:	77 14                	ja     28 <__times+0x28>
>   14:	48 83 f8 ff          	cmp    $0xffffffffffffffff,%rax
>   18:	ba 00 00 00 00       	mov    $0x0,%edx
>   1d:	48 0f 44 c2          	cmove  %rdx,%rax
>   21:	c3                   	retq
Andreas Schwab - Dec. 15, 2015, 9:12 a.m.
I think the correct fix is to make all *_SYSCALL macros operate on long
long when handling the syscall return value.

Andreas.
Mike Frysinger - Dec. 15, 2015, 3 p.m.
On 15 Dec 2015 10:12, Andreas Schwab wrote:
> I think the correct fix is to make all *_SYSCALL macros operate on long
> long when handling the syscall return value.

i think the concern is about the opposite behavior -- code relying on the
value being truncated/extended in most places to 32bits.  he started off
with that patch in the bug, but i'm not sure what came of it.
-mike
Andreas Schwab - Dec. 15, 2015, 3:14 p.m.
Mike Frysinger <vapier@gentoo.org> writes:

> On 15 Dec 2015 10:12, Andreas Schwab wrote:
>> I think the correct fix is to make all *_SYSCALL macros operate on long
>> long when handling the syscall return value.
>
> i think the concern is about the opposite behavior -- code relying on the
> value being truncated/extended in most places to 32bits.

All x86_64 and x32 syscalls return a 64-bit value.

Andreas.
H.J. Lu - Dec. 15, 2015, 3:48 p.m.
On Tue, Dec 15, 2015 at 7:14 AM, Andreas Schwab <schwab@suse.de> wrote:
> Mike Frysinger <vapier@gentoo.org> writes:
>
>> On 15 Dec 2015 10:12, Andreas Schwab wrote:
>>> I think the correct fix is to make all *_SYSCALL macros operate on long
>>> long when handling the syscall return value.
>>
>> i think the concern is about the opposite behavior -- code relying on the
>> value being truncated/extended in most places to 32bits.
>
> All x86_64 and x32 syscalls return a 64-bit value.
>

For x86_64 and x32, many, if not most, of system calls return int.
Some returns long.  Only 3, lseek, time and times, returns long long.
Andreas Schwab - Dec. 15, 2015, 5:18 p.m.
"H.J. Lu" <hjl.tools@gmail.com> writes:

> On Tue, Dec 15, 2015 at 7:14 AM, Andreas Schwab <schwab@suse.de> wrote:
>> Mike Frysinger <vapier@gentoo.org> writes:
>>
>>> On 15 Dec 2015 10:12, Andreas Schwab wrote:
>>>> I think the correct fix is to make all *_SYSCALL macros operate on long
>>>> long when handling the syscall return value.
>>>
>>> i think the concern is about the opposite behavior -- code relying on the
>>> value being truncated/extended in most places to 32bits.
>>
>> All x86_64 and x32 syscalls return a 64-bit value.
>>
>
> For x86_64 and x32, many, if not most, of system calls return int.
> Some returns long.  Only 3, lseek, time and times, returns long long.

On the assembler level, all of them return a 64-bit value.

Andreas.
H.J. Lu - Dec. 15, 2015, 5:35 p.m.
On Tue, Dec 15, 2015 at 9:18 AM, Andreas Schwab <schwab@suse.de> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>
>> On Tue, Dec 15, 2015 at 7:14 AM, Andreas Schwab <schwab@suse.de> wrote:
>>> Mike Frysinger <vapier@gentoo.org> writes:
>>>
>>>> On 15 Dec 2015 10:12, Andreas Schwab wrote:
>>>>> I think the correct fix is to make all *_SYSCALL macros operate on long
>>>>> long when handling the syscall return value.
>>>>
>>>> i think the concern is about the opposite behavior -- code relying on the
>>>> value being truncated/extended in most places to 32bits.
>>>
>>> All x86_64 and x32 syscalls return a 64-bit value.
>>>
>>
>> For x86_64 and x32, many, if not most, of system calls return int.
>> Some returns long.  Only 3, lseek, time and times, returns long long.
>
> On the assembler level, all of them return a 64-bit value.
>

We are talking C codes here, not assembly codes.
Andreas Schwab - Dec. 15, 2015, 6:18 p.m.
"H.J. Lu" <hjl.tools@gmail.com> writes:

> We are talking C codes here, not assembly codes.

No, we are talking about inline assembler.

Andreas.
H.J. Lu - Dec. 15, 2015, 6:19 p.m.
On Tue, Dec 15, 2015 at 10:18 AM, Andreas Schwab <schwab@suse.de> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>
>> We are talking C codes here, not assembly codes.
>
> No, we are talking about inline assembler.
>

It is still int, long and long long.
Andreas Schwab - Dec. 15, 2015, 11:28 p.m.
"H.J. Lu" <hjl.tools@gmail.com> writes:

> On Tue, Dec 15, 2015 at 10:18 AM, Andreas Schwab <schwab@suse.de> wrote:
>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>
>>> We are talking C codes here, not assembly codes.
>>
>> No, we are talking about inline assembler.
>>
>
> It is still int, long and long long.

No, the syscall always returns a 64-bit value.

Andreas.
H.J. Lu - Dec. 15, 2015, 11:36 p.m.
On Tue, Dec 15, 2015 at 3:28 PM, Andreas Schwab <schwab@suse.de> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>
>> On Tue, Dec 15, 2015 at 10:18 AM, Andreas Schwab <schwab@suse.de> wrote:
>>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>>
>>>> We are talking C codes here, not assembly codes.
>>>
>>> No, we are talking about inline assembler.
>>>
>>
>> It is still int, long and long long.
>
> No, the syscall always returns a 64-bit value.
>
> Andreas.
>

Inline asm statement doesn't use result from "syscall" instruction
directly. which is always converted to int/long/long long.
Andreas Schwab - Dec. 16, 2015, 8:35 a.m.
"H.J. Lu" <hjl.tools@gmail.com> writes:

> Inline asm statement doesn't use result from "syscall" instruction
> directly. which is always converted to int/long/long long.

Of course, it is converted by the syscall wrapper, but the syscall
itself always returns a 64-bit value.

Andreas.
H.J. Lu - Dec. 16, 2015, 1:12 p.m.
On Wed, Dec 16, 2015 at 12:35 AM, Andreas Schwab <schwab@suse.de> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>
>> Inline asm statement doesn't use result from "syscall" instruction
>> directly. which is always converted to int/long/long long.
>
> Of course, it is converted by the syscall wrapper, but the syscall
> itself always returns a 64-bit value.

Since syscall instruction isn't used directly, It is correct for x32 to
use/return long int in INTERNAL_SYSCALL* macros.  Only times
returns long long which is handled by my patch.
Andreas Schwab - Dec. 16, 2015, 1:35 p.m.
"H.J. Lu" <hjl.tools@gmail.com> writes:

> Since syscall instruction isn't used directly, It is correct for x32 to
> use/return long int in INTERNAL_SYSCALL* macros.

They use whatever is needed to do the syscall, which then returns a
64-bit value to user space.

Andreas.
H.J. Lu - Dec. 16, 2015, 1:51 p.m.
On Wed, Dec 16, 2015 at 5:35 AM, Andreas Schwab <schwab@suse.de> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>
>> Since syscall instruction isn't used directly, It is correct for x32 to
>> use/return long int in INTERNAL_SYSCALL* macros.
>
> They use whatever is needed to do the syscall, which then returns a
> 64-bit value to user space.
>

Then they have to use proper type to hold the return from "syscall".
In most cases, long works for x32.  Only lseek, time and times need
long long.
Andreas Schwab - Dec. 16, 2015, 2:17 p.m.
"H.J. Lu" <hjl.tools@gmail.com> writes:

> Then they have to use proper type to hold the return from "syscall".

The proper type is long long which works for both x86_64 and x32.

Andreas.
H.J. Lu - Dec. 16, 2015, 2:42 p.m.
On Wed, Dec 16, 2015 at 6:17 AM, Andreas Schwab <schwab@suse.de> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>
>> Then they have to use proper type to hold the return from "syscall".
>
> The proper type is long long which works for both x86_64 and x32.
>

I opened a syscall (), not "syscall" instruction, bug:

https://sourceware.org/bugzilla/show_bug.cgi?id=19371
Adhemerval Zanella Netto - Dec. 16, 2015, 4:46 p.m.
On 16-12-2015 12:42, H.J. Lu wrote:
> On Wed, Dec 16, 2015 at 6:17 AM, Andreas Schwab <schwab@suse.de> wrote:
>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>
>>> Then they have to use proper type to hold the return from "syscall".
>>
>> The proper type is long long which works for both x86_64 and x32.
>>
> 
> I opened a syscall (), not "syscall" instruction, bug:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=19371
> 

The question is why instead of reimplement each x32 syscall call that 
requires long long return with specific arch implementations (this 
very patch idea) we change the x32 syscall return to long long
instead?

So instead of redefine each INTERNAL_SYSCALL_XXX for x32 we make it
work on the current times implementation instead.
Florian Weimer - Dec. 16, 2015, 5:08 p.m.
On 12/16/2015 03:17 PM, Andreas Schwab wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
> 
>> Then they have to use proper type to hold the return from "syscall".
> 
> The proper type is long long which works for both x86_64 and x32.

Which specification location are we talking about?

In general, on x86_64, using long long where int is eventually required
can cause the compiler to emit an unnecessary truncation.

Florian
H.J. Lu - Dec. 16, 2015, 9:18 p.m.
On Wed, Dec 16, 2015 at 8:46 AM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
> On 16-12-2015 12:42, H.J. Lu wrote:
>> On Wed, Dec 16, 2015 at 6:17 AM, Andreas Schwab <schwab@suse.de> wrote:
>>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>>
>>>> Then they have to use proper type to hold the return from "syscall".
>>>
>>> The proper type is long long which works for both x86_64 and x32.
>>>
>>
>> I opened a syscall (), not "syscall" instruction, bug:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=19371
>>
>
> The question is why instead of reimplement each x32 syscall call that
> requires long long return with specific arch implementations (this
> very patch idea) we change the x32 syscall return to long long
> instead?
>
> So instead of redefine each INTERNAL_SYSCALL_XXX for x32 we make it
> work on the current times implementation instead.

Please think hard on this.  X32 is ILP32 and syscall puts return
value in 64-bit register RAX.   All, but 3, system calls have 32-bit
return value with zero upper 32-bits in RAX.  Now RAX has
0x80000000.  How do you extend it to 64-bit if syscall returns 64-bit?
Andreas Schwab - Dec. 17, 2015, 8:49 a.m.
Florian Weimer <fweimer@redhat.com> writes:

> On 12/16/2015 03:17 PM, Andreas Schwab wrote:
>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>> 
>>> Then they have to use proper type to hold the return from "syscall".
>> 
>> The proper type is long long which works for both x86_64 and x32.
>
> Which specification location are we talking about?

See the PSEUDO macro in .../linux/x86_64/sysdep.h.

Andreas.
Florian Weimer - Dec. 17, 2015, 9:06 a.m.
On 12/17/2015 09:49 AM, Andreas Schwab wrote:
> Florian Weimer <fweimer@redhat.com> writes:
> 
>> On 12/16/2015 03:17 PM, Andreas Schwab wrote:
>>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>>
>>>> Then they have to use proper type to hold the return from "syscall".
>>>
>>> The proper type is long long which works for both x86_64 and x32.
>>
>> Which specification location are we talking about?
> 
> See the PSEUDO macro in .../linux/x86_64/sysdep.h.

I think we are talking about the C level constructs in this thread.

(And I think the x86-64 psABI should be updated to make the code you
referenced actually valid.)

Florian
Andreas Schwab - Dec. 17, 2015, 9:24 a.m.
Florian Weimer <fweimer@redhat.com> writes:

> On 12/17/2015 09:49 AM, Andreas Schwab wrote:
>> Florian Weimer <fweimer@redhat.com> writes:
>> 
>>> On 12/16/2015 03:17 PM, Andreas Schwab wrote:
>>>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>>>
>>>>> Then they have to use proper type to hold the return from "syscall".
>>>>
>>>> The proper type is long long which works for both x86_64 and x32.
>>>
>>> Which specification location are we talking about?
>> 
>> See the PSEUDO macro in .../linux/x86_64/sysdep.h.
>
> I think we are talking about the C level constructs in this thread.

No, this is about the *_SYSCALL macros for inline syscalls.

Andreas.
Florian Weimer - Dec. 17, 2015, 9:26 a.m.
On 12/17/2015 10:24 AM, Andreas Schwab wrote:
> Florian Weimer <fweimer@redhat.com> writes:
> 
>> On 12/17/2015 09:49 AM, Andreas Schwab wrote:
>>> Florian Weimer <fweimer@redhat.com> writes:
>>>
>>>> On 12/16/2015 03:17 PM, Andreas Schwab wrote:
>>>>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>>>>
>>>>>> Then they have to use proper type to hold the return from "syscall".
>>>>>
>>>>> The proper type is long long which works for both x86_64 and x32.
>>>>
>>>> Which specification location are we talking about?
>>>
>>> See the PSEUDO macro in .../linux/x86_64/sysdep.h.
>>
>> I think we are talking about the C level constructs in this thread.
> 
> No, this is about the *_SYSCALL macros for inline syscalls.

I don't understand—the PSEUDO macro in that file is guarded by “#ifdef
__ASSEMBLER__”.

Florian
Andreas Schwab - Dec. 17, 2015, 9:29 a.m.
Florian Weimer <fweimer@redhat.com> writes:

> On 12/17/2015 10:24 AM, Andreas Schwab wrote:
>> Florian Weimer <fweimer@redhat.com> writes:
>> 
>>> On 12/17/2015 09:49 AM, Andreas Schwab wrote:
>>>> Florian Weimer <fweimer@redhat.com> writes:
>>>>
>>>>> On 12/16/2015 03:17 PM, Andreas Schwab wrote:
>>>>>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>>>>>
>>>>>>> Then they have to use proper type to hold the return from "syscall".
>>>>>>
>>>>>> The proper type is long long which works for both x86_64 and x32.
>>>>>
>>>>> Which specification location are we talking about?
>>>>
>>>> See the PSEUDO macro in .../linux/x86_64/sysdep.h.
>>>
>>> I think we are talking about the C level constructs in this thread.
>> 
>> No, this is about the *_SYSCALL macros for inline syscalls.
>
> I don't understand—the PSEUDO macro in that file is guarded by “#ifdef
> __ASSEMBLER__”.

It shows how the syscall interface is defined.

Andreas.
Adhemerval Zanella Netto - Dec. 17, 2015, 8:48 p.m.
On 16-12-2015 19:18, H.J. Lu wrote:
> On Wed, Dec 16, 2015 at 8:46 AM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>> On 16-12-2015 12:42, H.J. Lu wrote:
>>> On Wed, Dec 16, 2015 at 6:17 AM, Andreas Schwab <schwab@suse.de> wrote:
>>>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>>>
>>>>> Then they have to use proper type to hold the return from "syscall".
>>>>
>>>> The proper type is long long which works for both x86_64 and x32.
>>>>
>>>
>>> I opened a syscall (), not "syscall" instruction, bug:
>>>
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=19371
>>>
>>
>> The question is why instead of reimplement each x32 syscall call that
>> requires long long return with specific arch implementations (this
>> very patch idea) we change the x32 syscall return to long long
>> instead?
>>
>> So instead of redefine each INTERNAL_SYSCALL_XXX for x32 we make it
>> work on the current times implementation instead.
> 
> Please think hard on this.  X32 is ILP32 and syscall puts return
> value in 64-bit register RAX.   All, but 3, system calls have 32-bit
> return value with zero upper 32-bits in RAX.  Now RAX has
> 0x80000000.  How do you extend it to 64-bit if syscall returns 64-bit?
> 

I meant the macros at sysdep.h, not the ABI. Instead of internally
using 'long int' to handle the inline assembly, change it to 'long long'
to make it 64-bit for both x86_64 and x32, and let the compiler handle
the truncation if required.

Patch

diff --git a/sysdeps/unix/sysv/linux/times.c b/sysdeps/unix/sysv/linux/times.c
index 19b77cf..0dec69b 100644
--- a/sysdeps/unix/sysv/linux/times.c
+++ b/sysdeps/unix/sysv/linux/times.c
@@ -19,12 +19,16 @@ 
 #include <sys/times.h>
 #include <sysdep.h>
 
+#ifndef INTERNAL_SYSCALL_TIMES
+# define INTERNAL_SYSCALL_TIMES(err, buf) \
+  INTERNAL_SYSCALL (times, err, 1, buf)
+#endif
 
 clock_t
 __times (struct tms *buf)
 {
   INTERNAL_SYSCALL_DECL (err);
-  clock_t ret = INTERNAL_SYSCALL (times, err, 1, buf);
+  clock_t ret = INTERNAL_SYSCALL_TIMES(err, buf);
   if (INTERNAL_SYSCALL_ERROR_P (ret, err)
       && __builtin_expect (INTERNAL_SYSCALL_ERRNO (ret, err) == EFAULT, 0)
       && buf)
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..c32324f
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/times.c
@@ -0,0 +1,31 @@ 
+/* 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
+   <http://www.gnu.org/licenses/>.  */
+
+/* Incline Linux times system calls.  */
+# 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; })
+
+#include <sysdeps/unix/sysv/linux/times.c>