Message ID | CAMe9rOoERUBDk7zq-dqiP1wSemAEcvkYaT=cnPm=5_3F5Ca=8A@mail.gmail.com |
---|---|
State | New |
Headers | show |
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
diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h index fc132f6..c364d08 100644 --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h +++ 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...) \