On Mon, Apr 17, 2023 at 9:00 AM Joe Simmons-Talbott wrote: > > On Mon, Apr 17, 2023 at 08:54:17AM -0700, H.J. Lu wrote: > > On Mon, Apr 17, 2023 at 8:35 AM Joe Simmons-Talbott via Libc-alpha > > wrote: > > > > > > To make identifying syscalls easier during call tree analysis load the > > > syscall number just before performing the syscall. > > > > > > Compiler optimizations can place quite a few instructions between the > > > setting of the syscall number and the syscall instruction. During call > > > tree analysis the number of instructions between the two can lead to > > > more difficulty for both tools and humans in properly identifying the > > > syscall number. Having the syscall number set in the prior instruction > > > to the syscall instruction makes this task easier and less error prone. > > > Being able to reliably identify syscalls made by a given API will make > > > it easier to understand and verify the safety and security of glibc. > > > --- > > > sysdeps/unix/sysv/linux/x86_64/sysdep.h | 33 +++++++++++++++++++++++++ > > > 1 file changed, 33 insertions(+) > > > > > > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > > index cfb51be8c5..800a56723f 100644 > > > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > > @@ -250,12 +250,20 @@ > > > (long int) resultvar; \ > > > }) > > > > > > +#define MSTR_HELPER(x) #x > > > +#define MSTR(x) MSTR_HELPER(x) > > > + > > > #undef internal_syscall1 > > > #define internal_syscall1(number, arg1) \ > > > ({ \ > > > unsigned long int resultvar; \ > > > TYPEFY (arg1, __arg1) = ARGIFY (arg1); \ > > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > > + if (__builtin_constant_p(number)) \ > > > + asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \ > > > + : /* no outputs */ \ > > > + : "i" (number) \ > > > + : "eax"); \ > > > asm volatile ( \ > > > "syscall\n\t" \ > > > : "=a" (resultvar) \ > > > @@ -272,6 +280,11 @@ > > > TYPEFY (arg1, __arg1) = ARGIFY (arg1); \ > > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > > + if (__builtin_constant_p(number)) \ > > > + asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \ > > > + : /* no outputs */ \ > > > + : "i" (number) \ > > > + : "eax"); \ > > > asm volatile ( \ > > > "syscall\n\t" \ > > > : "=a" (resultvar) \ > > > @@ -290,6 +303,11 @@ > > > register TYPEFY (arg3, _a3) asm ("rdx") = __arg3; \ > > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > > + if (__builtin_constant_p(number)) \ > > > + asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \ > > > + : /* no outputs */ \ > > > + : "i" (number) \ > > > + : "eax"); \ > > > asm volatile ( \ > > > "syscall\n\t" \ > > > : "=a" (resultvar) \ > > > @@ -310,6 +328,11 @@ > > > register TYPEFY (arg3, _a3) asm ("rdx") = __arg3; \ > > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > > + if (__builtin_constant_p(number)) \ > > > + asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \ > > > + : /* no outputs */ \ > > > + : "i" (number) \ > > > + : "eax"); \ > > > asm volatile ( \ > > > "syscall\n\t" \ > > > : "=a" (resultvar) \ > > > @@ -332,6 +355,11 @@ > > > register TYPEFY (arg3, _a3) asm ("rdx") = __arg3; \ > > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > > + if (__builtin_constant_p(number)) \ > > > + asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \ > > > + : /* no outputs */ \ > > > + : "i" (number) \ > > > + : "eax"); \ > > > asm volatile ( \ > > > "syscall\n\t" \ > > > : "=a" (resultvar) \ > > > @@ -357,6 +385,11 @@ > > > register TYPEFY (arg3, _a3) asm ("rdx") = __arg3; \ > > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > > + if (__builtin_constant_p(number)) \ > > > + asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \ > > > + : /* no outputs */ \ > > > + : "i" (number) \ > > > + : "eax"); \ > > > asm volatile ( \ > > > "syscall\n\t" \ > > > : "=a" (resultvar) \ > > > -- > > > 2.39.2 > > > > > > > Won't the compiler load EAX twice when number is a constant? > > > > Yes. I'll have a new version combining the two asm sections into one > like for aarch64 soon. > > Joe > There is no need for __builtin_constant_p. Please try this. -- H.J.