[BZ,#19363] Use INTERNAL_SYSCALL_TIMES for Linux times
Commit Message
On Mon, Dec 14, 2015 at 7:59 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> 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 #
I will fix them.
>> + ({ \
>> + 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
I don't feel strongly about it. Glibc folks work on x86-64 system calls
know what they are doing. But I don't mind the patch like this:
Comments
On 14 Dec 2015 20:33, H.J. Lu wrote:
> On Mon, Dec 14, 2015 at 7:59 PM, Mike Frysinger wrote:
> > On 14 Dec 2015 19:27, H.J. Lu wrote:
> >> + ({ \
> >> + 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
>
> I don't feel strongly about it. Glibc folks work on x86-64 system calls
> know what they are doing.
that sort of thinking is what leads to desync in code paths (it's not
obvious at all that updates to the common sysdep.h needs to also be
deployed to this specific file), plus gcc changes behavior over time
and refines asm constraints. i'm sure you can find plenty of these
examples in the diff arches as i recall them going by in the past.
not that i'm strongly saying "make the define", just taking umbrage
to your statement here.
-mike
On Tue, Dec 15, 2015 at 6:58 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On 14 Dec 2015 20:33, H.J. Lu wrote:
>> On Mon, Dec 14, 2015 at 7:59 PM, Mike Frysinger wrote:
>> > On 14 Dec 2015 19:27, H.J. Lu wrote:
>> >> + ({ \
>> >> + 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
>>
>> I don't feel strongly about it. Glibc folks work on x86-64 system calls
>> know what they are doing.
>
> that sort of thinking is what leads to desync in code paths (it's not
> obvious at all that updates to the common sysdep.h needs to also be
> deployed to this specific file), plus gcc changes behavior over time
> and refines asm constraints. i'm sure you can find plenty of these
> examples in the diff arches as i recall them going by in the past.
>
> not that i'm strongly saying "make the define", just taking umbrage
> to your statement here.
It belongs a separate patch:
https://sourceware.org/git/?p=glibc.git;a=patch;h=d4f465df65a5723ede4cf933afee5582312fc603
I can submit it if we agree it is necessary.
On 15 Dec 2015 07:50, H.J. Lu wrote:
> On Tue, Dec 15, 2015 at 6:58 AM, Mike Frysinger wrote:
> > On 14 Dec 2015 20:33, H.J. Lu wrote:
> >> On Mon, Dec 14, 2015 at 7:59 PM, Mike Frysinger wrote:
> >> > On 14 Dec 2015 19:27, H.J. Lu wrote:
> >> >> + ({ \
> >> >> + 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
> >>
> >> I don't feel strongly about it. Glibc folks work on x86-64 system calls
> >> know what they are doing.
> >
> > that sort of thinking is what leads to desync in code paths (it's not
> > obvious at all that updates to the common sysdep.h needs to also be
> > deployed to this specific file), plus gcc changes behavior over time
> > and refines asm constraints. i'm sure you can find plenty of these
> > examples in the diff arches as i recall them going by in the past.
> >
> > not that i'm strongly saying "make the define", just taking umbrage
> > to your statement here.
>
> It belongs a separate patch:
>
> https://sourceware.org/git/?p=glibc.git;a=patch;h=d4f465df65a5723ede4cf933afee5582312fc603
>
> I can submit it if we agree it is necessary.
i like it, but i leave it to your discretion ;)
-mike
b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
@@ -218,6 +218,9 @@
# undef INTERNAL_SYSCALL_DECL
# define INTERNAL_SYSCALL_DECL(err) do { } while (0)
+/* Registers clobbered by syscall. */
+# define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx"
+
# define INTERNAL_SYSCALL_NCS(name, err, nr, args...) \
({ \
unsigned long int resultvar; \
@@ -226,7 +229,7 @@
asm volatile ( \
"syscall\n\t" \
: "=a" (resultvar) \
- : "0" (name) ASM_ARGS_##nr : "memory", "cc", "r11", "cx"); \
+ : "0" (name) ASM_ARGS_##nr : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \
(long int) resultvar; })
# undef INTERNAL_SYSCALL
# define INTERNAL_SYSCALL(name, err, nr, args...) \
@@ -240,7 +243,7 @@
asm volatile ( \
"syscall\n\t" \
: "=a" (resultvar) \
- : "0" (name) ASM_ARGS_##nr : "memory", "cc", "r11", "cx"); \
+ : "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...) \